fix: hive-forge agit pr-create sets multi-line body via rest patch

This commit is contained in:
damocles 2026-06-06 00:11:48 +02:00 committed by mara
commit 31d9cb561a

View file

@ -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 <pr#> --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 <pr#> --body-file -"
);
}
}
println!("{url}");
Ok(())
}
/// Extract the PR index from a Forgejo PR URL (`…/pulls/<n>`). Returns
/// `None` if the last path segment isn't a number.
fn pr_number_from_url(url: &str) -> Option<u64> {
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<String> {
@ -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 = "\