Merge 6306795cad into c9e3bb7ca4
This commit is contained in:
Waseem Ilyas
GitHub
commit
1a9d2a0847
|
|
@ -1629,6 +1629,203 @@ describe("issue execution policy transitions", () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe("replacing executionPolicy mid-cycle must not let done skip a live review", () => {
|
||||
it("rejects done when a new executionPolicy regenerates the stage id during changes_requested", () => {
|
||||
const originalPolicy = twoStagePolicy();
|
||||
const replacementPolicy = twoStagePolicy();
|
||||
|
||||
expect(() =>
|
||||
applyIssueExecutionPolicyTransition({
|
||||
issue: {
|
||||
status: "in_progress",
|
||||
assigneeAgentId: coderAgentId,
|
||||
assigneeUserId: null,
|
||||
executionPolicy: originalPolicy,
|
||||
executionState: {
|
||||
status: "changes_requested",
|
||||
currentStageId: originalPolicy.stages[0].id,
|
||||
currentStageIndex: 0,
|
||||
currentStageType: "review",
|
||||
currentParticipant: { type: "agent", agentId: qaAgentId },
|
||||
returnAssignee: { type: "agent", agentId: coderAgentId },
|
||||
completedStageIds: [],
|
||||
lastDecisionId: null,
|
||||
lastDecisionOutcome: "changes_requested",
|
||||
},
|
||||
},
|
||||
// A brand-new policy object has freshly generated stage ids, so the
|
||||
// stored currentStageId from the live review no longer resolves.
|
||||
policy: replacementPolicy,
|
||||
requestedStatus: "done",
|
||||
requestedAssigneePatch: {},
|
||||
actor: { agentId: coderAgentId },
|
||||
}),
|
||||
).toThrow(expect.objectContaining({ status: 422, details: { code: "unexecuted_review_stage" } }));
|
||||
});
|
||||
|
||||
it("rejects done when a new executionPolicy regenerates the stage id during a pending review", () => {
|
||||
const originalPolicy = twoStagePolicy();
|
||||
const replacementPolicy = twoStagePolicy();
|
||||
|
||||
expect(() =>
|
||||
applyIssueExecutionPolicyTransition({
|
||||
issue: {
|
||||
status: "in_review",
|
||||
assigneeAgentId: qaAgentId,
|
||||
assigneeUserId: null,
|
||||
executionPolicy: originalPolicy,
|
||||
executionState: {
|
||||
status: "pending",
|
||||
currentStageId: originalPolicy.stages[0].id,
|
||||
currentStageIndex: 0,
|
||||
currentStageType: "review",
|
||||
currentParticipant: { type: "agent", agentId: qaAgentId },
|
||||
returnAssignee: { type: "agent", agentId: coderAgentId },
|
||||
completedStageIds: [],
|
||||
lastDecisionId: null,
|
||||
lastDecisionOutcome: null,
|
||||
},
|
||||
},
|
||||
policy: replacementPolicy,
|
||||
requestedStatus: "done",
|
||||
requestedAssigneePatch: {},
|
||||
actor: { agentId: qaAgentId },
|
||||
}),
|
||||
).toThrow(expect.objectContaining({ status: 422, details: { code: "unexecuted_review_stage" } }));
|
||||
});
|
||||
|
||||
it("lets a board cancellation through: the live cycle is abandoned, not claimed complete", () => {
|
||||
const originalPolicy = twoStagePolicy();
|
||||
const replacementPolicy = twoStagePolicy();
|
||||
|
||||
const result = applyIssueExecutionPolicyTransition({
|
||||
issue: {
|
||||
status: "in_review",
|
||||
assigneeAgentId: qaAgentId,
|
||||
assigneeUserId: null,
|
||||
executionPolicy: originalPolicy,
|
||||
executionState: {
|
||||
status: "pending",
|
||||
currentStageId: originalPolicy.stages[0].id,
|
||||
currentStageIndex: 0,
|
||||
currentStageType: "review",
|
||||
currentParticipant: { type: "agent", agentId: qaAgentId },
|
||||
returnAssignee: { type: "agent", agentId: coderAgentId },
|
||||
completedStageIds: [],
|
||||
lastDecisionId: null,
|
||||
lastDecisionOutcome: null,
|
||||
},
|
||||
},
|
||||
policy: replacementPolicy,
|
||||
requestedStatus: "cancelled",
|
||||
requestedAssigneePatch: {},
|
||||
actor: { agentId: qaAgentId },
|
||||
});
|
||||
|
||||
// The orphaned cycle is cleared; the requested `cancelled` itself is
|
||||
// applied by the caller, so the transition emits no status override.
|
||||
expect(result.patch).toEqual({ executionState: null });
|
||||
});
|
||||
|
||||
it("lets a routine non-done status change through on the same abandon-and-clear path", () => {
|
||||
const originalPolicy = twoStagePolicy();
|
||||
const replacementPolicy = twoStagePolicy();
|
||||
|
||||
const result = applyIssueExecutionPolicyTransition({
|
||||
issue: {
|
||||
status: "in_review",
|
||||
assigneeAgentId: qaAgentId,
|
||||
assigneeUserId: null,
|
||||
executionPolicy: originalPolicy,
|
||||
executionState: {
|
||||
status: "pending",
|
||||
currentStageId: originalPolicy.stages[0].id,
|
||||
currentStageIndex: 0,
|
||||
currentStageType: "review",
|
||||
currentParticipant: { type: "agent", agentId: qaAgentId },
|
||||
returnAssignee: { type: "agent", agentId: coderAgentId },
|
||||
completedStageIds: [],
|
||||
lastDecisionId: null,
|
||||
lastDecisionOutcome: null,
|
||||
},
|
||||
},
|
||||
policy: replacementPolicy,
|
||||
requestedStatus: "blocked",
|
||||
requestedAssigneePatch: {},
|
||||
actor: { agentId: qaAgentId },
|
||||
});
|
||||
|
||||
expect(result.patch).toEqual({ executionState: null });
|
||||
});
|
||||
|
||||
it("still allows a bare status: done to route to the stored reviewer when the policy is unchanged", () => {
|
||||
const policy = twoStagePolicy();
|
||||
const result = applyIssueExecutionPolicyTransition({
|
||||
issue: {
|
||||
status: "in_progress",
|
||||
assigneeAgentId: coderAgentId,
|
||||
assigneeUserId: null,
|
||||
executionPolicy: policy,
|
||||
executionState: {
|
||||
status: "changes_requested",
|
||||
currentStageId: policy.stages[0].id,
|
||||
currentStageIndex: 0,
|
||||
currentStageType: "review",
|
||||
currentParticipant: { type: "agent", agentId: qaAgentId },
|
||||
returnAssignee: { type: "agent", agentId: coderAgentId },
|
||||
completedStageIds: [],
|
||||
lastDecisionId: null,
|
||||
lastDecisionOutcome: "changes_requested",
|
||||
},
|
||||
},
|
||||
policy,
|
||||
requestedStatus: "done",
|
||||
requestedAssigneePatch: {},
|
||||
actor: { agentId: coderAgentId },
|
||||
});
|
||||
|
||||
expect(result.patch).toMatchObject({
|
||||
status: "in_review",
|
||||
assigneeAgentId: qaAgentId,
|
||||
});
|
||||
});
|
||||
|
||||
it("still allows a policy edit with no requested status to clear the orphaned stage and return to the executor", () => {
|
||||
const originalPolicy = twoStagePolicy();
|
||||
const replacementPolicy = twoStagePolicy();
|
||||
|
||||
const result = applyIssueExecutionPolicyTransition({
|
||||
issue: {
|
||||
status: "in_review",
|
||||
assigneeAgentId: qaAgentId,
|
||||
assigneeUserId: null,
|
||||
executionPolicy: originalPolicy,
|
||||
executionState: {
|
||||
status: "pending",
|
||||
currentStageId: originalPolicy.stages[0].id,
|
||||
currentStageIndex: 0,
|
||||
currentStageType: "review",
|
||||
currentParticipant: { type: "agent", agentId: qaAgentId },
|
||||
returnAssignee: { type: "agent", agentId: coderAgentId },
|
||||
completedStageIds: [],
|
||||
lastDecisionId: null,
|
||||
lastDecisionOutcome: null,
|
||||
},
|
||||
},
|
||||
policy: replacementPolicy,
|
||||
requestedStatus: undefined,
|
||||
requestedAssigneePatch: {},
|
||||
actor: { agentId: qaAgentId },
|
||||
});
|
||||
|
||||
expect(result.patch).toMatchObject({
|
||||
status: "in_progress",
|
||||
assigneeAgentId: coderAgentId,
|
||||
executionState: null,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("monitor policy", () => {
|
||||
it("schedules a one-shot monitor on an active agent-owned issue", () => {
|
||||
const policy = normalizeIssueExecutionPolicy({
|
||||
|
|
|
|||
|
|
@ -696,6 +696,24 @@ function applyIssueExecutionStageTransition(input: TransitionInput): TransitionR
|
|||
}
|
||||
|
||||
if (existingState?.currentStageId && !currentStage) {
|
||||
// The stored stage id no longer resolves against the (possibly just
|
||||
// replaced) policy — e.g. the caller PATCHed a brand-new executionPolicy
|
||||
// in the same request that regenerated stage ids. When a review cycle is
|
||||
// genuinely live (pending, or awaiting the assignee's response to
|
||||
// changes requested), letting `done` ride along on that request would
|
||||
// silently drop the cycle and mark the issue complete through a review
|
||||
// that never ran — so `done` is rejected. Every other requested status,
|
||||
// including a board cancellation that deliberately abandons the cycle,
|
||||
// keeps the established clear/return-to-executor behavior below, exactly
|
||||
// like a bare policy edit with no requested status.
|
||||
const hasLiveReviewCycle =
|
||||
existingState.status === PENDING_STATUS || existingState.status === CHANGES_REQUESTED_STATUS;
|
||||
if (hasLiveReviewCycle && requestedStatus === "done") {
|
||||
throw unprocessable(
|
||||
"This issue has an unexecuted review stage from a live review cycle. Resolve the review before marking it done.",
|
||||
{ code: "unexecuted_review_stage" },
|
||||
);
|
||||
}
|
||||
clearExecutionStatePatch({
|
||||
patch,
|
||||
issueStatus: input.issue.status,
|
||||
|
|
|
|||
Loading…
Reference in New Issue