Skip to content

fix(codex): use repository hook root in linked worktrees - #2052

Merged
Soph merged 42 commits into
entireio:mainfrom
MuskanPaliwal:investigate-codex-linked-worktree-hooks
Aug 26, 2026
Merged

fix(codex): use repository hook root in linked worktrees#2052
Soph merged 42 commits into
entireio:mainfrom
MuskanPaliwal:investigate-codex-linked-worktree-hooks

Conversation

@MuskanPaliwal

@MuskanPaliwal MuskanPaliwal commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Fixes #2020.

Codex 0.149.0 can load project hooks from a different checkout than the one in which it is running. In a conventional linked worktree, its app-server hooks/list response reports the primary checkout's .codex/hooks.json; a divergent file present only in the linked worktree is not loaded.

This PR makes that effective location visible without giving Entire ownership of another checkout. Discovery and mutation are deliberately separate: Entire may inspect the file Codex actually loads, but installs, removes, and cleans hooks only in the exact checkout where the command was invoked.

Discovery and ownership

  • WorktreeHooksPath identifies <current-checkout>/.codex/hooks.json. It is the only mutation target used by install, uninstall, presence, freshness, deselection, and clean flows.
  • DiscoveredHooksPath identifies the project hooks file Codex is expected to load. It is used only for read-only configuration and trust diagnostics.
  • ResolveHookDiscovery consumes the shared gitrepo.ResolveGitLayout classification rather than maintaining a Codex-specific .git, commondir, or worktree-marker parser.
  • Conventional linked worktrees discover the primary checkout's hook file. Normal checkouts, ordinary submodules, and separate-Git-directory checkouts discover their own hook file.
  • .bare/worktrees, linked submodules, contradictory metadata, and other behavior not pinned against Codex are reported as unresolved. Entire does not claim an unsupported layout or guess a writable fallback.

HookDiscovery carries no write target, migration path, cleanup path, or lock path. Its RepositoryWide bit describes the reach of the configuration Codex discovers; it does not authorize repository-wide mutation.

Checkout-local mutation

entire enable, entire agent remove codex, agent deselection, and entire clean mutate only the current checkout's .codex/hooks.json. They do not create another checkout's .codex directory, migrate a worktree-local file into the primary checkout, remove a discovered file owned elsewhere, or perform repository-wide cleanup.

Concurrent updates are serialized only with <current-checkout>/.codex/hooks.json.lock. There is no entire-codex-hooks.lock in the Git common directory and no synchronization across worktrees. An old common-directory lock, if present from a previous build, is neither used nor cleaned from another checkout.

Current-checkout writes are hardened before the final atomic replacement:

  • the logical target must be exactly <current-checkout>/.codex/hooks.json;
  • redirected .codex directories, final-file symlinks, non-regular files, unrelated destinations, and Git-metadata redirection are rejected;
  • existing user hooks, unknown hook events, and unrelated top-level fields are preserved;
  • uninstall removes only Entire-managed entries;
  • an existing file keeps its permission bits, while a new file uses 0600.

Bounded read-only inspection

The local and discovered files are inspected independently as absent, user-only, Entire-managed, invalid, or unresolved. Inspection is contained to the exact canonical .codex directory, requires a stable regular file, caps hooks.json at 1 MiB, reads through a limit with one overflow byte, and checks file identity before and after the read. It never creates a directory, rewrites configuration, removes entries, or acquires a mutation lock.

The structured inspection also reports whether the core Entire events are present, which current events are missing, whether managed commands and timeouts match this CLI, whether the current checkout has the project layer Codex requires, whether local trust records were readable, and which declared hooks lack an approval record. Entire does not compute, copy, or synthesize Codex trust hashes.

Diagnostics and remedy

  • entire doctor reports the current-worktree path and Codex-discovered path, invalid or unresolved discovery, a missing project layer, managed hook drift, and trust gaps.
  • entire status keeps current-checkout installation/freshness semantics separate from effective Codex discovery and exposes the mismatch under the existing codex_hooks JSON object without changing the warning wording established in this PR.
  • SessionStart appends a short, bounded, read-only discovery or trust warning when applicable.

When hooks exist only in a linked worktree, Entire does not repair the primary checkout automatically. The user-owned remedy is to commit .codex/hooks.json and apply that commit to the primary checkout (typically by cherry-pick), or run entire enable from the primary checkout. Because linked worktrees share Git objects, the commit is immediately available to cherry-pick; no push or fetch is required.

Pinned Codex integration contract

TestCodexAppServerHooksList_LinkedWorktreeUsesPrimaryCheckout is a no-model integration test pinned to @openai/codex@0.149.0. It:

  1. creates a normal repository and conventional linked worktree with distinct hook markers;
  2. starts codex app-server in the linked worktree with an isolated CODEX_HOME;
  3. sends initialize, initialized, and hooks/list JSONL messages over stdio;
  4. requires the primary marker and primary sourcePath;
  5. rejects the linked-only marker; and
  6. requires empty warnings and errors.

The test makes no model request and needs no OpenAI credentials. Local integration runs skip when the exact Codex binary is unavailable. CI installs version 0.149.0 and sets ENTIRE_TEST_REQUIRE_CODEX_APP_SERVER=1, so a missing binary or version drift fails rather than silently skipping.

Verification status

The implementation includes focused unit coverage for Git-layout classification, path-role separation, current-checkout-only install/remove/clean behavior, exact-worktree locking, destination containment, symlink refusal, bounded reads, mode preservation, doctor/status/SessionStart diagnostics, trust inspection, and JSON output, plus the pinned app-server integration contract above.

Full formatting, lint, unit, integration, canary, duplication, and mise run check verification is intentionally deferred to implementation-order item 9.

@MuskanPaliwal
MuskanPaliwal marked this pull request as ready for review August 19, 2026 10:40
@MuskanPaliwal
MuskanPaliwal requested review from a team as code owners August 19, 2026 10:40
@peyton-alt

Copy link
Copy Markdown
Contributor

Hey @MuskanPaliwal, thanks again for your contribution, reviewed, this is really solid. A few things before merge:

  1. Project-layer premise — ProjectLayerExists assumes a linked checkout needs its own local .codex dir for Codex to discover the shared hooks. That claim gates AreHooksInstalled and a doctor state, and it isn't in the issue. Can you point at the codex-rs source for it, or share a hooks/list run from a linked checkout without a local .codex dir?
  2. Trust-key paths — trust matching prefixes on the symlink-resolved hooks.json path. If Codex writes its trust keys with the un-resolved path (macOS /var vs /private/var, symlinked checkouts), we'd report false trust gaps. Can you confirm which form Codex writes and pin it with a test?
  3. Lock wait is unbounded — acquireHooksLock retries until the context cancels, so a wedged holder hangs entire enable silently. Please bound the wait and return a clear error instead.
  4. Merge conflicts

Happy to merge once these are addressed. Thanks again!

@MuskanPaliwal

MuskanPaliwal commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Hey @peyton-alt , thanks for the review. I pushed the follow-up changes in 7020927d6.

Project-layer behavior: I tested hooks/list from a conventional linked worktree using the latest stable Codex, 0.148.0. With no local .codex directory, Codex returned hooks: []. After creating an empty local .codex directory, it found the hook from the primary checkout’s .codex/hooks.json and reported it as trustStatus: "untrusted". Everything else was kept unchanged. This confirms that the local directory enables project-layer discovery, while the hook configuration remains root-authoritative and approval remains separate. I have screenshots of the version, filesystem states, and raw responses.

Screenshots:

Screenshot 2026-08-20 at 12 21 07 PM Screenshot 2026-08-20 at 12 20 58 PM

Code changes: Trust matching now parses the complete <path>:<event>:<group>:<handler> key, validates the indexes, and compares canonicalized paths. The regression test covers a logical nested-symlink path against the physical authoritative path. Lock acquisition now times out after five seconds with a clear error, while caller cancellation still returns context.Canceled.

I also fixed three related cases found while rechecking this path: migration no longer risks deleting an aliased shared hook file, unknown and user-defined JSON fields are preserved, and uninstall can find removable shared hooks when the linked checkout lacks its local project layer.

Current main is merged, and the seven-hook set from #1958 remains intact, including SubagentStart and SubagentStop. The merged head passes mise run check, including race-enabled unit and integration tests, all 56 Vogon canaries, and all 4 Roger-Roger canaries.

Comment thread cmd/entire/cli/agent/codex/hook_root.go Outdated
Comment thread cmd/entire/cli/agent/codex/hook_root.go Outdated
Comment thread cmd/entire/cli/agent/codex/hooks.go Outdated
Comment thread cmd/entire/cli/agent/codex/hooks.go Outdated
Comment thread cmd/entire/cli/agent/codex/hooks.go Outdated
Comment thread cmd/entire/cli/agent/codex/hooks_lock_test.go Outdated
Comment thread cmd/entire/cli/config.go
Comment thread cmd/entire/cli/doctor.go Outdated
Comment thread cmd/entire/cli/setup.go Outdated
Comment thread cmd/entire/cli/agent/testutil/git.go Outdated
@peyton-alt

peyton-alt commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

@MuskanPaliwal thanks for the edits! I left a batch of inline comments. Only the hook_root.go (the derived hook root can land on CODEX_HOME or outside the repo entirely)one blocks, the rest are small.

@MuskanPaliwal

Copy link
Copy Markdown
Contributor Author

@MuskanPaliwal thanks for the edits! I left a batch of inline comments. Only the hook_root.go (the derived hook root can land on CODEX_HOME or outside the repo entirely)one blocks, the rest are small.

on it.

@MuskanPaliwal

Copy link
Copy Markdown
Contributor Author

hey @peyton-alt , I have addressed all the review comments. Thanks for the review. Kindly re-review it. Thanks!

Comment thread cmd/entire/cli/agent/codex/hook_root.go
Comment thread cmd/entire/cli/agent/codex/hooks.go Outdated
Comment thread cmd/entire/cli/setup.go
Comment thread cmd/entire/cli/setup.go Outdated
@MuskanPaliwal

Copy link
Copy Markdown
Contributor Author

Hi @peyton-alt let me know if anything else is pending here :))

@Soph

Soph commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Hey @MuskanPaliwal thanks for doing this. I had a look at this today, and I would like to ask if we can reduce the scope of this PR and maybe split it. What I like to keep is resolver + inspection + warnings but pull all the write parts out for now. I think the main issue here is surfacing that something is wrong but trying to fix it for the user might be a bit too much.

Like I if the user adds this to a worktree but then commits he can just bring that commit to his root worktree and things work. Writing across worktrees - with the potential of him missing the change in the root tree completely - is to much work from my point of view for something we just need to surface more properly.

On top of the split I have these findings / asks:

1. Fourth implementation of worktree traversal. gitrepo.resolveDotGitPath (gitrepo/repository.go:155) + resolveCommonGitPath (:183) already parse gitdir: and commondir; paths.parseWorktreeID already handles .git/worktrees/, .bare/worktrees/ and the submodule /modules/ carve-out; session.GetGitCommonDir (session/state.go:1021) is memoized. The problem isn't line count — parseWorktreeID treats .bare/worktrees/ as a linked worktree while ownsCommonDir makes bare fall back to worktree-local. Two answers in one repo, and dupl won't flag it. Reusing gitrepo's primitives also gets memoization, which matters since ResolveHookLocation is now on the SessionStart path via HookTrustGaps.

2. Cross-checkout write needs a gate. Measured: install from a linked worktree leaves M .codex/hooks.json in the primary checkout and resets its mode 0644 → 0600 (WriteFileAtomic chmods unconditionally, hooks.go:460; the os.WriteFile it replaced didn't). Worse in the removal direction — entire clean in a scratch worktree strips capture repo-wide, and writeSharedHooksRemovalNote prints after the confirm prompt in runUninstall. Suggest: scope text into the prompt, write behind a flag.

3. Containment stops at the checkout. hook_path.go:67/:95 accept any target under checkoutRoot, and .git/ qualifies. Measured with committed symlinks: .codex -> .git wrote .git/hooks.json; .codex/hooks.json -> ../package.json rewrote package.json at 0600. Both commit and clone cleanly. Still better than main (bare os.WriteFile followed symlinks anywhere) — just short. Fix is already in the PR: run the resolved destination through isInsideGitMetadata (hook_root.go:386) and require the project dir to be exactly <checkoutRoot>/.codex.

4. Nothing in CI catches this regressing. No e2e/ or integration_test/ file touched, and no e2e test runs in a linked worktree — every "worktree" mention in e2e/tests/ is a prompt saying "Do not use worktrees." The unit tests assert the resolver against our beliefs about Codex; the only anchor to Codex's real behavior is the comment at hook_root_test.go:115. Can we get one integration test driving codex app-server's hooks/list in a linked worktree?

Minor: ErrLinkedSubmoduleHooksUnsupported in the description doesn't exist (linked submodules fall back to worktree-local, no error), and the bare-layout description contradicts TestResolveHookLocation_BareRepositoryWorktreeUsesWorktreeLocalHooks#2020 saw the opposite on 0.147-era vs 0.149 in the comment, worth pinning down. Also hooks.go:341 / trust.go:96 read the repo-controlled hooks file unbounded while hook_root.go caps git metadata at 64KiB.

@MuskanPaliwal

MuskanPaliwal commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Hey @Soph, yes that makes sense. let me split the pr and get back to you :))

@MuskanPaliwal
MuskanPaliwal force-pushed the investigate-codex-linked-worktree-hooks branch from 763d1d0 to 978bc6b Compare August 26, 2026 07:15
@MuskanPaliwal

MuskanPaliwal commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

Hey @Soph, thanks for the detailed review. I reduced the scope as requested and pushed the follow-up changes in 5690551ce (on top of 8bd2757e4).

The resolver remains read-only. Install, uninstall, and clean only modify the checkout where the command runs; when that differs from the file Codex reads, setup, removal, doctor, status, and SessionStart warn instead of writing across checkouts.

I also clarified the warnings so they name the exact files involved: the current worktree's .codex/hooks.json and Codex's discovered .codex/hooks.json. They now explicitly say that hooks in the current-worktree file will not run when the paths differ. For a normal checkout, the user can run entire enable from the discovered checkout. For a .bare layout, the discovered project root is not a worktree, so the generated .codex/hooks.json change must be applied or merged there instead; entire enable should not be run from that root.

I added a doctor regression covering that .bare warning, kept the bounded read-only diagnostics, and retained the pinned Codex 0.149.0 app-server integration coverage. The tests drive hooks/list in both a conventional linked worktree and a .bare worktree, asserting the actual source path and rejecting the linked-worktree marker. CI verifies the installed Codex version and requires those tests.

Validation: focused Codex/doctor tests pass, git diff --check passes, and mise run lint passes with 0 issues. The local full CLI suite is blocked by this sandbox's inability to bind httptest localhost ports, and the local app-server tests skip when the pinned Codex binary is unavailable; CI is configured to fail rather than skip in that case. I did not run paid-agent E2E tests.

Screenshot 2026-08-26 at 8 28 51 PM

@MuskanPaliwal
MuskanPaliwal force-pushed the investigate-codex-linked-worktree-hooks branch from 5690551 to b810240 Compare August 26, 2026 12:27
@MuskanPaliwal

Copy link
Copy Markdown
Contributor Author

On top of this, I’m working on a broader consolidation of the Git traversal logic in a follow-up PR.

@Soph

Soph commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Ran this against f5b1be33 in an isolated repo with a primary checkout plus a linked worktree. The state machine and all three surfaces work; two changes.

1. There is no healthy state for a linked worktree.

Repro: entire enable --agent codex from the linked worktree, then commit .codex/hooks.json and merge it into the primary - the user does exactly the right thing and hooks genuinely fire:

Codex hooks: CURRENT-WORKTREE FILE NOT DISCOVERED
  Current-worktree hooks:  .../wt/.codex/hooks.json
  Codex-discovered hooks:  .../primary/.codex/hooks.json
  Codex is active from the discovered file; changes to this worktree's file do not affect it.
  Apply/merge the generated .codex/hooks.json change into the discovered project root.

entire status: ! Codex ignores this worktree's hooks file · run 'entire doctor'

The working configuration and the broken one differ only in which warning you get, and the remedy points at a merge that already happened.

codexHookStateWorktreePathNotDiscovered is the correct steady state for every linked worktree, not a fault. Codex resolves hooks to the root checkout deliberately - root_checkout_hooks_folder_for_dir in codex-rs/config/src/loader/mod.rs, added by openai/codex#21969 ("Use root repo hooks in linked worktrees", 2026-05-13). That PR rejected the per-worktree alternative on review because divergent worktree hook definitions would fight over one trusted_hash, so it's stable and not getting reverted.

Suggest splitting the state at doctor.go:750: when the discovered file exists, is Entire-managed, and is current, report success -

✓ Codex hooks: ACTIVE (via root checkout)
    .../primary/.codex/hooks.json

Keep the warning for missing / not-Entire-managed; stale is already codexHookStateOutdated. Same split for the status one-liner at codex_hook_diagnostics.go:152.

2. Trim the remedy to the one thing that applies.

.codex/hooks.json is a tracked file, so delivery is git, not a filesystem write - the same way .github/workflows/*.yml only take effect once merged to the default branch.

Both remedy blocks currently print two hedged lines unconditionally:

If that root is a Git checkout, run `entire enable` from that checkout.
In a .bare layout, the discovered project root is not a worktree; apply the file there instead.

Every linked-worktree user sees the .bare line whether or not it applies, and the "If that root is a Git checkout" line hedges over a distinction the code already makes - commonGitDirIsBare (hook_root.go:159) and hook_root.go:100 both detect it. Suggest dropping both lines from writeCodexPrimaryCheckoutRemedy (doctor.go:850) and from warnCodexHooksAfterSetup (setup.go:1698-1703), leaving:

  Codex will read the discovered file above, not the current-worktree file above.
  .codex/hooks.json is tracked - commit it and make sure the root worktree has it
  (merge to the default branch, or check that branch out there).

Anyone running a .bare layout already knows their project root isn't a checkout and will draw the right conclusion from "make sure the root worktree has it"; everyone else just loses a line about a layout they don't use. If it's ever worth saying explicitly, gate it on commonGitDirIsBare rather than hedging in prose.

Same trim in warnCodexHooksAfterRemoval (setup.go:1706-1719).

Verification: post-merge repro prints ✓ Codex hooks: ACTIVE (via root checkout) with no ! in status; enable and doctor each drop two lines.

@Soph Soph left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

and thanks for continuing! :)

Comment thread .github/workflows/ci.yml Outdated
@MuskanPaliwal

Copy link
Copy Markdown
Contributor Author

Hey @Soph , kindly re-review. Thanks :)

@Soph Soph left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks a lot. Looks good!

@Soph
Soph enabled auto-merge August 26, 2026 20:46
@Soph
Soph merged commit 8f40d2c into entireio:main Aug 26, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Codex sessions are not captured in linked worktrees because hooks are installed in an ignored worktree-local location

3 participants