From ad7a0572cdce0a95056ca4ce89d762f50be2cc98 Mon Sep 17 00:00:00 2001 From: damocles Date: Mon, 24 Aug 2026 10:33:59 +0200 Subject: [PATCH] hive-forge: explain the pr-blocked-by-issue dependency failure instead of a raw 500 --- hive-forge/src/verbs/dependency.rs | 37 +++++++++++++++++++++++++++++- hive-forge/src/verbs/mod.rs | 28 ++++++++++++++-------- 2 files changed, 54 insertions(+), 11 deletions(-) diff --git a/hive-forge/src/verbs/dependency.rs b/hive-forge/src/verbs/dependency.rs index 64a9d943..7fc8205d 100644 --- a/hive-forge/src/verbs/dependency.rs +++ b/hive-forge/src/verbs/dependency.rs @@ -17,7 +17,7 @@ use forgejo_api::structs::IssueMeta; use serde_json::Value; use crate::client::{Client, index}; -use crate::verbs::{dependency_summaries, print_json}; +use crate::verbs::{dependency_summaries, is_pr, print_json}; #[derive(ClapArgs)] pub struct Args { @@ -152,6 +152,10 @@ fn apply_and_verify( "hive-forge: warning: dependency on #{dep} converged despite a reported \ error (server-side issue after the write, not a real failure): {e:#}" ); + } else if let Some(hint) = + pr_blocked_by_issue_hint(client, owner, name, number, dep, adding) + { + real_failures.push(format!("#{dep}: {e:#}\n {hint}")); } else { real_failures.push(format!("#{dep}: {e:#}")); } @@ -168,6 +172,37 @@ fn apply_and_verify( Ok(()) } +/// A hint appended to a genuine add-failure when it matches the one known +/// unsupported shape: a **PR** depending on a plain **issue**. +/// +/// Confirmed via the full four-way matrix against the live forge: +/// issue→issue and issue→PR both work, PR→issue 500s every time with the +/// edge genuinely absent on read-back. `hive-forge`'s own request is +/// identical across all four directions, so this is an upstream Forgejo +/// limitation, not a bug this client can route around — there is no other +/// side to add the edge from. Best-effort: the two extra classification +/// GETs are swallowed on error rather than obscuring the real failure with +/// a diagnostic-aid failure. +fn pr_blocked_by_issue_hint( + client: &Client, + owner: &str, + name: &str, + number: u64, + dep: u64, + adding: bool, +) -> Option<&'static str> { + if !adding { + return None; + } + let source_is_pr = is_pr(client, owner, name, number).ok()?; + let target_is_pr = is_pr(client, owner, name, dep).ok()?; + (source_is_pr && !target_is_pr).then_some( + "known limitation: a PR cannot be marked as depending on a plain issue (an upstream \ + Forgejo limitation, not a hive-forge bug). Record the hold as prose in the PR \ + body/comment instead; there's no dependency-edge mechanism for this direction.", + ) +} + /// Build the `IssueMeta` body the create/remove endpoints want. /// /// `owner`/`repo` are filled in with the *same* repo the request URL diff --git a/hive-forge/src/verbs/mod.rs b/hive-forge/src/verbs/mod.rs index 6e901211..f2396837 100644 --- a/hive-forge/src/verbs/mod.rs +++ b/hive-forge/src/verbs/mod.rs @@ -131,20 +131,28 @@ pub(crate) enum Kind { Issue, } -/// Verify `number` is the expected kind before a kind-namespaced verb (one -/// of the generics that work on both — close/comment/labels/…) acts on it — -/// the validation win the `pr ` / `issue ` split buys over the -/// old generic verbs. Forgejo's `/issues/{n}` endpoint serves both issues and -/// PRs and marks PRs with a non-null `pull_request` field, so one GET -/// classifies it. Errors with a "use the other command" message on mismatch. -pub(crate) fn assert_kind(client: &Client, number: u64, expected: Kind) -> Result<()> { - let (owner, name) = client.owner_repo()?; +/// Whether `number` is a PR rather than a plain issue. Forgejo's +/// `/issues/{n}` endpoint serves both and marks PRs with a non-null +/// `pull_request` field, so one GET classifies it — shared by +/// [`assert_kind`] and by `dependency`'s failure-diagnosis path, which +/// needs the same classification without wanting an error on mismatch. +pub(crate) fn is_pr(client: &Client, owner: &str, name: &str, number: u64) -> Result { let issue = client .api() .issue_get_issue(owner, name, index(number)?) .send()?; - let is_pr = issue.pull_request.is_some(); - match (expected, is_pr) { + Ok(issue.pull_request.is_some()) +} + +/// Verify `number` is the expected kind before a kind-namespaced verb (one +/// of the generics that work on both — close/comment/labels/…) acts on it — +/// the validation win the `pr ` / `issue ` split buys over the +/// old generic verbs. Errors with a "use the other command" message on +/// mismatch. +pub(crate) fn assert_kind(client: &Client, number: u64, expected: Kind) -> Result<()> { + let (owner, name) = client.owner_repo()?; + let is_pr_result = is_pr(client, owner, name, number)?; + match (expected, is_pr_result) { (Kind::Pr, false) => { anyhow::bail!( "#{number} is an issue, not a PR — use `hive-forge issue {number}`"