mcpx: re-issue a broken modern request once, tools/call gated on annotations (#1638) by dezren39 · Pull Request #1645 · developing-today/code

dezren39 · 2026-10-28T10:20:14Z

Implements #1638 (Phase 2 of #1634): a modern request whose response stream breaks unanswered is re-issued exactly once with a new id, per the 2026-07-28 transport MUST.

Stream-lost identity is a data marker, not the code or the message. lost() (http.go) now stamps its synthesized codeStreamLost (-32000) error with data: {"mcpxStreamLost": true}. isStreamLost (client.go) requires code + marker: the receive loop's own -32000 "connection closed" and any server-sent unmarked -32000 are never re-issued, and there is no message matching anywhere.

Re-issue lives in Client.roundTrip as an attempt loop over roundTripOnce: on a marked stream-lost, when policy allows and ctx.Err() == nil, it re-sends once with a fresh id from nextID; a second marked failure surfaces. Era-gated to EraModern — legacy keeps resume(). Each MRTR round is its own roundTrip call, so each round re-issues independently. Cancellation is never re-issued (guarded both in the loop and in roundTripOnce, where a cancelled caller hears its own cancellation).

tools/call is annotation-gated. call/callReissue carry the caller's policy: true for every method except tools/call, where CallTool computes it from retained list-time annotations — re-issue only when the tool declared readOnlyHint or idempotentHint. Retention (recordToolHints, called from ListTools) follows the toolHeaders precedent but is NOT gated on mirrorsHeaders(), so the gate works on every transport. The gate reads only retained state (no lazy tools/list round trip in CallTool); on modern HTTP, toolCallHeaders lists before the first attempt, so the gate computed after it is current.

Conformance: gap streamable-http-client-reissue-broken-request retired, covered by TestModernReissueBrokenRequest; matrix regenerated.

Tests (new reissue_test.go, all passing): marked re-issue success with fresh id; second stream-lost surfaces; unmarked -32000 not re-issued (receive-loop shape, server-sent same-message, server-sent other-data); cancellation not re-issued; legacy era untouched; annotation gate (readOnly/idempotent re-issued, destructive/unannotated not). Full pkgs/mcpx packages verified on the laptop: mcpclient, conformance, daemon and all remaining packages pass (e2e is long-running locally; CI is the arbiter).

Queue task 1778. Refs #1634. Phase 1 (#1637, legacy) and Phase 3 (#1639) are out of scope, as scoped in the issue.

🤖 Agent note · queue worker (repo-worker-code), lane B · task 1778

^{Need help on this PR? Tag @codesmith-bot with what you need.}

…tations (#1638) lost() stamps its synthesized -32000 with a mcpxStreamLost data marker; Client.roundTrip re-issues a marked stream-lost exactly once with a fresh id on modern connections only (legacy keeps resume()). Unmarked -32000 (receive-loop connection-closed, server-sent) is never re-issued and no message matching is used. tools/call re-issues only when the tool retained list-time annotations declare readOnlyHint or idempotentHint; hints are retained in ListTools in every era/transport (not gated on mirrorsHeaders). Retires the streamable-http-client-reissue-broken-request conformance gap and regenerates the matrix.

coderabbitai · 2026-10-28T10:20:24Z

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 2 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: defaults
Review profile: CHILL
Plan: Advanced
Run ID: e26dc208-4702-40c6-8c13-ca718fdbc962

Commits

Reviewing files that changed from the base of the PR and between 211aa58 and d1b6023.

Files selected for processing: 5

pkgs/mcpx/docs/spec/conformance-matrix.md
pkgs/mcpx/internal/conformance/coverage_transport.go
pkgs/mcpx/internal/mcpclient/client.go
pkgs/mcpx/internal/mcpclient/http.go
pkgs/mcpx/internal/mcpclient/reissue_test.go

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

chatgpt-codex-connector · 2026-10-28T10:20:30Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-28T14:55:12.941330Z	`d1b6023`	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c2b26d2d7e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-10-28T10:23:28Z

+		if attempt == 0 {
+			// After toolCallHeaders: on a modern HTTP connection it
+			// has just listed, so the retained hints are current.
+			allowReissue = c.toolCallReissueAllowed(name)
+		}


Recompute retry eligibility after refreshing tool annotations

On modern HTTP, a HeaderMismatch causes the next iteration's toolCallHeaders(..., true) to refresh ListTools, which replaces the retained tool hints. However, allowReissue remains frozen at its initial value. If the refreshed definition removes readOnlyHint or idempotentHint, a subsequent broken response still reissues the now-unsafe tool using the stale permission, potentially executing its effects twice. Recompute eligibility after each toolCallHeaders call so it reflects the refreshed definition.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-10-28T10:23:28Z

 func (c *Client) call(ctx context.Context, method string, params json.RawMessage, out any) error {
+	return c.callReissue(ctx, method, params, out, true)


Apply the tool retry gate to generic requests

When a caller uses Client.Request(ctx, "tools/call", params) instead of the typed helper, request.go routes directly through c.call, which now unconditionally enables reissue. A broken modern HTTP response therefore repeats even a destructive or unannotated tool, bypassing the safety gate in CallTool and potentially duplicating side effects. Apply the tool-specific policy to this generic entry point as well, or conservatively disable reissue for generic tools/call requests.

Useful? React with 👍 / 👎.

…reissue # Conflicts: # pkgs/mcpx/docs/spec/conformance-matrix.md # pkgs/mcpx/internal/mcpclient/client.go

dezren39 · 2026-10-28T14:53:34Z

Merged current main (d1b60238): the only code conflict was in ListToolsWithHints — this branch's recordToolHints(all) and main's new schemaval pass (#1596) both kept; conformance matrix regenerated from the merged tree. Laptop: go test ./internal/mcpclient/ ./internal/conformance/ pass. Re-running CI on the merged head; will merge when green. — queue worker lane A, task 1778 resume

…1639) (#1764) * docs(mcpx): register truth-up TR-29 / PM-06 as-built + roadmap row (#1639) Phase 3 of design #1634, per triage 1772. TR-29 now describes the landed Phase 1 client-layer session-404 recovery (#1637, PR #1642) instead of mcpx missing; PM-06 describes the landed Phase 2 broken-stream re-issue (#1638, PR #1645) with the tools/call annotation gate recorded as mcpx's proxy decision, instead of not examined; roadmap Legacy client robustness row takes the triage-settled wording (status stays proposed). Docs only. * style(mcpx): treefmt (prettier) the #1639 register truth-up docs

dezren39 requested a review from a team as a code owner October 28, 2026 10:20

chatgpt-codex-connector Bot reviewed Oct 28, 2026

View reviewed changes

dezren39 mentioned this pull request Oct 28, 2026

mcpx: Phase 2 of #1634 — re-issue a broken modern request once (tools/call gated on annotations) #1638

Closed

github-actions Bot assigned dezren39 Oct 28, 2026

This was referenced Oct 28, 2026

mcpx: Phase 3 of #1634 — register truth-up (TR-29, PM-06) and nix#203 evidence #1639

Closed

fleet: laptop SSH unreachable since 2026-10-28 ~11:16Z — all laptop-worktree queue work blocked (MaxStartups pattern or laptop offline) #1667

Closed

dezren39 added 2 commits October 28, 2026 09:23

Merge remote-tracking branch 'origin/main' into queue/1778-1638-mcpx-…

a34ae7e

…reissue # Conflicts: # pkgs/mcpx/docs/spec/conformance-matrix.md # pkgs/mcpx/internal/mcpclient/client.go

Merge remote-tracking branch 'origin/main' into queue/1778-1638-mcpx-…

d1b6023

…reissue # Conflicts: # pkgs/mcpx/docs/spec/conformance-matrix.md # pkgs/mcpx/internal/mcpclient/client.go

dezren39 merged commit 18f9074 into main Oct 28, 2026
17 checks passed

dezren39 deleted the queue/1778-1638-mcpx-reissue branch October 28, 2026 15:27

dezren39 mentioned this pull request Nov 1, 2026

Design: mcpx client session & stream recovery — re-initialize on a legacy session 404, re-issue a broken modern request once #1634

Closed

4 tasks

Navigation Menu

Repository navigation

mcpx: re-issue a broken modern request once, tools/call gated on annotations (#1638) - #1645

mcpx: re-issue a broken modern request once, tools/call gated on annotations (#1638)#1645
dezren39 merged 3 commits into
mainfrom
queue/1778-1638-mcpx-reissue

dezren39 commented Oct 28, 2026 •

edited by blacksmith-sh Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 28, 2026 •

edited

Loading

Review limit reached

Uh oh!

chatgpt-codex-connector Bot commented Oct 28, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Oct 28, 2026

Uh oh!

chatgpt-codex-connector Bot Oct 28, 2026

Uh oh!

dezren39 commented Oct 28, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		func (c *Client) call(ctx context.Context, method string, params json.RawMessage, out any) error {
		return c.callReissue(ctx, method, params, out, true)

Conversation

dezren39 commented Oct 28, 2026 • edited by blacksmith-sh Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Oct 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

chatgpt-codex-connector Bot commented Oct 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Oct 28, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Oct 28, 2026

Choose a reason for hiding this comment

Uh oh!

dezren39 commented Oct 28, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

dezren39 commented Oct 28, 2026 •

edited by blacksmith-sh Bot

Loading

coderabbitai Bot commented Oct 28, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Oct 28, 2026 •

edited

Loading