Skip to content

fix(providers,agent-core): refuse tool calls whose final args are not complete JSON - #171

Closed
jasonkneen wants to merge 1 commit into
stepfun-ai:mainfrom
jasonkneen:fix/harness-strict-tool-args
Closed

jasonkneen wants to merge 1 commit into
stepfun-ai:mainfrom
jasonkneen:fix/harness-strict-tool-args

Conversation

@jasonkneen

Copy link
Copy Markdown

Problem

Invariant: tool-call args are validated as a complete JSON object before dispatch, on every stop reason.

  • packages/providers/src/utils/json-parse.ts parseStreamingJson never throws; it falls back to partial-json salvage and finally {}. It was used for the FINAL tool-call arguments in anthropic-messages.ts (content_block_stop, ~661), openai-completions.ts (finishBlock, ~444) and openai-responses-shared.ts (output_item.done, ~713).
  • packages/agent-core/src/agent-loop.ts (~244) only refuses tool calls when stopReason === "length".
  • Result: a normal tool_use / tool_calls response with truncated argument JSON such as {"path":"a.ts","cont executed with salvaged { path: "a.ts" } whenever the required fields survived (e.g. a write whose content was cut off).
  • packages/providers/src/utils/validation.ts (~332): return validator.Check(coerced) ? coerced : args; returned the original UNVALIDATED args for plain JSON schema (MCP) tools when root coercion changed a non-object value that still failed the schema.

Fix

  • New parseToolCallArguments() in json-parse.ts: strict parseJsonWithRepair (JSON.parse plus the existing escape-only repair). Empty or whitespace text is still a valid {} (no-arg tools; Anthropic streams empty partial_json). A parse failure or a non-object result (null, array, primitive) returns an error, with the lenient salvage kept for display.
  • New optional ToolCall.argumentsError?: string (providers/src/types.ts). All three adapters use the strict parser at finalization and set the field only on failure. Streaming deltas still use the lenient parser for the live UI.
  • agent-loop.ts prepareToolCall: when argumentsError is set, it returns an immediate isError result ("was not executed because its arguments were incomplete or invalid JSON ... Re-issue the tool call with complete arguments"). This runs before prepareArguments and validation.
  • Wire safety: each adapter's replay serializer emits only id, name and arguments (plus namespace for Responses). Tests confirm argumentsError never appears in the replayed request. Session persistence is plain JSON, so an extra optional field is fine.
  • validation.ts: a coerced non-object value that fails the schema now falls through to the normal validation error and throws. The coerced success path is unchanged.

Test

  • packages/providers/test/tool-call-arguments-strict.test.ts (new) runs across anthropic-messages, openai-completions and openai-responses with mocked streams:
    • truncated {"path":"a.ts","cont plus a normal tool-use finish: argumentsError is set and salvaged args are { path: "a.ts" }
    • complete args: no field
    • empty args: {} with no error
    • [1,2]: flagged
    • replay: argumentsError is not in the outgoing request
    • Before the fix, 9 cases failed with AssertionError: expected undefined to deeply equal Any<String> / expected undefined to be defined.
  • packages/providers/test/validation.test.ts: args "5" against schema {type:["object","integer"],minimum:10} must throw. Before the fix: AssertionError: expected [Function] to throw an error.
  • packages/agent-core/test/agent-loop.test.ts: a toolUse message whose toolCall has argumentsError and schema-valid salvaged args must not call execute, and must emit an error result. Before the fix: AssertionError: expected [ 'hel' ] to deeply equal [].
  • Suites:
    • providers: 438 passed
    • agent-core: 466 passed
    • coding-agent: 8 failed, all pre-existing baseline failures (context-projection x3, resource-loader x3, step-tool-profile x1, 2791-fswatch x1)
    • tsgo, biome and the repo pre-commit checks pass

Risk / behaviour change

  • Tool calls whose final argument text is not a complete JSON object are now refused on every stop reason, where before they ran with salvaged args. The model gets an error result and can re-issue the call. OpenAI-compatible servers that emit malformed but salvageable argument JSON, such as duplicated or concatenated objects, will now see refusals instead of silent best-effort execution.
  • Non-object argument JSON now becomes arguments: {} plus an error, instead of passing the raw array or primitive through.
  • validateToolArguments now throws in one edge case where it used to return unvalidated args.
  • No provider registry, catalog or default changes.
  • Propagation: git grep 'type: "toolCall"' over coding-agent/src and agent-core/src finds no code that rebuilds a ToolCall from an explicit field list; the only hit is proxy.ts toolcall_start, and its toolcall_end Object.assigns the server's toolCall, so the field carries through. No persisted-message schema uses additionalProperties: false.
  • Known gap, not introduced here: finalization only happens in each adapter's per-block end event (Responses output_item.done, Anthropic content_block_stop). If a server sends the terminal event without that per-block end, the call keeps its lenient streaming args and is not flagged. Real endpoints always send the per-block end, so this is left out of scope.

Merge note

This is one of five independent PRs that edit packages/agent-core/src/agent-loop.ts (tool-result cap, strict tool args, leak-retry transcript, turn cap, agentLoop rejection). Each is based on current main; whichever merges later needs a small rebase.

https://claude.ai/code/session_01GUdnnHEaDThHUATSwXBpV9

… complete JSON

Invariant: tool-call arguments are validated as a complete JSON object
before dispatch, on every stop reason.

Cause: the adapters finalized tool-call arguments with parseStreamingJson,
which never throws and salvages partial JSON (`{"path":"a.ts","cont` ->
{ path: "a.ts" }). agent-loop only refused calls on stopReason "length", so
a tool_use/stop response with truncated or malformed argument JSON executed
with salvaged args whenever the required fields survived. Separately,
validateToolArguments returned the original unvalidated args for plain JSON
schema tools when root coercion changed a non-object value that still
failed the schema.

Fix: add parseToolCallArguments (strict JSON.parse + existing escape repair;
empty text is a valid {}), used at final tool-call finalization in
anthropic-messages, openai-completions and openai-responses-shared. On
failure or a non-object result the ToolCall gets a new optional
argumentsError (salvaged args kept for display only). prepareToolCall
returns an immediate error result for such calls before prepareArguments or
validation. Replay serializers only emit id/name/arguments, so the field
never reaches the wire (covered by tests). validateToolArguments now throws
instead of returning unvalidated args.
Copilot AI lite review requested due to automatic review settings September 23, 2026 11:42

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The reviewed changes and tests support safe approval.

Review effort: Lite
Findings: None

What changed in this PR

This PR prevents incomplete or non-object tool-call arguments from being executed and strengthens validation.

Changes:

  • Adds strict final argument parsing across providers.
  • Rejects invalid tool calls before execution.
  • Adds replay-safety and regression tests.
File Summary
packages/​providers/​test/​validation.test.ts Tests failed argument coercion validation.
packages/​providers/​test/​tool-call-arguments-strict.test.ts Tests strict parsing and replay behavior.
packages/​providers/​src/​utils/​validation.ts Rejects invalid coerced values.
packages/​providers/​src/​utils/​json-parse.ts Adds strict tool-argument parsing.
packages/​providers/​src/​types.ts Adds optional argument error metadata.
packages/​providers/​src/​api/​openai-responses-shared.ts Applies strict parsing and safe replay serialization.
packages/​providers/​src/​api/​openai-completions.ts Applies strict parsing and safe replay serialization.
packages/​providers/​src/​api/​anthropic-messages.ts Applies strict parsing and safe replay serialization.
packages/​agent-core/​test/​agent-loop.test.ts Verifies invalid calls are not executed.
packages/​agent-core/​src/​agent-loop.ts Rejects invalid tool calls before execution.

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

@ZouR-Ma ZouR-Ma closed this Sep 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants