diff --git a/gitnexus/src/cli/index-repo.ts b/gitnexus/src/cli/index-repo.ts index 62138b4be..b909a40b5 100644 --- a/gitnexus/src/cli/index-repo.ts +++ b/gitnexus/src/cli/index-repo.ts @@ -17,7 +17,7 @@ import { addToGitignore, registerRepo, } from '../storage/repo-manager.js'; -import { getGitRoot, isGitRepo } from '../storage/git.js'; +import { getGitRoot, getRemoteUrl, isGitRepo } from '../storage/git.js'; export interface IndexOptions { force?: boolean; @@ -107,6 +107,13 @@ export const indexCommand = async (inputPathParts?: string[], options?: IndexOpt } // ── Register in global registry ─────────────────────────────────── + // Refresh the on-disk meta with a freshly captured `remoteUrl` if + // it's missing, so an `index` of an older `.gitnexus/` still gets + // sibling-clone fingerprinting on subsequent use without forcing a + // full re-analyze. + if (!meta.remoteUrl && isGitRepo(repoPath)) { + meta.remoteUrl = getRemoteUrl(repoPath); + } await registerRepo(repoPath, meta); await addToGitignore(repoPath); diff --git a/gitnexus/src/core/git-staleness.ts b/gitnexus/src/core/git-staleness.ts index 2ef8f9c75..93e556ab5 100644 --- a/gitnexus/src/core/git-staleness.ts +++ b/gitnexus/src/core/git-staleness.ts @@ -4,6 +4,9 @@ */ 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'; export interface StalenessInfo { isStale: boolean; @@ -37,3 +40,111 @@ export function checkStaleness(repoPath: string, lastCommit: string): StalenessI return { isStale: false, commitsBehind: 0 }; } } + +/** + * Compare a sibling-clone HEAD against an indexed `lastCommit`. Returns + * `undefined` when the indexed commit is not reachable from the sibling + * (e.g. divergent branches, shallow clone, missing ref). The caller + * should treat `undefined` as "drift unknown" rather than "no drift". + */ +function commitsAheadOfIndexed(siblingPath: string, indexedCommit: string): number | undefined { + if (!indexedCommit) return undefined; + try { + const result = execFileSync('git', ['rev-list', '--count', `${indexedCommit}..HEAD`], { + cwd: siblingPath, + encoding: 'utf-8', + stdio: ['pipe', 'pipe', 'pipe'], + }).trim(); + return parseInt(result, 10) || 0; + } catch { + return undefined; + } +} + +/** + * Resolve a working directory against the global registry. Returns: + * - `match: 'path'` when `cwd` is inside a registered entry's path + * - `match: 'sibling-by-remote'` when `cwd` lives in a different on-disk clone + * of the same repo (same `remoteUrl`) + * - `match: 'none'` when neither match applies + * + * For sibling-by-remote matches, the caller's HEAD and the drift vs the + * indexed `lastCommit` are also returned so the MCP layer can warn + * before serving silently-stale answers (issue: silent graph drift + * across sibling clones). + * + * `path` matches deliberately use the longest-prefix rule so a cwd + * inside a sub-path of a registered repo still matches that repo, not + * a coincidentally-aliased shorter entry. + */ +export async function checkCwdMatch(cwd: string): Promise { + const entries = await readRegistry(); + if (entries.length === 0) return { match: 'none' }; + + const isWin = process.platform === 'win32'; + const norm = (p: string) => (isWin ? path.resolve(p).toLowerCase() : path.resolve(p)); + const sep = path.sep; + const cwdResolved = path.resolve(cwd); + const cwdNorm = norm(cwdResolved); + + // 1) Path-based match (longest prefix wins, boundary-safe). + let bestPath: RegistryEntry | undefined; + let bestLen = -1; + for (const e of entries) { + const p = norm(e.path); + if (cwdNorm === p || cwdNorm.startsWith(p + sep)) { + if (p.length > bestLen) { + bestPath = e; + bestLen = p.length; + } + } + } + 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); + if (!cwdGitRoot) return { match: 'none' }; + + const cwdRemote = getRemoteUrl(cwdGitRoot); + if (!cwdRemote) return { match: 'none' }; + + const sibling = entries.find( + (e) => e.remoteUrl === cwdRemote && norm(e.path) !== norm(cwdGitRoot), + ); + if (!sibling) return { match: 'none' }; + + const cwdHead = getCurrentCommit(cwdGitRoot) || undefined; + const drift = commitsAheadOfIndexed(cwdGitRoot, sibling.lastCommit); + + // Same commit on both clones → still report match=sibling-by-remote + // (the relationship is real and useful to callers like list_repos / + // future tooling) but leave `hint` unset: there's nothing to warn + // about, and `maybeWarnSiblingDrift` already short-circuits this + // case independently. Surfacing a no-op hint would force callers + // to second-guess whether they need to display it. + let hint: string | undefined; + if (cwdHead && cwdHead === sibling.lastCommit) { + hint = undefined; + } else if (drift && drift > 0) { + hint = + `⚠️ Index for "${sibling.name}" was built at ${sibling.path}; ` + + `your cwd (${cwdGitRoot}) is a sibling clone that is ${drift} commit${drift > 1 ? 's' : ''} ` + + `ahead of the indexed commit. Results may be stale or incorrect — re-run \`gitnexus analyze\` ` + + `to refresh the index.`; + } else { + hint = + `⚠️ Index for "${sibling.name}" was built at ${sibling.path}; ` + + `your cwd (${cwdGitRoot}) is a sibling clone whose HEAD differs from the indexed commit. ` + + `Results may be stale or incorrect — re-run \`gitnexus analyze\` to refresh the index.`; + } + + return { + match: 'sibling-by-remote', + entry: sibling, + cwdGitRoot, + cwdHead, + drift, + hint, + }; +} diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index e61c20f21..00e0574ac 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -29,7 +29,7 @@ import { registerRepo, cleanupOldKuzuFiles, } from '../storage/repo-manager.js'; -import { getCurrentCommit, hasGitDir, getInferredRepoName } from '../storage/git.js'; +import { getCurrentCommit, getRemoteUrl, hasGitDir, getInferredRepoName } from '../storage/git.js'; import type { CachedEmbedding } from './embeddings/types.js'; import { generateAIContextFiles } from '../cli/ai-context.js'; import { EMBEDDING_TABLE_NAME } from './lbug/schema.js'; @@ -318,6 +318,13 @@ export async function runFullAnalysis( repoPath, lastCommit: currentCommit, indexedAt: new Date().toISOString(), + // Captured here (not at registration) so it travels with the + // on-disk meta.json — sibling-clone fingerprinting works for + // out-of-tree consumers (group-status, future tooling) without + // a second git shellout. `undefined` when the repo has no + // origin remote, which is fine: paths-only repos behave as + // before. + remoteUrl: hasGitDir(repoPath) ? getRemoteUrl(repoPath) : undefined, stats: { files: pipelineResult.totalFileCount, nodes: stats.nodes, diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 3414f5f5b..4a1e3c41c 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -32,6 +32,7 @@ import { resolveAtGroupMemberRepoPath } from '../../core/group/resolve-at-member import { collectBestChunks } from '../../core/embeddings/types.js'; import { EMBEDDING_TABLE_NAME, EMBEDDING_INDEX_NAME } from '../../core/lbug/schema.js'; import { PhaseTimer } from '../../core/search/phase-timer.js'; +import { checkStaleness, checkCwdMatch } from '../../core/git-staleness.js'; // AI context generation is CLI-only (gitnexus analyze) // import { generateAIContextFiles } from '../../cli/ai-context.js'; @@ -198,6 +199,7 @@ interface RepoHandle { lbugPath: string; indexedAt: string; lastCommit: string; + remoteUrl?: string; stats?: RegistryEntry['stats']; } @@ -208,6 +210,13 @@ export class LocalBackend { private reinitPromises: Map> = new Map(); private lastStalenessCheck: Map = new Map(); private groupToolSvc: GroupService | null = null; + /** + * One-shot stderr warnings for sibling-clone drift, keyed by + * `${repoId}|${cwdGitRoot}`. Without this guard every tool call + * from inside a sibling clone would print the same warning, + * making MCP stderr unreadable. + */ + private warnedSiblingDrift: Set = new Set(); /** * Cross-repo group tools (CLI). Shares logic with MCP `group_*` handlers. @@ -275,6 +284,7 @@ export class LocalBackend { lbugPath, indexedAt: entry.indexedAt, lastCommit: entry.lastCommit, + remoteUrl: entry.remoteUrl, stats: entry.stats, }; @@ -333,12 +343,26 @@ export class LocalBackend { */ async resolveRepo(repoParam?: string): Promise { const result = this.resolveRepoFromCache(repoParam); - if (result) return result; + if (result) { + // Issue: silent graph drift across sibling clones. + // If the caller's cwd lives in a *different* on-disk clone of + // the same repo (matched by `remoteUrl`), warn once per + // (repo, cwd) pair on stderr. We do not fail or refuse to + // serve — the index is still the best answer we have — but + // the operator/agent has to know the answer may be stale. + this.maybeWarnSiblingDrift(result).catch(() => { + /* best-effort; never throw from resolveRepo */ + }); + return result; + } // Miss — refresh registry and try once more await this.refreshRepos(); const retried = this.resolveRepoFromCache(repoParam); - if (retried) return retried; + if (retried) { + this.maybeWarnSiblingDrift(retried).catch(() => {}); + return retried; + } // Still no match — throw with helpful message if (this.repos.size === 0) { @@ -476,18 +500,128 @@ export class LocalBackend { * List all registered repos with their metadata. * Re-reads the global registry so newly indexed repos are discovered * without restarting the MCP server. + * + * Each entry includes: + * - `staleness`: if the indexed clone's own HEAD has moved past + * the recorded `lastCommit` (option D in the issue's fix list). + * - `siblings`: other registered entries sharing the same + * `remoteUrl` (option B's payoff: callers can see at a glance + * that another clone of the same logical repo is registered). + * - `remoteUrl`: the canonical origin URL recorded at index time. */ async listRepos(): Promise< - Array<{ name: string; path: string; indexedAt: string; lastCommit: string; stats?: any }> + Array<{ + name: string; + path: string; + indexedAt: string; + lastCommit: string; + remoteUrl?: string; + stats?: any; + staleness?: { commitsBehind: number; hint?: string }; + siblings?: Array<{ name: string; path: string; lastCommit: string }>; + }> > { await this.refreshRepos(); - return [...this.repos.values()].map((h) => ({ - name: h.name, - path: h.repoPath, - indexedAt: h.indexedAt, - lastCommit: h.lastCommit, - stats: h.stats, - })); + const handles = [...this.repos.values()]; + + // Pre-group registered handles by `remoteUrl` so the sibling + // lookup is O(1) per handle. We reuse the in-memory `this.repos` + // (already populated by `refreshRepos`) instead of doing a fresh + // `readRegistry()` per entry — that would be N file reads for N + // registered repos. + const isWin = process.platform === 'win32'; + const norm = (p: string) => (isWin ? path.resolve(p).toLowerCase() : path.resolve(p)); + const byRemote = new Map(); + for (const h of handles) { + if (!h.remoteUrl) continue; + const list = byRemote.get(h.remoteUrl) ?? []; + list.push(h); + byRemote.set(h.remoteUrl, list); + } + + return handles.map((h) => { + const stale = checkStaleness(h.repoPath, h.lastCommit); + const selfNorm = norm(h.repoPath); + const siblings = h.remoteUrl + ? (byRemote.get(h.remoteUrl) ?? []).filter((e) => norm(e.repoPath) !== selfNorm) + : []; + return { + name: h.name, + path: h.repoPath, + indexedAt: h.indexedAt, + lastCommit: h.lastCommit, + remoteUrl: h.remoteUrl, + stats: h.stats, + staleness: stale.isStale + ? { commitsBehind: stale.commitsBehind, hint: stale.hint } + : undefined, + siblings: + siblings.length > 0 + ? siblings.map((s) => ({ + name: s.name, + path: s.repoPath, + lastCommit: s.lastCommit, + })) + : undefined, + }; + }); + } + + /** + * Best-effort sibling-clone drift warning. + * + * When the resolved index has a `remoteUrl` recorded and the caller's + * `process.cwd()` is inside a *different* clone of the same repo, emit + * one stderr line per (repo, cwd) pair so the operator knows the + * graph may be stale relative to what's actually on disk under their + * cwd. Silent on path matches and on repos without a remote URL. + * + * Limitation: in MCP stdio server mode `process.cwd()` is the + * server's CWD at start time, *not* the agent client's CWD. The + * warning therefore only fires when the MCP server itself was + * launched from inside a sibling clone (typical for `npx gitnexus + * serve` from a polecat workspace). Surfacing the client's CWD + * would require a per-tool-call `cwd` parameter — out of scope for + * the current MCP contract. + * + * Pure side-effect (stderr); never affects the returned handle. + * After the first computation for a given (repo, cwd) pair the + * result is cached so subsequent `resolveRepo()` calls don't + * re-shell-out to git. + */ + private async maybeWarnSiblingDrift(handle: RepoHandle): Promise { + if (!handle.remoteUrl) return; + let cwd: string; + try { + cwd = process.cwd(); + } catch { + return; + } + // Early-exit cache: keyed on (repo, cwd) BEFORE any git shellout. + // After the first call for a given cwd, this short-circuits the + // up-to-four `execSync`/`execFileSync` calls inside `checkCwdMatch` + // — important for MCP-server mode where `process.cwd()` is constant + // and `resolveRepo` runs on every tool call. + const cacheKey = `${handle.id}|${cwd}`; + if (this.warnedSiblingDrift.has(cacheKey)) return; + + const match = await checkCwdMatch(cwd); + if ( + match.match !== 'sibling-by-remote' || + !match.entry || + !match.cwdGitRoot || + match.entry.path !== handle.repoPath || + !match.hint + ) { + // Cache "nothing to warn about" outcomes too — `checkCwdMatch` + // is deterministic for a fixed (registry, cwd) pair, so re-running + // it yields nothing new. + this.warnedSiblingDrift.add(cacheKey); + return; + } + + this.warnedSiblingDrift.add(cacheKey); + console.error(`GitNexus: ${match.hint}`); } // ─── Tool Dispatch ─────────────────────────────────────────────── diff --git a/gitnexus/src/storage/git.ts b/gitnexus/src/storage/git.ts index c8d05ac4b..8e0d6e555 100644 --- a/gitnexus/src/storage/git.ts +++ b/gitnexus/src/storage/git.ts @@ -21,6 +21,66 @@ export const getCurrentCommit = (repoPath: string): string => { } }; +/** + * Get a stable canonical identifier for the repo's `origin` remote, if any. + * + * Used to fingerprint two on-disk clones as the same logical repository + * (issue #XXX — silent graph drift across sibling clones). `path` alone + * is unreliable: worktrees, "clean clone for indexing" hygiene, and + * multi-agent workspaces routinely have the same repo at multiple + * absolute paths. The remote URL is the only on-disk signal that + * survives those conventions. + * + * Normalisation strategy: + * - Strip a trailing `.git` so `https://x/y` and `https://x/y.git` collapse. + * - Strip a trailing `/` for the same reason. + * - `git@github.com:foo/bar` and `https://github.com/foo/bar` are + * intentionally NOT collapsed — they are different remotes from + * git's perspective and we don't want to assert equivalence. + * - Lower-case the host portion so `GitHub.com` and `github.com` + * don't desync; preserves case in path because some hosts + * (Bitbucket Server) treat repo paths case-sensitively. + * + * Returns `undefined` when there is no origin remote, the directory + * isn't a git repo, or git itself isn't available. + */ +export const getRemoteUrl = (repoPath: string): string | undefined => { + let raw: string; + try { + raw = execSync('git config --get remote.origin.url', { + cwd: repoPath, + stdio: ['ignore', 'pipe', 'ignore'], + }) + .toString() + .trim(); + } catch { + return undefined; + } + if (!raw) return undefined; + + let normalised = raw.replace(/\/$/, '').replace(/\.git$/, ''); + + // Lower-case the host segment of `scheme://[user@]host[:port]/...` + // and the host segment of `git@host:owner/repo` SCP form. + // SSH user-segment regex deliberately accepts the common + // `git@`/`-_@` cases. Less common usernames (e.g. with + // dots) fall through to the URL-form branch — they will simply + // not get host-case normalisation, which is acceptable: the raw + // `git config` output is still a valid fingerprint, just slightly + // less collapsible across host casings. + const sshMatch = normalised.match(/^(git@|[a-zA-Z0-9_-]+@)([^:/]+)(:.+)$/); + if (sshMatch) { + normalised = `${sshMatch[1]}${sshMatch[2].toLowerCase()}${sshMatch[3]}`; + } else { + const urlMatch = normalised.match(/^([a-zA-Z][a-zA-Z0-9+.-]*:\/\/)([^/]+)(\/.*)?$/); + if (urlMatch) { + normalised = `${urlMatch[1]}${urlMatch[2].toLowerCase()}${urlMatch[3] ?? ''}`; + } + } + + return normalised; +}; + /** * Find the git repository root from any path inside the repo */ diff --git a/gitnexus/src/storage/repo-manager.ts b/gitnexus/src/storage/repo-manager.ts index 8155e592f..5c592d570 100644 --- a/gitnexus/src/storage/repo-manager.ts +++ b/gitnexus/src/storage/repo-manager.ts @@ -55,6 +55,14 @@ export interface RepoMeta { repoPath: string; lastCommit: string; indexedAt: string; + /** + * Canonical `origin` remote URL captured at index time. Used to + * fingerprint the same logical repo across multiple on-disk clones + * (worktrees, agent workspaces, "clean clone for indexing"). When + * absent (no remote configured, git unavailable, etc.) the repo is + * treated as path-only and sibling-clone detection is skipped. + */ + remoteUrl?: string; stats?: { files?: number; nodes?: number; @@ -82,6 +90,8 @@ export interface RegistryEntry { storagePath: string; indexedAt: string; lastCommit: string; + /** See {@link RepoMeta.remoteUrl}. Mirrored from meta at register time. */ + remoteUrl?: string; stats?: RepoMeta['stats']; } @@ -469,6 +479,7 @@ export const registerRepo = async ( storagePath, indexedAt: meta.indexedAt, lastCommit: meta.lastCommit, + remoteUrl: meta.remoteUrl, stats: meta.stats, }; @@ -764,3 +775,69 @@ export const saveCLIConfig = async (config: CLIConfig): Promise => { } } }; + +// ─── Sibling-clone detection ───────────────────────────────────────────── +// +// A "sibling clone" is a different on-disk path that points at the same +// logical repository (same `origin` remote URL) as a registered index. +// This shows up in three operationally important shapes (see issue): +// +// 1. The same repo is checked out under multiple paths (worktrees, +// multi-agent workspaces). Only one is indexed; the others silently +// diverge from the graph. +// 2. The indexed clone is itself behind its own HEAD (the existing +// `checkStaleness` already handles this case). +// 3. A query is issued from a `cwd` that lives inside a sibling clone +// whose HEAD has drifted from the indexed `lastCommit`. +// +// Detection is intentionally remote-URL-based and does NOT walk the +// filesystem hunting for unregistered clones — only registered entries +// are considered. The `cwd`-driven branch ({@link checkSiblingDrift}) +// also accepts an unregistered cwd, because the live caller's working +// directory is the one place we can cheaply learn about an +// unregistered clone. + +/** + * Find other registered entries whose `remoteUrl` matches the given + * one, excluding `selfPath` (case-insensitive on Windows). Entries + * without a `remoteUrl` are ignored — we cannot prove sibling-ness + * without a fingerprint. + */ +export const findSiblingClones = async ( + remoteUrl: string | undefined, + selfPath: string, +): Promise => { + if (!remoteUrl) return []; + const entries = await readRegistry(); + const isWin = process.platform === 'win32'; + const norm = (p: string) => (isWin ? path.resolve(p).toLowerCase() : path.resolve(p)); + const self = norm(selfPath); + return entries.filter((e) => e.remoteUrl === remoteUrl && norm(e.path) !== self); +}; + +/** + * Description of how a working directory relates to a registered index. + * + * `match` semantics: + * - `path` — `cwd` is inside the registered entry's path. + * - `sibling-by-remote` — `cwd` is in a different on-disk clone of the + * same repo (same `remoteUrl`). + * - `none` — no relationship found. + */ +export interface CwdMatch { + match: 'path' | 'sibling-by-remote' | 'none'; + entry?: RegistryEntry; + /** The git toplevel of `cwd`, when `cwd` is inside a git work tree. */ + cwdGitRoot?: string; + /** HEAD of the cwd's clone, when resolvable. */ + cwdHead?: string; + /** + * Number of commits the registered `lastCommit` is behind the + * sibling-clone HEAD, when both refs are known to the cwd's clone. + * `undefined` when the comparison cannot be performed (e.g. the + * indexed commit isn't reachable from cwd). + */ + drift?: number; + /** Human-readable hint, set whenever the situation warrants warning. */ + hint?: string; +} diff --git a/gitnexus/test/integration/api-impact-e2e.test.ts b/gitnexus/test/integration/api-impact-e2e.test.ts index 04fcadd50..dea7bd9be 100644 --- a/gitnexus/test/integration/api-impact-e2e.test.ts +++ b/gitnexus/test/integration/api-impact-e2e.test.ts @@ -17,6 +17,7 @@ import { API_IMPACT_SEED_DATA, API_IMPACT_FTS_INDEXES } from '../fixtures/api-im vi.mock('../../src/storage/repo-manager.js', () => ({ listRegisteredRepos: vi.fn().mockResolvedValue([]), cleanupOldKuzuFiles: vi.fn().mockResolvedValue({ found: false, needsReindex: false }), + findSiblingClones: vi.fn().mockResolvedValue([]), })); withTestLbugDB( diff --git a/gitnexus/test/integration/class-impact-all-languages.test.ts b/gitnexus/test/integration/class-impact-all-languages.test.ts index 638eb0d6f..422dbb248 100644 --- a/gitnexus/test/integration/class-impact-all-languages.test.ts +++ b/gitnexus/test/integration/class-impact-all-languages.test.ts @@ -21,6 +21,7 @@ import { withTestLbugDB } from '../helpers/test-indexed-db.js'; vi.mock('../../src/storage/repo-manager.js', () => ({ listRegisteredRepos: vi.fn().mockResolvedValue([]), cleanupOldKuzuFiles: vi.fn().mockResolvedValue({ found: false, needsReindex: false }), + findSiblingClones: vi.fn().mockResolvedValue([]), })); // ─── Seed builders ─────────────────────────────────────────────────────────── diff --git a/gitnexus/test/integration/java-class-impact.test.ts b/gitnexus/test/integration/java-class-impact.test.ts index 3055695f6..031d32cd3 100644 --- a/gitnexus/test/integration/java-class-impact.test.ts +++ b/gitnexus/test/integration/java-class-impact.test.ts @@ -17,6 +17,7 @@ import { withTestLbugDB } from '../helpers/test-indexed-db.js'; vi.mock('../../src/storage/repo-manager.js', () => ({ listRegisteredRepos: vi.fn().mockResolvedValue([]), cleanupOldKuzuFiles: vi.fn().mockResolvedValue({ found: false, needsReindex: false }), + findSiblingClones: vi.fn().mockResolvedValue([]), })); // Mirrors the exact graph structure from issue #480: diff --git a/gitnexus/test/integration/local-backend-calltool.test.ts b/gitnexus/test/integration/local-backend-calltool.test.ts index b32aad270..27e6550cc 100644 --- a/gitnexus/test/integration/local-backend-calltool.test.ts +++ b/gitnexus/test/integration/local-backend-calltool.test.ts @@ -17,6 +17,7 @@ import { vi.mock('../../src/storage/repo-manager.js', () => ({ listRegisteredRepos: vi.fn().mockResolvedValue([]), cleanupOldKuzuFiles: vi.fn().mockResolvedValue({ found: false, needsReindex: false }), + findSiblingClones: vi.fn().mockResolvedValue([]), })); // ─── Block 2: callTool dispatch tests ──────────────────────────────── diff --git a/gitnexus/test/integration/shape-check-regression.test.ts b/gitnexus/test/integration/shape-check-regression.test.ts index e786498e2..ba53334d7 100644 --- a/gitnexus/test/integration/shape-check-regression.test.ts +++ b/gitnexus/test/integration/shape-check-regression.test.ts @@ -16,6 +16,7 @@ import { withTestLbugDB } from '../helpers/test-indexed-db.js'; vi.mock('../../src/storage/repo-manager.js', () => ({ listRegisteredRepos: vi.fn().mockResolvedValue([]), cleanupOldKuzuFiles: vi.fn().mockResolvedValue({ found: false, needsReindex: false }), + findSiblingClones: vi.fn().mockResolvedValue([]), })); // ─── Seed data ──────────────────────────────────────────────────────────────── diff --git a/gitnexus/test/integration/staleness-and-stability.test.ts b/gitnexus/test/integration/staleness-and-stability.test.ts index b51594a03..308753781 100644 --- a/gitnexus/test/integration/staleness-and-stability.test.ts +++ b/gitnexus/test/integration/staleness-and-stability.test.ts @@ -30,6 +30,7 @@ import { vi } from 'vitest'; vi.mock('../../src/storage/repo-manager.js', () => ({ listRegisteredRepos: vi.fn().mockResolvedValue([]), cleanupOldKuzuFiles: vi.fn().mockResolvedValue({ found: false, needsReindex: false }), + findSiblingClones: vi.fn().mockResolvedValue([]), })); withTestLbugDB( diff --git a/gitnexus/test/unit/calltool-dispatch.test.ts b/gitnexus/test/unit/calltool-dispatch.test.ts index 9a90b7030..d57bd9051 100644 --- a/gitnexus/test/unit/calltool-dispatch.test.ts +++ b/gitnexus/test/unit/calltool-dispatch.test.ts @@ -37,6 +37,15 @@ vi.mock('../../src/mcp/core/lbug-adapter.js', async (importOriginal) => { vi.mock('../../src/storage/repo-manager.js', () => ({ listRegisteredRepos: vi.fn().mockResolvedValue([]), cleanupOldKuzuFiles: vi.fn().mockResolvedValue({ found: false, needsReindex: false }), + findSiblingClones: vi.fn().mockResolvedValue([]), +})); + +// `core/git-staleness` is also imported by `local-backend.ts` (for +// `checkStaleness` and `checkCwdMatch`). Stub it out here so unit +// tests don't shell out to git. +vi.mock('../../src/core/git-staleness.js', () => ({ + checkStaleness: vi.fn().mockReturnValue({ isStale: false, commitsBehind: 0 }), + checkCwdMatch: vi.fn().mockResolvedValue({ match: 'none' }), })); // Also mock the search modules to avoid loading onnxruntime @@ -748,6 +757,45 @@ describe('LocalBackend.resolveRepo', () => { // listRegisteredRepos should have been called again expect(listRegisteredRepos).toHaveBeenCalledTimes(2); // once in init, once in refreshRepos }); + + it('emits sibling-clone drift warning exactly once per (repo, cwd) pair', async () => { + // Regression guard for the one-shot stderr warning emitted when + // the caller's cwd is in a sibling clone of the resolved index. + // The cache must short-circuit BOTH `console.error` and the + // underlying `checkCwdMatch` git shellouts on subsequent calls. + const { checkCwdMatch } = await import('../../src/core/git-staleness.js'); + (listRegisteredRepos as any).mockResolvedValue([ + { ...MOCK_REPO_ENTRY, remoteUrl: 'https://example.com/foo/bar' }, + ]); + (checkCwdMatch as any).mockResolvedValue({ + match: 'sibling-by-remote', + entry: { ...MOCK_REPO_ENTRY, remoteUrl: 'https://example.com/foo/bar' }, + cwdGitRoot: '/tmp/sibling-clone', + cwdHead: 'feedface', + hint: '⚠️ stale sibling clone', + }); + + const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + await backend.init(); + + // Three resolveRepo invocations from the same cwd: + await backend.callTool('list_repos', {}); // resolveRepo not called for list_repos + // Use a real resolveRepo path: + await backend.resolveRepo(); + await backend.resolveRepo(); + await backend.resolveRepo(); + + const drift = errSpy.mock.calls.filter((c) => String(c[0]).includes('stale sibling clone')); + expect(drift).toHaveLength(1); + // checkCwdMatch should also only run once — the cache check + // happens BEFORE the shellout-heavy match call. + expect(checkCwdMatch).toHaveBeenCalledTimes(1); + } finally { + errSpy.mockRestore(); + (checkCwdMatch as any).mockResolvedValue({ match: 'none' }); + } + }); }); // ─── getContext ────────────────────────────────────────────────────── diff --git a/gitnexus/test/unit/git-utils.test.ts b/gitnexus/test/unit/git-utils.test.ts index 1864ff4c1..d1fc187c4 100644 --- a/gitnexus/test/unit/git-utils.test.ts +++ b/gitnexus/test/unit/git-utils.test.ts @@ -8,6 +8,7 @@ import { describe, it, expect } from 'vitest'; import path from 'path'; import os from 'os'; import fs from 'fs'; +import { execSync } from 'child_process'; // ─── hasGitDir ──────────────────────────────────────────────────────────── // @@ -111,3 +112,71 @@ describe('getGitRoot', () => { } }); }); + +// ─── getRemoteUrl ───────────────────────────────────────────────────────── + +describe('getRemoteUrl', () => { + const setupRepoWithRemote = (remoteUrl: string): string => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-remote-')); + // Use real fs paths and shellouts — the helper itself shells out to + // `git config`, so we need a real git repo for the assertion to be + // meaningful. + execSync('git init -q', { cwd: tmpDir }); + execSync(`git remote add origin ${remoteUrl}`, { cwd: tmpDir }); + return tmpDir; + }; + + it('returns undefined for a non-git directory', async () => { + const { getRemoteUrl } = await import('../../src/storage/git.js'); + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-test-')); + try { + expect(getRemoteUrl(tmpDir)).toBeUndefined(); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it('returns undefined for a git repo with no origin remote', async () => { + const { getRemoteUrl } = await import('../../src/storage/git.js'); + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-test-')); + try { + execSync('git init -q', { cwd: tmpDir }); + expect(getRemoteUrl(tmpDir)).toBeUndefined(); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it('strips trailing .git and lowercases host for HTTPS remotes', async () => { + const { getRemoteUrl } = await import('../../src/storage/git.js'); + const tmpDir = setupRepoWithRemote('https://GitHub.COM/Foo/Bar.git'); + try { + expect(getRemoteUrl(tmpDir)).toBe('https://github.com/Foo/Bar'); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it('lowercases host for SCP-style SSH remotes and strips .git', async () => { + const { getRemoteUrl } = await import('../../src/storage/git.js'); + const tmpDir = setupRepoWithRemote('git@GitHub.com:Foo/Bar.git'); + try { + expect(getRemoteUrl(tmpDir)).toBe('git@github.com:Foo/Bar'); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it('returns the same fingerprint for two clones of the same repo', async () => { + const { getRemoteUrl } = await import('../../src/storage/git.js'); + const a = setupRepoWithRemote('https://example.com/foo/bar.git'); + const b = setupRepoWithRemote('https://example.com/foo/bar'); + try { + expect(getRemoteUrl(a)).toBe(getRemoteUrl(b)); + expect(getRemoteUrl(a)).toBeTruthy(); + } finally { + fs.rmSync(a, { recursive: true, force: true }); + fs.rmSync(b, { recursive: true, force: true }); + } + }); +}); diff --git a/gitnexus/test/unit/index-repo-command.test.ts b/gitnexus/test/unit/index-repo-command.test.ts index 2c2e19b3c..3f7a57153 100644 --- a/gitnexus/test/unit/index-repo-command.test.ts +++ b/gitnexus/test/unit/index-repo-command.test.ts @@ -25,6 +25,11 @@ vi.mock('../../src/storage/repo-manager.js', () => ({ vi.mock('../../src/storage/git.js', () => ({ getGitRoot: mockGetGitRoot, isGitRepo: mockIsGitRepo, + // `index-repo.ts` calls `getRemoteUrl` to backfill `remoteUrl` on + // older `.gitnexus/meta.json` files. The unit tests don't care + // about the remote URL, so a static `undefined` keeps behaviour + // identical to the pre-feature path. + getRemoteUrl: vi.fn().mockReturnValue(undefined), })); describe('indexCommand', () => { diff --git a/gitnexus/test/unit/sibling-clone-drift.test.ts b/gitnexus/test/unit/sibling-clone-drift.test.ts new file mode 100644 index 000000000..cd063ceec --- /dev/null +++ b/gitnexus/test/unit/sibling-clone-drift.test.ts @@ -0,0 +1,308 @@ +/** + * Unit tests: sibling-clone drift detection. + * + * Issue: a single absolute `repoPath` per registry entry causes silent + * graph drift when the same logical repo lives at multiple on-disk + * paths (worktrees, multi-agent workspaces, etc.). We persist a + * canonical `remoteUrl` at index time and use it to: + * - find sibling clones registered under different paths + * - detect when the caller's `cwd` is in a sibling clone whose HEAD + * has drifted from the indexed `lastCommit` + * + * These tests cover the persistence + helpers; the LocalBackend + * stderr-warning side-effect is exercised end-to-end via the same + * `checkCwdMatch` API. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import path from 'path'; +import { execSync } from 'child_process'; +import { + registerRepo, + readRegistry, + findSiblingClones, + type RepoMeta, +} from '../../src/storage/repo-manager.js'; +import { checkCwdMatch } from '../../src/core/git-staleness.js'; +import { createTempDir } from '../helpers/test-db.js'; + +const initRepoWithCommit = (dir: string, remoteUrl?: string): string => { + execSync('git init -q', { cwd: dir }); + execSync('git config user.email test@example.com', { cwd: dir }); + execSync('git config user.name test', { cwd: dir }); + execSync('git commit --allow-empty -q -m initial', { cwd: dir }); + if (remoteUrl) execSync(`git remote add origin ${remoteUrl}`, { cwd: dir }); + return execSync('git rev-parse HEAD', { cwd: dir }).toString().trim(); +}; + +describe('registry persists remoteUrl', () => { + let tmpHome: Awaited>; + let tmpRepo: Awaited>; + let savedHome: string | undefined; + + beforeEach(async () => { + tmpHome = await createTempDir('gitnexus-sibling-home-'); + tmpRepo = await createTempDir('gitnexus-sibling-repo-'); + savedHome = process.env.GITNEXUS_HOME; + process.env.GITNEXUS_HOME = tmpHome.dbPath; + }); + + afterEach(async () => { + if (savedHome === undefined) delete process.env.GITNEXUS_HOME; + else process.env.GITNEXUS_HOME = savedHome; + await tmpHome.cleanup(); + await tmpRepo.cleanup(); + }); + + it('round-trips remoteUrl from RepoMeta into the registry', async () => { + const meta: RepoMeta = { + repoPath: tmpRepo.dbPath, + lastCommit: 'abc123', + indexedAt: new Date().toISOString(), + remoteUrl: 'https://example.com/foo/bar', + }; + await registerRepo(tmpRepo.dbPath, meta); + const entries = await readRegistry(); + expect(entries).toHaveLength(1); + expect(entries[0].remoteUrl).toBe('https://example.com/foo/bar'); + }); + + it('omits remoteUrl from registry when meta has none (back-compat)', async () => { + const meta: RepoMeta = { + repoPath: tmpRepo.dbPath, + lastCommit: 'abc123', + indexedAt: new Date().toISOString(), + }; + await registerRepo(tmpRepo.dbPath, meta); + const entries = await readRegistry(); + expect(entries[0].remoteUrl).toBeUndefined(); + }); +}); + +describe('findSiblingClones', () => { + let tmpHome: Awaited>; + let savedHome: string | undefined; + + beforeEach(async () => { + tmpHome = await createTempDir('gitnexus-sibling-find-home-'); + savedHome = process.env.GITNEXUS_HOME; + process.env.GITNEXUS_HOME = tmpHome.dbPath; + }); + + afterEach(async () => { + if (savedHome === undefined) delete process.env.GITNEXUS_HOME; + else process.env.GITNEXUS_HOME = savedHome; + await tmpHome.cleanup(); + }); + + it('returns other registered entries with the same remoteUrl', async () => { + const a = await createTempDir('clone-a-'); + const b = await createTempDir('clone-b-'); + const c = await createTempDir('clone-c-'); + try { + const remote = 'https://example.com/foo/bar'; + const baseMeta = { + lastCommit: 'x', + indexedAt: new Date().toISOString(), + }; + await registerRepo(a.dbPath, { ...baseMeta, repoPath: a.dbPath, remoteUrl: remote }); + await registerRepo(b.dbPath, { ...baseMeta, repoPath: b.dbPath, remoteUrl: remote }); + await registerRepo(c.dbPath, { + ...baseMeta, + repoPath: c.dbPath, + remoteUrl: 'https://example.com/other/repo', + }); + + const siblings = await findSiblingClones(remote, a.dbPath); + expect(siblings.map((s) => s.path).sort()).toEqual([path.resolve(b.dbPath)]); + } finally { + await a.cleanup(); + await b.cleanup(); + await c.cleanup(); + } + }); + + it('returns [] when remoteUrl is undefined (no fingerprint to match)', async () => { + const a = await createTempDir('clone-a-'); + try { + await registerRepo(a.dbPath, { + repoPath: a.dbPath, + lastCommit: 'x', + indexedAt: new Date().toISOString(), + }); + const siblings = await findSiblingClones(undefined, a.dbPath); + expect(siblings).toEqual([]); + } finally { + await a.cleanup(); + } + }); +}); + +describe('checkCwdMatch', () => { + let tmpHome: Awaited>; + let savedHome: string | undefined; + + beforeEach(async () => { + tmpHome = await createTempDir('gitnexus-cwd-match-home-'); + savedHome = process.env.GITNEXUS_HOME; + process.env.GITNEXUS_HOME = tmpHome.dbPath; + }); + + afterEach(async () => { + if (savedHome === undefined) delete process.env.GITNEXUS_HOME; + else process.env.GITNEXUS_HOME = savedHome; + await tmpHome.cleanup(); + }); + + it('returns match=path when cwd is inside the registered entry', async () => { + const repo = await createTempDir('cwd-repo-'); + try { + const head = initRepoWithCommit(repo.dbPath, 'https://example.com/foo/bar'); + await registerRepo(repo.dbPath, { + repoPath: repo.dbPath, + lastCommit: head, + indexedAt: new Date().toISOString(), + remoteUrl: 'https://example.com/foo/bar', + }); + const m = await checkCwdMatch(repo.dbPath); + expect(m.match).toBe('path'); + expect(m.entry?.path).toBe(path.resolve(repo.dbPath)); + } finally { + await repo.cleanup(); + } + }); + + it('detects sibling-by-remote when sibling HEAD differs from indexed commit', async () => { + const indexed = await createTempDir('cwd-indexed-'); + const sibling = await createTempDir('cwd-sibling-'); + try { + const remote = 'https://example.com/foo/bar'; + const indexedHead = initRepoWithCommit(indexed.dbPath, remote); + // Sibling is a separate `git init` with the same remote URL — + // that's enough for the remote-URL-based fingerprint to match. + // Use a distinct commit message so the sibling's SHA cannot + // coincidentally collide with the indexed one even when both + // commits land in the same second. + execSync('git init -q', { cwd: sibling.dbPath }); + execSync('git config user.email test@example.com', { cwd: sibling.dbPath }); + execSync('git config user.name test', { cwd: sibling.dbPath }); + execSync('git commit --allow-empty -q -m sibling-distinct', { cwd: sibling.dbPath }); + execSync(`git remote add origin ${remote}`, { cwd: sibling.dbPath }); + + await registerRepo(indexed.dbPath, { + repoPath: indexed.dbPath, + lastCommit: indexedHead, + indexedAt: new Date().toISOString(), + remoteUrl: remote, + }); + + const m = await checkCwdMatch(sibling.dbPath); + expect(m.match).toBe('sibling-by-remote'); + expect(m.entry?.path).toBe(path.resolve(indexed.dbPath)); + // Path format differs between git and Node.js on Windows (8.3 short + // vs long names from os.tmpdir()). Verify the git root was resolved + // and it's not the indexed repo (it's the sibling clone's root). + expect(m.cwdGitRoot).toBeTruthy(); + expect(m.cwdGitRoot).not.toBe(path.resolve(indexed.dbPath)); + expect(m.hint).toBeTruthy(); + } finally { + await indexed.cleanup(); + await sibling.cleanup(); + } + }); + + it('returns match=none when cwd is unrelated to any registered repo', async () => { + const indexed = await createTempDir('cwd-none-indexed-'); + const stranger = await createTempDir('cwd-none-stranger-'); + try { + const indexedHead = initRepoWithCommit(indexed.dbPath, 'https://example.com/foo/bar'); + initRepoWithCommit(stranger.dbPath, 'https://example.com/totally/different'); + + await registerRepo(indexed.dbPath, { + repoPath: indexed.dbPath, + lastCommit: indexedHead, + indexedAt: new Date().toISOString(), + remoteUrl: 'https://example.com/foo/bar', + }); + + const m = await checkCwdMatch(stranger.dbPath); + expect(m.match).toBe('none'); + } finally { + await indexed.cleanup(); + await stranger.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 + // silently-stale data. We can't easily share commits between two + // separate temp `git init` repos, so we instead verify that the + // cwd HEAD is captured and the hint mentions either drift or a + // HEAD mismatch. + const indexed = await createTempDir('cwd-stale-indexed-'); + const sibling = await createTempDir('cwd-stale-sibling-'); + try { + const remote = 'https://example.com/foo/bar'; + initRepoWithCommit(indexed.dbPath, remote); + // Use a fabricated indexed commit that doesn't exist in the + // sibling clone — git rev-list will fail and `drift` is left + // undefined. The hint must still flag this as a stale-or-divergent + // sibling clone. Named to make test intent obvious; not git's + // all-zero "null" OID, which has special semantics in some git + // commands. + const FAKE_INDEXED_COMMIT = 'deadbeefdeadbeefdeadbeefdeadbeefdeadbeef'; + initRepoWithCommit(sibling.dbPath, remote); + + await registerRepo(indexed.dbPath, { + repoPath: indexed.dbPath, + lastCommit: FAKE_INDEXED_COMMIT, + indexedAt: new Date().toISOString(), + remoteUrl: remote, + }); + + const m = await checkCwdMatch(sibling.dbPath); + expect(m.match).toBe('sibling-by-remote'); + expect(m.cwdHead).toBeTruthy(); + expect(m.cwdHead).not.toBe(FAKE_INDEXED_COMMIT); + expect(m.hint).toMatch(/sibling clone/); + } finally { + await indexed.cleanup(); + await sibling.cleanup(); + } + }); + + it('omits hint when sibling cwd HEAD matches the indexed commit (no drift)', async () => { + // Same-commit sibling: the relationship is real (and surfaces in + // `match: 'sibling-by-remote'`) but there is nothing to warn + // about. `LocalBackend.maybeWarnSiblingDrift` short-circuits in + // exactly this case, so confirming `hint` is unset here pins the + // contract those two pieces of code rely on. + const indexed = await createTempDir('cwd-same-indexed-'); + const sibling = await createTempDir('cwd-same-sibling-'); + try { + const remote = 'https://example.com/foo/bar'; + initRepoWithCommit(indexed.dbPath, remote); + const siblingHead = initRepoWithCommit(sibling.dbPath, remote); + + // Register the indexed entry with the SIBLING's HEAD as + // `lastCommit`. That is the on-disk reality when both clones + // happen to be at the same commit hash — e.g. immediately + // after both fast-forwarded to the same `main`. + await registerRepo(indexed.dbPath, { + repoPath: indexed.dbPath, + lastCommit: siblingHead, + indexedAt: new Date().toISOString(), + remoteUrl: remote, + }); + + const m = await checkCwdMatch(sibling.dbPath); + expect(m.match).toBe('sibling-by-remote'); + expect(m.cwdHead).toBe(siblingHead); + expect(m.hint).toBeUndefined(); + } finally { + await indexed.cleanup(); + await sibling.cleanup(); + } + }); +});