From aea6df3da75bdffa83772d4d66e0062f3bda6d14 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Thu, 23 Jul 2026 12:02:36 -0700 Subject: [PATCH] Revert "fix(onboard): stop repeating the same auto-remediation within a run (#2854)" This reverts commit 054badbe60d0c27d9c950ea5a70d2e38ec4e606e. --- src/core/remediation/run.ts | 17 ++----- test/remediation-run-loop.test.ts | 79 ------------------------------- 2 files changed, 5 insertions(+), 91 deletions(-) delete mode 100644 test/remediation-run-loop.test.ts diff --git a/src/core/remediation/run.ts b/src/core/remediation/run.ts index 1511a7169..78f55d44b 100644 --- a/src/core/remediation/run.ts +++ b/src/core/remediation/run.ts @@ -182,7 +182,6 @@ export async function runRemediation( // Real submission path const submitted: StepResult[] = []; const abortedIds = new Set(); - const attemptedIds = new Set(); const doctorRunId = crypto.randomUUID(); const { MinionQueue } = await import('../minions/queue.ts'); @@ -232,7 +231,6 @@ export async function runRemediation( if (completedFromCheckpoint.has(step.id)) { const result: StepResult = { step: stepCount, id: step.id, job_id: null, status: 'completed' }; submitted.push(result); - attemptedIds.add(step.id); hooks.onStepEnd?.(result); recs.shift(); continue; @@ -243,7 +241,6 @@ export async function runRemediation( const result: StepResult = { step: stepCount, id: step.id, job_id: null, status: 'skipped_dep_aborted' }; submitted.push(result); abortedIds.add(step.id); - attemptedIds.add(step.id); hooks.onStepEnd?.(result); recs.shift(); continue; @@ -302,17 +299,13 @@ export async function runRemediation( hooks.onStepEnd?.(errResult); } - attemptedIds.add(step.id); recs.shift(); // D7: scoped recheck — re-compute plan from fresh health snapshot. - // Queue-level max_attempts handles retries within a submitted attempt. - // A stuck health signal regenerates the same stable id, so keep ids this - // run already attempted out of the refreshed list to avoid re-enqueueing - // them forever. + // The next plan may drop completed steps and re-introduce failed + // steps with bumped retry suffix (D1). if (recs.length === 0 || stepCount >= maxJobs) break; const freshHealth = await engine.getHealth(); - recs = computeRecommendations(freshHealth, ctx) - .filter((r) => r.status === 'remediable' && !attemptedIds.has(r.id)); + recs = computeRecommendations(freshHealth, ctx).filter((r) => r.status === 'remediable'); } }; @@ -329,8 +322,8 @@ export async function runRemediation( } // Clear checkpoint on a clean run (no budget abort). Failed steps in the - // submitted set don't disqualify cleanup; an uncleared health signal can - // produce the same stable id again in a later run. + // submitted set don't disqualify the cleanup — they re-surface on the + // next plan with bumped suffixes. if (!budgetAbort) { clearRemediationCheckpoint(planHash); } diff --git a/test/remediation-run-loop.test.ts b/test/remediation-run-loop.test.ts deleted file mode 100644 index 235a2f403..000000000 --- a/test/remediation-run-loop.test.ts +++ /dev/null @@ -1,79 +0,0 @@ -import { describe, expect, mock, test } from 'bun:test'; -import type { BrainEngine } from '../src/core/engine.ts'; -import type { BrainHealth } from '../src/core/types.ts'; - -const attemptedJobs: string[] = []; - -mock.module('../src/core/minions/queue.ts', () => ({ - MinionQueue: class { - async add(name: string) { - attemptedJobs.push(name); - return { id: attemptedJobs.length }; - } - }, -})); - -mock.module('../src/core/minions/wait-for-completion.ts', () => ({ - waitForCompletion: async (_queue: unknown, jobId: number) => ({ - id: jobId, - status: 'completed', - }), -})); - -mock.module('../src/core/remediation-checkpoint.ts', () => ({ - computePlanHash: (ids: string[]) => [...ids].sort().join('|'), - saveRemediationCheckpoint: () => undefined, - loadRemediationCheckpoint: () => null, - listRemediationCheckpoints: () => [], - clearRemediationCheckpoint: () => undefined, -})); - -mock.module('../src/core/ai/gateway.ts', () => ({ - getEmbeddingModel: () => 'ollama:nomic-embed-text', - getEmbeddingDimensions: () => 768, - withBudgetTracker: async (_tracker: unknown, fn: () => Promise) => fn(), -})); - -const { runRemediation } = await import('../src/core/remediation/run.ts'); - -function makeHealth(): BrainHealth { - return { - page_count: 100, - embed_coverage: 1, - stale_pages: 1, - orphan_pages: 0, - missing_embeddings: 0, - brain_score: 80, - dead_links: 1, - link_coverage: 1, - timeline_coverage: 1, - most_connected: [], - embed_coverage_score: 35, - link_density_score: 25, - timeline_coverage_score: 15, - no_orphans_score: 15, - no_dead_links_score: 0, - }; -} - -describe('runRemediation recheck loop guard', () => { - test('attempts a stable stuck remediation once and continues to later work', async () => { - attemptedJobs.length = 0; - const health = makeHealth(); - const engine = { - kind: 'postgres', - getHealth: async () => health, - getConfig: async (key: string) => key === 'sync.repo_path' ? '/brain' : null, - } as BrainEngine; - - const result = await runRemediation(engine, { maxJobs: 4 }); - - expect(attemptedJobs.filter((name) => name === 'backlinks')).toHaveLength(1); - expect(attemptedJobs).toEqual(['backlinks', 'sync', 'extract']); - expect(result.submitted.map((step) => step.id)).toEqual([ - 'backlinks.fix', - 'sync.repo', - 'extract.all', - ]); - }); -});