diff --git a/src/backend/pullrequests/index.test.ts b/src/backend/pullrequests/index.test.ts index 0cc3051..475302e 100644 --- a/src/backend/pullrequests/index.test.ts +++ b/src/backend/pullrequests/index.test.ts @@ -6,6 +6,7 @@ import { createPullRequestComment, findOpenPullRequestForBranch, getPullRequest, + listEffectiveDefaultReviewers, listPullRequests, type PullRequest, type PullRequestDetail, @@ -547,6 +548,68 @@ describe("createPullRequest", () => { expect(seenBody!.draft).toBe(true); }); + test("omits the reviewers field when reviewerUuids is undefined", async () => { + let seenBody: Record | null = null; + server.use( + http.post(PR_LIST_PATH, async ({ request }) => { + seenBody = (await request.json()) as Record; + return HttpResponse.json(makePrDetail({ id: 110 }), { status: 201 }); + }), + ); + + await createPullRequest(creds, ref, { + title: "no reviewers", + description: "", + sourceBranch: "feature/x", + destinationBranch: "main", + }); + + expect(seenBody!).not.toHaveProperty("reviewers"); + }); + + test("omits the reviewers field when reviewerUuids is empty", async () => { + let seenBody: Record | null = null; + server.use( + http.post(PR_LIST_PATH, async ({ request }) => { + seenBody = (await request.json()) as Record; + return HttpResponse.json(makePrDetail({ id: 111 }), { status: 201 }); + }), + ); + + await createPullRequest(creds, ref, { + title: "empty reviewers", + description: "", + sourceBranch: "feature/x", + destinationBranch: "main", + reviewerUuids: [], + }); + + expect(seenBody!).not.toHaveProperty("reviewers"); + }); + + test("sends reviewers as [{type:'account', uuid}] when reviewerUuids is non-empty", async () => { + let seenBody: Record | null = null; + server.use( + http.post(PR_LIST_PATH, async ({ request }) => { + seenBody = (await request.json()) as Record; + return HttpResponse.json(makePrDetail({ id: 112 }), { status: 201 }); + }), + ); + + await createPullRequest(creds, ref, { + title: "with reviewers", + description: "", + sourceBranch: "feature/x", + destinationBranch: "main", + reviewerUuids: ["{alice}", "{bob}"], + }); + + expect(seenBody!.reviewers).toEqual([ + { type: "account", uuid: "{alice}" }, + { type: "account", uuid: "{bob}" }, + ]); + }); + test("throws PullRequestError on 400 (validation failure)", async () => { server.use( http.post(PR_LIST_PATH, () => @@ -569,6 +632,68 @@ describe("createPullRequest", () => { }); }); +describe("listEffectiveDefaultReviewers", () => { + const PATH = `${BITBUCKET_BASE}/repositories/ws/repo/effective-default-reviewers`; + + test("returns uuids in document order, skipping entries without one", async () => { + server.use( + http.get(PATH, () => + HttpResponse.json({ + values: [ + { + type: "default_reviewer_and_type", + reviewer_type: "repository", + user: { uuid: "{alice}", nickname: "alice" }, + }, + { + type: "default_reviewer_and_type", + reviewer_type: "project", + user: { uuid: "{bob}", nickname: "bob" }, + }, + // pathological: missing user → skip silently + { type: "default_reviewer_and_type", reviewer_type: "project" }, + // pathological: empty uuid → skip silently + { + type: "default_reviewer_and_type", + reviewer_type: "project", + user: { uuid: "", nickname: "ghost" }, + }, + ], + }), + ), + ); + + const out = await listEffectiveDefaultReviewers(creds, ref); + expect(out).toEqual(["{alice}", "{bob}"]); + }); + + test("empty list when no defaults configured", async () => { + server.use(http.get(PATH, () => HttpResponse.json({ values: [] }))); + + const out = await listEffectiveDefaultReviewers(creds, ref); + expect(out).toEqual([]); + }); + + test("treats missing values array as empty", async () => { + server.use(http.get(PATH, () => HttpResponse.json({}))); + + const out = await listEffectiveDefaultReviewers(creds, ref); + expect(out).toEqual([]); + }); + + test("throws PullRequestError on 403", async () => { + server.use( + http.get(PATH, () => + HttpResponse.json({ type: "error" }, { status: 403 }), + ), + ); + + const err = await listEffectiveDefaultReviewers(creds, ref).catch((e) => e); + expect(err).toBeInstanceOf(PullRequestError); + expect((err as PullRequestError).status).toBe(403); + }); +}); + describe("createPullRequestComment", () => { const COMMENTS_PATH = (id: number) => `${BITBUCKET_BASE}/repositories/ws/repo/pullrequests/${id}/comments`; diff --git a/src/backend/pullrequests/index.ts b/src/backend/pullrequests/index.ts index 7669f4b..07441ac 100644 --- a/src/backend/pullrequests/index.ts +++ b/src/backend/pullrequests/index.ts @@ -191,16 +191,24 @@ export type CreatePullRequestInput = { sourceBranch: string; destinationBranch: string; draft?: boolean; + /** + * UUIDs (curly-brace form) to send as reviewers. The caller is responsible + * for resolving + filtering them (e.g. removing the author, deduping). We + * pass them through verbatim. + */ + reviewerUuids?: string[]; }; /** * POSTs a new pull request to Bitbucket. The source repo is implied by the * path (we only create PRs in the current repo, never from forks at this - * stage). Reviewers are omitted: our Bitbucket workspaces auto-assign - * reviewers based on code-owner settings. + * stage). * * `draft: true` is only included in the body when explicitly set; we never * send `draft: false` so the server's default applies. + * + * `reviewers` is only included when `reviewerUuids` is non-empty — sending + * an empty array is legal but noisy. */ export async function createPullRequest( credentials: Credentials, @@ -221,6 +229,14 @@ export async function createPullRequest( source: { branch: { name: input.sourceBranch } }, destination: { branch: { name: input.destinationBranch } }, ...(input.draft ? { draft: true } : {}), + ...(input.reviewerUuids && input.reviewerUuids.length > 0 + ? { + reviewers: input.reviewerUuids.map((uuid) => ({ + type: "account" as const, + uuid, + })), + } + : {}), }, }, ); @@ -308,6 +324,52 @@ export async function findOpenPullRequestForBranch( return first ? toPullRequest(first as RawPullRequest) : null; } +/** + * Returns the UUIDs (curly-brace form) of the reviewers configured as defaults + * on the destination repo, including those inherited from the project. This + * matches what the Bitbucket UI shows under Repository settings → Default + * reviewers. + * + * Single page of pagelen=100 — real-world default-reviewer lists are tiny + * (<10). If anyone ever configures more we'll add pagination. + * + * Entries without a uuid are skipped silently. We surface a non-2xx as a + * PullRequestError; the caller MUST propagate it rather than silently + * creating a reviewerless PR. + */ +export async function listEffectiveDefaultReviewers( + credentials: Credentials, + ref: { workspace: string; slug: string }, +): Promise { + const client = createBitbucketClient(credentials); + const { data, response } = await client.GET( + "/repositories/{workspace}/{repo_slug}/effective-default-reviewers", + { + params: { + path: { workspace: ref.workspace, repo_slug: ref.slug }, + }, + }, + ); + + if (!response.ok || !data) { + throw new PullRequestError( + `Failed to fetch effective default reviewers: HTTP ${response.status}.`, + response.status, + ); + } + + const values = (data as { values?: Array<{ user?: { uuid?: string } }> }) + .values; + if (!values) return []; + + const uuids: string[] = []; + for (const entry of values) { + const uuid = entry.user?.uuid; + if (typeof uuid === "string" && uuid.length > 0) uuids.push(uuid); + } + return uuids; +} + export type PullRequestCommentResult = { id: number; url: string; diff --git a/src/commands/pullrequest/create.ts b/src/commands/pullrequest/create.ts index 48b7f7e..22e96d5 100644 --- a/src/commands/pullrequest/create.ts +++ b/src/commands/pullrequest/create.ts @@ -1,7 +1,9 @@ import { createPullRequest, + listEffectiveDefaultReviewers, PullRequestError, } from "../../backend/pullrequests/index.ts"; +import { getCurrentUser, UserError } from "../../backend/user/index.ts"; import { loadConfigOrExit } from "../../shared/config/index.ts"; import { BodyInputError, @@ -60,12 +62,27 @@ export async function runPullRequestCreate( bodyFile: options.bodyFile, }); + // Bitbucket's POST /pullrequests does not honor the repo's default + // reviewers configuration. To match the web UI we read the effective + // list (repo + project-inherited) and inline the UUIDs in the POST, + // minus the authenticated user (Bitbucket rejects a body where the + // author is also a reviewer). Failures here MUST propagate — we never + // silently create a reviewerless PR; that's the bug we're fixing. + const [me, defaultReviewerUuids] = await Promise.all([ + getCurrentUser(config), + listEffectiveDefaultReviewers(config, ref), + ]); + const reviewerUuids = defaultReviewerUuids.filter( + (uuid) => uuid !== me.uuid, + ); + const pr = await createPullRequest(config, ref, { title: options.title, description: body, sourceBranch: branch, destinationBranch: destination, draft: options.draft, + reviewerUuids, }); renderer.message(pr.url); @@ -73,6 +90,7 @@ export async function runPullRequestCreate( if ( err instanceof RepositoryResolutionError || err instanceof PullRequestError || + err instanceof UserError || err instanceof BodyInputError || err instanceof EditorError ) {