Repository navigation
agent: MCP 401s keep the server's reason; over-cap events recognized in any key order - #146
Merged
Merged
Conversation
…in on tool calls (F1)
Since every POST goes through post_bounded, any 401 became rmcp's bare `AuthRequired`, so a server
with a static key that stopped accepting it ("invalid API key", as a JSON-RPC error body) showed
only "Auth required". A 401 whose small body (<= 64 KiB) is a JSON-RPC error now fails as rmcp's
own bare-401 error does — `HTTP 401 Unauthorized: <message> (JSON-RPC error <code>)` — still a 401
to the OAuth refresh, and the tool error a server with no login shows. For a login, a 401 that
survives the refresh-and-retry says to run `agent mcp-login <server>` again, on the tool call and
not only at connect.
Tests (tests/mcp_unauthorized.rs, each failing without its half of the fix): a static-key server's
mid-session 401 is the tool error with the server's message; a login the server keeps refusing
names mcp-login on the tool error, with a JSON-RPC body and with a bare challenge. The rmcp-parity
test's divergence case now expects the 401 with its message.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk
…rder (F2) oversized_stand_in only recognized an over-cap event when `method` preceded `params`; JSON fixes no order, so a params-first event was looped on over HTTP (the stream errored and reconnected into it) and dropped silently over stdio. The bounded head is now walked order-independently — method, id, result/error and params wherever they fall, and every whole scalar and `_meta` inside params — and when the method lies past the window the message is still taken for an event unless the head proves it a request or a response. Over stdio, a stand-in whose routing lay past the head goes to every push stream on the connection; on a direct-HTTP stream it is that stream's. Either way the event is skipped, the gap reported, and the position advances with the next event or heartbeat. Tests: mcp_message_cap's over-cap push test runs in both key orders over both transports — the fixture's MCP_FIXTURE_KEY_ORDER=params_first writes params first with the payload first inside it, so nothing identifying is in the window (both new cases fail without the fix: no gap, and over HTTP a reconnect); unit `an_oversized_events_notification_is_recognized_in_any_key_order`. ARCHITECTURE.md: this and F1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk
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.
What
The two low findings from #136's audit. Each fix has a test that fails without it; I checked this by reverting each half of each fix in place and running its test.
post_bounded, which turned any 401 into rmcp's bareAuthRequired. A server with a static key that stops accepting it ("invalid API key", sent as a JSON-RPC error body) then showed only "Auth required".HTTP 401 Unauthorized: <message> (JSON-RPC error <code>). The OAuth refresh still treats it as a 401. For a server with no login, it is the tool error the model sees, with the server's message in it. For a login, a 401 that survives the refresh-and-retry now says to runagent mcp-login <server>again, on the tool call as well as at connect. Any other 401 is stillAuthRequired.mcp_unauthorized::a_401_from_a_server_with_a_static_key_is_the_tool_error_with_the_servers_message;…a_login_the_server_keeps_refusing_names_mcp_login_on_the_tool_error_and_keeps_its_message;…a_login_refused_with_a_bare_challenge_names_mcp_login_on_the_tool_error. The rmcp-parity test's divergence case now expects the 401 with its message.oversized_stand_inrecognised an over-cap event only whenmethodcame beforeparams, and JSON fixes no key order. Over HTTP, a params-first event made the stream error and reconnect into the same event, forever. Over stdio, it was dropped with no gap reported.method,id,result/errorandparamswherever they fall, and insideparamsevery whole scalar and_meta. Ifmethodlies past the window, the message is still taken for an event unless the head proves it is a request (anid) or a response. Over stdio, a stand-in whose routing lay past the window goes to every push stream on that connection; on a direct-HTTP stream it belongs to that stream. Either way the event is skipped, the gap is reported, and the position advances with the next event or heartbeat.mcp_message_cap::an_over_cap_push_event_with_params_first_over_{http,stdio}_…. Both fail without the fix: no gap, and over HTTP a reconnect. The usual-order HTTP and stdio cases still pass. Unit test:mcp_stdio::an_oversized_events_notification_is_recognized_in_any_key_order.The test fixtures gained two options:
MCP_FIXTURE_KEY_ORDER=params_first(events fixture): writesparamsbeforemethod, with the payload first insideparams. That is the worst case: nothing that identifies or routes the event is within the window.issue(token)accepts a static key, andreject_callsrefuses everytools/call, even after a refresh.Trade-off in F2. If an over-cap message's head shows neither its method nor an
id, it is assumed to be an event. On a connection that carries events, guessing wrong costs one spurious gap notice. Guessing the other way loses an event silently (stdio) or loops on it (HTTP). This is documented in the code and in ARCHITECTURE.md.Checks
tests/mcp_*.rssuite (including the newmcp_unauthorized),serve_reaper,serve_http,serve_drain,run_signal_handling,run_cli_flags,serve_harness_deadlines, and theagentandagent-corelib tests.cargo clippywith-D warningsis clean, both for the agent with code-mode and for the whole workspace.cargo fmt --checkanddprint checkare clean.check.pypasses 12/12 over HTTP and 12/12 over stdio.🤖 Generated with Claude Code
https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk