Skip to content

fix(devkit): scrub git repo-location env vars in spawned processes#400

Open
crs48 wants to merge 1 commit into
mainfrom
claude/vigilant-goldstine-346b8f
Open

fix(devkit): scrub git repo-location env vars in spawned processes#400
crs48 wants to merge 1 commit into
mainfrom
claude/vigilant-goldstine-346b8f

Conversation

@crs48

@crs48 crs48 commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Problem

The .husky/pre-commit final step pnpm vitest run --changed HEAD --passWithNoTests failed falsely whenever the changed-file set pulled in packages/devkit tests (bridge.test.ts, dev-loop.test.ts, git.test.ts). Those tests create real temp git repos and run git worktree add, which died with:

fatal: .git/index: index file open failed: Not a directory

git commit exports GIT_INDEX_FILE (and related repo-location vars) into the hook environment; NodeCommandRunner spread process.env into every child, so the tests' spawned git processes were silently redirected at the outer repo instead of their temp-repo cwd. The same tests pass outside the hook.

Fix

  • packages/devkit/src/command-runner.ts (the robust fix): NodeCommandRunner drops the git repo-location vars (GIT_DIR, GIT_WORK_TREE, GIT_INDEX_FILE, GIT_OBJECT_DIRECTORY, GIT_ALTERNATE_OBJECT_DIRECTORIES, GIT_COMMON_DIR, GIT_PREFIX) from the inherited env before spawning. An explicit value via options.env still wins. This covers both the Git class and the tests' direct runner.run('git', …) setup calls, and protects any devkit consumer that runs inside a hook.
  • .husky/pre-commit: the vitest step runs under env -u GIT_INDEX_FILE -u GIT_DIR -u GIT_WORK_TREE as defense for any other suite that spawns git.
  • New regression test in command-runner.test.ts pinning the scrub + override-wins behavior.
  • Changeset: @xnetjs/devkit patch.

Verification

  • Reproduced first: with GIT_INDEX_FILE/GIT_DIR pointed at the repo, all 5 git.test.ts tests failed with the reported error; after the fix, the full devkit suite (54 tests) passes under the same polluted env.
  • End-to-end: the commit on this branch ran the pre-commit hook itself with devkit in the changed set — vitest ran git.test.ts (incl. worktree add) inside the real hook environment and passed.
  • Full suite green on push (938 files / 10,212 tests via the pre-push hook).

🤖 Generated with Claude Code

Running devkit (or its tests) inside a git hook broke every spawned git:
'git commit' exports GIT_INDEX_FILE into the hook environment, so temp-repo
tests and 'git worktree add' were silently redirected at the hook's repo and
died with '.git/index: index file open failed: Not a directory'.

NodeCommandRunner now drops GIT_DIR/GIT_WORK_TREE/GIT_INDEX_FILE and friends
from the inherited env (explicit options.env still wins), and the pre-commit
hook scrubs them for the vitest step as defense for other suites.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

This PR needs a changelog entry.

Add a fragment file to the PR (it lands with the merge — nothing is
written to main separately, and the PR number is filled in at deploy):

node scripts/changelog/new.mjs --title "Short, benefit-first headline" \
  --summary "One sentence on what the user can now do." \
  --tags app,ai --highlight "A specific user-visible point"

No user-facing change? Add the skip-changelog label instead.

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Preview: https://xnet.fyi/pr/400/app/

github-actions Bot added a commit that referenced this pull request Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant