mirror of
https://github.com/paperclipai/paperclip
synced 2026-04-25 17:25:15 +02:00
Add loose review request handoffs
This commit is contained in:
@@ -326,6 +326,34 @@ describe("renderPaperclipWakePrompt", () => {
|
||||
expect(prompt).toContain("PAP-1723 Finish blocker (todo)");
|
||||
});
|
||||
|
||||
it("renders loose review request instructions for execution handoffs", () => {
|
||||
const prompt = renderPaperclipWakePrompt({
|
||||
reason: "execution_review_requested",
|
||||
issue: {
|
||||
id: "issue-1",
|
||||
identifier: "PAP-2011",
|
||||
title: "Review request handoff",
|
||||
status: "in_review",
|
||||
},
|
||||
executionStage: {
|
||||
wakeRole: "reviewer",
|
||||
stageId: "stage-1",
|
||||
stageType: "review",
|
||||
currentParticipant: { type: "agent", agentId: "agent-1" },
|
||||
returnAssignee: { type: "agent", agentId: "agent-2" },
|
||||
reviewRequest: {
|
||||
instructions: "Please focus on edge cases and leave a short risk summary.",
|
||||
},
|
||||
allowedActions: ["approve", "request_changes"],
|
||||
},
|
||||
fallbackFetchNeeded: false,
|
||||
});
|
||||
|
||||
expect(prompt).toContain("Review request instructions:");
|
||||
expect(prompt).toContain("Please focus on edge cases and leave a short risk summary.");
|
||||
expect(prompt).toContain("You are waking as the active reviewer for this issue.");
|
||||
});
|
||||
|
||||
it("includes continuation and child issue summaries in structured wake context", () => {
|
||||
const payload = {
|
||||
reason: "issue_children_completed",
|
||||
|
||||
@@ -282,6 +282,9 @@ type PaperclipWakeExecutionStage = {
|
||||
stageType: string | null;
|
||||
currentParticipant: PaperclipWakeExecutionPrincipal | null;
|
||||
returnAssignee: PaperclipWakeExecutionPrincipal | null;
|
||||
reviewRequest: {
|
||||
instructions: string;
|
||||
} | null;
|
||||
lastDecisionOutcome: string | null;
|
||||
allowedActions: string[];
|
||||
};
|
||||
@@ -484,11 +487,14 @@ function normalizePaperclipWakeExecutionStage(value: unknown): PaperclipWakeExec
|
||||
: [];
|
||||
const currentParticipant = normalizePaperclipWakeExecutionPrincipal(stage.currentParticipant);
|
||||
const returnAssignee = normalizePaperclipWakeExecutionPrincipal(stage.returnAssignee);
|
||||
const reviewRequestRaw = parseObject(stage.reviewRequest);
|
||||
const reviewInstructions = asString(reviewRequestRaw.instructions, "").trim();
|
||||
const reviewRequest = reviewInstructions ? { instructions: reviewInstructions } : null;
|
||||
const stageId = asString(stage.stageId, "").trim() || null;
|
||||
const stageType = asString(stage.stageType, "").trim() || null;
|
||||
const lastDecisionOutcome = asString(stage.lastDecisionOutcome, "").trim() || null;
|
||||
|
||||
if (!wakeRole && !stageId && !stageType && !currentParticipant && !returnAssignee && !lastDecisionOutcome && allowedActions.length === 0) {
|
||||
if (!wakeRole && !stageId && !stageType && !currentParticipant && !returnAssignee && !reviewRequest && !lastDecisionOutcome && allowedActions.length === 0) {
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -498,6 +504,7 @@ function normalizePaperclipWakeExecutionStage(value: unknown): PaperclipWakeExec
|
||||
stageType,
|
||||
currentParticipant,
|
||||
returnAssignee,
|
||||
reviewRequest,
|
||||
lastDecisionOutcome,
|
||||
allowedActions,
|
||||
};
|
||||
@@ -664,6 +671,13 @@ export function renderPaperclipWakePrompt(
|
||||
if (executionStage.allowedActions.length > 0) {
|
||||
lines.push(`- allowed actions: ${executionStage.allowedActions.join(", ")}`);
|
||||
}
|
||||
if (executionStage.reviewRequest) {
|
||||
lines.push(
|
||||
"",
|
||||
"Review request instructions:",
|
||||
executionStage.reviewRequest.instructions,
|
||||
);
|
||||
}
|
||||
lines.push("");
|
||||
if (executionStage.wakeRole === "reviewer" || executionStage.wakeRole === "approver") {
|
||||
lines.push(
|
||||
|
||||
@@ -631,6 +631,7 @@ export {
|
||||
updateIssueSchema,
|
||||
issueExecutionPolicySchema,
|
||||
issueExecutionStateSchema,
|
||||
issueReviewRequestSchema,
|
||||
issueExecutionWorkspaceSettingsSchema,
|
||||
checkoutIssueSchema,
|
||||
addIssueCommentSchema,
|
||||
|
||||
@@ -119,6 +119,7 @@ export type {
|
||||
IssueExecutionStage,
|
||||
IssueExecutionStageParticipant,
|
||||
IssueExecutionStagePrincipal,
|
||||
IssueReviewRequest,
|
||||
IssueExecutionDecision,
|
||||
IssueComment,
|
||||
IssueThreadInteractionActorFields,
|
||||
|
||||
@@ -168,6 +168,10 @@ export interface IssueExecutionPolicy {
|
||||
stages: IssueExecutionStage[];
|
||||
}
|
||||
|
||||
export interface IssueReviewRequest {
|
||||
instructions: string;
|
||||
}
|
||||
|
||||
export interface IssueExecutionState {
|
||||
status: IssueExecutionStateStatus;
|
||||
currentStageId: string | null;
|
||||
@@ -175,6 +179,7 @@ export interface IssueExecutionState {
|
||||
currentStageType: IssueExecutionStageType | null;
|
||||
currentParticipant: IssueExecutionStagePrincipal | null;
|
||||
returnAssignee: IssueExecutionStagePrincipal | null;
|
||||
reviewRequest: IssueReviewRequest | null;
|
||||
completedStageIds: string[];
|
||||
lastDecisionId: string | null;
|
||||
lastDecisionOutcome: IssueExecutionDecisionOutcome | null;
|
||||
|
||||
@@ -151,6 +151,7 @@ export {
|
||||
updateIssueSchema,
|
||||
issueExecutionPolicySchema,
|
||||
issueExecutionStateSchema,
|
||||
issueReviewRequestSchema,
|
||||
issueExecutionWorkspaceSettingsSchema,
|
||||
checkoutIssueSchema,
|
||||
addIssueCommentSchema,
|
||||
|
||||
@@ -105,6 +105,10 @@ export const issueExecutionPolicySchema = z.object({
|
||||
stages: z.array(issueExecutionStageSchema).default([]),
|
||||
});
|
||||
|
||||
export const issueReviewRequestSchema = z.object({
|
||||
instructions: z.string().trim().min(1).max(20000),
|
||||
}).strict();
|
||||
|
||||
export const issueExecutionStateSchema = z.object({
|
||||
status: z.enum(ISSUE_EXECUTION_STATE_STATUSES),
|
||||
currentStageId: z.string().uuid().nullable(),
|
||||
@@ -112,6 +116,7 @@ export const issueExecutionStateSchema = z.object({
|
||||
currentStageType: z.enum(ISSUE_EXECUTION_STAGE_TYPES).nullable(),
|
||||
currentParticipant: issueExecutionStagePrincipalSchema.nullable(),
|
||||
returnAssignee: issueExecutionStagePrincipalSchema.nullable(),
|
||||
reviewRequest: issueReviewRequestSchema.nullable().optional().default(null),
|
||||
completedStageIds: z.array(z.string().uuid()).default([]),
|
||||
lastDecisionId: z.string().uuid().nullable(),
|
||||
lastDecisionOutcome: z.enum(ISSUE_EXECUTION_DECISION_OUTCOMES).nullable(),
|
||||
@@ -164,6 +169,7 @@ export type CreateIssueLabel = z.infer<typeof createIssueLabelSchema>;
|
||||
export const updateIssueSchema = createIssueSchema.partial().extend({
|
||||
assigneeAgentId: z.string().trim().min(1).optional().nullable(),
|
||||
comment: z.string().min(1).optional(),
|
||||
reviewRequest: issueReviewRequestSchema.optional().nullable(),
|
||||
reopen: z.boolean().optional(),
|
||||
interrupt: z.boolean().optional(),
|
||||
hiddenAt: z.string().datetime().nullable().optional(),
|
||||
|
||||
@@ -795,6 +795,9 @@ describe("issue comment reopen routes", () => {
|
||||
status: "in_review",
|
||||
assigneeAgentId: null,
|
||||
assigneeUserId: "local-board",
|
||||
reviewRequest: {
|
||||
instructions: "Please verify the fix against the reproduction steps and note any residual risk.",
|
||||
},
|
||||
});
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
@@ -811,6 +814,9 @@ describe("issue comment reopen routes", () => {
|
||||
type: "agent",
|
||||
agentId: "22222222-2222-4222-8222-222222222222",
|
||||
},
|
||||
reviewRequest: {
|
||||
instructions: "Please verify the fix against the reproduction steps and note any residual risk.",
|
||||
},
|
||||
});
|
||||
expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith(
|
||||
"33333333-3333-4333-8333-333333333333",
|
||||
@@ -821,6 +827,9 @@ describe("issue comment reopen routes", () => {
|
||||
executionStage: expect.objectContaining({
|
||||
wakeRole: "reviewer",
|
||||
stageType: "review",
|
||||
reviewRequest: {
|
||||
instructions: "Please verify the fix against the reproduction steps and note any residual risk.",
|
||||
},
|
||||
allowedActions: ["approve", "request_changes"],
|
||||
}),
|
||||
}),
|
||||
|
||||
@@ -171,6 +171,38 @@ describe("issue execution policy transitions", () => {
|
||||
expect(result.decision).toBeUndefined();
|
||||
});
|
||||
|
||||
it("carries loose review instructions on the pending handoff", () => {
|
||||
const reviewInstructions = [
|
||||
"Please focus on whether the migration path is reversible.",
|
||||
"",
|
||||
"- Check failure handling",
|
||||
"- Call out any unclear operator instructions",
|
||||
].join("\n");
|
||||
|
||||
const result = applyIssueExecutionPolicyTransition({
|
||||
issue: {
|
||||
status: "in_progress",
|
||||
assigneeAgentId: coderAgentId,
|
||||
assigneeUserId: null,
|
||||
executionPolicy: policy,
|
||||
executionState: null,
|
||||
},
|
||||
policy,
|
||||
requestedStatus: "done",
|
||||
requestedAssigneePatch: {},
|
||||
actor: { agentId: coderAgentId },
|
||||
commentBody: "Implemented the migration",
|
||||
reviewRequest: { instructions: reviewInstructions },
|
||||
});
|
||||
|
||||
expect(result.patch.executionState).toMatchObject({
|
||||
status: "pending",
|
||||
currentStageType: "review",
|
||||
currentParticipant: { type: "agent", agentId: qaAgentId },
|
||||
reviewRequest: { instructions: reviewInstructions },
|
||||
});
|
||||
});
|
||||
|
||||
it("reviewer approves → advances to approval stage", () => {
|
||||
const reviewStageId = policy.stages[0].id;
|
||||
const result = applyIssueExecutionPolicyTransition({
|
||||
@@ -214,6 +246,44 @@ describe("issue execution policy transitions", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("lets a reviewer provide loose instructions for the next approval stage", () => {
|
||||
const reviewStageId = policy.stages[0].id;
|
||||
const approvalInstructions = "Please decide whether this is ready to ship, with any launch caveats.";
|
||||
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 },
|
||||
reviewRequest: { instructions: "Review the implementation details." },
|
||||
completedStageIds: [],
|
||||
lastDecisionId: null,
|
||||
lastDecisionOutcome: null,
|
||||
},
|
||||
},
|
||||
policy,
|
||||
requestedStatus: "done",
|
||||
requestedAssigneePatch: {},
|
||||
actor: { agentId: qaAgentId },
|
||||
commentBody: "QA signoff complete",
|
||||
reviewRequest: { instructions: approvalInstructions },
|
||||
});
|
||||
|
||||
expect(result.patch.executionState).toMatchObject({
|
||||
status: "pending",
|
||||
currentStageType: "approval",
|
||||
currentParticipant: { type: "user", userId: ctoUserId },
|
||||
reviewRequest: { instructions: approvalInstructions },
|
||||
});
|
||||
});
|
||||
|
||||
it("approver approves → marks completed (allows done)", () => {
|
||||
const reviewStageId = policy.stages[0].id;
|
||||
const approvalStageId = policy.stages[1].id;
|
||||
|
||||
@@ -101,6 +101,7 @@ type ExecutionStageWakeContext = {
|
||||
stageType: ParsedExecutionState["currentStageType"];
|
||||
currentParticipant: ParsedExecutionState["currentParticipant"];
|
||||
returnAssignee: ParsedExecutionState["returnAssignee"];
|
||||
reviewRequest: ParsedExecutionState["reviewRequest"];
|
||||
lastDecisionOutcome: ParsedExecutionState["lastDecisionOutcome"];
|
||||
allowedActions: string[];
|
||||
};
|
||||
@@ -124,6 +125,7 @@ function buildExecutionStageWakeContext(input: {
|
||||
stageType: input.state.currentStageType,
|
||||
currentParticipant: input.state.currentParticipant,
|
||||
returnAssignee: input.state.returnAssignee,
|
||||
reviewRequest: input.state.reviewRequest ?? null,
|
||||
lastDecisionOutcome: input.state.lastDecisionOutcome,
|
||||
allowedActions: input.allowedActions,
|
||||
};
|
||||
@@ -1788,6 +1790,7 @@ export function issueRoutes(
|
||||
: null;
|
||||
const {
|
||||
comment: commentBody,
|
||||
reviewRequest,
|
||||
reopen: reopenRequested,
|
||||
interrupt: interruptRequested,
|
||||
hiddenAt: hiddenAtRaw,
|
||||
@@ -1814,7 +1817,8 @@ export function issueRoutes(
|
||||
: false;
|
||||
let interruptedRunId: string | null = null;
|
||||
const closedExecutionWorkspace = await getClosedIssueExecutionWorkspace(existing);
|
||||
const isAgentWorkUpdate = req.actor.type === "agent" && Object.keys(updateFields).length > 0;
|
||||
const isAgentWorkUpdate =
|
||||
req.actor.type === "agent" && (Object.keys(updateFields).length > 0 || reviewRequest !== undefined);
|
||||
|
||||
if (closedExecutionWorkspace && (commentBody || isAgentWorkUpdate)) {
|
||||
respondClosedIssueExecutionWorkspace(res, closedExecutionWorkspace);
|
||||
@@ -1888,6 +1892,7 @@ export function issueRoutes(
|
||||
userId: actor.actorType === "user" ? actor.actorId : null,
|
||||
},
|
||||
commentBody,
|
||||
reviewRequest: reviewRequest ?? undefined,
|
||||
});
|
||||
const decisionId = transition.decision ? randomUUID() : null;
|
||||
if (decisionId) {
|
||||
@@ -1901,6 +1906,17 @@ export function issueRoutes(
|
||||
};
|
||||
}
|
||||
Object.assign(updateFields, transition.patch);
|
||||
if (reviewRequest !== undefined && transition.patch.executionState === undefined) {
|
||||
const existingExecutionState = parseIssueExecutionState(existing.executionState);
|
||||
if (!existingExecutionState || existingExecutionState.status !== "pending") {
|
||||
res.status(422).json({ error: "reviewRequest requires an active review or approval stage" });
|
||||
return;
|
||||
}
|
||||
updateFields.executionState = {
|
||||
...existingExecutionState,
|
||||
reviewRequest,
|
||||
};
|
||||
}
|
||||
|
||||
const nextAssigneeAgentId =
|
||||
updateFields.assigneeAgentId === undefined ? existing.assigneeAgentId : (updateFields.assigneeAgentId as string | null);
|
||||
|
||||
@@ -31,6 +31,7 @@ type TransitionInput = {
|
||||
requestedAssigneePatch: RequestedAssigneePatch;
|
||||
actor: ActorLike;
|
||||
commentBody?: string | null;
|
||||
reviewRequest?: IssueExecutionState["reviewRequest"] | null;
|
||||
};
|
||||
|
||||
type TransitionResult = {
|
||||
@@ -168,6 +169,7 @@ function buildCompletedState(previous: IssueExecutionState | null, currentStage:
|
||||
currentStageType: null,
|
||||
currentParticipant: null,
|
||||
returnAssignee: previous?.returnAssignee ?? null,
|
||||
reviewRequest: null,
|
||||
completedStageIds,
|
||||
lastDecisionId: previous?.lastDecisionId ?? null,
|
||||
lastDecisionOutcome: "approved",
|
||||
@@ -186,6 +188,7 @@ function buildStateWithCompletedStages(input: {
|
||||
currentStageType: input.previous?.currentStageType ?? null,
|
||||
currentParticipant: input.previous?.currentParticipant ?? null,
|
||||
returnAssignee: input.previous?.returnAssignee ?? input.returnAssignee,
|
||||
reviewRequest: input.previous?.reviewRequest ?? null,
|
||||
completedStageIds: input.completedStageIds,
|
||||
lastDecisionId: input.previous?.lastDecisionId ?? null,
|
||||
lastDecisionOutcome: input.previous?.lastDecisionOutcome ?? null,
|
||||
@@ -204,6 +207,7 @@ function buildSkippedStageCompletedState(input: {
|
||||
currentStageType: null,
|
||||
currentParticipant: null,
|
||||
returnAssignee: input.previous?.returnAssignee ?? input.returnAssignee,
|
||||
reviewRequest: null,
|
||||
completedStageIds: input.completedStageIds,
|
||||
lastDecisionId: input.previous?.lastDecisionId ?? null,
|
||||
lastDecisionOutcome: input.previous?.lastDecisionOutcome ?? null,
|
||||
@@ -216,6 +220,7 @@ function buildPendingState(input: {
|
||||
stageIndex: number;
|
||||
participant: IssueExecutionStagePrincipal;
|
||||
returnAssignee: IssueExecutionStagePrincipal | null;
|
||||
reviewRequest?: IssueExecutionState["reviewRequest"] | null;
|
||||
}): IssueExecutionState {
|
||||
return {
|
||||
status: PENDING_STATUS,
|
||||
@@ -224,6 +229,7 @@ function buildPendingState(input: {
|
||||
currentStageType: input.stage.type,
|
||||
currentParticipant: input.participant,
|
||||
returnAssignee: input.returnAssignee,
|
||||
reviewRequest: input.reviewRequest ?? null,
|
||||
completedStageIds: input.previous?.completedStageIds ?? [],
|
||||
lastDecisionId: input.previous?.lastDecisionId ?? null,
|
||||
lastDecisionOutcome: input.previous?.lastDecisionOutcome ?? null,
|
||||
@@ -236,6 +242,7 @@ function buildChangesRequestedState(previous: IssueExecutionState, currentStage:
|
||||
status: CHANGES_REQUESTED_STATUS,
|
||||
currentStageId: currentStage.id,
|
||||
currentStageType: currentStage.type,
|
||||
reviewRequest: null,
|
||||
lastDecisionOutcome: "changes_requested",
|
||||
};
|
||||
}
|
||||
@@ -247,6 +254,7 @@ function buildPendingStagePatch(input: {
|
||||
stage: IssueExecutionStage;
|
||||
participant: IssueExecutionStagePrincipal;
|
||||
returnAssignee: IssueExecutionStagePrincipal | null;
|
||||
reviewRequest?: IssueExecutionState["reviewRequest"] | null;
|
||||
}) {
|
||||
input.patch.status = "in_review";
|
||||
Object.assign(input.patch, patchForPrincipal(input.participant));
|
||||
@@ -256,6 +264,7 @@ function buildPendingStagePatch(input: {
|
||||
stageIndex: input.policy.stages.findIndex((candidate) => candidate.id === input.stage.id),
|
||||
participant: input.participant,
|
||||
returnAssignee: input.returnAssignee,
|
||||
reviewRequest: input.reviewRequest,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -359,6 +368,7 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra
|
||||
stage: activeStage,
|
||||
participant,
|
||||
returnAssignee: existingState?.returnAssignee ?? currentAssignee ?? actor,
|
||||
reviewRequest: input.reviewRequest ?? existingState?.reviewRequest ?? null,
|
||||
});
|
||||
return {
|
||||
patch,
|
||||
@@ -405,6 +415,7 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra
|
||||
stage: nextStage,
|
||||
participant,
|
||||
returnAssignee: existingState?.returnAssignee ?? currentAssignee ?? actor,
|
||||
reviewRequest: input.reviewRequest ?? null,
|
||||
});
|
||||
return {
|
||||
patch,
|
||||
@@ -461,6 +472,7 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra
|
||||
stage: activeStage,
|
||||
participant: currentParticipant,
|
||||
returnAssignee: existingState?.returnAssignee ?? currentAssignee ?? actor,
|
||||
reviewRequest: input.reviewRequest ?? existingState?.reviewRequest ?? null,
|
||||
});
|
||||
return {
|
||||
patch,
|
||||
@@ -538,6 +550,7 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra
|
||||
stage: pendingStage,
|
||||
participant,
|
||||
returnAssignee,
|
||||
reviewRequest: input.reviewRequest ?? null,
|
||||
});
|
||||
return {
|
||||
patch,
|
||||
|
||||
@@ -318,6 +318,7 @@ function createExecutionState(overrides: Partial<IssueExecutionState> = {}): Iss
|
||||
currentStageType: "review",
|
||||
currentParticipant: { type: "agent", agentId: "agent-1", userId: null },
|
||||
returnAssignee: { type: "agent", agentId: "agent-2", userId: null },
|
||||
reviewRequest: null,
|
||||
completedStageIds: [],
|
||||
lastDecisionId: null,
|
||||
lastDecisionOutcome: "changes_requested",
|
||||
|
||||
Reference in New Issue
Block a user