Commit 7813d3e82a88

Vincent Demeester <vincent@sbr.pm>
2026-08-04 15:52:36
feat(pi): supported batch PR reviews
Allowed one approval to cover several PR reviews or comments while preserving ordered execution, shared bodies, and partial failure reporting. Signed-off-by: Vincent Demeester <vincent@sbr.pm>
1 parent a5617fb
Changed files (5)
dots
dots/pi/agent/extensions/github/actions/pr.ts
@@ -31,10 +31,22 @@ import {
 	approvalGateWithBodyPreview,
 	buildModifyResult,
 	buildRejectResult,
+	normalizePRNumbers,
 } from "../utils";
 
 const PR_LIST_FIELDS = "number,title,state,author,headRefName,baseRefName,url,isDraft,labels,reviewDecision,additions,deletions,changedFiles,createdAt,updatedAt";
 
+function formatBatchResult(
+	action: string,
+	succeeded: number[],
+	failed: Array<{ number: number; error: string }>,
+): string {
+	const lines = [`Batch ${action}: ${succeeded.length} succeeded, ${failed.length} failed`];
+	for (const number of succeeded) lines.push(`✓ #${number}`);
+	for (const item of failed) lines.push(`✗ #${item.number}: ${item.error}`);
+	return lines.join("\n");
+}
+
 /**
  * List pull requests
  */
@@ -505,10 +517,11 @@ export async function handlePRReview(
 	onUpdate: any,
 	ctx: ExtensionContext,
 ): Promise<any> {
-	if (!params.number) {
+	const numbers = normalizePRNumbers(params);
+	if (numbers.length === 0) {
 		return {
-			content: [{ type: "text", text: "Error: 'number' parameter is required for pr-review action" }],
-			details: { action: "pr-review", error: "missing_number" } as GhDetails,
+			content: [{ type: "text", text: "Error: 'numbers' must contain at least one positive PR number" }],
+			details: { action: "pr-review", error: "invalid_numbers" } as GhDetails,
 			isError: true,
 		};
 	}
@@ -523,62 +536,46 @@ export async function handlePRReview(
 
 	// APPROVAL GATE
 	if (ctx.hasUI) {
-		const confirmMessage = buildReviewConfirmation(params);
+		const targets = await buildPRTargetSummary(pi, ctx, numbers, signal);
+		const confirmMessage = `${targets}\n\n${buildReviewConfirmation({ ...params, numbers })}`;
 		const approval = params.body
 			? await approvalGateWithBodyPreview(
 				ctx,
-				`Submit review on PR #${params.number}?`,
+				`Submit review on ${numbers.length} PR(s)?`,
 				confirmMessage,
-				`PR #${params.number} Review Body Preview (${params.body.length} chars):`,
+				`Shared review body for ${numbers.length} PR(s) (${params.body.length} chars):`,
 				params.body,
 			)
-			: await approvalGate(ctx, `Submit review on PR #${params.number}?`, confirmMessage);
+			: await approvalGate(ctx, `Submit review on ${numbers.length} PR(s)?`, confirmMessage);
 		if (approval.outcome === "modify") {
 			ctx.ui.notify("Review paused for modifications", "info");
-			return buildModifyResult("review", { action: "pr-review", prNumber: params.number });
+			return buildModifyResult("review", { action: "pr-review", prNumbers: numbers });
 		}
 		if (approval.outcome === "rejected") {
 			ctx.ui.notify("Review rejected", "info");
-			return buildRejectResult("review", { action: "pr-review", prNumber: params.number });
+			return buildRejectResult("review", { action: "pr-review", prNumbers: numbers });
 		}
 	}
 
-	const args = ["pr", "review", String(params.number)];
+	const succeeded: number[] = [];
+	const failed: Array<{ number: number; error: string }> = [];
+	for (const number of numbers) {
+		const args = ["pr", "review", String(number)];
+		if (params.reviewAction === "approve") args.push("--approve");
+		else if (params.reviewAction === "request-changes") args.push("--request-changes");
+		else args.push("--comment");
+		if (params.body) args.push("--body", params.body);
 
-	switch (params.reviewAction) {
-		case "approve":
-			args.push("--approve");
-			break;
-		case "request-changes":
-			args.push("--request-changes");
-			break;
-		case "comment":
-			args.push("--comment");
-			break;
-	}
-
-	if (params.body) args.push("--body", params.body);
-
-	onUpdate?.({ content: [{ type: "text", text: `Submitting review on PR #${params.number}...` }] });
-
-	const result = await execGh(pi, ctx, args, { signal, timeout: 30000 });
-
-	if (result.code !== 0) {
-		return {
-			content: [{ type: "text", text: getErrorMessage(result.stderr, "Submit review") }],
-			details: { action: "pr-review", error: result.stderr, prNumber: params.number } as GhDetails,
-			isError: true,
-		};
+		onUpdate?.({ content: [{ type: "text", text: `Submitting review on PR #${number}...` }] });
+		const result = await execGh(pi, ctx, args, { signal, timeout: 30000 });
+		if (result.code === 0) succeeded.push(number);
+		else failed.push({ number, error: getErrorMessage(result.stderr, "Submit review") });
 	}
 
+	const output = formatBatchResult(`${params.reviewAction} review`, succeeded, failed);
 	return {
-		content: [{ type: "text", text: `Submitted ${params.reviewAction} review on PR #${params.number}` }],
-		details: {
-			action: "pr-review",
-			output: result.stdout.trim(),
-			prNumber: params.number,
-			reviewAction: params.reviewAction,
-		} as GhDetails,
+		content: [{ type: "text", text: output }],
+		details: { action: "pr-review", output, prNumbers: numbers, reviewAction: params.reviewAction, succeeded, failed } as GhDetails,
 	};
 }
 
@@ -592,10 +589,11 @@ export async function handlePRComment(
 	onUpdate: any,
 	ctx: ExtensionContext,
 ): Promise<any> {
-	if (!params.number) {
+	const numbers = normalizePRNumbers(params);
+	if (numbers.length === 0) {
 		return {
-			content: [{ type: "text", text: "Error: 'number' parameter is required for pr-comment action" }],
-			details: { action: "pr-comment", error: "missing_number" } as GhDetails,
+			content: [{ type: "text", text: "Error: 'numbers' must contain at least one positive PR number" }],
+			details: { action: "pr-comment", error: "invalid_numbers" } as GhDetails,
 			isError: true,
 		};
 	}
@@ -610,42 +608,41 @@ export async function handlePRComment(
 
 	// APPROVAL GATE
 	if (ctx.hasUI) {
-		const confirmMessage = buildCommentConfirmation("PR", params.number, params.body);
+		const targets = await buildPRTargetSummary(pi, ctx, numbers, signal);
+		const confirmMessage = `${targets}\n\n${buildCommentConfirmation("PR", numbers, params.body)}`;
 		const approval = await approvalGateWithBodyPreview(
 			ctx,
-			`Comment on PR #${params.number}?`,
+			`Comment on ${numbers.length} PR(s)?`,
 			confirmMessage,
-			`PR #${params.number} Comment Preview (${params.body.length} chars):`,
+			`Shared comment for ${numbers.length} PR(s) (${params.body.length} chars):`,
 			params.body,
 		);
 		if (approval.outcome === "modify") {
 			ctx.ui.notify("Comment paused for modifications", "info");
-			return buildModifyResult("comment", { action: "pr-comment", prNumber: params.number });
+			return buildModifyResult("comment", { action: "pr-comment", prNumbers: numbers });
 		}
 		if (approval.outcome === "rejected") {
 			ctx.ui.notify("Comment rejected", "info");
-			return buildRejectResult("comment", { action: "pr-comment", prNumber: params.number });
+			return buildRejectResult("comment", { action: "pr-comment", prNumbers: numbers });
 		}
 	}
 
-	onUpdate?.({ content: [{ type: "text", text: `Adding comment to PR #${params.number}...` }] });
-
-	const result = await execGh(pi, ctx, ["pr", "comment", String(params.number), "--body", params.body], {
-		signal,
-		timeout: 20000,
-	});
-
-	if (result.code !== 0) {
-		return {
-			content: [{ type: "text", text: getErrorMessage(result.stderr, "Comment on PR") }],
-			details: { action: "pr-comment", error: result.stderr, prNumber: params.number } as GhDetails,
-			isError: true,
-		};
+	const succeeded: number[] = [];
+	const failed: Array<{ number: number; error: string }> = [];
+	for (const number of numbers) {
+		onUpdate?.({ content: [{ type: "text", text: `Adding comment to PR #${number}...` }] });
+		const result = await execGh(pi, ctx, ["pr", "comment", String(number), "--body", params.body], {
+			signal,
+			timeout: 20000,
+		});
+		if (result.code === 0) succeeded.push(number);
+		else failed.push({ number, error: getErrorMessage(result.stderr, "Comment on PR") });
 	}
 
+	const output = formatBatchResult("comment", succeeded, failed);
 	return {
-		content: [{ type: "text", text: `Added comment to PR #${params.number}` }],
-		details: { action: "pr-comment", output: result.stdout.trim(), prNumber: params.number } as GhDetails,
+		content: [{ type: "text", text: output }],
+		details: { action: "pr-comment", output, prNumbers: numbers, succeeded, failed } as GhDetails,
 	};
 }
 
@@ -753,6 +750,17 @@ export async function handlePRClose(
 	};
 }
 
+/** Build the target list shown once before a batch PR write. */
+async function buildPRTargetSummary(
+	pi: ExtensionAPI,
+	ctx: ExtensionContext,
+	numbers: number[],
+	signal?: AbortSignal,
+): Promise<string> {
+	const titles = await Promise.all(numbers.map((number) => getPRTitle(pi, ctx, number, signal)));
+	return numbers.map((number, index) => `#${number}${titles[index] ? ` ${truncate(titles[index]!, 80)}` : ""}`).join("\n");
+}
+
 /**
  * Helper: get the title for a PR (used in confirmation dialogs)
  */
dots/pi/agent/extensions/github/github.test.ts
@@ -46,7 +46,10 @@ import {
 	approvalGate,
 	buildModifyResult,
 	buildRejectResult,
+	normalizePRNumbers,
+	prepareGithubArguments,
 } from "./utils";
+import { handlePRComment, handlePRReview } from "./actions/pr";
 import type { GhDetails } from "./types";
 
 // ============================================================================
@@ -600,14 +603,14 @@ describe("Confirmation Builders", () => {
 	});
 
 	test("buildReviewConfirmation includes action", () => {
-		const msg = buildReviewConfirmation({ number: 456, reviewAction: "approve", body: "LGTM" });
+		const msg = buildReviewConfirmation({ numbers: [456], reviewAction: "approve", body: "LGTM" });
 		expect(msg).toContain("#456");
 		expect(msg).toContain("approve");
 		expect(msg).toContain("LGTM");
 	});
 
 	test("buildReviewConfirmation shows no comment when body is absent", () => {
-		const msg = buildReviewConfirmation({ number: 789, reviewAction: "approve" });
+		const msg = buildReviewConfirmation({ numbers: [789], reviewAction: "approve" });
 		expect(msg).toContain("#789");
 		expect(msg).toContain("approve");
 		expect(msg).toContain("(none)");
@@ -629,7 +632,7 @@ describe("Confirmation Builders", () => {
 
 	test("buildCommentConfirmation includes preview", () => {
 		const msg = buildCommentConfirmation("PR", 123, "Great work!");
-		expect(msg).toContain("PR: #123");
+		expect(msg).toContain("PRs: #123");
 		expect(msg).toContain("Great work!");
 		expect(msg).toContain("public comment");
 	});
@@ -831,6 +834,118 @@ describe("Auto-detection Patterns", () => {
 	});
 });
 
+// ============================================================================
+// Batch PR Write Tests
+// ============================================================================
+
+describe("Batch PR arguments", () => {
+	test("normalizePRNumbers accepts one or many PRs and preserves order", () => {
+		expect(normalizePRNumbers({ numbers: [123] })).toEqual([123]);
+		expect(normalizePRNumbers({ numbers: [123, 456, 123, 789] })).toEqual([123, 456, 789]);
+	});
+
+	test("normalizePRNumbers rejects missing, empty, and invalid lists", () => {
+		expect(normalizePRNumbers({})).toEqual([]);
+		expect(normalizePRNumbers({ numbers: [] })).toEqual([]);
+		expect(normalizePRNumbers({ numbers: [123, 0, -1, 1.5] })).toEqual([]);
+	});
+
+	test("prepareGithubArguments converts legacy singular PR writes", () => {
+		expect(prepareGithubArguments({ action: "pr-review", number: 123 })).toEqual({
+			action: "pr-review",
+			numbers: [123],
+		});
+		expect(prepareGithubArguments({ action: "pr-comment", number: 456, body: "LGTM" })).toEqual({
+			action: "pr-comment",
+			body: "LGTM",
+			numbers: [456],
+		});
+	});
+
+	test("prepareGithubArguments leaves other actions and explicit numbers unchanged", () => {
+		const review = { action: "pr-review", numbers: [123, 456] };
+		expect(prepareGithubArguments(review)).toBe(review);
+		const view = { action: "pr-view", number: 123 };
+		expect(prepareGithubArguments(view)).toBe(view);
+	});
+});
+
+describe("Batch PR writes", () => {
+	function batchContext(selectReturn = "✓ Accept") {
+		let selectCalls = 0;
+		let selectPrompt = "";
+		const ctx = {
+			hasUI: true,
+			cwd: "/repo",
+			ui: {
+				select: async (prompt: string) => {
+					selectCalls++;
+					selectPrompt = prompt;
+					return selectReturn;
+				},
+				notify: () => {},
+			},
+			sessionManager: { getBranch: () => [] },
+		} as any;
+		return { ctx, selectCalls: () => selectCalls, selectPrompt: () => selectPrompt };
+	}
+
+	function batchPi(failNumber?: number) {
+		const writes: string[][] = [];
+		const pi = {
+			exec: async (command: string, args: string[]) => {
+				if (command === "git") return { code: 1, stdout: "", stderr: "" };
+				if (args[0] === "pr" && args[1] === "view") {
+					return { code: 0, stdout: `PR ${args[2]}`, stderr: "" };
+				}
+				writes.push(args);
+				if (Number(args[2]) === failNumber) return { code: 1, stdout: "", stderr: "failed" };
+				return { code: 0, stdout: "ok", stderr: "" };
+			},
+		} as any;
+		return { pi, writes };
+	}
+
+	test("pr-review approves multiple PRs with one prompt and sequential writes", async () => {
+		const { ctx, selectCalls, selectPrompt } = batchContext();
+		const { pi, writes } = batchPi();
+		const result = await handlePRReview(
+			pi,
+			{ numbers: [123, 456], reviewAction: "approve", body: "LGTM" },
+			undefined,
+			undefined,
+			ctx,
+		);
+
+		expect(selectCalls()).toBe(1);
+		expect(selectPrompt()).toContain("#123 PR 123");
+		expect(selectPrompt()).toContain("#456 PR 456");
+		expect(writes).toEqual([
+			["pr", "review", "123", "--approve", "--body", "LGTM"],
+			["pr", "review", "456", "--approve", "--body", "LGTM"],
+		]);
+		expect(result.details.prNumbers).toEqual([123, 456]);
+		expect(result.details.succeeded).toEqual([123, 456]);
+	});
+
+	test("pr-comment continues after an individual failure", async () => {
+		const { ctx, selectCalls } = batchContext();
+		const { pi, writes } = batchPi(456);
+		const result = await handlePRComment(
+			pi,
+			{ numbers: [123, 456, 789], body: "Shared body" },
+			undefined,
+			undefined,
+			ctx,
+		);
+
+		expect(selectCalls()).toBe(1);
+		expect(writes.map((args) => Number(args[2]))).toEqual([123, 456, 789]);
+		expect(result.details.succeeded).toEqual([123, 789]);
+		expect(result.details.failed).toEqual([{ number: 456, error: "failed" }]);
+	});
+});
+
 // ============================================================================
 // Approval Gate Tests
 // ============================================================================
dots/pi/agent/extensions/github/index.ts
@@ -75,6 +75,7 @@ import {
 	formatRelativeDate,
 	resetGitRoot,
 	execGh,
+	prepareGithubArguments,
 } from "./utils";
 
 export default function (pi: ExtensionAPI) {
@@ -158,7 +159,7 @@ export default function (pi: ExtensionAPI) {
 		description:
 			"Manage GitHub PRs, issues, checks, and runs via gh CLI. " +
 			"Write operations require user approval. " +
-			"IMPORTANT: Call write operations (pr-create, pr-merge, pr-review, pr-comment, pr-close, pr-ready, pr-line-comment, pr-review-comments, issue-create, issue-close, issue-comment, issue-edit, checks-restart) ONE AT A TIME, never in parallel — parallel approval dialogs deadlock the UI. " +
+			"IMPORTANT: Call write operations ONE AT A TIME, never in parallel — parallel approval dialogs deadlock the UI. For pr-review and pr-comment, pass every target in numbers to use one approval dialog. " +
 			"checks-log accepts runId or number (PR) — PR auto-selects first failed run. " +
 			"pr-review-comments submits a review with inline comments. " +
 			"issue-create with parent auto-links as sub-issue.",
@@ -199,8 +200,9 @@ export default function (pi: ExtensionAPI) {
 				"release-list",
 			] as const),
 
-			// PR/Issue number
-			number: Type.Optional(Type.Number({ description: "PR or issue number" })),
+			// PR/Issue number. PR review/comment use numbers, including for one PR.
+			number: Type.Optional(Type.Number({ description: "PR or issue number (not for pr-review/pr-comment)" })),
+			numbers: Type.Optional(Type.Array(Type.Number(), { minItems: 1, description: "PR numbers for pr-review or pr-comment; use a one-item array for one PR" })),
 
 			// PR list filters
 			state: Type.Optional(Type.String({ description: "Filter by state: open, closed, merged, all" })),
@@ -269,6 +271,8 @@ export default function (pi: ExtensionAPI) {
 			subIssueNumber: Type.Optional(Type.Number({ description: "Sub-issue number (for issue-add-sub-issue, issue-remove-sub-issue)" })),
 		}),
 
+		prepareArguments: prepareGithubArguments,
+
 		async execute(toolCallId, params, signal, onUpdate, ctx) {
 			try {
 				switch (params.action) {
@@ -371,6 +375,9 @@ export default function (pi: ExtensionAPI) {
 			if (args.number) {
 				text += " " + theme.fg("accent", `#${args.number}`);
 			}
+			if (args.numbers?.length) {
+				text += " " + theme.fg("accent", args.numbers.map((number) => `#${number}`).join(", "));
+			}
 			if (args.runId) {
 				text += " " + theme.fg("accent", String(args.runId));
 			}
@@ -421,14 +428,8 @@ export default function (pi: ExtensionAPI) {
 						0,
 					);
 				case "pr-review":
-					return new Text(
-						theme.fg("success", `✓ ${details.reviewAction} `) +
-							theme.fg("accent", `#${details.prNumber}`),
-						0,
-						0,
-					);
 				case "pr-comment":
-					return new Text(theme.fg("success", "✓ Comment added to ") + theme.fg("accent", `#${details.prNumber}`), 0, 0);
+					return renderBatchWrite(details, theme);
 				case "pr-ready":
 					return new Text(theme.fg("success", "✓ PR ready for review: ") + theme.fg("accent", `#${details.prNumber}`), 0, 0);
 				case "pr-line-comment":
@@ -1200,6 +1201,17 @@ function setupIssueAutocomplete(pi: ExtensionAPI, ctx: ExtensionContext): void {
 // Rendering Functions
 // ============================================================================
 
+function renderBatchWrite(details: GhDetails, theme: Theme): Text {
+	const succeeded = details.succeeded ?? [];
+	const failed = details.failed ?? [];
+	let text = theme.fg("success", `✓ ${succeeded.length} succeeded`);
+	if (failed.length > 0) text += theme.fg("error", `, ${failed.length} failed`);
+	if (details.prNumbers?.length) {
+		text += theme.fg("muted", ` (${details.prNumbers.map((number) => `#${number}`).join(", ")})`);
+	}
+	return new Text(text, 0, 0);
+}
+
 function renderPRList(details: GhDetails, expanded: boolean, theme: Theme): Text {
 	if (!details.output) return new Text(theme.fg("dim", "No PRs found"), 0, 0);
 
dots/pi/agent/extensions/github/types.ts
@@ -16,6 +16,8 @@ export interface GhDetails {
 	prNumber?: number;
 	prNumbers?: number[];
 	prUrl?: string;
+	succeeded?: number[];
+	failed?: Array<{ number: number; error: string }>;
 	// Issue-specific
 	issueNumber?: number;
 	issueNumbers?: number[];
dots/pi/agent/extensions/github/utils.ts
@@ -510,6 +510,31 @@ export function getReviewDecisionText(decision: string): string {
 	}
 }
 
+// ============================================================================
+// PR write argument helpers
+// ============================================================================
+
+export function normalizePRNumbers(params: { numbers?: unknown }): number[] {
+	if (!Array.isArray(params.numbers) || params.numbers.length === 0) return [];
+	if (!params.numbers.every((number) => Number.isInteger(number) && (number as number) > 0)) return [];
+	return [...new Set(params.numbers as number[])];
+}
+
+/** Convert persisted calls from the former singular PR write interface. */
+export function prepareGithubArguments(args: unknown): unknown {
+	if (!args || typeof args !== "object") return args;
+	const params = args as { action?: string; number?: unknown; numbers?: unknown };
+	if (
+		(params.action === "pr-review" || params.action === "pr-comment") &&
+		params.numbers === undefined &&
+		typeof params.number === "number"
+	) {
+		const { number, ...rest } = params;
+		return { ...rest, numbers: [number] };
+	}
+	return args;
+}
+
 // ============================================================================
 // Approval gate helper (with mutex to prevent parallel dialog deadlocks)
 // ============================================================================
@@ -658,7 +683,8 @@ export function buildPRMergeConfirmation(params: any): string {
 }
 
 export function buildReviewConfirmation(params: any): string {
-	let msg = `PR: #${params.number}\n`;
+	const numbers = normalizePRNumbers(params);
+	let msg = `PRs: ${numbers.map((number) => `#${number}`).join(", ")}\n`;
 	msg += `Action: ${params.reviewAction}\n`;
 	if (params.body) {
 		const preview = params.body.length > 200 ? params.body.slice(0, 197) + "..." : params.body;
@@ -739,9 +765,10 @@ export function buildSubIssueConfirmation(action: "add" | "remove", parentNumber
 	return msg;
 }
 
-export function buildCommentConfirmation(kind: string, number: number, comment: string): string {
+export function buildCommentConfirmation(kind: string, numbers: number | number[], comment: string): string {
 	const preview = comment.length > 200 ? comment.slice(0, 197) + "..." : comment;
-	let msg = `${kind}: #${number}\n\n`;
+	const values = Array.isArray(numbers) ? numbers : [numbers];
+	let msg = `${kind}s: ${values.map((number) => `#${number}`).join(", ")}\n\n`;
 	msg += `Comment preview:\n"${preview}"\n\n`;
 	msg += "This will post a public comment.";
 	return msg;