Skip to content
Open
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
58 changes: 56 additions & 2 deletions src/backend/pullrequests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,14 @@ function makePrDetail(
summary: {
raw: "A detailed PR description.\n\n- fix thing\n- fix other thing",
},
source: { branch: { name: "feature/auth" } },
destination: { branch: { name: "main" } },
source: {
branch: { name: "feature/auth" },
repository: { full_name: "ws/repo" },
},
destination: {
branch: { name: "main" },
repository: { full_name: "ws/repo" },
},
links: { html: { href: "https://bitbucket.org/ws/repo/pull-requests/42" } },
participants: [
{
Expand Down Expand Up @@ -380,6 +386,8 @@ describe("getPullRequest", () => {
"A detailed PR description.\n\n- fix thing\n- fix other thing",
sourceBranch: "feature/auth",
destinationBranch: "main",
sourceRepositoryFullName: "ws/repo",
destinationRepositoryFullName: "ws/repo",
reviewers: [
{
account: { uuid: "{bob}", displayName: "Bob", nickname: "bob" },
Expand Down Expand Up @@ -415,6 +423,52 @@ describe("getPullRequest", () => {
expect((err as PullRequestError).status).toBe(404);
});

test("surfaces source vs destination repository full_name for fork detection", async () => {
// Fork-based PRs carry a different `source.repository.full_name` from
// the destination. Consumers (bb pr checkout) rely on this distinction
// to refuse fork checkouts cleanly.
server.use(
http.get(PR_DETAIL_PATH(42), () =>
HttpResponse.json(
makePrDetail({
source: {
branch: { name: "patch-1" },
repository: { full_name: "fork-ws/repo" },
},
destination: {
branch: { name: "main" },
repository: { full_name: "ws/repo" },
},
}),
),
),
);

const result = await getPullRequest(creds, ref, 42);
expect(result.sourceRepositoryFullName).toBe("fork-ws/repo");
expect(result.destinationRepositoryFullName).toBe("ws/repo");
});

test("falls back to empty strings when repository metadata is missing", async () => {
// Bitbucket occasionally omits the repository reference on same-repo
// PRs. Treating the fields as optional (empty) keeps `bb pr checkout`'s
// fork check from producing false positives.
server.use(
http.get(PR_DETAIL_PATH(42), () =>
HttpResponse.json(
makePrDetail({
source: { branch: { name: "feature/auth" } },
destination: { branch: { name: "main" } },
}),
),
),
);

const result = await getPullRequest(creds, ref, 42);
expect(result.sourceRepositoryFullName).toBe("");
expect(result.destinationRepositoryFullName).toBe("");
});

test("handles a PR with no participants", async () => {
server.use(
http.get(PR_DETAIL_PATH(42), () =>
Expand Down
12 changes: 12 additions & 0 deletions src/backend/pullrequests/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,14 @@ export type PullRequestDetail = PullRequest & {
description: string;
sourceBranch: string;
destinationBranch: string;
/**
* `workspace/slug` of the source repo. Differs from the destination when
* the PR is fork-based. Consumers that care about cross-repo semantics
* (e.g. `bb pr checkout`, which only supports same-repo PRs) compare
* against `destinationRepositoryFullName`.
*/
sourceRepositoryFullName: string;
destinationRepositoryFullName: string;
reviewers: Reviewer[];
participants: Participant[];
};
Expand Down Expand Up @@ -615,6 +623,10 @@ function toPullRequestDetail(pr: RawPullRequest): PullRequestDetail {
description: String(raw.summary?.raw ?? raw.description ?? ""),
sourceBranch: String(raw.source?.branch?.name ?? ""),
destinationBranch: String(raw.destination?.branch?.name ?? ""),
sourceRepositoryFullName: String(raw.source?.repository?.full_name ?? ""),
destinationRepositoryFullName: String(
raw.destination?.repository?.full_name ?? "",
),
reviewers: toReviewers(raw.participants),
participants: toParticipants(raw.participants),
};
Expand Down
118 changes: 118 additions & 0 deletions src/commands/pullrequest/checkout.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
import {
getPullRequest,
type PullRequestDetail,
PullRequestError,
} from "../../backend/pullrequests/index.ts";
import { loadConfigOrExit } from "../../shared/config/index.ts";
import type { Renderer } from "../../shared/renderer/index.ts";
import {
defaultGitRunner,
GitError,
type GitRunner,
RepositoryResolutionError,
resolveRepository,
} from "../../shared/repository/index.ts";

export type PullRequestCheckoutOptions = {
repository?: string;
};

/**
* Fetches the PR's source branch and either creates a local branch tracking
* it, or fast-forwards an existing local branch. Same-repo PRs only — fork
* checkouts are out of scope (and meaningfully harder: need a temporary
* remote, branch rename to avoid collisions, and cleanup semantics).
*/
export async function runPullRequestCheckout(
renderer: Renderer,
idArg: string,
options: PullRequestCheckoutOptions,
git: GitRunner = defaultGitRunner,
): Promise<void> {
const id = parseId(idArg);
if (id === null) {
renderer.error(`Invalid PR id '${idArg}'. Expected a positive integer.`);
process.exit(1);
}

const config = await loadConfigOrExit(renderer);

try {
const ref = await resolveRepository({ override: options.repository });
const pr = await getPullRequest(config, ref, id);

ensureSameRepo(pr, ref);

const branch = pr.sourceBranch;
if (!branch) {
renderer.error(
`Pull request #${id} has no source branch — nothing to check out.`,
);
process.exit(1);
}

const cwd = process.cwd();
const remote = "origin";
const remoteRef = `${remote}/${branch}`;

await git.fetchRef(cwd, remote, branch);

const existed = await git.hasLocalBranch(cwd, branch);
if (existed) {
await git.checkoutExistingBranch(cwd, branch);
await git.mergeFastForwardOnly(cwd, remoteRef);
} else {
await git.checkoutCreateTracking(cwd, branch, remoteRef);
}

const action = existed ? "fast-forwarded" : "created";
if (renderer.json) {
renderer.detail({ id, branch, action }, []);
return;
}
renderer.message(
existed
? `Checked out pull request #${id}: ${branch} (fast-forwarded).`
: `Checked out pull request #${id}: ${branch}.`,
);
} catch (err) {
if (
err instanceof RepositoryResolutionError ||
err instanceof PullRequestError
) {
renderer.error(err.message);
process.exit(1);
}
if (err instanceof GitError) {
// Surface git's own stderr verbatim — it's almost always clearer
// than anything we could paraphrase (dirty working tree, diverged
// branches, missing remote ref, etc.).
renderer.error(err.message);
process.exit(1);
}
throw err;
}
}

function ensureSameRepo(
pr: PullRequestDetail,
ref: { workspace: string; slug: string },
): void {
const source = pr.sourceRepositoryFullName;
const destination = pr.destinationRepositoryFullName;
// Empty strings mean Bitbucket didn't send the field — treat as same-repo
// (the common case for non-fork PRs; the server sometimes omits the
// repository reference when source == destination).
if (!source || !destination) return;
if (source.toLowerCase() === destination.toLowerCase()) return;
throw new PullRequestError(
`Pull request source repo '${source}' differs from '${ref.workspace}/${ref.slug}'. ` +
"Fork-based PR checkout is not supported yet — open a follow-up if you need this.",
);
}

function parseId(raw: string): number | null {
const n = Number(raw);
if (!Number.isInteger(n) || n <= 0) return null;
return n;
}
12 changes: 12 additions & 0 deletions src/commands/pullrequest/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { Command } from "commander";
import { withRenderer } from "../../shared/renderer/commander.ts";
import { runPullRequestCheckout } from "./checkout.ts";
import { runPullRequestComment } from "./comment.ts";
import { runPullRequestCreate } from "./create.ts";
import { runPullRequestDiff } from "./diff.ts";
Expand Down Expand Up @@ -86,6 +87,17 @@ export function registerPullRequestCommands(program: Command): void {
)
.action(withRenderer(runPullRequestDiff));

pr.command("checkout")
.description(
"Fetch and check out a pull request's source branch locally (same-repo PRs only)",
)
.argument("<id>", "Pull request number")
.option(
"-R, --repository <workspace/repo>",
"Override repository detection",
)
.action(withRenderer(runPullRequestCheckout));

pr.command("review")
.description(
"Submit a review on a pull request (defaults to the PR for the current branch)",
Expand Down
128 changes: 127 additions & 1 deletion src/shared/repository/git.integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { mkdtemp, rm } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { $ } from "bun";
import { defaultGitRunner } from "./git.ts";
import { defaultGitRunner, GitError } from "./git.ts";
import { RepositoryResolutionError, resolveRepository } from "./resolve.ts";

/**
Expand Down Expand Up @@ -164,6 +164,132 @@ describe("defaultGitRunner against a local bare remote", () => {
await defaultGitRunner.getDefaultBranchFromRemote(repoDir, "origin"),
).toBeUndefined();
});

test("fetchRef pulls a branch that was pushed to the remote after clone", async () => {
// Push a new branch to the bare remote from a side seed, then verify
// the clone can fetch it by name and ends up with the remote-tracking
// ref.
const sideDir = join(cloneDir, "..", "side");
await $`git clone -q ${bareDir} ${sideDir}`.quiet();
await $`git -C ${sideDir} -c user.email=s@b.co -c user.name=Side checkout -b feature/fetch-me`.quiet();
await $`git -C ${sideDir} -c user.email=s@b.co -c user.name=Side commit --allow-empty -m "side work"`.quiet();
await $`git -C ${sideDir} push -q origin feature/fetch-me`.quiet();

await defaultGitRunner.fetchRef(cloneDir, "origin", "feature/fetch-me");

const remoteSha = await defaultGitRunner.getSha(
cloneDir,
"refs/remotes/origin/feature/fetch-me",
);
expect(remoteSha).toBeDefined();
});

test("fetchRef throws GitError for a branch that does not exist on the remote", async () => {
const err = await defaultGitRunner
.fetchRef(cloneDir, "origin", "no-such-branch")
.catch((e: unknown) => e);
expect(err).toBeInstanceOf(GitError);
});

test("hasLocalBranch reports the presence of local branches", async () => {
expect(await defaultGitRunner.hasLocalBranch(cloneDir, "main")).toBe(true);
expect(
await defaultGitRunner.hasLocalBranch(cloneDir, "never-created"),
).toBe(false);
});

test("checkoutCreateTracking creates a local branch tracking the remote ref", async () => {
// Seed a branch on the remote, fetch it, then use checkoutCreateTracking
// to create a local copy.
const seedDir = join(cloneDir, "..", "seed2");
await $`git clone -q ${bareDir} ${seedDir}`.quiet();
await $`git -C ${seedDir} -c user.email=s@b.co -c user.name=Side checkout -b feature/track-me`.quiet();
await $`git -C ${seedDir} -c user.email=s@b.co -c user.name=Side commit --allow-empty -m "track"`.quiet();
await $`git -C ${seedDir} push -q origin feature/track-me`.quiet();

await defaultGitRunner.fetchRef(cloneDir, "origin", "feature/track-me");
await defaultGitRunner.checkoutCreateTracking(
cloneDir,
"feature/track-me",
"origin/feature/track-me",
);

expect(
await defaultGitRunner.hasLocalBranch(cloneDir, "feature/track-me"),
).toBe(true);
// Current branch is now feature/track-me
expect(await defaultGitRunner.getCurrentBranch(cloneDir)).toBe(
"feature/track-me",
);

// Clean up: switch back to main for subsequent tests.
await defaultGitRunner.checkoutExistingBranch(cloneDir, "main");
});

test("checkoutExistingBranch switches to an existing local branch", async () => {
// We should currently be on main (reset at the end of the previous test).
expect(await defaultGitRunner.getCurrentBranch(cloneDir)).toBe("main");

await defaultGitRunner.checkoutExistingBranch(cloneDir, "feature/track-me");
expect(await defaultGitRunner.getCurrentBranch(cloneDir)).toBe(
"feature/track-me",
);

await defaultGitRunner.checkoutExistingBranch(cloneDir, "main");
});

test("checkoutCreateTracking throws GitError when the branch already exists", async () => {
const err = await defaultGitRunner
.checkoutCreateTracking(
cloneDir,
"feature/track-me",
"origin/feature/track-me",
)
.catch((e: unknown) => e);
expect(err).toBeInstanceOf(GitError);
});

test("mergeFastForwardOnly fast-forwards when the branch is strictly behind", async () => {
// Advance the remote's main by one commit via a side clone, then
// fetch + ff in our main clone.
const ahead = join(cloneDir, "..", "ahead");
await $`git clone -q ${bareDir} ${ahead}`.quiet();
await $`git -C ${ahead} -c user.email=a@b.co -c user.name=Ahead commit --allow-empty -m "ahead"`.quiet();
await $`git -C ${ahead} push -q origin main`.quiet();

await defaultGitRunner.fetchRef(cloneDir, "origin", "main");
await defaultGitRunner.mergeFastForwardOnly(cloneDir, "origin/main");

const localSha = await defaultGitRunner.getSha(cloneDir, "HEAD");
const remoteSha = await defaultGitRunner.getSha(
cloneDir,
"refs/remotes/origin/main",
);
expect(localSha).toBe(remoteSha);
});

test("mergeFastForwardOnly throws GitError when the branches have diverged", async () => {
// Real divergence: make a fresh clone, commit locally (one side of
// the diverge), then push a different commit to origin/main from a
// second clone (the other side). Now the common ancestor is in the
// past but neither branch is an ancestor of the other → git refuses
// to ff.
const divergeDir = join(cloneDir, "..", "diverge");
await $`git clone -q ${bareDir} ${divergeDir}`.quiet();
await $`git -C ${divergeDir} -c user.email=d@b.co -c user.name=Diverge commit --allow-empty -m "local diverge"`.quiet();

const remoteSide = join(cloneDir, "..", "remote-side");
await $`git clone -q ${bareDir} ${remoteSide}`.quiet();
await $`git -C ${remoteSide} -c user.email=r@b.co -c user.name=Remote commit --allow-empty -m "remote diverge"`.quiet();
await $`git -C ${remoteSide} push -q origin main`.quiet();

await defaultGitRunner.fetchRef(divergeDir, "origin", "main");

const err = await defaultGitRunner
.mergeFastForwardOnly(divergeDir, "origin/main")
.catch((e: unknown) => e);
expect(err).toBeInstanceOf(GitError);
});
});

describe("resolveRepository with real git", () => {
Expand Down
Loading
Loading