RomanNum3ral
/
paperclip
mirror of https://github.com/paperclipai/paperclip.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix(server): enforce review policy on interaction verdicts 

Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Dotta 2026-08-15 01:04:35 +00:00
parent 8ee1fb21a6
commit 37fde84abd
3 changed files with 135 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
server/src/__tests__/issue-execution-policy-routes.test.ts
View File

@ -394,6 +394,63 @@ describe("issue execution policy routes", () => {
);
});
it("binds a user-designated confirmation to the review transition activity", async () => {
const issue = {
id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
companyId: "company-1",
status: "todo",
assigneeAgentId: null,
assigneeUserId: "local-board",
createdByUserId: "local-board",
identifier: "PAP-1004",
title: "Pending confirmation",
executionPolicy: null,
executionState: null,
};
mockIssueService.getById.mockResolvedValue(issue);
mockIssueThreadInteractionService.listForIssue.mockResolvedValue([{
id: "11111111-1111-4111-8111-111111111111",
kind: "request_confirmation",
status: "pending",
createdByAgentId: null,
createdByUserId: "local-board",
sourceRunId: null,
payload: { version: 1, prompt: "Approve this review?" },
}]);
mockIssueService.update.mockImplementation(async (_id: string, patch: Record<string, unknown>) => ({
...issue,
...patch,
changes: { status: { from: "todo", to: "in_review" } },
updatedAt: new Date(),
}));
const res = await request(await createApp())
.patch("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa")
.send({
status: "in_review",
reviewInteractionId: "11111111-1111-4111-8111-111111111111",
});
expect(res.status).toBe(200);
expect(mockIssueService.update).toHaveBeenCalledWith(
"aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
expect.not.objectContaining({ reviewInteractionId: expect.anything() }),
expect.anything(),
);
expect(mockLogActivity).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
action: "issue.updated",
actorType: "user",
actorId: "local-board",
details: expect.objectContaining({
reviewInteractionId: "11111111-1111-4111-8111-111111111111",
}),
}),
expect.any(Array),
);
});
it("keeps a review transition and its confirmation binding in one rollback boundary", async () => {
const issue = {
id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",

48
server/src/__tests__/issue-thread-interaction-routes.test.ts
View File

@ -1910,6 +1910,54 @@ describe.sequential("issue thread interaction routes", () => {
expect(mockInteractionService.acceptInteraction).not.toHaveBeenCalled();
});
it.each([
{ action: "accept", body: {} },
{ action: "reject", body: { reason: "Needs changes" } },
])("rejects an agent $action verdict under human_only when a user review transition omitted the interaction binding", async ({ action, body }) => {
mockReviewTransition.value = {
actorType: "user",
actorId: "local-board",
details: { status: "in_review", _previous: { status: "in_progress" } },
};
mockIssueService.getById.mockResolvedValueOnce(createIssue({
status: "in_review",
reviewPolicy: "human_only",
createdByAgentId: null,
createdByUserId: "local-board",
}));
mockInteractionService.getForIssue.mockResolvedValueOnce({
id: "interaction-user-review-unbound",
kind: "request_confirmation",
status: "pending",
createdByAgentId: null,
createdByUserId: "local-board",
sourceRunId: null,
requestedResolverPolicy: "board_or_agents",
effectiveResolverPolicy: "board_or_agents",
payload: { version: 1, prompt: "Approve this review?" },
});
const app = await createApp({
type: "agent",
agentId: ASSIGNEE_AGENT_ID,
companyId: "company-1",
runId: "run-2",
});
const res = await request(app)
.post(`/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/interactions/interaction-user-review-unbound/${action}`)
.send(body);
expect(res.status).toBe(403);
expect(res.body).toMatchObject({
details: {
code: "review_policy_denied",
policy: "human_only",
},
});
expect(mockInteractionService.acceptInteraction).not.toHaveBeenCalled();
expect(mockInteractionService.rejectInteraction).not.toHaveBeenCalled();
});
it("allows only the addressed agent or board to resolve an addressed interaction", async () => {
const addressed = {
id: "interaction-addressed",

43
server/src/routes/issues.ts
View File

@ -3411,7 +3411,7 @@ export function issueRoutes(
);
}
async function assertAgentInReviewReviewPath(input: {
async function assertInReviewReviewPath(input: {
existing: {
id: string;
companyId: string;
@ -3421,7 +3421,8 @@ export function issueRoutes(
monitorNextCheckAt?: Date | null;
};
updateFields: Record<string, unknown>;
actorType: string;
actorType: "agent" | "user";
actorId: string;
actorAgentId?: string | null;
actorRunId?: string | null;
reviewInteractionId?: string;
@ -3429,7 +3430,8 @@ export function issueRoutes(
const nextStatus = typeof input.updateFields.status === "string"
? input.updateFields.status
: input.existing.status;
if (input.actorType !== "agent" || input.existing.status === "in_review" || nextStatus !== "in_review") return null;
if (input.existing.status === "in_review" || nextStatus !== "in_review") return null;
if (input.actorType !== "agent" && !input.reviewInteractionId) return null;
const interactions = await issueThreadInteractionService(db).listForIssue(input.existing.id);
const pendingInteractions = interactions.filter((interaction) => interaction.status === "pending");
@ -3437,8 +3439,12 @@ export function issueRoutes(
const designatedReviewConfirmation = pendingInteractions.find((interaction) =>
interaction.id === input.reviewInteractionId
&& (interaction.kind === "request_confirmation" || interaction.kind === "request_checkbox_confirmation")
&& interaction.createdByAgentId === input.actorAgentId
&& interaction.sourceRunId === input.actorRunId
&& (
input.actorType === "agent"
? interaction.createdByAgentId === input.actorAgentId
&& interaction.sourceRunId === input.actorRunId
: interaction.createdByUserId === input.actorId
)
&& !(
interaction.kind === "request_confirmation"
&& interaction.payload
@ -3448,7 +3454,10 @@ export function issueRoutes(
)
);
if (!designatedReviewConfirmation) {
throw unprocessable("reviewInteractionId must identify a pending non-tool confirmation created by this agent run", {
const creatorDescription = input.actorType === "agent"
? "this agent run"
: "this user";
throw unprocessable(`reviewInteractionId must identify a pending non-tool confirmation created by ${creatorDescription}`, {
code: "invalid_review_interaction",
reviewInteractionId: input.reviewInteractionId,
});
@ -3456,6 +3465,8 @@ export function issueRoutes(
return designatedReviewConfirmation.id;
}
if (input.actorType !== "agent") return null;
const nextAssigneeUserId = input.updateFields.assigneeUserId === undefined
? input.existing.assigneeUserId
: input.updateFields.assigneeUserId;
@ -4154,7 +4165,6 @@ export function issueRoutes(
}
if (await assertLowTrustControlPlaneDenied(req, res, issue.companyId, issue)) return false;
if (!(await assertAgentIssueMutationAllowed(req, res, issue))) return false;
const isReviewConfirmationVerdict = await isPendingReviewConfirmationVerdict(issue, interaction);
const payload = interaction.payload && typeof interaction.payload === "object"
? interaction.payload as { toolAction?: unknown }
: null;
@ -4162,9 +4172,10 @@ export function issueRoutes(
res.status(403).json({ error: "Tool-action confirmations are always board-only" });
return false;
}
await assertPendingReviewInteractionVerdictAllowed(req, issue, interaction);
const isReviewConfirmationVerdict = await isPendingReviewConfirmationVerdict(issue, interaction);
if (isReviewConfirmationVerdict) {
if (!assertAgentInteractionActorAllowed(res, interaction, actorAgentId, runId)) return false;
await assertPendingReviewInteractionVerdictAllowed(req, issue, interaction);
return "review_verdict" as const;
}
if (interaction.effectiveResolverPolicy !== "board_or_agents") {
@ -4238,11 +4249,15 @@ export function issueRoutes(
createdByAgentId?: string | null;
createdByUserId?: string | null;
},
interaction: { status: string },
interaction: { kind: string; status: string },
) {
if (
issue.status !== "in_review"
|| interaction.status !== "pending"
|| (
interaction.kind !== "request_confirmation"
&& interaction.kind !== "request_checkbox_confirmation"
)
|| issue.reviewPolicy == null
|| issue.reviewPolicy === "anyone"
) return;
@ -6415,10 +6430,11 @@ export function issueRoutes(
? "owner_completed"
: outcome;
const updateFields = sourceIssueStatus ? { status: sourceIssueStatus } : {};
await assertAgentInReviewReviewPath({
await assertInReviewReviewPath({
existing,
updateFields,
actorType: req.actor.type,
actorType: actor.actorType,
actorId: actor.actorId,
actorAgentId: actor.agentId,
actorRunId: actor.runId,
});
@ -9097,10 +9113,11 @@ export function issueRoutes(
}
}
const reviewInteractionId = await assertAgentInReviewReviewPath({
const reviewInteractionId = await assertInReviewReviewPath({
existing,
updateFields,
actorType: req.actor.type,
actorType: actor.actorType,
actorId: actor.actorId,
actorAgentId: actor.agentId,
actorRunId: actor.runId,
reviewInteractionId: requestedReviewInteractionId,