mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-29 06:32:28 +00:00
feat(cli): analyze --name <alias> + duplicate-name guard for the repo registry (#955)
This commit is contained in:
azizur100389
GitHub
parent
d78880d887
commit
d52dbc9452
8 changed files with 564 additions and 13 deletions
|
|
@ -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 <alias>`);
|
||||
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
|
||||
|
|
|
|||
|
|
@ -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 <alias>',
|
||||
'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 <name>` ambiguous for the two paths; use -r <path> to disambiguate.',
|
||||
)
|
||||
.option('-v, --verbose', 'Enable verbose ingestion warnings (default: false)')
|
||||
.addHelpText(
|
||||
'after',
|
||||
|
|
|
|||
|
|
@ -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<string, number>();
|
||||
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}`);
|
||||
|
|
|
|||
|
|
@ -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)) {
|
||||
|
|
|
|||
|
|
@ -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<string, number>();
|
||||
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(', ')}`,
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -244,21 +244,127 @@ const writeRegistry = async (entries: RegistryEntry[]): Promise<void> => {
|
|||
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 <alias>` (#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 <alias> to register "${requestedPath}" under a different name, ` +
|
||||
`or --allow-duplicate-name to allow both paths under the same name (leaves -r <name> 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 <alias>` 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 <alias>`)
|
||||
* 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<void> => {
|
||||
export const registerRepo = async (
|
||||
repoPath: string,
|
||||
meta: RepoMeta,
|
||||
opts?: RegisterRepoOptions,
|
||||
): Promise<void> => {
|
||||
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<vo
|
|||
stats: meta.stats,
|
||||
};
|
||||
|
||||
if (existing >= 0) {
|
||||
entries[existing] = entry;
|
||||
if (existingIdx >= 0) {
|
||||
entries[existingIdx] = entry;
|
||||
} else {
|
||||
entries.push(entry);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<string, string>,
|
||||
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 `<parentTmp>/<basename>`
|
||||
* 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 <alias> + --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 <alias> 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);
|
||||
|
|
|
|||
|
|
@ -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 <alias> + 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<ReturnType<typeof createTempDir>>;
|
||||
let tmpRepoA: Awaited<ReturnType<typeof createTempDir>>;
|
||||
let tmpRepoB: Awaited<ReturnType<typeof createTempDir>>;
|
||||
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();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue