mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-08 03:08:13 +00:00
feat(staleness): report diverged and unknown index state instead of fresh (#3257)
* feat(staleness): report diverged and unknown index state instead of fresh
A staleness check collapsed every git failure into { isStale: false,
commitsBehind: 0 }. On a branch-pinned serve clone, a failed re-index
leaves the recorded commit orphaned by the --depth 1 fetch; once the
reflog expires and gc prunes it, rev-list fails and the index silently
reads as fresh while still behind.
checkStaleness / checkStalenessAsync now return an additive status:
current, behind, diverged (rev-list failed but HEAD resolved and differs
from lastCommit) or unknown (HEAD unresolvable, no lastCommit, timeout).
isStale and commitsBehind keep their values in every case.
One payload builder (core/staleness-status.ts) feeds MCP list_repos, the
hot read tools and /api/repos + /api/repo. diverged carries a hint and no
invented count; unknown appears on listings only. The helpers live in a
pure module so existing vi.mock stubs of git-staleness stay valid.
gitnexus group status no longer prints "-1 commits behind".
Fixes #3256
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Address PR review feedback (#3257)
- Document the `current` arm `fromHead` can return after a failed `rev-list`.
The `staleness-status.ts` status list, the `fromHead` docstring and the
`stalenessForTool` comment each named only `diverged`/`unknown`, so all three
described a contract the helpers do not have.
- Document both `unknown` rows on the group status type: no recorded commit
(`indexStale: true`, `commitsBehind: -1`) and a git probe that could not
answer (`indexStale: false`, `commitsBehind: 0`), which do not agree on
either field.
- Update both shipped `gitnexus-guide` copies to the new wire shape
(`{ status, commitsBehind?, hint? }`), add a `diverged` example carrying no
count, and state `unknown` is reported only by the `list_repos` listing.
Presence now means "not `current`", not "behind N". Pinned in
shipped-skills-sync.
- Lock the timed-out `rev-list` short-circuit with staleness-timeout.test.ts:
it asserts `unknown` from exactly one git spawn, so removing the `killed`
guard (which would probe HEAD again and double the #3232 bound) now fails.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Claim only the uncountable gap in the diverged hint (#3257)
`fromHead` reaches the `diverged` branch on any non-timeout `rev-list` failure
where `rev-parse HEAD` resolves to a different SHA. It compares the two SHAs and
runs no reachability check, so a transient object-read failure lands there while
`lastCommit` is still an ancestor of HEAD — and the hint told the operator the
commit was gone from history. `staleness-status.ts` already documents `diverged`
as only "provably not at HEAD; the count is unknown", so the string was also the
one place contradicting its own contract.
The hint now states what the check established and hedges the usual cause:
"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."
Updated with it: the `staleness.test.ts` diverged assertion, and the `diverged`
example plus its lead-in sentence in both shipped `gitnexus-guide` copies (still
byte-identical). `mcp/resources.ts` needs no change — it interpolates
`staleness.hint`, holding no copy of the text.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(staleness): cover the remaining #3256 review test gaps
The tri-review's lower-priority gaps, each now locked:
- staleness-fallback.test.ts: after a failed (not timed-out) rev-list,
both helpers report `current` when HEAD alone still resolves to the
indexed commit, `diverged` when it resolves elsewhere, and `unknown`
when it cannot be read, each from exactly rev-list + rev-parse. This
complements staleness-timeout.test.ts: a timeout spawns once, any other
failure probes HEAD.
- list_repos carries the listing-only `unknown` and a count-free
`diverged`; `current` carries nothing.
- group status: a resolvable repo with no recorded commit composes to
indexStale: true, commitsBehind: -1, status: unknown and renders as
"STALE (? commits behind)", through the real groupStatus path rather
than a hand-built row.
Each test was checked against a mutation of the code it guards (the
fromHead current arm, the group no-commit literal, list_repos
includeUnknown); every mutation fails its test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: tech-admin3 <tech-admin@kodnest.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
This commit is contained in:
Ankit Verma
GitHub
parent
c9b391357e
commit
8bd71c8335
22 changed files with 796 additions and 99 deletions
|
|
@ -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`)
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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`)
|
||||
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
27
gitnexus/src/cli/group-status-format.ts
Normal file
27
gitnexus/src/cli/group-status-format.ts
Normal file
|
|
@ -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)`;
|
||||
};
|
||||
|
|
@ -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}`);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<string | null> => {
|
||||
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<StalenessInfo> {
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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<Pick<RepoMeta, 'lastCommit' | 'indexedAt'>> =
|
||||
(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
|
||||
|
|
|
|||
89
gitnexus/src/core/staleness-status.ts
Normal file
89
gitnexus/src/core/staleness-status.ts
Normal file
|
|
@ -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<StalenessStatus, 'current'>;
|
||||
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 };
|
||||
};
|
||||
|
|
@ -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<string, unknown> {
|
|||
|
||||
/**
|
||||
* #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<StalenessInfo> {
|
||||
const now = Date.now();
|
||||
|
|
|
|||
|
|
@ -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';
|
||||
|
|
|
|||
|
|
@ -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 <old>..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) => ({
|
||||
|
|
|
|||
|
|
@ -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'),
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<typeof vi.fn>;
|
||||
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) ─────────────────────────────────────
|
||||
|
|
|
|||
33
gitnexus/test/unit/cli-group-status-format.test.ts
Normal file
33
gitnexus/test/unit/cli-group-status-format.test.ts
Normal file
|
|
@ -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 ');
|
||||
});
|
||||
});
|
||||
|
|
@ -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<GroupRepoHandle> => ({
|
||||
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();
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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"');
|
||||
}
|
||||
});
|
||||
|
||||
|
|
|
|||
100
gitnexus/test/unit/staleness-fallback.test.ts
Normal file
100
gitnexus/test/unit/staleness-fallback.test.ts
Normal file
|
|
@ -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 `<sha>..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<typeof import('node:child_process')>();
|
||||
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]);
|
||||
});
|
||||
});
|
||||
}
|
||||
});
|
||||
48
gitnexus/test/unit/staleness-timeout.test.ts
Normal file
48
gitnexus/test/unit/staleness-timeout.test.ts
Normal file
|
|
@ -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<typeof import('node:child_process')>();
|
||||
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`]]);
|
||||
});
|
||||
});
|
||||
|
|
@ -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<StalenessInfo>
|
||||
> = {
|
||||
checkStaleness: async (repoPath, lastCommit) => checkStaleness(repoPath, lastCommit),
|
||||
checkStalenessAsync,
|
||||
};
|
||||
|
||||
describe('staleness status (#3256)', () => {
|
||||
let root: string;
|
||||
let fixture: ReturnType<typeof makeRepo>;
|
||||
|
||||
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,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<string, unknown> };
|
||||
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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue