Self-heal the recurring core.bare/hooksPath corruption - #189
Conversation
Fix the recurring corruption where the main checkout's git operations fail with "this operation must be run in a work tree". Root cause: git's worktree machinery can leave core.bare=true in the shared .git/config, which is lethal to the main checkout whenever the per-worktree override is absent. The old `prepare` also wrote a relative core.hooksPath to the same shared surface, so a reset left the framework hook silent and the default .git/hooks (a stale copy) active. scripts/git-worktree-safe.mjs pins core.bare=false and an absolute core.hooksPath on the MAIN worktree (via extensions.worktreeConfig), where they survive a shared-config flip. It is idempotent and runs from `prepare` to self-heal on every npm install. A --check mode (npm run check:git) asserts the invariant for a regression guard; npm run fix:git heals on demand. The regression test simulates the broken bare state, proves --check catches it, ensure heals it, and the per-worktree override survives a later shared core.bare flip (the event that used to re-break the checkout).
Add a "Repo health" subsection to the framework-dev part of AGENTS.md covering why the shared core.bare can turn lethal, how scripts/git-worktree-safe.mjs pins the per-worktree override, and the npm run fix:git / check:git entry points.
Self-review found the --check hooksPath comparison resolved a relative configured value against the cwd, so a drift back to the old relative core.hooksPath passed from the repo root but failed from a subdirectory. Require the configured value to be absolute and match the .hooks dir, so the drift is rejected the same way regardless of where the check runs, and cover it with a cwd-independent counterfactual test. Also document that git copies the main worktree's pinned hooksPath into each linked worktree, so a commit inside a throwaway review worktree runs the framework hook (harmless: it only blocks main and generates a changelog on a version bump, and review subagents are read-only).
vivek7405
left a comment
There was a problem hiding this comment.
Went through the heal script and the regression test. The mechanism itself is sound. I confirmed the per-worktree override keeps the main checkout reading core.bare=false even after a later shared flip, which is the event that used to break things, and that spawning a worktree no longer corrupts the main repo.
Two things worth fixing, both done on this branch:
- The check:git hooksPath comparison resolved a relative configured value against the current directory, so a drift back to the old relative core.hooksPath slipped past when run from the repo root and only tripped from a subdirectory. Tightened it to require an absolute path and added a cwd-independent counterfactual.
- Linked worktrees inherit the pinned hooksPath, so a commit inside a throwaway review worktree runs the framework hook. It is harmless (the hook only blocks main and generates a changelog on a version bump), but I documented it so the behavior is not a surprise.
The ensure-mode try/catch is effectively unreachable because every git call uses allowFail. Leaving it as a defensive backstop.
extensions.* are only contractually honored at core.repositoryformatversion >= 1, and git's own `git worktree` bumps it when it enables worktreeConfig. At rfv=0 current git still honors the per-worktree override, but a stricter git could ignore it and silently re-expose the shared core.bare=true while check:git still reported green. Match git's native behavior so the override the fix depends on is durable, and assert rfv=1 after healing.
vivek7405
left a comment
There was a problem hiding this comment.
Second pass, zoomed in on the revised check logic, the heal ordering, and the fresh-clone path. The check is sound now (an unset hooksPath on a repo with a tracked .hooks correctly reads as broken, topLevel falls back through git-common-dir mid-corruption, and I could not construct a half-healed state that passes the check falsely). One durability gap worth closing, fixed below. The shared test fixture is order-dependent but the drift test re-heals at its end, so it stays green; fine to leave.
The prior commit bumped rfv in ensure but check:git never validated it, so a drift back to rfv=0 (which a stricter git treats as the worktree override being off) passed the guard. Assert rfv >= 1 in check alongside worktreeConfig, and cover it with a counterfactual that drops rfv and proves the check fails then heals.
vivek7405
left a comment
There was a problem hiding this comment.
Third pass over the rfv bump and cross-file consistency. The bump is safe and matches git's native worktree behavior, the counterfactual is genuine (reverting the per-worktree override fails the override-survival test), and the script, test, AGENTS.md, and package.json all agree. One gap: the guard added rfv to ensure but check never validated it, so a drift back to rfv=0 passed check:git. Closed that below.
The prepare runs `node scripts/git-worktree-safe.mjs`. In a Docker / Nixpacks build, npm install runs after COPY package.json but before the repo is fully copied, so scripts/ is not in the image yet: node exits 1 (module not found) and the build fails. This broke all four deployed services (#189 introduced the unguarded prepare; the prior one ended in `|| true`). Restore the guard: `node scripts/git-worktree-safe.mjs 2>/dev/null || true`, so a missing script (or any git error) is a no-op and never fails the install. The dev-machine self-heal still runs when the script is present. Regression test runs the actual prepare command in a scriptless temp dir and asserts exit 0.
…194) The prepare runs `node scripts/git-worktree-safe.mjs`. In a Docker / Nixpacks build, npm install runs after COPY package.json but before the repo is fully copied, so scripts/ is not in the image yet: node exits 1 (module not found) and the build fails. This broke all four deployed services (#189 introduced the unguarded prepare; the prior one ended in `|| true`). Restore the guard: `node scripts/git-worktree-safe.mjs 2>/dev/null || true`, so a missing script (or any git error) is a no-op and never fails the install. The dev-machine self-heal still runs when the script is present. Regression test runs the actual prepare command in a scriptless temp dir and asserts exit 0. Co-authored-by: t <t@t>
Summary
Closes #166
The main checkout kept landing in a state where every git operation that
needs a work tree failed with
fatal: this operation must be run in a work tree, needing a manualgit config core.bare falseto recover, severaltimes per session.
Root cause: git's worktree machinery (the review subagents spawn throwaway
worktrees under
.claude/worktrees/) can leavecore.bare=truein theshared
.git/config. That shared value is harmless only while the mainworktree carries a per-worktree override (
extensions.worktreeConfig=trueplus a
.git/config.worktreepinningcore.bare=false). When the overrideis absent, the main checkout reads the shared
core.bare=trueand breaks.The old
preparecompounded it by writing a relativecore.hooksPathtothat same shared surface, so a reset left the framework hook silent and a
stale
.git/hookscopy active (the live repo was in exactly this state:hooksPathpointed at.git/hooks, so.hooks/pre-commitwith changeloggeneration was not running).
What changed
scripts/git-worktree-safe.mjs: pinscore.bare=falseand an absolutecore.hooksPathon the MAIN worktree, where they survive a shared-configflip. Idempotent. Runs from
prepare, so everynpm installself-heals.npm run fix:githeals on demand;npm run check:gitasserts theinvariant and exits non-zero otherwise (regression guard).
Why this shape
A tracked file cannot fix a
.git/configproblem (the config is notcloned). The durable answer for a non-tracked surface is a self-healing
preparestep plus an on-demand check, which is what the issue's own"candidate mitigations" point at. The per-worktree override is preferred
over merely resetting the shared
core.bare, because it keeps the maincheckout reading
falseeven if a later worktree event flips the sharedvalue mid-session.
Test plan
test/repo-health/git-worktree-safe.test.mjs: simulates the brokenbare state, proves
--checkcatches it, ensure heals it, and theper-worktree override survives a later shared
core.bareflip (the eventthat used to re-break the checkout). Idempotency covered.
npm testfull suite: 1522 pass.npm run check:gitpasses and the framework.hooks/pre-commit(with changelog generation) is active again.Definition of done
scripts/,root
package.jsonscripts) only; it does not affectpackages/core,server,cli, the dist build, the importmap, or anything the appsserve.
scaffold has its own separate
.hooksstory.