Codex hooks: document layered precedence and guard inject-args against copying user hook groups (#12081)

Draft
AU
2mo ago
+187-2

Fixes https://github.com/manaflow-ai/cmux/issues/12081

What the issue claimed, and what is actually true

The issue reports that the cmux Codex wrapper's per-invocation -c hooks.<Event>=[...] arguments make Codex replace the user's hooks.json arrays for SessionStart, UserPromptSubmit, Stop, PreToolUse, PostToolUse, and PermissionRequest, so user hooks stop running inside cmux.

That premise is false for every Codex build cmux can inject into, and this PR verifies it rather than working around it:

  • Source. Codex's hook discovery (codex-rs/hooks/src/engine/discovery.rs in openai/codex) walks the config layer stack low→high and appends handlers from each layer: managed hooks, the user layer ($CODEX_HOME/hooks.json, then [hooks] in config.toml), project .codex layers, the command-line (-c, "session flags") layer, then plugin hooks. That loop has had this shape since config.toml hooks landed (openai/codex#18893, 2026‑04‑23), i.e. before --dangerously-bypass-hook-trust (openai/codex#21768, 2026‑05‑13) which cmux's injection requires. It is identical at the installed Codex 0.153.4.
  • Reproduction (issue steps, isolated CODEX_HOME, real Codex 0.153.4 binary, hooks.json with user-owned SessionStart/UserPromptSubmit/Stop handlers that append markers to a log):
RunExtra argvUser handlers that firedcmux handler firedCodex hook: dispatch log
A--enable hooks --dangerously-bypass-hook-trust onlySessionStart, UserPromptSubmit (turn failed on model capacity, so no Stop)n/a1× per event
Bthe exact cmux hooks codex inject-args output of the installed cmux 0.64.22 CLI (cmux-only per-event arrays)SessionStart, UserPromptSubmit, Stop(cmux scripts no-op without a surface)2× per event (user + cmux)
Ccmux-shaped -c hooks.SessionStart=[{hooks=[{…marker…}]}]SessionStart, UserPromptSubmit, Stopyes2× SessionStart
Dthe "combined" value from the two superseded commits on this branch (user group copied into -c + cmux group)SessionStart twice, UserPromptSubmit, Stopyes3× SessionStart

hooks.json was byte-identical after every run. Run B is the issue's step 3–5 with the shipped cmux CLI: the user's hooks still run. Run D shows why the "append the user's array into the -c value" fix that was first attempted on this branch must not ship: Codex registers the copied groups a second time and runs them twice.

What this PR changes

  • docs/agent-hooks.md: new Codex hook precedence section describing the layering above, that cmux never copies user groups (double execution), the one-cmux-producer-per-event rule with a persistent cmux hooks codex install, the trust side effect of --dangerously-bypass-hook-trust (untrusted user/project hooks run without the /hooks review prompt inside cmux), and the CMUX_CODEX_HOOKS_DISABLED=1 opt-out trade-off.
  • CLI/CMUXCLI+CodexFireAndForgetHooks.swift: comment-only; the emitter's doc comment now states the contract (Codex appends the -c layer; never copy user-owned groups, #12081). No code token changes, no runtime change.
  • tests/test_codex_inject_args_layering.py (+ one line in the Run CLI no-socket regressions CI step): behavioral regression test against the real CLI. With user-owned groups on all twelve Codex events, cmux hooks codex inject-args must emit exactly one cmux handler per event and never a user command, must leave hooks.json untouched, and must skip an event that already has a persistent cmux handler.

Trade-offs taken (stated, not absorbed)

  • No behavior change. The correct fix for a false premise is evidence plus documentation, not code that "preserves" arrays Codex never dropped.
  • Fresh branch. An earlier attempt on issue-12081-codex-hook-append (commits 337b2c5198 test + 20aab5c0ce fix, plus a later merge of main) implements exactly the double-execution regression shown in run D. That branch was never opened as a PR; it is superseded by this one and should be deleted rather than merged.
  • No red/green test commit pair. There is no cmux bug to catch, so the test guards the contract that would have been broken by the attempted fix. It passes on main by design; the PR says so instead of faking coverage.
  • Documenting the trust bypass is a deliberate addition: it is a real consequence of cmux's injection that the issue's "precedence" request touches.
  • No tagged app build or dogfood surface: nothing user-visible at runtime changes; CI builds the CLI and runs the new test against it in the app-host job.
  • Out of scope, noted for follow-up: cmux emits timeout=10000/120000 in its -c values; Codex parses timeout as seconds (normalize_command_hook leaves non-SessionEnd hooks uncapped). cmux's hook scripts return immediately, so this is latent, but the unit is wrong.

Tests run

  • python3 -m py_compile tests/test_codex_inject_args_layering.py
  • CMUX_CLI_BIN=<dev CLI built from a branch containing current main> python3 tests/test_codex_inject_args_layering.py → PASS (DerivedData cmux-cmd-2538, commit 97997dfa2). On the shipped 0.64.22 CLI (ddd4a01bc) case 1 passes and case 2 fails, because that release predates main's persistent-producer skip (6b79b62d9a); the test targets main's contract.
  • Mutation check: a fake CLI that copies a user group into the -c value fails the test ("user-owned hook copied into value"); one that emits two handlers for SessionStart fails ("unexpected cmux hook shape").
  • Comment-only Swift diff verified (git diff -U0 … | grep '^[+-]' | grep -v '^[+-] *///' empty)
  • Codex end-to-end experiment above (runs A–D)
  • CI: Run CLI no-socket regressions executes the new test against the PR-built CLI

Localization audit: no in-app, Settings, menu, schema, or web strings were added or changed; docs are English-only like the rest of docs/. No keyboard shortcuts involved. No iOS paths touched.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MPAVb9SSqAnuUnPEFQguE9


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Clarifies that Codex appends cmux's per-launch -c hook layer on top of user hooks from hooks.json and config.toml instead of replacing them — the assumption behind #12081 was wrong, so user hooks keep running — and adds a regression test guarding against copying user hook groups into injected values, which would register and run them twice. No runtime behavior changes.

  • docs/agent-hooks.md gains a Codex hook precedence section covering the layering, the one-cmux-producer-per-event rule, the --dangerously-bypass-hook-trust side effect, and the CMUX_CODEX_HOOKS_DISABLED=1 opt-out.
  • The inject-args emitter's doc comment states the never-copy-user-groups contract.
  • tests/test_codex_inject_args_layering.py (wired into the no-socket CLI regression step) asserts one cmux handler per event, never a copied user command, unchanged hooks.json, and skipping events with a persistent cmux handler.

[!NOTE]
Low Risk
Documentation and a CLI contract regression test only; inject-args behavior is unchanged.

Overview
Clarifies that Codex appends cmux’s per-launch -c hooks.<Event> layer on top of hooks.json / config.toml rather than replacing user hooks, and documents precedence, the one-cmux-producer-per-event rule, trust-bypass side effects, and CMUX_CODEX_HOOKS_DISABLED=1. No runtime change — the Swift inject-args emitter only gets an updated doc comment stating cmux must never copy user hook groups into -c values (that would double-register handlers).

Adds tests/test_codex_inject_args_layering.py and wires it into the app-host Run CLI no-socket regressions CI step. The test asserts cmux hooks codex inject-args leaves hooks.json untouched, emits only cmux-owned commands, and skips events that already have a persistent cmux handler in hooks.json.

Reviewed by Cursor Bugbot for commit 11d4c5844a343bc6ec084fb332342e4c41a816a0. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • Bug Fixes

    • Codex hook handling now preserves user-configured hooks without registering them twice.
    • Prevents duplicate cmux handlers for the same Codex event.
    • Existing persistent cmux hook installations are detected and not injected again.
  • Documentation

    • Added guidance on Codex hook precedence, layering, trust behavior, and handler ordering.
    • Documented CMUX_CODEX_HOOKS_DISABLED=1 for running Codex with unchanged hook configuration.
    • Added recommendations for persistent Codex hook installation.