Skip to content

fix(mcp): the tool bridge forwards AIToolDefinition.parameters as the tool's input schema, and the docblock stops describing a workaround that was never implemented - #13317

Merged
os-trump merged 5 commits into
mainfrom
claude/issue-13271-mcp-bridge-input-schema
Aug 30, 2026
Merged

fix(mcp): the tool bridge forwards AIToolDefinition.parameters as the tool's input schema, and the docblock stops describing a workaround that was never implemented#13317
os-trump merged 5 commits into
mainfrom
claude/issue-13271-mcp-bridge-input-schema

Conversation

@os-trump

Copy link
Copy Markdown
Collaborator

Fixes#13271

Route: PLUMB — the drop was not deliberate

Intent was established before any code moved, as the card requires. Five readings, all against the tree, none from memory:

  1. Two independent prose sites claim the schema IS forwarded.bridgeTools' docblock: "Each registered tool becomes an MCP tool with the same name, description, and JSON Schema parameters." And the comment on the call: "pass the JSON Schema as annotations metadata." A deliberate limitation does not get documented twice as the opposite.
  2. The workaround the comment describes was never available.annotations is typed ToolAnnotations — a closed five-key Zod object (title / readOnlyHint / destructiveHint / idempotentHint / openWorldHint), no passthrough — so an object literal carrying a JSON Schema would not have type-checked, and the client's own parse would strip it. The comment also names .tool(), which the code does not call. It describes an implementation that never existed.
  3. A sibling bridge in the same package does it the other way.wireBridgeTools in packages/mcp/src/mcp-http-tools.ts — the object/action tools, registered on BOTH transports — passes inputSchema for every tool it registers (list_objects, describe_object, validate_expression, query_records, aggregate_records, get_record, create_record, update_record, delete_record, list_actions, run_action). The package's own convention is to declare the input shape; this one call site is the outlier.
  4. The author's coping mechanism does not work either. The handler read extra.arguments, "the property the MCP SDK passes tool arguments via when registerTool is called without an inputSchema". RequestHandlerExtra has no arguments member — not in the type, and not in fullExtra as Protocol._onrequest builds it. A deliberate trade-off is not built on a member that does not exist.
  5. git history could not contribute. This container's checkout is shallow (457 commits, boundary af58a6fb), and the docblock predates the boundary, so git log -L attributes the whole hunk to the graft. Recorded as NOT MEASURED rather than folded into the four readings above.

⇒ accidental. The card's route (1) is therefore not the answer, and route (2) applies.

SDK question, measured against the installed version — not recalled

@modelcontextprotocol/sdk1.30.0 (pinned in pnpm-lock.yaml, resolved on disk at packages/mcp/node_modules/@modelcontextprotocol/sdk).

  • registerTool's inputSchema is typed ZodRawShapeCompat | AnySchema, and AnySchema = z3.ZodTypeAny | z4.$ZodType. A raw JSON Schema is not accepted: it reaches getZodSchemaObject() and throws inputSchema must be a Zod schema or raw shape, received an unrecognized object.
  • ⇒ a JSON-Schema-to-Zod conversion at the boundary is mandatory. zod@4.4.3 is already a direct dependency of packages/mcp and ships fromJSONSchema, so no new dependency. The SDK converts the result straight back to JSON Schema for tools/list via toJsonSchemaCompat; properties, types, descriptions, required, enums, nested objects and anyOf/oneOf all survive the round trip (measured), which adds $schema and additionalProperties: {}.

FROM to TO — this changes what a published integration surface emits

⚠️ Not a pure internal bug fix. Measured over a real StdioServerTransport at 74049254, for a bridged query_records declaring objectName: string (required) and limit: number:

FROM (before)TO (after)
tools/listinputSchema{"type":"object","properties":{}} — the SDK's EMPTY_OBJECT_JSON_SCHEMA, i.e. a positive claim that the tool takes no argumentsthe tool's own declared schema, round-tripped through Zod
tools/call reaching toolRegistry.executeinput: {}always, whatever the client sentthe client's arguments
non-conforming argumentsexecuted anyway, with {}isError result naming the offending field

The card's phrasing was "no input schema at all"; on the wire it is more precise to say the SDK synthesises an empty one, which is a stronger misstatement than silence — a well-behaved client reads it as "this tool takes nothing".

The second row is not a widening, it is the same fix.McpServer.executeToolHandler() branches on tool.inputSchema: a schema-less tool is invoked as handler(extra), and there is no arguments on that extra. Declaring the schema is what makes the SDK hand the call's arguments to the handler at all. The two halves cannot be separated, so the handler signature moves from (extra) to (args) in the same change.

The one fence this approached

⚠️registerTool with an inputSchema also turns on McpServer.validateToolInput(), and this SDK offers no advertise-without-validate mode. So schema validation on the call path arrives as an inseparable consequence, not as a second change folded in — no validation of my own was added anywhere, and toolRegistry.execute's contract is untouched. Flagged rather than silent, per the dispatch.

The premise the fence was written against turned out to be false in a way that shrinks its blast radius: it reads "today the handler reads extra.arguments unvalidated", but extra.arguments is undefined in every SDK version this package has depended on, so no bridged tool has ever received arguments. There is therefore no working call that validation can break — a call that previously could not have succeeded now either succeeds or returns a structured error. A tool that genuinely declares no arguments is unaffected (control below).

Docblock — before and after

The card's acceptance criterion is that the docblock and the code stop disagreeing. Both prose sites moved.

bridgeTools, before — "Each registered tool becomes an MCP tool with the same name, description, and JSON Schema parameters." True only of a bridge that forwards them.
After — names toolInputSchema and says the JSON Schema is converted into the Zod schema the SDK requires.

registerToolFromDefinition, before — "Since our tools use JSON Schema, we use the low-level .tool() with a raw callback and pass the JSON Schema as annotations metadata." Wrong on all three counts: not .tool(), nothing passed, annotations could not have carried it.
After — deleted, and replaced by prose that states what the call does and why declaring inputSchema is what makes arguments arrive. The measured SDK facts (the getZodSchemaObject throw, the EMPTY_OBJECT_JSON_SCHEMA synthesis, the missing extra.arguments) now live in toolInputSchema's docblock, next to the code that depends on them.

Verification

New pins in packages/mcp/src/mcp-tool-bridge-input-schema.test.ts speak newline-delimited JSON-RPC down a real StdioServerTransport — the wire a desktop MCP host uses. The suite that was green through the whole defect asserted the bridge's log line, which stays true of a bridge that forwards nothing.

Reverse verification, direction predicted before running: the two schema/argument pins and the consequence pin go red without the fix; the two controls stay green, so a red is a statement about the bridge and not about the harness.

  • Test-first, on the unmodified source (commit 601198c9, implementation absent): Tests 3 failed | 2 passed (5)expected undefined to match object { type: 'string', … }, expected {} to deeply equal { objectName: 'task', limit: 5 }.
  • After the fix: Tests 5 passed (5).
  • Ablation at the final head, mutation and restore both proven on disk by blob hash, never by an exit code:
    • MUTATION PROVEN: on-disk blob=e9cb5269… == pre-fix blob (and in the mutated file: inputSchema: toolInputSchema 0 hits, rawExtra.arguments 1 hit)
    • ablated run: Tests 3 failed | 2 passed (5) — the same three red, the same two controls green
    • RESTORE PROVEN: on-disk blob=0c6b4d2c… == HEAD blob; git diff HEAD empty
    • No build leg, and none skipped: the subject is imported by a same-package RELATIVE specifier (./mcp-server-runtime.js), which vitest resolves to src/, never through exports to dist/. The positive control is the ablation itself — the mutation flipped the result with no build in between, which a dist-resolved subject could not have done.

Controls that stay green in both directions:name and description reach the client (the harness works), and a tool that declares no parameters still registers and still executes (inputSchema.properties deep-equals {}, input deep-equals {} — the no-argument path is byte-identical before and after).

Gates, all at a876ebe6

pnpm --filter @objectstack/mcp testTest Files 24 passed (24) · Tests 258 passed (258)
pnpm --filter @objectstack/mcp typecheck — exit 0 (--listFiles confirms mcp-server-runtime.ts is in the program)
pnpm lint (eslint . --no-inline-config, whole repo, no narrowing) — exit 0
pnpm check:type-check-debtcheck-type-check-coverage --re-measure: OK — 30 ledger entr(ies) re-measured in 292.0s, 1560 raw tsc error(s) total, none above its recorded number.
pnpm check:dual-build-cjs-loads102 published require entry point(s) across 66 package(s) load; 610 emitted CommonJS file(s) parse; 1 cross-format behaviour probe(s) agree.
pnpm check:nul-bytesOK (scanned 7363 text file(s) … no raw ASCII control bytes)
Plus the other 25 families node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack derives from the real change set: all exit 0.

⚠️Two are NOT MEASURED, recorded with their own refusal text rather than folded into the green list.node scripts/check-test-completeness.mjs exit 3"PREREQUISITE NOT MET — this gate grades a saved turbo run test log, and no log was named … the local reading for this gate is NOT MEASURED."node scripts/pm/check-half-states.mjs exit 3"PREREQUISITE NOT MET — the token in the environment is not a valid GitHub credential."

One gate went genuinely red and was repaired, not baselined.check:type-check-debt first reported @objectstack/mcp: TEST_DEBT records 53 raw tsc error(s), tsc --noEmit now reports 54 (+1). The +1 was mine — TS6133: 'session' is declared but its value is never read in the new pin file. Fixed at the source by moving transport teardown into afterEach (which also closes the transport when an assertion fails). Re-measured with the exclusion lifted: 53, class-for-class matching the ledger note (TS18046 x51, TS6133 x1, TS2352 x1), with the new file contributing 0. The ledger entry is untouched.

⚠️packages/mcp/tsconfig.json excludes **/*.test.ts, so pnpm typecheck says nothing about the new test file — which is exactly why the TEST_DEBT ratchet above is the measurement that covers it.

Changeset

.changeset/mcp-bridge-forwards-tool-input-schema.md, @objectstack/mcp: minor — this changes what MCP clients receive from a published integration surface, so it is user-visible.

Scope

packages/spec/liveness/tool.json is untouched — tool.parameters stays live, and this card is evidence of the opposite of dead: the cloud LLM path (service-ai/src/adapters/vercel-adapter.ts, buildVercelOptions) reads the same key, unevenly consumed. Neither serially-locked file (packages/rest/src/rest-server.ts, packages/cli/src/commands/migrate/plan.ts) is on this card's surface or in this diff.


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

3 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 12 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 289cf91c84a7e0cb93e13217070b2c864285c737packageMentionDocs.

Which tree this was computed on

This run read content/docs from 7292896d10f1c3635a000528fe20bf312ceee5a2 — the merge of head a876ebe6fb3f9d0fee59e96cf55955b569470ee1 into base 289cf91c84a7e0cb93e13217070b2c864285c737, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 7292896d10f1c3635a000528fe20bf312ceee5a2 && git checkout 7292896d10f1c3635a000528fe20bf312ceee5a2
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 289cf91c84a7e0cb93e13217070b2c864285c737 a876ebe6fb3f9d0fee59e96cf55955b569470ee1 && git checkout -B drift-repro 289cf91c84a7e0cb93e13217070b2c864285c737 && git merge --no-ff a876ebe6fb3f9d0fee59e96cf55955b569470ee1
node scripts/docs-audit/affected-docs.mjs --json 289cf91c84a7e0cb93e13217070b2c864285c737

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 30, 2026
@os-trumpClaude

Copy link
Copy Markdown
CollaboratorAuthor

✅ PM review — ACCEPT once CI is green; ⛔ not while it is running (#13271)

CI started 03:23Z and is still running. Undrafting now so draft: true is not a second bar; arm follows on a complete green read. ⛔ No rework owed — do not push in response to this comment.

Clause ②: does not attach. Path limb measured silent (change set is 3 files, none under packages/spec/src/**), and the body carries no declaration. MCP's tools/list is a protocol surface, not a public REST door, so the door test does not engage. Recording the reasoning because this lane has two PRs (#13240, #13241) barred on exactly this gate and the difference should be legible.

⚠️ My fence was written on a false premise — that is my error, and it was handled correctly

The dispatch said: "⛔ Do not widen into handler-side validation. Today the handler reads extra.arguments unvalidated."

Measured: extra.arguments does not exist — not on RequestHandlerExtra, and not in fullExtra as Protocol._onrequest builds it — in any SDK version this package has depended on. So no bridged tool has ever received arguments, and the "unvalidated calls happening today" my fence was protecting do not exist. There is no working call that validation can break.

And validation could not have been declined anyway: registerTool with an inputSchema turns on McpServer.validateToolInput(), and SDK 1.30.0 offers no advertise-without-validate mode.

⭐ The handling is exactly right for a fence whose premise fails: it did not silently violate it, and it did not blindly obey a rule that had become nonsense. It measured the premise, added no validation of its own, left toolRegistry.execute's contract untouched, flagged it prominently in the body, and pinned it as a test labelled CONSEQUENCE rather than folding it in as if it were intended. That is the behaviour I want when a dispatch is wrong.

⚠️ The defect was worse than the card said — and the correction is load-bearing

The card said bridged tools reach clients with "no input schema at all". On the wire the SDK synthesises{"type":"object","properties":{}} (EMPTY_OBJECT_JSON_SCHEMA) — a positive claim that the tool takes no arguments, which a well-behaved client acts on. Silence would have been better. And tools/call reached toolRegistry.execute with input: {}always, whatever the client sent.

⇒ both halves are one fix, not a fix plus a widening: executeToolHandler() branches on tool.inputSchema and invokes a schema-less tool as handler(extra), so declaring the schema is what makes the SDK hand arguments to the handler at all. The (extra)(args) signature change is a consequence, not a choice.

What makes this trustworthy

  • Intent established before plumbing, as ordered — four readings against the tree (two prose sites claiming forwarding; annotations typed as a closed five-key object that could not have carried a schema, and the comment naming .tool() which the code never calls; the sibling wireBridgeTools in the same package passing inputSchema for all 11 of its tools; the author's own extra.arguments coping mechanism referencing a member that does not exist) — plus git history recorded NOT MEASURED rather than fudged, because the container's checkout is shallow (457 commits) and the docblock predates the graft boundary.
  • The old suite was green through the entire defect because it asserted the bridge's log line — which stays true of a bridge that forwards nothing. The new pins speak newline-delimited JSON-RPC down a real StdioServerTransport, i.e. the wire a desktop host actually uses. That is the right instrument upgrade, not more assertions on the same blind one.
  • A gate went genuinely red and was repaired at source, not baselined.check:type-check-debt reported +1 (TS6133, an unused session in the new pin file). Fixed by moving teardown into afterEach — which also closes the transport when an assertion fails, so it is strictly better than the code it replaced. Re-measured at 53, class-for-class against the ledger note, new file contributing 0, ledger entry untouched.
  • Test-first, then ablation: 3 failed / 2 passed against unmodified source; 5 passed after; then the ablation reproduced exactly the same 3-red / 2-green split, with mutation and restore proven by blob hash and git diff HEAD empty. The no-build claim carries its own positive control — the mutation flipped the result with no build between, which a dist-resolved subject could not do.
  • Unconvertible parameters is logged and degraded, deliberately not thrown, because this runs inside bridgeTools and one bad definition would otherwise take the server's entire tool surface down. Correct call, and stated as a decision rather than left implicit.

⭐ The most valuable thing here is a trap it closed for someone else

outputSchema is also never forwarded at this call site, and SDK 1.30.0's registerTooldoes accept it — so the obvious next move is to "finish the job". Measured: that would break every bridged tool, because validateToolOutput() throws "Tool X has an output schema but no structured content was provided" whenever an outputSchema is declared and the result carries none — and this bridge returns text content only. Recorded so nobody reads this PR as a template. ⛔ Do not plumb outputSchema the same way.

Owed by me, not by the dev

⚠️packages/spec/liveness/tool.json's parameters note goes stale the moment this lands. It asserts that registerToolFromDefinition registers each bridged tool with noinputSchema "so this key never reaches an MCP client", and cites #13271. This PR makes that false.

The dev correctly did not touch it — fence 3 forbade it, the file belongs to the #13042 close-out, and it pulls a different gate family. ⭐ Right restraint. I own the follow-up and have carried it into this seat's check-in so it does not evaporate.

Follow-up filed and triaged:#13318 (bug · p1 · security · pm:queue · domain:cli) — the same call site drops requiresConfirmation and derives destructiveHint / readOnlyHint from two hardcoded name sets, so every action-backed tool is served destructiveHint: false, the inverse of the MCP spec's conservative default of true. Graded p1 because it misinforms a host's confirmation gate in the unsafe direction; ⛔ security there is a topic marker, not a verdict — no boundary is crossed by the framework.


Generated by Claude Code

@os-trump
os-trump marked this pull request as ready for review August 30, 2026 03:32
@os-trump
os-trump added this pull request to the merge queueAug 30, 2026
Merged via the queue into main with commit e29fc21Aug 30, 2026
34 checks passed
@os-trump
os-trump deleted the claude/issue-13271-mcp-bridge-input-schema branch August 30, 2026 04:42
os-project-manager pushed a commit that referenced this pull request Aug 30, 2026
The `_note` on ToolSchema's `parameters` entry asserted that
registerToolFromDefinition registered every bridged tool with NO
inputSchema -- "so this key never reaches an MCP client" -- and cited
that as the asymmetry with `name` / `description` (filed as #13271).
PR #13317 (e29fc21, merged 2026-08-30T04:42:09Z) fixed exactly that:
mcp-server-runtime.ts#toolInputSchema now converts `parameters` through
zod@4's fromJSONSchema and registerToolFromDefinition forwards the
result as the SDK inputSchema, which the SDK converts straight back to
JSON Schema for tools/list -- the key reaches MCP clients too.
The note is corrected to record the fix while keeping the sharper
nuance the original note called out: the pre-fix behaviour was not
"no schema" but the SDK synthesising EMPTY_OBJECT_JSON_SCHEMA
(`{"type":"object","properties":{}}`) for a schema-less registration --
a positive claim that the tool takes no arguments, not silence.
The grade does not move: `parameters` was live before this change and
stays live -- the cloud LLM path (vercel-adapter.ts#buildVercelOptions)
has read it all along, and that is what the verdict has always rested
on. This closes an asymmetry between two consumers, not a change in
liveness status.
Fixes#13345
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KX8wnyjStaZcuMyAMNsy3N
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The MCP tool bridge never forwards parameters — bridged tools reach MCP clients with no input schema, and the docblock says it does forward one

2 participants

@os-trump@claude