From 13b7a942572a695cdba51093938c8915ca7bff68 Mon Sep 17 00:00:00 2001 From: damocles Date: Thu, 21 May 2026 22:08:56 +0200 Subject: [PATCH] forge_notify: fix notification type strings and PR state detection (fixes #201) --- hive-ag3nt/src/forge_notify.rs | 63 ++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 30 deletions(-) diff --git a/hive-ag3nt/src/forge_notify.rs b/hive-ag3nt/src/forge_notify.rs index 5f76608..048b718 100644 --- a/hive-ag3nt/src/forge_notify.rs +++ b/hive-ag3nt/src/forge_notify.rs @@ -158,8 +158,9 @@ async fn format_notification( .unwrap_or("") .trim(); + // Forgejo API returns "Pull" / "Issue", never "Pull Request". let kind = match notif_type { - "Pull Request" => "comment on PR", + "Pull" => "comment on PR", "Issue" => "comment on issue", _ => "comment", }; @@ -174,40 +175,42 @@ async fn format_notification( out } else { // Notification triggered by creation or state change of the subject. - let subject = fetch_json(client, subject_api_url, token).await; - let author = subject - .as_ref() - .and_then(|s| s["user"]["login"].as_str()) - .unwrap_or("?"); - let body = subject - .as_ref() - .and_then(|s| s["body"].as_str()) - .unwrap_or("") - .trim(); - let state = subject - .as_ref() - .and_then(|s| s["state"].as_str()) - .unwrap_or(""); - let merged = subject - .as_ref() - .and_then(|s| s["merged"].as_bool()) - .unwrap_or(false); + // + // Classification uses notif["subject"]["state"] directly — Forgejo + // returns "open" / "closed" / "merged" here. We do NOT rely on + // fetching the PR/issue detail for `merged`: + // - `subject.url` points to the *issues* endpoint, which returns + // `pull_request.merged`, not top-level `merged`. + // - Forgejo API type is "Pull" / "Issue", never "Pull Request". + let notif_state = notif["subject"]["state"].as_str().unwrap_or(""); - let kind = match (notif_type, state, merged) { - ("Pull Request", "closed", true) => "PR merged".to_owned(), - ("Pull Request", "closed", false) => "PR closed".to_owned(), - ("Pull Request", _, _) => "new PR".to_owned(), - ("Issue", "closed", _) => "issue closed".to_owned(), - ("Issue", _, _) => "new issue".to_owned(), + let kind = match (notif_type, notif_state) { + ("Pull", "merged") => "PR merged".to_owned(), + ("Pull", "closed") => "PR closed".to_owned(), + ("Pull", _) => "new PR".to_owned(), + ("Issue", "closed") => "issue closed".to_owned(), + ("Issue", _) => "new issue".to_owned(), _ => format!("new {notif_type}"), }; + // Fetch subject only for body/author on new (open) items — not + // worth an extra HTTP round-trip for already-closed/merged ones. + let is_open = notif_state == "open" || notif_state.is_empty(); let mut out = format!("[{kind}] {title}\nrepo: {repo}\nurl: {html_url}"); - if !body.is_empty() && !state.contains("closed") && !merged { - out.push_str(&format!( - "\n\n{author}: {}", - truncate(body, BODY_TRUNCATE) - )); + if is_open { + let subject = fetch_json(client, subject_api_url, token).await; + let author = subject + .as_ref() + .and_then(|s| s["user"]["login"].as_str()) + .unwrap_or("?"); + let body = subject + .as_ref() + .and_then(|s| s["body"].as_str()) + .unwrap_or("") + .trim(); + if !body.is_empty() { + out.push_str(&format!("\n\n{author}: {}", truncate(body, BODY_TRUNCATE))); + } } out }