hive-forge: verify dependency add/remove by read-back instead of trusting HTTP status

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<i64> = after
+        .iter()
+        .filter_map(|d| d.get("number").and_then(Value::as_i64))
+        .collect();
+    let real_failures: Vec<String> = 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