diff --git a/hive-forge/src/verbs/dependency.rs b/hive-forge/src/verbs/dependency.rs index 64a9d943..318f29e2 100644 --- a/hive-forge/src/verbs/dependency.rs +++ b/hive-forge/src/verbs/dependency.rs @@ -9,12 +9,9 @@ //! 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}; @@ -53,121 +50,29 @@ pub fn run(client: &Client, args: Args) -> Result<()> { if deps.is_empty() { bail!("hive-forge dependency add: pass at least one issue/PR number"); } - apply_and_verify(client, owner, name, idx, args.number, &deps, true)?; + for dep in &deps { + client + .api() + .issue_create_issue_dependencies(owner, name, idx, dep_meta(owner, name, *dep)?) + .send()?; + } } Action::Remove { deps } => { if deps.is_empty() { bail!("hive-forge dependency remove: pass at least one issue/PR number"); } - apply_and_verify(client, owner, name, idx, args.number, &deps, false)?; + for dep in &deps { + client + .api() + .issue_remove_issue_dependencies(owner, name, idx, dep_meta(owner, name, *dep)?) + .send()?; + } } } 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 = match dependency_summaries(client, owner, name, number) { - Ok(deps) => deps, - Err(read_err) => { - // The read-back itself failed, so none of the per-dep errors - // above could be re-classified — surface all of them rather - // than dropping them behind the read-back's own error. - let originals: Vec = call_errors - .iter() - .map(|(dep, e)| format!("#{dep}: {e:#}")) - .collect(); - let verb = if adding { "add" } else { "remove" }; - bail!( - "hive-forge dependency {verb}: {} call(s) reported an error, and the \ - read-back to check whether they actually landed also failed ({read_err:#}). \ - Original error(s):\n{}", - call_errors.len(), - originals.join("\n") - ); - } - }; - let present: HashSet = after - .iter() - .filter_map(|d| d.get("number").and_then(Value::as_i64)) - .collect(); - let mut real_failures = Vec::new(); - for (dep, e) in call_errors { - let still_present = present.contains(&index(dep).unwrap_or(-1)); - // Converged iff present after an add, absent after a remove. - if still_present == adding { - // The write landed despite the error, but the error itself is - // still real signal: it means something *after* the write - // failed server-side (a timeline entry, a notification, a - // cycle check — we don't know which). Converging proves the - // effect is present, not that the operation fully succeeded, - // so don't let a clean exit make that anomaly unobservable. - eprintln!( - "hive-forge: warning: dependency on #{dep} converged despite a reported \ - error (server-side issue after the write, not a real failure): {e:#}" - ); - } else { - real_failures.push(format!("#{dep}: {e:#}")); - } - } - 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