diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 3a3efbf50..1b514e18c 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -40,7 +40,7 @@ import { buildBasenameIndex, queryBasenameIndex, } from '../core/link-extraction.ts'; -import { isSourceUnchangedSinceSync } from '../core/git-head.ts'; +import { probeSourceGitState } from '../core/git-head.ts'; // v0.41.32.0: remote staleness reads the stored newest_content_at column via // this pure comparator (no git subprocess on the HTTP MCP doctor path). import { lagFromContentMs } from '../core/source-health.ts'; @@ -4062,29 +4062,51 @@ export async function checkSyncFreshness( // All four must hold; otherwise fall through to the time-based check. // The chunker version match is computed here (not in the helper) // because it depends on engine state, not git state. + // + // Clone-unavailable fallback: on stateless deploys (Docker on EB / + // K8s / Fly — the platforms the cloud recipes produce), a container + // restart wipes `local_path` and each clone is only re-materialized + // when that source's next sync job runs. Until then the HEAD probe + // cannot run at all ('unavailable'), which previously fell through to + // raw wall-clock age — and since a no-op sync doesn't advance + // `last_sync_at`, every QUIET source read as stale/FAIL after a + // restart (score-sinking alert storm; observed live: 16-source brain, + // 12 clones gone after a config-update restart, doctor 70→30). + // 'unavailable' + chunker match now reuses the v0.41.32.0 REMOTE lag + // signal (newest_content_at) below — DB-only, no subprocess, and it + // still reports staleness whenever content really is newer than the + // last sync. 'changed' (readable clone with real work) keeps + // wall-clock exactly as before, and a chunker mismatch is never + // masked (D7): it disables the fallback too. + let cloneUnavailable = false; if (localOnly) { - const gitUnchanged = isSourceUnchangedSinceSync( + const gitState = probeSourceGitState( source.local_path, source.last_commit, { requireCleanWorkingTree: 'ignore-untracked' }, ); const chunkerMatch = source.chunker_version === currentChunkerVersion; - if (gitUnchanged && chunkerMatch) { + if (gitState === 'unchanged' && chunkerMatch) { unchanged_count++; continue; } + cloneUnavailable = gitState === 'unavailable' && chunkerMatch; } // v0.41.32.0: REMOTE path (doctorReportRemote, !localOnly) computes lag // from the stored newest_content_at column — NO git subprocess on a // DB-supplied local_path (preserves the v0.41.27.0 trust boundary). A // quiet repo whose newest commit predates its last sync reports 0; NULL - // column → wall-clock fallback. LOCAL fall-through keeps wall-clock: the - // short-circuit already failed, so the source genuinely has work and - // "hours since last sync" is the right staleness measure. The `ageMs < 0` - // skew check above still runs on raw wall-clock for both paths (A1). + // column → wall-clock fallback. LOCAL fall-through keeps wall-clock when + // the clone is READABLE: the short-circuit failed on real evidence + // (HEAD moved / dirty tree), so the source genuinely has work and + // "hours since last sync" is the right staleness measure. A local clone + // that is UNAVAILABLE (not yet re-materialized, see above) carries no + // evidence either way, so it borrows this same DB-only lag. The + // `ageMs < 0` skew check above still runs on raw wall-clock for both + // paths (A1). let thresholdAgeMs = ageMs; - if (!localOnly) { + if (!localOnly || cloneUnavailable) { const contentMs = source.newest_content_at ? new Date(source.newest_content_at).getTime() : null; diff --git a/src/core/git-head.ts b/src/core/git-head.ts index 94955e79a..4dd61fae8 100644 --- a/src/core/git-head.ts +++ b/src/core/git-head.ts @@ -96,29 +96,71 @@ export interface GitFreshnessOpts { } /** - * Returns true iff `localPath` is a git repo whose current HEAD matches - * `lastCommit`, AND (when `requireCleanWorkingTree`) the working tree - * is clean. + * Three-state git probe verdict for a federated source clone. + * + * - `'unchanged'`: HEAD matches `last_commit` (and, when requested, the + * working tree is clean). Sync has nothing to do. + * - `'changed'`: the clone is readable but HEAD moved, the tree is + * dirty, or the DB never recorded a `last_commit` — + * sync genuinely has (or may have) work. + * - `'unavailable'`: the HEAD probe itself could not run — the clone + * directory is missing, not a git repo, or git errored. + * On stateless deploys (containers on EB / K8s / Fly, + * where `local_path` dies with the filesystem and is + * lazily re-materialized by the next per-source sync) + * this is a NORMAL steady state for quiet sources, not + * evidence of pending work. Callers can fall back to a + * DB-only freshness signal instead of wall-clock age. + */ +export type SourceGitState = 'unchanged' | 'changed' | 'unavailable'; + +/** + * Probe a source clone and classify it (see `SourceGitState`). * * This is NOT a full mirror of `gbrain sync`'s "do work?" predicate. * Chunker-version match is computed by the caller because it depends on * engine state (`sources.chunker_version` vs `CURRENT_CHUNKER_VERSION`). * See `src/commands/doctor.ts:checkSyncFreshness` for the AND * combination at the call site. + * + * NULL-input guard stays first: a NULL `last_commit` (legacy row) returns + * `'changed'` WITHOUT running the head probe — same short-circuit contract + * `isSourceUnchangedSinceSync` always had (pinned by doctor.test.ts case 4). + */ +export function probeSourceGitState( + localPath: string | null | undefined, + lastCommit: string | null | undefined, + opts?: GitFreshnessOpts, +): SourceGitState { + if (!localPath || !lastCommit) return 'changed'; + const head = _headProbe(localPath); + if (head === null) return 'unavailable'; + if (head !== lastCommit) return 'changed'; + if (opts?.requireCleanWorkingTree) { + const ignoreUntracked = opts.requireCleanWorkingTree === 'ignore-untracked'; + const isClean = _cleanProbe(localPath, ignoreUntracked); + // null (probe error) AND false (known dirty) both fail the gate. A clean + // probe error with a READABLE head is not classified 'unavailable' — + // fail toward "may have work" so the gate can only relax, never mask. + if (isClean !== true) return 'changed'; + } + return 'unchanged'; +} + +/** + * Returns true iff `localPath` is a git repo whose current HEAD matches + * `lastCommit`, AND (when `requireCleanWorkingTree`) the working tree + * is clean. + * + * Boolean façade over `probeSourceGitState` — `'unavailable'` and + * `'changed'` both collapse to `false`, preserving the v0.41.27.0 + * fail-open contract for callers that only care about the short-circuit + * (`src/core/source-health.ts`). */ export function isSourceUnchangedSinceSync( localPath: string | null | undefined, lastCommit: string | null | undefined, opts?: GitFreshnessOpts, ): boolean { - if (!localPath || !lastCommit) return false; - const head = _headProbe(localPath); - if (head === null || head !== lastCommit) return false; - if (opts?.requireCleanWorkingTree) { - const ignoreUntracked = opts.requireCleanWorkingTree === 'ignore-untracked'; - const isClean = _cleanProbe(localPath, ignoreUntracked); - // null (probe error) AND false (known dirty) both fail the gate. - if (isClean !== true) return false; - } - return true; + return probeSourceGitState(localPath, lastCommit, opts) === 'unchanged'; } diff --git a/test/core/git-head.test.ts b/test/core/git-head.test.ts index a84251143..763e66432 100644 --- a/test/core/git-head.test.ts +++ b/test/core/git-head.test.ts @@ -12,6 +12,7 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { isSourceUnchangedSinceSync, + probeSourceGitState, _setGitHeadProbeForTests, _setGitCleanProbeForTests, type GitHeadProbe, @@ -176,3 +177,60 @@ describe('isSourceUnchangedSinceSync — requireCleanWorkingTree (D7)', () => { expect(cleanCalls).toBe(0); }); }); + +describe('probeSourceGitState — three-state verdict', () => { + test('state 1: HEAD matches + clean → unchanged', () => { + _setGitHeadProbeForTests(() => 'abc123'); + _setGitCleanProbeForTests(() => true); + expect(probeSourceGitState('/tmp/repo', 'abc123', { requireCleanWorkingTree: 'ignore-untracked' })) + .toBe('unchanged'); + }); + + test('state 2: HEAD probe null (clone missing / not a repo / git error) → unavailable', () => { + _setGitHeadProbeForTests(() => null); + expect(probeSourceGitState('/tmp/gone', 'abc123')).toBe('unavailable'); + }); + + test('state 3: HEAD mismatch → changed', () => { + _setGitHeadProbeForTests(() => 'def456'); + expect(probeSourceGitState('/tmp/repo', 'abc123')).toBe('changed'); + }); + + test('state 4: dirty tree with readable HEAD → changed (NOT unavailable)', () => { + _setGitHeadProbeForTests(() => 'abc123'); + _setGitCleanProbeForTests(() => false); + expect(probeSourceGitState('/tmp/repo', 'abc123', { requireCleanWorkingTree: true })) + .toBe('changed'); + }); + + test('state 5: clean-probe ERROR with readable HEAD → changed (fail toward work)', () => { + _setGitHeadProbeForTests(() => 'abc123'); + _setGitCleanProbeForTests(() => null); + expect(probeSourceGitState('/tmp/repo', 'abc123', { requireCleanWorkingTree: true })) + .toBe('changed'); + }); + + test('state 6: NULL inputs → changed, head probe never called (case-4 contract)', () => { + let probeCalls = 0; + _setGitHeadProbeForTests(() => { probeCalls++; return 'abc'; }); + expect(probeSourceGitState(null, 'abc')).toBe('changed'); + expect(probeSourceGitState('/tmp/repo', null)).toBe('changed'); + expect(probeSourceGitState('', '')).toBe('changed'); + expect(probeCalls).toBe(0); + }); + + test('state 7: boolean façade parity — isSourceUnchangedSinceSync === (state is unchanged)', () => { + _setGitHeadProbeForTests(() => 'abc123'); + _setGitCleanProbeForTests(() => true); + for (const [path, commit] of [ + ['/tmp/repo', 'abc123'], // unchanged → true + ['/tmp/repo', 'other'], // changed → false + ] as const) { + expect(isSourceUnchangedSinceSync(path, commit)) + .toBe(probeSourceGitState(path, commit) === 'unchanged'); + } + _setGitHeadProbeForTests(() => null); // unavailable → false + expect(isSourceUnchangedSinceSync('/tmp/gone', 'abc123')) + .toBe(probeSourceGitState('/tmp/gone', 'abc123') === 'unchanged'); + }); +}); diff --git a/test/doctor.test.ts b/test/doctor.test.ts index c9adc3e9e..e7614e8a5 100644 --- a/test/doctor.test.ts +++ b/test/doctor.test.ts @@ -1580,3 +1580,174 @@ describe('BUG 4 — in-progress sync via live lock, not stale freshness', () => expect(result.status).toBe('fail'); }); }); + +// ============================================================================ +// sync_freshness — clone-unavailable content-lag fallback (stateless deploys) +// ============================================================================ +// A container restart (Docker on EB / K8s / Fly) wipes federated clones; +// each one is only re-materialized when that source's next sync job runs. +// Until then the LOCAL git short-circuit cannot probe HEAD at all. That is +// not evidence of pending work, so instead of falling through to raw +// wall-clock age (which no-op syncs never advance → false stale/FAIL for +// every quiet source after a restart), the check borrows the REMOTE path's +// newest_content_at lag (v0.41.32.0). Contracts: +// F1: clone unavailable + content at/before last sync → healthy (lag 0). +// F2: clone unavailable + content NEWER than last sync → still stale +// (wall-clock) — real missed work is never masked. +// F3: clone unavailable + NULL newest_content_at → wall-clock fallback +// (pre-migration parity with git short-circuit case 5). +// F4: chunker mismatch disables the fallback (D7 — a pending re-chunk is +// never masked). +// F5: a READABLE clone that failed the short-circuit (HEAD moved) keeps +// wall-clock even when newest_content_at is old — the fallback is +// scoped to 'unavailable' only. +// ============================================================================ +describe('sync_freshness — clone-unavailable content-lag fallback', () => { + function makeStubEngine(rows: any[]): any { + return { executeRaw: async () => rows }; + } + function agoMs(ms: number): Date { return new Date(Date.now() - ms); } + const HOURS = 60 * 60 * 1000; + let currentChunkerVersion: string; + + beforeEach(async () => { + const { _setGitHeadProbeForTests, _setGitCleanProbeForTests } = + await import('../src/core/git-head.ts'); + const { CHUNKER_VERSION } = await import('../src/core/chunkers/code.ts'); + currentChunkerVersion = String(CHUNKER_VERSION); + _setGitHeadProbeForTests(null); + _setGitCleanProbeForTests(null); + }); + + afterAll(async () => { + const { _setGitHeadProbeForTests, _setGitCleanProbeForTests } = + await import('../src/core/git-head.ts'); + _setGitHeadProbeForTests(null); + _setGitCleanProbeForTests(null); + }); + + test('F1: quiet source, clone gone, content predates last sync → ok', async () => { + const { checkSyncFreshness } = await import('../src/commands/doctor.ts'); + const { _setGitHeadProbeForTests, _setGitCleanProbeForTests } = + await import('../src/core/git-head.ts'); + _setGitHeadProbeForTests(() => null); // clone not re-materialized yet + _setGitCleanProbeForTests(() => true); + + const result = await checkSyncFreshness(makeStubEngine([ + { id: 'quiet-docs', name: '', local_path: '/tmp/quiet-docs', + last_sync_at: agoMs(40 * HOURS), + last_commit: 'abc', chunker_version: currentChunkerVersion, + newest_content_at: agoMs(72 * HOURS) }, // content older than last sync + ]), { localOnly: true }); + + expect(result.status).toBe('ok'); + expect(result.details).toEqual({ + unchanged_count: 0, synced_recently_count: 1, stale_count: 0, + }); + }); + + test('F2: clone gone but content NEWER than last sync → warn (real work not masked)', async () => { + const { checkSyncFreshness } = await import('../src/commands/doctor.ts'); + const { _setGitHeadProbeForTests, _setGitCleanProbeForTests } = + await import('../src/core/git-head.ts'); + _setGitHeadProbeForTests(() => null); + _setGitCleanProbeForTests(() => true); + + const result = await checkSyncFreshness(makeStubEngine([ + { id: 'missed-work', name: '', local_path: '/tmp/missed-work', + last_sync_at: agoMs(40 * HOURS), + last_commit: 'abc', chunker_version: currentChunkerVersion, + newest_content_at: agoMs(1 * HOURS) }, // content NEWER than last sync + ]), { localOnly: true }); + + expect(result.status).toBe('warn'); + expect(result.message).toMatch(/40h ago/); + expect(result.details?.stale_count).toBe(1); + }); + + test('F3: clone gone + NULL newest_content_at → wall-clock fallback (warn)', async () => { + const { checkSyncFreshness } = await import('../src/commands/doctor.ts'); + const { _setGitHeadProbeForTests, _setGitCleanProbeForTests } = + await import('../src/core/git-head.ts'); + _setGitHeadProbeForTests(() => null); + _setGitCleanProbeForTests(() => true); + + const result = await checkSyncFreshness(makeStubEngine([ + { id: 'pre-migration', name: '', local_path: '/tmp/pre-migration', + last_sync_at: agoMs(40 * HOURS), + last_commit: 'abc', chunker_version: currentChunkerVersion, + newest_content_at: null }, + ]), { localOnly: true }); + + expect(result.status).toBe('warn'); + expect(result.details?.stale_count).toBe(1); + }); + + test('F4: clone gone + chunker MISMATCH → fallback disabled, wall-clock warn', async () => { + const { checkSyncFreshness } = await import('../src/commands/doctor.ts'); + const { _setGitHeadProbeForTests, _setGitCleanProbeForTests } = + await import('../src/core/git-head.ts'); + _setGitHeadProbeForTests(() => null); + _setGitCleanProbeForTests(() => true); + + const result = await checkSyncFreshness(makeStubEngine([ + { id: 'needs-rechunk', name: '', local_path: '/tmp/needs-rechunk', + last_sync_at: agoMs(40 * HOURS), + last_commit: 'abc', + chunker_version: '0', // STALE — re-chunk pending + newest_content_at: agoMs(72 * HOURS) }, + ]), { localOnly: true }); + + expect(result.status).toBe('warn'); + expect(result.details?.stale_count).toBe(1); + }); + + test('F5: readable clone, HEAD moved → wall-clock even with old content', async () => { + const { checkSyncFreshness } = await import('../src/commands/doctor.ts'); + const { _setGitHeadProbeForTests, _setGitCleanProbeForTests } = + await import('../src/core/git-head.ts'); + _setGitHeadProbeForTests(() => 'NEW-HEAD'); // clone readable, real work + _setGitCleanProbeForTests(() => true); + + const result = await checkSyncFreshness(makeStubEngine([ + { id: 'has-commits', name: '', local_path: '/tmp/has-commits', + last_sync_at: agoMs(40 * HOURS), + last_commit: 'OLD-HEAD', chunker_version: currentChunkerVersion, + newest_content_at: agoMs(72 * HOURS) }, + ]), { localOnly: true }); + + expect(result.status).toBe('warn'); + expect(result.message).toMatch(/40h ago/); + expect(result.details?.stale_count).toBe(1); + }); + + test('F6: three-bucket invariant holds across rescued + unchanged + stale', async () => { + const { checkSyncFreshness } = await import('../src/commands/doctor.ts'); + const { _setGitHeadProbeForTests, _setGitCleanProbeForTests } = + await import('../src/core/git-head.ts'); + _setGitHeadProbeForTests((path) => path === '/tmp/frozen' ? 'frozen-sha' : null); + _setGitCleanProbeForTests(() => true); + + const result = await checkSyncFreshness(makeStubEngine([ + { id: 'frozen', name: '', local_path: '/tmp/frozen', // unchanged bucket + last_sync_at: agoMs(40 * HOURS), + last_commit: 'frozen-sha', chunker_version: currentChunkerVersion, + newest_content_at: agoMs(80 * HOURS) }, + { id: 'rescued', name: '', local_path: '/tmp/rescued', // clone gone, quiet → healthy + last_sync_at: agoMs(40 * HOURS), + last_commit: 'abc', chunker_version: currentChunkerVersion, + newest_content_at: agoMs(80 * HOURS) }, + { id: 'stale', name: '', local_path: '/tmp/stale', // clone gone, content newer → stale + last_sync_at: agoMs(5 * 24 * HOURS), + last_commit: 'def', chunker_version: currentChunkerVersion, + newest_content_at: agoMs(1 * HOURS) }, + ]), { localOnly: true }); + + expect(result.status).toBe('fail'); + expect(result.message).toContain(`'stale'`); + expect(result.message).not.toContain(`'rescued'`); + expect(result.details).toEqual({ + unchanged_count: 1, synced_recently_count: 1, stale_count: 1, + }); + }); +});