Enforce execution-policy stage handoffs

This commit is contained in:
dotta 2026-04-08 08:05:35 -05:00
parent 9eaf72ab31
commit ec75cabcd8
8 changed files with 949 additions and 138 deletions

View file

@ -369,6 +369,252 @@ describe("codex execute", () => {
}
});
it("renders execution-stage wake instructions for reviewer and executor roles", async () => {
const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-codex-execute-stage-wake-"));
const workspace = path.join(root, "workspace");
const commandPath = path.join(root, "codex");
const capturePath = path.join(root, "capture.json");
await fs.mkdir(workspace, { recursive: true });
await writeFakeCodexCommand(commandPath);
const previousHome = process.env.HOME;
process.env.HOME = root;
try {
const result = await execute({
runId: "run-stage-wake",
agent: {
id: "agent-1",
companyId: "company-1",
name: "Codex Coder",
adapterType: "codex_local",
adapterConfig: {},
},
runtime: {
sessionId: null,
sessionParams: null,
sessionDisplayId: null,
taskKey: null,
},
config: {
command: commandPath,
cwd: workspace,
env: {
PAPERCLIP_TEST_CAPTURE_PATH: capturePath,
},
promptTemplate: "Follow the paperclip heartbeat.",
},
context: {
issueId: "issue-1",
taskId: "issue-1",
wakeReason: "execution_review_requested",
paperclipWake: {
reason: "execution_review_requested",
issue: {
id: "issue-1",
identifier: "PAP-1207",
title: "implement the plan of PAP-1200",
status: "in_review",
priority: "medium",
},
executionStage: {
wakeRole: "reviewer",
stageId: "stage-1",
stageType: "review",
currentParticipant: { type: "agent", agentId: "qa-agent" },
returnAssignee: { type: "agent", agentId: "coder-agent" },
lastDecisionOutcome: null,
allowedActions: ["approve", "request_changes"],
},
commentIds: [],
latestCommentId: null,
comments: [],
commentWindow: {
requestedCount: 0,
includedCount: 0,
missingCount: 0,
},
truncated: false,
fallbackFetchNeeded: false,
},
},
authToken: "run-jwt-token",
onLog: async () => {},
});
expect(result.exitCode).toBe(0);
const capture = JSON.parse(await fs.readFile(capturePath, "utf8")) as CapturePayload;
expect(capture.prompt).toContain("execution wake role: reviewer");
expect(capture.prompt).toContain("You are waking as the active reviewer for this issue.");
expect(capture.prompt).toContain("Do not execute the task itself or continue executor work.");
expect(capture.prompt).toContain("allowed actions: approve, request_changes");
const executorCapturePath = path.join(root, "capture-executor.json");
const executorResult = await execute({
runId: "run-stage-wake-executor",
agent: {
id: "agent-1",
companyId: "company-1",
name: "Codex Coder",
adapterType: "codex_local",
adapterConfig: {},
},
runtime: {
sessionId: null,
sessionParams: null,
sessionDisplayId: null,
taskKey: null,
},
config: {
command: commandPath,
cwd: workspace,
env: {
PAPERCLIP_TEST_CAPTURE_PATH: executorCapturePath,
},
promptTemplate: "Follow the paperclip heartbeat.",
},
context: {
issueId: "issue-1",
taskId: "issue-1",
wakeReason: "execution_changes_requested",
paperclipWake: {
reason: "execution_changes_requested",
issue: {
id: "issue-1",
identifier: "PAP-1207",
title: "implement the plan of PAP-1200",
status: "in_progress",
priority: "medium",
},
executionStage: {
wakeRole: "executor",
stageId: "stage-1",
stageType: "review",
currentParticipant: { type: "agent", agentId: "qa-agent" },
returnAssignee: { type: "agent", agentId: "coder-agent" },
lastDecisionOutcome: "changes_requested",
allowedActions: ["address_changes", "resubmit"],
},
commentIds: [],
latestCommentId: null,
comments: [],
commentWindow: {
requestedCount: 0,
includedCount: 0,
missingCount: 0,
},
truncated: false,
fallbackFetchNeeded: false,
},
},
authToken: "run-jwt-token",
onLog: async () => {},
});
expect(executorResult.exitCode).toBe(0);
const executorCapture = JSON.parse(await fs.readFile(executorCapturePath, "utf8")) as CapturePayload;
expect(executorCapture.prompt).toContain("execution wake role: executor");
expect(executorCapture.prompt).toContain("You are waking because changes were requested in the execution workflow.");
expect(executorCapture.prompt).toContain("allowed actions: address_changes, resubmit");
} finally {
if (previousHome === undefined) delete process.env.HOME;
else process.env.HOME = previousHome;
await fs.rm(root, { recursive: true, force: true });
}
});
it("renders an issue-scoped wake prompt even when the wake has no comments yet", async () => {
const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-codex-execute-issue-wake-"));
const workspace = path.join(root, "workspace");
const commandPath = path.join(root, "codex");
const capturePath = path.join(root, "capture.json");
await fs.mkdir(workspace, { recursive: true });
await writeFakeCodexCommand(commandPath);
const previousHome = process.env.HOME;
process.env.HOME = root;
try {
const result = await execute({
runId: "run-issue-wake",
agent: {
id: "agent-1",
companyId: "company-1",
name: "Codex Coder",
adapterType: "codex_local",
adapterConfig: {},
},
runtime: {
sessionId: null,
sessionParams: null,
sessionDisplayId: null,
taskKey: null,
},
config: {
command: commandPath,
cwd: workspace,
env: {
PAPERCLIP_TEST_CAPTURE_PATH: capturePath,
},
promptTemplate: "Follow the paperclip heartbeat.",
},
context: {
issueId: "issue-1",
taskId: "issue-1",
wakeReason: "issue_assigned",
paperclipWake: {
reason: "issue_assigned",
issue: {
id: "issue-1",
identifier: "PAP-1201",
title: "Fix gallery opening for inline images",
status: "todo",
priority: "medium",
},
commentIds: [],
latestCommentId: null,
comments: [],
commentWindow: {
requestedCount: 0,
includedCount: 0,
missingCount: 0,
},
truncated: false,
fallbackFetchNeeded: false,
},
},
authToken: "run-jwt-token",
onLog: async () => {},
});
expect(result.exitCode).toBe(0);
expect(result.errorMessage).toBeNull();
const capture = JSON.parse(await fs.readFile(capturePath, "utf8")) as CapturePayload;
expect(capture.paperclipEnvKeys).toContain("PAPERCLIP_WAKE_PAYLOAD_JSON");
expect(capture.paperclipWakePayloadJson).not.toBeNull();
expect(JSON.parse(capture.paperclipWakePayloadJson ?? "{}")).toMatchObject({
reason: "issue_assigned",
issue: {
identifier: "PAP-1201",
title: "Fix gallery opening for inline images",
status: "todo",
priority: "medium",
},
commentIds: [],
});
expect(capture.prompt).toContain("## Paperclip Wake Payload");
expect(capture.prompt).toContain("Do not switch to another issue until you have handled this wake.");
expect(capture.prompt).toContain("- issue: PAP-1201 Fix gallery opening for inline images");
expect(capture.prompt).toContain("- pending comments: 0/0");
expect(capture.prompt).toContain("- issue status: todo");
} finally {
if (previousHome === undefined) delete process.env.HOME;
else process.env.HOME = previousHome;
await fs.rm(root, { recursive: true, force: true });
}
});
it("uses a compact wake delta instead of the full heartbeat prompt when resuming a session", async () => {
const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-codex-execute-resume-wake-"));
const workspace = path.join(root, "workspace");

View file

@ -272,6 +272,18 @@ describe("shouldResetTaskSessionForWake", () => {
expect(shouldResetTaskSessionForWake({ wakeReason: "issue_assigned" })).toBe(true);
});
it("resets session context on execution review wakes", () => {
expect(shouldResetTaskSessionForWake({ wakeReason: "execution_review_requested" })).toBe(true);
});
it("resets session context on execution approval wakes", () => {
expect(shouldResetTaskSessionForWake({ wakeReason: "execution_approval_requested" })).toBe(true);
});
it("resets session context on execution changes-requested wakes", () => {
expect(shouldResetTaskSessionForWake({ wakeReason: "execution_changes_requested" })).toBe(true);
});
it("preserves session context on timer heartbeats", () => {
expect(shouldResetTaskSessionForWake({ wakeSource: "timer" })).toBe(false);
});

View file

@ -7,6 +7,7 @@ import { normalizeIssueExecutionPolicy } from "../services/issue-execution-polic
const mockIssueService = vi.hoisted(() => ({
getById: vi.fn(),
assertCheckoutOwner: vi.fn(),
update: vi.fn(),
addComment: vi.fn(),
findMentionedAgents: vi.fn(),
@ -75,8 +76,12 @@ vi.mock("../services/index.js", () => ({
function createApp() {
const app = express();
app.use(express.json());
return app;
}
function installActor(app: express.Express, actor?: Record<string, unknown>) {
app.use((req, _res, next) => {
(req as any).actor = {
(req as any).actor = actor ?? {
type: "board",
userId: "local-board",
companyIds: ["company-1"],
@ -119,6 +124,10 @@ describe("issue comment reopen routes", () => {
mockIssueService.findMentionedAgents.mockResolvedValue([]);
mockIssueService.listWakeableBlockedDependents.mockResolvedValue([]);
mockIssueService.getWakeableParentAfterChildCompletion.mockResolvedValue(null);
mockIssueService.assertCheckoutOwner.mockResolvedValue({ adoptedFromRunId: null });
mockAccessService.canUser.mockResolvedValue(false);
mockAccessService.hasPermission.mockResolvedValue(false);
mockAgentService.getById.mockResolvedValue(null);
});
it("treats reopen=true as a no-op when the issue is already open", async () => {
@ -128,7 +137,7 @@ describe("issue comment reopen routes", () => {
...patch,
}));
const res = await request(createApp())
const res = await request(installActor(createApp()))
.patch("/api/issues/11111111-1111-4111-8111-111111111111")
.send({ comment: "hello", reopen: true, assigneeAgentId: "33333333-3333-4333-8333-333333333333" });
@ -157,7 +166,7 @@ describe("issue comment reopen routes", () => {
...patch,
}));
const res = await request(createApp())
const res = await request(installActor(createApp()))
.patch("/api/issues/11111111-1111-4111-8111-111111111111")
.send({ comment: "hello", reopen: true, assigneeAgentId: "33333333-3333-4333-8333-333333333333" });
@ -207,7 +216,7 @@ describe("issue comment reopen routes", () => {
status: "cancelled",
});
const res = await request(createApp())
const res = await request(installActor(createApp()))
.patch("/api/issues/11111111-1111-4111-8111-111111111111")
.send({ comment: "hello", interrupt: true, assigneeAgentId: "33333333-3333-4333-8333-333333333333" });
@ -265,7 +274,7 @@ describe("issue comment reopen routes", () => {
_tx: tx,
}));
const res = await request(createApp())
const res = await request(installActor(createApp()))
.patch("/api/issues/11111111-1111-4111-8111-111111111111")
.send({ status: "done", comment: "Approved for ship" });
@ -294,4 +303,146 @@ describe("issue comment reopen routes", () => {
}),
);
});
it("coerces executor handoff patches into workflow-controlled review wakes", async () => {
const policy = normalizeIssueExecutionPolicy({
stages: [
{
id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
type: "review",
participants: [{ type: "agent", agentId: "33333333-3333-4333-8333-333333333333" }],
},
],
})!;
const issue = {
...makeIssue("todo"),
status: "in_progress",
assigneeAgentId: "22222222-2222-4222-8222-222222222222",
executionPolicy: policy,
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(
installActor(createApp(), {
type: "agent",
agentId: "22222222-2222-4222-8222-222222222222",
companyId: "company-1",
runId: "run-1",
}),
)
.patch("/api/issues/11111111-1111-4111-8111-111111111111")
.send({
status: "in_review",
assigneeAgentId: null,
assigneeUserId: "local-board",
});
expect(res.status).toBe(200);
expect(mockIssueService.update).toHaveBeenCalledWith(
"11111111-1111-4111-8111-111111111111",
expect.objectContaining({
status: "in_review",
assigneeAgentId: "33333333-3333-4333-8333-333333333333",
assigneeUserId: null,
executionState: expect.objectContaining({
status: "pending",
currentStageType: "review",
currentParticipant: expect.objectContaining({
type: "agent",
agentId: "33333333-3333-4333-8333-333333333333",
}),
returnAssignee: expect.objectContaining({
type: "agent",
agentId: "22222222-2222-4222-8222-222222222222",
}),
}),
}),
);
expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith(
"33333333-3333-4333-8333-333333333333",
expect.objectContaining({
reason: "execution_review_requested",
payload: expect.objectContaining({
issueId: "11111111-1111-4111-8111-111111111111",
executionStage: expect.objectContaining({
wakeRole: "reviewer",
stageType: "review",
allowedActions: ["approve", "request_changes"],
}),
}),
}),
);
});
it("wakes the return assignee with execution_changes_requested", async () => {
const policy = normalizeIssueExecutionPolicy({
stages: [
{
id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
type: "review",
participants: [{ type: "agent", agentId: "33333333-3333-4333-8333-333333333333" }],
},
],
})!;
const issue = {
...makeIssue("todo"),
status: "in_review",
assigneeAgentId: "33333333-3333-4333-8333-333333333333",
executionPolicy: policy,
executionState: {
status: "pending",
currentStageId: policy.stages[0].id,
currentStageIndex: 0,
currentStageType: "review",
currentParticipant: { type: "agent", agentId: "33333333-3333-4333-8333-333333333333" },
returnAssignee: { type: "agent", agentId: "22222222-2222-4222-8222-222222222222" },
completedStageIds: [],
lastDecisionId: null,
lastDecisionOutcome: null,
},
};
mockIssueService.getById.mockResolvedValue(issue);
mockIssueService.update.mockImplementation(async (_id: string, patch: Record<string, unknown>) => ({
...issue,
...patch,
updatedAt: new Date(),
}));
const res = await request(
installActor(createApp(), {
type: "agent",
agentId: "33333333-3333-4333-8333-333333333333",
companyId: "company-1",
runId: "run-2",
}),
)
.patch("/api/issues/11111111-1111-4111-8111-111111111111")
.send({
status: "in_progress",
comment: "Needs another pass",
});
expect(res.status).toBe(200);
expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith(
"22222222-2222-4222-8222-222222222222",
expect.objectContaining({
reason: "execution_changes_requested",
payload: expect.objectContaining({
issueId: "11111111-1111-4111-8111-111111111111",
executionStage: expect.objectContaining({
wakeRole: "executor",
stageType: "review",
lastDecisionOutcome: "changes_requested",
allowedActions: ["address_changes", "resubmit"],
}),
}),
}),
);
});
});

View file

@ -413,33 +413,45 @@ describe("issue execution policy transitions", () => {
const policy = twoStagePolicy();
const reviewStageId = policy.stages[0].id;
it("non-participant cannot advance stage via status change", () => {
expect(() =>
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,
},
it("non-participant stage updates are coerced back to the active stage", () => {
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: coderAgentId },
commentBody: "Trying to bypass review",
}),
).toThrow("Only the active reviewer or approver can advance");
},
policy,
requestedStatus: "done",
requestedAssigneePatch: { assigneeUserId: boardUserId },
actor: { agentId: coderAgentId },
commentBody: "Trying to bypass review",
});
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", () => {
@ -663,6 +675,7 @@ describe("issue execution policy transitions", () => {
describe("no-op transitions", () => {
const policy = twoStagePolicy();
const reviewStageId = policy.stages[0].id;
it("non-done status change without review context is a no-op", () => {
const result = applyIssueExecutionPolicyTransition({
@ -682,6 +695,72 @@ describe("issue execution policy transitions", () => {
expect(result.patch).toEqual({});
});
it("coerces a malformed executor in_review patch into the first policy stage", () => {
const result = applyIssueExecutionPolicyTransition({
issue: {
status: "in_progress",
assigneeAgentId: coderAgentId,
assigneeUserId: null,
executionPolicy: policy,
executionState: null,
},
policy,
requestedStatus: "in_review",
requestedAssigneePatch: { assigneeUserId: boardUserId },
actor: { agentId: coderAgentId },
});
expect(result.patch).toMatchObject({
status: "in_review",
assigneeAgentId: qaAgentId,
assigneeUserId: null,
executionState: {
status: "pending",
currentStageType: "review",
currentParticipant: { type: "agent", agentId: qaAgentId },
returnAssignee: { type: "agent", agentId: coderAgentId },
},
});
});
it("reasserts the active stage when issue status drifted out of in_review", () => {
const result = applyIssueExecutionPolicyTransition({
issue: {
status: "in_progress",
assigneeAgentId: coderAgentId,
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: "in_progress",
requestedAssigneePatch: { assigneeAgentId: coderAgentId },
actor: { agentId: coderAgentId },
});
expect(result.patch).toMatchObject({
status: "in_review",
assigneeAgentId: qaAgentId,
assigneeUserId: null,
executionState: {
status: "pending",
currentStageId: reviewStageId,
currentStageType: "review",
currentParticipant: { type: "agent", agentId: qaAgentId },
},
});
});
it("no policy and no state is a no-op", () => {
const result = applyIssueExecutionPolicyTransition({
issue: {