Skip to content

fix(providers): synthetic tool-result text must say the outcome is unknown, not that it failed - #174

Closed
jasonkneen wants to merge 1 commit into
stepfun-ai:mainfrom
jasonkneen:fix/harness-synthetic-tool-result-wording
Closed

jasonkneen wants to merge 1 commit into
stepfun-ai:mainfrom
jasonkneen:fix/harness-synthetic-tool-result-wording

Conversation

@jasonkneen

Copy link
Copy Markdown

Problem

packages/providers/src/api/transform-messages.ts (~line 171, in insertSyntheticToolResults) synthesizes a placeholder toolResult for any tool call left without a real result — this happens after a crash/kill mid-tool, or when the agent loop aborts mid-batch (packages/agent-core/src/agent-loop.ts sequential/parallel executors break on signal?.aborted, lines ~554 and ~573, leaving later tool calls with no result in the in-memory transcript). The synthesized text was the literal string "No result provided" with isError: true.

That wording reads as "the tool call didn't execute" / "this failed cleanly." It does not tell the model that the side effect may already have happened. On resume, a model reading this synthetic result for e.g. a bash call has no signal to re-check state before retrying — it just sees an error and retries the (possibly non-idempotent) command blindly, which is exactly the invariant this recovery path is supposed to prevent: recovery must close an interrupted call with a structured error that says the outcome is unknown, never re-execute or imply nothing happened.

Fix

Replaced the literal string with a new exported helper, synthesizeInterruptedToolResultText(toolName), in packages/providers/src/api/transform-messages.ts:

Tool call "<name>" was interrupted before a result was recorded (session ended, aborted, or crashed). Its side effects may or may not have happened -- verify the current state (e.g. re-read files, check processes) before retrying.
  • Keeps isError: true and the existing message structure (role/toolCallId/toolName/content/timestamp) unchanged.
  • Names the specific tool that was interrupted.
  • States the call was interrupted, not that it failed, and explicitly says side effects "may or may not have happened."
  • Instructs verification (re-read files, check processes) before any retry.
  • No changes to packages/agent-core/src/agent-loop.ts — its abort breaks were already correct (they stop issuing new tool calls and leave the gap for this synthesis step to fill); execution semantics are untouched, only the synthesized wording changed.
  • Exported the helper so tests assert against it instead of duplicating the literal string, and so any future caller reuses the exact wording instead of re-inventing it. It is not re-exported from packages/providers/src/index.ts, so this stays package-internal surface, not new public API.

Grepped the whole repo (grep -rn "No result provided" . --exclude-dir=node_modules --exclude-dir=dist --exclude-dir=.git) — the only two hits were the implementation site and the one test file that pinned the old string; both are covered by this change. No other synthesis site or snapshot/doc pinned the old text.

Test

packages/providers/test/transform-messages-copilot-openai-to-anthropic.test.ts:

  • Updated the two existing tests that pinned the literal "No result provided" string (assertion lines 158 and 188) to assert against synthesizeInterruptedToolResultText(toolName) instead. Before the fix, these two tests failed against the new implementation output (actual text was the new interrupted-wording string, expected was still "No result provided") — that failure is the evidence the implementation changed the wording as intended. (Note: this is "old test pinned the buggy string, updated it to the new correct string, and confirmed it fails against the new impl if reverted" rather than "wrote a new failing test first" — being explicit about that since it's the more relevant evidence here.)
  • Added a new test, "synthesized interrupted tool result names the tool, marks it an error, and warns the outcome is unknown (not a failure)", asserting: role: "toolResult", isError: true, the text contains the tool name ("bash"), matches /interrupted/i, matches /may or may not have happened/i, and matches /verify/i. This test would fail against the old "No result provided" text (none of those patterns match it).
  • Added a new test, "does not duplicate a synthetic result when a real tool result is already present", confirming a tool call with an existing real toolResult produces exactly one toolResult message (no synthetic duplicate) — covers insertSyntheticToolResults's existingToolResultIds de-dup path.

All 6 tests in the file pass. Full packages/providers suite: 424 passed, 6 skipped, 0 failed. Full packages/agent-core suite: 465 passed, 1 skipped, 0 failed. packages/coding-agent full suite (--maxWorkers=2): 9 failed / 3465 passed / 51 skipped — matches the documented pre-existing baseline (context-projection.test.ts x3, resource-loader.test.ts x3, step-tool-profile.test.ts x1, 2791-fswatch-error-crash.test.ts x1, subagent-invocation.test.ts x1); none touch transform-messages or agent-loop and none are new.

Risk / behaviour change

Purely a user-facing string change inside a synthetic tool-result message; isError, message shape, and control flow are unchanged. Slightly longer text is sent to the model on resume after a crash/abort — no other behavioural change. No changes to provider registry/catalog/defaults, no provider add/remove, no web search touched.

https://claude.ai/code/session_01GUdnnHEaDThHUATSwXBpV9

Invariant: recovery closes an interrupted tool call with a structured
error that says the side effect may or may not have happened; it never
re-executes. transform-messages.ts synthesized "No result provided"
(isError true) for orphaned tool calls left after a crash/kill mid-tool
or an abort mid-batch (agent-loop.ts sequential/parallel executors
break on signal.aborted). That wording reads as "nothing happened" and
gave the model no reason to verify state before blindly retrying
non-idempotent commands like bash.

Replaced the literal string with synthesizeInterruptedToolResultText(),
which names the interrupted tool, states the call was interrupted (not
failed), says its side effects may or may not have happened, and tells
the model to verify current state before retrying. isError and message
shape are unchanged. agent-loop.ts's abort break semantics are
untouched -- only the synthesized wording changed.
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 implementation and tests are covered with no unresolved blocking issues.

Review effort: Lite
Findings: None

What changed in this PR

Updates synthetic tool-result messages to clarify that interrupted calls may have unknown outcomes.

Changes:

  • Added reusable interrupted-result wording with verification guidance.
  • Updated tests for wording, error status, and de-duplication.
File Summary
packages/​providers/​src/​api/​transform-messages.ts Generates explicit unknown-outcome messages for interrupted tool calls.
packages/​providers/​test/​transform-messages-copilot-openai-to-anthropic.test.ts Verifies recovery wording and duplicate prevention.

💡 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