Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion apps/server/src/services/execution-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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') {
Expand Down Expand Up @@ -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);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
this.eventBus.emitAutoModeEvent('auto_mode_error', {
featureId,
featureName: feature?.title,
Expand Down
13 changes: 12 additions & 1 deletion apps/server/src/services/pipeline-orchestrator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,7 @@ export class PipelineOrchestrator {
});
const abortController = runningEntry.abortController;
runningEntry.branchName = feature.branchName ?? null;
let pipelineCompleted = false;

try {
validateWorkingDirectory(projectPath);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For consistency with execution-service.ts, it's a good practice to log the error as soon as it's caught, at the beginning of the else block. This improves readability and ensures error logging is the first action in the error handling path.

Suggested change
// 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);
logger.error(`Pipeline resume failed for ${featureId}:`, error);
// 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.`
);
}
await this.updateFeatureStatusFn(projectPath, featureId, fallbackStatus);

this.eventBus.emitAutoModeEvent('auto_mode_error', {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
featureId,
featureName: feature.title,
Expand Down
55 changes: 55 additions & 0 deletions apps/server/tests/unit/services/execution-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});
Loading