fix(analyze): don't take the up-to-date fast path for an unregistered repo (#2264)

A prior 'analyze --name X' that hit a registry name collision writes meta.json
(meta-save runs before registerRepo) but fails before registering — leaving the
index up-to-date but UNREGISTERED. A later 'analyze --name X --allow-duplicate-name'
then matched the up-to-date gate and early-returned WITHOUT registering, so the
repo stayed invisible to list_repos/MCP and the CLI's assertAnalysisFinalized
rejected it. --allow-duplicate-name could never heal it.

This was latent on main, masked by the very close-hang this PR fixes: the lingering
process pushed the cli-e2e #829 step-3 analyze past its 60s spawn timeout
(status===null → the test's vacuous early-return). With the hang gone the analyze
exits promptly, exit 1 surfaces, and the bug becomes deterministic on all platforms.

Fix: the up-to-date fast path now short-circuits only when the repo is actually
registered (new isRepoRegistered helper, sharing assertAnalysisFinalized's exact
canonical/case-folded membership check). An indexed-but-unregistered repo falls
through to the pipeline, which registers it honoring allowDuplicateName. Already
registered repos keep the fast path unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm
This commit is contained in:
Gergo Magyar 2026-06-21 10:03:59 +00:00
parent d692a7fe51
commit 04ae8c29eb
3 changed files with 41 additions and 9 deletions

View file

@ -42,6 +42,7 @@ import {
loadMeta,
ensureGitNexusIgnored,
registerRepo,
isRepoRegistered,
cleanupOldKuzuFiles,
INCREMENTAL_SCHEMA_VERSION,
type RepoMeta,
@ -777,7 +778,15 @@ export async function runFullAnalysis(
return true; // conservative on git failure
}
})();
if (!dirty) {
// Only short-circuit when this repo is actually REGISTERED. A prior run
// can write meta.json and then fail before registerRepo (e.g. a rejected
// --name collision), leaving the index up-to-date but UNREGISTERED. Taking
// the fast path there returns an unregistered repo that the CLI's
// assertAnalysisFinalized rejects — and `--allow-duplicate-name` could
// never heal it (it would keep hitting this early-return). Fall through to
// the pipeline so it gets registered (honoring allowDuplicateName); already
// registered repos keep the fast path unchanged (#2264).
if (!dirty && (await isRepoRegistered(repoPath))) {
await ensureGitNexusIgnored(repoPath);
return {
// `resolveRepoIdentityRoot` collapses worktree roots to the

View file

@ -978,6 +978,22 @@ export class AnalysisNotFinalizedError extends Error {
}
}
/**
* True when the global registry already contains an entry whose canonical path
* matches `repoPath`. Uses the same canonical, case-folded (Windows) comparison
* as {@link assertAnalysisFinalized} so "is it registered?" answers identically
* at the analyze fast-path gate and at the finalize assertion. Pure read.
*/
export const isRepoRegistered = async (repoPath: string): Promise<boolean> => {
const entries = await readRegistry();
const canonicalInput = canonicalizePath(path.resolve(repoPath));
const isWin = process.platform === 'win32';
return entries.some((e) => {
const a = canonicalizePath(e.path);
return isWin ? a.toLowerCase() === canonicalInput.toLowerCase() : a === canonicalInput;
});
};
/**
* Verify that a successful `analyze` call actually produced an indexed,
* registered repo on disk. Two checks, both strictly required:
@ -1002,14 +1018,7 @@ export const assertAnalysisFinalized = async (repoPath: string): Promise<void> =
throw new AnalysisNotFinalizedError(resolved, storagePath, 'meta', getGlobalRegistryPath());
}
const entries = await readRegistry();
const canonicalInput = canonicalizePath(resolved);
const isWin = process.platform === 'win32';
const found = entries.some((e) => {
const a = canonicalizePath(e.path);
return isWin ? a.toLowerCase() === canonicalInput.toLowerCase() : a === canonicalInput;
});
if (!found) {
if (!(await isRepoRegistered(resolved))) {
throw new AnalysisNotFinalizedError(
resolved,
storagePath,

View file

@ -18,6 +18,7 @@ import fs from 'fs/promises';
import {
AnalysisNotFinalizedError,
assertAnalysisFinalized,
isRepoRegistered,
registerRepo,
saveMeta,
getStoragePaths,
@ -128,4 +129,17 @@ describe('assertAnalysisFinalized (#1169)', () => {
const variant = process.platform === 'win32' ? tmpRepo.dbPath.toUpperCase() : tmpRepo.dbPath; // POSIX is case-sensitive; assertion uses canonical form
await expect(assertAnalysisFinalized(variant)).resolves.toBeUndefined();
});
// isRepoRegistered backs the analyze up-to-date fast-path gate (#2264): the
// fast path must NOT short-circuit a repo that is indexed-but-unregistered
// (e.g. a prior --name collision wrote meta.json then failed before
// registerRepo), otherwise --allow-duplicate-name could never heal it.
it('isRepoRegistered is false when the repo has no registry entry', async () => {
expect(await isRepoRegistered(tmpRepo.dbPath)).toBe(false);
});
it('isRepoRegistered is true once a matching entry is written', async () => {
await registerRepo(tmpRepo.dbPath, meta);
expect(await isRepoRegistered(tmpRepo.dbPath)).toBe(true);
});
});