hive-forge: explain the pr-blocked-by-issue dependency failure instead of a raw 500
This commit is contained in:
parent
b2ee674415
commit
ad7a0572cd
2 changed files with 53 additions and 10 deletions
|
|
@ -17,7 +17,7 @@ use forgejo_api::structs::IssueMeta;
|
||||||
use serde_json::Value;
|
use serde_json::Value;
|
||||||
|
|
||||||
use crate::client::{Client, index};
|
use crate::client::{Client, index};
|
||||||
use crate::verbs::{dependency_summaries, print_json};
|
use crate::verbs::{dependency_summaries, is_pr, print_json};
|
||||||
|
|
||||||
#[derive(ClapArgs)]
|
#[derive(ClapArgs)]
|
||||||
pub struct Args {
|
pub struct Args {
|
||||||
|
|
@ -152,6 +152,10 @@ fn apply_and_verify(
|
||||||
"hive-forge: warning: dependency on #{dep} converged despite a reported \
|
"hive-forge: warning: dependency on #{dep} converged despite a reported \
|
||||||
error (server-side issue after the write, not a real failure): {e:#}"
|
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 {
|
} else {
|
||||||
real_failures.push(format!("#{dep}: {e:#}"));
|
real_failures.push(format!("#{dep}: {e:#}"));
|
||||||
}
|
}
|
||||||
|
|
@ -168,6 +172,37 @@ fn apply_and_verify(
|
||||||
Ok(())
|
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.
|
/// Build the `IssueMeta` body the create/remove endpoints want.
|
||||||
///
|
///
|
||||||
/// `owner`/`repo` are filled in with the *same* repo the request URL
|
/// `owner`/`repo` are filled in with the *same* repo the request URL
|
||||||
|
|
|
||||||
|
|
@ -131,20 +131,28 @@ pub(crate) enum Kind {
|
||||||
Issue,
|
Issue,
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Verify `number` is the expected kind before a kind-namespaced verb (one
|
/// Whether `number` is a PR rather than a plain issue. Forgejo's
|
||||||
/// of the generics that work on both — close/comment/labels/…) acts on it —
|
/// `/issues/{n}` endpoint serves both and marks PRs with a non-null
|
||||||
/// the validation win the `pr <verb>` / `issue <verb>` split buys over the
|
/// `pull_request` field, so one GET classifies it — shared by
|
||||||
/// old generic verbs. Forgejo's `/issues/{n}` endpoint serves both issues and
|
/// [`assert_kind`] and by `dependency`'s failure-diagnosis path, which
|
||||||
/// PRs and marks PRs with a non-null `pull_request` field, so one GET
|
/// needs the same classification without wanting an error on mismatch.
|
||||||
/// classifies it. Errors with a "use the other command" message on mismatch.
|
pub(crate) fn is_pr(client: &Client, owner: &str, name: &str, number: u64) -> Result<bool> {
|
||||||
pub(crate) fn assert_kind(client: &Client, number: u64, expected: Kind) -> Result<()> {
|
|
||||||
let (owner, name) = client.owner_repo()?;
|
|
||||||
let issue = client
|
let issue = client
|
||||||
.api()
|
.api()
|
||||||
.issue_get_issue(owner, name, index(number)?)
|
.issue_get_issue(owner, name, index(number)?)
|
||||||
.send()?;
|
.send()?;
|
||||||
let is_pr = issue.pull_request.is_some();
|
Ok(issue.pull_request.is_some())
|
||||||
match (expected, is_pr) {
|
}
|
||||||
|
|
||||||
|
/// 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 <verb>` / `issue <verb>` 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) => {
|
(Kind::Pr, false) => {
|
||||||
anyhow::bail!(
|
anyhow::bail!(
|
||||||
"#{number} is an issue, not a PR — use `hive-forge issue <verb> {number}`"
|
"#{number} is an issue, not a PR — use `hive-forge issue <verb> {number}`"
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue