Skip to content

fix: set GIT_WORK_TREE in pre-commit hook for git worktree compatibility#1207

Merged
gcko merged 2 commits intomainfrom
fix/pre-commit-worktree
Mar 17, 2026
Merged

fix: set GIT_WORK_TREE in pre-commit hook for git worktree compatibility#1207
gcko merged 2 commits intomainfrom
fix/pre-commit-worktree

Conversation

@clkao
Copy link
Contributor

@clkao clkao commented Mar 17, 2026

Summary

  • biome check --staged fails silently (0 files checked) when running in a git worktree because git sets GIT_DIR but not GIT_WORK_TREE when invoking hooks
  • biome needs both to resolve staged file paths from the js/ subdirectory
  • One-line fix: pass GIT_WORK_TREE="$REPO_ROOT" to the pnpm lint command

Test plan

  • Verify git commit with staged JS/TS files works in a git worktree
  • Verify git commit still works in the main repo (non-worktree)
  • Verify biome check --staged finds and checks the correct files

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
Signed-off-by: CL Kao clkao@datarecce.io

biome check --staged needs GIT_WORK_TREE to resolve staged file paths
when running from a subdirectory in a git worktree. Git sets GIT_DIR
but not GIT_WORK_TREE when invoking hooks.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: CL Kao <clkao@datarecce.io>
@gcko gcko self-requested a review March 17, 2026 02:59
@gcko gcko added the Improvement Created by Linear-GitHub Sync label Mar 17, 2026
Move GIT_WORK_TREE from inline on lint:staged to a top-level export in
pre-commit so it covers git diff --cached (the guard condition) and all
downstream tools. Add the same export to pre-push for consistency, since
vitest and tsc could shell out to git internally.

Add tests/test_hooks_worktree.sh to verify biome --staged behavior from
js/ with and without GIT_WORK_TREE.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Jared Scott <jared.scott@datarecce.io>
Copy link
Contributor

@gcko gcko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: Adversarial analysis + follow-up fixes

Issues found with the original PR

1. Only fixed part of the problem - pre-push was unaddressed
The pre-push hook has the same worktree exposure. While its git operations are mostly commit-based (git diff SHA..SHA), tools like vitest related and tsc could internally shell out to git. Consistency across hooks prevents future surprises.

2. Inline GIT_WORK_TREE only covered biome, not the guard condition
The original fix set GIT_WORK_TREE inline on line 14 (pnpm lint:staged), but git diff --cached on line 10 - which decides whether to lint at all - ran without it. In a worktree where git environment is incomplete, this guard could silently return empty, skipping lint entirely. Moving to a top-level export covers both.

3. Shifting diagnosis in the Slack thread raised confidence concerns
The root cause bounced between sandbox EPERM, biome .git file handling, monorepo path prefix mismatch, and missing GIT_WORK_TREE. Testing was needed to confirm the actual behavior.

What was done

  • Moved GIT_WORK_TREE from inline on pnpm lint:staged to export GIT_WORK_TREE at the top of pre-commit (covers all git operations in the script)
  • Added the same export to pre-push for consistency
  • Verified commit-msg needs no changes (only reads a file path, no git worktree interaction)
  • Added tests/test_hooks_worktree.sh for manual verification

Testing

Ran tests/test_hooks_worktree.sh - all 4 tests pass:

Test Result
biome check --staged finds staged files from js/ (no GIT_WORK_TREE) PASS
biome check --staged finds staged files from js/ (with GIT_WORK_TREE) PASS
git diff --cached returns repo-root-relative paths PASS
git rev-parse --show-toplevel resolves correctly PASS

This confirms:

  • biome --staged works correctly from js/ in normal repos (the Slack reply 30 "always been broken" theory was wrong)
  • Setting GIT_WORK_TREE redundantly in a non-worktree repo is harmless
  • The export approach is safe for both worktree and non-worktree usage

Copy link
Contributor

@gcko gcko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gcko gcko merged commit 4a0543a into main Mar 17, 2026
19 checks passed
@gcko gcko deleted the fix/pre-commit-worktree branch March 17, 2026 03:13
@codecov
Copy link

codecov bot commented Mar 17, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
see 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

Improvement Created by Linear-GitHub Sync

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants