diff --git a/server/src/__tests__/issue-execution-policy-routes.test.ts b/server/src/__tests__/issue-execution-policy-routes.test.ts index 2c2e494632..7302f13264 100644 --- a/server/src/__tests__/issue-execution-policy-routes.test.ts +++ b/server/src/__tests__/issue-execution-policy-routes.test.ts @@ -433,6 +433,234 @@ describe("issue execution policy routes", () => { expect(mockIssueApprovalService.listApprovalsForIssue).not.toHaveBeenCalled(); }); + it("allows a board user to cancel an active agent review task", async () => { + const policy = normalizeIssueExecutionPolicy({ + stages: [ + { + id: "11111111-1111-4111-8111-111111111111", + type: "review", + participants: [{ type: "agent", agentId: "33333333-3333-4333-8333-333333333333" }], + }, + ], + })!; + const issue = { + id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", + companyId: "company-1", + status: "in_review", + assigneeAgentId: "33333333-3333-4333-8333-333333333333", + assigneeUserId: null, + createdByUserId: "local-board", + identifier: "PAP-1008", + title: "Active review", + executionPolicy: policy, + executionState: { + status: "pending", + currentStageId: "11111111-1111-4111-8111-111111111111", + currentStageIndex: 0, + currentStageType: "review", + currentParticipant: { type: "agent", agentId: "33333333-3333-4333-8333-333333333333" }, + returnAssignee: { type: "agent", agentId: "44444444-4444-4444-8444-444444444444" }, + completedStageIds: [], + lastDecisionId: null, + lastDecisionOutcome: null, + }, + }; + mockIssueService.getById.mockResolvedValue(issue); + mockIssueService.update.mockImplementation(async (_id: string, patch: Record) => ({ + ...issue, + ...patch, + updatedAt: new Date(), + })); + + const res = await request(await createApp()) + .patch("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa") + .send({ status: "cancelled" }); + + expect(res.status).toBe(200); + expect(mockIssueService.update).toHaveBeenCalledWith( + "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", + expect.objectContaining({ + status: "cancelled", + executionState: null, + actorAgentId: null, + actorUserId: "local-board", + }), + ); + expect(mockHeartbeatService.cancelRun).not.toHaveBeenCalled(); + }); + + it("allows a board user to cancel a drifted pending agent review task", async () => { + const policy = normalizeIssueExecutionPolicy({ + stages: [ + { + id: "11111111-1111-4111-8111-111111111111", + type: "review", + participants: [{ type: "agent", agentId: "33333333-3333-4333-8333-333333333333" }], + }, + ], + })!; + const issue = { + id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", + companyId: "company-1", + status: "blocked", + assigneeAgentId: "44444444-4444-4444-8444-444444444444", + assigneeUserId: null, + createdByUserId: "local-board", + identifier: "PAP-1009", + title: "Drifted active review", + executionPolicy: policy, + executionState: { + status: "pending", + currentStageId: "11111111-1111-4111-8111-111111111111", + currentStageIndex: 0, + currentStageType: "review", + currentParticipant: { type: "agent", agentId: "33333333-3333-4333-8333-333333333333" }, + returnAssignee: { type: "agent", agentId: "44444444-4444-4444-8444-444444444444" }, + completedStageIds: [], + lastDecisionId: null, + lastDecisionOutcome: null, + }, + }; + mockIssueService.getById.mockResolvedValue(issue); + mockIssueService.update.mockImplementation(async (_id: string, patch: Record) => ({ + ...issue, + ...patch, + updatedAt: new Date(), + })); + + const res = await request(await createApp()) + .patch("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa") + .send({ status: "cancelled" }); + + expect(res.status).toBe(200); + expect(mockIssueService.update).toHaveBeenCalledWith( + "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", + expect.objectContaining({ + status: "cancelled", + executionState: null, + actorAgentId: null, + actorUserId: "local-board", + }), + ); + const updatePatch = mockIssueService.update.mock.calls[0]?.[1] as Record; + expect(updatePatch.status).toBe("cancelled"); + expect(updatePatch.assigneeAgentId).toBeUndefined(); + expect(updatePatch.assigneeUserId).toBeUndefined(); + expect(mockHeartbeatService.cancelRun).not.toHaveBeenCalled(); + }); + + it("keeps the review stage pending when a board user reassigns to an eligible participant", async () => { + const policy = normalizeIssueExecutionPolicy({ + stages: [ + { + id: "11111111-1111-4111-8111-111111111111", + type: "review", + participants: [ + { type: "agent", agentId: "33333333-3333-4333-8333-333333333333" }, + { type: "agent", agentId: "55555555-5555-4555-8555-555555555555" }, + ], + }, + ], + })!; + const issue = { + id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", + companyId: "company-1", + status: "in_review", + assigneeAgentId: "33333333-3333-4333-8333-333333333333", + assigneeUserId: null, + createdByUserId: "local-board", + identifier: "PAP-1010", + title: "Reassigned review", + executionPolicy: policy, + executionState: { + status: "pending", + currentStageId: "11111111-1111-4111-8111-111111111111", + currentStageIndex: 0, + currentStageType: "review", + currentParticipant: { type: "agent", agentId: "33333333-3333-4333-8333-333333333333" }, + returnAssignee: { type: "agent", agentId: "44444444-4444-4444-8444-444444444444" }, + completedStageIds: [], + lastDecisionId: null, + lastDecisionOutcome: null, + }, + }; + mockIssueService.getById.mockResolvedValue(issue); + mockIssueService.update.mockImplementation(async (_id: string, patch: Record) => ({ + ...issue, + ...patch, + updatedAt: new Date(), + })); + + const res = await request(await createApp()) + .patch("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa") + .send({ assigneeAgentId: "55555555-5555-4555-8555-555555555555" }); + + expect(res.status).toBe(200); + const updatePatch = mockIssueService.update.mock.calls[0]?.[1] as Record; + expect(updatePatch.status).toBe("in_review"); + expect(updatePatch.assigneeAgentId).toBe("55555555-5555-4555-8555-555555555555"); + expect(updatePatch.assigneeUserId).toBeNull(); + expect(updatePatch.executionState).toMatchObject({ + status: "pending", + currentStageId: "11111111-1111-4111-8111-111111111111", + currentStageType: "review", + currentParticipant: { type: "agent", agentId: "55555555-5555-4555-8555-555555555555" }, + returnAssignee: { type: "agent", agentId: "44444444-4444-4444-8444-444444444444" }, + }); + expect(mockHeartbeatService.cancelRun).not.toHaveBeenCalled(); + }); + + it("dissolves the review when a board user reassigns an in_review task to a non-participant", async () => { + const policy = normalizeIssueExecutionPolicy({ + stages: [ + { + id: "11111111-1111-4111-8111-111111111111", + type: "review", + participants: [{ type: "agent", agentId: "33333333-3333-4333-8333-333333333333" }], + }, + ], + })!; + const issue = { + id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", + companyId: "company-1", + status: "in_review", + assigneeAgentId: "33333333-3333-4333-8333-333333333333", + assigneeUserId: null, + createdByUserId: "local-board", + identifier: "PAP-1011", + title: "Reassigned away from review", + executionPolicy: policy, + executionState: { + status: "pending", + currentStageId: "11111111-1111-4111-8111-111111111111", + currentStageIndex: 0, + currentStageType: "review", + currentParticipant: { type: "agent", agentId: "33333333-3333-4333-8333-333333333333" }, + returnAssignee: { type: "agent", agentId: "44444444-4444-4444-8444-444444444444" }, + completedStageIds: [], + lastDecisionId: null, + lastDecisionOutcome: null, + }, + }; + mockIssueService.getById.mockResolvedValue(issue); + mockIssueService.update.mockImplementation(async (_id: string, patch: Record) => ({ + ...issue, + ...patch, + updatedAt: new Date(), + })); + + const res = await request(await createApp()) + .patch("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa") + .send({ assigneeAgentId: "55555555-5555-4555-8555-555555555555" }); + + expect(res.status).toBe(200); + const updatePatch = mockIssueService.update.mock.calls[0]?.[1] as Record; + expect(updatePatch.status).toBe("in_progress"); + expect(updatePatch.executionState).toBeNull(); + expect(updatePatch.assigneeAgentId).toBe("55555555-5555-4555-8555-555555555555"); + expect(mockHeartbeatService.cancelRun).not.toHaveBeenCalled(); + }); + it("does not auto-start execution review when reviewers are added to an already in_review issue", async () => { const policy = normalizeIssueExecutionPolicy({ stages: [ diff --git a/server/src/__tests__/issue-execution-policy.test.ts b/server/src/__tests__/issue-execution-policy.test.ts index e066f30b3e..8b9d560191 100644 --- a/server/src/__tests__/issue-execution-policy.test.ts +++ b/server/src/__tests__/issue-execution-policy.test.ts @@ -569,6 +569,181 @@ describe("issue execution policy transitions", () => { ).toThrow("Only the active reviewer or approver can advance"); }); + it("board override can cancel an active review without recording an approval decision", () => { + const result = applyIssueExecutionPolicyTransition({ + issue: { + status: "in_review", + assigneeAgentId: qaAgentId, + assigneeUserId: null, + executionPolicy: policy, + executionState: { + status: "pending", + currentStageId: reviewStageId, + currentStageIndex: 0, + currentStageType: "review", + currentParticipant: { type: "agent", agentId: qaAgentId }, + returnAssignee: { type: "agent", agentId: coderAgentId }, + completedStageIds: [], + lastDecisionId: null, + lastDecisionOutcome: null, + }, + }, + policy, + requestedStatus: "cancelled", + requestedAssigneePatch: {}, + actor: { userId: boardUserId }, + allowBoardOverride: true, + commentBody: "Cancelling this task", + }); + + expect(result.patch).toEqual({ executionState: null }); + expect(result.decision).toBeUndefined(); + expect(result.workflowControlledAssignment).toBeUndefined(); + }); + + it("board override can cancel a drifted pending review without rebuilding the pending stage", () => { + const result = applyIssueExecutionPolicyTransition({ + issue: { + status: "blocked", + assigneeAgentId: coderAgentId, + assigneeUserId: null, + executionPolicy: policy, + executionState: { + status: "pending", + currentStageId: reviewStageId, + currentStageIndex: 0, + currentStageType: "review", + currentParticipant: { type: "agent", agentId: qaAgentId }, + returnAssignee: { type: "agent", agentId: coderAgentId }, + completedStageIds: [], + lastDecisionId: null, + lastDecisionOutcome: null, + }, + }, + policy, + requestedStatus: "cancelled", + requestedAssigneePatch: {}, + actor: { userId: boardUserId }, + allowBoardOverride: true, + commentBody: "Cancelling this drifted task", + }); + + expect(result.patch).toEqual({ executionState: null }); + expect(result.decision).toBeUndefined(); + expect(result.workflowControlledAssignment).toBeUndefined(); + }); + + it("board override reassignment to an eligible participant re-pends the stage", () => { + const multiReviewerPolicy = makePolicy([ + { + type: "review", + participants: [ + { type: "agent", agentId: qaAgentId }, + { type: "agent", agentId: ctoAgentId }, + ], + }, + ]); + const multiReviewerStageId = multiReviewerPolicy.stages[0].id; + const result = applyIssueExecutionPolicyTransition({ + issue: { + status: "in_review", + assigneeAgentId: qaAgentId, + assigneeUserId: null, + executionPolicy: multiReviewerPolicy, + executionState: { + status: "pending", + currentStageId: multiReviewerStageId, + currentStageIndex: 0, + currentStageType: "review", + currentParticipant: { type: "agent", agentId: qaAgentId }, + returnAssignee: { type: "agent", agentId: coderAgentId }, + completedStageIds: [], + lastDecisionId: null, + lastDecisionOutcome: null, + }, + }, + policy: multiReviewerPolicy, + requestedAssigneePatch: { assigneeAgentId: ctoAgentId }, + actor: { userId: boardUserId }, + allowBoardOverride: true, + commentBody: "Swapping the reviewer", + }); + + expect(result.patch.status).toBe("in_review"); + expect(result.patch.assigneeAgentId).toBe(ctoAgentId); + expect(result.patch.assigneeUserId).toBeNull(); + expect(result.patch.executionState).toMatchObject({ + status: "pending", + currentStageId: multiReviewerStageId, + currentStageType: "review", + currentParticipant: { type: "agent", agentId: ctoAgentId }, + returnAssignee: { type: "agent", agentId: coderAgentId }, + }); + expect(result.decision).toBeUndefined(); + }); + + it("board override reassignment to a non-participant dissolves the review", () => { + const result = applyIssueExecutionPolicyTransition({ + issue: { + status: "in_review", + assigneeAgentId: qaAgentId, + assigneeUserId: null, + executionPolicy: policy, + executionState: { + status: "pending", + currentStageId: reviewStageId, + currentStageIndex: 0, + currentStageType: "review", + currentParticipant: { type: "agent", agentId: qaAgentId }, + returnAssignee: { type: "agent", agentId: coderAgentId }, + completedStageIds: [], + lastDecisionId: null, + lastDecisionOutcome: null, + }, + }, + policy, + requestedAssigneePatch: { assigneeAgentId: coderAgentId }, + actor: { userId: boardUserId }, + allowBoardOverride: true, + commentBody: "Handing the task back", + }); + + expect(result.patch).toEqual({ executionState: null, status: "in_progress" }); + expect(result.decision).toBeUndefined(); + expect(result.workflowControlledAssignment).toBeUndefined(); + }); + + it("board override unassignment dissolves the review instead of stranding in_review", () => { + const result = applyIssueExecutionPolicyTransition({ + issue: { + status: "in_review", + assigneeAgentId: qaAgentId, + assigneeUserId: null, + executionPolicy: policy, + executionState: { + status: "pending", + currentStageId: reviewStageId, + currentStageIndex: 0, + currentStageType: "review", + currentParticipant: { type: "agent", agentId: qaAgentId }, + returnAssignee: { type: "agent", agentId: coderAgentId }, + completedStageIds: [], + lastDecisionId: null, + lastDecisionOutcome: null, + }, + }, + policy, + requestedAssigneePatch: { assigneeAgentId: null, assigneeUserId: null }, + actor: { userId: boardUserId }, + allowBoardOverride: true, + commentBody: "Unassigning the reviewer", + }); + + expect(result.patch).toEqual({ executionState: null, status: "in_progress" }); + expect(result.decision).toBeUndefined(); + expect(result.workflowControlledAssignment).toBeUndefined(); + }); + it("non-participant can still post non-advancing updates", () => { const result = applyIssueExecutionPolicyTransition({ issue: { diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index ad04b3f19d..e314a6d42f 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -8005,6 +8005,7 @@ export function issueRoutes( agentId: actor.agentId ?? null, userId: actor.actorType === "user" ? actor.actorId : null, }, + allowBoardOverride: req.actor.type === "board", commentBody, reviewRequest: reviewRequest === undefined ? undefined : reviewRequest, monitorExplicitlyUpdated: req.body.executionPolicy !== undefined && monitorChanged, diff --git a/server/src/services/issue-execution-policy.ts b/server/src/services/issue-execution-policy.ts index 1c7a054d8d..859dc5fe1e 100644 --- a/server/src/services/issue-execution-policy.ts +++ b/server/src/services/issue-execution-policy.ts @@ -49,6 +49,7 @@ type TransitionInput = { requestedStatus?: string; requestedAssigneePatch: RequestedAssigneePatch; actor: ActorLike; + allowBoardOverride?: boolean; commentBody?: string | null; reviewRequest?: IssueExecutionState["reviewRequest"] | null; monitorExplicitlyUpdated?: boolean; @@ -837,6 +838,16 @@ function applyIssueExecutionStageTransition(input: TransitionInput): TransitionR }; } + if ( + input.allowBoardOverride && + requestedStatus && + requestedStatus !== "in_review" && + requestedStatus !== "in_progress" + ) { + patch.executionState = null; + return { patch }; + } + if (requestedStatus && requestedStatus !== "in_review") { if (!input.commentBody?.trim()) { throw unprocessable(`Requesting changes requires a comment. ${STAGE_DECISION_COMMENT_HINT}`); @@ -898,6 +909,37 @@ function applyIssueExecutionStageTransition(input: TransitionInput): TransitionR !principalsEqual(currentAssignee, currentParticipant) || !principalsEqual(existingState?.currentParticipant ?? null, currentParticipant); + if (input.allowBoardOverride && attemptedStageAdvance) { + if (requestedStatus !== undefined && requestedStatus !== "in_review") { + patch.executionState = null; + return { patch }; + } + // Assignee-only override: the issue stays in_review, so clearing the + // execution state would strand it with no participant or return + // assignment. Re-pend the stage when the board's chosen assignee is an + // eligible stage participant; otherwise (unassign or a non-participant) + // dissolve the review — storing an ineligible participant would be + // silently replaced by the stage-membership repair on the next + // transition. + if (explicitAssignee && stageHasParticipant(activeStage, explicitAssignee)) { + buildPendingStagePatch({ + patch, + previous: existingState, + policy: input.policy, + stage: activeStage, + participant: explicitAssignee, + returnAssignee: existingState?.returnAssignee ?? currentAssignee ?? actor, + reviewRequest: effectiveReviewRequest, + }); + return { patch }; + } + patch.executionState = null; + if (input.issue.status === "in_review") { + patch.status = "in_progress"; + } + return { patch }; + } + if (attemptedStageAdvance && !stageStateDrifted) { throw unprocessable("Only the active reviewer or approver can advance the current execution stage"); }