fix(doctor): report actively-running sync via live lock, not stale freshness

A slow source that makes partial progress every cycle but never fully
completes used to read as permanently "stale" / "never synced" because
last_sync_at only advances on a full successful sync. The naive fix
(treat recent checkpoint banking as "in progress") is unsafe: a blocked
sync banks the good files then writes no anchor, so banking can't tell
in-progress from wedged.

Use the only honest signal: a LIVE, non-expired per-source sync lock
(inspectLock + syncLockId against gbrain_cycle_locks). Every non-skipLock
sync holds it and refreshes it; a blocked/failed sync's process has exited
(no lock row) and a wedged holder stops refreshing (TTL lapses), so either
correctly falls through to the stale path and is NEVER masked. An
actively-syncing source (including a never-synced source doing its first
sync) counts as synced_recently, preserving the pinned 3-bucket invariant.
The lock lookup reuses doctor's existing dynamic db-lock import and swallows
any throw (stub engine, pre-lock-table brain) to false, so it can only ADD
an in-progress verdict, never suppress a real stale one.

Tests (real PGLiteEngine + real lock rows): stale+no-lock -> fail;
stale+live-lock -> ok; never-synced+live-lock -> ok; never-synced+no-lock
-> fail; expired-TTL lock -> fail (wedged not masked); blocked source with
banked checkpoint rows but no lock -> still fail.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-06-16 22:55:47 -07:00
co-authored by Claude Opus 4.8
parent c4106e706c
commit a29fb2c53a
2 changed files with 140 additions and 0 deletions
+33
View File
@@ -3394,6 +3394,32 @@ export async function checkSyncFreshness(
let hasWarnings = false;
let hasFailures = false;
// BUG 4 (v0.42.x): a source with a LIVE, non-expired per-source sync lock is
// actively syncing RIGHT NOW — it must not read as stale or never-synced.
// The live lock is the only honest "in progress" signal. Checkpoint banking
// is NOT usable: a blocked sync banks the good files then writes no anchor
// (test/sync-resumable-import.serial.test.ts), so banking can't tell
// in-progress from wedged. A blocked/failed sync's process has exited (no
// lock row) and a wedged holder stops refreshing (TTL lapses), so either
// correctly falls through to the stale path and is NEVER masked. Same
// dynamic import as the stale_locks check; any throw (stub engine in unit
// tests, pre-lock-table brain) is swallowed to false, so this can only ADD
// an in-progress verdict, never suppress a real stale one.
let isActivelySyncing: (sourceId: string) => Promise<boolean> = async () => false;
try {
const { inspectLock, syncLockId } = await import('../core/db-lock.ts');
isActivelySyncing = async (sourceId: string): Promise<boolean> => {
try {
const snap = await inspectLock(engine, syncLockId(sourceId));
return !!snap && !snap.ttl_expired;
} catch {
return false;
}
};
} catch {
/* db-lock unavailable — skip in-progress detection, staleness stands. */
}
for (const source of sources) {
// Embed source.id in user-visible messages so `gbrain sync --source <id>`
// matches what the user copy-pastes. Show display name in parens when set.
@@ -3401,6 +3427,13 @@ export async function checkSyncFreshness(
? `'${source.id}' (${source.name})`
: `'${source.id}'`;
// BUG 4: actively syncing (live lock) → healthy, count as synced_recently
// and skip the staleness checks. Keeps the 3-bucket invariant intact.
if (await isActivelySyncing(source.id)) {
synced_recently_count++;
continue;
}
if (!source.last_sync_at) {
issues.push(`Source ${display} has never been synced`);
hasFailures = true;
+107
View File
@@ -1330,3 +1330,110 @@ describe('v0.42 (#1699) — quarantined_pages + flagged_pages checks', () => {
expect(source).toMatch(/name: 'flagged_pages'/);
});
});
// ============================================================================
// BUG 4 (v0.42.x) — doctor reports an actively-running sync via the live lock,
// not stale freshness. Uses a REAL PGLiteEngine so inspectLock/syncLockId run
// against actual gbrain_cycle_locks rows (the stub engine can't model a lock).
// ============================================================================
describe('BUG 4 — in-progress sync via live lock, not stale freshness', () => {
let engine: any;
let syncLockId: (s: string) => string;
beforeAll(async () => {
const { PGLiteEngine } = await import('../src/core/pglite-engine.ts');
({ syncLockId } = await import('../src/core/db-lock.ts'));
engine = new PGLiteEngine();
await engine.connect({});
await engine.initSchema();
});
afterAll(async () => {
await engine.disconnect();
});
beforeEach(async () => {
const { resetPgliteState } = await import('./helpers/reset-pglite.ts');
await resetPgliteState(engine);
await engine.executeRaw(`DELETE FROM gbrain_cycle_locks`);
});
const staleDate = () => new Date(Date.now() - 5 * 24 * 60 * 60 * 1000); // 5d ago
async function addSource(id: string, lastSyncAt: Date | null) {
await engine.executeRaw(
`INSERT INTO sources (id, name, local_path, last_sync_at, config)
VALUES ($1, $1, $2, $3, '{"federated":true}'::jsonb)
ON CONFLICT (id) DO UPDATE SET last_sync_at = EXCLUDED.last_sync_at`,
[id, `/tmp/${id}`, lastSyncAt],
);
}
// ttlMinutes > 0 → live lock; <= 0 → already-expired (wedged) holder.
async function holdLock(sourceId: string, ttlMinutes: number) {
await engine.executeRaw(
`INSERT INTO gbrain_cycle_locks (id, holder_pid, holder_host, acquired_at, ttl_expires_at, last_refreshed_at)
VALUES ($1, 4242, 'testhost', now(), now() + ($2 || ' minutes')::interval, now())`,
[syncLockId(sourceId), String(ttlMinutes)],
);
}
test('stale source with NO live lock → fail (blocked/wedged is not masked)', async () => {
const { checkSyncFreshness } = await import('../src/commands/doctor.ts');
await addSource('wiki', staleDate());
const result = await checkSyncFreshness(engine);
expect(result.status).toBe('fail');
expect(result.message).toContain(`'wiki'`);
});
test('stale source WITH a live (non-expired) lock → ok (sync in progress)', async () => {
const { checkSyncFreshness } = await import('../src/commands/doctor.ts');
await addSource('wiki', staleDate());
await holdLock('wiki', 30);
const result = await checkSyncFreshness(engine);
expect(result.status).toBe('ok');
expect(result.details?.synced_recently_count).toBe(1);
expect(result.details?.stale_count).toBe(0);
});
test('never-synced source WITH a live lock → ok (initial sync in progress)', async () => {
const { checkSyncFreshness } = await import('../src/commands/doctor.ts');
await addSource('wiki', null);
await holdLock('wiki', 30);
const result = await checkSyncFreshness(engine);
expect(result.status).toBe('ok');
expect(result.details?.synced_recently_count).toBe(1);
});
test('never-synced source with NO lock → fail (unchanged behavior)', async () => {
const { checkSyncFreshness } = await import('../src/commands/doctor.ts');
await addSource('wiki', null);
const result = await checkSyncFreshness(engine);
expect(result.status).toBe('fail');
expect(result.message).toContain('never been synced');
});
test('expired-TTL lock does NOT mask staleness (wedged-but-not-refreshing holder)', async () => {
const { checkSyncFreshness } = await import('../src/commands/doctor.ts');
await addSource('wiki', staleDate());
await holdLock('wiki', -5); // ttl_expires_at 5 min in the past
const result = await checkSyncFreshness(engine);
expect(result.status).toBe('fail');
});
test('blocked source with banked checkpoint rows but NO live lock → still fail', async () => {
const { checkSyncFreshness } = await import('../src/commands/doctor.ts');
await addSource('wiki', staleDate());
// A blocked sync banks the good files then exits without an anchor and
// without a held lock. Banking must NOT be read as "in progress".
await engine.executeRaw(
`INSERT INTO op_checkpoints (op, fingerprint, completed_keys, updated_at)
VALUES ('sync', 'fp-blocked', '[]'::jsonb, now())`,
);
await engine.executeRaw(
`INSERT INTO op_checkpoint_paths (op, fingerprint, path) VALUES ('sync', 'fp-blocked', 'banked.md')`,
);
const result = await checkSyncFreshness(engine);
expect(result.status).toBe('fail');
});
});