fix(events): cancel orphaned IPC renders, live-socket ownership, streaming UTF-8, ms timeout precision, fresh diagnostic - #209
Conversation
Tie event handlers to socket teardown, decode streaming UTF-8 safely, retain millisecond deadlines until host projection, and replace the retired diagnostic code.
Probe Unix event endpoints before stale cleanup and guard shutdown unlinking by the listener's socket identity so concurrent runtimes cannot replace each other.
🦋 Changeset detectedLatest commit: ea7b0b4 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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea7b0b4c04
ℹ️ 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".
| )); | ||
| } | ||
| if (endpointState === 'stale') { | ||
| yield* liftPromise(() => rm(endpoint)).pipe( |
There was a problem hiding this comment.
Recheck stale endpoint ownership before unlinking
When two runtime processes start concurrently while a stale endpoint exists, both can observe endpointState === 'stale'; after the first process removes the stale file and successfully binds, the second can execute this unconditional rm(endpoint) and unlink the first process's live socket. The first server remains running but becomes unreachable to new hook clients, so stale cleanup must be tied to the probed file identity or otherwise serialized/rechecked immediately before removal.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in #216. Stale-endpoint cleanup is now performed only while holding an exclusive endpoint claim (<endpoint>.lock created with open('wx')), with a fresh probe re-run under the lock: a re-probed live endpoint fails with the existing live-server error instead of being removed, so a concurrent winner's socket can never be unlinked. A held claim is awaited with bounded retries, and an orphaned claim from a crashed process fails closed with a typed error rather than being stolen (stealing would recreate the same unlink race). Claim release is dev/inode identity-checked, mirroring removeOwnedEndpoint. Deterministic regression test freezes two servers in the historical probe window and asserts the loser errors while the winner keeps serving on its original socket.
Summary
Fixes the five event-IPC findings from #180 review:
mcp-server-runtimethreads the signal into dispatch.StringDecoderwith byte-accurate limit accounting and newline-delimited framing — multibyte code points split across chunks decode correctly.timeoutMsend-to-end; conversion to host seconds happens only at native host projection (docs/tests updated). Sub-second event-route timeouts survive to the generated wrapper.AB4817(AB4816 stays retired per the no-reuse rule); docs/diagnostics.md registry updated.Test plan
pnpm typecheck,pnpm lintall green after rebase onto current main