diff --git a/hive-forge/src/verbs/dependency.rs b/hive-forge/src/verbs/dependency.rs index 318f29e2..41d94619 100644 --- a/hive-forge/src/verbs/dependency.rs +++ b/hive-forge/src/verbs/dependency.rs @@ -9,9 +9,12 @@ //! action, which is a same-repo-only relationship (there is no //! cross-repo dependency support in this CLI, mirroring the UI). +use std::collections::HashSet; + use anyhow::{Result, bail}; use clap::{Args as ClapArgs, Subcommand}; use forgejo_api::structs::IssueMeta; +use serde_json::Value; use crate::client::{Client, index}; use crate::verbs::{dependency_summaries, print_json}; @@ -50,29 +53,93 @@ pub fn run(client: &Client, args: Args) -> Result<()> { if deps.is_empty() { bail!("hive-forge dependency add: pass at least one issue/PR number"); } - for dep in &deps { - client - .api() - .issue_create_issue_dependencies(owner, name, idx, dep_meta(owner, name, *dep)?) - .send()?; - } + apply_and_verify(client, owner, name, idx, args.number, &deps, true)?; } Action::Remove { deps } => { if deps.is_empty() { bail!("hive-forge dependency remove: pass at least one issue/PR number"); } - for dep in &deps { - client - .api() - .issue_remove_issue_dependencies(owner, name, idx, dep_meta(owner, name, *dep)?) - .send()?; - } + apply_and_verify(client, owner, name, idx, args.number, &deps, false)?; } } let deps = dependency_summaries(client, owner, name, args.number)?; print_json(&serde_json::json!(deps)) } +/// Apply an add/remove edit for each of `deps` against `number`, then +/// verify by reading the dependency list back rather than trusting the +/// HTTP status this verb family returns. +/// +/// Measured: the status code lies in both directions on this endpoint +/// pair. `create` can 500 on a call that actually wrote the +/// edge (an intermittent server-side double-processing, not a client +/// retry — ruled out separately). `remove` can answer `201 Created` on a +/// call that actually deleted the edge, which the typed client's +/// endpoint spec maps to an error since 201 isn't the expected status for +/// a deletion. So a `send()` error here only means "maybe" — the +/// authoritative signal is whether the edge is actually there afterward. +/// +/// Calls that errored are only re-classified as real failures if the +/// read-back does NOT show the expected converged state (edge present +/// after an add, absent after a remove) — a call that errored *and* +/// didn't converge is a genuine failure and still surfaces. +fn apply_and_verify( + client: &Client, + owner: &str, + name: &str, + idx: i64, + number: u64, + deps: &[u64], + adding: bool, +) -> Result<()> { + let mut call_errors = Vec::new(); + for dep in deps { + let meta = dep_meta(owner, name, *dep)?; + let res = if adding { + client + .api() + .issue_create_issue_dependencies(owner, name, idx, meta) + .send() + } else { + client + .api() + .issue_remove_issue_dependencies(owner, name, idx, meta) + .send() + }; + if let Err(e) = res { + call_errors.push((*dep, e)); + } + } + if call_errors.is_empty() { + return Ok(()); + } + let after = dependency_summaries(client, owner, name, number)?; + let present: HashSet = after + .iter() + .filter_map(|d| d.get("number").and_then(Value::as_i64)) + .collect(); + let real_failures: Vec = call_errors + .into_iter() + .filter(|(dep, _)| { + let still_present = present.contains(&index(*dep).unwrap_or(-1)); + // Converged iff present after an add, absent after a remove. + // A real failure is anything that didn't converge. + still_present != adding + }) + .map(|(dep, e)| format!("#{dep}: {e:#}")) + .collect(); + if !real_failures.is_empty() { + let verb = if adding { "add" } else { "remove" }; + bail!( + "hive-forge dependency {verb}: {} call(s) genuinely failed (read-back doesn't \ + show the expected state):\n{}", + real_failures.len(), + real_failures.join("\n") + ); + } + Ok(()) +} + /// Build the `IssueMeta` body the create/remove endpoints want. /// /// `owner`/`repo` are filled in with the *same* repo the request URL