fix(web): refuse a raw-JSON tool argument the schema would retype

[cliffhall]

Closes #2171

The decision

A raw-JSON argument the schema would retype is refused, not sent. The two JSON-authoring surfaces now give the same answer, which is what #2171's acceptance asked for — Edit-and-replay already refused, and the Tools/Apps Edit as JSON editor was silently sending the coerced value.

The issue left the direction open and named both options. This is the second one: "detect the coercion and refuse it, the way PR #2170's Edit-and-replay modal already does for the same conversion (reshapedReplayParam) — and the two paths should then agree."

The cost, stated plainly: the Inspector cannot send a deliberately schema-invalid argument from the UI. That was the other half of the issue's repro, and it is what the alternative direction would have bought. Refusing was chosen instead because the editor showing one payload while the wire carries another is the defect being fixed, and a refusal fixes it without inventing a second way for the two editors to disagree.

What changed

callTool converts every string-valued argument to the type the tool's inputSchema declares, because the widget form hands everything over as text — "2" against a numeric field has to become 2. A JSON draft already carries its own types, so a value that conversion would touch is one whose visible text is not what the wire would carry.

• core/json/jsonUtils.ts — coercedArgumentNames(inputSchema, args) and coercedArgumentsError(names, toolName?). The check runs the real conversion and diffs, rather than restating its rules, so no surface can drift from what would actually be sent; the message is shared so the same refusal can't be worded two ways. convertToolParameters is now a thin wrapper over a schema-keyed convertParametersForSchema, since a form holds the schema and the client holds the Tool.
• SchemaForm — the raw-JSON editor refuses such a draft, names the value, and reports invalid through the existing onValidityChange channel so Execute / Open App disable. The offending object is also not emitted upward, so a submit gated on something else can't send it either.
• ToolDetailPanel / AppDetailPanel — opt in via the new enforceToolArgumentTypes.
• protocolReplay.ts — its private coercedToolArgs copy is deleted in favour of the shared helper. Replay's behavior is unchanged.

Enforcement is opt-in rather than inferred from the schema. An elicitation renders through the same SchemaForm and its values never go through tools/call, so enforcing there would refuse a draft for a reason that isn't true of it. Only the two tool-argument panels pass it.

Screenshots

An add tool declaring a and b as numbers, with {"a":"01","b":2} typed into Edit as JSON. "01" is the probe because it survives Number() as 1, so the wire tells a coerced call from a verbatim one.

Before	After
Execute Tool enabled, no notice	``a would be converted to the type the schema declares — write the value with that type instead, Execute Tool disabled

And what the "before" build did on that click — the editor said "01", the wire carried 1, and the call "succeeded" with {"result": 3} for an argument the user never wrote:

The Edit-and-replay modal is deliberately absent from these: its behavior does not change, since it is the surface the other two are being brought into line with.

Tests

• core (src/test/core/jsonUtils.test.ts) — coercedArgumentNames against a retyped string, a value already of the declared type, a string the schema declares as a string, an undeclared argument, several names at once, and a type declared on a root union branch (Tool schemas with a root-level anyOf / oneOf render no input fields #2123), where the branch must be selected from the supplied values exactly as the conversion selects it. Plus both coercedArgumentsError forms.
• SchemaForm — refuses and names the value; blocks submit and unblocks when rewritten with the declared type; accepts values already correctly typed; and does not enforce when the caller has not opted in, which is the test that pins the elicitation path.
• ToolDetailPanel / AppDetailPanel — each panel actually opts in, asserted through the disabled Execute / Open App button rather than the prop.

npm run local:gate passes. Rebased on the latest v2/main (4d69dbf9).

[Copilot]

Pull request overview

Ensures JSON-authored tool arguments reach MCP servers without schema coercion across Tools, Apps, tasks, and protocol replay.

Changes:

• Adds shared ToolCallOptions.skipArgumentCoercion.
• Propagates raw-JSON mode through web execution paths.
• Updates replay behavior, tests, and documentation.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
core/mcp/types.ts	Defines shared tool-call options.
core/mcp/inspectorClient.ts	Applies optional argument coercion consistently.
core/mcp/index.ts	Exports the new options type.
clients/web/src/test/integration/mcp/inspectorClient-raw-json-args.test.ts	Verifies raw and coerced wire payloads.
clients/web/src/lib/protocolReplay.ts	Replays tool arguments without coercion.
clients/web/src/lib/protocolReplay.test.ts	Tests uncoerced replay dispatch.
clients/web/src/components/views/InspectorView/InspectorView.tsx	Extends execution callback contracts.
clients/web/src/components/screens/ToolsScreen/ToolsScreen.tsx	Forwards raw-mode state for tool calls.
clients/web/src/components/screens/ToolsScreen/ToolsScreen.test.tsx	Tests Tools callback propagation.
clients/web/src/components/screens/ProtocolScreen/ProtocolScreen.tsx	Removes obsolete tool propagation.
clients/web/src/components/screens/AppsScreen/AppsScreen.tsx	Forwards raw-mode state for Apps.
clients/web/src/components/screens/AppsScreen/AppsScreen.test.tsx	Tests Apps callback propagation.
clients/web/src/components/groups/ToolDetailPanel/ToolDetailPanel.tsx	Captures raw mode during execution.
clients/web/src/components/groups/ToolDetailPanel/ToolDetailPanel.test.tsx	Tests execution mode reporting.
clients/web/src/components/groups/SchemaForm/SchemaForm.tsx	Reports raw-editor mode changes.
clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx	Tests mode changes and cleanup.
clients/web/src/components/groups/ProtocolListPanel/ProtocolListPanel.tsx	Removes obsolete tools prop.
clients/web/src/components/groups/ProtocolEntry/ProtocolEntry.tsx	Removes replay schema plumbing.
clients/web/src/components/groups/MrtrConversation/MrtrConversation.tsx	Removes obsolete tools forwarding.
clients/web/src/components/groups/EditReplayModal/EditReplayModal.tsx	Allows schema-invalid JSON replay arguments.
clients/web/src/components/groups/EditReplayModal/EditReplayModal.test.tsx	Tests formerly coerced argument submission.
clients/web/src/components/groups/AppDetailPanel/AppDetailPanel.tsx	Reports raw mode when opening Apps.
clients/web/src/components/groups/AppDetailPanel/AppDetailPanel.test.tsx	Tests Apps detail mode reporting.
clients/web/src/App.tsx	Maps raw mode to core call options.
clients/web/README.md	Documents JSON argument semantics.

Suppressed comments (1)

clients/web/src/App.tsx:1067

• The Apps path has the same untested final handoff: AppsScreen verifies that it emits rawJsonArgs, and core verifies an explicitly supplied option, but no test drives App.onOpenApp and checks that this call combines skipArgumentCoercion: true with skipOutputValidation: true. A regression here would again give the app uncoerced input while sending coerced input to the server without failing the suite. Add an App-level assertion for the raw Apps callback and these options.

          {
            skipOutputValidation: true,
            skipArgumentCoercion: rawJsonArgs === true,
          },

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[cliffhall]

Review round 1 — addressed

Copilot's one finding (and its suppressed twin on the Apps path): the App.tsx seam was untested. Correct, and it was the one place the whole change could silently stop working — the screens' tests stop at the rawJsonArgs callback and core's tests start from an option passed in explicitly, so nothing observed App.tsx turning one into the other.

Fixed in 406f6adc. App.test.tsx's InspectorView double now exposes call-raw, call-as-task-raw and open-app-raw (it had no onOpenApp at all before), with four assertions on the sixth argument:

Path	Asserted options
plain raw call	{ skipArgumentCoercion: true }
ordinary call	{ skipArgumentCoercion: false }
task (callToolStream)	{ skipArgumentCoercion: true }
Apps	{ skipOutputValidation: true, skipArgumentCoercion: true }

The second row is the one worth keeping honest: the widget form still hands its values over as text, so this is not a one-way switch and coercion has to stay on for an ordinary call.

npm run local:gate passes.

[Copilot]

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.

[cliffhall]

Review round 2 — addressed

One finding: the new test helper used as unknown as, which AGENTS.md prohibits without a justification. Correct — clientInstances is already EventTarget[], so the double cast discarded a relationship that was there for free.

Fixed in 4059e95: clientInstances[0] as EventTarget & { callTool; callToolStream }, the same single intersection this file already uses at lines 991 and 2816, with a comment saying why.

The ~15 pre-existing as unknown as casts elsewhere in App.test.tsx are deliberately untouched — they predate this PR and converting them is a separate cleanup, not something to fold into a bug fix.

npm run local:gate passes.

[Copilot]

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.

[cliffhall]

Review round 3 — clean

Copilot reviewed 26 of 26 changed files and generated no new comments. Both earlier findings are addressed:

1. The App.tsx seam was untested (round 1, plus its suppressed Apps-path twin) — four App.test.tsx assertions now pin skipArgumentCoercion on the plain call, the task call, the Apps call (combined with skipOutputValidation), and — the one that keeps it honest — its absence being false on an ordinary widget-form call.
2. An unjustified double cast in the new test helper (round 2) — replaced with the single as EventTarget & {…} intersection this file already uses.

npm run local:gate passes on the final commit.

[cliffhall]

Direction reversed — the refusal now goes both ways

Maintainer call on the product question #2171 left open: a raw-JSON argument the schema would retype is refused, not sent. The earlier revisions of this branch did the opposite — they made both surfaces send verbatim — so the branch has been rewritten (force-push, 9516299) rather than amended, and rebased onto the current v2/main (4d69dbf9).

Everything the previous approach added is gone: no ToolCallOptions, no skipArgumentCoercion, no rawJsonArgs threading through the panels/screens/App.tsx, and no change to InspectorClient. Edit-and-replay keeps the behavior it already had; the Tools and Apps tabs' Edit as JSON is what changes, and it now agrees with it.

That also removes the clients/web/src/App.tsx conflict this PR had against v2/main — the new implementation stops at SchemaForm and never reaches the composition root being decomposed under #2126.

Earlier review rounds are superseded: both findings were against App.tsx plumbing and its tests, none of which exists now. npm run local:gate passes on 9516299.

Re-requesting review on the new diff.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

[cliffhall]

Review round 4 — addressed (61f58f9)

Copilot's one finding was right: the panel tests stopped at the disabled button, so #2171's "pins the wire rather than the form value" criterion was unverified. ToolsScreen and AppsScreen now assert onCallTool / onOpenApp are not called for a refused draft, and are called once the value is rewritten with the declared type — the second half being what proves the draft blocks the dispatch rather than the screen being inert.

Two deliberate departures from the suggestion, both argued in the inline reply: no separate "Run as task" case (the gate is upstream of the callTool/callToolStream split, which happens in App.tsx past the callback), and screen-level rather than integration-level (under refusal there is no payload to pin, InspectorClient is unmodified by this PR, and the web integration project is node-env with no DOM so it cannot drive the form).

On local gate status — stated plainly

I could not get a clean npm run local:gate read for this revision, and it is a machine problem, not a code one. This box is currently at load average 187, with other worktrees running their own suites. Across three attempts the gate aborted on two different unrelated tests, each time on a timing threshold:

Attempt	Failed on	Detail
1, 2	scripts/lib/render-smoke.test.mjs	416ms / 407ms against a 400ms deadline
3	ServerSettingsModal … (SEP-2350)	flat 5000ms vitest timeout, 7190 others passing

Both pass in isolation. Neither is in code this PR touches. What did execute was green — full validate:web unit suite on attempt 3 apart from that one timeout, and every new test in this PR passes.

Because validate is an && chain, each abort happened before coverage / smokes / Firefox / Storybook, so those remain unrun locally for this revision. CI is the real check here — it runs on a dedicated runner without the contention. I filed #2180 about the render-smoke margin and have since retracted its premise: I originally claimed it failed on an idle machine, which was wrong, and recommended closing it.

Flagging this rather than quietly re-running until something came up green.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.