mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-29 01:41:42 +00:00
* 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 #1138 Co-authored-by: Cursor <cursoragent@cursor.com> * test(mcp): address PR #1293 review follow-ups 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. --------- Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
1f7d45de10
commit
30982c6d38
4 changed files with 161 additions and 7 deletions
|
|
@ -6,7 +6,7 @@
|
|||
import { execFileSync } from 'node:child_process';
|
||||
import path from 'path';
|
||||
import { readRegistry, type RegistryEntry, type CwdMatch } from '../storage/repo-manager.js';
|
||||
import { getGitRoot, getCurrentCommit, getRemoteUrl } from '../storage/git.js';
|
||||
import { findGitRootByDotGit, getCurrentCommit, getRemoteUrl } from '../storage/git.js';
|
||||
|
||||
export interface StalenessInfo {
|
||||
isStale: boolean;
|
||||
|
|
@ -101,9 +101,10 @@ export async function checkCwdMatch(cwd: string): Promise<CwdMatch> {
|
|||
}
|
||||
if (bestPath) return { match: 'path', entry: bestPath };
|
||||
|
||||
// 2) Sibling-by-remote: locate the cwd's git root, get its remote
|
||||
// URL, and look for any registered entry with the same fingerprint.
|
||||
const cwdGitRoot = getGitRoot(cwdResolved);
|
||||
// 2) Sibling-by-remote: locate the cwd's git root using only ancestor
|
||||
// `.git` checks before shelling out. This keeps MCP startup from
|
||||
// running git in an unrelated launch cwd such as $HOME (#1138).
|
||||
const cwdGitRoot = findGitRootByDotGit(cwdResolved);
|
||||
if (!cwdGitRoot) return { match: 'none' };
|
||||
|
||||
const cwdRemote = getRemoteUrl(cwdGitRoot);
|
||||
|
|
|
|||
|
|
@ -93,6 +93,34 @@ export const getGitRoot = (fromPath: string): string | null => {
|
|||
return null;
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* Find a git root by checking only `.git` entries on the ancestor chain.
|
||||
*
|
||||
* Unlike `getGitRoot`, this does not spawn `git`, so MCP can cheaply decide
|
||||
* whether a launch cwd is a worktree before running any subprocess there.
|
||||
*/
|
||||
export const findGitRootByDotGit = (fromPath: string): string | null => {
|
||||
let current = path.resolve(fromPath);
|
||||
try {
|
||||
if (!statSync(current).isDirectory()) {
|
||||
current = path.dirname(current);
|
||||
}
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
|
||||
while (true) {
|
||||
try {
|
||||
statSync(path.join(current, '.git'));
|
||||
return current;
|
||||
} catch {
|
||||
const parent = path.dirname(current);
|
||||
if (parent === current) return null;
|
||||
current = parent;
|
||||
}
|
||||
}
|
||||
};
|
||||
/**
|
||||
* Check whether a directory contains a .git entry (file or folder).
|
||||
*
|
||||
|
|
|
|||
|
|
@ -1,6 +1,14 @@
|
|||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import { execSync } from 'child_process';
|
||||
import { isGitRepo, getCurrentCommit, getGitRoot } from '../../src/storage/git.js';
|
||||
import fs from 'fs';
|
||||
import os from 'os';
|
||||
import path from 'path';
|
||||
import {
|
||||
isGitRepo,
|
||||
getCurrentCommit,
|
||||
getGitRoot,
|
||||
findGitRootByDotGit,
|
||||
} from '../../src/storage/git.js';
|
||||
|
||||
// Mock child_process.execSync
|
||||
vi.mock('child_process', () => ({
|
||||
|
|
@ -92,4 +100,68 @@ describe('git utilities', () => {
|
|||
expect(result!.trim()).toBe(result);
|
||||
});
|
||||
});
|
||||
|
||||
describe('findGitRootByDotGit', () => {
|
||||
it('finds an ancestor .git directory without spawning git', () => {
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-dotgit-'));
|
||||
try {
|
||||
fs.mkdirSync(path.join(tmpDir, '.git'));
|
||||
const nested = path.join(tmpDir, 'packages', 'app');
|
||||
fs.mkdirSync(nested, { recursive: true });
|
||||
|
||||
expect(findGitRootByDotGit(nested)).toBe(path.resolve(tmpDir));
|
||||
expect(mockExecSync).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('returns null outside a git worktree without spawning git', () => {
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-nonrepo-'));
|
||||
try {
|
||||
expect(findGitRootByDotGit(tmpDir)).toBeNull();
|
||||
expect(mockExecSync).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
// Linked worktrees and submodules use a `.git` file (not directory) that
|
||||
// points at the real gitdir. statSync succeeds for both, so the ancestor
|
||||
// walk should treat such roots identically to ordinary repos.
|
||||
it('treats a .git file (linked worktree) as a valid root', () => {
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-worktree-'));
|
||||
try {
|
||||
fs.writeFileSync(path.join(tmpDir, '.git'), 'gitdir: /fake/worktrees/wt\n');
|
||||
const nested = path.join(tmpDir, 'src', 'pkg');
|
||||
fs.mkdirSync(nested, { recursive: true });
|
||||
|
||||
expect(findGitRootByDotGit(nested)).toBe(path.resolve(tmpDir));
|
||||
expect(mockExecSync).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('returns null when the input path does not exist', () => {
|
||||
const missing = path.join(os.tmpdir(), `gitnexus-missing-${Date.now()}-${Math.random()}`);
|
||||
expect(findGitRootByDotGit(missing)).toBeNull();
|
||||
expect(mockExecSync).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('walks from a file input by starting at its parent directory', () => {
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-fileinput-'));
|
||||
try {
|
||||
fs.mkdirSync(path.join(tmpDir, '.git'));
|
||||
const filePath = path.join(tmpDir, 'pkg', 'index.ts');
|
||||
fs.mkdirSync(path.dirname(filePath), { recursive: true });
|
||||
fs.writeFileSync(filePath, 'export {};\n');
|
||||
|
||||
expect(findGitRootByDotGit(filePath)).toBe(path.resolve(tmpDir));
|
||||
expect(mockExecSync).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -14,9 +14,24 @@
|
|||
* `checkCwdMatch` API.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
|
||||
import path from 'path';
|
||||
import { execSync } from 'child_process';
|
||||
import { execSync, execFileSync } from 'child_process';
|
||||
|
||||
// Wrap child_process exports in spies that pass through to the real
|
||||
// implementation. Test setup (initRepoWithCommit, etc.) keeps working
|
||||
// against real git; individual tests can clear + assert call counts to
|
||||
// prove no subprocess was launched. Both module specifiers are mocked
|
||||
// because git-staleness.ts imports from 'node:child_process' while this
|
||||
// file (and storage/git.ts) import from 'child_process'.
|
||||
vi.mock('child_process', async () => {
|
||||
const actual = await vi.importActual<typeof import('child_process')>('child_process');
|
||||
return { ...actual, execSync: vi.fn(actual.execSync), execFileSync: vi.fn(actual.execFileSync) };
|
||||
});
|
||||
vi.mock('node:child_process', async () => {
|
||||
const actual = await vi.importActual<typeof import('node:child_process')>('node:child_process');
|
||||
return { ...actual, execSync: vi.fn(actual.execSync), execFileSync: vi.fn(actual.execFileSync) };
|
||||
});
|
||||
import {
|
||||
registerRepo,
|
||||
readRegistry,
|
||||
|
|
@ -233,6 +248,44 @@ describe('checkCwdMatch', () => {
|
|||
}
|
||||
});
|
||||
|
||||
it('returns match=none for a non-git cwd before resolving sibling remotes', async () => {
|
||||
const indexed = await createTempDir('cwd-home-indexed-');
|
||||
const nonGitCwd = await createTempDir('cwd-home-non-git-');
|
||||
try {
|
||||
const indexedHead = initRepoWithCommit(indexed.dbPath, 'https://example.com/foo/bar');
|
||||
await registerRepo(indexed.dbPath, {
|
||||
repoPath: indexed.dbPath,
|
||||
lastCommit: indexedHead,
|
||||
indexedAt: new Date().toISOString(),
|
||||
remoteUrl: 'https://example.com/foo/bar',
|
||||
});
|
||||
|
||||
// Clear after setup so we only count subprocess calls made by
|
||||
// checkCwdMatch itself. The fix in #1138 must guarantee that no
|
||||
// git subprocess is launched when the cwd is outside any .git
|
||||
// ancestor — a return value of 'none' alone does not prove that
|
||||
// (the pre-fix code also returned 'none', just by failing the
|
||||
// spawn). The mocked module exports are vi.fn() wrappers that
|
||||
// pass through to real implementations; clearing tracks only the
|
||||
// calls made by checkCwdMatch below.
|
||||
vi.mocked(execSync).mockClear();
|
||||
vi.mocked(execFileSync).mockClear();
|
||||
const nodeCp = await import('node:child_process');
|
||||
vi.mocked(nodeCp.execSync).mockClear();
|
||||
vi.mocked(nodeCp.execFileSync).mockClear();
|
||||
|
||||
const m = await checkCwdMatch(nonGitCwd.dbPath);
|
||||
expect(m.match).toBe('none');
|
||||
expect(execSync).not.toHaveBeenCalled();
|
||||
expect(execFileSync).not.toHaveBeenCalled();
|
||||
expect(nodeCp.execSync).not.toHaveBeenCalled();
|
||||
expect(nodeCp.execFileSync).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
await indexed.cleanup();
|
||||
await nonGitCwd.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
it('reports sibling-by-remote with a stale hint when cwd HEAD has advanced', async () => {
|
||||
// Polecat-style scenario from the issue: index at path A, query
|
||||
// from cwd=path B (same repo), get a warning rather than
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue