gb_flea/paperclip
1
0
Fork 0
forked from farhoodlabs/paperclip
Code Pull Requests Activity

Fix execution policy edits on in-review issues

Browse Source
This commit is contained in:
dotta
2026-04-08 16:54:43 -05:00
parent 26d4cabb2e
commit 27ec1e0c8b
3 changed files with 313 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/__tests__/issue-execution-policy-routes.test.ts
+140
View File
@@ -0,0 +1,140 @@
import express from "express";
import request from "supertest";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { errorHandler } from "../middleware/index.js";
import { issueRoutes } from "../routes/issues.js";
import { normalizeIssueExecutionPolicy } from "../services/issue-execution-policy.ts";
const mockIssueService = vi.hoisted(() => ({
getById: vi.fn(),
assertCheckoutOwner: vi.fn(),
update: vi.fn(),
addComment: vi.fn(),
findMentionedAgents: vi.fn(),
getRelationSummaries: vi.fn(),
listWakeableBlockedDependents: vi.fn(),
getWakeableParentAfterChildCompletion: vi.fn(),
}));
const mockHeartbeatService = vi.hoisted(() => ({
wakeup: vi.fn(async () => undefined),
reportRunActivity: vi.fn(async () => undefined),
getRun: vi.fn(async () => null),
getActiveRunForAgent: vi.fn(async () => null),
cancelRun: vi.fn(async () => null),
}));
vi.mock("../services/index.js", () => ({
accessService: () => ({
canUser: vi.fn(async () => false),
hasPermission: vi.fn(async () => false),
}),
agentService: () => ({
getById: vi.fn(async () => null),
}),
documentService: () => ({}),
executionWorkspaceService: () => ({}),
feedbackService: () => ({
listIssueVotesForUser: vi.fn(async () => []),
saveIssueVote: vi.fn(async () => ({ vote: null, consentEnabledNow: false, sharingEnabled: false })),
}),
goalService: () => ({}),
heartbeatService: () => mockHeartbeatService,
instanceSettingsService: () => ({
get: vi.fn(async () => ({
id: "instance-settings-1",
general: {
censorUsernameInLogs: false,
feedbackDataSharingPreference: "prompt",
},
})),
listCompanyIds: vi.fn(async () => ["company-1"]),
}),
issueApprovalService: () => ({}),
issueService: () => mockIssueService,
logActivity: vi.fn(async () => undefined),
projectService: () => ({}),
routineService: () => ({
syncRunStatusForIssue: vi.fn(async () => undefined),
}),
workProductService: () => ({}),
}));
function createApp() {
const app = express();
app.use(express.json());
app.use((req, _res, next) => {
(req as any).actor = {
type: "board",
userId: "local-board",
companyIds: ["company-1"],
source: "local_implicit",
isInstanceAdmin: false,
};
next();
});
app.use("/api", issueRoutes({} as any, {} as any));
app.use(errorHandler);
return app;
}
describe("issue execution policy routes", () => {
beforeEach(() => {
vi.clearAllMocks();
mockIssueService.assertCheckoutOwner.mockResolvedValue({ adoptedFromRunId: null });
mockIssueService.findMentionedAgents.mockResolvedValue([]);
mockIssueService.getRelationSummaries.mockResolvedValue({ blockedBy: [], blocks: [] });
mockIssueService.listWakeableBlockedDependents.mockResolvedValue([]);
mockIssueService.getWakeableParentAfterChildCompletion.mockResolvedValue(null);
});
it("does not auto-start execution review when reviewers are added to an already in_review issue", async () => {
const policy = normalizeIssueExecutionPolicy({
stages: [
{
id: "11111111-1111-4111-8111-111111111111",
type: "review",
participants: [{ type: "agent", agentId: "33333333-3333-4333-8333-333333333333" }],
},
],
})!;
const issue = {
id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
companyId: "company-1",
status: "in_review",
assigneeAgentId: null,
assigneeUserId: "local-board",
createdByUserId: "local-board",
identifier: "PAP-999",
title: "Execution policy edit",
executionPolicy: null,
executionState: null,
};
mockIssueService.getById.mockResolvedValue(issue);
mockIssueService.update.mockImplementation(async (_id: string, patch: Record<string, unknown>) => ({
...issue,
...patch,
updatedAt: new Date(),
}));
const res = await request(createApp())
.patch("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa")
.send({ executionPolicy: policy });
expect(res.status).toBe(200);
expect(mockIssueService.update).toHaveBeenCalledWith(
"aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
expect.objectContaining({
executionPolicy: policy,
actorAgentId: null,
actorUserId: "local-board",
}),
);
const updatePatch = mockIssueService.update.mock.calls[0]?.[1] as Record<string, unknown>;
expect(updatePatch.status).toBeUndefined();
expect(updatePatch.assigneeAgentId).toBeUndefined();
expect(updatePatch.assigneeUserId).toBeUndefined();
expect(updatePatch.executionState).toBeUndefined();
expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled();
});
});
server/src/__tests__/issue-execution-policy.test.ts
+115
View File
@@ -778,6 +778,25 @@ describe("issue execution policy transitions", () => {
expect(result.patch).toEqual({});
});
it("does not auto-start workflow when policy is added to an already in_review issue", () => {
const reviewOnly = reviewOnlyPolicy();
const result = applyIssueExecutionPolicyTransition({
issue: {
status: "in_review",
assigneeAgentId: null,
assigneeUserId: boardUserId,
executionPolicy: null,
executionState: null,
},
policy: reviewOnly,
requestedStatus: undefined,
requestedAssigneePatch: {},
actor: { userId: boardUserId },
});
expect(result.patch).toEqual({});
});
});
describe("multi-participant stages", () => {
@@ -974,4 +993,100 @@ describe("issue execution policy transitions", () => {
expect(result.patch.assigneeUserId).toBe(boardUserId);
});
});
describe("policy edits while a stage is active", () => {
it("clears the active execution state when its stage is removed from the policy", () => {
const reviewAndApproval = twoStagePolicy();
const approvalOnly = approvalOnlyPolicy();
const result = applyIssueExecutionPolicyTransition({
issue: {
status: "in_review",
assigneeAgentId: qaAgentId,
assigneeUserId: null,
executionPolicy: reviewAndApproval,
executionState: {
status: "pending",
currentStageId: reviewAndApproval.stages[0].id,
currentStageIndex: 0,
currentStageType: "review",
currentParticipant: { type: "agent", agentId: qaAgentId },
returnAssignee: { type: "agent", agentId: coderAgentId },
completedStageIds: [],
lastDecisionId: null,
lastDecisionOutcome: null,
},
},
policy: approvalOnly,
requestedStatus: undefined,
requestedAssigneePatch: {},
actor: { userId: boardUserId },
});
expect(result.patch).toMatchObject({
status: "in_progress",
assigneeAgentId: coderAgentId,
assigneeUserId: null,
executionState: null,
});
});
it("reassigns the active stage when the current participant is removed", () => {
const policy = makePolicy([
{
type: "review",
participants: [
{ type: "agent", agentId: qaAgentId },
{ type: "agent", agentId: ctoAgentId },
],
},
]);
const updatedPolicy = makePolicy([
{
type: "review",
participants: [{ type: "agent", agentId: ctoAgentId }],
},
]);
const result = applyIssueExecutionPolicyTransition({
issue: {
status: "in_review",
assigneeAgentId: qaAgentId,
assigneeUserId: null,
executionPolicy: policy,
executionState: {
status: "pending",
currentStageId: policy.stages[0].id,
currentStageIndex: 0,
currentStageType: "review",
currentParticipant: { type: "agent", agentId: qaAgentId },
returnAssignee: { type: "agent", agentId: coderAgentId },
completedStageIds: [],
lastDecisionId: null,
lastDecisionOutcome: null,
},
},
policy: {
...updatedPolicy,
stages: [{ ...updatedPolicy.stages[0], id: policy.stages[0].id }],
},
requestedStatus: undefined,
requestedAssigneePatch: {},
actor: { userId: boardUserId },
});
expect(result.patch).toMatchObject({
status: "in_review",
assigneeAgentId: ctoAgentId,
assigneeUserId: null,
executionState: {
status: "pending",
currentStageId: policy.stages[0].id,
currentStageType: "review",
currentParticipant: { type: "agent", agentId: ctoAgentId },
returnAssignee: { type: "agent", agentId: coderAgentId },
},
});
});
});
});
server/src/services/issue-execution-policy.ts
+58 -2
View File
@@ -145,6 +145,11 @@ function selectStageParticipant(
return first ? { type: first.type, agentId: first.agentId ?? null, userId: first.userId ?? null } : null;
}
function stageHasParticipant(stage: IssueExecutionStage, participant: IssueExecutionStagePrincipal | null): boolean {
if (!participant) return false;
return stage.participants.some((candidate) => principalsEqual(candidate, participant));
}
function patchForPrincipal(principal: IssueExecutionStagePrincipal | null) {
if (!principal) {
return { assigneeAgentId: null, assigneeUserId: null };
@@ -218,6 +223,19 @@ function buildPendingStagePatch(input: {
});
}
function clearExecutionStatePatch(input: {
patch: Record<string, unknown>;
issueStatus: string;
requestedStatus?: string;
returnAssignee: IssueExecutionStagePrincipal | null;
}) {
input.patch.executionState = null;
if (input.requestedStatus === undefined && input.issueStatus === "in_review" && input.returnAssignee) {
input.patch.status = "in_progress";
Object.assign(input.patch, patchForPrincipal(input.returnAssignee));
}
}
export function applyIssueExecutionPolicyTransition(input: TransitionInput): TransitionResult {
const patch: Record<string, unknown> = {};
const existingState = parseIssueExecutionState(input.issue.executionState);
@@ -251,6 +269,16 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra
return { patch };
}
if (existingState?.currentStageId && !currentStage) {
clearExecutionStatePatch({
patch,
issueStatus: input.issue.status,
requestedStatus,
returnAssignee: existingState.returnAssignee,
});
return { patch };
}
if (activeStage) {
const currentParticipant =
existingState?.currentParticipant ??
@@ -261,6 +289,35 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra
throw unprocessable(`No eligible ${activeStage.type} participant is configured for this issue`);
}
if (!stageHasParticipant(activeStage, currentParticipant)) {
const participant = selectStageParticipant(activeStage, {
preferred: explicitAssignee ?? existingState?.currentParticipant ?? null,
exclude: existingState?.returnAssignee ?? null,
});
if (!participant) {
clearExecutionStatePatch({
patch,
issueStatus: input.issue.status,
requestedStatus,
returnAssignee: existingState?.returnAssignee ?? null,
});
return { patch };
}
buildPendingStagePatch({
patch,
previous: existingState,
policy: input.policy,
stage: activeStage,
participant,
returnAssignee: existingState?.returnAssignee ?? currentAssignee ?? actor,
});
return {
patch,
workflowControlledAssignment: true,
};
}
if (principalsEqual(currentParticipant, actor)) {
if (requestedStatus === "done") {
if (!input.commentBody?.trim()) {
@@ -362,8 +419,7 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra
const shouldStartWorkflow =
requestedStatus === "done" ||
requestedStatus === "in_review" ||
(input.issue.status === "in_review" && existingState == null);
requestedStatus === "in_review";
if (!shouldStartWorkflow) {
return { patch };