fix(cli): surface silent finalize-skips so analyze cannot exit 0 without persisting (#1169) (#1237)

* fix(cli): surface silent finalize-skips so analyze cannot exit 0 without persisting (#1169)

Closes #1169.

On Windows, `gitnexus analyze .` was observed to exit with code 0 after
printing only the "GitNexus Analyzer" banner. `.gitnexus/lbug.wal` was
written but `meta.json` was never persisted and the repo was not added
to `~/.gitnexus/registry.json`, so `gitnexus list` / `status` reported
no indexed repository. The reporter confirmed the same shape on both
LadybugDB (1.6.x) and the pre-LadybugDB KuzuDB build (1.4.1), so the
silent finalize-skip is upstream of the DB engine and indistinguishable
from a healthy index from the user's perspective.

This change makes that state a hard, actionable failure regardless of
the upstream root cause.

Behaviour change

- New `assertAnalysisFinalized()` invariant in `repo-manager.ts` checks
  that meta.json exists at `<repo>/.gitnexus/meta.json` AND that the
  global registry has a canonical-path-matching entry. Throws
  `AnalysisNotFinalizedError` (kind: "AnalysisNotFinalizedError") with a
  diagnostic that names the missing artifact and the storage path the
  user should inspect.
- `analyzeCommand` invokes the invariant on the rebuild path (skipped
  on `alreadyUpToDate`), so a future silent finalize-skip surfaces with
  exit code 1 and a recoverable error instead of a silent exit 0.
- `analyzeCommand` installs idempotent `unhandledRejection` and
  `uncaughtException` handlers that bypass the progress bar's console
  redirection by writing to a stderr handle captured at module load.
  This addresses the secondary symptom where the `barLog` redirection
  visually erased stack traces with `\x1b[2K\r` and stripped them via
  `String(err)`.
- The catch block also writes the failing error's full stack via the
  captured stderr, so failure diagnostics survive any downstream
  monkey-patching of `process.stdout`/`stderr`.

Tests

- `test/unit/repo-manager-finalize-invariant.test.ts` (4 tests): cover
  both `missing="meta"` and `missing="registry-entry"`, the happy path,
  and Windows case-insensitive registry path matching.
- `test/integration/cli-e2e.test.ts` adds a regression test that runs
  the real CLI on a fresh repo copy, asserts exit 0, AND verifies
  `meta.json` plus the matching registry entry are both written —
  catches any future regression of the wiring.

Validation

- `npx tsc --noEmit` passes.
- `npx vitest run --project default` passes for all my touched files
  (89 tests across 4 files). The full default suite reports 7188 pass
  with the known native LadybugDB Windows-worker flake unrelated to
  this change.
- `npx prettier --check` clean on the diff.
- `npx eslint` reports only pre-existing `any` warnings on the file;
  no new warnings introduced.
- Live repro on the issue's two-file Python fixture reproduces a
  successful index after the change: meta.json present (742 B), exit 0,
  `gitnexus list` shows the repo.

Rollback

Strictly additive — the success path is unchanged when `meta.json` is
written and the registry is updated. Reverting the four-file diff is
safe; the previous silent-finalize behaviour returns. No persisted
schema or registry shape changes.

DoD

- [x] Runtime wiring is complete on the affected CLI path.
- [x] Requested behavior is correct and existing contracts are preserved.
- [x] Smallest correct solution — one invariant, one helper, two
      handlers; no speculative abstraction.
- [x] Tests prove the changed behavior at unit AND integration level.
- [x] Required validation for `gitnexus/` was run.
- [x] Repo boundaries respected; no language-specific code, no shared
      ingestion changes, no new injection surfaces.
- [x] Diff contains only the intended change — no unrelated churn.

Made-with: Cursor

* fix(cli): enforce analyze finalization on fast path (#1169)

Address PR review feedback by checking finalization even when analyze reports already up to date, and by making the #1169 E2E guard fail on timeout instead of passing silently.

Made-with: Cursor

* test(cli): fix #1169 regression coverage on CI

Normalize macOS temp paths in the registry assertion and update the analyze worker timeout test mock for the new finalization invariant exports.

Made-with: Cursor
This commit is contained in:
Gergő Magyar 2026-04-30 21:36:28 +01:00 • committed by Evan Wang
parent 97a4661acd
commit 2909b201e0
5 changed files with 402 additions and 1 deletions

View file

@ -17,12 +17,52 @@ import {
getStoragePaths,
getGlobalRegistryPath,
RegistryNameCollisionError,
AnalysisNotFinalizedError,
assertAnalysisFinalized,
} from '../storage/repo-manager.js';
import { getGitRoot, hasGitDir } from '../storage/git.js';
import { runFullAnalysis } from '../core/run-analyze.js';
import { getMaxFileSizeBannerMessage } from '../core/ingestion/utils/max-file-size.js';
import fs from 'fs/promises';
// Capture stderr.write at module load BEFORE anything (LadybugDB native
// init, progress bar, console redirection) can monkey-patch it. The
// fatal handlers below MUST reach the user even when the analyze path
// has redirected console.* through the progress bar's bar.log() — the
// previous behaviour silently swallowed stack traces and made #1169
// indistinguishable from a no-op success on Windows.
const realStderrWrite = process.stderr.write.bind(process.stderr);
const writeFatalToStderr = (label: string, err: unknown): void => {
const isErr = err instanceof Error;
const message = isErr ? err.message : String(err);
realStderrWrite(`\n ${label}: ${message}\n`);
if (isErr && err.stack) realStderrWrite(`${err.stack}\n`);
};
let fatalHandlersInstalled = false;
/**
* Install one-shot `unhandledRejection` / `uncaughtException` handlers
* that surface the failure to the real stderr (bypassing any console
* redirection installed by the progress bar) and force a non-zero exit
* code. Without these, an async error escaping {@link analyzeCommand}'s
* try/catch was reported as exit 0 with no diagnostic — the silent
* failure mode tracked in #1169.
*/
const installFatalHandlers = (): void => {
if (fatalHandlersInstalled) return;
fatalHandlersInstalled = true;
process.on('unhandledRejection', (err) => {
writeFatalToStderr('Analysis failed (unhandled rejection)', err);
process.exit(1);
});
process.on('uncaughtException', (err) => {
writeFatalToStderr('Analysis failed (uncaught exception)', err);
process.exit(1);
});
};
const HEAP_MB = 8192;
const HEAP_FLAG = `--max-old-space-size=${HEAP_MB}`;
/** Increase default stack size (KB) to prevent stack overflow on deep class hierarchies. */
@ -102,6 +142,11 @@ export interface AnalyzeOptions {
export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOptions) => {
if (ensureHeap()) return;
// Install fatal handlers immediately after re-exec resolution so any
// async error that escapes the try/catch below (#1169) surfaces with
// a stack trace and a non-zero exit code instead of a silent exit 0.
installFatalHandlers();
if (options?.verbose) {
process.env.GITNEXUS_VERBOSE = '1';
}
@ -316,6 +361,11 @@ export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOption
);
if (result.alreadyUpToDate) {
// Even the fast path must prove the repo is discoverable. A prior
// run can write meta.json and then fail before registerRepo(); in
// that half-finalized state, runFullAnalysis returns alreadyUpToDate
// on the next invocation unless we check the registry here too.
await assertAnalysisFinalized(repoPath);
clearInterval(elapsedTimer);
process.removeListener('SIGINT', sigintHandler);
console.log = origLog;
@ -328,6 +378,15 @@ export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOption
return;
}
// Post-finalize invariant (#1169): runFullAnalysis nominally writes
// meta.json and registers the repo, but on Windows it has been
// observed to return successfully with neither artifact present
// (banner-only output, exit 0). Verify both before declaring
// success so the silent-finalize state surfaces with a non-zero
// exit code and an actionable error instead of being mistaken for
// a healthy index.
await assertAnalysisFinalized(repoPath);
// Skill generation (CLI-only, uses pipeline result from analysis)
if (options?.skills && result.pipelineResult) {
updateBar(99, 'Generating skill files...');
@ -429,7 +488,29 @@ export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOption
return;
}
console.error(`\n Analysis failed: ${msg}\n`);
// Finalize invariant failure (#1169) — keep the rich actionable
// message intact and write through realStderrWrite so it can't be
// erased by a leftover bar refresh on slow terminals.
if (err instanceof AnalysisNotFinalizedError) {
writeFatalToStderr('Analysis did not finalize', err);
realStderrWrite(
`\n Diagnostic checklist:\n` +
` 1. Re-run "gitnexus analyze" - transient native errors often clear on retry.\n` +
` 2. Inspect ${err.storagePath} - a leftover lbug.wal indicates an aborted write.\n` +
` 3. If the failure persists, run with NODE_OPTIONS="--max-old-space-size=8192 --trace-exit"\n` +
` and attach the trace to the GitNexus issue tracker.\n\n`,
);
process.exitCode = 1;
return;
}
// Bypass the redirected console.error and write the full stack to
// the real stderr captured at module load. The redirected
// console.error wraps every line with `\\x1b[2K\\r` (ANSI clear-line)
// and forces a bar.update() afterwards, which on some Windows
// terminals visually erases the failure message — the canonical
// shape of the silent-exit symptom in #1169.
writeFatalToStderr('Analysis failed', err);
// Provide helpful guidance for known failure modes
if (

View file

@ -561,6 +561,84 @@ export class RegistryAmbiguousTargetError extends Error {
}
}
/**
* Thrown by {@link assertAnalysisFinalized} when a successful `analyze`
* run did not actually persist `meta.json` or did not register the repo
* in `~/.gitnexus/registry.json` (#1169).
*
* Why this exists: on Windows, `gitnexus analyze` has been observed to
* exit cleanly (code 0) with `lbug.wal` written but no `meta.json`,
* leaving the repo invisible to `gitnexus list`/`status` and downstream
* MCP discovery. The only signal to the user was an empty banner —
* which is indistinguishable from a no-op early return. This invariant
* fails loudly with an actionable diagnostic so the silent-finalize bug
* surfaces with a non-zero exit code and a recoverable error message
* regardless of the upstream root cause (re-exec churn, native module
* side effects, antivirus, or future regressions).
*/
export class AnalysisNotFinalizedError extends Error {
readonly kind = 'AnalysisNotFinalizedError' as const;
constructor(
public readonly repoPath: string,
public readonly storagePath: string,
public readonly missing: 'meta' | 'registry-entry',
public readonly registryPath: string,
) {
const detail =
missing === 'meta'
? `meta.json was not written to ${path.join(storagePath, 'meta.json')}`
: `registry entry for ${repoPath} was not added to ${registryPath}`;
super(
`Analysis did not finalize for ${repoPath}: ${detail}. ` +
`The on-disk index is incomplete and was not registered. ` +
`Re-run "gitnexus analyze" — if the problem persists, inspect ` +
`${storagePath} for a stale lbug.wal that signals an aborted write.`,
);
this.name = 'AnalysisNotFinalizedError';
}
}
/**
* Verify that a successful `analyze` call actually produced an indexed,
* registered repo on disk. Two checks, both strictly required:
*
* 1. `meta.json` must exist at `<repoPath>/.gitnexus/meta.json`.
* 2. The global registry (`getGlobalRegistryPath()`) must contain an
* entry whose canonical path matches `repoPath`.
*
* Throws {@link AnalysisNotFinalizedError} on the first failure with the
* specific missing artifact. Pure read — does not mutate disk state.
*
* Callers must skip this assertion on the `alreadyUpToDate` early-return
* path, where the rebuild was deliberately not run.
*/
export const assertAnalysisFinalized = async (repoPath: string): Promise<void> => {
const resolved = path.resolve(repoPath);
const { storagePath, metaPath } = getStoragePaths(resolved);
try {
await fs.access(metaPath);
} catch {
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) {
throw new AnalysisNotFinalizedError(
resolved,
storagePath,
'registry-entry',
getGlobalRegistryPath(),
);
}
};
/**
* Thrown by {@link assertSafeStoragePath} when a registry entry's
* `storagePath` does NOT point at the expected `<entry.path>/.gitnexus`

View file

@ -200,6 +200,115 @@ describe('CLI end-to-end', () => {
expect(fs.statSync(gitnexusDir).isDirectory()).toBe(true);
}, 60_000);
// Regression guard for issue #1169 — analyze must produce BOTH a
// meta.json AND a global-registry entry on success. The previous
// failure mode on Windows was banner-only output + exit 0 with
// neither artifact persisted; the new finalize invariant
// (assertAnalysisFinalized) makes that state a hard failure.
//
// Uses a fresh per-test repo copy (not the shared MINI_REPO) so
// an earlier sibling test's analyze cannot push this one onto the
// alreadyUpToDate fast path, which would skip the very wiring this
// test is here to protect.
it('analyze persists meta.json AND a matching registry entry (#1169)', () => {
const gnHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-1169-home-'));
const repo = makeMiniRepoCopy('mini-repo', 'gn-1169-repo-');
const repoParent = path.dirname(repo);
try {
const result = runCliWithEnv(['analyze'], repo, { GITNEXUS_HOME: gnHome }, 60000);
expect(
result.status,
[
'analyze timed out before asserting finalization artifacts — this test guards #1169 and must not pass silently',
`stdout: ${result.stdout}`,
`stderr: ${result.stderr}`,
].join('\n'),
).not.toBeNull();
expect(
result.status,
[
`analyze exited with code ${result.status}`,
`stdout: ${result.stdout}`,
`stderr: ${result.stderr}`,
].join('\n'),
).toBe(0);
const metaPath = path.join(repo, '.gitnexus', 'meta.json');
expect(
fs.existsSync(metaPath),
`meta.json missing at ${metaPath} after analyze exited 0 — this is the #1169 silent-finalize symptom`,
).toBe(true);
const registryPath = path.join(gnHome, 'registry.json');
expect(
fs.existsSync(registryPath),
`registry.json missing at ${registryPath} after analyze exited 0`,
).toBe(true);
const entries = JSON.parse(fs.readFileSync(registryPath, 'utf-8')) as Array<{
name: string;
path: string;
}>;
expect(entries.length).toBeGreaterThanOrEqual(1);
const matchesRepo = entries.some((e) => {
const a = fs.realpathSync.native(e.path);
const b = fs.realpathSync.native(repo);
return process.platform === 'win32' ? a.toLowerCase() === b.toLowerCase() : a === b;
});
expect(
matchesRepo,
`registry has no entry for ${repo}; entries: ${JSON.stringify(entries.map((e) => e.path))}`,
).toBe(true);
} finally {
fs.rmSync(gnHome, { recursive: true, force: true });
fs.rmSync(repoParent, { recursive: true, force: true });
}
}, 60_000);
it('already-up-to-date analyze fails when registry entry is missing (#1169)', () => {
const gnHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-1169-fastpath-home-'));
const repo = makeMiniRepoCopy('mini-repo', 'gn-1169-fastpath-repo-');
const repoParent = path.dirname(repo);
try {
const first = runCliWithEnv(['analyze'], repo, { GITNEXUS_HOME: gnHome }, 60000);
expect(
first.status,
[
`initial analyze exited with code ${first.status}`,
`stdout: ${first.stdout}`,
`stderr: ${first.stderr}`,
].join('\n'),
).toBe(0);
const metaPath = path.join(repo, '.gitnexus', 'meta.json');
expect(fs.existsSync(metaPath)).toBe(true);
// Simulate the half-finalized state from the review: meta.json is
// present and lastCommit matches, but the repo is not discoverable
// because the global registry entry is missing.
fs.writeFileSync(path.join(gnHome, 'registry.json'), '[]', 'utf-8');
const second = runCliWithEnv(['analyze'], repo, { GITNEXUS_HOME: gnHome }, 60000);
expect(
second.status,
[
'second analyze timed out before proving alreadyUpToDate finalization',
`stdout: ${second.stdout}`,
`stderr: ${second.stderr}`,
].join('\n'),
).not.toBeNull();
expect(`${second.stdout}${second.stderr}`).toMatch(/Analysis did not finalize/i);
expect(`${second.stdout}${second.stderr}`).toMatch(/registry entry/i);
expect(second.status).toBe(1);
} finally {
fs.rmSync(gnHome, { recursive: true, force: true });
fs.rmSync(repoParent, { recursive: true, force: true });
}
}, 60_000);
// ─── analyze --name <alias> + --allow-duplicate-name (#829) ──────
//
// End-to-end regression guard for the name-collision feature:

View file

@ -14,6 +14,8 @@ vi.mock('../../src/storage/repo-manager.js', () => ({
getStoragePaths: vi.fn(() => ({ storagePath: '.gitnexus', lbugPath: '.gitnexus/lbug' })),
getGlobalRegistryPath: vi.fn(() => 'registry.json'),
RegistryNameCollisionError: class RegistryNameCollisionError extends Error {},
AnalysisNotFinalizedError: class AnalysisNotFinalizedError extends Error {},
assertAnalysisFinalized: vi.fn(async () => undefined),
}));
vi.mock('../../src/storage/git.js', () => ({

View file

@ -0,0 +1,131 @@
/**
* Regression tests for the analyze finalize invariant (#1169).
*
* Issue #1169: on Windows, `gitnexus analyze` was observed to exit
* cleanly with `lbug.wal` written but `meta.json` missing AND no
* registry entry for the repo. The user saw only the banner and exit
* code 0, indistinguishable from a healthy index. {@link
* assertAnalysisFinalized} is the runtime guard that catches that
* silent-finalize state regardless of the upstream root cause.
*
* These tests intentionally exercise the real disk and the real
* canonical-path comparison logic — they must fail if the invariant is
* weakened or removed.
*/
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import path from 'path';
import fs from 'fs/promises';
import {
AnalysisNotFinalizedError,
assertAnalysisFinalized,
registerRepo,
saveMeta,
getStoragePaths,
type RepoMeta,
} from '../../src/storage/repo-manager.js';
import { createTempDir } from '../helpers/test-db.js';
describe('assertAnalysisFinalized (#1169)', () => {
let tmpHome: Awaited<ReturnType<typeof createTempDir>>;
let tmpRepo: Awaited<ReturnType<typeof createTempDir>>;
let savedGitnexusHome: string | undefined;
const meta: RepoMeta = {
repoPath: '',
lastCommit: 'deadbee',
indexedAt: '2026-04-30T00:00:00.000Z',
stats: { files: 2, nodes: 9, edges: 17 },
};
beforeEach(async () => {
tmpHome = await createTempDir('gn-1169-home-');
tmpRepo = await createTempDir('gn-1169-repo-');
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 tmpRepo.cleanup();
});
it('throws missing="meta" when .gitnexus/meta.json was never written (the #1169 symptom)', async () => {
// Reproduce the exact disk shape from the user's repro: lbug.wal
// present, meta.json absent. analyze must report this as a hard
// failure, not silently return success.
const { storagePath, lbugPath } = getStoragePaths(tmpRepo.dbPath);
await fs.mkdir(storagePath, { recursive: true });
await fs.writeFile(`${lbugPath}.wal`, 'simulated uncommitted WAL data');
await expect(assertAnalysisFinalized(tmpRepo.dbPath)).rejects.toBeInstanceOf(
AnalysisNotFinalizedError,
);
try {
await assertAnalysisFinalized(tmpRepo.dbPath);
} catch (e) {
expect(e).toBeInstanceOf(AnalysisNotFinalizedError);
const err = e as AnalysisNotFinalizedError;
expect(err.missing).toBe('meta');
expect(err.kind).toBe('AnalysisNotFinalizedError');
expect(err.repoPath).toBe(path.resolve(tmpRepo.dbPath));
expect(err.storagePath).toBe(storagePath);
// Diagnostic message names the missing artifact and the storage
// path the user must inspect — required to clear DoD §2.8
// (errors must be actionable).
expect(err.message).toContain('meta.json');
expect(err.message).toContain(storagePath);
expect(err.message).toContain('lbug.wal');
}
});
it('throws missing="registry-entry" when meta.json exists but the registry was not updated', async () => {
// Half-finalized state — meta.json was written but registerRepo
// failed or was skipped. Surface this as a hard failure so the
// caller does not believe the repo is discoverable from MCP.
const { storagePath } = getStoragePaths(tmpRepo.dbPath);
await saveMeta(storagePath, meta);
await expect(assertAnalysisFinalized(tmpRepo.dbPath)).rejects.toBeInstanceOf(
AnalysisNotFinalizedError,
);
try {
await assertAnalysisFinalized(tmpRepo.dbPath);
} catch (e) {
expect(e).toBeInstanceOf(AnalysisNotFinalizedError);
const err = e as AnalysisNotFinalizedError;
expect(err.missing).toBe('registry-entry');
expect(err.message).toContain('registry entry');
expect(err.message).toContain(path.resolve(tmpRepo.dbPath));
}
});
it('resolves cleanly when meta.json exists AND a matching registry entry was written', async () => {
// Happy path — verify the invariant does NOT throw on a properly
// finalized repo, otherwise we would break every successful
// analyze run.
const { storagePath } = getStoragePaths(tmpRepo.dbPath);
await saveMeta(storagePath, meta);
await registerRepo(tmpRepo.dbPath, meta);
await expect(assertAnalysisFinalized(tmpRepo.dbPath)).resolves.toBeUndefined();
});
it('matches registry entries case-insensitively on Windows so 8.3 short-name paths still finalize', async () => {
// The registry comparison applies canonicalizePath + Windows
// case-insensitivity. If the analyze caller passes the path in a
// different case (uppercase drive letter, mixed-case parent dir),
// the invariant must still see the entry. Otherwise valid
// analyses would spuriously fail with AnalysisNotFinalizedError on
// Windows runners.
const { storagePath } = getStoragePaths(tmpRepo.dbPath);
await saveMeta(storagePath, meta);
await registerRepo(tmpRepo.dbPath, meta);
const variant = process.platform === 'win32' ? tmpRepo.dbPath.toUpperCase() : tmpRepo.dbPath; // POSIX is case-sensitive; assertion uses canonical form
await expect(assertAnalysisFinalized(variant)).resolves.toBeUndefined();
});
});