mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-06 02:49:56 +00:00
fix(staleness): detect rollback with one Git query (#3445)
This commit is contained in:
parent
668fac7635
commit
4f298d0ac0
11 changed files with 453 additions and 54 deletions
|
|
@ -4,16 +4,18 @@
|
|||
* against a real group, which is how `STALE (-1 commits behind)` went unnoticed
|
||||
* (#3256).
|
||||
*/
|
||||
import type { StalenessStatus } from '../core/staleness-status.js';
|
||||
|
||||
/** The fields of a `groupStatus` repo row the index column reads. */
|
||||
export interface GroupRepoIndexRow {
|
||||
indexStale: boolean;
|
||||
commitsBehind?: number;
|
||||
status?: StalenessStatus;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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 `?`.
|
||||
* The index column of a `group status` row. Diverged indexes differ from HEAD
|
||||
* without a forward commit count; an unavailable count renders as `?`.
|
||||
*
|
||||
* `group/service.ts` has always reported a repo with no recorded commit as
|
||||
* `{ indexStale: true, commitsBehind: -1 }`. The previous `?? '?'` fallback
|
||||
|
|
@ -21,6 +23,7 @@ export interface GroupRepoIndexRow {
|
|||
*/
|
||||
export const formatIndexStatusCell = (row: GroupRepoIndexRow): string => {
|
||||
if (!row.indexStale) return 'OK ';
|
||||
if (row.status === 'diverged') return 'STALE (index differs from HEAD)';
|
||||
const n = row.commitsBehind;
|
||||
const count = typeof n === 'number' && n >= 0 ? String(n) : '?';
|
||||
return `STALE (${count} commits behind)`;
|
||||
|
|
|
|||
|
|
@ -17,8 +17,8 @@ export type { StalenessInfo, StalenessStatus } from './staleness-status.js';
|
|||
const execFileAsync = promisify(execFile);
|
||||
|
||||
/**
|
||||
* Ceiling for one `git rev-list` staleness probe. Generous for the local
|
||||
* history walk this is, and short enough that an unresponsive working tree
|
||||
* Per-command ceiling for async `git rev-list` and both HEAD probes.
|
||||
* Generous for local Git queries, and short enough that an unresponsive working tree
|
||||
* degrades to "not stale" quickly rather than holding a request open.
|
||||
*/
|
||||
const STALENESS_TIMEOUT_MS = 5_000;
|
||||
|
|
@ -33,12 +33,23 @@ const behindHint = (n: number): string =>
|
|||
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.";
|
||||
|
||||
// `rev-list --count lastCommit..HEAD` answering 0 does NOT mean "HEAD is the
|
||||
// indexed commit" — it means "HEAD has no commits lastCommit lacks", which is
|
||||
// also true when HEAD is an *ancestor* of lastCommit (the working tree checked
|
||||
// out an older commit than the one indexed, or switched to a line of history
|
||||
// behind it). That read a rollback as `current` until this hint existed.
|
||||
const REGRESSED_HINT =
|
||||
'⚠️ Index is not at HEAD — the indexed commit is not reachable from the checked-out commit (the working tree may have checked out an older commit, or a different line of 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' };
|
||||
// Called only once a positive HEAD-only count is in hand.
|
||||
const behind = (commitsBehind: number): StalenessInfo => ({
|
||||
isStale: true,
|
||||
commitsBehind,
|
||||
hint: behindHint(commitsBehind),
|
||||
status: 'behind',
|
||||
});
|
||||
|
||||
/**
|
||||
* `rev-list` could not answer. Asking for HEAD alone needs no history walk and
|
||||
|
|
@ -53,6 +64,26 @@ const fromHead = (head: string | null, lastCommit: string): StalenessInfo => {
|
|||
return { isStale: false, commitsBehind: 0, hint: DIVERGED_HINT, status: 'diverged' };
|
||||
};
|
||||
|
||||
/**
|
||||
* `rev-list --left-right --count lastCommit...HEAD` measures both sides in
|
||||
* one process, so a later HEAD change cannot mix two snapshots. The left
|
||||
* count identifies a rollback even when the HEAD-only (right) count is 0.
|
||||
* Positive right counts keep the existing `behind` behavior, including when
|
||||
* both sides have commits (divergent branches or a re-shallowed clone).
|
||||
*/
|
||||
const fromCounts = (output: string): StalenessInfo => {
|
||||
const counts = /^(\d+)\s+(\d+)$/.exec(output.trim());
|
||||
if (!counts) return unknown();
|
||||
const indexedOnly = Number(counts[1]);
|
||||
const headOnly = Number(counts[2]);
|
||||
if (!Number.isSafeInteger(indexedOnly) || !Number.isSafeInteger(headOnly)) return unknown();
|
||||
if (headOnly > 0) return behind(headOnly);
|
||||
if (indexedOnly > 0) {
|
||||
return { isStale: true, commitsBehind: 0, hint: REGRESSED_HINT, status: 'diverged' };
|
||||
}
|
||||
return { isStale: false, commitsBehind: 0, status: 'current' };
|
||||
};
|
||||
|
||||
const readHeadSync = (repoPath: string): string | null => {
|
||||
try {
|
||||
return (
|
||||
|
|
@ -61,6 +92,7 @@ const readHeadSync = (repoPath: string): string | null => {
|
|||
encoding: 'utf-8',
|
||||
stdio: ['pipe', 'pipe', 'pipe'],
|
||||
windowsHide: true,
|
||||
timeout: STALENESS_TIMEOUT_MS,
|
||||
}).trim() || null
|
||||
);
|
||||
} catch {
|
||||
|
|
@ -89,14 +121,18 @@ export function checkStaleness(repoPath: string, lastCommit: string): StalenessI
|
|||
// 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,
|
||||
encoding: 'utf-8',
|
||||
stdio: ['pipe', 'pipe', 'pipe'],
|
||||
windowsHide: true,
|
||||
}).trim();
|
||||
const result = execFileSync(
|
||||
'git',
|
||||
['rev-list', '--left-right', '--count', `${lastCommit}...HEAD`],
|
||||
{
|
||||
cwd: repoPath,
|
||||
encoding: 'utf-8',
|
||||
stdio: ['pipe', 'pipe', 'pipe'],
|
||||
windowsHide: true,
|
||||
},
|
||||
);
|
||||
|
||||
return fromCount(parseInt(result, 10) || 0);
|
||||
return fromCounts(result);
|
||||
} catch {
|
||||
return fromHead(readHeadSync(repoPath), lastCommit);
|
||||
}
|
||||
|
|
@ -115,21 +151,25 @@ export async function checkStalenessAsync(
|
|||
try {
|
||||
// Note: promisified execFile captures stdout/stderr by default (no stdio option needed,
|
||||
// unlike the sync variant which requires explicit stdio: ['pipe','pipe','pipe']).
|
||||
const { stdout } = await execFileAsync('git', ['rev-list', '--count', `${lastCommit}..HEAD`], {
|
||||
cwd: repoPath,
|
||||
encoding: 'utf-8',
|
||||
windowsHide: true,
|
||||
// The catch below fails closed on every git ERROR, but a hang is not an
|
||||
// error — it is silence, and without a bound this await never settles.
|
||||
// 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, and the catch
|
||||
// below reports it as `unknown` — still the fail-closed `isStale: false`.
|
||||
timeout: STALENESS_TIMEOUT_MS,
|
||||
});
|
||||
const { stdout } = await execFileAsync(
|
||||
'git',
|
||||
['rev-list', '--left-right', '--count', `${lastCommit}...HEAD`],
|
||||
{
|
||||
cwd: repoPath,
|
||||
encoding: 'utf-8',
|
||||
windowsHide: true,
|
||||
// The catch below fails closed on every git ERROR, but a hang is not an
|
||||
// error — it is silence, and without a bound this await never settles.
|
||||
// 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, and the catch
|
||||
// below reports it as `unknown` — still the fail-closed `isStale: false`.
|
||||
timeout: STALENESS_TIMEOUT_MS,
|
||||
},
|
||||
);
|
||||
|
||||
return fromCount(parseInt(stdout.trim(), 10) || 0);
|
||||
return fromCounts(stdout);
|
||||
} 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.
|
||||
|
|
|
|||
|
|
@ -18,21 +18,35 @@
|
|||
* 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.
|
||||
* The successful probe is `rev-list --left-right --count lastCommit...HEAD`:
|
||||
* the left count is indexed-only commits, and the right is HEAD-only commits.
|
||||
* Both counts come from one HEAD snapshot, without a follow-up process.
|
||||
*
|
||||
* `isStale` and `commitsBehind` keep their historical values in every case, so
|
||||
* no existing consumer changes behaviour unless it reads `status`.
|
||||
* - `current` — both counts are 0, or `rev-list` could not answer but a
|
||||
* fallback `rev-parse HEAD` resolved to the indexed commit.
|
||||
* - `behind` — the right count is N > 0; `commitsBehind` is N, including
|
||||
* when the left count is positive too (divergent or shallow history).
|
||||
* - `diverged` — the index is provably not at HEAD, reached two different ways:
|
||||
* - The left count is positive and the right is 0: HEAD is an ancestor
|
||||
* of the indexed commit (#3127). The working tree checked out an older
|
||||
* commit than the one indexed, or a release branch behind the indexed
|
||||
* tip. The mismatch is established: `isStale` is `true` and
|
||||
* `commitsBehind` stays 0 (there is no forward count to report).
|
||||
* - `rev-list` could not answer at all, but HEAD resolved and is not the
|
||||
* indexed commit: 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. This arm keeps the
|
||||
* historical fail-open `isStale: false` (see below).
|
||||
* - `unknown` — the probe could not establish the relationship: no readable
|
||||
* HEAD, a timeout, no recorded commit, or malformed count output.
|
||||
*
|
||||
* `isStale` and `commitsBehind` keep their historical fail-open values
|
||||
* (`false` / `0`) whenever the check could not fully answer — every `unknown`,
|
||||
* and the `rev-list`-failure arm of `diverged` — so no existing consumer
|
||||
* changes behaviour there unless it reads `status`. The other arm of
|
||||
* `diverged` (the confirmed rollback) is a successful, computed
|
||||
* answer rather than a failure, so `isStale` reflects it (`true`) instead.
|
||||
*/
|
||||
export type StalenessStatus = 'current' | 'behind' | 'diverged' | 'unknown';
|
||||
|
||||
|
|
|
|||
|
|
@ -351,7 +351,7 @@ async function getContextResource(backend: LocalBackend, repoName?: string): Pro
|
|||
|
||||
// Check staleness using the current on-disk lastCommit (not the cached handle)
|
||||
const repoPath = repo.repoPath;
|
||||
const lastCommit = freshMeta?.lastCommit ?? repo.lastCommit ?? 'HEAD';
|
||||
const lastCommit = freshMeta?.lastCommit ?? repo.lastCommit ?? '';
|
||||
const staleness = repoPath
|
||||
? checkStaleness(repoPath, lastCommit)
|
||||
: { isStale: false, commitsBehind: 0 };
|
||||
|
|
|
|||
|
|
@ -207,4 +207,26 @@ describe('context resource freshness — out-of-process analyze (#2438)', () =>
|
|||
expect(result).toContain('symbols: 333');
|
||||
expect(result).toContain('processes: 4');
|
||||
});
|
||||
|
||||
it('does not claim divergence when disk metadata and the cached handle have no indexed commit', async function missingIndexedCommitDoesNotClaimDivergence() {
|
||||
writeFileSync(path.join(repoPath, 'a.ts'), 'export const a = 1;\n');
|
||||
runGit(repoPath, 'add', 'a.ts');
|
||||
runGit(repoPath, 'commit', '-m', 'c1');
|
||||
|
||||
// A legacy metadata/registry entry can omit lastCommit despite RepoMeta's
|
||||
// required field; seed that persisted shape rather than a symbolic ref.
|
||||
await seedIndexedRepo(repoPath, storagePath, {
|
||||
repoPath,
|
||||
indexedAt: '2024-01-01T00:00:00Z',
|
||||
stats: { files: 1, nodes: 1, processes: 0 },
|
||||
} as RepoMeta);
|
||||
|
||||
const backend = new LocalBackend();
|
||||
await backend.init();
|
||||
expect((await backend.resolveRepo('test-repo')).lastCommit).toBeUndefined();
|
||||
|
||||
const result = await readResource('gitnexus://repo/test-repo/context', backend);
|
||||
expect(result).not.toContain('staleness:');
|
||||
expect(result).toContain(' commit: ""');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -27,6 +27,29 @@ describe('formatIndexStatusCell', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it.each([0, 3])(
|
||||
'renders a stale diverged row without a commits-behind count of %i',
|
||||
(commitsBehind) => {
|
||||
const row = { indexStale: true, commitsBehind, status: 'diverged' as const };
|
||||
expect(formatIndexStatusCell(row)).toBe('STALE (index differs from HEAD)');
|
||||
},
|
||||
);
|
||||
|
||||
it('renders an explicit behind status with its counted gap', () => {
|
||||
const row = { indexStale: true, commitsBehind: 3, status: 'behind' as const };
|
||||
expect(formatIndexStatusCell(row)).toBe('STALE (3 commits behind)');
|
||||
});
|
||||
|
||||
it('renders an explicit current status as OK', () => {
|
||||
const row = { indexStale: false, commitsBehind: 0, status: 'current' as const };
|
||||
expect(formatIndexStatusCell(row)).toBe('OK ');
|
||||
});
|
||||
|
||||
it('preserves the fail-open cell when a diverged probe did not confirm staleness', () => {
|
||||
const row = { indexStale: false, commitsBehind: 0, status: 'diverged' as const };
|
||||
expect(formatIndexStatusCell(row)).toBe('OK ');
|
||||
});
|
||||
|
||||
it('keeps the OK cell byte-identical, padding included', () => {
|
||||
expect(formatIndexStatusCell({ indexStale: false, commitsBehind: 0 })).toBe('OK ');
|
||||
});
|
||||
|
|
|
|||
|
|
@ -2,13 +2,17 @@ import { describe, it, expect, vi } from 'vitest';
|
|||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import * as os from 'node:os';
|
||||
import { execFileSync } from 'node:child_process';
|
||||
import {
|
||||
GroupService,
|
||||
type GroupToolPort,
|
||||
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 {
|
||||
formatIndexStatusCell,
|
||||
type GroupRepoIndexRow,
|
||||
} 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 } {
|
||||
|
|
@ -717,10 +721,7 @@ repos:
|
|||
|
||||
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 }
|
||||
>;
|
||||
repos: Record<string, GroupRepoIndexRow & { missing: boolean }>;
|
||||
};
|
||||
|
||||
const row = result.repos['app/backend'];
|
||||
|
|
@ -736,5 +737,67 @@ repos:
|
|||
cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
it('renders a real rollback as an index that differs from HEAD', async () => {
|
||||
const { cleanup, tmpDir } = makeTmpGroup();
|
||||
try {
|
||||
vi.stubEnv('GITNEXUS_HOME', tmpDir);
|
||||
const repoPath = path.join(tmpDir, 'rollback');
|
||||
fs.mkdirSync(repoPath);
|
||||
const git = (...args: string[]): string =>
|
||||
execFileSync(
|
||||
'git',
|
||||
[
|
||||
'-c',
|
||||
'user.email=t@example.com',
|
||||
'-c',
|
||||
'user.name=T',
|
||||
'-c',
|
||||
'commit.gpgsign=false',
|
||||
...args,
|
||||
],
|
||||
{ cwd: repoPath, encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'] },
|
||||
).trim();
|
||||
git('init', '-q', '--initial-branch=main');
|
||||
git('commit', '--allow-empty', '-qm', 'first');
|
||||
const firstCommit = git('rev-parse', 'HEAD');
|
||||
git('commit', '--allow-empty', '-qm', 'indexed');
|
||||
const indexedCommit = git('rev-parse', 'HEAD');
|
||||
git('checkout', '-q', '--detach', firstCommit);
|
||||
|
||||
const storagePath = path.join(repoPath, '.gitnexus');
|
||||
fs.mkdirSync(storagePath);
|
||||
fs.writeFileSync(
|
||||
path.join(storagePath, 'gitnexus.json'),
|
||||
JSON.stringify({ lastCommit: indexedCommit, indexedAt: '2026-01-01T00:00:00.000Z' }),
|
||||
);
|
||||
const port = makePort({
|
||||
resolveRepo: vi.fn(
|
||||
async (name?: string): Promise<GroupRepoHandle> => ({
|
||||
id: name || 'test',
|
||||
name: name || 'test',
|
||||
repoPath,
|
||||
storagePath,
|
||||
}),
|
||||
),
|
||||
});
|
||||
|
||||
const svc = new GroupService(port);
|
||||
const result = (await svc.groupStatus({ name: 'test-group' })) as {
|
||||
repos: Record<string, GroupRepoIndexRow & { missing: boolean }>;
|
||||
};
|
||||
const row = result.repos['app/backend'];
|
||||
expect(row).toMatchObject({
|
||||
missing: false,
|
||||
indexStale: true,
|
||||
commitsBehind: 0,
|
||||
status: 'diverged',
|
||||
});
|
||||
expect(formatIndexStatusCell(row)).toBe('STALE (index differs from HEAD)');
|
||||
} finally {
|
||||
vi.unstubAllEnvs();
|
||||
cleanup();
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@
|
|||
* 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
|
||||
* 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
|
||||
|
|
@ -12,14 +12,21 @@
|
|||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
const { plan, spawnedArgs } = vi.hoisted(() => ({
|
||||
plan: { head: null as string | null },
|
||||
plan: { head: null as string | null, enoent: false },
|
||||
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.
|
||||
// answers `plan.head`, or fails when it is null. `plan.enoent` instead fails
|
||||
// every invocation the way Node reports a missing executable (git not on
|
||||
// PATH) — no exit code, `code: 'ENOENT'` — to check that shape of failure is
|
||||
// caught by the same `catch` as a present-but-failing git (#3127).
|
||||
const answer = (args: readonly string[]): { error: Error | null; stdout: string } => {
|
||||
spawnedArgs.push([...args]);
|
||||
if (plan.enoent) {
|
||||
const failure = Object.assign(new Error('spawn git ENOENT'), { code: 'ENOENT' });
|
||||
return { error: failure, stdout: '' };
|
||||
}
|
||||
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,
|
||||
|
|
@ -54,7 +61,7 @@ vi.mock('node:child_process', async (importOriginal) => {
|
|||
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_LIST = ['rev-list', '--left-right', '--count', `${INDEXED_COMMIT}...HEAD`];
|
||||
const REV_PARSE = ['rev-parse', 'HEAD'];
|
||||
|
||||
const bothHelpers = {
|
||||
|
|
@ -65,6 +72,7 @@ const bothHelpers = {
|
|||
describe('staleness after a failed (not timed-out) rev-list (#3256)', () => {
|
||||
beforeEach(() => {
|
||||
plan.head = null;
|
||||
plan.enoent = false;
|
||||
spawnedArgs.length = 0;
|
||||
});
|
||||
|
||||
|
|
@ -95,6 +103,19 @@ describe('staleness after a failed (not timed-out) rev-list (#3256)', () => {
|
|||
expect(result).toEqual({ isStale: false, commitsBehind: 0, status: 'unknown' });
|
||||
expect(spawnedArgs).toEqual([REV_LIST, REV_PARSE]);
|
||||
});
|
||||
|
||||
it('reports unknown — never current — when git itself is not on PATH', async () => {
|
||||
// nikolai-vysotskyi (#3127): "git not on PATH" specifically, as a
|
||||
// distinct failure shape (ENOENT, no exit code) from a present git
|
||||
// that fails against a bad ref. Same requirement either way: it must
|
||||
// never be silently read as fresh.
|
||||
plan.enoent = true;
|
||||
|
||||
const result = await check('/repo', INDEXED_COMMIT);
|
||||
|
||||
expect(result).toEqual({ isStale: false, commitsBehind: 0, status: 'unknown' });
|
||||
expect(spawnedArgs).toEqual([REV_LIST, REV_PARSE]);
|
||||
});
|
||||
});
|
||||
}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -43,6 +43,8 @@ describe('checkStalenessAsync — timed-out rev-list (#3256)', () => {
|
|||
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`]]);
|
||||
expect(spawnedArgs).toEqual([
|
||||
['rev-list', '--left-right', '--count', `${INDEXED_COMMIT}...HEAD`],
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
162
gitnexus/test/unit/staleness-zero-count.test.ts
Normal file
162
gitnexus/test/unit/staleness-zero-count.test.ts
Normal file
|
|
@ -0,0 +1,162 @@
|
|||
/**
|
||||
* Count both sides in one Git process to distinguish freshness from rollback
|
||||
* without mixing HEAD snapshots (#3127).
|
||||
* Keep the child-process mock isolated from tests that use real repositories.
|
||||
*/
|
||||
import type { ExecFileOptions } from 'node:child_process';
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
const { plan, invocations } = vi.hoisted(() => ({
|
||||
plan: { counts: '0\t0\n' as string | null, head: 'failure', advanceHead: false },
|
||||
invocations: [] as { args: string[]; options: ExecFileOptions }[],
|
||||
}));
|
||||
|
||||
const answer = (args: readonly string[], options: ExecFileOptions): string => {
|
||||
invocations.push({ args: [...args], options });
|
||||
if (args[0] === 'rev-list') {
|
||||
if (plan.counts === null) {
|
||||
throw Object.assign(new Error('Command failed: git rev-list'), { code: 128, killed: false });
|
||||
}
|
||||
// Simulate a checkout/commit after Git measured the relationship. A second
|
||||
// process would see this different HEAD and could misclassify the result.
|
||||
if (plan.advanceHead) plan.head = 'b'.repeat(40);
|
||||
return plan.counts;
|
||||
}
|
||||
if (plan.head === 'empty') return ' \n';
|
||||
if (plan.head === 'timeout') {
|
||||
throw Object.assign(new Error('Command failed: git rev-parse HEAD'), {
|
||||
killed: true,
|
||||
signal: 'SIGTERM',
|
||||
});
|
||||
}
|
||||
if (plan.head === 'failure') {
|
||||
throw Object.assign(new Error('Command failed: git rev-parse HEAD'), {
|
||||
code: 128,
|
||||
killed: false,
|
||||
});
|
||||
}
|
||||
return `${plan.head}\n`;
|
||||
};
|
||||
|
||||
vi.mock('node:child_process', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import('node:child_process')>();
|
||||
const { promisify } = await import('node:util');
|
||||
const execFile = (
|
||||
_file: string,
|
||||
args: readonly string[],
|
||||
options: ExecFileOptions,
|
||||
callback: (error: Error | null, stdout: string, stderr: string) => void,
|
||||
): void => {
|
||||
try {
|
||||
callback(null, answer(args, options), '');
|
||||
} catch (error) {
|
||||
callback(error as Error, '', '');
|
||||
}
|
||||
};
|
||||
// Real execFile has a custom promisifier returning both output streams.
|
||||
Object.defineProperty(execFile, promisify.custom, {
|
||||
value: async (_file: string, args: readonly string[], options: ExecFileOptions) => ({
|
||||
stdout: answer(args, options),
|
||||
stderr: '',
|
||||
}),
|
||||
});
|
||||
const execFileSync = (_file: string, args: readonly string[], options: ExecFileOptions): string =>
|
||||
answer(args, options);
|
||||
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', '--left-right', '--count', `${INDEXED_COMMIT}...HEAD`];
|
||||
const REV_PARSE = ['rev-parse', 'HEAD'];
|
||||
|
||||
const bothHelpers = {
|
||||
checkStaleness: async (repo: string, lastCommit: string) => checkStaleness(repo, lastCommit),
|
||||
checkStalenessAsync,
|
||||
};
|
||||
|
||||
describe('staleness from one relationship query (#3127)', () => {
|
||||
beforeEach(() => {
|
||||
plan.counts = '0\t0\n';
|
||||
plan.head = 'failure';
|
||||
plan.advanceHead = false;
|
||||
invocations.length = 0;
|
||||
});
|
||||
|
||||
for (const [name, check] of Object.entries(bothHelpers)) {
|
||||
describe(name, () => {
|
||||
it.each([
|
||||
{ counts: '0\t0\n', status: 'current', isStale: false, commitsBehind: 0 },
|
||||
{ counts: '2\t0\n', status: 'diverged', isStale: true, commitsBehind: 0 },
|
||||
{ counts: '0\t3\n', status: 'behind', isStale: true, commitsBehind: 3 },
|
||||
{ counts: '2\t3\n', status: 'behind', isStale: true, commitsBehind: 3 },
|
||||
])('reports $status from $counts in one process', async ({ counts, ...expected }) => {
|
||||
plan.counts = counts;
|
||||
|
||||
const result = await check('/repo', INDEXED_COMMIT);
|
||||
|
||||
expect(result).toMatchObject(expected);
|
||||
expect(invocations.map(({ args }) => args)).toEqual([REV_LIST]);
|
||||
});
|
||||
|
||||
it('keeps the count snapshot when HEAD advances after the query', async () => {
|
||||
plan.head = INDEXED_COMMIT;
|
||||
plan.advanceHead = true;
|
||||
|
||||
const result = await check('/repo', INDEXED_COMMIT);
|
||||
|
||||
expect(plan.head).not.toBe(INDEXED_COMMIT);
|
||||
expect(result).toEqual({ status: 'current', isStale: false, commitsBehind: 0 });
|
||||
expect(invocations.map(({ args }) => args)).toEqual([REV_LIST]);
|
||||
});
|
||||
|
||||
it.each(['', '0\n', '0\tbogus\n'])(
|
||||
'reports unknown for malformed counts %j',
|
||||
async (counts) => {
|
||||
plan.counts = counts;
|
||||
|
||||
const result = await check('/repo', INDEXED_COMMIT);
|
||||
|
||||
expect(result).toEqual({ status: 'unknown', isStale: false, commitsBehind: 0 });
|
||||
expect(invocations.map(({ args }) => args)).toEqual([REV_LIST]);
|
||||
},
|
||||
);
|
||||
|
||||
it('reports unknown when the follow-up HEAD command fails', async () => {
|
||||
plan.counts = null;
|
||||
const result = await check('/repo', INDEXED_COMMIT);
|
||||
|
||||
expect(result).toEqual({ status: 'unknown', isStale: false, commitsBehind: 0 });
|
||||
expect(invocations.map(({ args }) => args)).toEqual([REV_LIST, REV_PARSE]);
|
||||
});
|
||||
|
||||
it('reports unknown when the follow-up HEAD command returns no commit', async () => {
|
||||
plan.counts = null;
|
||||
plan.head = 'empty';
|
||||
|
||||
const result = await check('/repo', INDEXED_COMMIT);
|
||||
|
||||
expect(result).toEqual({ status: 'unknown', isStale: false, commitsBehind: 0 });
|
||||
expect(invocations.map(({ args }) => args)).toEqual([REV_LIST, REV_PARSE]);
|
||||
});
|
||||
|
||||
it('bounds the HEAD command and reports unknown without retrying after its timeout', async () => {
|
||||
plan.counts = null;
|
||||
plan.head = 'timeout';
|
||||
|
||||
const result = await check('/repo', INDEXED_COMMIT);
|
||||
|
||||
expect(result).toEqual({ status: 'unknown', isStale: false, commitsBehind: 0 });
|
||||
expect(invocations.map(({ args }) => args)).toEqual([REV_LIST, REV_PARSE]);
|
||||
const timeout = invocations[1].options.timeout;
|
||||
expect(Number.isFinite(timeout)).toBe(true);
|
||||
expect(timeout).toBeGreaterThan(0);
|
||||
});
|
||||
});
|
||||
}
|
||||
});
|
||||
|
|
@ -335,3 +335,52 @@ describe('branch-pinned serve clone once git prunes the indexed commit (#3256)',
|
|||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ── #3127: a 0 forward count is not "the index matches this tree" ───────────
|
||||
//
|
||||
// nikolai-vysotskyi (issue #3127, comment on the `--stale-policy` proposal):
|
||||
// `git rev-list --count lastCommit..HEAD` answers "commits reachable from
|
||||
// HEAD but not lastCommit", which is also 0 when HEAD is an *ancestor* of
|
||||
// lastCommit — i.e. the working tree checked out an older commit than the one
|
||||
// indexed, or a release branch behind the indexed tip. Before this fix that
|
||||
// read as `current`; a `--stale-policy error`-style caller would exit 0
|
||||
// exactly when the index is provably wrong about the checked-out tree.
|
||||
describe('staleness when the checkout has regressed behind the indexed commit (#3127)', () => {
|
||||
let root: string;
|
||||
let fixture: ReturnType<typeof makeRepo>;
|
||||
|
||||
beforeAll(() => {
|
||||
root = mkdtempSync(join(tmpdir(), 'gitnexus-staleness-regressed-'));
|
||||
fixture = makeRepo(root, 'repo');
|
||||
// Roll the working tree back to c1. `rev-list --count c3..HEAD` alone
|
||||
// answers 0 here (HEAD/c1 has no commits c3 lacks) — exactly the count a
|
||||
// pre-fix caller read as "index matches HEAD".
|
||||
git(fixture.repo, 'checkout', '-q', fixture.c1);
|
||||
});
|
||||
afterAll(() => removeTree(root));
|
||||
|
||||
for (const [name, check] of Object.entries(bothHelpers)) {
|
||||
describe(name, () => {
|
||||
it('reports diverged, not current, when HEAD is behind the indexed commit', async () => {
|
||||
const result = await check(fixture.repo, fixture.c3);
|
||||
|
||||
expect(result.status).toBe('diverged');
|
||||
// Established by a positive indexed-only count and a zero HEAD-only
|
||||
// count (not a `rev-list` failure), so — unlike the failure-path
|
||||
// `diverged` above — `isStale` reflects the mismatch instead of the
|
||||
// historical fail-open `false`.
|
||||
expect(result.isStale).toBe(true);
|
||||
expect(result.commitsBehind).toBe(0);
|
||||
expect(result.hint).toContain('not reachable from the checked-out commit');
|
||||
});
|
||||
|
||||
it('still reports current for the commit actually checked out', async () => {
|
||||
expect(await check(fixture.repo, fixture.c1)).toMatchObject({
|
||||
status: 'current',
|
||||
isStale: false,
|
||||
commitsBehind: 0,
|
||||
});
|
||||
});
|
||||
});
|
||||
}
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue