Skip to content

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

Open
MuskanPaliwal wants to merge 32 commits into
entireio:mainfrom
MuskanPaliwal:investigate-codex-linked-worktree-hooks
Open

fix(codex): use repository hook root in linked worktrees#2052
MuskanPaliwal wants to merge 32 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
Comment thread cmd/entire/cli/agent/codex/hooks_lock_test.go
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
@peyton-alt

Copy link
Copy Markdown
Contributor

@MuskanPaliwal Thanks again! I re-reviewed this and left a few more inline comments.

@MuskanPaliwal

Copy link
Copy Markdown
Contributor Author

hey @peyton-alt, the pr is ready for review. thanks!

@peyton-alt

Copy link
Copy Markdown
Contributor

Hey! @MuskanPaliwal I reviewed again and left one more in-line comment. Everything else looks good!

MuskanPaliwal and others added 2 commits August 22, 2026 00:23
uninstallDeselectedAgentHooks seeded from GetAgentsWithHooksInstalled,
which only reports Installed hooks. A linked worktree with Codex's
repository-wide hooks but no local .codex layer reports them as
Outdated instead, so deselecting Codex on that fresh-setup path left
the root-authoritative hooks.json in place for every worktree.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MG9uEFWJ1612CjBPPvAXSF
@MuskanPaliwal

Copy link
Copy Markdown
Contributor Author

Hey @peyton-alt, kindly take a look. Thanks!

@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 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

Hey @Soph, thanks for the detailed review. I went back through the implementation with your main concern in mind.

I agreed with the risky part you called out. Running Entire in one worktree must not leave a modified file in another worktree. I removed that behavior.

Entire now treats the checkout where the command runs as the write boundary. Git worktrees share objects, but they do not share working-tree state. The primary checkout may be on main with uncommitted changes while the linked worktree is on a feature branch. If entire enable in the feature worktree edits the primary checkout, it dirties a different branch without the user necessarily seeing it.

Doctor, status, SessionStart, and hook discovery do not write anywhere. Commands that already mean "change this checkout", such as entire enable, agent removal, and entire clean, still write to the current checkout. If Codex is reading hooks from the primary checkout, Entire reports that. The user can then run entire enable there or commit the local file and apply that commit to the primary checkout.

I took your request to pull out the write parts to mean removing the new cross-worktree write behavior. I did not make entire enable or entire clean read-only in linked worktrees. That would be a separate behavior change. If that is what you meant, I should handle it explicitly rather than claim this version does it.

On the worktree traversal, the Codex-specific parser is gone. Codex discovery now uses gitrepo.ResolveGitLayout, which uses the existing parsing in paths. Bare worktrees, linked submodules, contradictory metadata, and layouts we have not tested against Codex return an unresolved result. There is no guessed fallback.

On containment, the writable directory must be the current checkout's exact .codex directory. Entire rejects a redirected .codex, a destination inside Git metadata, a symlinked hooks.json, and a non-regular file. The two cases you found, .codex -> .git and .codex/hooks.json -> ../package.json, now fail without writing. Existing hook files keep their mode.

I also added the Codex integration test you asked for. It runs codex app-server hooks/list in a real linked worktree with @openai/codex@0.149.0. It checks both behaviors we observed. Without a local .codex directory, Codex returns no project hooks. After creating an empty local .codex directory, Codex returns the primary checkout's hook and sourcePath. CI requires that exact Codex version, so the test fails instead of skipping when the binary is missing or has the wrong version.

I fixed the smaller issues too. Linked submodules no longer fall back to a local answer. Bare-worktree behavior is unresolved until we pin it against Codex. Reads of both local and discovered hook files are capped at 1 MiB and reject symlinks, unstable files, and paths outside the expected .codex directory.

The full local check now passes, including the pinned Codex test and the deterministic canaries. No model request or OpenAI credentials were used.

@MuskanPaliwal
MuskanPaliwal force-pushed the investigate-codex-linked-worktree-hooks branch from 763d1d0 to 978bc6b Compare August 26, 2026 07:15
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