diff --git a/server/src/__tests__/issue-execution-policy.test.ts b/server/src/__tests__/issue-execution-policy.test.ts index 3d8a649b7d4..6aeac5540fc 100644 --- a/server/src/__tests__/issue-execution-policy.test.ts +++ b/server/src/__tests__/issue-execution-policy.test.ts @@ -875,6 +875,83 @@ describe("issue execution policy transitions", () => { // coderAgentId is the returnAssignee, so QA should be selected expect(result.patch.assigneeAgentId).toBe(qaAgentId); }); + + it("skips a self-review-only stage and completes the workflow", () => { + const policy = makePolicy([ + { + type: "review", + participants: [{ type: "agent", agentId: coderAgentId }], + }, + ]); + + const result = applyIssueExecutionPolicyTransition({ + issue: { + status: "in_progress", + assigneeAgentId: coderAgentId, + assigneeUserId: null, + executionPolicy: policy, + executionState: null, + }, + policy, + requestedStatus: "done", + requestedAssigneePatch: {}, + actor: { agentId: coderAgentId }, + commentBody: "Done", + }); + + expect(result.patch).toMatchObject({ + executionState: { + status: "completed", + currentStageType: null, + currentParticipant: null, + returnAssignee: { type: "agent", agentId: coderAgentId }, + completedStageIds: [policy.stages[0].id], + }, + }); + expect(result.patch.status).toBeUndefined(); + expect(result.patch.assigneeAgentId).toBeUndefined(); + }); + + it("skips a self-review-only review stage and advances to approval", () => { + const policy = makePolicy([ + { + type: "review", + participants: [{ type: "agent", agentId: coderAgentId }], + }, + { + type: "approval", + participants: [{ type: "user", userId: ctoUserId }], + }, + ]); + + const result = applyIssueExecutionPolicyTransition({ + issue: { + status: "in_progress", + assigneeAgentId: coderAgentId, + assigneeUserId: null, + executionPolicy: policy, + executionState: null, + }, + policy, + requestedStatus: "done", + requestedAssigneePatch: {}, + actor: { agentId: coderAgentId }, + commentBody: "Done", + }); + + expect(result.patch).toMatchObject({ + status: "in_review", + assigneeAgentId: null, + assigneeUserId: ctoUserId, + executionState: { + status: "pending", + currentStageType: "approval", + currentParticipant: { type: "user", userId: ctoUserId }, + returnAssignee: { type: "agent", agentId: coderAgentId }, + completedStageIds: [policy.stages[0].id], + }, + }); + }); }); describe("changes requested with no return assignee", () => { diff --git a/server/src/services/issue-execution-policy.ts b/server/src/services/issue-execution-policy.ts index 6a3045a1e05..428a11013dc 100644 --- a/server/src/services/issue-execution-policy.ts +++ b/server/src/services/issue-execution-policy.ts @@ -174,6 +174,42 @@ function buildCompletedState(previous: IssueExecutionState | null, currentStage: }; } +function buildStateWithCompletedStages(input: { + previous: IssueExecutionState | null; + completedStageIds: string[]; + returnAssignee: IssueExecutionStagePrincipal | null; +}): IssueExecutionState { + return { + status: input.previous?.status ?? PENDING_STATUS, + currentStageId: input.previous?.currentStageId ?? null, + currentStageIndex: input.previous?.currentStageIndex ?? null, + currentStageType: input.previous?.currentStageType ?? null, + currentParticipant: input.previous?.currentParticipant ?? null, + returnAssignee: input.previous?.returnAssignee ?? input.returnAssignee, + completedStageIds: input.completedStageIds, + lastDecisionId: input.previous?.lastDecisionId ?? null, + lastDecisionOutcome: input.previous?.lastDecisionOutcome ?? null, + }; +} + +function buildSkippedStageCompletedState(input: { + previous: IssueExecutionState | null; + completedStageIds: string[]; + returnAssignee: IssueExecutionStagePrincipal | null; +}): IssueExecutionState { + return { + status: COMPLETED_STATUS, + currentStageId: null, + currentStageIndex: null, + currentStageType: null, + currentParticipant: null, + returnAssignee: input.previous?.returnAssignee ?? input.returnAssignee, + completedStageIds: input.completedStageIds, + lastDecisionId: input.previous?.lastDecisionId ?? null, + lastDecisionOutcome: input.previous?.lastDecisionOutcome ?? null, + }; +} + function buildPendingState(input: { previous: IssueExecutionState | null; stage: IssueExecutionStage; @@ -236,6 +272,18 @@ function clearExecutionStatePatch(input: { } } +function canAutoSkipPendingStage(input: { + stage: IssueExecutionStage; + returnAssignee: IssueExecutionStagePrincipal | null; + requestedStatus?: string; +}) { + if (input.requestedStatus !== "done" || input.stage.type !== "review" || !input.returnAssignee) { + return false; + } + return input.stage.participants.length > 0 && + input.stage.participants.every((participant) => principalsEqual(participant, input.returnAssignee)); +} + export function applyIssueExecutionPolicyTransition(input: TransitionInput): TransitionResult { const patch: Record = {}; const existingState = parseIssueExecutionState(input.issue.executionState); @@ -431,27 +479,61 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra return { patch }; } - const pendingStage = + let pendingStage = existingState?.status === CHANGES_REQUESTED_STATUS && currentStage ? currentStage : nextPendingStage(input.policy, existingState); if (!pendingStage) return { patch }; const returnAssignee = existingState?.returnAssignee ?? currentAssignee; - const participant = selectStageParticipant(pendingStage, { + const skippedStageIds = [...(existingState?.completedStageIds ?? [])]; + let participant = selectStageParticipant(pendingStage, { preferred: existingState?.status === CHANGES_REQUESTED_STATUS ? explicitAssignee ?? existingState.currentParticipant ?? null : explicitAssignee, exclude: returnAssignee, }); + while (!participant && canAutoSkipPendingStage({ stage: pendingStage, returnAssignee, requestedStatus })) { + skippedStageIds.push(pendingStage.id); + pendingStage = nextPendingStage( + input.policy, + buildStateWithCompletedStages({ + previous: existingState, + completedStageIds: skippedStageIds, + returnAssignee, + }), + ); + if (!pendingStage) { + patch.executionState = buildSkippedStageCompletedState({ + previous: existingState, + completedStageIds: skippedStageIds, + returnAssignee, + }); + return { patch }; + } + participant = selectStageParticipant(pendingStage, { + preferred: + existingState?.status === CHANGES_REQUESTED_STATUS + ? explicitAssignee ?? existingState.currentParticipant ?? null + : explicitAssignee, + exclude: returnAssignee, + }); + } if (!participant) { throw unprocessable(`No eligible ${pendingStage.type} participant is configured for this issue`); } buildPendingStagePatch({ patch, - previous: existingState, + previous: + skippedStageIds.length === (existingState?.completedStageIds ?? []).length + ? existingState + : buildStateWithCompletedStages({ + previous: existingState, + completedStageIds: skippedStageIds, + returnAssignee, + }), policy: input.policy, stage: pendingStage, participant,