diff --git a/docs/forge.md b/docs/forge.md index a2dfd793..527ca5ac 100644 --- a/docs/forge.md +++ b/docs/forge.md @@ -129,16 +129,16 @@ best-effort: logged at debug/warn and retried next tick. Forgejo fires notifications for the agent's own actions (it opened a PR, posted a comment, submitted a review). Surfacing those would -loop claude on its own writes. Two filter rules drop them silently -(mark-read without delivery): +loop claude on its own writes. The comment/review case is dropped +silently (mark-read without delivery): -- **Self-authored new items** — notifications with - `reason == "author"` AND subject state `open` (or missing). State - transitions (merge / close) on the agent's own PRs DO surface, - since those are triggered by someone else. - **Self-authored comments / reviews** — comment payload's `user.login` matches `own_login`. +Self-authored *new items* (an agent opening its own PR/issue) are not +filtered and do surface — the notification subject carries no author +field to match against without a per-notification fetch. + `own_login` is fetched once at startup via `GET /api/v1/user`. On fetch failure the filter degrades open (no filtering) rather than crashing the task — a noisy inbox beats a silently-stuck poller. @@ -181,11 +181,11 @@ Five shapes, distinguished by the notification's classification: | Trigger | Wrapper | | --- | --- | -| Comment on issue / PR | `[comment on PR #N owner/repo] title\nurl: ...\n\nauthor: body\nassignee: ...\nreason: mention` | -| Review submission | `[PR approved #N owner/repo] title\nurl: ...\n\nreviewer: body\nassignee: ...\nreason: review_requested` | -| New issue / PR | `[new PR #N owner/repo] title\nurl: ...\n\n\nassignee: ...\nreason: subscribed` | -| Later activity (open, not creation) | `[activity on PR #N owner/repo] title\nurl: ...\n\n\nassignee: ...\nreason: subscribed` | -| State change | `[PR merged #N owner/repo] title\nurl: ...\nassignee: ...\nreason: subscribed` | +| Comment on issue / PR | `[comment on PR #N owner/repo] title\nurl: ...\n\nauthor: body\nassignee: ...` | +| Review submission | `[PR approved #N owner/repo] title\nurl: ...\n\nreviewer: body\nassignee: ...` | +| New issue / PR | `[new PR #N owner/repo] title\nurl: ...\n\n\nassignee: ...` | +| Later activity (open, not creation) | `[activity on PR #N owner/repo] title\nurl: ...\n\n\nassignee: ...` | +| State change | `[PR merged #N owner/repo] title\nurl: ...\nassignee: ...` | Review labels come from the Forgejo `state` field: `APPROVED` → `approved`, `REQUEST_CHANGES` → `changes requested`, `COMMENT` → @@ -222,14 +222,6 @@ Every wrapper ends with one or more of: the line shape is stable. - `reviewer: ` — PR notifications only, present only when `requested_reviewers` is non-empty. -- `reason: ` — always present when the notification - carries a reason; absent when the field is null/missing. - -The `reason` line distinguishes otherwise-identical messages: Forgejo -emits one notification per applicable reason for the same event -(e.g. both `mention` and `subscribed` arrive for a PR comment that -tags the agent). Without the suffix, the agent would see duplicated -wrapper text with no signal which Forgejo path triggered each copy. ### Review-request override diff --git a/hive-ag3nt/src/forge_notify.rs b/hive-ag3nt/src/forge_notify.rs index b56b4c7d..4a8c5b26 100644 --- a/hive-ag3nt/src/forge_notify.rs +++ b/hive-ag3nt/src/forge_notify.rs @@ -382,8 +382,7 @@ async fn format_notification( }; let is_pr = matches!(notif_type, "Pull Request" | "Pull"); - let reason = notif["reason"].as_str().unwrap_or(""); - let meta_suffix = build_meta_suffix(subject.as_ref(), is_pr, reason); + let meta_suffix = build_meta_suffix(subject.as_ref(), is_pr); // Determine whether this notification was triggered by a comment/review or // by creation/state-change of the subject itself. @@ -396,7 +395,6 @@ async fn format_notification( num, repo, meta_suffix, - reason, subject, is_pr, }; @@ -411,7 +409,7 @@ async fn format_notification( ) .await } else { - format_state_change_notification(notif, &meta, own_login) + Some(format_state_change_notification(notif, &meta, own_login)) } } @@ -423,19 +421,15 @@ struct NotifMeta<'a> { num: String, repo: String, meta_suffix: String, - /// Forgejo `reason` value (e.g. "mention", "assigned", "subscribed"). - /// Appended to every wrapper as the `reason:` line in the meta - /// suffix (see `docs/forge.md::Meta suffix`). - reason: &'a str, /// Fetched subject detail (issue/PR JSON); used for review-request detection. subject: Option, is_pr: bool, } -/// Build the `\nassignee: ...` (and optionally `\nreviewer: ...` and -/// `\nreason: ...`) suffix appended to every wrapper. Shape + -/// presence rules live in `docs/forge.md::Meta suffix`. -fn build_meta_suffix(subject: Option<&serde_json::Value>, is_pr: bool, reason: &str) -> String { +/// Build the `\nassignee: ...` (and optionally `\nreviewer: ...`) +/// suffix appended to every wrapper. Shape + presence rules live in +/// `docs/forge.md::Meta suffix`. +fn build_meta_suffix(subject: Option<&serde_json::Value>, is_pr: bool) -> String { let assignees: Vec<&str> = subject .and_then(|s| s["assignees"].as_array()) .map(|arr| arr.iter().filter_map(|a| a["login"].as_str()).collect()) @@ -459,21 +453,10 @@ fn build_meta_suffix(subject: Option<&serde_json::Value>, is_pr: bool, reason: & } else { None }; - // Always include reason so multiple notifications for the same - // event (each with a different Forgejo reason) stay - // distinguishable. - 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 } @@ -574,7 +557,7 @@ fn format_state_change_notification( notif: &serde_json::Value, meta: &NotifMeta<'_>, own_login: &str, -) -> Option { +) -> String { // Classification uses notif["subject"]["state"] directly — Forgejo // returns "open" / "closed" / "merged" here. We do NOT rely on // fetching the PR/issue detail for `merged`: @@ -583,14 +566,9 @@ fn format_state_change_notification( // - Forgejo API type is "Pull" / "Issue", never "Pull Request". let notif_state = notif["subject"]["state"].as_str().unwrap_or(""); - // Self-notification filter: drop new items we authored ourselves - // (`reason == "author"` + open state). State transitions on our - // own PRs (merge / close) come from someone else, so those stay. + // "New" = the subject is open (or state is absent). Used below for + // the review-request override. let is_new = notif_state == "open" || notif_state.is_empty(); - if is_new && meta.reason == "author" && !own_login.is_empty() { - debug!(%own_login, "forge_notify: skipping self-authored new item"); - return None; - } let NotifMeta { title, @@ -599,7 +577,6 @@ fn format_state_change_notification( num, repo, meta_suffix, - reason: _, subject, is_pr, } = meta; @@ -658,7 +635,7 @@ fn format_state_change_notification( let mut out = format!("[{kind}] {title}\nurl: {html_url}{body_block}"); out.push_str(meta_suffix); - Some(out) + out } /// Decide whether a state-change notification represents the subject's