Skip to content

fix(events): cancel orphaned IPC renders, live-socket ownership, streaming UTF-8, ms timeout precision, fresh diagnostic - #209

Merged
ScriptedAlchemy merged 2 commits into
mainfrom
fix/pr180-ipc
Sep 1, 2026
Merged

fix(events): cancel orphaned IPC renders, live-socket ownership, streaming UTF-8, ms timeout precision, fresh diagnostic#209
ScriptedAlchemy merged 2 commits into
mainfrom
fix/pr180-ipc

Conversation

@ScriptedAlchemy

Copy link
Copy Markdown
Owner

Summary

Fixes the five event-IPC findings from #180 review:

  • ipc.ts:186 (P1): the per-connection handler now receives an AbortSignal tied to socket teardown (close/end/error), and the connection fiber itself is interrupted — a client disconnect or timeout cancels the in-flight shared render instead of letting it run to completion invisibly. mcp-server-runtime threads the signal into dispatch.
  • ipc.ts:223 (P1): startup no longer unconditionally unlinks the endpoint. It probes the existing socket first: a live server fails startup with a typed error, only a stale (ECONNREFUSED) file is removed. Shutdown removes the endpoint only when this instance still owns it (device/inode identity check), so a replacement bound later is never deleted.
  • ipc.ts:117 (P2): request decoding uses a per-connection StringDecoder with byte-accurate limit accounting and newline-delimited framing — multibyte code points split across chunks decode correctly.
  • normalize.ts:447 (P2): the canonical hook model now carries timeoutMs end-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.
  • validate.ts:1722 (P2): the missing-shared-runtime condition gets fresh code AB4817 (AB4816 stays retired per the no-reuse rule); docs/diagnostics.md registry updated.

Test plan

  • New transport regressions: disconnect interrupts the in-flight handler; split-multibyte decode; live-server rejection without disturbing the owner; stale-socket replacement (ported to the effect-rstest harness from test(effect): adopt effect-rstest helpers #192)
  • Timeout precision fixture (1250 ms preserved in the model, host doc projects ceil-seconds); AB4817 assertions
  • Scoped suites (event-ipc, hooks, route-graph, target-hook-contract, cursor-adapter), pnpm typecheck, pnpm lint all green after rebase onto current main

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-bot

changeset-bot Bot commented Sep 1, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: ea7b0b4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
agent-bundle Patch

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

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-01T21:11:40.277742Z ea7b0b4 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@ScriptedAlchemy
ScriptedAlchemy merged commit c69c6b5 into main Sep 1, 2026
9 checks passed
@ScriptedAlchemy
ScriptedAlchemy deleted the fix/pr180-ipc branch September 1, 2026 21:10

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

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.

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.

1 participant