diff --git a/server/src/__tests__/issue-execution-policy.test.ts b/server/src/__tests__/issue-execution-policy.test.ts index 7271b49935c..3ed1027a0bc 100644 --- a/server/src/__tests__/issue-execution-policy.test.ts +++ b/server/src/__tests__/issue-execution-policy.test.ts @@ -214,6 +214,37 @@ describe("issue execution policy transitions", () => { }); }); + it("rejects non-participants trying to advance the active stage", () => { + const reviewStageId = policy.stages[0].id; + + expect(() => + 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: "done", + requestedAssigneePatch: {}, + actor: { userId: ctoUserId }, + commentBody: "I'm not the reviewer.", + }), + ).toThrow("Only the active review or approval participant can advance this execution stage"); + }); + it("approver approves → marks completed (allows done)", () => { const reviewStageId = policy.stages[0].id; const approvalStageId = policy.stages[1].id; @@ -413,45 +444,33 @@ describe("issue execution policy transitions", () => { const policy = twoStagePolicy(); const reviewStageId = policy.stages[0].id; - it("non-participant stage updates are coerced back to the active stage", () => { - 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, + it("rejects non-participant stage updates when the issue is already locked to the active stage", () => { + expect(() => + 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: "done", - requestedAssigneePatch: { assigneeUserId: boardUserId }, - actor: { agentId: coderAgentId }, - commentBody: "Trying to bypass review", - }); - - expect(result.patch).toMatchObject({ - status: "in_review", - assigneeAgentId: qaAgentId, - assigneeUserId: null, - executionState: { - status: "pending", - currentStageId: reviewStageId, - currentStageType: "review", - currentParticipant: { type: "agent", agentId: qaAgentId }, - returnAssignee: { type: "agent", agentId: coderAgentId }, - }, - }); - expect(result.decision).toBeUndefined(); + policy, + requestedStatus: "done", + requestedAssigneePatch: { assigneeUserId: boardUserId }, + actor: { agentId: coderAgentId }, + commentBody: "Trying to bypass review", + }), + ).toThrow("Only the active review or approval participant can advance this execution stage"); }); it("non-participant can still post non-advancing updates", () => { diff --git a/server/src/services/issue-execution-policy.ts b/server/src/services/issue-execution-policy.ts index 6f4ba7b5557..bf04d72675b 100644 --- a/server/src/services/issue-execution-policy.ts +++ b/server/src/services/issue-execution-policy.ts @@ -393,6 +393,35 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra } } + const stageStateIsCanonical = + input.issue.status === "in_review" && + principalsEqual(currentAssignee, currentParticipant) && + principalsEqual(existingState?.currentParticipant ?? null, currentParticipant); + + if (!stageStateIsCanonical) { + buildPendingStagePatch({ + patch, + previous: existingState, + policy: input.policy, + stage: activeStage, + participant: currentParticipant, + returnAssignee: existingState?.returnAssignee ?? currentAssignee ?? actor, + }); + return { + patch, + workflowControlledAssignment: true, + }; + } + + if (!principalsEqual(currentParticipant, actor)) { + if (requestedStatus !== undefined && requestedStatus !== "in_review") { + throw unprocessable("Only the active review or approval participant can advance this execution stage"); + } + if (requestedAssigneePatchProvided && !principalsEqual(explicitAssignee, currentParticipant)) { + throw unprocessable("Assignment during review is controlled by the active execution stage participant"); + } + } + if ( input.issue.status !== "in_review" || !principalsEqual(currentAssignee, currentParticipant) ||