feat(hooks): add Codex deny-retry hook#1550
Conversation
📊 Automated PR Analysis
SummaryAdds programmatic Codex CLI hook support using a deny-with-suggestion pattern, since Codex's PreToolUse hooks cannot apply updatedInput in-place. Includes Review Checklist
Linked issues: #1003 Analyzed automatically by wshm · This is an automated analysis, not a human review. |
cb45305 to
7aa7352
Compare
|
|
7aa7352 to
38f8f13
Compare
There was a problem hiding this comment.
Pull request overview
Adds first-class OpenAI Codex CLI integration to RTK’s hooks layer, aligning Codex with existing agent hook support while documenting why Codex currently requires a deny-with-suggestion workflow instead of in-place rewrites.
Changes:
- Add
rtk hook codexhandler and wire it into the CLI subcommands. - Extend
rtk init --codexto install/manage Codexhooks.json+ enablefeatures.codex_hooks = truein Codexconfig.toml, plus show/uninstall enhancements. - Update hook documentation across repo to reflect Codex’s deny-with-suggestion behavior.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main.rs | Adds the hook codex subcommand and routes execution to the Codex hook handler. |
| src/hooks/init.rs | Implements Codex hooks.json + config.toml install/state display/uninstall logic; adds related tests. |
| src/hooks/hook_cmd.rs | Implements Codex PreToolUse hook processing and deny-with-suggestion response shape; adds tests. |
| src/hooks/constants.rs | Adds CONFIG_TOML constant for Codex config handling. |
| src/hooks/README.md | Updates hooks matrix and implementation notes to include Codex behavior. |
| hooks/codex/README.md | Documents Codex hook wiring and deny-with-suggestion behavior. |
| hooks/README.md | Updates global hook docs to describe Codex’s non-transparent rewrite approach. |
| docs/contributing/TECHNICAL.md | Updates contributor docs to reflect Codex hook support. |
| README.md | Updates user-facing overview and supported tools table for Codex hook behavior. |
|
Wouldn't his also work on the codex app, ie not codex CLI? |
Adds `rtk hook codex` subcommand for Codex CLI PreToolUse hooks. Since Codex doesn't yet apply updatedInput, uses deny-with-suggestion to prompt the agent to rerun with the rtk-rewritten command. Based on rtk-ai#1550.
Adds `rtk hook codex` subcommand for Codex CLI PreToolUse hooks. Since Codex doesn't yet apply updatedInput, uses deny-with-suggestion to prompt the agent to rerun with the rtk-rewritten command. Based on rtk-ai#1550.
|
@BjornMelin @aeppling waiting for this |
DISK EMERGENCY (operator: 94M free / 100% used)
Caught: cleanup hook was stuck in dry-run forever
(.planning/.cleanup-authorized never created). Even worse —
parse_duration_ms in apps/desk/src-tauri/src/desk_hooks.rs only
handled ms/s/m suffixes, so frontmatter `interval=24h` fell to
bare-number parse → returned None → HookSchedule::default()
used interval_ms: 10_000 = hook fired every 10 seconds for HOURS.
Same bug hit forecast-tick.mjs (interval=1h).
GLOBAL-T-248-13 (V5) — extended storage-auto-cleanup-tick:
+ Tauri bundle artifact sweep (keep top-2 versions per app)
+ rtk tee log cleanup (>7 days)
+ Cargo target audit-only recommendations
+ Fixed parse_duration_ms to handle h/d suffixes + 3 tests
+ Belt-and-suspenders: storage hook frontmatter changed
24h → 86400s so old desk builds also schedule correctly
Live run freed 941MB removing 72 old bundle .exe/.sig files.
Main-thread cargo debug nuke (live op, in addition to V5):
rm -rf apps/{desk,hub,desktop,desk-consumer}/src-tauri/target/debug
rm -rf apps/{desk,hub,desktop,desk-consumer}/src-tauri/target/x86_64-pc-windows-msvc/debug
Freed ~12 GB. Release builds untouched — operator's running
binaries unaffected.
Net: 94M → 13G free.
PHASES CLOSED:
92 — onboarding (3 of 8): state machine + TutorialOverlay +
FirstShipSuccess. 5 left planned (UX copy decisions).
96 — analytics (5 of 6): event-bus + vercel-traffic adapter +
edits-errors aggregator + types/identity + 421-line design
doc. 96-07 deferred to post-phase-95.
100 — project export (4 of 6): zip via GitHub zipball proxy,
soft revoke (revoked_at column + 410 Gone enforcement),
settings page. SQL stub (501 + dashboard link). 2 left
(live SQL, eval).
256 — git ops (2 more): 256-12 GitHub Releases list/view/create
with assets, 256-15 inline Rust tests (11 cases).
103-07 (FOLLOW-UP) — @ai-sdk/openai-compatible migration
Removed @ai-sdk/openai + @ai-sdk/gateway. Added
@ai-sdk/openai-compatible. Migrated:
apps/platform/lib/agent/gateway.ts
apps/platform/app/api/llm/chat/route.ts
pnpm-lock regenerated. lint-ai-sdk now PASSES (was 5 violations).
Cost attribution preserved structurally — gatewayCostUsd will be
undefined → falls to estimateCostUsd() path, stamps
cost_source='estimated'. Follow-up 103-08 will wire
metadataExtractor against x-vercel-ai-gateway-* response
headers for gateway-source parity.
245-14 / 245-15 (CANCELLED + ROLLBACK)
gad tasks dedupe shipped in wave T was a false-positive generator
— grouped tasks by suffix alone (200-01 = scaffolding-hooks vs
models-state-tick are DIFFERENT TASKS sharing suffix because they
are both task #1 of phase 200). Operator caught it before --apply
would have destroyed data. Tool deleted entirely. Error logged
to .planning/ERRORS-AND-ATTEMPTS.xml: NEVER ship a task-merge
tool using suffix alone — requires goal-similarity AND
operator-per-group confirm.
264-09 — rtk upstream PR queued (NOT FILED): subagent U2 found
PR rtk-ai/rtk#1550 already covers identical scope with a
technically better deny-retry approach. Filing duplicate
rejected. Operator path: comment on #1550 offering rebase help.
EVIDENCE:
node scripts/lint-ai-sdk.mjs — OK no provider-locked imports
pnpm vitest run lib/onboarding/__tests__/ — 9/9 + 10/10 + 8/8
pnpm tsc --noEmit apps/platform — touched files clean
df -h /c → 13G free (was 94M)
storage hook smoke: WROTE 72 old tauri bundles (941.1 MB)
second run: NOOP nothing to clean (idempotent)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds `rtk hook codex` subcommand for Codex CLI PreToolUse hooks. Since Codex doesn't yet apply updatedInput, uses deny-with-suggestion to prompt the agent to rerun with the rtk-rewritten command. Based on rtk-ai#1550.
|
Hey @BjornMelin Thanks for addressing this. I'm aware of the codex CLI gap for the updatedInput that is not honored, thus being said the deny retry approach can be costly :
RTK want to be a LLM proxy, transparent and efficient, and for this we want to detach from context based actions for LLMs, to avoid polluting context + more precision and reliable because deterministic. This is not possible yet because not all CLI support hooks, but for Codex we seems close to be able to do this. Your PR is clean and thanks for this, it has a lot of clean part that could be favorized, the approach is what i want to discuss. You can refer to my review on #2033 about the codex CLI updatedInput problem. |
Adds `rtk hook codex` subcommand for Codex CLI PreToolUse hooks. Since Codex doesn't yet apply updatedInput, uses deny-with-suggestion to prompt the agent to rerun with the rtk-rewritten command. Based on rtk-ai#1550.
Summary
Adds official Codex CLI hook support for RTK.
rtk hook codexfor CodexPreToolUseJSONrtk init --codexto writehooks.jsonand enable[features].codex_hooks = trueupdatedInputCloses #1003.
Codex hook behavior
Codex currently parses
updatedInputinPreToolUsehook output but does not apply it in practice. This PR uses the working path instead:The installed
hooks.jsoncommand is portable:{ "hooks": { "PreToolUse": [ { "matcher": "Bash", "hooks": [ { "type": "command", "command": "rtk hook codex", "statusMessage": "Checking RTK rewrite" } ] } ] } }Validation
cargo fmt --allcargo test codex -- --nocapturecargo test --allcargo clippy --all-targetsHOME=$(mktemp -d) ./target/debug/rtk init -g --codexgit statusreturns Codex deny payload withRerun that as: rtk git statusgit status --short, surfacedRerun that as: rtk git status --short, then reran the RTK commandNotes
cargo clippy --all-targets -- -D warningsis not the current CI command and fails on existing baseline warnings unrelated to this change. The repository CI command,cargo clippy --all-targets, passes.