scriptoriumadmin/GitNexus
1
0
Fork 0
mirror of https://github.com/abhigyanpatwari/GitNexus.git synced 2026-08-28 05:25:25 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 9

fix(analyze): single-writer lock for the index write path (#2658) (#2677)

Browse source
This commit is contained in:
Gergő Magyar 2026-07-25 05:08:13 +01:00 • committed by GitHub
parent d3d4fa31bb
commit 1e764cd475
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
15 changed files with 1977 additions and 101 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
gitnexus/scripts/cross-platform-tests.ts
View file

@ -79,6 +79,13 @@ 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',
// 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.
// The Ubuntu suite only proves the Linux abstract-socket path, so run it here
// to exercise the Windows named-pipe backend and the macOS file fallback on
// their real platforms (#2658 review H3).
'test/unit/index-lock.test.ts',
];
// Native LadybugDB integration tests — exercise the @ladybugdb/core
@ -147,6 +154,14 @@ const SPAWN_CLI = [
'test/integration/antigravity-hook-e2e.test.ts',
'test/unit/local-cli-subprocess.test.ts',
'test/unit/runner-exec-tail.test.ts',
// Real cross-process single-writer lock coordination (#2658): child processes
// contend for the lock and race to reclaim a dead holder. Process spawning,
// kernel socket auto-release (Win named pipe / Linux abstract socket), and the
// FILE-backend rename-steal reclaim (macOS/BSD default) all vary across OSes —
// the exact behaviors the Windows/macOS matrix must prove. macOS timing first
// exposed a file-backend double-admit race here (#2658 review); the reclaim is
// now judgment-verified so a live holder is never displaced.
'test/integration/analyze-index-lock-concurrency.test.ts',
];
// Worker threads tests — exercise real worker_threads which have

37
gitnexus/src/cli/analyze.ts
View file

@ -34,6 +34,7 @@ import {
type AnalyzerRunnerIdentity,
} from '../storage/repo-manager.js';
import { getGitRoot, hasGitDir, getDefaultBranch } from '../storage/git.js';
import { IndexLockTimeoutError } from '../storage/index-lock.js';
import {
loadAnalyzeConfig,
mergeAnalyzeOptions,
@ -1553,11 +1554,21 @@ const analyzeCommandImpl = async (
// progress-bar log() that fired mid-run has already scrolled away, so the
// degraded-search state must also appear in the final summary (#1161).
if (result.ftsSkipped) {
console.log(
`\n Warning: full-text/BM25 search is disabled — the LadybugDB FTS extension was unavailable.\n` +
` Install it once with network access (GITNEXUS_LBUG_EXTENSION_INSTALL=auto) then rerun, or\n` +
` run \`gitnexus analyze --repair-fts\` when connected. Run \`gitnexus doctor\` for details.`,
);
// #2658 review L2: a build/verify failure is NOT an extension-unavailable
// problem — sending the user to install the extension is the wrong remedy.
if (result.ftsSkipReason === 'build-failed') {
console.log(
`\n Warning: full-text/BM25 search is disabled — the search index build failed this run.\n` +
` The FTS extension is available; rerun \`gitnexus analyze --repair-fts\`. If it persists,\n` +
` check the disk for space or corruption. Run \`gitnexus doctor\` for details.`,
);
} else {
console.log(
`\n Warning: full-text/BM25 search is disabled — the LadybugDB FTS extension was unavailable.\n` +
` Install it once with network access (GITNEXUS_LBUG_EXTENSION_INSTALL=auto) then rerun, or\n` +
` run \`gitnexus analyze --repair-fts\` when connected. Run \`gitnexus doctor\` for details.`,
);
}
}
try {
@ -1594,6 +1605,22 @@ const analyzeCommandImpl = async (
return;
}
// Another analyze held the index lock past the configured wait ceiling
// (#2658, GITNEXUS_INDEX_LOCK_TIMEOUT_MS). The on-disk index is being
// refreshed by the holder — this is a clean, expected condition, not a
// crash, so render the message without a stack trace.
if (err instanceof IndexLockTimeoutError) {
cliError(
` Another gitnexus analyze (pid ${err.holder.pid} on ${err.holder.hostname}) is ` +
`already refreshing this index and did not finish within the wait window.\n` +
` The on-disk index is being updated by that run. Retry later, or raise\n` +
` GITNEXUS_INDEX_LOCK_TIMEOUT_MS to wait longer.\n`,
{ recoveryHint: 'index-lock-timeout', holderPid: err.holder.pid },
);
process.exitCode = 1;
return;
}
// 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.

3
gitnexus/src/cli/cli-message.ts
View file

@ -59,7 +59,8 @@ export type RecoveryHint =
| 'npm-resolution'
| 'module-not-found'
| 'gitnexusrc-invalid'
| 'default-branch-invalid';
| 'default-branch-invalid'
| 'index-lock-timeout';
/**
* Common shape for the optional structured-field bag passed to

254
gitnexus/src/core/run-analyze.ts
View file

@ -11,7 +11,9 @@
import path from 'path';
import fs from 'fs/promises';
import { randomUUID } from 'node:crypto';
import { retryRename } from '../storage/fs-atomic.js';
import { acquireIndexLock } from '../storage/index-lock.js';
import { runPipelineFromRepo } from './ingestion/pipeline.js';
import type { KnowledgeGraph } from './graph/types.js';
import { resetDegradedParseCounter } from './tree-sitter/safe-parse.js';
@ -40,6 +42,7 @@ import { estimateBufferPool, setBufferPoolSizeHint } from './lbug/lbug-config.js
import { escapeCypherString } from './lbug/cypher-escape.js';
import {
buildSearchIndexesOrDegrade,
ftsFailureIsFatal,
createSearchFTSIndexes,
dropSearchFTSIndexes,
initialiseSearchFTSStemmer,
@ -358,6 +361,15 @@ export interface AnalyzeResult {
* the persisted meta surface the degraded state instead of reporting healthy.
*/
ftsSkipped?: boolean;
/**
* Why FTS was skipped, when `ftsSkipped` is true (#2658 review L2):
* `extension-unavailable` (the LadybugDB FTS extension could not load — the
* offline-first case, remedied by installing it) vs `build-failed` (the
* extension loaded but the index build/verify failed non-fatally — remedied by
* `--repair-fts`, not by installing the extension). Lets the CLI show the
* correct recovery hint instead of always blaming a missing extension.
*/
ftsSkipReason?: 'extension-unavailable' | 'build-failed';
/**
* True when the index this run produced/validated is the flat workspace
* slot (#2106 R2, inverted by #2354 to follow the checked-out branch).
@ -625,34 +637,175 @@ export const pdgModeMismatch = (recorded: RepoMeta['pdg'], options: PdgOptions):
return false;
};
/**
* The storage paths + resolved branch placement a run will write to. Computed
* once, up front, so the `runFullAnalysis` wrapper can lock the ACTUAL write
* directory (#2658). `metaDir` — not `getStoragePaths(repoPath, options.branch)`
* — is the lock scope: a `--branch X` that owns the flat slot resolves to the
* flat `.gitnexus`, so scoping off the raw option would lock the wrong dir.
*/
interface WriteTarget {
storagePath: string;
repoHasGit: boolean;
currentCommit: string;
checkedOutBranch: string | null;
branchLabel: string | null;
placement: { branch?: string };
lbugPath: string;
metaPath: string;
metaDir: string;
}
/**
* Resolve which storage slot this analyze writes to, including branch
* placement (#2106/#2354). Extracted from the top of the pipeline so the lock
* scope (`metaDir`) is known before the lock is acquired. Throws the same
* `--branch` / checked-out mismatch error the pipeline used to throw inline, so
* that failure still surfaces before any lock is taken.
*/
async function resolveWriteTarget(repoPath: string, options: AnalyzeOptions): Promise<WriteTarget> {
// `storagePath` is ALWAYS the flat `.gitnexus` — content-addressed caches
// (parse-cache, parsedfile-store) and kuzu-migration cleanup live there and
// are shared across branches (#2106 KTD7).
const { storagePath } = getStoragePaths(repoPath);
const repoHasGit = hasGitDir(repoPath);
const currentCommit = repoHasGit ? getCurrentCommit(repoPath) : '';
// Normalize the auto-detected branch the same way an explicit `--branch` is
// validated (#2106 R1): a git ref the branch-name rules forbid becomes `null`
// → the flat slot, matching that a later `--branch <that-ref>` query would
// also be rejected. A normal ref round-trips index-time/query-time labels.
const checkedOutBranch = repoHasGit
? (sanitizeDetectedBranch(getCurrentBranch(repoPath)) ?? null)
: null;
// Analyze indexes the working tree, not an arbitrary ref. An explicit
// `--branch X` while a DIFFERENT branch Y is checked out would write Y's
// content into X's slot, corrupting X (#2106). Refuse the mismatch. Detached
// HEAD / non-git (checkedOutBranch === null) still allow an explicit label.
if (options.branch && checkedOutBranch && options.branch !== checkedOutBranch) {
throw new Error(
`--branch "${options.branch}" does not match the checked-out branch "${checkedOutBranch}". ` +
`Check out "${options.branch}" before indexing it, or omit --branch to index the current branch.`,
);
}
const branchLabel = options.branch ?? checkedOutBranch;
const placement = options.branch ? await resolveBranchPlacement(repoPath, branchLabel) : {};
const { lbugPath, metaPath } = getStoragePaths(repoPath, placement.branch);
return {
storagePath,
repoHasGit,
currentCommit,
checkedOutBranch,
branchLabel,
placement,
lbugPath,
metaPath,
metaDir: path.dirname(metaPath),
};
}
/**
* Run the full analysis under an exclusive, index-directory-scoped write lock
* (#2658). A second concurrent `analyze` on the same slot waits here for the
* first to finish, then falls through to the normal freshness check inside —
* so a run whose work the holder already did returns `alreadyUpToDate` in
* seconds instead of rebuilding (single-flight coalescing), while a run for a
* genuinely-changed tree does one follow-up incremental. No new flag: waiting
* is the default, which is what hook-driven re-index wants.
*
* The lock is held by whichever process runs the pipeline (the heap-respawn
* child, or the original) — see index-lock.ts for why ownership lives with the
* writer, not a supervising parent. Released as soon as the write completes or
* throws; the post-analysis steps in the CLI (skills, registry) run lock-free.
*/
export async function runFullAnalysis(
repoPath: string,
options: AnalyzeOptions,
callbacks: AnalyzeCallbacks,
runnerIdentityAtBootstrap?: AnalyzerRunnerIdentity,
): Promise<AnalyzeResult> {
// Validate operator-provided FTS config before anything else — a typo fails
// here in ms, without taking the lock. (createSearchFTSIndexes reuses the
// cached value via getSearchFTSStemmer.)
initialiseSearchFTSStemmer();
initialiseSearchFTSCjkSegmentation();
// Scope the degraded-parse log throttle to this run (module-level counter
// would otherwise stay saturated on a reused process).
resetDegradedParseCounter();
const log = (msg: string) => callbacks.onLog?.(msg);
const acquireOpts = {
log,
onWaitStart: () =>
callbacks.onProgress('lock', 0, 'Waiting for another analyze to finish on this index…'),
};
let writeTarget = await resolveWriteTarget(repoPath, options);
let lock = await acquireIndexLock(writeTarget.metaDir, acquireOpts);
try {
// #2658 review H2: acquireIndexLock can wait up to the timeout ceiling,
// during which git HEAD/branch — and thus the resolved write slot — may
// change (a commit lands, a branch is switched, or another writer adopts the
// flat slot). The pre-wait snapshot must NOT be reused: re-resolve UNDER the
// lock so the freshness check (`existingMeta.lastCommit === currentCommit`)
// and the meta stamps see current git state, honoring the module's "re-check
// freshness after acquiring" contract. If the slot itself moved we hold the
// WRONG lock — release and re-acquire the correct one. Bounded so a
// pathologically churning checkout can't loop forever; after the cap we
// proceed on the current lock. The loop is INSIDE the try so a re-resolve
// that throws (e.g. a `--branch` that stopped matching the now-switched
// checkout) still releases the held lock via `finally` (no leak).
const MAX_RELOCK = 3;
for (let attempt = 0; attempt < MAX_RELOCK; attempt++) {
const fresh = await resolveWriteTarget(repoPath, options);
if (fresh.metaDir === writeTarget.metaDir) {
writeTarget = fresh; // same slot — adopt the freshly-read commit/branch/placement
break;
}
log(
`Index write target moved while waiting for the lock ` +
`(${writeTarget.metaDir} → ${fresh.metaDir}); re-acquiring the correct slot.`,
);
lock.release();
writeTarget = fresh;
lock = await acquireIndexLock(fresh.metaDir, acquireOpts);
if (attempt === MAX_RELOCK - 1) {
log('Index write target still moving after repeated re-acquire; proceeding on this lock.');
}
}
return await runFullAnalysisInner(
repoPath,
options,
callbacks,
writeTarget,
runnerIdentityAtBootstrap,
);
} finally {
lock.release();
}
}
async function runFullAnalysisInner(
repoPath: string,
options: AnalyzeOptions,
callbacks: AnalyzeCallbacks,
writeTarget: WriteTarget,
runnerIdentityAtBootstrap?: AnalyzerRunnerIdentity,
): Promise<AnalyzeResult> {
const log = (msg: string) => callbacks.onLog?.(msg);
const progress = (phase: string, percent: number, message: string) =>
callbacks.onProgress(phase, percent, message);
// Resolve + validate operator-provided FTS config once, before the expensive
// parse/load phases. A typo fails here in ms; createSearchFTSIndexes reuses
// the cached value via getSearchFTSStemmer.
initialiseSearchFTSStemmer();
initialiseSearchFTSCjkSegmentation();
// FTS-config validation and the degraded-parse counter reset happen in the
// `runFullAnalysis` wrapper (before the lock is taken).
// Scope the degraded-parse log throttle to this run. On a reused process
// (e.g. tests, or any host that calls runFullAnalysis more than once) the
// module-level counter would otherwise stay saturated and suppress every
// degraded-parse log after the first run. The per-parse worker holds its own
// counter in its own module instance and is process-scoped, so no separate
// worker-side reset is needed (see safe-parse.ts ParseTimeoutError contract).
resetDegradedParseCounter();
// `storagePath` is ALWAYS the flat `.gitnexus` — content-addressed caches
// (parse-cache, parsedfile-store) and the kuzu-migration cleanup live there
// and are shared across branches (#2106 KTD7).
const { storagePath } = getStoragePaths(repoPath);
// Write target (storage paths + resolved branch placement) was computed by
// the `runFullAnalysis` wrapper — which needs `metaDir` up front to acquire
// the exclusive index lock BEFORE any of the freshness/write work below
// (#2658). `storagePath` is ALWAYS the flat `.gitnexus`; `placement.branch`
// selects a `branches/<slug>/` sub-slot only for an explicit `--branch` that
// does not own the flat slot. See resolveWriteTarget for the full contract.
const { storagePath, repoHasGit, currentCommit, branchLabel, placement, lbugPath, metaDir } =
writeTarget;
// Start each analyze with a clean buffer-pool hint: any pre-pipeline DB open
// (e.g. the embeddings-cache open) falls back to the default until the hint is
@ -665,44 +818,6 @@ export async function runFullAnalysis(
log('Migrating from KuzuDB to LadybugDB — rebuilding index...');
}
const repoHasGit = hasGitDir(repoPath);
const currentCommit = repoHasGit ? getCurrentCommit(repoPath) : '';
// ── #2106/#2354: resolve which branch slot this run writes to ─────────
// `branchLabel` is the branch identity recorded in meta.json (incl. the
// flat workspace slot). `placement.branch` is undefined for the flat slot
// (the lbug/meta paths stay byte-identical to single-branch behavior) and
// set for a `branches/<slug>/` sub-directory. Only an explicit `--branch`
// can route to a sub-directory; a plain analyze ALWAYS targets the flat
// slot, which follows the checked-out working tree (#2354) — the
// auto-detected branch (null for detached HEAD / non-git) is recorded as
// the slot's informational label only.
// Normalize the auto-detected branch the same way an explicit `--branch` is
// validated (#2106 R1): a git ref the branch-name rules forbid (backtick,
// `~ ^ : ? *`, leading `-`, `..`) becomes `null` → the flat slot, matching
// that a later `--branch <that-ref>` query would also be rejected. A normal
// ref passes through unchanged so index-time and query-time labels round-trip.
const checkedOutBranch = repoHasGit
? (sanitizeDetectedBranch(getCurrentBranch(repoPath)) ?? null)
: null;
// Analyze indexes the working tree, not an arbitrary ref. An explicit
// `--branch X` while a DIFFERENT branch Y is checked out would write Y's
// content (and Y's commit) into X's index slot, corrupting X (#2106). Refuse
// the mismatch. Detached HEAD / non-git (checkedOutBranch === null) still
// allow an explicit label so CI checkouts can name their snapshot.
if (options.branch && checkedOutBranch && options.branch !== checkedOutBranch) {
throw new Error(
`--branch "${options.branch}" does not match the checked-out branch "${checkedOutBranch}". ` +
`Check out "${options.branch}" before indexing it, or omit --branch to index the current branch.`,
);
}
const branchLabel = options.branch ?? checkedOutBranch;
const placement = options.branch ? await resolveBranchPlacement(repoPath, branchLabel) : {};
const { lbugPath, metaPath } = getStoragePaths(repoPath, placement.branch);
// metaPath now points to the metadata file (gitnexus.json) in a branch-specific directory.
// metaDir is the directory containing the metadata file (and branch-specific DBs).
const metaDir = path.dirname(metaPath);
// Keep gitnexus.json and the legacy meta.json mirror in sync (fresher
// indexedAt wins; nothing is deleted). Best-effort: loadMeta has its own
// legacy fallback, so a reconciliation failure (read-only mount, full disk)
@ -1380,7 +1495,12 @@ export async function runFullAnalysis(
log('atomic-incremental: live index carries orphan sidecars — using in-place writeback');
}
const useAtomicSwap = (isFullRebuild || atomicIncremental) && (posixSwap || windowsSwapOk);
const buildPath = useAtomicSwap ? `${lbugPath}.new` : lbugPath;
// #2658: a per-run staging name (was the fixed `lbug.new`). Even under the
// single-writer lock, a unique name means a crashed run's half-built staging
// file can never be mistaken for — or clobber — a live run's; the lock's
// orphan sweep (sweepStagingArtifacts) reclaims stragglers on the next
// acquire. The `.staging.` prefix is what that sweep matches.
const buildPath = useAtomicSwap ? `${lbugPath}.staging.${randomUUID()}` : lbugPath;
if (isIncremental && hashDiff) {
log(
@ -1906,6 +2026,11 @@ export async function runFullAnalysis(
// build/verify step itself fails, so capabilities.fts.status / ftsSkipped
// stay honest even though that failure no longer aborts the whole analyze.
let ftsReady = ftsAvailable;
// Why FTS ended up skipped (#2658 review L2): extension-unavailable up front,
// or build-failed in the degrade branch below.
let ftsSkipReason: 'extension-unavailable' | 'build-failed' | undefined = ftsAvailable
? undefined
: 'extension-unavailable';
if (ftsAvailable) {
// Degrade rather than throw: createSearchFTSIndexes re-tokenizes every
// stored row on every run, so a native tokenizer error on a single
@ -1921,8 +2046,24 @@ export async function runFullAnalysis(
});
if (ftsResult.ok) {
progress('fts', 90, 'Search indexes ready');
} else if (ftsFailureIsFatal(ftsResult.failureClass, useAtomicSwap)) {
// #2658: an IO/rename/checkpoint/corruption failure while building FTS
// is a genuinely broken build on this disk — not a concurrent writer
// (the single-writer lock rules that out). ONLY fatal on the atomic-swap
// path: the graph was built into a throwaway staging DB, so throwing
// before the swap abandons the staging file and leaves the previous live
// index intact. On an in-place build the live DB is already mutated and
// cannot be rolled back by throwing (see ftsFailureIsFatal) — those
// degrade in the branch below instead.
throw new Error(
`Search index build failed with an integrity error and the analysis was aborted ` +
`to avoid publishing a broken index: ${ftsResult.error}. The previous index is ` +
`left intact. Re-run \`gitnexus analyze\`; if it persists, check the disk for space ` +
`or corruption.`,
);
} else {
ftsReady = false;
ftsSkipReason = 'build-failed';
log(
`FTS index build failed (${ftsResult.error}) — keyword search degraded this run. ` +
'Graph and embeddings analysis completed successfully. Run `gitnexus analyze --repair-fts` to retry.',
@ -2573,6 +2714,7 @@ export async function runFullAnalysis(
stats: meta.stats,
pipelineResult,
ftsSkipped: !ftsReady,
ftsSkipReason: ftsReady ? undefined : ftsSkipReason,
isPrimaryBranch: !placement.branch,
};
} catch (err) {

81
gitnexus/src/core/search/fts-indexes.ts
View file

@ -193,9 +193,81 @@ export async function verifySearchFTSIndexes(
return missing;
}
/**
* Why an FTS build failed, so the caller can react correctly (#2658):
*
* - `capability`: the environment can't support FTS this run, or a single
* pre-existing row can't be tokenized (#2544/#2546 "Invalid UTF-8"). The
* graph/embeddings work is sound — degrade keyword search and keep exit 0.
* - `integrity`: an IO / rename / checkpoint / corruption failure while
* writing the index. With the single-writer lock (#2658) this is no longer
* "some other analyze racing us" — it's a genuinely broken build on this
* disk, so the run must fail loudly rather than publish a clean-looking
* index whose search silently never worked.
*/
export type FtsBuildFailureClass = 'capability' | 'integrity';
// Checked before integrity signatures: a row-level tokenizer error that happens
// to mention an integrity word still degrades (it isn't a broken build).
const FTS_CAPABILITY_SIGNATURES = ['invalid utf-8', 'failed calling lower', 'tokeniz'] as const;
// IO / durability / corruption signatures that mean the build itself broke.
// Deliberately SPECIFIC (#2658 review L1): generic OS errors a capability/config
// failure can also carry — bare 'no such file or directory' (ENOENT, e.g. a
// missing FTS extension asset) and 'bad file descriptor'/'ebadf' — are NOT here,
// so an ambiguous failure degrades (the pre-#2658 safe behavior) instead of
// newly aborting the whole analyze. A genuine write/rename/checkpoint integrity
// failure still matches via 'error renaming' / 'io exception' / 'checkpoint'
// (the #2658 repro message "Error renaming … : No such file or directory" hits
// both 'io exception' and 'error renaming').
const FTS_INTEGRITY_SIGNATURES = [
'io exception',
'i/o error',
'io error',
'error renaming',
'checkpoint',
'corrupt',
'no space',
'enospc',
'double free',
'segmentation',
] as const;
/**
* Classify an FTS build failure message. Defaults to `capability` (degrade) —
* only clearly-integrity failures escalate, so the long-standing resilience to
* row-level tokenizer errors is preserved and we never newly fail a run on an
* unrecognised message.
*/
export const classifyFtsBuildError = (message: string): FtsBuildFailureClass => {
const m = message.toLowerCase();
if (FTS_CAPABILITY_SIGNATURES.some((s) => m.includes(s))) return 'capability';
if (FTS_INTEGRITY_SIGNATURES.some((s) => m.includes(s))) return 'integrity';
return 'capability';
};
/**
* Whether an FTS build failure should ABORT the analyze (throw before publish)
* rather than degrade to a search-less-but-queryable index (#2658).
*
* Only an `integrity` failure on the atomic-swap path is fatal: there the graph
* was built into a throwaway staging DB, so throwing abandons the staging file
* and leaves the previous live index intact. On an in-place build
* (`useAtomicSwap === false`: incremental, Windows default) the graph DML
* already mutated the LIVE database, so there is nothing to roll back by
* throwing — degrading to a queryable index with FTS marked unavailable is
* strictly better than exiting mid-finalization over a dirty, partially-indexed
* live DB. `capability` failures always degrade.
*/
export const ftsFailureIsFatal = (
failureClass: FtsBuildFailureClass | undefined,
useAtomicSwap: boolean,
): boolean => failureClass === 'integrity' && useAtomicSwap;
export interface BuildSearchIndexesResult {
ok: boolean;
error?: string;
/** Present only when `ok` is false. See {@link FtsBuildFailureClass}. */
failureClass?: FtsBuildFailureClass;
}
/**
@ -216,10 +288,15 @@ export async function buildSearchIndexesOrDegrade(
await createSearchFTSIndexes(options);
const missing = await verifySearchFTSIndexes(executeQuery);
if (missing.length > 0) {
return { ok: false, error: `missing indexes after build: ${missing.join(', ')}` };
// Structural incompleteness with no thrown error — treat as capability
// (degrade), matching prior behavior; a broken *write* surfaces as a
// thrown IO/checkpoint error below and is classified integrity there.
const error = `missing indexes after build: ${missing.join(', ')}`;
return { ok: false, error, failureClass: classifyFtsBuildError(error) };
}
return { ok: true };
} catch (e) {
return { ok: false, error: e instanceof Error ? e.message : String(e) };
const error = e instanceof Error ? e.message : String(e);
return { ok: false, error, failureClass: classifyFtsBuildError(error) };
}
}

11
gitnexus/src/server/analyze-worker-core.ts
View file

@ -16,6 +16,9 @@ import type { AnalyzeOptions } from '../core/run-analyze.js';
import type { WorkerMessage } from './analyze-worker.js';
import type { AnalyzerRunnerIdentity } from '../storage/repo-manager.js';
import { projectAnalyzeResultForIpc } from './analyze-worker-ipc.js';
// Value import (instanceof): index-lock is a lightweight storage primitive
// (node:fs/net/crypto only), so this does NOT pull in run-analyze/repo-manager.
import { IndexLockTimeoutError } from '../storage/index-lock.js';
export interface WorkerAnalysisDeps {
runFullAnalysis: typeof import('../core/run-analyze.js').runFullAnalysis;
@ -74,7 +77,13 @@ export async function runWorkerAnalysis(
} catch (err: unknown) {
// Report the failure to the parent over IPC (the parent surfaces the message).
const message = err instanceof Error ? err.message : 'Analysis failed';
terminal = { type: 'error', message };
// #2658 review M2: a lock-wait timeout is transient contention (another
// analyze held the single-writer lock), not a broken build — tag it so the
// parent can surface a retry signal instead of an opaque hard failure.
terminal =
err instanceof IndexLockTimeoutError
? { type: 'error', message, code: 'index-lock-timeout', retryable: true }
: { type: 'error', message };
}
// P3 (#2264): only report if a SIGTERM cancellation hasn't already claimed the

10
gitnexus/src/server/analyze-worker.ts
View file

@ -40,6 +40,16 @@ export interface CompleteMessage {
export interface ErrorMessage {
type: 'error';
message: string;
/**
* Machine-readable failure code for a parent that wants to branch instead of
* only surfacing the string. `index-lock-timeout` (#2658 review M2) means
* another analyze held the single-writer lock past the wait ceiling — a
* transient, retryable condition, not a broken build. Absent for a generic
* failure.
*/
code?: 'index-lock-timeout';
/** True when the failure is expected to clear on retry (e.g. lock contention). */
retryable?: boolean;
}
/** Child → parent IPC messages. Shared with the parent-side launcher. */

722
gitnexus/src/storage/index-lock.ts Normal file
View file

@ -0,0 +1,722 @@
/**
* Cross-process single-writer lock for a GitNexus index directory (#2658).
*
* `analyze` is the only writer of a `.gitnexus/` (or `branches/<slug>/`) slot,
* but nothing stopped two `analyze` runs — e.g. two editor/agent SessionStart
* hooks firing on the same repo at once — from wiping and rebuilding the same
* store concurrently. They raced on `lbug` and its sidecars, wasted N× CPU
* producing one index, and left orphaned WAL fragments (#2637). This module
* gives the write path an exclusive, index-directory-scoped lock so a second
* writer waits for the first instead of colliding; after acquiring, the caller
* re-runs its normal freshness check, so a run whose work the holder already
* did exits up-to-date rather than rebuilding (single-flight coalescing).
*
* Ownership lives with the process that runs the pipeline (the heap-respawn
* child when a respawn happens, the original otherwise) — NOT a supervising
* parent — so the entity the OS tracks for liveness is always the real writer.
* See run-analyze.ts for the acquire site.
*
* TWO BACKENDS behind the {@link acquireIndexLock} seam:
*
* - **socket** (Windows named pipe / Linux abstract socket, via `net`) — the
* preferred, KERNEL-OWNED lock. Holding it = holding a listening endpoint the
* kernel binds to this process; `EADDRINUSE` therefore means a *live* holder,
* and the kernel drops the binding the instant the holder exits for ANY reason
* (clean exit, crash, OOM, SIGKILL). That makes it provably race-free: no
* stale detection, no pid-reuse guess, no takeover, and — since the endpoint
* lives outside the index dir — no filesystem write, so it works unchanged on
* a read-only index mount. This is the same class of kernel object as the
* Windows named mutex the issue's reporter used as an external workaround, but
* built from Node's stdlib `net`, so it adds NO native dependency and cannot
* break `npx gitnexus` install anywhere.
*
* - **file** (`O_EXCL` pidfile) — the portable fallback for macOS/BSD (no
* abstract sockets; filesystem sockets don't release cleanly on death) and
* for any environment where the socket backend can't bind. It uses pid-
* liveness staleness, an atomic rename-steal reclaim, bounded malformed-file
* handling, read-only tolerance, and a finite wait timeout (a reused pid can
* masquerade as live where process start-time isn't verifiable, so waiting is
* bounded rather than a hang). Its stale-takeover has an irreducible narrow
* race — inherent to file-based advisory locks — which is precisely why the
* socket backend is preferred; only a kernel primitive closes it.
*
* Scope: cross-process, same logical index dir. The file backend never steals a
* foreign-host lock (pid liveness is meaningless across hosts); the socket
* backend is single-host by nature. The motivating case (local hook-driven
* re-index) is single-host. See AcquireOptions.timeoutMs for the wait ceiling.
*/
import {
openSync,
writeSync,
closeSync,
readFileSync,
unlinkSync,
renameSync,
existsSync,
mkdirSync,
readdirSync,
realpathSync,
} from 'node:fs';
import net from 'node:net';
import path from 'node:path';
import os from 'node:os';
import { randomBytes, randomUUID, createHash } from 'node:crypto';
const LOCK_FILENAME = 'analyze.lock';
const LOCK_RECORD_VERSION = 1 as const;
/** Base poll interval while waiting for a live holder; jittered per attempt. */
const DEFAULT_POLL_MS = 250;
/** How often to re-emit the "still waiting for pid N" diagnostic. */
const DIAGNOSTIC_INTERVAL_MS = 15_000;
/**
* Default wait ceiling (10 min). Generous enough to sit behind a normal
* analyze, finite so a pid-reuse ghost on a platform without start-time
* verification can't wedge acquisition forever (see AcquireOptions.timeoutMs).
* A repo whose analyze legitimately runs longer can raise
* GITNEXUS_INDEX_LOCK_TIMEOUT_MS (or set it ≤ 0 for unbounded).
*/
const DEFAULT_TIMEOUT_MS = 600_000;
/**
* How long a lock file must stay unreadable (empty/partial JSON) before we
* treat it as a crash orphan and reclaim it. Tolerates the microsecond
* create→write→close window of a *live* owner (see acquireIndexLock), so we
* never steal a lock that is a poll-interval away from being written. Scaled
* off the poll interval, floored at 1s.
*/
const malformedGraceMs = (pollMs: number): number => Math.max(1000, pollMs * 2);
/**
* On-disk lock record. `token` proves ownership on release/steal; `startTime`
* (Linux only) defends against pid reuse; `invocationId` is a human-traceable
* id distinct from the security-irrelevant `token`.
*/
export interface LockRecord {
v: typeof LOCK_RECORD_VERSION;
pid: number;
hostname: string;
/** /proc/<pid>/stat starttime (clock ticks) on Linux; null where unavailable. */
startTime: string | null;
token: string;
invocationId: string;
acquiredAt: string;
}
export interface IndexLockHandle {
/** Our own record — `invocationId` is shown to waiters as the holder id. */
readonly record: LockRecord;
/** Idempotent; only removes the lock file if it still carries our token. */
release(): void;
}
export interface AcquireOptions {
log?: (msg: string) => void;
/**
* Give up waiting after this long (ms), throwing {@link IndexLockTimeoutError}.
* Default: {@link DEFAULT_TIMEOUT_MS} ({@link resolveTimeoutMs}). A finite
* default is deliberate: on platforms without process start-time verification
* (anything but Linux — see {@link readProcStartTime}) a crashed holder whose
* pid was reused by an unrelated long-lived process reads as a live holder and
* would otherwise block acquisition forever. Timing out is safe — it stops
* *waiting*, never *steals* a possibly-live holder — and names the holder so
* the caller can retry. Override (including to unbounded, value ≤ 0) via
* GITNEXUS_INDEX_LOCK_TIMEOUT_MS.
*/
timeoutMs?: number;
/** Base poll interval (ms); jittered. Default 250. */
pollMs?: number;
/** Called once when we start waiting on a live holder. */
onWaitStart?: (holder: LockRecord) => void;
}
export class IndexLockTimeoutError extends Error {
readonly holder: LockRecord;
/**
* Whether `holder` carries a real, identifiable owner. False on the socket
* backend (and the file backend's malformed/vanished-lock timeouts), where the
* holder is a placeholder (`pid -1`) — the OS socket lock exposes no owner
* metadata (#2658 review M3). Consumers must not present `holder.pid` as a real
* pid when this is false.
*/
readonly holderKnown: boolean;
constructor(holder: LockRecord, waitedMs: number, holderKnown = true) {
super(
holderKnown
? `Timed out after ${waitedMs}ms waiting for another gitnexus analyze ` +
`(pid ${holder.pid} on ${holder.hostname}, invocation ${holder.invocationId}) ` +
`to release the index lock.`
: `Timed out after ${waitedMs}ms waiting for another gitnexus analyze ` +
`(holder identity unknown) to release the index lock.`,
);
this.name = 'IndexLockTimeoutError';
this.holder = holder;
this.holderKnown = holderKnown;
}
}
const HOSTNAME = os.hostname();
/** Linux: field 22 of /proc/<pid>/stat (starttime). null elsewhere / on error. */
const readProcStartTime = (pid: number): string | null => {
if (process.platform !== 'linux') return null;
try {
const stat = readFileSync(`/proc/${pid}/stat`, 'utf8');
// comm (field 2) is parenthesized and may contain spaces/')' — split after
// the last ')' so the remaining fields align to their documented numbers.
const afterComm = stat
.slice(stat.lastIndexOf(') ') + 2)
.trim()
.split(' ');
// afterComm[0] is field 3 (state); starttime is field 22 → index 19.
return afterComm[19] ?? null;
} catch {
return null;
}
};
/** true if the pid exists (signal 0). EPERM means it exists but isn't ours. */
const pidAlive = (pid: number): boolean => {
try {
process.kill(pid, 0);
return true;
} catch (err) {
return (err as NodeJS.ErrnoException).code === 'EPERM';
}
};
const buildRecord = (): LockRecord => ({
v: LOCK_RECORD_VERSION,
pid: process.pid,
hostname: HOSTNAME,
startTime: readProcStartTime(process.pid),
token: randomBytes(16).toString('hex'),
invocationId: randomUUID(),
acquiredAt: new Date().toISOString(),
});
const readRecord = (lockPath: string): LockRecord | null => {
try {
const raw = readFileSync(lockPath, 'utf8');
const parsed = JSON.parse(raw) as Partial<LockRecord>;
// `typeof NaN === 'number'`, so a bare number check lets NaN/0/-1/Infinity/
// fractional pids reach process.kill (#2658 review L4): a garbled or crafted
// lock file with `{"pid":0}` reads as a live holder and wedges a real analyze
// for the full wait timeout. A real pid is a positive integer.
if (!Number.isInteger(parsed.pid) || (parsed.pid as number) <= 0) return null;
if (typeof parsed.token !== 'string') return null;
return parsed as LockRecord;
} catch {
// Missing (won the race, file gone) or malformed/half-written → treat as
// "no readable holder"; the caller retries the O_EXCL create.
return null;
}
};
/**
* A same-host holder is stale iff its process is gone, or (Linux) its pid is
* alive but was reused — a different start time. A live holder is never stolen
* on age alone (a large repo legitimately analyzes for many minutes), and a
* foreign-host holder is never stale (its liveness is unknowable here). Where
* start-time verification is unavailable (non-Linux), a reused pid cannot be
* distinguished from a genuine live holder, so it is NOT stolen — the finite
* acquire timeout is what bounds that case instead (see AcquireOptions).
*/
const isStale = (holder: LockRecord): boolean => {
if (holder.hostname !== HOSTNAME) return false;
if (!pidAlive(holder.pid)) return true;
const now = readProcStartTime(holder.pid);
if (holder.startTime && now && holder.startTime !== now) return true; // pid reused
return false;
};
/**
* Reclaim a lock file we judged reclaimable — a dead holder (`expected` = its
* record) or a malformed/unreadable crash-orphan (`expected` = null) — moving
* the exact inode aside in ONE `rename` syscall to a token-unique name so two
* waiters reclaiming the same orphan can't both win (the loser's rename ENOENTs).
*
* CRITICAL (#2658 review): the reclaim must not act on a STALE judgment. The
* staleness decision (`isStale` / malformed-grace) happened a few syscalls ago;
* a live writer may have O_EXCL-created its own lock at `lockPath` since. Blindly
* renaming that live lock aside would delete it and admit a SECOND writer — the
* exact double-writer this lock exists to prevent (reproduced: ~18%/round under
* 4-way reclaim contention on the file backend). So:
* 1. re-read `lockPath` immediately BEFORE the rename and confirm it still holds
* exactly what we judged (same token, or still-unreadable) — shrinking the
* window to the single gap between this read and the rename;
* 2. after the rename, confirm what we ACTUALLY moved matches the judgment; if a
* live lock slipped into that residual gap, RESTORE it (rename back) so its
* holder is never displaced, and lose the reclaim.
* A concurrent creator whose fresh lock the restore overwrites is caught by the
* acquire loop's post-write read-back verify (see acquireViaFile), so it backs
* off rather than proceeding as a second writer.
*
* Returns true if we won the reclaim (caller retries the create), false if we
* lost the race or the judgment went stale (caller re-loops and re-reads).
*/
const matchesJudgment = (record: LockRecord | null, expected: LockRecord | null): boolean =>
expected === null ? record === null : record?.token === expected.token;
const stealLock = (lockPath: string, me: LockRecord, expected: LockRecord | null): boolean => {
// (1) Re-verify the judgment still holds right before we move anything.
if (!matchesJudgment(readRecord(lockPath), expected)) return false;
if (expected === null && !existsSync(lockPath)) return false; // malformed → but now vanished
const aside = `${lockPath}.dead.${me.token}`;
try {
renameSync(lockPath, aside);
} catch (err) {
if ((err as NodeJS.ErrnoException).code === 'ENOENT') return false; // another stealer won
throw err;
}
// (2) Confirm what we moved is what we judged; if a live lock slipped into the
// read→rename gap, put it back — a live holder must never be displaced.
if (!matchesJudgment(readRecord(aside), expected)) {
try {
renameSync(aside, lockPath); // restore; an overwritten concurrent creator's read-back backs it off
} catch {
/* slot re-taken between our move and restore — leave it; we lost the reclaim */
}
return false;
}
try {
unlinkSync(aside); // uniquely ours by token → safe; best-effort
} catch {
/* leftover .dead.<token> is inert (not analyze.lock, not swept) — harmless */
}
return true;
};
/**
* Placeholder holder for an {@link IndexLockTimeoutError} thrown while the lock
* file exists but no valid record can be read (malformed/partial), or it keeps
* vanishing — there is no real holder to name, but the error still needs one so
* the CLI's `err.holder.pid` stays defined. This path is a rare backstop:
* malformed files are reclaimed within {@link MALFORMED_GRACE_MS}.
*/
const unknownHolder = (): LockRecord => ({
v: LOCK_RECORD_VERSION,
pid: -1,
hostname: HOSTNAME,
startTime: null,
token: '',
invocationId: '<unreadable>',
acquiredAt: '',
});
/**
* Filesystem-create error codes we tolerate by proceeding lock-free: a
* read-only mount (EROFS) or a denied create (EACCES/EPERM). Such a filesystem
* rejects every index WRITE in the same directory too, so no concurrent writer
* can exist and the lock is moot — an already-indexed repo on a `:ro` mount
* must still reach its `alreadyUpToDate` fast path (#2658). A genuinely-needed
* write fails later exactly as it would have without the lock.
*/
export const LOCK_UNWRITABLE_CODES: ReadonlySet<string> = new Set(['EROFS', 'EACCES', 'EPERM']);
export const isLockUnwritableCode = (code: string | undefined): boolean =>
code !== undefined && LOCK_UNWRITABLE_CODES.has(code);
/** A lock handle that owns nothing — returned when the filesystem refuses to
* create the lock file (see {@link LOCK_UNWRITABLE_CODES}). Release is a no-op. */
const noopHandle = (record: LockRecord): IndexLockHandle => ({ record, release: () => {} });
/**
* Delete orphaned build/staging artifacts left in the lock directory by a
* crashed prior writer. Safe precisely because we hold the exclusive lock: no
* other writer can be creating these here right now, so anything present is a
* crash orphan. Matches this slot's staging files ONLY — never `lbug` itself,
* never `lbug.wal`/`lbug.shadow` (the LIVE index's own sidecars), and never a
* `branches/<slug>/` sub-slot (which owns its own lock + sweep). Non-recursive.
*/
export const sweepStagingArtifacts = (lockDir: string, log?: (msg: string) => void): void => {
// Matches `lbug.new`, `lbug.new.wal`, `lbug.staging.<id>`, `lbug.staging.<id>.wal`, …
// Does NOT match `lbug`, `lbug.wal`, `lbug.shadow`.
const stagingRe = /^lbug\.(staging\..+|new(\..+)?)$/;
let removed = 0;
let entries: string[];
try {
entries = readdirSync(lockDir);
} catch {
return;
}
for (const name of entries) {
if (!stagingRe.test(name)) continue;
try {
unlinkSync(path.join(lockDir, name));
removed++;
} catch {
/* best-effort */
}
}
if (removed > 0) {
log?.(`Cleared ${removed} orphaned index-staging file(s) from a prior interrupted analyze.`);
}
};
const sleep = (ms: number): Promise<void> => new Promise((resolve) => setTimeout(resolve, ms));
/** Poll delay with jitter (avoids two waiters lock-stepping), clamped so it
* never overshoots the remaining timeout budget. Callers guarantee
* `waited < timeoutMs`, so the result is ≥ 1. */
const jitteredDelay = (pollMs: number, timeoutMs: number, waited: number): number => {
const jitter = Math.floor(Math.random() * pollMs);
const remaining = timeoutMs - waited;
return Math.max(1, Math.min(pollMs + jitter, remaining));
};
/**
* Resolve the wait ceiling. Explicit `opt` wins; else
* GITNEXUS_INDEX_LOCK_TIMEOUT_MS; else {@link DEFAULT_TIMEOUT_MS}. A value ≤ 0
* (from either source) means unbounded.
*/
const resolveTimeoutMs = (opt?: number): number => {
const raw =
typeof opt === 'number'
? opt
: (() => {
const env = process.env.GITNEXUS_INDEX_LOCK_TIMEOUT_MS;
if (env === undefined || env === '') return DEFAULT_TIMEOUT_MS;
const n = Number(env);
return Number.isFinite(n) ? n : DEFAULT_TIMEOUT_MS;
})();
return raw <= 0 ? Number.POSITIVE_INFINITY : raw;
};
/**
* Acquire the exclusive write lock for `lockDir` (the resolved index slot
* directory, e.g. `<repo>/.gitnexus` or `<repo>/.gitnexus/branches/<slug>`).
*
* Blocks until the lock is held (waiting only on live holders, stealing dead
* ones immediately), then sweeps orphaned staging files under the lock and
* returns a handle. Rejects with `IndexLockTimeoutError` if `timeoutMs` is
* exceeded while a live holder still holds the lock.
*/
/**
* File-based (O_EXCL pidfile) backend. The portable fallback used on platforms
* without the socket backend (macOS/BSD) or when the OS socket lock is
* unavailable. Carries the pid-liveness staleness, atomic rename-steal reclaim,
* bounded malformed-file handling, and read-only tolerance. Its stale-takeover
* has an irreducible (narrow) race — see the module header — which is why the
* socket backend is preferred where available.
*/
const acquireViaFile = async (
lockDir: string,
me: LockRecord,
opts: AcquireOptions,
): Promise<IndexLockHandle> => {
try {
mkdirSync(lockDir, { recursive: true });
} catch (err) {
// Read-only / denied filesystem → proceed lock-free (see LOCK_UNWRITABLE_CODES).
if (isLockUnwritableCode((err as NodeJS.ErrnoException).code)) return noopHandle(me);
throw err;
}
const lockPath = path.join(lockDir, LOCK_FILENAME);
const pollMs = opts.pollMs ?? DEFAULT_POLL_MS;
const timeoutMs = resolveTimeoutMs(opts.timeoutMs);
const startedAt = Date.now();
let announcedWait = false;
let lastDiagnosticAt = 0;
// When the lock file exists but has no readable record, the timestamp we
// first observed it unreadable — used to reclaim a crash-orphan after a grace.
let malformedSince: number | null = null;
for (;;) {
try {
// O_WRONLY | O_CREAT | O_EXCL — the atomic arbiter of ownership.
const fd = openSync(lockPath, 'wx');
try {
writeSync(fd, JSON.stringify(me));
} finally {
closeSync(fd);
}
// Read-back verify (#2658 review L5): if this process stalled (a >graceMs
// GC pause) between the O_EXCL create of the *empty* file and the write
// above, a waiter could have reclaimed the empty file (renamed it aside)
// and O_EXCL-created its own lock at `lockPath`. Our write then landed on
// the renamed-aside inode, not `lockPath`. Confirm `lockPath` still carries
// our token before claiming ownership; if it was stolen, contend normally.
const confirmed = readRecord(lockPath);
if (!confirmed || confirmed.token !== me.token) continue;
return {
record: me,
release: () => {
const current = readRecord(lockPath);
if (current && current.token !== me.token) return; // no longer ours
try {
unlinkSync(lockPath);
} catch {
/* already gone */
}
},
};
} catch (err) {
const code = (err as NodeJS.ErrnoException).code;
if (code === 'EEXIST') {
// fall through to holder inspection / wait / reclaim below
} else if (isLockUnwritableCode(code)) {
return noopHandle(me); // read-only / denied → proceed lock-free
} else {
throw err;
}
}
const holder = readRecord(lockPath);
const waited = Date.now() - startedAt;
if (holder) {
malformedSince = null;
if (isStale(holder)) {
opts.log?.(
`Reclaiming stale index lock from dead analyze (pid ${holder.pid}, ` +
`invocation ${holder.invocationId}).`,
);
stealLock(lockPath, me, holder); // reclaim ONLY this dead record; live locks are never stolen
continue;
}
// Live holder → wait.
if (!announcedWait) {
announcedWait = true;
opts.onWaitStart?.(holder);
opts.log?.(
`Another gitnexus analyze (pid ${holder.pid} on ${holder.hostname}) is ` +
`refreshing this index — waiting for it to finish.`,
);
}
if (waited >= timeoutMs) throw new IndexLockTimeoutError(holder, waited);
if (Date.now() - lastDiagnosticAt >= DIAGNOSTIC_INTERVAL_MS) {
lastDiagnosticAt = Date.now();
if (waited >= DIAGNOSTIC_INTERVAL_MS) {
opts.log?.(
`Still waiting for analyze pid ${holder.pid} (${Math.round(waited / 1000)}s elapsed).`,
);
}
}
await sleep(jitteredDelay(pollMs, timeoutMs, waited));
continue;
}
// holder === null: the lock file is either gone (vanished between the failed
// create and our read) or present-but-unreadable (a crash between the
// O_EXCL create and the record write, or a partial write). NEVER hot-loop
// here — both branches are bounded by sleep + timeout.
if (!existsSync(lockPath)) {
malformedSince = null; // genuinely vanished → the next create likely wins
if (waited >= timeoutMs) throw new IndexLockTimeoutError(unknownHolder(), waited, false);
await sleep(jitteredDelay(pollMs, timeoutMs, waited));
continue;
}
// Malformed orphan present. Reclaim only after a grace, so a live owner's
// microsecond create→write window is never mistaken for a crash.
if (malformedSince === null) malformedSince = Date.now();
if (Date.now() - malformedSince >= malformedGraceMs(pollMs)) {
opts.log?.('Reclaiming a malformed/partial index lock file (no readable owner record).');
stealLock(lockPath, me, null); // reclaim ONLY while still unreadable; a live lock written since is left
malformedSince = null;
continue;
}
if (waited >= timeoutMs) throw new IndexLockTimeoutError(unknownHolder(), waited, false);
await sleep(jitteredDelay(pollMs, timeoutMs, waited));
}
};
/** Signals that the OS socket backend can't be used here (e.g. abstract
* namespace disabled, sandbox, or an unexpected bind error) so the caller
* should fall back to the file backend. NOT thrown for EADDRINUSE (that is a
* live holder → wait) or timeouts (those propagate as IndexLockTimeoutError). */
class SocketLockUnavailable extends Error {
constructor(readonly cause: NodeJS.ErrnoException) {
super(`OS socket lock unavailable: ${cause.code ?? cause.message}`);
this.name = 'SocketLockUnavailable';
}
}
/**
* Canonicalize a path to its real filesystem identity so lexical aliases of the
* same directory (a symlink, a bind-mount path, a Windows junction, a `\\?\`
* prefix) map to ONE name (#2658 review H1). `lockDir` (the index slot) often
* does not exist yet, so `realpathSync` the deepest existing ancestor and
* re-append the not-yet-created remainder. A path with no symlink components
* realpaths to itself, so the common (non-aliased) case is unchanged — a holder
* that used the old resolved name is never orphaned.
*/
const canonicalizeDir = (p: string): string => {
const resolved = path.resolve(p);
const tail: string[] = [];
let dir = resolved;
for (;;) {
try {
const real = realpathSync(dir);
return tail.length ? path.join(real, ...tail.reverse()) : real;
} catch {
const parent = path.dirname(dir);
if (parent === dir) return resolved; // reached the root with nothing to resolve
tail.push(path.basename(dir));
dir = parent;
}
}
};
/**
* Stable OS-IPC endpoint name for an index directory. The name is derived from
* the REAL path (case-folded on Windows), so two processes targeting the same
* physical slot — even via different lexical aliases — collide, and separate
* worktrees/branches never do. The endpoint lives OUTSIDE the index directory
* (abstract namespace / pipe namespace), so the lock needs no filesystem write
* and is unaffected by a read-only index mount.
*/
const socketLockName = (lockDir: string): string => {
const resolved = canonicalizeDir(lockDir);
const key = createHash('sha256')
.update(process.platform === 'win32' ? resolved.toLowerCase() : resolved)
.digest('hex')
.slice(0, 32);
return process.platform === 'win32'
? `\\\\.\\pipe\\gitnexus-idx-${key}`
: `\0gitnexus-idx-${key}`; // Linux abstract socket (no filesystem entry)
};
/** Attempt to listen; resolve to null on success or the error on failure. */
const tryListen = (server: net.Server, name: string): Promise<NodeJS.ErrnoException | null> =>
new Promise((resolve) => {
const onError = (err: NodeJS.ErrnoException): void => {
server.removeListener('listening', onListening);
resolve(err);
};
const onListening = (): void => {
server.removeListener('error', onError);
resolve(null);
};
server.once('error', onError);
server.once('listening', onListening);
server.listen(name);
});
/**
* OS-owned socket/pipe backend (Windows named pipe, Linux abstract socket).
* Holding the lock = holding a listening endpoint the kernel binds to this
* process; `EADDRINUSE` therefore means a *live* holder, and the kernel drops
* the binding the instant the holder exits (clean exit, crash, OOM, SIGKILL) —
* so there is no stale detection, no reclaim, and no takeover race. See the
* module header for why this is preferred over the file backend.
*/
const acquireViaSocket = async (
lockDir: string,
me: LockRecord,
opts: AcquireOptions,
): Promise<IndexLockHandle> => {
const name = socketLockName(lockDir);
const pollMs = opts.pollMs ?? DEFAULT_POLL_MS;
const timeoutMs = resolveTimeoutMs(opts.timeoutMs);
const startedAt = Date.now();
let announcedWait = false;
let lastDiagnosticAt = 0;
for (;;) {
const server = net.createServer();
// Never keep the process alive on the lock's account, and never hold an
// incoming connection (nothing should connect; drop any stray peer).
server.unref();
server.on('connection', (sock) => sock.destroy());
const listenErr = await tryListen(server, name);
if (!listenErr) {
return {
record: me,
release: () => {
try {
server.close();
} catch {
/* already closed / releasing on exit */
}
},
};
}
// This server never bound (listen failed); release its handle before the
// next poll or the fallback, so a long contended wait doesn't churn one
// unclosed net.Server per iteration (#2658 review L3).
try {
server.close();
} catch {
/* never listened */
}
// Only EADDRINUSE means "held by a live holder → wait". Anything else means
// this environment can't use the socket backend → fall back to the file one.
if (listenErr.code !== 'EADDRINUSE') throw new SocketLockUnavailable(listenErr);
if (!announcedWait) {
announcedWait = true;
opts.onWaitStart?.(me);
opts.log?.('Another gitnexus analyze is refreshing this index — waiting for it to finish.');
}
const waited = Date.now() - startedAt;
// Socket backend exposes no owner metadata → holder identity is unknown (M3).
if (waited >= timeoutMs) throw new IndexLockTimeoutError(unknownHolder(), waited, false);
if (Date.now() - lastDiagnosticAt >= DIAGNOSTIC_INTERVAL_MS) {
lastDiagnosticAt = Date.now();
if (waited >= DIAGNOSTIC_INTERVAL_MS) {
opts.log?.(`Still waiting for another analyze (${Math.round(waited / 1000)}s elapsed).`);
}
}
await sleep(jitteredDelay(pollMs, timeoutMs, waited));
}
};
/** Platforms whose OS IPC namespace gives a clean, auto-releasing lock via
* `net`: Windows named pipes and Linux abstract sockets. Elsewhere (macOS/BSD)
* the file backend is used (no abstract namespace; filesystem sockets don't
* release cleanly on death). Override for tests via GITNEXUS_INDEX_LOCK_BACKEND
* = 'socket' | 'file'.
*
* Scope caveat: the socket backend's mutual-exclusion domain is NOT uniform.
* Windows `\\.\pipe\` names are machine-wide (all sessions); Linux abstract
* sockets are network-namespace-scoped (network_namespaces(7)). So two writers
* that share a bind-mounted index dir but sit in separate netns (e.g. two
* containers, Docker's default) do NOT collide on Linux — "single-host" is
* really "single-netns" here. That cross-netns-shared-mount case is the one
* the file backend (shared-filesystem O_EXCL) would cover; set
* GITNEXUS_INDEX_LOCK_BACKEND=file there. The motivating case (local hook-
* driven re-index) is single-netns, so the default socket backend covers it. */
const selectBackend = (): 'socket' | 'file' => {
const override = process.env.GITNEXUS_INDEX_LOCK_BACKEND;
if (override === 'socket' || override === 'file') return override;
return process.platform === 'win32' || process.platform === 'linux' ? 'socket' : 'file';
};
/**
* Acquire the exclusive write lock for `lockDir` (the resolved index slot
* directory). Uses the OS socket/pipe backend where available (Windows/Linux),
* falling back to the file backend otherwise or if the socket backend is
* unusable in this environment. After acquiring, sweeps orphaned staging files
* under the lock (best-effort; a no-op on a read-only mount). Rejects with
* `IndexLockTimeoutError` if `timeoutMs` elapses while another live holder holds
* the lock.
*/
export const acquireIndexLock = async (
lockDir: string,
opts: AcquireOptions = {},
): Promise<IndexLockHandle> => {
const me = buildRecord();
let handle: IndexLockHandle;
if (selectBackend() === 'socket') {
try {
handle = await acquireViaSocket(lockDir, me, opts);
} catch (err) {
if (!(err instanceof SocketLockUnavailable)) throw err; // timeout etc. propagate
opts.log?.('Index lock: OS socket lock unavailable here — using the file lock.');
handle = await acquireViaFile(lockDir, me, opts);
}
} else {
handle = await acquireViaFile(lockDir, me, opts);
}
// Reclaim crashed-build staging orphans while we hold the lock. Best-effort:
// a read-only mount (no orphans reachable) just no-ops.
try {
sweepStagingArtifacts(lockDir, opts.log);
} catch {
/* best-effort */
}
return handle;
};

58
gitnexus/test/fixtures/index-lock-child.mjs vendored Normal file
View file

@ -0,0 +1,58 @@
/**
* Child process for the cross-process index-lock tests (#2658), using the BUILT
* module (LOCK_MODULE). Two modes:
*
* - default (HOLD): acquire the lock on LOCK_DIR, write MARKER once held, then
* hold until killed. Proves real cross-process exclusion and SIGKILL
* kill-recovery against a parent that uses the source module.
*
* - MODE=EXCLUSIVE (SENTINEL set): acquire, then enter a critical section
* guarded by an O_EXCL sentinel create — if the sentinel already exists,
* another process holds the lock at the same time, which is the exact
* single-writer violation the test hunts. Hold briefly, remove the sentinel,
* release, exit 0. Exit 3 if the sentinel was already present (overlap).
* Used by the multi-reclaimer test where ≥2 children reclaim one dead holder.
*/
import { writeFileSync, openSync, closeSync, unlinkSync } from 'node:fs';
import { pathToFileURL } from 'node:url';
// LOCK_MODULE is an absolute path. On Windows `import('C:\\…')` throws
// ERR_UNSUPPORTED_ESM_URL_SCHEME (a bare drive path is read as a URL scheme), so
// convert to a file:// URL — required on Windows, harmless on POSIX.
const { acquireIndexLock } = await import(pathToFileURL(process.env.LOCK_MODULE).href);
if (process.env.MODE === 'EXCLUSIVE') {
const lock = await acquireIndexLock(process.env.LOCK_DIR, { timeoutMs: 30_000, pollMs: 25 });
try {
// O_EXCL create fails if any other process is simultaneously in its own
// critical section — that is a broken single-writer invariant.
let fd;
try {
fd = openSync(process.env.SENTINEL, 'wx');
} catch {
process.exit(3); // overlap detected: two holders at once
}
closeSync(fd);
// Hold the section briefly so concurrent reclaimers would collide here.
await new Promise((r) => setTimeout(r, 150));
unlinkSync(process.env.SENTINEL);
} finally {
lock.release();
}
process.exit(0);
} else {
const lock = await acquireIndexLock(process.env.LOCK_DIR, { timeoutMs: 30_000, pollMs: 25 });
writeFileSync(process.env.MARKER, String(process.pid));
// Hold the lock until the parent kills us.
setInterval(() => {}, 1000);
// Release on a graceful signal (the SIGKILL path in the test never reaches this).
const release = () => {
try {
lock.release();
} finally {
process.exit(0);
}
};
process.on('SIGTERM', release);
process.on('SIGINT', release);
}

12
gitnexus/test/integration/analyze-atomic-swap.test.ts
View file

@ -1,9 +1,10 @@
/**
* Integration test for the #2 atomic full-rebuild swap.
*
* A full rebuild builds the fresh index at `<lbugPath>.new` and swaps it over
* the live index in one atomic rename (POSIX). Two invariants:
* - success publishes a single valid `lbug` with no `.new` temp left behind,
* A full rebuild builds the fresh index at a per-run `<lbugPath>.staging.<uuid>`
* (#2658) and swaps it over the live index in one atomic rename (POSIX). Two
* invariants:
* - success publishes a single valid `lbug` with no staging temp left behind,
* and a repeat rebuild replaces the inode (proving the swap, not an in-place
* edit); and
* - a failure BEFORE the swap leaves the previous index byte-for-byte intact
@ -49,7 +50,10 @@ const identity = async (p: string): Promise<string> => {
const lingeringTemp = async (lbugPath: string): Promise<string[]> => {
const base = path.basename(lbugPath);
const entries = await fs.readdir(path.dirname(lbugPath));
return entries.filter((e) => e.startsWith(`${base}.new`));
// Staging temps are the legacy fixed `${base}.new*` and the current per-run
// `${base}.staging.<uuid>*` (#2658). Match both so this leftover-temp guard
// still catches a failed swap under the new naming.
return entries.filter((e) => e.startsWith(`${base}.new`) || e.startsWith(`${base}.staging.`));
};
describe.skipIf(isWin)('atomic full-rebuild swap (#2)', () => {

173
gitnexus/test/integration/analyze-index-lock-concurrency.test.ts Normal file
View file

@ -0,0 +1,173 @@
/**
* Real cross-process tests for the index write lock (#2658): child processes
* contend for the lock on the same directory as this process.
*
* - Test 1 exercises the DEFAULT backend (the OS socket/pipe lock on
* Linux/Windows): while the child holds it, our acquire blocks and times out;
* after the child is SIGKILLed the kernel drops the binding and our next
* acquire succeeds — the kernel-auto-release guarantee, no stale handling.
* - Test 2 pins the FILE backend and races several children reclaiming one dead
* holder, asserting the atomic rename-steal never lets two into the critical
* section at once.
*
* The child imports the BUILT module (dist/) and this process imports the
* source, proving the guarantee is a genuine cross-process one (and, for the
* socket backend, that both derive the same endpoint name for a given dir).
*/
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { spawn, type ChildProcess } from 'node:child_process';
import { mkdtempSync, rmSync, existsSync, readFileSync, writeFileSync } from 'node:fs';
import os from 'node:os';
import path from 'node:path';
import { fileURLToPath } from 'node:url';
import { acquireIndexLock, IndexLockTimeoutError } from '../../src/storage/index-lock.js';
const testDir = path.dirname(fileURLToPath(import.meta.url));
const repoRoot = path.resolve(testDir, '../..');
const lockModule = path.join(repoRoot, 'dist', 'storage', 'index-lock.js');
const childScript = path.resolve(testDir, '..', 'fixtures', 'index-lock-child.mjs');
let dir: string;
let marker: string;
let child: ChildProcess | undefined;
const waitFor = async (predicate: () => boolean, timeoutMs: number): Promise<void> => {
const start = Date.now();
for (;;) {
if (predicate()) return;
if (Date.now() - start > timeoutMs) throw new Error('condition not met within timeout');
await new Promise((r) => setTimeout(r, 25));
}
};
const waitForExit = (proc: ChildProcess, timeoutMs: number): Promise<void> =>
new Promise((resolve, reject) => {
const timer = setTimeout(() => reject(new Error('child did not exit')), timeoutMs);
proc.once('exit', () => {
clearTimeout(timer);
resolve();
});
});
beforeEach(() => {
dir = mkdtempSync(path.join(os.tmpdir(), 'gnx-lock-xp-'));
marker = path.join(dir, 'held.marker');
});
afterEach(() => {
if (child && child.exitCode === null && child.signalCode === null) child.kill('SIGKILL');
rmSync(dir, { recursive: true, force: true });
});
describe('index lock across processes (#2658)', () => {
it('excludes a second writer while held, then recovers after the holder is killed', async () => {
if (!existsSync(lockModule)) {
throw new Error(
`dist/storage/index-lock.js missing — run \`npm run build\` first ` +
`(or use \`npm run test:integration\`, which builds via pretest:integration).`,
);
}
child = spawn(process.execPath, [childScript], {
env: { ...process.env, LOCK_MODULE: lockModule, LOCK_DIR: dir, MARKER: marker },
stdio: ['ignore', 'pipe', 'pipe'],
});
// Generous marker wait: Windows process startup is ~5x slower and the
// platform-sensitive shard runs heavy suites in parallel, so a child spawn
// can be badly delayed under load — the wait must tolerate that, not race it.
await waitFor(() => existsSync(marker), 40_000);
const holderPid = Number(readFileSync(marker, 'utf8'));
expect(holderPid).toBeGreaterThan(0);
// Mutual exclusion: the live holder is waited on, then we time out.
await expect(acquireIndexLock(dir, { timeoutMs: 500, pollMs: 25 })).rejects.toBeInstanceOf(
IndexLockTimeoutError,
);
// Kill recovery: with the holder gone, its lock becomes reclaimable.
child.kill('SIGKILL');
await waitForExit(child, 30_000);
const lock = await acquireIndexLock(dir, { timeoutMs: 15_000, pollMs: 25 });
expect(lock.record.pid).toBe(process.pid);
lock.release();
}, 90_000);
// The FILE backend is the DEFAULT only on macOS/BSD; Windows and Linux default
// to the race-free kernel lock (named pipe / abstract socket). This case FORCES
// the file backend to stress its rename-steal reclaim, so it runs where that
// backend is actually production (macOS — where the double-admit bug this
// guards lived and is now fixed) plus Linux. It is skipped on Windows, where
// the file backend is never the default; Windows' real lock (the named pipe) is
// covered by index-lock.test.ts on the Windows matrix and by the
// default-backend cross-process case above.
it.skipIf(process.platform === 'win32')(
'lets multiple waiters reclaim one dead holder without ever admitting two writers',
async () => {
if (!existsSync(lockModule)) {
throw new Error(
`dist/storage/index-lock.js missing — run \`npm run build\` first ` +
`(or use \`npm run test:integration\`, which builds via pretest:integration).`,
);
}
// This case targets the FILE backend's reclaim path specifically (the socket
// backend has no stale file to reclaim). Seed a stale lock owned by a dead,
// same-host holder — every child must reclaim it, and the reclaim must let
// exactly one at a time win so no two children are ever in their O_EXCL
// sentinel section together.
//
// The reclaim's rename-steal must NOT act on a stale staleness judgment: a
// waiter that judged the dead record must re-verify the file still holds it
// before renaming, or it will rename a live winner's freshly-created lock
// aside and admit a second writer (#2658 review — this reproduced at ~18% per
// round of 4-way contention before the judgment-verified steal). One round
// catches that regression only ~1-in-6 of the time, so loop several rounds to
// make it a reliable guard; with the fix every round is clean.
const sentinel = path.join(dir, 'critical.sentinel');
const seedDeadHolder = (): void => {
writeFileSync(
path.join(dir, 'analyze.lock'),
JSON.stringify({
v: 1,
pid: 999_999_999,
hostname: os.hostname(),
startTime: null,
token: 'dead-holder-token',
invocationId: 'dead-holder',
acquiredAt: new Date(0).toISOString(),
}),
);
};
const runChild = (): Promise<{ code: number | null; signal: NodeJS.Signals | null }> =>
new Promise((resolve) => {
const c = spawn(process.execPath, [childScript], {
env: {
...process.env,
LOCK_MODULE: lockModule,
LOCK_DIR: dir,
SENTINEL: sentinel,
MODE: 'EXCLUSIVE',
GITNEXUS_INDEX_LOCK_BACKEND: 'file',
},
stdio: ['ignore', 'pipe', 'pipe'],
});
c.once('exit', (code, signal) => resolve({ code, signal }));
});
const ROUNDS = 8;
const KIDS = 5;
for (let round = 0; round < ROUNDS; round++) {
seedDeadHolder(); // the previous round's winner released (unlinked) the lock
const results = await Promise.all(Array.from({ length: KIDS }, () => runChild()));
// Every child acquired, ran its exclusive section, and exited cleanly (0).
// Exit 3 = it found the sentinel already present = two holders at once.
for (const r of results) {
expect(r.signal).toBeNull();
expect(r.code).toBe(0);
}
// No leftover sentinel — the last holder cleaned up.
expect(existsSync(sentinel)).toBe(false);
}
},
60_000,
);
});

92
gitnexus/test/integration/analyze-wal-checkpoint-failure.test.ts
View file

@ -72,45 +72,73 @@ afterAll(() => {
if (suiteGitnexusHome) cleanupTempDirSync(suiteGitnexusHome);
});
const runAnalyze = () =>
spawnSync(process.execPath, [...CLI_SPAWN_PREFIX, 'analyze', '--skip-skills'], {
cwd: repoPath,
encoding: 'utf8',
// Generous timeout: the test does real CSV/COPY work before the
// first failing checkpoint, and CI runners are slow.
timeout: process.env.CI ? 120_000 : 60_000,
stdio: ['pipe', 'pipe', 'pipe'],
env: {
...process.env,
GITNEXUS_HOME: suiteGitnexusHome,
// Skip ensureHeap re-exec (which drops the tsx loader).
NODE_OPTIONS: `${process.env.NODE_OPTIONS || ''} --max-old-space-size=8192`.trim(),
// Tiny threshold forces auto-checkpoint on every write so the
// first write into the WAL trips the planted rename blocker.
GITNEXUS_WAL_CHECKPOINT_THRESHOLD: '1',
CI: '1',
},
});
describe('analyze WAL auto-checkpoint rename failure (real lbug, no mocks)', () => {
it('surfaces the --wal-checkpoint-threshold recovery hint when the rename target is blocked', () => {
// Plant a non-empty directory at the path Ladybug's auto-checkpoint
// will try to rename `<db>.wal` over. `fs.rename` cannot overwrite a
// non-empty directory, and the adapter's orphan-sidecar cleanup uses
// `fs.unlink` (which fails on directories) — so the blocker persists
// through `doInitLbug` and trips the very first auto-checkpoint that
// a `GITNEXUS_WAL_CHECKPOINT_THRESHOLD=1` setting forces.
// The checkpoint rename target must be a PREDICTABLE path so the blocker
// can be pre-planted. A full rebuild builds into a per-run
// `lbug.staging.<uuid>` and checkpoints `lbug.staging.<uuid>.wal.checkpoint`
// (#2658) — an unknowable name. An INCREMENTAL run instead writes the live
// index in place, so its auto-checkpoint targets the fixed
// `lbug.wal.checkpoint`. So: first do a clean full analyze to create the
// index, then plant the blocker and drive an incremental analyze into it.
const storageDir = path.join(repoPath, '.gitnexus');
fs.mkdirSync(storageDir, { recursive: true });
// A full rebuild now builds into `lbug.new` and swaps atomically (POSIX), so
// its auto-checkpoint targets `lbug.new.wal.checkpoint`; on the in-place /
// Windows path it targets `lbug.wal.checkpoint`. Block BOTH so the planted
// rename blocker trips the first checkpoint whichever path analyze takes.
for (const name of ['lbug.wal.checkpoint', 'lbug.new.wal.checkpoint']) {
const blockerDir = path.join(storageDir, name);
fs.mkdirSync(blockerDir, { recursive: true });
fs.writeFileSync(path.join(blockerDir, 'blocker'), 'cannot-be-renamed-over');
}
const result = spawnSync(process.execPath, [...CLI_SPAWN_PREFIX, 'analyze', '--skip-skills'], {
// 1) Clean full analyze (no blocker) — builds the index into staging and
// swaps it in. Must succeed; the staging checkpoint name is unblocked.
const first = runAnalyze();
expect(first.status === null ? 'timeout' : first.status).toBe(0);
// 2) Change a tracked source file and commit, so the next analyze is an
// incremental writeback (in-place), not a full rebuild.
const churnFile = path.join(repoPath, 'src', 'logger.ts');
fs.appendFileSync(churnFile, `\nexport const walChurnMarker = ${Date.now()};\n`);
const gitEnv = {
...process.env,
GIT_AUTHOR_NAME: 'test',
GIT_AUTHOR_EMAIL: 'test@test',
GIT_COMMITTER_NAME: 'test',
GIT_COMMITTER_EMAIL: 'test@test',
};
spawnSync('git', ['add', '-A'], { cwd: repoPath, stdio: 'pipe' });
spawnSync('git', ['commit', '-m', 'churn for incremental'], {
cwd: repoPath,
encoding: 'utf8',
// Generous timeout: the test does real CSV/COPY work before the
// first failing checkpoint, and CI runners are slow.
timeout: process.env.CI ? 120_000 : 60_000,
stdio: ['pipe', 'pipe', 'pipe'],
env: {
...process.env,
GITNEXUS_HOME: suiteGitnexusHome,
// Skip ensureHeap re-exec (which drops the tsx loader).
NODE_OPTIONS: `${process.env.NODE_OPTIONS || ''} --max-old-space-size=8192`.trim(),
// Tiny threshold forces auto-checkpoint on every write so the
// first write into the WAL trips the planted rename blocker.
GITNEXUS_WAL_CHECKPOINT_THRESHOLD: '1',
CI: '1',
},
stdio: 'pipe',
env: gitEnv,
});
// 3) Plant a non-empty directory at `lbug.wal.checkpoint`, the fixed rename
// target of the in-place checkpoint. `fs.rename` cannot overwrite a
// non-empty directory, and the adapter's orphan-sidecar cleanup uses
// `fs.unlink` (which fails on a directory) — so the blocker persists through
// `doInitLbug` and trips the auto-checkpoint the incremental writeback
// forces at `GITNEXUS_WAL_CHECKPOINT_THRESHOLD=1`.
const blockerDir = path.join(storageDir, 'lbug.wal.checkpoint');
fs.rmSync(blockerDir, { recursive: true, force: true });
fs.mkdirSync(blockerDir, { recursive: true });
fs.writeFileSync(path.join(blockerDir, 'blocker'), 'cannot-be-renamed-over');
// 4) Incremental analyze into the blocked checkpoint target.
const result = runAnalyze();
const combined = `${result.stderr}\n${result.stdout}`;
// The CLI must exit non-zero. status === null means the timeout fired

32
gitnexus/test/unit/analyze-worker-core.test.ts
View file

@ -20,6 +20,7 @@ import {
import type { AnalyzeResult } from '../../src/core/run-analyze.js';
import type { WorkerMessage } from '../../src/server/analyze-worker.js';
import type { AnalyzerRunnerIdentity } from '../../src/storage/repo-manager.js';
import { IndexLockTimeoutError, type LockRecord } from '../../src/storage/index-lock.js';
const baseResult: AnalyzeResult = {
repoName: 'repo',
@ -121,6 +122,37 @@ describe('runWorkerAnalysis — finalize guard (#2264 P2)', () => {
expect(send).toHaveBeenCalledWith({ type: 'error', message: 'boom' });
expect(finalize).not.toHaveBeenCalled();
});
it('tags an index-lock timeout as a retryable index-lock-timeout error (#2658 review M2)', async () => {
const send = vi.fn<(msg: WorkerMessage) => void>();
const holder: LockRecord = {
v: 1,
pid: -1,
hostname: 'host',
startTime: null,
token: '',
invocationId: 'unknown',
acquiredAt: '',
};
const lockContended: WorkerAnalysisDeps['runFullAnalysis'] = vi.fn(async () => {
throw new IndexLockTimeoutError(holder, 600_000, false);
});
await runWorkerAnalysis(
'/repo',
{},
{
runFullAnalysis: lockContended,
assertAnalysisFinalized: okFinalize,
send,
claimTerminal: alwaysClaim,
},
);
expect(send).toHaveBeenCalledWith(
expect.objectContaining({ type: 'error', code: 'index-lock-timeout', retryable: true }),
);
});
});
describe('runWorkerAnalysis — terminal-claim coordination (#2264 P3)', () => {

381
gitnexus/test/unit/index-lock.test.ts Normal file
View file

@ -0,0 +1,381 @@
/**
* Unit tests for the cross-process index write lock (#2658).
*
* These exercise the lock's decision logic deterministically by pre-seeding
* `analyze.lock` records and asserting acquire/steal/release/sweep behavior —
* including the kill-recovery mechanism (a dead holder's lock is reclaimed) and
* mutual exclusion (a live holder is waited on, never stolen). A real
* two-process exclusion + SIGKILL-recovery test lives in
* test/integration/analyze-index-lock-concurrency.test.ts.
*/
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import {
mkdtempSync,
rmSync,
writeFileSync,
readFileSync,
existsSync,
chmodSync,
symlinkSync,
} from 'node:fs';
import os from 'node:os';
import path from 'node:path';
import {
acquireIndexLock,
sweepStagingArtifacts,
isLockUnwritableCode,
IndexLockTimeoutError,
type LockRecord,
} from '../../src/storage/index-lock.js';
import { classifyFtsBuildError, ftsFailureIsFatal } from '../../src/core/search/fts-indexes.js';
let dir: string;
const lockPath = () => path.join(dir, 'analyze.lock');
const seedLock = (overrides: Partial<LockRecord>): void => {
const record: LockRecord = {
v: 1,
pid: 999999999, // implausible pid → dead by default
hostname: os.hostname(),
startTime: null,
token: 'seed-token',
invocationId: 'seed-invocation',
acquiredAt: new Date().toISOString(),
...overrides,
};
writeFileSync(lockPath(), JSON.stringify(record));
};
beforeEach(() => {
dir = mkdtempSync(path.join(os.tmpdir(), 'gnx-lock-'));
// These suites exercise the file (O_EXCL pidfile) backend directly. On Linux
// the default is the socket backend, so pin the file backend explicitly.
process.env.GITNEXUS_INDEX_LOCK_BACKEND = 'file';
});
afterEach(() => {
delete process.env.GITNEXUS_INDEX_LOCK_BACKEND;
rmSync(dir, { recursive: true, force: true });
});
describe('acquireIndexLock', () => {
it('acquires a free directory and writes a record carrying our pid', async () => {
const lock = await acquireIndexLock(dir);
expect(existsSync(lockPath())).toBe(true);
const onDisk = JSON.parse(readFileSync(lockPath(), 'utf8')) as LockRecord;
expect(onDisk).toMatchObject({ v: 1, pid: process.pid, hostname: os.hostname() });
expect(lock.record.token).toBe(onDisk.token);
lock.release();
expect(existsSync(lockPath())).toBe(false);
});
it('reclaims a stale lock left by a dead process (kill recovery)', async () => {
seedLock({ pid: 999999999, token: 'dead-holder' });
const lock = await acquireIndexLock(dir, { timeoutMs: 2000 });
const onDisk = JSON.parse(readFileSync(lockPath(), 'utf8')) as LockRecord;
expect(onDisk.pid).toBe(process.pid);
expect(onDisk.token).not.toBe('dead-holder');
lock.release();
});
it('waits on a live holder and times out instead of stealing (mutual exclusion)', async () => {
// A live pid (our own) with a different token — never stale, so acquire
// must block and then time out rather than clobber the holder.
seedLock({ pid: process.pid, startTime: null, token: 'live-holder' });
await expect(acquireIndexLock(dir, { timeoutMs: 300, pollMs: 20 })).rejects.toBeInstanceOf(
IndexLockTimeoutError,
);
// The live holder's record is untouched.
const onDisk = JSON.parse(readFileSync(lockPath(), 'utf8')) as LockRecord;
expect(onDisk.token).toBe('live-holder');
});
it('surfaces the holder identity on timeout', async () => {
seedLock({ pid: process.pid, startTime: null, token: 'live-holder', invocationId: 'held-run' });
await expect(acquireIndexLock(dir, { timeoutMs: 200, pollMs: 20 })).rejects.toMatchObject({
holder: { invocationId: 'held-run', pid: process.pid },
});
});
it.skipIf(process.platform !== 'linux')(
'treats a reused pid (live pid, different start time) as stale',
async () => {
// Our pid is alive but the seeded start time cannot match it → reused.
seedLock({ pid: process.pid, startTime: '1', token: 'reused-pid' });
const lock = await acquireIndexLock(dir, { timeoutMs: 2000 });
const onDisk = JSON.parse(readFileSync(lockPath(), 'utf8')) as LockRecord;
expect(onDisk.token).not.toBe('reused-pid');
lock.release();
},
);
it('reclaims an empty lock file (crash between O_EXCL create and record write) without hanging', async () => {
// Pre-fix, readRecord→null hot-looped forever here. Post-fix it reclaims
// the malformed orphan after the grace and acquires.
writeFileSync(lockPath(), '');
const lock = await acquireIndexLock(dir, { timeoutMs: 5000, pollMs: 20 });
const onDisk = JSON.parse(readFileSync(lockPath(), 'utf8')) as LockRecord;
expect(onDisk.pid).toBe(process.pid);
lock.release();
});
it('reclaims a partial/malformed record (valid JSON, missing token) without hanging', async () => {
writeFileSync(lockPath(), '{"pid":123}');
const lock = await acquireIndexLock(dir, { timeoutMs: 5000, pollMs: 20 });
const onDisk = JSON.parse(readFileSync(lockPath(), 'utf8')) as LockRecord;
expect(onDisk.pid).toBe(process.pid);
expect(onDisk.token.length).toBeGreaterThan(0);
lock.release();
});
it('treats a non-positive/NaN pid as no readable holder and reclaims (never wedges on process.kill) (#2658 review L4)', async () => {
// `{"pid":0}` pre-fix: typeof 0 === 'number' passed readRecord, then
// process.kill(0,0) reported the process group "alive" → treated as a live
// holder → the acquire wedged until the full timeout. Post-fix a pid that is
// not a positive integer makes readRecord return null, so the file is a
// malformed orphan that is reclaimed after the grace.
seedLock({ pid: 0, token: 'zero-pid' });
const lock = await acquireIndexLock(dir, { timeoutMs: 5000, pollMs: 20 });
const onDisk = JSON.parse(readFileSync(lockPath(), 'utf8')) as LockRecord;
expect(onDisk.pid).toBe(process.pid);
expect(onDisk.token).not.toBe('zero-pid');
lock.release();
});
it('honors GITNEXUS_INDEX_LOCK_TIMEOUT_MS as the wait ceiling (bounds pid-reuse hangs)', async () => {
// A live holder we cannot steal (own pid, no start-time recorded). Without a
// finite ceiling this would hang; the env var must bound it (#2658).
seedLock({ pid: process.pid, startTime: null, token: 'live-holder' });
const prev = process.env.GITNEXUS_INDEX_LOCK_TIMEOUT_MS;
process.env.GITNEXUS_INDEX_LOCK_TIMEOUT_MS = '150';
try {
// No explicit timeoutMs → the env ceiling applies (not the 10-min default).
await expect(acquireIndexLock(dir, { pollMs: 20 })).rejects.toBeInstanceOf(
IndexLockTimeoutError,
);
} finally {
if (prev === undefined) delete process.env.GITNEXUS_INDEX_LOCK_TIMEOUT_MS;
else process.env.GITNEXUS_INDEX_LOCK_TIMEOUT_MS = prev;
}
});
});
describe('release', () => {
it('does not remove a lock that has been re-taken by another owner', async () => {
const lock = await acquireIndexLock(dir);
// Simulate the file being replaced by a different owner after we acquired.
seedLock({ pid: process.pid, token: 'someone-else' });
lock.release();
expect(existsSync(lockPath())).toBe(true); // not ours → left intact
const onDisk = JSON.parse(readFileSync(lockPath(), 'utf8')) as LockRecord;
expect(onDisk.token).toBe('someone-else');
});
it('is idempotent', async () => {
const lock = await acquireIndexLock(dir);
lock.release();
expect(() => lock.release()).not.toThrow();
});
});
describe('sweepStagingArtifacts', () => {
it('removes only staging files, never the live index or its sidecars', () => {
const files = [
'lbug',
'lbug.wal',
'lbug.shadow',
'lbug.new',
'lbug.new.wal',
'lbug.new.wal.checkpoint',
'lbug.staging.abc-123',
'lbug.staging.abc-123.wal',
'lbug.staging.abc-123.shadow',
'gitnexus.json',
];
for (const f of files) writeFileSync(path.join(dir, f), 'x');
sweepStagingArtifacts(dir);
const survives = (f: string) => existsSync(path.join(dir, f));
expect(survives('lbug')).toBe(true);
expect(survives('lbug.wal')).toBe(true);
expect(survives('lbug.shadow')).toBe(true);
expect(survives('gitnexus.json')).toBe(true);
expect(survives('lbug.new')).toBe(false);
expect(survives('lbug.new.wal')).toBe(false);
expect(survives('lbug.new.wal.checkpoint')).toBe(false);
expect(survives('lbug.staging.abc-123')).toBe(false);
expect(survives('lbug.staging.abc-123.wal')).toBe(false);
expect(survives('lbug.staging.abc-123.shadow')).toBe(false);
});
it('runs the sweep automatically on acquire', async () => {
writeFileSync(path.join(dir, 'lbug.staging.orphan'), 'x');
writeFileSync(path.join(dir, 'lbug'), 'x');
const lock = await acquireIndexLock(dir);
expect(existsSync(path.join(dir, 'lbug.staging.orphan'))).toBe(false);
expect(existsSync(path.join(dir, 'lbug'))).toBe(true);
lock.release();
});
});
describe('classifyFtsBuildError', () => {
it('classifies IO/rename/checkpoint/corruption failures as integrity', () => {
expect(
classifyFtsBuildError(
'IO exception: Error renaming file lbug.new.wal to lbug.new.wal.checkpoint. ErrorMessage: No such file or directory',
),
).toBe('integrity');
expect(classifyFtsBuildError('checkpoint failed')).toBe('integrity');
expect(classifyFtsBuildError('database file is corrupt')).toBe('integrity');
expect(classifyFtsBuildError('write failed: no space left on device (ENOSPC)')).toBe(
'integrity',
);
});
it('classifies row-level tokenizer failures as capability (degrade)', () => {
expect(classifyFtsBuildError('Failed calling LOWER: Invalid UTF-8')).toBe('capability');
expect(classifyFtsBuildError('tokenizer error on row 5')).toBe('capability');
});
it('defaults unknown failures to capability so runs are not newly failed', () => {
expect(classifyFtsBuildError('some unrecognised message')).toBe('capability');
expect(classifyFtsBuildError('missing indexes after build: File.name_fts')).toBe('capability');
});
it('keeps a bare ENOENT / bad-fd as capability so it degrades, not aborts (#2658 review L1)', () => {
// A missing extension asset / closed handle reports a generic OS error; those
// must NOT escalate to an abort on the atomic-swap path. Only a specific
// write/rename/checkpoint failure is integrity.
expect(classifyFtsBuildError('ENOENT: no such file or directory, open fts.ext')).toBe(
'capability',
);
expect(classifyFtsBuildError('read failed: bad file descriptor (EBADF)')).toBe('capability');
// The genuine build-broke rename race is still integrity via 'error renaming'.
expect(
classifyFtsBuildError('Error renaming lbug.new.wal to checkpoint: No such file or directory'),
).toBe('integrity');
});
it('lets a row-level tokenizer error win even if it mentions an integrity word', () => {
// A tokenizer error is a bad row, not a broken build — must still degrade.
expect(classifyFtsBuildError('Invalid UTF-8 during io exception path')).toBe('capability');
});
});
describe('ftsFailureIsFatal (#2658)', () => {
it('is fatal ONLY for an integrity failure on the atomic-swap path', () => {
// Atomic swap: staging DB, previous index intact → integrity may abort.
expect(ftsFailureIsFatal('integrity', true)).toBe(true);
// In-place: live DB already mutated, nothing to roll back → degrade.
expect(ftsFailureIsFatal('integrity', false)).toBe(false);
// Capability never aborts, either path.
expect(ftsFailureIsFatal('capability', true)).toBe(false);
expect(ftsFailureIsFatal('capability', false)).toBe(false);
// Missing class (ok result, or no classification) never aborts.
expect(ftsFailureIsFatal(undefined, true)).toBe(false);
});
});
// The OS socket/pipe backend is only meaningful where `net` gives a clean,
// auto-releasing namespace: Linux abstract sockets and Windows named pipes.
describe.skipIf(process.platform !== 'linux' && process.platform !== 'win32')(
'OS socket lock backend (#2658)',
() => {
// Override the file-backend pin from the outer beforeEach.
beforeEach(() => {
process.env.GITNEXUS_INDEX_LOCK_BACKEND = 'socket';
});
it('holds no filesystem lock file (works on a read-only index dir)', async () => {
const lock = await acquireIndexLock(dir, { timeoutMs: 2000 });
expect(existsSync(lockPath())).toBe(false); // endpoint is outside the dir
lock.release();
});
it('excludes a second acquire on the same dir, then frees it on release', async () => {
const first = await acquireIndexLock(dir, { timeoutMs: 2000 });
// A second acquire on the SAME slot is refused by the kernel (EADDRINUSE)
// and waits, then times out — the live holder is never displaced.
await expect(acquireIndexLock(dir, { timeoutMs: 300, pollMs: 20 })).rejects.toBeInstanceOf(
IndexLockTimeoutError,
);
first.release();
// Once released, the endpoint is free again.
const second = await acquireIndexLock(dir, { timeoutMs: 2000 });
second.release();
});
it('reports the holder as unknown on timeout — never a bogus "pid -1" (#2658 review M3)', async () => {
// The OS socket lock exposes no owner metadata, so a contended-wait timeout
// must not surface the unknownHolder() placeholder pid (-1) as if it were a
// real process the operator can look up.
const first = await acquireIndexLock(dir, { timeoutMs: 2000 });
try {
const err = await acquireIndexLock(dir, { timeoutMs: 200, pollMs: 20 }).catch((e) => e);
expect(err).toBeInstanceOf(IndexLockTimeoutError);
expect((err as IndexLockTimeoutError).holderKnown).toBe(false);
expect((err as IndexLockTimeoutError).message).not.toContain('pid -1');
} finally {
first.release();
}
});
it('excludes an acquire reaching the same physical dir via a symlink alias (#2658 review H1)', async () => {
// Pre-fix the endpoint name hashed the LEXICAL path, so `alias` (a symlink
// to `dir`) produced a different name and BOTH acquired — a double-writer.
// Post-fix both canonicalize to `dir`'s real path → one name → excluded.
const alias = mkdtempSync(path.join(os.tmpdir(), 'gnx-lock-aliasparent-'));
const aliasLink = path.join(alias, 'link');
symlinkSync(dir, aliasLink);
try {
const first = await acquireIndexLock(dir, { timeoutMs: 2000 });
await expect(
acquireIndexLock(aliasLink, { timeoutMs: 300, pollMs: 20 }),
).rejects.toBeInstanceOf(IndexLockTimeoutError);
first.release();
} finally {
rmSync(alias, { recursive: true, force: true });
}
});
it('gives independent locks to different index dirs', async () => {
const other = mkdtempSync(path.join(os.tmpdir(), 'gnx-lock-other-'));
try {
const a = await acquireIndexLock(dir, { timeoutMs: 2000 });
const b = await acquireIndexLock(other, { timeoutMs: 2000 }); // distinct name → no contention
a.release();
b.release();
} finally {
rmSync(other, { recursive: true, force: true });
}
});
},
);
describe('read-only / permission-denied filesystem (#2658)', () => {
it('classifies EROFS/EACCES/EPERM as tolerable, others not', () => {
expect(isLockUnwritableCode('EROFS')).toBe(true);
expect(isLockUnwritableCode('EACCES')).toBe(true);
expect(isLockUnwritableCode('EPERM')).toBe(true);
expect(isLockUnwritableCode('EEXIST')).toBe(false);
expect(isLockUnwritableCode('ENOENT')).toBe(false);
expect(isLockUnwritableCode(undefined)).toBe(false);
});
// Mode bits are bypassed for uid 0, so the denied-create path only reproduces
// as non-root. The predicate test above is the always-on guard.
it.skipIf(!process.getuid || process.getuid() === 0)(
'returns a no-op handle instead of throwing when the lock dir cannot be written',
async () => {
chmodSync(dir, 0o555);
try {
const lock = await acquireIndexLock(dir, { timeoutMs: 2000 });
expect(typeof lock.release).toBe('function');
expect(() => lock.release()).not.toThrow();
expect(existsSync(lockPath())).toBe(false); // lock file was never created
} finally {
chmodSync(dir, 0o755);
}
},
);
});

197
gitnexus/test/unit/run-analyze-fts-repair.test.ts
View file

@ -493,6 +493,8 @@ describe('runFullAnalysis FTS repair and verification failure paths', () => {
ok: false,
error: 'missing indexes after build: Function.function_fts',
})),
ftsFailureIsFatal: (fc: 'capability' | 'integrity' | undefined, swap: boolean) =>
fc === 'integrity' && swap,
}));
vi.doMock('../../src/core/ingestion/pipeline.js', () => ({
runPipelineFromRepo: vi.fn(async (repoPath: string) => ({
@ -513,6 +515,7 @@ describe('runFullAnalysis FTS repair and verification failure paths', () => {
);
expect(result.ftsSkipped).toBe(true);
expect(result.ftsSkipReason).toBe('build-failed'); // #2658 review L2
expect(logs.join('\n')).toMatch(
/FTS index build failed.*missing indexes after build.*keyword search degraded this run/i,
);
@ -525,6 +528,77 @@ describe('runFullAnalysis FTS repair and verification failure paths', () => {
}
});
it('ABORTS (throws before publish, leaves the previous index intact) on an FTS integrity failure on the atomic-swap path (#2658 review M1)', async () => {
// The single-writer lock rules out a concurrent-writer race, so an
// integrity-class FTS failure on the atomic-swap (--force) path is a real
// broken build: run-analyze must throw BEFORE swapping the staging DB in,
// leaving the previous live index untouched — not silently publish a
// search-less index as success. This end-to-end throw path was previously
// untested (only the ftsFailureIsFatal truth table was).
vi.doMock('../../src/core/lbug/lbug-adapter.js', () => ({
initLbug: vi.fn(async () => undefined),
loadGraphToLbug: vi.fn(async () => undefined),
getLbugStats: vi.fn(async () => ({ nodes: 0, edges: 0, communities: 0, processes: 0 })),
executeQuery: vi.fn(async () => []),
executeWithReusedStatement: vi.fn(async () => []),
closeLbug: vi.fn(async () => undefined),
wipeLbugDbFiles: vi.fn(async () => undefined),
loadCachedEmbeddings: vi.fn(async () => ({ embeddingNodeIds: new Set(), embeddings: [] })),
deleteNodesForFile: vi.fn(async () => undefined),
deleteNodesForFiles: vi.fn(async () => undefined),
deleteAllCommunitiesAndProcesses: vi.fn(async () => undefined),
queryImporters: vi.fn(async () => []),
queryImportersBatch: vi.fn(async () => []),
loadFTSExtension: vi.fn(async () => true),
}));
// Import the REAL classifier/predicate (not a re-stub) so the test pins the
// actual fatal-decision logic, per the #2658 review.
vi.doMock('../../src/core/search/fts-indexes.js', async () => {
const actual = await vi.importActual<typeof import('../../src/core/search/fts-indexes.js')>(
'../../src/core/search/fts-indexes.js',
);
return {
...actual,
initialiseSearchFTSStemmer: vi.fn(() => 'porter'),
buildSearchIndexesOrDegrade: vi.fn(async () => ({
ok: false,
failureClass: 'integrity' as const,
error: 'IO exception: Error renaming lbug.staging.wal to checkpoint',
})),
};
});
vi.doMock('../../src/core/ingestion/pipeline.js', () => ({
runPipelineFromRepo: vi.fn(async (repoPath: string) => ({
repoPath,
graph: { forEachNode: () => undefined },
})),
}));
const tmpRepo = await createTempDir('gitnexus-run-analyze-integrity-abort-');
try {
const { storagePath, lbugPath } = getStoragePaths(tmpRepo.dbPath);
await fs.mkdir(storagePath, { recursive: true });
// A pre-existing "previous index" that must survive the aborted rebuild.
await createPlaceholderGraphStore(lbugPath);
const before = await fs.readFile(lbugPath);
const { runFullAnalysis } = await import('../../src/core/run-analyze.js');
const message = await runFullAnalysis(
tmpRepo.dbPath,
{ force: true },
{ onProgress: () => {}, onLog: () => {} },
).catch((e: unknown) => (e instanceof Error ? e.message : String(e)));
expect(message).toMatch(/integrity error/i);
expect(message).toMatch(/aborted|previous index is\s+left intact/i);
// The previous index bytes are untouched (throw happened before the swap).
const after = await fs.readFile(lbugPath);
expect(after.equals(before)).toBe(true);
} finally {
await tmpRepo.cleanup();
}
});
it('full analyze degrades gracefully (no throw, warns, skips index creation) when FTS extension is unavailable', async () => {
// Offline-first degradation: when loadFTSExtension() returns false, the
// analyze path must NOT call createSearchFTSIndexes / verifySearchFTSIndexes
@ -584,6 +658,7 @@ describe('runFullAnalysis FTS repair and verification failure paths', () => {
);
expect(result.ftsSkipped).toBe(true);
expect(result.ftsSkipReason).toBe('extension-unavailable'); // #2658 review L2
expect(createSearchFTSIndexes).not.toHaveBeenCalled();
expect(verifySearchFTSIndexes).not.toHaveBeenCalled();
expect(logs.join('\n')).toMatch(/FTS extension unavailable; skipping search-index creation/i);
@ -665,6 +740,7 @@ describe('runFullAnalysis FTS repair and verification failure paths', () => {
);
expect(result.ftsSkipped).toBe(true);
expect(result.ftsSkipReason).toBe('extension-unavailable'); // #2658 review L2
const degradeLine = logs
.filter((l) => l.includes('skipping search-index creation'))
.join('\n');
@ -1005,3 +1081,124 @@ describe('runFullAnalysis dirty-recovery parking failure fails fast (this shippi
}
});
});
describe('runFullAnalysis re-resolves git state under the lock (#2658 review H2)', () => {
afterEach(() => {
vi.doUnmock('../../src/storage/git.js');
vi.doUnmock('../../src/core/ingestion/pipeline.js');
vi.resetModules();
vi.clearAllMocks();
});
it('re-reads HEAD after acquiring the lock, so a commit that lands during the wait is not missed', async () => {
// acquireIndexLock can wait up to the timeout ceiling; HEAD may advance
// during that wait. Pre-fix, resolveWriteTarget was called ONCE (before the
// lock) and its stale snapshot fed the freshness check — a waiter could
// return alreadyUpToDate against the OLD commit. Post-fix the wrapper
// re-resolves UNDER the lock, so getCurrentCommit is called again and the
// post-wait commit is what the pipeline uses. Simulate the advance by making
// getCurrentCommit return a new value on each call.
const commits = ['commit-before-wait', 'commit-after-wait'];
let call = 0;
const getCurrentCommit = vi.fn(
() => commits[call < commits.length ? call++ : commits.length - 1],
);
vi.doMock('../../src/storage/git.js', async () => {
const actual = await vi.importActual<typeof import('../../src/storage/git.js')>(
'../../src/storage/git.js',
);
return {
...actual,
getCurrentCommit,
hasGitDir: () => true,
getCurrentBranch: () => 'main',
isWorkingTreeDirty: () => false,
};
});
// Stop the run right after the wrapper's two resolveWriteTarget calls so the
// test pins the re-resolve, not the full pipeline.
vi.doMock('../../src/core/ingestion/pipeline.js', () => ({
runPipelineFromRepo: vi.fn(async () => {
throw new Error('stop-after-resolve');
}),
}));
const tmpRepo = await createTempDir('gitnexus-h2-relock-');
try {
const { runFullAnalysis } = await import('../../src/core/run-analyze.js');
await runFullAnalysis(
tmpRepo.dbPath,
{ force: true },
{ onProgress: () => {}, onLog: () => {} },
).catch(() => undefined);
// Pre-fix: exactly 1 (single pre-lock resolve). Post-fix: >= 2 (re-resolve
// under the lock), and the second call observed the post-wait commit.
expect(getCurrentCommit.mock.calls.length).toBeGreaterThanOrEqual(2);
expect(getCurrentCommit.mock.results[1]?.value).toBe('commit-after-wait');
} finally {
await tmpRepo.cleanup();
}
});
it('releases the lock when the under-lock re-resolve throws (no leak) (#2658 review H2 self-review)', async () => {
// The re-resolve runs UNDER the held lock and can throw (e.g. a `--branch`
// that no longer matches a checkout switched during the wait). That throw
// must still release the lock — the loop lives inside the try/finally.
vi.doUnmock('../../src/storage/git.js');
const release = vi.fn();
vi.doMock('../../src/storage/index-lock.js', async () => {
const actual = await vi.importActual<typeof import('../../src/storage/index-lock.js')>(
'../../src/storage/index-lock.js',
);
return {
...actual,
acquireIndexLock: vi.fn(async () => ({
record: {
v: 1,
pid: 1,
hostname: 'h',
startTime: null,
token: 't',
invocationId: 'i',
acquiredAt: '',
},
release,
})),
};
});
// getCurrentCommit succeeds on the pre-lock resolve, then throws on the
// under-lock re-resolve — the exact shape a mid-wait git change produces.
let call = 0;
vi.doMock('../../src/storage/git.js', async () => {
const actual = await vi.importActual<typeof import('../../src/storage/git.js')>(
'../../src/storage/git.js',
);
return {
...actual,
hasGitDir: () => true,
getCurrentBranch: () => 'main',
isWorkingTreeDirty: () => false,
getCurrentCommit: () => {
if (call++ === 0) return 'c1';
throw new Error('git HEAD read failed mid-wait');
},
};
});
const tmpRepo = await createTempDir('gitnexus-h2-leak-');
try {
const { runFullAnalysis } = await import('../../src/core/run-analyze.js');
const err = await runFullAnalysis(
tmpRepo.dbPath,
{ force: true },
{ onProgress: () => {}, onLog: () => {} },
).catch((e: unknown) => e);
expect(err).toBeInstanceOf(Error);
expect(release).toHaveBeenCalledTimes(1); // lock freed despite the throw
} finally {
vi.doUnmock('../../src/storage/index-lock.js');
await tmpRepo.cleanup();
}
});
});