Conversation
…ions The hosted kit.openiap.dev/mcp endpoint keeps StreamableHTTP sessions in a per-process Map, so with more than one Fly machine behind the proxy a valid mcp-session-id was rejected with 400 "initialize first" whenever the request landed on a sibling machine (~65% of calls in the issue repro, consistent with 3-machine round-robin). - Prefix session ids with FLY_MACHINE_ID and answer requests for a foreign machine's session with a fly-replay header so Fly's proxy re-routes them to the owner. No shared store needed; the transport object holds live SSE state and cannot be serialized anyway. - Never replay twice (fly-replay-src guard) and never replay to self, so stale machine ids after a deploy cannot loop. - Answer 404 (-32001 Session not found) instead of 400 for a session this process genuinely cannot serve — the MCP spec makes clients transparently re-initialize on 404, so restarts now self-heal. - Add packages/mcp-server/** to deploy-kit.yml triggers: kit's Fly binary imports the MCP handler from source, so MCP fixes previously merged without ever deploying. Also run the MCP server's own vitest suite in the verify job — no CI ran it before. Fixes #287 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe MCP server now supports Fly machine-affinity session IDs. Unknown sessions can replay requests to their owning machine. Unavailable sessions return 404 responses, while invalid requests retain their existing 400 responses. CI verifies the MCP server package. ChangesMCP session routing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MCPClient
participant MCPHandler
participant routeUnknownSession
participant OwningMachine
MCPClient->>MCPHandler: Send request with machine-prefixed session ID
MCPHandler->>routeUnknownSession: Resolve unknown session
routeUnknownSession-->>MCPHandler: Return replay target or not-found
MCPHandler->>OwningMachine: Request Fly replay when target exists
OwningMachine-->>MCPClient: Process request or return 404
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/deploy-kit.yml:
- Around line 19-22: Update the pull_request.paths configuration in
deploy-kit.yml to mirror the push path filters by adding bun.lock, package.json,
and .github/workflows/deploy-kit.yml, while preserving the existing packages/kit
and packages/mcp-server entries.
In `@packages/mcp-server/src/http.ts`:
- Line 87: Normalize the configured machine ID before storing it in both
handlers: update the machine ID assignment in packages/mcp-server/src/http.ts at
lines 87-87 and packages/mcp-server/src/web.ts at lines 48-48 to apply the
existing normalizer to options.machineId while preserving currentMachineId()
fallback behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 44dc84e2-ea4c-4263-adae-60e77d660fbc
📒 Files selected for processing (7)
.github/workflows/deploy-kit.ymlpackages/mcp-server/src/http.tspackages/mcp-server/src/session-routing.tspackages/mcp-server/src/web.tspackages/mcp-server/test/http.test.tspackages/mcp-server/test/session-routing.test.tspackages/mcp-server/test/web.test.ts
| pull_request: | ||
| paths: | ||
| - "packages/kit/**" | ||
| - "packages/mcp-server/**" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Mirror the dependency filters for pull requests.
The pull_request.paths list omits bun.lock, package.json, and .github/workflows/deploy-kit.yml. A pull request that changes only one of these files skips this workflow, including the MCP server suite. Add the same dependency and workflow paths used by push.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/deploy-kit.yml around lines 19 - 22, Update the
pull_request.paths configuration in deploy-kit.yml to mirror the push path
filters by adding bun.lock, package.json, and .github/workflows/deploy-kit.yml,
while preserving the existing packages/kit and packages/mcp-server entries.
| const allowedOrigins = | ||
| options.allowedOrigins ?? | ||
| parseAllowedOrigins(process.env.IAPKIT_MCP_ALLOWED_ORIGINS); | ||
| const machineId = options.machineId ?? currentMachineId(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Validate configured machine IDs in both handlers.
Both handlers accept an explicit machineId without applying the validation used for FLY_MACHINE_ID. A value containing . creates an ID that resolves to a different replay target.
packages/mcp-server/src/http.ts#L87-L87: normalizeoptions.machineIdbefore storing it.packages/mcp-server/src/web.ts#L48-L48: use the same normalizer before storingoptions.machineId.
📍 Affects 2 files
packages/mcp-server/src/http.ts#L87-L87(this comment)packages/mcp-server/src/web.ts#L48-L48
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/mcp-server/src/http.ts` at line 87, Normalize the configured machine
ID before storing it in both handlers: update the machine ID assignment in
packages/mcp-server/src/http.ts at lines 87-87 and
packages/mcp-server/src/web.ts at lines 48-48 to apply the existing normalizer
to options.machineId while preserving currentMachineId() fallback behavior.
1 participant
Summary
Fixes #287 —
kit.openiap.dev/mcpintermittently rejected a valid, freshly issuedmcp-session-idwith400 "initialize first"(13/20 failures in the issue repro).Root cause (verified):
createIapKitWebMcpHandlerkeeps StreamableHTTP transports in a per-processMap(web.ts).fly.tomlsetsmin_machines_running = 1, which is a floor, not a cap — with N machines round-robining behind the Fly proxy, only the machine that servedinitializerecognizes the session. A ~35% success rate matches N=3 exactly. The SDK registers the session before the initialize response is sent, and no code path evicts sessions, so instance split-brain is the only mechanism consistent with the interleaved 200/400 pattern.Fix
<FLY_MACHINE_ID>.<uuid>. A request for a session owned by a sibling machine is answered with afly-replay: instance=<id>header, so Fly's proxy re-sends the original request to the owner. No shared store or infra change; a live SSE transport can't be serialized into Redis anyway.fly-replay-src, never replays to itself, and validates the attacker-controlled prefix (^[A-Za-z0-9]{1,32}$) before echoing it into a response header.FLY_MACHINE_ID), ids stay plain UUIDs and behavior is unchanged.-32001 Session not found). The old 400 broke that recovery path, which is why clients hard-failed instead of self-healing after machine restarts/deploys.deploy-kit.ymldid not trigger onpackages/mcp-server/**, yet kit's Fly binary imports the MCP handler from source. An MCP-only fix would merge and silently never ship. Paths added, and the verify job now also runs the MCP server's own vitest suite (previously it ran in no CI at all).Both entry points are covered: the production web handler (
web.ts, mounted by kit's Hono server) and the standalone Node server (http.ts).Tests
packages/mcp-server: 44 tests pass (bun run lint+bun run test) — new suites for the routing rules and for the web handler end-to-end (prefixed ids, replay, replay-once guard, self-restart 404, off-Fly 404, GET/DELETE parity, unchanged 400s for missing-session requests).packages/kit: 913 tests pass (kit mounts this handler inserver/mcp.ts).Post-merge verification
After deploy, rerun the issue's repro (20×
tools/liston one session) — expect 20/20, with cross-machine hops served via replay.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests