Repository navigation
mcpx: Phase 2 of #1634 — re-issue a broken modern request once (tools/call gated on annotations) #1638
Description
Phase 2 of design #1634 (mcpx client session & stream recovery). Design doc: files/designs/code-mcpx-client-session-stream-recovery.md in the keep-the-four-repos-healthy goal workspace. Independent of Phase 1 (different era, different trigger) — either may land first.
Scope
Re-issue a modern (2026-07-28) request once when its response stream breaks — the 2026 spec's client MUST — without ever silently double-executing a tool call.
- Give the stream-lost error a stable, checkable identity. Today
lost()(internal/mcpclient/http.go~L378-389) synthesises code-32000(codeStreamLost), which the receive loop's "connection closed" failure shares; matching on message text is not acceptable. A marker in the error'sdataor an equivalent in-package sentinel — triage settles the shape. - Re-issue at the request layer (
Client.roundTrip,internal/mcpclient/client.go~L953), so each MRTRinput_requiredround is its own request: same method and params, once only, with a NEW request id. No re-issue when the caller's context is done. A second stream-lost surfaces the error. tools/callgate (decision recorded on Design: mcpx client session & stream recovery — re-initialize on a legacy session 404, re-issue a broken modern request once #1634, from PM-06's own hazard note): every other method (lists, reads, ping/server/discover, completions, prompt gets — reads by protocol nature) is re-issued automatically.tools/callis re-issued automatically only when the tool's annotations declarereadOnlyHintoridempotentHint; otherwise the stream-lost error surfaces unchanged and the host decides. Annotations are retained at list time for call time —Tool.Annotations(client.go~L159-170) carries them raw, and the client's per-tooltoolHeadersretention (modern.go) is the precedent.- Legacy era is excluded: legacy's broken-stream mechanism is resumability (
resume(), http.go ~L338); when resumption is exhausted the error surfaces, as today. - Retire conformance gap
streamable-http-client-reissue-broken-request(internal/conformance/coverage_transport.go).
Evidence (main ed8e591, 2026-10-28)
- Gap at
internal/conformance/coverage_transport.go:114(origin mcpx client: legacy Streamable HTTP and stdio robustness — session 404, GET stream, SSE retry, string ids, stream timeout, version header dezren39/darwin-nix#203): "a broken 2026-07-28 response stream is reported lost to the caller (http.go); nothing re-issues the request with a new id". - A modern SSE stream ending unanswered calls
t.lost(reqID)(internal/mcpclient/http.go~L190-205) and the call fails with the synthesised-32000. - PM-06 (
docs/compare/register/process-model.md:133-146): 2026-07-28 changelog — "clients MUST re-issue it as a new request with a new request ID"; and the register's own risk note — "re-issuing a non-idempotent tool executes it twice; nothing makes the server deduplicate" — which is why thetools/callgate exists and why a blanket re-issue was rejected on Design: mcpx client session & stream recovery — re-initialize on a legacy session 404, re-issue a broken modern request once #1634.
Acceptance
- Fake modern upstream that kills the response stream: a list call succeeds after exactly one re-issue, and the two attempts carried distinct request ids.
- A
tools/callagainst a tool annotatedreadOnlyHint(and one annotatedidempotentHint) is re-issued once and its result returned. - A
tools/callagainst an unannotated or destructive-annotated tool surfaces the stream-lost error, and the upstream observed exactly one execution. - A stream that breaks on the re-issue as well → the error surfaces; no third attempt.
- Caller-cancelled context → no re-issue.
- Legacy broken-stream behaviour unchanged (resume path untouched).
- Gap
streamable-http-client-reissue-broken-requestretired;go test ./...for pkgs/mcpx passes in CI.
Out of scope
Legacy session-404 recovery (Phase 1), register truth-up (Phase 3), a configurable re-issue count (open question on #1634 — one is the recorded default), re-issuing server-initiated requests from a dead stream.
Activity
developing-today commentedon Oct 28, 2026
CODE-657
dezren39 commentedon Oct 28, 2026
🤖 Agent note · done (triaged)
ed8e5910); both shapes settled; implementation packet enqueued as queue task 1778. Phase 2 is independently landable (modern-era only; no Phase 1 / mcpx: Phase 1 of #1634 — re-initialize on a legacy session 404 and retry the request once #1637 dependency).datamarker, never message matching.lost()(internal/mcpclient/http.go:378) synthesisescodeStreamLost(-32000, const :388) with no marker today, while the receive loop (client.go:870) synthesises its own -32000 "connection closed" — same code, different meaning (transport death; re-issuing into it is wrong), and a server can send -32000 too. Task 1778:lost()addsdata: {"mcpxStreamLost": true}; only marked errors trigger re-issue.Tool.Annotations(kept raw today, client.go ~:159) at list time underc.mu, refreshed on ListTools — followingcheckToolHeaders(modern.go:499) in shape, but NOT gated onmirrorsHeaders(), since the tools/call gate must work whether header mirroring is on or not. Coordination note in the packet: queue task 1641 (mcpx: Phase 2 of #1554 — client task execution for taskSupport "required" tools (internal/mcpclient) #1556) is adding a per-Client cached tool lookup in the same CallTool/ListTools area — one shared cached snapshot beats two.roundTrip(client.go:953 — single-shot today; fresh id per attempt fromnextID; each MRTR round is its own roundTrip so rounds re-issue independently; no re-issue whenctx.Err() != nil— the transport already suppresseslost()on cancellation); policy comes from the caller viacall/CallTool(:902/:1164): automatic for all methods except tools/call, which requires retained readOnlyHint or idempotentHint. A second marked failure surfaces. The conformance gap (coverage_transport.go:114) is retired in the same PR.hidden_files/packets/task1771-followup-1638.json(goal workspace) · design Design: mcpx client session & stream recovery — re-initialize on a legacy session 404, re-issue a broken modern request once #163486 remaining items
dezren39 commentedon Oct 28, 2026
🤖 Agent note · parked
a34ae7e6but CONFLICTING with current main. I merged main on the laptop worktree (d1b60238): the only code conflict was inListToolsWithHints— this branch'srecordToolHints(all)and main's schemaval pass (feat(mcpx): validate CallTool arguments against inputSchema (-32602), warn on output mismatch (#1483) #1596) both kept; conformance matrix regenerated; laptopgo test ./internal/mcpclient/ ./internal/conformance/pass. Parked on CI for the merged head: every job still queued after >6 min (runner backlog).queue/1778-1638-mcpx-reissuedezren39 commentedon Oct 28, 2026
🤖 Agent note · picked up
Merge pull request #1645 from developing-today/queue/1778-1638-mcpx-r…
dezren39 commentedon Oct 28, 2026
🤖 Agent note · done
docs(mcpx): register truth-up TR-29 / PM-06 as-built + roadmap row (#…