From d52dbc94528183977cd888e06a34d12bc6dcf1fb Mon Sep 17 00:00:00 2001 From: azizur100389 Date: Sun, 19 Apr 2026 07:23:48 +0100 Subject: [PATCH] feat(cli): analyze --name + duplicate-name guard for the repo registry (#955) --- gitnexus/src/cli/analyze.ts | 46 ++++- gitnexus/src/cli/index.ts | 10 ++ gitnexus/src/cli/list.ts | 13 +- gitnexus/src/core/run-analyze.ts | 31 +++- gitnexus/src/mcp/local/local-backend.ts | 22 ++- gitnexus/src/storage/repo-manager.ts | 116 ++++++++++++- gitnexus/test/integration/cli-e2e.test.ts | 201 ++++++++++++++++++++++ gitnexus/test/unit/repo-manager.test.ts | 138 +++++++++++++++ 8 files changed, 564 insertions(+), 13 deletions(-) diff --git a/gitnexus/src/cli/analyze.ts b/gitnexus/src/cli/analyze.ts index 1e75ea6753..46cedc4349 100644 --- a/gitnexus/src/cli/analyze.ts +++ b/gitnexus/src/cli/analyze.ts @@ -13,7 +13,11 @@ import { execFileSync } from 'child_process'; import v8 from 'v8'; import cliProgress from 'cli-progress'; import { closeLbug } from '../core/lbug/lbug-adapter.js'; -import { getStoragePaths, getGlobalRegistryPath } from '../storage/repo-manager.js'; +import { + getStoragePaths, + getGlobalRegistryPath, + RegistryNameCollisionError, +} from '../storage/repo-manager.js'; import { getGitRoot, hasGitDir } from '../storage/git.js'; import { runFullAnalysis } from '../core/run-analyze.js'; import fs from 'fs/promises'; @@ -59,6 +63,21 @@ export interface AnalyzeOptions { noStats?: boolean; /** Index the folder even when no .git directory is present. */ skipGit?: boolean; + /** + * Override the default basename-derived registry `name` with a + * user-supplied alias (#829). Disambiguates repos whose paths share a + * basename. Persisted — subsequent re-analyses of the same path without + * `--name` preserve the alias. + */ + name?: string; + /** + * Allow registration even when another path already uses the same + * `--name` alias (#829). Intentionally a distinct flag from `--force` + * because the user may want to coexist under the same name WITHOUT + * paying the cost of a pipeline re-index. Maps to registerRepo's + * `allowDuplicateName` option end-to-end. + */ + allowDuplicateName?: boolean; } export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOptions) => { @@ -186,11 +205,20 @@ export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOption const result = await runFullAnalysis( repoPath, { + // Pipeline re-index — OR'd with --skills because skill generation + // needs a fresh pipelineResult. Has no bearing on the registry + // collision guard (see allowDuplicateName below). force: options?.force || options?.skills, embeddings: options?.embeddings, skipGit: options?.skipGit, skipAgentsMd: options?.skipAgentsMd, noStats: options?.noStats, + registryName: options?.name, + // Registry-collision bypass — its own CLI flag, intentionally NOT + // overloading --force. A user who hits the collision guard should + // be able to accept the duplicate name without also paying the + // cost of a full pipeline re-index. See #829 review round 2. + allowDuplicateName: options?.allowDuplicateName, }, { onProgress: (_phase, percent, message) => { @@ -298,6 +326,22 @@ export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOption bar.stop(); const msg = err.message || String(err); + + // Registry name-collision from --name (#829) — surface as an + // actionable error rather than a generic stack-trace. + if (err instanceof RegistryNameCollisionError) { + console.error(`\n Registry name collision:\n`); + console.error(` "${err.registryName}" is already used by "${err.existingPath}".\n`); + console.error(` Options:`); + console.error(` • Pick a different alias: gitnexus analyze --name `); + console.error( + ` • Allow the duplicate: gitnexus analyze --allow-duplicate-name (leaves "-r ${err.registryName}" ambiguous)`, + ); + console.error(''); + process.exitCode = 1; + return; + } + console.error(`\n Analysis failed: ${msg}\n`); // Provide helpful guidance for known failure modes diff --git a/gitnexus/src/cli/index.ts b/gitnexus/src/cli/index.ts index 02581ae478..dca5983e0c 100644 --- a/gitnexus/src/cli/index.ts +++ b/gitnexus/src/cli/index.ts @@ -28,6 +28,16 @@ program .option('--skip-agents-md', 'Skip updating the gitnexus section in AGENTS.md and CLAUDE.md') .option('--no-stats', 'Omit volatile file/symbol counts from AGENTS.md and CLAUDE.md') .option('--skip-git', 'Index a folder without requiring a .git directory') + .option( + '--name ', + 'Register this repo under a custom name in ~/.gitnexus/registry.json ' + + '(disambiguates repos whose paths share a basename, e.g. two different .../app folders)', + ) + .option( + '--allow-duplicate-name', + 'Register this repo even if another path already uses the same --name alias. ' + + 'Leaves `-r ` ambiguous for the two paths; use -r to disambiguate.', + ) .option('-v, --verbose', 'Enable verbose ingestion warnings (default: false)') .addHelpText( 'after', diff --git a/gitnexus/src/cli/list.ts b/gitnexus/src/cli/list.ts index 722c8ad596..5da9a86f0e 100644 --- a/gitnexus/src/cli/list.ts +++ b/gitnexus/src/cli/list.ts @@ -17,12 +17,23 @@ export const listCommand = async () => { console.log(`\n Indexed Repositories (${entries.length})\n`); + // Count occurrences of each name so colliding entries can be + // disambiguated in the header (#829). Unique-name entries render + // identically to pre-#829 output; only collisions gain a suffix. + const nameCounts = new Map(); + for (const e of entries) { + const key = e.name.toLowerCase(); + nameCounts.set(key, (nameCounts.get(key) ?? 0) + 1); + } + for (const entry of entries) { const indexedDate = new Date(entry.indexedAt).toLocaleString(); const stats = entry.stats || {}; const commitShort = entry.lastCommit?.slice(0, 7) || 'unknown'; + const hasCollision = (nameCounts.get(entry.name.toLowerCase()) ?? 0) > 1; + const header = hasCollision ? `${entry.name} (${entry.path})` : entry.name; - console.log(` ${entry.name}`); + console.log(` ${header}`); console.log(` Path: ${entry.path}`); console.log(` Indexed: ${indexedDate}`); console.log(` Commit: ${commitShort}`); diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index 910624f5ff..b17fb8e573 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -46,6 +46,12 @@ export interface AnalyzeCallbacks { } export interface AnalyzeOptions { + /** + * Force a full re-index of the pipeline. Callers may OR this with + * other flags that imply re-analysis (e.g. `--skills`), so the value + * here is the PIPELINE-force signal, NOT the registry-collision + * bypass. See `allowDuplicateName` below. + */ force?: boolean; embeddings?: boolean; skipGit?: boolean; @@ -53,6 +59,21 @@ export interface AnalyzeOptions { skipAgentsMd?: boolean; /** Omit volatile symbol/relationship counts from AGENTS.md and CLAUDE.md. */ noStats?: boolean; + /** + * User-provided alias for the registry `name` (#829). When set, + * forwarded to `registerRepo` so the indexed repo is stored under + * this alias instead of the path-derived basename. + */ + registryName?: string; + /** + * Bypass the `RegistryNameCollisionError` guard and allow two paths + * to register under the same `name` (#829). Controlled by the + * dedicated `--allow-duplicate-name` CLI flag, intentionally + * independent from `--force` — users who hit the collision guard + * should be able to accept the duplicate without paying the cost + * of a pipeline re-index. + */ + allowDuplicateName?: boolean; } export interface AnalyzeResult { @@ -313,7 +334,15 @@ export async function runFullAnalysis( }, }; await saveMeta(storagePath, meta); - await registerRepo(repoPath, meta); + // Forward the --name alias and the registry-collision bypass bit. + // `allowDuplicateName` is its own concern — independent from the + // pipeline `force` above. The CLI maps it from + // `--allow-duplicate-name` only; `--force` and `--skills` both + // trigger pipeline re-run but never bypass the registry guard. + await registerRepo(repoPath, meta, { + name: options.registryName, + allowDuplicateName: options.allowDuplicateName, + }); // Only attempt to update .gitignore when a .git directory is present. if (hasGitDir(repoPath)) { diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 5b73885cd2..55157cf054 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -342,13 +342,25 @@ export class LocalBackend { if (this.repos.size === 0) { throw new Error('No indexed repositories. Run: gitnexus analyze'); } - if (repoParam) { - const names = [...this.repos.values()].map((h) => h.name); - throw new Error(`Repository "${repoParam}" not found. Available: ${names.join(', ')}`); + + // Build a disambiguated "Available: …" list (#829). When two handles + // share a name, annotate each colliding label with its path so the + // caller can actually pick the right one. Single-name entries render + // identically to pre-#829 output. + const nameCounts = new Map(); + for (const h of this.repos.values()) { + const key = h.name.toLowerCase(); + nameCounts.set(key, (nameCounts.get(key) ?? 0) + 1); + } + const labels = [...this.repos.values()].map((h) => + (nameCounts.get(h.name.toLowerCase()) ?? 0) > 1 ? `${h.name} (${h.repoPath})` : h.name, + ); + + if (repoParam) { + throw new Error(`Repository "${repoParam}" not found. Available: ${labels.join(', ')}`); } - const names = [...this.repos.values()].map((h) => h.name); throw new Error( - `Multiple repositories indexed. Specify which one with the "repo" parameter. Available: ${names.join(', ')}`, + `Multiple repositories indexed. Specify which one with the "repo" parameter. Available: ${labels.join(', ')}`, ); } diff --git a/gitnexus/src/storage/repo-manager.ts b/gitnexus/src/storage/repo-manager.ts index b233d44fb8..4ba17b21b7 100644 --- a/gitnexus/src/storage/repo-manager.ts +++ b/gitnexus/src/storage/repo-manager.ts @@ -244,21 +244,127 @@ const writeRegistry = async (entries: RegistryEntry[]): Promise => { await fs.writeFile(getGlobalRegistryPath(), JSON.stringify(entries, null, 2), 'utf-8'); }; +/** + * Options for {@link registerRepo}. All optional — callers without any + * disambiguation requirement can keep calling `registerRepo(path, meta)` + * unchanged. + */ +export interface RegisterRepoOptions { + /** + * User-provided alias from `analyze --name ` (#829). Overrides + * the default basename-derived registry `name`. Persisted — subsequent + * re-analyses of the same path without `--name` preserve the alias. + */ + name?: string; + /** + * Allow two DIFFERENT repo paths to register under the same alias + * (#829). Mapped from the `--allow-duplicate-name` CLI flag. + * + * Scope: this flag governs cross-path alias sharing only — one repo + * path always has exactly one registry entry (and therefore exactly + * one alias). Re-analyzing the same path with `--name Y` overwrites + * a previous `--name X`; it does NOT create a second entry or a + * second alias for the same path (see the upsert-by-resolved-path + * logic in {@link registerRepo} and the + * `re-registerRepo with a different name overrides the previous + * alias` test in `test/unit/repo-manager.test.ts`). + * + * Distinct from `--force` (which only triggers pipeline re-index); + * a user accepting a duplicate alias should not be forced to also + * re-run the full pipeline. + */ + allowDuplicateName?: boolean; +} + +/** + * Thrown by {@link registerRepo} when a requested name is already in + * use by a DIFFERENT path. The CLI layer surfaces this as an actionable + * error instead of relying on `.message` string-matching. + * + * The colliding alias is exposed as `err.registryName` (not `err.name`). + * `err.name` keeps its inherited `Error.prototype.name` semantics (the + * class name) so downstream code can do the usual `err.name === + * 'RegistryNameCollisionError'` checks; use the `kind` discriminant or + * `instanceof RegistryNameCollisionError` for type-safe narrowing. + */ +export class RegistryNameCollisionError extends Error { + readonly kind = 'RegistryNameCollisionError' as const; + constructor( + public readonly registryName: string, + public readonly existingPath: string, + public readonly requestedPath: string, + ) { + super( + `Registry name "${registryName}" is already used by "${existingPath}".\n` + + `Pass --name to register "${requestedPath}" under a different name, ` + + `or --allow-duplicate-name to allow both paths under the same name (leaves -r ambiguous for these two).`, + ); + this.name = 'RegistryNameCollisionError'; + } +} + +/** Returns true when a previously-registered entry's `name` differs from + * `path.basename(entry.path)` — i.e. a user explicitly aliased it via + * `analyze --name ` on a prior run. Used to preserve the alias + * across re-analyses that omit `--name`. */ +const hasCustomAlias = (entry: RegistryEntry): boolean => { + return entry.name !== path.basename(path.resolve(entry.path)); +}; + /** * Register (add or update) a repo in the global registry. * Called after `gitnexus analyze` completes. + * + * Name resolution precedence (#829): + * 1. explicit `opts.name` (from `analyze --name `) + * 2. preserved alias on an existing entry for this path + * 3. `path.basename(repoPath)` (the original default) + * + * Duplicate-name guard: if another path already uses the resolved + * `name`, throw {@link RegistryNameCollisionError} unless + * `opts.allowDuplicateName` is set. The guard ONLY fires when the user explicitly passed a + * `name`; un-aliased basename collisions continue to register silently + * so existing users who don't know about `--name` see no behaviour + * change. */ -export const registerRepo = async (repoPath: string, meta: RepoMeta): Promise => { +export const registerRepo = async ( + repoPath: string, + meta: RepoMeta, + opts?: RegisterRepoOptions, +): Promise => { const resolved = path.resolve(repoPath); - const name = path.basename(resolved); const { storagePath } = getStoragePaths(resolved); const entries = await readRegistry(); - const existing = entries.findIndex((e) => { + const existingIdx = entries.findIndex((e) => { const a = path.resolve(e.path); const b = resolved; return process.platform === 'win32' ? a.toLowerCase() === b.toLowerCase() : a === b; }); + const existing = existingIdx >= 0 ? entries[existingIdx] : null; + + // Precedence: explicit --name > preserved alias > basename. + const name = + opts?.name ?? (existing && hasCustomAlias(existing) ? existing.name : path.basename(resolved)); + + // Duplicate-name guard: only fire when the user EXPLICITLY asked for + // this name (via opts.name or a preserved alias). Unqualified basename + // collisions are preserved for backward-compat — they still register, + // and the user sees the ambiguity at `-r` / `list` resolution time + // (which is already improved by the disambiguated error messages and + // list output this PR also ships). + const explicitName = opts?.name !== undefined || (existing && hasCustomAlias(existing)); + if (explicitName && !opts?.allowDuplicateName) { + const collidingEntry = entries.find( + (e, i) => + i !== existingIdx && + e.name.toLowerCase() === name.toLowerCase() && + path.resolve(e.path) !== resolved, + ); + if (collidingEntry) { + throw new RegistryNameCollisionError(name, collidingEntry.path, resolved); + } + } const entry: RegistryEntry = { name, @@ -269,8 +375,8 @@ export const registerRepo = async (repoPath: string, meta: RepoMeta): Promise= 0) { - entries[existing] = entry; + if (existingIdx >= 0) { + entries[existingIdx] = entry; } else { entries.push(entry); } diff --git a/gitnexus/test/integration/cli-e2e.test.ts b/gitnexus/test/integration/cli-e2e.test.ts index abdd436a4d..844e3978d6 100644 --- a/gitnexus/test/integration/cli-e2e.test.ts +++ b/gitnexus/test/integration/cli-e2e.test.ts @@ -110,6 +110,55 @@ function runCliRaw(extraArgs: string[], cwd: string, timeoutMs = 15000) { }); } +/** + * Like runCliRaw but accepts extra env vars. Used by tests that need to + * isolate the global registry via GITNEXUS_HOME so they don't touch the + * developer / CI agent's real ~/.gitnexus/registry.json (#829). + */ +function runCliWithEnv( + extraArgs: string[], + cwd: string, + extraEnv: Record, + timeoutMs = 15000, +) { + return spawnSync(process.execPath, ['--import', tsxImportUrl, cliEntry, ...extraArgs], { + cwd, + encoding: 'utf8', + timeout: timeoutMs, + stdio: ['pipe', 'pipe', 'pipe'], + env: { + ...process.env, + NODE_OPTIONS: `${process.env.NODE_OPTIONS || ''} --max-old-space-size=8192`.trim(), + ...extraEnv, + }, + }); +} + +/** + * Create a fresh git-initialised throwaway repo at `/` + * and return its path. Used for tests that need multiple repos whose + * basenames intentionally collide (#829 reproduction). + */ +function makeMiniRepoCopy(basename: string, prefix: string): string { + const parent = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + const repo = path.join(parent, basename); + fs.cpSync(FIXTURE_SRC, repo, { recursive: true }); + spawnSync('git', ['init'], { cwd: repo, stdio: 'pipe' }); + spawnSync('git', ['add', '-A'], { cwd: repo, stdio: 'pipe' }); + spawnSync('git', ['commit', '-m', 'initial commit'], { + cwd: repo, + stdio: 'pipe', + env: { + ...process.env, + GIT_AUTHOR_NAME: 'test', + GIT_AUTHOR_EMAIL: 'test@test', + GIT_COMMITTER_NAME: 'test', + GIT_COMMITTER_EMAIL: 'test@test', + }, + }); + return repo; +} + describe('CLI end-to-end', () => { it('status command exits cleanly', () => { const result = runCli('status', MINI_REPO); @@ -144,6 +193,158 @@ describe('CLI end-to-end', () => { expect(fs.statSync(gitnexusDir).isDirectory()).toBe(true); }); + // ─── analyze --name + --allow-duplicate-name (#829) ────── + // + // End-to-end regression guard for the name-collision feature: + // 1. `analyze --name X` persists the alias to ~/.gitnexus/registry.json + // 2. A second `analyze --name X` on a DIFFERENT path is rejected with + // a collision error (exit code 1, "already used" in output) + // 3. `analyze --name X --allow-duplicate-name` bypasses the guard; + // both entries coexist in registry.json + // 4. Pipeline-re-index flags (e.g. --skills) WITHOUT + // --allow-duplicate-name must STILL hit the collision guard — + // the bypass must stay gated on its dedicated flag so it isn't + // silently triggered by unrelated pipeline signals + // (review round 2/3 design decision). + // + // This test invokes the real CLI → runFullAnalysis → registerRepo + // chain, so any wiring regression fails here. + describe('analyze --name and --allow-duplicate-name (#829)', () => { + // Path-equality assertions across CLI spawn boundaries are fragile + // cross-platform: + // - macOS: os.tmpdir() returns /var/folders/...; child processes + // resolve the symlink to /private/var/folders/... + // - Windows: os.tmpdir() on GitHub runners returns 8.3 short-name + // form (C:\Users\RUNNER~1\...); the child sees the long form + // (C:\Users\runneradmin\...). fs.realpathSync does NOT reliably + // expand 8.3 to long form. + // Rather than fight the platform-path quagmire, we assert STRUCTURAL + // properties: entry count, alias value, path basename, path + // distinctness. That covers the behavior this test is here to + // protect without depending on exact-string path equality. + + it('--name alias stores; collision rejects; --allow-duplicate-name bypasses', () => { + // Isolate the global registry so this test never touches the + // developer's real ~/.gitnexus. + const gnHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-home-')); + + // Two mini-repo copies whose basenames intentionally collide. + const repoA = makeMiniRepoCopy('collide-app', 'gn-collide-a-'); + const repoB = makeMiniRepoCopy('collide-app', 'gn-collide-b-'); + const parentA = path.dirname(repoA); + const parentB = path.dirname(repoB); + + try { + // Step 1: analyze repoA with --name shared → registry entry created. + const r1 = runCliWithEnv( + ['analyze', '--name', 'shared'], + repoA, + { GITNEXUS_HOME: gnHome }, + 60000, + ); + if (r1.status === null) return; // CI timeout tolerance + expect( + r1.status, + [`step 1 exited with ${r1.status}`, `stdout: ${r1.stdout}`, `stderr: ${r1.stderr}`].join( + '\n', + ), + ).toBe(0); + + const registryPath = path.join(gnHome, 'registry.json'); + const afterStep1 = JSON.parse(fs.readFileSync(registryPath, 'utf-8')); + expect(Array.isArray(afterStep1)).toBe(true); + expect(afterStep1).toHaveLength(1); + expect(afterStep1[0].name).toBe('shared'); + expect(path.basename(afterStep1[0].path)).toBe('collide-app'); + + // Step 2: analyze repoB with the SAME --name → collision error. + const r2 = runCliWithEnv( + ['analyze', '--name', 'shared'], + repoB, + { GITNEXUS_HOME: gnHome }, + 60000, + ); + if (r2.status === null) return; + expect(r2.status).toBe(1); + const r2Output = `${r2.stdout}${r2.stderr}`; + expect(r2Output).toMatch(/Registry name collision|already used/i); + + // Registry still has just the first entry — step 2 must not have + // silently added, overwritten, or corrupted anything. + const afterStep2 = JSON.parse(fs.readFileSync(registryPath, 'utf-8')); + expect(afterStep2).toHaveLength(1); + // Registry still has only the step-1 entry — the failed call + // must not have silently added, overwritten, or corrupted state. + expect(afterStep2[0].path).toBe(afterStep1[0].path); + + // Step 3: REGRESSION GUARD for the missing collision-bypass wire + // (originally a --force passthrough bug; per review round 3 the + // bypass moved to its own --allow-duplicate-name flag to avoid + // conflating it with pipeline re-index). + const r3 = runCliWithEnv( + ['analyze', '--name', 'shared', '--allow-duplicate-name'], + repoB, + { GITNEXUS_HOME: gnHome }, + 60000, + ); + if (r3.status === null) return; + expect( + r3.status, + [ + `step 3 (--allow-duplicate-name bypass) exited with ${r3.status}`, + `stdout: ${r3.stdout}`, + `stderr: ${r3.stderr}`, + ].join('\n'), + ).toBe(0); + + const afterStep3 = JSON.parse(fs.readFileSync(registryPath, 'utf-8')); + expect(afterStep3).toHaveLength(2); + expect(afterStep3.every((e: { name: string }) => e.name === 'shared')).toBe(true); + // Both entries point to distinct paths (we registered two different + // repos under the same alias) and both have the right basename. + const step3Basenames = afterStep3.map((e: { path: string }) => path.basename(e.path)); + expect(step3Basenames).toEqual(['collide-app', 'collide-app']); + const step3Paths = new Set(afterStep3.map((e: { path: string }) => e.path)); + expect(step3Paths.size).toBe(2); + // One of the two entries is the original from step 1 — unchanged. + expect(afterStep3.map((e: { path: string }) => e.path)).toContain(afterStep1[0].path); + + // Step 4: REGRESSION GUARD for the design decision in review + // round 2/3 — pipeline-re-index flags must NOT bypass the + // registry collision guard. `--skills` triggers pipeline + // re-run (skills generation needs a fresh pipelineResult) but + // must leave the registry guard in force. Bypass requires the + // explicit --allow-duplicate-name flag. + const repoC = makeMiniRepoCopy('collide-app', 'gn-collide-c-'); + const parentC = path.dirname(repoC); + try { + const r4 = runCliWithEnv( + ['analyze', '--name', 'shared', '--skills'], + repoC, + { GITNEXUS_HOME: gnHome }, + 60000, + ); + if (r4.status === null) return; + expect(r4.status).toBe(1); + const r4Output = `${r4.stdout}${r4.stderr}`; + expect(r4Output).toMatch(/Registry name collision|already used/i); + // The error hint should point at the new flag. + expect(r4Output).toMatch(/--allow-duplicate-name/); + + // Registry unchanged — still only A + B under "shared". + const afterStep4 = JSON.parse(fs.readFileSync(registryPath, 'utf-8')); + expect(afterStep4).toHaveLength(2); + } finally { + fs.rmSync(parentC, { recursive: true, force: true }); + } + } finally { + fs.rmSync(gnHome, { recursive: true, force: true }); + fs.rmSync(parentA, { recursive: true, force: true }); + fs.rmSync(parentB, { recursive: true, force: true }); + } + }, 360000); // 6-min outer budget (4 × ~60s analyze calls + fixture setup) + }); + describe('unhappy path', () => { it('exits with error when no command is given', () => { const result = runCliRaw([], MINI_REPO); diff --git a/gitnexus/test/unit/repo-manager.test.ts b/gitnexus/test/unit/repo-manager.test.ts index 08afceaff8..b9eabccb43 100644 --- a/gitnexus/test/unit/repo-manager.test.ts +++ b/gitnexus/test/unit/repo-manager.test.ts @@ -13,6 +13,10 @@ import { getStoragePaths, readRegistry, loadCLIConfig, + registerRepo, + listRegisteredRepos, + RegistryNameCollisionError, + type RepoMeta, } from '../../src/storage/repo-manager.js'; import { createTempDir } from '../helpers/test-db.js'; @@ -133,3 +137,137 @@ describe('API key file permissions', () => { expect(source).toContain("process.platform !== 'win32'"); }); }); + +// ─── analyze --name + duplicate-name guard (#829) ──────────── +// +// Each test isolates the global registry by pointing GITNEXUS_HOME at a +// per-test tmpdir. `getGlobalDir()` honors that env var, so registerRepo +// writes/reads a sandboxed registry.json without touching the user's +// real ~/.gitnexus. + +describe('registerRepo name override + collision guard (#829)', () => { + let tmpHome: Awaited>; + let tmpRepoA: Awaited>; + let tmpRepoB: Awaited>; + let savedGitnexusHome: string | undefined; + + const meta: RepoMeta = { + repoPath: '', + lastCommit: 'abc1234', + indexedAt: '2026-04-18T12:00:00.000Z', + stats: { files: 1, nodes: 1 }, + }; + + beforeEach(async () => { + tmpHome = await createTempDir('gitnexus-registry-home-'); + tmpRepoA = await createTempDir('gitnexus-repo-a-'); + tmpRepoB = await createTempDir('gitnexus-repo-b-'); + savedGitnexusHome = process.env.GITNEXUS_HOME; + process.env.GITNEXUS_HOME = tmpHome.dbPath; + }); + + afterEach(async () => { + if (savedGitnexusHome === undefined) delete process.env.GITNEXUS_HOME; + else process.env.GITNEXUS_HOME = savedGitnexusHome; + await tmpHome.cleanup(); + await tmpRepoA.cleanup(); + await tmpRepoB.cleanup(); + }); + + it('registerRepo({ name: "alias" }) stores the alias instead of basename', async () => { + await registerRepo(tmpRepoA.dbPath, meta, { name: 'custom-alias' }); + + const entries = await listRegisteredRepos(); + expect(entries).toHaveLength(1); + expect(entries[0].name).toBe('custom-alias'); + expect(entries[0].name).not.toBe(path.basename(tmpRepoA.dbPath)); + }); + + it('re-registerRepo on same path without name preserves an existing alias', async () => { + await registerRepo(tmpRepoA.dbPath, meta, { name: 'custom-alias' }); + // Second call with no opts should keep the alias, not revert to basename. + await registerRepo(tmpRepoA.dbPath, meta); + + const entries = await listRegisteredRepos(); + expect(entries).toHaveLength(1); + expect(entries[0].name).toBe('custom-alias'); + }); + + it('re-registerRepo with a different name overrides the previous alias', async () => { + await registerRepo(tmpRepoA.dbPath, meta, { name: 'old-alias' }); + await registerRepo(tmpRepoA.dbPath, meta, { name: 'new-alias' }); + + const entries = await listRegisteredRepos(); + expect(entries).toHaveLength(1); + expect(entries[0].name).toBe('new-alias'); + }); + + it('registerRepo throws RegistryNameCollisionError when another path uses the name', async () => { + await registerRepo(tmpRepoA.dbPath, meta, { name: 'shared' }); + + await expect(registerRepo(tmpRepoB.dbPath, meta, { name: 'shared' })).rejects.toBeInstanceOf( + RegistryNameCollisionError, + ); + + // And the colliding entry in the error carries enough info for the + // CLI layer to surface an actionable message without string-matching. + try { + await registerRepo(tmpRepoB.dbPath, meta, { name: 'shared' }); + } catch (e) { + expect(e).toBeInstanceOf(RegistryNameCollisionError); + const err = e as RegistryNameCollisionError; + // err.registryName carries the colliding alias (exposed as its own + // field so err.name retains the inherited Error.prototype.name + // semantics for downstream `err.name === '…Error'` checks). + expect(err.registryName).toBe('shared'); + expect(err.name).toBe('RegistryNameCollisionError'); + expect(path.resolve(err.existingPath)).toBe(path.resolve(tmpRepoA.dbPath)); + expect(path.resolve(err.requestedPath)).toBe(path.resolve(tmpRepoB.dbPath)); + } + + // Registry still only has the first entry — the failed call didn't + // corrupt state. + const entries = await listRegisteredRepos(); + expect(entries).toHaveLength(1); + expect(entries[0].name).toBe('shared'); + }); + + it('registerRepo({ name, allowDuplicateName: true }) allows the duplicate to coexist', async () => { + await registerRepo(tmpRepoA.dbPath, meta, { name: 'shared' }); + await registerRepo(tmpRepoB.dbPath, meta, { name: 'shared', allowDuplicateName: true }); + + const entries = await listRegisteredRepos(); + expect(entries).toHaveLength(2); + expect(entries.every((e) => e.name === 'shared')).toBe(true); + // Both paths are stored distinctly — the collision is surfaced to the + // user via resolveRepo / list output, not hidden at the storage layer. + const paths = entries.map((e) => path.resolve(e.path)).sort(); + expect(paths).toEqual([path.resolve(tmpRepoA.dbPath), path.resolve(tmpRepoB.dbPath)].sort()); + }); + + it('basename collisions without an explicit --name still register silently (backward-compat)', async () => { + // Create two sibling dirs whose basenames collide. Neither caller + // passes { name }, so the guard must NOT fire — this preserves the + // pre-#829 behaviour for users who don't know about --name yet. + const parentA = await createTempDir('gitnexus-collide-parent-a-'); + const parentB = await createTempDir('gitnexus-collide-parent-b-'); + const sharedBasename = 'app'; + const pathA = path.join(parentA.dbPath, sharedBasename); + const pathB = path.join(parentB.dbPath, sharedBasename); + await fs.mkdir(pathA, { recursive: true }); + await fs.mkdir(pathB, { recursive: true }); + + try { + await registerRepo(pathA, meta); + await registerRepo(pathB, meta); // must NOT throw + + const entries = await listRegisteredRepos(); + expect(entries).toHaveLength(2); + expect(entries[0].name).toBe(sharedBasename); + expect(entries[1].name).toBe(sharedBasename); + } finally { + await parentA.cleanup(); + await parentB.cleanup(); + } + }); +});