From 749900298331a1eb181f91365838a229da5b6194 Mon Sep 17 00:00:00 2001 From: Nicolas Medda Date: Wed, 15 Apr 2026 08:31:18 +0000 Subject: [PATCH] BBC2-17 add bb pr comment with shared body-input and current-PR helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New command: bb pr comment [] --body/--body-file/-. Defaults to the open PR for the current branch when no id is supplied. Posts the raw body as markdown (content.markup="markdown" is explicit — the server's implicit default isn't documented, verifying via smoke test). Extracts two helpers while we're here, since this is the second command using each pattern: - src/shared/editor/body-input.ts — resolves --body / --body-file / stdin (via --body-file -) / $EDITOR fallback in one place. bb pr create now uses it too, which incidentally unblocks BBC2-10's deferred stdin support. - src/commands/pullrequest/current.ts — resolves [] vs current-branch PR detection. bb pr view switches to it; bb pr comment uses it. --- src/backend/pullrequests/index.test.ts | 71 ++++++++++++++++++++++++ src/backend/pullrequests/index.ts | 49 +++++++++++++++++ src/commands/pullrequest/comment.ts | 69 ++++++++++++++++++++++++ src/commands/pullrequest/create.ts | 29 ++++------ src/commands/pullrequest/current.ts | 74 ++++++++++++++++++++++++++ src/commands/pullrequest/index.ts | 17 ++++++ src/commands/pullrequest/view.ts | 43 +++------------ src/shared/editor/body-input.test.ts | 44 +++++++++++++++ src/shared/editor/body-input.ts | 47 ++++++++++++++++ 9 files changed, 388 insertions(+), 55 deletions(-) create mode 100644 src/commands/pullrequest/comment.ts create mode 100644 src/commands/pullrequest/current.ts create mode 100644 src/shared/editor/body-input.test.ts create mode 100644 src/shared/editor/body-input.ts diff --git a/src/backend/pullrequests/index.test.ts b/src/backend/pullrequests/index.test.ts index 62146c3..0cc3051 100644 --- a/src/backend/pullrequests/index.test.ts +++ b/src/backend/pullrequests/index.test.ts @@ -3,6 +3,7 @@ import { HttpResponse, http } from "msw"; import { BITBUCKET_BASE, server, setupMsw } from "../../test/msw/server.ts"; import { createPullRequest, + createPullRequestComment, findOpenPullRequestForBranch, getPullRequest, listPullRequests, @@ -567,3 +568,73 @@ describe("createPullRequest", () => { expect((err as PullRequestError).status).toBe(400); }); }); + +describe("createPullRequestComment", () => { + const COMMENTS_PATH = (id: number) => + `${BITBUCKET_BASE}/repositories/ws/repo/pullrequests/${id}/comments`; + + test("POSTs body and markup=markdown, returns id and html url", async () => { + let seenBody: Record | null = null; + server.use( + http.post(COMMENTS_PATH(42), async ({ request }) => { + seenBody = (await request.json()) as Record; + return HttpResponse.json( + { + type: "pullrequest_comment", + id: 7, + content: { + raw: "hello", + markup: "markdown", + html: "

hello

", + }, + links: { + html: { + href: "https://bitbucket.org/ws/repo/pull-requests/42/_#comment-7", + }, + }, + }, + { status: 201 }, + ); + }), + ); + + const out = await createPullRequestComment(creds, ref, 42, "hello"); + + expect(seenBody!).toEqual({ + type: "pullrequest_comment", + content: { raw: "hello", markup: "markdown" }, + }); + expect(out).toEqual({ + id: 7, + url: "https://bitbucket.org/ws/repo/pull-requests/42/_#comment-7", + }); + }); + + test("throws PullRequestError on 403", async () => { + server.use( + http.post(COMMENTS_PATH(42), () => + HttpResponse.json({ type: "error" }, { status: 403 }), + ), + ); + + const err = await createPullRequestComment(creds, ref, 42, "nope").catch( + (e) => e, + ); + expect(err).toBeInstanceOf(PullRequestError); + expect((err as PullRequestError).status).toBe(403); + }); + + test("throws PullRequestError on 404", async () => { + server.use( + http.post(COMMENTS_PATH(99), () => + HttpResponse.json({ type: "error" }, { status: 404 }), + ), + ); + + const err = await createPullRequestComment(creds, ref, 99, "nope").catch( + (e) => e, + ); + expect(err).toBeInstanceOf(PullRequestError); + expect((err as PullRequestError).status).toBe(404); + }); +}); diff --git a/src/backend/pullrequests/index.ts b/src/backend/pullrequests/index.ts index ec34df1..7669f4b 100644 --- a/src/backend/pullrequests/index.ts +++ b/src/backend/pullrequests/index.ts @@ -308,6 +308,55 @@ export async function findOpenPullRequestForBranch( return first ? toPullRequest(first as RawPullRequest) : null; } +export type PullRequestCommentResult = { + id: number; + url: string; +}; + +/** + * Posts a top-level comment on a pull request. `markup: "markdown"` is + * explicit — the web UI defaults to it but the API's server-side default + * isn't documented, so we send it to be safe. If a later smoke-test shows + * the server already defaults to markdown we can drop the field. + */ +export async function createPullRequestComment( + credentials: Credentials, + ref: { workspace: string; slug: string }, + pullRequestId: number, + body: string, +): Promise { + const client = createBitbucketClient(credentials); + const { data, response } = await client.POST( + "/repositories/{workspace}/{repo_slug}/pullrequests/{pull_request_id}/comments", + { + params: { + path: { + workspace: ref.workspace, + repo_slug: ref.slug, + pull_request_id: pullRequestId, + }, + }, + body: { + type: "pullrequest_comment", + content: { raw: body, markup: "markdown" }, + }, + }, + ); + + if (!response.ok || !data) { + throw new PullRequestError( + `Failed to post comment on pull request #${pullRequestId}: HTTP ${response.status}.`, + response.status, + ); + } + + const raw = data as Record; + return { + id: Number(raw.id ?? 0), + url: String(raw.links?.html?.href ?? ""), + }; +} + function toPullRequestDetail(pr: RawPullRequest): PullRequestDetail { const base = toPullRequest(pr); const raw = pr as Record; diff --git a/src/commands/pullrequest/comment.ts b/src/commands/pullrequest/comment.ts new file mode 100644 index 0000000..b1227f9 --- /dev/null +++ b/src/commands/pullrequest/comment.ts @@ -0,0 +1,69 @@ +import { + createPullRequestComment, + PullRequestError, +} from "../../backend/pullrequests/index.ts"; +import { loadConfigOrExit } from "../../shared/config/index.ts"; +import { + BodyInputError, + resolveBodyInput, +} from "../../shared/editor/body-input.ts"; +import { EditorError } from "../../shared/editor/index.ts"; +import type { Renderer } from "../../shared/renderer/index.ts"; +import { + RepositoryResolutionError, + resolveRepository, +} from "../../shared/repository/index.ts"; +import { resolveCurrentPullRequestId } from "./current.ts"; + +export type PullRequestCommentOptions = { + repository?: string; + body?: string; + bodyFile?: string; +}; + +export async function runPullRequestComment( + renderer: Renderer, + idArg: string | undefined, + options: PullRequestCommentOptions, +): Promise { + const config = await loadConfigOrExit(renderer); + + if (options.body !== undefined && options.bodyFile !== undefined) { + renderer.error("Pass either --body or --body-file, not both."); + process.exit(1); + } + + try { + const ref = await resolveRepository({ override: options.repository }); + const id = await resolveCurrentPullRequestId(idArg, { + renderer, + config, + ref, + commandName: "comment", + }); + + const body = await resolveBodyInput({ + body: options.body, + bodyFile: options.bodyFile, + }); + + if (body.trim() === "") { + renderer.error("Comment body is empty; nothing to post."); + process.exit(1); + } + + const comment = await createPullRequestComment(config, ref, id, body); + renderer.message(comment.url); + } catch (err) { + if ( + err instanceof RepositoryResolutionError || + err instanceof PullRequestError || + err instanceof BodyInputError || + err instanceof EditorError + ) { + renderer.error(err.message); + process.exit(1); + } + throw err; + } +} diff --git a/src/commands/pullrequest/create.ts b/src/commands/pullrequest/create.ts index 5420b6e..48b7f7e 100644 --- a/src/commands/pullrequest/create.ts +++ b/src/commands/pullrequest/create.ts @@ -3,7 +3,11 @@ import { PullRequestError, } from "../../backend/pullrequests/index.ts"; import { loadConfigOrExit } from "../../shared/config/index.ts"; -import { EditorError, openEditor } from "../../shared/editor/index.ts"; +import { + BodyInputError, + resolveBodyInput, +} from "../../shared/editor/body-input.ts"; +import { EditorError } from "../../shared/editor/index.ts"; import type { Renderer } from "../../shared/renderer/index.ts"; import { defaultGitRunner, @@ -51,7 +55,10 @@ export async function runPullRequestCreate( const destination = options.base ?? (await defaultBase(renderer, cwd)); - const body = await resolveBody(renderer, options); + const body = await resolveBodyInput({ + body: options.body, + bodyFile: options.bodyFile, + }); const pr = await createPullRequest(config, ref, { title: options.title, @@ -66,6 +73,7 @@ export async function runPullRequestCreate( if ( err instanceof RepositoryResolutionError || err instanceof PullRequestError || + err instanceof BodyInputError || err instanceof EditorError ) { renderer.error(err.message); @@ -113,20 +121,3 @@ async function defaultBase(renderer: Renderer, cwd: string): Promise { } return branch; } - -async function resolveBody( - renderer: Renderer, - options: PullRequestCreateOptions, -): Promise { - if (options.body !== undefined) return options.body; - if (options.bodyFile !== undefined) { - const file = Bun.file(options.bodyFile); - if (!(await file.exists())) { - renderer.error(`--body-file '${options.bodyFile}' does not exist.`); - process.exit(1); - } - return await file.text(); - } - // No flag: drop into the user's editor with a blank file. - return await openEditor(); -} diff --git a/src/commands/pullrequest/current.ts b/src/commands/pullrequest/current.ts new file mode 100644 index 0000000..368adb3 --- /dev/null +++ b/src/commands/pullrequest/current.ts @@ -0,0 +1,74 @@ +import { + findOpenPullRequestForBranch, + PullRequestError, +} from "../../backend/pullrequests/index.ts"; +import type { Credentials } from "../../shared/bitbucket-http/index.ts"; +import type { Renderer } from "../../shared/renderer/index.ts"; +import { defaultGitRunner } from "../../shared/repository/index.ts"; + +export type CurrentPullRequestLookup = { + renderer: Renderer; + config: Credentials; + ref: { workspace: string; slug: string }; + /** Command name to embed in error hints, e.g. "view", "comment". */ + commandName: string; +}; + +/** + * Resolves the PR id for a command that accepts `[]` and falls back to + * "the open PR for the current branch" when omitted. Exits the process with + * a clear error if neither can be determined. + * + * Extracted from `view.ts` so every command with this UX contract uses the + * same detection, error messages, and hints. + */ +export async function resolveCurrentPullRequestId( + idArg: string | undefined, + lookup: CurrentPullRequestLookup, +): Promise { + if (idArg !== undefined) { + const parsed = parseId(idArg); + if (parsed === null) { + lookup.renderer.error( + `Invalid PR id '${idArg}'. Expected a positive integer.`, + ); + process.exit(1); + } + return parsed; + } + + const branch = await defaultGitRunner.getCurrentBranch(process.cwd()); + if (!branch) { + lookup.renderer.error( + `Could not determine the current branch (detached HEAD or not a git repo). Pass a PR number explicitly: bb pr ${lookup.commandName} .`, + ); + process.exit(1); + } + + try { + const summary = await findOpenPullRequestForBranch( + lookup.config, + lookup.ref, + branch, + ); + if (!summary) { + lookup.renderer.error( + `No open pull request for branch '${branch}'. Pass a PR number explicitly (bb pr ${lookup.commandName} ) or list available PRs (bb pr list).`, + ); + process.exit(1); + } + return summary.id; + } catch (err) { + if (err instanceof PullRequestError) { + lookup.renderer.error(err.message); + process.exit(1); + } + throw err; + } +} + +function parseId(raw: string): number | null { + const n = Number(raw); + if (!Number.isInteger(n) || n <= 0) return null; + return n; +} diff --git a/src/commands/pullrequest/index.ts b/src/commands/pullrequest/index.ts index c1c61fb..50bc08c 100644 --- a/src/commands/pullrequest/index.ts +++ b/src/commands/pullrequest/index.ts @@ -1,5 +1,6 @@ import type { Command } from "commander"; import { withRenderer } from "../../shared/renderer/commander.ts"; +import { runPullRequestComment } from "./comment.ts"; import { runPullRequestCreate } from "./create.ts"; import { runPullRequestList } from "./list.ts"; import { runPullRequestView } from "./view.ts"; @@ -55,4 +56,20 @@ export function registerPullRequestCommands(program: Command): void { ) .option("--draft", "Create as a draft pull request") .action(withRenderer(runPullRequestCreate)); + + pr.command("comment") + .description( + "Post a top-level comment on a pull request (defaults to the PR for the current branch)", + ) + .argument("[id]", "Pull request number") + .option( + "-R, --repository ", + "Override repository detection", + ) + .option("-b, --body ", "Comment body (markdown)") + .option( + "-F, --body-file ", + "Read comment body from a file ('-' for stdin)", + ) + .action(withRenderer(runPullRequestComment)); } diff --git a/src/commands/pullrequest/view.ts b/src/commands/pullrequest/view.ts index a2d70d9..d8c9fed 100644 --- a/src/commands/pullrequest/view.ts +++ b/src/commands/pullrequest/view.ts @@ -1,5 +1,4 @@ import { - findOpenPullRequestForBranch, getPullRequest, type PullRequestDetail, PullRequestError, @@ -8,11 +7,11 @@ import { import { loadConfigOrExit } from "../../shared/config/index.ts"; import type { Renderer } from "../../shared/renderer/index.ts"; import { - defaultGitRunner, RepositoryResolutionError, resolveRepository, } from "../../shared/repository/index.ts"; import { formatRelativeTime } from "../../shared/time/index.ts"; +import { resolveCurrentPullRequestId } from "./current.ts"; export type PullRequestViewOptions = { repository?: string; @@ -37,36 +36,14 @@ export async function runPullRequestView( ): Promise { const config = await loadConfigOrExit(renderer); - let id: number | undefined; - if (idArg !== undefined) { - const parsed = parseId(idArg); - if (parsed === null) { - renderer.error(`Invalid PR id '${idArg}'. Expected a positive integer.`); - process.exit(1); - } - id = parsed; - } - try { const ref = await resolveRepository({ override: options.repository }); - - if (id === undefined) { - const branch = await defaultGitRunner.getCurrentBranch(process.cwd()); - if (!branch) { - renderer.error( - "Could not determine the current branch (detached HEAD or not a git repo). Pass a PR number explicitly: bb pr view .", - ); - process.exit(1); - } - const summary = await findOpenPullRequestForBranch(config, ref, branch); - if (!summary) { - renderer.error( - `No open pull request for branch '${branch}'. Pass a PR number explicitly (bb pr view ) or list available PRs (bb pr list).`, - ); - process.exit(1); - } - id = summary.id; - } + const id = await resolveCurrentPullRequestId(idArg, { + renderer, + config, + ref, + commandName: "view", + }); const pr = await getPullRequest(config, ref, id); render(renderer, pr); @@ -126,9 +103,3 @@ function render(renderer: Renderer, pr: PullRequestDetail): void { ); } } - -function parseId(raw: string): number | null { - const n = Number(raw); - if (!Number.isInteger(n) || n <= 0) return null; - return n; -} diff --git a/src/shared/editor/body-input.test.ts b/src/shared/editor/body-input.test.ts new file mode 100644 index 0000000..4663ea9 --- /dev/null +++ b/src/shared/editor/body-input.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, test } from "bun:test"; +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { BodyInputError, resolveBodyInput } from "./body-input.ts"; + +describe("resolveBodyInput", () => { + test("--body wins outright", async () => { + const out = await resolveBodyInput({ + body: "literal", + bodyFile: "ignored", + }); + expect(out).toBe("literal"); + }); + + test("empty --body is still returned (empty is a valid input here)", async () => { + const out = await resolveBodyInput({ body: "" }); + expect(out).toBe(""); + }); + + test("--body-file reads the file contents", async () => { + const dir = await mkdtemp(join(tmpdir(), "bbcli-body-input-")); + const path = join(dir, "body.md"); + await Bun.write(path, "from file\n"); + try { + const out = await resolveBodyInput({ bodyFile: path }); + expect(out).toBe("from file\n"); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + test("missing --body-file throws BodyInputError with the path in the message", async () => { + const err = await resolveBodyInput({ + bodyFile: "/definitely/does/not/exist.md", + }).catch((e) => e); + expect(err).toBeInstanceOf(BodyInputError); + expect((err as Error).message).toContain("/definitely/does/not/exist.md"); + }); + + // Note: we don't unit-test `--body-file -` (stdin) or the editor fallback + // here — Bun.stdin.text() and the editor subprocess are owned by their own + // integration paths. Adding fakes for either would outweigh the value. +}); diff --git a/src/shared/editor/body-input.ts b/src/shared/editor/body-input.ts new file mode 100644 index 0000000..70ae051 --- /dev/null +++ b/src/shared/editor/body-input.ts @@ -0,0 +1,47 @@ +import { openEditor } from "./index.ts"; + +export class BodyInputError extends Error { + override name = "BodyInputError"; +} + +export type BodyInputOptions = { + /** `--body "text"` — literal string wins over everything. */ + body?: string; + /** `--body-file ` — read from disk. Use "-" for stdin. */ + bodyFile?: string; + /** When falling through to the editor, pre-fill the buffer with this. */ + editorInitial?: string; +}; + +/** + * Resolves a user-supplied text body from CLI flags, stdin, or the user's + * `$EDITOR`. Shared between `bb pr create`, `bb pr comment`, and any future + * command that takes a free-form body. + * + * Precedence: + * 1. `--body` (literal) + * 2. `--body-file` (path, or `-` for stdin) + * 3. `$EDITOR` with an optional pre-fill + * + * The command layer is responsible for rejecting empty output when empty + * isn't meaningful (e.g. a comment). This helper returns whatever was typed. + */ +export async function resolveBodyInput( + options: BodyInputOptions, +): Promise { + if (options.body !== undefined) return options.body; + + if (options.bodyFile !== undefined) { + if (options.bodyFile === "-") return await Bun.stdin.text(); + + const file = Bun.file(options.bodyFile); + if (!(await file.exists())) { + throw new BodyInputError( + `--body-file '${options.bodyFile}' does not exist.`, + ); + } + return await file.text(); + } + + return await openEditor(options.editorInitial ?? ""); +}