fix(codex): drop invalid top-level version field + correct timeout unit (seconds)#482
Conversation
Codex CLI strictly expects only 'hooks' at the top level and fails to parse 'version'. This removes the version injection for Codex configuration and handles existing config files with a version field.
Codex reads the hooks `timeout` field in SECONDS (its `timeout_sec` field, default 600), not milliseconds like Claude/Cursor/Gemini. The old 60000 meant ~16.7h, not 60s. Fix `codex.buildHookEntry` and regenerate the dogfood `.codex/hooks.json` to `timeout: 60`, add a unit assertion, and add the 0.0.12-beta.0 CHANGELOG entry covering both the version-field drop and this timeout correction. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughRemoves the invalid top-level ChangesCodex Hooks Timeout and Version Fix
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant Integrations as integrations.ts
participant Settings as CodexSettingsFile
Caller->>Integrations: install hooks
Integrations->>Settings: readSettings()
Integrations->>Integrations: stripLegacyVersion(settings)
Integrations->>Settings: writeHookEntries(timeout=60)
Caller->>Integrations: remove hooks
Integrations->>Integrations: stripLegacyVersion(settings)
Integrations->>Settings: write back if removed>0 or hadVersion
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
__tests__/e2e/hooks/codex-integration.e2e.test.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. __tests__/hooks/integrations.test.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. src/hooks/integrations.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🔍 Automated code review started — analyzing [{"additions":6,"deletions":7,"path":".codex/hooks.json"},{"additions":5,"deletions":0,"path":"CHANGELOG.md"},{"additions":1,"deletions":1,"path":"tests/e2e/hooks/codex-integration.e2e.test.ts"},{"additions":19,"deletions":6,"path":"tests/hooks/integrations.test.ts"},{"additions":22,"deletions":9,"path":"src/hooks/integrations.ts"},{"additions":2,"deletions":2,"path":"src/hooks/types.ts"}] files, +55/-25... ⏱️ This may take a few minutes. Results will be posted here when complete. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Around line 3-7: Add the missing PR reference to the changelog entry by
updating the 0.0.12-beta.0 bullet in CHANGELOG.md to include the appropriate
(#<PR_NUMBER>) suffix; keep the existing release note text intact and only amend
the bullet that describes the Codex hooks fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fc274a87-084c-446a-a27f-6d04f6572cf3
📒 Files selected for processing (6)
.codex/hooks.jsonCHANGELOG.md__tests__/e2e/hooks/codex-integration.e2e.test.ts__tests__/hooks/integrations.test.tssrc/hooks/integrations.tssrc/hooks/types.ts
| ## 0.0.12-beta.0 — 2026-07-09 | ||
|
|
||
| ### Fixes | ||
| - Codex hooks: drop the invalid top-level `version` field from `.codex/hooks.json` (Codex CLI v0.142+ rejects it with `unknown field 'version'`, refusing to start any session), and strip any leftover `version` on the next install/uninstall so previously-broken configs self-heal. Also correct the Codex `timeout` unit from `60000` to `60` — Codex reads `timeout` in seconds (its `timeout_sec` field), so the old value meant ~16.7h instead of 60s. Copilot and Cursor legitimately carry `version: 1` in their own schemas and are untouched. | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the CHANGELOG version matches package.json
rg '"version"' package.json | head -1Repository: FailproofAI/failproofai
Length of output: 191
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== package.json version ==\n'
rg -n '"version"' package.json | head -1
printf '\n== CHANGELOG.md excerpt ==\n'
cat -n CHANGELOG.md | sed -n '1,20p'Repository: FailproofAI/failproofai
Length of output: 4098
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
text = Path('CHANGELOG.md').read_text()
print('PR marker present:', '(#' in text or 'PR' in text)
PYRepository: FailproofAI/failproofai
Length of output: 185
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
text = Path('CHANGELOG.md').read_text()
lines = text.splitlines()
for i, line in enumerate(lines[:20], 1):
if re.search(r'^\s*-\s+', line):
print(f'bullet@{i}: {line}')
PYRepository: FailproofAI/failproofai
Length of output: 3862
Add the PR number to this changelog entry.
The 0.0.12-beta.0 release header matches package.json; the bullet still needs a (#<PR_NUMBER>) suffix.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CHANGELOG.md` around lines 3 - 7, Add the missing PR reference to the
changelog entry by updating the 0.0.12-beta.0 bullet in CHANGELOG.md to include
the appropriate (#<PR_NUMBER>) suffix; keep the existing release note text
intact and only amend the bullet that describes the Codex hooks fix.
Source: Coding guidelines
|
⏳ [progress] Deep review in progress — analyzed the diff (Codex |
| // • Stdin event names arrive snake_case (pre_tool_use); we canonicalize to PascalCase before policy lookup | ||
| // • No "local" scope | ||
| // • Settings file carries a top-level "version": 1 marker | ||
| // • Settings file does NOT carry a top-level "version" marker (Codex strictly expects only `hooks`) |
There was a problem hiding this comment.
🔵 Info (doc consistency): This line correctly updates the Codex header to note it no longer carries a top-level version. But two parallel comments in this same file still describe Codex as carrying version: 1 and are now stale:
src/hooks/integrations.ts:390(Copilot header):`{ "version": 1, "hooks": {...} }`, mirroring Codex.— Codex no longer mirrors this shape.src/hooks/integrations.ts:541(Cursor header):carries `version: 1` like Codex/Copilot.— should readlike Copilot.
The PR already fixed the exact twins of these in src/hooks/types.ts (Copilot: dropped “like Codex’s hooks.json”; Cursor: “like Codex/Copilot” → “like Copilot”), so these two in integrations.ts were just missed.
Fix: drop the “mirroring Codex” / “like Codex” references at lines 390 and 541. Documentation-only — not blocking.
🔍 Automated Code Review📋 Executive SummaryThis is a tight, correct, well-scoped bugfix. It removes the invalid top-level 📊 Change Architecturegraph TD
A["failproofai policies --install / --uninstall (--cli codex)"] -->|"invokes"| B[codex Integration]
B --> C["stripLegacyVersion(settings) 🟢"]
B --> D["buildHookEntry: timeout 60000 → 60 🟡"]
C --> E["writeHookEntries: strip version, write 6 events"]
C --> F["removeHooksFromFile: strip version + remove hooks 🔵"]
F -->|"write only if removed>0 OR hadVersion 🟢"| G[".codex/hooks.json (no version, timeout 60)"]
E --> G
D --> G
G -->|"deny_unknown_fields OK"| H["Codex CLI loads hooks ✅"]
style C fill:#90EE90
style F fill:#87CEEB
style D fill:#FFD700
style H fill:#90EE90
Legend: 🟢 New helper / logic | 🔵 Modified | 🟡 Value fix (breaking-bug fix) 🔴 Breaking Changes✅ No new breaking changes — this PR fixes a breaking bug.
|
hermes-exosphere
left a comment
hermes-exosphere
left a comment
There was a problem hiding this comment.
Automated review complete — Approved with suggestions.
Correct, minimal, well-scoped fix (drop invalid top-level version, correct Codex timeout unit to seconds, self-heal on install/uninstall). Verified end-to-end: full suite green (1907 unit + 10 Codex e2e, tsc/eslint clean), and I reproduced all four self-heal paths against the real code. No correctness issues.
Non-blocking suggestions: two stale doc comments still tie Codex to version: 1 (integrations.ts:390 and :541 — twins of the ones already fixed in types.ts), and the branch should rebase onto origin/main to pick up #475. Details in the summary + inline comment.
…lation sync (#484) Add the 17 Dependabot bumps (#463-#481) and the docs translation sync (#462) merged this cycle to the 0.0.12-beta.0 CHANGELOG section, and append the missing (#482) reference to the Codex hooks Fixes entry, matching the repo convention that every release documents its dependency bumps. Claude-Session: https://claude.ai/code/session_01FQLQSMW6WJH5jA2Dm9qbF8 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* feat: sync agent CLI harnesses with upstream docs Append newly-documented hook events and fix two timeout-unit bugs found by comparing each integrated CLI's upstream docs to this repo. Scope 1 (event names): - Claude: append `Setup`, `MessageDisplay` (no event map — build stays green) - Codex: append `subagent_start`, `pre_compact`, `post_compact`, `subagent_stop`. CODEX_EVENT_MAP entries are intentionally NOT added so `tsc` fails until a reviewer picks the canonical HookEventType (per the map-bearing-CLI rule) — see PR checklist. - Copilot: append `PostToolUseFailure`, `ErrorOccurred`, `PreCompact`, `PermissionRequest` (docs show explicit PascalCase variants; no event map — build stays green). `notification` / `subagentStart` are camelCase-only upstream and deferred to the reviewer. Scope 3 (settings-file shape, #482-class timeout-unit fix): - Claude and Cursor read `timeout` in SECONDS per their docs (Claude "60 for agent"; Cursor "Execution timeout in seconds"), so the old `60000` meant ~16.7h. Corrected writer + dogfood fixtures + unit tests to `60`. Gemini's `timeout` is genuinely milliseconds (default 60000) and is left unchanged. Verified each claim by fetching the primary docs directly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(codex): complete CODEX_EVENT_MAP for the 4 newly-synced events The prior commit added subagent_start/pre_compact/post_compact/subagent_stop to CODEX_HOOK_EVENT_TYPES but intentionally left their CODEX_EVENT_MAP entries out to fail tsc as a reviewer flag. Each has an exact 1:1 canonical HookEventType (SubagentStart/PreCompact/PostCompact/SubagentStop, all already in HOOK_EVENT_TYPES), so fill them in directly. Besides unblocking tsc, this fixes a real runtime bug: with no mapping, writeHookEntries resolved CODEX_EVENT_MAP[event] to undefined and collapsed all four events into a single literal "undefined" key in users' .codex/hooks.json (the source of the removeHooksFromFile "7 vs 10" test failure). The map is exhaustive, so tsc still guards against a future event-added-mapping-missing drift. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(claude): keep Setup, defer observe-only MessageDisplay The prior commit appended both Setup and MessageDisplay to HOOK_EVENT_TYPES. writeHookEntries installs a failproofai hook for every entry in that array, so appending MessageDisplay would spawn a subprocess on every assistant message render — and the docs mark MessageDisplay observe-only (cannot block or modify, no matcher support), so that cost buys zero enforcement value. Keep Setup (fires only on --init/--maintenance; low-frequency, harmless to install) and defer MessageDisplay to the reviewer checklist with a documented NOTE, mirroring how the sync bot defers uncertain items. Net event count is 29, so update the two manager.test count assertions and the description. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(copilot): append Notification, correct the deferral note The prior commit's comment claimed both notification and subagentStart are camelCase-only upstream. Re-checking the cli-hooks-reference: Notification DOES have a documented PascalCase "VS Code compatible" variant, so append it to COPILOT_HOOK_EVENT_TYPES (no event map, build stays green). Only subagentStart is genuinely camelCase-only, so it stays deferred to the reviewer checklist. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: correct Codex timeout field name + update changelog for review - The Codex timeout comment said the field is `timeout_sec`; docs confirm it is literally `timeout` (unit seconds, default 600). Fix the comment. - Rewrite the 0.0.13-beta.0 changelog entry to describe the reviewed result: Codex map completed, Copilot Notification added, MessageDisplay deferred. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: failproofai-hook-sync <hook-sync-bot@exosphere.host> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: chhhee10 <chetanraghuvanshi85@gmail.com>
3 participants
Problem
Installing failproofai hooks for OpenAI Codex wrote a top-level
"version": 1into
.codex/hooks.json. Codex CLI'sHooksFilestruct is#[serde(deny_unknown_fields)]and permits onlyhooks(+ optionaldescription), so Codex rejects the file. Confirmed against real Codex CLIv0.143.0:
When this fires the hooks fail to load entirely — so no failproofai policy
evaluates on Codex sessions.
Separately, the Codex
timeoutwas written as60000. Codex readstimeoutin seconds (its
timeout_secfield, default 600), so60000meant ~16.7h,not 60s.
Fix (Codex-only)
version, and strip any leftoverversionon the nextinstall and uninstall, so a previously-broken
hooks.jsonself-heals onreinstall (upgraders already have
version: 1on disk).60000→60(seconds) incodex.buildHookEntryandthe regenerated dogfood
.codex/hooks.json.__failproofai_hook__marker (Codex's internally-taggedHookHandlerConfig::Commandhas nodeny_unknown_fields, so it's toleratedand stays the primary install-detection key).
stripLegacyVersion()helper used by both
writeHookEntriesandremoveHooksFromFile.version: 1in their own schemas andare untouched.
Verification
Tested against real Codex CLI v0.143.0:
version: 1) → reproduces the exact parse warning above; hooksdon't load.
version,timeout: 60), generated by the realwriteHookEntriesinstall path → parses cleanly, hooks load.Plus: unit 1907 passed, Codex e2e 10 passed,
tsc --noEmitclean,eslint0 errors,bun run buildgreen, and the config self-heals on bothinstall and uninstall.
Summary by CodeRabbit