fix(adapters): persist the MCP lane before the agent can see the response - #279
Merged
Conversation
…stand-in
EVIDENCE, NOT A FIX. This test is expected to FAIL. Do not merge.
0.8.0 fixed "the MCP stand-in no longer loses its recording when the agent
kills it" -- the lane had been written only at stdin EOF, so an abrupt kill
lost everything. The lane is now persisted after every captured call.
It is not enough. An agent that kills the stand-in immediately after reading
a response loses THAT call from the lane, while every earlier call survives.
Observed, with the real server's own log as the oracle:
server received: initialize, notifications/initialized, tools/list,
tools/call get_weather
lane recorded: initialize, tools/list
`record` exited 0 and minted a trace. So a tool the agent demonstrably
invoked is absent from the evidence, and nothing said so.
That is the defect class this milestone exists to eliminate, in the artifact
the MCP boundary's claims rest on. A lane that silently runs short understates
what the agent did, and every assertion evaluated over it inherits the
omission.
The fixture is an agent identical to the suite's polite one but for its last
act: proc.kill() instead of stdin.close(); wait(). Every existing MCP test
closes politely, which is exactly the path the 0.8.0 defect did not affect.
Reported rather than fixed: the brief forbids self-authorising a fix for a
defect the same run discovered.
Refs #257
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Formatting only. The single intended CI failure on this branch is the assertion that documents the finding, not a lint. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…onse 0.8.0 fixed the stand-in losing its whole recording when an agent killed it, by persisting the lane after every captured call. It was not enough. The stand-in forwarded the server's response to the agent BEFORE persisting it. The moment the agent has a response it may kill the stand-in, so the final call of a session was lost -- while `record` still exited 0. A tool the agent really invoked was simply absent from the evidence, silently. Found by the falsifiability suite (#273), with the real server's own request log as the oracle: the server was asked for `tools/call get_weather`, the lane held only `initialize` and `tools/list`, and nothing said so. That matters more than a lost line. A lane that silently runs short UNDERSTATES what the agent did, and every assertion evaluated over it inherits the omission -- including a guard claim that a tool was never called. Capture and persistence now happen before the response is forwarded. By the time the agent can act on a response, the evidence for it is on disk. The mocked-tool path already had this ordering and never raced; only the forwarded path did. And a failed flush is no longer swallowed. The old code deliberately ignored it, on the reasoning that the write at stdin EOF might still save the run -- but that write is exactly what an abrupt kill skips, so it was never a fallback. A flush that fails now fails the record by name rather than minting a partial lane. Evidence lost silently is the one outcome this tool cannot ship. Ships with the test that found it, cherry-picked from the evidence branch unchanged: it failed on the old code and passes here. Closes #273 Closes #257 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
Fixes the false green found by the falsifiability suite. Closes #273 and #257, and supersedes draft PR #274 (which was the evidence).
The defect
0.8.0 fixed the stand-in losing its whole recording on an abrupt kill, by persisting the lane after every captured call. It was not enough.
The stand-in forwarded the server's response to the agent before persisting it (
mcp_stdio.rs:363, capture at:416). The moment the agent has a response it may kill the stand-in — so the final call of a session was lost, whilerecordstill exited 0.With the real server's own request log as the oracle:
initialize,notifications/initialized,tools/list,tools/call get_weatherinitialize,tools/listA lane that silently runs short understates what the agent did, and every assertion over it inherits the omission — including a guard claim that a tool was never called.
The fix, both halves
1. Ordering. Capture and persist happen before the response is forwarded. By the time the agent can act on a response, the evidence for it is on disk. The mocked-tool path already had this ordering and never raced; only the forwarded path did.
2. Detectability. A failed flush is no longer swallowed. The old code ignored it deliberately, reasoning that the write at stdin EOF might still save the run — but that write is exactly what an abrupt kill skips, so it was never a fallback. A flush that fails now fails the record by name rather than minting a partial lane.
That second half is the one that matters beyond this instance: it turns "the lane ran short" from silent into loud.
Verified
Ships with the test that found it, cherry-picked from the evidence branch unchanged — it failed on the old code and passes here. No adjusting the test to fit the fix.
falsifiability_mcp_killmcp_stdio_e2emcp_http_e2eflowproof-adaptersfmt --check/clippy -D warningsClippy caught a second
flush_lanecall site whoseResultI had left unhandled — the exact swallowed-error case this change exists to remove.Note
#274 can be closed once this merges; its content is here, and its test now passes.
🤖 Generated with Claude Code