From b1be9670a6e2390c1cb35f84bbb951fbde4f1c01 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 4 Mar 2026 17:09:11 -0800 Subject: [PATCH 1/4] Changes from fix/pipeline-not-finishing --- apps/server/src/services/execution-service.ts | 13 ++++- .../src/services/pipeline-orchestrator.ts | 13 ++++- .../unit/services/execution-service.test.ts | 55 +++++++++++++++++++ 3 files changed, 79 insertions(+), 2 deletions(-) diff --git a/apps/server/src/services/execution-service.ts b/apps/server/src/services/execution-service.ts index dc0555c5a..80d85d6d8 100644 --- a/apps/server/src/services/execution-service.ts +++ b/apps/server/src/services/execution-service.ts @@ -179,6 +179,7 @@ ${feature.spec} const abortController = tempRunningFeature.abortController; if (isAutoMode) await this.saveExecutionStateFn(projectPath); let feature: Feature | null = null; + let pipelineCompleted = false; try { validateWorkingDirectory(projectPath); @@ -431,6 +432,7 @@ Please continue from where you left off and complete all remaining tasks. Use th testAttempts: 0, maxTestAttempts: 5, }); + pipelineCompleted = true; // Check if pipeline set a terminal status (e.g., merge_conflict) — don't overwrite it const refreshed = await this.loadFeatureFn(projectPath, featureId); if (refreshed?.status === 'merge_conflict') { @@ -541,7 +543,16 @@ Please continue from where you left off and complete all remaining tasks. Use th } } else { logger.error(`Feature ${featureId} failed:`, error); - await this.updateFeatureStatusFn(projectPath, featureId, 'backlog'); + // If pipeline steps completed successfully, don't send the feature back to backlog. + // The pipeline work is done — set to waiting_approval so the user can review. + const fallbackStatus = pipelineCompleted ? 'waiting_approval' : 'backlog'; + if (pipelineCompleted) { + logger.info( + `[executeFeature] Feature ${featureId} failed after pipeline completed. ` + + `Setting status to waiting_approval instead of backlog to preserve pipeline work.` + ); + } + await this.updateFeatureStatusFn(projectPath, featureId, fallbackStatus); this.eventBus.emitAutoModeEvent('auto_mode_error', { featureId, featureName: feature?.title, diff --git a/apps/server/src/services/pipeline-orchestrator.ts b/apps/server/src/services/pipeline-orchestrator.ts index de9800013..6f1168051 100644 --- a/apps/server/src/services/pipeline-orchestrator.ts +++ b/apps/server/src/services/pipeline-orchestrator.ts @@ -350,6 +350,7 @@ export class PipelineOrchestrator { }); const abortController = runningEntry.abortController; runningEntry.branchName = feature.branchName ?? null; + let pipelineCompleted = false; try { validateWorkingDirectory(projectPath); @@ -403,6 +404,7 @@ export class PipelineOrchestrator { }; await this.executePipeline(context); + pipelineCompleted = true; // Re-fetch feature to check if executePipeline set a terminal status (e.g., merge_conflict) const reloadedFeature = await this.featureStateManager.loadFeature(projectPath, featureId); @@ -439,8 +441,17 @@ export class PipelineOrchestrator { }); } } else { + // If pipeline steps completed successfully, don't send the feature back to backlog. + // The pipeline work is done — set to waiting_approval so the user can review. + const fallbackStatus = pipelineCompleted ? 'waiting_approval' : 'backlog'; + if (pipelineCompleted) { + logger.info( + `[resumeFromStep] Feature ${featureId} failed after pipeline completed. ` + + `Setting status to waiting_approval instead of backlog to preserve pipeline work.` + ); + } logger.error(`Pipeline resume failed for ${featureId}:`, error); - await this.updateFeatureStatusFn(projectPath, featureId, 'backlog'); + await this.updateFeatureStatusFn(projectPath, featureId, fallbackStatus); this.eventBus.emitAutoModeEvent('auto_mode_error', { featureId, featureName: feature.title, diff --git a/apps/server/tests/unit/services/execution-service.test.ts b/apps/server/tests/unit/services/execution-service.test.ts index 0d976c9f7..af02bada8 100644 --- a/apps/server/tests/unit/services/execution-service.test.ts +++ b/apps/server/tests/unit/services/execution-service.test.ts @@ -2000,5 +2000,60 @@ describe('execution-service.ts', () => { // The only non-in_progress status call should be absent since merge_conflict returns early expect(statusCalls.length).toBe(0); }); + + it('sets waiting_approval instead of backlog when error occurs after pipeline completes', async () => { + // Set up pipeline with steps + vi.mocked(pipelineService.getPipelineConfig).mockResolvedValue({ + version: 1, + steps: [{ id: 'step-1', name: 'Step 1', order: 1, instructions: 'Do step 1' }] as any, + }); + + // Pipeline succeeds, but reading agent output throws after pipeline completes + mockExecutePipelineFn = vi.fn().mockResolvedValue(undefined); + // Simulate an error after pipeline completes by making loadFeature throw + // on the post-pipeline refresh call + let loadCallCount = 0; + mockLoadFeatureFn = vi.fn().mockImplementation(() => { + loadCallCount++; + if (loadCallCount === 1) return testFeature; // initial load + // Second call is the task-retry check, third is the pipeline refresh + if (loadCallCount <= 2) return testFeature; + throw new Error('Unexpected post-pipeline error'); + }); + + const svc = createServiceWithMocks(); + await svc.executeFeature('/test/project', 'feature-1'); + + // Should set to waiting_approval, NOT backlog, since pipeline completed + const backlogCalls = vi + .mocked(mockUpdateFeatureStatusFn) + .mock.calls.filter((call) => call[2] === 'backlog'); + expect(backlogCalls.length).toBe(0); + + const waitingCalls = vi + .mocked(mockUpdateFeatureStatusFn) + .mock.calls.filter((call) => call[2] === 'waiting_approval'); + expect(waitingCalls.length).toBeGreaterThan(0); + }); + + it('still sets backlog when error occurs before pipeline completes', async () => { + // Set up pipeline with steps + vi.mocked(pipelineService.getPipelineConfig).mockResolvedValue({ + version: 1, + steps: [{ id: 'step-1', name: 'Step 1', order: 1, instructions: 'Do step 1' }] as any, + }); + + // Pipeline itself throws (e.g., agent error during pipeline step) + mockExecutePipelineFn = vi.fn().mockRejectedValue(new Error('Agent execution failed')); + + const svc = createServiceWithMocks(); + await svc.executeFeature('/test/project', 'feature-1'); + + // Should still set to backlog since pipeline did NOT complete + const backlogCalls = vi + .mocked(mockUpdateFeatureStatusFn) + .mock.calls.filter((call) => call[2] === 'backlog'); + expect(backlogCalls.length).toBe(1); + }); }); }); From 2c25e4f21592e00760b0d983aff6394fd6e5748c Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 4 Mar 2026 18:48:31 -0800 Subject: [PATCH 2/4] fix: Prevent overwriting merge_conflict status in pipeline error handlers --- apps/server/src/services/execution-service.ts | 6 +++++- apps/server/src/services/pipeline-orchestrator.ts | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/apps/server/src/services/execution-service.ts b/apps/server/src/services/execution-service.ts index 80d85d6d8..967b3f1ef 100644 --- a/apps/server/src/services/execution-service.ts +++ b/apps/server/src/services/execution-service.ts @@ -552,7 +552,11 @@ Please continue from where you left off and complete all remaining tasks. Use th `Setting status to waiting_approval instead of backlog to preserve pipeline work.` ); } - await this.updateFeatureStatusFn(projectPath, featureId, fallbackStatus); + // Don't overwrite terminal states like 'merge_conflict' that were set during pipeline execution + const currentFeature = await this.loadFeatureFn(projectPath, featureId); + if (currentFeature?.status !== 'merge_conflict') { + await this.updateFeatureStatusFn(projectPath, featureId, fallbackStatus); + } this.eventBus.emitAutoModeEvent('auto_mode_error', { featureId, featureName: feature?.title, diff --git a/apps/server/src/services/pipeline-orchestrator.ts b/apps/server/src/services/pipeline-orchestrator.ts index 6f1168051..c3b1e53a1 100644 --- a/apps/server/src/services/pipeline-orchestrator.ts +++ b/apps/server/src/services/pipeline-orchestrator.ts @@ -451,7 +451,11 @@ export class PipelineOrchestrator { ); } logger.error(`Pipeline resume failed for ${featureId}:`, error); - await this.updateFeatureStatusFn(projectPath, featureId, fallbackStatus); + // Don't overwrite terminal states like 'merge_conflict' that were set during pipeline execution + const currentFeature = await this.featureStateManager.loadFeature(projectPath, featureId); + if (currentFeature?.status !== 'merge_conflict') { + await this.updateFeatureStatusFn(projectPath, featureId, fallbackStatus); + } this.eventBus.emitAutoModeEvent('auto_mode_error', { featureId, featureName: feature.title, From c196bc6627a26081004cf74a3508d807efefc2b2 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 4 Mar 2026 19:01:26 -0800 Subject: [PATCH 3/4] fix: Handle feature loading failures gracefully in error recovery --- apps/server/src/services/execution-service.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/apps/server/src/services/execution-service.ts b/apps/server/src/services/execution-service.ts index 967b3f1ef..ff24deae4 100644 --- a/apps/server/src/services/execution-service.ts +++ b/apps/server/src/services/execution-service.ts @@ -553,8 +553,14 @@ Please continue from where you left off and complete all remaining tasks. Use th ); } // Don't overwrite terminal states like 'merge_conflict' that were set during pipeline execution - const currentFeature = await this.loadFeatureFn(projectPath, featureId); - if (currentFeature?.status !== 'merge_conflict') { + let currentStatus: string | undefined; + try { + const currentFeature = await this.loadFeatureFn(projectPath, featureId); + currentStatus = currentFeature?.status; + } catch { + // If loading fails, proceed with the status update anyway + } + if (currentStatus !== 'merge_conflict') { await this.updateFeatureStatusFn(projectPath, featureId, fallbackStatus); } this.eventBus.emitAutoModeEvent('auto_mode_error', { From 4f4eae709d9780e0b41902502dcddfc67818096d Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 4 Mar 2026 20:08:06 -0800 Subject: [PATCH 4/4] ``` fix: Add logging when feature reload fails during status update ``` --- apps/server/src/services/execution-service.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/apps/server/src/services/execution-service.ts b/apps/server/src/services/execution-service.ts index ff24deae4..949f2e104 100644 --- a/apps/server/src/services/execution-service.ts +++ b/apps/server/src/services/execution-service.ts @@ -557,8 +557,12 @@ Please continue from where you left off and complete all remaining tasks. Use th try { const currentFeature = await this.loadFeatureFn(projectPath, featureId); currentStatus = currentFeature?.status; - } catch { - // If loading fails, proceed with the status update anyway + } catch (loadErr) { + // If loading fails, log it and proceed with the status update anyway + logger.warn( + `[executeFeature] Failed to reload feature ${featureId} for status check:`, + loadErr + ); } if (currentStatus !== 'merge_conflict') { await this.updateFeatureStatusFn(projectPath, featureId, fallbackStatus);