fix: resolve real git in test shims instead of hardcoding /usr/bin/git - #19
Open
felipelalli wants to merge 1 commit into
Open
fix: resolve real git in test shims instead of hardcoding /usr/bin/git#19felipelalli wants to merge 1 commit into
felipelalli wants to merge 1 commit into
Conversation
The mock git shims in merge-branch-cli.test.ts and run.test.ts install a script named `git` into a temp dir, prepend that dir to PATH, and delegate the non-mocked commands to /usr/bin/git. That hardcoded path is not portable. On systems where /usr/bin is an envfs mount — the default on NixOS with programs.nix-ld or envfs enabled — a lookup of /usr/bin/git is resolved against the *caller's* PATH. The caller here is the shim, whose own directory sits at the front of PATH, so /usr/bin/git resolves back to the shim and the delegation recurses without bound. On this machine `npm test` never finished: the serial lane sat on run.test.ts for over an hour and the recursion had spawned ~71k processes before it was killed. On systems where /usr/bin/git is simply absent the failure is quicker but still wrong — two merge-branch-cli tests fail with ENOENT from the delegating spawn. Resolve the real git once, via `command -v git` evaluated in the test process (whose PATH has not been shadowed), and bake that absolute path into the generated shims. The resolved path points outside /usr/bin, so the delegation can no longer re-enter the shim on any system. merge-branch-cli.test.ts: 13 pass / 2 fail -> 15 pass. run.test.ts: hung indefinitely -> 39 pass in 8.2s.
There was a problem hiding this comment.
Pull request overview
This PR fixes a portability and correctness issue in test-only Git shims by resolving the real git binary once (before the shim shadows PATH) and delegating to that absolute path, rather than hardcoding /usr/bin/git. This prevents infinite recursion on envfs-mounted /usr/bin setups (e.g., NixOS) and avoids ENOENT failures on non-FHS systems.
Changes:
- Add
resolveRealGit()and a module-levelREAL_GITconstant in two test files to capture an absolutegitpath from the unmodifiedPATH. - Update the generated shim scripts to delegate to
${REAL_GIT}instead of/usr/bin/git.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/installer/run.test.ts | Resolve the real git once and update bash shim delegation to avoid /usr/bin/git recursion/absence. |
| src/cli/merge-branch-cli.test.ts | Resolve the real git once and update the Node-based shim to spawn the resolved absolute path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The mock git shims in
merge-branch-cli.test.tsandrun.test.tsinstall a script namedgitinto a temp dir, prepend that dir toPATH, and delegate the non-mocked commands to/usr/bin/git.That hardcoded path is not portable, and it fails in two different ways:
/usr/binis an envfs mount — the default on NixOS withprograms.nix-ldorenvfsenabled — a lookup of/usr/bin/gitis resolved against the caller'sPATH. The caller here is the shim, whose own directory sits at the front ofPATH, so/usr/bin/gitresolves back to the shim and the delegation recurses without bound. On the machine this was found on,npm testnever finished: the serial lane sat onrun.test.tsfor over an hour and the recursion had spawned ~71k processes before it was killed./usr/bin/gitis simply absent (non-FHS systems generally) the failure is quicker but still wrong — twomerge-branch-clitests fail withENOENTfrom the delegating spawn.The fix resolves the real git once, via
command -v gitevaluated in the test process (whosePATHhas not been shadowed), and bakes that absolute path into the generated shims. The resolved path points outside/usr/bin, so the delegation can no longer re-enter the shim on any system. On FHS systems this resolves to/usr/bin/gitand behaviour is unchanged.Results on the affected machine:
merge-branch-cli.test.ts: 13 pass / 2 fail → 15 passrun.test.ts: hung indefinitely → 39 pass in 8.2s