Skip to content

Make MCP resource subscriptions reliable across HTTP workers - #2436

Open
kylebernhardy wants to merge 13 commits into
mainfrom
codex/fix-mcp-cross-worker-subscriptions-1751
Open

Make MCP resource subscriptions reliable across HTTP workers#2436
kylebernhardy wants to merge 13 commits into
mainfrom
codex/fix-mcp-cross-worker-subscriptions-1751

Conversation

@kylebernhardy

@kylebernhardy kylebernhardy commented Sep 1, 2026

Copy link
Copy Markdown
Member

MCP resource subscriptions now follow the session's active GET-SSE stream across HTTP workers through a fenced owner claim, so resources/subscribe and resources/unsubscribe no longer depend on both requests landing on the same worker. The routing path keeps the full local principal, projects only the fields needed across ITC, and rejects stale ownership commands. Storage-native serialization prevents concurrent subscription changes from losing durable updates.

Fixes #1751.

For the human reviewer

  1. The active GET worker and a random stream token are persisted on the non-replicating MCP session row when the SSE stream registers its owner. POST workers route directly to that owner, and the live token rejects stale commands. When ITC is unavailable, the owner claim is cleared and only the local unwired worker may use its registry entry.
  2. Routed operations have an ambiguous outcome at the 30-second timeout boundary, so the JSON-RPC response asks the client to retry. Exactly-once operation IDs would add durable protocol state for a rare outcome and can be added later without changing successful requests.
  3. Durable subscription mutations use Harper's native per-record-store lock across workers rather than a persisted CAS/version field. The acquire is bounded and failures become retryable JSON-RPC errors. This keeps the session schema and protocol unchanged across RocksDB and LMDB.

Verification

  • npm run build — passed.
  • Focused MCP unit suite (session, subscriptionRouting, subscriptions, transport) — 129 passing.
  • Changed-file lint, Prettier, and git diff --check — passed.
  • Final independent Claude review — no remaining code findings.
  • Full repository lint currently reports 14 unrelated pre-existing warnings outside the changed files.
  • The local MCP integration harness cannot start because this macOS host lacks the required 127.0.0.2127.0.0.32 loopback aliases. The cross-worker assertion passed in the Node integration shard and remains fail-closed there. Bun skips this socket-placement proof because all pinned probes are assigned to one HTTP worker.

Comment generated by kAIle (GPT-5.6)

Complexity: complicated

Review-Coverage: authored=codex; ran=claude; declined=gemini,cursor-grok,cursor-composer,domain; rounds=12 @ 5c60fea

Human-Review-Need: 3 @ 5c60fea

@kylebernhardy kylebernhardy added this to the v5.2 milestone Sep 1, 2026
@github-actions

This comment has been minimized.

gemini-code-assist[bot]

This comment was marked as resolved.

@kylebernhardy

Copy link
Copy Markdown
Member Author

Resolved the v5.2 cherry-pick conflict on cherry-pick/v5.2/pr-2436 at 28feda035. The cherry-pick workflow re-dispatch regenerated the markers instead of testing the resolved branch, so I restored the resolution and dispatched the underlying release gates directly:

Comment generated by kAIle (GPT-5.6 Sol)

@kylebernhardy
kylebernhardy marked this pull request as ready for review September 1, 2026 16:39
Comment thread components/mcp/session.ts
@claude

This comment has been minimized.

Comment thread components/mcp/session.ts

@kriszyp kriszyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm skeptical we want to try to do this with cross-thread communication rather than persisting enough state to be able to pick up the subscription from any thread. Or alternately, we could look at establishing sufficient session affinity in symphony to generally route to the same thread. WDYT?
🤖 Reviewed with Claude

Comment thread components/mcp/session.ts
Comment thread components/mcp/subscriptionRouting.ts
Comment thread components/mcp/transport.ts Outdated
Comment thread components/mcp/transport.ts Outdated
Comment thread components/mcp/subscriptionRouting.ts Outdated
Comment thread integrationTests/mcp/sse-listchanged.test.ts Outdated
Comment thread components/mcp/subscriptionRouting.ts Outdated
@kylebernhardy

Copy link
Copy Markdown
Member Author

@kriszyp I traced the persistence-first option. A worker can watch durable subscription state, but a simple state update leaves a gap: POST may return before the GET owner installs the live iterator, and custom Resource.subscribe streams are not necessarily replayable. Closing that gap appears to require an applied-revision acknowledgment, owner fencing/lease, and principal handling. Watching mcp_session would also audit every lastActivity update, so a dedicated subscription table seems cleaner if we take that route.

Bounded ITC looks smaller for this issue, while a durable state controller may be stronger long term. What persisted-state flow did you have in mind? More detail would help guide the implementation.

Comment generated by kAIle (GPT-5.6)

@kylebernhardy

Copy link
Copy Markdown
Member Author

@kriszyp I addressed the review threads, including the bounded lock, ownership races, local/remote authorization behavior, and fail-closed cross-worker coverage. The final Claude pass found no remaining code issues. When you have a chance, could you take another look?

Comment generated by kAIle (GPT-5.6)

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.

MCP resources/subscribe fails to establish 88% of the time under default multi-worker deployment

2 participants