BBC2-18 add bb pr review (-a / -r / -w) + surface approvals in bb pr view

src/backend/pullrequests/index.test.ts

@@ -2,6 +2,7 @@ import { describe, expect, test } from "bun:test";
 import { HttpResponse, http } from "msw";
 import { BITBUCKET_BASE, server, setupMsw } from "../../test/msw/server.ts";
 import {
+	approvePullRequest,
 	createPullRequest,
 	createPullRequestComment,
 	findOpenPullRequestForBranch,
@@ -11,6 +12,9 @@ import {
 	type PullRequest,
 	type PullRequestDetail,
 	PullRequestError,
+	requestChangesOnPullRequest,
+	unapprovePullRequest,
+	withdrawRequestChanges,
 } from "./index.ts";
 
 setupMsw();
@@ -69,7 +73,8 @@ function makePrDetail(
 				user: { uuid: "{dave}", display_name: "Dave", nickname: "dave" },
 				state: null,
 			},
-			// non-reviewer participants (plain commenters) should be dropped
+			// PARTICIPANT with no review state (plain commenter) — surfaces
+			// in participants[] with state=pending, stays out of reviewers[]
 			{
 				role: "PARTICIPANT",
 				user: { uuid: "{eve}", display_name: "Eve", nickname: "eve" },
@@ -388,6 +393,12 @@ describe("getPullRequest", () => {
 					state: "pending",
 				},
 			],
+			participants: [
+				{
+					account: { uuid: "{eve}", displayName: "Eve", nickname: "eve" },
+					state: "pending",
+				},
+			],
 		});
 	});
 
@@ -412,6 +423,46 @@ describe("getPullRequest", () => {
 
 		const result = await getPullRequest(creds, ref, 42);
 		expect(result.reviewers).toEqual([]);
+		expect(result.participants).toEqual([]);
+	});
+
+	test("surfaces ad-hoc approvers as participants (Snyk-style no-reviewer PR)", async () => {
+		// Snyk auto-PRs ship with no formal reviewers. When someone approves
+		// one, Bitbucket records them as role=PARTICIPANT with state=approved.
+		// We want that surfaced so the approver can see their own action
+		// landed.
+		server.use(
+			http.get(PR_DETAIL_PATH(42), () =>
+				HttpResponse.json(
+					makePrDetail({
+						participants: [
+							{
+								role: "PARTICIPANT",
+								user: {
+									uuid: "{nico}",
+									display_name: "Nicolas",
+									nickname: "nicolas",
+								},
+								state: "approved",
+							},
+						],
+					}),
+				),
+			),
+		);
+
+		const result = await getPullRequest(creds, ref, 42);
+		expect(result.reviewers).toEqual([]);
+		expect(result.participants).toEqual([
+			{
+				account: {
+					uuid: "{nico}",
+					displayName: "Nicolas",
+					nickname: "nicolas",
+				},
+				state: "approved",
+			},
+		]);
 	});
 });
 
@@ -763,3 +814,108 @@ describe("createPullRequestComment", () => {
 		expect((err as PullRequestError).status).toBe(404);
 	});
 });
+
+describe("review action endpoints (approve / unapprove / request-changes / unrequest-changes)", () => {
+	const APPROVE_PATH = (id: number) =>
+		`${BITBUCKET_BASE}/repositories/ws/repo/pullrequests/${id}/approve`;
+	const REQUEST_CHANGES_PATH = (id: number) =>
+		`${BITBUCKET_BASE}/repositories/ws/repo/pullrequests/${id}/request-changes`;
+
+	test("approvePullRequest POSTs with no body", async () => {
+		let seenMethod: string | null = null;
+		let seenBody: string | null = null;
+		server.use(
+			http.post(APPROVE_PATH(42), async ({ request }) => {
+				seenMethod = request.method;
+				seenBody = await request.text();
+				return HttpResponse.json(
+					{ type: "participant", role: "REVIEWER", approved: true },
+					{ status: 200 },
+				);
+			}),
+		);
+
+		await approvePullRequest(creds, ref, 42);
+
+		expect(seenMethod!).toBe("POST");
+		expect(seenBody!).toBe("");
+	});
+
+	test("unapprovePullRequest DELETEs and tolerates 204 no-content", async () => {
+		let seenMethod: string | null = null;
+		server.use(
+			http.delete(APPROVE_PATH(42), ({ request }) => {
+				seenMethod = request.method;
+				return new HttpResponse(null, { status: 204 });
+			}),
+		);
+
+		await unapprovePullRequest(creds, ref, 42);
+
+		expect(seenMethod!).toBe("DELETE");
+	});
+
+	test("unapprovePullRequest surfaces 400 (PR already merged) cleanly", async () => {
+		// Documented case per the spec: DELETE /approve returns 400 if the
+		// PR has already been merged. We propagate the status so the command
+		// layer can show a clean message rather than a raw HTTP error.
+		server.use(
+			http.delete(APPROVE_PATH(42), () =>
+				HttpResponse.json(
+					{
+						type: "error",
+						error: { message: "PR cannot be unapproved (merged)" },
+					},
+					{ status: 400 },
+				),
+			),
+		);
+
+		const err = await unapprovePullRequest(creds, ref, 42).catch((e) => e);
+		expect(err).toBeInstanceOf(PullRequestError);
+		expect((err as PullRequestError).status).toBe(400);
+	});
+
+	test("requestChangesOnPullRequest POSTs to /request-changes", async () => {
+		let seenMethod: string | null = null;
+		server.use(
+			http.post(REQUEST_CHANGES_PATH(42), ({ request }) => {
+				seenMethod = request.method;
+				return HttpResponse.json(
+					{ type: "participant", role: "REVIEWER", state: "changes_requested" },
+					{ status: 200 },
+				);
+			}),
+		);
+
+		await requestChangesOnPullRequest(creds, ref, 42);
+
+		expect(seenMethod!).toBe("POST");
+	});
+
+	test("withdrawRequestChanges DELETEs to /request-changes", async () => {
+		let seenMethod: string | null = null;
+		server.use(
+			http.delete(REQUEST_CHANGES_PATH(42), ({ request }) => {
+				seenMethod = request.method;
+				return new HttpResponse(null, { status: 204 });
+			}),
+		);
+
+		await withdrawRequestChanges(creds, ref, 42);
+
+		expect(seenMethod!).toBe("DELETE");
+	});
+
+	test("approvePullRequest surfaces 404 when the PR does not exist", async () => {
+		server.use(
+			http.post(APPROVE_PATH(99), () =>
+				HttpResponse.json({ type: "error" }, { status: 404 }),
+			),
+		);
+
+		const err = await approvePullRequest(creds, ref, 99).catch((e) => e);
+		expect(err).toBeInstanceOf(PullRequestError);
+		expect((err as PullRequestError).status).toBe(404);
+	});
+});

src/backend/pullrequests/index.ts

@@ -48,11 +48,23 @@ export type Reviewer = {
 	state: ReviewState;
 };
 
+/**
+ * A non-reviewer participant on a PR. State is populated when the person
+ * has approved or requested changes without being on the formal reviewer
+ * list (e.g. someone approving a Snyk auto-PR with no assigned reviewers).
+ * Pure commenters appear with state="pending".
+ */
+export type Participant = {
+	account: PullRequestAuthor;
+	state: ReviewState;
+};
+
 export type PullRequestDetail = PullRequest & {
 	description: string;
 	sourceBranch: string;
 	destinationBranch: string;
 	reviewers: Reviewer[];
+	participants: Participant[];
 };
 
 export type ListPullRequestsOptions = {
@@ -419,6 +431,145 @@ export async function createPullRequestComment(
 	};
 }
 
+/**
+ * Records the authenticated user's review state on a PR. Approve and
+ * request-changes are two independent states; they map to symmetric
+ * POST/DELETE pairs on `/approve` and `/request-changes`.
+ *
+ * Idempotency is NOT documented by Bitbucket for any of these endpoints.
+ * We let non-2xx errors propagate; callers who need "re-running is a
+ * no-op" semantics should handle expected errors (e.g. a 409 conflict on
+ * re-approval) themselves. Smoke-test to find the actual behavior.
+ *
+ * `DELETE /approve` is documented to return `400` when the PR has already
+ * been merged — surface that at the command layer.
+ */
+async function postParticipantAction(
+	credentials: Credentials,
+	ref: { workspace: string; slug: string },
+	pullRequestId: number,
+	action: "approve" | "request-changes",
+): Promise<void> {
+	const client = createBitbucketClient(credentials);
+	const { response } =
+		action === "approve"
+			? await client.POST(
+					"/repositories/{workspace}/{repo_slug}/pullrequests/{pull_request_id}/approve",
+					{
+						params: {
+							path: {
+								workspace: ref.workspace,
+								repo_slug: ref.slug,
+								pull_request_id: pullRequestId,
+							},
+						},
+					},
+				)
+			: await client.POST(
+					"/repositories/{workspace}/{repo_slug}/pullrequests/{pull_request_id}/request-changes",
+					{
+						params: {
+							path: {
+								workspace: ref.workspace,
+								repo_slug: ref.slug,
+								pull_request_id: pullRequestId,
+							},
+						},
+					},
+				);
+
+	if (!response.ok) {
+		throw new PullRequestError(
+			`Failed to ${action} pull request #${pullRequestId}: HTTP ${response.status}.`,
+			response.status,
+		);
+	}
+}
+
+async function deleteParticipantAction(
+	credentials: Credentials,
+	ref: { workspace: string; slug: string },
+	pullRequestId: number,
+	action: "approve" | "request-changes",
+): Promise<void> {
+	const client = createBitbucketClient(credentials);
+	const { response } =
+		action === "approve"
+			? await client.DELETE(
+					"/repositories/{workspace}/{repo_slug}/pullrequests/{pull_request_id}/approve",
+					{
+						params: {
+							path: {
+								workspace: ref.workspace,
+								repo_slug: ref.slug,
+								pull_request_id: pullRequestId,
+							},
+						},
+					},
+				)
+			: await client.DELETE(
+					"/repositories/{workspace}/{repo_slug}/pullrequests/{pull_request_id}/request-changes",
+					{
+						params: {
+							path: {
+								workspace: ref.workspace,
+								repo_slug: ref.slug,
+								pull_request_id: pullRequestId,
+							},
+						},
+					},
+				);
+
+	if (!response.ok) {
+		throw new PullRequestError(
+			`Failed to withdraw ${action} on pull request #${pullRequestId}: HTTP ${response.status}.`,
+			response.status,
+		);
+	}
+}
+
+export async function approvePullRequest(
+	credentials: Credentials,
+	ref: { workspace: string; slug: string },
+	pullRequestId: number,
+): Promise<void> {
+	await postParticipantAction(credentials, ref, pullRequestId, "approve");
+}
+
+export async function unapprovePullRequest(
+	credentials: Credentials,
+	ref: { workspace: string; slug: string },
+	pullRequestId: number,
+): Promise<void> {
+	await deleteParticipantAction(credentials, ref, pullRequestId, "approve");
+}
+
+export async function requestChangesOnPullRequest(
+	credentials: Credentials,
+	ref: { workspace: string; slug: string },
+	pullRequestId: number,
+): Promise<void> {
+	await postParticipantAction(
+		credentials,
+		ref,
+		pullRequestId,
+		"request-changes",
+	);
+}
+
+export async function withdrawRequestChanges(
+	credentials: Credentials,
+	ref: { workspace: string; slug: string },
+	pullRequestId: number,
+): Promise<void> {
+	await deleteParticipantAction(
+		credentials,
+		ref,
+		pullRequestId,
+		"request-changes",
+	);
+}
+
 function toPullRequestDetail(pr: RawPullRequest): PullRequestDetail {
 	const base = toPullRequest(pr);
 	const raw = pr as Record<string, any>;
@@ -428,6 +579,7 @@ function toPullRequestDetail(pr: RawPullRequest): PullRequestDetail {
 		sourceBranch: String(raw.source?.branch?.name ?? ""),
 		destinationBranch: String(raw.destination?.branch?.name ?? ""),
 		reviewers: toReviewers(raw.participants),
+		participants: toParticipants(raw.participants),
 	};
 }
 
@@ -444,6 +596,19 @@ function toReviewers(raw: unknown): Reviewer[] {
 	return out;
 }
 
+function toParticipants(raw: unknown): Participant[] {
+	if (!Array.isArray(raw)) return [];
+	const out: Participant[] = [];
+	for (const p of raw as RawParticipant[]) {
+		const pp = p as Record<string, any>;
+		if (pp.role !== "PARTICIPANT") continue;
+		const account = toAuthor(pp.user);
+		if (!account) continue;
+		out.push({ account, state: toReviewState(pp.state) });
+	}
+	return out;
+}
+
 function toReviewState(raw: unknown): ReviewState {
 	if (raw === "approved") return "approved";
 	if (raw === "changes_requested") return "changes_requested";