diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 922a69f85..03f59cd40 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -22,7 +22,13 @@ export { isWriteQuery }; // at MCP server startup — crashes on unsupported Node ABI versions (#89) // git utilities available if needed // import { isGitRepo, getCurrentCommit, getGitRoot } from '../../storage/git.js'; -import { parseDiffHunks, type FileDiff } from '../../storage/git.js'; +import { + parseDiffHunks, + getCanonicalRepoRoot, + getGitRoot, + type FileDiff, +} from '../../storage/git.js'; +import { realpathSync } from 'fs'; import { listRegisteredRepos, cleanupOldKuzuFiles, @@ -211,6 +217,55 @@ interface RepoHandle { stats?: RegistryEntry['stats']; } +/** Resolve symlinks for path comparison; falls back to path.resolve on error. + * Uses `realpathSync.native` (not the pure-JS `realpathSync`) so that Windows + * 8.3 short names (e.g. RUNNER~1 → runneradmin) are expanded to long form, + * matching the output of `git rev-parse --show-toplevel`. */ +function tryRealpath(p: string): string { + try { + return realpathSync.native(p); + } catch { + return path.resolve(p); + } +} + +/** + * Resolve the git diff cwd for detect_changes, auto-detecting linked worktrees. + * + * When `launchCwd` is a linked worktree of the same canonical repository as + * `repoPath` (i.e. `getGitRoot(launchCwd)` differs from `repoPath` but both + * share the same `getCanonicalRepoRoot`), returns the worktree's git root so + * that `git diff` sees the correct working directory and index. + * + * Returns `repoPath` unchanged in all other cases (non-worktree, git + * unavailable, unrelated repo). + * + * Extracted as a module-level export so tests can pass any `launchCwd` instead + * of relying on `process.cwd()`, which is fixed to the server launch directory + * and cannot be changed mid-process. + */ +export function resolveWorktreeCwd(repoPath: string, launchCwd: string): string { + try { + const launchGitRoot = getGitRoot(launchCwd); + if (launchGitRoot) { + // Normalise via realpathSync before comparing so macOS /var → /private/var + // symlinks (and Windows 8.3 short names) don't create false mismatches. + const realLaunch = tryRealpath(launchGitRoot); + const realRepo = tryRealpath(repoPath); + if (realLaunch !== realRepo) { + const launchCanonical = getCanonicalRepoRoot(launchCwd); + const repoCanonical = getCanonicalRepoRoot(repoPath); + if (launchCanonical && repoCanonical && launchCanonical === repoCanonical) { + return launchGitRoot; + } + } + } + } catch { + // Best-effort; fall through to repoPath. + } + return repoPath; +} + export class LocalBackend { private repos: Map = new Map(); private contextCache: Map = new Map(); @@ -2133,6 +2188,7 @@ export class LocalBackend { params: { scope?: string; base_ref?: string; + worktree?: string; }, ): Promise { await this.ensureInitialized(repo.id); @@ -2161,11 +2217,51 @@ export class LocalBackend { let diffOutput: string; try { + // Resolve the cwd for git diff. + // + // In a linked worktree (e.g. /repo/wt-feature/), the user's staged and + // unstaged changes live in that worktree's separate working directory and + // index. Running `git diff` from the canonical repo root sees a different + // working tree and returns empty output. + // + // Resolution order (see resolveWorktreeCwd for details): + // 1. params.worktree — explicit override, validated against the + // registered repo's canonical root. + // 2. Auto-detect — if the server's launch cwd (process.cwd()) is a + // linked worktree of the same canonical repo, use its git root. + // 3. repo.repoPath — fallback (original behaviour, handled inside + // resolveWorktreeCwd when no worktree is detected). + // + // Start with the auto-detected value; override with the validated + // explicit param when provided. This avoids a dead initial assignment. + let diffCwd = resolveWorktreeCwd(repo.repoPath, process.cwd()); + if (params.worktree) { + if (!path.isAbsolute(params.worktree)) { + return { + error: `worktree must be an absolute path, got: "${params.worktree}"`, + }; + } + const providedResolved = path.resolve(params.worktree); + const repoCanonical = getCanonicalRepoRoot(repo.repoPath); + if (!repoCanonical) { + return { + error: `Could not determine canonical root for repo "${repo.repoPath}". Is git available?`, + }; + } + const worktreeCanonical = getCanonicalRepoRoot(providedResolved); + if (!worktreeCanonical || tryRealpath(worktreeCanonical) !== tryRealpath(repoCanonical)) { + return { + error: `worktree "${params.worktree}" is not a worktree of repo "${repo.repoPath}". Ensure the path is inside the same git repository.`, + }; + } + diffCwd = providedResolved; + } + // maxBuffer raised from Node's 1MB default to 256MB to avoid ENOBUFS on // repos with large unstaged/untracked diffs (e.g. unignored build folders). // See issue: spawnSync git ENOBUFS in detect_changes(scope="unstaged"). diffOutput = execFileSync('git', diffArgs, { - cwd: repo.repoPath, + cwd: diffCwd, encoding: 'utf-8', maxBuffer: 256 * 1024 * 1024, }); diff --git a/gitnexus/src/mcp/tools.ts b/gitnexus/src/mcp/tools.ts index a85298c04..28646c66f 100644 --- a/gitnexus/src/mcp/tools.ts +++ b/gitnexus/src/mcp/tools.ts @@ -253,6 +253,8 @@ Maps git diff hunks to indexed symbols, then traces which processes are impacted WHEN TO USE: Before committing — to understand what your changes affect. Pre-commit review, PR preparation. AFTER THIS: Review affected processes. Use context() on high-risk symbols. READ gitnexus://repo/{name}/process/{name} for full traces. +GIT WORKTREE SUPPORT: GitNexus automatically detects when the MCP server was launched from inside a linked git worktree and runs git diff against that worktree — no extra parameters needed in the common case. Pass "worktree" explicitly only when the server was started from a different directory than the worktree you are editing (e.g., the server runs from the canonical root but your changes are in a linked worktree at a different path). + Returns: changed symbols, affected processes, and a risk summary.`, annotations: READ_ONLY_TOOL_ANNOTATIONS, inputSchema: { @@ -268,6 +270,11 @@ Returns: changed symbols, affected processes, and a risk summary.`, type: 'string', description: 'Branch/commit for "compare" scope (e.g., "main")', }, + worktree: { + type: 'string', + description: + 'Absolute path to a linked git worktree. Pass this when your changes are in a worktree (the .git entry at that path is a file, not a directory). GitNexus will run git diff from that worktree so staged/unstaged changes are correctly detected.', + }, repo: { type: 'string', description: 'Repository name or path. Omit if only one repo is indexed.', diff --git a/gitnexus/test/unit/detect-changes-worktree.test.ts b/gitnexus/test/unit/detect-changes-worktree.test.ts new file mode 100644 index 000000000..02e440100 --- /dev/null +++ b/gitnexus/test/unit/detect-changes-worktree.test.ts @@ -0,0 +1,369 @@ +/** + * Tests for detect_changes worktree support. + * + * When a caller is editing inside a linked git worktree the canonical + * repo.repoPath (main checkout root) is a different working directory. + * Running `git diff` from the canonical root returns empty output while + * the actual changes live in the linked worktree. + * + * The `worktree` param pins the cwd for git diff to the linked worktree + * after verifying it belongs to the same canonical repository. + */ +import { describe, expect, it } from 'vitest'; +import { readFileSync, mkdtempSync, rmSync, writeFileSync, realpathSync } from 'fs'; +import { execSync, execFileSync } from 'child_process'; +import path from 'path'; +import os from 'os'; +import { fileURLToPath } from 'url'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const backendSrc = readFileSync( + path.join(__dirname, '../../src/mcp/local/local-backend.ts'), + 'utf-8', +); +const toolsSrc = readFileSync(path.join(__dirname, '../../src/mcp/tools.ts'), 'utf-8'); + +// ── Structural tests (source-grep) ─────────────────────────────────────────── +// +// NOTE: These grep the source as plain text and verify that key patterns are +// present. They are a useful backstop to catch accidental regressions (e.g. +// someone moves the import back to a dynamic one, or removes the error +// messages). They do NOT prove the guards work correctly at runtime — that is +// what the E2E real-worktree tests below are for. + +describe('detect_changes worktree support — structural', () => { + it('getCanonicalRepoRoot is statically imported from storage/git (not dynamic)', () => { + // Must be a top-level static import, not a dynamic await import inside the function. + expect(backendSrc).toMatch( + /^import\s*\{[^}]*getCanonicalRepoRoot[^}]*\}\s*from\s*['"].*storage\/git/m, + ); + // Confirm the dynamic import is gone. + expect(backendSrc).not.toMatch(/await import\(.*storage\/git/); + }); + + it('detect_changes tool schema declares a "worktree" property', () => { + expect(toolsSrc).toMatch(/worktree/); + }); + + it('detectChanges() signature includes worktree in its params type', () => { + expect(backendSrc).toMatch(/worktree\?:\s*string/); + }); + + it('uses diffCwd as the cwd for execFileSync (not hard-coded repo.repoPath)', () => { + expect(backendSrc).toMatch(/cwd:\s*diffCwd/); + }); + + it('defaults diffCwd via resolveWorktreeCwd (falls back to repo.repoPath internally)', () => { + // diffCwd is now initialised directly from resolveWorktreeCwd, which + // returns repo.repoPath when no linked worktree is detected. The old + // dead `let diffCwd = repo.repoPath` was removed to fix CodeQL + // "useless assignment to local variable". + expect(backendSrc).toMatch(/let diffCwd\s*=\s*resolveWorktreeCwd\(/); + }); + + it('rejects relative paths with an absolute-path error', () => { + expect(backendSrc).toMatch(/worktree must be an absolute path/); + }); + + it('returns a distinct error when git is unavailable (null repoCanonical)', () => { + expect(backendSrc).toMatch(/Could not determine canonical root for repo/); + }); + + it('returns a mismatch error when the worktree belongs to a different repo', () => { + expect(backendSrc).toMatch(/is not a worktree of repo/); + }); + + it('explicit params.worktree is wired through to execFileSync cwd', () => { + // A full callTool() integration test requires a live LadybugDB; instead + // we verify the wiring via two complementary structural assertions that + // would both need to be wrong simultaneously to hide a real bug: + // 1. The validated explicit path is stored in diffCwd. + // 2. diffCwd is the value passed to execFileSync as cwd. + // If either assignment were swapped back to repo.repoPath the tests in + // this file would immediately fail. + expect(backendSrc).toMatch(/diffCwd\s*=\s*providedResolved/); + // Also verify canonical roots are compared via tryRealpath (Finding 3). + expect(backendSrc).toMatch( + /tryRealpath\(worktreeCanonical\)\s*!==\s*tryRealpath\(repoCanonical\)/, + ); + }); + + it('auto-detects linked worktree via process.cwd() when worktree param is omitted', () => { + // The else branch must delegate to the exported resolveWorktreeCwd helper. + expect(backendSrc).toMatch(/resolveWorktreeCwd/); + // The helper must be exported so tests can call it directly. + expect(backendSrc).toMatch(/export function resolveWorktreeCwd/); + // detectChanges passes process.cwd() to the helper. + expect(backendSrc).toMatch(/resolveWorktreeCwd\(repo\.repoPath,\s*process\.cwd\(\)\)/); + }); + + it('git worktree support is documented in the tool description', () => { + expect(toolsSrc).toMatch(/GIT WORKTREE SUPPORT/); + // Auto-detection is the primary path now. + expect(toolsSrc).toMatch(/automatically detects/); + }); +}); + +// ── resolveWorktreeCwd — auto-detection helper (behavioural) ───────────────── +// +// resolveWorktreeCwd is extracted from detectChanges specifically so tests can +// pass any launchCwd instead of being stuck with the fixed process.cwd(). + +import { resolveWorktreeCwd } from '../../src/mcp/local/local-backend.js'; +import { getCanonicalRepoRoot } from '../../src/storage/git.js'; + +describe('resolveWorktreeCwd — auto-detection helper', () => { + it('returns repoPath unchanged when launchCwd is the same git root', () => { + const repoDir = mkdtempSync(path.join(os.tmpdir(), 'gitnexus-rwc-same-')); + try { + execSync('git init -q', { cwd: repoDir, stdio: 'ignore' }); + // Compare via realpathSync.native: mkdtempSync may return a symlink path + // on macOS (/var vs /private/var) or a Windows 8.3 short name + // (RUNNER~1 vs runneradmin) while getGitRoot returns the expanded form. + const result = resolveWorktreeCwd(repoDir, repoDir); + expect(realpathSync.native(result)).toBe(realpathSync.native(repoDir)); + } finally { + rmSync(repoDir, { recursive: true, force: true }); + } + }); + + it('returns repoPath unchanged when launchCwd is a non-git directory', () => { + const repoDir = mkdtempSync(path.join(os.tmpdir(), 'gitnexus-rwc-repo-')); + const plainDir = mkdtempSync(path.join(os.tmpdir(), 'gitnexus-rwc-plain-')); + try { + execSync('git init -q', { cwd: repoDir, stdio: 'ignore' }); + // plainDir has no git repo — no git root found → fall through to repoPath + const result = resolveWorktreeCwd(repoDir, plainDir); + expect(result).toBe(repoDir); + } finally { + rmSync(repoDir, { recursive: true, force: true }); + rmSync(plainDir, { recursive: true, force: true }); + } + }); + + it('returns worktreeDir when launchCwd is a linked worktree of the same repo', () => { + const repoDir = mkdtempSync(path.join(os.tmpdir(), 'gitnexus-rwc-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-auto'); + execSync(`git worktree add -q -b auto "${worktreeDir}"`, { + cwd: repoDir, + stdio: 'ignore', + }); + + // Key assertion: passing the worktree as launchCwd returns it, + // proving the auto-detect logic in detectChanges works correctly. + // Use realpathSync.native: mkdtempSync may return a symlink or 8.3 + // short-name path while getGitRoot returns the expanded canonical form. + const result = resolveWorktreeCwd(repoDir, worktreeDir); + expect(realpathSync.native(result)).toBe(realpathSync.native(worktreeDir)); + // Confirm it's NOT the canonical root (auto-detection fired). + expect(realpathSync.native(result)).not.toBe(realpathSync.native(repoDir)); + } finally { + try { + execSync('git worktree remove -f wt-auto', { cwd: repoDir, stdio: 'ignore' }); + } catch { + // ignore + } + rmSync(repoDir, { recursive: true, force: true }); + } + }); + + it('returns repoPath when launchCwd belongs to a different (unrelated) repo', () => { + const repoA = mkdtempSync(path.join(os.tmpdir(), 'gitnexus-rwc-a-')); + const repoB = mkdtempSync(path.join(os.tmpdir(), 'gitnexus-rwc-b-')); + try { + execSync('git init -q', { cwd: repoA, stdio: 'ignore' }); + execSync('git init -q', { cwd: repoB, stdio: 'ignore' }); + // repoB has a different canonical root — guard must reject it. + const result = resolveWorktreeCwd(repoA, repoB); + expect(result).toBe(repoA); + } finally { + rmSync(repoA, { recursive: true, force: true }); + rmSync(repoB, { recursive: true, force: true }); + } + }); +}); + +// ── Guard logic via real path arithmetic ───────────────────────────────────── + +describe('detect_changes worktree support — guard logic', () => { + it('getCanonicalRepoRoot returns the same root for the main checkout and a sub-path', () => { + const fromRoot = getCanonicalRepoRoot(path.join(__dirname, '../..')); + const fromSub = getCanonicalRepoRoot(path.join(__dirname, '../../src')); + if (fromRoot === null) { + expect(fromSub).toBeNull(); + } else { + expect(fromSub).toBe(fromRoot); + } + }); + + it('getCanonicalRepoRoot returns null for a non-git directory', () => { + const tmpDir = mkdtempSync(path.join(os.tmpdir(), 'gitnexus-nonrepo-')); + try { + expect(getCanonicalRepoRoot(tmpDir)).toBeNull(); + } finally { + rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it('getCanonicalRepoRoot equates a worktree path with the canonical root', () => { + // This directly exercises the comparison the guard performs: + // both paths must yield the same canonical root for the guard to pass. + const repoDir = mkdtempSync(path.join(os.tmpdir(), 'gitnexus-guard-')); + 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, 'a.ts'), 'export const a = 1;\n'); + execSync('git add a.ts', { cwd: repoDir, stdio: 'ignore' }); + execSync('git commit -q -m "initial"', { cwd: repoDir, stdio: 'ignore' }); + + const worktreeDir = path.join(repoDir, 'wt-guard'); + execSync(`git worktree add -q -b guard "${worktreeDir}"`, { + cwd: repoDir, + stdio: 'ignore', + }); + + const fromRepo = getCanonicalRepoRoot(repoDir); + const fromWorktree = getCanonicalRepoRoot(worktreeDir); + + // Both must be non-null and equal — the guard's passing condition. + expect(fromRepo).not.toBeNull(); + expect(fromWorktree).toBe(fromRepo); + } finally { + try { + execSync('git worktree remove -f wt-guard', { cwd: repoDir, stdio: 'ignore' }); + } catch { + // ignore cleanup failure + } + rmSync(repoDir, { recursive: true, force: true }); + } + }); + + it('getCanonicalRepoRoot returns different roots for two unrelated repos', () => { + // The guard's rejection condition: roots must NOT match for unrelated repos. + const repoA = mkdtempSync(path.join(os.tmpdir(), 'gitnexus-repoA-')); + const repoB = mkdtempSync(path.join(os.tmpdir(), 'gitnexus-repoB-')); + try { + execSync('git init -q', { cwd: repoA, stdio: 'ignore' }); + execSync('git init -q', { cwd: repoB, stdio: 'ignore' }); + const rootA = getCanonicalRepoRoot(repoA); + const rootB = getCanonicalRepoRoot(repoB); + expect(rootA).not.toBeNull(); + expect(rootB).not.toBeNull(); + expect(rootA).not.toBe(rootB); + } finally { + rmSync(repoA, { recursive: true, force: true }); + rmSync(repoB, { recursive: true, force: true }); + } + }); +}); + +// ── End-to-end: real git worktree + real git diff ──────────────────────────── +// +// These tests prove the core bug scenario without going through LocalBackend: +// - git diff from the canonical root misses changes in a linked worktree +// - git diff with cwd set to the worktree correctly finds them +// - getCanonicalRepoRoot equates canonical root and worktree (guard passes) + +describe('detect_changes worktree support — end-to-end with real worktree', () => { + it('git diff from canonical root misses unstaged changes in a linked worktree, but worktree cwd finds them', () => { + const repoDir = mkdtempSync(path.join(os.tmpdir(), 'gitnexus-wt-detect-')); + 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, 'main.ts'), 'export const x = 1;\n'); + execSync('git add main.ts', { cwd: repoDir, stdio: 'ignore' }); + execSync('git commit -q -m "initial"', { cwd: repoDir, stdio: 'ignore' }); + + const worktreeDir = path.join(repoDir, 'wt-feature'); + execSync(`git worktree add -q -b feature "${worktreeDir}"`, { + cwd: repoDir, + stdio: 'ignore', + }); + + // Make an unstaged change inside the linked worktree only. + writeFileSync(path.join(worktreeDir, 'main.ts'), 'export const x = 2;\n'); + + // Bug: git diff from canonical root → empty (misses worktree changes). + const diffFromCanonical = execFileSync('git', ['diff', '-U0'], { + cwd: repoDir, + encoding: 'utf-8', + }); + expect(diffFromCanonical.trim()).toBe(''); + + // Fix: git diff with cwd = worktree → finds the change. + const diffFromWorktree = execFileSync('git', ['diff', '-U0'], { + cwd: worktreeDir, + encoding: 'utf-8', + }); + expect(diffFromWorktree).toContain('main.ts'); + expect(diffFromWorktree).toContain('+export const x = 2;'); + + // Guard: getCanonicalRepoRoot equates both paths → guard approves this worktree. + const canonicalFromRepo = getCanonicalRepoRoot(repoDir); + const canonicalFromWorktree = getCanonicalRepoRoot(worktreeDir); + expect(canonicalFromRepo).not.toBeNull(); + expect(canonicalFromWorktree).toBe(canonicalFromRepo); + } finally { + try { + execSync('git worktree remove -f wt-feature', { cwd: repoDir, stdio: 'ignore' }); + } catch { + // ignore on cleanup failure + } + rmSync(repoDir, { recursive: true, force: true }); + } + }); + + it('git diff --staged from worktree cwd sees staged changes in that worktree', () => { + const repoDir = mkdtempSync(path.join(os.tmpdir(), 'gitnexus-wt-staged-')); + 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, 'foo.ts'), 'export const a = 1;\n'); + execSync('git add foo.ts', { cwd: repoDir, stdio: 'ignore' }); + execSync('git commit -q -m "initial"', { cwd: repoDir, stdio: 'ignore' }); + + const worktreeDir = path.join(repoDir, 'wt-staged'); + execSync(`git worktree add -q -b staged-branch "${worktreeDir}"`, { + cwd: repoDir, + stdio: 'ignore', + }); + + // Stage a change inside the linked worktree. + writeFileSync(path.join(worktreeDir, 'foo.ts'), 'export const a = 99;\n'); + execSync('git add foo.ts', { cwd: worktreeDir, stdio: 'ignore' }); + + // Staged diff from canonical root → empty. + const stagedFromCanonical = execFileSync('git', ['diff', '--staged', '-U0'], { + cwd: repoDir, + encoding: 'utf-8', + }); + expect(stagedFromCanonical.trim()).toBe(''); + + // Staged diff from worktree cwd → has output. + const stagedFromWorktree = execFileSync('git', ['diff', '--staged', '-U0'], { + cwd: worktreeDir, + encoding: 'utf-8', + }); + expect(stagedFromWorktree).toContain('foo.ts'); + expect(stagedFromWorktree).toContain('+export const a = 99;'); + } finally { + try { + execSync('git worktree remove -f wt-staged', { cwd: repoDir, stdio: 'ignore' }); + } catch { + // ignore + } + rmSync(repoDir, { recursive: true, force: true }); + } + }); +});