Skip to content

fix: use canonical codex hooks flag - #244

Merged
centdix merged 1 commit into
mainfrom
adapt-status-hooks
May 18, 2026
Merged

fix: use canonical codex hooks flag#244
centdix merged 1 commit into
mainfrom
adapt-status-hooks

Conversation

@centdix

@centdix centdix commented May 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Update Webmux's managed Codex launch command to enable lifecycle hooks with the canonical hooks feature flag now documented by Codex, replacing the deprecated codex_hooks alias.

Changes

  • Switch generated Codex agent commands from --enable codex_hooks to --enable hooks
  • Update agent command builder expectations for host and docker launches
  • Update lifecycle resume expectations for managed Codex worktrees

Test plan

  • bun test backend/src/__tests__/agent-service.test.ts backend/src/__tests__/lifecycle-service.test.ts backend/src/__tests__/agent-runtime.test.ts
  • bun run --cwd backend check

Generated with Claude Code

@claude

claude Bot commented May 18, 2026

Copy link
Copy Markdown

Claude finished @centdix's task in 1m 30s —— View job


Code Review

  • Read changed files and understand the diff
  • Verify no other references to codex_hooks remain in the codebase
  • Check parity across all surfaces (backend, frontend, CLI)
  • Provide review feedback

Verdict: Looks good — minimal, surgical, and complete. Safe to merge.

What the PR does

Renames the Codex hooks feature flag from --enable codex_hooks to --enable hooks in backend/src/services/agent-service.ts:36, and updates the three test assertions that pin the rendered command string.

Coverage check

  • grep -rn codex_hooks across the entire repo returns zero hits — no stale references in docs, comments, frontend, CLI, or other backend modules.
  • The flag is only constructed inside buildBuiltInAgentInvocation (agent-service.ts) and threaded through all three branches (resume, with system prompt, default) via the shared hooksFlag local — so the single-line rename correctly covers all three command shapes.
  • Tests cover all three command-shape variants: host launch with prompt (agent-service.test.ts:127, :199), docker launch (:127), resume (:171), plus the end-to-end lifecycle resume in lifecycle-service.test.ts:1027.
  • The .codex/hooks.json file path and the hook-fixture wiring in backend/src/adapters/agent-runtime.ts are correctly untouched — that's the hooks file location Codex reads, not the launch flag.

Parity

The flag is fully encapsulated inside the managed Codex command builder. There's no frontend toggle or CLI-surfaced option to update, so AGENTS.md §3 (frontend/CLI parity) doesn't apply here.

Minor observations (non-blocking)

  1. No fallback for older Codex versions. If a user is pinned to a Codex build that only accepts the deprecated codex_hooks alias, launching will now fail with an unknown-flag error. Given the PR description states hooks is the canonical, currently-documented flag, this seems intentional — but it might be worth a one-liner in the next release notes telling users to upgrade Codex if they hit this. No code change needed.
  2. Test assertions are still toContain substrings. That's consistent with the surrounding style, so no change requested — just noting that none of the test changes tightened or loosened verification, they're pure string updates matching the production rename.

Nothing actionable. Approving in spirit (I can't formally approve PRs).
Branch: adapt-status-hooks

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
webmux 094120f Commit Preview URL

Branch Preview URL
May 18 2026, 07:36 PM

@centdix
centdix marked this pull request as ready for review May 18, 2026 19:36
@centdix
centdix merged commit d2ef583 into main May 18, 2026
4 checks passed
@centdix
centdix deleted the adapt-status-hooks branch May 18, 2026 19:37
@claude

claude Bot commented May 18, 2026

Copy link
Copy Markdown

Claude finished @centdix's task in 1m 57s —— View job


Code Review

  • Read changed files and understand the diff
  • Verify references to codex_hooks across the codebase
  • Check parity across surfaces (backend-only change, no frontend/CLI surface)
  • Provide review feedback

Findings

🐛 Bug: one test assertion still references the old codex_hooks flag

backend/src/__tests__/agent-service.test.ts:204 was missed by the rename and still asserts the deprecated flag:

expect(command).toContain("codex --enable codex_hooks --yolo resume --last -- 'ship the fix'");

Production code now emits --enable hooks (see backend/src/services/agent-service.ts:38), so this toContain(...) assertion can never match — the test for "appends the follow-up prompt to codex resume --last when one is provided" (line 191) must be failing.

This contradicts the PR's test plan, which says bun test backend/src/__tests__/agent-service.test.ts passed. Worth double-checking what was actually run — the other three assertions in the same file (lines 142, 185, 229) were correctly updated, only this one slipped through. Quick fix:

-    expect(command).toContain("codex --enable codex_hooks --yolo resume --last -- 'ship the fix'");
+    expect(command).toContain("codex --enable hooks --yolo resume --last -- 'ship the fix'");

Fix this →

✅ Everything else looks good

  • agent-service.ts:38 — flag renamed correctly, used uniformly for direct, system-prompt, and resume launches.
  • lifecycle-service.test.ts:1099 — assertion updated correctly.
  • agent-service.test.ts:142, :185, :229 — assertions updated correctly.
  • Full repo grep confirms no other codex_hooks references remain (only an unrelated bin/src/init-helpers.ts:539 comment that uses "hooks" generically).
  • No CLI or frontend surface for this flag — backend-only change is appropriate; no parity work needed per AGENTS.md.
  • The diff is minimal and scoped to the rename — no scope creep, follows the "keep it minimal" guidance.

Nit (optional)

agent-service.ts:38 declares hooksFlag as a const inside buildBuiltInAgentInvocation. Since it is now an unconditional string for the codex branch, it could be inlined into the three template literals on lines 43, 46, and 48 without losing readability. Not worth a follow-up on its own, but if you're already touching the file.


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant