From 7bba2bb9e38a6e052941131851bef544fdb89136 Mon Sep 17 00:00:00 2001 From: Sanchal Ranjan <84386862+sanchalr@users.noreply.github.com> Date: Thu, 23 Jul 2026 13:38:10 -0500 Subject: [PATCH] fix(onboard): stop repeating the same auto-remediation within a run (#2854) When the recommendation list is refreshed between remediation steps, a remediation that doesn't clear its own health signal is reintroduced under its stable id and attempted again, indefinitely on long runs. Track attempted recommendation ids for the run and skip re-attempts. Includes a behavioral regression test: a persistently-stuck signal is attempted once, the loop terminates, and other remediations still run. --- src/core/remediation/run.ts | 20 +++++--- test/remediation-run-loop.test.ts | 79 +++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 7 deletions(-) create mode 100644 test/remediation-run-loop.test.ts diff --git a/src/core/remediation/run.ts b/src/core/remediation/run.ts index dfde35afa..cec2b643d 100644 --- a/src/core/remediation/run.ts +++ b/src/core/remediation/run.ts @@ -183,6 +183,7 @@ 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,6 +233,7 @@ 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; @@ -242,6 +244,7 @@ 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; @@ -300,19 +303,22 @@ export async function runRemediation( hooks.onStepEnd?.(errResult); } + attemptedIds.add(step.id); recs.shift(); // D7: scoped recheck — re-compute plan from fresh health snapshot. - // The next plan may drop completed steps and re-introduce failed - // steps with bumped retry suffix (D1). + // 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. if (recs.length === 0 || stepCount >= maxJobs) break; const freshHealth = await engine.getHealth(); // Extras carry a static status:'remediable' — a fresh health snapshot // never ages them out the way health-derived steps drop. Filter out // ids this run already processed (any terminal status), or the recheck // would resubmit completed extras every iteration, forever. - const processedIds = new Set(submitted.map((s) => s.id)); - const pendingExtras = extraRemediations.filter((r) => !processedIds.has(r.id)); - recs = computeRecommendations(freshHealth, ctx, pendingExtras).filter((r) => r.status === 'remediable'); + const pendingExtras = extraRemediations.filter((r) => !attemptedIds.has(r.id)); + recs = computeRecommendations(freshHealth, ctx, pendingExtras) + .filter((r) => r.status === 'remediable' && !attemptedIds.has(r.id)); } }; @@ -329,8 +335,8 @@ export async function runRemediation( } // Clear checkpoint on a clean run (no budget abort). Failed steps in the - // submitted set don't disqualify the cleanup — they re-surface on the - // next plan with bumped suffixes. + // submitted set don't disqualify cleanup; an uncleared health signal can + // produce the same stable id again in a later run. if (!budgetAbort) { clearRemediationCheckpoint(planHash); } diff --git a/test/remediation-run-loop.test.ts b/test/remediation-run-loop.test.ts new file mode 100644 index 000000000..235a2f403 --- /dev/null +++ b/test/remediation-run-loop.test.ts @@ -0,0 +1,79 @@ +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', + ]); + }); +});