Repository navigation
feat(mcp): add optional fields projection to five tool outputs - #40
Merged
Merged
Conversation
- Add CatalogEntry and pure build/render in core catalog (Functional Core) - Add summary field (skipped) to Tool - Extract registered_tools, slim handle_tools_list, add tool_catalog - Add Commands::Catalog and arm in run - Add 61 summaries - Add 8 required tests in core and shell - Update docs in CLAUDE.md and mcp-server.md All per slice spec. Demo and lint verified.
QA claimed catalog_tests passed via cargo xtask lint; they were never written. Add the four required tests in the mcptools tools module.
- core rank_tools with idf + stemming + issue key rewrite - shell wrapper find_tools joins real inputSchema - CLI subcommand with -k and JSON output - golden fixture split by kind - required unit and integration tests
- add FindToolsArgs and handle_find_tools in mcp/tools/find_tools.rs - register Tool in registered_tools as last entry - add match arm in handle_tools_call - filter find_tools out of tool_catalog - update catalog_names test - update contract counts to 62 and pilot list and sample for find_tools
Tool carries a required kind (Read, Write, Spend), skipped on the wire. annotations_for maps kind to readOnlyHint/destructiveHint and handle_tools_list injects the annotations object per entry. Pilot kinds: jira_search Read, jira_create Write, images_generate Spend; remaining 59 entries carry an explicit Read placeholder for V2 to correct per the full oracle. Registry count locked at 62.
Flip 15 Write and 2 Spend placeholders to oracle kinds; rest stay Read. Adds oracle table test over live handle_tools_list output.
New catalog test asserts registered (name, kind) pairs equal the reviewed oracle sets: 16 Write + 3 Spend + 43 Read. An unreviewed 63rd tool fails the gate until its name enters the oracle with a reviewed kind. No runtime change.
…_all; build per-tool, collect {tool}: {path}: {msg}
- Add Deserialize to Annotation*Output, SavedOutput, Found* - Declare conformance_tests under cfg(test) - Implement sample, roundtrip, roundtrip_by_tool (62 arms), strip_optional with visited for cycles - N9 samples_roundtrip_through_output_types - N11 sparse_outputs_validate using jsonschema draft202012 - N5: output_schema_for strips only envelope, no normalize_prop (input keeps) - Updated 40+ samples to include explicit null for Option fields so roundtrips match - Verified N13: no output normalize asserted in contract_* or schema unit tests - Demo: pdf_info title now ["string","null"]; all 62 sparse pass
…form_comment in ticket transform; update render_adf to handle bare string and mention nodes; simplify CLI comment print to use rendered body; change FoundTool.input_schema to Map; implement loose_nodes and output_schemas_have_no_any_nodes test; update jira samples to new comment shape
…contract samples for zero unreached paths - add pure unreached and unreached_collect following strip/loose walker conventions with visited guard for recursion - update 41 samples to populate optional fields with non-nulls and ensure arrays have elements - N9 roundtrip, N11 sparse, wire validation, and coverage now all pass
…est, parse_ranking, select
…hemas; add six stub tests proving each reason and secret never leaks; update 529 test
…ackend/fallback and advisory note in mcp-server.md, add Jev vars and live summary in testing.md
Hosts call tools/list once and load every schema into the agent's context, so find_tools saved nothing while the server listed all 62 tools. With --discovery or MCPTOOLS_DISCOVERY=true, tools/list returns only find_tools. tools/call still dispatches every tool by name, and find_tools still ranks and attaches schemas from the full registry. The default list is unchanged. The gate is a pure function, listed_tools, in the shell crate because Tool is a shell-crate type. The flag sits on Global beside --verbose, so stdio and SSE both honor it through the shared handle_request. The find_tools description gains one sentence that tells the agent the returned tools are callable by name even when unlisted. Test spawners remove MCPTOOLS_DISCOVERY from the child environment so an exported value cannot break the full-list assertions. The discovery contract test also removes the Jev variables and asserts the local backend, which keeps it offline. Deliberate simplifications: - bool flag, not an enum; no third list mode exists. - The flag is global, so it appears in every subcommand's help and is inert outside mcp. - The env var accepts only true or false; any other value is a clap parse error. - Host reachability is out of scope. Hosts that declare only listed tools to the model, such as Claude Code, cannot yet call the found tool. A planned execute command owns that gap. - No SSE-specific test; SSE shares handle_request with stdio. - The contract test proves dispatch with jira_query_list, which runs offline. linear_issue_get needs credentials and was not run live. Refs: GUZ-162
Pure function over tool name, description, and input/output JSON schemas. Emits input interface, prefixed output then input $defs, output interface, function JSDoc, and declare function line. Type arrays become unions, enums become string-literal unions, $refs gain the tool Pascal prefix, trailing (default: X) description suffixes become @default.
declaration maps one Tool through the core emitter; declarations joins selected blocks with one blank line and rejects unknown names. New Commands::Declarations variant with run arm printing the joined text.
…ole catalog ts_type_at now renders anyOf as a member union and prefixItems as a tuple, checked after $ref and enum and before type. pdf_toc page_range becomes [number, number]; pdf_peek id, images usage and find_tools fallback become nullable unions. Live registry check: page_range carries prefixItems with no sibling items key, so tuple preference over items never triggers in the catalog. The only literal-true schema in all 62 tools is an additionalProperties flag, and jira_get/jira_create render zero unknown, so the sweep pins the unknown set to find_tools alone with the raw schema evidence in the handoff.
Add 7 required rows to golden-queries.tsv and a required_rows_each_pass test: rank-1 in expected set and all expected in top 5, NONE row must score higher none than every required tool row. Add synonym step to tokenize() in core find_tools: mark->update, done|resolv|resolve|finish->close, ticket->issue, identity otherwise. Mid-slice the new test failed with only [mark the ticket done]; after the synonym step cargo xtask lint is green and mark the ticket done ranks linear_issue_update 0.624 first, jira_update 0.552 second.
testing.md gains a live-result table under the Jev summary: 7 queries against the opencode preset with backend, none score, tools, and per-call latency. mcp-server.md gains an Agent loop section before Discovery mode with the 6-step loop and a link to the known limitation. CLAUDE.md gains a short Agent loop paragraph linking to the full section.
FoundTool carries the same declaration text that mcptools mcp declarations prints, rendered through the shared declaration function so the two can never diverge. FoundTools carries a root usage line from a new core USAGE constant. The per-tool inputSchema field is gone from the result in every mode (CLI, MCP, local and Jev). The tools/list description, the discovery contract assertion, the declaration sweep, and the recorded contract sample follow the new shape.
The result row lists usage and per-tool declaration, the loop step reads the declaration instead of the schema, and the discovery-gap paragraph states that returned declarations name the tools/call arguments and results.
Discovery tools/list carries one object: find_tools. Serialize it compact and fail the test with the measured length when it exceeds the 1300-char ceiling.
Workspace gains rquickjs 0.14 with the futures feature only, used by the
mcptools binary crate. Core gains pure sandbox types: Limits (30s, 64 MiB,
256 KiB defaults), a byte-capped LogBuffer, Output, and the SandboxError
enum (Timeout, OutputLimit, Js).
The runner builds a fresh AsyncRuntime per call inside spawn_blocking plus
Handle::current().block_on, applies set_memory_limit, binds a __log host
function, and evals a prelude that maps console.{log,info,warn,error,debug}
into it. User code runs under promise-mode eval; the completion value is
unwrapped from the {value} wrapper, awaited when it is itself a promise so
rejections like failed module loads surface, then JSON-stringified into
serde_json::Value (undefined and functions become null). JS exceptions are
read from ctx.catch() as name/message; other rquickjs errors and join
errors map to Js {name: Error}.
…utput stop cell Wire SandboxError::Timeout and SandboxError::OutputLimit into run: - interrupt handler checks a shared stop cell and a wall-clock deadline; on deadline it records StopReason::Timeout and raises the uncatchable interrupt exception, so try/catch and regex backtracking cannot evade it - __log sets StopReason::OutputLimit and throws a JS RangeError when LogBuffer::push rejects; the stop cell, not the exception, is the enforcement (exceptions are catchable cosmetics) - eval drive wrapped in tokio::time::timeout for the never-resolving promise case, where no bytecode runs and the interrupt never fires - mapping order in run_inner: stop cell, then elapsed, then the JS exception path, then Output::check_size on the built output - Output::check_size (core, pure): sum of log bytes plus serialized result length must stay within the cap; From<OutputLimit> for SandboxError lets the run_inner ? carry it - eval's with-closure return type annotated now that From<OutputLimit> makes the error conversion ambiguous
Thread Bindings (names + Global) through run/run_inner/eval and install one async function per bound tool name after the PRELUDE eval. Dispatch goes through handle_tools_call; plain Error exceptions carry code and data. mcp::tools visibility widened to pub(crate) so sandbox::bindings can reach it.
Top-level return scripts parse-fail as QuickJS eval scripts ('return not in a function'); eval retries once wrapped in an uninvoked-async-arrow body, keyed on that exact parse message so runtime-thrown SyntaxErrors never re-run side effects. Completion value then rides the existing {value} wrapper and into_promise path.
Deviations recorded: throw_rpc returns rquickjs::Result so ? works; pub mod bindings; closure clones name/global in the sync body; 18 run call sites, not 19.
Run carries logs through every error path. execute_output maps SandboxError to ExecuteError: TimeoutError, OutputLimitError, JS names pass through.
run_inner always reaches the log drain after the runtime drops, so timeout, throw, and memory errors keep prior console lines. bound_names excludes execute from its own bindings.
execute maps sandbox runs through execute_output, sets isError when error is present, and returns logs plus result on success. Registry entry is destructive; tool_catalog drops it beside find_tools.
mcp-server.md gains the Execute section with inputs, result shape, error names, and flags. CONTEXT.md gains the Execute requirement with seven scenarios.
The roundtrip conformance test deserializes ExecuteOutput.
Registry, annotation table, and sweep accept the 63rd tool with execute-scoped allowances only. Conformance roundtrips execute and exempts its Value result node. Contract counts move to 63 and execute gains a recorded sample.
A script that references a write or spend tool that did not bind throws QuickJS ReferenceError '<name> is not defined'. handle_execute now swaps that message for one that names the tool, its kind, and the flag that binds it (allowWrites / allowSpend). error.name stays ReferenceError; every other ReferenceError, unknown global, and 'execute' itself keeps the plain message. The remap is a pure fn keyed off the same kinds vector handle_execute builds for bound_names. Docs: split the bare-ReferenceError sentence in mcp-server.md and add the gated-tool scenario to the Execute requirement in CONTEXT.md.
Harness spawns the real binary (CARGO_BIN_EXE_mcptools mcp stdio), clears ambient env that flips server behavior (MCPTOOLS_DISCOVERY, ATLASSIAN_BASE_URL, JEV/gateway keys), applies per-test overrides, and completes initialize + initialized internally. Line-delimited JSON-RPC framing follows contract_discovery.rs: one response line per request line. Smoke test asserts tools/list carries execute and find_tools, and tools_call returns structuredContent.
Adds a stdio MCP end-to-end test that runs an execute script with allowWrites: true: list sprints, search open-sprint issues, update each issue, log the count. A wiremock stub serves the three Jira REST routes from fixtures under tests/fixtures/jira/. Assertions: structuredContent logs == ["3"], result null, no issue keys in the response payload, and exactly 3 PUTs on the update route in the stub request log. The script throws on partial_failure so a 404 on the update route fails the logs assertion instead of passing silently. wiremock 0.6.5 added to mcptools dev-dependencies.
Section A (s-1-0) carries one embedded image; Section B (s-1-1) carries only text starting with the PDF_CHAIN_SECTION_B_MARKER marker and no images. Generated by hand (zlib-flattened RGB image XObject, Helvetica headings at 20pt over 11pt body text so heading detection classifies both lines as level-1 sections). Verified with the mcptools pdf CLI: toc 2 sections, read s-1-1 contains the marker, images whole/A/B = 1/1/0.
The execute script chains pdf_toc, pdf_read, and pdf_images against a tempdir copy of the two-section fixture and must log exactly ["2 true 1"]: 2 top-level sections, the section-B marker present in section-B text, 1 image in section A. A direct pdf_images tools/call scoped to section B must return 0 images, and the whole-document call must return 1, so the section filter is proven outside the script. Script result is pinned to null because it ends in console.log.
The linear chart command needs the same open/xdg-open launch. The warning on launch failure is now generic instead of naming images.
linear chart <ISSUE>... walks the transitive closure of sub-issues and blocked_by blockers (cap 300), drops canceled issues, classifies the rest as complete/in progress/frontier/fog, and writes a full-viewport dark-themed Mermaid HTML page to a temp path (or --out), opening it in the default browser unless --no-open. Same input yields byte-identical output. Behavior documented in CONTEXT.md and the Linear topic file.
ServeFlags { discovery, code_mode } replaces Global.discovery. Flags are
flattened into mcp::App with global = true, so both 'mcp --discovery stdio'
and 'mcp stdio --discovery' parse; MCPTOOLS_DISCOVERY / MCPTOOLS_CODE_MODE
env vars still apply. Flags thread by value from mcp::run through the stdio
and sse runners into handle_request, the list and call handlers, and the
sandbox Bindings, so a bound find_tools call inside execute inherits them.
Breaking: 'mcptools --discovery mcp stdio' no longer parses; the flag must
come at or after 'mcp'. Deliberate, accepted.
code_mode parses but nothing consumes it yet; the tools/list gate still
branches only on discovery.
Both flags use BoolishValueParser: clap's default bool parser rejects
'MCPTOOLS_DISCOVERY=1', only 'true'/'false', and the acceptance demo uses =1.
Add CODE_MODE_USAGE const in core. handle_find_tools overwrites result.usage from flags.code_mode after rank resolve: code mode gets the execute-global text, plain mode keeps the declaration text. Shell find_tools() keeps USAGE unconditionally (no flags). Nested find_tools inside execute inherits flags through Bindings, so the text stays coherent. Tests cover usage-per-mode and nested-in-execute.
listed_tools now takes ServeFlags and branches code_mode first, then discovery, then identity. handle_tools_list passes the flags whole. Adds a four-combo gate truth table (63 / find_tools / find_tools+execute x2) and a joint budget test that sums estimate_tokens over both code-mode entries against a 1000-token cap. Migrates the execute test and discovery pin callers to serve_flags; pin budget stays 1300 chars.
project() selects named paths from a JSON value: keeps ancestors, maps over arrays, widens on prefix overlap, and errors with the first path that inserts zero leaves.
projected_output_schema_for strips required recursively for projected tool outputs; to_dual_result_projected maps unknown field paths to -32602. pdf_read gains an optional fields param and its projected output schema; conformance sparse validation skips the Rust roundtrip for projected tools.
Covers projected keys, full output without fields, and the Unknown field path error envelope.
Both list tools gain an optional fields arg, delegate to to_dual_result_projected as their final step, and switch to projected output schemas. Description sentences document the param; declaration and property-name contract tests regenerated for the new optional input and the stripped required sets. Sample-based projection tests plus a wiremock e2e prove per-element array mapping and the -32602 unknown-path error.
MdFetchArgs and BitbucketPRReadArgs gain fields; both handlers end with to_dual_result_projected (bitbucket after the lineLimit truncation). Both Tool entries switch to projected_output_schema_for and gain the fields description sentence. Adds sample-based projection tests for both tools and a sweep test over the five projected tools: fields is optional array-of-string input, the description documents it, and no output schema keeps a required key.
Prefix the three pdf_read e2e test names with fields_projection so the fields_projection test filter catches all four tests in the file. Bodies unchanged.
Add md_fetch_declaration and bitbucket_pr_read_declaration to the declaration_tests module, in the same style as the existing pdf_read, jira_search, and linear_issue_list declaration tests. Expected strings generated from the real declaration() output: input keeps fields?: string[] in alphabetical order and every output property stays optional.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
GUZ-178: plain tool-calling hosts need a cheap way to shrink large tool responses before they consume model context. This is the interim win scoped by GUZ-177: an optional
fieldsparam on five read tools, no new dependencies, no query language. Supersedes GUZ-113 (canceled).Scope
crates/core/src/projection.rs— pureproject(value, paths)withProjectionError; array mapping, ancestor retention, wider-path-wins, resolve-nowhere rule; 9 unit testscrates/mcptools/src/mcp/tools/mod.rs— sharedto_dual_result_projected(value, fields): absent/empty delegates toto_dual_result(byte-identical), unknown path maps to-32602 Unknown field path: "<path>"crates/mcptools/src/mcp/tools/schema.rs—projected_output_schema_for::<T>()stripsrequiredrecursively for the five output types onlyfields: Option<Vec<String>>onPdfReadArgs,MdFetchArgs,JiraSearchArgs,BitbucketPRReadArgs,IssueListArgs; each handler extracts before arg moves and projects as the final call (bitbucket after lineLimit truncation)fieldswith a per-tool example path; five output schemas switch to the projected variantPROJECTED_TOOLSsparse validation, 5 sample/sweep tests, newtests/e2e_fields_projection.rs(3 pdf over stdio + 1 jira over wiremock); 5 declaration tests pin the TS shape —pdf_read,jira_search,linear_issue_listregenerated for the new input/output shape,md_fetchandbitbucket_pr_readaddedCONTEXT.md— new Behavior requirement "Tool output field projection" (Doc Sync)Tradeoffs
requiredstripped for these five outputs only, not all ~50 tools — broad stripping would ripple throughfind_tools/executeTS declarations-32602) instead of silently dropping — typo visibility beats quiet no-ops; absent or emptyfieldsstill returns the full outputBlast Radius
fields?: string[]); hosts see an additive change — pinned bypdf_read_declaration,jira_search_declaration,linear_issue_list_declarationrequired; pinned byfields_projection_covers_five_registered_tools(recursive no-requiredwalk) and the regenerated declaration tests; the other ~45 tools keepoutput_schema_foruntouched — pinned by the unchanged full conformance suiteproperty_names_match_previous_contractupdated forIssueListArgsgaining"fields"between"cycle"and"label"samples_roundtrip_through_output_typesstays green on full samples for all toolsfieldsbehavior unchanged:to_dual_result_projecteddelegates toto_dual_result, proven bypdf_read_without_fields_returns_full_outputVerification
At
85c602a(clean tree):cargo fmt --all --check— cleancargo clippy --all-targets --all-features -- -D warnings— 0 findingscargo test --workspace— 18 suites, 0 failedcargo test -p mcptools --test e2e_fields_projection fields_projection— 4 passed (pdf project/omit/error + jira wiremock; all four match the filter)cargo test -p mcptools fields_projection— 9 passed (linear, jira, md, bitbucket sample tests + five-tool sweep + the 4 e2e)cargo test -p mcptools_core projection— 9 passed