From 4077ccd343464d651469e80164b9a2b522d422f1 Mon Sep 17 00:00:00 2001 From: dotta Date: Thu, 9 Apr 2026 14:48:12 -0500 Subject: [PATCH] Fix signoff stage access and comment wake retries --- .../__tests__/issue-execution-policy.test.ts | 64 ++++++++----------- server/src/services/heartbeat.ts | 23 +++++++ server/src/services/issue-execution-policy.ts | 16 +++-- 3 files changed, 60 insertions(+), 43 deletions(-) diff --git a/server/src/__tests__/issue-execution-policy.test.ts b/server/src/__tests__/issue-execution-policy.test.ts index 7271b499..3d8a649b 100644 --- a/server/src/__tests__/issue-execution-policy.test.ts +++ b/server/src/__tests__/issue-execution-policy.test.ts @@ -413,45 +413,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("non-participant cannot advance 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 reviewer or approver can advance"); }); it("non-participant can still post non-advancing updates", () => { diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 954fe51b..45c5efd2 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -707,6 +707,18 @@ export function shouldResetTaskSessionForWake( return false; } +function shouldRequireIssueCommentForWake( + contextSnapshot: Record | null | undefined, +) { + const wakeReason = readNonEmptyString(contextSnapshot?.wakeReason); + return ( + wakeReason === "issue_assigned" || + wakeReason === "execution_review_requested" || + wakeReason === "execution_approval_requested" || + wakeReason === "execution_changes_requested" + ); +} + export function formatRuntimeWorkspaceWarningLog(warning: string) { return { stream: "stdout" as const, @@ -2035,6 +2047,17 @@ export function heartbeatService(db: Db) { return { outcome: "retry_exhausted" as const, queuedRun: null }; } + if (!shouldRequireIssueCommentForWake(contextSnapshot)) { + if (run.issueCommentStatus !== "not_applicable") { + await patchRunIssueCommentStatus(run.id, { + issueCommentStatus: "not_applicable", + issueCommentSatisfiedByCommentId: null, + issueCommentRetryQueuedAt: null, + }); + } + return { outcome: "not_applicable" as const, queuedRun: null }; + } + const queuedRun = await enqueueMissingIssueCommentRetry(run, agent, issueId); if (queuedRun) { await appendRunEvent(run, await nextRunEventSeq(run.id), { diff --git a/server/src/services/issue-execution-policy.ts b/server/src/services/issue-execution-policy.ts index 6f4ba7b5..6a3045a1 100644 --- a/server/src/services/issue-execution-policy.ts +++ b/server/src/services/issue-execution-policy.ts @@ -393,13 +393,19 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra } } - if ( + const attemptedStageAdvance = + (requestedStatus !== undefined && requestedStatus !== "in_review") || + (requestedAssigneePatchProvided && !principalsEqual(explicitAssignee, currentParticipant)); + const stageStateDrifted = input.issue.status !== "in_review" || !principalsEqual(currentAssignee, currentParticipant) || - !principalsEqual(existingState?.currentParticipant ?? null, currentParticipant) || - (requestedStatus !== undefined && requestedStatus !== "in_review") || - (requestedAssigneePatchProvided && !principalsEqual(explicitAssignee, currentParticipant)) - ) { + !principalsEqual(existingState?.currentParticipant ?? null, currentParticipant); + + if (attemptedStageAdvance && !stageStateDrifted) { + throw unprocessable("Only the active reviewer or approver can advance the current execution stage"); + } + + if (stageStateDrifted) { buildPendingStagePatch({ patch, previous: existingState,