Skip to content

fix(mcp): advertise a relative SSE endpoint so clients work behind the /server proxy - #1855

Merged
jokob-sk merged 1 commit into
netalertx:mainfrom
frdmn:fix/mcp-sse-relative-endpoint
Oct 23, 2026
Merged

jokob-sk merged 1 commit into
netalertx:mainfrom
frdmn:fix/mcp-sse-relative-endpoint

Conversation

@frdmn

@frdmn frdmn commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

📌 Description

The MCP SSE stream advertises its message endpoint as the absolute path /mcp/messages?session_id=.... MCP clients resolve that against the origin, so a client connected through the bundled nginx proxy (http://<host>:20211/server/mcp/sse) posts to /mcp/messages instead of /server/mcp/messages. nginx has no route for that path, falls through to static file serving (open() "/app/front/mcp/messages" failed), returns 404, and the client never finishes connecting.

This change advertises a relative path (messages?session_id=...) instead. Clients resolve it against whichever SSE URL they used:

Connected via Resolves to (before) Resolves to (after)
:20212/mcp/sse (direct) /mcp/messages ✅ /mcp/messages ✅
:20211/server/mcp/sse (proxy) /mcp/messages ❌ 404 /server/mcp/messages ✅

No routes, nginx config or settings change.


🔍 Related Issues

Fixes #1854


📋 Type of Change

  • 🐛 Bug fix
  • ✨ New feature
  • ♻️ Code refactor
  • 📚 Documentation update
  • 🧪 Test addition or change
  • 🔧 Build/config update
  • 🚀 Performance improvement
  • 🔨 CI/CD or automation
  • 🧹 Cleanup / chore

📷 Screenshots or Logs (if applicable)

New test before the fix:

FAILED test/api_endpoints/test_mcp_sse_endpoint.py::test_sse_endpoint_resolves_to_messages_route[http://host:20211/server/mcp/sse-/server/mcp/messages]
1 failed, 1 passed

After the fix:

2 passed

🧪 Testing Steps

  • Added test/api_endpoints/test_mcp_sse_endpoint.py. It opens /mcp/sse, reads the endpoint event and resolves it with urljoin (as the Python MCP SDK does) against both a direct and a /server-prefixed SSE URL. Wrote it first and confirmed the proxy case failed, then applied the fix.
  • Full suite in the devcontainer (pytest -m 'not (docker or compose or feature_complete)'): 1738 passed, 30 skipped, 0 failed. test/docker_tests was excluded locally because building the image through the Docker socket fails on Docker Desktop for macOS (build context not visible to the host daemon); unrelated to this change.
  • flake8 with the CI settings passes on the changed files.
  • Manually against a production instance: POST to /server/mcp/messages?session_id=... through port 20211 returns 202, the unprefixed path returns 404.

✅ Checklist

  • I have read the Contribution Guidelines
  • I have tested my changes locally
  • I have updated relevant documentation (if applicable)
  • I have verified my changes do not break existing behavior
  • If this PR adds/modifies a plugin, it passes the Conventions Checklist
  • I am willing to respond to requested changes and feedback

🙋 Additional Notes

  • Spec-compliant clients are unaffected for direct connections: the Python SDK uses urljoin(url, sse.data) and the TypeScript SDK uses new URL(data, this._url), both of which resolve relative paths. Both SDKs' same-origin checks still pass, since a relative path can't change the origin.
  • Only a client that builds the URL as origin + data (not per spec) would be affected; such a client already can't work behind the proxy.
  • No docs change needed: the docs reference the /mcp/messages route, which is unchanged, and don't describe the event payload.

Summary by CodeRabbit

  • Bug Fixes

    • Corrected the SSE endpoint advertisement so clients can resolve the messages route correctly across supported deployment paths.
  • Tests

    • Added coverage for successful SSE responses, endpoint URL resolution, and required session identifiers.

The SSE endpoint event advertised the absolute path /mcp/messages, which drops the /server prefix when clients connect through the bundled nginx proxy on port 20211. Clients then POST to /mcp/messages, nginx serves it as a static file and returns 404, so MCP clients can't connect via the proxy. A relative path resolves against whichever SSE URL the client used, so it works both directly and behind the prefix.
@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: netalertx/NetAlertX/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 517a4124-6c33-4ea1-a47d-7e7ab78747a5
📥 Commits

Reviewing files that changed from the base of the PR and between 32e2ddd and 68aaeaf.

📒 Files selected for processing: 2
  • server/api_server/mcp_endpoint.py
  • test/api_endpoints/test_mcp_sse_endpoint.py

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Walkthrough

The SSE endpoint now advertises a relative messages URL. New tests check resolution for direct and /server-prefixed deployments and require a session_id query parameter.

Changes: MCP SSE endpoint URL resolution

Layer / File(s) Summary
Advertise and verify the relative messages URL
server/api_server/mcp_endpoint.py, test/api_endpoints/test_mcp_sse_endpoint.py
The SSE endpoint emits a relative messages URL. Tests read the first endpoint event and check that its URL resolves to the expected messages route for direct and /server-prefixed paths, with a session_id query parameter.

Suggested reviewers: jokob-sk

Priority: ➖ Normal

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 68aae

No concrete merge-blocking risk remains; the advertised endpoint works for both supported deployment paths.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: advertising a relative MCP SSE endpoint to support clients behind the /server proxy.
Linked Issues check Passed The PR satisfies the coding requirements in [#1854]. server/api_server/mcp_endpoint.py now emits messages?session_id=... as a relative endpoint. URL resolution preserves /mcp/messages for direct…
Out of Scope Changes check Passed The changes stay within [#1854]. The PR changes only the MCP SSE endpoint payload and adds focused tests for direct and /server-prefixed URL resolution. No unrelated route, nginx, setting, or featur…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files.
Full details: Linked Issues check

Explanation

The PR satisfies the coding requirements in [#1854]. server/api_server/mcp_endpoint.py now emits messages?session_id=... as a relative endpoint. URL resolution preserves /mcp/messages for direct connections and produces /server/mcp/messages for the bundled proxy path. The new test covers both resolutions and requires a session_id query parameter.

Full details: Out of Scope Changes check

Explanation

The changes stay within [#1854]. The PR changes only the MCP SSE endpoint payload and adds focused tests for direct and /server-prefixed URL resolution. No unrelated route, nginx, setting, or feature changes are present.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • 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.

@jokob-sk
jokob-sk merged commit 7e8f24a into netalertx:main Oct 23, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] MCP SSE endpoint event omits the /server prefix, so clients behind the built-in nginx proxy get a 404 on /mcp/messages

2 participants