fix(core): preserve MCP status for optional -32601 responses - #12500

@qwen-code /review

@qwen-code /review

@qwen-code /triage

Real-environment verification: PR #12500 @ 8a0d3bf

Verdict: behaviour matches the PR's contract end to end, and I recommend merging. In the real CLI, a tools-only server that answers the optional prompts/list / resources/list calls with a JSON-RPC -32601 stays connected. Every negative control I tried still disconnects. The notes below are non-blocking.

Merge context: #12562 fixes the same issue (#12496) and conflicts with this PR in packages/core/src/tools/mcp-client.ts, so only one of them can land. I ran it on the same rig as a third arm (§5).

Setup

Rig scripts: pr12500/rig/

1. The #12496 case in the real TUI

• base: /mcp shows ✗ disconnected right after startup and again 210 s later. The replica's ledger records a full re-handshake at +0.0 s, +95.4 s and +190.5 s. The health monitor keeps reconnecting because the status was poisoned, and each reconnect re-sends prompts/list and resources/list and gets 404 again.
• head: ✓ connected at both points, with one handshake and no further requests over the 210 s.
• The model's get_mcp_server_version call returns 19.4.0-replica in both arms. In headless -p mode base still declares and calls the tools, and the only visible symptom there is a false Warning: MCP server(s) failed to start. The user-facing damage is the TUI status plus the reconnect loop.

2. Status matrix: 29 optional-method rows plus 8 GET-stream rows, real CLI

Section A: optional-method errors

Each row is a real SDK error raised through the real client transports:

• Streamable HTTP raises SdkHttpError{status,text}.
• Legacy SSE raises a plain Error("Error POSTing to endpoint (HTTP nnn): …").

Head matches the PR's stated contract on 29/29 rows.

Section B: optional GET SSE stream

The PR also adds 422 and 501 to STREAMABLE_HTTP_GET_SSE_FALLBACK_STATUSES.

Head matches on 8/8 rows.

3. Negative control mid-session: real outages still disconnect

The replica starts healthy. It then switches to (a) an nginx 502 HTML page and (b) a 502 text page that quotes a -32601 payload. The model calls a GitLab tool and then /mcp is reopened.

• Head goes from ✓ connected to ✗ disconnected in both cases (left panel of Fig. 3 below).
• Base was already showing a poisoned ✗ disconnected before the outage.

4. Tests and mutation

Killed mutants:

• drop the pooled guard (10 fail)
• drop the standalone guard (1)
• accept any status (5)
• accept a missing status (1)
• drop the jsonrpc === '2.0' check (1)
• accept any error code (1)
• substring match instead of JSON.parse (1)
• drop the transport short-circuit in isMethodNotFound (1)
• revert the GET fallback to [400, 404] (2)

Survivors:

• M5: add 503 to LEGACY_MCP_METHOD_NOT_FOUND_STATUSES. The whole suite stays green because the negative table only has 401/403 status rows and no 5xx row. This is the same gap as R4-5 in the round-5 deferred list. The runtime behaviour is already right (h500, h502, h503 and s503 disconnect in §2); only a test would stop a regression.
• M9 (drop the ^ anchor) and M10 (read (HTTP nnn) from anywhere in the message). These are equivalent on every message the SDK actually emits. The SSE client always starts the message with Error POSTing to endpoint (HTTP nnn):, and Streamable HTTP carries a structured data.status. The peer body therefore has no way to supply the status, which the h404quote and s404quote rows confirm end to end. No action needed.

5. Comparison with #12562 (same issue, conflicting)

On the real-world shapes (GitLab 404 + -32601, SDK-style 400) both PRs fix the bug and keep tools usable. #12562 looks at the error code only. It ignores the HTTP status, parses from the first { to the last }, and does not require "jsonrpc":"2.0". On this rig that leads to three differences:

• 11 of the 16 negative rows stay connected under fix(core): keep MCP server connected on a -32601 onerror (#12496) #12562: 401/403/500/502/503 with a -32601 body, the body without a JSON-RPC version, and the text page quoting -32601 (both transports).
• In the TUI outage it stays ✓ connected through the quoting 502 page while the tool call itself fails (Fig. 3, right). With a plain nginx 502 both PRs disconnect.
• It does not include the GET 422/501 widening, which is not in its scope.

These divergent rows are synthetic. None of the real server implementations surveyed in my #12562 report (comment) produced them. So this is a difference in strictness, not a field bug reproduced against #12562. On this evidence #12500 is the stricter and slightly broader of the two. I did not re-measure #12562's separate mcp-retry.ts change here.

6. Merge readiness

• git merge-tree against current main c662f78 is clean. In the merge result, mcp-client.ts and mcp-client.test.ts are byte-identical to the tested head.
• CI: Test (ubuntu), Lint & Static and Integration (no-AK) are green.
• Review state: the bot's round-3 CHANGES_REQUESTED had its only Critical (R1-2) ruled fixed. Round 5 on 8a0d3bf posted no findings, and the bot then approved 8a0d3bf. reviewDecision is now REVIEW_REQUIRED, which means it is waiting on a human review.
• The triage sandbox verification (run 36256587959) was still in progress when I posted this. This report is independent of it.

Non-blocking notes for the author

1. Add a 5xx negative row, e.g. legacyOptionalMethodTransportError(503) plus an SSE 503 row. That kills M5 (R4-5).
2. The PR body doesn't mention the GET-stream widening. Adding 422/501 to STREAMABLE_HTTP_GET_SSE_FALLBACK_STATUSES is a separate behaviour change (§2-B). One line under What this PR does / Risk & Scope would help reviewers. The Tested on table can also now show macOS as tested by this run.
3. The standalone connectAndDiscover guard has no in-repo production caller. It is only reachable through core's public export; the CLI goes through McpClient / the manager. It is fine as a consistency guard, and its only coverage is the unit test.