fix(mcp): avoid git from non-repo cwd in sibling cwd match (#1138) by magyargergo · Pull Request #1293 · abhigyanpatwari/GitNexus

magyargergo · 2026-05-03T07:31:45Z

Summary

MCP stdio servers often run with \process.cwd()\ set to a generic directory (for example the user home directory). \LocalBackend.resolveRepo()\ calls \checkCwdMatch(cwd)\ for sibling-clone drift warnings. That path used \getGitRoot(), which shells out to \git rev-parse\ from that cwd even when it is not inside any git work tree.

This change resolves the cwd git root with a cheap ancestor .git\ walk (\indGitRootByDotGit) before any \git\ subprocess runs for sibling detection. If there is no .git\ ancestor, we return \match: none\ immediately.

Fixes mcp server shells find from $HOME instead of configured repo roots — trips macOS TCC prompts for Desktop / Music / Photos #1138
Does not subsume PR fix(server): sanitize repo name to prevent argument injection #1088 (clone path sanitization) or issue npx gitnexus analyze crashes with 'Cannot destructure property package of node.target as it is null' #819 (npm/npx); those remain separate.

Testing

\cd gitnexus && npm test -- test/unit/git.test.ts test/unit/sibling-clone-drift.test.ts\
\cd gitnexus && npx tsc --noEmit\
\cd gitnexus && npm test\ (full suite; Vitest reported one worker exit warning with exit code 0)

Definition of Done

Runtime path for MCP sibling \cwd\ match no longer invokes \git\ from arbitrary non-repo cwd.
Unit tests cover \indGitRootByDotGit\ and \checkCwdMatch\ for a non-git cwd with a registered repo.

Made with Cursor

checkCwdMatch used getGitRoot(cwd), which runs git rev-parse from the launch cwd (often \C:\Users\gergo in MCP stdio). Resolve the cwd git root via ancestor .git checks first, then keep existing remote-based sibling logic. Fixes #1138 Co-authored-by: Cursor <cursoragent@cursor.com>

vercel · 2026-05-03T07:31:50Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
gitnexus	Ready	Preview, Comment	May 3, 2026 8:00am

github-actions · 2026-05-03T07:39:10Z

Claude finished @magyargergo's task in 3m 45s —— View job

PR #1293 Review — fix(mcp): avoid git from non-repo cwd in sibling cwd match

Tasks:

Review bar for this PR

checkCwdMatch() must not spawn git when cwd has no .git ancestor (core fix, DoD §2.1)
Exact path and ancestor path matching must still happen before any git-root detection (DoD §2.1 — real runtime path)
Sibling-by-remote detection must still work when cwd is inside a normal git repo (DoD §2.1 — regression bar)
findGitRootByDotGit() must be pure filesystem: no subprocess, cross-platform, handles file/dir/missing inputs (DoD §2.5 security, §2.7 tests)
getGitRoot() must be unchanged for all other callers (DoD §2.4 contracts)
Tests must cover the real changed path and be deterministic (DoD §2.7)
CI (all platforms), typecheck, format, lint, and Docker must be green (DoD §4.2)

Issue and PR context considered

#1138 symptom: MCP stdio servers launch with process.cwd() = $HOME (or another non-repo dir). LocalBackend.resolveRepo() calls maybeWarnSiblingDrift() → checkCwdMatch(cwd), which previously called getGitRoot(cwd) → execSync('git rev-parse --show-toplevel', { cwd }). From $HOME or any non-repo dir, git exits non-zero, causing a subprocess error on every MCP tool call before the warnedSiblingDrift cache primes.

Changed files: git-staleness.ts (5 lines changed), git.ts (+28 new function), git.test.ts (+35 tests), sibling-clone-drift.test.ts (+20 test).

CI status (at time of review):

Job	Status
tests / ubuntu / coverage	✅ success
tests / macos-latest	✅ success
tests / windows-latest	⏳ in progress
quality / typecheck	✅ success
quality / typecheck-web	✅ success
quality / lint	✅ success
quality / format	✅ success
Docker Build & Push gitnexus-web	✅ success
Docker Build & Push gitnexus	⏳ in progress

Local validation claims: focused tests passed, npx tsc --noEmit passed, npm test passed with one Vitest worker warning (exit code 0). Not yet confirmed in CI for Windows.

Core risks: .git file behavior (linked worktrees) not tested; no-spawn assertion in sibling drift test is behaviorally weak; Windows CI not yet green.

Understanding of the change

Before this PR: checkCwdMatch() reached the sibling-by-remote branch and called getGitRoot(cwd) → execSync('git rev-parse --show-toplevel', { cwd: nonRepoCwd }). Git exits 128, getGitRoot catches and returns null, checkCwdMatch returns { match: 'none' }. The git subprocess was still spawned and errored on every call until the warnedSiblingDrift cache primed (and the cache only primes after the full checkCwdMatch call completes).

After this PR: checkCwdMatch() calls findGitRootByDotGit(cwdResolved) first. This walks ancestor directories checking statSync(path.join(current, '.git')) — pure filesystem, no subprocess. If no .git ancestor exists, it returns null and checkCwdMatch returns { match: 'none' } immediately. getRemoteUrl() (which calls execSync) is only reached after a .git ancestor is confirmed.

Unchanged stages:

Stage 1: exact path / longest-prefix path match (git-staleness.ts:90–102) — untouched
Stage 3: getGitRoot() still available for all other callers (analyze CLI, etc.) — untouched
Remote URL fingerprint normalization — untouched
maybeWarnSiblingDrift caching and stderr output — untouched

Findings

[medium] No-spawn assertion in sibling drift test is behaviorally weak

Category: Test coverage
Files: gitnexus/test/unit/sibling-clone-drift.test.ts:236–254
Issue: The new test "returns match=none for a non-git cwd before resolving sibling remotes" only asserts m.match === 'none'. Before this PR, the old path (getGitRoot → execSync → throws) would also have returned { match: 'none' } from a non-repo temp dir — so this test would have passed before the fix. It does not prove that no subprocess is spawned.
Why it matters here: The bug in mcp server shells find from $HOME instead of configured repo roots — trips macOS TCC prompts for Desktop / Music / Photos #1138 is a silent subprocess side-effect, not an incorrect return value. If the fix is accidentally reverted or partially broken, this test will not catch it.
Recommended fix: Add a vi.spyOn on child_process.execSync (and execFileSync) at the test level, call checkCwdMatch(nonGitCwd), and assert neither spy was called. Alternatively, add a spy on getRemoteUrl from git.ts and assert it was not called. The git.test.ts mock approach (vi.mock('child_process')) is the right model.
Blocks merge: No — the code fix is correct and git.test.ts independently proves findGitRootByDotGit doesn't call execSync. But this is a meaningful test gap given the bug class. Fix this →

[medium] Linked worktree / .git-as-file case not tested

Category: Test coverage / edge case
Files: gitnexus/test/unit/git.test.ts, gitnexus/src/storage/git.ts:103-123
Issue: findGitRootByDotGit uses statSync(path.join(current, '.git')) which succeeds for both .git directories and .git files (linked worktrees, submodules). The code is correct — returning the directory containing the .git file is the right root for git -C <root> remote .... But there is no test that creates a .git file and asserts the function returns the parent directory, nor that getRemoteUrl() works from that root. hasGitDir() (already in git.ts) takes the same approach and lacks a .git-file test too.
Why it matters here: GitNexus users working from git worktrees (common in monorepo workflows) will exercise this path. If .git-as-file behavior regresses in a future refactor, there's no test to catch it.
Recommended fix: Add a test in git.test.ts that creates a temp dir with a .git file (e.g., fs.writeFileSync(path.join(tmpDir, '.git'), 'gitdir: /fake')) and asserts findGitRootByDotGit(nested) returns tmpDir. This covers the worktree case without needing a real git worktree.
Blocks merge: No — the code is correct and the risk is reasoned through. Fix this →

[low] Nonexistent and file-input paths not tested

Category: Test coverage
Files: gitnexus/test/unit/git.test.ts
Issue: findGitRootByDotGit('/nonexistent/path') should return null (caught by initial statSync). A file path inside a repo should return the repo root (via the isDirectory() check and dirname step). Neither is tested.
Why it matters here: MCP launch directories that no longer exist (deleted /tmp dirs, race conditions) will hit the nonexistent path. The implementation handles it correctly but without test coverage.
Recommended fix: Add findGitRootByDotGit('/tmp/does-not-exist-XXXX') → null and findGitRootByDotGit(path.join(repoRoot, 'file.ts')) → repoRoot test cases.
Blocks merge: No.

[critical — blocks merge] Windows CI and Docker still in progress

Category: Validation / CI
Files: CI
Issue: tests / windows-latest and Docker Build & Push gitnexus are still running. findGitRootByDotGit uses path.resolve, path.join, and path.dirname which are nominally cross-platform, but Windows path separator behavior (drive letters, UNC paths, path.sep = '\\') and the parent === current root-detection termination are untested in CI.
Why it matters here: The parent === current sentinel correctly terminates at the filesystem root on Unix (/). On Windows, drive root is C:\ and path.dirname('C:\\') returns 'C:\\' — this is correct, but must be confirmed green in CI.
Recommended fix: Wait for Windows CI and Docker to complete before merging.
Blocks merge: Yes.

Definition of Done check

DoD Item	Status
`checkCwdMatch()` does not spawn git when `cwd` is outside any `.git` ancestor	✅ satisfied
Exact path and ancestor path matching still work before sibling-remote matching	✅ satisfied
Sibling-by-remote matching still works when `cwd` is inside a normal git repo	✅ satisfied
Linked worktrees / `.git` files handled acceptably or documented	⚠️ partially satisfied (code correct, not tested)
Invalid `.git` entries do not create noisy errors or misleading matches	✅ satisfied (`getRemoteUrl` fails silently on invalid git dirs)
`findGitRootByDotGit()` is cross-platform, handles file/dir inputs, never shells out	⚠️ partially satisfied (Windows CI in progress)
`getGitRoot()` behavior remains unchanged for other callers	✅ satisfied
Tests prove the non-git cwd fix and preserve existing sibling drift behavior	⚠️ partially satisfied (no-spawn proof is weak; return-value tests pass)
CI, typecheck, and focused tests are green	❌ not satisfied (Windows CI + Docker in progress)

Git root detection assessment

findGitRootByDotGit implementation (git.ts:103-123):

path.resolve(fromPath) normalizes the input correctly ✅
If fromPath is a file (not a directory), uses path.dirname — correct ✅
If fromPath does not exist, statSync throws and returns null ✅
Ancestor walk: terminates when path.dirname(current) === current (filesystem root) ✅
Accepts .git directory and .git file equally (statSync succeeds for both) ✅
No subprocess (no import of child_process) ✅
Cross-platform: path.join, path.resolve, path.dirname handle both separators ✅
Permission errors on intermediate dirs: statSync(path.join(current, '.git')) throws → moves to parent ✅ (acceptable)
Does not shadow getGitRoot() — separate export ✅

Normal .git directory: tested in git.test.ts with real temp dir ✅

.git file / linked worktree: code is correct (statSync sees the file), not tested ⚠️

Nonexistent paths: implementation handles it (initial statSync returns null), not tested ⚠️

File inputs: implementation handles it (dirname step), not tested ⚠️

Invalid .git entries (random file named .git): findGitRootByDotGit returns the parent as if it were a git root. getRemoteUrl then runs git config --get remote.origin.url from that dir, which fails silently → returns undefined → checkCwdMatch returns { match: 'none' }. No noisy error ✅

Sibling drift assessment

Exact/path match precedence: Unchanged. Lines 90–102 in git-staleness.ts run before any .git detection. Stage 1 returns on first match. ✅

Non-git cwd behavior: findGitRootByDotGit returns null → { match: 'none' } without any subprocess ✅

Sibling-by-remote behavior: Preserved. Once a .git root is found, getRemoteUrl → remote fingerprint → entries.find() → sibling match logic is identical to pre-PR code ✅

Stale hint behavior: commitsAheadOfIndexed and hint construction are unchanged ✅

Remote fingerprint matching: getRemoteUrl() normalization logic unchanged ✅

No subprocess from arbitrary cwd: Confirmed by code inspection. findGitRootByDotGit uses only statSync. getRemoteUrl (subprocess) is only reached after findGitRootByDotGit returns non-null ✅

Test assessment

git.test.ts:

findGitRootByDotGit ancestor test: creates real .git dir, walks from nested subdir, asserts result and mockExecSync.not.toHaveBeenCalled() ✅
Non-repo test: real temp dir, no .git, asserts null and no execSync ✅
vi.mock('child_process') is module-level, applies to all tests in file ✅
beforeEach clears mocks to prevent cross-test pollution ✅
Temp dirs cleaned up in finally blocks ✅
Existing getGitRoot tests still exercise the subprocess path ✅
Missing: .git file test, nonexistent input test, file-input test ⚠️

sibling-clone-drift.test.ts:

New test creates a registered repo + non-git temp dir, calls checkCwdMatch(nonGitCwd), asserts match: none ✅
Critical weakness: This test would have passed before the PR (old getGitRoot also returns null from non-git dir, yielding the same { match: 'none' } result). No spy proves git wasn't spawned ⚠️
Existing sibling-by-remote tests use real git repos, real commits, real execSync — they prove the happy path still works ✅
Hint/drift/no-drift cases well covered ✅

No-spawn proof strength: Strong for findGitRootByDotGit in isolation (git.test.ts mock). Weak for checkCwdMatch end-to-end in the non-git scenario (no spy in sibling-clone-drift.test.ts). Overall: acceptable for merge given code-level reasoning, but worth improving.

Validation assessment

Check	Status
typecheck (`npx tsc --noEmit`)	✅ CI green
lint	✅ CI green
format (prettier)	✅ CI green
tests / ubuntu	✅ CI green
tests / macos	✅ CI green
tests / windows	⏳ in progress
Docker Build & Push gitnexus	⏳ in progress
Vitest worker warning	Unverified — not visible in CI logs yet
Local focused tests	Claimed by PR author; not CI-verified for Windows

Final verdict

not production-ready

Windows CI (tests / windows-latest) and Docker Build & Push gitnexus are still in progress. Per the review bar, CI must be fully green before merge — the fix touches cross-platform path logic (parent === current termination on Windows drive roots, path.sep behavior) that must pass on all CI platforms.

The code fix itself is correct: findGitRootByDotGit is a clean, subprocess-free ancestor walk; the import is wired correctly in checkCwdMatch; getGitRoot is preserved for all other callers; and the sibling drift happy-path tests remain solid. Two medium-severity test gaps (no-spawn proof in sibling-clone-drift.test.ts, missing .git-file worktree test) do not block merge but should be addressed as follow-ups or pre-merge additions.

Once Windows CI and Docker complete green, and the Vitest worker warning is confirmed non-causal, this can be re-evaluated as production-ready with minor follow-ups.

View CI run · View Docker run · PR branch

github-actions · 2026-05-03T07:43:56Z

CI Report

✅ All checks passed

Pipeline Status

Stage	Status	Details
✅ Typecheck	`success`	tsc --noEmit
✅ Tests	`success`	unit tests, 3 platforms
✅ E2E	`success`	gitnexus-web changes only

Test Results

Tests	Passed	Failed	Skipped	Duration
7791	7790	0	1	356s

✅ All 7790 tests passed

1 test(s) skipped — expand for details

buildTypeEnv > known limitations (documented skip tests) > Ruby block parameter: users.each { |user| } — closure param inference, different feature

Code Coverage

Tests

Metric	Coverage	Covered	Base	Delta	Status
Statements	77.25%	23082/29876	77.02%	📈 +0.2	🟢 ███████████████░░░░░
Branches	66.13%	14794/22370	65.97%	📈 +0.2	🟢 █████████████░░░░░░░
Functions	82.23%	2269/2759	81.86%	📈 +0.4	🟢 ████████████████░░░░
Lines	80.15%	20828/25986	79.89%	📈 +0.3	🟢 ████████████████░░░░

_{📋 View full run · Generated by CI}

Three test gaps flagged by review on the #1138 fix: - sibling-clone-drift.test.ts: the existing "non-git cwd" test only asserted match=none, which the pre-fix code also returned (by silently failing the spawn). Wrap child_process / node:child_process with passthrough vi.fn() spies and assert no execSync/execFileSync call is recorded when checkCwdMatch runs against a non-git cwd, so a regression that re-introduces the spawn fails loudly. - git.test.ts: add coverage for findGitRootByDotGit's three untested inputs — a `.git` FILE (linked worktree / submodule), a path that does not exist, and a file path inside a repo (must walk from the parent dir). Each asserts no subprocess was spawned. No production code changes. Test additions only.

…atwari#1138) (abhigyanpatwari#1293) * fix(mcp): avoid git shellout from non-repo cwd for sibling match checkCwdMatch used getGitRoot(cwd), which runs git rev-parse from the launch cwd (often \C:\Users\gergo in MCP stdio). Resolve the cwd git root via ancestor .git checks first, then keep existing remote-based sibling logic. Fixes abhigyanpatwari#1138 Co-authored-by: Cursor <cursoragent@cursor.com> * test(mcp): address PR abhigyanpatwari#1293 review follow-ups Three test gaps flagged by review on the abhigyanpatwari#1138 fix: - sibling-clone-drift.test.ts: the existing "non-git cwd" test only asserted match=none, which the pre-fix code also returned (by silently failing the spawn). Wrap child_process / node:child_process with passthrough vi.fn() spies and assert no execSync/execFileSync call is recorded when checkCwdMatch runs against a non-git cwd, so a regression that re-introduces the spawn fails loudly. - git.test.ts: add coverage for findGitRootByDotGit's three untested inputs — a `.git` FILE (linked worktree / submodule), a path that does not exist, and a file path inside a repo (must walk from the parent dir). Each asserts no subprocess was spawned. No production code changes. Test additions only. --------- Co-authored-by: Cursor <cursoragent@cursor.com>

vercel Bot deployed to Preview May 3, 2026 07:31 View deployment

vercel Bot deployed to Preview May 3, 2026 08:00 View deployment

magyargergo merged commit 114d530 into main May 3, 2026
20 checks passed

magyargergo deleted the fix/mcp-cwdmatch-dotgit-guard-1138 branch May 3, 2026 08:17

magyargergo mentioned this pull request May 10, 2026

chore: release v1.6.4 #1473

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(mcp): avoid git from non-repo cwd in sibling cwd match (#1138)#1293

fix(mcp): avoid git from non-repo cwd in sibling cwd match (#1138)#1293
magyargergo merged 2 commits into
mainfrom
fix/mcp-cwdmatch-dotgit-guard-1138

magyargergo commented May 3, 2026

Uh oh!

vercel Bot commented May 3, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 3, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 3, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

magyargergo commented May 3, 2026

Summary

Related

Testing

Definition of Done

Uh oh!

vercel Bot commented May 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented May 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR #1293 Review — fix(mcp): avoid git from non-repo cwd in sibling cwd match

Review bar for this PR

Issue and PR context considered

Understanding of the change

Findings

[medium] No-spawn assertion in sibling drift test is behaviorally weak

[medium] Linked worktree / .git-as-file case not tested

[low] Nonexistent and file-input paths not tested

[critical — blocks merge] Windows CI and Docker still in progress

Definition of Done check

Git root detection assessment

Sibling drift assessment

Test assessment

Validation assessment

Final verdict

Uh oh!

github-actions Bot commented May 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI Report

Pipeline Status

Test Results

Code Coverage

Tests

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vercel Bot commented May 3, 2026 •

edited

Loading

github-actions Bot commented May 3, 2026 •

edited

Loading

github-actions Bot commented May 3, 2026 •

edited

Loading