Skip to content

[package-deps-hash] Fix build cache failures in git linked worktrees caused by GIT_DIR set by pre-commit hooks#5815

Open
istateside wants to merge 8 commits into
microsoft:mainfrom
istateside:fix/git-dir-worktree-hook-repo-root
Open

[package-deps-hash] Fix build cache failures in git linked worktrees caused by GIT_DIR set by pre-commit hooks#5815
istateside wants to merge 8 commits into
microsoft:mainfrom
istateside:fix/git-dir-worktree-hook-repo-root

Conversation

@istateside

@istateside istateside commented Jun 3, 2026

Copy link
Copy Markdown

Summary

Fixes build cache failures when Rush commands that trigger incremental build analysis run inside a git linked worktree via a pre-commit hook.

Fixes #5479

When git invokes a pre-commit hook in a linked worktree, it sets GIT_DIR to the per-worktree metadata directory (e.g. .git/worktrees/{name}) but does not set GIT_WORK_TREE. Child processes inherit this environment. getRepoRoot() calls git rev-parse --show-toplevel and inherits this GIT_DIR, causing git to return the currentWorkingDirectory argument (e.g. the rushJsonFolder subdirectory) instead of the actual worktree root. All subsequent git calls use this wrong root, causing git status -u to miss the top-level .gitignore, surfacing node_modules/ symlinks as untracked files, which git hash-object cannot hash — ultimately reported as "Build cache is only supported if running in a Git repository."

Details

The fix strips GIT_DIR and GIT_WORK_TREE from the environment before spawning any git subprocess in getRepoState.ts. This lets git auto-discover the correct repo root by scanning up from currentWorkingDirectory, which correctly resolves to the linked worktree root regardless of the hook-injected environment. Three call sites are patched: getRepoRoot, spawnGitAsync (used by getDetailedRepoStateAsync and hashFilesAsync), and getRepoChanges.

This regression was introduced in #5500, which switched from git ls-tree -r HEAD (reads committed objects, never surfaces node_modules/) to git ls-files --cached + git status -u (scans the work tree, exposing the broken .gitignore context).

The diff also includes prettier reformatting the WINDOWS_RESERVED_BASENAMES array from a compact multi-line form to one-entry-per-line — this is an unrelated side effect of the project's pre-commit hook. Happy to add a // prettier-ignore comment to suppress it if preferred.

How it was tested

Added a unit test to getRepoDeps.test.ts that sets GIT_DIR to a nonexistent path (simulating hook interference) and verifies that getRepoRoot still returns the correct repo root. Without the fix, git rev-parse exits 128 ("not a git repository") and the function throws; with the fix, GIT_DIR is stripped and git auto-discovers the root correctly.

Also manually reproduced the original failure by running a Rush build command from within a git linked worktree via a pre-commit hook and confirmed the build cache error no longer occurs with this fix applied.

Reproduction

The bug can be reproduced with this shell script: https://gist.github.com/istateside/e8b0c5f694424a423ae29fe9203ec895

The script bootstraps a Rush monorepo with all of the requisite contributing factors to recreate the bug:

  1. The repo is a Rush monorepo
  2. The base directory for the Rush monorepo is not the base directory of the git repo (rush.json is not in the root directory of the git repo)
  3. The build cache is enabled
  4. A pre-commit hook runs a Rush command

In that environment, the bug is triggered if you are in a linked worktree and make a commit, to trigger the pre-commit rush command.

@github-project-automation github-project-automation Bot moved this to Needs triage in Bug Triage Jun 3, 2026
@istateside istateside force-pushed the fix/git-dir-worktree-hook-repo-root branch from 7b20309 to 7bfddde Compare June 3, 2026 14:49
Comment thread libraries/package-deps-hash/src/getRepoState.ts
@istateside

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree company="Squarespace"

@istateside istateside marked this pull request as ready for review June 5, 2026 15:36
… calls to fix build cache in linked worktrees

When a git pre-commit hook runs in a linked worktree, git sets GIT_DIR to the
per-worktree metadata directory (.git/worktrees/{name}) without setting
GIT_WORK_TREE. With GIT_DIR set this way, `git rev-parse --show-toplevel`
returns the CWD (e.g. the rushJsonFolder subdirectory) instead of the actual
worktree root, causing all subsequent git calls to use the wrong root directory.
This makes `git status -u` miss the top-level .gitignore, surfacing node_modules
symlinks as untracked files, which then causes `git hash-object` to fail on
symlink-to-directory entries and ultimately breaks the build cache.

Fix: strip GIT_DIR and GIT_WORK_TREE from the environment in getRepoRoot,
spawnGitAsync, and getRepoChanges so git auto-discovers the correct repo root
from the working directory regardless of hook-injected env vars.
{
currentWorkingDirectory
currentWorkingDirectory,
environment: getCleanGitEnvironment()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we strip those env vars iff they point to nonexistent locations/locations without a rush.json?

Also note that AFAIK rush.json doesn't have to be at the repo root.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

the GIT_DIR directory always points to the metadata directory (./.git/, not the root of the git repository - so when GIT_DIR is set, it will never point to a directory with a rush.json file in it. So only stripping the vars if they point to locations without a rush.json would effectively be the same as always stripping them.

As for when they point to nonexistent locations - that wouldn't apply in the bug case, because the GIT_DIR value points to the metadata directory, which does exist. I believe GIT_DIR is not set in the node env when you are working in the "main worktree" (i.e. you haven't created a separate worktree at all), so if that var is set at all, it should always point to a directory that does exist. In older git versions, GIT_DIR is set to the default .git location, but stripping it still forces git to navigate upwards to find the worktree root naturally, resulting in the same logic.

The only users that might be incidentally affected by this would be those who are explicitly setting GIT_DIR to some unexpected value - but even in that case, I think stripping the variables is the better choice. The changed utils are all given a currentWorkingDirectory value, and I would expect that git would follow its typical logic to find the repo root by navigating up from the given directory. When GIT_DIR is set and GIT_WORK_TREE is missing, the logic for git rev-parse --show-toplevel changes, and it treats the cwd as the root without navigating upwards. Stripping GIT_DIR fixes the git environment for subprocesses created within hooks (such as when running rush commands in a pre-commit hook, which then run their own git commands to determine repo state)

The way this works right now is that it strips variables that are only defined in the case that we need to fix - so stripping them unconditionally feels like the right move, even though it might appear overly aggressive. I'm open to making the change more limited if you have concerns.

Also note that AFAIK rush.json doesn't have to be at the repo root.

To be clear, our use case is exactly that - our rush.json file lives in a subdirectory one level deeper than the repo root (./our-special-directory/rush.json instead of ./rush.json). Part of the intention of this fix is specifically to handle cases where the rush.json is not at the repo root, where the currentWorkingDirectory value is the subdirectory where the rush.json file lives. In the "standard" case, where the rush.json file does live at the repo root, users wouldn't be affected by this bug anyway.

Comment thread libraries/package-deps-hash/src/getRepoState.ts Outdated
istateside and others added 2 commits June 8, 2026 09:54
…ee-hook_2026-06-03-00-00.json

Co-authored-by: Ian Clanton-Thuon <iclanton@users.noreply.github.com>
Co-authored-by: Ian Clanton-Thuon <iclanton@users.noreply.github.com>
Comment thread libraries/package-deps-hash/src/getRepoState.ts Outdated
istateside and others added 2 commits June 8, 2026 14:59
"changes": [
{
"packageName": "@microsoft/rush",
"comment": "No-op change to trigger changeset for rush publish for package-deps-hash changes.",

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Not sure if we need something more descriptive here, if it ends up in a changelog or anything.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It does end up in the changelog. Can you describe the fix from a Rush user's perspective here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Actually, the text you have in the @rushstack/package-deps-hash changefile would be better here. That changefile should be updated to describe what will change for consumers of that package (without independent of Rush).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I've updated this changefile to use what was previously the description in the package-deps-hash changefile, and updated the other changefile's description to now read Strip GIT_DIR and GIT_WORK_TREE Node env variables to fix issues with miscalculating the git repo root when working in a linked worktree.

How's that read to you?

istateside and others added 2 commits June 9, 2026 10:15
…undary

The previous test set process.env.GIT_DIR and shelled out to git, but the
Jest environment does not propagate in-process env writes to child processes,
so git never saw the variable and the test passed with or without the fix.

Assert instead that getRepoRoot invokes Executable.spawnSync with an explicit
environment that omits GIT_DIR/GIT_WORK_TREE. This fails against the pre-fix
code (no environment passed) and passes with the fix.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Needs triage

Development

Successfully merging this pull request may close these issues.

[rush] rush build with build cache enabled will crash when a symlink file links to a directory

2 participants