From 9d8367bb2499179c826f64a986e618adac241f08 Mon Sep 17 00:00:00 2001 From: atlas Date: Mon, 15 Jun 2026 09:54:33 +0200 Subject: [PATCH] feat(hive-forge): add pr-merge verb (#1670) Adds `hive-forge pr-merge [--method merge|rebase] [--keep-branch] [--force]` wrapping POST /repos/{owner}/{repo}/pulls/{n}/merge, so agents on the peer-review-and-merge workflow have a CLI path instead of the raw API. - Methods: merge (default) | rebase. Squash is intentionally not offered. - Deletes the head branch after merge unless --keep-branch. - Safe by default: refuses unless the PR is mergeable, CI is not red, and no review's current verdict requests changes (latest-per-reviewer wins, so a later approval clears an earlier request-changes). --force overrides and also sets Forgejo's force_merge. - New client helper post_no_content for the 200-empty-body merge response. --- docs/tools/forge.md | 3 + hive-forge/src/client.rs | 23 ++++ hive-forge/src/main.rs | 5 + hive-forge/src/verbs/mod.rs | 1 + hive-forge/src/verbs/pr_merge.rs | 199 +++++++++++++++++++++++++++++++ 5 files changed, 231 insertions(+) create mode 100644 hive-forge/src/verbs/pr_merge.rs diff --git a/docs/tools/forge.md b/docs/tools/forge.md index f3ff1afb..2bf2ee44 100644 --- a/docs/tools/forge.md +++ b/docs/tools/forge.md @@ -47,6 +47,9 @@ hive-forge lint stale-branches --days 14 # branches with no recent activity hive-forge lint assignments # per-assignee open item count hive-forge pr-status --pr 42 # PR health: mergeable, CI, reviews, last comment (exit 0 = ready) hive-forge pr-status --sha # CI-only fast path for an explicit commit sha +hive-forge pr-merge 42 # merge (refuses unless mergeable + CI not red + no changes-requested); deletes head branch +hive-forge pr-merge 42 --method rebase # rebase-merge instead of a merge commit (no squash option) +hive-forge pr-merge 42 --keep-branch --force # keep the head branch; override the readiness gate hive-forge timeline 42 # audit trail: closes, label changes, assignments, commit refs hive-forge attach-issue 42 /path/to/file # upload a file attachment to an issue; prints download URL hive-forge attach-comment 18042 /path/to/file # upload a file attachment to a comment; prints download URL diff --git a/hive-forge/src/client.rs b/hive-forge/src/client.rs index 2bc0093a..ddadac19 100644 --- a/hive-forge/src/client.rs +++ b/hive-forge/src/client.rs @@ -229,6 +229,29 @@ impl Client { Ok(()) } + /// POST a JSON body to `/` for an endpoint that returns a + /// 2xx with an empty body (so there is nothing to decode). Used by + /// `pr-merge` — Forgejo's merge endpoint answers `200 OK` with no body + /// on success and a non-2xx (e.g. `405`) when the PR is not mergeable. + /// + /// # Errors + /// + /// Returns an error if the request fails to send (transport/network + /// error) or the server responds with a non-2xx status (the response + /// body is included in the error). + pub fn post_no_content(&self, path: &str, body: &B) -> Result<()> { + let url = format!("{}{}", self.api(), path); + let resp = self + .http + .post(&url) + .header(CONTENT_TYPE, "application/json") + .json(body) + .send() + .context("POST")?; + check_status(resp, &format!("POST {url}"))?; + Ok(()) + } + /// DELETE `/`. Optional JSON body for endpoints that /// need it (Forgejo's subscription unwatch uses bodyless DELETE). pub fn delete(&self, path: &str, body: Option<&Value>) -> Result<()> { diff --git a/hive-forge/src/main.rs b/hive-forge/src/main.rs index 7b63d70c..b7524fc4 100644 --- a/hive-forge/src/main.rs +++ b/hive-forge/src/main.rs @@ -95,6 +95,10 @@ enum Verb { List(verbs::list::Args), /// Manage milestones (list / create / close). Milestone(verbs::milestone::Args), + /// Merge a PR (`--method merge|rebase`, default merge). Refuses unless + /// mergeable + CI not red + no changes requested (`--force` overrides). + /// Deletes the head branch unless `--keep-branch`. No squash option. + PrMerge(verbs::pr_merge::Args), /// List reviews on a PR. PrReviews(verbs::pr_reviews::Args), /// List branches, optionally filtered. @@ -142,6 +146,7 @@ fn main() -> Result<()> { Verb::Lint(a) => verbs::lint::run(&client, a), Verb::List(a) => verbs::list::run(&client, a), Verb::Milestone(a) => verbs::milestone::run(&client, a), + Verb::PrMerge(a) => verbs::pr_merge::run(&client, a), Verb::PrReviews(a) => verbs::pr_reviews::run(&client, a), Verb::Branches(a) => verbs::branches::run(&client, a), Verb::TreeSha(a) => verbs::tree_sha::run(&client, a), diff --git a/hive-forge/src/verbs/mod.rs b/hive-forge/src/verbs/mod.rs index 578e1610..9f3da25b 100644 --- a/hive-forge/src/verbs/mod.rs +++ b/hive-forge/src/verbs/mod.rs @@ -23,6 +23,7 @@ pub mod list; pub mod milestone; pub mod pr; pub mod pr_create; +pub mod pr_merge; pub mod pr_reviews; pub mod pr_status; pub mod repo_add_collaborator; diff --git a/hive-forge/src/verbs/pr_merge.rs b/hive-forge/src/verbs/pr_merge.rs new file mode 100644 index 00000000..c222e4a0 --- /dev/null +++ b/hive-forge/src/verbs/pr_merge.rs @@ -0,0 +1,199 @@ +//! `pr-merge [--method merge|rebase] [--keep-branch] [--force]` +//! — merge a pull request. +//! +//! Wraps `POST /api/v1/repos/{owner}/{repo}/pulls/{n}/merge` so agents on a +//! peer-review-and-merge workflow (e.g. the paper repo, where agents merge +//! each other's PRs without an operator approval) have a CLI path instead of +//! reaching for the raw API. Pairs with `pr-status` (the merge-readiness +//! verdict this verb pre-checks) and `pr-create`. +//! +//! Safe by default: refuses unless the PR is mergeable, CI is not red, and no +//! review requests changes — pass `--force` to override (which also sets +//! Forgejo's own `force_merge`). The head branch is deleted after a successful +//! merge unless `--keep-branch` is given. Squash is intentionally not offered. + +use anyhow::{Result, bail}; +use clap::{Args as ClapArgs, ValueEnum}; +use serde_json::{Value, json}; + +use crate::client::Client; + +/// Merge strategy. Squash is deliberately omitted (hive convention: keep the +/// per-commit history, so a squash option isn't exposed). +#[derive(Clone, Copy, ValueEnum)] +pub enum Method { + /// Create a merge commit (Forgejo `Do: merge`). + Merge, + /// Rebase the head branch onto the base then fast-forward (Forgejo `Do: rebase`). + Rebase, +} + +impl Method { + /// The Forgejo `Do` field value for this strategy. + fn forgejo_do(self) -> &'static str { + match self { + Method::Merge => "merge", + Method::Rebase => "rebase", + } + } +} + +#[derive(ClapArgs)] +pub struct Args { + /// PR number to merge. + number: u64, + /// Merge strategy (default: a merge commit). Squash is not offered. + #[arg(long, value_enum, default_value = "merge")] + method: Method, + /// Keep the head branch after merging. By default the head branch is + /// deleted once the merge succeeds. + #[arg(long = "keep-branch")] + keep_branch: bool, + /// Merge even if the PR is not mergeable, CI is not green, or a review + /// requests changes. Also sets Forgejo's `force_merge` so the server does + /// not refuse on its own status checks. + #[arg(long)] + force: bool, +} + +/// # Errors +/// +/// Returns an error if the PR lookup or any readiness GET fails, if the PR is +/// already merged or closed, if a pre-merge readiness check fails without +/// `--force` (not mergeable / CI not green / changes requested), or if the +/// merge POST itself returns a non-2xx (e.g. Forgejo `405` when the PR cannot +/// be merged). +pub fn run(client: &Client, args: Args) -> Result<()> { + let repo = client.repo(); + let pull = client.get_json(&format!("/repos/{repo}/pulls/{}", args.number))?; + + if pull.get("merged").and_then(Value::as_bool).unwrap_or(false) { + bail!("pr-merge: PR #{} is already merged", args.number); + } + if pull.get("state").and_then(Value::as_str) == Some("closed") { + bail!("pr-merge: PR #{} is closed", args.number); + } + + if !args.force { + check_ready(client, repo, args.number, &pull)?; + } + + let payload = json!({ + "Do": args.method.forgejo_do(), + "delete_branch_after_merge": !args.keep_branch, + "force_merge": args.force, + }); + client.post_no_content( + &format!("/repos/{repo}/pulls/{}/merge", args.number), + &payload, + )?; + + let deleted = if args.keep_branch { + "" + } else { + " (head branch deleted)" + }; + println!( + "merged PR #{} via {}{deleted}", + args.number, + args.method.forgejo_do() + ); + Ok(()) +} + +/// Pre-merge readiness gate, mirroring `pr-status`'s verdict: the PR must be +/// mergeable, CI must not be red/pending, and no review may request changes. +/// Bails with an actionable message (pointing at `--force`) on the first +/// failure. +fn check_ready(client: &Client, repo: &str, number: u64, pull: &Value) -> Result<()> { + match pull.get("mergeable").and_then(Value::as_bool) { + Some(true) => {} + Some(false) => bail!( + "pr-merge: PR #{number} is not mergeable (conflicts). Rebase it, or pass --force." + ), + None => bail!( + "pr-merge: PR #{number} mergeability is still being computed. Retry shortly, or pass --force." + ), + } + + if let Some(sha) = pull + .get("head") + .and_then(|h| h.get("sha")) + .and_then(Value::as_str) + { + let combined = client.get_json(&format!("/repos/{repo}/commits/{sha}/status"))?; + let state = combined.get("state").and_then(Value::as_str).unwrap_or(""); + let has_statuses = combined + .get("statuses") + .and_then(Value::as_array) + .is_some_and(|a| !a.is_empty()); + // An empty status set means no CI is configured — not a blocker. + // Anything other than success once CI exists blocks the merge. + if has_statuses && state != "success" { + bail!( + "pr-merge: PR #{number} CI is not green (state: {state}). Wait for green, or pass --force." + ); + } + } + + // Latest verdict per reviewer wins (reviews page oldest-first), so a + // later APPROVED supersedes an earlier REQUEST_CHANGES. Block only on + // reviewers whose *current* verdict requests changes. + let reviews = client.get_json_all(&format!("/repos/{repo}/pulls/{number}/reviews"), 10)?; + let mut latest: Vec<(String, String)> = Vec::new(); + for r in &reviews { + let Some(login) = r + .get("user") + .and_then(|u| u.get("login")) + .and_then(Value::as_str) + else { + continue; + }; + let st = r.get("state").and_then(Value::as_str).unwrap_or(""); + if st == "COMMENT" || st == "PENDING" || st.is_empty() { + continue; + } + if let Some(slot) = latest.iter_mut().find(|(l, _)| l == login) { + st.clone_into(&mut slot.1); + } else { + latest.push((login.to_owned(), st.to_owned())); + } + } + let blockers: Vec = latest + .into_iter() + .filter(|(_, st)| st == "REQUEST_CHANGES") + .map(|(login, _)| login) + .collect(); + if !blockers.is_empty() { + bail!( + "pr-merge: PR #{number} has changes requested by {}. Resolve the review, or pass --force.", + blockers.join(", ") + ); + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn method_maps_to_forgejo_do() { + assert_eq!(Method::Merge.forgejo_do(), "merge"); + assert_eq!(Method::Rebase.forgejo_do(), "rebase"); + } + + #[test] + fn merge_payload_shape() { + // delete-by-default: keep_branch=false → delete_branch_after_merge=true. + let payload = json!({ + "Do": Method::Merge.forgejo_do(), + "delete_branch_after_merge": true, + "force_merge": false, + }); + assert_eq!(payload["Do"], "merge"); + assert_eq!(payload["delete_branch_after_merge"], true); + assert_eq!(payload["force_merge"], false); + } +}