From dd5ccb5ce741efbb44073edd239ab85b157f9dda Mon Sep 17 00:00:00 2001 From: damocles Date: Thu, 13 Aug 2026 11:23:58 +0200 Subject: [PATCH] hive-forge: don't assert 'conflicts' from a bare mergeable=false --- hive-forge/src/verbs/pr_merge.rs | 12 +++++++++++- hive-forge/src/verbs/pr_status.rs | 17 ++++++++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/hive-forge/src/verbs/pr_merge.rs b/hive-forge/src/verbs/pr_merge.rs index 5564218c..ff3f886a 100644 --- a/hive-forge/src/verbs/pr_merge.rs +++ b/hive-forge/src/verbs/pr_merge.rs @@ -127,8 +127,18 @@ pub fn run(client: &Client, args: Args) -> Result<()> { fn check_ready(client: &Client, repo: &str, number: u64, pull: &PullRequest) -> Result<()> { match pull.mergeable { Some(true) => {} + // Forgejo's `mergeable` is one boolean covering several distinct + // causes (draft, a real conflict, a blocked required check, ...) — + // asserting "conflicts" and telling the caller to rebase is only + // correct for one of those causes and actively wrong advice for the + // others (rebasing a draft does nothing). Only name the + // cause when the API actually confirms it (draft); otherwise report + // the bare observation and let `pr-status` be consulted for detail. + Some(false) if pull.draft.unwrap_or(false) => bail!( + "pr-merge: PR #{number} is a draft, not mergeable. Mark it ready for review, or pass --force." + ), Some(false) => bail!( - "pr-merge: PR #{number} is not mergeable (conflicts). Rebase it, or pass --force." + "pr-merge: PR #{number} is not mergeable. Check `pr-status --pr {number}` for detail, or pass --force." ), None => bail!( "pr-merge: PR #{number} mergeability is still being computed. Retry shortly, or pass --force." diff --git a/hive-forge/src/verbs/pr_status.rs b/hive-forge/src/verbs/pr_status.rs index 11746fa6..3008e608 100644 --- a/hive-forge/src/verbs/pr_status.rs +++ b/hive-forge/src/verbs/pr_status.rs @@ -107,6 +107,7 @@ fn pr_status(client: &Client, repo: &str, pr: u64) -> Result<()> { let merged = pull.merged.unwrap_or(false); // `mergeable` is `null` while the forge is still computing it. let mergeable = pull.mergeable; + let draft = pull.draft.unwrap_or(false); let sha = pull .head .as_ref() @@ -132,6 +133,7 @@ fn pr_status(client: &Client, repo: &str, pr: u64) -> Result<()> { "state": state, "merged": merged, "mergeable": mergeable, + "draft": draft, "head_sha": sha, "ci_state": ci_state, "ci_statuses": ci_statuses, @@ -154,6 +156,7 @@ fn pr_status(client: &Client, repo: &str, pr: u64) -> Result<()> { state, merged, mergeable, + draft, &sha, &ci_state, &ci_statuses, @@ -304,6 +307,7 @@ fn print_pr( state: &str, merged: bool, mergeable: Option, + draft: bool, sha: &str, ci_state: &str, ci_statuses: &[Value], @@ -315,14 +319,25 @@ fn print_pr( let state_line = if merged { "merged".to_owned() } else { + // Forgejo's `mergeable` is a single boolean folding together several + // distinct causes (draft, real conflicts, blocked required checks, + // ...) — printing a specific cause we didn't actually observe is a + // wrong diagnosis in a health view, worse than no diagnosis at all. + // Only name a cause we can actually confirm from the API response; + // otherwise report the bare observation. let m = match mergeable { Some(true) => "mergeable: yes", - Some(false) => "mergeable: NO (conflicts)", + Some(false) => "mergeable: NO", None => "mergeable: computing…", }; format!("{state} ({m})") }; println!(" state: {state_line}"); + // Surfaced as its own line rather than folded into the mergeable text + // above — it's a real, confirmed cause (unlike the ones we can't name), + // and it's currently invisible unless you happen to notice a `WIP:` + // title prefix. + println!(" draft: {}", if draft { "yes" } else { "no" }); print!(" CI: "); print_ci(sha, ci_state, ci_statuses);