From 170eefd4a0893be9d255dc3581d51fd448ce2e96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Sat, 29 Aug 2026 13:56:41 +0100 Subject: [PATCH 1/3] fix(status): judge freshness by covered files, not a dirty working tree (#3083) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `gitnexus status` reported "stale (re-run gitnexus analyze)" whenever the working tree held any modified or untracked file, including files the index never reads. Because `analyze` cannot commit, stash or delete such a file, the remedy it prescribed could not clear the verdict — the only way back to up-to-date was to remove the file. `meta.fileHashes` already records the exact set of files a run covered, so answer the question directly: compare those hashes against disk, reusing analyze's own scan, hash and diff helpers so the two cannot disagree about what "changed" means. A new coverable file still counts as stale (the index is genuinely incomplete then), but one `analyze` now settles it. The repo-wide dirty flag survives only as the fallback for metadata written before `fileHashes` existed. Both freshness checks now read GitNexus's own analyze output (AGENTS.md, CLAUDE.md, the agent skill mirrors) from one shared list. They previously held separate copies, and since analyze rewrites those files after recording hashes, a per-file comparison that missed them would report a freshly indexed repository as permanently stale. Closes #3077 Co-authored-by: Gergo Magyar Co-authored-by: Cursor --- gitnexus/src/cli/i18n/en.ts | 11 + gitnexus/src/cli/i18n/zh-CN.ts | 9 + gitnexus/src/cli/status.ts | 107 ++++- gitnexus/src/core/index-content-drift.ts | 170 ++++++++ .../src/core/ingestion/filesystem-walker.ts | 37 +- .../core/ingestion/pipeline-phases/scan.ts | 37 +- gitnexus/src/core/run-analyze.ts | 13 + gitnexus/src/storage/file-hash.ts | 22 +- gitnexus/src/storage/git.ts | 116 +++-- .../src/storage/gitnexus-managed-paths.ts | 53 +++ gitnexus/src/storage/repo-meta.ts | 12 + gitnexus/test/unit/git-utils.test.ts | 207 +++++++++ .../test/unit/index-content-drift.test.ts | 404 ++++++++++++++++++ gitnexus/test/unit/list-status-branch.test.ts | 1 + .../test/unit/status-content-drift.test.ts | 279 ++++++++++++ 15 files changed, 1429 insertions(+), 49 deletions(-) create mode 100644 gitnexus/src/core/index-content-drift.ts create mode 100644 gitnexus/src/storage/gitnexus-managed-paths.ts create mode 100644 gitnexus/test/unit/index-content-drift.test.ts create mode 100644 gitnexus/test/unit/status-content-drift.test.ts diff --git a/gitnexus/src/cli/i18n/en.ts b/gitnexus/src/cli/i18n/en.ts index 15e1644e7..698e27d4e 100644 --- a/gitnexus/src/cli/i18n/en.ts +++ b/gitnexus/src/cli/i18n/en.ts @@ -33,6 +33,17 @@ export const en = { 'status.workspaceIndexLabel': "Workspace index: last analyzed on '{{primary}}' (re-run gitnexus analyze to follow the current branch)", 'status.status': 'Status', + 'status.indexContentCurrent': 'Index content: matches all {{count}} covered file(s)', + 'status.indexContentDrifted': + 'Index content: {{changed}} changed, {{added}} added, {{deleted}} deleted', + 'status.indexContentMore': ' ...and {{count}} more {{label}}', + 'status.indexContentUnmeasurable': + 'Index content: not comparable ({{reason}}); fell back to the working-tree check', + 'status.indexContentScanFailed': + 'Index content: coverage scan failed; treating the index as stale', + 'status.driftChanged': 'changed', + 'status.driftAdded': 'added', + 'status.driftDeleted': 'deleted', 'status.upToDate': '✅ up-to-date', 'status.stale': '⚠️ stale (re-run gitnexus analyze)', 'clean.deleteAll': 'This will delete GitNexus indexes for {{count}} repo(s):', diff --git a/gitnexus/src/cli/i18n/zh-CN.ts b/gitnexus/src/cli/i18n/zh-CN.ts index 63e290e54..24a6a2ad6 100644 --- a/gitnexus/src/cli/i18n/zh-CN.ts +++ b/gitnexus/src/cli/i18n/zh-CN.ts @@ -37,6 +37,15 @@ export const zhCN = { 'status.workspaceIndexLabel': "工作区索引：最近在 '{{primary}}' 分支上分析（重新运行 gitnexus analyze 以跟随当前分支）", 'status.status': '状态', + 'status.indexContentCurrent': '索引内容：与覆盖的全部 {{count}} 个文件一致', + 'status.indexContentDrifted': + '索引内容：{{changed}} 个已修改，{{added}} 个新增，{{deleted}} 个已删除', + 'status.indexContentMore': ' ……另有 {{count}} 个 {{label}}', + 'status.indexContentUnmeasurable': '索引内容：无法比对（{{reason}}），已回退到工作区检查', + 'status.indexContentScanFailed': '索引内容：覆盖扫描失败，按过期处理', + 'status.driftChanged': '已修改', + 'status.driftAdded': '新增', + 'status.driftDeleted': '已删除', 'status.upToDate': '✅ 已是最新', 'status.stale': '⚠️ 已过期（重新运行 gitnexus analyze）', 'clean.deleteAll': '将删除 {{count}} 个仓库的 GitNexus 索引：', diff --git a/gitnexus/src/cli/status.ts b/gitnexus/src/cli/status.ts index 09eb415d3..e0ee08bcc 100644 --- a/gitnexus/src/cli/status.ts +++ b/gitnexus/src/cli/status.ts @@ -18,8 +18,69 @@ import { resolveAnalyzerRunnerIdentity, } from '../core/analyzer-identity.js'; import { getIndexIncompleteReasons } from '../core/index-freshness.js'; +import { detectIndexContentDrift, type IndexContentDrift } from '../core/index-content-drift.js'; import { t } from './i18n/index.js'; +/** How many drifted paths the report names before summarizing the rest. */ +const DRIFT_SAMPLE_LIMIT = 10; + +/** + * Machine-readable form of the per-file comparison. `'not-checked'` is its own + * value rather than a silent omission: it says the index was already stale on + * metadata alone, so the scan was skipped, which is not the same claim as a + * scan that ran and found nothing. + */ +const describeContentDrift = (drift: IndexContentDrift | undefined) => { + if (!drift) return { status: 'not-checked' as const }; + if (drift.kind === 'current') { + return { status: 'current' as const, coveredFiles: drift.coveredFileCount }; + } + if (drift.kind === 'unmeasurable') { + return { status: 'unmeasurable' as const, reason: drift.reason }; + } + return { + status: 'drifted' as const, + counts: { + changed: drift.changed.length, + added: drift.added.length, + deleted: drift.deleted.length, + }, + changed: drift.changed.slice(0, DRIFT_SAMPLE_LIMIT), + added: drift.added.slice(0, DRIFT_SAMPLE_LIMIT), + deleted: drift.deleted.slice(0, DRIFT_SAMPLE_LIMIT), + truncated: { + changed: drift.changed.length > DRIFT_SAMPLE_LIMIT, + added: drift.added.length > DRIFT_SAMPLE_LIMIT, + deleted: drift.deleted.length > DRIFT_SAMPLE_LIMIT, + }, + }; +}; + +/** Escape control characters in repo-relative paths before printing. */ +const formatDriftPath = (rel: string): string => + /[\u0000-\u001f\u007f]/.test(rel) ? JSON.stringify(rel) : rel; +const printDriftDetail = (drift: Extract): void => { + console.log( + t('status.indexContentDrifted', { + changed: drift.changed.length, + added: drift.added.length, + deleted: drift.deleted.length, + }), + ); + const labelled: [string, readonly string[]][] = [ + [t('status.driftChanged'), drift.changed], + [t('status.driftAdded'), drift.added], + [t('status.driftDeleted'), drift.deleted], + ]; + for (const [label, paths] of labelled) { + for (const p of paths.slice(0, DRIFT_SAMPLE_LIMIT)) { + console.log(` ${label}: ${formatDriftPath(p)}`); + } + const remaining = paths.length - DRIFT_SAMPLE_LIMIT; + if (remaining > 0) console.log(t('status.indexContentMore', { count: remaining, label })); + } +}; + export interface StatusOptions { json?: boolean; } @@ -85,14 +146,36 @@ export const statusCommand = async (options: StatusOptions = {}) => { currentRunnerIdentity, ); const incompleteReasons = getIndexIncompleteReasons(activeMeta); - // A matching HEAD is not enough: `analyze` re-indexes a dirty working tree, - // so a repo with uncommitted source changes is stale even at the same commit. - // Skip the check for non-git folders (currentCommit === '') to match analyze. - const isUpToDate = + const metadataIsCurrent = currentCommit === activeMeta.lastCommit && runnerIdentityIsCurrent && - incompleteReasons.length === 0 && - (currentCommit === '' || !isWorkingTreeDirty(repo.repoPath)); + incompleteReasons.length === 0; + + // A matching HEAD is not enough: `analyze` re-indexes changed content at the + // same commit, so the files the index covers must still be compared against + // disk. Only worth the scan once the cheap metadata checks agree, and skipped + // for non-git folders (currentCommit === '') to match analyze. + const contentDrift: IndexContentDrift | undefined = + metadataIsCurrent && currentCommit !== '' + ? await detectIndexContentDrift( + repo.repoPath, + activeMeta.fileHashes, + activeMeta.indexCoverage, + ) + : undefined; + + // The repo-wide dirty flag survives only as the fallback for metadata written + // before `fileHashes` existed. Where the per-file comparison can run it + // decides, so a file the index does not cover no longer pins a byte-current + // index to a "stale" verdict that `analyze` is powerless to clear (#3077). + const contentIsCurrent = + contentDrift === undefined || + contentDrift.kind === 'current' || + (contentDrift.kind === 'unmeasurable' && + contentDrift.reason === 'no-file-hashes' && + !isWorkingTreeDirty(repo.repoPath)); + + const isUpToDate = metadataIsCurrent && contentIsCurrent; if (options.json) { console.log( JSON.stringify({ @@ -111,6 +194,7 @@ export const statusCommand = async (options: StatusOptions = {}) => { commit: currentCommit, runnerIdentity: currentRunnerIdentity, }, + contentDrift: describeContentDrift(contentDrift), status: isUpToDate ? 'up-to-date' : 'stale', }), ); @@ -137,5 +221,16 @@ export const statusCommand = async (options: StatusOptions = {}) => { console.log(`Index incomplete reasons: ${JSON.stringify(incompleteReasons)}`); } console.log(`${t('status.currentRunnerIdentity')}: ${JSON.stringify(currentRunnerIdentity)}`); + if (contentDrift?.kind === 'current') { + console.log(t('status.indexContentCurrent', { count: contentDrift.coveredFileCount })); + } else if (contentDrift?.kind === 'drifted') { + printDriftDetail(contentDrift); + } else if (contentDrift?.kind === 'unmeasurable') { + if (contentDrift.reason === 'scan-failed') { + console.log(t('status.indexContentScanFailed')); + } else if (!isUpToDate) { + console.log(t('status.indexContentUnmeasurable', { reason: contentDrift.reason })); + } + } console.log(`${t('status.status')}: ${isUpToDate ? t('status.upToDate') : t('status.stale')}`); }; diff --git a/gitnexus/src/core/index-content-drift.ts b/gitnexus/src/core/index-content-drift.ts new file mode 100644 index 000000000..3599db7b1 --- /dev/null +++ b/gitnexus/src/core/index-content-drift.ts @@ -0,0 +1,170 @@ +/** + * Does the index still reflect the files it actually covers? + * + * `status` used to answer this with a repo-wide `git status --porcelain` + * boolean, which says something different: whether the working tree differs + * from HEAD. Those two questions diverge in both directions. A scratch file, + * a build artifact, or a tracked file under a tool directory the indexer + * never reads makes the tree dirty while every indexed file is byte-current — + * and because `analyze` cannot commit or delete that file, the resulting + * "stale (re-run gitnexus analyze)" verdict was unclearable (#3077). It also + * misses the reverse case: reverting a file that was indexed while dirty + * leaves a clean tree over an index holding the pre-revert content. + * + * `meta.fileHashes` already records the exact set of files the last run + * covered, so the question can be answered directly. This module recomputes + * the coverage set with the same `walkRepositoryPaths` scan (ignore rules and + * dotfile handling stay shared) and the large-file cap recorded in + * `meta.indexCoverage`, hashes only the paths that can actually have changed + * since that run, and diffs against what was recorded. + */ + +import { constants as fsConstants } from 'node:fs'; +import { access } from 'node:fs/promises'; +import path from 'node:path'; +import { walkRepositoryPaths } from './ingestion/filesystem-walker.js'; +import { computeFileHashesDetailed } from '../storage/file-hash.js'; +import { listWorkingTreeDirtyPaths } from '../storage/git.js'; +import { isGitNexusManagedPath } from '../storage/gitnexus-managed-paths.js'; +import { chunk } from '../lib/utils.js'; +import { logger } from './logger.js'; +import type { RepoMeta } from '../storage/repo-meta.js'; + +/** Why the recorded coverage set could not be compared against disk at all. */ +export type IndexContentUnmeasurableReason = + /** Metadata predates per-file hashes, or the run recorded none (non-git). */ + | 'no-file-hashes' + /** The repository scan or hashing pass threw. */ + | 'scan-failed'; + +/** + * A three-way verdict. `'unmeasurable'` is kept apart from `'current'` on + * purpose: it means the comparison never ran, which is not evidence the index + * is fresh. Legacy metadata without hashes still falls back to the working-tree + * check; a failed scan must not. + */ +export type IndexContentDrift = + | { kind: 'current'; coveredFileCount: number } + | { kind: 'drifted'; changed: string[]; added: string[]; deleted: string[] } + | { kind: 'unmeasurable'; reason: IndexContentUnmeasurableReason }; + +export type IndexCoveragePolicy = NonNullable; + +const HASH_BATCH = 100; + +const collectUnreadablePaths = async ( + repoPath: string, + relPaths: readonly string[], +): Promise => { + const unreadable: string[] = []; + for (const batch of chunk(relPaths, HASH_BATCH)) { + await Promise.all( + batch.map(async (rel) => { + try { + await access(path.join(repoPath, rel), fsConstants.R_OK); + } catch { + unreadable.push(rel); + } + }), + ); + } + unreadable.sort(); + return unreadable; +}; + +/** + * Compare the files recorded in `fileHashes` against the current working tree. + * + * `added` covers files the index would pick up but has never seen, so a new + * source file still reports stale — the index is genuinely incomplete then, + * and comparing only the recorded entries would wave that through. + */ +export const detectIndexContentDrift = async ( + repoPath: string, + fileHashes: Readonly> | undefined, + coverage?: IndexCoveragePolicy, +): Promise => { + if (!fileHashes || Object.keys(fileHashes).length === 0) { + return { kind: 'unmeasurable', reason: 'no-file-hashes' }; + } + + // Excluded from BOTH sides, or GitNexus's own output guarantees a mismatch: + // analyze rewrites AGENTS.md/CLAUDE.md after recording hashes, so they read + // as `added` on a first run and `changed` on every run after that — a fresh + // index would report itself stale forever. + const recorded = Object.fromEntries( + Object.entries(fileHashes).filter(([rel]) => !isGitNexusManagedPath(rel)), + ); + if (Object.keys(recorded).length === 0) { + return { kind: 'unmeasurable', reason: 'no-file-hashes' }; + } + + try { + const scanned = await walkRepositoryPaths(repoPath, undefined, { + quiet: true, + maxFileSizeBytes: coverage?.maxFileSizeBytes, + }); + const scannedPaths = scanned.map((file) => file.path).filter((p) => !isGitNexusManagedPath(p)); + const scannedSet = new Set(scannedPaths); + const recordedSet = new Set(Object.keys(recorded)); + + // Legacy indexes have `fileHashes` but no `indexCoverage`. A later default + // cap would omit a still-present hashed file and call it deleted. Recorded + // paths that still exist stay in the coverage set even if this walk skipped + // them for size. + const recovered = new Set(); + for (const rel of recordedSet) { + if (scannedSet.has(rel)) continue; + try { + await access(path.join(repoPath, rel), fsConstants.R_OK); + recovered.add(rel); + scannedSet.add(rel); + } catch { + // Missing or unreadable: stays deleted / changed below. + } + } + + const added = scannedPaths.filter((p) => !recordedSet.has(p)).sort(); + const deleted = [...recordedSet].filter((p) => !scannedSet.has(p)).sort(); + const intersection = [...recordedSet].filter((p) => scannedSet.has(p)); + + const dirtyNow = listWorkingTreeDirtyPaths(repoPath); + const dirtyAtIndex = coverage?.dirtyPaths; + const dirtyNowSet = dirtyNow === null ? null : new Set(dirtyNow); + const dirtyAtIndexSet = dirtyAtIndex === undefined ? undefined : new Set(dirtyAtIndex); + const hashCandidates = + dirtyNowSet === null || dirtyAtIndexSet === undefined + ? intersection + : intersection.filter( + (p) => dirtyAtIndexSet.has(p) || dirtyNowSet.has(p) || recovered.has(p), + ); + + const hashCandidateSet = new Set(hashCandidates); + const skipHash = intersection.filter((p) => !hashCandidateSet.has(p)); + const unreadableFromAccess = await collectUnreadablePaths(repoPath, skipHash); + const unreadableSet = new Set(unreadableFromAccess); + const { hashes: hashed, unreadable: unreadableFromHash } = await computeFileHashesDetailed( + repoPath, + hashCandidates, + ); + for (const p of unreadableFromHash) unreadableSet.add(p); + const changed: string[] = []; + for (const p of intersection) { + if (unreadableSet.has(p)) { + changed.push(p); + continue; + } + const currentHash = hashed.get(p) ?? recorded[p]; + if (currentHash !== recorded[p]) changed.push(p); + } + changed.sort(); + + if (changed.length === 0 && added.length === 0 && deleted.length === 0) { + return { kind: 'current', coveredFileCount: scannedSet.size }; + } + return { kind: 'drifted', changed, added, deleted }; + } catch (err) { + logger.warn({ err, repoPath }, 'index content drift scan failed'); + return { kind: 'unmeasurable', reason: 'scan-failed' }; + } +}; diff --git a/gitnexus/src/core/ingestion/filesystem-walker.ts b/gitnexus/src/core/ingestion/filesystem-walker.ts index 804921a62..7e41c07c2 100644 --- a/gitnexus/src/core/ingestion/filesystem-walker.ts +++ b/gitnexus/src/core/ingestion/filesystem-walker.ts @@ -57,16 +57,49 @@ const warnLargeFileSkip = (message: string): void => { logger.warn(message); }; +export interface WalkRepositoryOptions { + /** + * Suppress the operator-facing large-file notice. Set by read-only callers + * such as `status`, which reuse this scan purely to learn which files the + * index covers and must not emit analyze's progress commentary. + */ + quiet?: boolean; + /** + * Override the large-file cap. `status` replays the bytes recorded at + * analyze time so `--max-file-size` / `GITNEXUS_MAX_FILE_SIZE` cannot + * silently drop a file that the index actually covers. + */ + maxFileSizeBytes?: number; +} + /** * Phase 1: Scan repository — stat files to get paths + sizes, no content loaded. * Memory: ~10MB for 100K files vs ~1GB+ with content. */ +const assertWalkRootIsDirectory = async (repoPath: string): Promise => { + let st; + try { + st = await fs.stat(repoPath); + } catch (err) { + const code = (err as NodeJS.ErrnoException).code; + if (code === 'ENOENT' || code === 'ENOTDIR') { + throw new Error(`walkRepositoryPaths: path does not exist: ${repoPath}`); + } + throw err; + } + if (!st.isDirectory()) { + throw new Error(`walkRepositoryPaths: not a directory: ${repoPath}`); + } +}; + export const walkRepositoryPaths = async ( repoPath: string, onProgress?: (current: number, total: number, filePath: string) => void, + options: WalkRepositoryOptions = {}, ): Promise => { + await assertWalkRootIsDirectory(repoPath); const ignoreFilter = await createIgnoreFilter(repoPath); - const maxFileSizeBytes = getMaxFileSizeBytes(); + const maxFileSizeBytes = options.maxFileSizeBytes ?? getMaxFileSizeBytes(); const filtered = await glob('**/*', { cwd: repoPath, @@ -117,7 +150,7 @@ export const walkRepositoryPaths = async ( left.path < right.path ? -1 : left.path > right.path ? 1 : 0, ); - if (skippedLarge > 0) { + if (skippedLarge > 0 && !options.quiet) { const isDefault = maxFileSizeBytes === DEFAULT_MAX_FILE_SIZE_BYTES; const isOverrideUnset = !process.env.GITNEXUS_MAX_FILE_SIZE; const suffix = isDefault ? ', likely generated/vendored' : ''; diff --git a/gitnexus/src/core/ingestion/pipeline-phases/scan.ts b/gitnexus/src/core/ingestion/pipeline-phases/scan.ts index 5a1353267..f8629ea0d 100644 --- a/gitnexus/src/core/ingestion/pipeline-phases/scan.ts +++ b/gitnexus/src/core/ingestion/pipeline-phases/scan.ts @@ -30,20 +30,31 @@ export const scanPhase: PipelinePhase = { message: 'Scanning repository...', }); - const scannedFiles = await walkRepositoryPaths(ctx.repoPath, (current, total, filePath) => { - const scanProgress = Math.round((current / total) * 15); - ctx.onProgress({ - phase: 'extracting', - percent: scanProgress, - message: 'Scanning repository...', - detail: filePath, - stats: { - filesProcessed: current, - totalFiles: total, - nodesCreated: ctx.graph.nodeCount, - }, + let scannedFiles; + try { + scannedFiles = await walkRepositoryPaths(ctx.repoPath, (current, total, filePath) => { + const scanProgress = Math.round((current / total) * 15); + ctx.onProgress({ + phase: 'extracting', + percent: scanProgress, + message: 'Scanning repository...', + detail: filePath, + stats: { + filesProcessed: current, + totalFiles: total, + nodesCreated: ctx.graph.nodeCount, + }, + }); }); - }); + } catch (err) { + // Missing roots throw so status cannot treat an empty glob as "every + // covered file was deleted". The pipeline still reports an empty scan + // for a path that is not a directory, matching analyze of a bad cwd. + if (err instanceof Error && err.message.startsWith('walkRepositoryPaths:')) { + return { scannedFiles: [], allPaths: [], totalFiles: 0 }; + } + throw err; + } const totalFiles = scannedFiles.length; const allPaths = scannedFiles.map((f) => f.path); diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index 94d53de16..018efc42a 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -153,8 +153,11 @@ import { hasGitDir, getInferredRepoName, isWorkingTreeDirty, + listWorkingTreeDirtyPaths, resolveRepoIdentityRoot, } from '../storage/git.js'; +import { isGitNexusManagedPath } from '../storage/gitnexus-managed-paths.js'; +import { getMaxFileSizeBytes } from './ingestion/utils/max-file-size.js'; import type { CachedEmbedding } from './embeddings/types.js'; import { generateAIContextFiles } from '../cli/ai-context.js'; import { sanitizeDetectedBranch } from '../cli/analyze-config.js'; @@ -3642,6 +3645,16 @@ async function runFullAnalysisInner( // absence has exactly one meaning — an index older than the field. embeddingDims: EMBEDDING_DIMS, fileHashes: hasGitDir(repoPath) ? newFileHashesRecord : undefined, + indexCoverage: hasGitDir(repoPath) + ? { + maxFileSizeBytes: getMaxFileSizeBytes(), + dirtyPaths: ( + listWorkingTreeDirtyPaths(repoPath) ?? Object.keys(newFileHashesRecord) + ).filter( + (rel) => newFileHashesRecord[rel] !== undefined && !isGitNexusManagedPath(rel), + ), + } + : undefined, // This branch's full live chunk-key set (#2106 R6). `usedKeys` is every // chunk hash touched in this scan — cache HITS included (see parse-impl // usedKeys.add) — so it's complete even on an incremental run. Persisted diff --git a/gitnexus/src/storage/file-hash.ts b/gitnexus/src/storage/file-hash.ts index 2b2fc9491..2c002a83a 100644 --- a/gitnexus/src/storage/file-hash.ts +++ b/gitnexus/src/storage/file-hash.ts @@ -44,18 +44,32 @@ export const computeFileHashes = async ( repoPath: string, relPaths: readonly string[], ): Promise> => { - const out = new Map(); + const { hashes } = await computeFileHashesDetailed(repoPath, relPaths); + return hashes; +}; + +/** Like {@link computeFileHashes}, but keeps paths whose content could not be read. */ +export const computeFileHashesDetailed = async ( + repoPath: string, + relPaths: readonly string[], +): Promise<{ hashes: Map; unreadable: string[] }> => { + const hashes = new Map(); + const unreadable: string[] = []; const BATCH = 100; for (const batch of chunk(relPaths, BATCH)) { const results = await Promise.all( batch.map(async (rel) => { const h = await computeFileHash(path.join(repoPath, rel)); - return h ? ([rel, h] as const) : null; + return { rel, h }; }), ); - for (const r of results) if (r) out.set(r[0], r[1]); + for (const { rel, h } of results) { + if (h) hashes.set(rel, h); + else unreadable.push(rel); + } } - return out; + unreadable.sort(); + return { hashes, unreadable }; }; /** Result of comparing the current on-disk hashes against stored ones. */ diff --git a/gitnexus/src/storage/git.ts b/gitnexus/src/storage/git.ts index f4bf32366..792c1010b 100644 --- a/gitnexus/src/storage/git.ts +++ b/gitnexus/src/storage/git.ts @@ -4,42 +4,32 @@ import path from 'path'; import os from 'os'; import { logger } from '../core/logger.js'; import { toZeroBasedLine } from '../core/ingestion/utils/line-base.js'; +import { GITNEXUS_MANAGED_PATH_EXCLUDES, isGitNexusManagedPath } from './gitnexus-managed-paths.js'; // Git utilities for repository detection, commit tracking, and diff analysis const chompGitOutput = (value: Buffer): string => value.toString().replace(/\r?\n$/, ''); +const GIT_PATH_LIST_MAX_BUFFER = 64 * 1024 * 1024; /** * True when the working tree has uncommitted changes that analyze would - * re-index, even at a matching HEAD. Excludes the paths GitNexus writes during - * analyze (.gitnexus/, .claude/, .cursor/, AGENTS.md, CLAUDE.md, and the - * repo-local .agents/ mirror) so its own output never counts as dirty - * (regression vs PR #1233 behavior). The entire .agents/ tree is excluded, - * matching the .claude/ treatment, because the skill mirror writes across - * .agents/skills/ and deeper paths. Conservative on any git failure. Shared - * so `analyze`'s fast-path gate and `status`'s freshness report agree on what - * "dirty" means. + * re-index, even at a matching HEAD. Excludes GITNEXUS_MANAGED_PATHS so + * GitNexus's own analyze output never counts as dirty (regression vs PR #1233 + * behavior); whole directory trees are excluded, not just their root entries, + * because the skill mirror writes across .agents/skills/ and deeper paths. + * Conservative on any git failure. + * + * This drives `analyze`'s up-to-date fast path. It is deliberately coarse: + * a false "dirty" here costs only a hash diff that finds nothing. `status` + * reaches for the per-file comparison in core/index-content-drift.ts instead, + * because there the same false positive is a verdict the user cannot clear + * (#3077), and falls back to this only when that comparison cannot run. */ export const isWorkingTreeDirty = (repoPath: string): boolean => { try { const out = execFileSync( 'git', - [ - 'status', - '--porcelain', - '--', - '.', - ':(exclude).gitnexus', - ':(exclude).gitnexus/**', - ':(exclude).claude', - ':(exclude).claude/**', - ':(exclude).cursor', - ':(exclude).cursor/**', - ':(exclude)AGENTS.md', - ':(exclude)CLAUDE.md', - ':(exclude).agents', - ':(exclude).agents/**', - ], + ['status', '--porcelain', '--', '.', ...GITNEXUS_MANAGED_PATH_EXCLUDES], { cwd: repoPath, stdio: ['ignore', 'pipe', 'ignore'], @@ -53,6 +43,84 @@ export const isWorkingTreeDirty = (repoPath: string): boolean => { } }; +const parsePorcelainPaths = (porcelain: string): string[] => { + const paths = new Set(); + const records = porcelain.split('\0'); + for (let i = 0; i < records.length; i++) { + const record = records[i]; + if (record.length < 4) continue; + + const status = record.slice(0, 2); + paths.add(record.slice(3)); + + // In porcelain v1 `-z` mode, rename/copy source and destination paths are + // separate NUL records (with no human-facing ` -> ` delimiter). Keep both: + // either side may be present in the previous coverage set. + if (status.includes('R') || status.includes('C')) { + const pairedPath = records[++i]; + if (pairedPath) paths.add(pairedPath); + } + } + return [...paths]; +}; + +const gitPathListExec = { + stdio: ['ignore', 'pipe', 'ignore'] as ['ignore', 'pipe', 'ignore'], + encoding: 'utf8' as const, + maxBuffer: GIT_PATH_LIST_MAX_BUFFER, +}; + +const listHiddenIndexPaths = (repoPath: string): string[] => { + const out = execFileSync('git', ['ls-files', '-v', '-z', '--'], { + cwd: repoPath, + windowsHide: true, + ...gitPathListExec, + }); + const paths: string[] = []; + for (const record of out.split('\0')) { + if (record.length < 3 || record[1] !== ' ') continue; + const tag = record[0]; + // `S` marks skip-worktree. With `-v`, an assume-unchanged entry's + // ordinary tag is lower-cased (`H` -> `h`, `S` -> `s`, etc.). + if (tag === 'S' || (tag >= 'a' && tag <= 'z')) paths.push(record.slice(2)); + } + return paths; +}; + +/** + * Repo-relative paths `git status` reports as dirty or untracked, using the + * same managed-path excludes as {@link isWorkingTreeDirty}, plus tracked paths + * whose assume-unchanged or skip-worktree bits can hide content changes from + * porcelain. `null` means either query failed — callers must not treat that as + * a clean tree. + */ +export const listWorkingTreeDirtyPaths = (repoPath: string): string[] | null => { + try { + const out = execFileSync( + 'git', + [ + 'status', + '--porcelain=v1', + '-z', + '--untracked-files=all', + '--', + '.', + ...GITNEXUS_MANAGED_PATH_EXCLUDES, + ], + { cwd: repoPath, windowsHide: true, ...gitPathListExec }, + ); + return [ + ...new Set( + [...parsePorcelainPaths(out), ...listHiddenIndexPaths(repoPath)].filter( + (rel) => !isGitNexusManagedPath(rel), + ), + ), + ]; + } catch { + return null; + } +}; + /** * Snapshot, per candidate file, whether it is safe for `selfCommitContextFiles` * to auto-commit — call this BEFORE `analyze` writes AGENTS.md/CLAUDE.md. diff --git a/gitnexus/src/storage/gitnexus-managed-paths.ts b/gitnexus/src/storage/gitnexus-managed-paths.ts new file mode 100644 index 000000000..cbdd7b81b --- /dev/null +++ b/gitnexus/src/storage/gitnexus-managed-paths.ts @@ -0,0 +1,53 @@ +/** + * The paths GitNexus itself writes during `analyze`. + * + * `analyze` rewrites the stats blocks in AGENTS.md/CLAUDE.md and refreshes the + * agent skill mirrors as its final step — after it has recorded the per-file + * hashes for the run. Counting its own output as a repository change makes + * every completed run look immediately out of date, which is the regression + * PR #1233 introduced and #1233's fix excluded these paths to prevent. + * + * Two freshness checks depend on this list agreeing: `isWorkingTreeDirty` + * (analyze's up-to-date fast-path gate) and the per-file comparison behind + * `status`. They used to hold separate copies of it, so a path added to one + * silently became a permanent "stale" verdict in the other. One list, imported + * by both. + */ + +/** + * Repository-root-relative. A directory entry covers everything beneath it; + * a file entry matches only itself. Prefix collisions are NOT matches — + * `.agentsrc` is an ordinary file, not part of the `.agents` tree. + */ +export const GITNEXUS_MANAGED_PATHS = [ + '.gitnexus', + '.claude', + '.cursor', + '.agents', + 'AGENTS.md', + 'CLAUDE.md', +] as const; + +/** + * Git pathspecs excluding {@link GITNEXUS_MANAGED_PATHS} from a `git status` + * run rooted at the repository. Patterns include `./` so they match only at + * the repo root: a slash-free `:(exclude)AGENTS.md` would also drop + * `docs/AGENTS.md`, which {@link isGitNexusManagedPath} does not treat as + * managed. Both forms are emitted per entry: the root path itself, and `/**` + * for directory contents. + */ +export const GITNEXUS_MANAGED_PATH_EXCLUDES: readonly string[] = GITNEXUS_MANAGED_PATHS.flatMap( + (managed) => [`:(exclude,glob)./${managed}`, `:(exclude,glob)./${managed}/**`], +); + +/** + * True when a repository-relative path is GitNexus's own output. Mirrors the + * pathspec semantics above: root-relative, whole path segments only, so + * neither `.agentsrc` nor a nested `subdir/.agents/` is treated as managed. + */ +export const isGitNexusManagedPath = (relPath: string): boolean => { + const normalized = relPath.replace(/\\/g, '/'); + return GITNEXUS_MANAGED_PATHS.some( + (managed) => normalized === managed || normalized.startsWith(`${managed}/`), + ); +}; diff --git a/gitnexus/src/storage/repo-meta.ts b/gitnexus/src/storage/repo-meta.ts index 3cda63545..b7ab3b871 100644 --- a/gitnexus/src/storage/repo-meta.ts +++ b/gitnexus/src/storage/repo-meta.ts @@ -284,6 +284,18 @@ export interface RepoMeta { * Map keys are repo-relative paths. */ fileHashes?: Record; + /** + * Coverage policy used when `fileHashes` was recorded. `status` replays it + * so analyze-time `--max-file-size` / `GITNEXUS_MAX_FILE_SIZE` cannot make + * a later default-cap walk drop a file the index actually covers. + * `dirtyPaths` are covered files that were dirty vs HEAD at that moment — + * status must re-hash those even after Git becomes clean (indexed-dirty then + * restore). Absent on indexes written before this field. + */ + indexCoverage?: { + maxFileSizeBytes: number; + dirtyPaths?: string[]; + }; /** * Set when a run finished but the persisted edge count came back far short * of what the pipeline produced — the B2 "refresh reports SUCCESS while the diff --git a/gitnexus/test/unit/git-utils.test.ts b/gitnexus/test/unit/git-utils.test.ts index feb49effd..a5cb94083 100644 --- a/gitnexus/test/unit/git-utils.test.ts +++ b/gitnexus/test/unit/git-utils.test.ts @@ -928,3 +928,210 @@ describe('isWorkingTreeDirty', () => { } }); }); + +describe('listWorkingTreeDirtyPaths', () => { + it('returns an empty list for a clean repository', async () => { + const { listWorkingTreeDirtyPaths } = await import('../../src/storage/git.js'); + const repo = makeIsolatedGitRepo(); + try { + fs.writeFileSync(path.join(repo, 'README.md'), 'hi'); + execFileSync(gitExecutable, ['add', '--', 'README.md'], { cwd: repo, stdio: 'ignore' }); + execFileSync(gitExecutable, ['commit', '-q', '-m', 'init'], { + cwd: repo, + stdio: 'ignore', + }); + + expect(listWorkingTreeDirtyPaths(repo)).toEqual([]); + } finally { + fs.rmSync(repo, { recursive: true, force: true }); + } + }); + + it('returns dirty source paths and omits GitNexus-managed writes', async () => { + const { listWorkingTreeDirtyPaths } = await import('../../src/storage/git.js'); + const repo = makeIsolatedGitRepo(); + try { + fs.writeFileSync(path.join(repo, 'README.md'), 'hi'); + execSync('git add -A && git commit -q -m init', { cwd: repo, stdio: 'ignore' }); + fs.mkdirSync(path.join(repo, 'src'), { recursive: true }); + fs.writeFileSync(path.join(repo, 'src', 'foo.ts'), 'export const x = 1;'); + fs.mkdirSync(path.join(repo, '.gitnexus'), { recursive: true }); + fs.writeFileSync(path.join(repo, '.gitnexus', 'meta.json'), '{}'); + fs.writeFileSync(path.join(repo, 'AGENTS.md'), 'x'); + + expect(listWorkingTreeDirtyPaths(repo)).toEqual(['src/foo.ts']); + } finally { + fs.rmSync(repo, { recursive: true, force: true }); + } + }); + + it('still reports nested lookalikes that are not GitNexus-managed', async () => { + const { isWorkingTreeDirty, listWorkingTreeDirtyPaths } = + await import('../../src/storage/git.js'); + const repo = makeIsolatedGitRepo(); + try { + fs.mkdirSync(path.join(repo, 'docs'), { recursive: true }); + fs.writeFileSync(path.join(repo, 'docs', 'AGENTS.md'), 'project notes'); + execFileSync(gitExecutable, ['add', '--', 'docs/AGENTS.md'], { cwd: repo, stdio: 'ignore' }); + execFileSync(gitExecutable, ['commit', '-q', '-m', 'init'], { + cwd: repo, + stdio: 'ignore', + }); + fs.writeFileSync(path.join(repo, 'docs', 'AGENTS.md'), 'edited notes'); + + expect(isWorkingTreeDirty(repo)).toBe(true); + expect(listWorkingTreeDirtyPaths(repo)).toEqual(['docs/AGENTS.md']); + } finally { + fs.rmSync(repo, { recursive: true, force: true }); + } + }); + + it('returns null (not an empty list) outside a git repository', async () => { + const { listWorkingTreeDirtyPaths } = await import('../../src/storage/git.js'); + const dir = makeIsolatedTempDir('gn-nongit-paths-'); + try { + expect(listWorkingTreeDirtyPaths(dir)).toBeNull(); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('preserves non-ASCII, newline, and arrow-shaped filenames exactly', async () => { + const { listWorkingTreeDirtyPaths } = await import('../../src/storage/git.js'); + const repo = makeIsolatedGitRepo(); + const names = ['src/ä.ts']; + if (process.platform !== 'win32') { + names.push('src/a -> b.ts', 'src/line\nbreak.ts', 'src/tab\tname.ts', 'src/back\\slash.ts'); + } + try { + for (const name of names) { + fs.mkdirSync(path.dirname(path.join(repo, name)), { recursive: true }); + fs.writeFileSync(path.join(repo, name), 'before'); + } + execFileSync(gitExecutable, ['add', '--', ...names], { cwd: repo, stdio: 'ignore' }); + execFileSync(gitExecutable, ['commit', '-q', '-m', 'init'], { cwd: repo, stdio: 'ignore' }); + for (const name of names) fs.writeFileSync(path.join(repo, name), 'after'); + + expect(listWorkingTreeDirtyPaths(repo)?.sort()).toEqual([...names].sort()); + } finally { + fs.rmSync(repo, { recursive: true, force: true }); + } + }); + + it.skipIf(process.platform === 'win32')( + 'returns both paths for a rename without parsing filename text', + async () => { + const { listWorkingTreeDirtyPaths } = await import('../../src/storage/git.js'); + const repo = makeIsolatedGitRepo(); + const before = 'src/before -> literal.ts'; + const after = 'src/after -> literal.ts'; + try { + fs.mkdirSync(path.join(repo, 'src'), { recursive: true }); + fs.writeFileSync(path.join(repo, before), 'content'); + execFileSync(gitExecutable, ['add', '--', before], { cwd: repo, stdio: 'ignore' }); + execFileSync(gitExecutable, ['commit', '-q', '-m', 'init'], { cwd: repo, stdio: 'ignore' }); + execFileSync(gitExecutable, ['mv', '--', before, after], { cwd: repo, stdio: 'ignore' }); + + expect(listWorkingTreeDirtyPaths(repo)?.sort()).toEqual([after, before].sort()); + } finally { + fs.rmSync(repo, { recursive: true, force: true }); + } + }, + ); + + it.each(['--assume-unchanged', '--skip-worktree'])( + 'includes paths hidden by git update-index %s', + async (flag) => { + const { listWorkingTreeDirtyPaths } = await import('../../src/storage/git.js'); + const repo = makeIsolatedGitRepo(); + try { + fs.writeFileSync(path.join(repo, 'hidden.ts'), 'before'); + execFileSync(gitExecutable, ['add', '--', 'hidden.ts'], { cwd: repo, stdio: 'ignore' }); + execFileSync(gitExecutable, ['commit', '-q', '-m', 'init'], { + cwd: repo, + stdio: 'ignore', + }); + execFileSync(gitExecutable, ['update-index', flag, '--', 'hidden.ts'], { + cwd: repo, + stdio: 'ignore', + }); + fs.writeFileSync(path.join(repo, 'hidden.ts'), 'after'); + + expect(listWorkingTreeDirtyPaths(repo)).toContain('hidden.ts'); + } finally { + fs.rmSync(repo, { recursive: true, force: true }); + } + }, + ); + + it.each(['--assume-unchanged', '--skip-worktree'])( + 'omits GitNexus-managed paths hidden by git update-index %s', + async (flag) => { + const { listWorkingTreeDirtyPaths } = await import('../../src/storage/git.js'); + const repo = makeIsolatedGitRepo(); + try { + fs.writeFileSync(path.join(repo, 'README.md'), 'hi'); + fs.writeFileSync(path.join(repo, 'AGENTS.md'), 'before'); + execFileSync(gitExecutable, ['add', '--', 'README.md', 'AGENTS.md'], { + cwd: repo, + stdio: 'ignore', + }); + execFileSync(gitExecutable, ['commit', '-q', '-m', 'init'], { + cwd: repo, + stdio: 'ignore', + }); + execFileSync(gitExecutable, ['update-index', flag, '--', 'AGENTS.md'], { + cwd: repo, + stdio: 'ignore', + }); + fs.writeFileSync(path.join(repo, 'AGENTS.md'), 'after'); + + expect(listWorkingTreeDirtyPaths(repo)).toEqual([]); + } finally { + fs.rmSync(repo, { recursive: true, force: true }); + } + }, + ); + + it.skipIf(process.platform === 'win32')( + 'preserves exact unusual names hidden by index bits, including both bits', + async () => { + const { listWorkingTreeDirtyPaths } = await import('../../src/storage/git.js'); + const repo = makeIsolatedGitRepo(); + const names = ['ä.ts', 'a -> b.ts', 'tab\tname.ts', 'line\nbreak.ts']; + try { + for (const name of names) fs.writeFileSync(path.join(repo, name), 'before'); + execFileSync(gitExecutable, ['add', '--', ...names], { cwd: repo, stdio: 'ignore' }); + execFileSync(gitExecutable, ['commit', '-q', '-m', 'init'], { + cwd: repo, + stdio: 'ignore', + }); + execFileSync(gitExecutable, ['update-index', '--assume-unchanged', '--', names[0]], { + cwd: repo, + stdio: 'ignore', + }); + execFileSync(gitExecutable, ['update-index', '--skip-worktree', '--', names[1]], { + cwd: repo, + stdio: 'ignore', + }); + execFileSync(gitExecutable, ['update-index', '--assume-unchanged', '--', names[2]], { + cwd: repo, + stdio: 'ignore', + }); + execFileSync(gitExecutable, ['update-index', '--skip-worktree', '--', names[2]], { + cwd: repo, + stdio: 'ignore', + }); + execFileSync(gitExecutable, ['update-index', '--skip-worktree', '--', names[3]], { + cwd: repo, + stdio: 'ignore', + }); + for (const name of names) fs.writeFileSync(path.join(repo, name), 'after'); + + expect(listWorkingTreeDirtyPaths(repo)?.sort()).toEqual([...names].sort()); + } finally { + fs.rmSync(repo, { recursive: true, force: true }); + } + }, + ); +}); diff --git a/gitnexus/test/unit/index-content-drift.test.ts b/gitnexus/test/unit/index-content-drift.test.ts new file mode 100644 index 000000000..04b26785b --- /dev/null +++ b/gitnexus/test/unit/index-content-drift.test.ts @@ -0,0 +1,404 @@ +/** + * Unit Tests: per-file index freshness (core/index-content-drift.ts) + * + * Issue #3077: `status` answered "is the index fresh?" with a repo-wide + * `git status --porcelain` boolean, so a modified or untracked file the index + * never reads pinned the verdict to "stale" — and because `analyze` cannot + * commit or delete that file, the advice it printed could never clear it. + * + * These tests use real temporary directories rather than mocks: the whole + * point of the helper is that it reuses analyze's own scan, so the ignore + * rules and the large-file cap are exactly what the assertions are about. + */ +import { describe, it, expect, afterEach } from 'vitest'; +import path from 'path'; +import os from 'os'; +import fs from 'fs'; +import { execFileSync } from 'child_process'; + +import { detectIndexContentDrift } from '../../src/core/index-content-drift.js'; +import { walkRepositoryPaths } from '../../src/core/ingestion/filesystem-walker.js'; +import { computeFileHashes } from '../../src/storage/file-hash.js'; +import { listWorkingTreeDirtyPaths } from '../../src/storage/git.js'; +import { + GITNEXUS_MANAGED_PATH_EXCLUDES, + isGitNexusManagedPath, +} from '../../src/storage/gitnexus-managed-paths.js'; + +const gitExecutable = (() => { + if (process.platform !== 'win32') return 'git'; + try { + return ( + execFileSync('where.exe', ['git'], { encoding: 'utf8' }).split(/\r?\n/).find(Boolean) ?? 'git' + ); + } catch { + return 'git'; + } +})(); + +const isolatedTmpRoot = (() => { + const root = + process.platform === 'win32' + ? path.join(path.parse(os.tmpdir()).root, 'gitnexus-drift') + : path.join(os.tmpdir(), 'gitnexus-drift'); + fs.mkdirSync(root, { recursive: true }); + return root; +})(); + +const createdRepos: string[] = []; + +const makeRepo = (files: Record): string => { + const dir = fs.mkdtempSync(path.join(isolatedTmpRoot, 'repo-')); + createdRepos.push(dir); + for (const [rel, content] of Object.entries(files)) { + const abs = path.join(dir, rel); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, content); + } + return dir; +}; + +/** Reproduce what `analyze` records in `meta.fileHashes` for a repository. */ +const recordCoverage = async ( + repoPath: string, + walkOptions?: Parameters[2], +): Promise> => { + const scanned = await walkRepositoryPaths(repoPath, undefined, walkOptions); + const hashes = await computeFileHashes( + repoPath, + scanned.map((f) => f.path), + ); + return Object.fromEntries(hashes); +}; + +afterEach(() => { + while (createdRepos.length > 0) { + fs.rmSync(createdRepos.pop()!, { recursive: true, force: true }); + } +}); + +describe('detectIndexContentDrift', () => { + it('reports current when every covered file still matches disk', async () => { + const repo = makeRepo({ 'a.js': 'export const a = 1;\n' }); + const recorded = await recordCoverage(repo); + + const drift = await detectIndexContentDrift(repo, recorded); + + expect(drift).toEqual({ kind: 'current', coveredFileCount: Object.keys(recorded).length }); + }); + + it('stays current when a file the index does not cover is modified (#3077)', async () => { + // `.lock` is an ignored extension, so the indexer never reads this file. + // Under the old repo-wide dirty check its edit forced an unclearable + // "stale" verdict on an index that was byte-current with its own coverage. + const repo = makeRepo({ + 'a.js': 'export const a = 1;\n', + 'toolingdir/state.lock': 'before\n', + }); + const recorded = await recordCoverage(repo); + expect(Object.keys(recorded)).not.toContain('toolingdir/state.lock'); + + fs.writeFileSync(path.join(repo, 'toolingdir/state.lock'), 'after\n'); + + expect(await detectIndexContentDrift(repo, recorded)).toMatchObject({ kind: 'current' }); + }); + + it('stays current when an ignored directory changes', async () => { + const repo = makeRepo({ 'a.js': 'export const a = 1;\n' }); + const recorded = await recordCoverage(repo); + + fs.mkdirSync(path.join(repo, 'node_modules', 'left-pad'), { recursive: true }); + fs.writeFileSync( + path.join(repo, 'node_modules', 'left-pad', 'index.js'), + 'module.exports=1;\n', + ); + + expect(await detectIndexContentDrift(repo, recorded)).toMatchObject({ kind: 'current' }); + }); + + it('reports the covered file that changed', async () => { + const repo = makeRepo({ 'a.js': 'export const a = 1;\n', 'b.js': 'export const b = 2;\n' }); + const recorded = await recordCoverage(repo); + + fs.writeFileSync(path.join(repo, 'b.js'), 'export const b = 3;\n'); + + const drift = await detectIndexContentDrift(repo, recorded); + expect(drift).toMatchObject({ kind: 'drifted', changed: ['b.js'], added: [], deleted: [] }); + }); + + it('reports a new coverable file as added rather than certifying the index', async () => { + // The index is missing a file `analyze` would pick up, so "up-to-date" + // would be a false all-clear even though every recorded hash matches. + const repo = makeRepo({ 'a.js': 'export const a = 1;\n' }); + const recorded = await recordCoverage(repo); + + fs.writeFileSync(path.join(repo, 'new-source.js'), 'export const n = 1;\n'); + + expect(await detectIndexContentDrift(repo, recorded)).toMatchObject({ + kind: 'drifted', + added: ['new-source.js'], + changed: [], + }); + }); + + it('reports a removed covered file as deleted', async () => { + const repo = makeRepo({ 'a.js': 'export const a = 1;\n', 'b.js': 'export const b = 2;\n' }); + const recorded = await recordCoverage(repo); + + fs.rmSync(path.join(repo, 'b.js')); + + expect(await detectIndexContentDrift(repo, recorded)).toMatchObject({ + kind: 'drifted', + deleted: ['b.js'], + changed: [], + }); + }); + + it('clears back to current once the coverage set is re-recorded', async () => { + // The loop the issue reports: `analyze` ran, reported success, and the + // verdict did not move. Re-recording coverage must settle the verdict. + const repo = makeRepo({ 'a.js': 'export const a = 1;\n' }); + const stale = await recordCoverage(repo); + fs.writeFileSync(path.join(repo, 'notes.txt'), 'scratch\n'); + expect(await detectIndexContentDrift(repo, stale)).toMatchObject({ kind: 'drifted' }); + + const reanalyzed = await recordCoverage(repo); + + expect(await detectIndexContentDrift(repo, reanalyzed)).toMatchObject({ kind: 'current' }); + }); + + it("ignores GitNexus's own analyze output on both sides", async () => { + // analyze rewrites AGENTS.md/CLAUDE.md after recording hashes. Counting + // them made a freshly indexed repo report itself stale: absent from the + // first run's coverage, then rewritten on every run after that. + const repo = makeRepo({ 'a.js': 'export const a = 1;\n' }); + const firstRun = await recordCoverage(repo); + fs.writeFileSync(path.join(repo, 'AGENTS.md'), 'stats block\n'); + fs.writeFileSync(path.join(repo, 'CLAUDE.md'), 'stats block\n'); + + expect(await detectIndexContentDrift(repo, firstRun)).toMatchObject({ kind: 'current' }); + + const secondRun = await recordCoverage(repo); + expect(Object.keys(secondRun)).toContain('AGENTS.md'); + fs.writeFileSync(path.join(repo, 'AGENTS.md'), 'refreshed stats block\n'); + + expect(await detectIndexContentDrift(repo, secondRun)).toMatchObject({ kind: 'current' }); + }); + + it('is unmeasurable, not current, when metadata carries no file hashes', async () => { + const repo = makeRepo({ 'a.js': 'export const a = 1;\n' }); + + expect(await detectIndexContentDrift(repo, undefined)).toEqual({ + kind: 'unmeasurable', + reason: 'no-file-hashes', + }); + expect(await detectIndexContentDrift(repo, {})).toEqual({ + kind: 'unmeasurable', + reason: 'no-file-hashes', + }); + }); + + it('is unmeasurable when the repository scan throws', async () => { + const drift = await detectIndexContentDrift('/no-such-gitnexus-drift-repo', { + 'a.js': 'deadbeef', + }); + expect(drift).toEqual({ kind: 'unmeasurable', reason: 'scan-failed' }); + }); + + it('replays a recorded max-file-size so a later default cap cannot drop coverage', async () => { + // `.bin` is a hardcoded ignore; a large source file is what analyze would + // actually hash once `--max-file-size` / GITNEXUS_MAX_FILE_SIZE is raised. + const raisedCap = 1024 * 1024; + const repo = makeRepo({ 'a.js': 'export const a = 1;\n' }); + fs.writeFileSync(path.join(repo, 'payload.js'), Buffer.alloc(700 * 1024, 1)); + const recorded = await recordCoverage(repo, { maxFileSizeBytes: raisedCap, quiet: true }); + expect(Object.keys(recorded)).toContain('payload.js'); + + const withPolicy = await detectIndexContentDrift(repo, recorded, { + maxFileSizeBytes: raisedCap, + }); + expect(withPolicy).toMatchObject({ kind: 'current' }); + + // No persisted policy (indexes from before `indexCoverage`): the file is + // still on disk and hashed, so a later default cap must not call it deleted. + expect(await detectIndexContentDrift(repo, recorded)).toMatchObject({ kind: 'current' }); + }); + + it('treats a covered file that can no longer be read as changed, not current', async () => { + if (typeof process.getuid === 'function' && process.getuid() === 0) { + return; + } + const repo = makeRepo({ 'a.js': 'export const a = 1;\n' }); + const recorded = await recordCoverage(repo); + const target = path.join(repo, 'a.js'); + fs.chmodSync(target, 0); + try { + expect(await detectIndexContentDrift(repo, recorded)).toMatchObject({ + kind: 'drifted', + changed: ['a.js'], + }); + } finally { + fs.chmodSync(target, 0o644); + } + }); + + it('re-hashes a path that was dirty at index time even after Git is clean', async () => { + const repo = makeRepo({ 'a.js': 'export const a = 1;\n' }); + execFileSync(gitExecutable, ['init'], { cwd: repo }); + execFileSync(gitExecutable, ['add', '.'], { cwd: repo }); + execFileSync( + gitExecutable, + ['-c', 'user.email=t@t.test', '-c', 'user.name=t', 'commit', '-m', 'i'], + { cwd: repo }, + ); + fs.writeFileSync(path.join(repo, 'a.js'), 'export const a = 2;\n'); + const recorded = await recordCoverage(repo); + execFileSync(gitExecutable, ['checkout', '--', 'a.js'], { cwd: repo }); + + const skipped = await detectIndexContentDrift(repo, recorded, { + maxFileSizeBytes: 512 * 1024, + dirtyPaths: [], + }); + expect(skipped).toMatchObject({ kind: 'current' }); + + const restored = await detectIndexContentDrift(repo, recorded, { + maxFileSizeBytes: 512 * 1024, + dirtyPaths: ['a.js'], + }); + expect(restored).toMatchObject({ kind: 'drifted', changed: ['a.js'] }); + }); + + it.each(['--assume-unchanged', '--skip-worktree'])( + 'does not let git update-index %s hide covered-file drift', + async (flag) => { + const repo = makeRepo({ 'a.js': 'export const a = 1;\n' }); + execFileSync(gitExecutable, ['init', '-q'], { cwd: repo }); + execFileSync(gitExecutable, ['add', '--', 'a.js'], { cwd: repo }); + execFileSync( + gitExecutable, + ['-c', 'user.email=t@t.test', '-c', 'user.name=t', 'commit', '-q', '-m', 'init'], + { cwd: repo }, + ); + const recorded = await recordCoverage(repo); + execFileSync(gitExecutable, ['update-index', flag, '--', 'a.js'], { cwd: repo }); + fs.writeFileSync(path.join(repo, 'a.js'), 'export const a = 2;\n'); + const listed = listWorkingTreeDirtyPaths(repo); + expect(listed).not.toBeNull(); + expect(listed).toContain('a.js'); + + expect( + await detectIndexContentDrift(repo, recorded, { + maxFileSizeBytes: 512 * 1024, + dirtyPaths: [], + }), + ).toMatchObject({ kind: 'drifted', changed: ['a.js'] }); + }, + ); + + it('hashes the full intersection when the Git path query fails', async () => { + const repo = makeRepo({ 'a.js': 'export const a = 1;\n' }); + execFileSync(gitExecutable, ['init', '-q'], { cwd: repo }); + execFileSync(gitExecutable, ['add', '--', 'a.js'], { cwd: repo }); + execFileSync( + gitExecutable, + ['-c', 'user.email=t@t.test', '-c', 'user.name=t', 'commit', '-q', '-m', 'init'], + { cwd: repo }, + ); + const recorded = await recordCoverage(repo); + fs.writeFileSync(path.join(repo, 'a.js'), 'export const a = 2;\n'); + + const savedPath = process.env.PATH; + try { + process.env.PATH = ''; + expect(listWorkingTreeDirtyPaths(repo)).toBeNull(); + expect( + await detectIndexContentDrift(repo, recorded, { + maxFileSizeBytes: 512 * 1024, + dirtyPaths: [], + }), + ).toMatchObject({ kind: 'drifted', changed: ['a.js'] }); + } finally { + process.env.PATH = savedPath; + } + }); + + it.each(process.platform === 'win32' ? ['ä.js'] : ['ä.js', 'a -> b.js', 'line\nbreak.js'])( + 'detects drift for porcelain-sensitive filename %j', + async (fileName) => { + const repo = makeRepo({ [fileName]: 'export const a = 1;\n' }); + execFileSync(gitExecutable, ['init', '-q'], { cwd: repo }); + execFileSync(gitExecutable, ['add', '--', fileName], { cwd: repo }); + execFileSync( + gitExecutable, + ['-c', 'user.email=t@t.test', '-c', 'user.name=t', 'commit', '-q', '-m', 'init'], + { cwd: repo }, + ); + const recorded = await recordCoverage(repo); + fs.writeFileSync(path.join(repo, fileName), 'export const a = 2;\n'); + + expect( + await detectIndexContentDrift(repo, recorded, { + maxFileSizeBytes: 512 * 1024, + dirtyPaths: [], + }), + ).toMatchObject({ kind: 'drifted', changed: [fileName] }); + }, + ); +}); + +describe('isGitNexusManagedPath', () => { + it('matches managed files and whole managed trees', () => { + expect(isGitNexusManagedPath('AGENTS.md')).toBe(true); + expect(isGitNexusManagedPath('CLAUDE.md')).toBe(true); + expect(isGitNexusManagedPath('.agents/skills/gitnexus-area-auth/SKILL.md')).toBe(true); + expect(isGitNexusManagedPath('.gitnexus/meta.json')).toBe(true); + }); + + it('does not match prefix collisions or nested lookalikes', () => { + // Same boundaries the `:(exclude)` pathspecs enforce for isWorkingTreeDirty. + expect(isGitNexusManagedPath('.agentsrc')).toBe(false); + expect(isGitNexusManagedPath('.claudefoo')).toBe(false); + expect(isGitNexusManagedPath('subdir/.agents/x')).toBe(false); + expect(isGitNexusManagedPath('docs/AGENTS.md')).toBe(false); + }); + + it('emits root-anchored recursive pathspecs for every managed path', () => { + expect(GITNEXUS_MANAGED_PATH_EXCLUDES).toContain(':(exclude,glob)./AGENTS.md'); + expect(GITNEXUS_MANAGED_PATH_EXCLUDES).toContain(':(exclude,glob)./.agents'); + expect(GITNEXUS_MANAGED_PATH_EXCLUDES).toContain(':(exclude,glob)./.agents/**'); + }); +}); + +describe('walkRepositoryPaths quiet option', () => { + const savedMaxFileSize = process.env.GITNEXUS_MAX_FILE_SIZE; + const savedProgressActive = process.env.GITNEXUS_ANALYZE_PROGRESS_ACTIVE; + + afterEach(() => { + if (savedMaxFileSize === undefined) delete process.env.GITNEXUS_MAX_FILE_SIZE; + else process.env.GITNEXUS_MAX_FILE_SIZE = savedMaxFileSize; + if (savedProgressActive === undefined) delete process.env.GITNEXUS_ANALYZE_PROGRESS_ACTIVE; + else process.env.GITNEXUS_ANALYZE_PROGRESS_ACTIVE = savedProgressActive; + }); + + it('suppresses the large-file notice so read-only callers stay silent', async () => { + const repo = makeRepo({ 'big.js': `// ${'x'.repeat(4096)}\n` }); + process.env.GITNEXUS_MAX_FILE_SIZE = '1'; // 1KB cap — big.js is skipped + process.env.GITNEXUS_ANALYZE_PROGRESS_ACTIVE = '1'; // routes the notice to console.warn + + const warnings: unknown[][] = []; + const originalWarn = console.warn; + console.warn = (...args: unknown[]) => void warnings.push(args); + try { + const noisy = await walkRepositoryPaths(repo); + const noisyCount = warnings.length; + warnings.length = 0; + const quiet = await walkRepositoryPaths(repo, undefined, { quiet: true }); + + expect(noisyCount).toBeGreaterThan(0); + expect(warnings).toEqual([]); + expect(quiet).toEqual(noisy); + } finally { + console.warn = originalWarn; + } + }); +}); diff --git a/gitnexus/test/unit/list-status-branch.test.ts b/gitnexus/test/unit/list-status-branch.test.ts index f472397f8..2f0368614 100644 --- a/gitnexus/test/unit/list-status-branch.test.ts +++ b/gitnexus/test/unit/list-status-branch.test.ts @@ -66,6 +66,7 @@ vi.mock('../../src/storage/git.js', () => ({ getCurrentBranch: vi.fn().mockReturnValue('main'), getGitRoot: vi.fn((p: string) => p), isWorkingTreeDirty: vi.fn().mockReturnValue(false), + listWorkingTreeDirtyPaths: vi.fn().mockReturnValue([]), })); import { listCommand } from '../../src/cli/list.js'; diff --git a/gitnexus/test/unit/status-content-drift.test.ts b/gitnexus/test/unit/status-content-drift.test.ts new file mode 100644 index 000000000..a773f7b80 --- /dev/null +++ b/gitnexus/test/unit/status-content-drift.test.ts @@ -0,0 +1,279 @@ +/** + * Unit Tests: `status` freshness verdict from per-file drift (#3077) + * + * The reported defect was a verdict nobody could clear: any modified or + * untracked file in the working tree — including files the index never reads — + * made `status` print "stale (re-run gitnexus analyze)", and running `analyze` + * left it unchanged. These tests pin the new decision order: the per-file + * comparison decides when it can run, and the repo-wide dirty flag survives + * only as the fallback for metadata written before `fileHashes` existed. + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; + +const { runnerIdentity } = vi.hoisted(() => ({ + runnerIdentity: { + schemaVersion: 4 as const, + runtime: { + executablePath: '/usr/bin/node', + version: 'v22.0.0', + platform: 'linux', + architecture: 'x64', + modulesAbi: '127', + libc: 'glibc:2.39', + }, + cliVersion: '1.6.10', + invokedArtifact: { path: '/opt/gitnexus/dist/cli/index.js', digest: 'sha256:entry' }, + build: { + kind: 'distribution' as const, + rootPath: '/opt/gitnexus/dist', + canonicalization: 'gitnexus-analyzer-build-v2' as const, + digest: 'sha256:build', + }, + dependencyRuntime: { + manifestPath: '/opt/gitnexus/package.json', + lockfilePath: '/opt/package-lock.json', + canonicalization: 'gitnexus-analyzer-dependency-runtime-v4' as const, + packageCount: 42, + artifactCount: 12, + digest: 'sha256:dependencies', + }, + }, +})); + +vi.mock('../../src/storage/repo-manager.js', () => ({ + listRegisteredRepos: vi.fn(), + findRepo: vi.fn(), + getStoragePaths: vi.fn((repoPath: string) => ({ + storagePath: `${repoPath}/.gitnexus`, + lbugPath: `${repoPath}/.gitnexus/lbug`, + metaPath: `${repoPath}/.gitnexus/meta.json`, + })), + loadMeta: vi.fn(), + hasKuzuIndex: vi.fn().mockResolvedValue(false), +})); + +vi.mock('../../src/core/analyzer-identity.js', () => ({ + resolveAnalyzerRunnerIdentity: vi.fn(() => runnerIdentity), + analyzerRunnerIdentitiesEqual: vi.fn((indexed: unknown, current: unknown) => indexed === current), +})); + +vi.mock('../../src/storage/git.js', () => ({ + isGitRepo: vi.fn().mockReturnValue(true), + getCurrentCommit: vi.fn().mockReturnValue('headsha0'), + getCurrentBranch: vi.fn().mockReturnValue('main'), + getGitRoot: vi.fn((p: string) => p), + isWorkingTreeDirty: vi.fn().mockReturnValue(false), + listWorkingTreeDirtyPaths: vi.fn().mockReturnValue([]), +})); + +vi.mock('../../src/core/index-content-drift.js', () => ({ + detectIndexContentDrift: vi.fn(), +})); + +import { statusCommand } from '../../src/cli/status.js'; +import { setCliLanguage } from '../../src/cli/i18n/index.js'; +import { findRepo } from '../../src/storage/repo-manager.js'; +import { getCurrentCommit, isWorkingTreeDirty } from '../../src/storage/git.js'; +import { detectIndexContentDrift } from '../../src/core/index-content-drift.js'; + +let logSpy: ReturnType; +const output = () => logSpy.mock.calls.map((c) => c.join(' ')).join('\n'); + +const repoWithCoverage = { + repoPath: '/repo', + storagePath: '/repo/.gitnexus', + lbugPath: '/repo/.gitnexus/lbug', + metaPath: '/repo/.gitnexus/meta.json', + meta: { + repoPath: '/repo', + lastCommit: 'headsha0', + indexedAt: '2026-08-28T12:00:00.000Z', + branch: 'main', + runnerIdentity, + fileHashes: { 'a.js': 'sha-a' }, + scopeExtractionReceipt: 1 as const, + }, +}; + +beforeEach(() => { + vi.clearAllMocks(); + logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + (findRepo as any).mockResolvedValue(repoWithCoverage); + (getCurrentCommit as any).mockReturnValue('headsha0'); + (isWorkingTreeDirty as any).mockReturnValue(false); +}); + +afterEach(() => { + setCliLanguage(null); + logSpy.mockRestore(); +}); + +describe('status freshness from per-file drift (#3077)', () => { + it('is up-to-date when every covered file matches, despite a dirty working tree', async () => { + // The reported case: one modified file outside the index's coverage. The + // old repo-wide check called this stale and `analyze` could not clear it. + (isWorkingTreeDirty as any).mockReturnValue(true); + (detectIndexContentDrift as any).mockResolvedValue({ kind: 'current', coveredFileCount: 210 }); + + await statusCommand({ json: true }); + + expect(JSON.parse(output())).toMatchObject({ + status: 'up-to-date', + contentDrift: { status: 'current', coveredFiles: 210 }, + }); + }); + + it('reports covered-file drift as stale and names the files', async () => { + (detectIndexContentDrift as any).mockResolvedValue({ + kind: 'drifted', + changed: ['src/app.ts'], + added: [], + deleted: [], + }); + + await statusCommand(); + + const out = output(); + expect(out).not.toContain('up-to-date'); + expect(out).toContain('1 changed, 0 added, 0 deleted'); + expect(out).toContain('changed: src/app.ts'); + }); + + it('escapes control characters in drifted path names', async () => { + (detectIndexContentDrift as any).mockResolvedValue({ + kind: 'drifted', + changed: ['src/\u001b[31mevil.ts'], + added: [], + deleted: [], + }); + + await statusCommand(); + + const out = output(); + expect(out).toContain(JSON.stringify('src/\u001b[31mevil.ts')); + expect(out).not.toContain('\u001b[31m'); + }); + + it('localizes overflow category labels in zh-CN', async () => { + setCliLanguage('zh-CN'); + const changed = Array.from({ length: 12 }, (_, i) => `src/file-${i}.ts`); + (detectIndexContentDrift as any).mockResolvedValue({ + kind: 'drifted', + changed, + added: [], + deleted: [], + }); + + await statusCommand(); + + const out = output(); + expect(out).toContain('已修改: src/file-0.ts'); + expect(out).toContain('另有 2 个 已修改'); + expect(out).not.toMatch(/\bchanged\b/); + }); + + it('names a failed coverage scan in human output instead of falling back', async () => { + (detectIndexContentDrift as any).mockResolvedValue({ + kind: 'unmeasurable', + reason: 'scan-failed', + }); + + await statusCommand(); + + const out = output(); + expect(out).toContain('coverage scan failed'); + expect(out).toContain('stale'); + expect(out).not.toContain('fell back to the working-tree check'); + }); + + it('exposes drift counts and a capped sample in --json', async () => { + const changed = Array.from({ length: 25 }, (_, i) => `src/file-${i}.ts`); + (detectIndexContentDrift as any).mockResolvedValue({ + kind: 'drifted', + changed, + added: [], + deleted: [], + }); + + await statusCommand({ json: true }); + + const parsed = JSON.parse(output()); + expect(parsed.status).toBe('stale'); + expect(parsed.contentDrift.counts).toEqual({ changed: 25, added: 0, deleted: 0 }); + expect(parsed.contentDrift.changed).toHaveLength(10); + expect(parsed.contentDrift.truncated).toEqual({ + changed: true, + added: false, + deleted: false, + }); + }); + + it('falls back to the working-tree check when coverage cannot be compared', async () => { + (detectIndexContentDrift as any).mockResolvedValue({ + kind: 'unmeasurable', + reason: 'no-file-hashes', + }); + (isWorkingTreeDirty as any).mockReturnValue(true); + + await statusCommand({ json: true }); + + expect(JSON.parse(output())).toMatchObject({ + status: 'stale', + contentDrift: { status: 'unmeasurable', reason: 'no-file-hashes' }, + }); + }); + + it('is stale when coverage cannot be compared because the scan failed', async () => { + (detectIndexContentDrift as any).mockResolvedValue({ + kind: 'unmeasurable', + reason: 'scan-failed', + }); + + await statusCommand({ json: true }); + + expect(JSON.parse(output())).toMatchObject({ + status: 'stale', + contentDrift: { status: 'unmeasurable', reason: 'scan-failed' }, + }); + }); + + it('is up-to-date on a clean tree when hashes are missing (legacy metadata)', async () => { + (detectIndexContentDrift as any).mockResolvedValue({ + kind: 'unmeasurable', + reason: 'no-file-hashes', + }); + + await statusCommand({ json: true }); + + expect(JSON.parse(output())).toMatchObject({ + status: 'up-to-date', + contentDrift: { status: 'unmeasurable', reason: 'no-file-hashes' }, + }); + }); + + it('skips the scan when the index is already stale on metadata alone', async () => { + // A moved HEAD is decided without paying for a repository-wide hash pass. + (getCurrentCommit as any).mockReturnValue('othersha'); + + await statusCommand({ json: true }); + + expect(detectIndexContentDrift).not.toHaveBeenCalled(); + expect(JSON.parse(output())).toMatchObject({ + status: 'stale', + contentDrift: { status: 'not-checked' }, + }); + }); + + it('replays persisted indexCoverage into the drift check', async () => { + const coverage = { maxFileSizeBytes: 1024 * 1024, dirtyPaths: ['a.js'] }; + (findRepo as any).mockResolvedValue({ + ...repoWithCoverage, + meta: { ...repoWithCoverage.meta, indexCoverage: coverage }, + }); + (detectIndexContentDrift as any).mockResolvedValue({ kind: 'current', coveredFileCount: 1 }); + + await statusCommand({ json: true }); + + expect(detectIndexContentDrift).toHaveBeenCalledWith('/repo', { 'a.js': 'sha-a' }, coverage); + }); +}); From 5bad2d8b0b699a0cbb2ff08db370285f1ac5b79a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=AE=80=E5=BE=8B=E7=BA=AF?= Date: Sun, 30 Aug 2026 05:07:43 +0800 Subject: [PATCH 2/3] fix(mcp): resolve omitted repo from cwd (#3085) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(mcp): resolve omitted repo from cwd * test(mcp): cover cwd repository routing gaps * fix(mcp): harden cwd repository routing * docs(mcp): clarify cwd repository boundary * fix(mcp): preserve resolver compatibility * fix(mcp): align restricted repository routing --------- Co-authored-by: Gergő Magyar --- GUARDRAILS.md | 4 +- README.md | 4 +- gitnexus/README.md | 2 +- gitnexus/scripts/cross-platform-tests.ts | 3 + gitnexus/src/mcp/local/local-backend.ts | 121 ++++-- gitnexus/src/mcp/repository-policy.ts | 64 +++- gitnexus/src/mcp/resources.ts | 5 +- gitnexus/src/mcp/server.ts | 12 +- gitnexus/src/mcp/tools.ts | 49 ++- gitnexus/test/unit/calltool-dispatch.test.ts | 362 +++++++++++++++++- .../test/unit/mcp-repository-policy.test.ts | 24 ++ gitnexus/test/unit/resources.test.ts | 5 +- gitnexus/test/unit/server.test.ts | 43 ++- gitnexus/test/unit/tools.test.ts | 19 + 14 files changed, 639 insertions(+), 78 deletions(-) diff --git a/GUARDRAILS.md b/GUARDRAILS.md index 72e9c1e59..f34cf79d5 100644 --- a/GUARDRAILS.md +++ b/GUARDRAILS.md @@ -73,8 +73,8 @@ Format: **Trigger → Instruction → Reason**. Append new Signs when the same m ### Wrong repo in multi-repo setups - **Trigger:** Query/impact results belong to another project. -- **Do:** Call `list_repos`, then pass `repo` on subsequent tools. -- **Why:** Default target is ambiguous when multiple repos are registered. +- **Do:** Confirm an MCP default is configured or the GitNexus process was launched inside the intended registered path without crossing into an unindexed nested Git checkout. Otherwise call `list_repos`, then pass `repo` on subsequent tools; pass it for mutating tools when multiple repos are registered and no MCP default exists. +- **Why:** Read-only tools derive their default from MCP configuration or a process cwd that stays within one registered Git boundary. Outside those paths the target remains ambiguous, and mutating tools stay explicit unless configuration supplies the target. ### LadybugDB lock / "database busy" diff --git a/README.md b/README.md index 5e0536a48..173cd1d42 100644 --- a/README.md +++ b/README.md @@ -179,7 +179,7 @@ flowchart TB | `group_list` | List configured repository groups | | `group_sync` | Rebuild a group's Contract Registry and cross-repo links | -> Per-repo tools take an optional `repo` parameter (omit it when only one repo is indexed) and an optional `branch` for indexes pinned with `gitnexus analyze --branch`. Omitting `branch` queries the workspace index, which follows your checked-out working tree — switching branches and re-running `gitnexus analyze` updates it incrementally. `explain` and `pdg_query` need an index built with `gitnexus analyze --pdg`. +> Per-repo read-only tools take an optional `repo` parameter. Omit it when only one repo is indexed, an MCP default is configured, or the GitNexus process cwd is inside a registered path without crossing into an unindexed nested Git checkout; otherwise pass it explicitly. Mutating tools require `repo` when multiple repos are indexed and no MCP default exists. Per-repo tools also take an optional `branch` for indexes pinned with `gitnexus analyze --branch`. Omitting `branch` queries the workspace index, which follows your checked-out working tree — switching branches and re-running `gitnexus analyze` updates it incrementally. `explain` and `pdg_query` need an index built with `gitnexus analyze --pdg`. ### Resources for instant context @@ -612,7 +612,7 @@ GitNexus builds a complete knowledge graph of your codebase through a multi-phas GitNexus uses a **global registry** so one MCP server can serve multiple indexed repos. No per-project MCP config needed — set it up once and it works everywhere. -Each `gitnexus analyze` stores the index in `.gitnexus/` inside the repo (portable, gitignored) and registers a pointer in `~/.gitnexus/registry.json`. When an AI agent starts, the MCP server reads the registry and can serve any indexed repo. LadybugDB connections are opened lazily on first query and evicted after 5 minutes of inactivity (max 5 concurrent). If only one repo is indexed, the `repo` parameter is optional on all tools — agents don't need to change anything. +Each `gitnexus analyze` stores the index in `.gitnexus/` inside the repo (portable, gitignored) and registers a pointer in `~/.gitnexus/registry.json`. When an AI agent starts, the MCP server reads the registry and can serve any indexed repo. LadybugDB connections are opened lazily on first query and evicted after 5 minutes of inactivity (max 5 concurrent). Read-only tools can omit `repo` when only one repo is indexed, an MCP default is configured, or the GitNexus process cwd is inside a registered path without crossing into an unindexed nested Git checkout. Outside those paths—and for mutating tools with multiple indexed repos and no MCP default—pass `repo` explicitly.
Architecture diagram diff --git a/gitnexus/README.md b/gitnexus/README.md index 93b2dd354..20ea65ef5 100644 --- a/gitnexus/README.md +++ b/gitnexus/README.md @@ -204,7 +204,7 @@ Your AI agent gets **17 tools** (15 per-repo + 2 group) automatically: | `group_list` | List configured repository groups | | `group_sync` | Rebuild a group's Contract Registry and cross-repo links | -> With one indexed repo, the `repo` param is optional. With multiple, specify which: `query({search_query: "auth", repo: "my-app"})`. Per-repo tools also take an optional `branch` for indexes pinned with `gitnexus analyze --branch`; omitting it queries the workspace index, which follows your checked-out working tree. `explain` and `pdg_query` need an index built with `gitnexus analyze --pdg`. +> Read-only tools can omit `repo` when one repo is indexed, an MCP default is configured, or the GitNexus process cwd is inside a registered path without crossing into an unindexed nested Git checkout. Otherwise—and for mutating tools with multiple indexed repos and no MCP default—specify it explicitly: `query({search_query: "auth", repo: "my-app"})`. Per-repo tools also take an optional `branch` for indexes pinned with `gitnexus analyze --branch`; omitting it queries the workspace index, which follows your checked-out working tree. `explain` and `pdg_query` need an index built with `gitnexus analyze --pdg`. ## MCP Resources diff --git a/gitnexus/scripts/cross-platform-tests.ts b/gitnexus/scripts/cross-platform-tests.ts index 3f96c5c40..44e45fbbb 100644 --- a/gitnexus/scripts/cross-platform-tests.ts +++ b/gitnexus/scripts/cross-platform-tests.ts @@ -114,6 +114,9 @@ const PLATFORM_LOGIC = [ // POSIX and Windows — the fail-closed path-claim semantics must hold on the // real windows-latest path implementation (#2419/#2420). 'test/unit/server-api-repo-resolution.test.ts', + // #3073: cwd-based repository selection canonicalizes real paths, compares + // platform separators/case, and rejects nested Git-boundary fallthrough. + 'test/unit/calltool-dispatch.test.ts', // The index write-lock (#2658) selects its backend by process.platform — the // OS socket lock (Windows named pipe / Linux abstract socket) vs the file // fallback — and its socket-backend describe block is gated to linux/win32. diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 544f64c12..3e2f5e8f7 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -38,6 +38,7 @@ import { parseDiffHunks, coalesceHunksByPath, hunksOverlapRange, + findGitRootByDotGit, getCanonicalRepoRoot, getGitRoot, type FileDiff, @@ -1716,21 +1717,61 @@ export class LocalBackend { * - If only 1 repo, use it * - If 0 or multiple without param, throw with helpful message * - * On a miss, re-reads the registry once in case a new repo was indexed - * while the MCP server was running. + * Re-reads the registry before an omitted implicit target or after an + * explicit miss, so long-running servers see newly indexed repositories. */ async resolveRepo(repoParam?: string, branch?: string): Promise { - let refreshedAfterAmbiguity = false; + return this.selectToolRepository(repoParam, branch); + } + + /** + * Internal resolver variant for CLI/MCP tool routing and discovery. + * - If repoParam is given, match by name or path + * - If only 1 repo, use it + * - If multiple repos exist and repoParam is omitted, callers may opt in to + * the registered repo containing process.cwd() + * - If 0 repos exist, or cwd cannot disambiguate multiple repos, throw + * + * Omitted-repo resolution re-reads the registry before accepting any + * implicit target, including a cached singleton. A caller that just obtained + * a fresh registry snapshot may disable that refresh explicitly. + */ + async selectToolRepository( + repoParam?: string, + branch?: string, + options: { allowCwdDefault?: boolean; refreshRegistry?: boolean } = {}, + ): Promise { + const allowCwdDefault = options.allowCwdDefault === true; + const mayRefresh = options.refreshRegistry !== false; + let refreshed = false; + + // A cached singleton is also an implicit choice: another process may have + // registered a second repo since init, which must not let a repo-less + // mutating call bypass the multi-repo ambiguity guard. + if (!repoParam && mayRefresh) { + await this.refreshRepos(); + refreshed = true; + } + let result: RepoHandle | null; try { - result = this.resolveRepoFromCache(repoParam); + result = this.resolveRepoFromCache(repoParam, allowCwdDefault); } catch (err) { if (!(err instanceof RegistryAmbiguousTargetError)) throw err; + if (!mayRefresh || refreshed) throw err; // Stale in-memory duplicate siblings can linger after unregister; refresh // once before re-throwing so a resolved registry can disambiguate (#1658). await this.refreshRepos(); - refreshedAfterAmbiguity = true; - result = this.resolveRepoFromCache(repoParam); + refreshed = true; + result = this.resolveRepoFromCache(repoParam, allowCwdDefault); + } + + // Explicit misses retain the existing one-refresh retry. Omitted targets + // already refreshed above unless a same-snapshot caller opted out. + if (!result && mayRefresh && !refreshed) { + await this.refreshRepos(); + refreshed = true; + result = this.resolveRepoFromCache(repoParam, allowCwdDefault); } if (result) { @@ -1746,16 +1787,6 @@ export class LocalBackend { return this.applyBranchScope(result, branch); } - // Miss — refresh registry and try once more (skip if already refreshed above) - if (!refreshedAfterAmbiguity) { - await this.refreshRepos(); - } - const retried = this.resolveRepoFromCache(repoParam); - if (retried) { - this.maybeWarnSiblingDrift(retried).catch(() => {}); - return this.applyBranchScope(retried, branch); - } - // Still no match — throw with helpful message if (this.repos.size === 0) { throw new Error('No indexed repositories. Run: gitnexus analyze'); @@ -1905,7 +1936,7 @@ export class LocalBackend { * Throws {@link RegistryAmbiguousTargetError} when `repoParam` matches * multiple handles by name and cwd cannot disambiguate (#1658). */ - private resolveRepoFromCache(repoParam?: string): RepoHandle | null { + private resolveRepoFromCache(repoParam?: string, allowCwdDefault = false): RepoHandle | null { if (this.repos.size === 0) return null; if (repoParam) { @@ -1938,6 +1969,9 @@ export class LocalBackend { ); if (nameMatches.length === 1) return nameMatches[0]; if (nameMatches.length > 1) { + // Explicit duplicate aliases retain the legacy fail-closed contract: + // only an exact cwd Git-root match may disambiguate them. Deepest path + // containment is reserved for an omitted read-only repo (#3073). const cwdPick = this.pickRepoHandleForCwd(nameMatches); if (cwdPick) return cwdPick; throw new RegistryAmbiguousTargetError( @@ -1969,26 +2003,50 @@ export class LocalBackend { return this.repos.values().next().value!; } + if (allowCwdDefault) { + const cwdPick = this.pickRepoHandleForCwd([...this.repos.values()], true); + if (cwdPick) return cwdPick; + } + return null; // Multiple repos, no param — ambiguous } /** - * Prefer the indexed repo whose path matches the git root of process.cwd(). + * Match process.cwd() against indexed repositories. * - * In MCP stdio server mode, `process.cwd()` is the server's launch directory, - * not the agent client's cwd. If the server was started from an unrelated - * directory, `getGitRoot` returns null and duplicate-name resolution throws - * {@link RegistryAmbiguousTargetError} — callers should pass an absolute path. + * Explicit duplicate aliases use exact Git-root matching only. Omitted + * read-only calls opt into deepest containing-path selection. In that mode a + * candidate must not sit above cwd's Git root, so an unindexed nested checkout + * cannot fall through to an indexed ancestor. The `.git` ancestor fallback + * preserves that boundary when the git executable is unavailable. */ - private pickRepoHandleForCwd(candidates: RepoHandle[]): RepoHandle | null { - const cwdRoot = getGitRoot(process.cwd()); - if (!cwdRoot) return null; - const canonicalCwd = canonicalizePath(cwdRoot); + private pickRepoHandleForCwd( + candidates: RepoHandle[], + allowContaining = false, + ): RepoHandle | null { + const cwd = process.cwd(); + const normalize = (value: string): string => { + const canonical = canonicalizePath(value); + return process.platform === 'win32' ? canonical.toLowerCase() : canonical; + }; + const isSameOrDescendant = (parent: string, child: string): boolean => + child === parent || + child.startsWith(parent.endsWith(path.sep) ? parent : `${parent}${path.sep}`); + const canonicalCwd = normalize(cwd); + const cwdRoot = getGitRoot(cwd) ?? findGitRootByDotGit(cwd); + const canonicalRoot = cwdRoot ? normalize(cwdRoot) : null; + if (allowContaining) { + const containing = candidates + .map((handle) => ({ handle, repoPath: normalize(handle.repoPath) })) + .filter(({ repoPath }) => isSameOrDescendant(repoPath, canonicalCwd)) + .filter(({ repoPath }) => !canonicalRoot || isSameOrDescendant(canonicalRoot, repoPath)) + .sort((a, b) => b.repoPath.length - a.repoPath.length); + if (containing.length > 0) return containing[0].handle; + } + + if (!canonicalRoot) return null; const cwdMatches = candidates.filter((handle) => { - const stored = canonicalizePath(handle.repoPath); - return process.platform === 'win32' - ? stored.toLowerCase() === canonicalCwd.toLowerCase() - : stored === canonicalCwd; + return normalize(handle.repoPath) === canonicalRoot; }); return cwdMatches.length === 1 ? cwdMatches[0] : null; } @@ -2415,9 +2473,10 @@ export class LocalBackend { // Resolve repo from optional param (re-reads registry on miss). An optional // `branch` param scopes the resolved handle to that branch's index (#2106). - const repo = await this.resolveRepo( + const repo = await this.selectToolRepository( p.repo as string | undefined, p.branch as string | undefined, + { allowCwdDefault: method !== 'rename' }, ); switch (method) { diff --git a/gitnexus/src/mcp/repository-policy.ts b/gitnexus/src/mcp/repository-policy.ts index 2204334f1..a309ec0cf 100644 --- a/gitnexus/src/mcp/repository-policy.ts +++ b/gitnexus/src/mcp/repository-policy.ts @@ -179,9 +179,44 @@ export class McpRepositoryPolicy { }); } - async requiresExplicitRepo(backend: LocalBackend): Promise { - if (this.defaultRepo) return false; - return (await this.listAllowedRepos(backend)).length > 1; + async toolSchemaRepoRequirements(backend: LocalBackend): Promise<{ + readOnlyRequiresRepo: boolean; + mutatingRequiresRepo: boolean; + }> { + if (this.defaultRepo) { + return { readOnlyRequiresRepo: false, mutatingRequiresRepo: false }; + } + + // Runtime selection is based on the configured allowlist, not on which + // entries happen to remain visible in a later registry refresh. Keep the + // advertised schema aligned with repoForArgs() when that listing shrinks. + if (this.restricted) { + const requiresRepo = this.allowed.length > 1; + return { + readOnlyRequiresRepo: requiresRepo, + mutatingRequiresRepo: requiresRepo, + }; + } + + // One fresh listing supplies both schema decisions. Besides keeping the + // advertised contract internally consistent, this avoids doing two full + // per-repo staleness fan-outs for every tools/list request. + const visibleRepos = await this.listAllowedRepos(backend); + if (visibleRepos.length <= 1) { + return { readOnlyRequiresRepo: false, mutatingRequiresRepo: false }; + } + try { + // listAllowedRepos() refreshed this backend immediately above. Resolve + // against that exact cache snapshot instead of racing another registry + // read; only read-only schemas may advertise the cwd-derived default. + await backend.selectToolRepository(undefined, undefined, { + allowCwdDefault: true, + refreshRegistry: false, + }); + return { readOnlyRequiresRepo: false, mutatingRequiresRepo: true }; + } catch { + return { readOnlyRequiresRepo: true, mutatingRequiresRepo: true }; + } } private async listReposPage( @@ -241,6 +276,22 @@ export class McpRepositoryPolicy { return backend.resolveRepo(selected?.path, branch); } + private async selectToolRepository( + backend: LocalBackend, + repo?: string, + branch?: string, + options?: Parameters[2], + ): Promise>> { + if (!this.configured) return backend.selectToolRepository(repo, branch, options); + if (!this.restricted) { + return backend.selectToolRepository(repo ?? this.defaultRepo?.path, branch, options); + } + const selected = this.repoForArgs(repo === undefined ? undefined : { repo }); + // Restricted policies never allow cwd to select outside the configured + // set; once policy supplies an explicit path, the public resolver is enough. + return backend.resolveRepo(selected?.path, branch); + } + assertResourceUri(uri: string): void { if (!this.restricted) return; let parsed: URL; @@ -307,6 +358,13 @@ export class McpRepositoryPolicy { if (property === 'resolveRepo') { return (repo?: string, branch?: string) => policy.resolveRepo(target, repo, branch); } + if (property === 'selectToolRepository') { + return ( + repo?: string, + branch?: string, + options?: Parameters[2], + ) => policy.selectToolRepository(target, repo, branch, options); + } if (property === 'getContext' && policy.restricted) { return (repoId?: string) => { if (!repoId || !policy.uniqueAllowedContextNames.has(repoId.toLowerCase())) return null; diff --git a/gitnexus/src/mcp/resources.ts b/gitnexus/src/mcp/resources.ts index 3411878bd..3e6c59daf 100644 --- a/gitnexus/src/mcp/resources.ts +++ b/gitnexus/src/mcp/resources.ts @@ -313,7 +313,10 @@ async function getReposResource(backend: LocalBackend): Promise { if (repos.length > 1) { lines.push(''); - lines.push('# Multiple repos indexed. Use repo parameter in tool calls:'); + lines.push( + '# Multiple repos indexed. Read-only tools may omit repo when an MCP default is configured or GitNexus process.cwd() is inside one listed path without crossing an unindexed nested Git checkout.', + ); + lines.push('# Otherwise—and for mutating tools without an MCP default—pass repo explicitly:'); lines.push(`# query({search_query: "auth", repo: "${repos[0].name}"})`); } diff --git a/gitnexus/src/mcp/server.ts b/gitnexus/src/mcp/server.ts index 8370e5c4a..a3df48b4c 100644 --- a/gitnexus/src/mcp/server.ts +++ b/gitnexus/src/mcp/server.ts @@ -185,11 +185,12 @@ export function createMCPServer( } }); - // With multiple visible repositories and no process-wide default, make the - // routing requirement machine-readable. Agents then supply `repo` before the - // call instead of discovering the ambiguity through a failed tool response. + // Make the effective routing contract machine-readable. Read-only tools may + // use a cwd-derived default; mutating rename remains explicit unless policy + // supplies a single/default repository. server.setRequestHandler(ListToolsRequestSchema, async () => { - const requireRepo = await repositoryPolicy.requiresExplicitRepo(backend); + const { readOnlyRequiresRepo, mutatingRequiresRepo } = + await repositoryPolicy.toolSchemaRepoRequirements(backend); return { tools: GITNEXUS_TOOLS.filter( (tool) => @@ -201,7 +202,8 @@ export function createMCPServer( name: tool.name, description: tool.description, inputSchema: - requireRepo && REPO_SCOPED_TOOLS.has(tool.name) + (tool.name === 'rename' ? mutatingRequiresRepo : readOnlyRequiresRepo) && + REPO_SCOPED_TOOLS.has(tool.name) ? { ...tool.inputSchema, required: [...new Set([...tool.inputSchema.required, 'repo'])], diff --git a/gitnexus/src/mcp/tools.ts b/gitnexus/src/mcp/tools.ts index 4244a95bf..e5f7e8d43 100644 --- a/gitnexus/src/mcp/tools.ts +++ b/gitnexus/src/mcp/tools.ts @@ -82,6 +82,11 @@ export const PDG_QUERY_MAX_LIMIT = 200; // PDG direct backend callers also enforce it before running traversal. export const IMPACT_MAX_DEPTH = 32; +const CWD_AWARE_REPO_OMISSION = + 'Omit when only one repo is indexed, an MCP default is configured, or the GitNexus process cwd is inside a registered path without crossing an unindexed nested Git checkout; otherwise specify it explicitly.'; +const MUTATING_REPO_OMISSION = + 'Omit only when one repo is indexed or an MCP default is configured; otherwise mutating tools require an explicit repo.'; + export const GITNEXUS_TOOLS: ToolDefinition[] = [ { name: 'list_repos', @@ -94,8 +99,10 @@ PAGINATION: Results are paginated so a large registry is not truncated by MCP/LL WHEN TO USE: First step when multiple repos are indexed, or to discover available repos. AFTER THIS: READ gitnexus://repo/{name}/context for the repo you want to work with. -When multiple repos are indexed, you MUST specify the "repo" parameter -on other tools (query, context, impact, etc.) to target the correct one.`, +When multiple repos are indexed, repo-scoped read-only tools use the configured +MCP default or the registered path containing the GitNexus process cwd, unless +cwd has crossed into an unindexed nested Git checkout. If neither applies, +specify the "repo" parameter explicitly.`, annotations: READ_ONLY_TOOL_ANNOTATIONS, inputSchema: { type: 'object', @@ -184,8 +191,7 @@ SERVICE: optional monorepo path prefix (POSIX-style, case-sensitive segments). W }, repo: { type: 'string', - description: - 'Indexed repository name or path, or group mode "@" / "@/" (member path keys from group.yaml). Omit when only one indexed repo exists.', + description: `Indexed repository name or path, or group mode "@" / "@/" (member path keys from group.yaml). ${CWD_AWARE_REPO_OMISSION}`, }, service: { type: 'string', @@ -266,7 +272,7 @@ TIPS: }, repo: { type: 'string', - description: 'Repository name or path. Omit if only one repo is indexed.', + description: `Repository name or path. ${CWD_AWARE_REPO_OMISSION}`, }, }, required: ['statement'], @@ -331,8 +337,7 @@ SERVICE: optional monorepo path prefix (case-sensitive path segments). When "rep }, repo: { type: 'string', - description: - 'Indexed repository name or path, or group mode "@" / "@/". Omit if only one repo is indexed.', + description: `Indexed repository name or path, or group mode "@" / "@/". ${CWD_AWARE_REPO_OMISSION}`, }, service: { type: 'string', @@ -378,7 +383,7 @@ Returns: changed symbols, affected processes, and a risk summary. }, repo: { type: 'string', - description: 'Repository name or path. Omit if only one repo is indexed.', + description: `Repository name or path. ${CWD_AWARE_REPO_OMISSION}`, }, }, required: [], @@ -417,7 +422,7 @@ A graph too large to analyze at all returns \`{ error, truncated: true }\` with }, repo: { type: 'string', - description: 'Repository name or path. Omit if only one repo is indexed.', + description: `Repository name or path. ${CWD_AWARE_REPO_OMISSION}`, }, }, required: [], @@ -454,7 +459,7 @@ Handles disambiguation via context()'s payload verbatim: an ambiguous symbol_nam }, repo: { type: 'string', - description: 'Repository name or path. Omit if only one repo is indexed.', + description: `Repository name or path. ${MUTATING_REPO_OMISSION}`, }, }, required: ['new_name'], @@ -593,8 +598,7 @@ SERVICE: optional monorepo path prefix (case-sensitive path segments). When "rep }, repo: { type: 'string', - description: - 'Indexed repository name or path, or group mode "@" / "@/". Omit if only one repo is indexed.', + description: `Indexed repository name or path, or group mode "@" / "@/". ${CWD_AWARE_REPO_OMISSION}`, }, service: { type: 'string', @@ -690,7 +694,7 @@ Findings are deliberately NOT part of impact()'s traversal or the web schema — }, repo: { type: 'string', - description: 'Repository name or path. Omit if only one repo is indexed.', + description: `Repository name or path. ${CWD_AWARE_REPO_OMISSION}`, }, }, required: [], @@ -742,7 +746,7 @@ CONTRACT CAVEATS: }, repo: { type: 'string', - description: 'Repository name or path. Omit if only one repo is indexed.', + description: `Repository name or path. ${CWD_AWARE_REPO_OMISSION}`, }, }, required: ['mode', 'target'], @@ -766,7 +770,7 @@ Returns: route nodes with their handlers, middleware wrapper chains (e.g., withA }, repo: { type: 'string', - description: 'Repository name or path. Omit if only one repo is indexed.', + description: `Repository name or path. ${CWD_AWARE_REPO_OMISSION}`, }, }, required: [], @@ -784,7 +788,10 @@ Returns: tool nodes with their handler files and descriptions.`, type: 'object', properties: { tool: { type: 'string', description: 'Filter by tool name. Omit for all tools.' }, - repo: { type: 'string', description: 'Repository name or path.' }, + repo: { + type: 'string', + description: `Repository name or path. ${CWD_AWARE_REPO_OMISSION}`, + }, }, required: [], }, @@ -807,7 +814,7 @@ Returns routes that have both detected response keys AND consumers. Shows top-le }, repo: { type: 'string', - description: 'Repository name or path. Omit if only one repo is indexed.', + description: `Repository name or path. ${CWD_AWARE_REPO_OMISSION}`, }, }, required: [], @@ -833,7 +840,10 @@ Response shape is keyed on how many routes match, not on the data: exactly one m description: 'Optional HTTP verb — GET, POST, PUT, PATCH, DELETE, etc. — to narrow a multi-verb route or file lookup to a single method. Returns an error if no matched route uses that verb.', }, - repo: { type: 'string', description: 'Repository name or path.' }, + repo: { + type: 'string', + description: `Repository name or path. ${CWD_AWARE_REPO_OMISSION}`, + }, }, required: [], }, @@ -948,8 +958,7 @@ DESTINATION TRACE (cross-repo): for an "@groupName" trace, OMIT to/to_uid/to_fil }, repo: { type: 'string', - description: - 'Repository name or path, or "@groupName" / "@groupName/memberPath" for a cross-repo trace over a group. Omit if only one repo is indexed.', + description: `Repository name or path, or "@groupName" / "@groupName/memberPath" for a cross-repo trace over a group. ${CWD_AWARE_REPO_OMISSION}`, }, }, required: [], diff --git a/gitnexus/test/unit/calltool-dispatch.test.ts b/gitnexus/test/unit/calltool-dispatch.test.ts index f5693f090..a98b25730 100644 --- a/gitnexus/test/unit/calltool-dispatch.test.ts +++ b/gitnexus/test/unit/calltool-dispatch.test.ts @@ -420,7 +420,7 @@ describe('LocalBackend.callTool', () => { ['impact', { name: 'validate', symbol: 'login', direction: 'upstream' }], ['context', { name: 'validate', file_path: 'src/auth.ts', file: 'src/login.ts' }], ])('rejects conflicting %s aliases before repository resolution', async (method, params) => { - const resolveSpy = vi.spyOn(backend, 'resolveRepo'); + const resolveSpy = vi.spyOn(backend, 'selectToolRepository'); const result = await backend.callTool(method, params); @@ -434,7 +434,7 @@ describe('LocalBackend.callTool', () => { ['context', { name: 'validate', file: ' ' }], ['context', { name: 'validate', file: null }], ])('rejects invalid %s aliases before repository resolution', async (method, params) => { - const resolveSpy = vi.spyOn(backend, 'resolveRepo'); + const resolveSpy = vi.spyOn(backend, 'selectToolRepository'); const result = await backend.callTool(method, params); @@ -443,7 +443,7 @@ describe('LocalBackend.callTool', () => { }); it('rejects a missing impact target before repository resolution', async () => { - const resolveSpy = vi.spyOn(backend, 'resolveRepo'); + const resolveSpy = vi.spyOn(backend, 'selectToolRepository'); const result = await backend.callTool('impact', { direction: 'upstream' }); @@ -3381,12 +3381,351 @@ describe('LocalBackend.resolveRepo', () => { ); }); - it('throws for ambiguous repos without param', async () => { - setupMultipleRepos(); - await backend.init(); - await expect(backend.callTool('query', { query: 'test' })).rejects.toThrow( - 'Multiple repositories indexed', - ); + it('throws for ambiguous repos when cwd is outside every indexed path', async () => { + const cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue('/tmp/test-project-sibling'); + + try { + setupMultipleRepos(); + await backend.init(); + await expect(backend.callTool('query', { query: 'test' })).rejects.toThrow( + 'Multiple repositories indexed', + ); + } finally { + cwdSpy.mockRestore(); + } + }); + + it('defaults to the deepest indexed repo containing cwd (#3073)', async () => { + const outerDir = mkdtempSync(path.join(os.tmpdir(), 'gnx-cwd-outer-')); + const nestedDir = path.join(outerDir, 'packages', 'nested'); + const cwdDir = path.join(nestedDir, 'src'); + mkdirSync(cwdDir, { recursive: true }); + duplicateFixtureDirs.push(outerDir); + (listRegisteredRepos as any).mockResolvedValue([ + { + ...MOCK_REPO_ENTRY, + name: 'outer', + path: outerDir, + storagePath: path.join(outerDir, '.gitnexus'), + }, + { + ...MOCK_REPO_ENTRY, + name: 'nested', + path: nestedDir, + storagePath: path.join(nestedDir, '.gitnexus'), + }, + ]); + const cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(cwdDir); + + try { + await backend.init(); + const resolved = await backend.selectToolRepository(undefined, undefined, { + allowCwdDefault: true, + }); + expect(resolved.repoPath).toBe(nestedDir); + const explicit = await backend.resolveRepo('outer'); + expect(explicit.repoPath).toBe(outerDir); + } finally { + cwdSpy.mockRestore(); + } + }); + + it('refreshes before accepting a cached cwd ancestor (#3073)', async () => { + const outerDir = mkdtempSync(path.join(os.tmpdir(), 'gnx-cwd-stale-outer-')); + const otherDir = mkdtempSync(path.join(os.tmpdir(), 'gnx-cwd-stale-other-')); + const nestedDir = path.join(outerDir, 'vendor', 'nested'); + const cwdDir = path.join(nestedDir, 'src'); + mkdirSync(cwdDir, { recursive: true }); + duplicateFixtureDirs.push(outerDir, otherDir); + + const outerEntry = { + ...MOCK_REPO_ENTRY, + name: 'outer', + path: outerDir, + storagePath: path.join(outerDir, '.gitnexus'), + }; + const nestedEntry = { + ...MOCK_REPO_ENTRY, + name: 'nested', + path: nestedDir, + storagePath: path.join(nestedDir, '.gitnexus'), + }; + const otherEntry = { + ...MOCK_REPO_ENTRY, + name: 'other', + path: otherDir, + storagePath: path.join(otherDir, '.gitnexus'), + }; + (listRegisteredRepos as any) + .mockResolvedValueOnce([outerEntry, otherEntry]) + .mockResolvedValue([outerEntry, nestedEntry, otherEntry]); + (getGitRoot as any).mockImplementation((value: string) => { + const resolved = path.resolve(value); + if (resolved === nestedDir || resolved.startsWith(`${nestedDir}${path.sep}`)) { + return nestedDir; + } + if (resolved === outerDir || resolved.startsWith(`${outerDir}${path.sep}`)) { + return outerDir; + } + if (resolved === otherDir || resolved.startsWith(`${otherDir}${path.sep}`)) { + return otherDir; + } + return null; + }); + const cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(cwdDir); + + try { + await backend.init(); + const resolved = await backend.selectToolRepository(undefined, undefined, { + allowCwdDefault: true, + }); + expect(resolved.repoPath).toBe(nestedDir); + expect(listRegisteredRepos).toHaveBeenCalledTimes(2); + } finally { + cwdSpy.mockRestore(); + } + }); + + it('refreshes a cached singleton before repo-less read dispatch (#3073)', async () => { + const outerDir = mkdtempSync(path.join(os.tmpdir(), 'gnx-cwd-singleton-outer-')); + const nestedDir = path.join(outerDir, 'packages', 'nested'); + const cwdDir = path.join(nestedDir, 'src'); + mkdirSync(cwdDir, { recursive: true }); + duplicateFixtureDirs.push(outerDir); + + const outerEntry = { + ...MOCK_REPO_ENTRY, + name: 'outer', + path: outerDir, + storagePath: path.join(outerDir, '.gitnexus'), + }; + const nestedEntry = { + ...MOCK_REPO_ENTRY, + name: 'nested', + path: nestedDir, + storagePath: path.join(nestedDir, '.gitnexus'), + }; + (listRegisteredRepos as any) + .mockResolvedValueOnce([outerEntry]) + .mockResolvedValue([outerEntry, nestedEntry]); + (getGitRoot as any).mockReturnValue(outerDir); + const cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(cwdDir); + + try { + await backend.init(); + (executeParameterized as any).mockResolvedValue([]); + + await backend.callTool('cypher', { statement: 'MATCH (n) RETURN n LIMIT 1' }); + + expect((executeParameterized as any).mock.calls.at(-1)?.[0]).toBe( + path.join(nestedDir, '.gitnexus', 'lbug'), + ); + expect(listRegisteredRepos).toHaveBeenCalledTimes(2); + } finally { + cwdSpy.mockRestore(); + } + }); + + it('refreshes a cached singleton before enforcing repo-less rename safety (#3073)', async () => { + const outerDir = mkdtempSync(path.join(os.tmpdir(), 'gnx-cwd-rename-outer-')); + const otherDir = mkdtempSync(path.join(os.tmpdir(), 'gnx-cwd-rename-other-')); + const cwdDir = path.join(outerDir, 'src'); + mkdirSync(cwdDir, { recursive: true }); + duplicateFixtureDirs.push(outerDir, otherDir); + + const outerEntry = { + ...MOCK_REPO_ENTRY, + name: 'outer', + path: outerDir, + storagePath: path.join(outerDir, '.gitnexus'), + }; + const otherEntry = { + ...MOCK_REPO_ENTRY, + name: 'other', + path: otherDir, + storagePath: path.join(otherDir, '.gitnexus'), + }; + (listRegisteredRepos as any) + .mockResolvedValueOnce([outerEntry]) + .mockResolvedValue([outerEntry, otherEntry]); + (getGitRoot as any).mockReturnValue(outerDir); + const cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(cwdDir); + + try { + await backend.init(); + await expect( + backend.callTool('rename', { + symbol_name: 'oldName', + new_name: 'newName', + dry_run: false, + }), + ).rejects.toThrow('Multiple repositories indexed'); + expect(listRegisteredRepos).toHaveBeenCalledTimes(2); + } finally { + cwdSpy.mockRestore(); + } + }); + + it('keeps explicit duplicate aliases on exact git-root disambiguation (#3073)', async () => { + const outerDir = mkdtempSync(path.join(os.tmpdir(), 'gnx-cwd-alias-outer-')); + const nestedDir = path.join(outerDir, 'packages', 'nested'); + const cwdDir = path.join(nestedDir, 'src'); + mkdirSync(cwdDir, { recursive: true }); + duplicateFixtureDirs.push(outerDir); + (listRegisteredRepos as any).mockResolvedValue([ + { + ...MOCK_REPO_ENTRY, + name: 'shared', + path: outerDir, + storagePath: path.join(outerDir, '.gitnexus'), + }, + { + ...MOCK_REPO_ENTRY, + name: 'shared', + path: nestedDir, + storagePath: path.join(nestedDir, '.gitnexus'), + }, + ]); + (getGitRoot as any).mockReturnValue(outerDir); + const cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(cwdDir); + + try { + await backend.init(); + const resolved = await backend.resolveRepo('shared'); + expect(resolved.repoPath).toBe(outerDir); + } finally { + cwdSpy.mockRestore(); + } + }); + + it('does not cross a nested git boundary when git root shelling fails (#3073)', async () => { + const outerDir = mkdtempSync(path.join(os.tmpdir(), 'gnx-cwd-rootless-outer-')); + const otherDir = mkdtempSync(path.join(os.tmpdir(), 'gnx-cwd-rootless-other-')); + const nestedDir = path.join(outerDir, 'vendor', 'nested'); + const cwdDir = path.join(nestedDir, 'src'); + mkdirSync(path.join(nestedDir, '.git'), { recursive: true }); + mkdirSync(cwdDir, { recursive: true }); + duplicateFixtureDirs.push(outerDir, otherDir); + (listRegisteredRepos as any).mockResolvedValue([ + { + ...MOCK_REPO_ENTRY, + name: 'outer', + path: outerDir, + storagePath: path.join(outerDir, '.gitnexus'), + }, + { + ...MOCK_REPO_ENTRY, + name: 'other', + path: otherDir, + storagePath: path.join(otherDir, '.gitnexus'), + }, + ]); + (getGitRoot as any).mockReturnValue(null); + const cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(cwdDir); + + try { + await backend.init(); + await expect(backend.callTool('query', { query: 'test' })).rejects.toThrow( + 'Multiple repositories indexed', + ); + } finally { + cwdSpy.mockRestore(); + } + }); + + it('keeps cwd routing opt-in for direct backend helpers (#3073)', async () => { + const outerDir = mkdtempSync(path.join(os.tmpdir(), 'gnx-cwd-direct-outer-')); + const otherDir = mkdtempSync(path.join(os.tmpdir(), 'gnx-cwd-direct-other-')); + const cwdDir = path.join(outerDir, 'src'); + mkdirSync(cwdDir, { recursive: true }); + duplicateFixtureDirs.push(outerDir, otherDir); + (listRegisteredRepos as any).mockResolvedValue([ + { + ...MOCK_REPO_ENTRY, + name: 'outer', + path: outerDir, + storagePath: path.join(outerDir, '.gitnexus'), + }, + { + ...MOCK_REPO_ENTRY, + name: 'other', + path: otherDir, + storagePath: path.join(otherDir, '.gitnexus'), + }, + ]); + (getGitRoot as any).mockReturnValue(outerDir); + const cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(cwdDir); + + try { + await backend.init(); + await expect(backend.queryProcesses()).rejects.toThrow('Multiple repositories indexed'); + } finally { + cwdSpy.mockRestore(); + } + }); + + it('does not default across an unindexed nested git boundary (#3073)', async () => { + const outerDir = mkdtempSync(path.join(os.tmpdir(), 'gnx-cwd-git-outer-')); + const otherDir = mkdtempSync(path.join(os.tmpdir(), 'gnx-cwd-git-other-')); + const nestedDir = path.join(outerDir, 'vendor', 'nested'); + const cwdDir = path.join(nestedDir, 'src'); + mkdirSync(cwdDir, { recursive: true }); + duplicateFixtureDirs.push(outerDir, otherDir); + (listRegisteredRepos as any).mockResolvedValue([ + { + ...MOCK_REPO_ENTRY, + name: 'outer', + path: outerDir, + storagePath: path.join(outerDir, '.gitnexus'), + }, + { + ...MOCK_REPO_ENTRY, + name: 'other', + path: otherDir, + storagePath: path.join(otherDir, '.gitnexus'), + }, + ]); + (getGitRoot as any).mockImplementation((value: string) => { + const resolved = path.resolve(value); + if (resolved === nestedDir || resolved.startsWith(`${nestedDir}${path.sep}`)) { + return nestedDir; + } + if (resolved === outerDir || resolved.startsWith(`${outerDir}${path.sep}`)) { + return outerDir; + } + if (resolved === otherDir || resolved.startsWith(`${otherDir}${path.sep}`)) { + return otherDir; + } + return null; + }); + const cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(cwdDir); + + try { + await backend.init(); + await expect( + backend.selectToolRepository(undefined, undefined, { allowCwdDefault: true }), + ).rejects.toThrow('Multiple repositories indexed'); + } finally { + cwdSpy.mockRestore(); + } + }); + + it('keeps mutating rename explicit with multiple repos (#3073)', async () => { + const cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue('/tmp/test-project/src'); + + try { + setupMultipleRepos(); + await backend.init(); + await expect( + backend.callTool('rename', { + symbol_name: 'oldName', + new_name: 'newName', + dry_run: true, + }), + ).rejects.toThrow('Multiple repositories indexed'); + } finally { + cwdSpy.mockRestore(); + } }); it('resolves repo by name parameter', async () => { @@ -4682,7 +5021,7 @@ describe('LocalBackend tool-staleness cache keying (#2655 review)', () => { lbugPath: `/r/.gitnexus/${path.join('branches', 'x', 'lbug')}`, lastCommit: 'BRANCHSHA', }; - vi.spyOn(backend, 'resolveRepo') + vi.spyOn(backend, 'selectToolRepository') .mockResolvedValueOnce(flat as any) .mockResolvedValueOnce(branch as any); // The tool itself returns a plain (staleness-carryable) object. @@ -4736,7 +5075,8 @@ describe('LocalBackend tool-staleness signal (#2655 review)', () => { lastCommit: 'HEADSHA', }; - const stubResolve = () => vi.spyOn(backend, 'resolveRepo').mockResolvedValue(handle as any); + const stubResolve = () => + vi.spyOn(backend, 'selectToolRepository').mockResolvedValue(handle as any); const stubStale = async () => { const { checkStalenessAsync } = await import('../../src/core/git-staleness.js'); diff --git a/gitnexus/test/unit/mcp-repository-policy.test.ts b/gitnexus/test/unit/mcp-repository-policy.test.ts index d5809d9bc..11dc4cd43 100644 --- a/gitnexus/test/unit/mcp-repository-policy.test.ts +++ b/gitnexus/test/unit/mcp-repository-policy.test.ts @@ -46,6 +46,11 @@ function createBackend(repos = REPOS) { repoPath: repo ?? repos[0]?.path, lastCommit: 'a'.repeat(40), })), + selectToolRepository: vi.fn().mockImplementation(async (repo?: string) => ({ + name: repos.find((entry) => entry.path === repo)?.name ?? repo ?? repos[0]?.name, + repoPath: repo ?? repos[0]?.path, + lastCommit: 'a'.repeat(40), + })), getContext: vi.fn().mockReturnValue(null), queryClusters: vi.fn().mockResolvedValue({ clusters: [] }), queryProcesses: vi.fn().mockResolvedValue({ processes: [] }), @@ -138,6 +143,24 @@ describe('MCP repository policy', () => { expect(backend.callTool).not.toHaveBeenCalled(); }); + it('keeps restricted schemas explicit when a multi-repo allowlist listing shrinks', async () => { + const backend = createBackend(); + const policy = await createMcpRepositoryPolicy(backend, { + GITNEXUS_MCP_ALLOWED_REPOS: 'Alpha,Beta', + }); + const alpha = REPOS[0]; + if (!alpha) throw new Error('Alpha fixture is required'); + vi.mocked(backend.listRepos).mockResolvedValue([{ ...alpha }]); + + await expect(policy.toolSchemaRepoRequirements(backend)).resolves.toEqual({ + readOnlyRequiresRepo: true, + mutatingRequiresRepo: true, + }); + await expect( + policy.scopeBackend(backend).callTool('query', { search_query: 'auth' }), + ).rejects.toThrow(/explicit repo.*multiple repositories are allowed/i); + }); + it('fails startup when the default is outside the allowlist after canonical resolution', async () => { const backend = createBackend(); await expect( @@ -199,6 +222,7 @@ describe('MCP repository policy', () => { const scoped = policy.scopeBackend(backend); await expect(scoped.resolveRepo('Beta')).rejects.toThrow(/not available/i); + await expect(scoped.selectToolRepository('Beta')).rejects.toThrow(/not available/i); await expect(scoped.readGroupStatusResource('portfolio')).rejects.toThrow( /group.*unavailable/i, ); diff --git a/gitnexus/test/unit/resources.test.ts b/gitnexus/test/unit/resources.test.ts index ff3894be1..bae0e65f6 100644 --- a/gitnexus/test/unit/resources.test.ts +++ b/gitnexus/test/unit/resources.test.ts @@ -396,7 +396,10 @@ describe('readResource', () => { }); const result = await readResource('gitnexus://repos', backend); expect(result).toContain('Multiple repos indexed'); - expect(result).toContain('repo parameter'); + expect(result).toContain('process.cwd()'); + expect(result).toContain('unindexed nested Git checkout'); + expect(result).toContain('mutating tools without an MCP default'); + expect(result).toContain('pass repo explicitly'); // The example must use a registered tool name, not the unregistered // `gitnexus_search` / `gitnexus_*` prefix (#2059). // #2175: advertise the renamed param, not the legacy "query" key. diff --git a/gitnexus/test/unit/server.test.ts b/gitnexus/test/unit/server.test.ts index 68d124ee5..8e10bccae 100644 --- a/gitnexus/test/unit/server.test.ts +++ b/gitnexus/test/unit/server.test.ts @@ -34,6 +34,9 @@ function createMockBackend(overrides: Record = {}): any { resolveRepo: vi .fn() .mockResolvedValue({ name: 'test', repoPath: '/tmp/test', lastCommit: 'abc' }), + selectToolRepository: vi + .fn() + .mockResolvedValue({ name: 'test', repoPath: '/tmp/test', lastCommit: 'abc' }), getContext: vi.fn().mockReturnValue(null), queryClusters: vi.fn().mockResolvedValue({ clusters: [] }), queryProcesses: vi.fn().mockResolvedValue({ processes: [] }), @@ -105,12 +108,13 @@ describe('createMCPServer', () => { await server.close(); } }); - it('requires repo in repo-scoped tool schemas when multiple repos are visible', async () => { + it('requires repo in repo-scoped tool schemas when cwd cannot resolve multiple repos', async () => { const backend = createMockBackend({ listRepos: vi.fn().mockResolvedValue([ { name: 'alpha', path: '/tmp/alpha' }, { name: 'beta', path: '/tmp/beta' }, ]), + selectToolRepository: vi.fn().mockRejectedValue(new Error('Multiple repositories indexed')), }); const server = createMCPServer(backend); const client = new Client({ name: 'multi-repo-client', version: '0.0.0' }); @@ -133,6 +137,43 @@ describe('createMCPServer', () => { } }); + it('keeps repo optional when cwd resolves one of multiple visible repos', async () => { + const backend = createMockBackend({ + listRepos: vi.fn().mockResolvedValue([ + { name: 'alpha', path: '/tmp/alpha' }, + { name: 'beta', path: '/tmp/beta' }, + ]), + selectToolRepository: vi + .fn() + .mockResolvedValue({ name: 'alpha', repoPath: '/tmp/alpha', lastCommit: 'abc' }), + }); + const server = createMCPServer(backend); + const client = new Client({ name: 'cwd-repo-client', version: '0.0.0' }); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + + try { + await Promise.all([server.connect(serverTransport), client.connect(clientTransport)]); + const tools = await client.listTools(); + const context = tools.tools.find((tool) => tool.name === 'context'); + const rename = tools.tools.find((tool) => tool.name === 'rename'); + + expect(context?.inputSchema.required).not.toContain('repo'); + expect(rename?.inputSchema.required).toContain('repo'); + const response = await client.callTool({ name: 'context', arguments: { name: 'Example' } }); + expect(response.isError).not.toBe(true); + expect(backend.callTool).toHaveBeenCalledWith('context', { name: 'Example' }); + expect(backend.listRepos).toHaveBeenCalledTimes(1); + expect(backend.selectToolRepository).toHaveBeenCalledTimes(1); + expect(backend.selectToolRepository).toHaveBeenCalledWith(undefined, undefined, { + allowCwdDefault: true, + refreshRegistry: false, + }); + } finally { + await client.close(); + await server.close(); + } + }); + it('keeps repo optional when a default repo is configured', async () => { const backend = createMockBackend({ listRepos: vi.fn().mockResolvedValue([ diff --git a/gitnexus/test/unit/tools.test.ts b/gitnexus/test/unit/tools.test.ts index 700407f78..bfa409405 100644 --- a/gitnexus/test/unit/tools.test.ts +++ b/gitnexus/test/unit/tools.test.ts @@ -283,6 +283,25 @@ describe('GITNEXUS_TOOLS', () => { } }); + it('repo descriptions explain the cwd default and mutating exception (#3073)', () => { + expect(GITNEXUS_TOOLS.find((tool) => tool.name === 'list_repos')?.description).toMatch( + /process cwd/i, + ); + expect(GITNEXUS_TOOLS.find((tool) => tool.name === 'list_repos')?.description).toMatch( + /unindexed nested Git checkout/i, + ); + for (const tool of GITNEXUS_TOOLS) { + if (tool.name === 'list_repos' || GROUP_TOOLS.has(tool.name)) continue; + const description = tool.inputSchema.properties.repo.description; + if (tool.name === 'rename') { + expect(description).toMatch(/mutating tools require an explicit repo/i); + } else { + expect(description).toMatch(/process cwd/i); + expect(description).toMatch(/unindexed nested Git checkout/i); + } + } + }); + it('per-repo tools have an optional branch scope param (#2106); group/list tools do not', () => { for (const tool of GITNEXUS_TOOLS) { if (tool.name === 'list_repos' || GROUP_TOOLS.has(tool.name)) { From 7e993ab8972386294fb96bf14a8665d0b5325397 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Sat, 29 Aug 2026 22:48:46 +0100 Subject: [PATCH 3/3] fix(group): fail ambiguous sync names and honor analyze --name (#3094) * fix(group): fail sync when a member name is ambiguous Silent first-match bound the wrong clone when --allow-duplicate-name registered two paths under one alias. Refs #3028. Co-authored-by: Cursor * fix(analyze): apply --name on the already-up-to-date path A rename should not require --force when the index is already current. Register before the same-commit branch restamp. Refs #3028. Co-authored-by: Cursor * fix(group): hint member path when impact --repo is an alias $localRepo stays the yaml key; joining on the registry alias is a non-join. List matching keys so operators can retry. Refs #3028. Co-authored-by: Cursor * fix(group): keep injected sync and alias hints consistent Workspace-deps path maps reuse the resolved handle so duplicate names cannot throw after an injected resolver. Alias hints match case-insensitively. Co-authored-by: Cursor --------- Co-authored-by: Gergo Magyar Co-authored-by: Cursor --- gitnexus/src/cli/analyze.ts | 3 + gitnexus/src/cli/group.ts | 8 +- gitnexus/src/core/group/config-parser.ts | 11 +- gitnexus/src/core/group/cross-impact.ts | 11 + gitnexus/src/core/group/service.ts | 6 +- gitnexus/src/core/group/sync.ts | 43 ++- gitnexus/src/core/run-analyze.ts | 36 +++ gitnexus/src/storage/repo-manager.ts | 28 +- .../test/integration/group/group-cli.test.ts | 45 +++ .../test/unit/group-service-not-found.test.ts | 5 +- .../test/unit/group/config-parser.test.ts | 19 ++ gitnexus/test/unit/group/cross-impact.test.ts | 98 +++++++ .../group/resolve-bridge-neighbors.test.ts | 56 ++++ .../group/service-group-sync-payload.test.ts | 1 + .../group/sync-partial-extraction.test.ts | 12 +- .../unit/group/sync-registry-identity.test.ts | 273 ++++++++++++++++++ .../unit/group/sync-unreadable-repos.test.ts | 12 +- .../group/sync-windowed-resolution.test.ts | 12 +- .../repo-manager-registry-strict-read.test.ts | 16 +- gitnexus/test/unit/repo-manager.test.ts | 8 + .../test/unit/run-analyze-fts-repair.test.ts | 46 +++ gitnexus/test/unit/run-analyze.test.ts | 193 +++++++++++++ 22 files changed, 918 insertions(+), 24 deletions(-) create mode 100644 gitnexus/test/unit/group/sync-registry-identity.test.ts diff --git a/gitnexus/src/cli/analyze.ts b/gitnexus/src/cli/analyze.ts index 54bf12d11..2719b8f40 100644 --- a/gitnexus/src/cli/analyze.ts +++ b/gitnexus/src/cli/analyze.ts @@ -1436,6 +1436,9 @@ const analyzeCommandImpl = async ( console.error = origError; bar.stop(); console.log(' Already up to date\n'); + if (runOptions.registryName) { + console.log(` Registry name: ${result.repoName}\n`); + } if (baseRefRefreshed.length > 0) { console.log( ` Updated base_ref to "${resolvedDefaultBranch}" in ${baseRefRefreshed.join(', ')}\n`, diff --git a/gitnexus/src/cli/group.ts b/gitnexus/src/cli/group.ts index 7bb7bc180..abc13fa2b 100644 --- a/gitnexus/src/cli/group.ts +++ b/gitnexus/src/cli/group.ts @@ -219,8 +219,9 @@ export function registerGroupCommands(program: Command): void { .action(async (name: string, opts: Record) => { const { getGroupDir, getDefaultGitnexusDir } = await import('../core/group/storage.js'); const { loadGroupConfig } = await import('../core/group/config-parser.js'); - const { syncGroup } = await import('../core/group/sync.js'); + const { syncGroup, formatGroupSyncAmbiguousError } = await import('../core/group/sync.js'); const { GroupSyncLockError } = await import('../core/group/group-lock.js'); + const { RegistryAmbiguousTargetError } = await import('../storage/repo-manager.js'); const groupDir = getGroupDir(getDefaultGitnexusDir(), name); const config = await loadGroupConfig(groupDir); @@ -235,6 +236,11 @@ export function registerGroupCommands(program: Command): void { exactOnly: Boolean(opts.exactOnly), }); } catch (err) { + if (err instanceof RegistryAmbiguousTargetError) { + logger.error(`⚠️ Did not sync group "${name}": ${formatGroupSyncAmbiguousError(err)}`); + process.exitCode = 1; + return; + } // A sync that could not take the group's lock did NOT run and wrote // nothing (R9 fails closed). That is an operator-actionable outcome, not // a crash, so report it as a failed command rather than letting it diff --git a/gitnexus/src/core/group/config-parser.ts b/gitnexus/src/core/group/config-parser.ts index b831c6706..373e5e5c4 100644 --- a/gitnexus/src/core/group/config-parser.ts +++ b/gitnexus/src/core/group/config-parser.ts @@ -60,7 +60,16 @@ export function parseGroupConfig(yamlContent: string): GroupConfig { throw new Error('repos is required in group.yaml (must be a mapping)'); } - const repos = raw.repos as Record; + const reposRaw = raw.repos as Record; + const repos: Record = {}; + for (const [memberPath, registryName] of Object.entries(reposRaw)) { + if (typeof registryName !== 'string' || registryName.trim() === '') { + throw new Error( + `repos["${memberPath}"] must be a non-empty registry name string, not ${typeof registryName}`, + ); + } + repos[memberPath] = registryName.trim(); + } const repoPaths = new Set(Object.keys(repos)); const rawLinks = (raw.links as unknown[]) || []; diff --git a/gitnexus/src/core/group/cross-impact.ts b/gitnexus/src/core/group/cross-impact.ts index 485b1ee8c..fd2ae0779 100644 --- a/gitnexus/src/core/group/cross-impact.ts +++ b/gitnexus/src/core/group/cross-impact.ts @@ -244,6 +244,17 @@ async function resolveGroupRepo( ): Promise { const registryName = config.repos[repoPath]; if (!registryName) { + const matchingMemberPaths = Object.entries(config.repos) + .filter(([, alias]) => alias.toLowerCase() === repoPath.toLowerCase()) + .map(([memberPath]) => memberPath); + if (matchingMemberPaths.length > 0) { + return { + error: + `Unknown repo path "${repoPath}" in this group. ` + + `That value is a registry alias for member path(s): ${matchingMemberPaths.join(', ')}. ` + + `Pass the group.yaml key to --repo, not the alias.`, + }; + } return { error: `Unknown repo path "${repoPath}" in this group.` }; } try { diff --git a/gitnexus/src/core/group/service.ts b/gitnexus/src/core/group/service.ts index af1f1385a..d0aa4882c 100644 --- a/gitnexus/src/core/group/service.ts +++ b/gitnexus/src/core/group/service.ts @@ -479,8 +479,9 @@ export class GroupService { // group tools never need it — so deferring it here keeps that closure off // MCP server startup entirely and off every non-sync group call. The CLI // already does exactly this at `cli/group.ts`'s sync command. - const { syncGroup } = await import('./sync.js'); + const { syncGroup, formatGroupSyncAmbiguousError } = await import('./sync.js'); const { GroupSyncLockError } = await import('./group-lock.js'); + const { RegistryAmbiguousTargetError } = await import('../../storage/repo-manager.js'); let result: Awaited>; try { result = await syncGroup(config, { @@ -492,6 +493,9 @@ export class GroupService { // expects. `SyncOptions.verbose` stays for the CLI, which can see them. }); } catch (err) { + if (err instanceof RegistryAmbiguousTargetError) { + return { error: formatGroupSyncAmbiguousError(err) }; + } // Fails closed (R9): this sync could not be protected against a concurrent // one, so it did not run and wrote nothing. Return it through the same // error channel a missing group uses — NEVER as a success payload of zeroes, diff --git a/gitnexus/src/core/group/sync.ts b/gitnexus/src/core/group/sync.ts index d79cd4de5..7981a3d0c 100644 --- a/gitnexus/src/core/group/sync.ts +++ b/gitnexus/src/core/group/sync.ts @@ -8,8 +8,12 @@ import { getMaxResidentRepos, } from '../lbug/pool-adapter.js'; import { + findRegistryEntryByName, + canonicalizePath, + registryPathEquals, readRegistry, readRegistryStrict, + RegistryAmbiguousTargetError, type RegistryEntry, } from '../../storage/repo-manager.js'; import type { @@ -128,9 +132,19 @@ export function stableRepoPoolId(entry: RegistryEntry, allEntries: RegistryEntry return base; } +/** Operator copy for group sync — unique `--name`, not a path in yaml. */ +export function formatGroupSyncAmbiguousError(err: RegistryAmbiguousTargetError): string { + const listing = err.matches.map((m) => ` - ${m.path}`).join('\n'); + return ( + `Multiple registered repos are named "${err.target}":\n${listing}\n` + + `Give each clone a unique registry name with \`gitnexus analyze --name\`, then re-sync. ` + + `Do not put a filesystem path in group.yaml.` + ); +} + function defaultResolveHandle(allEntries: RegistryEntry[]) { return async (registryName: string, groupPath: string): Promise => { - const e = allEntries.find((en) => en.name === registryName); + const e = findRegistryEntryByName(allEntries, registryName); if (!e) return null; const poolId = stableRepoPoolId(e, allEntries); return { @@ -277,7 +291,10 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis // Group-path → pool identity for repos that successfully initialized. Drives // windowed manifest resolution below (re-init + lease per window). Keyed by // group path because manifest links reference repos by group path. - const repoHandles = new Map(); + const repoHandles = new Map(); + // Keep resolved disk paths even when extraction fails and removes the + // corresponding handle; workspace discovery does not need a readable index. + const resolvedRepoPaths = new Map(); // Every eviction lease this sync holds. Window loops release their own leases // (bounding residency); this set is the defensive outer-finally sweep — // release disposers are idempotent, so double-release is a safe no-op. @@ -295,6 +312,11 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis registryEntries = await readRegistryStrict(); const entries = registryEntries; const resolve = opts?.resolveRepoHandle ?? defaultResolveHandle(entries); + if (!opts?.resolveRepoHandle) { + for (const regName of Object.values(config.repos)) { + findRegistryEntryByName(entries, regName); + } + } const httpEx = new HttpRouteExtractor(); const graphqlEx = new GraphqlExtractor(); const grpcEx = new GrpcExtractor(); @@ -308,6 +330,7 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis missingRepos.push(groupPath); continue; } + resolvedRepoPaths.set(groupPath, handle.repoPath); const poolId = handle.id; const lbugPath = path.join(handle.storagePath, 'lbug'); @@ -327,7 +350,7 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis // resolution no longer reuses these executors — it re-inits + leases // each repo per window (see windowed resolution below, issue #2189). // Record the pool identity so windowed resolution can re-init. - repoHandles.set(groupPath, { poolId, lbugPath }); + repoHandles.set(groupPath, { poolId, lbugPath, repoPath: handle.repoPath }); const executor: CypherExecutor = (query, params) => executeParameterized(poolId, query, params ?? {}); @@ -409,7 +432,10 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis lastCommit: m.lastCommit || '', }; } catch { - const e = entries.find((en) => en.name === regName); + const resolvedHandlePath = canonicalizePath(handle.repoPath); + const e = entries.find((en) => + registryPathEquals(canonicalizePath(en.path), resolvedHandlePath), + ); repoSnapshots[groupPath] = { indexedAt: e?.indexedAt || '', lastCommit: e?.lastCommit || '', @@ -431,6 +457,7 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis // read. The loop bounds the append by memory instead. for (const contract of repoContracts) autoContracts.push(contract); } catch (err) { + if (err instanceof RegistryAmbiguousTargetError) throw err; // This spans initLbug plus all contract extraction for the repo. The // error used to be discarded entirely, so the only trace of (say) a // storage-version mismatch was an empty contracts.json and a later @@ -465,7 +492,13 @@ export async function syncGroup(config: GroupConfig, opts?: SyncOptions): Promis const repoPaths = new Map(); if (!registryEntries) registryEntries = await readRegistry(); for (const [groupPath, regName] of Object.entries(config.repos)) { - const e = registryEntries.find((en) => en.name === regName); + const resolvedPath = resolvedRepoPaths.get(groupPath); + if (resolvedPath) { + repoPaths.set(groupPath, resolvedPath); + continue; + } + if (opts?.resolveRepoHandle) continue; + const e = findRegistryEntryByName(registryEntries, regName); if (e) repoPaths.set(groupPath, e.path); } diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index 018efc42a..ad9841406 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -1308,6 +1308,13 @@ async function runFullAnalysisInner( } progress('fts', 90, 'Search indexes ready'); progress('done', 100, 'Done'); + if (options.registryName) { + await registerRepo(repoPath, existingMeta, { + name: options.registryName, + allowDuplicateName: options.allowDuplicateName, + branch: placement.branch, + }); + } return { repoName: options.registryName ?? @@ -1687,6 +1694,35 @@ async function runFullAnalysisInner( // later read on a host where it loads — which is a legitimate, common // state, and the invariant `analyzer-identity-cli.test.ts` pins. if (!dirty && !healUnregistered) { + if (options.registryName) { + await registerRepo(repoPath, existingMeta, { + name: options.registryName, + allowDuplicateName: options.allowDuplicateName, + branch: placement.branch, + }); + if (!placement.branch) { + try { + await generateAIContextFiles( + repoPath, + storagePath, + options.registryName, + existingMeta.stats ?? {}, + undefined, + { + skipAgentsMd: options.skipAgentsMd, + skipSkills: options.skipSkills, + noStats: options.noStats, + defaultBranch: options.defaultBranch, + // Fast path does not re-run PDG. Using `options.pdg` would + // strip PDG bullets from AGENTS.md on a rename-only analyze. + hasPdg: existingMeta.pdg != null, + }, + ); + } catch { + /* best-effort — never fail the fast path over a context refresh */ + } + } + } // ── #2354: restamp the workspace label on a same-commit branch flip ── // The flat slot follows the checked-out working tree; a branch switch // at the SAME commit with a clean tree changes nothing the pipeline diff --git a/gitnexus/src/storage/repo-manager.ts b/gitnexus/src/storage/repo-manager.ts index b223d5f69..3f68df80a 100644 --- a/gitnexus/src/storage/repo-manager.ts +++ b/gitnexus/src/storage/repo-manager.ts @@ -909,7 +909,10 @@ const registerRepoUnlocked = async ( // falling back to `path.resolve` when the path doesn't exist. const canonicalInput = canonicalizePath(repoPath); - const entries = await readRegistry(); + // Mutating writes must not treat an unreadable/truncated registry as empty + // (#3094): lenient `readRegistry()` returns `[]` on parse failure and would + // replace the machine-wide file with only this entry. ENOENT stays empty. + const entries = await readRegistryStrict(); const existingIdx = entries.findIndex((e) => { // Canonicalise the STORED entry too so pre-canonicalisation // registries (written by older versions, or paths passed in a @@ -1024,7 +1027,7 @@ const registerRepoUnlocked = async ( // R9): re-derive THIS run's delta against the FRESHEST snapshot so a // concurrent change to the OTHER axis (a branch upsert vs a primary refresh) // survives instead of being clobbered by a stale entry-time view. - const fresh = await readRegistry(); + const fresh = await readRegistryStrict(); const freshIdx = fresh.findIndex((e) => { const a = canonicalizePath(e.path); return registryPathEquals(a, canonicalInput); @@ -1469,6 +1472,27 @@ export const resolveRegistryEntry = (entries: RegistryEntry[], target: string): throw new RegistryNotFoundError(target, availableNames); }; +/** + * Name-only registry match (the name tier of {@link resolveRegistryEntry}, + * without path matching). Used by `group.yaml` member *values*, which are + * registry aliases, not filesystem paths. + * + * Zero matches → `undefined` (caller treats as missing). One match → that + * entry. Two or more → {@link RegistryAmbiguousTargetError}. + */ +export const findRegistryEntryByName = ( + entries: RegistryEntry[], + name: string, +): RegistryEntry | undefined => { + const targetLower = name.toLowerCase(); + const nameMatches = entries.filter((e) => e.name.toLowerCase() === targetLower); + if (nameMatches.length === 1) return nameMatches[0]; + if (nameMatches.length > 1) { + throw new RegistryAmbiguousTargetError(name, nameMatches); + } + return undefined; +}; + /** * List all registered repos from the global registry. * diff --git a/gitnexus/test/integration/group/group-cli.test.ts b/gitnexus/test/integration/group/group-cli.test.ts index a9c1840cf..45dda9e7a 100644 --- a/gitnexus/test/integration/group/group-cli.test.ts +++ b/gitnexus/test/integration/group/group-cli.test.ts @@ -55,6 +55,51 @@ describe('group CLI', () => { expect(l.stdout).toContain('acme'); }); + it('sync exits nonzero with formatted copy when a member name is ambiguous', () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-group-cli-amb-')); + try { + fs.mkdirSync(path.join(home, 'groups', 'g1'), { recursive: true }); + fs.writeFileSync( + path.join(home, 'groups', 'g1', 'group.yaml'), + `version: 1 +name: g1 +repos: + demo/api: demo-api +`, + ); + const cloneA = path.join(home, 'clone-a'); + const cloneB = path.join(home, 'clone-b'); + fs.mkdirSync(path.join(cloneA, '.gitnexus'), { recursive: true }); + fs.mkdirSync(path.join(cloneB, '.gitnexus'), { recursive: true }); + fs.writeFileSync( + path.join(home, 'registry.json'), + JSON.stringify([ + { + name: 'demo-api', + path: cloneA, + storagePath: path.join(cloneA, '.gitnexus'), + indexedAt: '2026-01-01T00:00:00.000Z', + lastCommit: 'aaa', + }, + { + name: 'demo-api', + path: cloneB, + storagePath: path.join(cloneB, '.gitnexus'), + indexedAt: '2026-01-01T00:00:00.000Z', + lastCommit: 'bbb', + }, + ]), + ); + const r = runGroupIn(home, ['sync', 'g1']); + expect(r.status).not.toBe(0); + // CLI logs JSON (pino): quotes around the group name are escaped in the byte stream. + expect(`${r.stderr}${r.stdout}`).toMatch(/Did not sync group \\"g1\\"/); + expect(`${r.stderr}${r.stdout}`).toContain('demo-api'); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + it('test_create_with_invalid_name_fails', () => { const result = runGroup(['create', '../../evil']); expect(result.status).not.toBe(0); diff --git a/gitnexus/test/unit/group-service-not-found.test.ts b/gitnexus/test/unit/group-service-not-found.test.ts index b16f9074f..f3961b9ab 100644 --- a/gitnexus/test/unit/group-service-not-found.test.ts +++ b/gitnexus/test/unit/group-service-not-found.test.ts @@ -21,7 +21,10 @@ vi.mock('../../src/core/group/storage.js', () => ({ listGroups: listGroupsMock, })); -vi.mock('../../src/core/group/sync.js', () => ({ syncGroup: syncGroupMock })); +vi.mock('../../src/core/group/sync.js', () => ({ + syncGroup: syncGroupMock, + formatGroupSyncAmbiguousError: (err: Error) => err.message, +})); vi.mock('../../src/core/git-staleness.js', () => ({ checkStaleness: vi.fn() })); describe('GroupService — missing group error handling', () => { diff --git a/gitnexus/test/unit/group/config-parser.test.ts b/gitnexus/test/unit/group/config-parser.test.ts index 9644b304b..a17ebe148 100644 --- a/gitnexus/test/unit/group/config-parser.test.ts +++ b/gitnexus/test/unit/group/config-parser.test.ts @@ -245,6 +245,25 @@ links: expect(() => parseGroupConfig('version: 1\nname: test')).toThrow(/repos.*required/i); }); + it('throws when a repos value is not a string (YAML number/boolean)', () => { + expect(() => + parseGroupConfig(`version: 1 +name: test +repos: + app: 12 +`), + ).toThrow(/non-empty registry name string/); + }); + + it('trims padded registry aliases so they match the registry name', () => { + const config = parseGroupConfig(`version: 1 +name: test +repos: + app: " my-app " +`); + expect(config.repos.app).toBe('my-app'); + }); + it('allows empty repos object (fresh group before first add)', () => { const yaml = `version: 1 name: new-group diff --git a/gitnexus/test/unit/group/cross-impact.test.ts b/gitnexus/test/unit/group/cross-impact.test.ts index bceaa9195..9d718a0bf 100644 --- a/gitnexus/test/unit/group/cross-impact.test.ts +++ b/gitnexus/test/unit/group/cross-impact.test.ts @@ -444,4 +444,102 @@ describe('cross-impact', () => { cleanup(); } }); + + it('hints the yaml member path when --repo is the registry alias', async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-ci-alias-')); + const groupDir = path.join(tmpDir, 'groups', 'g1'); + fs.mkdirSync(groupDir, { recursive: true }); + fs.writeFileSync( + path.join(groupDir, 'group.yaml'), + `version: 1 +name: g1 +repos: + demo/api: demo-api + demo/web: demo-web +`, + ); + vi.stubEnv('GITNEXUS_HOME', tmpDir); + try { + const port: GroupToolPort = { + resolveRepo: vi.fn(), + impact: vi.fn(), + query: vi.fn(), + impactByUid: vi.fn(), + context: vi.fn(), + }; + const r = await runGroupImpact( + { port, gitnexusDir: tmpDir }, + { + name: 'g1', + repo: 'demo-api', + target: 'Sym', + direction: 'upstream', + }, + ); + expect('error' in r).toBe(true); + if ('error' in r) { + expect(r.error).toContain('demo/api'); + expect(r.error).toMatch(/registry alias/i); + expect(r.error).not.toContain('demo/web'); + } + const mixedCase = await runGroupImpact( + { port, gitnexusDir: tmpDir }, + { + name: 'g1', + repo: 'Demo-API', + target: 'Sym', + direction: 'upstream', + }, + ); + expect('error' in mixedCase).toBe(true); + if ('error' in mixedCase) { + expect(mixedCase.error).toContain('demo/api'); + } + } finally { + vi.unstubAllEnvs(); + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it('lists every member path that shares the same registry alias', async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-ci-alias-dup-')); + const groupDir = path.join(tmpDir, 'groups', 'g1'); + fs.mkdirSync(groupDir, { recursive: true }); + fs.writeFileSync( + path.join(groupDir, 'group.yaml'), + `version: 1 +name: g1 +repos: + demo/api: shared + demo/other: shared +`, + ); + vi.stubEnv('GITNEXUS_HOME', tmpDir); + try { + const port: GroupToolPort = { + resolveRepo: vi.fn(), + impact: vi.fn(), + query: vi.fn(), + impactByUid: vi.fn(), + context: vi.fn(), + }; + const r = await runGroupImpact( + { port, gitnexusDir: tmpDir }, + { + name: 'g1', + repo: 'shared', + target: 'Sym', + direction: 'upstream', + }, + ); + expect('error' in r).toBe(true); + if ('error' in r) { + expect(r.error).toContain('demo/api'); + expect(r.error).toContain('demo/other'); + } + } finally { + vi.unstubAllEnvs(); + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); }); diff --git a/gitnexus/test/unit/group/resolve-bridge-neighbors.test.ts b/gitnexus/test/unit/group/resolve-bridge-neighbors.test.ts index 73ffa5b7e..13876c897 100644 --- a/gitnexus/test/unit/group/resolve-bridge-neighbors.test.ts +++ b/gitnexus/test/unit/group/resolve-bridge-neighbors.test.ts @@ -127,4 +127,60 @@ describe('resolveBridgeNeighbors', () => { expect(rows).toEqual([]); await closeBridgeDb(handle!); }); + + itLbugReopen( + 'registry alias as localRepo does not join contracts stamped with the member path', + async () => { + const consumer = makeContract({ + repo: 'demo/api', + role: 'consumer', + symbolUid: 'consumer-uid', + symbolRef: { filePath: 'src/api.ts', name: 'fetchUsers' }, + symbolName: 'fetchUsers', + contractId: 'http::GET::/api/users', + confidence: 0.5, + }); + const provider = makeContract({ + repo: 'demo/api', + role: 'provider', + symbolUid: 'provider-uid', + symbolRef: { filePath: 'src/routes.ts', name: 'getUsers' }, + symbolName: 'getUsers', + contractId: 'http::GET::/api/users', + confidence: 0.9, + }); + const link: CrossLink = { + from: { repo: 'web', symbolUid: 'web-uid', symbolRef: consumer.symbolRef }, + to: { repo: 'demo/api', symbolUid: 'provider-uid', symbolRef: provider.symbolRef }, + type: 'http', + contractId: 'http::GET::/api/users', + matchType: 'manifest', + confidence: 0.9, + }; + await writeBridge(tmpDir, { + contracts: [{ ...consumer, repo: 'web' }, provider], + crossLinks: [link], + repoSnapshots: {}, + missingRepos: [], + }); + const handle = await openBridgeDbReadOnly(tmpDir); + const aliasMiss = await resolveBridgeNeighbors(handle!, { + localRepo: 'demo-api', + uids: ['provider-uid'], + direction: 'upstream', + }); + expect(aliasMiss).toEqual([]); + const pathHit = await resolveBridgeNeighbors(handle!, { + localRepo: 'demo/api', + uids: ['provider-uid'], + direction: 'upstream', + }); + expect(pathHit).toHaveLength(1); + expect(pathHit[0]).toMatchObject({ + neighborRepo: 'web', + matchType: 'manifest', + }); + await closeBridgeDb(handle!); + }, + ); }); diff --git a/gitnexus/test/unit/group/service-group-sync-payload.test.ts b/gitnexus/test/unit/group/service-group-sync-payload.test.ts index ddfa32d71..ea2792b7d 100644 --- a/gitnexus/test/unit/group/service-group-sync-payload.test.ts +++ b/gitnexus/test/unit/group/service-group-sync-payload.test.ts @@ -50,6 +50,7 @@ const syncGroupMock = vi.fn<() => Promise>(); vi.mock('../../../src/core/group/sync.js', () => ({ syncGroup: (...args: unknown[]) => syncGroupMock(...(args as [])), + formatGroupSyncAmbiguousError: (err: Error) => err.message, })); const { GroupService } = await import('../../../src/core/group/service.js'); diff --git a/gitnexus/test/unit/group/sync-partial-extraction.test.ts b/gitnexus/test/unit/group/sync-partial-extraction.test.ts index 06b3bf450..0e7d41228 100644 --- a/gitnexus/test/unit/group/sync-partial-extraction.test.ts +++ b/gitnexus/test/unit/group/sync-partial-extraction.test.ts @@ -54,10 +54,14 @@ vi.mock('../../../src/core/lbug/pool-adapter.js', () => ({ getMaxResidentRepos: vi.fn(() => 5), })); -vi.mock('../../../src/storage/repo-manager.js', () => ({ - readRegistry: vi.fn(async () => []), - readRegistryStrict: vi.fn(async () => []), -})); +vi.mock('../../../src/storage/repo-manager.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + readRegistry: vi.fn(async () => []), + readRegistryStrict: vi.fn(async () => []), + }; +}); vi.mock('../../../src/core/group/extractors/http-route-extractor.js', () => ({ HttpRouteExtractor: class { diff --git a/gitnexus/test/unit/group/sync-registry-identity.test.ts b/gitnexus/test/unit/group/sync-registry-identity.test.ts new file mode 100644 index 000000000..309a2c0fd --- /dev/null +++ b/gitnexus/test/unit/group/sync-registry-identity.test.ts @@ -0,0 +1,273 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import fs from 'node:fs/promises'; +import { mkdirSync } from 'node:fs'; +import path from 'node:path'; +import { syncGroup } from '../../../src/core/group/sync.js'; +import { RegistryAmbiguousTargetError } from '../../../src/storage/repo-manager.js'; +import { createTempDir } from '../../helpers/test-db.js'; +import type { GroupConfig } from '../../../src/core/group/types.js'; +import { GroupService } from '../../../src/core/group/service.js'; +import type { GroupToolPort } from '../../../src/core/group/service.js'; + +const initLbugMock = vi.fn(async () => {}); + +vi.mock('../../../src/core/lbug/pool-adapter.js', () => ({ + initLbug: (...args: unknown[]) => initLbugMock(...args), + executeParameterized: vi.fn(async () => []), + pinRepo: vi.fn(() => () => {}), + getMaxResidentRepos: vi.fn(() => 5), +})); + +const makeConfig = (repos: Record, extra?: Partial): GroupConfig => ({ + version: 1, + name: 'test', + description: '', + repos, + links: [], + packages: {}, + detect: { + http: false, + graphql: false, + grpc: false, + thrift: false, + topics: false, + includes: false, + workspace_deps: false, + }, + matching: {}, + ...extra, +}); + +const row = ( + tmpHome: string, + name: string, + clone: string, +): { + name: string; + path: string; + storagePath: string; + indexedAt: string; + lastCommit: string; +} => { + mkdirSync(path.join(tmpHome, 'repos', clone), { recursive: true }); + return { + name, + path: path.join(tmpHome, 'repos', clone), + storagePath: path.join(tmpHome, 'repos', clone, '.gitnexus'), + indexedAt: '2026-01-01T00:00:00.000Z', + lastCommit: 'abc123', + }; +}; + +describe('syncGroup registry name identity', () => { + let tmpHome: Awaited>; + let savedGitnexusHome: string | undefined; + let registryPath: string; + + beforeEach(async () => { + initLbugMock.mockReset(); + initLbugMock.mockResolvedValue(undefined); + tmpHome = await createTempDir('gitnexus-sync-registry-id-'); + savedGitnexusHome = process.env.GITNEXUS_HOME; + process.env.GITNEXUS_HOME = tmpHome.dbPath; + registryPath = path.join(tmpHome.dbPath, 'registry.json'); + }); + + afterEach(async () => { + if (savedGitnexusHome === undefined) delete process.env.GITNEXUS_HOME; + else process.env.GITNEXUS_HOME = savedGitnexusHome; + await tmpHome.cleanup(); + }); + + it('throws RegistryAmbiguousTargetError and does not rewrite group dir files', async () => { + const a = row(tmpHome.dbPath, 'demo-api', 'clone-a'); + const b = row(tmpHome.dbPath, 'demo-api', 'clone-b'); + await fs.writeFile(registryPath, JSON.stringify([a, b])); + + const groupDir = path.join(tmpHome.dbPath, 'groups', 'g'); + await fs.mkdir(groupDir, { recursive: true }); + const contractsPath = path.join(groupDir, 'contracts.json'); + const prior = '{"contracts":[],"crossLinks":[],"marker":"keep"}\n'; + await fs.writeFile(contractsPath, prior); + + await expect(syncGroup(makeConfig({ 'demo/api': 'demo-api' }), { groupDir })).rejects.toSatisfy( + (err: unknown) => { + expect(err).toBeInstanceOf(RegistryAmbiguousTargetError); + const amb = err as RegistryAmbiguousTargetError; + expect(amb.matches).toHaveLength(2); + expect(amb.matches.map((m) => m.path).sort()).toEqual([a.path, b.path].sort()); + return true; + }, + ); + + expect(await fs.readFile(contractsPath, 'utf-8')).toBe(prior); + await expect(fs.access(path.join(groupDir, 'bridge.lbug'))).rejects.toThrow(); + }); + + it('records an unknown yaml value as missing and still extracts other members', async () => { + const known = row(tmpHome.dbPath, 'backend-repo', 'backend'); + await fs.writeFile(registryPath, JSON.stringify([known])); + + const result = await syncGroup( + makeConfig({ 'app/backend': 'backend-repo', 'app/ghost': 'ghost' }), + { skipWrite: true }, + ); + + expect(result.missingRepos).toEqual(['app/ghost']); + expect(result.unreadableRepos).toEqual([]); + expect(result.repoSnapshots['app/backend']).toEqual({ + indexedAt: known.indexedAt, + lastCommit: known.lastCommit, + }); + }); + + it('treats mixed missing and ambiguous names as a terminal ambiguity with no write', async () => { + const a = row(tmpHome.dbPath, 'demo-api', 'clone-a'); + const b = row(tmpHome.dbPath, 'demo-api', 'clone-b'); + await fs.writeFile(registryPath, JSON.stringify([a, b])); + + const groupDir = path.join(tmpHome.dbPath, 'groups', 'g'); + await fs.mkdir(groupDir, { recursive: true }); + const contractsPath = path.join(groupDir, 'contracts.json'); + await fs.writeFile(contractsPath, '{"keep":true}'); + + await expect( + syncGroup(makeConfig({ 'demo/api': 'demo-api', 'app/ghost': 'ghost' }), { groupDir }), + ).rejects.toBeInstanceOf(RegistryAmbiguousTargetError); + + expect(await fs.readFile(contractsPath, 'utf-8')).toBe('{"keep":true}'); + }); + + it('injected resolveRepoHandle still bypasses default name matching', async () => { + const a = row(tmpHome.dbPath, 'demo-api', 'clone-a'); + const b = row(tmpHome.dbPath, 'demo-api', 'clone-b'); + await fs.writeFile(registryPath, JSON.stringify([a, b])); + + const result = await syncGroup(makeConfig({ 'demo/api': 'demo-api' }), { + skipWrite: true, + resolveRepoHandle: async (_name, groupPath) => ({ + id: 'injected', + path: groupPath, + repoPath: a.path, + storagePath: a.storagePath, + }), + }); + + expect(result.missingRepos).toEqual([]); + expect(result.unreadableRepos).toEqual([]); + }); + + it('does not treat a filesystem path yaml value as a registry hit', async () => { + const known = row(tmpHome.dbPath, 'backend-repo', 'backend'); + await fs.writeFile(registryPath, JSON.stringify([known])); + + const result = await syncGroup(makeConfig({ 'app/backend': known.path }), { skipWrite: true }); + + expect(result.missingRepos).toEqual(['app/backend']); + expect(result.repoSnapshots['app/backend']).toBeUndefined(); + }); + + it('injected resolveRepoHandle plus workspace_deps does not throw on duplicate names', async () => { + const a = row(tmpHome.dbPath, 'demo-api', 'clone-a'); + const b = row(tmpHome.dbPath, 'demo-api', 'clone-b'); + await fs.writeFile(registryPath, JSON.stringify([a, b])); + + const result = await syncGroup( + makeConfig( + { 'demo/api': 'demo-api' }, + { + detect: { + http: false, + graphql: false, + grpc: false, + thrift: false, + topics: false, + includes: false, + workspace_deps: true, + }, + }, + ), + { + skipWrite: true, + resolveRepoHandle: async (_name, groupPath) => ({ + id: 'injected', + path: groupPath, + repoPath: a.path, + storagePath: a.storagePath, + }), + }, + ); + + expect(result.missingRepos).toEqual([]); + }); + + it('injected resolveRepoHandle plus workspace_deps still bypasses name lookup after extraction failure', async () => { + const a = row(tmpHome.dbPath, 'demo-api', 'clone-a'); + const b = row(tmpHome.dbPath, 'demo-api', 'clone-b'); + await fs.writeFile(registryPath, JSON.stringify([a, b])); + initLbugMock.mockRejectedValueOnce(new Error('init failed')); + + const result = await syncGroup( + makeConfig( + { 'demo/api': 'demo-api' }, + { + detect: { + http: false, + graphql: false, + grpc: false, + thrift: false, + topics: false, + includes: false, + workspace_deps: true, + }, + }, + ), + { + skipWrite: true, + resolveRepoHandle: async (_name, groupPath) => ({ + id: 'injected', + path: groupPath, + repoPath: a.path, + storagePath: a.storagePath, + }), + }, + ); + + expect(result.missingRepos).toEqual([]); + expect(result.unreadableRepos).toEqual(['demo/api']); + }); + + it('MCP groupSync returns { error } for an ambiguous registry name', async () => { + const a = row(tmpHome.dbPath, 'demo-api', 'clone-a'); + const b = row(tmpHome.dbPath, 'demo-api', 'clone-b'); + await fs.writeFile(registryPath, JSON.stringify([a, b])); + + const groupDir = path.join(tmpHome.dbPath, 'groups', 'g1'); + await fs.mkdir(groupDir, { recursive: true }); + await fs.writeFile( + path.join(groupDir, 'group.yaml'), + `version: 1 +name: g1 +repos: + demo/api: demo-api +`, + ); + + const port: GroupToolPort = { + resolveRepo: vi.fn(), + impact: vi.fn(), + query: vi.fn(), + impactByUid: vi.fn(), + context: vi.fn(), + }; + const svc = new GroupService(port); + const payload = (await svc.groupSync({ name: 'g1' })) as { error?: string }; + + expect(payload.error).toBeDefined(); + expect(payload.error).toContain('demo-api'); + expect(payload.error).toContain(a.path); + expect(payload.error).toContain(b.path); + expect(payload.error).toMatch(/unique registry name/i); + expect(payload.error).not.toMatch(/Pass the absolute path/); + }); +}); diff --git a/gitnexus/test/unit/group/sync-unreadable-repos.test.ts b/gitnexus/test/unit/group/sync-unreadable-repos.test.ts index 12a85439c..5f565d443 100644 --- a/gitnexus/test/unit/group/sync-unreadable-repos.test.ts +++ b/gitnexus/test/unit/group/sync-unreadable-repos.test.ts @@ -66,10 +66,14 @@ vi.mock('../../../src/core/lbug/pool-adapter.js', () => ({ getMaxResidentRepos: vi.fn(() => 5), })); -vi.mock('../../../src/storage/repo-manager.js', () => ({ - readRegistry: (...args: unknown[]) => readRegistryLenientMock(...args), - readRegistryStrict: (...args: unknown[]) => readRegistryStrictMock(...args), -})); +vi.mock('../../../src/storage/repo-manager.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + readRegistry: (...args: unknown[]) => readRegistryLenientMock(...args), + readRegistryStrict: (...args: unknown[]) => readRegistryStrictMock(...args), + }; +}); /** * Armed by the bridge-write-failure suite at the bottom of this file, `null` diff --git a/gitnexus/test/unit/group/sync-windowed-resolution.test.ts b/gitnexus/test/unit/group/sync-windowed-resolution.test.ts index aaf361b87..d96eeec26 100644 --- a/gitnexus/test/unit/group/sync-windowed-resolution.test.ts +++ b/gitnexus/test/unit/group/sync-windowed-resolution.test.ts @@ -156,10 +156,14 @@ vi.mock('../../../src/core/lbug/sidecar-recovery.js', () => ({ // The registry read happens in syncGroup's else branch; resolveRepoHandle is // supplied, so an empty registry is fine (only the meta.json fallback reads it). -vi.mock('../../../src/storage/repo-manager.js', () => ({ - readRegistry: vi.fn().mockResolvedValue([]), - readRegistryStrict: vi.fn().mockResolvedValue([]), -})); +vi.mock('../../../src/storage/repo-manager.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + readRegistry: vi.fn().mockResolvedValue([]), + readRegistryStrict: vi.fn().mockResolvedValue([]), + }; +}); const { syncGroup } = await import('../../../src/core/group/sync.js'); const { closeLbug, getMaxResidentRepos } = await import('../../../src/core/lbug/pool-adapter.js'); diff --git a/gitnexus/test/unit/repo-manager-registry-strict-read.test.ts b/gitnexus/test/unit/repo-manager-registry-strict-read.test.ts index 76d79e295..23241517d 100644 --- a/gitnexus/test/unit/repo-manager-registry-strict-read.test.ts +++ b/gitnexus/test/unit/repo-manager-registry-strict-read.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import fs from 'node:fs/promises'; import path from 'node:path'; import { inspect } from 'node:util'; -import { readRegistry, readRegistryStrict } from '../../src/storage/repo-manager.js'; +import { readRegistry, readRegistryStrict, registerRepo } from '../../src/storage/repo-manager.js'; import { _captureLogger, type LoggerCapture } from '../../src/core/logger.js'; import { createTempDir } from '../helpers/test-db.js'; import { syncGroup } from '../../src/core/group/sync.js'; @@ -116,6 +116,20 @@ describe('readRegistryStrict', () => { await expect(readRegistryStrict()).rejects.toThrow(); }); + it('registerRepo refuses to overwrite a truncated registry with a single entry', async () => { + const prior = '{"truncated": '; + await fs.writeFile(registryPath, prior); + await expect( + registerRepo('/repos/one', { + repoPath: '/repos/one', + lastCommit: 'abc', + indexedAt: '2026-01-01T00:00:00.000Z', + stats: {}, + }), + ).rejects.toThrow('registry is corrupt'); + expect(await fs.readFile(registryPath, 'utf-8')).toBe(prior); + }); + it('throws when a row is missing the fields the resolver needs', async () => { // `[{}]` is a JSON array, so an array-shape check alone waved it through. // Every configured repo then failed to resolve and landed in missingRepos; diff --git a/gitnexus/test/unit/repo-manager.test.ts b/gitnexus/test/unit/repo-manager.test.ts index 2080e2f45..468096a61 100644 --- a/gitnexus/test/unit/repo-manager.test.ts +++ b/gitnexus/test/unit/repo-manager.test.ts @@ -28,6 +28,7 @@ import { adoptFlatBranchLabel, listRegisteredRepos, resolveRegistryEntry, + findRegistryEntryByName, canonicalizePath, registryPathEquals, cloneDirBelongsToEntry, @@ -1535,6 +1536,13 @@ describe('resolveRegistryEntry (#664)', () => { expect(resolveRegistryEntry(entries, 'Website')).toBe(entries[2]); }); + it('findRegistryEntryByName is name-only: a filesystem path is a miss, not a path-tier hit', () => { + expect(findRegistryEntryByName(entries, pathA)).toBeUndefined(); + expect(findRegistryEntryByName(entries, 'website')).toBe(entries[2]); + expect(findRegistryEntryByName(entries, 'WEBSITE')).toBe(entries[2]); + expect(() => findRegistryEntryByName(entries, 'app')).toThrow(RegistryAmbiguousTargetError); + }); + it('path match is case-insensitive on Windows only', () => { if (process.platform !== 'win32') { // On POSIX, a differently-cased path must NOT match. Verify by diff --git a/gitnexus/test/unit/run-analyze-fts-repair.test.ts b/gitnexus/test/unit/run-analyze-fts-repair.test.ts index 24d03844f..091c7f915 100644 --- a/gitnexus/test/unit/run-analyze-fts-repair.test.ts +++ b/gitnexus/test/unit/run-analyze-fts-repair.test.ts @@ -359,6 +359,52 @@ describe('runFullAnalysis FTS repair and verification failure paths', () => { } }); + it('--repair-fts applies analyze --name without a full re-index', async () => { + vi.doMock('../../src/core/lbug/lbug-adapter.js', () => mockRepairSuccessLbugAdapter()); + vi.doMock('../../src/core/search/fts-indexes.js', () => ({ + initialiseSearchFTSStemmer: vi.fn(() => 'porter'), + createSearchFTSIndexes: vi.fn(async () => []), + verifySearchFTSIndexes: vi.fn(async () => []), + })); + vi.doMock('../../src/storage/repo-manager.js', async (importActual) => ({ + ...(await importActual()), + ensureGitNexusIgnored: vi.fn(async () => undefined), + })); + + const tmpRepo = await createTempDir('gitnexus-run-analyze-repair-name-'); + const tmpHome = await createTempDir('gitnexus-run-analyze-repair-name-home-'); + const savedHome = process.env.GITNEXUS_HOME; + process.env.GITNEXUS_HOME = tmpHome.dbPath; + try { + const { storagePath, lbugPath } = getStoragePaths(tmpRepo.dbPath); + await fs.mkdir(storagePath, { recursive: true }); + const seeded: RepoMeta = { + repoPath: tmpRepo.dbPath, + lastCommit: 'abc123', + indexedAt: new Date().toISOString(), + stats: { files: 1, nodes: 1, edges: 1 }, + }; + await saveMeta(storagePath, seeded); + const { registerRepo, readRegistry } = await import('../../src/storage/repo-manager.js'); + await registerRepo(tmpRepo.dbPath, seeded, { name: 'old' }); + await createPlaceholderGraphStore(lbugPath); + + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + const result = await runFullAnalysis( + tmpRepo.dbPath, + { repairFts: true, registryName: 'new' }, + { onProgress: () => {} }, + ); + expect(result.ftsRepairedOnly).toBe(true); + expect((await readRegistry())[0].name).toBe('new'); + } finally { + if (savedHome === undefined) delete process.env.GITNEXUS_HOME; + else process.env.GITNEXUS_HOME = savedHome; + await tmpHome.cleanup(); + await tmpRepo.cleanup(); + } + }); + it('--repair-fts backfills a full capabilities object when the existing meta predates the field entirely (#2767)', async () => { vi.doMock('../../src/core/lbug/lbug-adapter.js', () => mockRepairSuccessLbugAdapter()); vi.doMock('../../src/core/search/fts-indexes.js', () => ({ diff --git a/gitnexus/test/unit/run-analyze.test.ts b/gitnexus/test/unit/run-analyze.test.ts index aef31907c..ea4525b61 100644 --- a/gitnexus/test/unit/run-analyze.test.ts +++ b/gitnexus/test/unit/run-analyze.test.ts @@ -14,6 +14,8 @@ import { loadMeta, registerRepo, saveMeta, + readRegistry, + RegistryNameCollisionError, type RepoMeta, } from '../../src/storage/repo-manager.js'; import { SCHEMA_FINGERPRINT } from '../../src/core/lbug/schema.js'; @@ -90,6 +92,197 @@ describe('run-analyze module', () => { } }); + it('applies analyze --name on the already-up-to-date path without --force', async () => { + const tmpRepo = await createTempDir('gitnexus-run-analyze-fast-name-'); + const tmpHome = await createTempDir('gitnexus-run-analyze-fast-name-home-'); + const savedHome = process.env.GITNEXUS_HOME; + process.env.GITNEXUS_HOME = tmpHome.dbPath; + try { + execSync('git init', { cwd: tmpRepo.dbPath, stdio: 'pipe' }); + execSync('git -c user.name=t -c user.email=t@t commit --allow-empty -m init', { + cwd: tmpRepo.dbPath, + stdio: 'pipe', + }); + const currentCommit = execSync('git rev-parse HEAD', { + cwd: tmpRepo.dbPath, + encoding: 'utf-8', + }).trim(); + const { storagePath } = getStoragePaths(tmpRepo.dbPath); + const meta: RepoMeta = { + repoPath: tmpRepo.dbPath, + lastCommit: currentCommit, + indexedAt: new Date().toISOString(), + schemaFingerprint: SCHEMA_FINGERPRINT, + analysisFeatures: CURRENT_ANALYSIS_FEATURES, + runnerIdentity: currentRunnerIdentity(), + }; + await saveMeta(storagePath, meta); + await registerRepo(tmpRepo.dbPath, meta, { name: 'old' }); + + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + const result = await runFullAnalysis( + tmpRepo.dbPath, + { registryName: 'new' }, + { onProgress: () => {} }, + ); + + expect(result.alreadyUpToDate).toBe(true); + expect(result.repoName).toBe('new'); + const entries = await readRegistry(); + expect(entries).toHaveLength(1); + expect(entries[0].name).toBe('new'); + const agents = await fs.readFile(path.join(tmpRepo.dbPath, 'AGENTS.md'), 'utf-8'); + expect(agents).toContain('**new**'); + } finally { + if (savedHome === undefined) delete process.env.GITNEXUS_HOME; + else process.env.GITNEXUS_HOME = savedHome; + await tmpHome.cleanup(); + await tmpRepo.cleanup(); + } + }); + + it('repeating the same --name on the fast path is a no-op, not an error', async () => { + const tmpRepo = await createTempDir('gitnexus-run-analyze-fast-name-repeat-'); + const tmpHome = await createTempDir('gitnexus-run-analyze-fast-name-repeat-home-'); + const savedHome = process.env.GITNEXUS_HOME; + process.env.GITNEXUS_HOME = tmpHome.dbPath; + try { + execSync('git init', { cwd: tmpRepo.dbPath, stdio: 'pipe' }); + execSync('git -c user.name=t -c user.email=t@t commit --allow-empty -m init', { + cwd: tmpRepo.dbPath, + stdio: 'pipe', + }); + const currentCommit = execSync('git rev-parse HEAD', { + cwd: tmpRepo.dbPath, + encoding: 'utf-8', + }).trim(); + const { storagePath } = getStoragePaths(tmpRepo.dbPath); + const meta: RepoMeta = { + repoPath: tmpRepo.dbPath, + lastCommit: currentCommit, + indexedAt: new Date().toISOString(), + schemaFingerprint: SCHEMA_FINGERPRINT, + analysisFeatures: CURRENT_ANALYSIS_FEATURES, + runnerIdentity: currentRunnerIdentity(), + }; + await saveMeta(storagePath, meta); + await registerRepo(tmpRepo.dbPath, meta, { name: 'kept' }); + + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + const result = await runFullAnalysis( + tmpRepo.dbPath, + { registryName: 'kept' }, + { onProgress: () => {} }, + ); + + expect(result.alreadyUpToDate).toBe(true); + expect((await readRegistry())[0].name).toBe('kept'); + } finally { + if (savedHome === undefined) delete process.env.GITNEXUS_HOME; + else process.env.GITNEXUS_HOME = savedHome; + await tmpHome.cleanup(); + await tmpRepo.cleanup(); + } + }); + + it('fast-path --name still collides when another path already owns the alias', async () => { + const tmpA = await createTempDir('gitnexus-run-analyze-fast-name-col-a-'); + const tmpB = await createTempDir('gitnexus-run-analyze-fast-name-col-b-'); + const tmpHome = await createTempDir('gitnexus-run-analyze-fast-name-col-home-'); + const savedHome = process.env.GITNEXUS_HOME; + process.env.GITNEXUS_HOME = tmpHome.dbPath; + try { + for (const tmp of [tmpA, tmpB]) { + execSync('git init', { cwd: tmp.dbPath, stdio: 'pipe' }); + execSync('git -c user.name=t -c user.email=t@t commit --allow-empty -m init', { + cwd: tmp.dbPath, + stdio: 'pipe', + }); + } + const commitB = execSync('git rev-parse HEAD', { + cwd: tmpB.dbPath, + encoding: 'utf-8', + }).trim(); + const metaA: RepoMeta = { + repoPath: tmpA.dbPath, + lastCommit: 'aaaa', + indexedAt: new Date().toISOString(), + schemaFingerprint: SCHEMA_FINGERPRINT, + analysisFeatures: CURRENT_ANALYSIS_FEATURES, + runnerIdentity: currentRunnerIdentity(), + }; + await registerRepo(tmpA.dbPath, metaA, { name: 'new' }); + + const { storagePath } = getStoragePaths(tmpB.dbPath); + const metaB: RepoMeta = { + repoPath: tmpB.dbPath, + lastCommit: commitB, + indexedAt: new Date().toISOString(), + schemaFingerprint: SCHEMA_FINGERPRINT, + analysisFeatures: CURRENT_ANALYSIS_FEATURES, + runnerIdentity: currentRunnerIdentity(), + }; + await saveMeta(storagePath, metaB); + await registerRepo(tmpB.dbPath, metaB, { name: 'old' }); + + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + await expect( + runFullAnalysis(tmpB.dbPath, { registryName: 'new' }, { onProgress: () => {} }), + ).rejects.toBeInstanceOf(RegistryNameCollisionError); + } finally { + if (savedHome === undefined) delete process.env.GITNEXUS_HOME; + else process.env.GITNEXUS_HOME = savedHome; + await tmpHome.cleanup(); + await tmpA.cleanup(); + await tmpB.cleanup(); + } + }); + + it('plain fast path does not call registerRepo when --name is absent', async () => { + const tmpRepo = await createTempDir('gitnexus-run-analyze-fast-no-name-'); + const tmpHome = await createTempDir('gitnexus-run-analyze-fast-no-name-home-'); + const savedHome = process.env.GITNEXUS_HOME; + process.env.GITNEXUS_HOME = tmpHome.dbPath; + try { + execSync('git init', { cwd: tmpRepo.dbPath, stdio: 'pipe' }); + execSync('git -c user.name=t -c user.email=t@t commit --allow-empty -m init', { + cwd: tmpRepo.dbPath, + stdio: 'pipe', + }); + const currentCommit = execSync('git rev-parse HEAD', { + cwd: tmpRepo.dbPath, + encoding: 'utf-8', + }).trim(); + const { storagePath } = getStoragePaths(tmpRepo.dbPath); + const meta: RepoMeta = { + repoPath: tmpRepo.dbPath, + lastCommit: currentCommit, + indexedAt: new Date().toISOString(), + schemaFingerprint: SCHEMA_FINGERPRINT, + analysisFeatures: CURRENT_ANALYSIS_FEATURES, + runnerIdentity: currentRunnerIdentity(), + }; + await saveMeta(storagePath, meta); + await registerRepo(tmpRepo.dbPath, meta, { name: 'original' }); + + const registerSpy = vi.spyOn( + await import('../../src/storage/repo-manager.js'), + 'registerRepo', + ); + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + const result = await runFullAnalysis(tmpRepo.dbPath, {}, { onProgress: () => {} }); + expect(result.alreadyUpToDate).toBe(true); + expect(registerSpy).not.toHaveBeenCalled(); + expect((await readRegistry())[0].name).toBe('original'); + registerSpy.mockRestore(); + } finally { + if (savedHome === undefined) delete process.env.GITNEXUS_HOME; + else process.env.GITNEXUS_HOME = savedHome; + await tmpHome.cleanup(); + await tmpRepo.cleanup(); + } + }); + it('resumes a matching embedding checkpoint instead of taking the clean fast path', async () => { const tmpRepo = await createTempDir('gitnexus-run-analyze-embedding-checkpoint-'); const tmpHome = await createTempDir('gitnexus-run-analyze-embedding-checkpoint-home-');