diff --git a/README.md b/README.md index c9845cf0c..c9877312a 100644 --- a/README.md +++ b/README.md @@ -155,7 +155,7 @@ flowchart TB | `group_list` | List configured repository groups | | `group_sync` | Rebuild a group's Contract Registry and cross-repo links | -> Per-repo tools take an optional `repo` parameter (omit it when only one repo is indexed) and an optional `branch` for multi-branch indexes. `explain` and `pdg_query` need an index built with `gitnexus analyze --pdg`. +> Per-repo tools take an optional `repo` parameter (omit it when only one repo is indexed) and an optional `branch` for indexes pinned with `gitnexus analyze --branch`. Omitting `branch` queries the workspace index, which follows your checked-out working tree — switching branches and re-running `gitnexus analyze` updates it incrementally. `explain` and `pdg_query` need an index built with `gitnexus analyze --pdg`. ### Resources for instant context diff --git a/gitnexus/README.md b/gitnexus/README.md index f2fa459a3..8f24fea3f 100644 --- a/gitnexus/README.md +++ b/gitnexus/README.md @@ -144,7 +144,7 @@ Your AI agent gets **17 tools** (15 per-repo + 2 group) automatically: | `group_list` | List configured repository groups | | `group_sync` | Rebuild a group's Contract Registry and cross-repo links | -> With one indexed repo, the `repo` param is optional. With multiple, specify which: `query({search_query: "auth", repo: "my-app"})`. Per-repo tools also take an optional `branch` for multi-branch indexes. `explain` and `pdg_query` need an index built with `gitnexus analyze --pdg`. +> With one indexed repo, the `repo` param is optional. With multiple, specify which: `query({search_query: "auth", repo: "my-app"})`. Per-repo tools also take an optional `branch` for indexes pinned with `gitnexus analyze --branch`; omitting it queries the workspace index, which follows your checked-out working tree. `explain` and `pdg_query` need an index built with `gitnexus analyze --pdg`. ## MCP Resources diff --git a/gitnexus/src/cli/analyze.ts b/gitnexus/src/cli/analyze.ts index d399c5082..82ae50910 100644 --- a/gitnexus/src/cli/analyze.ts +++ b/gitnexus/src/cli/analyze.ts @@ -1317,10 +1317,11 @@ const analyzeCommandImpl = async ( // preserving the rest of the block (incl. --skills community rows). No-op // when the value already matches, so a routine up-to-date run is silent // (#1996 tri-review P2). - // Only refresh the repo-root AGENTS.md/CLAUDE.md base_ref for the - // PRIMARY/flat index (#2106 R2). A non-primary branch's up-to-date - // analyze must not churn the committed AGENTS.md — this mirrors the - // in-pipeline `if (!placement.branch)` gate around generateAIContextFiles. + // Only refresh the repo-root AGENTS.md/CLAUDE.md base_ref for the flat + // WORKSPACE index (#2106 R2, #2354). A pinned --branch sub-index's + // up-to-date analyze must not churn the committed AGENTS.md — this + // mirrors the in-pipeline `if (!placement.branch)` gate around + // generateAIContextFiles. let baseRefRefreshed: string[] = []; if (result.isPrimaryBranch !== false) { try { diff --git a/gitnexus/src/cli/i18n/en.ts b/gitnexus/src/cli/i18n/en.ts index 525460258..d23743a61 100644 --- a/gitnexus/src/cli/i18n/en.ts +++ b/gitnexus/src/cli/i18n/en.ts @@ -28,8 +28,8 @@ export const en = { 'status.currentCommit': 'Current commit', 'status.branch': 'Branch', 'status.detached': '(detached HEAD)', - 'status.branchNotIndexed': - "⚠️ current branch not indexed (primary index is for '{{primary}}'; run gitnexus analyze)", + 'status.workspaceIndexLabel': + "Workspace index: last analyzed on '{{primary}}' (re-run gitnexus analyze to follow the current branch)", 'status.status': 'Status', 'status.upToDate': '✅ up-to-date', 'status.stale': '⚠️ stale (re-run gitnexus analyze)', @@ -212,7 +212,7 @@ export const en = { 'help.option.force.confirmation': 'Skip confirmation prompt', 'help.option.uninstall.force': 'Apply the changes (default is a dry-run preview)', 'help.option.clean.all': 'Clean all indexed repos', - 'help.option.clean.branch': 'Delete only the named branch index (not the primary)', + 'help.option.clean.branch': 'Delete only the named branch index (not the workspace index)', 'help.option.clean.lbugSidecars': 'Clean quarantined LadybugDB missing-shadow WAL sidecars', 'help.option.wiki.force': 'Force full regeneration even if up to date', 'help.option.wiki.provider': diff --git a/gitnexus/src/cli/i18n/zh-CN.ts b/gitnexus/src/cli/i18n/zh-CN.ts index bda0dd673..44437b1f3 100644 --- a/gitnexus/src/cli/i18n/zh-CN.ts +++ b/gitnexus/src/cli/i18n/zh-CN.ts @@ -32,8 +32,8 @@ export const zhCN = { 'status.currentCommit': '当前提交', 'status.branch': '分支', 'status.detached': '（分离 HEAD）', - 'status.branchNotIndexed': - "⚠️ 当前分支未索引（主索引对应 '{{primary}}'；请运行 gitnexus analyze）", + 'status.workspaceIndexLabel': + "工作区索引：最近在 '{{primary}}' 分支上分析（重新运行 gitnexus analyze 以跟随当前分支）", 'status.status': '状态', 'status.upToDate': '✅ 已是最新', 'status.stale': '⚠️ 已过期（重新运行 gitnexus analyze）', @@ -199,7 +199,7 @@ export const zhCN = { 'help.option.force.confirmation': '跳过确认提示', 'help.option.uninstall.force': '应用更改（默认仅为预演预览）', 'help.option.clean.all': '清理所有已索引仓库', - 'help.option.clean.branch': '仅删除指定分支的索引（不影响主索引）', + 'help.option.clean.branch': '仅删除指定分支的索引（不影响工作区索引）', 'help.option.clean.lbugSidecars': '清理已隔离的 LadybugDB missing-shadow WAL sidecar', 'help.option.wiki.force': '即使已是最新也强制完整重新生成', 'help.option.wiki.provider': diff --git a/gitnexus/src/cli/index.ts b/gitnexus/src/cli/index.ts index b238d0d3d..7b6c17717 100644 --- a/gitnexus/src/cli/index.ts +++ b/gitnexus/src/cli/index.ts @@ -83,9 +83,9 @@ program ) .option( '--branch ', - 'Index the working tree under a specific branch slot (multi-branch indexing). ' + - 'Defaults to the checked-out branch; the primary/first-indexed branch keeps the ' + - 'flat index and others get their own. Distinct from --default-branch (cosmetic base_ref).', + 'Pin the working tree into a dedicated per-branch index slot (multi-branch indexing). ' + + 'Without this flag, analyze always updates the workspace index, which follows the ' + + 'checked-out working tree. Distinct from --default-branch (cosmetic base_ref).', ) .option('--no-stats', 'Omit volatile file/symbol counts from AGENTS.md and CLAUDE.md') .option( @@ -245,7 +245,7 @@ program .description('Delete GitNexus index for current repo') .option('-f, --force', 'Skip confirmation prompt') .option('--all', 'Clean all indexed repos') - .option('--branch ', 'Delete only the named branch index (not the primary)') + .option('--branch ', 'Delete only the named branch index (not the workspace index)') .option('--lbug-sidecars', 'Clean quarantined LadybugDB missing-shadow WAL sidecars') .action(createLazyAction(() => import('./clean.js'), 'cleanCommand')); diff --git a/gitnexus/src/cli/status.ts b/gitnexus/src/cli/status.ts index 89ff9697d..0d2fa3531 100644 --- a/gitnexus/src/cli/status.ts +++ b/gitnexus/src/cli/status.ts @@ -35,27 +35,25 @@ export const statusCommand = async () => { const currentCommit = getCurrentCommit(repo.repoPath); const currentBranch = getCurrentBranch(repo.repoPath); - // Pick the index matching the checked-out branch (#2106). The flat index - // belongs to the primary branch (repo.meta.branch); when the current branch - // differs and has its own index, report that one. Legacy/no-branch metas and - // detached HEAD fall through to the flat index (unchanged behavior). + // Pick the index matching the checked-out branch (#2106/#2354). A pinned + // `--branch` sub-index for the current branch wins; otherwise report the + // flat workspace index, which follows the checked-out working tree — the + // commit comparison below then says whether it needs a re-analyze. Legacy/ + // no-branch metas and detached HEAD also fall through to the flat index. let activeMeta = repo.meta; - let currentBranchIndexed = true; + let workspaceLagsBranch = false; if (currentBranch && repo.meta.branch && currentBranch !== repo.meta.branch) { const { metaPath } = getStoragePaths(repo.repoPath, currentBranch); const branchMeta = await loadMeta(path.dirname(metaPath)); if (branchMeta) activeMeta = branchMeta; - else currentBranchIndexed = false; + else workspaceLagsBranch = true; } console.log(`${t('status.repository')}: ${repo.repoPath}`); console.log(`${t('status.branch')}: ${currentBranch ?? t('status.detached')}`); - if (!currentBranchIndexed) { - console.log( - `${t('status.status')}: ${t('status.branchNotIndexed', { primary: repo.meta.branch ?? '' })}`, - ); - return; + if (workspaceLagsBranch) { + console.log(t('status.workspaceIndexLabel', { primary: repo.meta.branch ?? '' })); } const isUpToDate = currentCommit === activeMeta.lastCommit; diff --git a/gitnexus/src/core/run-analyze.ts b/gitnexus/src/core/run-analyze.ts index 66d3a6cd2..0300c0416 100644 --- a/gitnexus/src/core/run-analyze.ts +++ b/gitnexus/src/core/run-analyze.ts @@ -53,6 +53,8 @@ import { loadMeta, ensureGitNexusIgnored, registerRepo, + adoptFlatBranchLabel, + isReadOnlyFilesystemError, isRepoRegistered, cleanupOldKuzuFiles, reconcileMetadataFiles, @@ -97,7 +99,6 @@ import { import { getCurrentCommit, getCurrentBranch, - getDefaultBranch, getRemoteUrl, hasGitDir, getInferredRepoName, @@ -208,13 +209,13 @@ export interface AnalyzeOptions { */ defaultBranch?: string; /** - * Index-branch selector (#2106). Distinct from `defaultBranch` (which only - * affects generated AGENTS.md/CLAUDE.md base_ref text). When set, this run is - * labelled as that branch and routed to a per-branch index slot unless it is - * the primary branch. When `undefined`, the branch is auto-detected from the - * checked-out HEAD (the flat/primary slot for the first-indexed branch, a - * `branches//` sub-directory otherwise). Detached HEAD / non-git always - * maps to the flat slot. + * Index-branch selector (#2106, #2354). Distinct from `defaultBranch` (which + * only affects generated AGENTS.md/CLAUDE.md base_ref text). When set, this + * run is pinned to a per-branch index slot (`branches//`) unless the + * label matches the flat slot's recorded branch. When `undefined`, the run + * always targets the flat workspace slot, which follows the checked-out + * working tree; the auto-detected branch is only recorded as the slot's + * informational label. Detached HEAD / non-git also map to the flat slot. */ branch?: string; /** @@ -282,10 +283,12 @@ export interface AnalyzeResult { */ ftsSkipped?: boolean; /** - * True when the index this run produced/validated is the primary/flat slot - * (#2106 R2). `false` for a non-primary branch index. Lets the CLI skip - * repo-root AGENTS.md/CLAUDE.md refreshes (e.g. the base_ref fast-path) for a + * True when the index this run produced/validated is the flat workspace + * slot (#2106 R2, inverted by #2354 to follow the checked-out branch). + * `false` for a pinned `--branch` sub-index. Lets the CLI skip repo-root + * AGENTS.md/CLAUDE.md refreshes (e.g. the base_ref fast-path) for a pinned * branch analyze, mirroring the in-pipeline `if (!placement.branch)` gate. + * (The historical "primary" name is kept — it is public API surface.) */ isPrimaryBranch?: boolean; } @@ -343,27 +346,6 @@ export const PHASE_LABELS: Record = { * the {@link AnalyzeCallbacks} interface — it never writes to stdout/stderr * directly and never calls `process.exit()`. */ -/** - * Build the primary-inversion warning (#2106 R8), or `undefined` when there is - * nothing to warn about. Pure + exported for testing. Both inputs are trimmed - * (a diagnostic — a missed warning is low-harm; a false warning is the thing to - * avoid). `defaultBranch` is the repo's `origin/HEAD` branch (null when unset, - * e.g. fresh clones / CI), `flatOwner` is the branch that owns the flat slot. - */ -export const primaryInversionWarning = ( - defaultBranch: string | null | undefined, - flatOwner: string | null | undefined, -): string | undefined => { - const norm = (s: string | null | undefined): string | undefined => s?.trim() || undefined; - const d = norm(defaultBranch); - const o = norm(flatOwner); - if (!d || !o || d === o) return undefined; - return ( - `Warning: the default branch "${d}" is not the primary index — "${o}" owns the flat slot. ` + - `Run \`gitnexus clean --branch ${o}\` then re-index on "${d}", or query it explicitly with \`--branch ${d}\`.` - ); -}; - /** * Collect the recorded parse-cache chunk keys across the flat + every branch * metadata directory under a flat `.gitnexus` storage, EXCLUDING `excludeDir` @@ -600,13 +582,15 @@ export async function runFullAnalysis( const repoHasGit = hasGitDir(repoPath); const currentCommit = repoHasGit ? getCurrentCommit(repoPath) : ''; - // ── #2106: resolve which branch slot this run writes to ─────────────── + // ── #2106/#2354: resolve which branch slot this run writes to ───────── // `branchLabel` is the branch identity recorded in meta.json (incl. the - // primary). `placement.branch` is undefined for the flat/primary slot (the - // lbug/meta paths stay byte-identical to single-branch behavior) and set for - // a `branches//` sub-directory. Explicit `--branch` is always honored; - // otherwise auto-detect the checked-out branch (null for detached HEAD / - // non-git → flat slot). + // 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//` 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 @@ -627,7 +611,7 @@ export async function runFullAnalysis( ); } const branchLabel = options.branch ?? checkedOutBranch; - const placement = await resolveBranchPlacement(repoPath, branchLabel); + 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). @@ -647,21 +631,6 @@ export async function runFullAnalysis( const existingMeta = await loadMeta(metaDir); - // ── #2106 (R8): warn when the repo's default branch is not the primary ── - // A non-default branch can own the flat slot (it was indexed first). That - // index is still fully queryable via `--branch`, so this is an ergonomics - // wart, not data loss — we only warn (no risky relocation of a live DB). - if (repoHasGit) { - // Who owns the flat slot after this run? For a flat/primary run it is this - // run's resolved label (carrying an existing stamp forward); for a branch - // run the flat owner is unchanged, so read the flat meta. - const flatOwner = placement.branch - ? (await loadMeta(storagePath))?.branch - : (branchLabel ?? existingMeta?.branch); - const warning = primaryInversionWarning(getDefaultBranch(repoPath), flatOwner); - if (warning) log(warning); - } - // ── FTS-only repair path ──────────────────────────────────────────── if (options.repairFts) { if (!existingMeta) { @@ -888,6 +857,39 @@ export async function runFullAnalysis( const healUnregistered = options.allowDuplicateName === true && !(await isRepoRegistered(repoPath)); if (!dirty && !healUnregistered) { + // ── #2354: restamp the workspace label on a same-commit branch flip ── + // The flat slot follows the checked-out working tree; a branch switch + // at the SAME commit with a clean tree changes nothing the pipeline + // must rebuild, but the slot's informational `branch` label (and the + // registry copy that query-side branch scoping reads) would go stale. + // Detached HEAD / non-git (branchLabel === null) keeps the existing + // stamp, mirroring the end-of-run meta write. + if (!placement.branch && branchLabel && existingMeta.branch !== branchLabel) { + // Adopt first, stamp last (#2364 review F3): this block's retry + // guard is `existingMeta.branch !== branchLabel`, so stamping the + // meta before the registry/shadow cleanup would flip the guard and + // lock in any partial failure — with saveMeta last, a failed adopt + // leaves the guard true and the next same-commit run self-heals + // (adopt is idempotent). The whole sync is best-effort: the label + // is informational and the flat DB content is byte-valid for both + // labels here (same commit, clean tree), so an "Already up to + // date" run must not fail over it; read-only storage — the + // documented Docker :ro workflow (#1549) — degrades to a warning. + try { + await adoptFlatBranchLabel(repoPath, branchLabel); + await saveMeta(metaDir, { ...existingMeta, branch: branchLabel }); + } catch (err) { + // EACCES/EPERM also arise from ownership problems and transient + // Windows locks, so keep the real error visible alongside the + // #1549 read-only hint instead of replacing it. + const reason = isReadOnlyFilesystemError(err) + ? `${(err as Error).message} — storage may be read-only (#1549)` + : (err as Error).message; + log( + `Warning: could not restamp the workspace branch label (${reason}); will retry on the next run.`, + ); + } + } await ensureGitNexusIgnored(repoPath); return { // `resolveRepoIdentityRoot` collapses worktree roots to the @@ -1620,6 +1622,26 @@ export async function runFullAnalysis( branch: placement.branch, }); + // ── #2354: the flat workspace slot has adopted this run's branch ────── + // Drop a now-shadowed `branches//` sub-index for the same label + // (unreachable once the flat slot serves it) and align the registry's + // top-level branch label. Best-effort like the parse-cache save above + // (#2364 review F5): the index is complete and registered, and a failure + // here leaves only a stale registry label / undeleted shadowed dir — + // never wrong routing, because the flat meta this run already stamped is + // what applyBranchScope trusts. Retried by the next content-changing run + // (same-commit fast-path runs skip it: their guard compares the + // already-stamped meta label). + if (!placement.branch && branchLabel) { + try { + await adoptFlatBranchLabel(repoPath, branchLabel); + } catch (e) { + log( + `Warning: could not sync the workspace branch label (${(e as Error).message}); continuing.`, + ); + } + } + // Keep generated .gitnexus contents ignored without editing the user's root .gitignore. await ensureGitNexusIgnored(repoPath); diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index fa59dfe53..b3e5017d8 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -396,7 +396,7 @@ interface RepoHandle { stats?: RegistryEntry['stats']; /** Primary/flat branch name, when known (#2106). */ branch?: string; - /** Non-primary branch indexes available for this repo (#2106). */ + /** Pinned `--branch` sub-indexes available for this repo, distinct from the flat workspace slot (#2106/#2354). */ branches?: BranchSummary[]; } @@ -584,7 +584,7 @@ export interface RepoListing { siblings?: Array<{ name: string; path: string; lastCommit: string }>; /** Primary/flat branch name, when known (#2106). */ branch?: string; - /** Non-primary branch indexes available for this repo (#2106). */ + /** Pinned `--branch` sub-indexes available for this repo, distinct from the flat workspace slot (#2106/#2354). */ branches?: Array>; } @@ -1120,45 +1120,118 @@ export class LocalBackend { /** * Re-point a resolved repo handle at a specific branch index (#2106). * - * - No `branch` (default) → the primary/flat handle, unchanged (backward + * - No `branch` (default) → the flat workspace handle, unchanged (backward * compatible: every existing caller passes no branch). - * - `branch` equal to the known primary → the flat handle. - * - `branch` matching an indexed non-primary branch → a handle whose + * - `branch` equal to the flat slot's **on-disk** recorded branch → the + * flat handle. The disk meta is read before any cached state is trusted + * (#2364 review F1): the flat slot follows the checked-out working tree + * (#2354), so a plain analyze after a branch switch restamps the meta + * without any repo-resolution miss that would refresh a long-lived + * server's cached handle — the cached label can otherwise serve another + * branch's content under the old name (the pool staleness reinit + * hot-swaps content without updating `handle.branch`). + * - `branch` matching an indexed pinned branch → a handle whose * `lbugPath` points at `branches//lbug`; the connection pool keys by - * `lbugPath`, so this is the only change needed to scope every tool. - * - `branch` that was never indexed → a clear error (never a silently-empty - * result against the wrong DB). + * `lbugPath`, so this is the only change needed to scope every tool. The + * sub-index lbug must actually exist on disk — `adoptFlatBranchLabel` + * deletes the whole dir when the flat slot takes ownership, and a stale + * cached summary must not route to the deleted path. + * - Cached `handle.branch` is trusted only when there is no readable flat + * meta to contradict it (legacy shapes, #2106 R4). + * - Any miss → a clear error (never a silently-empty result against the + * wrong DB), after exactly one `refreshRepos()` so newly-pinned branches + * and restamped labels the cached handle predates resolve on the next + * call. */ private async applyBranchScope(handle: RepoHandle, branch?: string): Promise { if (!branch) return handle; - if (handle.branch && handle.branch === branch) return handle; - const summary = handle.branches?.find((b) => b.branch === branch); - if (summary) { - const { lbugPath } = getStoragePaths(handle.repoPath, branch); + // At most one cache refresh per resolution: enough for the NEXT call to + // see fresh handles, without paying two registry re-scans when several + // stale arms fire in one degraded resolution. + let refreshed = false; + const refreshOnce = async (): Promise => { + if (refreshed) return; + refreshed = true; + await this.refreshRepos().catch(() => {}); + }; + // One small JSON read per scoped call; mid-run meta writes preserve the + // old label until the end-of-run atomic stamp (run-analyze dirty stamps + // spread the existing meta), so this read never runs ahead of the DB. + const flatMeta = await loadMeta(path.dirname(handle.lbugPath)); + if (flatMeta?.branch && flatMeta.branch === branch) { + // The disk meta decides routing, so it also supplies the metadata — + // the cached handle's label/commit/stats can predate the restamp. return { ...handle, - lbugPath, - indexedAt: summary.indexedAt, - lastCommit: summary.lastCommit, - stats: summary.stats, + branch: flatMeta.branch, + indexedAt: flatMeta.indexedAt ?? handle.indexedAt, + lastCommit: flatMeta.lastCommit ?? handle.lastCommit, + stats: flatMeta.stats ?? handle.stats, }; } - // Legacy entry (pre-#2106): the registry has no recorded primary `branch`, - // so a `--branch ` request misses the checks above. Read the flat - // meta.json (next to the flat handle's lbug) to learn the primary and serve - // the flat handle only when it actually matches — never serve flat for an - // arbitrary unindexed branch (#2106 R4). - if (!handle.branch) { - const flatMeta = await loadMeta(path.dirname(handle.lbugPath)); - if (flatMeta?.branch && flatMeta.branch === branch) return handle; + + // A registry entry claiming `branch` both as the flat label AND as a + // pinned summary is an adopt-degraded state (rm kept the summary while + // the label restamped) — never serve the possibly stale-vintage pin for + // a label the flat slot claims; fall through to the honest error. + const summary = + handle.branch !== branch ? handle.branches?.find((b) => b.branch === branch) : undefined; + if (summary) { + const { lbugPath } = getStoragePaths(handle.repoPath, branch); + // The lbug is the artifact the pool opens, so its presence is the + // serviceability truth — a half-deleted dir can outlive its meta.json + // while the lbug is gone, and vice versa (#2364 review F1 arm ii). + // Only provably-absent errno counts as missing: a transient EACCES/EIO + // on a healthy pinned sub-index must serve the handle (the pool open + // surfaces the real error) rather than a false "not indexed". + const probeCode = await fs.access(lbugPath).then( + () => null, + (e: unknown) => (e as NodeJS.ErrnoException)?.code ?? 'UNKNOWN', + ); + const subIndexMissing = probeCode === 'ENOENT' || probeCode === 'ENOTDIR'; + if (!subIndexMissing) { + return { + ...handle, + lbugPath, + indexedAt: summary.indexedAt, + lastCommit: summary.lastCommit, + stats: summary.stats, + }; + } + // Stale summary (sub-index adopted/deleted): refresh so later calls see + // fresh handles, then fall through — the flat meta above is the truth. + await refreshOnce(); } - const indexed = [handle.branch, ...(handle.branches?.map((b) => b.branch) ?? [])].filter( - Boolean, + + if (handle.branch && handle.branch === branch) { + // No readable flat meta (missing/corrupt — loadMeta → null): keep the + // pre-#2354 trust in the cached label (#2106 R4 legacy shapes). A + // readable meta that names another branch means the label is stale. + if (!flatMeta?.branch) return handle; + } + + // Every miss refreshes once before erroring: newly-pinned branches and + // restamped labels the cached handle predates become resolvable on the + // caller's next attempt (the cache otherwise only refreshes on repo- + // resolution misses and list_repos). + await refreshOnce(); + + // The flat slot's label comes from the authoritative meta when readable — + // never echo a cached label the meta just contradicted (a "not indexed: + // main / indexed: main" self-contradiction). Cached summaries may still + // lag; they are a hint, not a promise. + const flatLabel = flatMeta?.branch ?? handle.branch; + const indexed = [flatLabel, ...(handle.branches?.map((b) => b.branch) ?? [])].filter( + (b) => Boolean(b) && b !== branch, ); - const available = indexed.length > 0 ? indexed.join(', ') : '(primary only)'; + const available = indexed.length > 0 ? indexed.join(', ') : '(workspace only)'; + // Post-#2354 a bare `analyze --branch ` refuses to run unless X is + // checked out, so the guidance must lead with the checkout (#2364 F6). throw new Error( `Branch "${branch}" is not indexed for "${handle.name}". ` + - `Indexed branches: ${available}. Run: gitnexus analyze --branch ${branch}`, + `Indexed branches: ${available}. The workspace index follows the ` + + `checked-out branch — check out "${branch}" and re-run: gitnexus analyze ` + + `(add --branch ${branch} while it is checked out to pin a separate sub-index).`, ); } diff --git a/gitnexus/src/mcp/tools.ts b/gitnexus/src/mcp/tools.ts index 38444c276..f118c8c7b 100644 --- a/gitnexus/src/mcp/tools.ts +++ b/gitnexus/src/mcp/tools.ts @@ -906,11 +906,12 @@ for (const tool of GITNEXUS_TOOLS) { if (!BRANCH_SCOPED_TOOLS.has(tool.name)) continue; if (tool.inputSchema.properties.branch) continue; // Optional — `required` is left unchanged so omitting `branch` keeps today's - // default/primary-branch behavior. Ignored in group mode (repo starts "@"). + // workspace-index behavior. Ignored in group mode (repo starts "@"). tool.inputSchema.properties.branch = { type: 'string', description: - 'Optional: scope to a specific branch index (multi-branch repos, #2106). ' + - 'Omit for the default/primary branch. Ignored in group mode.', + 'Optional: scope to a pinned branch index (multi-branch repos, #2106). ' + + 'Omit for the workspace index, which follows the checked-out working tree. ' + + 'Ignored in group mode.', }; } diff --git a/gitnexus/src/storage/branch-index.ts b/gitnexus/src/storage/branch-index.ts index b1debf6a4..7eca9d47c 100644 --- a/gitnexus/src/storage/branch-index.ts +++ b/gitnexus/src/storage/branch-index.ts @@ -45,18 +45,20 @@ export const branchSlug = (rawRef: string): string => { }; /** - * Decide where a freshly-analyzed branch's index lives: the flat (primary) slot - * or a per-branch sub-directory (#2106 KTD2). + * Decide where an EXPLICIT `--branch` run's index lives: the flat workspace + * slot or a per-branch sub-directory (#2106 KTD2, #2354). * - * Returns `{}` for the flat/primary placement (byte-identical layout) or - * `{ branch }` for a `branches//` sub-directory. The flat slot is owned by - * the FIRST branch indexed, recorded as `branch` in the flat `meta.json`; a - * different checked-out branch then auto-routes to its own sub-directory so it - * never overwrites the primary index. + * Only explicit `--branch` runs consult this — a plain analyze always targets + * the flat slot, which follows the checked-out working tree (#2354; gated at + * the `runFullAnalysis` call site). Returns `{}` for the flat placement + * (byte-identical layout) or `{ branch }` for a `branches//` + * sub-directory: when the requested label matches the flat slot's recorded + * `branch` label the run updates the flat slot in place (identical content — + * `--branch` requires the label to be checked out); any other label gets its + * own pinned sub-directory that plain analyzes won't touch. * - * `label` is the resolved index-branch (explicit `--branch`, else the - * checked-out branch, else `null`). A `null` label — detached HEAD, non-git - * folder, or CI checkout — always maps to the flat slot. + * A `null` label — detached HEAD, non-git folder, or CI checkout — always + * maps to the flat slot. */ export const resolveBranchPlacement = async ( repoPath: string, diff --git a/gitnexus/src/storage/repo-manager.ts b/gitnexus/src/storage/repo-manager.ts index 2ad952481..0f721c76b 100644 --- a/gitnexus/src/storage/repo-manager.ts +++ b/gitnexus/src/storage/repo-manager.ts @@ -657,7 +657,7 @@ export const findRepo = async (startPath: string): Promise = return null; }; -function isReadOnlyFilesystemError(err: unknown): boolean { +export function isReadOnlyFilesystemError(err: unknown): boolean { const code = (err as NodeJS.ErrnoException)?.code; return code === 'EROFS' || code === 'EACCES' || code === 'EPERM'; } @@ -1120,6 +1120,90 @@ export const removeBranchIndex = async (repoPath: string, branch: string): Promi return true; }; +/** + * Record that the flat workspace slot now serves `branch` (#2354). + * + * The flat index follows the checked-out working tree, so when a plain + * analyze lands on a branch that also has a pinned `branches//` + * sub-index, that sub-index becomes permanently shadowed — explicit + * `--branch` runs re-resolve to the flat slot and query-side branch scoping + * serves the flat handle first. Delete the shadowed directory and drop its + * registry summary in the same pass (leaving either half behind would strand + * un-cleanable disk bloat), and refresh the entry's top-level `branch` label + * so `list`/`list_repos`/branch-scoped queries stay coherent. + * + * Deliberately narrow for the analyze fast path: a missing registry entry is + * a no-op — including the sub-index deletion, which only runs for registered + * repos (never self-heals an unregistered repo, per #2264/#1169; the registry + * check precedes the rm per #2364 review F2) — and no subprocess is spawned. + */ +export const adoptFlatBranchLabel = async (repoPath: string, branch: string): Promise => { + const canonicalInput = canonicalizePath(repoPath); + const isRegistered = (list: RegistryEntry[]): number => + list.findIndex((e) => registryPathEquals(canonicalizePath(e.path), canonicalInput)); + // Cheap membership gate only (#2364 review F2): never touch the disk for an + // unregistered repo. The mutate below re-reads its own fresh snapshot. + if (isRegistered(await readRegistry()) < 0) return; // no-op, disk included (no self-heal) + + const resolved = path.resolve(repoPath); + const { storagePath } = getStoragePaths(resolved); + // Remove a shadowed sub-index directory, mirroring `clean --branch`'s + // containment guard: the target MUST live under .gitnexus/branches/. + const branchDir = path.join(storagePath, BRANCHES_DIR, branchSlug(branch)); + const branchesRoot = path.join(storagePath, BRANCHES_DIR) + path.sep; + let dirGone = false; + if (branchDir.startsWith(branchesRoot)) { + let rmError: NodeJS.ErrnoException | undefined; + await fs.rm(branchDir, { recursive: true, force: true }).catch((err: unknown) => { + rmError = err as NodeJS.ErrnoException; + }); + // The registry summary may be dropped only for a verifiably-gone + // directory: `clean --branch` resolves its target solely via the + // recorded summary, so dropping it while the dir survives (e.g. Windows + // EBUSY on an lbug held open by a live MCP server) would strand + // un-cleanable disk bloat (#2364 review F4). A resolved force:true rm + // proves absence; on failure, probe the disk and treat only + // provably-absent errno as gone — EACCES/EIO are "not provably absent", + // the same polarity as listRegisteredRepos({ validate: true }). + if (!rmError) { + dirGone = true; + } else { + const probeCode = await fs.access(branchDir).then( + () => null, + (e: unknown) => (e as NodeJS.ErrnoException)?.code ?? 'UNKNOWN', + ); + dirGone = probeCode === 'ENOENT' || probeCode === 'ENOTDIR'; + } + if (dirGone) { + // Non-recursive by design: only removes the parent when no other pinned + // sub-index remains, so an empty branches/ dir doesn't read as "pinned". + await fs.rmdir(path.join(storagePath, BRANCHES_DIR)).catch(() => {}); + } else { + logger.warn( + { path: branchDir, code: rmError?.code }, + 'Could not remove the shadowed branch sub-index; keeping its registry summary so `gitnexus clean --branch` can still target it.', + ); + } + } + + // Re-read AFTER the potentially slow recursive rm: the registry is a + // multi-writer whole-file overwrite, and writing a pre-rm snapshot would + // silently clobber concurrent registerRepo/removeBranchIndex writers — + // the #2106 R9 re-read-before-write discipline registerRepo follows. + const entries = await readRegistry(); + const idx = isRegistered(entries); + if (idx < 0) return; // unregistered concurrently → still a no-op + const entry = entries[idx]; + const remaining = dirGone ? entry.branches?.filter((b) => b.branch !== branch) : entry.branches; + const droppedSummary = (entry.branches?.length ?? 0) !== (remaining?.length ?? 0); + if (entry.branch === branch && !droppedSummary) return; // already coherent + entry.branch = branch; + if (remaining && remaining.length > 0) entry.branches = remaining; + else delete entry.branches; + entries[idx] = entry; + await writeRegistry(entries); +}; + /** * Thrown by {@link resolveRegistryEntry} when no registered repo matches * the caller's target string (by alias, basename, remote-inferred name, diff --git a/gitnexus/test/integration/multi-branch-analyze.test.ts b/gitnexus/test/integration/multi-branch-analyze.test.ts index 213b14512..bf2fabc9b 100644 --- a/gitnexus/test/integration/multi-branch-analyze.test.ts +++ b/gitnexus/test/integration/multi-branch-analyze.test.ts @@ -7,10 +7,13 @@ import { getStoragePaths, loadMeta, listRegisteredRepos } from '../../src/storag import { createTempDir } from '../helpers/test-db.js'; /** - * #2106 — multi-branch indexing end-to-end. Proves that analyzing a second - * branch creates its own index under `.gitnexus/branches//` and does NOT - * overwrite the primary (flat) index, and that the primary single-branch - * layout stays at `.gitnexus/{lbug,meta.json}`. + * #2106/#2354 — branch handling end-to-end. Proves that a plain analyze + * always updates the flat workspace index (following the checked-out working + * tree, no `branches/` sub-directory, no slot-ownership friction), that an + * explicit `--branch` run pins a separate index under + * `.gitnexus/branches//` without touching the flat slot, and that a + * pinned sub-index shadowed by a later plain analyze on the same branch is + * cleaned up. */ const git = (args: string[], cwd: string): string => execSync(['git', ...args].join(' '), { cwd, stdio: 'pipe', encoding: 'utf-8' }).trim(); @@ -37,7 +40,7 @@ describe('multi-branch analyze (#2106)', () => { await tmpHome.cleanup(); }); - it('indexes a second branch without overwriting the first', async () => { + it('a plain analyze follows a branch switch into the flat workspace slot (#2354)', async () => { const tmp = await createTempDir('gitnexus-multibranch-'); const repo = tmp.dbPath; try { @@ -52,16 +55,13 @@ describe('multi-branch analyze (#2106)', () => { const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); await runFullAnalysis(repo, {}, { onProgress: () => {} }); - // Primary branch lands in the flat slot, byte-identical layout. + // First analyze lands in the flat slot, byte-identical layout. const flat = getStoragePaths(repo); expect(path.dirname(flat.lbugPath)).toBe(flat.storagePath); expect(existsSync(flat.lbugPath)).toBe(true); const flatMeta = await loadMeta(flat.storagePath); expect(flatMeta?.branch).toBe('main'); expect(flatMeta?.lastCommit).toBe(mainCommit); - // main records its live chunk keys so a later branch prune can keep them. - const mainCacheKeys = flatMeta?.cacheKeys ?? []; - expect(mainCacheKeys.length).toBeGreaterThan(0); // Switch to a feature branch with different content and re-analyze. git(['checkout', '-b', 'feature/x'], repo); @@ -73,13 +73,60 @@ describe('multi-branch analyze (#2106)', () => { await runFullAnalysis(repo, {}, { onProgress: () => {} }); - // The flat (main) index is untouched — NOT overwritten by the feature run. + // The flat workspace index followed the working tree — updated in place, + // no `branches/` sub-directory, no slot-ownership error or warning. + expect(existsSync(flat.lbugPath)).toBe(true); + const flatMetaAfter = await loadMeta(flat.storagePath); + expect(flatMetaAfter?.branch).toBe('feature/x'); + expect(flatMetaAfter?.lastCommit).toBe(featureCommit); + expect(existsSync(path.join(flat.storagePath, 'branches'))).toBe(false); + + // The registry follows along: one entry, relabelled, no branches[]. + const entries = await listRegisteredRepos(); + const entry = entries.find((e) => path.resolve(e.path) === path.resolve(repo)); + expect(entry).toBeDefined(); + expect(entry?.branch).toBe('feature/x'); + expect(entry?.lastCommit).toBe(featureCommit); + expect(entry?.branches).toBeUndefined(); + } finally { + await tmp.cleanup(); + } + }, 180_000); + + it('an explicit --branch run pins a sub-index; a later plain analyze on that branch reclaims it', async () => { + const tmp = await createTempDir('gitnexus-multibranch-pin-'); + const repo = tmp.dbPath; + try { + git(['init'], repo); + await fs.writeFile(path.join(repo, 'a.ts'), 'export const a = 1;\n'); + git(['add', '-A'], repo); + commit(repo, 'a'); + git(['branch', '-M', 'main'], repo); + const mainCommit = git(['rev-parse', 'HEAD'], repo); + + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + await runFullAnalysis(repo, {}, { onProgress: () => {} }); + const flat = getStoragePaths(repo); + // main records its live chunk keys so a later branch prune can keep them. + const mainCacheKeys = (await loadMeta(flat.storagePath))?.cacheKeys ?? []; + expect(mainCacheKeys.length).toBeGreaterThan(0); + + // Pin the feature branch into its own sub-index with explicit --branch. + git(['checkout', '-b', 'feature/x'], repo); + await fs.writeFile(path.join(repo, 'b.ts'), 'export const b = 2;\n'); + git(['add', '-A'], repo); + commit(repo, 'b'); + const featureCommit = git(['rev-parse', 'HEAD'], repo); + + await runFullAnalysis(repo, { branch: 'feature/x' }, { onProgress: () => {} }); + + // The flat (main) index is untouched — NOT overwritten by the pinned run. expect(existsSync(flat.lbugPath)).toBe(true); const flatMetaAfter = await loadMeta(flat.storagePath); expect(flatMetaAfter?.branch).toBe('main'); expect(flatMetaAfter?.lastCommit).toBe(mainCommit); - // The feature index is a separate DB under branches//. + // The pinned index is a separate DB under branches//. const branchPaths = getStoragePaths(repo, 'feature/x'); const branchDir = path.dirname(branchPaths.lbugPath); expect(branchDir.includes(path.join('.gitnexus', 'branches'))).toBe(true); @@ -88,7 +135,7 @@ describe('multi-branch analyze (#2106)', () => { expect(branchMeta?.branch).toBe('feature/x'); expect(branchMeta?.lastCommit).toBe(featureCommit); - // #2106 R6: the feature analyze must NOT have evicted main's chunks from + // #2106 R6: the pinned analyze must NOT have evicted main's chunks from // the SHARED parse cache (they were unioned in via main's recorded keys). const { loadParseCache } = await import('../../src/storage/parse-cache.js'); const sharedCache = await loadParseCache(flat.storagePath); @@ -97,14 +144,25 @@ describe('multi-branch analyze (#2106)', () => { expect(onDisk.has(k), `main chunk ${k} survives the feature prune`).toBe(true); } - // The global registry keeps one entry per path: primary at top level, - // the feature branch nested under branches[] (#2106 U4). - const entries = await listRegisteredRepos(); - const entry = entries.find((e) => path.resolve(e.path) === path.resolve(repo)); - expect(entry).toBeDefined(); + // The global registry keeps one entry per path: flat label at top level, + // the pinned branch nested under branches[] (#2106 U4). + let entries = await listRegisteredRepos(); + let entry = entries.find((e) => path.resolve(e.path) === path.resolve(repo)); expect(entry?.branch).toBe('main'); - expect(entry?.lastCommit).toBe(mainCommit); expect(entry?.branches?.map((b) => b.branch)).toEqual(['feature/x']); + + // A plain analyze on the pinned branch adopts the flat workspace slot + // and removes the now-shadowed sub-index (#2354): the flat handle would + // always win for this label, leaving the sub-index unreachable bloat. + await runFullAnalysis(repo, {}, { onProgress: () => {} }); + const reclaimed = await loadMeta(flat.storagePath); + expect(reclaimed?.branch).toBe('feature/x'); + expect(reclaimed?.lastCommit).toBe(featureCommit); + expect(existsSync(branchDir)).toBe(false); + entries = await listRegisteredRepos(); + entry = entries.find((e) => path.resolve(e.path) === path.resolve(repo)); + expect(entry?.branch).toBe('feature/x'); + expect(entry?.branches).toBeUndefined(); } finally { await tmp.cleanup(); } @@ -132,17 +190,17 @@ describe('multi-branch analyze (#2106)', () => { await runFullAnalysis(repo, { force: true }, { onProgress: () => {} }); expect((await loadMeta(flat.storagePath))?.branch).toBe('main'); - // Now a feature analyze must still route to a sub-dir (the stamp survived), - // leaving the primary index intact rather than claiming the flat slot. + // Now an explicit --branch analyze must still route to a sub-dir (the + // stamp survived), leaving the flat index intact rather than updating it. git(['checkout', '-b', 'feature/y'], repo); await fs.writeFile(path.join(repo, 'b.ts'), 'export const b = 2;\n'); git(['add', '-A'], repo); commit(repo, 'b'); - await runFullAnalysis(repo, {}, { onProgress: () => {} }); + await runFullAnalysis(repo, { branch: 'feature/y' }, { onProgress: () => {} }); const flatMeta = await loadMeta(flat.storagePath); expect(flatMeta?.branch).toBe('main'); - expect(flatMeta?.lastCommit).toBe(mainCommit); // primary NOT overwritten + expect(flatMeta?.lastCommit).toBe(mainCommit); // flat NOT touched by the pinned run expect(existsSync(getStoragePaths(repo, 'feature/y').lbugPath)).toBe(true); } finally { await tmp.cleanup(); diff --git a/gitnexus/test/integration/run-analyze-adopt-failure.test.ts b/gitnexus/test/integration/run-analyze-adopt-failure.test.ts new file mode 100644 index 000000000..facb76024 --- /dev/null +++ b/gitnexus/test/integration/run-analyze-adopt-failure.test.ts @@ -0,0 +1,89 @@ +/** + * End-of-run adopt is best-effort (#2364 review F5): a completed, registered + * analyze must not exit non-zero because the post-registration branch-label + * sync failed (e.g. registry write ENOSPC). Integration-level because the + * full pipeline opens a real LadybugDB (multi-branch-analyze.test.ts + * precedent); the delegating vi.mock makes adoptFlatBranchLabel fail on + * demand (vi.spyOn cannot intercept ESM namespace exports). + * + * Once-mock starvation hazard: the delegating mock intercepts every + * repo-manager call in the process — arm mockRejectedValueOnce only + * immediately before the call under test. + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { execSync } from 'child_process'; +import fs from 'fs/promises'; +import path from 'path'; + +type RepoManagerModule = typeof import('../../src/storage/repo-manager.js'); + +const rmCtx = vi.hoisted(() => ({ + adoptMock: vi.fn(), + realAdopt: null as RepoManagerModule['adoptFlatBranchLabel'] | null, +})); + +vi.mock('../../src/storage/repo-manager.js', async (importOriginal) => { + const actual = await importOriginal(); + rmCtx.realAdopt = actual.adoptFlatBranchLabel; + rmCtx.adoptMock.mockImplementation(actual.adoptFlatBranchLabel); + return { + ...actual, + adoptFlatBranchLabel: rmCtx.adoptMock, + }; +}); + +import { listRegisteredRepos } from '../../src/storage/repo-manager.js'; +import { runFullAnalysis } from '../../src/core/run-analyze.js'; +import { createTempDir } from '../helpers/test-db.js'; + +describe('end-of-run adopt is best-effort (#2364 F5)', () => { + let tmpHome: Awaited>; + let savedGitnexusHome: string | undefined; + + beforeEach(async () => { + tmpHome = await createTempDir('gitnexus-adopt-besteffort-home-'); + savedGitnexusHome = process.env.GITNEXUS_HOME; + process.env.GITNEXUS_HOME = tmpHome.dbPath; + rmCtx.adoptMock.mockReset(); + rmCtx.adoptMock.mockImplementation( + (...args: Parameters) => rmCtx.realAdopt!(...args), + ); + }); + + afterEach(async () => { + if (savedGitnexusHome === undefined) delete process.env.GITNEXUS_HOME; + else process.env.GITNEXUS_HOME = savedGitnexusHome; + await tmpHome.cleanup(); + }); + + it('a failed label sync warns and the run still succeeds, already registered', async () => { + const tmp = await createTempDir('gitnexus-adopt-besteffort-'); + const repo = tmp.dbPath; + try { + execSync('git init', { cwd: repo, stdio: 'pipe' }); + await fs.writeFile(path.join(repo, 'a.ts'), 'export const a = 1;\n'); + execSync('git add -A', { cwd: repo, stdio: 'pipe' }); + execSync('git -c user.name=t -c user.email=t@t commit -m a', { cwd: repo, stdio: 'pipe' }); + execSync('git branch -M main', { cwd: repo, stdio: 'pipe' }); + + const logs: string[] = []; + rmCtx.adoptMock.mockRejectedValueOnce(new Error('mock registry write failure')); + const result = await runFullAnalysis( + repo, + {}, + { onProgress: () => {}, onLog: (m) => logs.push(m) }, + ); + + // The run resolved (no throw), the adopt was attempted and its failure + // surfaced as a warning… + expect(result.alreadyUpToDate).toBeFalsy(); + expect(rmCtx.adoptMock).toHaveBeenCalledWith(repo, 'main'); + expect(logs.some((m) => m.includes('could not sync the workspace branch label'))).toBe(true); + // …and registration had already completed before the label sync. + const entries = await listRegisteredRepos(); + expect(entries.some((e) => path.resolve(e.path) === path.resolve(repo))).toBe(true); + } finally { + await tmp.cleanup(); + } + }, 180_000); +}); diff --git a/gitnexus/test/unit/calltool-dispatch.test.ts b/gitnexus/test/unit/calltool-dispatch.test.ts index e02609188..da5f6a560 100644 --- a/gitnexus/test/unit/calltool-dispatch.test.ts +++ b/gitnexus/test/unit/calltool-dispatch.test.ts @@ -7,7 +7,7 @@ * These are pure unit tests that mock the LadybugDB layer to test * the dispatch and error handling logic in isolation. */ -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterAll } from 'vitest'; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'fs'; import fsPromises from 'fs/promises'; import os from 'os'; @@ -114,7 +114,9 @@ import { CALLEES_TRUNCATED_SENTINEL } from '../../src/core/ingestion/cfg/emit.js import { listRegisteredRepos, cleanupOldKuzuFiles, + getStoragePaths, loadMeta, + type RegistryEntry, } from '../../src/storage/repo-manager.js'; import { getGitRoot } from '../../src/storage/git.js'; import { _captureLogger } from '../../src/core/logger.js'; @@ -3503,10 +3505,14 @@ describe('cypher result formatting', () => { describe('LocalBackend.resolveRepo branch scope (#2106)', () => { let backend: LocalBackend; + // Per-run unique dir: a fixed shared os.tmpdir() path lets concurrent + // vitest runs on one host rm each other's materialized sub-index stub + // mid-test (the documented parallel-agents workflow). + const MULTI_DIR = mkdtempSync(path.join(os.tmpdir(), 'gnx-2106-multi-')); const BRANCH_ENTRY = { name: 'multi', - path: path.join(os.tmpdir(), 'gnx-2106-multi'), - storagePath: path.join(os.tmpdir(), 'gnx-2106-multi', '.gitnexus'), + path: MULTI_DIR, + storagePath: path.join(MULTI_DIR, '.gitnexus'), indexedAt: '2026-06-10T12:00:00Z', lastCommit: 'mainsha', branch: 'main', @@ -3515,25 +3521,39 @@ describe('LocalBackend.resolveRepo branch scope (#2106)', () => { }; const flatLbug = path.join(BRANCH_ENTRY.storagePath, 'lbug'); + // The pinned sub-index must exist on disk: applyBranchScope serves a + // branches[] summary only when its lbug is really there (#2364 review F1 + // arm ii — a stale summary must not route to an adopt-deleted dir). + const branchLbug = getStoragePaths(BRANCH_ENTRY.path, 'feature/x').lbugPath; beforeEach(async () => { vi.clearAllMocks(); + mkdirSync(path.dirname(branchLbug), { recursive: true }); + writeFileSync(branchLbug, 'stub'); backend = new LocalBackend(); (listRegisteredRepos as any).mockResolvedValue([BRANCH_ENTRY]); await backend.init(); }); - it('no branch param resolves the flat/primary lbug', async () => { + afterEach(() => { + rmSync(BRANCH_ENTRY.storagePath, { recursive: true, force: true }); + }); + + afterAll(() => { + rmSync(MULTI_DIR, { recursive: true, force: true }); + }); + + it('no branch param resolves the flat workspace lbug', async () => { const handle = await backend.resolveRepo('multi'); expect(handle.lbugPath).toBe(flatLbug); }); - it('the primary branch name resolves the flat lbug', async () => { + it('the workspace-recorded branch name resolves the flat lbug', async () => { const handle = await backend.resolveRepo('multi', 'main'); expect(handle.lbugPath).toBe(flatLbug); }); - it('an indexed non-primary branch resolves a branches/ lbug', async () => { + it('an indexed pinned branch resolves a branches/ lbug', async () => { const handle = await backend.resolveRepo('multi', 'feature/x'); expect(handle.lbugPath).not.toBe(flatLbug); expect(handle.lbugPath).toContain(path.join('.gitnexus', 'branches')); @@ -3544,6 +3564,14 @@ describe('LocalBackend.resolveRepo branch scope (#2106)', () => { it('an un-indexed branch throws a clear error', async () => { await expect(backend.resolveRepo('multi', 'nope')).rejects.toThrow(/not indexed/i); + // Post-#2354 guidance: a bare `analyze --branch ` refuses unless X is + // checked out, so the message must lead with the checkout (#2364 F6). + await expect(backend.resolveRepo('multi', 'nope')).rejects.toThrow( + /workspace index follows the checked-out branch/, + ); + await expect(backend.resolveRepo('multi', 'nope')).rejects.toThrow( + /check out "nope" and re-run: gitnexus analyze/, + ); }); it('a legacy entry with no top-level branch still routes an indexed branch', async () => { @@ -3555,10 +3583,11 @@ describe('LocalBackend.resolveRepo branch scope (#2106)', () => { expect(handle.lbugPath).toContain(path.join('.gitnexus', 'branches')); }); - it('a legacy entry resolves --branch via the flat meta (#2106 R4)', async () => { + it('a legacy entry resolves --branch via the flat meta (#2106 R4)', async () => { // Pre-#2106 flat index: registry entry has no `branch`/`branches`, but the - // flat meta.json records the primary. `--branch ` must resolve to - // the flat handle (read from meta), while an unindexed branch still errors. + // flat meta.json records the workspace branch. `--branch ` + // must resolve to the flat handle (read from meta), while an unindexed + // branch still errors. const dir = mkdtempSync(path.join(os.tmpdir(), 'gnx-2106-legacy-')); const storagePath = path.join(dir, '.gitnexus'); mkdirSync(storagePath, { recursive: true }); @@ -3579,6 +3608,132 @@ describe('LocalBackend.resolveRepo branch scope (#2106)', () => { } }); + it('a stale cached handle still resolves the restamped workspace branch via flat meta (#2354)', async () => { + // The flat workspace slot follows the checked-out working tree: a plain + // analyze after a branch switch restamps the flat meta.json without any + // repo-resolution miss that would refresh a long-lived server's handle. + // The cached handle still says branch 'main'; the on-disk flat meta is the + // truth ('feature/z') and must win over a stale "not indexed" error. + const dir = mkdtempSync(path.join(os.tmpdir(), 'gnx-2354-restamp-')); + const storagePath = path.join(dir, '.gitnexus'); + mkdirSync(storagePath, { recursive: true }); + writeFileSync( + path.join(storagePath, 'meta.json'), + JSON.stringify({ repoPath: dir, lastCommit: 'zzz', indexedAt: 'now', branch: 'feature/z' }), + ); + try { + (listRegisteredRepos as any).mockResolvedValue([ + { + name: 'flipped', + path: dir, + storagePath, + indexedAt: 'now', + lastCommit: 'aaa', + branch: 'main', + }, + ]); + await backend.init(); + const handle = await backend.resolveRepo('flipped', 'feature/z'); + expect(handle.lbugPath).toBe(path.join(storagePath, 'lbug')); + // A genuinely unindexed branch still errors (never serves the wrong DB). + await expect(backend.resolveRepo('flipped', 'nope')).rejects.toThrow(/not indexed/i); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('a stale cached label errors instead of serving the flat handle (#2364 F1 arm i)', async () => { + // Long-lived server cached branch 'main'; a plain analyze on feature/z + // restamped the flat meta (and the pool reinit will hot-swap content). + // Requesting the OLD label must error — the flat DB no longer holds main. + const dir = mkdtempSync(path.join(os.tmpdir(), 'gnx-2364-stale-label-')); + const storagePath = path.join(dir, '.gitnexus'); + mkdirSync(storagePath, { recursive: true }); + writeFileSync( + path.join(storagePath, 'meta.json'), + JSON.stringify({ repoPath: dir, lastCommit: 'zzz', indexedAt: 'now', branch: 'feature/z' }), + ); + try { + const entry: RegistryEntry = { + name: 'flipped', + path: dir, + storagePath, + indexedAt: 'now', + lastCommit: 'aaa', + branch: 'main', + }; + vi.mocked(listRegisteredRepos).mockResolvedValue([entry]); + await backend.init(); + const callsBefore = vi.mocked(listRegisteredRepos).mock.calls.length; + await expect(backend.resolveRepo('flipped', 'main')).rejects.toThrow(/not indexed/i); + // Exactly one refreshRepos fired for cache coherence (observed via its + // unconditional first call — refreshRepos itself is private). + expect(vi.mocked(listRegisteredRepos).mock.calls.length - callsBefore).toBe(1); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('a stale summary whose sub-index was adopted falls through to the flat handle (#2364 F1 arm ii)', async () => { + // The cached branches[] summary still lists feature/z, but adopt deleted + // branches// and the flat slot now owns the label: serve flat. + const dir = mkdtempSync(path.join(os.tmpdir(), 'gnx-2364-adopted-')); + const storagePath = path.join(dir, '.gitnexus'); + mkdirSync(storagePath, { recursive: true }); + writeFileSync( + path.join(storagePath, 'meta.json'), + JSON.stringify({ repoPath: dir, lastCommit: 'zzz', indexedAt: 'now', branch: 'feature/z' }), + ); + try { + const entry: RegistryEntry = { + name: 'adopted', + path: dir, + storagePath, + indexedAt: 'now', + lastCommit: 'aaa', + branch: 'main', + branches: [{ branch: 'feature/z', indexedAt: 'now', lastCommit: 'zzz' }], + }; + vi.mocked(listRegisteredRepos).mockResolvedValue([entry]); + await backend.init(); + const handle = await backend.resolveRepo('adopted', 'feature/z'); + expect(handle.lbugPath).toBe(path.join(storagePath, 'lbug')); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('a dangling summary with a disagreeing flat meta errors honestly (#2364 F3 window)', async () => { + // Partial fast-path failure: adopt deleted the sub-index but the flat + // meta was never restamped (saveMeta runs last). The degraded state must + // yield the not-indexed error — no ghost route, no wrong data. + const dir = mkdtempSync(path.join(os.tmpdir(), 'gnx-2364-dangling-')); + const storagePath = path.join(dir, '.gitnexus'); + mkdirSync(storagePath, { recursive: true }); + writeFileSync( + path.join(storagePath, 'meta.json'), + JSON.stringify({ repoPath: dir, lastCommit: 'aaa', indexedAt: 'now', branch: 'main' }), + ); + try { + const entry: RegistryEntry = { + name: 'dangling', + path: dir, + storagePath, + indexedAt: 'now', + lastCommit: 'aaa', + branch: 'main', + branches: [{ branch: 'feature/z', indexedAt: 'now', lastCommit: 'zzz' }], + }; + vi.mocked(listRegisteredRepos).mockResolvedValue([entry]); + await backend.init(); + const callsBefore = vi.mocked(listRegisteredRepos).mock.calls.length; + await expect(backend.resolveRepo('dangling', 'feature/z')).rejects.toThrow(/not indexed/i); + expect(vi.mocked(listRegisteredRepos).mock.calls.length - callsBefore).toBe(1); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + it('callTool threads the branch param through resolveRepo (un-indexed branch errors)', async () => { // If callTool dropped `branch` from repoParams, this would resolve the flat // handle and NOT throw — so the rejection proves the param is threaded. diff --git a/gitnexus/test/unit/list-status-branch.test.ts b/gitnexus/test/unit/list-status-branch.test.ts index a3f4a3ee2..a07f745a6 100644 --- a/gitnexus/test/unit/list-status-branch.test.ts +++ b/gitnexus/test/unit/list-status-branch.test.ts @@ -112,16 +112,31 @@ describe('status branch rendering (#2106)', () => { expect(out).toContain('up-to-date'); }); - it('reports when the checked-out branch is not indexed', async () => { + it('falls through to the workspace index when the branch has no pinned index (#2354)', async () => { (findRepo as any).mockResolvedValue(baseRepo); (getCurrentBranch as any).mockReturnValue('feature/y'); (getCurrentCommit as any).mockReturnValue('headsha9'); - (loadMeta as any).mockResolvedValue(null); // feature/y has no index + (loadMeta as any).mockResolvedValue(null); // feature/y has no pinned index await statusCommand(); const out = output(); expect(out).toContain('Branch: feature/y'); - expect(out).toContain('current branch not indexed'); + // The flat workspace index (last analyzed on main) is reported, with the + // commit comparison saying it lags this branch's tree. + expect(out).toContain("Workspace index: last analyzed on 'main'"); + expect(out).toContain('stale'); + }); + + it('same-commit branch flip reports up-to-date against the workspace index (#2354)', async () => { + (findRepo as any).mockResolvedValue(baseRepo); + (getCurrentBranch as any).mockReturnValue('feature/y'); + (getCurrentCommit as any).mockReturnValue('headsha0'); // same commit as flat meta + (loadMeta as any).mockResolvedValue(null); // feature/y has no pinned index + + await statusCommand(); + const out = output(); + expect(out).toContain("Workspace index: last analyzed on 'main'"); + expect(out).toContain('up-to-date'); }); it('compares against the branch index when the current branch has one', async () => { diff --git a/gitnexus/test/unit/repo-manager-rm-failure.test.ts b/gitnexus/test/unit/repo-manager-rm-failure.test.ts new file mode 100644 index 000000000..4055f94ea --- /dev/null +++ b/gitnexus/test/unit/repo-manager-rm-failure.test.ts @@ -0,0 +1,134 @@ +/** + * rm-failure paths for adoptFlatBranchLabel (#2364 review F4). + * Separate from repo-manager.test.ts: Vitest cannot vi.spyOn ESM namespace + * exports of fs/promises; a delegating vi.mock is required for mock rejects + * (same split as repo-manager-ensure-ignore-readonly.test.ts, #1549). + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import path from 'path'; + +const fsCtx = vi.hoisted(() => ({ + rmMock: vi.fn(), + realRm: null as ((...args: unknown[]) => Promise) | null, +})); + +vi.mock('fs/promises', async (importOriginal) => { + const actual = await importOriginal(); + const d = actual.default; + fsCtx.realRm = d.rm.bind(d); + fsCtx.rmMock.mockImplementation((...args) => fsCtx.realRm!(...args)); + return { + default: new Proxy(d, { + get(target, prop) { + if (prop === 'rm') return fsCtx.rmMock; + const v = Reflect.get(target, prop, target) as unknown; + return typeof v === 'function' ? (v as (...args: unknown[]) => unknown).bind(target) : v; + }, + }), + }; +}); + +import fs from 'fs/promises'; +import { + adoptFlatBranchLabel, + registerRepo, + listRegisteredRepos, + getStoragePaths, + saveMeta, + type RepoMeta, +} from '../../src/storage/repo-manager.js'; +import { _captureLogger } from '../../src/core/logger.js'; +import { createTempDir } from '../helpers/test-db.js'; + +describe('adoptFlatBranchLabel — rm failure keeps the branch summary (#2364 F4)', () => { + let tmpHome: Awaited>; + let tmpRepo: Awaited>; + let savedGitnexusHome: string | undefined; + + const metaFor = (branch: string, lastCommit: string): RepoMeta => ({ + repoPath: '', + lastCommit, + indexedAt: '2026-07-03T12:00:00.000Z', + branch, + stats: { files: 1, nodes: 1 }, + }); + + beforeEach(async () => { + tmpHome = await createTempDir('gitnexus-rm-failure-home-'); + tmpRepo = await createTempDir('gitnexus-rm-failure-repo-'); + savedGitnexusHome = process.env.GITNEXUS_HOME; + process.env.GITNEXUS_HOME = tmpHome.dbPath; + fsCtx.rmMock.mockClear(); + fsCtx.rmMock.mockImplementation((...args) => fsCtx.realRm!(...args)); + }); + + afterEach(async () => { + if (savedGitnexusHome === undefined) delete process.env.GITNEXUS_HOME; + else process.env.GITNEXUS_HOME = savedGitnexusHome; + await tmpHome.cleanup(); + await tmpRepo.cleanup(); + }); + + it('keeps the summary, warns with the errno, and still restamps the label on EBUSY', async () => { + await registerRepo(tmpRepo.dbPath, metaFor('main', 'aaa1111')); + await registerRepo(tmpRepo.dbPath, metaFor('feature/x', 'bbb2222'), { branch: 'feature/x' }); + const { metaPath } = getStoragePaths(tmpRepo.dbPath, 'feature/x'); + await saveMeta(path.dirname(metaPath), metaFor('feature/x', 'bbb2222')); + + const cap = _captureLogger(); + fsCtx.rmMock.mockRejectedValueOnce(Object.assign(new Error('mock busy'), { code: 'EBUSY' })); + try { + await adoptFlatBranchLabel(tmpRepo.dbPath, 'feature/x'); + } finally { + cap.restore(); + } + + const [entry] = await listRegisteredRepos(); + // The informational label still restamps… + expect(entry.branch).toBe('feature/x'); + // …but the summary survives so `clean --branch` can still target the dir… + expect(entry.branches?.map((b) => b.branch)).toEqual(['feature/x']); + // …which is still on disk. + await expect(fs.access(path.dirname(metaPath))).resolves.toBeUndefined(); + expect( + cap + .records() + .some( + (r) => + r.level === 40 && + r.code === 'EBUSY' && + typeof r.path === 'string' && + String(r.msg ?? '').includes('clean --branch'), + ), + ).toBe(true); + }); + + it('a later adopt retries the rm and drops the summary once the dir is gone', async () => { + await registerRepo(tmpRepo.dbPath, metaFor('main', 'aaa1111')); + await registerRepo(tmpRepo.dbPath, metaFor('feature/x', 'bbb2222'), { branch: 'feature/x' }); + const { metaPath } = getStoragePaths(tmpRepo.dbPath, 'feature/x'); + await saveMeta(path.dirname(metaPath), metaFor('feature/x', 'bbb2222')); + + fsCtx.rmMock.mockRejectedValueOnce(Object.assign(new Error('mock busy'), { code: 'EBUSY' })); + await adoptFlatBranchLabel(tmpRepo.dbPath, 'feature/x'); + // Retry with the real rm restored: cleanup completes. + await adoptFlatBranchLabel(tmpRepo.dbPath, 'feature/x'); + + const [entry] = await listRegisteredRepos(); + expect(entry.branch).toBe('feature/x'); + expect(entry.branches).toBeUndefined(); + await expect(fs.access(path.dirname(metaPath))).rejects.toThrow(); + }); + + it('treats a never-materialized sub-index as gone (summary dropped, idempotent)', async () => { + await registerRepo(tmpRepo.dbPath, metaFor('main', 'aaa1111')); + await registerRepo(tmpRepo.dbPath, metaFor('feature/x', 'bbb2222'), { branch: 'feature/x' }); + // No saveMeta for the sub-index: nothing on disk, force:true rm is a no-op. + + await adoptFlatBranchLabel(tmpRepo.dbPath, 'feature/x'); + + const [entry] = await listRegisteredRepos(); + expect(entry.branch).toBe('feature/x'); + expect(entry.branches).toBeUndefined(); + }); +}); diff --git a/gitnexus/test/unit/repo-manager.test.ts b/gitnexus/test/unit/repo-manager.test.ts index 4238738aa..02954b491 100644 --- a/gitnexus/test/unit/repo-manager.test.ts +++ b/gitnexus/test/unit/repo-manager.test.ts @@ -24,6 +24,7 @@ import { loadCLIConfig, registerRepo, removeBranchIndex, + adoptFlatBranchLabel, listRegisteredRepos, resolveRegistryEntry, canonicalizePath, @@ -133,6 +134,9 @@ describe('branchSlug (#2106)', () => { }); // ─── resolveBranchPlacement (#2106 KTD2) ───────────────────────────── +// Since #2354 only explicit `--branch` runs consult this (a plain analyze +// always targets the flat workspace slot); these cases pin the explicit-run +// contract. describe('resolveBranchPlacement (#2106)', () => { let tmpRepo: Awaited>; @@ -172,7 +176,7 @@ describe('resolveBranchPlacement (#2106)', () => { expect(await resolveBranchPlacement(tmpRepo.dbPath, 'main')).toEqual({}); }); - it('non-primary checked-out branch → its own sub-directory', async () => { + it('explicit label differing from the recorded flat branch → its own sub-directory', async () => { const { storagePath } = getStoragePaths(tmpRepo.dbPath); await saveMeta(storagePath, baseMeta('main')); expect(await resolveBranchPlacement(tmpRepo.dbPath, 'feature')).toEqual({ branch: 'feature' }); @@ -967,6 +971,50 @@ describe('registerRepo branch nesting (#2106)', () => { expect(entry.branches?.map((b) => b.branch)).toEqual(['feature/y']); }); + // ─── adoptFlatBranchLabel (#2354) ─────────────────────────────────── + + it('adoptFlatBranchLabel relabels the entry and removes a shadowed sub-index', async () => { + await registerRepo(tmpRepo.dbPath, metaFor('main', 'aaa1111')); + await registerRepo(tmpRepo.dbPath, metaFor('feature/x', 'bbb2222'), { branch: 'feature/x' }); + // Materialize the pinned sub-index on disk so the shadow cleanup has a + // real directory to remove. + const { metaPath } = getStoragePaths(tmpRepo.dbPath, 'feature/x'); + await saveMeta(path.dirname(metaPath), metaFor('feature/x', 'bbb2222')); + + await adoptFlatBranchLabel(tmpRepo.dbPath, 'feature/x'); + + const [entry] = await listRegisteredRepos(); + expect(entry.branch).toBe('feature/x'); + expect(entry.branches).toBeUndefined(); // shadowed summary dropped + await expect(fs.access(path.dirname(metaPath))).rejects.toThrow(); // dir deleted + }); + + it('adoptFlatBranchLabel keeps other pinned branch summaries', async () => { + await registerRepo(tmpRepo.dbPath, metaFor('main', 'aaa1111')); + await registerRepo(tmpRepo.dbPath, metaFor('feature/x', 'bbb2222'), { branch: 'feature/x' }); + await registerRepo(tmpRepo.dbPath, metaFor('feature/y', 'ccc3333'), { branch: 'feature/y' }); + + await adoptFlatBranchLabel(tmpRepo.dbPath, 'feature/x'); + + const [entry] = await listRegisteredRepos(); + expect(entry.branch).toBe('feature/x'); + expect(entry.branches?.map((b) => b.branch)).toEqual(['feature/y']); + }); + + it('adoptFlatBranchLabel never self-heals an unregistered repo', async () => { + // No registerRepo call — the registry has no entry for this path (#2264/#1169). + // The no-op must cover the disk too: a materialized pinned sub-index + // survives, because the shadow rm only runs for registered repos + // (#2364 review F2 — the rm used to fire before the registry check). + const { metaPath } = getStoragePaths(tmpRepo.dbPath, 'feature/x'); + await saveMeta(path.dirname(metaPath), metaFor('feature/x', 'bbb2222')); + + await adoptFlatBranchLabel(tmpRepo.dbPath, 'feature/x'); + + expect(await listRegisteredRepos()).toHaveLength(0); + await expect(fs.access(path.dirname(metaPath))).resolves.toBeUndefined(); // dir survives + }); + // ─── re-read-before-write merge (#2106 R9) ────────────────────────── it('a branch run preserves the freshest top-level fields (alias survives)', async () => { diff --git a/gitnexus/test/unit/run-analyze-adopt-failure.test.ts b/gitnexus/test/unit/run-analyze-adopt-failure.test.ts new file mode 100644 index 000000000..23e771072 --- /dev/null +++ b/gitnexus/test/unit/run-analyze-adopt-failure.test.ts @@ -0,0 +1,163 @@ +/** + * Fast-path restamp failure modes (#2364 review F3, test gaps 4 and 7). + * Separate from run-analyze.test.ts, which stays pure-real: these scenarios + * need a delegating vi.mock of repo-manager (vi.spyOn cannot intercept ESM + * namespace exports) to make adoptFlatBranchLabel / saveMeta fail on demand. + * + * Once-mock starvation hazard: the delegating mock intercepts EVERY + * repo-manager call in the process, including this file's own fixture setup + * (saveMeta seeds metas) — arm mockRejectedValueOnce only AFTER setup, + * immediately before the runFullAnalysis call under test. + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { execSync } from 'child_process'; +import fs from 'fs/promises'; +import path from 'path'; + +type RepoManagerModule = typeof import('../../src/storage/repo-manager.js'); + +const rmCtx = vi.hoisted(() => ({ + adoptMock: vi.fn(), + saveMetaMock: vi.fn(), + realAdopt: null as RepoManagerModule['adoptFlatBranchLabel'] | null, + realSaveMeta: null as RepoManagerModule['saveMeta'] | null, +})); + +vi.mock('../../src/storage/repo-manager.js', async (importOriginal) => { + const actual = await importOriginal(); + rmCtx.realAdopt = actual.adoptFlatBranchLabel; + rmCtx.realSaveMeta = actual.saveMeta; + rmCtx.adoptMock.mockImplementation(actual.adoptFlatBranchLabel); + rmCtx.saveMetaMock.mockImplementation(actual.saveMeta); + return { + ...actual, + adoptFlatBranchLabel: rmCtx.adoptMock, + saveMeta: rmCtx.saveMetaMock, + }; +}); + +import { + getStoragePaths, + registerRepo, + loadMeta, + INCREMENTAL_SCHEMA_VERSION, + type RepoMeta, +} from '../../src/storage/repo-manager.js'; +import { runFullAnalysis } from '../../src/core/run-analyze.js'; +import { createTempDir } from '../helpers/test-db.js'; + +describe('fast-path restamp failure modes (#2364 F3)', () => { + let tmpHome: Awaited>; + let tmpRepo: Awaited>; + let savedGitnexusHome: string | undefined; + + beforeEach(async () => { + tmpHome = await createTempDir('gitnexus-adopt-failure-home-'); + tmpRepo = await createTempDir('gitnexus-adopt-failure-repo-'); + savedGitnexusHome = process.env.GITNEXUS_HOME; + process.env.GITNEXUS_HOME = tmpHome.dbPath; + rmCtx.adoptMock.mockReset(); + rmCtx.saveMetaMock.mockReset(); + rmCtx.adoptMock.mockImplementation( + (...args: Parameters) => rmCtx.realAdopt!(...args), + ); + rmCtx.saveMetaMock.mockImplementation((...args: Parameters) => + rmCtx.realSaveMeta!(...args), + ); + }); + + afterEach(async () => { + if (savedGitnexusHome === undefined) delete process.env.GITNEXUS_HOME; + else process.env.GITNEXUS_HOME = savedGitnexusHome; + await tmpHome.cleanup(); + await tmpRepo.cleanup(); + }); + + /** git repo on feature/x at one commit, flat meta labeled main, pinned feature/x sub-index, registered. */ + const seedFlippedWorkspace = async (): Promise<{ + flatStorage: string; + branchMetaDir: string; + }> => { + execSync('git init', { cwd: tmpRepo.dbPath, stdio: 'pipe' }); + execSync('git -c user.name=t -c user.email=t@t commit --allow-empty -m init', { + cwd: tmpRepo.dbPath, + stdio: 'pipe', + }); + execSync('git branch -M main', { cwd: tmpRepo.dbPath, stdio: 'pipe' }); + execSync('git checkout -b feature/x', { cwd: tmpRepo.dbPath, stdio: 'pipe' }); + const commit = execSync('git rev-parse HEAD', { + cwd: tmpRepo.dbPath, + encoding: 'utf-8', + }).trim(); + const metaFor = (branch: string): RepoMeta => ({ + repoPath: tmpRepo.dbPath, + lastCommit: commit, + indexedAt: new Date().toISOString(), + branch, + schemaVersion: INCREMENTAL_SCHEMA_VERSION, + }); + const flat = getStoragePaths(tmpRepo.dbPath); + await rmCtx.realSaveMeta!(flat.storagePath, metaFor('main')); + const branch = getStoragePaths(tmpRepo.dbPath, 'feature/x'); + await rmCtx.realSaveMeta!(path.dirname(branch.metaPath), metaFor('feature/x')); + await registerRepo(tmpRepo.dbPath, metaFor('main')); + await registerRepo(tmpRepo.dbPath, metaFor('feature/x'), { branch: 'feature/x' }); + return { flatStorage: flat.storagePath, branchMetaDir: path.dirname(branch.metaPath) }; + }; + + it('a failed adopt keeps the retry guard true and the next run self-heals (gap 4)', async () => { + const { flatStorage, branchMetaDir } = await seedFlippedWorkspace(); + const logs: string[] = []; + + rmCtx.adoptMock.mockRejectedValueOnce(new Error('mock adopt failure')); + const first = await runFullAnalysis(tmpRepo.dbPath, {}, { onLog: (m) => logs.push(m) }); + + expect(first.alreadyUpToDate).toBe(true); + expect(logs.some((m) => m.includes('could not restamp the workspace branch label'))).toBe(true); + // saveMeta runs AFTER adopt, so the failed sync left the guard untouched… + const stale = await loadMeta(flatStorage); + expect(stale?.branch).toBe('main'); + await expect(fs.access(branchMetaDir)).resolves.toBeUndefined(); + + // …and the next same-commit run retries and completes the whole sync. + const second = await runFullAnalysis(tmpRepo.dbPath, {}, {}); + expect(second.alreadyUpToDate).toBe(true); + const healed = await loadMeta(flatStorage); + expect(healed?.branch).toBe('feature/x'); + await expect(fs.access(branchMetaDir)).rejects.toThrow(); + }); + + it('adopt is invoked before the meta restamp on a successful flip', async () => { + const { flatStorage } = await seedFlippedWorkspace(); + rmCtx.adoptMock.mockClear(); + rmCtx.saveMetaMock.mockClear(); + + const result = await runFullAnalysis(tmpRepo.dbPath, {}, {}); + + expect(result.alreadyUpToDate).toBe(true); + expect(rmCtx.adoptMock).toHaveBeenCalledTimes(1); + expect(rmCtx.saveMetaMock).toHaveBeenCalledTimes(1); + expect(rmCtx.adoptMock.mock.invocationCallOrder[0]).toBeLessThan( + rmCtx.saveMetaMock.mock.invocationCallOrder[0], + ); + const meta = await loadMeta(flatStorage); + expect(meta?.branch).toBe('feature/x'); + }); + + it.each(['EROFS', 'EACCES', 'EPERM'] as const)( + '"Already up to date" still succeeds when the restamp hits %s (#1549, gap 7)', + async (code) => { + const { flatStorage } = await seedFlippedWorkspace(); + const logs: string[] = []; + + rmCtx.saveMetaMock.mockRejectedValueOnce(Object.assign(new Error('mock ro'), { code })); + const result = await runFullAnalysis(tmpRepo.dbPath, {}, { onLog: (m) => logs.push(m) }); + + expect(result.alreadyUpToDate).toBe(true); + expect(logs.some((m) => m.includes('read-only') && m.includes('#1549'))).toBe(true); + // The stamp never landed, so the guard stays true for the next run. + const meta = await loadMeta(flatStorage); + expect(meta?.branch).toBe('main'); + }, + ); +}); diff --git a/gitnexus/test/unit/run-analyze.test.ts b/gitnexus/test/unit/run-analyze.test.ts index 8f8703f8a..030a6d577 100644 --- a/gitnexus/test/unit/run-analyze.test.ts +++ b/gitnexus/test/unit/run-analyze.test.ts @@ -9,6 +9,8 @@ import { } from '../../src/core/embedding-mode.js'; import { getStoragePaths, + loadMeta, + registerRepo, saveMeta, INCREMENTAL_SCHEMA_VERSION, type RepoMeta, @@ -72,7 +74,170 @@ describe('run-analyze module', () => { } }); - it('reports isPrimaryBranch false for an up-to-date non-primary branch (#2106 R2)', async () => { + it('plain analyze on another branch adopts the flat workspace slot (#2354)', async () => { + const tmpRepo = await createTempDir('gitnexus-run-analyze-workspace-'); + const tmpHome = await createTempDir('gitnexus-run-analyze-workspace-home-'); + const savedHome = process.env.GITNEXUS_HOME; + process.env.GITNEXUS_HOME = tmpHome.dbPath; + try { + execSync('git init', { cwd: tmpRepo.dbPath, stdio: 'pipe' }); + execSync('git -c user.name=t -c user.email=t@t commit --allow-empty -m init', { + cwd: tmpRepo.dbPath, + stdio: 'pipe', + }); + execSync('git branch -M main', { cwd: tmpRepo.dbPath, stdio: 'pipe' }); + execSync('git checkout -b feature/x', { cwd: tmpRepo.dbPath, stdio: 'pipe' }); + const commit = execSync('git rev-parse HEAD', { + cwd: tmpRepo.dbPath, + encoding: 'utf-8', + }).trim(); + + // Flat slot last analyzed on main; feature/x also has a pinned sub-index. + // Both metas stamp the current schema version so the run-analyze + // schema-mismatch guard (#2289 P1) does not force a rebuild before the + // fast path runs. + const flat = getStoragePaths(tmpRepo.dbPath); + const flatMetaSeed: RepoMeta = { + repoPath: tmpRepo.dbPath, + lastCommit: commit, + indexedAt: new Date().toISOString(), + branch: 'main', + schemaVersion: INCREMENTAL_SCHEMA_VERSION, + }; + await saveMeta(flat.storagePath, flatMetaSeed); + const branch = getStoragePaths(tmpRepo.dbPath, 'feature/x'); + await saveMeta(path.dirname(branch.metaPath), { + repoPath: tmpRepo.dbPath, + lastCommit: commit, + indexedAt: new Date().toISOString(), + branch: 'feature/x', + schemaVersion: INCREMENTAL_SCHEMA_VERSION, + }); + // Register the repo in an isolated registry: the shadow cleanup only + // runs for registered repos (#2364 review F2 — unregistered repos must + // never lose a pinned sub-index). + await registerRepo(tmpRepo.dbPath, flatMetaSeed); + await registerRepo( + tmpRepo.dbPath, + { ...flatMetaSeed, branch: 'feature/x' }, + { branch: 'feature/x' }, + ); + + // A plain analyze ignores the pinned sub-index and serves the flat + // workspace slot; the same-commit clean-tree fast path restamps the + // slot's branch label and removes the now-shadowed sub-index. + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + const result = await runFullAnalysis(tmpRepo.dbPath, {}, { onProgress: () => {} }); + expect(result.alreadyUpToDate).toBe(true); + expect(result.isPrimaryBranch).toBe(true); + const flatMeta = await loadMeta(flat.storagePath); + expect(flatMeta?.branch).toBe('feature/x'); + await expect(fs.access(path.dirname(branch.metaPath))).rejects.toThrow(); + } finally { + if (savedHome === undefined) delete process.env.GITNEXUS_HOME; + else process.env.GITNEXUS_HOME = savedHome; + await tmpHome.cleanup(); + await tmpRepo.cleanup(); + } + }); + + it('the fast-path restamp leaves an unregistered repo pinned sub-index intact (#2364 F2)', async () => { + const tmpRepo = await createTempDir('gitnexus-run-analyze-unregistered-'); + const tmpHome = await createTempDir('gitnexus-run-analyze-unregistered-home-'); + const savedHome = process.env.GITNEXUS_HOME; + process.env.GITNEXUS_HOME = tmpHome.dbPath; + try { + execSync('git init', { cwd: tmpRepo.dbPath, stdio: 'pipe' }); + execSync('git -c user.name=t -c user.email=t@t commit --allow-empty -m init', { + cwd: tmpRepo.dbPath, + stdio: 'pipe', + }); + execSync('git branch -M main', { cwd: tmpRepo.dbPath, stdio: 'pipe' }); + execSync('git checkout -b feature/x', { cwd: tmpRepo.dbPath, stdio: 'pipe' }); + const commit = execSync('git rev-parse HEAD', { + cwd: tmpRepo.dbPath, + encoding: 'utf-8', + }).trim(); + + const flat = getStoragePaths(tmpRepo.dbPath); + await saveMeta(flat.storagePath, { + repoPath: tmpRepo.dbPath, + lastCommit: commit, + indexedAt: new Date().toISOString(), + branch: 'main', + schemaVersion: INCREMENTAL_SCHEMA_VERSION, + }); + const branch = getStoragePaths(tmpRepo.dbPath, 'feature/x'); + await saveMeta(path.dirname(branch.metaPath), { + repoPath: tmpRepo.dbPath, + lastCommit: commit, + indexedAt: new Date().toISOString(), + branch: 'feature/x', + schemaVersion: INCREMENTAL_SCHEMA_VERSION, + }); + // Deliberately NO registerRepo: the empty isolated registry makes this + // repo unregistered, so the adopt must be a full no-op on disk + // (#2264/#1169 no-self-heal, #2364 review F2). + + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + const result = await runFullAnalysis(tmpRepo.dbPath, {}, { onProgress: () => {} }); + expect(result.alreadyUpToDate).toBe(true); + const flatMeta = await loadMeta(flat.storagePath); + // The informational flat label still restamps… + expect(flatMeta?.branch).toBe('feature/x'); + // …but the pinned sub-index survives untouched. + await expect(fs.access(path.dirname(branch.metaPath))).resolves.toBeUndefined(); + } finally { + if (savedHome === undefined) delete process.env.GITNEXUS_HOME; + else process.env.GITNEXUS_HOME = savedHome; + await tmpHome.cleanup(); + await tmpRepo.cleanup(); + } + }); + + it('a detached HEAD at the same commit skips the fast-path restamp (#2364 F3 gap 6)', async () => { + const tmpRepo = await createTempDir('gitnexus-run-analyze-detached-'); + const tmpHome = await createTempDir('gitnexus-run-analyze-detached-home-'); + const savedHome = process.env.GITNEXUS_HOME; + process.env.GITNEXUS_HOME = tmpHome.dbPath; + try { + execSync('git init', { cwd: tmpRepo.dbPath, stdio: 'pipe' }); + execSync('git -c user.name=t -c user.email=t@t commit --allow-empty -m init', { + cwd: tmpRepo.dbPath, + stdio: 'pipe', + }); + execSync('git branch -M main', { cwd: tmpRepo.dbPath, stdio: 'pipe' }); + execSync('git checkout --detach', { cwd: tmpRepo.dbPath, stdio: 'pipe' }); + const commit = execSync('git rev-parse HEAD', { + cwd: tmpRepo.dbPath, + encoding: 'utf-8', + }).trim(); + + const flat = getStoragePaths(tmpRepo.dbPath); + await saveMeta(flat.storagePath, { + repoPath: tmpRepo.dbPath, + lastCommit: commit, + indexedAt: new Date().toISOString(), + branch: 'main', + schemaVersion: INCREMENTAL_SCHEMA_VERSION, + }); + + // Detached HEAD → branchLabel is null → the restamp block must not + // fire: the existing stamp survives, mirroring the end-of-run write. + const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); + const result = await runFullAnalysis(tmpRepo.dbPath, {}, { onProgress: () => {} }); + expect(result.alreadyUpToDate).toBe(true); + const flatMeta = await loadMeta(flat.storagePath); + expect(flatMeta?.branch).toBe('main'); + } finally { + if (savedHome === undefined) delete process.env.GITNEXUS_HOME; + else process.env.GITNEXUS_HOME = savedHome; + await tmpHome.cleanup(); + await tmpRepo.cleanup(); + } + }); + + it('reports isPrimaryBranch false for an up-to-date explicit --branch run (#2106 R2)', async () => { const tmpRepo = await createTempDir('gitnexus-run-analyze-nonprimary-'); try { execSync('git init', { cwd: tmpRepo.dbPath, stdio: 'pipe' }); @@ -87,10 +252,8 @@ describe('run-analyze module', () => { encoding: 'utf-8', }).trim(); - // Flat slot owned by main; feature/x has its own up-to-date branch index. - // Both metas stamp the current schema version so the run-analyze - // schema-mismatch guard (#2289 P1) does not force a rebuild before the - // fast path runs. + // Flat slot recorded for main; feature/x has its own up-to-date pinned + // sub-index, so an explicit `--branch feature/x` run routes there. const flat = getStoragePaths(tmpRepo.dbPath); await saveMeta(flat.storagePath, { repoPath: tmpRepo.dbPath, @@ -109,9 +272,15 @@ describe('run-analyze module', () => { }); const { runFullAnalysis } = await import('../../src/core/run-analyze.js'); - const result = await runFullAnalysis(tmpRepo.dbPath, {}, { onProgress: () => {} }); + const result = await runFullAnalysis( + tmpRepo.dbPath, + { branch: 'feature/x' }, + { onProgress: () => {} }, + ); expect(result.alreadyUpToDate).toBe(true); expect(result.isPrimaryBranch).toBe(false); + // The pinned sub-index is untouched by an explicit branch run. + await expect(fs.access(path.dirname(branch.metaPath))).resolves.toBeUndefined(); } finally { await tmpRepo.cleanup(); } @@ -213,36 +382,6 @@ describe('collectBranchCacheKeys (#2106 R6)', () => { }); }); -describe('primaryInversionWarning (#2106 R8)', () => { - it('warns when the default branch is not the flat-slot owner', async () => { - const { primaryInversionWarning } = await import('../../src/core/run-analyze.js'); - const w = primaryInversionWarning('main', 'feature/x'); - expect(w).toContain('default branch "main"'); - expect(w).toContain('"feature/x" owns the flat slot'); - expect(w).toContain('clean --branch feature/x'); - }); - - it('does not warn when the default branch is null (no origin/HEAD)', async () => { - const { primaryInversionWarning } = await import('../../src/core/run-analyze.js'); - expect(primaryInversionWarning(null, 'feature/x')).toBeUndefined(); - }); - - it('does not warn when the default owns the flat slot', async () => { - const { primaryInversionWarning } = await import('../../src/core/run-analyze.js'); - expect(primaryInversionWarning('main', 'main')).toBeUndefined(); - }); - - it('trims both sides so trivial whitespace does not false-warn', async () => { - const { primaryInversionWarning } = await import('../../src/core/run-analyze.js'); - expect(primaryInversionWarning(' main ', 'main')).toBeUndefined(); - }); - - it('does not warn when there is no flat owner yet', async () => { - const { primaryInversionWarning } = await import('../../src/core/run-analyze.js'); - expect(primaryInversionWarning('main', undefined)).toBeUndefined(); - }); -}); - describe('deriveEmbeddingMode', () => { // Default `analyze` on a repo with existing embeddings: must preserve, must // NOT regenerate, must load the cache so phase 3.5 can re-insert vectors.