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

fix(server): add explicit review verdict policies (#10931) 

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Issues use `in_review` to request a final decision from an
authorized writer
> - The server rejected an assignee agent that tried to close its own
review, even when the issue had no independent-review rule
> - This rejection stopped the default agent workflow and did not
represent the configured execution-stage rules
> - Paperclip needs an open default and explicit issue-level constraints
for teams that require an independent or human verdict
> - This pull request removes the unconditional rejection and adds
`anyone`, `not_creator`, and `human_only` review policies
> - The benefit is a working default path with opt-in, authenticated
verdict controls

## Linked Issues or Issue Description

Refs #10635, #4429, and #10671.

The related public work covers execution-stage independence,
self-approval fallback behavior, and durable review paths. This change
is distinct. It controls who can resolve an issue review verdict. It
keeps configured execution stages active.

## What Changed

- Added a nullable `review_policy` issue column. Null has the same
meaning as `anyone`. The migration does not backfill existing issues.
- Added shared create, update, response, and compact issue contracts for
`anyone`, `not_creator`, and `human_only`.
- Removed the unconditional agent self-approval rejection for
`in_review` issues.
- Added one reusable verdict-actor check for terminal status changes and
pending interaction accept or reject actions.
- Used the authenticated principal type for `human_only`. Agent keys and
run tokens remain agent principals.
- Used the latest transition into `in_review` to identify the requester
for `not_creator`.
- Added actionable 403 responses that name the policy, the allowed
actor, and the next step.
- Kept the configured execution-stage transition and signoff behavior.
- Added focused contract, helper, status-route, interaction-route, and
execution-stage regression tests.
- Updated the implementation specification for the new issue field.

## Verification

- `pnpm exec vitest run packages/shared/src/validators/issue.test.ts
server/src/__tests__/issue-review-policy.test.ts
server/src/__tests__/issue-stalled-review-decision-routes.test.ts
--reporter=dot` passed: 42 tests.
- `pnpm --filter @paperclipai/shared typecheck` passed.
- `pnpm --filter @paperclipai/db typecheck` passed, including migration
numbering and safety checks.
- `pnpm --filter @paperclipai/server typecheck` passed.
- `pnpm run typecheck:build-gaps` passed across server, CLI, plugin
SDK/examples, plugin wiki, and UI.
- `git diff --check origin/master...HEAD` passed.
- SecurityEngineer review approved the authenticated-principal checks
and accepted policy-relaxation tradeoff with no required changes.
- Greptile reviewed the latest head at 5/5 with zero inline comments or
follow-ups.
- The latest-head GitHub rollup passed build, typecheck,
server/workspace tests, serialized suites, canary, e2e, and external
security checks.

## Risks

- The migration adds one nullable text column. It has no default and no
backfill.
- `not_creator` reads the latest recorded transition into `in_review`.
It denies the verdict when it cannot identify the requester.
- Agents can change or relax `reviewPolicy` when they have issue write
access. This is intentional for this issue-level control.
- Null and `anyone` do not add a database query to the verdict path.
- Configured execution-stage checks still run after the issue-level
policy check.

> This work aligns with the completed "Agent Reviews and Approvals" and
"Enforced Outcomes" roadmap items. It does not add a new roadmap
capability.

## Model Used

- OpenAI Codex, GPT-5. The exact deployment ID and context-window size
are not exposed to the agent. The run used reasoning, repository tools,
code execution, and GitHub CLI access.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Dotta 2026-08-05 23:12:41 -05:00 committed by GitHub
parent 5b62a3883f
commit f554d67377
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
38 changed files with 39201 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
doc/SPEC-implementation.md
View File

@ -230,6 +230,7 @@ Routine execution issues add a routine-scoped env overlay after project env and
- `description` text null
- `status` enum: `backlog | todo | in_progress | in_review | done | blocked | cancelled`
- `priority` enum: `critical | high | medium | low`
- `review_policy` nullable enum: `anyone | not_creator | human_only`; null is equivalent to `anyone`
- `assignee_agent_id` uuid fk `agents.id` null
- `assignee_user_id` text null
- checkout/execution locks: `checkout_run_id`, `execution_run_id`, `execution_agent_name_key`, `execution_locked_at`

1
packages/db/src/migrations/0208_keen_sharon_carter.sql Normal file
View File

@ -0,0 +1 @@
ALTER TABLE "issues" ADD COLUMN "review_policy" text;

38595
packages/db/src/migrations/meta/0208_snapshot.json Normal file
View File

File diff suppressed because it is too large Load Diff

9
packages/db/src/migrations/meta/_journal.json
View File

@ -1443,6 +1443,13 @@
"when": 1785903493352,
"tag": "0207_moaning_amazoness",
"breakpoints": true
},
{
"idx": 208,
"version": "7",
"when": 1785988680096,
"tag": "0208_keen_sharon_carter",
"breakpoints": true
}
]
}
}

4
packages/db/src/schema/issues.ts
View File

@ -17,8 +17,7 @@ import { companies } from "./companies.js";
import { heartbeatRuns } from "./heartbeat_runs.js";
import { projectWorkspaces } from "./project_workspaces.js";
import { executionWorkspaces } from "./execution_workspaces.js";
import type { SourceTrustMetadata } from "@paperclipai/shared";
import type { IssueUnblockDescriptor } from "@paperclipai/shared";
import type { IssueReviewPolicy, IssueUnblockDescriptor, SourceTrustMetadata } from "@paperclipai/shared";
export const issues = pgTable(
"issues",
@ -35,6 +34,7 @@ export const issues = pgTable(
workMode: text("work_mode").notNull().default("standard"),
harnessKind: text("harness_kind"),
priority: text("priority").notNull().default("medium"),
reviewPolicy: text("review_policy").$type<IssueReviewPolicy>(),
assigneeAgentId: uuid("assignee_agent_id").references(() => agents.id),
assigneeUserId: text("assignee_user_id"),
checkoutRunId: uuid("checkout_run_id").references(() => heartbeatRuns.id, { onDelete: "set null" }),

1
packages/plugins/plugin-llm-wiki/tests/plugin.spec.ts
View File

@ -571,6 +571,7 @@ function paperclipIssue(overrides: Partial<Issue> = {}): Issue {
status: "todo",
workMode: "standard",
priority: "medium",
reviewPolicy: null,
assigneeAgentId: null,
assigneeUserId: null,
responsibleUserId: null,

1
packages/plugins/sdk/src/testing.ts
View File

@ -1603,6 +1603,7 @@ export function createTestHarness(options: TestHarnessOptions): TestHarness {
status: input.status ?? "todo",
workMode: "standard",
priority: input.priority ?? "medium",
reviewPolicy: null,
assigneeAgentId: input.assigneeAgentId ?? null,
assigneeUserId: input.assigneeUserId ?? null,
checkoutRunId: null,

2
packages/shared/src/constants.ts
View File

@ -213,6 +213,8 @@ export const INBOX_MINE_ISSUE_STATUS_FILTER = INBOX_MINE_ISSUE_STATUSES.join(","
export const ISSUE_PRIORITIES = ["critical", "high", "medium", "low"] as const;
export type IssuePriority = (typeof ISSUE_PRIORITIES)[number];
export const ISSUE_REVIEW_POLICIES = ["anyone", "not_creator", "human_only"] as const;
export type IssueReviewPolicy = (typeof ISSUE_REVIEW_POLICIES)[number];
export const ISSUE_WORK_MODES = ["standard", "ask", "planning", "skill_test"] as const;
export type IssueWorkMode = (typeof ISSUE_WORK_MODES)[number];
export const ISSUE_HARNESS_KINDS = ["skill_test"] as const;

2
packages/shared/src/index.ts
View File

@ -270,6 +270,7 @@ export {
INBOX_MINE_ISSUE_STATUSES,
INBOX_MINE_ISSUE_STATUS_FILTER,
ISSUE_PRIORITIES,
ISSUE_REVIEW_POLICIES,
ISSUE_WORK_MODES,
ISSUE_HARNESS_KINDS,
MAX_ISSUE_REQUEST_DEPTH,
@ -457,6 +458,7 @@ export {
type ProjectIconName,
type IssueStatus,
type IssuePriority,
type IssueReviewPolicy,
type IssueWorkMode,
type IssueHarnessKind,
type SummarySlotScopeKind,

4
packages/shared/src/types/issue.ts
View File

@ -17,6 +17,7 @@ import type {
IssueHarnessKind,
IssueOriginKind,
IssuePriority,
IssueReviewPolicy,
IssueRecoveryActionKind,
IssueRecoveryActionOutcome,
IssueRecoveryActionOwnerType,
@ -151,6 +152,7 @@ export interface AcceptedPlanDecompositionChild {
workMode: IssueWorkMode;
harnessKind?: IssueHarnessKind | null;
priority: IssuePriority;
reviewPolicy?: IssueReviewPolicy | null;
assigneeAgentId?: string | null;
assigneeUserId?: string | null;
requestDepth?: number;
@ -780,6 +782,7 @@ export interface Issue {
status: IssueStatus;
workMode: IssueWorkMode;
priority: IssuePriority;
reviewPolicy: IssueReviewPolicy | null;
assigneeAgentId: string | null;
assigneeUserId: string | null;
checkoutRunId: string | null;
@ -864,6 +867,7 @@ export type CompactIssue = Pick<
| "status"
| "workMode"
| "priority"
| "reviewPolicy"
| "assigneeAgentId"
| "assigneeUserId"
| "checkoutRunId"

9
packages/shared/src/validators/issue.test.ts
View File

@ -48,6 +48,15 @@ describe("issue validators", () => {
.toBeUndefined();
});
it("accepts review policies on create and update while rejecting unknown values", () => {
expect(createIssueSchema.parse({ title: "Human review", reviewPolicy: "human_only" }).reviewPolicy)
.toBe("human_only");
expect(updateIssueSchema.parse({ reviewPolicy: "not_creator" }).reviewPolicy)
.toBe("not_creator");
expect(updateIssueSchema.parse({ reviewPolicy: null }).reviewPolicy).toBeNull();
expect(updateIssueSchema.safeParse({ reviewPolicy: "creator_only" }).success).toBe(false);
});
it("normalizes JSON-escaped line breaks in issue descriptions", () => {
const parsed = createIssueSchema.parse({
title: "Follow up PR",

2
packages/shared/src/validators/issue.ts
View File

@ -20,6 +20,7 @@ import {
ISSUE_RECOVERY_ACTION_OUTCOMES,
ISSUE_RECOVERY_ACTION_OWNER_TYPES,
ISSUE_RECOVERY_ACTION_STATUSES,
ISSUE_REVIEW_POLICIES,
ISSUE_WORK_MODES,
clampIssueRequestDepth,
ISSUE_STATUSES,
@ -446,6 +447,7 @@ const createIssueBaseSchema = z.object({
workMode: z.enum(ISSUE_WORK_MODES).optional().default("standard"),
harnessKind: z.enum(ISSUE_HARNESS_KINDS).optional().nullable(),
priority: z.enum(ISSUE_PRIORITIES).optional().default("medium"),
reviewPolicy: z.enum(ISSUE_REVIEW_POLICIES).optional().nullable(),
assigneeAgentId: z.string().uuid().optional().nullable(),
assigneeUserId: z.string().optional().nullable(),
requestDepth: issueRequestDepthInputSchema.optional().default(0),

186
server/src/__tests__/issue-review-policy.test.ts Normal file
View File

@ -0,0 +1,186 @@
import { randomUUID } from "node:crypto";
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
import { activityLog, agents, companies, createDb, issues, type Db } from "@paperclipai/db";
import { HttpError } from "../errors.js";
import { assertIssueReviewVerdictActorAllowed } from "../services/issue-review-policy.js";
import {
getEmbeddedPostgresTestSupport,
startEmbeddedPostgresTestDatabase,
} from "./helpers/embedded-postgres.js";
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
describeEmbeddedPostgres("issue review verdict policy", () => {
let db!: ReturnType<typeof createDb>;
let tempDb: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | null = null;
beforeAll(async () => {
tempDb = await startEmbeddedPostgresTestDatabase("paperclip-issue-review-policy-");
db = createDb(tempDb.connectionString);
}, 30_000);
afterEach(async () => {
await db.delete(activityLog);
await db.delete(issues);
await db.delete(agents);
await db.delete(companies);
});
afterAll(async () => {
await tempDb?.cleanup();
});
async function seedReview(policy: "not_creator" | "human_only") {
const companyId = randomUUID();
const requesterAgentId = randomUUID();
const peerAgentId = randomUUID();
const issueId = randomUUID();
await db.insert(companies).values({
id: companyId,
name: "Review Policy Company",
issuePrefix: "RPC",
requireBoardApprovalForNewAgents: false,
});
await db.insert(agents).values([
{
id: requesterAgentId,
companyId,
name: "Requester",
role: "engineer",
status: "idle",
adapterType: "codex_local",
adapterConfig: {},
runtimeConfig: {},
permissions: {},
},
{
id: peerAgentId,
companyId,
name: "Peer reviewer",
role: "engineer",
status: "idle",
adapterType: "codex_local",
adapterConfig: {},
runtimeConfig: {},
permissions: {},
},
]);
const [issue] = await db.insert(issues).values({
id: issueId,
companyId,
title: "Review this",
status: "in_review",
priority: "medium",
reviewPolicy: policy,
createdByAgentId: requesterAgentId,
}).returning();
return { issue, companyId, requesterAgentId, peerAgentId };
}
it("does no database work for the default anyone policy", async () => {
const noQueryDb = new Proxy({}, {
get() {
throw new Error("default policy must not query");
},
}) as Db;
await expect(assertIssueReviewVerdictActorAllowed(noQueryDb, {
issue: { id: randomUUID(), companyId: randomUUID(), reviewPolicy: null },
actor: { type: "agent", id: randomUUID() },
})).resolves.toBeUndefined();
});
it("blocks the in-review requester under not_creator and admits another agent", async () => {
const seeded = await seedReview("not_creator");
expect(seeded.issue.reviewPolicy).toBe("not_creator");
await db.insert(activityLog).values({
companyId: seeded.companyId,
actorType: "agent",
actorId: seeded.requesterAgentId,
agentId: seeded.requesterAgentId,
action: "issue.updated",
entityType: "issue",
entityId: seeded.issue.id,
details: { status: "in_review", _previous: { status: "in_progress" } },
});
const denied = assertIssueReviewVerdictActorAllowed(db, {
issue: seeded.issue,
actor: { type: "agent", id: seeded.requesterAgentId },
});
await expect(denied).rejects.toMatchObject<HttpError>({
status: 403,
details: expect.objectContaining({
code: "review_policy_denied",
policy: "not_creator",
allowedActor: "writer_other_than_review_requester",
}),
});
await expect(assertIssueReviewVerdictActorAllowed(db, {
issue: seeded.issue,
actor: { type: "agent", id: seeded.peerAgentId },
})).resolves.toBeUndefined();
});
it("ignores later in-review snapshots that did not record a status transition", async () => {
const seeded = await seedReview("not_creator");
await db.insert(activityLog).values([
{
companyId: seeded.companyId,
actorType: "agent",
actorId: seeded.requesterAgentId,
agentId: seeded.requesterAgentId,
action: "issue.updated",
entityType: "issue",
entityId: seeded.issue.id,
details: { status: "in_review", _previous: { status: "in_progress" } },
createdAt: new Date("2026-08-06T00:00:00.000Z"),
},
{
companyId: seeded.companyId,
actorType: "agent",
actorId: seeded.peerAgentId,
agentId: seeded.peerAgentId,
action: "issue.updated",
entityType: "issue",
entityId: seeded.issue.id,
details: { status: "in_review", priority: "high" },
createdAt: new Date("2026-08-06T00:01:00.000Z"),
},
]);
await expect(assertIssueReviewVerdictActorAllowed(db, {
issue: seeded.issue,
actor: { type: "agent", id: seeded.requesterAgentId },
})).rejects.toMatchObject<HttpError>({
status: 403,
details: expect.objectContaining({ code: "review_policy_denied" }),
});
await expect(assertIssueReviewVerdictActorAllowed(db, {
issue: seeded.issue,
actor: { type: "agent", id: seeded.peerAgentId },
})).resolves.toBeUndefined();
});
it("uses authenticated principal type for human_only", async () => {
const seeded = await seedReview("human_only");
expect(seeded.issue.reviewPolicy).toBe("human_only");
const denied = assertIssueReviewVerdictActorAllowed(db, {
issue: seeded.issue,
actor: { type: "agent", id: seeded.requesterAgentId },
});
await expect(denied).rejects.toMatchObject<HttpError>({
status: 403,
details: expect.objectContaining({
code: "review_policy_denied",
policy: "human_only",
allowedActor: "authenticated_user_with_issue_write_access",
}),
});
await expect(assertIssueReviewVerdictActorAllowed(db, {
issue: seeded.issue,
actor: { type: "user", id: "board-user" },
})).resolves.toBeUndefined();
});
});

212
server/src/__tests__/issue-stalled-review-decision-routes.test.ts
View File

@ -71,6 +71,7 @@ describeEmbeddedPostgres("stalled review decision routes", () => {
const assigneeAgentId = randomUUID();
const peerAgentId = randomUUID();
const memberUserId = `${prefix.toLowerCase()}-member`;
const peerUserId = `${prefix.toLowerCase()}-peer`;
const viewerUserId = `${prefix.toLowerCase()}-viewer`;
await db.insert(companies).values({
id: companyId,
@ -110,6 +111,13 @@ describeEmbeddedPostgres("stalled review decision routes", () => {
status: "active",
membershipRole: "operator",
},
{
companyId,
principalType: "user",
principalId: peerUserId,
status: "active",
membershipRole: "operator",
},
{
companyId,
principalType: "user",
@ -118,7 +126,7 @@ describeEmbeddedPostgres("stalled review decision routes", () => {
membershipRole: "viewer",
},
]);
return { companyId, assigneeAgentId, peerAgentId, memberUserId, viewerUserId };
return { companyId, assigneeAgentId, peerAgentId, memberUserId, peerUserId, viewerUserId };
}
async function seedReview(input: {
@ -127,6 +135,7 @@ describeEmbeddedPostgres("stalled review decision routes", () => {
identifier: string;
status?: string;
covered?: boolean;
reviewPolicy?: "anyone" | "not_creator" | "human_only" | null;
}) {
const issueId = randomUUID();
await db.insert(issues).values({
@ -137,6 +146,7 @@ describeEmbeddedPostgres("stalled review decision routes", () => {
status: input.status ?? "in_review",
priority: "medium",
assigneeAgentId: input.assigneeAgentId,
reviewPolicy: input.reviewPolicy ?? null,
});
if (input.covered) {
await db.insert(issueThreadInteractions).values({
@ -176,16 +186,30 @@ describeEmbeddedPostgres("stalled review decision routes", () => {
};
}
function agentActor(companyId: string, agentId: string) {
function agentActor(companyId: string, agentId: string, runId = randomUUID()) {
return {
type: "agent",
source: "agent_key",
companyId,
agentId,
runId: randomUUID(),
runId,
};
}
async function seedRun(companyId: string, agentId: string, issueId: string) {
const runId = randomUUID();
await db.insert(heartbeatRuns).values({
id: runId,
companyId,
agentId,
invocationSource: "assignment",
triggerDetail: "system",
status: "running",
contextSnapshot: { issueId, wakeReason: "issue_assigned" },
});
return runId;
}
it("denies agents, viewers, and cross-company users without exposing issue existence", async () => {
const primary = await seedCompany("SRD");
const foreign = await seedCompany("FRN");
@ -219,10 +243,12 @@ describeEmbeddedPostgres("stalled review decision routes", () => {
.expect(404);
expect(crossCompany.body).toEqual(missing.body);
await request(app(agentActor(primary.companyId, primary.assigneeAgentId)))
const selfRunId = await seedRun(primary.companyId, primary.assigneeAgentId, issueId);
const selfApproval = await request(app(agentActor(primary.companyId, primary.assigneeAgentId, selfRunId)))
.patch(`/api/issues/${issueId}`)
.send({ status: "done" })
.expect(403, { error: "Agents cannot approve their own in-review work" });
.send({ status: "done" });
expect(selfApproval.status, JSON.stringify(selfApproval.body)).toBe(200);
expect(selfApproval.body).toMatchObject({ id: issueId, status: "done" });
});
it("still lets the pending execution-policy stage participant sign off as done", async () => {
@ -235,10 +261,21 @@ describeEmbeddedPostgres("stalled review decision routes", () => {
assigneeAgentId: seeded.assigneeAgentId,
identifier: "SGN-1",
});
const stageId = randomUUID();
await db.update(issues).set({
executionPolicy: {
mode: "normal",
commentRequired: true,
stages: [{
id: stageId,
type: "review",
approvalsNeeded: 1,
participants: [{ id: randomUUID(), type: "agent", agentId: seeded.assigneeAgentId }],
}],
},
executionState: {
status: "pending",
currentStageId: randomUUID(),
currentStageId: stageId,
currentStageIndex: 0,
currentStageType: "review",
currentParticipant: { type: "agent", agentId: seeded.assigneeAgentId },
@ -248,12 +285,169 @@ describeEmbeddedPostgres("stalled review decision routes", () => {
lastDecisionOutcome: null,
},
}).where(eq(issues.id, issueId));
const stageRunId = await seedRun(seeded.companyId, seeded.assigneeAgentId, issueId);
const res = await request(app(agentActor(seeded.companyId, seeded.assigneeAgentId)))
const res = await request(app(agentActor(seeded.companyId, seeded.assigneeAgentId, stageRunId)))
.patch(`/api/issues/${issueId}`)
.send({ status: "done", comment: "Stage signoff." });
expect(res.body?.error).not.toBe("Agents cannot approve their own in-review work");
expect(res.status, JSON.stringify(res.body)).toBe(200);
expect(res.body).toMatchObject({ id: issueId, status: "done" });
});
it("enforces not_creator for status verdicts and admits another agent", async () => {
const seeded = await seedCompany("NCR");
const issueId = await seedReview({
companyId: seeded.companyId,
assigneeAgentId: seeded.assigneeAgentId,
identifier: "NCR-1",
reviewPolicy: "not_creator",
});
await db.insert(activityLog).values({
companyId: seeded.companyId,
actorType: "agent",
actorId: seeded.assigneeAgentId,
agentId: seeded.assigneeAgentId,
action: "issue.updated",
entityType: "issue",
entityId: issueId,
details: { status: "in_review", _previous: { status: "in_progress" } },
});
const requesterVerdict = await request(app(agentActor(seeded.companyId, seeded.assigneeAgentId)))
.patch(`/api/issues/${issueId}`)
.send({ status: "done" });
expect(requesterVerdict.status).toBe(403);
expect(requesterVerdict.body).toMatchObject({
error: expect.stringContaining("someone other than"),
details: {
code: "review_policy_denied",
policy: "not_creator",
allowedActor: "writer_other_than_review_requester",
remediation: expect.stringContaining("another writer"),
},
});
const peerRunId = await seedRun(seeded.companyId, seeded.peerAgentId, issueId);
const peerVerdict = await request(app(agentActor(seeded.companyId, seeded.peerAgentId, peerRunId)))
.patch(`/api/issues/${issueId}`)
.send({ status: "done" });
expect(peerVerdict.status, JSON.stringify(peerVerdict.body)).toBe(200);
expect(peerVerdict.body).toMatchObject({ id: issueId, status: "done" });
});
it("enforces human_only from the authenticated principal and admits a user", async () => {
const seeded = await seedCompany("HUM");
const issueId = await seedReview({
companyId: seeded.companyId,
assigneeAgentId: seeded.assigneeAgentId,
identifier: "HUM-1",
reviewPolicy: "human_only",
});
const agentVerdict = await request(app(agentActor(seeded.companyId, seeded.assigneeAgentId)))
.patch(`/api/issues/${issueId}`)
.send({ status: "cancelled" });
expect(agentVerdict.status).toBe(403);
expect(agentVerdict.body).toMatchObject({
error: expect.stringContaining("authenticated user"),
details: {
code: "review_policy_denied",
policy: "human_only",
allowedActor: "authenticated_user_with_issue_write_access",
remediation: expect.stringContaining("authenticated user"),
},
});
const userVerdict = await request(app(boardActor(seeded.companyId, seeded.memberUserId)))
.patch(`/api/issues/${issueId}`)
.send({ status: "cancelled" });
expect(userVerdict.status, JSON.stringify(userVerdict.body)).toBe(200);
expect(userVerdict.body).toMatchObject({ id: issueId, status: "cancelled" });
});
it("allows an agent writer to relax reviewPolicy in the verdict patch", async () => {
const seeded = await seedCompany("RLP");
const issueId = await seedReview({
companyId: seeded.companyId,
assigneeAgentId: seeded.assigneeAgentId,
identifier: "RLP-1",
reviewPolicy: "human_only",
});
const runId = await seedRun(seeded.companyId, seeded.assigneeAgentId, issueId);
const verdict = await request(app(agentActor(seeded.companyId, seeded.assigneeAgentId, runId)))
.patch(`/api/issues/${issueId}`)
.send({ status: "done", reviewPolicy: "anyone" });
expect(verdict.status, JSON.stringify(verdict.body)).toBe(200);
expect(verdict.body).toMatchObject({ id: issueId, status: "done", reviewPolicy: "anyone" });
});
it("enforces not_creator when accepting or rejecting pending review interactions", async () => {
const seeded = await seedCompany("INT");
const issueId = await seedReview({
companyId: seeded.companyId,
assigneeAgentId: seeded.assigneeAgentId,
identifier: "INT-1",
reviewPolicy: "not_creator",
});
await db.insert(activityLog).values({
companyId: seeded.companyId,
actorType: "user",
actorId: seeded.memberUserId,
action: "issue.updated",
entityType: "issue",
entityId: issueId,
details: { status: "in_review", _previous: { status: "in_progress" } },
});
await db.update(issues).set({ assigneeAgentId: null }).where(eq(issues.id, issueId));
const interactions = await db.insert(issueThreadInteractions).values([
{
companyId: seeded.companyId,
issueId,
kind: "request_confirmation",
status: "pending",
continuationPolicy: "none",
payload: { version: 1, prompt: "Accept this review?" },
},
{
companyId: seeded.companyId,
issueId,
kind: "request_confirmation",
status: "pending",
continuationPolicy: "none",
payload: { version: 1, prompt: "Reject this review?" },
},
]).returning();
const [acceptInteraction, rejectInteraction] = interactions;
for (const [interactionId, action] of [
[acceptInteraction.id, "accept"],
[rejectInteraction.id, "reject"],
] as const) {
const blocked = await request(app(boardActor(seeded.companyId, seeded.memberUserId)))
.post(`/api/issues/${issueId}/interactions/${interactionId}/${action}`)
.send(action === "reject" ? { reason: "Not yet" } : {});
expect(blocked.status).toBe(403);
expect(blocked.body.details).toMatchObject({
code: "review_policy_denied",
policy: "not_creator",
allowedActor: "writer_other_than_review_requester",
});
}
const accepted = await request(app(boardActor(seeded.companyId, seeded.peerUserId)))
.post(`/api/issues/${issueId}/interactions/${acceptInteraction.id}/accept`)
.send({});
expect(accepted.status, JSON.stringify(accepted.body)).toBe(200);
expect(accepted.body).toMatchObject({ id: acceptInteraction.id, status: "accepted" });
const rejected = await request(app(boardActor(seeded.companyId, seeded.peerUserId)))
.post(`/api/issues/${issueId}/interactions/${rejectInteraction.id}/reject`)
.send({ reason: "Needs revision" });
expect(rejected.status, JSON.stringify(rejected.body)).toBe(200);
expect(rejected.body).toMatchObject({ id: rejectInteraction.id, status: "rejected" });
});
it("persists request-changes notes as attributed comments and only wakes with a typed reference", async () => {

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

@ -89,6 +89,7 @@ import {
type IssueWakeDiagnosticWakeRequest,
type IssueWakeDiagnosticsResponse,
type IssueRelationIssueSummary,
type IssueReviewPolicy,
type IssueCommentPresentation,
type IssueWatchdogDiscoveryKind,
type ProjectWorkspace,
@ -214,6 +215,7 @@ import {
} from "../services/trust-preset-resolver.js";
import { externalObjectService } from "../services/external-objects.js";
import { deliverAgentUnblockNotification } from "../services/routable-blocked.js";
import { assertIssueReviewVerdictActorAllowed } from "../services/issue-review-policy.js";
import {
crossIssueInfluenceLimitError,
crossIssueInfluenceRunContextError,
@ -1743,17 +1745,6 @@ async function assertCanManageIssueMonitor(
throw forbidden("Only the assignee agent or a board user can manage issue monitors");
}
// True when the agent is the participant of the currently pending execution-policy
// stage. Such an agent owns the stage's signoff, so its `in_review -> done` PATCH is
// a stage advance rather than a self-approval of its own work.
function isPendingExecutionStageParticipant(executionState: unknown, agentId: string | null | undefined) {
if (!agentId) return false;
const state = parseIssueExecutionState(executionState);
if (state?.status !== "pending") return false;
const participant = state.currentParticipant;
return participant?.type === "agent" && participant.agentId === agentId;
}
function summarizeIssueMonitor(
issue: {
monitorNextCheckAt?: Date | null;
@ -2254,6 +2245,7 @@ function toCompactIssue(issue: any): CompactIssue {
status: issue.status,
workMode: issue.workMode,
priority: issue.priority,
reviewPolicy: issue.reviewPolicy,
assigneeAgentId: issue.assigneeAgentId,
assigneeUserId: issue.assigneeUserId,
checkoutRunId: issue.checkoutRunId,
@ -4095,6 +4087,31 @@ export function issueRoutes(
return true;
}
async function assertPendingReviewInteractionVerdictAllowed(
req: Request,
issue: {
id: string;
companyId: string;
status: string;
reviewPolicy?: IssueReviewPolicy | null;
createdByAgentId?: string | null;
createdByUserId?: string | null;
},
interaction: { status: string },
) {
if (
issue.status !== "in_review"
|| interaction.status !== "pending"
|| issue.reviewPolicy == null
|| issue.reviewPolicy === "anyone"
) return;
const actor = getActorInfo(req);
await assertIssueReviewVerdictActorAllowed(db, {
issue,
actor: { type: actor.actorType, id: actor.actorId },
});
}
async function assertIssueThreadInteractionWithdrawalAllowed(
req: Request,
res: Response,
@ -8321,21 +8338,6 @@ export function issueRoutes(
const existing = await getAccessibleResource(req, res, svc.getById(id), "Issue not found");
if (!existing) return;
assertNoAgentHostWorkspaceCommandMutation(req, collectIssueWorkspaceCommandPaths(req.body));
// An agent may not rubber-stamp its own `in_review` work as `done` — approving
// is the board's/reviewer's call (PAP-16080 §4.4). Execution-policy signoff is
// the explicit exception: there, the policy reassigns the issue to each stage's
// participant, so the current reviewer/approver *is* the assignee and their
// `done` PATCH is a stage advance governed by
// `applyIssueExecutionPolicyTransition`, not a self-approval.
if (
req.actor.type === "agent"
&& existing.status === "in_review"
&& req.body.status === "done"
&& existing.assigneeAgentId === req.actor.agentId
&& !isPendingExecutionStageParticipant(existing.executionState, req.actor.agentId)
) {
throw forbidden("Agents cannot approve their own in-review work");
}
if (req.actor.type === "agent" && req.body.onBehalfOfUserId != null) {
await auditAgentIssueCommentAttributionSpoof({
db,
@ -8382,6 +8384,21 @@ export function issueRoutes(
onBehalfOfUserId: _requestedOnBehalfOfUserId,
...updateFields
} = req.body;
const effectiveReviewPolicy = req.body.reviewPolicy === undefined
? existing.reviewPolicy
: req.body.reviewPolicy;
if (
existing.status === "in_review"
&& (updateFields.status === "done" || updateFields.status === "cancelled")
&& effectiveReviewPolicy != null
&& effectiveReviewPolicy !== "anyone"
) {
await assertIssueReviewVerdictActorAllowed(db, {
issue: existing,
actor: { type: actor.actorType, id: actor.actorId },
reviewPolicy: effectiveReviewPolicy,
});
}
const shouldCancelActiveRunForCancelledStatus =
existing.status !== "cancelled" && updateFields.status === "cancelled";
if (resumeRequested === true && !commentBody) {
@ -10129,6 +10146,7 @@ export function issueRoutes(
const interactionSvc = issueThreadInteractionService(db);
const current = await interactionSvc.getForIssue(issue, interactionId);
if (!(await assertIssueThreadInteractionResolutionAllowed(req, res, issue, current))) return;
await assertPendingReviewInteractionVerdictAllowed(req, issue, current);
const actor = getActorInfo(req);
const { interaction, createdIssues, continuationIssue } = await interactionSvc.acceptInteraction(issue, interactionId, req.body, {
@ -10281,6 +10299,7 @@ export function issueRoutes(
const interactionSvc = issueThreadInteractionService(db);
const current = await interactionSvc.getForIssue(issue, interactionId);
if (!(await assertIssueThreadInteractionResolutionAllowed(req, res, issue, current))) return;
await assertPendingReviewInteractionVerdictAllowed(req, issue, current);
const actor = getActorInfo(req);
const interaction = await interactionSvc.rejectInteraction(issue, interactionId, req.body, {

4
server/src/services/index.ts
View File

@ -41,6 +41,10 @@ export {
type IssueFilters,
} from "./issues.js";
export { issueThreadInteractionService } from "./issue-thread-interactions.js";
export {
assertIssueReviewVerdictActorAllowed,
type IssueReviewVerdictActor,
} from "./issue-review-policy.js";
export { issueTreeControlService } from "./issue-tree-control.js";
export { issueApprovalService } from "./issue-approvals.js";
export { issueReferenceService } from "./issue-references.js";

111
server/src/services/issue-review-policy.ts Normal file
View File

@ -0,0 +1,111 @@
import { and, desc, eq, sql } from "drizzle-orm";
import { activityLog, type Db } from "@paperclipai/db";
import type { IssueReviewPolicy } from "@paperclipai/shared";
import { forbidden } from "../errors.js";
export interface IssueReviewVerdictActor {
type: "agent" | "user";
id: string;
}
interface ReviewPolicyIssue {
id: string;
companyId: string;
reviewPolicy?: IssueReviewPolicy | null;
createdByAgentId?: string | null;
createdByUserId?: string | null;
}
async function findReviewRequester(
db: Db,
issue: ReviewPolicyIssue,
): Promise<IssueReviewVerdictActor | null> {
const transition = await db
.select({
actorType: activityLog.actorType,
actorId: activityLog.actorId,
})
.from(activityLog)
.where(and(
eq(activityLog.companyId, issue.companyId),
eq(activityLog.entityType, "issue"),
eq(activityLog.entityId, issue.id),
eq(activityLog.action, "issue.updated"),
sql`(
(
${activityLog.details} ->> 'status' = 'in_review'
AND ${activityLog.details} -> '_previous' ->> 'status' IS NOT NULL
AND ${activityLog.details} -> '_previous' ->> 'status' <> 'in_review'
)
OR
(
${activityLog.details} -> 'changes' -> 'status' ->> 'to' = 'in_review'
AND ${activityLog.details} -> 'changes' -> 'status' ->> 'from' IS NOT NULL
AND ${activityLog.details} -> 'changes' -> 'status' ->> 'from' <> 'in_review'
)
)`,
))
.orderBy(desc(activityLog.createdAt), desc(activityLog.id))
.limit(1)
.then((rows) => rows[0] ?? null);
if (transition?.actorType === "agent" || transition?.actorType === "user") {
return { type: transition.actorType, id: transition.actorId };
}
if (issue.createdByAgentId && !issue.createdByUserId) {
return { type: "agent", id: issue.createdByAgentId };
}
if (issue.createdByUserId && !issue.createdByAgentId) {
return { type: "user", id: issue.createdByUserId };
}
return null;
}
export async function assertIssueReviewVerdictActorAllowed(
db: Db,
input: {
issue: ReviewPolicyIssue;
actor: IssueReviewVerdictActor;
reviewPolicy?: IssueReviewPolicy | null;
},
): Promise<void> {
const policy = input.reviewPolicy ?? input.issue.reviewPolicy ?? "anyone";
if (policy === "anyone") return;
if (policy === "human_only") {
if (input.actor.type === "user") return;
throw forbidden(
"Review policy `human_only` allows only an authenticated user to approve or reject this review.",
{
code: "review_policy_denied",
policy,
allowedActor: "authenticated_user_with_issue_write_access",
remediation: "Have an authenticated user with issue write access submit the verdict, or change reviewPolicy to `anyone`.",
},
);
}
const requester = await findReviewRequester(db, input.issue);
if (!requester) {
throw forbidden(
"Review policy `not_creator` requires a different writer, but the review requester could not be determined.",
{
code: "review_policy_denied",
policy,
allowedActor: "writer_other_than_review_requester",
remediation: "Change reviewPolicy to `anyone`, or move the issue out of and back into `in_review` to record a requester before another writer submits the verdict.",
},
);
}
if (requester.type !== input.actor.type || requester.id !== input.actor.id) return;
throw forbidden(
"Review policy `not_creator` requires someone other than the writer who moved the issue into `in_review` to approve or reject it.",
{
code: "review_policy_denied",
policy,
allowedActor: "writer_other_than_review_requester",
remediation: "Have another writer with issue write access submit the verdict, or change reviewPolicy to `anyone`.",
},
);
}

1
server/src/services/issues.ts
View File

@ -3102,6 +3102,7 @@ const issueListSelect = {
workMode: issues.workMode,
harnessKind: issues.harnessKind,
priority: issues.priority,
reviewPolicy: issues.reviewPolicy,
assigneeAgentId: issues.assigneeAgentId,
assigneeUserId: issues.assigneeUserId,
checkoutRunId: issues.checkoutRunId,

1
ui/src/components/IssueDocumentsSection.test.tsx
View File

@ -252,6 +252,7 @@ function createIssue(): Issue {
status: "in_progress",
workMode: "standard",
priority: "medium",
reviewPolicy: null,
assigneeAgentId: null,
assigneeUserId: null,
responsibleUserId: null,

1
ui/src/components/IssueLinkQuicklook.test.tsx
View File

@ -32,6 +32,7 @@ function createIssue(overrides: Partial<Issue> = {}): Issue {
description: "Quicklook description",
status: "todo",
priority: "medium",
reviewPolicy: null,
assigneeAgentId: null,
assigneeUserId: null,
responsibleUserId: null,

1
ui/src/components/IssueProperties.test.tsx
View File

@ -209,6 +209,7 @@ function createIssue(overrides: Partial<Issue> = {}): Issue {
description: null,
status: "todo",
priority: "medium",
reviewPolicy: null,
assigneeAgentId: null,
assigneeUserId: null,
responsibleUserId: null,

1
ui/src/components/IssueRow.test.tsx
View File

@ -52,6 +52,7 @@ function createIssue(overrides: Partial<Issue> = {}): Issue {
description: null,
status: "todo",
priority: "medium",
reviewPolicy: null,
assigneeAgentId: null,
assigneeUserId: null,
responsibleUserId: null,

1
ui/src/components/IssueRunLedger.test.tsx
View File

@ -91,6 +91,7 @@ function createIssue(overrides: Partial<Issue> = {}): Issue {
description: null,
status: "todo",
priority: "medium",
reviewPolicy: null,
assigneeAgentId: null,
assigneeUserId: null,
responsibleUserId: null,

1
ui/src/components/IssueWorkspaceCard.test.tsx
View File

@ -91,6 +91,7 @@ function createIssue(overrides: Partial<Issue> = {}): Issue {
description: null,
status: "in_progress",
priority: "medium",
reviewPolicy: null,
assigneeAgentId: "agent-1",
assigneeUserId: null,
responsibleUserId: null,

1
ui/src/components/IssuesList.test.tsx
View File

@ -198,6 +198,7 @@ function createIssue(overrides: Partial<Issue> = {}): Issue {
description: null,
status: "todo",
priority: "medium",
reviewPolicy: null,
assigneeAgentId: null,
assigneeUserId: null,
responsibleUserId: null,

1
ui/src/components/KanbanBoard.test.tsx
View File

@ -43,6 +43,7 @@ function createIssue(index: number, status: IssueStatus): Issue {
status,
workMode: "standard",
priority: "medium",
reviewPolicy: null,
assigneeAgentId: index === 1 ? "agent-1" : null,
assigneeUserId: null,
responsibleUserId: null,

1
ui/src/lib/inbox.test.ts
View File

@ -189,6 +189,7 @@ function makeIssue(id: string, isUnreadForMe: boolean): Issue {
status: "todo",
workMode: "standard",
priority: "medium",
reviewPolicy: null,
assigneeAgentId: null,
assigneeUserId: null,
responsibleUserId: null,

1
ui/src/lib/issue-filters.test.ts
View File

@ -22,6 +22,7 @@ function makeIssue(overrides: Partial<Issue> = {}): Issue {
description: null,
status: "todo",
priority: "medium",
reviewPolicy: null,
assigneeAgentId: null,
assigneeUserId: null,
responsibleUserId: null,

1
ui/src/lib/issue-tree.test.ts
View File

@ -16,6 +16,7 @@ function makeIssue(id: string, parentId: string | null = null): Issue {
status: "todo",
workMode: "standard",
priority: "medium",
reviewPolicy: null,
assigneeAgentId: null,
assigneeUserId: null,
responsibleUserId: null,

1
ui/src/lib/issueDetailBreadcrumb.test.ts
View File

@ -44,6 +44,7 @@ describe("issueDetailBreadcrumb", () => {
description: null,
status: "todo",
priority: "medium",
reviewPolicy: null,
assigneeAgentId: null,
assigneeUserId: null,
responsibleUserId: null,

1
ui/src/lib/issueDetailCache.test.ts
View File

@ -29,6 +29,7 @@ function createIssue(overrides: Partial<Issue> = {}): Issue {
description: null,
status: "todo",
priority: "medium",
reviewPolicy: null,
assigneeAgentId: null,
assigneeUserId: null,
responsibleUserId: null,

1
ui/src/lib/issueDetailQuery.test.tsx
View File

@ -31,6 +31,7 @@ function makeIssue(overrides: Partial<Issue> = {}): Issue {
description: null,
status: "todo",
priority: "medium",
reviewPolicy: null,
assigneeAgentId: null,
assigneeUserId: null,
responsibleUserId: null,

4
ui/src/lib/optimistic-issue-comments.test.ts
View File

@ -448,6 +448,7 @@ describe("optimistic issue comments", () => {
status: "done",
workMode: "standard",
priority: "medium",
reviewPolicy: null,
assigneeAgentId: "agent-1",
assigneeUserId: null,
responsibleUserId: null,
@ -519,6 +520,7 @@ describe("optimistic issue comments", () => {
status: "todo",
workMode: "standard",
priority: "medium",
reviewPolicy: null,
assigneeAgentId: "agent-1",
assigneeUserId: null,
responsibleUserId: null,
@ -695,6 +697,7 @@ describe("optimistic issue comments", () => {
status: "todo",
workMode: "standard",
priority: "medium",
reviewPolicy: null,
assigneeAgentId: "agent-1",
assigneeUserId: null,
responsibleUserId: null,
@ -738,6 +741,7 @@ describe("optimistic issue comments", () => {
status: "todo",
workMode: "standard",
priority: "medium",
reviewPolicy: null,
assigneeAgentId: "agent-2",
assigneeUserId: null,
responsibleUserId: null,

1
ui/src/lib/subIssueDefaults.test.ts
View File

@ -47,6 +47,7 @@ function makeIssue(overrides: Partial<Issue> = {}): Issue {
description: null,
status: "todo",
priority: "medium",
reviewPolicy: null,
assigneeAgentId: null,
assigneeUserId: null,
responsibleUserId: null,

1
ui/src/pages/Inbox.test.tsx
View File

@ -184,6 +184,7 @@ function createIssue(overrides: Partial<Issue> = {}): Issue {
description: null,
status: "todo",
priority: "medium",
reviewPolicy: null,
assigneeAgentId: null,
assigneeUserId: null,
responsibleUserId: null,

1
ui/src/pages/Routines.test.tsx
View File

@ -333,6 +333,7 @@ function createIssue(overrides: Partial<Issue> = {}): Issue {
description: null,
status: "todo",
priority: "medium",
reviewPolicy: null,
assigneeAgentId: "agent-1",
assigneeUserId: null,
responsibleUserId: null,

1
ui/storybook/fixtures/paperclipData.ts
View File

@ -719,6 +719,7 @@ export function createIssue(overrides: Partial<Issue> = {}): Issue {
description: "Set up Storybook and move UX review surfaces into stories.",
status: "in_progress",
priority: "high",
reviewPolicy: null,
assigneeAgentId: "agent-codex",
assigneeUserId: null,
responsibleUserId: null,

1
ui/storybook/stories/document-annotations.stories.tsx
View File

@ -236,6 +236,7 @@ function makeIntegratedIssue(): Issue {
status: "in_progress",
workMode: "standard",
priority: "medium",
reviewPolicy: null,
assigneeAgentId: null,
assigneeUserId: null,
responsibleUserId: null,