refactor(#1896): remove dead forge_notify reason plumbing + fix docs
This commit is contained in:
damocles
mara
parent
abd70531d5
commit
00f682991c
2 changed files with 21 additions and 52 deletions
|
|
@ -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<body excerpt>\nassignee: ...\nreason: subscribed` |
|
||||
| Later activity (open, not creation) | `[activity on PR #N owner/repo] title\nurl: ...\n\n<body excerpt>\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<body excerpt>\nassignee: ...` |
|
||||
| Later activity (open, not creation) | `[activity on PR #N owner/repo] title\nurl: ...\n\n<body excerpt>\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: <list>` — PR notifications only, present only when
|
||||
`requested_reviewers` is non-empty.
|
||||
- `reason: <forgejo-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
|
||||
|
||||
|
|
|
|||
|
|
@ -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<serde_json::Value>,
|
||||
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> {
|
||||
) -> 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
|
||||
|
|
|
|||
Loading…
Reference in a new issue