fix(mcp): treat any HTTP response as alive in health check (#732)

[penso]

Summary

• Fix MCP status showing "dead" for working Streamable HTTP servers with Bearer token / OAuth auth ([Bug]: MCP status is wrong in UI #732)
• Root cause: SseTransport::is_alive() sent a GET health probe and only treated 2xx/401 as "alive", but GET is optional in the MCP Streamable HTTP spec — servers commonly return 405, 403, or 400
• Fix: any HTTP response = alive; only network errors (connection refused, timeout, DNS) = dead
• Add mock MCP Streamable HTTP server (mock-mcp-server.js) for testing
• Add 5 Rust unit tests for is_alive() covering 200, 401, 405, 403, 400
• Add E2E test that starts mock server, adds via UI with Bearer token, verifies "running" status

Validation

Completed

• cargo test -p moltis-mcp — all 93 tests pass
• biome check --write — JS lint clean
• cargo +nightly-2025-11-30 fmt — no formatting issues in changed files

Remaining

• npx playwright test e2e/specs/mcp.spec.js — E2E tests
• ./scripts/local-validate.sh 733

Manual QA

1. Start mock server: node crates/web/ui/e2e/mock-mcp-server.js --bearer-token test123
2. Add in Settings > MCP > Streamable HTTP with URL http://127.0.0.1:<port> and header Authorization=Bearer test123
3. Verify status badge shows "running" (was "dead" before fix)
4. Verify tool count shows "1 tool"

Fixes #732

[codspeed-hq]

Merging this PR will not alter performance

✅ 39 untouched benchmarks
⏩ 5 skipped benchmarks¹

---

_{Comparing zesty-variety (19deecb) with main (1120eb8)}

Footnotes

1. 5 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[greptile-apps]

Greptile Summary

Fixes MCP status showing "dead" for Streamable HTTP servers that return non-2xx on GET health probes (e.g. 405, 403). The fix is correct and minimal: is_alive() now returns true for any HTTP response and false only on network errors, matching the MCP spec's optional-GET semantics.

The five new unit tests correctly assert true for all target status codes, and the E2E test properly uses toHaveText(\"running\", { timeout }) for retry-safe assertions. All previously noted review concerns are addressed in this version.

Confidence Score: 5/5

Safe to merge — the fix is correct and focused, all prior review concerns are addressed, and remaining findings are P2 style/completeness suggestions.

The core change is a two-line simplification of a condition that was provably wrong per the MCP spec. Unit tests cover all target HTTP status codes. The E2E test uses proper retry-safe Playwright assertions. Only two P2 suggestions remain: adding a debug log for the Err branch and a test for the false path — neither blocks merge.

crates/mcp/src/sse_transport.rs — Err branch should log at debug level and deserves one test case.

Important Files Changed

Filename	Overview
crates/mcp/src/sse_transport.rs	Core fix: is_alive() now returns true for any HTTP response, false only on network errors. Change is minimal and correct. Five new unit tests cover 200/401/405/403/400 → true; the Err branch lacks a test and the error is swallowed without a debug log.
crates/web/ui/e2e/mock-mcp-server.js	New mock MCP Streamable HTTP server for E2E tests. Returns 405 for GET (configurable via --get-status), handles POST JSON-RPC with optional Bearer auth. Well-structured for its purpose.
crates/web/ui/e2e/specs/mcp.spec.js	New E2E test suite for #732: starts mock server via fork(), adds server via UI, asserts status badge shows "running". Correctly uses toHaveText() with retry, stderr captured, clearTimeout called on resolve.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[is_alive called] --> B[Build GET request with auth + session headers]
    B --> C{req.send}
    C -- Ok(resp) --> D[store_session_id_from_response]
    D --> E[return true - Any HTTP status = reachable]
    C -- Err: connection refused / timeout / DNS --> F[return false - Server is dead]
    style E fill:#22c55e,color:#fff
    style F fill:#ef4444,color:#fff

Loading

_{Reviews (4): Last reviewed commit: "test(mcp): address greptile review feedb..." | Re-trigger Greptile}

[penso]

@greptile review

[penso]

@greptile review

[penso]

@greptile review