Repository navigation
fix(events): stand by instead of exiting when the event runtime socket is owned - #561
Conversation
…t is owned A generated MCP server exited before answering `initialize` whenever another process from the same plugin install already owned the event runtime socket (`Event runtime endpoint already has a live server.`), so every session after the first launched from one install root got no server at all. `createEventRuntimeServer` gains `whenOwned: 'fail' | 'standby'` (default `'fail'`, unchanged). Under `'standby'` a live owner is not fatal: the server resolves in the `standby` role and a scope-bound fiber probes the endpoint every ~1 s (jittered) and runs the existing `openServer` path the moment it is free; the claim lock lets exactly one standby win. `EventRuntimeServer` exposes `role()` and `onRoleChange()`. The generated runtime passes `'standby'` and announces both the standby start and the takeover on stderr only. Closes #559
🦋 Changeset detectedLatest commit: 212aa15 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 752879a963
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pid: process.pid, | ||
| startedAt, | ||
| }), | ||
| whenOwned: 'standby', |
There was a problem hiding this comment.
When two sessions run concurrently, this policy leaves session B's MCP requests on B's dispatcher and lineage registry, but every hook wrapper still connects to the single install-wide socket owned by session A. Consequently B's hook events render against A's module/provider state, B's pre-tool observations cannot correlate with tool calls handled by B's registry, and event-triggered notifications use A's MCP connection. This breaks the documented guarantee that hooks share state with tools in exactly the concurrent-session scenario being enabled; the new integration test only invokes a stateless ping tool and never sends a hook belonging to the standby session. The endpoint needs per-session routing or equivalent affinity rather than one active handler per install.
AGENTS.md reference: AGENTS.md:L78-L81
Useful? React with 👍 / 👎.
… guard role listeners Self-review findings on #561 (752879a): - close() now takes the endpoint claim before it stops listening and holds it through the identity-checked removal, so no standby can bind a new socket at the path — possibly on the inode the kernel just freed — before the old owner's identity check runs. Standbys wait out the claim (bounded retries). - The standby loop only swallows "still owned"; every other takeover failure keeps it standing by but is reported through the new `onStandbyError` option, which the generated runtime writes to stderr at one line per distinct message per 30 s. - Role listeners run in their own guards: a throwing listener is reported via `onStandbyError` and never stops the remaining listeners or the transition.
…ndby loop or skip role listeners Every onStandbyError invocation goes through one guarded helper that swallows a reporter throw, so the standby keeps probing and the remaining role listeners still run (second-pass self-review finding on 43a85a1).
Closes #559
Symptom
A generated stdio MCP server exits during startup — before answering
initialize— whenever another process from the same plugin install already hosts the event runtime socket. Hosts seeCONNECTION_CLOSED; the MCP log showsEventRuntimeTransportError: Event runtime endpoint already has a live server.Observed today with Claude Desktop (every Claude Code session shares one--plugin-dir, so three sessions'haulerservers died at start) and Cursor (imports Claude-installed plugins from the same cache root, held the endpoint, every new Claude session lost). Details in #559.Root cause
entry-shell.tsderives the event runtimeendpointIdfrom artifact epoch + target + plugin root, so every process launched from one install maps to one socket.events/ipc.tsopenServer/claimEndpointtreat a live server on that socket as fatal (runtime-failed), andmcp-server-runtime.tsstartEventRuntimeawaitscreateEventRuntimeServer, so the rejection propagates out ofcreateGeneratedRouteMcpServerand the generated entry. A per-install singleton that fails closed is the wrong contract: several concurrent sessions per host, and several hosts sharing one install, are the normal case.Fix — standby with takeover
packages/agent-bundle/src/events/ipc.tscreateEventRuntimeServergainswhenOwned?: 'fail' | 'standby'(EventRuntimeOwnedEndpointPolicy). The default'fail'is byte-for-byte the old behaviour: sameEventRuntimeTransportError, samecode/message/name.'standby', a live owner is not fatal. The server resolves in thestandbyrole with nothing bound, and a scope-bound fiber (standByForEndpoint,Effect.forkScoped) sleepsSTANDBY_PROBE_INTERVAL(1 s) plus 0–STANDBY_PROBE_JITTER_MS(250 ms) of jitter, probes the endpoint, and as soon as it is notliveruns the sameopenServerpath an unowned start uses — claim lock, identity-checked stale-socket removal, listen, release. Losing the race to another standby (the existing claim lock serializes claims) leaves it standing by for the next tick; only one wins.openServeris extended, not forked: the three sites that can see a live server (pre-claim probe, claim-loop probe, under-claim probe) now fail with a privateEventRuntimeEndpointOwnedErrormarker subclass, andlistenServermapsEADDRINUSE(the only owner signal Windows has) to the same marker, so the standby loop retries exactly "owned elsewhere" and nothing else. Permission errors, an unrecoverable claim, or a stale socket the owner cannot remove still fail closed under both policies.EventRuntimeServergainsrole(): 'owner' | 'standby'andonRoleChange(listener) => unsubscribe;endpointis unchanged. The service state is now{ endpoint, listener: Ref, roleListeners, standby: Fiber | undefined }; the bind and theRef.setare one uninterruptibleacquireRelease, andclose()interrupts and awaits the standby fiber before reading the listener, so aclose()racing a takeover still finds and removes the socket that takeover just bound. Every existing invariant is preserved — identity-checked lock/socket removal, uninterruptible claim/release pairing, the Linux recovery gate, epoch checks.close()takes the endpoint claim before it stops listening and holds it through the identity-checked removal (releaseOwnedEndpoint), so a standby cannot bind at the path while the old owner is still tearing down; standbys wait out the claim through their bounded retries.createEventRuntimeServeralso gainsonStandbyError?: (error: EventRuntimeTransportError) => void: every failure the standby loop recovers from by retrying, other than "still owned" (probe/claim/stale-removal/listen/release failures, and a role listener that threw asEvent runtime role listener failed.). Role listeners each run in their own guard.packages/agent-bundle/src/mcp-server-runtime.tsstartEventRuntimepasseswhenOwned: 'standby', and writes to stderr only (stdout is the protocol stream):agent-bundle event runtime: <endpoint> is owned by another process; standing byat start, andagent-bundle event runtime: <endpoint> was released by its owner; took it overon takeover.onStandbyErroris wired tocreateStandbyErrorReporter:agent-bundle event runtime: <endpoint> takeover failed, still standing by: <message>, one line per distinct message, repeated at most every 30 s.Not changed on purpose:
roleis not added to the status payload. Only the owner has a socket to answer on, so a status response's role is alwaysowner— the field would carry no information — andeventRuntimeStatusPayloadSchemais.strict()and parsed byinstall/doctor.ts, which runs from a different install ofagent-bundlethan the server it probes; an additive field would make an older doctor reject a newer server's status.adapters/hook-contract.ts) are untouched: with an owner alive they are served as before; during the ≤ ~1 s takeover gap they seeruntime-unavailableand the documentedfallback: 'standalone'applies.Test plan
Unit —
packages/agent-bundle/tests/event-ipc.test.ts(the existing ipc file, samecreateEventRuntimeServerForTestpatterns):role() === 'standby',requestEventRuntimeanswered by the first, first closes,onRoleChange('owner')fires within 2 s, second serves requests, second closes → socket and lock gonebeforeEndpointRemovalpauses the owner after it stopped listening (path already unlinked) and before its identity check; the standby's attempt inside that window is held off by the claim (reported asclaim did not clear), the role staysstandby, and after the resume the standby takes over with its socket intactafterEndpointProbethrows once on the first free probe;onStandbyErrorreceives the typed error and the next attempt takes overUnit —
packages/agent-bundle/tests/mcp-server-runtime.test.ts:createStandbyErrorReporterreports each distinct takeover failure once, and again only after the repeat intervalprocess.stderr.write)Integration —
packages/agent-bundle/tests/generated-route-server.test.ts(boots real stdio children from a built artifact;connectGeneratedServerrefactored onto a newconnectGeneratedEntryhelper that keeps the child's stderr):initialize+tools/listconcurrently; the second's stderr contains the standby line; the first servesping; after the first exits the second logs the takeover, owns the socket (requestEventRuntimeStatuspid matches), and servesping; closing it removes the socket and lock.origin/main: withorigin/main'sevents/ipc.ts+mcp-server-runtime.tsand this test, the second child dies withEventRuntimeTransportError: Event runtime endpoint already has a live server.(code: 'runtime-failed') →Generated route server failed to connecton all three attempts; passes with this branch.Gates on the final tree:
pnpm typecheck✓ ·pnpm lint✓ (0 errors, 0 warnings) ·pnpm test:unit✓ (3370 passed) ·pnpm test:projection✓ (171 passed) ·node scripts/run-packed-tests.mjs✓ (27 passed, 10 files) ·pnpm docs:site:build✓ (dead-link and language-parity checks pass). No new module was added, so there is nothing to check for a missing production importer.CI: every check green.
Verify (Node 24)failed once onpackages/workbench/tests/mcp-app-real.e2e.test.ts› "runs a generated SDK-v2 App …" (page.waitForRequesttimeout on the/api/mcp/apps/*/close+DELETE /api/mcp/sessions/<id>requests after clicking Close MCP session — the known Workbench MCP session-close timing race, no event runtime involved); the failed jobs were rerun once and passed.Docs
website/docs/en/guide/authoring/hooks.mdxandwebsite/docs/zh/guide/authoring/hooks.mdx: new Concurrent sessions paragraph in the event runtime section — one owner per install root, others stand by and take over, the two stderr lines, the claim lock picking one winner, and hooks'standalonefallback during the gap. (docs/entry-conventions.mddoes not describe the entry's event wiring, so it is unchanged.).changeset/event-runtime-standby.md—agent-bundle: patch.Self-review
Reviewer:
gpt-5.6-sol-medium(change-risk-reviewer) against 752879a. Three findings, all fixed in 43a85a1:close()now acquires the endpoint claim before it stops listening and holds it through the identity-checkedstat+rm(releaseOwnedEndpoint), so takeover and teardown are serialized by the same lock the takeover path uses and no successor can bind at the path — possibly on the inode the kernel just freed — before the old owner's identity check runs. (Note on the literal window: Node/libuv unlinks the socket path when the server closes, so the old owner'sstatnever sees its own inode; the real hazard was a successor binding before the identity check and inode reuse defeating it — the claim closes both.) Test: deterministic interleaving via the existingbeforeEndpointRemovalhook; fails on 752879a withexpected 'owner' to be 'standby'.EventRuntimeEndpointOwnedError; every other failure keeps it standing by but is reported through the newonStandbyErroroption, which the generated runtime writes to stderr rate-limited to one line per distinct message per 30 s (createStandbyErrorReporter). Tests:afterEndpointProbethrowing once → callback fires, next attempt takes over (fails on 752879a:expected [] to have a length of 1); reporter rate-limit unit test and stderr wiring test (fail on 752879a: export missing / two lines instead of three).notifyRoleListeners); a throw is reported viaonStandbyErrorasEvent runtime role listener failed.and never stops the remaining listeners or the transition. Test: throwing listener registered before a recording one → the recording one still receives'owner'(fails on 752879a:Timed out after 5000ms).Docs: en/zh Concurrent sessions paragraph gained the sentence that takeover failures other than "still owned" are reported on the server's stderr. Changeset unchanged (behavioural summary only).
Second pass
Reviewer:
gpt-5.6-sol-medium(change-risk-reviewer) against 43a85a1. One finding (Warning), fixed in 78c76ba:onStandbyErrorwas invoked unguarded from the standby loop and from the throwing-role-listener handler, so a reporter that threw could end the standby fiber or skip later listeners. Every invocation now goes through one non-throwing helper (guardedStandbyErrorReporter) that swallows a reporter throw; both paths use it. Tests: reporter that throws on the first non-owned failure (afterEndpointProbethrow-once) → standby still takes over on the next attempt; throwing listener + throwing reporter → a later recording listener still receives'owner'. Both fail on 43a85a1 withTimed out after 5000msand pass after.Cleared without change: close latency bounded (~990 ms + I/O);
uninterruptibleMaskpreserves claim release; race test has explicit 10/30 s bounds; rate-limit keys are static messages;createStandbyErrorReporteris not package-exported, so no changeset addition.