diff --git a/src/backend/pullrequests/index.test.ts b/src/backend/pullrequests/index.test.ts index e30b5f4..c80f122 100644 --- a/src/backend/pullrequests/index.test.ts +++ b/src/backend/pullrequests/index.test.ts @@ -6,10 +6,12 @@ import { createPullRequest, createPullRequestComment, findOpenPullRequestForBranch, + getMergeTaskStatus, getPullRequest, getPullRequestDiff, listEffectiveDefaultReviewers, listPullRequests, + mergePullRequest, type PullRequest, type PullRequestDetail, PullRequestError, @@ -380,6 +382,8 @@ describe("getPullRequest", () => { "A detailed PR description.\n\n- fix thing\n- fix other thing", sourceBranch: "feature/auth", destinationBranch: "main", + defaultMergeStrategy: null, + allowedMergeStrategies: [], reviewers: [ { account: { uuid: "{bob}", displayName: "Bob", nickname: "bob" }, @@ -415,6 +419,72 @@ describe("getPullRequest", () => { expect((err as PullRequestError).status).toBe(404); }); + test("requests merge-strategy fields additively via fields=+...", async () => { + // Bitbucket omits `default_merge_strategy` and `merge_strategies` + // from the default serialization; bb pr merge needs both. Assert the + // `fields` query param is present so we don't silently stop asking + // for them. + let seenFields: string | null = null; + server.use( + http.get(PR_DETAIL_PATH(42), ({ request }) => { + seenFields = new URL(request.url).searchParams.get("fields"); + return HttpResponse.json(makePrDetail()); + }), + ); + + await getPullRequest(creds, ref, 42); + + expect(seenFields!).toBe( + "+destination.branch.default_merge_strategy,+destination.branch.merge_strategies", + ); + }); + + test("maps destination.branch merge-strategy fields", async () => { + server.use( + http.get(PR_DETAIL_PATH(42), () => + HttpResponse.json( + makePrDetail({ + destination: { + branch: { + name: "main", + default_merge_strategy: "squash", + merge_strategies: [ + "merge_commit", + "squash", + "unknown_future_value", + ], + }, + }, + }), + ), + ), + ); + + const result = await getPullRequest(creds, ref, 42); + expect(result.defaultMergeStrategy).toBe("squash"); + // Unknown strategies are dropped from the allowed list — we'd rather + // refuse to forward a bogus value than silently forward it to the + // API and take a 400 later. + expect(result.allowedMergeStrategies).toEqual(["merge_commit", "squash"]); + }); + + test("default merge strategy is null when server sends an unknown or missing value", async () => { + server.use( + http.get(PR_DETAIL_PATH(42), () => + HttpResponse.json( + makePrDetail({ + destination: { + branch: { name: "main", default_merge_strategy: "bogus" }, + }, + }), + ), + ), + ); + + const result = await getPullRequest(creds, ref, 42); + expect(result.defaultMergeStrategy).toBeNull(); + }); + test("handles a PR with no participants", async () => { server.use( http.get(PR_DETAIL_PATH(42), () => @@ -921,6 +991,215 @@ describe("review action endpoints (approve / unapprove / request-changes / unreq }); }); +describe("mergePullRequest", () => { + const MERGE_PATH = (id: number) => + `${BITBUCKET_BASE}/repositories/ws/repo/pullrequests/${id}/merge`; + + test("POSTs the merge_strategy and returns the merged PR on 200", async () => { + let seenBody: Record | null = null; + server.use( + http.post(MERGE_PATH(42), async ({ request }) => { + seenBody = (await request.json()) as Record; + return HttpResponse.json(makePrDetail({ id: 42, state: "MERGED" }), { + status: 200, + }); + }), + ); + + const result = await mergePullRequest(creds, ref, 42, { + mergeStrategy: "squash", + }); + + expect(seenBody!).toEqual({ + type: "pullrequest_merge_parameters", + merge_strategy: "squash", + }); + expect(result.kind).toBe("done"); + if (result.kind === "done") { + expect(result.pr.id).toBe(42); + expect(result.pr.state).toBe("MERGED"); + } + }); + + test("passes message and close_source_branch through when set", async () => { + let seenBody: Record | null = null; + server.use( + http.post(MERGE_PATH(42), async ({ request }) => { + seenBody = (await request.json()) as Record; + return HttpResponse.json(makePrDetail({ id: 42, state: "MERGED" })); + }), + ); + + await mergePullRequest(creds, ref, 42, { + mergeStrategy: "merge_commit", + message: "custom msg", + closeSourceBranch: true, + }); + + expect(seenBody!).toEqual({ + type: "pullrequest_merge_parameters", + merge_strategy: "merge_commit", + message: "custom msg", + close_source_branch: true, + }); + }); + + test("omits optional fields when not provided", async () => { + let seenBody: Record | null = null; + server.use( + http.post(MERGE_PATH(42), async ({ request }) => { + seenBody = (await request.json()) as Record; + return HttpResponse.json(makePrDetail({ id: 42, state: "MERGED" })); + }), + ); + + await mergePullRequest(creds, ref, 42, { mergeStrategy: "squash" }); + + expect(seenBody!).not.toHaveProperty("message"); + expect(seenBody!).not.toHaveProperty("close_source_branch"); + }); + + test("returns {kind:'async', taskUrl} on 202 with Location header", async () => { + const location = `${MERGE_PATH(42)}/task-status/abc123`; + server.use( + http.post( + MERGE_PATH(42), + () => + new HttpResponse(null, { + status: 202, + headers: { Location: location }, + }), + ), + ); + + const result = await mergePullRequest(creds, ref, 42, { + mergeStrategy: "merge_commit", + }); + + expect(result).toEqual({ kind: "async", taskUrl: location }); + }); + + test("throws PullRequestError on 202 with no Location header", async () => { + server.use( + http.post(MERGE_PATH(42), () => new HttpResponse(null, { status: 202 })), + ); + + const err = await mergePullRequest(creds, ref, 42, { + mergeStrategy: "merge_commit", + }).catch((e) => e); + + expect(err).toBeInstanceOf(PullRequestError); + expect((err as PullRequestError).status).toBe(202); + }); + + test("409 surfaces as a PullRequestError with a retry-friendly message", async () => { + server.use( + http.post(MERGE_PATH(42), () => new HttpResponse(null, { status: 409 })), + ); + + const err = await mergePullRequest(creds, ref, 42, { + mergeStrategy: "merge_commit", + }).catch((e) => e); + + expect(err).toBeInstanceOf(PullRequestError); + expect((err as PullRequestError).status).toBe(409); + expect((err as Error).message).toContain("refs changed"); + }); + + test("555 surfaces as a PullRequestError with a timeout message", async () => { + server.use( + http.post(MERGE_PATH(42), () => + HttpResponse.json( + { type: "error", error: { message: "timed out" } }, + { status: 555 }, + ), + ), + ); + + const err = await mergePullRequest(creds, ref, 42, { + mergeStrategy: "merge_commit", + }).catch((e) => e); + + expect(err).toBeInstanceOf(PullRequestError); + expect((err as PullRequestError).status).toBe(555); + expect((err as Error).message).toContain("timed out"); + }); +}); + +describe("getMergeTaskStatus", () => { + const TASK_URL = + "https://api.bitbucket.org/2.0/repositories/ws/repo/pullrequests/42/merge/task-status/abc"; + + test("maps PENDING task status to { status: 'PENDING' }", async () => { + server.use( + http.get(TASK_URL, () => + HttpResponse.json({ + task_status: "PENDING", + links: { self: { href: TASK_URL } }, + }), + ), + ); + + const result = await getMergeTaskStatus(creds, TASK_URL); + expect(result).toEqual({ status: "PENDING" }); + }); + + test("maps SUCCESS task status to { status:'SUCCESS', pr }", async () => { + server.use( + http.get(TASK_URL, () => + HttpResponse.json({ + task_status: "SUCCESS", + links: { self: { href: TASK_URL } }, + merge_result: makePrDetail({ id: 42, state: "MERGED" }), + }), + ), + ); + + const result = await getMergeTaskStatus(creds, TASK_URL); + expect(result.status).toBe("SUCCESS"); + if (result.status === "SUCCESS") { + expect(result.pr.id).toBe(42); + expect(result.pr.state).toBe("MERGED"); + } + }); + + test("maps any other task status to { status:'FAILED', error }", async () => { + server.use( + http.get(TASK_URL, () => + HttpResponse.json({ + task_status: "FAILED", + error: { message: "conflict at src/x.ts" }, + }), + ), + ); + + const result = await getMergeTaskStatus(creds, TASK_URL); + expect(result).toEqual({ + status: "FAILED", + error: "conflict at src/x.ts", + }); + }); + + test("SUCCESS without merge_result throws a PullRequestError", async () => { + server.use( + http.get(TASK_URL, () => HttpResponse.json({ task_status: "SUCCESS" })), + ); + + const err = await getMergeTaskStatus(creds, TASK_URL).catch((e) => e); + expect(err).toBeInstanceOf(PullRequestError); + }); + + test("non-2xx propagates as PullRequestError with status", async () => { + server.use( + http.get(TASK_URL, () => new HttpResponse(null, { status: 500 })), + ); + + const err = await getMergeTaskStatus(creds, TASK_URL).catch((e) => e); + expect(err).toBeInstanceOf(PullRequestError); + expect((err as PullRequestError).status).toBe(500); + }); +}); + describe("getPullRequestDiff", () => { const DIFF_PATH = (id: number) => `${BITBUCKET_BASE}/repositories/ws/repo/pullrequests/${id}/diff`; diff --git a/src/backend/pullrequests/index.ts b/src/backend/pullrequests/index.ts index 0d7236f..0a810ab 100644 --- a/src/backend/pullrequests/index.ts +++ b/src/backend/pullrequests/index.ts @@ -27,6 +27,26 @@ export type PullRequestState = | "DECLINED" | "SUPERSEDED"; +/** + * Merge strategies recognized by Bitbucket on `POST /pullrequests/{id}/merge`. + * Mirrors the `pullrequest_merge_parameters.merge_strategy` enum (generated + * schema). The list *allowed* on any given destination branch is a subset + * exposed as `PullRequestDetail.allowedMergeStrategies`. + */ +export const MERGE_STRATEGIES = [ + "merge_commit", + "squash", + "fast_forward", + "squash_fast_forward", + "rebase_fast_forward", + "rebase_merge", +] as const; +export type MergeStrategy = (typeof MERGE_STRATEGIES)[number]; + +export function isMergeStrategy(value: string): value is MergeStrategy { + return (MERGE_STRATEGIES as readonly string[]).includes(value); +} + export type PullRequestAuthor = { uuid: string; displayName: string; @@ -65,6 +85,20 @@ export type PullRequestDetail = PullRequest & { description: string; sourceBranch: string; destinationBranch: string; + /** + * The destination branch's configured default merge strategy, or null + * when the branch has no default set (Bitbucket's own fallback is + * `"merge_commit"`). Only populated when the fetch explicitly requests + * `fields=+destination.branch.default_merge_strategy`. + */ + defaultMergeStrategy: MergeStrategy | null; + /** + * Strategies the destination branch permits on merge. Empty array when + * the branch has no restrictions configured (in which case the server + * allows all). Only populated when the fetch explicitly requests + * `fields=+destination.branch.merge_strategies`. + */ + allowedMergeStrategies: MergeStrategy[]; reviewers: Reviewer[]; participants: Participant[]; }; @@ -266,9 +300,25 @@ export async function createPullRequest( } /** - * Fetches a single pull request by id. Single typed call — the overlay - * (BBC2-38) gives us typed path params and response shape, so we stay on - * openapi-fetch's client rather than dropping to raw fetch. + * Fields the default serialization omits, requested additively via Bitbucket's + * `fields=+foo,+bar` mechanism. Both live on `destination.branch` and are + * required for `bb pr merge` — the default strategy and the client-side + * strategy validation list. See docs/bb-notes.md → "Default merge strategy". + * + * Adding to the default response (rather than dropping fields) means + * existing consumers of this function are unaffected. + */ +const PR_EXTRA_FIELDS = [ + "+destination.branch.default_merge_strategy", + "+destination.branch.merge_strategies", +].join(","); + +/** + * Fetches a single pull request by id. Augments the default response with + * merge-strategy fields via `fields=+...` — the typed path params stay + * intact; only the query params need a local `as any` because openapi-fetch's + * generated types declare `query?: never` for this endpoint (see + * docs/bb-notes.md → "Gotcha: `fields` is not declared in the OpenAPI spec"). */ export async function getPullRequest( credentials: Credentials, @@ -285,6 +335,10 @@ export async function getPullRequest( repo_slug: ref.slug, pull_request_id: id, }, + // `fields` isn't in the generated types for this endpoint; see + // docs/bb-notes.md → "Gotcha: `fields` is not declared in the + // OpenAPI spec". Cast to loosen the typed contract just here. + query: { fields: PR_EXTRA_FIELDS } as unknown as undefined, }, }, ); @@ -607,6 +661,142 @@ export async function withdrawRequestChanges( ); } +export type MergeInput = { + mergeStrategy: MergeStrategy; + /** Free-form merge commit message. Ignored by `fast_forward`. Max 128 KiB. */ + message?: string; + /** + * Server-side branch cleanup. When true, Bitbucket deletes the source + * branch from the remote after the merge lands. Local-side cleanup is + * the command layer's responsibility. + */ + closeSourceBranch?: boolean; +}; + +/** + * Outcome of the sync merge POST. The server returns: + * 200 → merge complete, body is the updated PR (mapped to `{kind:"done"}`). + * 202 → merge running async (either because it exceeded the sync timeout + * or the caller opted in). The `Location` header carries the + * task-status polling URL (mapped to `{kind:"async"}`). + * + * 4xx/5xx/555 propagate as `PullRequestError` — callers map them to user + * messages (409 "refs changed, retry", 555 "timed out, retry", etc.). + */ +export type MergeResult = + | { kind: "done"; pr: PullRequestDetail } + | { kind: "async"; taskUrl: string }; + +export async function mergePullRequest( + credentials: Credentials, + ref: { workspace: string; slug: string }, + pullRequestId: number, + input: MergeInput, +): Promise { + const client = createBitbucketClient(credentials); + const { data, response } = await client.POST( + "/repositories/{workspace}/{repo_slug}/pullrequests/{pull_request_id}/merge", + { + params: { + path: { + workspace: ref.workspace, + repo_slug: ref.slug, + pull_request_id: pullRequestId, + }, + }, + body: { + type: "pullrequest_merge_parameters", + merge_strategy: input.mergeStrategy, + ...(input.message !== undefined ? { message: input.message } : {}), + ...(input.closeSourceBranch !== undefined + ? { close_source_branch: input.closeSourceBranch } + : {}), + }, + }, + ); + + if (response.status === 202) { + const location = response.headers.get("location"); + if (!location) { + throw new PullRequestError( + `Merge of pull request #${pullRequestId} returned 202 with no Location header — cannot poll.`, + 202, + ); + } + return { kind: "async", taskUrl: location }; + } + + if (!response.ok || !data) { + throw new PullRequestError( + mergeErrorMessage(pullRequestId, response.status), + response.status, + ); + } + + return { kind: "done", pr: toPullRequestDetail(data as RawPullRequest) }; +} + +function mergeErrorMessage(pullRequestId: number, status: number): string { + switch (status) { + case 409: + return `Failed to merge pull request #${pullRequestId}: refs changed mid-merge (HTTP 409). Re-run after fetching the latest state.`; + case 555: + return `Merge of pull request #${pullRequestId} timed out server-side (HTTP 555). Re-run after giving it a moment.`; + default: + return `Failed to merge pull request #${pullRequestId}: HTTP ${status}.`; + } +} + +export type MergeTaskStatus = + | { status: "PENDING" } + | { status: "SUCCESS"; pr: PullRequestDetail } + | { status: "FAILED"; error: string }; + +/** + * Polls a single merge-task-status URL (the one from the 202 Location + * header). Uses raw fetch because the 202 URL is opaque (built by the + * server; openapi-fetch can't route it as a typed endpoint). + */ +export async function getMergeTaskStatus( + credentials: Credentials, + taskUrl: string, +): Promise { + const response = await fetch(taskUrl, { + method: "GET", + headers: { + Authorization: basicAuthHeader(credentials), + Accept: "application/json", + }, + }); + + if (!response.ok) { + throw new PullRequestError( + `Failed to poll merge task: HTTP ${response.status}.`, + response.status, + ); + } + + const raw = (await response.json()) as Record; + const status = String(raw.task_status ?? ""); + if (status === "SUCCESS") { + const mergeResult = raw.merge_result as RawPullRequest | undefined; + if (!mergeResult) { + throw new PullRequestError( + "Merge task reported SUCCESS but returned no merge_result.", + ); + } + return { status: "SUCCESS", pr: toPullRequestDetail(mergeResult) }; + } + if (status === "PENDING") return { status: "PENDING" }; + // Anything else (typically FAILED / ERROR) surfaces the server's error + // message verbatim. Bitbucket's shape: { type: "error", error: { message } }. + const message = + typeof raw.error?.message === "string" + ? raw.error.message + : `Merge task reported '${status}'.`; + return { status: "FAILED", error: message }; +} + function toPullRequestDetail(pr: RawPullRequest): PullRequestDetail { const base = toPullRequest(pr); const raw = pr as Record; @@ -615,11 +805,31 @@ function toPullRequestDetail(pr: RawPullRequest): PullRequestDetail { description: String(raw.summary?.raw ?? raw.description ?? ""), sourceBranch: String(raw.source?.branch?.name ?? ""), destinationBranch: String(raw.destination?.branch?.name ?? ""), + defaultMergeStrategy: toMergeStrategyOrNull( + raw.destination?.branch?.default_merge_strategy, + ), + allowedMergeStrategies: toMergeStrategyList( + raw.destination?.branch?.merge_strategies, + ), reviewers: toReviewers(raw.participants), participants: toParticipants(raw.participants), }; } +function toMergeStrategyOrNull(raw: unknown): MergeStrategy | null { + if (typeof raw !== "string" || !raw) return null; + return isMergeStrategy(raw) ? raw : null; +} + +function toMergeStrategyList(raw: unknown): MergeStrategy[] { + if (!Array.isArray(raw)) return []; + const out: MergeStrategy[] = []; + for (const entry of raw) { + if (typeof entry === "string" && isMergeStrategy(entry)) out.push(entry); + } + return out; +} + function toReviewers(raw: unknown): Reviewer[] { if (!Array.isArray(raw)) return []; const out: Reviewer[] = []; diff --git a/src/commands/pullrequest/index.ts b/src/commands/pullrequest/index.ts index eb4197f..a7605d9 100644 --- a/src/commands/pullrequest/index.ts +++ b/src/commands/pullrequest/index.ts @@ -4,6 +4,7 @@ import { runPullRequestComment } from "./comment.ts"; import { runPullRequestCreate } from "./create.ts"; import { runPullRequestDiff } from "./diff.ts"; import { runPullRequestList } from "./list.ts"; +import { runPullRequestMerge } from "./merge.ts"; import { runPullRequestReview } from "./review.ts"; import { runPullRequestView } from "./view.ts"; @@ -86,6 +87,26 @@ export function registerPullRequestCommands(program: Command): void { ) .action(withRenderer(runPullRequestDiff)); + pr.command("merge") + .description( + "Merge an open pull request (defaults to the PR for the current branch)", + ) + .argument("[id]", "Pull request number") + .option( + "-R, --repository ", + "Override repository detection", + ) + .option( + "-s, --strategy ", + "Merge strategy: merge_commit, squash, fast_forward, squash_fast_forward, rebase_fast_forward, rebase_merge (default: branch's configured strategy)", + ) + .option("-m, --message ", "Override the merge commit message") + .option( + "--delete", + "Delete the source branch remotely and locally after merging", + ) + .action(withRenderer(runPullRequestMerge)); + pr.command("review") .description( "Submit a review on a pull request (defaults to the PR for the current branch)", diff --git a/src/commands/pullrequest/merge.ts b/src/commands/pullrequest/merge.ts new file mode 100644 index 0000000..bc97345 --- /dev/null +++ b/src/commands/pullrequest/merge.ts @@ -0,0 +1,222 @@ +import { + getMergeTaskStatus, + getPullRequest, + isMergeStrategy, + MERGE_STRATEGIES, + type MergeStrategy, + mergePullRequest, + type PullRequestDetail, + PullRequestError, + type PullRequestState, +} 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"; +import { resolveCurrentPullRequestId } from "./current.ts"; + +export type PullRequestMergeOptions = { + repository?: string; + strategy?: string; + message?: string; + delete?: boolean; +}; + +const TERMINAL_STATES: ReadonlySet = new Set([ + "MERGED", + "DECLINED", + "SUPERSEDED", +]); + +/** Bitbucket's own fallback when the destination branch has no configured default. */ +const API_DEFAULT_STRATEGY: MergeStrategy = "merge_commit"; + +/** + * Budget for polling an async (202) merge. 15 tries * ~2s backoff ≈ 30s + * before giving up — plenty for ordinary merges, short enough to not feel + * stuck. Callers can always re-run; the task is server-side and persists. + */ +const POLL_MAX_ATTEMPTS = 15; +const POLL_INITIAL_MS = 500; +const POLL_MAX_MS = 4000; + +export async function runPullRequestMerge( + renderer: Renderer, + idArg: string | undefined, + options: PullRequestMergeOptions, + git: GitRunner = defaultGitRunner, +): Promise { + const config = await loadConfigOrExit(renderer); + + try { + const ref = await resolveRepository({ override: options.repository }); + const id = await resolveCurrentPullRequestId(idArg, { + renderer, + config, + ref, + commandName: "merge", + }); + + const pr = await getPullRequest(config, ref, id); + if (TERMINAL_STATES.has(pr.state)) { + renderer.error( + `Pull request #${id} is ${pr.state.toLowerCase()}; cannot merge.`, + ); + process.exit(1); + } + + const strategy = resolveStrategy(renderer, pr, options.strategy); + + const result = await mergePullRequest(config, ref, id, { + mergeStrategy: strategy, + ...(options.message !== undefined ? { message: options.message } : {}), + // Only include close_source_branch when --delete is set. Omitting + // it lets the PR's created-with value apply; passing `false` + // would override it for this merge and could surprise users who + // created the PR with "delete branch on merge" checked. + ...(options.delete ? { closeSourceBranch: true } : {}), + }); + + const merged = + result.kind === "done" + ? result.pr + : await pollUntilDone(config, result.taskUrl, renderer); + + if (renderer.json) { + renderer.detail(merged, []); + } else { + renderer.message(`Merged pull request #${id}: ${merged.url}`); + } + + if (options.delete) { + await cleanupLocalSourceBranch(renderer, git, pr); + } + } catch (err) { + if ( + err instanceof RepositoryResolutionError || + err instanceof PullRequestError + ) { + renderer.error(err.message); + process.exit(1); + } + throw err; + } +} + +function resolveStrategy( + renderer: Renderer, + pr: PullRequestDetail, + raw: string | undefined, +): MergeStrategy { + if (raw !== undefined) { + if (!isMergeStrategy(raw)) { + renderer.error( + `Unknown merge strategy '${raw}'. Allowed: ${MERGE_STRATEGIES.join(", ")}.`, + ); + process.exit(1); + } + // Validate against the branch's allowed list when Bitbucket populated + // it. Empty list means "no restriction" (or the server didn't report + // the field) — fall through and let the API have the final word. + if ( + pr.allowedMergeStrategies.length > 0 && + !pr.allowedMergeStrategies.includes(raw) + ) { + renderer.error( + `Merge strategy '${raw}' is not allowed on branch '${pr.destinationBranch}'. Allowed: ${pr.allowedMergeStrategies.join(", ")}.`, + ); + process.exit(1); + } + return raw; + } + + // No --strategy: use the branch's default; fall back to Bitbucket's own + // default if none configured. + return pr.defaultMergeStrategy ?? API_DEFAULT_STRATEGY; +} + +/** + * Polls a 202 task-status URL with capped exponential backoff until the + * merge reaches a terminal state or we exhaust the attempt budget. Returns + * the merged PR on success; errors surface as PullRequestError from the + * backend and bubble up to runPullRequestMerge. + */ +async function pollUntilDone( + credentials: Parameters[0], + taskUrl: string, + renderer: Renderer, +): Promise { + renderer.message("Merge accepted — polling task status…"); + let delay = POLL_INITIAL_MS; + for (let attempt = 0; attempt < POLL_MAX_ATTEMPTS; attempt++) { + const status = await getMergeTaskStatus(credentials, taskUrl); + if (status.status === "SUCCESS") return status.pr; + if (status.status === "FAILED") { + throw new PullRequestError(`Merge task failed: ${status.error}`); + } + await sleep(delay); + delay = Math.min(delay * 2, POLL_MAX_MS); + } + throw new PullRequestError( + `Merge task did not finish within ${POLL_MAX_ATTEMPTS} polls. Re-run the merge to check its current state.`, + ); +} + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +/** + * Best-effort local cleanup after `--delete`. Guards against the usual + * footguns: only deletes the local branch if its name matches the PR's + * source (so we never wipe a branch with the same name from a different + * origin); switches off it first when it's currently checked out. + * + * Errors here are non-fatal — the merge already landed. Surface them as + * warnings so the user knows the local branch wasn't cleaned up but the + * command's exit status still reflects the successful merge. + */ +async function cleanupLocalSourceBranch( + renderer: Renderer, + git: GitRunner, + pr: PullRequestDetail, +): Promise { + const cwd = process.cwd(); + const source = pr.sourceBranch; + const destination = pr.destinationBranch; + + if (!source) return; + try { + if (!(await git.hasLocalBranch(cwd, source))) return; + + const current = await git.getCurrentBranch(cwd); + if (current === source) { + // Can't delete the branch we're on — try switching to the + // destination first. If that branch doesn't exist locally or the + // checkout errors (dirty tree), bail out with a warning. + if (!destination || !(await git.hasLocalBranch(cwd, destination))) { + renderer.message( + `Warning: did not delete local branch '${source}' — no local '${destination}' to switch to first. Run 'git checkout && git branch -d ${source}' manually.`, + ); + return; + } + await git.checkoutExistingBranch(cwd, destination); + } + + await git.deleteLocalBranch(cwd, source); + renderer.message(`Deleted local branch '${source}'.`); + } catch (err) { + if (err instanceof GitError) { + renderer.message( + `Warning: could not clean up local branch '${source}': ${err.message}`, + ); + return; + } + throw err; + } +} diff --git a/src/shared/repository/git.ts b/src/shared/repository/git.ts index 628d10d..33aafa5 100644 --- a/src/shared/repository/git.ts +++ b/src/shared/repository/git.ts @@ -1,5 +1,22 @@ import { $ } from "bun"; +/** + * Error from a git command that was expected to succeed. Carries git's stderr + * so callers can surface the underlying message (which is usually more + * actionable than anything bbcli could synthesize) to the user. + */ +export class GitError extends Error { + override name = "GitError"; + readonly stderr: string; + readonly exitCode: number; + + constructor(message: string, exitCode: number, stderr: string) { + super(message); + this.exitCode = exitCode; + this.stderr = stderr; + } +} + /** * Thin wrapper around the git CLI. Injected into resolveRepository so tests * can simulate every failure mode without shelling out. Production code uses @@ -42,6 +59,19 @@ export interface GitRunner { cwd: string, remote: string, ): Promise; + /** Returns true iff a local branch with the given name exists. */ + hasLocalBranch(cwd: string, branch: string): Promise; + /** + * Switches to an existing local branch. Throws GitError on non-zero + * exit (most commonly: a dirty working tree that would be clobbered). + */ + checkoutExistingBranch(cwd: string, branch: string): Promise; + /** + * Deletes a local branch safely (`git branch -d`). Throws GitError on + * non-zero exit, which git returns when the branch hasn't been merged + * anywhere git can see — we never force with `-D` from this wrapper. + */ + deleteLocalBranch(cwd: string, branch: string): Promise; } export const defaultGitRunner: GitRunner = { @@ -120,4 +150,37 @@ export const defaultGitRunner: GitRunner = { const prefix = `${remote}/`; return full.startsWith(prefix) ? full.slice(prefix.length) : full; }, + + async hasLocalBranch(cwd, branch) { + // `show-ref --verify --quiet` exits 0 iff the ref exists; no output. + const result = + await $`git -C ${cwd} show-ref --verify --quiet ${`refs/heads/${branch}`}` + .nothrow() + .quiet(); + return result.exitCode === 0; + }, + + async checkoutExistingBranch(cwd, branch) { + const result = await $`git -C ${cwd} checkout ${branch}`.nothrow().quiet(); + throwIfFailed(result, `git checkout ${branch}`); + }, + + async deleteLocalBranch(cwd, branch) { + const result = await $`git -C ${cwd} branch -d ${branch}`.nothrow().quiet(); + throwIfFailed(result, `git branch -d ${branch}`); + }, }; + +function throwIfFailed( + result: { exitCode: number; stderr: Buffer; stdout: Buffer }, + command: string, +): void { + if (result.exitCode === 0) return; + const stderr = result.stderr.toString().trim(); + const stdout = result.stdout.toString().trim(); + // Git writes most diagnostics to stderr, but a few to stdout; prefer + // stderr, fall back to stdout so we never lose the actual reason. + const message = + stderr || stdout || `${command} exited with code ${result.exitCode}`; + throw new GitError(message, result.exitCode, stderr); +} diff --git a/src/shared/repository/index.ts b/src/shared/repository/index.ts index f6bb27a..4f18d37 100644 --- a/src/shared/repository/index.ts +++ b/src/shared/repository/index.ts @@ -1,5 +1,5 @@ export type { GitRunner } from "./git.ts"; -export { defaultGitRunner } from "./git.ts"; +export { defaultGitRunner, GitError } from "./git.ts"; export type { RepositoryRef } from "./parse-url.ts"; export { parseBitbucketRemoteUrl } from "./parse-url.ts"; export { diff --git a/src/shared/repository/resolve.test.ts b/src/shared/repository/resolve.test.ts index dd78455..f3ca5ed 100644 --- a/src/shared/repository/resolve.test.ts +++ b/src/shared/repository/resolve.test.ts @@ -32,6 +32,11 @@ function fakeGit(s: Setup = {}): GitRunner { async getDefaultBranchFromRemote() { return undefined; }, + async hasLocalBranch() { + return false; + }, + async checkoutExistingBranch() {}, + async deleteLocalBranch() {}, }; } @@ -57,35 +62,48 @@ describe("resolveRepository — override", () => { test("does not consult git when override is given", async () => { let called = false; + const markCalled = () => { + called = true; + }; const git: GitRunner = { async isInsideWorkTree() { - called = true; + markCalled(); return true; }, async listRemotes() { - called = true; + markCalled(); return []; }, async getRemoteUrl() { - called = true; + markCalled(); return undefined; }, async getCurrentBranch() { - called = true; + markCalled(); return undefined; }, async getSha() { - called = true; + markCalled(); return undefined; }, async getRemoteBranchSha() { - called = true; + markCalled(); return undefined; }, async getDefaultBranchFromRemote() { - called = true; + markCalled(); return undefined; }, + async hasLocalBranch() { + markCalled(); + return false; + }, + async checkoutExistingBranch() { + markCalled(); + }, + async deleteLocalBranch() { + markCalled(); + }, }; await resolveRepository({ override: "ws/repo" }, git); expect(called).toBe(false);