fix(hive-forge-notify): stop embedding the issue/PR description in todos

This commit is contained in:
damocles 2026-08-16 23:19:06 +02:00 committed by mara
commit 4db3d1e4ae
2 changed files with 45 additions and 23 deletions

View file

@ -190,9 +190,10 @@ self-echo filtering off for the whole process lifetime.
### Body excerpt + truncation + heading escape ### Body excerpt + truncation + heading escape
The wake message embeds the comment / review / new-item body so the The wake message embeds the comment / review body so the agent sees
agent sees actual content without a follow-up fetch. Three pipeline actual content without a follow-up fetch — **not** a new issue/PR's own
steps in order: description, which the wrapper table below deliberately omits. Three
pipeline steps in order:
1. **Truncate** to `BODY_TRUNCATE = 500` chars at a char-boundary; 1. **Truncate** to `BODY_TRUNCATE = 500` chars at a char-boundary;
appends `…` when cut. Truncation happens BEFORE escape so the appends `…` when cut. Truncation happens BEFORE escape so the
@ -226,11 +227,17 @@ Five shapes, distinguished by the notification's classification:
| Trigger | Wrapper | | Trigger | Wrapper |
| ----------------------------------- | --------------------------------------------------------------------------------- | | ----------------------------------- | --------------------------------------------------------------------------------- |
| Comment on issue / PR | `[comment on PR #N owner/repo] title\nurl: ...\n\nauthor: body\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: ...` | | 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<body excerpt>\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: ...\n\n<body excerpt>\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: ...` | | 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` Review labels come from the Forgejo `state` field: `APPROVED`
`approved`, `REQUEST_CHANGES``changes requested`, `COMMENT` `approved`, `REQUEST_CHANGES``changes requested`, `COMMENT`

View file

@ -594,20 +594,15 @@ fn format_state_change_notification(
kind kind
}; };
// Include the start of the issue/PR description so the agent gets context // Deliberately NOT embedding the issue/PR's own description here — mara,
// without a follow-up fetch. Same body pipeline as comment bodies. // triage: "dont pollute todos with full issue or pr description... just
let body_block = subject // the issue title should be enough." Unlike a comment or review body
.as_ref() // (the actual new content a notification exists to surface), the
.and_then(|s| s["body"].as_str()) // subject's `body` is the description the agent already wrote or can
.map(str::trim) // read from the URL — repeating it in every "new"/"activity on" wake
.filter(|s| !s.is_empty()) // for the same subject is noise, not context. `title` + `url` is the
.map(|raw| { // whole point of this wrapper.
let (excerpt, mentions) = render_body_excerpt(raw); let mut out = format!("[{kind}] {title}\nurl: {html_url}");
format!("\n\n{excerpt}{mentions}")
})
.unwrap_or_default();
let mut out = format!("[{kind}] {title}\nurl: {html_url}{body_block}");
// Append a comment that raced the merge/close (best of both worlds), when // Append a comment that raced the merge/close (best of both worlds), when
// the caller found one genuinely newer than `closed_at`. // the caller found one genuinely newer than `closed_at`.
if let Some(tail) = comment_tail { if let Some(tail) = comment_tail {
@ -1464,7 +1459,7 @@ mod tests {
#[test] #[test]
fn format_state_change_appends_comment_tail() { 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. // suffix, so the assignee line stays last.
let meta = state_change_meta(serde_json::json!({ let meta = state_change_meta(serde_json::json!({
"user": { "login": "someone-else" }, "user": { "login": "someone-else" },
@ -1484,4 +1479,24 @@ mod tests {
let assignee_at = out.find("assignee:").unwrap(); let assignee_at = out.find("assignee:").unwrap();
assert!(tail_at < assignee_at); 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"));
}
} }