fix(hooks): keep checkpoint delivery working when Lefthook reclaims hooks - #2365
Closed
peyton-alt wants to merge 19 commits into
Closed
fix(hooks): keep checkpoint delivery working when Lefthook reclaims hooks#2365peyton-alt wants to merge 19 commits into
peyton-alt wants to merge 19 commits into
Conversation
Entire-Checkpoint: 01M249K6A6PDMRNWJNP2A7CSE4
Entire-Checkpoint: 01M24BJBM0GWCV9BZ1BHBZT784
Entire-Checkpoint: 01M24E35K7XAGQK0557PTKTYCD
Entire-Checkpoint: 01M24J3PW87EJGK3XBWDAK2SDQ
Entire-Checkpoint: 01M24KK4BXHY6C4D09RB925ACB
Entire-Checkpoint: 01M24P0GM5H2XV15NPNHT3MTZZ
Entire-Checkpoint: 01M262A4WPJDHD696BKB482ZEE
Entire-Checkpoint: 01M267YFTF5DD35PEVV4ATCFW1
The read-only session listing has nothing to do with Git hooks — it changes the --json active_sessions shape and removes the finalizeExitedSessions sweep from status, both of which deserve their own review. Split to #2363. Restores main's session rendering here: worktreeGroup, the grouped writeActiveSessions with its sweep, the per-agent JSON dedup, and main's sessionBriefJSON. Keeps the field session state needs for hook-health warning dedup. Also drops the redact test-path change, which main already carries in an equivalent %q form. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M27CPSNS5PRE8NMWB51AEJC2
looksLikeLefthookHook matched Lefthook's generated wrapper body byte for byte, pinning every package-manager probe branch it ships with — mise, devbox, uv, bundle, yarn, pnpm, mint, go tool. The next Lefthook release changes that body, at which point the check returns false, inspectActiveHookDelivery reports the bridge absent, and Entire writes native wrappers over Lefthook's own hooks. That is #1349 in reverse, triggered by a routine upgrade, with no error and no test that would notice. Its sibling knownDirectEntireLefthookBody was worse in kind: the two strings it matched were byte-identical to fixtures written by this repo's own integration and e2e tests. Shipping code had been extended to recognize test doubles, which meant no test exercised the branch real Lefthook takes. Replace both with lefthookBodyDispatches. Structure alone will not do — a no-op wrapper carries the same preamble and dispatch line, and accepting it would report healthy delivery while no hook runs. A bare substring will not either: the word appears in paths (.lefthook-local/<hook>/entire.sh dispatches to Entire) and in messages (Lefthook's own fallback ends `echo "Can't find lefthook in PATH"`). So drop quoted literals containing whitespace, then require a word whose basename is the binary. Validated against the real lefthook 2.1.10 generated body plus every fixture in the tree and three adversarial shapes. The simulated launchers now dispatch through a shim binary named lefthook rather than exec'ing entire.sh inline, so they exercise the same path a real install does instead of a shape only our tests produce. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M27D6XYCC5RD2N8NH7C6KHJT
installLefthookFilesAt carried a staging-and-rollback engine — stage, deactivate, publish, finishDeactivation, roll back — to place seven small files. Its only caller is EnsureGitHookIntegration, which snapshots every path it touches (the config, info/exclude, every owned script, every native hook and its backup) before calling in and restores all of them on any error path, including the installer's own. So there were two nested rollback layers and the inner one could only ever undo a subset of what the outer one already undoes. Replace it with ordered atomic writes through the existing jsonutil.WriteFileAtomicIn. Write order is the transaction: scripts and the exclude entry first, config last, because the config is what makes Lefthook dispatch to them — no run can see a config pointing at a script that is not there yet. A partial failure leaves orphan scripts that the next install overwrites, and EnsureGitHookIntegration runs at every turn start. The deactivate/finishDeactivation dance is gone with it. It existed to hide the config while scripts were being written, a window of microseconds in a file read only when a git hook fires, and config-last closes it by construction. TestInstallLefthookFilesRollsBackPublishedArtifacts drove the installer directly to inject into the inner layer. It now drives EnsureGitHookIntegration and asserts the same user-visible property against the layer that survives. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M27DKGHMMN5T7YSAYVCF7VQD
Groundwork for replacing the Lefthook integration mechanism. Entire will own entire-lefthook.yml outright and add a single extends entry to the local config, instead of merging source_dir_local plus a nested scripts entry per hook into a file the user owns. Verified against the real lefthook 2.1.10 binary before writing any of this: a config reached via extends can define scripts, those scripts receive git's pre-push args (origin + URL), source_dir_local is honored from the extended file, and the whole thing survives both `lefthook install -f` (#2263's npm postinstall trigger) and a lefthook.yml edit (#1349's checksum trigger). The main config's format stops mattering because Entire never reads it — the .toml, .json and dotted-name repos the old code refused now work. One caveat drove the design: lefthook-local.yml shadows lefthook-local.toml. So the entry goes into whichever local config Lefthook actually reads, probed in precedence order, and a non-YAML local config is refused rather than shadowed — writing into a shadowed file would be a silent no-op. This commit is additive; the install and inspection paths still use the old merge and are replaced next. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M27E4WRARSE6W7WJ0R6K0N3A
…ning its config Entire used to merge itself into the user's lefthook-local.yml: source_dir_local plus a nested scripts entry for each of five hooks, each marked with a YAML comment. That is what required node-level surgery over a file the user owns — a merge that had to preserve their content, their comments and their ordering — and it still refused a large part of Lefthook's configuration surface, because finding source_dir_local meant parsing the MAIN config. A .toml, .jsonc or .config/lefthook.yml repo, or one using extends or remotes, was declined and left on the native bridge, which Lefthook then clobbered on its next refresh. Entire now owns entire-lefthook.yml outright and adds exactly one extends entry to the local config. The main config is never read, so its format and contents stop mattering: the layouts listed above now work, and the test that pinned them as unsupported asserts the opposite. Inspection follows: Entire's config is present and current, the local config extends it, the scripts are ours and current. No walking the user's file. Removed with the merge: validateLefthookMainConfig and the whole main-config-derived rejection path, mergeLefthookLocalConfig and its owned-node machinery, and the eight near-identical yaml.Node mapping traversals they needed. lefthook_yaml.go is gone entirely. One caveat shapes the design, verified against the real binary: lefthook-local.yml shadows lefthook-local.toml. So the entry goes into whichever local config Lefthook actually reads, and a non-YAML local config is refused rather than shadowed — detection already declined that repo as ambiguous, and a test now pins that nothing is written in that case. Also corrects an ordering slip made while rewriting inspection: a stale script that lost its ownership marker is "ours but outdated", not a foreign file. By content alone the two are indistinguishable, and reporting a repo Entire installed as someone else's would be wrong. Genuinely foreign scripts are refused at install time, where ownership is checked before writing. Verified end to end against lefthook 2.1.10 with a lefthook.toml main config: enable writes both artifacts, the team config keeps zero Entire references, status reports mode=lefthook state=current, and after `lefthook install -f` removes every native hook a commit still lands a checkpoint and a push still delivers it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M27FCP4XZY82D4D5EBCQ2XJ8
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 297893a. Configure here.
| hookIntegrationSnapshot{path: filepath.Join(hooks.path, hook), root: hooks.root, name: hooks.name(hook)}, | ||
| hookIntegrationSnapshot{path: filepath.Join(hooks.path, hook) + backupSuffix, root: hooks.root, name: hooks.name(hook) + backupSuffix}, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Rollback omits Entire Lefthook config
Medium Severity
snapshotHookIntegration never records entire-lefthook.yml, even though install writes that file before the activating extends entry. A failed EnsureGitHookIntegration therefore cannot restore or delete it, so rollback leaves an Entire-owned artifact that later health checks and uninstall still treat as installed.
Reviewed by Cursor Bugbot for commit 297893a. Configure here.
| } | ||
| if err := osroot.RemoveNoSymlinks(snapshot.root, snapshot.name); err != nil { | ||
| errs = append(errs, fmt.Errorf("remove newly-created %s: %w", snapshot.path, err)) | ||
| } |
There was a problem hiding this comment.
Rollback fails on missing files
Medium Severity
When a snapshotted path did not exist, restore always calls RemoveNoSymlinks and treats any error as rollback failure. Sibling removals already ignore os.IsNotExist, so a mid-install error can report a broken rollback even when those absent paths were never created.
Reviewed by Cursor Bugbot for commit 297893a. Configure here.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical findings remain around ownership-safe file handling and pre-push compatibility.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds durable native/Lefthook hook integration so checkpoint capture and delivery survive Lefthook refreshes.
Changes:
- Adds transactional hook installation, repair, removal, and health detection.
- Integrates hook health across setup, lifecycle, status, and doctor flows.
- Expands documentation and unit, integration, and E2E coverage.
File summaries
| File | Reviewed changes |
|---|---|
redact/betterleaks_env_test.go |
Portable environment test module generation. |
e2e/tests/lefthook_durable_test.go |
Durable Lefthook checkpoint scenario. |
docs/architecture/sessions-and-checkpoints.md |
Documents hook delivery and health. |
cmd/entire/cli/strategy/lefthook_integration.go |
Defines Lefthook integration behavior. |
cmd/entire/cli/strategy/lefthook_artifacts.go |
Manages Lefthook artifacts and ownership markers. |
cmd/entire/cli/strategy/hooks_test.go |
Tests generated native hooks. |
cmd/entire/cli/strategy/hook_repair.go |
Handles native hook repair and compatibility. |
cmd/entire/cli/strategy/hook_repair_test.go |
Tests hook repair classification. |
cmd/entire/cli/strategy/hook_managers.go |
Detects hook managers and builds warnings. |
cmd/entire/cli/strategy/common.go |
Runs setup-time hook integration repair. |
cmd/entire/cli/setup.go |
Routes setup and uninstall through integration. |
cmd/entire/cli/session/state.go |
Persists hook warning and transcript state. |
cmd/entire/cli/session/state_test.go |
Tests persisted session state. |
cmd/entire/cli/osroot/rootbase_guard_test.go |
Registers filesystem root usage. |
cmd/entire/cli/integration_test/status_health_test.go |
Tests status hook-health output. |
cmd/entire/cli/integration_test/local_dev_migration_test.go |
Updates local development migration coverage. |
cmd/entire/cli/integration_test/hooks_test.go |
Tests lifecycle hook behavior. |
cmd/entire/cli/integration_test/hook_repair_test.go |
Tests repair warnings and recovery. |
cmd/entire/cli/hooks_git_cmd.go |
Handles optional pre-push arguments. |
cmd/entire/cli/doctor.go |
Reports and repairs hook health. |
cmd/entire/cli/doctor_test.go |
Tests degraded and outdated hook reporting. |
cmd/entire/cli/agent/geminicli/lifecycle.go |
Implements Gemini standalone responses. |
cmd/entire/cli/agent/agent.go |
Adds standalone hook-response capability. |
Review details
Suppressed comments (6)
cmd/entire/cli/strategy/hook_integration_transaction.go:563
entire-lefthook.ymlis written after the snapshot is captured, but it is not included insnapshots. If artifact verification or any later step fails,restoreHookIntegrationSnapshotscannot restore an existing file or remove a newly created one, leaving a localextendsentry pointing at partially installed scripts. Include this config in the transaction snapshot so rollback is complete.
snapshots := []hookIntegrationSnapshot{
{path: filepath.Join(repoRoot, lefthookLocalConfigName), root: workRoot, name: lefthookLocalConfigName},
{path: filepath.Join(commonDir, "info", "exclude"), root: gitRoot, name: "info/exclude"},
cmd/entire/cli/strategy/lefthook_artifacts.go:71
- Checking Unix execute bits here makes a valid Lefthook integration fail on Windows, where
os.FileModedoes not reliably expose executable permissions. The same predicate is used when inspecting active hooks, so final verification can roll back every Windows install even though Git for Windows can execute the hook files. Use a platform-aware executability check (or skip this bit check on Windows).
if content == nil || info.Mode().Perm()&0o111 == 0 ||
!lefthookScriptOwned(string(content)) ||
string(content) != renderLefthookScript(spec) {
return false, nil
cmd/entire/cli/strategy/lefthook_artifacts.go:94
- The generated
entire-lefthook.ymlis clone-local, but this exclude block protects onlylefthook-local.ymland the scripts. Agit clean -fdor an untracked-file cleanup can delete the config while the localextendsentry remains, so Lefthook silently stops dispatching Entire until another repair runs. Add the generated config to the same Git exclude block.
entries := []string{"/" + lefthookLocalConfigName}
for _, hook := range gitHookNames {
entries = append(entries, "/"+filepath.ToSlash(filepath.Join(lefthookLocalDir, hook, lefthookScriptName)))
cmd/entire/cli/strategy/lefthook_detection.go:108
findLefthookLocalConfigand its tests support alternate YAML local names such as.lefthook-local.yml, but this selector rejects every local path exceptlefthook-local.yml. Those repositories therefore fall back to the native bridge even though the integration can safely edit the YAML file; the next Lefthook refresh can then clobber Entire's hooks. Accept the supported YAML local variants and reject only non-YAML locals.
cmd/entire/cli/strategy/lefthook_extends.go:245ensureExtendsEntryleaves a pre-existing exactentire-lefthook.ymlentry untouched and without Entire's marker, but removal matches only the value. Disabling Entire will consequently remove a user-managedextendsentry that it did not add. Require the ownership marker when selecting entries to remove.
cmd/entire/cli/strategy/lefthook_extends.go:52findLefthookLocalConfigdoes not distinguish tracked from clone-local configs, so this write can modify a trackedlefthook-local.ymland commit an Entire-specificextendspointing at the excluded clone-local file. Other clones then get a dangling reference and the shared config changes unexpectedly. Reject tracked local configs, or use a repository-safe integration, before writing.
- Files reviewed: 42/42 changed files
- Comments generated: 7
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+486
to
+490
| if data, _, err := readOptionalRegular(root, entireLefthookConfigName); err == nil && data != nil { | ||
| if err := integrationFault("write", entireLefthookConfigName); err != nil { | ||
| return 0, err | ||
| } | ||
| if err := osroot.RemoveNoSymlinks(root, entireLefthookConfigName); err != nil && !os.IsNotExist(err) { |
| // bugs can `ENTIRE_OPF=no git push` for a one-off bypass while | ||
| // the bug is fixed. | ||
| prePushCmd := gitHookCommand(cmdPrefix, `pre-push "$1"`, false) | ||
| prePushCmd := gitHookCommand(cmdPrefix, `pre-push "$1" "$2"`, false) |
| if !changed { | ||
| return name, nil | ||
| } | ||
| if err := jsonutil.WriteFileAtomicIn(root, name, updated, 0o644); err != nil { |
Comment on lines
+89
to
+92
| writes = append(writes, | ||
| lefthookWrite{root: gitRoot, name: "info/exclude", data: mergedExclude, mode: 0o644}, | ||
| lefthookWrite{root: root, name: entireLefthookConfigName, data: renderEntireLefthookConfig(), mode: 0o644, counted: true}, | ||
| ) |
Comment on lines
+114
to
+118
| // Install or heal the durable integration selected by the repository's hook | ||
| // manager. The coordinator is idempotent, so the hook hot path can repair a | ||
| // manager refresh immediately. | ||
| if _, err := EnsureGitHookIntegration(ctx, hookSettingsFromConfig(ctx)); err != nil { | ||
| return fmt.Errorf("failed to install git hooks: %w", err) |
Comment on lines
90
to
+94
| fmt.Fprintf(&b, "Warning: %s detected (%s)\n", m.Name, m.ConfigPath) | ||
| fmt.Fprintf(&b, "\n") | ||
| if m.IntegrationKind != hookManagerIntegrationHookDirectory { | ||
| fmt.Fprintf(&b, " %s may overwrite hooks installed by Entire when it installs or refreshes hooks.\n", m.Name) | ||
| fmt.Fprintf(&b, " If %s reinstalls hooks, run 'entire enable' to restore Entire's hooks.\n", m.Name) |
Comment on lines
+87
to
+90
| // simulatedLefthookLauncher keeps the structure of a Lefthook v2 launcher but | ||
| // dispatches directly to the clone-local script. That makes the test | ||
| // deterministic and validates the same argv/stdin boundary without requiring a | ||
| // separately installed Lefthook binary in every agent E2E environment. |
Removes the warning subsystem: ~480 lines of production code and ~460 of tests, across strategy/session_state.go, lifecycle.go, agent/agent.go and the Gemini lifecycle. It does not belong here. This change closes #2264 (status lies about hook health), #1349 (Lefthook reclaims the hooks) and #2263 (wrong warning text and no permanent fix). Telling an agent mid-turn that its hooks are broken is a different feature, and #1965 is the open issue that asks for it — it was never filed against any of the three. The machinery was also disproportionate to the job. Nearly all of it existed because the warning was routed through stdout, which Claude and Codex require to be a single JSON value: hence a payload merge, a standalone response writer for Gemini, a claim/hint/transfer/rollback protocol over session state to show one sentence at most once, and a rollback path that had to run under context.WithoutCancel with its own lock budget. #1965 notes that hook stderr already reaches the agent that ran the commit, which needs none of that. Status and doctor still report hook health truthfully, so nothing here depends on the warning to be useful. TestHookRepair_UserEditWarnsOnAgentTurn asserted two properties. The systemMessage half goes with the feature; the half that matters — a hand-edited hook survives an agent turn untouched, and status says so — is kept and renamed. That is c960407's protection and it is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M27JGEF6JRGNT7PAPK7ZS63R
24 copies of the same git-init / clear-global-hooksPath / Chdir opener, and ~25 copies of the same write-or-Fatal block. The package already had a newLefthookTestRepo doing nearly the same job and this file used it zero times. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M27JV9E6AB0YPNJ9H76HBKAG
…Lefthook erroring Two gaps found by reproducing a repo that had already hit #1349. **The advice was still false.** `entire enable` printed "If Lefthook reinstalls hooks, run 'entire enable' to restore Entire's hooks" immediately after installing the integration that makes Lefthook's reinstalls harmless. Flipping OverwritesHooks upgraded the message from "Note:" to "Warning:" and left the text saying the opposite of what is now true. #2263 was filed about this message, so shipping the flag without the text would have closed it without fixing it. The test asserted the false advice, so it is updated to assert the true version and to fail if the old sentence returns. **Stale backups outlive the fix.** A repo that hit #1349 carries <hook>.pre-entire, where Entire stashed Lefthook's launcher, and <hook>.old, where Lefthook then stashed Entire's wrapper. The second one is not merely untidy: Lefthook refuses to move a hook aside when its backup already exists, so it reports could not replace the hook: can't rename pre-push to pre-push.old - file already exists on every sync until the file is gone. That is #1349 step 4, and it survives the integration. Both are now cleared when the Lefthook integration installs, each only when its contents prove whose it was — a .pre-entire holding a Lefthook launcher, a .old holding an Entire wrapper. Anything else is a real user hook and is left alone. Verified against a reproduced post-#1349 repo: both backups cleared, and `lefthook install` after a config change now reports sync hooks ✔️ instead of the rename error. Also checked strategy_options.push_sessions=false against the new mechanism: the integration still installs (Entire no longer owns the hook file, which was #1349's objection) and the Lefthook-invoked pre-push no-ops cleanly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M2914X028JVDZWHP66JAQ9PX
A corrupted script could not be repaired by anything: not the next turn, not `entire doctor --force`. Corruption destroys the ownership marker, so the script reads as a foreign file, and install refused to touch it. Status reported degraded forever and the fix it pointed at errored out. The native path had already answered this question. A foreign file at .git/hooks/<hook> is backed up to <hook>.pre-entire and replaced; nothing is destroyed and the hook recovers. Measured against a native repo: a deleted hook heals, a corrupted hook heals, and only a hand-edited one that still carries the marker is deliberately left alone (c960407). The Lefthook path classified the markerless case identically and then did the opposite thing. Lefthook scripts now behave the same way. An unowned script at Entire's path is written to <script>.pre-entire and replaced, so a corrupted script heals on the next turn and a genuine user file survives at the backup path. Two guards kept: a second unowned file will not bury an earlier backup, and a script that still carries Entire's marker is never displaced. TestInstallLefthookFilesRejectsBroadMarkerScriptConflict asserted the refusal and so asserted the trap. It now asserts the user's script is preserved at the backup path and Entire's is installed, plus a new test for the do-not-bury-a-backup guard. Verified end to end: corrupt the script, status reports degraded, one turn later status reports ready, and the corrupted bytes are intact at entire.sh.pre-entire. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M291GNSF49K855C6A2RE62GY
Two trail findings, both correct. **The generated config was not excluded.** .lefthook-local/ and lefthook-local.yml were in the clone-local exclude block; entire-lefthook.yml was not. So enabling Entire left every Lefthook worktree dirty, which makes an untracked-but-expected file the normal state and eventually gets Entire's integration file committed into someone's repository by an agent that sees it in git status. It is generated and clone-local like the scripts and now sits with them. **Ownership was assumed from the filename.** Uninstall deleted anything at entire-lefthook.yml and install overwrote it, neither checking for Entire's marker. A user's own file at that path would have been destroyed with no backup and no error. That is the same discipline already applied to the scripts, and applying it to the scripts while skipping the config was inconsistent: the path is one Entire chose, but the repository is the user's. A file there Entire did not write is now displaced to entire-lefthook.yml.pre-entire on install and left untouched by uninstall. The removal verifier and AnyGitHookIntegrationInstalled ask the same question, so a deliberately preserved foreign file no longer reads as a failed uninstall. Verified in a real Lefthook repo: after enable, all three artifacts are present and git-ignored and the worktree is clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M2945CGZG3YQBB0T74MT4JKZ
#2337 landed while this branch was in flight and rewrote the same status rendering, so this is a semantic merge rather than a textual one. status.go is rebuilt on main's version: computeCheckpointSyncInfo, resolveDedicatedReadSource and the read-source/push-disabled rendering are main's verbatim. This branch's contribution is layered on top — the snapshot that resolves every operational fact once, the Git-hook health lines, and the three JSON fields. Where the two disagreed, #2337 wins on sync semantics and this branch wins on hook health: - Our dedicated-override behaviour for a missing elected remote is dropped; main's resolveDedicatedReadSource supersedes it and #2337 covers the case with missing_configured_remote_with_dedicated. Our test asserting the old behaviour is removed. - A blocked state no longer suppresses the diagnostics below it. It relabels "Checkpoints sync to:" as "Configured destination:", which removes the false promise #2264 was filed about while keeping the configuration diagnostics #2337 added — status is the only surface that shows them. - TestRunStatus_CheckpointPushDisabledSettingsPrecedence now installs hooks. It predates hook-health awareness and asserted a sync destination in a repo with no hooks; installing them isolates the push-settings precedence it is actually testing. Also takes main's postRewriteHook constant over this branch's duplicate, and main's %q form in the redact test over the equivalent strconv.Quote. Pre-existing failure, unrelated and not introduced here: TestRunStatus_CheckpointPushDisabledDestinations/*/dedicated_rejected_by_fetch_owner fails identically on origin/main. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M295XM12FZ0Q44GNTE7TY6SK
Contributor
Author
|
Superseded by #2388, which does the same job against |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://entire.io/gh/entireio/cli/trails/1301
Draft. Replaces the implementation in #2343, which bundled this with an unrelated session-listing change (now #2363).
Problem
In a repo that also uses Lefthook, Lefthook reclaims Entire's Git hooks and checkpoint capture stops silently. The trigger is routine: any
lefthook.ymledit — including pulling a teammate's — or anynpm install, via Lefthook's postinstall runninginstall -f.It is not a narrow window. Lefthook's installed hooks self-sync on every run, so the reclaim happens at the top of the very
pre-committhe agent's own commit triggers — after Entire's turn-start repair has already run. Entire's existing self-heal cannot win that race. Meanwhileentire statusreports "Checkpoints sync to: origin" (#2264) andentire doctorreports "NOT INSTALLED" — the two disagree.Approach
Entire owns
entire-lefthook.ymloutright and adds oneextendsentry to the clone-local config. Lefthook resolvesextendsat run time, so an already-installed launcher dispatches to Entire immediately — nolefthook install, no user action. Entire writes nothing into.git/hooks/*, so there is nothing left to reclaim.Verified end to end against the real lefthook 2.1.10 binary, including with a
lefthook.tomlmain config — a layout the previous mechanism refused outright.Why only Lefthook
I tested the other managers the same way. pre-commit, Overcommit and hk all overwrite Entire's hooks at install time — and all three are currently classified as not overwriting, which is fixed here. But none of them reclaims a different hook during a later commit. Only Lefthook does, which is why only Lefthook defeats the self-heal and justifies a config-level integration. #1250's general backend is not built: there is now evidence rather than a guess that a second adapter is not imminent.
What this fixes beyond the headline
pre-push.old; Lefthook then refuses to move the hook aside and errors on every sync. That outlives the integration unless the leftovers are cleared, which they now are — each only when its contents prove whose it was.entire enabletold users to re-run it after every Lefthook refresh, printed immediately after installing the integration that makes refreshes harmless. That message is what Lefthook is classified as not overwriting Entire's hooks, so users get the weaker warning and no permanent fix #2263 was filed about.doctor --force. The native path had already settled this by backing up and replacing; the two now behave the same.entire-lefthook.yml. The path is Entire's choice, the repository is the user's. A file there Entire did not write is displaced to.pre-entire, never destroyed.What was removed from #2343
lefthook-local.yml, and with itvalidateLefthookMainConfig, the main-config-derived rejections, and eight near-identicalyaml.Nodetraversals.lefthook_yaml.gois gone.Size
2,145 production / 2,841 test / 155 docs+e2e. Production is down from 3,156 in #2343. The test ratio is 1.32, below the
strategypackage's own 1.71.Known issue, not introduced here
The generated pre-push script passes
"$2"unconditionally, so hooks written by this version fail against an olderentireonPATH(accepts 1 arg(s), received 2). The ABI widening came from #2343; flagging it as a real mixed-version hazard.Still to do
Rebase onto current main.
Closes #2264, #1349, #2263. Does not close #2237 — that backup-chain path is untouched here.
🤖 Generated with Claude Code