feat: add pr merge and update-branch (task 0012) #12

Merged
alexion merged 1 commits from task-0012-pr-merge-and-update-branch into main 2026-07-13 20:24:02 -04:00
4 changed files with 579 additions and 7 deletions
Showing only changes of commit 40859b9e6b - Show all commits

View File

@@ -14,10 +14,26 @@ Merge-blocked conditions surface through the standard 405/409 → `VALIDATION_ER
## Acceptance criteria
- [ ] `--method` accepts all six methods and the shorthands map to their methods; conflicting or duplicate action flags yield `VALIDATION_ERROR` (exit 2) before any API call
- [ ] `--merge-commit-id` without `manually-merged`, or `manually-merged` without `--merge-commit-id`, both yield `VALIDATION_ERROR` locally
- [ ] Successful merge outputs `merged: { number, status: "ok", method }`
- [ ] An already-merged PR short-circuits to the entity block with `merged_by` and `merged_at`, exit 0, no merge API call
- [ ] A 405 not-mergeable response surfaces as `VALIDATION_ERROR` with help suggesting `pr update-branch <n>` or `pr checkout <n>`
- [ ] `pr update-branch <n> --style rebase` calls the update endpoint with the style param and outputs `updated: { number, status: "ok" }`
- [ ] Fixture-server tests cover each method, the local validations, the idempotent no-op, and the 405/409 mappings
- [x] `--method` accepts all six methods and the shorthands map to their methods; conflicting or duplicate action flags yield `VALIDATION_ERROR` (exit 2) before any API call
- [x] `--merge-commit-id` without `manually-merged`, or `manually-merged` without `--merge-commit-id`, both yield `VALIDATION_ERROR` locally
- [x] Successful merge outputs `merged: { number, status: "ok", method }`
- [x] An already-merged PR short-circuits to the entity block with `merged_by` and `merged_at`, exit 0, no merge API call
- [x] A 405 not-mergeable response surfaces as `VALIDATION_ERROR` with help suggesting `pr update-branch <n>` or `pr checkout <n>`
- [x] `pr update-branch <n> --style rebase` calls the update endpoint with the style param and outputs `updated: { number, status: "ok" }`
- [x] Fixture-server tests cover each method, the local validations, the idempotent no-op, and the 405/409 mappings
## Implementation Notes
**No merge method given → `Do: merge` on the wire, `method: default` in the output.**
Gitea's merge endpoint requires a concrete `Do`, so with no `--method`/shorthand the command sends the baseline `merge` while reporting `method: "default"`.
The reported field describes the caller's choice (none was made), matching the gh-axi interface's documented shape; it is not a seventh merge method.
A consequence worth flagging: a repository configured to disallow plain merge commits (e.g. squash-only) will reject a bare `pr merge` with a 405, which surfaces with the server's message.
Respecting the repo's `default_merge_style` on the no-method path (an extra repo GET) is a possible follow-up if that turns out to bite.
**Conflicting/duplicate method flags share one message.**
Any combination of more than one method selector (`--method`, `--merge`, `--squash`, `--rebase`) yields a single `VALIDATION_ERROR`: `Choose only one merge method (--method, --merge, --squash, or --rebase)`.
The gh-axi interface doc lists three separate strings (multiple shorthands, `--method`+shorthand, invalid value); the combined message covers the first two cases in one and reads at least as clearly, and the task's own criteria only require `VALIDATION_ERROR` before any API call.
**`--merge-commit-id` remediation and 405/409 handling.**
`manually-merged` is reachable only through `--method` (it has no shorthand), so the `--merge-commit-id` pairing check can never collide with a shorthand.
Merge-blocked 405/409 responses reuse `classifyHttpError`'s `VALIDATION_ERROR` mapping (preserving the server's message) but swap in two remediation help lines pointing at `pr update-branch <n>` and `pr checkout <n>` — the latter lands in task 0014, so the suggestion currently names a command that does not exist yet.

View File

@@ -2,6 +2,7 @@ import type {
Comment,
CreatePullRequestOption,
EditPullRequestOption,
MergePullRequestOption,
PullRequest,
PullReview,
PullReviewComment,
@@ -51,6 +52,8 @@ commands:
checks Show a pull request's CI check results
create Create a pull request
edit Edit a pull request's title, body, labels, assignees, reviewers, milestone, or base
merge Merge a pull request
update-branch Merge the base branch into a pull request's head branch
close Close a pull request
reopen Reopen a closed pull request
comment Post a comment on a pull request
@@ -108,6 +111,45 @@ global flags:
--login <name> Select a tea login profile by name
`;
export const PR_MERGE_HELP = `usage: gitea-axi pr merge <number> [flags]
Merge a pull request in the current repository. An already-merged pull request
is reported as-is without re-merging.
flags:
--method <method> Merge method: merge, squash, rebase, rebase-merge,
fast-forward-only, or manually-merged (default: merge)
--merge Shorthand for --method merge
--squash Shorthand for --method squash
--rebase Shorthand for --method rebase
--auto Merge automatically once required checks succeed
--delete-branch Delete the head branch after a successful merge
--merge-commit-id <sha> The existing merge commit; required with, and only
valid for, --method manually-merged
--subject <text> Override the merge commit subject line
--body <text> Override the merge commit message body
--body-file <path> Read the merge commit message body from a file
--help Show this help
global flags:
-R, --repo <OWNER/NAME> Override the repository detected from the git origin remote
--login <name> Select a tea login profile by name
`;
export const PR_UPDATE_BRANCH_HELP = `usage: gitea-axi pr update-branch <number> [flags]
Merge the base branch into a pull request's head branch, bringing the head up
to date with the base.
flags:
--style <merge|rebase> How to update the head branch (default: merge)
--help Show this help
global flags:
-R, --repo <OWNER/NAME> Override the repository detected from the git origin remote
--login <name> Select a tea login profile by name
`;
export const PR_VIEW_HELP = `usage: gitea-axi pr view <number> [flags]
Show a single pull request, including its CI checks and review summary.
@@ -205,6 +247,39 @@ const PR_LIST_HELP_SUGGESTION = [
"Run `gitea-axi pr list --help` to see available flags",
];
const PR_MERGE_HELP_SUGGESTION = [
"Run `gitea-axi pr merge --help` to see available flags",
];
const PR_UPDATE_BRANCH_HELP_SUGGESTION = [
"Run `gitea-axi pr update-branch --help` to see available flags",
];
// The six methods Gitea's merge endpoint accepts as its `Do` field, in the
// order the help text lists them.
const MERGE_METHODS = [
"merge",
"squash",
"rebase",
"rebase-merge",
"fast-forward-only",
"manually-merged",
] as const;
type MergeMethod = (typeof MERGE_METHODS)[number];
// The bare-switch shorthands for the three common methods. `manually-merged` and
// the two rebase variants have no shorthand — they are reachable only via
// `--method`, which is why `--merge-commit-id` (a manually-merged-only flag)
// can never collide with a shorthand.
const MERGE_SHORTHANDS: Record<string, MergeMethod> = {
"--merge": "merge",
"--squash": "squash",
"--rebase": "rebase",
};
const UPDATE_STYLES = ["merge", "rebase"] as const;
type UpdateStyle = (typeof UPDATE_STYLES)[number];
// The `review` column is not one of these: it comes from a separate reviews
// fetch per PR (ADR 0006), so it is set on each row after the decision resolves,
// slotting in after `draft` and before any `--fields` extras.
@@ -1213,6 +1288,176 @@ async function prReopen(deps: CliDeps, args: string[]): Promise<string> {
});
}
/**
* The merge method to send and the value to report. A single explicit selector
* — `--method` or one of the {@link MERGE_SHORTHANDS} — is resolved to its
* method; giving more than one, in any combination, is a `VALIDATION_ERROR`
* ("conflicting or duplicate action flags"). With no selector the method is
* `undefined`: the caller sends Gitea's baseline `merge` but reports `default`,
* signalling that no method was chosen.
*/
function resolveMergeMethod(flags: Record<string, string | true>): MergeMethod | undefined {
const selected: MergeMethod[] = [];
for (const [flag, method] of Object.entries(MERGE_SHORTHANDS)) {
if (flags[flag] === true) {
selected.push(method);
}
}
const hasMethodFlag = flags["--method"] !== undefined;
if (selected.length + (hasMethodFlag ? 1 : 0) > 1) {
throw axiError(
"Choose only one merge method (--method, --merge, --squash, or --rebase)",
"VALIDATION_ERROR",
PR_MERGE_HELP_SUGGESTION,
);
}
if (hasMethodFlag) {
// parseEnumFlag never returns undefined here — the flag is present — but its
// signature allows it, so the non-null assertion documents that.
return parseEnumFlag(flags["--method"], "--method", MERGE_METHODS, PR_MERGE_HELP_SUGGESTION)!;
}
return selected[0];
}
async function prMerge(deps: CliDeps, args: string[]): Promise<string> {
if (args.includes("--help")) {
return PR_MERGE_HELP;
}
const { flags, positionals } = parseFlags(
args,
{
"--method": { takesValue: true },
"--merge": { takesValue: false },
"--squash": { takesValue: false },
"--rebase": { takesValue: false },
"--auto": { takesValue: false },
"--delete-branch": { takesValue: false },
"--merge-commit-id": { takesValue: true },
"--subject": { takesValue: true },
"--body": { takesValue: true },
"--body-file": { takesValue: true },
},
"pr merge",
);
const number = parsePositionalNumber(positionals, "pr merge", "pull request");
// Everything the caller's own input can settle is checked before any request
// goes out, so a rejected invocation never merges. The method resolves first,
// then the manually-merged/`--merge-commit-id` pairing, then the body source.
const method = resolveMergeMethod(flags);
const effectiveMethod: MergeMethod = method ?? "merge";
const mergeCommitId = flagValue(flags, "--merge-commit-id");
if (effectiveMethod === "manually-merged" && mergeCommitId === undefined) {
throw axiError(
"--method manually-merged requires --merge-commit-id <sha>",
"VALIDATION_ERROR",
PR_MERGE_HELP_SUGGESTION,
);
}
if (effectiveMethod !== "manually-merged" && mergeCommitId !== undefined) {
throw axiError(
"--merge-commit-id is only valid with --method manually-merged",
"VALIDATION_ERROR",
PR_MERGE_HELP_SUGGESTION,
);
}
const body = resolveBodySource(deps, flags, "pr merge");
const subject = flagValue(flags, "--subject");
const context = await resolveRepoContext(deps);
const api = createClient(context);
// Read the current state first: an already-merged pull request short-circuits
// to the idempotent entity block below rather than issuing a merge that Gitea
// would reject as redundant.
const pull = await getPull(api, context, number);
if (pull.merged) {
return renderDetail({
noun: "pull_request",
item: {
number,
state: "merged",
merged_by: pull.merged_by?.login ?? null,
merged_at: relativeTime(pull.merged_at, new Date()),
},
help: [suggestCommand(context, `pr view ${number}`, "to see the merged pull request")],
});
}
const payload: MergePullRequestOption = { Do: effectiveMethod };
if (mergeCommitId !== undefined) {
payload.MergeCommitID = mergeCommitId;
}
if (subject !== undefined) {
payload.MergeTitleField = subject;
}
if (body !== undefined) {
payload.MergeMessageField = body;
}
if (flags["--auto"] === true) {
payload.merge_when_checks_succeed = true;
}
if (flags["--delete-branch"] === true) {
payload.delete_branch_after_merge = true;
}
try {
await api.repos.repoMergePullRequest(context.owner, context.name, number, payload);
} catch (error) {
// A merge-blocked pull request (stale head, failing checks, conflicts) comes
// back 405/409, which classifyHttpError already maps to VALIDATION_ERROR. Its
// classified message (the server's own detail) is kept, and remediation lines
// are added pointing at the two commands that unblock it: update-branch for a
// stale head, checkout to resolve conflicts locally.
const classified = classifyHttpError(error);
const status = httpStatus(error);
if (status === 405 || status === 409) {
throw axiError(classified.message, "VALIDATION_ERROR", [
suggestCommand(context, `pr update-branch ${number}`, "to merge the base branch into a stale head"),
suggestCommand(context, `pr checkout ${number}`, "to check the branch out and resolve conflicts locally"),
]);
}
throw classified;
}
// The mutation ran, so the block is named for the action. `method` reports the
// caller's choice: the resolved method, or `default` when none was given.
return renderDetail({
noun: "merged",
item: { number, status: "ok", method: method ?? "default" },
help: [suggestCommand(context, `pr view ${number}`, "to see the merged pull request")],
});
}
async function prUpdateBranch(deps: CliDeps, args: string[]): Promise<string> {
if (args.includes("--help")) {
return PR_UPDATE_BRANCH_HELP;
}
const { flags, positionals } = parseFlags(
args,
{ "--style": { takesValue: true } },
"pr update-branch",
);
const number = parsePositionalNumber(positionals, "pr update-branch", "pull request");
const style: UpdateStyle =
parseEnumFlag(flags["--style"], "--style", UPDATE_STYLES, PR_UPDATE_BRANCH_HELP_SUGGESTION) ??
"merge";
const context = await resolveRepoContext(deps);
const api = createClient(context);
try {
await api.repos.repoUpdatePullRequest(context.owner, context.name, number, { style });
} catch (error) {
throw classifyHttpError(error);
}
return renderDetail({
noun: "updated",
item: { number, status: "ok" },
help: [suggestCommand(context, `pr checks ${number}`, "to monitor CI after the update")],
});
}
export function prCommand(deps: CliDeps) {
return async (args: string[]): Promise<string> => {
const [subcommand, ...rest] = args;
@@ -1234,6 +1479,12 @@ export function prCommand(deps: CliDeps) {
if (subcommand === "edit") {
return prEdit(deps, rest);
}
if (subcommand === "merge") {
return prMerge(deps, rest);
}
if (subcommand === "update-branch") {
return prUpdateBranch(deps, rest);
}
if (subcommand === "close") {
return prClose(deps, rest);
}

239
test/pr-merge.test.ts Normal file
View File

@@ -0,0 +1,239 @@
import { afterEach, describe, expect, it } from "vitest";
import { startFixtureServer, type FixtureServer } from "./fixture-server.js";
import { runCliTest, tempFiles, testModeEnv, type TempFiles } from "./harness.js";
const REPO_PATH = "/api/v1/repos/testowner/testrepo";
const PR_PATH = `${REPO_PATH}/pulls/9`;
const MERGE_PATH = `${PR_PATH}/merge`;
let server: FixtureServer;
const files: TempFiles = tempFiles();
afterEach(async () => {
await server.close();
files.cleanup();
});
/** The open PR the idempotency GET reads before a merge is attempted. */
function openPull() {
return { method: "GET", path: PR_PATH, body: { number: 9, state: "open" } } as const;
}
function mergePosted() {
return server.requests.find(
(request) => request.method === "POST" && request.path === MERGE_PATH,
);
}
describe("pr merge", () => {
it.each([
["--method", "merge", "merge"],
["--method", "squash", "squash"],
["--method", "rebase", "rebase"],
["--method", "rebase-merge", "rebase-merge"],
["--method", "fast-forward-only", "fast-forward-only"],
])("sends Do=%s %s and reports the method", async (flag, value, expected) => {
server = await startFixtureServer([openPull(), { method: "POST", path: MERGE_PATH, body: {} }]);
const { stdout, exitCode } = await runCliTest(["pr", "merge", "9", flag, value], {
env: testModeEnv(server.url),
});
expect(exitCode).toBe(0);
expect(stdout).toContain("merged:");
expect(stdout).toContain("number: 9");
expect(stdout).toContain("status: ok");
expect(stdout).toContain(`method: ${expected}`);
expect(mergePosted()?.body).toEqual({ Do: value });
});
it("merges the manually-merged method with its required --merge-commit-id", async () => {
server = await startFixtureServer([openPull(), { method: "POST", path: MERGE_PATH, body: {} }]);
const { stdout, exitCode } = await runCliTest(
["pr", "merge", "9", "--method", "manually-merged", "--merge-commit-id", "abc123"],
{ env: testModeEnv(server.url) },
);
expect(exitCode).toBe(0);
expect(stdout).toContain("method: manually-merged");
expect(mergePosted()?.body).toEqual({ Do: "manually-merged", MergeCommitID: "abc123" });
});
it.each([
["--merge", "merge"],
["--squash", "squash"],
["--rebase", "rebase"],
])("maps the %s shorthand to Do=%s", async (shorthand, expected) => {
server = await startFixtureServer([openPull(), { method: "POST", path: MERGE_PATH, body: {} }]);
const { stdout, exitCode } = await runCliTest(["pr", "merge", "9", shorthand], {
env: testModeEnv(server.url),
});
expect(exitCode).toBe(0);
expect(stdout).toContain(`method: ${expected}`);
expect(mergePosted()?.body).toEqual({ Do: expected });
});
it("defaults to Do=merge but reports method: default when none is given", async () => {
server = await startFixtureServer([openPull(), { method: "POST", path: MERGE_PATH, body: {} }]);
const { stdout, exitCode } = await runCliTest(["pr", "merge", "9"], {
env: testModeEnv(server.url),
});
expect(exitCode).toBe(0);
expect(stdout).toContain("method: default");
expect(mergePosted()?.body).toEqual({ Do: "merge" });
});
it("forwards --auto, --delete-branch, --subject, and --body-file", async () => {
server = await startFixtureServer([openPull(), { method: "POST", path: MERGE_PATH, body: {} }]);
const path = files.write("msg.txt", "Body from a file");
const { exitCode } = await runCliTest(
[
"pr",
"merge",
"9",
"--squash",
"--auto",
"--delete-branch",
"--subject",
"Ship it",
"--body-file",
path,
],
{ env: testModeEnv(server.url) },
);
expect(exitCode).toBe(0);
expect(mergePosted()?.body).toEqual({
Do: "squash",
MergeTitleField: "Ship it",
MergeMessageField: "Body from a file",
merge_when_checks_succeed: true,
delete_branch_after_merge: true,
});
});
it.each([
["--merge", "--squash"],
["--method", "merge", "--rebase"],
])("rejects conflicting method flags before any API call", async (...args) => {
server = await startFixtureServer([]);
const { stdout, exitCode } = await runCliTest(["pr", "merge", "9", ...args], {
env: testModeEnv(server.url),
});
expect(exitCode).toBe(2);
expect(stdout).toContain("code: VALIDATION_ERROR");
expect(stdout).toContain("Choose only one merge method");
expect(server.requests).toHaveLength(0);
});
it("rejects an invalid --method value", async () => {
server = await startFixtureServer([]);
const { stdout, exitCode } = await runCliTest(["pr", "merge", "9", "--method", "octopus"], {
env: testModeEnv(server.url),
});
expect(exitCode).toBe(2);
expect(stdout).toContain("code: VALIDATION_ERROR");
expect(server.requests).toHaveLength(0);
});
it("rejects --merge-commit-id without --method manually-merged", async () => {
server = await startFixtureServer([]);
const { stdout, exitCode } = await runCliTest(
["pr", "merge", "9", "--squash", "--merge-commit-id", "abc123"],
{ env: testModeEnv(server.url) },
);
expect(exitCode).toBe(2);
expect(stdout).toContain("code: VALIDATION_ERROR");
expect(stdout).toContain("--merge-commit-id is only valid with --method manually-merged");
expect(server.requests).toHaveLength(0);
});
it("rejects --method manually-merged without --merge-commit-id", async () => {
server = await startFixtureServer([]);
const { stdout, exitCode } = await runCliTest(
["pr", "merge", "9", "--method", "manually-merged"],
{ env: testModeEnv(server.url) },
);
expect(exitCode).toBe(2);
expect(stdout).toContain("code: VALIDATION_ERROR");
expect(stdout).toContain("--method manually-merged requires --merge-commit-id");
expect(server.requests).toHaveLength(0);
});
it("short-circuits an already-merged pull request without calling the merge API", async () => {
server = await startFixtureServer([
{
method: "GET",
path: PR_PATH,
body: {
number: 9,
state: "closed",
merged: true,
merged_by: { login: "octocat" },
merged_at: new Date(Date.now() - 2 * 24 * 60 * 60 * 1000).toISOString(),
},
},
]);
const { stdout, exitCode } = await runCliTest(["pr", "merge", "9"], {
env: testModeEnv(server.url),
});
expect(exitCode).toBe(0);
expect(stdout).toContain("pull_request:");
expect(stdout).toContain("state: merged");
expect(stdout).toContain("merged_by: octocat");
expect(stdout).toContain("merged_at: 2d ago");
expect(mergePosted()).toBeUndefined();
});
it("maps a 405 not-mergeable response to VALIDATION_ERROR with remediation help", async () => {
server = await startFixtureServer([
openPull(),
{
method: "POST",
path: MERGE_PATH,
status: 405,
body: { message: "The pull request cannot be merged, base branch is out of date" },
},
]);
const { stdout, exitCode } = await runCliTest(["pr", "merge", "9"], {
env: testModeEnv(server.url),
});
expect(exitCode).toBe(2);
expect(stdout).toContain("code: VALIDATION_ERROR");
expect(stdout).toContain("base branch is out of date");
expect(stdout).toContain("pr update-branch 9");
expect(stdout).toContain("pr checkout 9");
});
it("maps a 409 conflict response to VALIDATION_ERROR", async () => {
server = await startFixtureServer([
openPull(),
{ method: "POST", path: MERGE_PATH, status: 409, body: { message: "merge conflict" } },
]);
const { stdout, exitCode } = await runCliTest(["pr", "merge", "9", "--rebase"], {
env: testModeEnv(server.url),
});
expect(exitCode).toBe(2);
expect(stdout).toContain("code: VALIDATION_ERROR");
expect(stdout).toContain("merge conflict");
});
it("prints help with --help without calling the API", async () => {
server = await startFixtureServer([]);
const { stdout, exitCode } = await runCliTest(["pr", "merge", "--help"], {
env: testModeEnv(server.url),
});
expect(exitCode).toBe(0);
expect(stdout).toContain("usage: gitea-axi pr merge");
expect(server.requests).toHaveLength(0);
});
});

View File

@@ -0,0 +1,66 @@
import { afterEach, describe, expect, it } from "vitest";
import { startFixtureServer, type FixtureServer } from "./fixture-server.js";
import { runCliTest, testModeEnv } from "./harness.js";
const REPO_PATH = "/api/v1/repos/testowner/testrepo";
const UPDATE_PATH = `${REPO_PATH}/pulls/9/update`;
let server: FixtureServer;
afterEach(async () => {
await server.close();
});
function updateRequest() {
return server.requests.find(
(request) => request.method === "POST" && request.path === UPDATE_PATH,
);
}
describe("pr update-branch", () => {
it("updates the head branch with the default merge style", async () => {
server = await startFixtureServer([{ method: "POST", path: UPDATE_PATH, body: {} }]);
const { stdout, exitCode } = await runCliTest(["pr", "update-branch", "9"], {
env: testModeEnv(server.url),
});
expect(exitCode).toBe(0);
expect(stdout).toContain("updated:");
expect(stdout).toContain("number: 9");
expect(stdout).toContain("status: ok");
expect(updateRequest()?.query.style).toBe("merge");
});
it("passes --style rebase to the update endpoint", async () => {
server = await startFixtureServer([{ method: "POST", path: UPDATE_PATH, body: {} }]);
const { stdout, exitCode } = await runCliTest(["pr", "update-branch", "9", "--style", "rebase"], {
env: testModeEnv(server.url),
});
expect(exitCode).toBe(0);
expect(stdout).toContain("updated:");
expect(updateRequest()?.query.style).toBe("rebase");
});
it("rejects an invalid --style value before any API call", async () => {
server = await startFixtureServer([]);
const { stdout, exitCode } = await runCliTest(["pr", "update-branch", "9", "--style", "cherry"], {
env: testModeEnv(server.url),
});
expect(exitCode).toBe(2);
expect(stdout).toContain("code: VALIDATION_ERROR");
expect(server.requests).toHaveLength(0);
});
it("prints help with --help without calling the API", async () => {
server = await startFixtureServer([]);
const { stdout, exitCode } = await runCliTest(["pr", "update-branch", "--help"], {
env: testModeEnv(server.url),
});
expect(exitCode).toBe(0);
expect(stdout).toContain("usage: gitea-axi pr update-branch");
expect(server.requests).toHaveLength(0);
});
});