fix(codex): accept hook event names newer than the pinned protocol #35885
main fromrefs/pull/35885/head·merged by juliusmarminge View on GitHubCodex 0.166+ sends hook/started and hook/completed notifications for preModelRequest and postModelRequest hooks. The generated Codex bindings (pinned to an older protocol ref) closed HookEventName over 12 literal names, so these notifications failed to decode and client.ts dropped them. CodexAdapterV2 never saw the stopped run, and the hook's stop reason was lost. The fix adds HookEventName: { type: "string" } to the generator's DefinitionOverrides, next to PlanType, which is already open for the same reason. Regenerating from the same pinned ref (687a119f) turns the five closed *__HookEventName lists in schema.gen.ts into Schema.String and changes nothing else. The generator also added a header comment that the checked-in files lack; it was removed so meta.gen.ts and namespaces.gen.ts stay unchanged. The only consumer is CodexAdapterV2, which puts eventName into a stop-reason string and works the same with any string. No new regex or parser.
Fixes pingdotgg/t3code#35853
Reproduction on main#
On origin/main 95cb068c10 (Node v26.12.1), after vp i in a fresh worktree, I ran the reporter's probe as an ES module from packages/effect-codex-app-server. It uses makeInMemoryStdio plus Client.make, registers a hook/completed handler and an unknown-notification barrier, then sends hook/completed frames for eventName preCompact (control), preModelRequest and postModelRequest. Each frame has a stopped run with a stop entry, followed by a barrier frame. Output: Delivered hook events: ["preCompact"], so both model-request hook notifications were dropped. I then added a schema test to packages/effect-codex-app-server/src/schema.test.ts that checks V2HookStartedNotification and V2HookCompletedNotification against both new event names. vp test run packages/effect-codex-app-server/src/schema.test.ts fails on main (1 failed, 5 passed); the same fixture with eventName preCompact passes. No live Codex process or UI was exercised.
Evidence#
Send preCompact (control), preModelRequest and postModelRequest through the real Codex client. Each hook/completed frame contains a stopped run and the stop reason Checkpoint required before continuing., followed by an interrupted terminal notification. The base loses both model-request reasons; the head preserves them. The control works on both.
Environment on both sides: Linux, Node v26.12.1, Chromium 151, light theme, 1440×900 viewport, identically cropped to the 1120×900 conversation area. Real servers and web clients with synthetic CLI-created projects and a controlled Codex transport.
| Before | After |
|---|---|
| 95cb068c10 | 729fed007f |
![]() | ![]() |
| Before recording | After recording |
| Base runtime transcript | Head runtime transcript |
Protocol fixture · CLI driver · Reproduction instructions
The scenario replay provider uses ACP and bypasses this Codex decoder, so this proof used the manual path. No live Codex account or actual model request was exercised.
Not shown:
- Actual Codex hook execution and live model requests were not exercised; controlled protocol frames reproduced the reported decoder failure through the real app.
- hook/started and arbitrary future hook event names were not exercised; this proof covers hook/completed for preModelRequest and postModelRequest.
- Desktop and mobile clients were not exercised.
Tests#
Before the fix: vp test run packages/effect-codex-app-server/src/schema.test.ts failed 1 of 6 (the repro test), reproducing the bug. After the fix: the same command passed 6 of 6. vp run check:changed passed: format, lint, knip, typecheck of 4 packages, and tests in 2 files (8 passed). No live Codex process was run.
Not in this change#
- Bumping the pinned upstream Codex protocol ref (UPSTREAM_REF) to 0.166+ would add the new literals and any other protocol changes; that is a broader regeneration than this fix.
- The generator writes a 'generated file / upstream ref' header that the checked-in generated files lack; generator output and checked-in files have drifted on that header.
Who did the work#
- Written by Claude Opus 5.5, in Claude Code
- Reviewed by GPT-6.1 Sol, in Codex, and by Claude Opus 5.5, each on its own
- Proof captured by GPT-6.1 Sol and checked by Claude Opus 5.5
Run by the t3code factory in T3 Code.
- 1
tanstack-source added labelsbug
github-actions[bot](GitHub profile, opens in a new tab) added labelsvouch:trusted
github-actions[bot](GitHub profile, opens in a new tab) added labelssize:L
macroscopeapp[bot](GitHub profile, opens in a new tab) approved these changes
Thread transfer impact#
✅ Thread transfer remains within every enforced ceiling.
ℹ️ The exact PR base did not have a successful artifact. Baseline uses the latest successful
mainmeasurement shown below.Provider Metric Main baseline This PR Impact PR ceiling Codex Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅ Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅ Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅ Codex Live turn WebSocket decoded 21.5 KiB 21.5 KiB 0 B (0.0%) 24.4 KiB ✅ Codex Live turn messages 1 1 0 (0.0%) 8 ✅ Claude Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅ Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅ Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅ Claude Live turn WebSocket decoded 21.9 KiB 21.9 KiB 0 B (0.0%) 24.4 KiB ✅ Claude Live turn messages 1 1 0 (0.0%) 8 ✅ Other scenarios#
All 64 scenario measurements match
main.All 64 scenario measurements
Provider Scenario Metric Main baseline This PR Impact PR ceiling Codex short turn resumeWebSocket decoded10.5 KiB 10.5 KiB 0 B (0.0%) 12.7 KiB ✅ Codex short turn resumeWebSocket messages2 2 0 (0.0%) 2 ✅ Codex long turn running, cold openHTTP decoded281.7 KiB 281.7 KiB 0 B (0.0%) 302.7 KiB ✅ Codex long turn running, cold openrows75 75 0 (0.0%) 75 ✅ Codex long turn running, history pageHTTP decoded92.4 KiB 92.4 KiB 0 B (0.0%) 107.4 KiB ✅ Codex long turn running, history pagerows75 75 0 (0.0%) 75 ✅ Codex long turn finished, cold openHTTP decoded381.5 KiB 381.5 KiB 0 B (0.0%) 410.2 KiB ✅ Codex long turn finished, cold openrows151 151 0 (0.0%) 151 ✅ Codex long turn finished, socket cold openWebSocket decoded381.6 KiB 381.6 KiB 0 B (0.0%) 410.2 KiB ✅ Codex long turn finished, socket cold openWebSocket messages2 2 0 (0.0%) 2 ✅ Codex long turn finished, socket cold openrows151 151 0 (0.0%) 151 ✅ Codex long turn finished, work pageHTTP decoded186.2 KiB 186.2 KiB 0 B (0.0%) 200.2 KiB ✅ Codex long turn finished, work pagerows150 150 0 (0.0%) 150 ✅ Codex idle thread resumeWebSocket decoded65 B 65 B 0 B (0.0%) 200 B ✅ Codex idle thread resumeWebSocket messages1 1 0 (0.0%) 1 ✅ Codex fleet shell snapshotHTTP decoded42.4 KiB 42.4 KiB 0 B (0.0%) 53.7 KiB ✅ Codex fleet shell snapshotrows27 27 0 (0.0%) 27 ✅ Codex fleet shell subscribeWebSocket decoded65 B 65 B 0 B (0.0%) 200 B ✅ Codex fleet shell subscribeWebSocket messages1 1 0 (0.0%) 1 ✅ Codex fleet shell resume checkHTTP decoded18 B 18 B 0 B (0.0%) 100 B ✅ Codex fleet shell resume checkrows0 0 0 0 ✅ Codex stale shell reloadHTTP decoded42.4 KiB 42.4 KiB 0 B (0.0%) 53.7 KiB ✅ Codex stale shell reloadrows27 27 0 (0.0%) 27 ✅ Codex stale shell resumeWebSocket decoded140 B 140 B 0 B (0.0%) 400 B ✅ Codex stale shell resumeWebSocket messages2 2 0 (0.0%) 2 ✅ Codex stale todo list cold openHTTP decoded72.4 KiB 72.4 KiB 0 B (0.0%) 83.0 KiB ✅ Codex stale todo list cold openrows75 75 0 (0.0%) 75 ✅ Codex stale todo list cold opennodes2 2 0 (0.0%) 4 ✅ Codex stale todo list cold opensubagents2 2 0 (0.0%) 4 ✅ Codex stale todo list cold openplans1 1 0 (0.0%) 2 ✅ Codex stale todo list cold openproviderTurns1 1 0 (0.0%) 2 ✅ Codex stale todo list cold opencheckpoints1 1 0 (0.0%) 2 ✅ Claude short turn resumeWebSocket decoded10.6 KiB 10.6 KiB 0 B (0.0%) 12.7 KiB ✅ Claude short turn resumeWebSocket messages2 2 0 (0.0%) 2 ✅ Claude long turn running, cold openHTTP decoded283.3 KiB 283.3 KiB 0 B (0.0%) 302.7 KiB ✅ Claude long turn running, cold openrows75 75 0 (0.0%) 75 ✅ Claude long turn running, history pageHTTP decoded93.4 KiB 93.4 KiB 0 B (0.0%) 107.4 KiB ✅ Claude long turn running, history pagerows75 75 0 (0.0%) 75 ✅ Claude long turn finished, cold openHTTP decoded384.2 KiB 384.2 KiB 0 B (0.0%) 410.2 KiB ✅ Claude long turn finished, cold openrows151 151 0 (0.0%) 151 ✅ Claude long turn finished, socket cold openWebSocket decoded384.3 KiB 384.3 KiB 0 B (0.0%) 410.2 KiB ✅ Claude long turn finished, socket cold openWebSocket messages2 2 0 (0.0%) 2 ✅ Claude long turn finished, socket cold openrows151 151 0 (0.0%) 151 ✅ Claude long turn finished, work pageHTTP decoded188.1 KiB 188.1 KiB 0 B (0.0%) 200.2 KiB ✅ Claude long turn finished, work pagerows150 150 0 (0.0%) 150 ✅ Claude idle thread resumeWebSocket decoded66 B 66 B 0 B (0.0%) 200 B ✅ Claude idle thread resumeWebSocket messages1 1 0 (0.0%) 1 ✅ Claude fleet shell snapshotHTTP decoded43.0 KiB 43.0 KiB 0 B (0.0%) 53.7 KiB ✅ Claude fleet shell snapshotrows27 27 0 (0.0%) 27 ✅ Claude fleet shell subscribeWebSocket decoded66 B 66 B 0 B (0.0%) 200 B ✅ Claude fleet shell subscribeWebSocket messages1 1 0 (0.0%) 1 ✅ Claude fleet shell resume checkHTTP decoded18 B 18 B 0 B (0.0%) 100 B ✅ Claude fleet shell resume checkrows0 0 0 0 ✅ Claude stale shell reloadHTTP decoded43.0 KiB 43.0 KiB 0 B (0.0%) 53.7 KiB ✅ Claude stale shell reloadrows27 27 0 (0.0%) 27 ✅ Claude stale shell resumeWebSocket decoded140 B 140 B 0 B (0.0%) 400 B ✅ Claude stale shell resumeWebSocket messages2 2 0 (0.0%) 2 ✅ Claude stale todo list cold openHTTP decoded73.4 KiB 73.4 KiB 0 B (0.0%) 83.0 KiB ✅ Claude stale todo list cold openrows75 75 0 (0.0%) 75 ✅ Claude stale todo list cold opennodes2 2 0 (0.0%) 4 ✅ Claude stale todo list cold opensubagents2 2 0 (0.0%) 4 ✅ Claude stale todo list cold openplans1 1 0 (0.0%) 2 ✅ Claude stale todo list cold openproviderTurns1 1 0 (0.0%) 2 ✅ Claude stale todo list cold opencheckpoints1 1 0 (0.0%) 2 ✅ Baseline:
36371de· PR result:729fed0· Source CI: successScenario and decoded snapshot size
10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
- Codex decoded thread snapshot: 108.5 KiB
- Claude decoded thread snapshot: 108.8 KiB
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.
No actionable comments were generated in the recent review.
Walkthrough
-
The generator now overrides
HookEventNamewith a string schema. Tests check that hook-started and hook-completed notification predicates accept stopped synchronouspreModelRequestandpostModelRequestnotifications.Changes: Model-request hook notification schemas
Layer / File(s) Summary Hook event schema and validation
packages/effect-codex-app-server/scripts/generate.ts,packages/effect-codex-app-server/src/schema.test.tsThe generator overrides HookEventNamewith a string schema. Tests check that hook-started and hook-completed predicates accept stopped synchronous notifications forpreModelRequestandpostModelRequest.Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Medium
Suggested reviewers:
saphid,macroscopeapp,maria-rcks
Merge Risk:
Minimal · up to729fe-
This change lets hook notifications for the newer model-request events be accepted instead of being rejected by the schema. No actionable merge-blocking risk remains.
Pre-merge checks:
3 · 1-
Failed checks: 1 inconclusiveCheck name Status Explanation Resolution Linked Issues check Issue #35853requires the typed client to deliver hook/completed notifications with eventName preModelRequest and postModelRequest. The fix depends on the closed HookEventName enum in packages/effect-…Provide review access to the regenerated packages/effect-codex-app-server/src/generated/schema.gen.ts, or confirm its diff (the five *_HookEventName lists changed to Schema.String and no other changes), so the generated decode behavior ca… Passed checks: 3 passed-
Check name Status Explanation Title check The title names the fix and its scope: the Codex app-server client now accepts hook event names newer than the pinned protocol. The PR adds a string override for HookEventName in the generator, regene… Out of Scope Changes check All reviewable changes relate to the objective of issue #35853. The generator override in scripts/generate.ts (+2/-0) is the root cause fix for the closed HookEventName enum. The test additions in src…Description check The description follows the repository template. It covers the problem (Codex 0.166+ hook/completed notifications for preModelRequest and postModelRequest failed to decode and were dropped), the chang… Full details: Title check
-
Explanation
The title names the fix and its scope: the Codex app-server client now accepts hook event names newer than the pinned protocol. The PR adds a string override for HookEventName in the generator, regenerates the five closed HookEventName lists to Schema.String, and adds a schema test for preModelRequest and postModelRequest hook notifications. The title is conventional, specific, and concise.
Full details: Out of Scope Changes check
-
Explanation
All reviewable changes relate to the objective of issue
#35853. The generator override in scripts/generate.ts (+2/-0) is the root cause fix for the closed HookEventName enum. The test additions in src/schema.test.ts (+25/-0) cover the two new event names and reproduce the reported failure. The excluded schema.gen.ts changes are the generated output of that override and are not assessed here. The PR excludes the UPSTREAM_REF bump and broader protocol regeneration. No unrelated reviewable change was found in the whole-PR summary.
Full details: Description check
-
Explanation
The description follows the repository template. It covers the problem (Codex 0.166+ hook/completed notifications for preModelRequest and postModelRequest failed to decode and were dropped), the change (HookEventName overridden to a string schema in the generator, with regeneration from pinned ref 687a119f), scope (a linked issue, pingdotgg/t3code#35853, is provided), and verification (a before/after reproduction on main, a schema test that fails before and passes after, and check:changed passing). It also states what was not exercised: no live Codex process, no hook execution, and no desktop or mobile clients. Sections are populated with specific evidence. Minor gaps: the 'Problem' section is a short paragraph rather than the one-to-two-sentence format, and the agent attribution line is present. Those are non-critical.
-
Full details: Linked Issues check
-
Explanation
Issue
#35853requires the typed client to deliver hook/completed notifications with eventName preModelRequest and postModelRequest. The fix depends on the closed HookEventName enum in packages/effect-codex-app-server/src/generated/schema.gen.ts. That file is excluded by the ignored_files rule '!/_generated/', so its content is not in review. The reviewable evidence is consistent with the fix. scripts/generate.ts adds a HookEventName: { type: "string" } override next to PlanType (+2 lines). src/schema.test.ts adds predicates and a test for V2HookStartedNotification and V2HookCompletedNotification that accept both new event names. The PR description reports that regeneration turns the five closed *_HookEventName lists into Schema.String, and that the test passes 6 of 6 after the fix. None of this can be verified from the reviewed diff. Whether the regenerated schema.gen.ts actually removes the closed literals, and whether the test passes against it, is unknown. The client.ts decode path was not changed in the reviewable scope, so it is not shown to be fixed independently of the generated schema. The issue's coding requirement therefore cannot be confirmed. The Evidence section of the PR description is still a placeholder and is not evidence.Resolution
Provide review access to the regenerated packages/effect-codex-app-server/src/generated/schema.gen.ts, or confirm its diff (the five *_HookEventName lists changed to Schema.String and no other changes), so the generated decode behavior can be checked. Confirm that src/schema.test.ts passes against that generated file. Confirm the regeneration output for the generator-header removal described in the PR description.
-
Finishing Touches
-
Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info-
Run configuration-
- Source mode: Auto
- Repository: pingdotgg/t3code
- Branch: factory/issue-35853
- File: .coderabbit.config.ts
- Commit: 729fed0
- Layers: Repository TS config
- Review profile: CHILL
- Plan: Team
- Run ID:
1751afa7-7325-4221-a4ee-effdb74a14c6
Commits-
Reviewing files that changed from the base of the PR and between 95cb068c10a6cf0b33eed0a0cab3b298763fe852 and 729fed007fabcd3d1d00d1a09862d4d38dad0fc5.
Files ignored due to path filters: 1-
packages/effect-codex-app-server/src/_generated/schema.gen.tsis excluded by!**/_generated/**
Files selected for processing: 2-
packages/effect-codex-app-server/scripts/generate.tspackages/effect-codex-app-server/src/schema.test.ts
Limit details: You’ve used all 10 included reviews currently available. Your 17 included PR review attempts over the past 7 days set your current allowance at 10 reviews per hour.
-
-
juliusmarminge merged this
6f108e8juliusmarminge closed this
Sign in to join this conversation.


Approvability#
Verdict: Approved at
729fed0Macroscope's review found this PR approvable — Codex protocol decoding now accepts newer hook-event names instead of dropping their notifications. Existing stopped-run and turn/thread checks remain intact, keeping this a narrowly scoped compatibility change.
You can add or adjust custom eligibility rules. [Learn more](https://docs.macroscope.com/approvability#example-custom-eligibility-rules).