feat(server): cap agent review rounds and escalate exhausted reviews to the responsible human (#10650)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Execution policies let one agent implement and another review, cycling through changes-requested → addressed rounds > - Nothing bounds that cycle: no round counter, no escalation, no termination signal — two agents can ping-pong indefinitely, especially when the review's success criteria drift to something the implementer cannot satisfy > - On a real multi-agent instance this produced 6+ unattended rounds (~8 runs) that continued even after the human had merged the PR under review > - This pull request counts consecutive agent-initiated changes-requested rounds and, at a configurable cap, hands the still-pending review to the responsible human instead of bouncing back to the implementer > - The benefit is that unattended review loops terminate in a human decision instead of burning runs forever ## Linked Issues or Issue Description Fixes #10643 ## What Changed - `IssueExecutionState.changesRequestedCount` (schema + type, default 0): consecutive agent-initiated changes-requested rounds on the current stage. Carries through executor resubmissions, resets to 0 on approval, and resets when a **human** makes the changes-requested decision — the cap targets unattended agent↔agent ping-pong, never human review. - `IssueExecutionPolicy.maxReviewRounds` (optional, 1–50, default null → server default `DEFAULT_MAX_REVIEW_ROUNDS = 3`). - At the cap, the transition records the reviewer's changes-requested decision as usual but keeps the stage **pending** with the responsible human (`responsibleUserId`, falling back to `createdByUserId`) as the participant: the issue is assigned to that human and the pending review surfaces through the existing attention/review UI. The human then approves, requests changes (resetting the counter and handing back to the implementer), or re-scopes. - The escalated hold is sticky: transitions from anyone other than the escalated human no longer re-select a configured agent participant for the stage (which would have silently undone the escalation on the next unrelated PATCH). The escalated human's own decisions flow through the normal participant decision branch. - Issues with no responsible human keep today's hand-back behavior; the counter still accumulates so operators can see the churn. ## Verification - `pnpm vitest run server/src/__tests__/issue-execution-policy.test.ts` — 8 new cases: round counting on hand-back, count carried through resubmission, escalation at the default cap, sticky hold across unrelated transitions, human changes-requested resets the counter, human approval completes the stage, no-responsible-human fallback, and a `maxReviewRounds: 1` policy override. - `pnpm vitest run server/src/__tests__/issue-execution-policy-routes.test.ts` and the full `@paperclipai/shared` suite (387 tests) — schema additions are backward compatible (both fields optional with defaults; persisted states without the counter parse as 0). - `pnpm --filter @paperclipai/shared exec tsc --noEmit` and `cd server && pnpm run typecheck`. ## Risks - Behavior change: an agent-only review loop that previously ran forever now escalates to a human after 3 agent rounds by default. Instances that want longer loops can set `maxReviewRounds` per policy. Flows where a human participates are unaffected (human decisions reset the counter). - Escalation requires a `responsibleUserId`/`createdByUserId` on the issue; without one, behavior is unchanged. - Persisted execution states from before this change parse with `changesRequestedCount: 0` — no migration needed. ## Model Used Claude Fable 5 (`claude-fable-5`, Anthropic) via Claude Code — extended thinking, agentic tool use. No other models involved. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge
This commit is contained in:
Devin Foley
GitHub
parent
8444e5735c
commit
27f8c8dbcf
|
|
@ -631,6 +631,12 @@ export interface IssueExecutionPolicy {
|
|||
monitor?: IssueExecutionMonitorPolicy | null;
|
||||
reviewPreset?: LowTrustReviewPresetPolicy;
|
||||
authorizationPolicy?: TrustAuthorizationPolicy;
|
||||
/**
|
||||
* Maximum consecutive agent-initiated changes-requested rounds before the
|
||||
* pending stage escalates to the responsible human. Null uses the server
|
||||
* default. Human decisions reset the round counter.
|
||||
*/
|
||||
maxReviewRounds?: number | null;
|
||||
}
|
||||
|
||||
export interface IssueExecutionMonitorState {
|
||||
|
|
@ -666,6 +672,8 @@ export interface IssueExecutionState {
|
|||
lastDecisionId: string | null;
|
||||
lastDecisionOutcome: IssueExecutionDecisionOutcome | null;
|
||||
monitor?: IssueExecutionMonitorState | null;
|
||||
/** Consecutive agent-initiated changes-requested rounds on the current stage. */
|
||||
changesRequestedCount?: number;
|
||||
}
|
||||
|
||||
export interface IssueExecutionDecision {
|
||||
|
|
|
|||
|
|
@ -249,6 +249,7 @@ export const issueExecutionPolicySchema = z.object({
|
|||
monitor: issueExecutionMonitorPolicySchema.optional().nullable(),
|
||||
reviewPreset: lowTrustReviewPresetPolicySchema.optional(),
|
||||
authorizationPolicy: trustAuthorizationPolicySchema.optional(),
|
||||
maxReviewRounds: z.number().int().positive().max(50).optional().nullable().default(null),
|
||||
});
|
||||
|
||||
export const issueExecutionMonitorStateSchema = z.object({
|
||||
|
|
@ -284,6 +285,7 @@ export const issueExecutionStateSchema = z.object({
|
|||
lastDecisionId: z.string().uuid().nullable(),
|
||||
lastDecisionOutcome: z.enum(ISSUE_EXECUTION_DECISION_OUTCOMES).nullable(),
|
||||
monitor: issueExecutionMonitorStateSchema.optional().nullable(),
|
||||
changesRequestedCount: z.number().int().nonnegative().optional().default(0),
|
||||
});
|
||||
|
||||
export const issueRecoveryActionReadModelSchema = z.object({
|
||||
|
|
|
|||
|
|
@ -1619,3 +1619,294 @@ describe("issue execution policy transitions", () => {
|
|||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("review round circuit breaker", () => {
|
||||
const policy = reviewOnlyPolicy();
|
||||
const reviewStageId = policy.stages[0].id;
|
||||
|
||||
function reviewPendingIssue(overrides: Record<string, unknown> = {}, stateOverrides: Record<string, unknown> = {}) {
|
||||
return {
|
||||
status: "in_review",
|
||||
assigneeAgentId: qaAgentId,
|
||||
assigneeUserId: null,
|
||||
responsibleUserId: boardUserId,
|
||||
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,
|
||||
...stateOverrides,
|
||||
},
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
it("counts agent-initiated changes-requested rounds on the hand-back", () => {
|
||||
const result = applyIssueExecutionPolicyTransition({
|
||||
issue: reviewPendingIssue(),
|
||||
policy,
|
||||
requestedStatus: "in_progress",
|
||||
requestedAssigneePatch: {},
|
||||
actor: { agentId: qaAgentId },
|
||||
commentBody: "Round one feedback",
|
||||
});
|
||||
|
||||
expect(result.patch.status).toBe("in_progress");
|
||||
expect(result.patch.assigneeAgentId).toBe(coderAgentId);
|
||||
expect(result.patch.executionState).toMatchObject({
|
||||
status: "changes_requested",
|
||||
changesRequestedCount: 1,
|
||||
});
|
||||
});
|
||||
|
||||
it("carries the round count through the executor's resubmission", () => {
|
||||
const result = applyIssueExecutionPolicyTransition({
|
||||
issue: {
|
||||
status: "in_progress",
|
||||
assigneeAgentId: coderAgentId,
|
||||
assigneeUserId: null,
|
||||
responsibleUserId: boardUserId,
|
||||
executionPolicy: policy,
|
||||
executionState: {
|
||||
status: "changes_requested",
|
||||
currentStageId: reviewStageId,
|
||||
currentStageIndex: 0,
|
||||
currentStageType: "review",
|
||||
currentParticipant: { type: "agent", agentId: qaAgentId },
|
||||
returnAssignee: { type: "agent", agentId: coderAgentId },
|
||||
completedStageIds: [],
|
||||
lastDecisionId: null,
|
||||
lastDecisionOutcome: "changes_requested",
|
||||
changesRequestedCount: 2,
|
||||
},
|
||||
},
|
||||
policy,
|
||||
requestedStatus: "done",
|
||||
requestedAssigneePatch: {},
|
||||
actor: { agentId: coderAgentId },
|
||||
commentBody: "Addressed round two",
|
||||
});
|
||||
|
||||
expect(result.patch.status).toBe("in_review");
|
||||
expect(result.patch.executionState).toMatchObject({
|
||||
status: "pending",
|
||||
changesRequestedCount: 2,
|
||||
});
|
||||
});
|
||||
|
||||
it("escalates the pending stage to the responsible human at the round cap", () => {
|
||||
const result = applyIssueExecutionPolicyTransition({
|
||||
issue: reviewPendingIssue({}, { changesRequestedCount: 2 }),
|
||||
policy,
|
||||
requestedStatus: "in_progress",
|
||||
requestedAssigneePatch: {},
|
||||
actor: { agentId: qaAgentId },
|
||||
commentBody: "Round three feedback — still not converging",
|
||||
});
|
||||
|
||||
// The decision is still recorded, but the stage stays pending with the
|
||||
// responsible human as participant instead of bouncing to the executor.
|
||||
expect(result.decision).toMatchObject({ outcome: "changes_requested" });
|
||||
expect(result.patch.status).toBe("in_review");
|
||||
expect(result.patch.assigneeAgentId).toBeNull();
|
||||
expect(result.patch.assigneeUserId).toBe(boardUserId);
|
||||
expect(result.patch.executionState).toMatchObject({
|
||||
status: "pending",
|
||||
currentStageId: reviewStageId,
|
||||
currentParticipant: { type: "user", userId: boardUserId },
|
||||
changesRequestedCount: 3,
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps the escalated hold sticky across unrelated transitions", () => {
|
||||
const result = applyIssueExecutionPolicyTransition({
|
||||
issue: reviewPendingIssue(
|
||||
{ assigneeAgentId: null, assigneeUserId: boardUserId },
|
||||
{
|
||||
currentParticipant: { type: "user", userId: boardUserId },
|
||||
changesRequestedCount: 3,
|
||||
},
|
||||
),
|
||||
policy,
|
||||
requestedAssigneePatch: {},
|
||||
actor: { agentId: coderAgentId },
|
||||
});
|
||||
|
||||
expect(result.patch.executionState).toBeUndefined();
|
||||
expect(result.patch.assigneeAgentId).toBeUndefined();
|
||||
});
|
||||
|
||||
it("rejects a non-escalated actor advancing the stage during the hold", () => {
|
||||
expect(() =>
|
||||
applyIssueExecutionPolicyTransition({
|
||||
issue: reviewPendingIssue(
|
||||
{ assigneeAgentId: null, assigneeUserId: boardUserId },
|
||||
{
|
||||
currentParticipant: { type: "user", userId: boardUserId },
|
||||
changesRequestedCount: 3,
|
||||
},
|
||||
),
|
||||
policy,
|
||||
requestedStatus: "done",
|
||||
requestedAssigneePatch: {},
|
||||
actor: { agentId: qaAgentId },
|
||||
commentBody: "Agent trying to close it anyway",
|
||||
}),
|
||||
).toThrow("Only the escalated reviewer can advance the current execution stage");
|
||||
});
|
||||
|
||||
it("rejects a non-escalated actor reassigning the issue during the hold", () => {
|
||||
expect(() =>
|
||||
applyIssueExecutionPolicyTransition({
|
||||
issue: reviewPendingIssue(
|
||||
{ assigneeAgentId: null, assigneeUserId: boardUserId },
|
||||
{
|
||||
currentParticipant: { type: "user", userId: boardUserId },
|
||||
changesRequestedCount: 3,
|
||||
},
|
||||
),
|
||||
policy,
|
||||
requestedAssigneePatch: { assigneeAgentId: coderAgentId },
|
||||
actor: { agentId: coderAgentId },
|
||||
}),
|
||||
).toThrow("Only the escalated reviewer can advance the current execution stage");
|
||||
});
|
||||
|
||||
it("re-asserts the hold when the assignee has drifted away from the escalated human", () => {
|
||||
const result = applyIssueExecutionPolicyTransition({
|
||||
issue: reviewPendingIssue(
|
||||
// Assignee drifted back to the agent reviewer while the state still
|
||||
// records the escalated human as participant.
|
||||
{ assigneeAgentId: qaAgentId, assigneeUserId: null },
|
||||
{
|
||||
currentParticipant: { type: "user", userId: boardUserId },
|
||||
changesRequestedCount: 3,
|
||||
},
|
||||
),
|
||||
policy,
|
||||
requestedAssigneePatch: {},
|
||||
actor: { agentId: coderAgentId },
|
||||
});
|
||||
|
||||
expect(result.patch.status).toBe("in_review");
|
||||
expect(result.patch.assigneeAgentId).toBeNull();
|
||||
expect(result.patch.assigneeUserId).toBe(boardUserId);
|
||||
expect(result.patch.executionState).toMatchObject({
|
||||
status: "pending",
|
||||
currentParticipant: { type: "user", userId: boardUserId },
|
||||
changesRequestedCount: 3,
|
||||
});
|
||||
});
|
||||
|
||||
it("resets the counter when the escalated human requests changes", () => {
|
||||
const result = applyIssueExecutionPolicyTransition({
|
||||
issue: reviewPendingIssue(
|
||||
{ assigneeAgentId: null, assigneeUserId: boardUserId },
|
||||
{
|
||||
currentParticipant: { type: "user", userId: boardUserId },
|
||||
changesRequestedCount: 3,
|
||||
},
|
||||
),
|
||||
policy,
|
||||
requestedStatus: "in_progress",
|
||||
requestedAssigneePatch: {},
|
||||
actor: { userId: boardUserId },
|
||||
commentBody: "Human direction: do X instead",
|
||||
});
|
||||
|
||||
expect(result.patch.status).toBe("in_progress");
|
||||
expect(result.patch.assigneeAgentId).toBe(coderAgentId);
|
||||
expect(result.patch.executionState).toMatchObject({
|
||||
status: "changes_requested",
|
||||
changesRequestedCount: 0,
|
||||
});
|
||||
});
|
||||
|
||||
it("lets the escalated human approve the stage", () => {
|
||||
const result = applyIssueExecutionPolicyTransition({
|
||||
issue: reviewPendingIssue(
|
||||
{ assigneeAgentId: null, assigneeUserId: boardUserId },
|
||||
{
|
||||
currentParticipant: { type: "user", userId: boardUserId },
|
||||
changesRequestedCount: 3,
|
||||
},
|
||||
),
|
||||
policy,
|
||||
requestedStatus: "done",
|
||||
requestedAssigneePatch: {},
|
||||
actor: { userId: boardUserId },
|
||||
commentBody: "Good enough — shipping",
|
||||
});
|
||||
|
||||
expect(result.decision).toMatchObject({ outcome: "approved" });
|
||||
expect(result.patch.executionState).toMatchObject({
|
||||
status: "completed",
|
||||
changesRequestedCount: 0,
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps handing back to the executor when no responsible human exists", () => {
|
||||
const result = applyIssueExecutionPolicyTransition({
|
||||
issue: reviewPendingIssue({ responsibleUserId: null }, { changesRequestedCount: 9 }),
|
||||
policy,
|
||||
requestedStatus: "in_progress",
|
||||
requestedAssigneePatch: {},
|
||||
actor: { agentId: qaAgentId },
|
||||
commentBody: "Round ten feedback",
|
||||
});
|
||||
|
||||
expect(result.patch.status).toBe("in_progress");
|
||||
expect(result.patch.assigneeAgentId).toBe(coderAgentId);
|
||||
expect(result.patch.executionState).toMatchObject({
|
||||
status: "changes_requested",
|
||||
changesRequestedCount: 10,
|
||||
});
|
||||
});
|
||||
|
||||
it("honors a policy maxReviewRounds override", () => {
|
||||
const strictPolicy = normalizeIssueExecutionPolicy({
|
||||
stages: [{ type: "review", participants: [{ type: "agent", agentId: qaAgentId }] }],
|
||||
maxReviewRounds: 1,
|
||||
})!;
|
||||
const stageId = strictPolicy.stages[0].id;
|
||||
|
||||
const result = applyIssueExecutionPolicyTransition({
|
||||
issue: {
|
||||
status: "in_review",
|
||||
assigneeAgentId: qaAgentId,
|
||||
assigneeUserId: null,
|
||||
responsibleUserId: boardUserId,
|
||||
executionPolicy: strictPolicy,
|
||||
executionState: {
|
||||
status: "pending",
|
||||
currentStageId: stageId,
|
||||
currentStageIndex: 0,
|
||||
currentStageType: "review",
|
||||
currentParticipant: { type: "agent", agentId: qaAgentId },
|
||||
returnAssignee: { type: "agent", agentId: coderAgentId },
|
||||
completedStageIds: [],
|
||||
lastDecisionId: null,
|
||||
lastDecisionOutcome: null,
|
||||
},
|
||||
},
|
||||
policy: strictPolicy,
|
||||
requestedStatus: "in_progress",
|
||||
requestedAssigneePatch: {},
|
||||
actor: { agentId: qaAgentId },
|
||||
commentBody: "First and only agent round",
|
||||
});
|
||||
|
||||
expect(result.patch.assigneeUserId).toBe(boardUserId);
|
||||
expect(result.patch.executionState).toMatchObject({
|
||||
status: "pending",
|
||||
currentParticipant: { type: "user", userId: boardUserId },
|
||||
changesRequestedCount: 1,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -20,6 +20,8 @@ type AssigneeLike = {
|
|||
|
||||
type IssueLike = AssigneeLike & {
|
||||
status: string;
|
||||
responsibleUserId?: string | null;
|
||||
createdByUserId?: string | null;
|
||||
executionPolicy?: IssueExecutionPolicy | Record<string, unknown> | null;
|
||||
executionState?: IssueExecutionState | Record<string, unknown> | null;
|
||||
monitorNextCheckAt?: Date | null;
|
||||
|
|
@ -58,6 +60,13 @@ type TransitionResult = {
|
|||
workflowControlledAssignment?: boolean;
|
||||
};
|
||||
|
||||
/**
|
||||
* Consecutive agent-initiated changes-requested rounds tolerated on one stage
|
||||
* before the pending review escalates to the responsible human. Policies can
|
||||
* override via `maxReviewRounds`; human decisions always reset the counter.
|
||||
*/
|
||||
export const DEFAULT_MAX_REVIEW_ROUNDS = 3;
|
||||
|
||||
const COMPLETED_STATUS: IssueExecutionState["status"] = "completed";
|
||||
const PENDING_STATUS: IssueExecutionState["status"] = "pending";
|
||||
const CHANGES_REQUESTED_STATUS: IssueExecutionState["status"] = "changes_requested";
|
||||
|
|
@ -400,6 +409,7 @@ export function normalizeIssueExecutionPolicy(input: unknown): IssueExecutionPol
|
|||
...(monitor ? { monitor } : {}),
|
||||
...(reviewPreset ? { reviewPreset } : {}),
|
||||
...(authorizationPolicy ? { authorizationPolicy } : {}),
|
||||
...(parsed.data.maxReviewRounds != null ? { maxReviewRounds: parsed.data.maxReviewRounds } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -432,6 +442,24 @@ function principalsEqual(a: IssueExecutionStagePrincipal | null, b: IssueExecuti
|
|||
return a.type === "agent" ? a.agentId === b.agentId : a.userId === b.userId;
|
||||
}
|
||||
|
||||
function resolveMaxReviewRounds(policy: IssueExecutionPolicy | null): number {
|
||||
const configured = policy?.maxReviewRounds;
|
||||
return typeof configured === "number" && configured > 0 ? configured : DEFAULT_MAX_REVIEW_ROUNDS;
|
||||
}
|
||||
|
||||
/**
|
||||
* The human a review stage escalates to when agents exhaust their
|
||||
* changes-requested rounds. Without one the loop keeps handing back to the
|
||||
* return assignee (pre-existing behavior) rather than stalling the stage.
|
||||
*/
|
||||
function reviewEscalationUserId(issue: IssueLike): string | null {
|
||||
const responsible = issue.responsibleUserId?.trim();
|
||||
if (responsible) return responsible;
|
||||
const creator = issue.createdByUserId?.trim();
|
||||
if (creator) return creator;
|
||||
return null;
|
||||
}
|
||||
|
||||
function findStageById(policy: IssueExecutionPolicy, stageId: string | null | undefined) {
|
||||
if (!stageId) return null;
|
||||
return policy.stages.find((stage) => stage.id === stageId) ?? null;
|
||||
|
|
@ -497,6 +525,7 @@ function buildCompletedState(previous: IssueExecutionState | null, currentStage:
|
|||
lastDecisionId: previous?.lastDecisionId ?? null,
|
||||
lastDecisionOutcome: "approved",
|
||||
monitor: previous?.monitor ?? null,
|
||||
changesRequestedCount: 0,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -547,6 +576,7 @@ function buildPendingState(input: {
|
|||
participant: IssueExecutionStagePrincipal;
|
||||
returnAssignee: IssueExecutionStagePrincipal | null;
|
||||
reviewRequest?: IssueExecutionState["reviewRequest"] | null;
|
||||
changesRequestedCount?: number;
|
||||
}): IssueExecutionState {
|
||||
return {
|
||||
status: PENDING_STATUS,
|
||||
|
|
@ -560,10 +590,15 @@ function buildPendingState(input: {
|
|||
lastDecisionId: input.previous?.lastDecisionId ?? null,
|
||||
lastDecisionOutcome: input.previous?.lastDecisionOutcome ?? null,
|
||||
monitor: input.previous?.monitor ?? null,
|
||||
changesRequestedCount: input.changesRequestedCount ?? input.previous?.changesRequestedCount ?? 0,
|
||||
};
|
||||
}
|
||||
|
||||
function buildChangesRequestedState(previous: IssueExecutionState, currentStage: IssueExecutionStage): IssueExecutionState {
|
||||
function buildChangesRequestedState(
|
||||
previous: IssueExecutionState,
|
||||
currentStage: IssueExecutionStage,
|
||||
changesRequestedCount: number,
|
||||
): IssueExecutionState {
|
||||
return {
|
||||
...previous,
|
||||
status: CHANGES_REQUESTED_STATUS,
|
||||
|
|
@ -571,6 +606,7 @@ function buildChangesRequestedState(previous: IssueExecutionState, currentStage:
|
|||
currentStageType: currentStage.type,
|
||||
reviewRequest: null,
|
||||
lastDecisionOutcome: "changes_requested",
|
||||
changesRequestedCount,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -582,6 +618,7 @@ function buildPendingStagePatch(input: {
|
|||
participant: IssueExecutionStagePrincipal;
|
||||
returnAssignee: IssueExecutionStagePrincipal | null;
|
||||
reviewRequest?: IssueExecutionState["reviewRequest"] | null;
|
||||
changesRequestedCount?: number;
|
||||
}) {
|
||||
input.patch.status = "in_review";
|
||||
Object.assign(input.patch, patchForPrincipal(input.participant));
|
||||
|
|
@ -592,6 +629,7 @@ function buildPendingStagePatch(input: {
|
|||
participant: input.participant,
|
||||
returnAssignee: input.returnAssignee,
|
||||
reviewRequest: input.reviewRequest,
|
||||
changesRequestedCount: input.changesRequestedCount,
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -676,7 +714,45 @@ function applyIssueExecutionStageTransition(input: TransitionInput): TransitionR
|
|||
throw unprocessable(`No eligible ${activeStage.type} participant is configured for this issue`);
|
||||
}
|
||||
|
||||
if (!stageHasParticipant(activeStage, currentParticipant)) {
|
||||
// An escalated review is deliberately held by a human who is not in the
|
||||
// stage's configured participants. Re-selecting a configured (agent)
|
||||
// participant would silently undo the escalation on the next unrelated
|
||||
// PATCH, so the hold is sticky until the escalated human decides — their
|
||||
// own decisions fall through to the participant decision branch below.
|
||||
const escalatedHold =
|
||||
currentParticipant.type === "user" &&
|
||||
!stageHasParticipant(activeStage, currentParticipant) &&
|
||||
(existingState?.changesRequestedCount ?? 0) >= resolveMaxReviewRounds(input.policy);
|
||||
if (escalatedHold && !principalsEqual(currentParticipant, actor)) {
|
||||
// An empty patch would not override the caller's own requested fields,
|
||||
// so a status or assignee change from a non-escalated actor must be
|
||||
// rejected outright — mirroring the "only the active participant can
|
||||
// advance" rule below — and a drifted assignee must be re-asserted
|
||||
// rather than left pointing away from the escalated human.
|
||||
const attemptedAdvanceDuringHold =
|
||||
(requestedStatus !== undefined && requestedStatus !== "in_review") ||
|
||||
(requestedAssigneePatchProvided && !principalsEqual(explicitAssignee, currentParticipant));
|
||||
if (attemptedAdvanceDuringHold) {
|
||||
throw unprocessable("Only the escalated reviewer can advance the current execution stage");
|
||||
}
|
||||
const holdDrifted =
|
||||
input.issue.status !== "in_review" ||
|
||||
!principalsEqual(currentAssignee, currentParticipant);
|
||||
if (holdDrifted) {
|
||||
patch.status = "in_review";
|
||||
Object.assign(patch, patchForPrincipal(currentParticipant));
|
||||
patch.executionState = buildPendingState({
|
||||
previous: existingState,
|
||||
stage: activeStage,
|
||||
stageIndex: input.policy.stages.findIndex((candidate) => candidate.id === activeStage.id),
|
||||
participant: currentParticipant,
|
||||
returnAssignee: existingState?.returnAssignee ?? null,
|
||||
reviewRequest: effectiveReviewRequest,
|
||||
});
|
||||
}
|
||||
return { patch };
|
||||
}
|
||||
if (!escalatedHold && !stageHasParticipant(activeStage, currentParticipant)) {
|
||||
const participant = selectStageParticipant(activeStage, {
|
||||
preferred: explicitAssignee ?? existingState?.currentParticipant ?? null,
|
||||
exclude: existingState?.returnAssignee ?? null,
|
||||
|
|
@ -768,17 +844,47 @@ function applyIssueExecutionStageTransition(input: TransitionInput): TransitionR
|
|||
if (!existingState?.returnAssignee) {
|
||||
throw unprocessable("This execution stage has no return assignee");
|
||||
}
|
||||
const decision = {
|
||||
stageId: activeStage.id,
|
||||
stageType: activeStage.type,
|
||||
outcome: "changes_requested" as const,
|
||||
body: input.commentBody.trim(),
|
||||
};
|
||||
// Human decisions reset the round counter: the cap exists to stop
|
||||
// unattended agent↔agent ping-pong, not to limit human review.
|
||||
const actorIsHuman = actor?.type === "user";
|
||||
const nextRounds = actorIsHuman ? 0 : (existingState.changesRequestedCount ?? 0) + 1;
|
||||
if (!actorIsHuman && nextRounds >= resolveMaxReviewRounds(input.policy)) {
|
||||
const escalationUserId = reviewEscalationUserId(input.issue);
|
||||
if (escalationUserId) {
|
||||
// Rounds exhausted: keep the stage pending but hand it to the
|
||||
// responsible human instead of bouncing back to the implementer.
|
||||
// The recorded changes-requested decision carries the reviewer's
|
||||
// reasoning; the human approves, requests changes (resetting the
|
||||
// counter), or re-scopes.
|
||||
buildPendingStagePatch({
|
||||
patch,
|
||||
previous: existingState,
|
||||
policy: input.policy,
|
||||
stage: activeStage,
|
||||
participant: { type: "user", agentId: null, userId: escalationUserId },
|
||||
returnAssignee: existingState.returnAssignee,
|
||||
reviewRequest: effectiveReviewRequest,
|
||||
changesRequestedCount: nextRounds,
|
||||
});
|
||||
return {
|
||||
patch,
|
||||
decision,
|
||||
workflowControlledAssignment: true,
|
||||
};
|
||||
}
|
||||
}
|
||||
patch.status = "in_progress";
|
||||
Object.assign(patch, patchForPrincipal(existingState.returnAssignee));
|
||||
patch.executionState = buildChangesRequestedState(existingState, activeStage);
|
||||
patch.executionState = buildChangesRequestedState(existingState, activeStage, nextRounds);
|
||||
return {
|
||||
patch,
|
||||
decision: {
|
||||
stageId: activeStage.id,
|
||||
stageType: activeStage.type,
|
||||
outcome: "changes_requested",
|
||||
body: input.commentBody.trim(),
|
||||
},
|
||||
decision,
|
||||
workflowControlledAssignment: true,
|
||||
};
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue