Preserve pipeline work by using waiting_approval on post-completion errors (#836)

* Changes from fix/pipeline-not-finishing

* fix: Prevent overwriting merge_conflict status in pipeline error handlers

* fix: Handle feature loading failures gracefully in error recovery

* ```
fix: Add logging when feature reload fails during status update
```

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,30 @@ 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.`
+          );
+        }
+        // Don't overwrite terminal states like 'merge_conflict' that were set during pipeline execution
+        let currentStatus: string | undefined;
+        try {
+          const currentFeature = await this.loadFeatureFn(projectPath, featureId);
+          currentStatus = currentFeature?.status;
+        } 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);
+        }
         this.eventBus.emitAutoModeEvent('auto_mode_error', {
           featureId,
           featureName: feature?.title,

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,21 @@ 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');
+        // 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,

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);
+    });
   });
 });