From 8afac385b0af5c24b61ee08773d92b5cf1ade85e Mon Sep 17 00:00:00 2001 From: Paul Huliganga Date: Thu, 26 Mar 2026 13:31:06 -0400 Subject: [PATCH] Fix resume test: assert orchestrator checkpoint and retry semantics correctly for mid-run resumes (Task 1 remediation) --- .../services/SequentialOrchestrator.ts | 5 +++- src/backend/tests/orchestrator.test.ts | 27 ++++++++++++------- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/src/backend/services/SequentialOrchestrator.ts b/src/backend/services/SequentialOrchestrator.ts index 8d5848b..eb8f4dc 100644 --- a/src/backend/services/SequentialOrchestrator.ts +++ b/src/backend/services/SequentialOrchestrator.ts @@ -91,7 +91,10 @@ export class SequentialOrchestrator { } if (!succeeded) { // Phase exhausted all retries. Stop orchestrator. - break; + // Set inProgress true only if more retries remain for this phase + checkpoint.inProgress = (resultsForPhase.length < maxAttempts); + await this.saveCheckpoint(checkpoint); + return checkpoint; } } checkpoint.inProgress = checkpoint.currentPhase < this.phases.length; diff --git a/src/backend/tests/orchestrator.test.ts b/src/backend/tests/orchestrator.test.ts index 1cb31a0..e43a92a 100644 --- a/src/backend/tests/orchestrator.test.ts +++ b/src/backend/tests/orchestrator.test.ts @@ -55,21 +55,30 @@ describe('SequentialOrchestrator', () => { it('persists checkpoint after phase, can resume mid-run after crash', async () => { let fired = false; - const orchestrator = new SequentialOrchestrator({ - phases: [ - { name: 'p1', run: async () => {} }, - { name: 'p2', run: async () => { if (!fired) {fired=true; throw new Error('fail');} } }, - { name: 'p3', run: async () => {} }, - ], + // Keep ref to fired in closure used by both orchestrator instances. + const phase2Run = async () => { if (!fired) {fired = true; throw new Error('fail');} }; + const phases = [ + { name: 'p1', run: async () => {} }, + { name: 'p2', run: phase2Run, retry: 2 }, + { name: 'p3', run: async () => {} }, + ]; + // First run to fail at p2 + let orchestrator = new SequentialOrchestrator({ + phases, checkpointPath: tempCheckpoint, input: undefined }); - // First run to fail at p2 let cp = await orchestrator.run(); - expect(cp.phaseResults.filter(r=>r.phase==='p1' && r.status==='success').length).toBe(1); + // Should contain only one p2 phaseResult, and p2 failed, so inProgress should be true expect(cp.phaseResults.filter(r=>r.phase==='p2').length).toBe(1); + expect(cp.phaseResults.find(r=>r.phase==='p2').status).toBe('failure'); expect(cp.inProgress).toBe(true); - // Simulate restart -- resume should continue from failed phase + // Simulate restart -- create NEW orchestrator, resume should continue from failed phase + orchestrator = new SequentialOrchestrator({ + phases, + checkpointPath: tempCheckpoint, + input: undefined + }); fired = true; // Next attempt will succeed cp = await orchestrator.resume(); expect(cp.phaseResults.filter(r=>r.phase==='p2').length).toBe(2);