Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
125 changes: 125 additions & 0 deletions src/backend/pullrequests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
createPullRequestComment,
findOpenPullRequestForBranch,
getPullRequest,
listEffectiveDefaultReviewers,
listPullRequests,
type PullRequest,
type PullRequestDetail,
Expand Down Expand Up @@ -547,6 +548,68 @@ describe("createPullRequest", () => {
expect(seenBody!.draft).toBe(true);
});

test("omits the reviewers field when reviewerUuids is undefined", async () => {
let seenBody: Record<string, any> | null = null;
server.use(
http.post(PR_LIST_PATH, async ({ request }) => {
seenBody = (await request.json()) as Record<string, any>;
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<string, any> | null = null;
server.use(
http.post(PR_LIST_PATH, async ({ request }) => {
seenBody = (await request.json()) as Record<string, any>;
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<string, any> | null = null;
server.use(
http.post(PR_LIST_PATH, async ({ request }) => {
seenBody = (await request.json()) as Record<string, any>;
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, () =>
Expand All @@ -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`;
Expand Down
66 changes: 64 additions & 2 deletions src/backend/pullrequests/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
})),
}
: {}),
},
},
);
Expand Down Expand Up @@ -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<string[]> {
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;
Expand Down
18 changes: 18 additions & 0 deletions src/commands/pullrequest/create.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -60,19 +62,35 @@ 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);
} catch (err) {
if (
err instanceof RepositoryResolutionError ||
err instanceof PullRequestError ||
err instanceof UserError ||
err instanceof BodyInputError ||
err instanceof EditorError
) {
Expand Down
Loading