forge_notify: include reason field in all notification messages (closes #110)

This commit is contained in:
damocles 2026-05-22 23:58:30 +02:00 committed by Mara
parent 7f97acf19e
commit edf7f1602d

View file

@ -223,14 +223,19 @@ fn review_state_label(state: &str) -> Option<&str> {
/// and should be silently discarded (and marked read by the caller). /// and should be silently discarded (and marked read by the caller).
/// ///
/// Formats: /// Formats:
/// - Comment: `[comment on PR #N repo] title\nurl: ...\n\nauthor: body\nassignee: user` /// - Comment: `[comment on PR #N repo] title\nurl: ...\n\nauthor: body\nassignee: user\nreason: mention`
/// - Review: `[PR approved #N repo] title\nurl: ...\n\nreviewer: body\nassignee: user` /// - Review: `[PR approved #N repo] title\nurl: ...\n\nreviewer: body\nassignee: user\nreason: review_requested`
/// - New item: `[new issue #N repo] title\nurl: ...\nassignee: user` /// - New item: `[new issue #N repo] title\nurl: ...\nassignee: user\nreason: author`
/// - State: `[PR merged #N repo] title\nurl: ...\nassignee: user` /// - State: `[PR merged #N repo] title\nurl: ...\nassignee: user\nreason: subscribed`
/// ///
/// Assignees (and, for PRs, `requested_reviewers`) are appended unconditionally /// Assignees (and, for PRs, `requested_reviewers`) are appended unconditionally
/// on all issue/PR notifications (closes #256). /// on all issue/PR notifications (closes #256).
/// ///
/// The `reason` field from the Forgejo notification is always appended (closes #110).
/// Forgejo emits one notification entry per reason for the same event, so including
/// it makes otherwise-identical messages distinguishable (e.g. `mention` vs
/// `subscribed` both arriving for the same PR comment).
///
/// Number is extracted from `html_url` last path segment before any `#`. /// Number is extracted from `html_url` last path segment before any `#`.
/// Repo slug (`owner/name`) is always included — agents may watch multiple repos. /// Repo slug (`owner/name`) is always included — agents may watch multiple repos.
async fn format_notification( async fn format_notification(
@ -277,13 +282,14 @@ async fn format_notification(
}; };
let is_pr = matches!(notif_type, "Pull Request" | "Pull"); let is_pr = matches!(notif_type, "Pull Request" | "Pull");
let meta_suffix = build_meta_suffix(subject.as_ref(), is_pr); let reason = notif["reason"].as_str().unwrap_or("");
let meta_suffix = build_meta_suffix(subject.as_ref(), is_pr, reason);
// Determine whether this notification was triggered by a comment/review or // Determine whether this notification was triggered by a comment/review or
// by creation/state-change of the subject itself. // by creation/state-change of the subject itself.
let has_comment = !comment_api_url.is_empty() && comment_api_url != subject_api_url; let has_comment = !comment_api_url.is_empty() && comment_api_url != subject_api_url;
let meta = NotifMeta { title, notif_type, html_url, num, repo, meta_suffix, subject, is_pr }; let meta = NotifMeta { title, notif_type, html_url, num, repo, meta_suffix, reason, subject, is_pr };
if has_comment { if has_comment {
format_comment_notification(client, token, &meta, comment_api_url, comment_html_url, own_login).await format_comment_notification(client, token, &meta, comment_api_url, comment_html_url, own_login).await
} else { } else {
@ -299,14 +305,18 @@ struct NotifMeta<'a> {
num: String, num: String,
repo: String, repo: String,
meta_suffix: String, meta_suffix: String,
/// Forgejo `reason` value (e.g. "mention", "assigned", "subscribed").
/// Appended to every formatted message so that multiple notifications for
/// the same event (each with a different reason) are distinguishable (closes #110).
reason: &'a str,
/// Fetched subject detail (issue/PR JSON); used for review-request detection. /// Fetched subject detail (issue/PR JSON); used for review-request detection.
subject: Option<serde_json::Value>, subject: Option<serde_json::Value>,
is_pr: bool, is_pr: bool,
} }
/// Build the `\nassignee: ...` (and optionally `\nreviewer: ...`) suffix /// Build the `\nassignee: ...` (and optionally `\nreviewer: ...` and `\nreason: ...`) suffix
/// appended to all notification kinds. /// appended to all notification kinds.
fn build_meta_suffix(subject: Option<&serde_json::Value>, is_pr: bool) -> String { fn build_meta_suffix(subject: Option<&serde_json::Value>, is_pr: bool, reason: &str) -> String {
let assignees: Vec<&str> = subject let assignees: Vec<&str> = subject
.and_then(|s| s["assignees"].as_array()) .and_then(|s| s["assignees"].as_array())
.map(|arr| arr.iter().filter_map(|a| a["login"].as_str()).collect()) .map(|arr| arr.iter().filter_map(|a| a["login"].as_str()).collect())
@ -326,10 +336,13 @@ fn build_meta_suffix(subject: Option<&serde_json::Value>, is_pr: bool) -> String
} else { } else {
None None
}; };
match reviewer_line { // Always include reason so multiple notifications for the same event
Some(r) => format!("\n{assignee_line}\n{r}"), // (each with a different Forgejo reason) are distinguishable (closes #110).
None => format!("\n{assignee_line}"), let reason_line = if reason.is_empty() { None } else { Some(format!("reason: {reason}")) };
} let mut out = format!("\n{assignee_line}");
if let Some(r) = reviewer_line { write!(out, "\n{r}").ok(); }
if let Some(r) = reason_line { write!(out, "\n{r}").ok(); }
out
} }
/// Format a notification triggered by a new comment or review submission. /// Format a notification triggered by a new comment or review submission.
@ -412,19 +425,18 @@ fn format_state_change_notification(
// `pull_request.merged`, not top-level `merged`. // `pull_request.merged`, not top-level `merged`.
// - Forgejo API type is "Pull" / "Issue", never "Pull Request". // - Forgejo API type is "Pull" / "Issue", never "Pull Request".
let notif_state = notif["subject"]["state"].as_str().unwrap_or(""); let notif_state = notif["subject"]["state"].as_str().unwrap_or("");
let reason = notif["reason"].as_str().unwrap_or("");
// Self-notification filter (#230): skip new items we authored ourselves. // Self-notification filter (#230): skip new items we authored ourselves.
// `reason == "author"` combined with open state means we just opened the // `reason == "author"` combined with open state means we just opened the
// issue/PR. We do NOT filter merged/closed state changes — those are // issue/PR. We do NOT filter merged/closed state changes — those are
// triggered by someone else and we want them. // triggered by someone else and we want them.
let is_new = notif_state == "open" || notif_state.is_empty(); let is_new = notif_state == "open" || notif_state.is_empty();
if is_new && reason == "author" && !own_login.is_empty() { if is_new && meta.reason == "author" && !own_login.is_empty() {
debug!(%own_login, "forge_notify: skipping self-authored new item"); debug!(%own_login, "forge_notify: skipping self-authored new item");
return None; return None;
} }
let NotifMeta { title, notif_type, html_url, num, repo, meta_suffix, subject, is_pr } = meta; let NotifMeta { title, notif_type, html_url, num, repo, meta_suffix, reason: _, subject, is_pr } = meta;
let label = notif_type_label(notif_type); let label = notif_type_label(notif_type);
let kind = match notif_state { let kind = match notif_state {
"merged" => format!("{label} merged{num}{repo}"), "merged" => format!("{label} merged{num}{repo}"),