From 7bad72d3547881e6dedb55b5f0ba6c713f50e1c7 Mon Sep 17 00:00:00 2001 From: damocles Date: Sun, 2 Aug 2026 04:41:52 +0200 Subject: [PATCH] hive-forge: hard-error unresolved --label names; render dependency timeline events --- hive-forge/src/verbs/issue_create.rs | 11 ++-- hive-forge/src/verbs/labels.rs | 78 +++++++++++++++++++++++++--- hive-forge/src/verbs/pr_create.rs | 20 +++---- hive-forge/src/verbs/timeline.rs | 45 ++++++++++++++++ 4 files changed, 134 insertions(+), 20 deletions(-) diff --git a/hive-forge/src/verbs/issue_create.rs b/hive-forge/src/verbs/issue_create.rs index 79c5b3e0..f162714d 100644 --- a/hive-forge/src/verbs/issue_create.rs +++ b/hive-forge/src/verbs/issue_create.rs @@ -24,8 +24,9 @@ pub struct Args { #[arg(long)] assignee: Option, /// Label name to attach, repeatable (e.g. `--label area/ops --label - /// type/bug`). Same spelling `labels add` accepts. Unknown names are - /// silently dropped, matching `labels add`'s existing behavior. + /// type/bug`). Same spelling `labels add` accepts. An unresolved name + /// errors out (before the issue is created) rather than silently + /// attaching fewer labels than asked for. #[arg(long = "label")] labels: Vec, } @@ -35,8 +36,8 @@ pub struct Args { /// Propagates any I/O error from the body input (`--body-file`, /// stdin), any transport error from the Forgejo REST call (network /// unreachable, 4xx/5xx response, token missing/invalid, the -/// `--label` lookup's own list-labels call), and any I/O error from -/// writing the issue URL to stdout. +/// `--label` lookup's own list-labels call), an unresolved `--label` +/// name, and any I/O error from writing the issue URL to stdout. pub fn run(client: &Client, args: Args) -> Result<()> { let body = body::resolve(args.body.as_deref(), args.body_file.as_deref())?.unwrap_or_default(); let (owner, name) = client.owner_repo()?; @@ -44,7 +45,7 @@ pub fn run(client: &Client, args: Args) -> Result<()> { None } else { let all = labels::repo_labels(client)?; - Some(labels::resolve_ids(&all, &args.labels)) + Some(labels::resolve_ids(&all, &args.labels)?) }; let payload = CreateIssueOption { assignee: None, diff --git a/hive-forge/src/verbs/labels.rs b/hive-forge/src/verbs/labels.rs index 8a8c067e..9a55ca7a 100644 --- a/hive-forge/src/verbs/labels.rs +++ b/hive-forge/src/verbs/labels.rs @@ -46,7 +46,7 @@ pub fn run(client: &Client, args: Args) -> Result<()> { bail!("hive-forge labels add: pass at least one label name"); } let all = repo_labels(client)?; - let ids: Vec = resolve_ids(&all, &labels) + let ids: Vec = resolve_ids(&all, &labels)? .into_iter() .map(|id| json!(id)) .collect(); @@ -104,11 +104,36 @@ pub(crate) fn repo_labels(client: &Client) -> Result> { Ok(labels) } -/// Resolve label names to ids, silently dropping any name that doesn't -/// match a repo label (same behavior as `labels add`/`remove` above — -/// keeps this one non-fatal-typo policy in one place). -pub(crate) fn resolve_ids(all: &[Label], names: &[String]) -> Vec { - names.iter().filter_map(|n| lookup_id(all, n)).collect() +/// Resolve label names to ids, hard-erroring if any name doesn't match an +/// existing repo label. A typo used to silently produce fewer labels than +/// intended with no signal — not even a nonzero exit code — so callers +/// (`labels add`, `issue-create --label`, `pr-create --label`) had no way +/// to notice without manually diffing what they asked for against what +/// landed. The error lists both the exact names that didn't resolve and +/// every label actually available on the repo, so it's fixable from the +/// error alone without a second round-trip to `labels list`. +pub(crate) fn resolve_ids(all: &[Label], names: &[String]) -> Result> { + let mut ids = Vec::with_capacity(names.len()); + let mut unresolved = Vec::new(); + for n in names { + match lookup_id(all, n) { + Some(id) => ids.push(id), + None => unresolved.push(n.as_str()), + } + } + if !unresolved.is_empty() { + let available: Vec<&str> = all.iter().filter_map(|l| l.name.as_deref()).collect(); + bail!( + "unresolved label name(s): {} — available labels: {}", + unresolved.join(", "), + if available.is_empty() { + "(none)".to_owned() + } else { + available.join(", ") + } + ); + } + Ok(ids) } fn lookup_id(all: &[Label], name: &str) -> Option { @@ -121,3 +146,44 @@ fn print_label_names(labels: &[Label]) { let names: Vec<&str> = labels.iter().filter_map(|l| l.name.as_deref()).collect(); let _ = print_json(&json!(names)); } + +#[cfg(test)] +mod tests { + use super::resolve_ids; + use forgejo_api::structs::Label; + + fn label(id: i64, name: &str) -> Label { + Label { + color: Some(String::new()), + description: Some(String::new()), + exclusive: None, + id: Some(id), + is_archived: None, + name: Some(name.to_owned()), + url: None, + } + } + + #[test] + fn all_names_resolve_returns_ids_in_order() { + let all = vec![label(1, "area/ops"), label(2, "type/bug")]; + let ids = resolve_ids(&all, &["type/bug".to_owned(), "area/ops".to_owned()]).unwrap(); + assert_eq!(ids, vec![2, 1]); + } + + #[test] + fn unresolved_name_errors_listing_the_typo_and_available_labels() { + let all = vec![label(1, "area/ops"), label(2, "type/bug")]; + let err = resolve_ids(&all, &["area/op".to_owned()]).unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("area/op"), "missing typo'd name: {msg}"); + assert!(msg.contains("area/ops"), "missing available label: {msg}"); + assert!(msg.contains("type/bug"), "missing available label: {msg}"); + } + + #[test] + fn unresolved_name_on_empty_repo_says_none_available() { + let err = resolve_ids(&[], &["anything".to_owned()]).unwrap_err(); + assert!(err.to_string().contains("(none)")); + } +} diff --git a/hive-forge/src/verbs/pr_create.rs b/hive-forge/src/verbs/pr_create.rs index 6ff0fd3e..905e9b54 100644 --- a/hive-forge/src/verbs/pr_create.rs +++ b/hive-forge/src/verbs/pr_create.rs @@ -69,11 +69,13 @@ pub struct Args { #[arg(long)] topic: Option, /// Label name to attach, repeatable (e.g. `--label area/ops --label - /// type/bug`). Same spelling `labels add` accepts. In `--agit` mode - /// this is applied as a follow-up call once the PR number is known - /// (the `AGit` push itself has no label field), so it's silently - /// skipped if the PR URL couldn't be parsed back out of the push - /// output — same fallback as the deferred multi-line body. + /// type/bug`). Same spelling `labels add` accepts — an unresolved name + /// errors out rather than silently attaching fewer labels than asked + /// for. In `--agit` mode this is applied as a follow-up call once the + /// PR number is known (the `AGit` push itself has no label field), so + /// it's silently skipped (not a label-resolution error) if the PR URL + /// couldn't be parsed back out of the push output — same fallback as + /// the deferred multi-line body. #[arg(long = "label")] labels: Vec, } @@ -83,8 +85,8 @@ pub struct Args { /// Propagates any I/O error from the body input (`--body-file`, /// stdin) or the `--push` shellout to git, any transport error from /// the Forgejo REST call (network unreachable, 4xx/5xx response, -/// token missing/invalid), and any I/O error from writing the PR -/// URL to stdout. +/// token missing/invalid), an unresolved `--label` name, and any I/O +/// error from writing the PR URL to stdout. pub fn run(client: &Client, args: Args) -> Result<()> { let body = body::resolve(args.body.as_deref(), args.body_file.as_deref())?.unwrap_or_default(); if args.agit { @@ -105,7 +107,7 @@ pub fn run(client: &Client, args: Args) -> Result<()> { None } else { let all = labels::repo_labels(client)?; - Some(labels::resolve_ids(&all, &args.labels)) + Some(labels::resolve_ids(&all, &args.labels)?) }; // Note: Forgejo's CreatePullRequestOption has no `draft` / // `allow_maintainer_edit` fields (verified against the instance's @@ -249,7 +251,7 @@ fn apply_agit_labels(client: &Client, names: &[String], url: &str) -> Result<()> }; let (owner, name) = client.owner_repo()?; let all = labels::repo_labels(client)?; - let ids: Vec = labels::resolve_ids(&all, names) + let ids: Vec = labels::resolve_ids(&all, names)? .into_iter() .map(|id| json!(id)) .collect(); diff --git a/hive-forge/src/verbs/timeline.rs b/hive-forge/src/verbs/timeline.rs index dc848862..2f976115 100644 --- a/hive-forge/src/verbs/timeline.rs +++ b/hive-forge/src/verbs/timeline.rs @@ -196,6 +196,23 @@ fn format_event(ev: &Value) -> String { } } "comment_ref" | "issue_ref" => "referenced from another issue/PR".to_owned(), + "add_dependency" | "remove_dependency" => { + let verb = if event_type == "add_dependency" { + "added dependency on" + } else { + "removed dependency on" + }; + let dep = ev.get("dependent_issue"); + let number = dep + .and_then(|d| d.get("number")) + .and_then(Value::as_u64) + .map_or_else(|| "?".to_owned(), |n| n.to_string()); + let title = dep + .and_then(|d| d.get("title")) + .and_then(Value::as_str) + .unwrap_or("?"); + format!("{verb} #{number} ({title})") + } "changed_target_branch" => "changed target branch".to_owned(), "review" => "submitted a review".to_owned(), "lock" => "locked the conversation".to_owned(), @@ -344,6 +361,34 @@ mod tests { ); } + #[test] + fn add_dependency_renders_target_issue() { + let ev = serde_json::json!({ + "type": "add_dependency", + "user": { "login": "mara" }, + "created_at": "2026-05-31T12:00:00Z", + "dependent_issue": { "number": 2850, "title": "extract swarm controller" }, + }); + assert_eq!( + format_event(&ev), + "**mara @ 2026-05-31T12:00:00Z**: added dependency on #2850 (extract swarm controller)" // lint:allow: sample issue number in a test fixture string, not a tracker tag + ); + } + + #[test] + fn remove_dependency_renders_target_issue() { + let ev = serde_json::json!({ + "type": "remove_dependency", + "user": { "login": "mara" }, + "created_at": "2026-05-31T12:00:00Z", + "dependent_issue": { "number": 2850, "title": "extract swarm controller" }, + }); + assert_eq!( + format_event(&ev), + "**mara @ 2026-05-31T12:00:00Z**: removed dependency on #2850 (extract swarm controller)" // lint:allow: sample issue number in a test fixture string, not a tracker tag + ); + } + #[test] fn unknown_event_type_renders_bracketed_placeholder() { // Future-proofing: a forge schema bump that adds a new event