Skip to content

feat(execution): execution_approved wake reason + self-approval fallback #4429

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ramonmatias19 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat(execution): execution_approved wake reason + self-approval fallback #4429

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions packages/adapter-utils/src/server-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -689,11 +689,19 @@ export function renderPaperclipWakePrompt(
"",
);
} else if (executionStage.wakeRole === "executor") {
lines.push(
"You are waking because changes were requested in the execution workflow.",
"Address the requested changes on this issue and resubmit when the work is ready.",
"",
);
if (executionStage.lastDecisionOutcome === "approved") {
lines.push(
"Your submitted work on this issue was APPROVED and the execution workflow is complete.",
"Acknowledge the approval, wrap up any remaining handoff notes, and do not continue executor work on this issue.",
"",
);
} else {
lines.push(
"You are waking because changes were requested in the execution workflow.",
"Address the requested changes on this issue and resubmit when the work is ready.",
"",
);
}
Comment on lines 691 to +704
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Approved-prompt fires on lastDecisionOutcome alone, not tied to wake reason

The condition executionStage.lastDecisionOutcome === "approved" is used to decide which executor prompt to show, but the buildCompletedState function sets lastDecisionOutcome: "approved" as a permanent field even if the state later transitions elsewhere. If an executor receives a non-execution_approved wake while the last decision happens to be "approved" (e.g. a liveness / comment wake during a completed-but-not-yet-acknowledged window), they will see the completion prompt instead of a general-purpose one.

Tying this branch to a wake-reason check (executionStage.wakeRole === "executor" && wakeReason === "execution_approved") — or including the wake reason in the ExecutionStageWakeContext passed to this renderer — would make the condition explicit and safe.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/adapter-utils/src/server-utils.ts
Line: 691-704

Comment:
**Approved-prompt fires on `lastDecisionOutcome` alone, not tied to wake reason**

The condition `executionStage.lastDecisionOutcome === "approved"` is used to decide which executor prompt to show, but the `buildCompletedState` function sets `lastDecisionOutcome: "approved"` as a permanent field even if the state later transitions elsewhere. If an executor receives a non-`execution_approved` wake while the last decision happens to be `"approved"` (e.g. a liveness / comment wake during a completed-but-not-yet-acknowledged window), they will see the completion prompt instead of a general-purpose one.

Tying this branch to a wake-reason check (`executionStage.wakeRole === "executor" && wakeReason === "execution_approved"`) — or including the wake reason in the `ExecutionStageWakeContext` passed to this renderer — would make the condition explicit and safe.

How can I resolve this? If you propose a fix, please make it concise.

}
}

Expand Down
75 changes: 75 additions & 0 deletions server/src/__tests__/issue-execution-policy.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1261,4 +1261,79 @@ describe("issue execution policy transitions", () => {
});
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Missing test for shouldResetTaskSessionForWake with new wake reason

The existing test suite in heartbeat-workspace-session.test.ts has explicit cases for execution_review_requested, execution_approval_requested, and execution_changes_requested, but no case for the newly added execution_approved reason. Adding a parallel test there would ensure consistency and guard against regressions if the function is refactored.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: server/src/__tests__/issue-execution-policy.test.ts
Line: 1261

Comment:
**Missing test for `shouldResetTaskSessionForWake` with new wake reason**

The existing test suite in `heartbeat-workspace-session.test.ts` has explicit cases for `execution_review_requested`, `execution_approval_requested`, and `execution_changes_requested`, but no case for the newly added `execution_approved` reason. Adding a parallel test there would ensure consistency and guard against regressions if the function is refactored.

How can I resolve this? If you propose a fix, please make it concise.

});
});

describe("single-participant stage coinciding with return assignee", () => {
it("starts workflow by falling back to the sole participant even if it is the return assignee", () => {
const policy = makePolicy([
{ type: "approval", 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: "Self-approval allowed because policy lists only this participant",
});

expect(result.patch).toMatchObject({
status: "in_review",
assigneeAgentId: coderAgentId,
executionState: {
status: "pending",
currentStageType: "approval",
currentParticipant: { type: "agent", agentId: coderAgentId },
},
});
});

it("advances to a next stage whose sole participant is the return assignee", () => {
const policy = makePolicy([
{ type: "review", participants: [{ type: "agent", agentId: qaAgentId }] },
{ type: "approval", participants: [{ type: "agent", agentId: coderAgentId }] },
]);
const reviewStageId = policy.stages[0].id;
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: "done",
requestedAssigneePatch: {},
actor: { agentId: qaAgentId },
commentBody: "QA signoff",
});

expect(result.patch).toMatchObject({
status: "in_review",
assigneeAgentId: coderAgentId,
executionState: {
status: "pending",
currentStageType: "approval",
currentParticipant: { type: "agent", agentId: coderAgentId },
},
});
expect(result.decision?.outcome).toBe("approved");
});
});
});
39 changes: 39 additions & 0 deletions server/src/routes/issues.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,45 @@ function buildExecutionStageWakeup(input: {
};
}

if (nextState.status === "completed") {
const agentId = nextState.returnAssignee?.type === "agent" ? (nextState.returnAssignee.agentId ?? null) : null;
const becameCompleted =
previousState?.status !== "completed" ||
previousState?.lastDecisionId !== nextState.lastDecisionId;
if (!agentId || !becameCompleted) return null;

const executionStage = buildExecutionStageWakeContext({
state: nextState,
wakeRole: "executor",
allowedActions: ["acknowledge_completion"],
});

return {
agentId,
wakeup: {
source: "assignment" as const,
triggerDetail: "system" as const,
reason: "execution_approved",
payload: {
issueId,
mutation: "update",
executionStage,
...(interruptedRunId ? { interruptedRunId } : {}),
},
requestedByActorType: input.requestedByActorType,
requestedByActorId: input.requestedByActorId,
contextSnapshot: {
issueId,
taskId: issueId,
wakeReason: "execution_approved",
source: "issue.execution_stage",
executionStage,
...(interruptedRunId ? { interruptedRunId } : {}),
},
},
};
}

return null;
}

Expand Down
4 changes: 3 additions & 1 deletion server/src/services/heartbeat.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1326,7 +1326,8 @@ export function shouldResetTaskSessionForWake(
wakeReason === "issue_assigned" ||
wakeReason === "execution_review_requested" ||
wakeReason === "execution_approval_requested" ||
wakeReason === "execution_changes_requested"
wakeReason === "execution_changes_requested" ||
wakeReason === "execution_approved"
) {
return true;
}
Expand Down Expand Up @@ -1401,6 +1402,7 @@ function describeSessionResetReason(
if (wakeReason === "execution_review_requested") return "wake reason is execution_review_requested";
if (wakeReason === "execution_approval_requested") return "wake reason is execution_approval_requested";
if (wakeReason === "execution_changes_requested") return "wake reason is execution_changes_requested";
if (wakeReason === "execution_approved") return "wake reason is execution_approved";
return null;
}

Expand Down
39 changes: 34 additions & 5 deletions server/src/services/issue-execution-policy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,10 +403,26 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra
};
}

const participant = selectStageParticipant(nextStage, {
let participant = selectStageParticipant(nextStage, {
preferred: explicitAssignee,
exclude: existingState?.returnAssignee ?? null,
});
// Fall back to the return assignee when the next stage has no other
// eligible participants and cannot be auto-skipped — the policy
// lists only the return assignee on this stage, so self-approval is
// intentional. Skippable stages (e.g. review with only the return
// assignee) are handled by the start-workflow skip loop on the
// next transition instead.
if (
!participant &&
!canAutoSkipPendingStage({
stage: nextStage,
returnAssignee: existingState?.returnAssignee ?? null,
requestedStatus,
})
) {
participant = selectStageParticipant(nextStage, { preferred: explicitAssignee });
}
if (!participant) {
throw unprocessable(`No eligible ${nextStage.type} participant is configured for this issue`);
}
Expand Down Expand Up @@ -502,13 +518,26 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra

const returnAssignee = existingState?.returnAssignee ?? currentAssignee;
const skippedStageIds = [...(existingState?.completedStageIds ?? [])];
const selectPreferred =
existingState?.status === CHANGES_REQUESTED_STATUS
? explicitAssignee ?? existingState.currentParticipant ?? null
: explicitAssignee;
let participant = selectStageParticipant(pendingStage, {
preferred:
existingState?.status === CHANGES_REQUESTED_STATUS
? explicitAssignee ?? existingState.currentParticipant ?? null
: explicitAssignee,
preferred: selectPreferred,
exclude: returnAssignee,
});
// When the pending stage's only eligible participant is the return
// assignee and the stage cannot be auto-skipped (e.g. an approval stage),
// fall back to self-assignment — policies that list only one participant
// on a non-skippable stage imply self-approval is intentional.
if (
!participant &&
!canAutoSkipPendingStage({ stage: pendingStage, returnAssignee, requestedStatus })
) {
participant = selectStageParticipant(pendingStage, {
preferred: selectPreferred,
});
}
while (!participant && canAutoSkipPendingStage({ stage: pendingStage, returnAssignee, requestedStatus })) {
skippedStageIds.push(pendingStage.id);
pendingStage = nextPendingStage(
Expand Down
Loading