From 4db3d1e4ae338c8f34d839a42c5197f88690e3d9 Mon Sep 17 00:00:00 2001 From: damocles Date: Sun, 16 Aug 2026 23:19:06 +0200 Subject: [PATCH] fix(hive-forge-notify): stop embedding the issue/PR description in todos --- docs/forge.md | 23 +++++++++++------ hive-forge-notify/src/notify.rs | 45 ++++++++++++++++++++++----------- 2 files changed, 45 insertions(+), 23 deletions(-) diff --git a/docs/forge.md b/docs/forge.md index 51e8c7cb..40445952 100644 --- a/docs/forge.md +++ b/docs/forge.md @@ -190,9 +190,10 @@ self-echo filtering off for the whole process lifetime. ### Body excerpt + truncation + heading escape -The wake message embeds the comment / review / new-item body so the -agent sees actual content without a follow-up fetch. Three pipeline -steps in order: +The wake message embeds the comment / review body so the agent sees +actual content without a follow-up fetch — **not** a new issue/PR's own +description, which the wrapper table below deliberately omits. Three +pipeline steps in order: 1. **Truncate** to `BODY_TRUNCATE = 500` chars at a char-boundary; appends `…` when cut. Truncation happens BEFORE escape so the @@ -226,11 +227,17 @@ 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: ...` | -| Review submission | `[PR approved #N owner/repo] title\nurl: ...\n\nauthor: 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: ...` | +| 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\nauthor: body\nassignee: ...` | +| New issue / PR | `[new PR #N owner/repo] title\nurl: ...\nassignee: ...` | +| Later activity (open, not creation) | `[activity on PR #N owner/repo] title\nurl: ...\nassignee: ...` | +| State change | `[PR merged #N owner/repo] title\nurl: ...\nassignee: ...` | + +The issue/PR's own description is deliberately never embedded here (only a +comment/review body is — that's the actual new content a notification +exists to surface): repeating a subject's own description on every "new" +or "activity on" wake for it is noise the agent already has via the URL, +not context (mara, triage: "just the issue title should be enough"). Review labels come from the Forgejo `state` field: `APPROVED` → `approved`, `REQUEST_CHANGES` → `changes requested`, `COMMENT` → diff --git a/hive-forge-notify/src/notify.rs b/hive-forge-notify/src/notify.rs index fbeb60a6..c80afeb7 100644 --- a/hive-forge-notify/src/notify.rs +++ b/hive-forge-notify/src/notify.rs @@ -594,20 +594,15 @@ fn format_state_change_notification( kind }; - // Include the start of the issue/PR description so the agent gets context - // without a follow-up fetch. Same body pipeline as comment bodies. - let body_block = subject - .as_ref() - .and_then(|s| s["body"].as_str()) - .map(str::trim) - .filter(|s| !s.is_empty()) - .map(|raw| { - let (excerpt, mentions) = render_body_excerpt(raw); - format!("\n\n{excerpt}{mentions}") - }) - .unwrap_or_default(); - - let mut out = format!("[{kind}] {title}\nurl: {html_url}{body_block}"); + // Deliberately NOT embedding the issue/PR's own description here — mara, + // triage: "dont pollute todos with full issue or pr description... just + // the issue title should be enough." Unlike a comment or review body + // (the actual new content a notification exists to surface), the + // subject's `body` is the description the agent already wrote or can + // read from the URL — repeating it in every "new"/"activity on" wake + // for the same subject is noise, not context. `title` + `url` is the + // whole point of this wrapper. + let mut out = format!("[{kind}] {title}\nurl: {html_url}"); // Append a comment that raced the merge/close (best of both worlds), when // the caller found one genuinely newer than `closed_at`. if let Some(tail) = comment_tail { @@ -1464,7 +1459,7 @@ mod tests { #[test] fn format_state_change_appends_comment_tail() { - // The racing-comment tail lands between the body block and the meta + // The racing-comment tail lands between the url line and the meta // suffix, so the assignee line stays last. let meta = state_change_meta(serde_json::json!({ "user": { "login": "someone-else" }, @@ -1484,4 +1479,24 @@ mod tests { let assignee_at = out.find("assignee:").unwrap(); assert!(tail_at < assignee_at); } + + #[test] + fn format_state_change_never_embeds_the_subject_body() { + // mara, triage: "dont pollute todos with full issue or pr + // description... just the issue title should be enough." Covers + // both branches that used to embed it — the "new" path and a + // later state change — so a regression can't sneak back in via + // either. + let meta = state_change_meta(serde_json::json!({ + "user": { "login": "someone-else" }, + "body": "the PR description", + })); + let new_out = format_state_change_notification(None, "open", &meta, "damocles", None) + .expect("open notification must render"); + assert!(!new_out.contains("the PR description")); + + let merged_out = format_state_change_notification(None, "merged", &meta, "damocles", None) + .expect("merge notification must render"); + assert!(!merged_out.contains("the PR description")); + } }