Add request changes review outcome

Co-Authored-By: Paperclip <noreply@paperclip.ing>

cli/src/__tests__/pipelines.test.ts

@@ -0,0 +1,96 @@
+import { mkdtemp, rm, writeFile } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { Command } from "commander";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import { registerPipelineCommands } from "../commands/pipelines.js";
+
+const COMPANY_ID = "22222222-2222-4222-8222-222222222222";
+const CASE_ID = "11111111-1111-4111-8111-111111111111";
+
+function createProgram(): Command {
+  const program = new Command();
+  program.exitOverride();
+  program.configureOutput({
+    writeOut: () => {},
+    writeErr: () => {},
+  });
+  registerPipelineCommands(program);
+  return program;
+}
+
+async function run(args: string[]): Promise<void> {
+  await createProgram().parseAsync([
+    ...args,
+    "--api-base",
+    "http://localhost:3100",
+    "--api-key",
+    "board-token",
+    "--company-id",
+    COMPANY_ID,
+  ], { from: "user" });
+}
+
+describe("pipeline CLI commands", () => {
+  beforeEach(() => {
+    vi.restoreAllMocks();
+    delete process.env.PAPERCLIP_API_KEY;
+    delete process.env.PAPERCLIP_API_URL;
+    vi.spyOn(console, "log").mockImplementation(() => {});
+  });
+
+  afterEach(() => {
+    vi.restoreAllMocks();
+  });
+
+  it("sends request-changes review decisions", async () => {
+    const fetchMock = vi.fn().mockResolvedValue(jsonResponse({ case: { id: CASE_ID } }));
+    vi.stubGlobal("fetch", fetchMock);
+
+    await run([
+      "pipelines",
+      "case",
+      "review",
+      CASE_ID,
+      "--request-changes",
+      "--reason",
+      "Needs edits",
+      "--expected-version",
+      "2",
+    ]);
+
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+    expect(fetchMock.mock.calls[0]?.[0]).toBe(`http://localhost:3100/api/cases/${CASE_ID}/review`);
+    expect(JSON.parse(String(fetchMock.mock.calls[0]?.[1]?.body))).toMatchObject({
+      decision: "request_changes",
+      reason: "Needs edits",
+      expectedVersion: 2,
+    });
+  });
+
+  it("passes request_changes rows through review-bulk", async () => {
+    const dir = await mkdtemp(join(tmpdir(), "paperclip-pipeline-cli-"));
+    const file = join(dir, "review-bulk.json");
+    await writeFile(file, JSON.stringify([
+      { caseId: CASE_ID, decision: "request_changes", reason: "Needs edits", expectedVersion: 2 },
+    ]));
+    const fetchMock = vi.fn().mockResolvedValue(jsonResponse({ results: [] }));
+    vi.stubGlobal("fetch", fetchMock);
+
+    try {
+      await run(["pipelines", "review-bulk", "--file", file]);
+    } finally {
+      await rm(dir, { recursive: true, force: true });
+    }
+
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+    expect(fetchMock.mock.calls[0]?.[0]).toBe(`http://localhost:3100/api/companies/${COMPANY_ID}/review-cases/bulk`);
+    expect(JSON.parse(String(fetchMock.mock.calls[0]?.[1]?.body))).toEqual({
+      items: [{ caseId: CASE_ID, decision: "request_changes", reason: "Needs edits", expectedVersion: 2 }],
+    });
+  });
+});
+
+function jsonResponse(body: unknown = { ok: true }, init: ResponseInit = { status: 200 }): Response {
+  return new Response(JSON.stringify(body), init);
+}

cli/src/commands/pipelines.ts

@@ -168,6 +168,7 @@ interface ResolveSuggestionOptions extends PipelineOptions {
 interface ReviewOptions extends PipelineOptions {
   approve?: boolean;
   reject?: boolean;
+  requestChanges?: boolean;
   reason?: string;
   expectedVersion: string;
   editsJson?: string;
@@ -404,7 +405,7 @@ export function registerPipelineCommands(program: Command): void {
   addPipelineOptions(
     pipelines
       .command("review-bulk")
-      .description("Apply bulk review decisions")
+      .description("Apply bulk review decisions: approve, reject, or request_changes")
       .requiredOption("--file <path>", "JSON file containing an array or { items }")
       .action((opts: ReviewBulkOptions) => withPipelineErrors(async () => {
         const ctx = resolvePipelineContext(opts);
@@ -575,10 +576,11 @@ function registerCaseCommands(caseCommand: Command): void {
   addPipelineOptions(
     caseCommand
       .command("review")
-      .description("Approve or reject a case in a review stage")
+      .description("Approve, reject, or request changes for a case in a review stage")
       .argument("<caseId>", "Case ID")
       .option("--approve", "Approve the case")
       .option("--reject", "Reject the case")
+      .option("--request-changes", "Request changes for the case")
       .option("--reason <text>", "Decision reason")
       .requiredOption("--expected-version <n>", "Expected case version")
       .option("--edits-json <json>", "Review edits JSON")
@@ -590,7 +592,7 @@ function registerCaseCommands(caseCommand: Command): void {
       .option("--lease-token <token>", "Lease token")
       .action((caseId: string, opts: ReviewOptions) => withPipelineErrors(async () => {
         const ctx = resolvePipelineContext(opts);
-        const decision = exactlyOneFlag(opts.approve, opts.reject, "--approve", "--reject") === "--approve" ? "approve" : "reject";
+        const decision = reviewDecisionFromOptions(opts);
         const edits = await buildReviewEdits(opts);
         printOutput(await ctx.api.post(apiPath`/api/cases/${caseId}/review`, {
           decision,
@@ -726,6 +728,18 @@ function exactlyOneFlag(first: boolean | undefined, second: boolean | undefined,
   return first ? firstName : secondName;
 }
 
+function reviewDecisionFromOptions(opts: ReviewOptions): "approve" | "reject" | "request_changes" {
+  const selected = [
+    opts.approve ? { flag: "--approve", decision: "approve" as const } : null,
+    opts.reject ? { flag: "--reject", decision: "reject" as const } : null,
+    opts.requestChanges ? { flag: "--request-changes", decision: "request_changes" as const } : null,
+  ].filter((item): item is NonNullable<typeof item> => item !== null);
+  if (selected.length !== 1) {
+    throw new Error("Pass exactly one of --approve, --reject, or --request-changes.");
+  }
+  return selected[0]!.decision;
+}
+
 function printPipeline(row: PipelineDetail | PipelineSummary | null, ctx: ResolvedClientContext): void {
   if (!row) return printOutput(null, { json: ctx.json });
   if (ctx.json) return printOutput(row, { json: true });

server/src/__tests__/pipelines-routes.test.ts

@@ -543,6 +543,30 @@ describeEmbeddedPostgres("pipeline routes", () => {
     const pipeline = await http.post(`/api/companies/${company.id}/pipelines`).send({ key: "valid-review", name: "Valid review" }).expect(201);
     const intake = pipeline.body.stages.find((stage: { key: string }) => stage.key === "intake");
     await http.patch(`/api/pipelines/${pipeline.body.id}/stages/${intake.id}`).send({ kind: "review" }).expect(422);
+
+    await http
+      .post(`/api/companies/${company.id}/pipelines`)
+      .send({
+        key: "bad-request-changes-target",
+        name: "Bad request changes target",
+        stages: [
+          { key: "intake", name: "Intake", kind: "open", position: 100 },
+          {
+            key: "review",
+            name: "Review",
+            kind: "review",
+            position: 200,
+            config: {
+              approveToStageKey: "done",
+              rejectToStageKey: "cancelled",
+              requestChangesToStageKey: "missing",
+            },
+          },
+          { key: "done", name: "Done", kind: "done", position: 900 },
+          { key: "cancelled", name: "Cancelled", kind: "cancelled", position: 1000 },
+        ],
+      })
+      .expect(422);
   });
 
   it("applies review decisions atomically with edits and stores reject reasons verbatim", async () => {
@@ -590,6 +614,71 @@ describeEmbeddedPostgres("pipeline routes", () => {
     expect(reviewEvent.payload.reason).toBe(reason);
   });
 
+  it("routes request-changes review decisions to the configured stage", async () => {
+    const company = await seedCompany();
+    const http = request(app(boardActor));
+    const pipeline = await http
+      .post(`/api/companies/${company.id}/pipelines`)
+      .send({
+        key: "review-request-changes",
+        name: "Review request changes",
+        stages: [
+          { key: "drafting", name: "Drafting", kind: "working", position: 100 },
+          {
+            key: "review",
+            name: "Review",
+            kind: "review",
+            position: 200,
+            config: {
+              approveToStageKey: "done",
+              rejectToStageKey: "cancelled",
+              requestChangesToStageKey: "drafting",
+            },
+          },
+          { key: "done", name: "Done", kind: "done", position: 900 },
+          { key: "cancelled", name: "Cancelled", kind: "cancelled", position: 1000 },
+        ],
+      })
+      .expect(201);
+
+    const created = await http
+      .post(`/api/pipelines/${pipeline.body.id}/cases`)
+      .send({ caseKey: "needs-edits", title: "Needs edits" })
+      .expect(201);
+    await http.post(`/api/cases/${created.body.case.id}/transition`).send({ toStageKey: "review", expectedVersion: 1 }).expect(200);
+    await http
+      .post(`/api/cases/${created.body.case.id}/review`)
+      .send({ decision: "request_changes", expectedVersion: 2 })
+      .expect(422);
+
+    const changed = await http
+      .post(`/api/cases/${created.body.case.id}/review`)
+      .send({ decision: "request_changes", reason: "Tighten the framing", expectedVersion: 2 })
+      .expect(200);
+    expect(changed.body.case.version).toBe(3);
+    const detail = await http.get(`/api/cases/${created.body.case.id}`).expect(200);
+    expect(detail.body.stage.key).toBe("drafting");
+    const events = await http.get(`/api/cases/${created.body.case.id}/events`).expect(200);
+    const reviewEvent = events.body.items.find((event: { type: string }) => event.type === "review_decided");
+    expect(reviewEvent.payload.decision).toBe("request_changes");
+    expect(reviewEvent.payload.reason).toBe("Tighten the framing");
+
+    const defaultPipeline = await http
+      .post(`/api/companies/${company.id}/pipelines`)
+      .send({ key: "review-request-changes-missing", name: "Review request changes missing" })
+      .expect(201);
+    const missingConfigCase = await http
+      .post(`/api/pipelines/${defaultPipeline.body.id}/cases`)
+      .send({ caseKey: "missing-config", title: "Missing config" })
+      .expect(201);
+    await http.post(`/api/cases/${missingConfigCase.body.case.id}/transition`).send({ toStageKey: "review", expectedVersion: 1 }).expect(200);
+    const missingConfig = await http
+      .post(`/api/cases/${missingConfigCase.body.case.id}/review`)
+      .send({ decision: "request_changes", reason: "Needs a loop", expectedVersion: 2 })
+      .expect(422);
+    expect(missingConfig.body.code).toBe("validation");
+  });
+
   it("aggregates the review inbox across pipelines with parent and review config context", async () => {
     const company = await seedCompany();
     const http = request(app(boardActor));
@@ -634,7 +723,30 @@ describeEmbeddedPostgres("pipeline routes", () => {
   it("bulk reviews partial successes without aborting stale items", async () => {
     const company = await seedCompany();
     const http = request(app(boardActor));
-    const pipeline = await http.post(`/api/companies/${company.id}/pipelines`).send({ key: "bulk-review", name: "Bulk review" }).expect(201);
+    const pipeline = await http
+      .post(`/api/companies/${company.id}/pipelines`)
+      .send({
+        key: "bulk-review",
+        name: "Bulk review",
+        stages: [
+          { key: "intake", name: "Intake", kind: "open", position: 100 },
+          { key: "in_progress", name: "In progress", kind: "working", position: 200 },
+          {
+            key: "review",
+            name: "Review",
+            kind: "review",
+            position: 300,
+            config: {
+              approveToStageKey: "done",
+              rejectToStageKey: "cancelled",
+              requestChangesToStageKey: "in_progress",
+            },
+          },
+          { key: "done", name: "Done", kind: "done", position: 900 },
+          { key: "cancelled", name: "Cancelled", kind: "cancelled", position: 1000 },
+        ],
+      })
+      .expect(201);
     const caseIds: string[] = [];
     for (let index = 0; index < 50; index += 1) {
       const created = await http
@@ -650,13 +762,19 @@ describeEmbeddedPostgres("pipeline routes", () => {
 
     const bulk = await http
       .post(`/api/companies/${company.id}/review-cases/bulk`)
-      .send({ items: caseIds.map((caseId) => ({ caseId, decision: "approve", expectedVersion: 2 })) })
+      .send({
+        items: caseIds.map((caseId, index) => index === 3
+          ? { caseId, decision: "request_changes", reason: "Revise this item", expectedVersion: 2 }
+          : { caseId, decision: "approve", expectedVersion: 2 }),
+      })
       .expect(200);
 
     expect(bulk.body.results.filter((item: { ok: boolean }) => item.ok)).toHaveLength(47);
     const failed = bulk.body.results.filter((item: { ok: boolean }) => !item.ok);
     expect(failed).toHaveLength(3);
     expect(failed.every((item: { error: { code: string } }) => item.error.code === "version_conflict")).toBe(true);
+    const requestChangesDetail = await http.get(`/api/cases/${caseIds[3]}`).expect(200);
+    expect(requestChangesDetail.body.stage.key).toBe("in_progress");
   });
 
   it("returns conflict bodies with code, current version, and stage", async () => {

server/src/routes/pipelines.ts

@@ -125,7 +125,7 @@ const reviewEditsSchema = z.object({
   parentCaseId: z.string().uuid().nullable().optional(),
 });
 const reviewCaseSchema = z.object({
-  decision: z.enum(["approve", "reject"]),
+  decision: z.enum(["approve", "reject", "request_changes"]),
   reason: z.string().max(4_000).nullable().optional(),
   edits: reviewEditsSchema.optional(),
   expectedVersion: z.number().int().positive(),

server/src/services/pipelines.ts

@@ -54,6 +54,7 @@ export type PipelineStageConfig = Record<string, unknown> & {
   autoAdvanceOnChildrenTerminal?: string;
   approveToStageKey?: string;
   rejectToStageKey?: string;
+  requestChangesToStageKey?: string;
   requireRejectReason?: boolean;
   reviewerKind?: "human" | "any";
   onEnter?: {
@@ -63,6 +64,8 @@ export type PipelineStageConfig = Record<string, unknown> & {
   };
 };
 
+export type PipelineReviewDecision = "approve" | "reject" | "request_changes";
+
 export type PipelineAutomationExecutionResult =
   | { status: "none" }
   | { status: "succeeded"; execution: typeof pipelineAutomationExecutions.$inferSelect }
@@ -156,6 +159,12 @@ function normalizeStageConfig(kind: PipelineStageKind | string, config?: Pipelin
   if (typeof next.rejectToStageKey !== "string" || next.rejectToStageKey.trim().length === 0) {
     throw unprocessable("Review stages require rejectToStageKey", { code: "validation" });
   }
+  if (
+    next.requestChangesToStageKey !== undefined &&
+    (typeof next.requestChangesToStageKey !== "string" || next.requestChangesToStageKey.trim().length === 0)
+  ) {
+    throw unprocessable("Review stage requestChangesToStageKey must be a non-empty string", { code: "validation" });
+  }
   if (next.requireRejectReason !== undefined && typeof next.requireRejectReason !== "boolean") {
     throw unprocessable("Review stage requireRejectReason must be boolean", { code: "validation" });
   }
@@ -167,6 +176,7 @@ function normalizeStageConfig(kind: PipelineStageKind | string, config?: Pipelin
     ...next,
     approveToStageKey: next.approveToStageKey.trim(),
     rejectToStageKey: next.rejectToStageKey.trim(),
+    ...(next.requestChangesToStageKey !== undefined ? { requestChangesToStageKey: next.requestChangesToStageKey.trim() } : {}),
     requireRejectReason: next.requireRejectReason ?? true,
     reviewerKind: next.reviewerKind ?? "human",
   };
@@ -188,6 +198,18 @@ function assertReviewTargetsInSet(
   if (!stageKeys.has(config.rejectToStageKey!)) {
     throw unprocessable("Review rejectToStageKey references an unknown stage", { code: "validation" });
   }
+  if (config.requestChangesToStageKey !== undefined && !stageKeys.has(config.requestChangesToStageKey)) {
+    throw unprocessable("Review requestChangesToStageKey references an unknown stage", { code: "validation" });
+  }
+}
+
+function targetStageKeyForReviewDecision(config: PipelineStageConfig, decision: PipelineReviewDecision) {
+  if (decision === "approve") return config.approveToStageKey!;
+  if (decision === "reject") return config.rejectToStageKey!;
+  if (!config.requestChangesToStageKey) {
+    throw unprocessable("Review stage does not configure requestChangesToStageKey", { code: "validation" });
+  }
+  return config.requestChangesToStageKey;
 }
 
 function stageAutomation(stage: typeof pipelineStages.$inferSelect) {
@@ -1605,7 +1627,7 @@ export function pipelineService(db: Db, deps: { heartbeat?: IssueAssignmentWakeu
     async reviewCase(input: {
       companyId: string;
       caseId: string;
-      decision: "approve" | "reject";
+      decision: PipelineReviewDecision;
       reason?: string | null;
       edits?: {
         title?: string;
@@ -1626,10 +1648,10 @@ export function pipelineService(db: Db, deps: { heartbeat?: IssueAssignmentWakeu
         if (input.actor.type === "agent" && config.reviewerKind !== "any") {
           throw new HttpError(403, "Human review is required", { code: "review_required" });
         }
-        if (input.decision === "reject" && config.requireRejectReason !== false && !input.reason?.trim()) {
-          throw unprocessable("Reject reason is required", { code: "validation" });
+        if (input.decision !== "approve" && config.requireRejectReason !== false && !input.reason?.trim()) {
+          throw unprocessable("Review decision reason is required", { code: "validation" });
         }
-        const toStageKey = input.decision === "approve" ? config.approveToStageKey! : config.rejectToStageKey!;
+        const toStageKey = targetStageKeyForReviewDecision(config, input.decision);
         const suggestionId = detail.case.pendingSuggestion?.id ?? null;
         let expectedVersion = input.expectedVersion;
         let updateEvent: typeof pipelineCaseEvents.$inferSelect | null = null;