Fix signoff stage access and comment wake retries

This commit is contained in:
dotta 2026-04-09 14:48:12 -05:00
parent 03dff1a29a
commit 4077ccd343
3 changed files with 60 additions and 43 deletions

View file

@ -413,45 +413,33 @@ describe("issue execution policy transitions", () => {
const policy = twoStagePolicy(); const policy = twoStagePolicy();
const reviewStageId = policy.stages[0].id; const reviewStageId = policy.stages[0].id;
it("non-participant stage updates are coerced back to the active stage", () => { it("non-participant cannot advance the active stage", () => {
const result = applyIssueExecutionPolicyTransition({ expect(() =>
issue: { applyIssueExecutionPolicyTransition({
status: "in_review", issue: {
assigneeAgentId: qaAgentId, status: "in_review",
assigneeUserId: null, assigneeAgentId: qaAgentId,
executionPolicy: policy, assigneeUserId: null,
executionState: { executionPolicy: policy,
status: "pending", executionState: {
currentStageId: reviewStageId, status: "pending",
currentStageIndex: 0, currentStageId: reviewStageId,
currentStageType: "review", currentStageIndex: 0,
currentParticipant: { type: "agent", agentId: qaAgentId }, currentStageType: "review",
returnAssignee: { type: "agent", agentId: coderAgentId }, currentParticipant: { type: "agent", agentId: qaAgentId },
completedStageIds: [], returnAssignee: { type: "agent", agentId: coderAgentId },
lastDecisionId: null, completedStageIds: [],
lastDecisionOutcome: null, lastDecisionId: null,
lastDecisionOutcome: null,
},
}, },
}, policy,
policy, requestedStatus: "done",
requestedStatus: "done", requestedAssigneePatch: { assigneeUserId: boardUserId },
requestedAssigneePatch: { assigneeUserId: boardUserId }, actor: { agentId: coderAgentId },
actor: { agentId: coderAgentId }, commentBody: "Trying to bypass review",
commentBody: "Trying to bypass review", }),
}); ).toThrow("Only the active reviewer or approver can advance");
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();
}); });
it("non-participant can still post non-advancing updates", () => { it("non-participant can still post non-advancing updates", () => {

View file

@ -707,6 +707,18 @@ export function shouldResetTaskSessionForWake(
return false; return false;
} }
function shouldRequireIssueCommentForWake(
contextSnapshot: Record<string, unknown> | 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) { export function formatRuntimeWorkspaceWarningLog(warning: string) {
return { return {
stream: "stdout" as const, stream: "stdout" as const,
@ -2035,6 +2047,17 @@ export function heartbeatService(db: Db) {
return { outcome: "retry_exhausted" as const, queuedRun: null }; 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); const queuedRun = await enqueueMissingIssueCommentRetry(run, agent, issueId);
if (queuedRun) { if (queuedRun) {
await appendRunEvent(run, await nextRunEventSeq(run.id), { await appendRunEvent(run, await nextRunEventSeq(run.id), {

View file

@ -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" || input.issue.status !== "in_review" ||
!principalsEqual(currentAssignee, currentParticipant) || !principalsEqual(currentAssignee, currentParticipant) ||
!principalsEqual(existingState?.currentParticipant ?? null, currentParticipant) || !principalsEqual(existingState?.currentParticipant ?? null, currentParticipant);
(requestedStatus !== undefined && requestedStatus !== "in_review") ||
(requestedAssigneePatchProvided && !principalsEqual(explicitAssignee, currentParticipant)) if (attemptedStageAdvance && !stageStateDrifted) {
) { throw unprocessable("Only the active reviewer or approver can advance the current execution stage");
}
if (stageStateDrifted) {
buildPendingStagePatch({ buildPendingStagePatch({
patch, patch,
previous: existingState, previous: existingState,