Skip to content

fix(hooks): clear inherited GIT_DIR before the pre-push test run - #1021

Open
vernonstinebaker wants to merge 2 commits into
nullclaw:mainfrom
vernonstinebaker:fix/pre-push-worktree-git-dir
Open

fix(hooks): clear inherited GIT_DIR before the pre-push test run#1021
vernonstinebaker wants to merge 2 commits into
nullclaw:mainfrom
vernonstinebaker:fix/pre-push-worktree-git-dir

Conversation

@vernonstinebaker

@vernonstinebaker vernonstinebaker commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

Fixes #1020.

The problem

.githooks/pre-push cannot pass from a worktree, which is this repo's documented workflow (docs/nullclaw-maintainer-worktree-workflow.md).

git push from a worktree exports GIT_DIR (and GIT_PREFIX); from a normal clone it exports neither. The suite inherits it, so every test that spawns git against a fixture repository of its own instead operates on this repository — 10 spurious failures:

error: 'skills.test.installSkillFromGit installs from local git repository' failed:   (x7)
error: 'workspace_audit.test.workspace audit staged diff finds raw token in added line' failed:
Build Summary: 11/13 steps succeeded (1 failed); 7380/7400 tests passed (10 skipped, 10 failed)

Because it fails loudly but wrongly, the only options were --no-verify or pushing from the root checkout — both defeat the point of the gate.

Reproduced deterministically

Environment Result
no GIT_DIR green
GIT_DIR=<worktree gitdir> 7380/7400, 10 failed — identical set

And with a throwaway bare repo + a hook dumping env | grep ^GIT:

  • push from a normal clone → (no GIT_* exported)
  • push from a worktree → GIT_DIR=/…/.git/worktrees/wt, GIT_PREFIX=

Git must export GIT_DIR from a worktree because .git is a file there, which is why this never reproduces outside one.

The fix

Clear the inherited git environment before running the suite:

unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE GIT_COMMON_DIR GIT_PREFIX

Validation

  • Hook run with GIT_DIR set (the exact failing condition): 7387/7396, exit 0 — previously 7380/7400 with 10 failed.
  • End-to-end: this branch was pushed without --no-verify, and the fixed hook ran live under a real git push from a worktree — Running tests... → 7387/7396 (9 skipped) → push succeeded.
  • zig fmt --check src/ clean; docs-only + hook, no source changes.

The deeper fix — having the git-spawning tests sanitize their own child environment — is tracked in #1020 as a follow-up, so the tests are correct regardless of how the suite is launched.

@vernonstinebaker
fix(hooks): clear inherited GIT_DIR before the pre-push test run
02d9bfd 
`git push` from a worktree exports GIT_DIR and GIT_PREFIX; a push from a
normal clone exports neither (verified in nullclaw#1020 with a hook that dumps its
environment). The suite inherits it, so every test that spawns `git` against
a fixture repository of its own instead operates on this repository — 10
failures (skills.installSkillFromGit x7, workspace_audit staged-diff, and 2
more), all spurious.

That makes the gate unusable in a worktree, which is this repo's documented
workflow: the only options were --no-verify or pushing from the root checkout,
and both defeat the point of the hook.

Unset the inherited git state before running the suite so it behaves the same
however the push was launched. Verified by running this hook with GIT_DIR set
(the condition that previously produced 7380/7400 with 10 failed): now
7387/7396, exit 0.

The deeper fix is for the git-spawning tests to sanitize their child
environment themselves; that is tracked separately in nullclaw#1020.

Fixes nullclaw#1020
@github-actions github-actions Bot mentioned this pull request Oct 4, 2026
Open
@vernonstinebaker vernonstinebaker mentioned this pull request Oct 5, 2026
Open
@vernonstinebaker
fix(hooks): block a push when tests leave untracked files
40b30a9 
Extends the same hook that clears GIT_DIR (nullclaw#1020) so it also refuses to
let a push through when the test run left untracked files behind.

The suite can drop fixtures into the working tree. They are harmless
where they sit, but the next `git add -A` commits them silently. That
is how nullclaw#1012 and nullclaw#1019 each ended up carrying workspace-audit and
skills test fixtures -- `config.txt`, `history.env`, `SKILL.md`,
`skill.json`, `assets/payload.txt`, and fixture directories under
`skills/` -- along with a one-line `# Not a skill` stub that had
replaced the 974-line README. Both branches were single squashed
commits, which is the shape that captures whatever happens to be lying
around in the tree.

Deliberately limited to untracked ("??") entries. Modified tracked
files are nearly always work in progress, and blocking on those would
make the hook hostile to normal development; the residue problem is
specifically about files appearing from nowhere and being swept up by
an add-everything commit.

The check depends on the `unset` above it: with GIT_DIR still exported
from a worktree push, `git status` reports on the wrong repository.

Verified by running the hook directly: clean tree passes, an untracked
file fails with the offending paths listed, and a modified tracked file
still passes.
@vernonstinebaker

vernonstinebaker commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Update with a reproduction and a verification, since the scope of this PR turns out to be larger than the original 7-line fix suggested.

This is not only a spurious-test-failure bug — it corrupts branches

The original description says inherited GIT_DIR "fails 10 of them spuriously". That undersells it. Under the same condition the suite also writes into the repository: it creates commits and drops fixtures.

Reproduced on a fresh worktree at main, running the suite with GIT_DIR exported exactly as git push from a worktree does:

$ GIT_DIR=<worktree gitdir> zig build test --summary all
error: 'skills.test.installSkillFromGit installs from local git repository' failed:
error: 'skills.test.installSkillFromGit supports root markdown-only repository' failed:
error: 'skills.test.installSkillFromGit installs all skills from repository skills directory' failed:

$ git log --oneline -3
d2e298e0 init
bf747d23 init
072f4ad5 init

$ git show d2e298e0:README.md
# Not a skill

That last commit adds skills/unsafe/SKILL.md and its README is # Not a skill.

This is exactly the residue in #1012 and #1019

Both those PRs arrived carrying SKILL.md, skill.json, history.env, config.txt, assets/payload.txt, fixture directories under skills/, and a stub README replacing the 974-line original. Byte-for-byte the same set this run produces.

The chain:

  1. git push from a worktree exports GIT_DIR.
  2. Tests that shell out to git — skills.zig's installSkillFromGit, workspace_audit.zig — inherit it and operate on this repository instead of their fixture repos.
  3. They run git init / git add / commit, producing a chain of init commits on the real branch.
  4. They write their fixtures into the working tree.
  5. A squashed git add -A commit captures the lot.

Both affected branches are single squashed commits — the shape that sweeps up whatever is lying around.

Verification of the fix

Same worktree, same inherited GIT_DIR, but running the hook rather than zig build test directly, so the unset applies:

check result
suite 13/13 steps, 7387/7396 passed, 9 skipped, 0 failures
HEAD unchanged — no init commits created
git status --porcelain clean, 0 untracked
residue at root none
README.md intact

So the unset is the actual remedy for both the spurious failures and the branch corruption.

Scope of this PR, restated

I previously framed the untracked-file check as the fix for the residue. That was wrong — it is a safety net, and it addresses the residue only indirectly. The residue itself is a symptom of this PR's original bug, so this PR is closer to a data-integrity fix than a developer-ergonomics one, and is the highest-value open item on my side for that reason.

For the record, this also corrects an earlier note of mine: I had described the residue as a second, unrelated leak and pointed at #1038. #1038 is unrelated — it touches only src/config_paths.zig and fixes the cron/session leak into the real $HOME/.nullclaw. The residue is fixed here.

This was referenced Oct 6, 2026
Open
Open
Open

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DonPrus DonPrus Awaiting requested review from DonPrus

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

BUG: .githooks/pre-push always fails from a worktree — GIT_DIR leaks into git-spawning tests

1 participant

@vernonstinebaker