diff --git a/.claude/skills/gitnexus-guide/SKILL.md b/.claude/skills/gitnexus-guide/SKILL.md index e52560422..bf3c73948 100644 --- a/.claude/skills/gitnexus-guide/SKILL.md +++ b/.claude/skills/gitnexus-guide/SKILL.md @@ -83,15 +83,23 @@ Notes: `offset` ≥ `total` returns an empty page (with `total` still reported). ### Inline staleness signal (`query` / `context` / `impact` / `cypher`) -These four hot read tools attach a non-blocking `staleness` field to their response when the index is behind the checkout's current HEAD — the same `{ commitsBehind, hint }` shape `list_repos` already reports — so a direct tool call surfaces a behind-HEAD index without a separate `list_repos` call: +These four hot read tools attach a non-blocking `staleness` field to their response when the index is not at the checkout's current HEAD — the same `{ status, commitsBehind?, hint? }` shape `list_repos` already reports — so a direct tool call surfaces a stale index without a separate `list_repos` call: ```jsonc { /* …the tool's normal result… */ - "staleness": { "commitsBehind": 3, "hint": "⚠️ Index is 3 commits behind HEAD. Run analyze tool to update." } + "staleness": { "status": "behind", "commitsBehind": 3, "hint": "⚠️ Index is 3 commits behind HEAD. Run analyze tool to update." } } ``` -The field is **absent when the index is current** (or when the freshness check can't run), so its presence is the signal. It is only ever added to object results — raw-array `cypher` output and error envelopes are returned unchanged. `@group`-targeted calls do not carry it (multi-repo staleness is ill-defined). When you see it, the graph may be behind the working tree — re-run `analyze` before trusting blast-radius or dependence answers. +`commitsBehind` is present only when git counted the gap. When git could not count it but HEAD still resolves to a commit other than the indexed one — usually because the indexed commit is no longer in the clone's history — the index is provably not at HEAD with no countable gap, so no number is reported: + +```jsonc +{ /* …the tool's normal result… */ + "staleness": { "status": "diverged", "hint": "⚠️ Index is not at HEAD and the commit gap could not be counted — the recorded commit may no longer be in this clone's history. Run analyze tool to update." } +} +``` + +The field is **absent when the index is current**, and these four tools also omit it when the freshness check could not run at all — that case is `status: "unknown"`, which only the `list_repos` listing reports. So its presence means the status is not `current`: read `status` before using `commitsBehind`. It is only ever added to object results — raw-array `cypher` output and error envelopes are returned unchanged. `@group`-targeted calls do not carry it (multi-repo staleness is ill-defined). When you see it, the graph may be behind the working tree — re-run `analyze` before trusting blast-radius or dependence answers. ### Taint findings (`explain`) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index a1d2589bb..1c6a99c36 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -24,7 +24,7 @@ Monorepo: **CLI/MCP** (`gitnexus/`) + **browser UI** (`gitnexus-web/`). - **HTTP bridge:** `serve.ts` → Express (`api.ts`, `mcp-http.ts`) for web UI - **CLI direct:** `gitnexus query|context|impact|cypher` in `tool.ts` -4. **Staleness** — `staleness.ts` compares indexed `lastCommit` to `HEAD`, surfaces hints. +4. **Staleness** — `core/git-staleness.ts` compares indexed `lastCommit` to `HEAD` and classifies the result as `current`, `behind`, `diverged` (HEAD moved off the indexed commit, gap uncountable) or `unknown`; `core/staleness-status.ts` builds the one `staleness` payload that MCP `list_repos`, the read tools and the `serve` repo routes all emit. ## MCP tools diff --git a/gitnexus-claude-plugin/skills/gitnexus-guide/SKILL.md b/gitnexus-claude-plugin/skills/gitnexus-guide/SKILL.md index e52560422..bf3c73948 100644 --- a/gitnexus-claude-plugin/skills/gitnexus-guide/SKILL.md +++ b/gitnexus-claude-plugin/skills/gitnexus-guide/SKILL.md @@ -83,15 +83,23 @@ Notes: `offset` ≥ `total` returns an empty page (with `total` still reported). ### Inline staleness signal (`query` / `context` / `impact` / `cypher`) -These four hot read tools attach a non-blocking `staleness` field to their response when the index is behind the checkout's current HEAD — the same `{ commitsBehind, hint }` shape `list_repos` already reports — so a direct tool call surfaces a behind-HEAD index without a separate `list_repos` call: +These four hot read tools attach a non-blocking `staleness` field to their response when the index is not at the checkout's current HEAD — the same `{ status, commitsBehind?, hint? }` shape `list_repos` already reports — so a direct tool call surfaces a stale index without a separate `list_repos` call: ```jsonc { /* …the tool's normal result… */ - "staleness": { "commitsBehind": 3, "hint": "⚠️ Index is 3 commits behind HEAD. Run analyze tool to update." } + "staleness": { "status": "behind", "commitsBehind": 3, "hint": "⚠️ Index is 3 commits behind HEAD. Run analyze tool to update." } } ``` -The field is **absent when the index is current** (or when the freshness check can't run), so its presence is the signal. It is only ever added to object results — raw-array `cypher` output and error envelopes are returned unchanged. `@group`-targeted calls do not carry it (multi-repo staleness is ill-defined). When you see it, the graph may be behind the working tree — re-run `analyze` before trusting blast-radius or dependence answers. +`commitsBehind` is present only when git counted the gap. When git could not count it but HEAD still resolves to a commit other than the indexed one — usually because the indexed commit is no longer in the clone's history — the index is provably not at HEAD with no countable gap, so no number is reported: + +```jsonc +{ /* …the tool's normal result… */ + "staleness": { "status": "diverged", "hint": "⚠️ Index is not at HEAD and the commit gap could not be counted — the recorded commit may no longer be in this clone's history. Run analyze tool to update." } +} +``` + +The field is **absent when the index is current**, and these four tools also omit it when the freshness check could not run at all — that case is `status: "unknown"`, which only the `list_repos` listing reports. So its presence means the status is not `current`: read `status` before using `commitsBehind`. It is only ever added to object results — raw-array `cypher` output and error envelopes are returned unchanged. `@group`-targeted calls do not carry it (multi-repo staleness is ill-defined). When you see it, the graph may be behind the working tree — re-run `analyze` before trusting blast-radius or dependence answers. ### Taint findings (`explain`) diff --git a/gitnexus-web/src/services/backend-client.ts b/gitnexus-web/src/services/backend-client.ts index 1cf1abde2..592e6b16c 100644 --- a/gitnexus-web/src/services/backend-client.ts +++ b/gitnexus-web/src/services/backend-client.ts @@ -33,11 +33,18 @@ export interface BackendRepo { /** Non-primary branch indexes recorded for the same path. */ branches?: Array<{ branch: string; indexedAt?: string; lastCommit?: string }>; /** - * Present only when the index is behind the repo's checked-out HEAD; absent - * means either up to date or not answerable (see the server's - * `repo-projection.ts`). Same shape MCP `list_repos` returns. + * Absent when the index is at the repo's checked-out HEAD. Otherwise `status` + * says what the server could establish: `behind` (with the counted + * `commitsBehind`), `diverged` (HEAD has moved off the indexed commit but the + * history needed to count the gap is gone, so there is no `commitsBehind`), + * or `unknown` (the repository could not be measured). Same shape MCP + * `list_repos` returns; see the server's `core/staleness-status.ts` (#3256). */ - staleness?: { commitsBehind: number; hint?: string }; + staleness?: { + status: 'behind' | 'diverged' | 'unknown'; + commitsBehind?: number; + hint?: string; + }; stats?: { files?: number; nodes?: number; diff --git a/gitnexus/src/cli/group-status-format.ts b/gitnexus/src/cli/group-status-format.ts new file mode 100644 index 000000000..f5d09c47e --- /dev/null +++ b/gitnexus/src/cli/group-status-format.ts @@ -0,0 +1,27 @@ +/** + * Rendering for `gitnexus group status` rows, kept out of the Commander action + * so it can be tested. Inline, the cell was only reachable by booting a backend + * against a real group, which is how `STALE (-1 commits behind)` went unnoticed + * (#3256). + */ + +/** The fields of a `groupStatus` repo row the index column reads. */ +export interface GroupRepoIndexRow { + indexStale: boolean; + commitsBehind?: number; +} + +/** + * The index column of a `group status` row. The output is unchanged except + * for one case: a count that is not a real count renders as `?`. + * + * `group/service.ts` has always reported a repo with no recorded commit as + * `{ indexStale: true, commitsBehind: -1 }`. The previous `?? '?'` fallback + * never caught that, because `??` only falls back on `null` / `undefined`. + */ +export const formatIndexStatusCell = (row: GroupRepoIndexRow): string => { + if (!row.indexStale) return 'OK '; + const n = row.commitsBehind; + const count = typeof n === 'number' && n >= 0 ? String(n) : '?'; + return `STALE (${count} commits behind)`; +}; diff --git a/gitnexus/src/cli/group.ts b/gitnexus/src/cli/group.ts index abc13fa2b..824b13b6a 100644 --- a/gitnexus/src/cli/group.ts +++ b/gitnexus/src/cli/group.ts @@ -4,6 +4,7 @@ import type { Command } from 'commander'; import type { RegistryWriteOutcome } from '../core/group/sync.js'; import type { MatchType } from '../core/group/types.js'; import { logger } from '../core/logger.js'; +import { formatIndexStatusCell } from './group-status-format.js'; const _require = createRequire(import.meta.url); const yaml = _require('js-yaml') as typeof import('js-yaml'); @@ -161,9 +162,7 @@ export function registerGroupCommands(program: Command): void { console.log(` ${repoPath.padEnd(25)} MISSING (no entry in the registry)`); continue; } - const idx = row.indexStale - ? `STALE (${row.commitsBehind ?? '?'} commits behind)` - : 'OK '; + const idx = formatIndexStatusCell(row); const ctr = row.contractsStale ? ' CONTRACTS_STALE' : ''; console.log(` ${repoPath.padEnd(25)} ${idx}${ctr}`); } diff --git a/gitnexus/src/core/git-staleness.ts b/gitnexus/src/core/git-staleness.ts index 5fbad1223..ece63a87c 100644 --- a/gitnexus/src/core/git-staleness.ts +++ b/gitnexus/src/core/git-staleness.ts @@ -8,6 +8,11 @@ import { promisify } from 'node:util'; import path from 'path'; import { readRegistry, type RegistryEntry, type CwdMatch } from '../storage/repo-manager.js'; import { findGitRootByDotGit, getCurrentCommit, getRemoteUrl } from '../storage/git.js'; +import type { StalenessInfo } from './staleness-status.js'; + +// The status/payload types and helpers live in the pure `staleness-status.ts` +// (#3256); the types are re-exported here for existing importers. +export type { StalenessInfo, StalenessStatus } from './staleness-status.js'; const execFileAsync = promisify(execFile); @@ -18,16 +23,71 @@ const execFileAsync = promisify(execFile); */ const STALENESS_TIMEOUT_MS = 5_000; -export interface StalenessInfo { - isStale: boolean; - commitsBehind: number; - hint?: string; -} +const behindHint = (n: number): string => + `⚠️ Index is ${n} commit${n > 1 ? 's' : ''} behind HEAD. Run analyze tool to update.`; + +// Says only what a failed count plus a resolved HEAD establish: the index is not +// at HEAD and the gap is uncountable. Reaching here does NOT prove the indexed +// commit left history — that is the usual cause (a pruned `fetch --depth 1`), +// but any other `rev-list` failure lands here too, so the cause is hedged. +const DIVERGED_HINT = + "⚠️ Index is not at HEAD and the commit gap could not be counted — the recorded commit may no longer be in this clone's history. Run analyze tool to update."; + +const unknown = (): StalenessInfo => ({ isStale: false, commitsBehind: 0, status: 'unknown' }); + +const fromCount = (commitsBehind: number): StalenessInfo => + commitsBehind > 0 + ? { isStale: true, commitsBehind, hint: behindHint(commitsBehind), status: 'behind' } + : { isStale: false, commitsBehind: 0, status: 'current' }; + +/** + * `rev-list` could not answer. Asking for HEAD alone needs no history walk and + * still separates all three answers: HEAD unreadable is `unknown`, HEAD past the + * indexed commit is `diverged`, and HEAD still *at* it is `current` — the ref + * prints the indexed SHA, so the index is at HEAD however `rev-list` failed. The + * historical fail-open values are kept either way; only `status` differs. + */ +const fromHead = (head: string | null, lastCommit: string): StalenessInfo => { + if (!head) return unknown(); + if (head === lastCommit) return { isStale: false, commitsBehind: 0, status: 'current' }; + return { isStale: false, commitsBehind: 0, hint: DIVERGED_HINT, status: 'diverged' }; +}; + +const readHeadSync = (repoPath: string): string | null => { + try { + return ( + execFileSync('git', ['rev-parse', 'HEAD'], { + cwd: repoPath, + encoding: 'utf-8', + stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, + }).trim() || null + ); + } catch { + return null; + } +}; + +const readHeadAsync = async (repoPath: string): Promise => { + try { + const { stdout } = await execFileAsync('git', ['rev-parse', 'HEAD'], { + cwd: repoPath, + encoding: 'utf-8', + windowsHide: true, + timeout: STALENESS_TIMEOUT_MS, + }); + return stdout.trim() || null; + } catch { + return null; + } +}; /** * Check how many commits the index is behind HEAD (synchronous; uses git CLI). */ export function checkStaleness(repoPath: string, lastCommit: string): StalenessInfo { + // No recorded commit is not "at HEAD": there is nothing to measure against. + if (!lastCommit) return unknown(); try { const result = execFileSync('git', ['rev-list', '--count', `${lastCommit}..HEAD`], { cwd: repoPath, @@ -36,19 +96,9 @@ export function checkStaleness(repoPath: string, lastCommit: string): StalenessI windowsHide: true, }).trim(); - const commitsBehind = parseInt(result, 10) || 0; - - if (commitsBehind > 0) { - return { - isStale: true, - commitsBehind, - hint: `⚠️ Index is ${commitsBehind} commit${commitsBehind > 1 ? 's' : ''} behind HEAD. Run analyze tool to update.`, - }; - } - - return { isStale: false, commitsBehind: 0 }; + return fromCount(parseInt(result, 10) || 0); } catch { - return { isStale: false, commitsBehind: 0 }; + return fromHead(readHeadSync(repoPath), lastCommit); } } @@ -61,6 +111,7 @@ export async function checkStalenessAsync( repoPath: string, lastCommit: string, ): Promise { + if (!lastCommit) return unknown(); try { // Note: promisified execFile captures stdout/stderr by default (no stdio option needed, // unlike the sync variant which requires explicit stdio: ['pipe','pipe','pipe']). @@ -73,24 +124,17 @@ export async function checkStalenessAsync( // A working tree on a disconnected network mount or behind a stuck lock // does exactly that, and `/api/repos` fans this out once per registered // repo, so one unreachable mount could hold the whole listing open - // (#3232 review). The timeout kills the child and rejects, which routes - // a hang into the same fail-closed "not stale" answer as a bad SHA. + // (#3232 review). The timeout kills the child and rejects, and the catch + // below reports it as `unknown` — still the fail-closed `isStale: false`. timeout: STALENESS_TIMEOUT_MS, }); - const commitsBehind = parseInt(stdout.trim(), 10) || 0; - - if (commitsBehind > 0) { - return { - isStale: true, - commitsBehind, - hint: `⚠️ Index is ${commitsBehind} commit${commitsBehind > 1 ? 's' : ''} behind HEAD. Run analyze tool to update.`, - }; - } - - return { isStale: false, commitsBehind: 0 }; - } catch { - return { isStale: false, commitsBehind: 0 }; + return fromCount(parseInt(stdout.trim(), 10) || 0); + } catch (err) { + // A rev-list that timed out means the working tree is not answering. Asking + // it again for HEAD would only double the bound #3232 put on a hung mount. + if ((err as { killed?: boolean }).killed) return unknown(); + return fromHead(await readHeadAsync(repoPath), lastCommit); } } diff --git a/gitnexus/src/core/group/service.ts b/gitnexus/src/core/group/service.ts index 6f41d6586..d003c3601 100644 --- a/gitnexus/src/core/group/service.ts +++ b/gitnexus/src/core/group/service.ts @@ -6,6 +6,7 @@ import fsp from 'node:fs/promises'; import path from 'node:path'; import { checkStaleness } from '../git-staleness.js'; +import { stalenessStatus, type StalenessInfo, type StalenessStatus } from '../staleness-status.js'; import { canonicalizePath, loadMeta, @@ -843,6 +844,17 @@ export class GroupService { /** Set only when `unresolvable`; says what could not be resolved. */ unresolvableReason?: string; commitsBehind?: number; + /** + * What the staleness check could establish (#3256). Additive: + * `indexStale` and `commitsBehind` keep their meaning. Two rows report + * `unknown` and they do NOT agree on those two fields, so read them + * together with this one: no commit was recorded (`indexStale: true`, + * `commitsBehind: -1`, the sentinel that case has always used), or the + * git probe could not answer (`indexStale: false`, `commitsBehind: 0`, + * straight from `checkStaleness`). MCP/HTTP `stalenessPayload` reports + * neither number — it omits `commitsBehind` for `unknown` entirely. + */ + status?: StalenessStatus; } > = {}; @@ -872,9 +884,9 @@ export class GroupService { const meta: Partial> = (await loadMeta(repoObj.storagePath)) ?? {}; - const staleness = meta.lastCommit + const staleness: StalenessInfo = meta.lastCommit ? checkStaleness(repoObj.repoPath, meta.lastCommit) - : { isStale: true, commitsBehind: -1 }; + : { isStale: true, commitsBehind: -1, status: 'unknown' }; const snapshot = registry?.repoSnapshots?.[repoPath]; const contractsStale = @@ -886,6 +898,7 @@ export class GroupService { missing: false, unresolvable: false, commitsBehind: staleness.commitsBehind, + status: stalenessStatus(staleness), }; } catch (err) { // The registry read succeeded, so its answer about this row is diff --git a/gitnexus/src/core/staleness-status.ts b/gitnexus/src/core/staleness-status.ts new file mode 100644 index 000000000..f4823c66c --- /dev/null +++ b/gitnexus/src/core/staleness-status.ts @@ -0,0 +1,89 @@ +/** + * The shape of a staleness answer, and the one wire payload every surface + * emits for it (#3256). Pure: no git, no I/O. + * + * Kept apart from `git-staleness.ts` on purpose. Tests across the suite stub + * that module with a fixed `vi.mock` factory so nothing shells out to git; a + * pure helper exported from it would come back `undefined` under every such + * stub. Here it is imported for real wherever the git probes are mocked. + */ + +/** + * What a staleness check was able to establish. + * + * `isStale` / `commitsBehind` alone cannot say "could not tell": every git + * failure collapses into `{ isStale: false, commitsBehind: 0 }`. That is + * deliberate — pinned by the fail-open tests, because the hot read tools must + * never fail or nag on an index they cannot measure — but it also made a + * provably stale index indistinguishable from a fresh one. `status` is the + * additive channel that separates them for a caller that wants to act on it: + * + * - `current` — the index is at HEAD: `rev-list` answered 0, or it could not + * answer but HEAD alone resolved to the indexed commit. + * - `behind` — `rev-list` answered N > 0; `commitsBehind` is N. + * - `diverged` — `rev-list` could not answer, but HEAD resolved and is not the + * indexed commit. The index is provably not at HEAD; only the count is + * unknown. A branch-pinned `serve` clone reaches this once git prunes the + * commit a failed re-index left behind — the pinned update is a + * `fetch --depth 1`, which orphans it — and a rewritten history reaches it + * directly. It is the rule the Claude hook already applies: + * HEAD !== lastCommit. + * - `unknown` — HEAD could not be resolved at all: not a git repository, git + * timed out, or no commit was recorded. + * + * `isStale` and `commitsBehind` keep their historical values in every case, so + * no existing consumer changes behaviour unless it reads `status`. + */ +export type StalenessStatus = 'current' | 'behind' | 'diverged' | 'unknown'; + +export interface StalenessInfo { + isStale: boolean; + commitsBehind: number; + hint?: string; + /** + * Always set by `checkStaleness` and `checkStalenessAsync`. Optional on the + * type so a hand-built info (tests, legacy literals) still compiles; read it + * through {@link stalenessStatus}, which derives it from `isStale` when absent. + */ + status?: StalenessStatus; +} + +/** `info.status`, or the answer `isStale` implies for an info built without one. */ +export const stalenessStatus = (info: StalenessInfo): StalenessStatus => + info.status ?? (info.isStale ? 'behind' : 'current'); + +/** + * The wire shape for staleness on every surface: MCP `list_repos`, the hot read + * tools, and the `serve` repo routes. One builder so one fact has one shape + * (#3232 review: "same sentinel as MCP"). + * + * Absent for `current`, as before. `commitsBehind` is present only when git + * actually counted it, so `diverged` carries `status` and `hint` but no number — + * inventing one would be the silent wrong answer this exists to remove. + */ +export interface StalenessPayload { + status: Exclude; + commitsBehind?: number; + hint?: string; +} + +/** + * Project a check into {@link StalenessPayload}, or `undefined` when there is + * nothing to report. + * + * `unknown` is emitted only when `includeUnknown` is set. A listing a monitor + * reads wants it; the hot read tools do not, because a `--skip-git` folder has + * no history to measure and would otherwise repeat that on every response. + */ +export const stalenessPayload = ( + info: StalenessInfo | undefined, + opts: { includeUnknown?: boolean } = {}, +): StalenessPayload | undefined => { + if (!info) return undefined; + const status = stalenessStatus(info); + const hint = info.hint ? { hint: info.hint } : {}; + if (status === 'current') return undefined; + if (status === 'unknown') return opts.includeUnknown ? { status } : undefined; + if (status === 'diverged') return { status, ...hint }; + return { status, commitsBehind: info.commitsBehind, ...hint }; +}; diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index bcb2cf1b4..b90ef8fcf 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -97,11 +97,12 @@ import { isSupportedCjkSegmentationMode, MAX_CJK_SEGMENTATION_QUERY_LENGTH, } from '../../core/search/cjk-segmentation.js'; +import { checkStalenessAsync, checkCwdMatch } from '../../core/git-staleness.js'; import { - checkStalenessAsync, - checkCwdMatch, + stalenessPayload, type StalenessInfo, -} from '../../core/git-staleness.js'; + type StalenessPayload, +} from '../../core/staleness-status.js'; import { logger } from '../../core/logger.js'; import { isLocalEmbeddingRuntimeBlockerMessage, @@ -1338,7 +1339,7 @@ export interface RepoListing { lastCommit: string; remoteUrl?: string; stats?: any; - staleness?: { commitsBehind: number; hint?: string }; + staleness?: StalenessPayload; siblings?: Array<{ name: string; path: string; lastCommit: string }>; /** Primary/flat branch name, when known (#2106). */ branch?: string; @@ -1428,21 +1429,21 @@ function canCarryStaleness(result: unknown): result is Record { /** * #2655: attach a non-blocking `staleness` signal to a tool result when the - * index is behind HEAD, mirroring the `list_repos` `{commitsBehind, hint}` - * shape. Only ever ADDS a field to a carryable object result (see + * index is not at HEAD, in the same {@link stalenessPayload} shape `list_repos` + * returns. Only ever ADDS a field to a carryable object result (see * {@link canCarryStaleness}) — it never changes an existing result's shape. + * + * `diverged` is attached: it is a positive finding that the index is not at + * HEAD, only uncountable. `unknown` is not — these are the hot read tools, and a + * `--skip-git` folder has no history to measure, so it would ride on every + * response as noise rather than signal (#3256). */ -export function attachToolStaleness( - result: unknown, - staleness: StalenessInfo | undefined, -): unknown { - if (!staleness?.isStale || !canCarryStaleness(result)) { +export function attachToolStaleness(result: unknown, info: StalenessInfo | undefined): unknown { + const staleness = stalenessPayload(info); + if (!staleness || !canCarryStaleness(result)) { return result; } - return { - ...result, - staleness: { commitsBehind: staleness.commitsBehind, hint: staleness.hint }, - }; + return { ...result, staleness }; } /** tri-review Residual-2: see `LocalBackend.lastObservedPoolState`'s doc comment. */ @@ -2468,9 +2469,7 @@ export class LocalBackend { lastCommit: h.lastCommit, remoteUrl: h.remoteUrl, stats: h.stats, - staleness: stale.isStale - ? { commitsBehind: stale.commitsBehind, hint: stale.hint } - : undefined, + staleness: stalenessPayload(stale, { includeUnknown: true }), siblings: siblings.length > 0 ? siblings.map((s) => ({ @@ -2624,9 +2623,10 @@ export class LocalBackend { * one `git rev-list` per index per TTL window; the resolved value is cached * for TOOL_STALENESS_TTL_MS. Keyed by lbugPath so flat and branch handles * (same repoPath, different lastCommit) don't share an entry. Non-blocking by - * construction: `checkStalenessAsync` swallows git failures to - * `{ isStale: false }`, so a git error never fails the tool — it just omits - * the `staleness` field. + * construction: `checkStalenessAsync` keeps `isStale: false` on every git + * failure and reports what it could still establish in `status` (`diverged`, + * `unknown`, or `current` when HEAD alone matches the index), so a git error + * never fails the tool — at most it attaches a `diverged` staleness field. */ private stalenessForTool(repo: RepoHandle): Promise { const now = Date.now(); diff --git a/gitnexus/src/mcp/staleness.ts b/gitnexus/src/mcp/staleness.ts index 811889f3a..9ee20710e 100644 --- a/gitnexus/src/mcp/staleness.ts +++ b/gitnexus/src/mcp/staleness.ts @@ -1,6 +1,8 @@ /** - * Staleness Check — re-export from core (see `core/git-staleness.ts`). + * Staleness Check — re-export from core (see `core/git-staleness.ts` and + * `core/staleness-status.ts`). */ -export type { StalenessInfo } from '../core/git-staleness.js'; +export type { StalenessInfo, StalenessPayload, StalenessStatus } from '../core/staleness-status.js'; export { checkStaleness } from '../core/git-staleness.js'; +export { stalenessPayload, stalenessStatus } from '../core/staleness-status.js'; diff --git a/gitnexus/src/server/repo-projection.ts b/gitnexus/src/server/repo-projection.ts index 9ada19403..1084260c3 100644 --- a/gitnexus/src/server/repo-projection.ts +++ b/gitnexus/src/server/repo-projection.ts @@ -9,34 +9,33 @@ * These are pure: the caller resolves the registry entry, the on-disk metadata * and the staleness check, and passes the results in. */ -import type { StalenessInfo } from '../core/git-staleness.js'; +import { + stalenessPayload, + type StalenessInfo, + type StalenessPayload, +} from '../core/staleness-status.js'; import type { RegistryEntry } from '../storage/repo-manager.js'; import type { RepoMeta } from '../storage/repo-meta.js'; /** - * Staleness in the shape MCP `list_repos` already returns - * (`mcp/local/local-backend.ts`): the key is present only when the index is - * actually behind, so "fresh" stays the absence of a field rather than a second - * thing for a client to interpret. Deliberately identical across the two - * surfaces — the same fact should not have two shapes. + * Staleness through the shared {@link stalenessPayload} builder, so this route + * and MCP `list_repos` emit one shape for one fact (#3232 review, #3256). * - * `checkStalenessAsync` self-catches and reports 0 commits behind when the - * commit cannot be resolved, so an unanswerable check degrades to "not stale" - * rather than failing the request that carries it. + * Absent when the index is current. Otherwise `staleness.status` says what git + * could establish: `behind` with the counted `commitsBehind`; `diverged` when + * HEAD has provably moved off the indexed commit but the history needed to + * count the gap is gone — the state a branch-pinned `url` clone reaches once + * git prunes the commit a failed re-index left behind; or `unknown` when the + * repository could not be measured at all. This is a listing a monitor reads, + * so `unknown` is included here, unlike on the hot read tools. * - * That makes the field meaningful mainly for `path`-registered repositories, - * where an operator commits into the working tree the index was built from. - * A `url` repository is cloned `--depth 1` and is re-analyzed by the same run - * that pulls it, so its recorded commit is HEAD; and were the two ever to - * diverge, `git rev-list ..HEAD` cannot walk a shallow history and fails - * closed to "not stale". Reporting fresh there is not a claim that the remote - * has not moved — this measures the index against the local working tree, the - * same thing `gitnexus status` and MCP `list_repos` measure. + * All of it measures the index against the local working tree — the same thing + * `gitnexus status` and MCP `list_repos` measure — not against the remote. */ -export const stalenessField = ( - info: StalenessInfo, -): { staleness?: { commitsBehind: number; hint?: string } } => - info.isStale ? { staleness: { commitsBehind: info.commitsBehind, hint: info.hint } } : {}; +export const stalenessField = (info: StalenessInfo): { staleness?: StalenessPayload } => { + const staleness = stalenessPayload(info, { includeUnknown: true }); + return staleness ? { staleness } : {}; +}; /** One entry of `GET /api/repos`. */ export const projectRepoListEntry = (entry: RegistryEntry, staleness: StalenessInfo) => ({ diff --git a/gitnexus/test/integration/server-repo-freshness.test.ts b/gitnexus/test/integration/server-repo-freshness.test.ts index 16d5312e6..6a058b565 100644 --- a/gitnexus/test/integration/server-repo-freshness.test.ts +++ b/gitnexus/test/integration/server-repo-freshness.test.ts @@ -190,6 +190,7 @@ describeBlock('repo routes expose branch and freshness (real server)', () => { expect(entry.branch).toBe('main'); // Genuinely computed by `git rev-list`, not a fixture constant. expect(entry.staleness).toEqual({ + status: 'behind', commitsBehind: 1, hint: expect.stringContaining('1 commit behind'), }); @@ -216,6 +217,7 @@ describeBlock('repo routes expose branch and freshness (real server)', () => { expect(repo.lastCommit).toMatch(/^[0-9a-f]{40}$/); expect(repo.branch).toBe('main'); expect(repo.staleness).toEqual({ + status: 'behind', commitsBehind: 1, hint: expect.stringContaining('1 commit behind'), }); diff --git a/gitnexus/test/unit/calltool-dispatch.test.ts b/gitnexus/test/unit/calltool-dispatch.test.ts index 4e8c42ef1..a9a704183 100644 --- a/gitnexus/test/unit/calltool-dispatch.test.ts +++ b/gitnexus/test/unit/calltool-dispatch.test.ts @@ -4437,6 +4437,37 @@ describe('LocalBackend.listRepos', () => { // listRegisteredRepos called: once in init, once per listRepos expect(listRegisteredRepos).toHaveBeenCalledTimes(3); }); + + // #3256: `unknown` is listing-only (the hot read tools drop it; see + // tool-staleness.test.ts), so list_repos is where it must appear. `diverged` + // carries its hint and no invented count; `current` carries nothing. + it('reports unknown and diverged staleness on the listing (#3256)', async () => { + setupSingleRepo(); + await backend.init(); + const { checkStalenessAsync } = await import('../../src/core/git-staleness.js'); + const check = checkStalenessAsync as unknown as ReturnType; + try { + check.mockResolvedValue({ isStale: false, commitsBehind: 0, status: 'unknown' }); + expect((await backend.listRepos())[0].staleness).toEqual({ status: 'unknown' }); + + check.mockResolvedValue({ + isStale: false, + commitsBehind: 0, + status: 'diverged', + hint: 'HEAD moved on', + }); + expect((await backend.listRepos())[0].staleness).toEqual({ + status: 'diverged', + hint: 'HEAD moved on', + }); + + check.mockResolvedValue({ isStale: false, commitsBehind: 0, status: 'current' }); + expect((await backend.listRepos())[0].staleness).toBeUndefined(); + } finally { + // The module-level mock is shared; put back the factory's default. + check.mockResolvedValue({ isStale: false, commitsBehind: 0 }); + } + }); }); // ─── list_repos pagination (#2119) ───────────────────────────────────── diff --git a/gitnexus/test/unit/cli-group-status-format.test.ts b/gitnexus/test/unit/cli-group-status-format.test.ts new file mode 100644 index 000000000..32278dd04 --- /dev/null +++ b/gitnexus/test/unit/cli-group-status-format.test.ts @@ -0,0 +1,33 @@ +/** + * The index column of `gitnexus group status` (#3256). + * + * `group/service.ts` reports a repo with no recorded commit as + * `{ indexStale: true, commitsBehind: -1 }`. The CLI rendered that with + * `commitsBehind ?? '?'`, and `??` only falls back on `null` / `undefined`, so + * the command printed `STALE (-1 commits behind)`. Nothing covered it, because + * the cell lived inside a Commander action that only runs against a real group. + */ +import { describe, expect, it } from 'vitest'; +import { formatIndexStatusCell } from '../../src/cli/group-status-format.js'; + +describe('formatIndexStatusCell', () => { + it('renders the no-recorded-commit sentinel as ?, not -1', () => { + expect(formatIndexStatusCell({ indexStale: true, commitsBehind: -1 })).toBe( + 'STALE (? commits behind)', + ); + }); + + it('renders a missing count as ?', () => { + expect(formatIndexStatusCell({ indexStale: true })).toBe('STALE (? commits behind)'); + }); + + it('renders a counted gap unchanged', () => { + expect(formatIndexStatusCell({ indexStale: true, commitsBehind: 3 })).toBe( + 'STALE (3 commits behind)', + ); + }); + + it('keeps the OK cell byte-identical, padding included', () => { + expect(formatIndexStatusCell({ indexStale: false, commitsBehind: 0 })).toBe('OK '); + }); +}); diff --git a/gitnexus/test/unit/group/service.test.ts b/gitnexus/test/unit/group/service.test.ts index 8c5c0ed6d..4d51e51f3 100644 --- a/gitnexus/test/unit/group/service.test.ts +++ b/gitnexus/test/unit/group/service.test.ts @@ -8,6 +8,7 @@ import { type GroupRepoHandle, } from '../../../src/core/group/service.js'; import { writeContractRegistry } from '../../../src/core/group/storage.js'; +import { formatIndexStatusCell } from '../../../src/cli/group-status-format.js'; import type { ContractRegistry, StoredContract, CrossLink } from '../../../src/core/group/types.js'; function makeTmpGroup(): { tmpDir: string; groupDir: string; cleanup: () => void } { @@ -535,5 +536,49 @@ repos: cleanup(); } }); + + // #3256: a resolvable repo with no recorded commit. The formatter tests + // hand-build this row; this one drives the real composition, so swapping + // the no-commit literal for the git-probe `unknown()` (indexStale: false, + // commitsBehind: 0) would print `OK` here and fail. + it('test_groupStatus_reports_no_recorded_commit_as_unknown_and_renders_it_as_?', async () => { + const { cleanup, tmpDir } = makeTmpGroup(); + try { + vi.stubEnv('GITNEXUS_HOME', tmpDir); + // Resolves, but its storage holds no meta.json, so nothing was recorded. + const storagePath = path.join(tmpDir, 'no-meta', '.gitnexus'); + fs.mkdirSync(storagePath, { recursive: true }); + const port = makePort({ + resolveRepo: vi.fn( + async (name?: string): Promise => ({ + id: name || 'test', + name: name || 'test', + repoPath: path.join(tmpDir, 'no-meta'), + storagePath, + }), + ), + }); + + const svc = new GroupService(port); + const result = (await svc.groupStatus({ name: 'test-group' })) as { + repos: Record< + string, + { indexStale: boolean; commitsBehind?: number; status?: string; missing: boolean } + >; + }; + + const row = result.repos['app/backend']; + expect(row).toMatchObject({ + missing: false, + indexStale: true, + commitsBehind: -1, + status: 'unknown', + }); + expect(formatIndexStatusCell(row)).toBe('STALE (? commits behind)'); + } finally { + vi.unstubAllEnvs(); + cleanup(); + } + }); }); }); diff --git a/gitnexus/test/unit/repo-projection.test.ts b/gitnexus/test/unit/repo-projection.test.ts index 2d2d82043..c2968a764 100644 --- a/gitnexus/test/unit/repo-projection.test.ts +++ b/gitnexus/test/unit/repo-projection.test.ts @@ -58,16 +58,30 @@ describe('stalenessField', () => { it('reports commits behind and the hint when the index is behind', () => { expect(stalenessField(BEHIND)).toEqual({ - staleness: { commitsBehind: 3, hint: BEHIND.hint }, + staleness: { status: 'behind', commitsBehind: 3, hint: BEHIND.hint }, }); }); - it('treats an unresolvable check as fresh rather than as an error', () => { - // checkStalenessAsync self-catches to {isStale:false, commitsBehind:0} for a - // shallow clone, rewritten history or a non-git path. That must degrade to a - // normal response, never a 500 on a route whose job is to list repos. + it('reads an info built without a status as current, and omits the key', () => { + // Hand-built and legacy infos carry no `status`; `stalenessStatus` derives + // it from `isStale`, so a fail-open `{isStale:false}` stays a normal + // response rather than an error on a route whose job is to list repos. expect(stalenessField({ isStale: false, commitsBehind: 0 })).toEqual({}); }); + + it('reports diverged with its hint and no invented count (#3256)', () => { + // HEAD has moved off the indexed commit but the history needed to count the + // gap is gone: the state a branch-pinned url clone reaches after gc. + expect( + stalenessField({ isStale: false, commitsBehind: 0, status: 'diverged', hint: 'moved on' }), + ).toEqual({ staleness: { status: 'diverged', hint: 'moved on' } }); + }); + + it('reports unknown on a listing, where a monitor is looking (#3256)', () => { + expect(stalenessField({ isStale: false, commitsBehind: 0, status: 'unknown' })).toEqual({ + staleness: { status: 'unknown' }, + }); + }); }); describe('projectRepoListEntry — GET /api/repos', () => { @@ -117,6 +131,7 @@ describe('projectRepoListEntry — GET /api/repos', () => { it('carries staleness through for a behind index', () => { expect(projectRepoListEntry(entry(), BEHIND).staleness).toEqual({ + status: 'behind', commitsBehind: 3, hint: BEHIND.hint, }); diff --git a/gitnexus/test/unit/shipped-skills-sync.test.ts b/gitnexus/test/unit/shipped-skills-sync.test.ts index f0641ed91..fd27892ea 100644 --- a/gitnexus/test/unit/shipped-skills-sync.test.ts +++ b/gitnexus/test/unit/shipped-skills-sync.test.ts @@ -265,6 +265,10 @@ describe('intended standard-skill improvements stay in every applicable copy', ( const content = fs.readFileSync(file, 'utf-8'); expect(content).toContain('### Inline staleness signal'); expect(content).toContain('commitsBehind'); + // #3256: the field gained `status`, and the `diverged` arm carries no + // count — the reason an agent has to read `status` before the number. + expect(content).toContain('{ status, commitsBehind?, hint? }'); + expect(content).toContain('"status": "diverged"'); } }); diff --git a/gitnexus/test/unit/staleness-fallback.test.ts b/gitnexus/test/unit/staleness-fallback.test.ts new file mode 100644 index 000000000..427c52b40 --- /dev/null +++ b/gitnexus/test/unit/staleness-fallback.test.ts @@ -0,0 +1,100 @@ +/** + * #3256: what a staleness check reports once `rev-list` has failed for a reason + * OTHER than a timeout. `staleness.test.ts` reaches `diverged` and `unknown` + * against real repositories, but not the third arm of `fromHead`: HEAD still + * resolves to the indexed commit, so the index is at HEAD however `rev-list` + * failed. Real git cannot fail `..HEAD` while HEAD prints that same SHA + * without a corrupted object store, so this drives it through a mock. + * + * Its own file for the same reason as `staleness-timeout.test.ts`: the mock + * replaces `node:child_process` for the whole module graph. + */ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { plan, spawnedArgs } = vi.hoisted(() => ({ + plan: { head: null as string | null }, + spawnedArgs: [] as string[][], +})); + +// `rev-list` exits 128 the way git does for a missing object; `rev-parse HEAD` +// answers `plan.head`, or fails when it is null. +const answer = (args: readonly string[]): { error: Error | null; stdout: string } => { + spawnedArgs.push([...args]); + if (args[0] === 'rev-parse' && plan.head) return { error: null, stdout: `${plan.head}\n` }; + const failure = Object.assign(new Error(`Command failed: git ${args.join(' ')}`), { + code: 128, + killed: false, + }); + return { error: failure, stdout: '' }; +}; + +vi.mock('node:child_process', async (importOriginal) => { + const actual = await importOriginal(); + const execFile = ( + _file: string, + args: readonly string[], + _options: unknown, + callback: (error: Error | null, result: { stdout: string; stderr: string }) => void, + ): void => { + const { error, stdout } = answer(args); + callback(error, { stdout, stderr: '' }); + }; + const execFileSync = (_file: string, args: readonly string[]): string => { + const { error, stdout } = answer(args); + if (error) throw error; + return stdout; + }; + return { + ...actual, + execFile: execFile as unknown as typeof actual.execFile, + execFileSync: execFileSync as unknown as typeof actual.execFileSync, + }; +}); + +import { checkStaleness, checkStalenessAsync } from '../../src/core/git-staleness.js'; + +const INDEXED_COMMIT = 'a'.repeat(40); +const REV_LIST = ['rev-list', '--count', `${INDEXED_COMMIT}..HEAD`]; +const REV_PARSE = ['rev-parse', 'HEAD']; + +const bothHelpers = { + checkStaleness: async (repo: string, lastCommit: string) => checkStaleness(repo, lastCommit), + checkStalenessAsync, +}; + +describe('staleness after a failed (not timed-out) rev-list (#3256)', () => { + beforeEach(() => { + plan.head = null; + spawnedArgs.length = 0; + }); + + for (const [name, check] of Object.entries(bothHelpers)) { + describe(name, () => { + it('reports current when HEAD alone still resolves to the indexed commit', async () => { + plan.head = INDEXED_COMMIT; + + const result = await check('/repo', INDEXED_COMMIT); + + expect(result).toEqual({ isStale: false, commitsBehind: 0, status: 'current' }); + // The answer came from the HEAD probe, not from rev-list. + expect(spawnedArgs).toEqual([REV_LIST, REV_PARSE]); + }); + + it('reports diverged when HEAD resolves elsewhere', async () => { + plan.head = 'b'.repeat(40); + + const result = await check('/repo', INDEXED_COMMIT); + + expect(result).toMatchObject({ isStale: false, commitsBehind: 0, status: 'diverged' }); + expect(spawnedArgs).toEqual([REV_LIST, REV_PARSE]); + }); + + it('reports unknown when HEAD cannot be read either', async () => { + const result = await check('/repo', INDEXED_COMMIT); + + expect(result).toEqual({ isStale: false, commitsBehind: 0, status: 'unknown' }); + expect(spawnedArgs).toEqual([REV_LIST, REV_PARSE]); + }); + }); + } +}); diff --git a/gitnexus/test/unit/staleness-timeout.test.ts b/gitnexus/test/unit/staleness-timeout.test.ts new file mode 100644 index 000000000..1b80df5b8 --- /dev/null +++ b/gitnexus/test/unit/staleness-timeout.test.ts @@ -0,0 +1,48 @@ +/** + * #3256 + #3232: a `rev-list` that TIMED OUT must answer `unknown` from the + * timeout alone. The #3232 bound exists per request, so asking the same + * unresponsive working tree for HEAD afterwards would double it — and could + * report `diverged`/`current` off a HEAD a hung tree may still answer for. + * + * Its own file because the mock replaces `node:child_process` for the whole + * module graph, while `staleness.test.ts` drives these helpers against real git + * repositories and must keep the real one. + */ +import { describe, expect, it, vi } from 'vitest'; + +const { spawnedArgs } = vi.hoisted(() => ({ spawnedArgs: [] as string[][] })); + +vi.mock('node:child_process', async (importOriginal) => { + const actual = await importOriginal(); + const execFile = ( + _file: string, + args: readonly string[], + _options: unknown, + callback: (error: Error | null, result: { stdout: string; stderr: string }) => void, + ): void => { + spawnedArgs.push([...args]); + // What `promisify(execFile)` rejects with when `timeout` kills the child: + // Node 22 reports `killed: true` alongside `signal: 'SIGTERM'`. + const timedOut = Object.assign(new Error('Command failed: git rev-list'), { + killed: true, + signal: 'SIGTERM', + }); + callback(timedOut, { stdout: '', stderr: '' }); + }; + return { ...actual, execFile: execFile as unknown as typeof actual.execFile }; +}); + +const INDEXED_COMMIT = 'a'.repeat(40); + +describe('checkStalenessAsync — timed-out rev-list (#3256)', () => { + it('reports unknown without probing HEAD again', async () => { + const { checkStalenessAsync } = await import('../../src/core/git-staleness.js'); + + const result = await checkStalenessAsync('/repo', INDEXED_COMMIT); + + expect(result).toEqual({ status: 'unknown', isStale: false, commitsBehind: 0 }); + // Exactly one spawn. A `rev-parse HEAD` follow-up here is the regression: + // it doubles the hung-mount bound and can flip this answer. + expect(spawnedArgs).toEqual([['rev-list', '--count', `${INDEXED_COMMIT}..HEAD`]]); + }); +}); diff --git a/gitnexus/test/unit/staleness.test.ts b/gitnexus/test/unit/staleness.test.ts index 1645d9987..ef4a6a517 100644 --- a/gitnexus/test/unit/staleness.test.ts +++ b/gitnexus/test/unit/staleness.test.ts @@ -6,9 +6,17 @@ * - HEAD differs → stale with commit count * - Git failure → fail open (not stale) */ -import { describe, it, expect } from 'vitest'; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from 'vitest'; import { execFileSync } from 'child_process'; -import { checkStaleness, checkStalenessAsync } from '../../src/core/git-staleness.js'; +import { + checkStaleness, + checkStalenessAsync, + type StalenessInfo, +} from '../../src/core/git-staleness.js'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { pathToFileURL } from 'node:url'; // We test checkStaleness with a real git repo (the project itself) // since mocking execFileSync across ESM modules is complex. @@ -144,3 +152,186 @@ describe('checkStalenessAsync', () => { expect(parallelMs).toBeLessThan(sequentialMs * 1.5); }); }); + +// ── #3256: the additive `status` channel ───────────────────────────────────── +// +// The fail-open tests above pin `isStale` / `commitsBehind` and are unchanged. +// These pin `status`, which separates "could not tell" from "fresh". They build +// their own repositories rather than leaning on this checkout, so every answer +// is exact in a shallow CI clone too. + +const git = (cwd: string, ...args: string[]): string => + execFileSync( + 'git', + ['-c', 'user.email=t@example.com', '-c', 'user.name=T', '-c', 'commit.gpgsign=false', ...args], + { cwd, encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] }, + ).trim(); + +const commit = (repo: string, text: string): string => { + writeFileSync(join(repo, 'a.txt'), `${text}\n`); + git(repo, 'add', '-A'); + git(repo, 'commit', '-q', '-m', text); + return git(repo, 'rev-parse', 'HEAD'); +}; + +/** A repository with three commits. */ +const makeRepo = (root: string, name: string) => { + const repo = join(root, name); + git(root, 'init', '-q', '--initial-branch=main', repo); + const c1 = commit(repo, 'c1'); + const c2 = commit(repo, 'c2'); + const c3 = commit(repo, 'c3'); + return { repo, c1, c2, c3 }; +}; + +const removeTree = (dir: string) => + rmSync(dir, { recursive: true, force: true, maxRetries: 3, retryDelay: 100 }); + +const bothHelpers: Record< + string, + (repoPath: string, lastCommit: string) => Promise +> = { + checkStaleness: async (repoPath, lastCommit) => checkStaleness(repoPath, lastCommit), + checkStalenessAsync, +}; + +describe('staleness status (#3256)', () => { + let root: string; + let fixture: ReturnType; + + beforeAll(() => { + root = mkdtempSync(join(tmpdir(), 'gitnexus-staleness-')); + fixture = makeRepo(root, 'repo'); + }); + afterAll(() => removeTree(root)); + + for (const [name, check] of Object.entries(bothHelpers)) { + describe(name, () => { + it('reports current when the index is at HEAD', async () => { + expect(await check(fixture.repo, fixture.c3)).toMatchObject({ + status: 'current', + isStale: false, + commitsBehind: 0, + }); + }); + + it('reports behind with the counted gap', async () => { + expect(await check(fixture.repo, fixture.c1)).toMatchObject({ + status: 'behind', + isStale: true, + commitsBehind: 2, + }); + }); + + it('reports diverged, not current, when the recorded commit is not in history', async () => { + const result = await check(fixture.repo, '0000000000000000000000000000000000000abc'); + expect(result.status).toBe('diverged'); + // The hint claims only the uncountable gap, not that the commit left + // history — every other `rev-list` failure reaches the same branch. + expect(result.hint).toContain('could not be counted'); + // The fail-open values the tests above pin are unchanged. + expect(result).toMatchObject({ isStale: false, commitsBehind: 0 }); + }); + + it('reports unknown when the path is not a git repository', async () => { + const notGit = mkdtempSync(join(root, 'not-git-')); + expect(await check(notGit, fixture.c3)).toMatchObject({ + status: 'unknown', + isStale: false, + commitsBehind: 0, + }); + }); + + it('reports unknown when no commit was recorded', async () => { + expect(await check(fixture.repo, '')).toMatchObject({ + status: 'unknown', + isStale: false, + commitsBehind: 0, + }); + }); + }); + } +}); + +describe('branch-pinned serve clone once git prunes the indexed commit (#3256)', () => { + // The pinned update is `fetch --depth 1` + `checkout -B` + // (fetchAndCheckoutRequestedBranch in server/git-clone.ts). It re-shallows at + // the new tip, which orphans the indexed commit. While the HEAD reflog holds + // that commit, rev-list still counts; once the reflog entry expires and gc + // prunes it, rev-list cannot answer at all. + let root: string; + + beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'gitnexus-staleness-shallow-')); + }); + afterEach(() => removeTree(root)); + + /** A depth-1 clone whose upstream has since moved on by one commit. */ + const shallowCloneBehindUpstream = () => { + const upstream = makeRepo(root, 'upstream'); + const clone = join(root, 'clone'); + git( + root, + '-c', + 'protocol.file.allow=always', + 'clone', + '-q', + '--depth', + '1', + pathToFileURL(upstream.repo).href, + clone, + ); + const indexed = git(clone, 'rev-parse', 'HEAD'); + commit(upstream.repo, 'c4'); // the re-index that pulls this is the one that fails + return { clone, indexed }; + }; + + const expireAndPrune = (clone: string) => { + git(clone, 'reflog', 'expire', '--expire-unreachable=now', '--all'); + git(clone, 'gc', '-q', '--prune=now'); + }; + + it('reports diverged instead of fresh once the orphaned commit is pruned', async () => { + const { clone, indexed } = shallowCloneBehindUpstream(); + git( + clone, + 'fetch', + '-q', + '--depth', + '1', + 'origin', + '+refs/heads/main:refs/remotes/origin/main', + ); + git(clone, 'checkout', '-q', '-B', 'main', 'origin/main'); + + // Countable while the reflog still holds the indexed commit. + expect(await checkStalenessAsync(clone, indexed)).toMatchObject({ + status: 'behind', + commitsBehind: 1, + }); + + expireAndPrune(clone); + + // Before #3256 this read as `{ isStale: false, commitsBehind: 0 }`, with + // nothing to tell it apart from an index that is genuinely current. + expect(await checkStalenessAsync(clone, indexed)).toMatchObject({ + status: 'diverged', + isStale: false, + }); + expect(checkStaleness(clone, indexed).status).toBe('diverged'); + }); + + it('keeps an unpinned pull --ff-only clone countable after gc', async () => { + // The unpinned update keeps the indexed commit reachable as HEAD's parent, + // so gc never prunes it and the count survives. + const { clone, indexed } = shallowCloneBehindUpstream(); + git(clone, 'pull', '-q', '--ff-only'); + expireAndPrune(clone); + + expect(await checkStalenessAsync(clone, indexed)).toMatchObject({ + status: 'behind', + isStale: true, + commitsBehind: 1, + }); + }); +}); diff --git a/gitnexus/test/unit/tool-staleness.test.ts b/gitnexus/test/unit/tool-staleness.test.ts index e5fe6e8f0..6d36102ef 100644 --- a/gitnexus/test/unit/tool-staleness.test.ts +++ b/gitnexus/test/unit/tool-staleness.test.ts @@ -71,3 +71,35 @@ describe('attachToolStaleness (#2655)', () => { expect(out.staleness).toMatchObject({ commitsBehind: 1 }); }); }); + +describe('attachToolStaleness — status (#3256)', () => { + it('labels a counted gap as behind', () => { + const out = attachToolStaleness({ ok: true }, STALE) as { staleness: unknown }; + expect(out.staleness).toEqual({ status: 'behind', commitsBehind: 3, hint: STALE.hint }); + }); + + it('attaches diverged with its hint and no invented count', () => { + const out = attachToolStaleness( + { ok: true }, + { isStale: false, commitsBehind: 0, status: 'diverged', hint: 'HEAD moved on' }, + ) as { staleness: Record }; + expect(out.staleness).toEqual({ status: 'diverged', hint: 'HEAD moved on' }); + expect('commitsBehind' in out.staleness).toBe(false); + }); + + it('does not attach unknown to a hot read tool result', () => { + // A `--skip-git` folder has no history to measure; repeating that on every + // read tool response is noise, so `unknown` stays off this path. + const result = { ok: true }; + expect( + attachToolStaleness(result, { isStale: false, commitsBehind: 0, status: 'unknown' }), + ).toBe(result); + }); + + it('still leaves a current index untouched', () => { + const result = { ok: true }; + expect( + attachToolStaleness(result, { isStale: false, commitsBehind: 0, status: 'current' }), + ).toBe(result); + }); +});