From 609e63bb9df361f2cab0b369c970b996180e0bd3 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sun, 7 Jun 2026 08:45:06 -0700 Subject: [PATCH] fix(doctor): sync_failures severity via one shared decision on both surfaces (#1939) Local buildChecks and remote doctorReportRemote now both route through decideSyncFailureSeverity, so a stuck bookmark escalates WARN -> FAIL consistently (oldest-open age > fail cadence, or large unresolved count), auto-skipped pages stay visible (WARN, not hidden), and the acknowledged/acknowledged_at field-split that caused drift is gone. The remote surface stays subprocess-free (file read + Date.parse only). --- src/commands/doctor.ts | 71 ++++++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 34 deletions(-) diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index d4621a4d7..d8346cd82 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -569,29 +569,24 @@ export async function doctorReportRemote(engine: BrainEngine): Promise l.trim()); - for (const line of lines) { - try { - const entry = JSON.parse(line) as { acknowledged_at?: string | null }; - if (!entry.acknowledged_at) unacked++; - } catch { /* skip malformed line */ } - } - } - checks.push({ - name: 'sync_failures', - status: unacked === 0 ? 'ok' : 'warn', - message: unacked === 0 - ? 'No unacked failures' - : `${unacked} unacked failure(s) — run \`gbrain sync --skip-failed\` on the host to acknowledge`, - }); + const { loadSyncFailures, decideSyncFailureSeverity } = await import('../core/sync.ts'); + const entries = loadSyncFailures(); + const failHours = _resolveSyncFreshnessHours('GBRAIN_SYNC_FRESHNESS_FAIL_HOURS', 72); + const sev = decideSyncFailureSeverity({ entries, nowMs: Date.now(), failHours }); + const msg = + sev.unresolved === 0 + ? 'No unresolved sync failures' + : `${sev.unresolved} unresolved sync failure(s)` + + (sev.auto_skipped > 0 ? ` (${sev.auto_skipped} auto-skipped — pages NOT indexed)` : '') + + ` — run \`gbrain sync --skip-failed\` on the host to acknowledge`; + checks.push({ name: 'sync_failures', status: sev.status, message: msg }); } catch { checks.push({ name: 'sync_failures', status: 'ok', message: 'No failures recorded' }); } @@ -4326,19 +4321,25 @@ export async function buildChecks( // Without this doctor check, users see "sync blocked" and have no // surface showing which files to fix. try { - const { unacknowledgedSyncFailures, loadSyncFailures, summarizeFailuresByCode } = await import('../core/sync.ts'); - const unacked = unacknowledgedSyncFailures(); + const { unacknowledgedSyncFailures, loadSyncFailures, summarizeFailuresByCode, decideSyncFailureSeverity } = await import('../core/sync.ts'); const all = loadSyncFailures(); - if (unacked.length > 0) { - const codeSummary = summarizeFailuresByCode(unacked); + // issue #1939: "unresolved" = open + auto_skipped. Severity (ok/warn/fail) + // comes from the SAME shared decision the remote surface uses, so a stuck + // bookmark blocked past the fail cadence (or a large unresolved count) + // escalates to FAIL instead of staying a quiet WARN forever. + const unresolved = unacknowledgedSyncFailures(); + if (unresolved.length > 0) { + const failHours = _resolveSyncFreshnessHours('GBRAIN_SYNC_FRESHNESS_FAIL_HOURS', 72); + const sev = decideSyncFailureSeverity({ entries: all, nowMs: Date.now(), failHours }); + const codeSummary = summarizeFailuresByCode(unresolved); const codeBreakdown = codeSummary.map(s => `${s.code}=${s.count}`).join(', '); - const preview = unacked.slice(0, 3).map(f => `${f.path} (${f.error.slice(0, 60)})`).join('; '); + const preview = unresolved.slice(0, 3).map(f => `${f.path} (${f.error.slice(0, 60)})`).join('; '); // v0.40.3.0 T8b (D8 + D12 Bug 3): emit a single sync-retry-failed // step. sync-skip-failed is DELIBERATELY NOT emitted as a remediation // — auto-skipping failed syncs hides data loss. Operators can still // run `gbrain sync --skip-failed` manually. const { makeRemediationStep } = await import('../core/remediation-step.ts'); - const oldestTs = unacked.reduce( + const oldestTs = unresolved.reduce( (acc, f) => (acc === '' || f.ts < acc ? f.ts : acc), '', ); @@ -4347,18 +4348,20 @@ export async function buildChecks( job: 'sync-retry-failed', // Content-stable per codex D12 Bug 2: count + oldest_ts captures // the relevant state without using a real timestamp. - params: { failure_count: unacked.length, oldest_failure: oldestTs }, - severity: unacked.length >= 10 ? 'high' : 'medium', + params: { failure_count: unresolved.length, oldest_failure: oldestTs }, + severity: sev.status === 'fail' ? 'high' : 'medium', est_seconds: 30, est_usd_cost: 0, - rationale: `Retry ${unacked.length} unacked sync failure(s) (codes: ${codeBreakdown})`, + rationale: `Retry ${unresolved.length} unresolved sync failure(s) (codes: ${codeBreakdown})`, }); checks.push({ name: 'sync_failures', - status: 'warn', + status: sev.status, message: - `${unacked.length} unacknowledged sync failure(s) [${codeBreakdown}]. ${preview}` + - `${unacked.length > 3 ? `, and ${unacked.length - 3} more` : ''}. ` + + `${unresolved.length} unresolved sync failure(s) [${codeBreakdown}]` + + (sev.auto_skipped > 0 ? ` — ${sev.auto_skipped} auto-skipped (pages NOT indexed)` : '') + + `. ${preview}` + + `${unresolved.length > 3 ? `, and ${unresolved.length - 3} more` : ''}. ` + `Fix the file(s) and re-run 'gbrain sync', or use 'gbrain sync --skip-failed' to acknowledge.`, remediation: [retryStep], remediation_status: 'remediable',