Add a Codex project hook approval flow to Agent Mode

[mplibunao]

Fixes #665.

What this does

Codex 0.145.0 only runs repo-level .codex PreToolUse hooks when the effective CODEX_HOME config carries a [hooks.state] trusted_hash approval entry for each hook; otherwise the hooks are silently skipped. Because CE binds its bundled Codex app server to an app-private CODEX_HOME, hook approvals users made in ~/.codex never carry over, so repo guardrail hooks (for example, hooks that block terraform destroy) silently stopped running for CE Agent Mode sessions — fail-open, with no signal.

This PR adds a fail-closed approval lifecycle. After a thread binds but before the first turn/start, the coordinator lists project hooks for the execution cwd. Any untrusted or modified project hook blocks the first turn behind a review interaction, surfaced both as a SwiftUI card and over the agent_run MCP contract. Approval writes the trusted_hash entries through config/batchWrite into CE's private home, re-verifies with a second hooks/list, and only then releases the pending turn. Live threads pick up the trust write without rebinding (verified against the pinned runtime).

Design highlights

• Lazy, per-controller-binding gate covering every dispatch route that can start a first turn: direct send, fallback idle pump, terminal-successor fallback, and managed-auth replay. Restore/reconnect/goal/compact bindings don't trigger review until a turn is actually sent.
• Serialized trust writes. config/batchWrite is read-modify-write per process with no cross-process merge (verified empirically: concurrent writers clobber, last writer wins), so CE serializes all trust writes behind an app-global mutex, held until server-side settlement with cancellation shielding.
• Fail closed everywhere. Discovery, parse, write, and verification failures block the turn with retry/continue options; unknown trust statuses fail decode; verification runs a second hooks/list after every write. Inventory drift to zero during review resolves as a distinct resolved_externally audit, never as a silent bypass.
• Explicit continuation, never a default. Continuing without hooks is a deliberate user action, recorded in the transcript with a counts-only audit. A discovery-failure bypass reports its skipped count as unknown rather than zero.
• Opt-in strict mode. By default, the review card lets a human explicitly choose "Continue Without Hooks" — the session runs unguarded, and the transcript records that choice. Checking Settings → Agent Mode → "Require Codex project-hook approval" removes that option: sessions stay blocked until the displayed hooks are approved (or become trusted externally, e.g. via the codex TUI). An "In this workspace" picker — App default (labeled with the live global value) / Always require approval / Don't require approval — lets a single workspace keep its own value instead of the global one, e.g. global off for scratch repos, strict on for an infrastructure repo whose hooks guard terraform destroy. The setting is human-only — the app_settings MCP surface rejects writes to it.
• Headless parity. MCP hook_approval interaction with phase-specific options, hook_keys selective approval, an exact top-level argument allowlist, and atomic snapshot decoding so wait/poll round trips can't silently drop the interaction. Full hook metadata (source path, command, hash) is intentionally present in the pending interaction — it's required for informed approval — while diagnostics, stale-ID errors, transcripts, and audits stay counts-only.
• Schema gate. hooks/list and config/batchWrite shapes are pinned in the app-server contract fixture (floor 0.145.0) so runtime drift is caught by make dev-codex-schema-check.

Notes for reviewers

• The MCP approval surface uses the same authority model as the existing approval/question interactions: the agent_run caller (the user's orchestrator) resolves interactions. The gated session itself cannot self-approve — its first turn has not started while the gate is pending.
• Project trust still gates hook discovery entirely (codex behavior): repos not trusted in the private home list no hooks, so the hook gate composes with — and does not replace — project trust handling.
• One codex upstream quirk documented during verification: a schema-invalid hook declaration (flat command on the matcher table) is silently ignored by hooks/list with zero errors, indistinguishable from a hook-less repo. The gate cannot catch that class; candidate for an upstream codex report.
• FileSystemContentLoadingConcurrencyTests.testCancelledQueuedContentReadWorkerPermitWaitRecordsCancellationWithoutAcquisitionOrLeak failed under full-suite load in two separate full-suite runs on this branch and passed focused (24/24) with zero content change both times; the subsystem is untouched here. Flagging as a pre-existing load-sensitive flake.

Testing

• New/expanded unit coverage across the five commits: controller trust operations (25), gate core + coverage batch (20), FIFO gate ordering (21), card state, MCP lifecycle (11), MCP diagnostics/privacy, settings persistence + human-only rejection, plus empty-inventory compatibility across existing suites. Contract ledger verified.
• Full root suite, dev-lint, dev-format-check, both product builds, dev-codex-schema-check, and the repo pr-ready preflight lane green.
• Live end-to-end canary on the debug build: the gate blocked the first turn on a scratch repo with an untrusted PreToolUse hook; trust_all over MCP wrote the hooks.state entries and the deny hook then blocked bash -c true on the same live thread with its stderr reason — no rebind. Changing the hook declaration re-triggered review as modified; continue_without_hooks resumed the turn with the hook skipped and a counts-only transcript notice. On a real guarded infrastructure repo, both project hooks were trusted via the flow and terraform apply -help was then blocked by that repo's own guardrail gate while echo passed through the same chain — the original Agent Mode silently skips untrusted Codex project hooks in isolated CODEX_HOME — security hooks fail open #665 bypass is closed with no over-blocking.

Screenshots

Attached in the first comment: the settings card (default state, open picker, and global-on + workspace opt-out) and Agent Mode transcripts showing block-after-approval and the explicit-bypass notice.

Release note

Agent Mode may now pause before its first Codex turn to ask you to approve repository-declared Codex hooks (.codex config). Approvals persist per hook version; an opt-in strict mode (Settings → Agent Mode) requires approval and removes the continue-without-hooks option.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Fph5yrP8bV9yu3BjD2xh32

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4a7dcf0848

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[baron]

Blocking on two correctness/availability issues in the new security gate:

1. [P1] Re-check strict mode after the approval write suspends

   In CodexAgentModeCoordinator.swift:943-1046, a partial approveSelected is checked against strict mode before awaiting trustHooksForCurrentWorkspace. If strict mode is enabled while that request is suspended, the post-write path completes and memoizes the gate without re-reading the setting, leaving the intentionally skipped hooks unresolved.

   Before completeCodexHookReview, re-read the effective strict-mode value. If it is enabled and verified.unresolvedProjectHooks is nonempty, refresh the review from the verified inventory rather than resuming the first turn.

2. [P1] Add bounded recovery for an indefinitely unsettled trust write

   CodexHookTrustService.swift:35-61,126-137 deliberately sends config/batchWrite without a timeout, while CodexNativeSessionController.swift:852-905 holds both the controller-local mutex and the app-global trust-write mutex until that request settles. If the app server neither responds nor disconnects, one request blocks hook approval for every workspace indefinitely, and cancel/retry cannot recover.

   Preserve the no-late-write invariant, but add a settlement deadline that terminates the owning app-server process and awaits its exit before releasing serialization. Reconnect and use hooks/list as semantic authority. Please also add fault-injection coverage and settlement/lock-wait diagnostics.

The final commit does correctly address the earlier strict partial-selection and post-write inventory-drift findings. All hosted checks are green at head 5f7daf2b2b88a0ccf6acb5e6a30b1790efab6fef, but the two paths above should be resolved before merge.

[morluto]

Audit disposition — liveness/security-administration blocker (2026-08-14)

The approval model is strong in several respects, but config/batchWrite is awaited with no request timeout while both the controller-local operation mutex and the app-global trust-write mutex are held. If the app server stops responding, one operation can permanently wedge hook approval writes for every session in the process. Cancellation shielding does not retire work that never settles.

Please add a bounded settlement deadline, generation fencing, and a recoverable poisoned-lane/retirement policy before merging. Discovery and parsing should also conservatively distinguish an actually empty hook inventory from an incomplete or malformed inventory. Then rerun the live trust/modified/bypass scenarios.

[morluto]

Deep-review assessment — 2026-08-14

Disposition: liveness/security blocker. The fail-closed first-turn gate, correlated inventory review, post-write verification, strict-mode policy, and headless parity are strong. The trust-write path holds both the controller-local mutex and the process-global trust-write mutex while issuing config/batchWrite with no request timeout. If the app server never settles, one request can permanently wedge all future trust writes in the process; cancellation shielding still waits for the stuck work.

Use a bounded request and settlement deadline, generation-fence late completion, and define recovery for a timed-out/poisoned lane before releasing the locks. Also ensure malformed, oversized, or partially decoded hooks/list data cannot collapse to an apparently empty trusted inventory. These must be fixed before merging a security gate.