Repository navigation
fix(hooks): register Entire with Lefthook instead of owning its hook files - #2388
Open
peyton-alt wants to merge 19 commits into
Open
peyton-alt wants to merge 19 commits into
peyton-alt wants to merge 19 commits into
Conversation
This was referenced Sep 11, 2026
![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 6 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 2a36c3e. Configure here.
| return "# " + lefthookOwnedMarker + "\n" + | ||
| "# Managed by Entire. Remove the extends entry in your Lefthook local\n" + | ||
| "# config to detach; edits here are overwritten.\n" + string(out), nil | ||
| } |
There was a problem hiding this comment.
Post-rewrite hangs under Lefthook
High Severity
Lefthook scripts do not get a closing stdin unless use_stdin is set, but PostRewrite reads git's old/new SHA mappings with a scanner until EOF. Amend and rebase then block forever on that read, so session remapping never finishes and the git operation never returns.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 2a36c3e. Configure here.
| fmt.Fprintln(w, " Run `entire doctor --force` to apply it.") | ||
| return nil | ||
| } | ||
| if _, err := strategy.EnsureLefthookIntegration(ctx, false); err != nil { |
There was a problem hiding this comment.
Doctor ignores hook path setting
Medium Severity
checkGitHooks always calls CheckHookDelivery and EnsureLefthookIntegration with absolutePath false. When absolute_git_hook_path is on, doctor treats current absolute-path scripts as stale, reports they are not delivering, and --force rewrites them to a PATH lookup that the next EnsureSetup writes back.
Reviewed by Cursor Bugbot for commit 2a36c3e. Configure here.
| default: | ||
| return HookDelivery{Manager: LefthookManagerName, | ||
| Reason: "Entire is not registered with Lefthook, so Lefthook's hooks do not run it."} | ||
| } |
There was a problem hiding this comment.
Status misses native Lefthook fallback
Medium Severity
When a repo is Lefthook-managed but the integration is declined (non-YAML local config), Entire keeps native hooks, yet CheckHookDelivery always reports they are not delivering. entire status then claims checkpoints are not captured, and entire doctor --force tries a registration that cannot succeed.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 2a36c3e. Configure here.
| // Last: this is what makes Lefthook load any of the above. | ||
| if _, err := ensureExtendsEntry(root); err != nil { | ||
| return 0, err | ||
| } |
There was a problem hiding this comment.
Failed register leaves dirty artifacts
Medium Severity
EnsureLefthookIntegration writes entire-lefthook.yml and .lefthook-local/**/entire.sh before the extends entry, then excludeArtifacts never runs if that entry is refused. In a repo with a non-YAML local config those files stay untracked and unignored on every turn, which is the dirty-worktree problem the exclude block was added to prevent.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 2a36c3e. Configure here.
| seq := mapping.Content[found+1] | ||
| if seq.Kind != yaml.SequenceNode { | ||
| return nil, nil, errors.New("extends must be a sequence") | ||
| } |
There was a problem hiding this comment.
Scalar extends aborts setup
Medium Severity
extendsSequence rejects a scalar extends value. Lefthook accepts both a string and a list, so a lefthook-local.yml that uses the string form makes EnsureLefthookIntegration fail, and EnsureSetup then errors on every agent turn instead of appending Entire's config.
Reviewed by Cursor Bugbot for commit 2a36c3e. Configure here.
| paths.ClearWorktreeRootCache() | ||
| t.Cleanup(paths.ClearWorktreeRootCache) | ||
| return dir | ||
| } |
There was a problem hiding this comment.
Lefthook tests skip git isolation
Medium Severity
newLefthookRepo inits a temp repo and then exercises git rev-parse --git-path hooks via reconcile and ReinstallGitHooks without testutil.IsolateGitConfigEnv. A host core.hooksPath (or other global git config) can make those tests read and rewrite the developer's real hooks directory.
Triggered by learned rule: Tests shelling out to git CLI must call testutil.IsolateGitConfigEnv(t)
Reviewed by Cursor Bugbot for commit 2a36c3e. Configure here.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
One or more issues must be addressed before approval.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds durable Lefthook integration so Entire hooks survive Lefthook refreshes, while improving hook-health reporting and native fallback behavior.
Changes:
- Registers Entire through Lefthook configuration and generated scripts.
- Reconciles legacy hook conflicts and updates installation/uninstallation.
- Adds status, doctor, detection, and regression-test coverage.
File summaries
| File | Description |
|---|---|
| cmd/entire/cli/strategy/lefthook_test.go | Updated as part of this pull request. |
| cmd/entire/cli/strategy/hooks.go | Updated as part of this pull request. |
| cmd/entire/cli/strategy/hook_managers.go | Updated as part of this pull request. |
| cmd/entire/cli/strategy/hook_managers_test.go | Updated as part of this pull request. |
| cmd/entire/cli/strategy/common.go | Updated as part of this pull request. |
| cmd/entire/cli/status.go | Updated as part of this pull request. |
| cmd/entire/cli/status_test.go | Updated as part of this pull request. |
| cmd/entire/cli/setup.go | Updated as part of this pull request. |
| cmd/entire/cli/doctor.go | Updated as part of this pull request. |
| CLAUDE.md | Updated as part of this pull request. |
Review details
Suppressed comments (9)
cmd/entire/cli/doctor.go:641
- This branch is selected solely by the presence of a Lefthook config, so
entire doctornow reports a never-enabled Lefthook repository asNOT DELIVERING, anddoctor --forcewrites Entire's config/scripts andextendsentry into it. The native-hook path below deliberately avoids this takeover withsettings.IsSetUpAny; apply the same opt-in rule (while preserving the existing stale-hook handling) before repairing Lefthook integration.
if delivery := strategy.CheckHookDelivery(ctx, false); delivery.Manager == strategy.LefthookManagerName {
if delivery.OK {
cmd/entire/cli/setup.go:2671
- This new cleanup step is unreachable when Lefthook artifacts are the only remaining Entire state:
runUninstall's earlier "not installed" return does not countentire-lefthook.yml, theextendsentry, or scripts. After a partial uninstall or manual.entiredeletion, users therefore cannot reach this removal and generated artifacts remain. Include a read-only integration-presence check in the uninstall guard and summary.
// The Lefthook registration is separate from the hook files, so uninstall
// has to take both. Only artifacts carrying Entire's marker are removed.
lefthookRemoved, err := strategy.RemoveLefthookIntegration(ctx)
if err != nil {
p.stepFailed("Failed to remove the Lefthook integration")
cmd/entire/cli/strategy/common.go:1846
- The integration is only registered from
EnsureSetup;entire configure --forceand the absolute-path update path callInstallGitHookdirectly. On an upgraded Lefthook repo whose registration is missing or stale, those commands therefore write native wrappers over Lefthook's launcher, recreating the ownership fight until the next agent turn. Route all hook installation through this coordinator (or call it from those setup paths).
if _, err := EnsureLefthookIntegration(ctx, absolute); err != nil {
if errors.Is(err, ErrLefthookLocalConfigUnwritable) {
return nil
}
return fmt.Errorf("failed to register Entire with Lefthook: %w", err)
cmd/entire/cli/strategy/hook_managers.go:88
- When a Lefthook main config is present alongside
lefthook-local.toml/.toml,EnsureLefthookIntegrationdeliberately declines and keeps native hooks, so Lefthook can still reclaim them on its next install. This warning nevertheless saysNo action needed, leaving the user with a non-durable setup and no instruction to migrate/edit the local config. The warning needs to reflect the declined integration (or be emitted only when the registration actually succeeds).
case LefthookManagerName:
// Entire registers itself in Lefthook's own config, so Lefthook's
// regenerations no longer remove it and there is nothing for the
// user to do. See EnsureLefthookIntegration.
fmt.Fprintf(&b, "Note: %s detected (%s)\n\n", m.Name, m.ConfigPath)
fmt.Fprintf(&b, " %s regenerates Git hooks whenever its config changes or it runs.\n", m.Name)
fmt.Fprintf(&b, " Entire registers itself in %s's own configuration instead of owning\n", m.Name)
fmt.Fprintf(&b, " the hook files, so those regenerations no longer remove it.\n")
fmt.Fprintf(&b, " No action needed.\n\n")
cmd/entire/cli/strategy/lefthook.go:403
- All read errors are treated as if the artifact were absent. For an unreadable file or symlink, the subsequent atomic write can therefore replace the existing path without creating the promised
.pre-entirebackup, losing the only recoverable copy of a foreign artifact. Ignore onlyos.IsNotExist; return other errors (or preserve the entry by renaming it) before claiming the path.
data, err := osroot.ReadFileNoFollow(root, name)
if err != nil || strings.Contains(string(data), lefthookOwnedMarker) {
return nil //nolint:nilerr // absent or ours: nothing to displace
cmd/entire/cli/strategy/lefthook.go:423
- A read failure is silently treated as a missing higher-precedence local config.
WriteFileAtomicInthen replaces that path, including a symlink or an unreadable file, instead of refusing to touch the user's config. Only skip candidates foros.IsNotExist; propagate other errors so the integration fails closed rather than redirecting or discarding a local Lefthook configuration.
body, readErr := osroot.ReadFileNoFollow(root, candidate)
if readErr != nil {
continue
}
cmd/entire/cli/strategy/lefthook.go:496
removeExtendsEntryremoves everyextends: entire-lefthook.ymlvalue, even when the entry predated Entire or was added by the user.ensureExtendsEntryreturns early for an existing value, so that entry has no Entire marker, yet uninstall will still delete it and can change the user's Lefthook behavior. Remove only the marker-bearing node (or otherwise prove the integration owns the entry).
kept := make([]*yaml.Node, 0, len(seq.Content))
changed := false
for _, item := range seq.Content {
if item.Value == entireLefthookConfig {
changed = true
continue
}
cmd/entire/cli/strategy/lefthook.go:410
- The existing-backup check only treats a readable backup as blocking. If
<name>.pre-entireis a symlink or unreadable,ReadFileNoFollowerrors and the following atomic write replaces it, burying the earlier backup despiteErrLefthookArtifactBlockedbeing intended to prevent exactly that. Check for the backup's existence with a no-followLstatand fail closed on any non-ENOENTerror.
backup := name + GitHookBackupSuffix
if _, err := osroot.ReadFileNoFollow(root, backup); err == nil {
return fmt.Errorf("%w: %s already exists; remove it to let Entire reinstall %s",
ErrLefthookArtifactBlocked, backup, name)
}
if err := jsonutil.WriteFileAtomicIn(root, backup, data, mode); err != nil {
cmd/entire/cli/strategy/lefthook.go:180
findLocalConfigsupports alternate names such aslefthook-local.yamland.lefthook-local.yml, but the exclude block ignores onlylefthookLocalConfigNames[0]. When an alternate local config is the file being modified, the integration remains unignored and can show up as an untracked/committable generated change, contradicting the per-clone artifact handling described here. Add the selected local-config path (or consistently cover all supported local names) to the owned exclude block.
"/" + entireLefthookConfig + "\n" +
"/" + lefthookScriptDir + "/\n" +
"/" + lefthookLocalConfigNames[0] + "\n" +
- Files reviewed: 11/11 changed files
- Comments generated: 5
- 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
+146
to
+150
| if _, err := ensureExtendsEntry(root); err != nil { | ||
| return 0, err | ||
| } | ||
| if err := excludeArtifacts(ctx, repoRoot); err != nil { | ||
| return 0, err |
Comment on lines
38
to
44
| // Lefthook supports {.,}lefthook{,-local}.{yml,yaml,json,toml} | ||
| for _, prefix := range []string{"", "."} { | ||
| for _, variant := range []string{"", "-local"} { | ||
| for _, ext := range []string{"yml", "yaml", "json", "toml"} { | ||
| name := prefix + "lefthook" + variant + "." + ext | ||
| checks = append(checks, hookManager{"Lefthook", name, false}) | ||
| checks = append(checks, hookManager{LefthookManagerName, prefix + "lefthook" + variant + "." + ext}) | ||
| } | ||
| } |
Comment on lines
+316
to
+320
| if owned, ownErr := fileIsOwned(root, entireLefthookConfig); ownErr == nil && owned { | ||
| if err := osroot.RemoveNoSymlinks(root, entireLefthookConfig); err != nil && !os.IsNotExist(err) { | ||
| return removed, fmt.Errorf("remove %s: %w", entireLefthookConfig, err) | ||
| } | ||
| removed++ |
Comment on lines
+680
to
+690
| if LefthookManaged(repoRoot) { | ||
| current, checkErr := LefthookIntegrationCurrent(ctx, absolutePath) | ||
| switch { | ||
| case checkErr != nil: | ||
| return HookDelivery{Manager: LefthookManagerName, | ||
| Reason: fmt.Sprintf("Could not inspect Entire's Lefthook integration: %v", checkErr)} | ||
| case current: | ||
| return HookDelivery{OK: true, Manager: LefthookManagerName} | ||
| default: | ||
| return HookDelivery{Manager: LefthookManagerName, | ||
| Reason: "Entire is not registered with Lefthook, so Lefthook's hooks do not run it."} |
Comment on lines
+686
to
+690
| case current: | ||
| return HookDelivery{OK: true, Manager: LefthookManagerName} | ||
| default: | ||
| return HookDelivery{Manager: LefthookManagerName, | ||
| Reason: "Entire is not registered with Lefthook, so Lefthook's hooks do not run it."} |
…files Lefthook regenerates every file in .git/hooks from its own config, gated by a checksum of that config, and it does so at the top of whatever hook fires next — typically the pre-commit of the very commit being made. Entire's turn-start self-heal therefore always loses the race: any `lefthook install -f` (npm postinstall) or lefthook.yml edit silently takes the hook back, and the next commit is not captured. #2264 is the symptom users see — `entire status` naming a checkpoint destination while no hook existed to reach it. Stop wanting the file. Entire writes a config of its own (entire-lefthook.yml) declaring a script per hook, registers it with a single `extends` entry in Lefthook's local config, and lets Lefthook invoke those scripts. Lefthook resolves `extends` at run time, so this takes effect with no `lefthook install` and no user action, and Lefthook's own config is never touched. Hook-file ownership follows from that: once Lefthook dispatches Entire, Entire owning the file too would run every hook twice, so the native install skips any hook carrying Lefthook's launcher and reconcileHookFiles moves a launcher Entire had displaced back over its own hook. It also clears the <hook>.old backups that otherwise make every later Lefthook sync fail with "can't rename pre-push to pre-push.old" (#1349). A hook Lefthook has not taken over is still installed natively: in a repo where nobody ran `lefthook install`, Entire's own hooks are the only delivery there is. CheckHookDelivery is now the single question `entire status` and `entire doctor` ask, and in a Lefthook repo it is answered by the registration rather than by .git/hooks/*. Status gained a delivery line plus hooks_deliver / hooks_manager / hooks_deliver_reason in --json; `doctor --force` re-registers and reconciles. detectHookManagers' advice was wrong in both directions and is corrected: pre-commit 4.6.2, Overcommit 0.73.0 and hk 1.58.1 all overwrite Entire's hooks at install (verified), and Lefthook no longer needs any user action at all. Verified end-to-end against lefthook 2.1.10: hooks dispatch exactly once per commit, the user's own Lefthook commands keep running, the three artifacts are ignored via .git/info/exclude, and uninstall removes only what Entire wrote. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M29BM8KGTDZXD1N3PJ3HGQFW
EnsureLefthookIntegration wrote its six artifacts and only then reached ensureExtendsEntry, so a repo with a non-YAML lefthook-local.* — the documented ErrLefthookLocalConfigUnwritable case — got entire-lefthook.yml and .lefthook-local/ written before the refusal, and excludeArtifacts never ran. Since ensureLefthookIntegrationIfManaged treats that refusal as a decision rather than a failure, it recurred on every turn rather than resolving itself: six untracked, un-excluded files in the worktree permanently, which is the accidental-commit risk this file's own comments warn about. Two changes, covering two different failures. The local config is probed before anything is written, so declining to shadow it now means touching nothing at all. And the exclude entries go in while the paths are still empty — they name paths, not existing files, so writing them first costs nothing and a write that dies part way through (a blocked artifact path, a full disk) can no longer leave anything unignored. Verifying that against real lefthook surfaced a second bug in the same repo shape: `doctor --force` exited with a raw "lefthook local config is not YAML" and `entire status` reported "Checkpoints are NOT being captured" — in a repo where Entire's own hooks are installed and firing. A declined integration is permanent, so there was no repair coming and no fix to offer. CheckHookDelivery now judges such a repo by its native hooks and reports HookDelivery.Declined, which doctor prints and status --json exposes as hooks_lefthook_declined, so the repo reads as working while still saying why Lefthook is not the one carrying it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M29CCR921GJWJSAJ1PB1WD04
…l's reverse order Two review findings on the Lefthook integration. The turn-start step ran on every prompt with no gate, on a path EnsureSetup documents as synchronous and timed. Measured, the cost was not the file reads it looked like: LefthookIntegrationCurrent spent 11.2ms resolving the git common dir for the exclude check, and reconcileHookFiles another 13.0ms resolving the hooks dir — two `git rev-parse` subprocesses, the first on a currency check a single turn runs two or three times. The common dir is now memoized per worktree root (a worktree's does not move while the process lives, and only the path is cached — osroot.Shared already memoizes the root and is what keeps a handle from outliving a deleted directory), and reconcileHookFiles uses the already-cached GetHooksDir instead of the by-path variant. The whole step goes from 23.6ms to 0.42ms, against ~10-14ms for the worktree-root subprocess EnsureSetup already pays for. A non-Lefthook repo still exits after eight stats. RemoveLefthookIntegration deleted entire-lefthook.yml and its scripts before dropping the `extends` entry naming them, so an interruption or a failed config write left Lefthook's config pointing at a file that was gone — the reverse of the install, which deliberately writes that entry last. It now removes the entry first. Lefthook 2.1.10 tolerates a dangling extends (run, install, validate and dump all succeed, and the user's own commands still fire), so this is not repairing a break; it is declining to depend on another tool's tolerance, and it fails in the better direction, since giving up at that step now leaves the integration whole rather than half-deleted. The test helper clears both process-global caches, which are keyed by a directory each test has just left. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M29DEVHFN0HX551G3RB0MCEM
…ook on Windows Registering with Lefthook is not delivery. Git runs a hook only if the FILE exists, and Lefthook generates one per hook it knew about at its last install; Entire's config arrives through `extends` at run time, so a repo whose lefthook.yml declares only pre-commit has no file for the other four. Every artifact Entire owns can be present and correct while nothing dispatches Entire at all — and `entire status` reported hooks_deliver true in exactly that repo, which is the same false OK (#2264) this integration exists to remove. CheckHookDelivery now asks the question git asks: is there a file here, and does it lead to Entire — Lefthook's launcher, or Entire's own hook. `doctor --force` fills the gaps after registering, which the native install does by skipping the paths Lefthook already owns. Recognising Lefthook's launcher is now structural rather than a search for the word "lefthook". A false positive there is silent and permanent: the file is restored over Entire's hook by reconcileHookFiles and then skipped forever by installSkipsHook, so capture stops for that hook with nothing to repair it. The test is Lefthook's template shape — the call_lefthook function definition, plus the dispatch line for THIS hook — with a table covering the near misses, including a hand-written hook that runs lefthook itself. Windows was the one path in this integration that is not true by construction: Entire's hooks reach Lefthook as `scripts` with a bash runner, so Lefthook resolves bash through PATH and spawns it with a POSIX script on a platform whose shell is not POSIX. Two tests now cover it, named for ci.yml's windows-latest selector: one runs the rendered script under bash and checks the hook's arguments survive, and one drives the whole chain with the real Lefthook binary — git runs Lefthook's hook, Lefthook resolves the extends and spawns Entire's script. The Windows job installs lefthook 2.1.10 so the second is a test rather than a skip, and ENTIRE_TEST_REQUIRE_LEFTHOOK makes a failed install fail the job instead of passing silently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M29EJEGJGNNNFTYPX5DT5VYK
The Windows job's first run showed the Lefthook integration working there — bash resolved, the script ran with the hook's arguments, and every assertion in the lefthook-driven end-to-end test passed — but the test still failed, in t.TempDir's cleanup: unlinkat ...\.git: The process cannot access the file because it is being used by another process osroot.Shared memoizes a *os.Root per directory, so the worktree root and git common dir this test opened stay open for the life of the process, and an open handle stops Windows deleting the directory under it. The helper now calls osroot.ResetShared alongside the two path caches it already cleared; being registered after t.TempDir, LIFO runs it first. Also ToSlash the recorder path interpolated into the stub's shell script: bash keeps backslashes literal inside single quotes, so a Windows path would have become a mangled filename rather than a directory walk. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M29F1G8DD3NWCDJ44WFBP13R
…g no action needed Two review findings, both about the paths where Entire declines to register. hookManagerWarning printed "No action needed" for every Lefthook repo, without checking whether Entire could actually register. Where it has declined, the repo runs on native hooks that Lefthook reclaims — so the message was the same false advice (#2263) this integration set out to remove, reintroduced in the branch that replaced it. The Lefthook case now splits: a Note with "No action needed" only when registration is possible, and otherwise a Warning naming the reason and saying plainly that Lefthook reclaims the hooks it manages, Entire reinstalls them on the next agent turn, and a commit made in between is not captured. ensureExtendsEntry wrote into whatever lefthook-local.* it found, with no check for whether the repository carries that file. Lefthook documents it as personal and gitignored, but a committed one is the team's file: adding Entire's entry modifies something shared, and a developer who reverts it gets it back on the next turn. Entire now declines (ErrLefthookLocalConfigTracked), reusing the Declined channel that already reports the non-YAML refusal. The check runs only where the entry would actually be ADDED, so a tracked config that already carries it — committed by the team, or added by hand — is honoured and costs nothing after the first install. One correction to the finding: the entry was written once, not on every turn; ensureExtendsEntry is idempotent and only runs when the currency check fails. The tracked probe is settings.PathIsTracked rather than a second index scan. probePathIsVersioned took a path but used it only to locate the repo, comparing against the EntireSettingsLocalFile constant — a single-purpose helper whose signature looked general — so it now takes the repo root and a repo-relative name, and the Dir(Dir(path)) inference moved to the settings callers whose path shape justifies it. Duplicating it would have been worse than untidy: a153514 and 6c4adc4 exist because an exact comparison misses a tracked file on a case-insensitive volume, and the open-coded copy this replaces had reintroduced exactly that bug. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M29H2FBXXRQ51MV5028Q0MDK
`entire doctor` called CheckHookDelivery and EnsureLefthookIntegration with a
hardcoded absolutePath=false while status passed the setting and EnsureSetup
resolved it. In a repo configured with absolute_git_hook_path, doctor
therefore compared the installed scripts against the bare-command rendering,
reported a working install as NOT DELIVERING, and under --force rewrote the
scripts and config down to a bare `entire` — silently breaking delivery for
the GUI git clients that setting exists to serve.
Fixing doctor alone would leave the footgun: this is exactly what
ReinstallGitHooks was introduced to prevent ("so callers outside this package
cannot open-code it and silently drop absolute_git_hook_path"), and the new
exported functions had reintroduced it. They now resolve the prefix from the
repository's settings themselves, and the parameter is gone, so no caller can
choose it wrongly. InstallGitHook keeps its own flag; only the question "what
does this repository's installed integration say" is resolved internally.
TestLefthookIntegration_HonoursAbsoluteGitHookPath pins it: an absolute-path
repo installs the absolute form, reads as current rather than needing repair,
and is byte-identical after a second install.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 01M29HECQX4J0E382N3R8HZBZ6
Main moved the strategy key-file list out of CLAUDE.md, so the rebase dropped this entry with the conflict. Same text, new home. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Entire-Checkpoint: 01M37SP78XWXMRXA3E7BCBDSAS
entire-lefthook.yml set source_dir_local so Lefthook would find Entire's
scripts. A config reached through extends merges into the user's, so this
overrode any source_dir_local the user had chosen: their own local scripts
stopped resolving and Lefthook failed every commit with "script does not
exist". Dropping the key alone would break Entire's scripts in those repos
instead.
Declare each hook as a command that runs Entire's script by path. {0} is
the hook's arguments joined unescaped, which is exactly what Lefthook
appends to a script's command line, so the scripts see the same "$@".
Existing installs no longer match the rendered config and are rewritten on
the next turn.
The real-Lefthook test now keeps a user script in a custom directory and
requires it to still run.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Entire-Checkpoint: 01M37SRW0CWQ3KAFVFJYM33BBK
git passes post-rewrite's old/new commit pairs on stdin. Lefthook hands a job an empty stdin unless it sets use_stdin, so under Lefthook PostRewrite read no pairs and returned: every amend and rebase silently skipped the session remap. (It did not hang, as a review suggested; Lefthook's null reader returns EOF at once.) Set use_stdin on the post-rewrite command. The real-Lefthook test now amends and requires the pairs to reach Entire; it fails without this. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Entire-Checkpoint: 01M37SSJC06Y3V7FEE647QHD6R
Entire-Checkpoint: 01M37X0CCQY73EJ6KKV5TB5EF0
peyton-alt
force-pushed
the
fix/lefthook-integration
branch
from
September 23, 2026 19:50
024b6db to
5820bf6
Compare
Entire-Checkpoint: 01M382M5H75WAH035FJBHZ7DWQ
Entire-Checkpoint: 01M388FDS46YXDYX0F9D6RQ5SW
Entire-Checkpoint: 01M38C4143ERYFH2TNVNGRZA88
With Entire's Git hooks not delivering, status warned that checkpoints are not being captured but still told the user their pending checkpoints "sync with your next 'git push origin'" — which #2264 calls out as a promise that cannot happen, since no pre-push hook will run. The unpushed counter now keeps the count and replaces the promise with "won't sync until Entire's Git hooks run again" whenever hook delivery fails. The pushing-disabled phrasing already promised nothing and is unchanged. Fixtures that assert the promise now install the hooks first. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M38P4MC6TJMWSECRGGDDRA90
Lefthook's launcher exits on LEFTHOOK=0 before reading any config, so once Entire runs as a Lefthook job it is skipped along with the user's own hooks. A commit made that way gets no checkpoint trailer and its turn is attributed to no commit; nothing inside Lefthook can opt out of it (its rc: file is sourced after the exit). CheckHookDelivery now reports that as SkippedBy, keeping OK true since nothing is broken. status warns beneath "Git hooks · via Lefthook" and drops the next-push promise from the unpushed counter, status --json carries hooks_skipped_by, and doctor adds a warning under its OK line. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M38TYBGW4REG8Y69B2X662A8
hk 2.x installs Git's config-based hooks (hook.<name>.command) on Git 2.54+, which git runs alongside .git/hooks/* — verified with hk 2.1.0 on Git 2.55: both hk's hooks and Entire's fire, and re-running hk install leaves Entire's hook files alone. Only hk's legacy mode (older Git, or --legacy) writes shims over them. entire enable now warns about hk only when it will write shims: Git older than 2.54, an unknown version, or an hk shim already in the hooks directory. Otherwise it prints a note that no action is needed and names --legacy as the mode that does replace Entire's hooks. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M39YSN6R5DK81ZPXH3AWKKYH
Install now filters out the hooks Lefthook delivers before main's locked installHooks runs, and uninstall adds the Lefthook removal to main's GitHookRemoval count. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Entire-Checkpoint: 01M5B22A5EEPDNFFTKV5X87AHE
In a Lefthook repo .git/hooks/* are Lefthook's launchers, so the uninstall summary listed no git hooks and a repo with only the Lefthook registration read as not installed, leaving the extends entry behind. Check Entire's Lefthook artifacts on their own and show them as a row. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Entire-Checkpoint: 01M5B27NP9BV35XK55A3XXWDS6
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/1314
The bug
Lefthook regenerates every file in
.git/hooksfrom its own config, gated by a checksum of that config — and it does so at the top of whatever hook fires next, typically thepre-commitof the very commit being made. Entire's turn-start self-heal (EnsureSetup→ reinstall if not installed) therefore always loses the race:lefthook install -f(what Lefthook's own npm postinstall runs) takes the hook back;lefthook.ymledit, via the checksum;The user-visible symptom is #2264:
entire statusnames a checkpoint destination while no hook exists to reach it. #1349 is the second-order damage — once Entire and Lefthook have fought over a hook,<hook>.oldis left behind and Lefthook then fails every later sync withcan't rename pre-push to pre-push.old - file already exists.The fix
Stop wanting the file.
entire-lefthook.ymlextends:entry inlefthook-local.yml.lefthook-local/<hook>/entire.shLefthook resolves
extendsat run time, so this takes effect with nolefthook installand no user action, and Lefthook's own config is never touched.Hook-file ownership follows from that. Once Lefthook dispatches Entire, Entire owning the file too runs every hook twice (its own copy, then Lefthook's launcher via the chain call — measured: 2×
prepare-commit-msg,commit-msg,post-commitper commit). So:reconcileHookFilesmoves a launcher Entire had displaced back over its own hook, and clears the<hook>.oldbackups behind entire and lefthook fight over the pre-push hook; session push gets silently disabled #1349;lefthook install, Entire's own hooks are the only delivery there is.CheckHookDeliveryis now the single questionentire statusandentire doctorask, and in a Lefthook repo it is answered by the registration rather than by.git/hooks/*.Also corrected
detectHookManagerswas wrong in both directions and gave users false advice:.pre-commit.legacy; Overcommit moves old hooks aside). They were all flagged as harmless.hook.<name>.command), which git runs alongside.git/hooks/*(verified with hk 2.1.0 on Git 2.55 — both fire, and re-runninghk installleaves Entire's files alone).entire enablewarns about hk only on older Git, an unknown version, or when an hk legacy shim is already installed; otherwise it prints a no-action note.entire enableis wrong. Only Lefthook reclaims continuously, which is why only Lefthook gets a config integration.Safety
<name>.pre-entire, never destroyed; an existing backup is never buried (ErrLefthookArtifactBlocked).lefthook-local.ymlshadowslefthook-local.toml, so Entire refuses to create it beside a non-YAML local config (ErrLefthookLocalConfigUnwritable) and keeps its native hooks instead.yaml.Node, so comments and key order survive..git/info/exclude, not the user's.gitignore. A partial block from an older version repairs itself.Verification
Against real lefthook 2.1.10 with the real
entirebinary:prepare-commit-msg,commit-msg,post-commit— 1 each), and the user's own Lefthook commands still run;entire doctor --force;entire statusreportsGit hooks · via Lefthook;mise run checkpasses. The twodedicated_rejected_by_fetch_ownersubtests inTestRunStatus_CheckpointPushDisabledDestinationsfail identically onorigin/main(e8780c3) — pre-existing, from fix(status): report disabled checkpoint pushing #2337.LEFTHOOK=0skips Entire tooLefthook's launcher exits on
LEFTHOOK=0before reading any config (itsrc:file is sourced after that exit), so no registration can opt out; escaping it would mean owning the hook files again, which is #1349. A commit made under it gets no trailer; a push made under it defers checkpoints to the next push.status/doctor/status --json(hooks_skipped_by) now warn while it's set, and the counter drops its next-push promise. The endless carry-forward such a commit caused is fixed separately in #2577 (also onmain).Follow-ups (not in this PR)
enable -f,doctor --force, native reinstall, and Lefthook reconcile) treats a hook carrying Entire's marker as Entire-owned and replaces it without a backup. Protecting user edits to Entire's own hook would be one change across all of them (trail finding 01M38P8P6M1S, dismissed here for that reason).Size
+1,413 / −60 — roughly 890 production, 490 test, 30 docs. This replaces the Lefthook half of #2343 (+6,807) and supersedes draft #2365 (+5,025).
🤖 Generated with Claude Code
Note
Medium Risk
Changes how Git hooks are installed and diagnosed across Lefthook and native paths; mistakes could miss checkpoint capture or double-run hooks, but behavior is heavily tested and fails closed on ambiguous local config.
Overview
Adds first-class Lefthook integration so checkpoint hooks survive Lefthook reclaiming
.git/hooks/*on every run (#2264, #1349). Instead of reinstalling native wrappers, Entire writesentire-lefthook.yml, hook scripts under.lefthook-local/, and anextends:entry inlefthook-local.yml(YAML-only; refuses to shadowlefthook-local.toml). Native install skips hooks Lefthook already owns, reconciles double-dispatch and stale<hook>.oldbackups, and ignores generated files via.git/info/exclude.CheckHookDeliveryis now whatentire status(human + JSON:hooks_deliver,hooks_manager) andentire doctoruse—Lefthook repos are judged on registration, not hook file contents. Doctor can re-register or reconcile with--force. Uninstall removes Lefthook artifacts as well as native hooks.EnsureSetupruns Lefthook registration before native install.Hook-manager detection warnings are corrected: Lefthook gets a “no action” note; pre-commit, Overcommit, and hk are warned as install-time overwriters (not harmless).
Reviewed by Cursor Bugbot for commit 2a36c3e. Configure here.