fix(codex): use repository hook root in linked worktrees - #2052
fix(codex): use repository hook root in linked worktrees#2052MuskanPaliwal wants to merge 32 commits into
Conversation
|
Hey @MuskanPaliwal, thanks again for your contribution, reviewed, this is really solid. A few things before merge:
Happy to merge once these are addressed. Thanks again! |
|
Hey @peyton-alt , thanks for the review. I pushed the follow-up changes in 7020927d6. Project-layer behavior: I tested Screenshots:
Code changes: Trust matching now parses the complete 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 |
|
@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. |
|
hey @peyton-alt , I have addressed all the review comments. Thanks for the review. Kindly re-review it. Thanks! |
|
@MuskanPaliwal Thanks again! I re-reviewed this and left a few more inline comments. |
|
hey @peyton-alt, the pr is ready for review. thanks! |
|
Hey! @MuskanPaliwal I reviewed again and left one more in-line comment. Everything else looks good! |
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
|
Hey @peyton-alt, kindly take a look. Thanks! |
|
Hi @peyton-alt let me know if anything else is pending here :)) |
|
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. 2. Cross-checkout write needs a gate. Measured: install from a linked worktree leaves 3. Containment stops at the checkout. 4. Nothing in CI catches this regressing. No Minor: |
|
Hey @Soph, yes that makes sense. let me split the pr and get back to you :)) |
|
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 Doctor, status, SessionStart, and hook discovery do not write anywhere. Commands that already mean "change this checkout", such as I took your request to pull out the write parts to mean removing the new cross-worktree write behavior. I did not make On the worktree traversal, the Codex-specific parser is gone. Codex discovery now uses On containment, the writable directory must be the current checkout's exact I also added the Codex integration test you asked for. It runs 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 The full local check now passes, including the pinned Codex test and the deterministic canaries. No model request or OpenAI credentials were used. |
763d1d0 to
978bc6b
Compare


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/listresponse 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
WorktreeHooksPathidentifies<current-checkout>/.codex/hooks.json. It is the only mutation target used by install, uninstall, presence, freshness, deselection, and clean flows.DiscoveredHooksPathidentifies the project hooks file Codex is expected to load. It is used only for read-only configuration and trust diagnostics.ResolveHookDiscoveryconsumes the sharedgitrepo.ResolveGitLayoutclassification rather than maintaining a Codex-specific.git,commondir, or worktree-marker parser..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.HookDiscoverycarries no write target, migration path, cleanup path, or lock path. ItsRepositoryWidebit 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, andentire cleanmutate only the current checkout's.codex/hooks.json. They do not create another checkout's.codexdirectory, 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 noentire-codex-hooks.lockin 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:
<current-checkout>/.codex/hooks.json;.codexdirectories, final-file symlinks, non-regular files, unrelated destinations, and Git-metadata redirection are rejected;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
.codexdirectory, requires a stable regular file, capshooks.jsonat 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 doctorreports the current-worktree path and Codex-discovered path, invalid or unresolved discovery, a missing project layer, managed hook drift, and trust gaps.entire statuskeeps current-checkout installation/freshness semantics separate from effective Codex discovery and exposes the mismatch under the existingcodex_hooksJSON object without changing the warning wording established in this PR.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.jsonand apply that commit to the primary checkout (typically by cherry-pick), or runentire enablefrom 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_LinkedWorktreeUsesPrimaryCheckoutis a no-model integration test pinned to@openai/codex@0.149.0. It:codex app-serverin the linked worktree with an isolatedCODEX_HOME;initialize,initialized, andhooks/listJSONL messages over stdio;sourcePath;warningsanderrors.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 checkverification is intentionally deferred to implementation-order item 9.