Commit Graph

29 Commits

Author SHA1 Message Date
teknium1
7b6e2ac375 fix(mcp): redirect header stripper builds httpx.AsyncClient at call time so proxy mounts stay observable
The stripper was an AsyncClient subclass minted before the client kwargs were
assembled, so the class the transport constructed was no longer the SDK httpx
AsyncClient the caller sees (tests/tools/test_mcp_http_proxy.py patches it to
assert both builders and the preflight probe carry the proxy mounts next to the
body cap - KeyError: mounts on CI). Return a factory that constructs
httpx_mod.AsyncClient(**kwargs) and installs the cross-origin
_build_redirect_request boundary on that instance instead: the same strip rules
apply (TestRedirectHeaderStripper unchanged), mounts/transport/headers reach the
real client class, and the proxy-mount tests observe them again.
2026-09-20 10:07:26 -07:00
beardthelion
a8afb3b567 fix(mcp): enforce strict_redirect_headers on redirects and the preflight probe (#115155)
The redirect credential boundary never actually worked: the stripper was an
httpx response event hook reading response.next_request, which httpx/httpx2
only populate after response hooks run — so it always early-returned and
strict_redirect_headers silently did nothing on the Streamable HTTP client.
The preflight probe additionally followed redirects with no boundary at all,
leaking configured headers (e.g. X-API-Key) to cross-origin targets.

The stripper is now an AsyncClient subclass overriding
_build_redirect_request — the only seam that sees exclusively redirect
follow-ups. (A request hook would also fire on the OAuth auth flow's
token/registration requests to a different-origin authorization server and
strip their credentials.) strict_redirect_headers is plumbed from server
config into the probe, which now enforces the same boundary.

Non-strict behaviour is unchanged: Authorization is withheld cross-origin
(httpx native, now explicit) and configured headers forward per the
documented compat contract.
2026-09-20 10:07:26 -07:00
teknium1
9bd8257c02 fix(mcp): complete the handshake when a stateless server names a modern protocolVersion and lacks server/discover
A server that answers the legacy initialize with HTTP 200 but reports
2026-07-28 (regardless of the version offered) makes the SDK raise
'Unsupported protocol version from the server'. auto mode then falls back
to server/discover, which such a server rejects with a non-JSON 4xx, and
the connect died with 'both Streamable HTTP and SSE transports failed
(Streamable HTTP: Server returned an error response; SSE: 405)' even
though the same initialize/tools/list POSTs succeed from curl (#113359).

When both fail for that reason, re-run initialize ourselves, adopt the
result pinned to the version we offered (keeps later requests
legacy-shaped, which the handshake just proved the server accepts), send
notifications/initialized and proceed to tools/list.
2026-09-18 12:48:42 -07:00
teknium1
f0c26f0554 fix(mcp): name the HTTP status, URL and body behind "Server returned an error response"
mcp >= 2.0's Streamable HTTP client folds any non-2xx whose body it cannot parse
as a JSON-RPC error into the opaque `-32603 Server returned an error response`.
Hermes printed that text verbatim in the SSE-fallback warning and in the
"both transports failed" ConnectionError, so users saw no status, no URL and
none of the server's own words (e.g. `400 {"code":-32020,"message":"Unsupported
MCP-Protocol-Version"}`) and had to reach for curl to learn what the server
actually said (#114350, #113359).

- `_make_http_rejection_recorder`: response hook on the owned SDK-httpx client
  that remembers the last 4xx/5xx (status, method, URL, head of the body; SSE
  bodies are never read). Sibling of the redirect-header stripper hook.
- `_describe_http_failure`: appends that detail only when the root cause is
  the SDK's opaque -32603 text, so a real JSON-RPC error or an httpx status
  error is never duplicated.
- `_run_http`: the fallback warning and both raise paths (both-transports
  ConnectionError; the no-fallback re-raise after a proven session, strict
  redirect headers or a non-rejection) carry the detail. Debug-logs the
  endpoint each connect attempt uses.
- Docs: troubleshooting entry for reading the new message.

Verified live against a real Streamable HTTP server (`hermes mcp test`, temp
HERMES_HOME): base prints `Streamable HTTP: Server returned an error response`;
fixed head prints `... (HTTP 400 from POST http://127.0.0.1:PORT/mcp:
{"jsonrpc":"2.0","id":null,"error":{"code":-32020,...}})`. Control: a 400
served as application/json already surfaces the JSON-RPC message and gets no
appendix; servers negotiating `initialize` down to 2025-06-18 (fixture and a
real FastMCP on mcp 1.12.4) connect and list tools on base and fix alike — the
pinned mcp 2.0.0 stamps the negotiated version on every post-handshake request
(wire-recorded), so the sticky seed in `_run_http` is not the cause of the
reported 400 and stays as designed (#14816).

Co-authored-by: JoaoMarcos44 <joaomarcosdias444@gmail.com>
2026-09-18 12:48:42 -07:00
beardthelion
b1dfaf40b0 fix(mcp): count SSE events across boundaries split between chunks
The per-event body cap searched each streamed chunk in isolation, so an
event terminator split across two chunks was never seen: the finished
event's bytes were charged to the next event and the cap tripped early
on ordinary TCP fragmentation. A last-boundary rfind also lumped
multiple completed events in one chunk into a single charge, and the
separator tuple missed spec-legal CR and mixed line-ending styles.

Scan the carried suffix plus each new chunk with a terminator-pair
regex, walking every boundary left to right so each completed event is
charged once. The lookahead keeps a lone CRLF line ending from parsing
as a boundary, so multi-line events cannot evade the cap.
2026-09-16 16:52:05 -07:00
teknium1
55e2986dfd fix: walk a group's __cause__/__context__ in the exception node walker
Review finding: _exc_children returned only .exceptions for a group, so
_is_session_expired_error missed a session-expiry marker (or the
InterruptedError override) hanging off a group's __cause__/__context__
that main used to inspect. Groups now yield nested + chain like every
other node; _flatten_messages' "group str() is opaque" rule is unchanged.
2026-09-15 19:02:39 -07:00
teknium1
e1114bdcf9 refactor(mcp): one cycle-safe exception walker for every connect-error scan
The salvaged fix gave `_find_missing` and `_flatten_messages` each their own
visited-set loop, next to the one `_is_session_expired_error` already had —
three copies of the same idiom in one module. Collapse them into
`_iter_exception_nodes` (pre-order, left-to-right, each node once, bounded by
`_EXC_TRAVERSAL_MAX_NODES`) and read all three scans off that list. Acyclic
output is byte-identical: the missing-executable search keeps its depth-first
order and a message-less leaf still renders as its class name.

Tests move from the issue-numbered file into `tests/tools/test_mcp_tool_errors.py`
(mirror of the source module): a two-node cycle renders the real messages, and a
missing stdio binary wrapped deeper than the recursion limit with the chain
looping back to the top is still reported as the missing executable. Both are
red on origin/main (RecursionError).

Co-authored-by: Stephan Mongstad <stephan@users.noreply.github.com>
2026-09-15 19:02:39 -07:00
KoNit-K
030d4caa0e fix(mcp): bound nested connection error traversal 2026-09-15 19:02:39 -07:00
Teknium
a6fdadfcee fix(mcp): cap HTTP/SSE response bodies before SDK parse
Port from openclaw/openclaw#123194: a hostile or misbehaving remote MCP
server could stream an unbounded HTTP catalog/tool-result body that the
MCP SDK buffers and JSON-parses before any of Hermes' post-parse limits
(resource cap, tool-result truncation) run.

New _make_mcp_body_cap_transport wraps the owned httpx AsyncClient's
transport on the Streamable HTTP (mcp >= 1.24) and SSE paths:
- finite HTTP bodies capped at 10 MiB (Content-Length rejected up front,
  streamed bodies capped chunk-by-chunk);
- each SSE event capped at 10 MiB, with accounting reset at completed
  event boundaries so long-lived streams/keepalives are unlimited;
- violations raise httpx.ReadError naming the byte cap, handled by the
  existing transport teardown/reconnect path (#66092).

verify/cert now live on the inner AsyncHTTPTransport (client-level TLS
kwargs are inert once a custom transport is passed); the SSE
httpx_client_factory is always injected so the cap applies with default
TLS too. Legacy mcp < 1.24 path (SDK-internal client, no hook) stays
uncapped — same degradation as strict_redirect_headers.
2026-09-13 20:45:25 -07:00
Teknium
a565e2d493 fix(mcp): widen the SSE fallback trigger and harden its guards (salvage #53764)
Relocated onto the decomposed module layout and hardened:

- Trigger covers the rejection CLASS, not just literal 400: SSE-only
  servers' load balancers answer the chunked Streamable HTTP initialize
  POST with 400/405/406/411, and the mcp>=2.0 SDK surfaces many such
  rejections as an opaque -32603 'Server returned an error response'
  (error class per #104363 by @RohithPariki). Timeouts and 5xx never
  trigger the fallback: they are not transport mismatches.
- Reconnect exclusion via _ever_connected instead of _ready: run()
  clears _ready before re-entering the transport, so the original guard
  also fired on reconnects after a proven session.
- Successful fallback latches _sse_fallback so reconnects go straight
  to SSE, and logs a warning suggesting the user pin transport: sse.
- Both transports failing raises a ConnectionError naming both errors
  and suggesting transport: sse / checking the URL.
- No fallback with strict_redirect_headers (SSE cannot enforce that
  boundary) or when transport is explicitly configured.
- Tests trimmed to 3 invariant contracts (proven red on base): fallback
  connects + latches; no fallback on reconnect/timeout/5xx; both-fail
  error is actionable.

The extracted SSE path reuses _sse_transport/_serve_transport from main,
preserving the bounded handshake timeout and reconnect-retry semantics.

Fixes #53676
2026-09-12 21:05:29 -07:00
Teknium
0d8a1575c5 refactor(mcp): keep the passive status RPC, drop the SDK contract and reason codes
mcp.servers.status now rides the shared _mcp_rpc decorator (profile scope, 4064,
5024 with the real message) instead of a hand-rolled try/finally with a blanket
except. Drop the _MCPConnectErrorText str subclass and reason taxonomy: the
existing status/error fields already carry the state, and a whitelist on the RPC
keeps error text out of the wire. The Desktop connections.health contribution
contract is held back until its consumer plugin is public. Tests trimmed to the
scope invariants (per-profile runtime visibility, scoped shutdown clears only its
own status, launch runtime never leaks into another profile).
2026-09-06 13:18:19 -07:00
Joey
a6699d60f4 feat(mcp): expose profile-scoped cached connection health 2026-09-06 13:18:19 -07:00
Teknium
f4d4831e70 simplify(compat): tools/mcp_tool facade — drop 13 re-export blocks (127 names) + shutil re-import; siblings read sibling-defined names directly (_core kept for facade state) 2026-09-03 13:28:47 -07:00
Teknium
e83816a4d1 review-fix(comments): restore lost #NNNN rationale comments across non-test source (mechanical sweep, condensed, code unchanged)
For each issue anchor present in BASE 63279301bc non-test .py and absent on HEAD, the BASE comment/docstring block was re-attached at the HEAD location of the code it explained (matched by the distinctive code line / enclosing def). Sentences already covered by an existing HEAD comment were deduped; the issue number always survives. Insert-only: no code lines changed.
2026-09-03 09:44:26 -07:00
Teknium
35b3888fc5 refactor(tools): MCP _dispatch absorbs _invoke_with_recovery; oauth 401 recovery flattened; small predicate folds 2026-09-03 01:37:03 -07:00
Teknium
bccfd1de26 refactor(tools): MCP handlers inline single-use render helpers, drop banners; registration foreign-owner log inlined; body blank squeeze 2026-09-03 01:31:18 -07:00
Teknium
58a993a54d refactor(tools): MCP group L docstring/comment compaction, >118-col fixes 2026-09-03 01:30:13 -07:00
Teknium
9458f4c27a refactor(tools): MCP group L docstring rewrap 2026-09-03 01:20:43 -07:00
Teknium
760ac0b609 refactor(tools): MCP handlers/errors/transport/config/health/registration/oauth-manager compaction — shared _dispatch, folded utility factory, auth-type cache tuple 2026-09-03 01:18:04 -07:00
Teknium
f7a4ccc081 refactor(tools): flatten identity-header/loop-stop control flow, inline SSE client factory 2026-09-03 00:01:18 -07:00
Teknium
672cd01c7c refactor(tools): fold legacy HTTP transport into streamable path, tighten MCP classifier/sampling bodies 2026-09-02 23:35:14 -07:00
Teknium
6a7d04e89d refactor(tools): collapse defensive layers and compact prose in MCP transport/loop/sampling/errors 2026-09-02 22:53:26 -07:00
Teknium
87c033a3e9 refactor(tools): hug lone closers in MCP errors/transport (AST-neutral) 2026-09-02 22:22:55 -07:00
Teknium
1ad360b3aa refactor(tools): compact MCP transport/loop/sampling/errors helpers, defensive collapse 2026-09-02 22:16:12 -07:00
Teknium
607a5b711e refactor(mcp): errors — one _jsonrpc_matches for both code/marker classifiers, one _exc_children walk for connect-error rendering, docstrings compacted by hand (455 -> 384; differential fuzz 20k cases identical) 2026-09-02 17:05:28 -07:00
Teknium
1623697cb7 refactor(mcp): compact session-expired marker table 2026-09-02 16:34:23 -07:00
Teknium
304118797b refactor(mcp): join short multi-line statements across the transport/handlers subset (AST-identical) 2026-09-02 16:29:53 -07:00
Teknium
1f7beadae7 refactor(mcp): dedupe error classification ladders, optional-type import helper, cause-chain walker 2026-09-02 15:41:56 -07:00
Teknium
c1f8af1e86 refactor(tools/mcp): split mcp_tool.py into transport/lifecycle/schema/handlers/... sibling modules; compact watchdog and schema cache 2026-09-02 14:44:15 -07:00