Skip to content

fix(codex): write Orca's hook into ~/.codex only when something changed - #25743

Merged
Jinwoo-H merged 29 commits into
mainfrom
jinwoo/codex-hook-c-real-home-reconcile
Oct 6, 2026
Merged

Jinwoo-H merged 29 commits into
mainfrom
jinwoo/codex-hook-c-real-home-reconcile

Conversation

@Jinwoo-H

@Jinwoo-H Jinwoo-H commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 38 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​2339 $\color{#cf222e}{\Huge{\mathbf{−}}}$​3174 $\color{#cf222e}{\Huge{\mathbf{−}}}$​835
Prod 30 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​798 $\color{#cf222e}{\Huge{\mathbf{−}}}$​982 $\color{#cf222e}{\Huge{\mathbf{−}}}$​184

Stack: #25741 (merged) → #25742 (merged) → #25743 (~/.codex written only on change) → #25744 (cleanup). This is part 3 of 4; review and merge in order. Supersedes #24234.

ELI5

Codex asks you to review any hook in ~/.codex/hooks.json that it has no approval for. Today Orca rewrites its status hook there on every Codex launch and resume, then approves it by running a background Codex session. When many agents launch at once, those writes race, and a launch can reach Codex before the approval does. After this change, Orca checks its hook against what it would write, and touches ~/.codex only when something actually changed.

What Changed

  • Before:
    • Each Codex launch or resume on ~/.codex re-added Orca's hook entry, then started codex app-server in the background, for up to 30 s, to approve it.
    • Until the approval landed, or whenever the status hook could not be trusted, new terminals for the system-default account were silently moved to Orca's own Codex home, so they could run under a stale copy of your login.
    • Parallel launches raced on hooks.json and config.toml.
  • After:
    • One check, written only on change. It runs at app start (after the shell PATH loads), when you turn hooks on, once per terminal you open, and on Codex launches and resumes (which wait at most 3 s). When nothing changed, it only reads files and starts nothing. While Codex's live answer is still coming (for example, a slow codex --version at app start), it uses the answer Codex gave last time for the same binary, so a restart writes nothing.
    • No Codex, no writes. When Orca can't find Codex, it doesn't create or change ~/.codex; status says Codex was not found.
    • Codex's own fingerprint, approval first. It uses the fingerprint Codex itself reported (from the previous PR) and writes the approval before the entry. If the entry write fails, it takes the approval back. A /hooks switch-off of Orca's hook is turned back on, because Orca's setting is its off switch.
    • One copy, in its own group. Orca's entry lives alone in its own group with no matcher, last in the list, or kept where it already sits. Copies anywhere else are removed, including inside your own matcher groups and a hook that runs Orca's script through its arguments (turning hooks off already removed those). Your hooks' approvals move with them, under both spellings of a symlinked ~/.codex. An Orca entry you edited is rewritten in place and re-approved.
    • A hook problem costs only status. It never changes which Codex home or login a terminal uses: the system-default account always runs on ~/.codex. When ~/.codex/hooks.json has a shape Orca can't add to (it doesn't parse, has top-level keys other than hooks and description, or an event that isn't a list), Orca leaves it untouched and status says why.
    • Status reports on the home your next terminal uses. It says why there's no status: Codex too old, Codex not found, not asked yet, approved by Orca but not yet confirmed by Codex, a hooks.json problem, or hook approvals kept inline in config.toml.
    • Removed: the background approval session for ~/.codex, its withdrawal of unapproved entries, and the per-launch writer. WSL keeps its approval session.

Why

  • Writing only on change is what removes the race. Most launches now find nothing to do. Computing the fingerprint in Orca broke before (Fix Codex hook trust hash for matcherless events (UserPromptSubmit/Stop) #7110); asking Codex once per version avoids that.
  • No check after writing. The fingerprint comes from the same Codex for the same entry. An edited entry is caught by comparing the whole entry, and a dropped approval is fixed by the next check. A real-Codex contract in CI pins the behavior, on two pinned versions and latest.
  • Why one copy in its own group. Codex's fingerprint includes a group's matcher, so a copy inside your matcher group could never be approved with Orca's fingerprint.

Linked Issue

Fixes #13746

Visual Proof

N/A: no UI change. Only the hook status detail text differs.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated
    • Unit:
      • nothing changed means no writes and no Codex start, across 20 concurrent launches;
      • an inserted user hook, a Codex fingerprint change, a /hooks switch-off, Orca inside a matcher group, an edited entry;
      • older-build entries converted only at app start or when hooks turn on, at the first check that writes ~/.codex (so not lost while hooks are off or a managed account is selected);
      • no ~/.codex writes while Codex is missing, and none on a restart with a slow version check;
      • a broken hooks.json, an unknown Codex, a too-old Codex, or inline approvals keep the system default on ~/.codex, and hooks.json is left untouched;
      • a description key in hooks.json survives Orca's write;
      • a concurrent hooks.json save is retried;
      • managed accounts and custom CODEX_HOME leave ~/.codex alone;
      • turning hooks off removes both spellings and older Codex fingerprints;
      • status reasons.
    • Real Codex (CI on pinned and latest; locally 0.160.1 with throwaway homes):
      • Codex lists the entry as trusted and enabled;
      • a /hooks switch-off is undone;
      • an inserted hook shows for review until the next check;
      • an inline config.toml stays loadable;
      • a real TUI start shows no review screen and the hook posts.

Notes

  • Cross-platform: paths go through the existing helpers. A spawn triggers the check only for native terminals, never WSL.
  • SSH / WSL: unchanged; their own installers are follow-ups.
  • Less status than before in a few cases:
    • approvals kept inline in config.toml (status explains why);
    • Codex older than hooks/list (status says to update);
    • (closed) until Codex answers, Orca approves its own entry with its own hash, as before.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

One correctness finding in the pending-Codex stopgap path that can defeat the purpose of this stack — inline comment below.

Reviewed changes — first pass over the 8 commits from 256e90a to 532078b.

  • New central reconcile (codex-hook-reconcile.ts): the single writer of Orca's ~/.codex entry, with a coalescing runner, an off-spawn schedule for every native pane, and a bounded wait for Codex launches/resumes. When Codex has not answered within 500ms it writes a stopgap approval from Orca's own hash and re-runs once Codex answers.
  • Install rewrite (codex-real-home-hook-install.ts): ensureRealHomeCodexHookState becomes reconcileRealHomeCodexHookEntries with unchanged/written/unavailable/disabled outcomes, a prune-then-write loop (MAX_PASSES), and a one-time retry on HooksJsonChangedError.
  • Entry plan rewrite (codex-real-home-hook-entry-plan.ts): Orca's entry must be alone in a matcher-less group; duplicate/direct copies are pruned and user-hook positions are preserved via mutateRealHomeHooksPreservingUserTrust.
  • Removals: codex-real-home-background-grant.ts, codex-real-home-entry-trust.ts, codex-real-home-hook-withdrawal.ts, runOutsideCodexTrustConfigLanes, and their tests — no dangling references remain at HEAD; tc:node and tc:cli pass.
  • Status/routing wiring: no-arg getStatus() now resolves the home the next pane uses; the real-home lane gate is now !hooksEnabled || readRealHomeHooksFileShapeProblem() === null.

ℹ️ The PR description no longer matches the change

The body describes removing background-approval options and says "Nothing a user sees changes", but the diff adds a new reconcile that writes Orca's entry into ~/.codex on every launch/resume and a status path that now reports the real home. That is precisely the user-visible behavior this PR governs, so the description under-sells (and misdirects reviewers away from) the actual surface. Worth rewriting before merge.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@Jinwoo-H
Jinwoo-H force-pushed the jinwoo/codex-hook-b-codex-derived-hash branch from 256e90a to 9d5efd6 Compare October 6, 2026 02:45
@Jinwoo-H
Jinwoo-H force-pushed the jinwoo/codex-hook-c-real-home-reconcile branch from 532078b to b4a574a Compare October 6, 2026 02:45
@Jinwoo-H
Jinwoo-H force-pushed the jinwoo/codex-hook-b-codex-derived-hash branch from 9d5efd6 to 668ae79 Compare October 6, 2026 04:11
@Jinwoo-H
Jinwoo-H force-pushed the jinwoo/codex-hook-c-real-home-reconcile branch 3 times, most recently from 5da068f to ff3652c Compare October 6, 2026 07:08
@Jinwoo-H
Jinwoo-H force-pushed the jinwoo/codex-hook-b-codex-derived-hash branch from 9a95263 to 4c50d0e Compare October 6, 2026 07:08
@Jinwoo-H
Jinwoo-H force-pushed the jinwoo/codex-hook-c-real-home-reconcile branch 3 times, most recently from ed88c6d to 26945c1 Compare October 6, 2026 18:19
@Jinwoo-H
Jinwoo-H requested a review from brennanb2025 as a code owner October 6, 2026 18:19
@Jinwoo-H
Jinwoo-H changed the base branch from jinwoo/codex-hook-b-codex-derived-hash to main October 6, 2026 18:19
@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: fd4611a8-5e95-4b2c-80da-ead3fbdaf145
📥 Commits

Reviewing files that changed from the base of the PR and between 5eeb083f31c416d77a0b0d291a66d0e04bbfa11d and c0266f3.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: dc964301-2d8c-4ef1-ac7d-b9ef5233e520
📥 Commits

Reviewing files that changed from the base of the PR and between 26945c19273e9fbfc30f3efe17d7733a879812c1 and 5eeb083f31c416d77a0b0d291a66d0e04bbfa11d.

📒 Files selected for processing (2)
  • src/main/codex/codex-real-home-hook-entries.test.ts
  • src/main/codex/codex-real-home-hook-install.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/main/codex/codex-real-home-hook-entries.test.ts
  • src/main/codex/codex-real-home-hook-install.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The PR replaces the real-home Codex hook grant flow with hash-based hook reconciliation. Reconciliation runs at startup and native pane spawns, and selected real-home launches and resumes can await it. Hook status now supports real and managed homes and reports file and inline-approval conditions. The PR updates opt-out trust cleanup, hook planning, and rollback checks. It also adds reconciliation and status tests, removes tests for the earlier grant flow, and updates startup and project configuration.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 5eeb0

No confirmed issue in the selected changes currently prevents merging after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 43.53% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 85 functions across 49 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: Orca writes its hook to ~/.codex only when needed.
Description check ✅ Passed The description covers the main template sections and gives detailed scope, rationale, linked issue, visual-proof status, and automated testing. Manual testing is unchecked, and the Review, Agent skil…
Linked Issues check ✅ Passed [#13746] The new reconciliation removes hook-trust availability as a routing condition. The regression test covers old or missing Codex, invalid or unsupported hooks.json, and inline approvals; in eac…
Out of Scope Changes check ✅ Passed The production changes and related tests implement [#13746]’s separation of optional hook status from system-default home routing, plus reconciliation and cleanup of Orca hook entries. Startup, PTY wi…
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e4414838-ff4b-4b34-a2f1-74db9f58b0a2
📥 Commits

Reviewing files that changed from the base of the PR and between 8d049b5 and 26945c19273e9fbfc30f3efe17d7733a879812c1.

📒 Files selected for processing (68)
  • config/scripts/ci-shard-timings.json
  • config/scripts/pr-code-change-scope.mjs
  • config/scripts/pr-code-change-scope.test.mjs
  • config/tsconfig.cli.json
  • src/main/agent-hooks/managed-agent-hook-registry.ts
  • src/main/codex-accounts/runtime-home-real-home-lane-routing.test.ts
  • src/main/codex-accounts/runtime-home-retained-auth-provenance.test.ts
  • src/main/codex-accounts/runtime-home-service-home-routing.ts
  • src/main/codex-accounts/runtime-home-service-per-account-migration.test.ts
  • src/main/codex-accounts/runtime-home-session-migration-pass.test.ts
  • src/main/codex-accounts/runtime-home-status-hook-independence.test.ts
  • src/main/codex/codex-hook-approval-first-write.test.ts
  • src/main/codex/codex-hook-approval-first-write.ts
  • src/main/codex/codex-hook-command-form.test.ts
  • src/main/codex/codex-hook-file-entry-binary-contract.test.ts
  • src/main/codex/codex-hook-hash-lookup.test.ts
  • src/main/codex/codex-hook-hash-lookup.ts
  • src/main/codex/codex-hook-legacy-cleanup.ts
  • src/main/codex/codex-hook-local-install.ts
  • src/main/codex/codex-hook-local-maintenance.ts
  • src/main/codex/codex-hook-orca-approvals.ts
  • src/main/codex/codex-hook-real-home-status.test.ts
  • src/main/codex/codex-hook-reconcile.test.ts
  • src/main/codex/codex-hook-reconcile.ts
  • src/main/codex/codex-hook-service-implementation.ts
  • src/main/codex/codex-hook-status.ts
  • src/main/codex/codex-hook-trust-cleanup.ts
  • src/main/codex/codex-hook-trust-derivation.ts
  • src/main/codex/codex-hook-trust-failure-memo.test.ts
  • src/main/codex/codex-real-home-background-grant.ts
  • src/main/codex/codex-real-home-entry-trust.test.ts
  • src/main/codex/codex-real-home-entry-trust.ts
  • src/main/codex/codex-real-home-failed-grant-concurrent-edit.test.ts
  • src/main/codex/codex-real-home-frozen-entry.test.ts
  • src/main/codex/codex-real-home-hook-entries.test.ts
  • src/main/codex/codex-real-home-hook-entry-plan.ts
  • src/main/codex/codex-real-home-hook-install.test.ts
  • src/main/codex/codex-real-home-hook-install.ts
  • src/main/codex/codex-real-home-hook-key-spellings.test.ts
  • src/main/codex/codex-real-home-hook-removal.test.ts
  • src/main/codex/codex-real-home-hook-sweep.ts
  • src/main/codex/codex-real-home-hook-withdrawal.ts
  • src/main/codex/codex-real-home-hooks-json.ts
  • src/main/codex/codex-real-home-shared-entry.test.ts
  • src/main/codex/codex-real-home-slow-app-server.test.ts
  • src/main/codex/codex-real-home-trust-withdrawal.test.ts
  • src/main/codex/codex-real-home-user-hook-positions.test.ts
  • src/main/codex/codex-real-home-windows-key-spellings.test.ts
  • src/main/codex/codex-trust-config-mutation-queue.ts
  • src/main/codex/codex-trust-grant-ledger.test.ts
  • src/main/codex/codex-trust-grant-ledger.ts
  • src/main/codex/hook-service-legacy-cleanup.test.ts
  • src/main/codex/hook-service-managed-codex-hashes.test.ts
  • src/main/codex/hook-service-runtime-trust-repair.test.ts
  • src/main/codex/hook-service-windows-key-spellings.test.ts
  • src/main/codex/windows-hook-upgrade.test.ts
  • src/main/ipc/pty/host-env/assembly.ts
  • src/main/ipc/pty/host-env/codex-hook-reconcile-spawn.test.ts
  • src/main/startup/codex-account-restart-resume.test.ts
  • src/main/startup/codex-hook-hash-warmup-wiring.test.ts
  • src/main/startup/codex-hook-startup-wiring.test.ts
  • src/main/startup/codex-launch-during-real-home-grant.test.ts
  • src/main/startup/codex-launch-per-agent-hook-opt-out.test.ts
  • src/main/startup/codex-launch-preparation.ts
  • src/main/startup/codex-session-resume-daemon-guard.test.ts
  • src/main/startup/codex-session-resume-launch.ts
  • src/main/startup/main-process-account-services.ts
  • src/main/startup/main-process-ready-runtime.ts
💤 Files with no reviewable changes (18)
  • src/main/codex/codex-real-home-hook-install.test.ts
  • src/main/startup/codex-hook-hash-warmup-wiring.test.ts
  • src/main/codex/codex-real-home-failed-grant-concurrent-edit.test.ts
  • src/main/codex/codex-real-home-trust-withdrawal.test.ts
  • src/main/startup/codex-launch-during-real-home-grant.test.ts
  • src/main/codex/codex-real-home-user-hook-positions.test.ts
  • src/main/codex-accounts/runtime-home-service-home-routing.ts
  • src/main/codex/codex-real-home-entry-trust.ts
  • src/main/codex/codex-real-home-entry-trust.test.ts
  • src/main/codex/codex-real-home-frozen-entry.test.ts
  • src/main/codex/codex-real-home-slow-app-server.test.ts
  • config/scripts/ci-shard-timings.json
  • src/main/codex-accounts/runtime-home-retained-auth-provenance.test.ts
  • src/main/startup/main-process-account-services.ts
  • src/main/codex/codex-trust-config-mutation-queue.ts
  • src/main/codex/codex-real-home-hook-withdrawal.ts
  • src/main/codex/codex-real-home-background-grant.ts
  • src/main/codex-accounts/runtime-home-real-home-lane-routing.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

One reconcile replaces the per-launch writer and its background approval
session. It runs at app start after PATH hydration, when the setting turns on,
once per native pane spawn, and (with a bounded 3 s wait) on Codex launches and
resumes. Orca's entry lives alone in a matcherless group, appended last unless
already in place; other copies are removed and the shifted user approvals move
verbatim under both key spellings. The approval, with Codex's own hash, goes in
first. The routing gate closes only for a hooks.json with a bad shape.
Status reads ~/.codex (both key spellings) when launches use it, or the CLI
has no selection, and the selected managed home otherwise. It explains: update
Codex, Codex not found, not asked yet, approved by Orca but not yet confirmed
by Codex, a hooks.json shape that moves panes to Orca's own home, and inline
config.toml approvals Orca cannot add to.
…it on those files

The contract now also writes Orca's entry into a throwaway ~/.codex and checks
that Codex lists it trusted and enabled, turns it back on over a /hooks
switch-off, lists it for review after a user inserts a hook ahead until the next
check, keeps an inline config.toml loadable, and shows no review in a real TUI
start. The real-binary CI job now runs when the ~/.codex writer changes.
…spawn trigger

Adds tests that failed against mutants which survived the first pass: an
entry alone in a matcher group, a current copy's approval in an event left to
an older build, one run per spawn, a spawn riding a running reconcile, and the
native-only spawn trigger. The opt-out's Codex-hash cleanup now runs once,
after the sweep, instead of twice.
…r is pending

The reconcile waits at most 0.5 s for Codex's hash. If the lookup is still
running, an entry already in place keeps its approval and nothing is written;
a missing entry or approval gets Orca's computed hash, approval first, inside a
launch's 3 s wait, as main's did. When the lookup lands, the reconcile runs
again and Codex's hash replaces it.
…cile

startCodexHooks replaces the two start functions and takes the PATH wait that
the reconcile request's after option carried. A spawn reconciles unless one is
running, without a scheduled flag, launches call one reconcileCodexHooksForLaunch
on the shared withTimeout, and the reconcile alone reads the hooks setting. The
startup test now runs the ready phase instead of matching its source text.
…pproval reader

The planner prunes with removeManagedCommands, as the opt-out does (so a hook
that runs Orca's script through its args goes too), and returns a prune or
settle union. While Codex has not answered, the kept approval comes from the
shared per-event reader. The reconcile result is its outcome alone, a
concurrent save spends a pass of the one bound, the opt-out removes Orca's
approvals once, the approval-first writer takes the hooks path, and
getRealHomeConfigTomlPath gives way to getSystemCodexConfigTomlPath.
…its own home, hash and user data

Also passes every approval to the approval-first writer, which already skips the ones in place.
…swer flag

The lookup start only records the hydrated PATH; one catch, one request type, and the app config is kept as given.
Also makes the ~/.codex opt-out take Codex's hashes, as every production caller passes them.
…nswer uses the saved one, a conversion waits for a run that writes, and status and the reconcile read one home and one Orca-hash rule
…~/.codex rewrites the backslash key Codex on Windows reads
@Jinwoo-H
Jinwoo-H force-pushed the jinwoo/codex-hook-c-real-home-reconcile branch from 5eeb083 to c0266f3 Compare October 6, 2026 19:24
@Jinwoo-H
Jinwoo-H merged commit ac8ea9f into main Oct 6, 2026
43 checks passed
brennanb2025 added a commit that referenced this pull request Oct 6, 2026
…de-probes

codex-hook-trust-grant.ts takes main's side: #25743/#25744 removed the background
trust-grant timeout whose comment PR 2 had updated, so that comment change is
dropped. tsconfig.cli.json keeps main's edits plus PR 2's teardown-chain entries,
without the two real-home grant files main deleted.
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.

[Bug]: System-default Codex can use stale managed credentials when status-hook trust is unavailable

1 participant