fix(detect-changes): guard resolveWorktreeCwd against overriding a separately-indexed worktree (#1691)
Some checks are pending
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
CodeQL / Analyze (python) (push) Waiting to run
Gitleaks / gitleaks (push) Waiting to run
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run

* fix(detect-changes): guard resolveWorktreeCwd against overriding a separately-indexed worktree

When the repo registry entry points to a linked worktree (both main
checkout and worktree indexed separately), resolveWorktreeCwd was
incorrectly replacing the correct worktree repoPath with the server's
main-checkout launch directory. Both share the same canonical root so
the existing same-repo check passed, causing git diff to run from the
wrong directory and return 0 changes (issue #1659).

Fix: early-exit guard — if tryRealpath(repoPath) differs from
tryRealpath(getCanonicalRepoRoot(repoPath)), repoPath is itself a
linked worktree and is returned unchanged. Auto-detection only fires
when repoPath equals the canonical main-checkout root.

Also normalises the launchCanonical comparison in the auto-detect path
to use tryRealpath for cross-platform consistency.

Regression test: 'returns worktreeDir unchanged when repoPath IS a
linked worktree and launchCwd is the main checkout'.

* test(detect-changes): add worktreeA→worktreeB case and assumption comment

Cover the missing case from the production-readiness review:
repoPath = wt-A (indexed), launchCwd = wt-B (server on a different
linked worktree). The guard fires on repoPath being a worktree
regardless of launchCwd, so wt-A is returned unchanged.

Also add an inline comment documenting the assumption that repoPath
is a git root or linked-worktree root (not an arbitrary subdirectory),
as noted in Finding 2 of the review.

* refactor(detect-changes): validate repoPath is a git root before canonical comparison

Instead of relying on a comment asserting repoPath is always a git
root, call getGitRoot(repoPath) first. Only if the result matches
repoPath itself do we call getCanonicalRepoRoot and apply the guard.

This eliminates the over-classification risk for subdirectory repoPath
values and makes the assumption explicit in code. repoCanonical is
shared across both the guard and the auto-detect block.

---------

Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
This commit is contained in:
Nilotpal Kashyap 2026-05-20 11:16:16 +05:30 committed by GitHub
parent 92ad0f5491
commit d7e1815aa3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 104 additions and 2 deletions

View file

@ -248,6 +248,29 @@ function tryRealpath(p: string): string {
*/
export function resolveWorktreeCwd(repoPath: string, launchCwd: string): string {
try {
// Verify repoPath is a git root before comparing against its canonical
// root. If getGitRoot returns a different path, repoPath is an arbitrary
// subdirectory — skip both the linked-worktree guard and auto-detection
// and fall through to the repoPath fallback.
const repoGitRoot = getGitRoot(repoPath);
const repoCanonical =
repoGitRoot && tryRealpath(repoGitRoot) === tryRealpath(repoPath)
? getCanonicalRepoRoot(repoPath)
: null;
// Early exit: if repoPath is a linked worktree (differs from its canonical
// main-checkout root), return it unchanged. Do NOT override it with the
// server's launch directory — that would silently replace the explicitly-
// resolved worktree index with the main checkout.
//
// getCanonicalRepoRoot returns the main-checkout path for both the checkout
// and all linked worktrees:
// repoPath === canonical → main checkout (auto-detect may fire below)
// repoPath !== canonical → linked worktree (return as-is)
if (repoCanonical && tryRealpath(repoPath) !== tryRealpath(repoCanonical)) {
return repoPath;
}
const launchGitRoot = getGitRoot(launchCwd);
if (launchGitRoot) {
// Normalise via realpathSync before comparing so macOS /var → /private/var
@ -256,8 +279,12 @@ export function resolveWorktreeCwd(repoPath: string, launchCwd: string): string
const realRepo = tryRealpath(repoPath);
if (realLaunch !== realRepo) {
const launchCanonical = getCanonicalRepoRoot(launchCwd);
const repoCanonical = getCanonicalRepoRoot(repoPath);
if (launchCanonical && repoCanonical && launchCanonical === repoCanonical) {
// Use tryRealpath on both canonical values for cross-platform safety.
if (
launchCanonical &&
repoCanonical &&
tryRealpath(launchCanonical) === tryRealpath(repoCanonical)
) {
return launchGitRoot;
}
}

View file

@ -189,6 +189,81 @@ describe('resolveWorktreeCwd — auto-detection helper', () => {
rmSync(repoB, { recursive: true, force: true });
}
});
it('returns worktreeDir unchanged when repoPath IS a linked worktree and launchCwd is the main checkout', () => {
// Regression for: detect_changes returns no changes when the MCP server
// runs from the main checkout but the resolved repo index is a separately-
// indexed linked worktree (issue #1659 / dpearson2699 report).
//
// Before the fix, resolveWorktreeCwd would detect that launchCwd (main
// checkout) and repoPath (worktree) share the same canonical root and
// wrongly override repoPath with the main-checkout path, causing git diff
// to run from the wrong directory and return 0 changes.
const repoDir = mkdtempSync(path.join(os.tmpdir(), 'gitnexus-rwc-idx-wt-'));
try {
execSync('git init -q', { cwd: repoDir, stdio: 'ignore' });
execSync('git config user.email "test@example.com"', { cwd: repoDir, stdio: 'ignore' });
execSync('git config user.name "Test"', { cwd: repoDir, stdio: 'ignore' });
writeFileSync(path.join(repoDir, 'x.ts'), 'export const x = 1;\n');
execSync('git add x.ts', { cwd: repoDir, stdio: 'ignore' });
execSync('git commit -q -m "initial"', { cwd: repoDir, stdio: 'ignore' });
const worktreeDir = path.join(repoDir, 'wt-indexed');
execSync(`git worktree add -q -b indexed "${worktreeDir}"`, {
cwd: repoDir,
stdio: 'ignore',
});
// Simulate: repo registry entry points to the worktree (repoPath = worktreeDir)
// but the MCP server was launched from the main checkout (launchCwd = repoDir).
// resolveWorktreeCwd must NOT override the correct worktree path with repoDir.
const result = resolveWorktreeCwd(worktreeDir, repoDir);
expect(realpathSync.native(result)).toBe(realpathSync.native(worktreeDir));
expect(realpathSync.native(result)).not.toBe(realpathSync.native(repoDir));
} finally {
try {
execSync('git worktree remove -f wt-indexed', { cwd: repoDir, stdio: 'ignore' });
} catch {
// ignore
}
rmSync(repoDir, { recursive: true, force: true });
}
});
it('returns worktreeA unchanged when both repoPath and launchCwd are different linked worktrees of the same repo', () => {
// Covers: repoPath = wt-A (indexed), launchCwd = wt-B (server launched from another worktree).
// The guard fires on repoPath being a linked worktree regardless of what launchCwd is,
// so wt-A must be returned unchanged — not wt-B, not the main checkout.
const repoDir = mkdtempSync(path.join(os.tmpdir(), 'gitnexus-rwc-two-wt-'));
try {
execSync('git init -q', { cwd: repoDir, stdio: 'ignore' });
execSync('git config user.email "test@example.com"', { cwd: repoDir, stdio: 'ignore' });
execSync('git config user.name "Test"', { cwd: repoDir, stdio: 'ignore' });
writeFileSync(path.join(repoDir, 'x.ts'), 'export const x = 1;\n');
execSync('git add x.ts', { cwd: repoDir, stdio: 'ignore' });
execSync('git commit -q -m "initial"', { cwd: repoDir, stdio: 'ignore' });
const worktreeA = path.join(repoDir, 'wt-a');
const worktreeB = path.join(repoDir, 'wt-b');
execSync(`git worktree add -q -b branch-a "${worktreeA}"`, { cwd: repoDir, stdio: 'ignore' });
execSync(`git worktree add -q -b branch-b "${worktreeB}"`, { cwd: repoDir, stdio: 'ignore' });
// repoPath = wt-A (the indexed worktree), launchCwd = wt-B (where the server runs).
// resolveWorktreeCwd must return wt-A — the indexed path — unchanged.
const result = resolveWorktreeCwd(worktreeA, worktreeB);
expect(realpathSync.native(result)).toBe(realpathSync.native(worktreeA));
expect(realpathSync.native(result)).not.toBe(realpathSync.native(worktreeB));
expect(realpathSync.native(result)).not.toBe(realpathSync.native(repoDir));
} finally {
try {
execSync('git worktree remove -f wt-a', { cwd: repoDir, stdio: 'ignore' });
execSync('git worktree remove -f wt-b', { cwd: repoDir, stdio: 'ignore' });
} catch {
// ignore
}
rmSync(repoDir, { recursive: true, force: true });
}
});
});
// ── Guard logic via real path arithmetic ─────────────────────────────────────