From 31d9cb561ae3434da55a1b2b3a024617337b56b1 Mon Sep 17 00:00:00 2001 From: damocles Date: Sat, 6 Jun 2026 00:11:48 +0200 Subject: [PATCH] fix: hive-forge agit pr-create sets multi-line body via rest patch --- hive-forge/src/verbs/pr_create.rs | 80 ++++++++++++++++++++++++++++--- 1 file changed, 74 insertions(+), 6 deletions(-) diff --git a/hive-forge/src/verbs/pr_create.rs b/hive-forge/src/verbs/pr_create.rs index 3ca586bf..97a2b23d 100644 --- a/hive-forge/src/verbs/pr_create.rs +++ b/hive-forge/src/verbs/pr_create.rs @@ -17,6 +17,9 @@ //! is a read-only collaborator who can't push branches. Re-running with //! the same `--topic` updates the open PR. This is the path agents use //! to contribute to `internal/knowledge`; pair it with `hive-forge clone`. +//! A multi-line `--body` can't ride as a git push option (the wire +//! protocol forbids newlines there), so it's pushed title-only and the +//! body is set on the resulting PR via a REST PATCH. use std::process::{Command, Stdio}; @@ -82,7 +85,7 @@ pub struct Args { pub fn run(client: &Client, args: Args) -> Result<()> { let body = body::resolve(args.body.as_deref(), args.body_file.as_deref())?.unwrap_or_default(); if args.agit { - return agit_create(&args, &body); + return agit_create(client, &args, &body); } // Normal REST flow. `--head` is required (clap enforces it unless // `--agit`), so unwrap is safe here. @@ -116,12 +119,24 @@ pub fn run(client: &Client, args: Args) -> Result<()> { /// `force-push=true` lets a re-run with the same topic UPDATE the open /// PR (the new commit branches from base's tip, which the forge sees as /// non-fast-forward against the existing PR head and rejects without it). -fn agit_create(args: &Args, body: &str) -> Result<()> { +/// +/// The git wire protocol forbids newline characters in push options, so +/// a multi-line `description=` would make the push fail outright. A +/// single-line body still rides along as a push option (it's guaranteed +/// to land even if we can't parse the PR URL back out); a multi-line +/// body is pushed title-only and then set on the resulting PR via a REST +/// PATCH, mirroring the manual `pr-create --title …` + `issue-edit +/// --body-file` two-step. +fn agit_create(client: &Client, args: &Args, body: &str) -> Result<()> { let remote = args.remote.as_deref().unwrap_or("origin"); let topic = match &args.topic { Some(t) => t.clone(), None => current_branch().unwrap_or_else(|| "contribution".to_owned()), }; + // A body with any newline can't be a push option; defer it to a REST + // PATCH after the push. A single-line body is safe inline. + let inline_body = !body.is_empty() && !body.contains('\n'); + let deferred_body = !body.is_empty() && body.contains('\n'); let refspec = format!("HEAD:refs/for/{}/{topic}", args.base); let mut push_args = vec![ "push".to_owned(), @@ -134,7 +149,7 @@ fn agit_create(args: &Args, body: &str) -> Result<()> { "-o".to_owned(), "force-push=true".to_owned(), ]; - if !body.is_empty() { + if inline_body { push_args.push("-o".to_owned()); push_args.push(format!("description={body}")); } @@ -149,20 +164,51 @@ fn agit_create(args: &Args, body: &str) -> Result<()> { if !output.status.success() { anyhow::bail!("`git push {remote}` (AGit) failed:\n{stderr}"); } - if let Some(url) = parse_pr_url(&stderr) { - println!("{url}"); - } else { + let Some(url) = parse_pr_url(&stderr) else { // Push landed but no PR URL in the sideband — surface the raw // remote output so the caller can find the link themselves. eprint!("{stderr}"); + if deferred_body { + eprintln!( + "warning: multi-line body NOT set — PR URL not parsed, so it could \ + not be PATCHed. Set it manually: hive-forge issue-edit --body-file -" + ); + } println!( "AGit push to refs/for/{}/{topic} succeeded (PR URL not parsed from output above)", args.base ); + return Ok(()); + }; + if deferred_body { + if let Some(number) = pr_number_from_url(&url) { + let repo = client.repo(); + client + .patch_json( + &format!("/repos/{repo}/issues/{number}"), + &json!({ "body": body }), + ) + .with_context(|| format!("set body on AGit PR #{number}"))?; + } else { + eprintln!( + "warning: multi-line body NOT set — could not parse PR number from {url}. \ + Set it manually: hive-forge issue-edit --body-file -" + ); + } } + println!("{url}"); Ok(()) } +/// Extract the PR index from a Forgejo PR URL (`…/pulls/`). Returns +/// `None` if the last path segment isn't a number. +fn pr_number_from_url(url: &str) -> Option { + url.trim_end_matches('/') + .rsplit('/') + .next() + .and_then(|seg| seg.parse().ok()) +} + /// Current git branch name (`git rev-parse --abbrev-ref HEAD`), or /// `None` on detached HEAD / error. fn current_branch() -> Option { @@ -308,6 +354,28 @@ remote: http://forge.example/internal/knowledge/pulls/7\n"; assert_eq!(parse_pr_url("To http://localhost:3000/x/y.git\n"), None); } + #[test] + fn pr_number_parses_trailing_index() { + assert_eq!( + pr_number_from_url("http://forge.example/internal/knowledge/pulls/42"), + Some(42) + ); + // tolerant of a trailing slash + assert_eq!( + pr_number_from_url("http://forge.example/org/repo/pulls/7/"), + Some(7) + ); + // not a number → None (caller falls back to the manual-edit hint) + assert_eq!( + pr_number_from_url("http://forge.example/org/repo/pulls/"), + None + ); + assert_eq!( + pr_number_from_url("http://forge.example/org/repo/compare"), + None + ); + } + #[test] fn strips_create_pr_hint_block() { let stderr = "\