fix(mcp): fail closed on stdio child loss

[steipete]

Summary

• make stdio MCP connection lifecycle generation-owned so child exit or stdout EOF atomically closes the active generation
• clear MCPClient connectivity and cached tools on unexpected closure, fail every pending request immediately with typed transportClosed, and refuse later requests or notifications before writing
• cancel and remove request timeout tasks exactly once while preventing late responses or old-child callbacks from affecting a replacement session
• preserve the serialized JSON-line frame writer, make explicit disconnect idempotent, and require tool discovery to succeed before publishing a connected client
• add deterministic child-exit, 12-request concurrent drain, timeout cleanup, superseded-frame, discovery-EOF, reconnect, and no-late-invalidation regressions

Proof

• TACHIKOMA_TEST_MODE=mock TACHIKOMA_DISABLE_API_TESTS=true swift test --parallel — 837 core + 62 MCP tests passed
• focused MCP client/adapter/frame/lifecycle suites — 38 tests passed
• swift build -c release
• swift package resolve with no Package.resolved drift
• swiftformat --lint .
• swiftlint lint --config .swiftlint.yml --strict — 0 violations
• structured autoreview clean with no actionable findings

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: needs changes before merge. Reviewed August 14, 2026, 6:22 AM ET / 10:22 UTC.

ClawSweeper review

What this changes

This PR makes stdio MCP connections fail closed when their child process exits or stdout ends, clearing client state and failing pending requests.

Merge readiness

⚠️ Ready for maintainer review - 5 items remain

Keep open: the EOF close task can overtake tasks that deliver complete responses already read from stdout, causing a successful MCP response to be discarded as transportClosed.

Priority: P1
Reviewed head: e870d9744a23e38685caf4d90bf0a30458e31fe2

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The repair is focused and well-covered in related cases, but the response-before-EOF ordering defect blocks merge.
Proof confidence	🌊 off-meta tidepool	Not applicable: This collaborator PR is exempt from the external-contributor proof gate; its body nevertheless reports real-child lifecycle coverage and the repository validation suite.
Patch quality	🦐 gold shrimp (3/6)	2 actionable review findings remain.

Verification

Check	Result	Evidence
Real behavior	Not applicable	Not applicable: This collaborator PR is exempt from the external-contributor proof gate; its body nevertheless reports real-child lifecycle coverage and the repository validation suite.
Evidence reviewed	4 items	Current main lacks fail-closed EOF handling: On current main, stdout EOF cancels only the read source; it does not close the connection or resolve pending requests, so the central problem remains unfixed there. Response-delivery race: Complete stdout messages are handed to unstructured response tasks, then EOF starts a separate unstructured close task. There is no ordering guarantee, and closure invalidates the generation before a response task can claim its continuation. Existing lifecycle coverage: The branch adds deterministic tests for child EOF, pending-request draining, timeout cleanup, reconnect, and discovery EOF, but none covers a complete response immediately followed by EOF.
Findings	2 actionable findings	[P1] Drain received responses before closing the transport [P3] Remove the release-owned changelog entry
Security	None	None.

How this fits together

Tachikoma’s MCP client discovers tools through a transport; the stdio transport launches a child process, exchanges JSON-line requests and responses, and reports closure back to the client. This change controls teardown and reconnect behavior when that child transport ends.

flowchart LR
  A[MCP client] --> B[Stdio transport]
  B --> C[Child process]
  C --> D[Response or EOF]
  D --> E[Deliver response]
  D --> F[Close connection]
  F --> G[Fail pending requests]
  F --> H[Clear client tools]

Loading

Before merge

• Drain received responses before closing the transport (P1) - Each complete stdout response is dispatched in an unstructured task, and EOF then starts another unstructured task that closes the generation. The close can run first, fail the pending continuation, and make the already-received response a no-op. Preserve ordering and add a child-response-then-EOF regression.
• Remove the release-owned changelog entry (P3) - CHANGELOG.md is release-owned, so this implementation PR should leave the release note to the release process; its PR body already provides the needed user-visible context.
• Resolve merge risk (P1) - A child that writes a complete tool response and immediately exits can have that response suppressed, turning a successful call into transportClosed.
• Resolve merge risk (P1) - The branch edits release-owned CHANGELOG.md, which should remain with the release process.
• Complete next step (P2) - A narrow, source-proven ordering repair and regression test can resolve the merge blocker.

Findings

• [P1] Drain received responses before closing the transport — Sources/TachikomaMCP/Client/StdioTransport.swift:520-522
• [P3] Remove the release-owned changelog entry — CHANGELOG.md:11

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Implementation and test delta	implementation +427/-161; tests +279/-3; release note +1	The lifecycle rewrite is substantial, so the missing response-before-EOF case is material validation coverage rather than optional polish.

Merge-risk options

Maintainer options:

1. Preserve responses read before EOF (recommended)
   Order response settlement before generation invalidation and add a regression where the child writes a valid response then immediately exits.

Copy recommended automerge instruction

@clawsweeper automerge

Special instructions:
Preserve complete stdout responses read before EOF, add a response-then-EOF regression, and remove the CHANGELOG.md edit.

Technical review

Best possible solution:

Serialize delivery of all complete stdout responses ahead of EOF teardown, add a response-then-EOF regression, and remove the release-owned changelog edit.

Do we have a high-confidence way to reproduce the issue?

Yes: a fixture child can write one valid JSON-RPC response and exit immediately; source ordering shows EOF teardown may win before the response continuation is resumed.

Is this the best way to solve the issue?

No: fail-closing is appropriate, but teardown must first settle complete responses already read from stdout rather than race them.

Full review comments:

• [P1] Drain received responses before closing the transport — Sources/TachikomaMCP/Client/StdioTransport.swift:520-522
  Each complete stdout response is dispatched in an unstructured task, and EOF then starts another unstructured task that closes the generation. The close can run first, fail the pending continuation, and make the already-received response a no-op. Preserve ordering and add a child-response-then-EOF regression.
  Confidence: 0.96
• [P3] Remove the release-owned changelog entry — CHANGELOG.md:11
  CHANGELOG.md is release-owned, so this implementation PR should leave the release note to the release process; its PR body already provides the needed user-visible context.
  Confidence: 0.94

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 0d2eb8d070a3.

Labels

Label changes:

• add P1: The branch can discard a completed MCP tool response, breaking active agent tool workflows.
• add merge-risk: 🚨 message-delivery: EOF teardown may suppress a valid JSON-RPC response that was already received from the child process.
• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
• add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: This collaborator PR is exempt from the external-contributor proof gate; its body nevertheless reports real-child lifecycle coverage and the repository validation suite.

Label justifications:

• P1: The branch can discard a completed MCP tool response, breaking active agent tool workflows.
• merge-risk: 🚨 message-delivery: EOF teardown may suppress a valid JSON-RPC response that was already received from the child process.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: This collaborator PR is exempt from the external-contributor proof gate; its body nevertheless reports real-child lifecycle coverage and the repository validation suite.

Evidence

Acceptance criteria:

• [P1] swiftformat --lint .
• [P1] swiftlint lint --config .swiftlint.yml --strict.
• [P1] TACHIKOMA_TEST_MODE=mock TACHIKOMA_DISABLE_API_TESTS=true swift test --parallel.

What I checked:

• Current main lacks fail-closed EOF handling: On current main, stdout EOF cancels only the read source; it does not close the connection or resolve pending requests, so the central problem remains unfixed there. (Sources/TachikomaMCP/Client/StdioTransport.swift:350, 0d2eb8d070a3)
• Response-delivery race: Complete stdout messages are handed to unstructured response tasks, then EOF starts a separate unstructured close task. There is no ordering guarantee, and closure invalidates the generation before a response task can claim its continuation. (Sources/TachikomaMCP/Client/StdioTransport.swift:520, e870d9744a23)
• Existing lifecycle coverage: The branch adds deterministic tests for child EOF, pending-request draining, timeout cleanup, reconnect, and discovery EOF, but none covers a complete response immediately followed by EOF. (Tests/TachikomaMCPTests/StdioTransportLifecycleTests.swift:8, e870d9744a23)
• Feature-history ownership: Recent stdio transport and MCP failure-path commits are by Peter Steinberger, including the prior frame-serialization work. (Sources/TachikomaMCP/Client/StdioTransport.swift:221, b59f26043f7c)

Likely related people:

• steipete: Current-main history attributes the recent stdio frame serialization and structured MCP failure work to Peter Steinberger; this PR is also authored by the corresponding GitHub account. (role: recent MCP transport contributor; confidence: high; commits: b59f26043f7c, 7f3db20ea441; files: Sources/TachikomaMCP/Client/StdioTransport.swift, Sources/TachikomaMCP/Client/MCPClient.swift)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Add a regression where a complete response is followed immediately by child EOF and assert the call succeeds.
• Remove the release-owned CHANGELOG.md line.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.