fix: prevent "Server not initialized" race on first request - #198
Conversation
📝 WalkthroughMarks Streamable HTTP sessions as initialized as soon as WalkthroughIn the 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches✨ Simplify code
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. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/anubis/server/session.ex (1)
669-675: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winP1:
initialized: truenow opens the gate beforemodule.init/2runs.Lines 674-675 let the first post-
initializerequest/notification through, but the server setup callback still only runs inhandle_notification("notifications/initialized")on Lines 891-915. That means the race fixed by this PR can now route work throughhandle_request/2with a pre-init frame. The auto-init path already avoids this by callingmaybe_call_init/3before serving traffic, so the normal init path should do the same (or split “accept requests” from “server callback completed” into separate flags) to avoid this new boss fight.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3a2f2d3c-143a-4bdf-9f18-6cf950b951ea
📒 Files selected for processing (2)
lib/anubis/server/session.extest/anubis/server/session_test.exs
🚀 Want to release this? --- ## [1.7.0](v1.6.2...v1.7.0) (2026-07-16) ### Features * **streamable_http:** per-subscriber metadata and targeted sends ([#218](#218)) ([c606658](c606658)) ### Bug Fixes * Forward configured :headers on the DELETE session-teardown request (follow-up to [#180](#180)) ([#213](#213)) ([b32134a](b32134a)) * prevent "Server not initialized" race on first request ([#198](#198)) ([e84624c](e84624c)) * **server:** resolve session names via Registry to prevent atom-exhaustion DoS ([#188](#188)) ([17e4a6d](17e4a6d)) * **session:** trap_exit so terminate/2 runs on supervisor shutdown ([#209](#209)) ([6335cf4](6335cf4)) * **streamable_http:** don't close superseded SSE handler to prevent reconnect flap ([#215](#215)) ([a1e0ce6](a1e0ce6)) ### Continuous Integration * add new elixir versions ([3b636a8](3b636a8)) * add pr-quality workflow ([c0ca08f](c0ca08f)) * fix zig correct version for burrito ([6e410bd](6e410bd)) * use mlugg/setup-zig 0.15.2 in release-please auto build job ([2ed6187](2ed6187)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
## Problem On Streamable HTTP, the client sends `notifications/initialized` and its first request (e.g. `tools/list`) as two separate, near-simultaneous HTTP requests. The client sends them in order, but the transport doesn't guarantee order, so `tools/list` sometimes lands before the notification. When it does, the session rejects it with `"Server not initialized"` ([gate](https://github.com/zoedsoupe/anubis-mcp/blob/main/lib/anubis/server/session.ex#L603-L605), [error](https://github.com/zoedsoupe/anubis-mcp/blob/main/lib/anubis/server/session.ex#L635-L645)). A client that doesn't retry then stalls. In my case, the Claude Desktop client hangs ~30s, then sends `notifications/cancelled`. The spec lets a client send requests as soon as the server responds to `initialize`, so this is stricter than the spec requires. ## Solution Mark the session initialized on the `initialize` response, not only in the `notifications/initialized` handler. Adds a test for a request arriving after `initialize` but before the notification. ## Rationale Matches the official SDKs, which mark the session ready on the `initialize` response: - TypeScript ([streamableHttp.ts#L501](https://github.com/modelcontextprotocol/typescript-sdk/blob/caa25503cdfc449d116c204e866bccc2617d7037/src/server/streamableHttp.ts#L501)) - Python ([#1478](modelcontextprotocol/python-sdk#1478)) - Rust ([#788](modelcontextprotocol/rust-sdk#788)) The spec says **SHOULD NOT**, not **MUST NOT**, so the notification isn't a hard gate.
🚀 Want to release this? --- ## [1.7.0](v1.6.2...v1.7.0) (2026-07-16) ### Features * **streamable_http:** per-subscriber metadata and targeted sends ([#218](#218)) ([d6cf7b1](d6cf7b1)) ### Bug Fixes * Forward configured :headers on the DELETE session-teardown request (follow-up to [#180](#180)) ([#213](#213)) ([4edd2c0](4edd2c0)) * prevent "Server not initialized" race on first request ([#198](#198)) ([6bb60f9](6bb60f9)) * **server:** resolve session names via Registry to prevent atom-exhaustion DoS ([#188](#188)) ([fdbc238](fdbc238)) * **session:** trap_exit so terminate/2 runs on supervisor shutdown ([#209](#209)) ([a224c00](a224c00)) * **streamable_http:** don't close superseded SSE handler to prevent reconnect flap ([#215](#215)) ([e1cc4a8](e1cc4a8)) ### Continuous Integration * add new elixir versions ([26dd267](26dd267)) * add pr-quality workflow ([d67eaa9](d67eaa9)) * fix zig correct version for burrito ([afec768](afec768)) * use mlugg/setup-zig 0.15.2 in release-please auto build job ([428cace](428cace)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Problem
On Streamable HTTP, the client sends
notifications/initializedand its first request (e.g.tools/list) as two separate, near-simultaneous HTTP requests. The client sends them in order, but the transport doesn't guarantee order, sotools/listsometimes lands before the notification.When it does, the session rejects it with
"Server not initialized"(gate, error). A client that doesn't retry then stalls. In my case, the Claude Desktop client hangs ~30s, then sendsnotifications/cancelled.The spec lets a client send requests as soon as the server responds to
initialize, so this is stricter than the spec requires.Solution
Mark the session initialized on the
initializeresponse, not only in thenotifications/initializedhandler. Adds a test for a request arriving afterinitializebut before the notification.Rationale
Matches the official SDKs, which mark the session ready on the
initializeresponse:The spec says SHOULD NOT, not MUST NOT, so the notification isn't a hard gate.