hyperhive/hive-forge/src/verbs/dependency.rs

//! `dependency <number> [list|add|remove] [numbers...]` — manage an
//! issue/PR's "blocked by" dependency links. Default action: list.
//!
//! Forgejo's dependency endpoints work on the shared issue/PR index (a
//! PR is an issue internally), so this is wired into both `issue <verb>`
//! and `pr <verb>` the same way `labels`/`assign`/`timeline` are.
//! Setting `<number>`'s dependency on `<dep>` means `<number>` is
//! blocked by `<dep>` — matching the forge web UI's "add dependency"
//! 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::ForgejoError;
use forgejo_api::structs::IssueMeta;
use serde_json::Value;

use crate::client::{Client, index};
use crate::verbs::{dependency_summaries, print_json};

#[derive(ClapArgs)]
pub struct Args {
    /// Issue or PR number.
    pub(crate) number: u64,
    #[command(subcommand)]
    action: Option<Action>,
}

#[derive(Subcommand)]
enum Action {
    /// List dependencies (default when no action is given) — the
    /// issues/PRs this one is blocked by.
    List,
    /// Add one or more issues/PRs this one is blocked by.
    Add {
        /// Issue/PR numbers to add as dependencies.
        deps: Vec<u64>,
    },
    /// Remove one or more dependency links.
    Remove {
        /// Issue/PR numbers to remove as dependencies.
        deps: Vec<u64>,
    },
}

pub fn run(client: &Client, args: Args) -> Result<()> {
    let (owner, name) = client.owner_repo()?;
    let idx = index(args.number)?;
    match args.action.unwrap_or(Action::List) {
        Action::List => {}
        Action::Add { deps } => {
            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)?;
        }
        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)?;
        }
    }
    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: `create` can 500 on a call that actually wrote the edge (an
/// intermittent server-side double-processing, not a client retry —
/// ruled out separately). A `send()` error there only means "maybe" —
/// the authoritative signal is whether the edge is actually there
/// afterward.
///
/// `remove`'s known quirk is handled below, before it ever reaches this
/// path: the forge always answers `201 Created` for a successful
/// deletion, which the generated client's endpoint spec doesn't expect
/// (it only treats 200 as success for that verb), so that specific
/// status is treated as success outright rather than routed through the
/// read-back below.
///
/// 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 {
            // `remove` always answers 201 for a successful deletion (not
            // the 200 the generated client expects for that verb) — a
            // deterministic, always-on mismatch, not a "something else
            // went wrong after the write" anomaly, so it's not worth a
            // read-back or a warning: just accept it.
            let is_remove_201 = !adding
                && matches!(
                    &e,
                    ForgejoError::UnexpectedStatusCode(code) if *code == reqwest::StatusCode::CREATED
                );
            if !is_remove_201 {
                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<String> = 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<i64> = 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
/// already targets — not left `None`. Forgejo's dependency handler
/// resolves the dependency's repo from these body fields rather than
/// defaulting an absent value to the URL's own owner/repo, so omitting
/// them 404s looking up an empty-string repo (`owner/repo: not found`
/// — measured, not guessed: `hive-forge dependency <n> add <dep>`
/// failed this way for every same-repo pair before this fix). This CLI
/// still only supports same-repo dependencies (matching the forge web
/// UI) — the fields are populated, not opened up to cross-repo.
fn dep_meta(owner: &str, repo: &str, dep: u64) -> Result<IssueMeta> {
    Ok(IssueMeta {
        index: Some(index(dep)?),
        owner: Some(owner.to_owned()),
        repo: Some(repo.to_owned()),
    })
}

#[cfg(test)]
mod tests {
    use super::*;

    /// The comment above `dep_meta` records a measured failure: leaving
    /// `owner`/`repo` as `None` made every same-repo dependency 404,
    /// because forgejo resolves the dependency's repo from the *body*
    /// rather than falling back to the request URL. Nothing but a test
    /// stops a future tidy-up from "simplifying" them back to `None`.
    #[test]
    fn dep_meta_populates_owner_and_repo_rather_than_leaving_them_none() {
        let m = dep_meta("hyperhive", "hyperhive", 3950).expect("in range");
        assert_eq!(m.owner.as_deref(), Some("hyperhive"));
        assert_eq!(m.repo.as_deref(), Some("hyperhive"));
        assert_eq!(m.index, Some(3950));
    }

    /// Out-of-range must be an error, not a wrapped negative index that
    /// the forge would then look up as some other issue.
    #[test]
    fn dep_meta_refuses_a_number_no_forge_could_have() {
        assert!(dep_meta("o", "r", u64::MAX).is_err());
    }
}