diff --git a/docs/forge.md b/docs/forge.md index 0ea3f431..093b5860 100644 --- a/docs/forge.md +++ b/docs/forge.md @@ -213,7 +213,7 @@ 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\nreviewer: 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: ...` | @@ -224,6 +224,10 @@ Review labels come from the Forgejo `state` field: `APPROVED` → submitted yet — no peer-visible event). Unknown states fall back to the generic comment wrapper. +A review submitted with **no body** renders `reviewed by: ` in +place of the `: ` line — deliberately worded to not collide +with the meta-suffix `reviewer:` line (requested reviewers, below). + ### "new" vs "activity on" A review submitted with **no body** carries no `latest_comment_url`, diff --git a/hive-ag3nt/src/forge_notify.rs b/hive-ag3nt/src/forge_notify.rs index 855c3f87..99a1f225 100644 --- a/hive-ag3nt/src/forge_notify.rs +++ b/hive-ag3nt/src/forge_notify.rs @@ -6,8 +6,8 @@ //! off forge's own unread-state, and the agent reading the thread via the //! CLI is what marks it read. A delivery-dedupe cursor (thread id → //! last-delivered `updated_at`) stops the still-unread notification from -//! re-firing a wake every poll; self-echo and drop-listed notifications -//! are still marked read directly. The cursor is persisted as the +//! re-firing a wake every poll; self-echo notifications (the agent's own +//! writes) are still marked read directly. The cursor is persisted as the //! `forge_cursor` field of the harness's consolidated `hyperhive-harness.json` //! (via [`crate::events`]) and reloaded on boot so a container //! rebuild/restart doesn't re-deliver the whole currently-unread backlog — @@ -247,7 +247,7 @@ fn truncate(s: &str, max: usize) -> String { let end = s .char_indices() .map(|(i, _)| i) - .take_while(|&i| i <= max - 3) + .take_while(|&i| i <= max.saturating_sub(3)) .last() .unwrap_or(0); format!("{}…", &s[..end]) @@ -314,6 +314,20 @@ fn render_truncated_mentions(lines: &[&str]) -> String { out } +/// The escaped body excerpt + overflowed-mention suffix for one issue / PR / +/// comment body, computed in the one required order: truncate → diff the +/// overflowed `@mention` lines against the *unescaped* excerpt → heading-escape +/// the excerpt. Both notification formatters build their body block from this +/// pair so the ordering contract lives in exactly one place. Returns +/// `(escaped_excerpt, mentions_suffix)`; `mentions_suffix` is empty when the +/// body fit (nothing was truncated away). +fn render_body_excerpt(raw: &str) -> (String, String) { + let raw_excerpt = truncate(raw, BODY_TRUNCATE); + let mentions = render_truncated_mentions(&extract_truncated_mention_lines(raw, &raw_excerpt)); + let excerpt = escape_md_headings(&raw_excerpt); + (excerpt, mentions) +} + /// Map a Forgejo review state to a readable action label. /// Returns `None` for non-review states (regular comments have no `state` field; /// `PENDING` means the review was saved but not submitted yet). @@ -378,7 +392,9 @@ async fn format_notification( fetch_json(client, subject_api_url, token).await }; - let is_pr = matches!(notif_type, "Pull Request" | "Pull"); + // Forgejo's notification `subject.type` is "Pull" / "Issue" (never + // "Pull Request") — see the comment in `build_meta_suffix` below. + let is_pr = notif_type == "Pull"; let meta_suffix = build_meta_suffix(subject.as_ref(), is_pr); // Determine whether this notification was triggered by a comment/review or @@ -513,23 +529,17 @@ async fn format_comment_notification( .. } = meta; - // Truncate → mention-overflow → escape, in that order. See - // `docs/forge.md::Body excerpt + truncation + heading escape` for - // why truncate comes before escape (mention diff compares against - // unescaped raw body). - let raw_excerpt = truncate(body_text, BODY_TRUNCATE); - let truncated_mentions = if body_text.len() > BODY_TRUNCATE { - render_truncated_mentions(&extract_truncated_mention_lines(body_text, &raw_excerpt)) - } else { - String::new() - }; - let body_for_embed = escape_md_headings(&raw_excerpt); + // Truncate → mention-overflow → escape (see `render_body_excerpt`). + let (body_for_embed, truncated_mentions) = render_body_excerpt(body_text); if let Some(review_label) = review_state { // Review submission on a PR. let kind = format!("PR {review_label}{num}{repo}"); let mut out = format!("[{kind}] {title}\nurl: {url}"); if body_text.is_empty() { - write!(out, "\n\nreviewer: {author}").ok(); + // Bodiless review: name who reviewed. `reviewed by:` (not + // `reviewer:`) to avoid colliding with the `reviewer:` line the + // meta suffix carries for a PR's *requested* reviewers. + write!(out, "\n\nreviewed by: {author}").ok(); } else { write!(out, "\n\n{author}: {body_for_embed}{truncated_mentions}").ok(); } @@ -541,9 +551,6 @@ async fn format_comment_notification( let mut out = format!( "[{kind}] {title}\nurl: {url}\n\n{author}: {body_for_embed}{truncated_mentions}" ); - if out.ends_with('\n') { - out.pop(); - } out.push_str(meta_suffix); Some(out) } @@ -633,20 +640,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 truncate → - // mention-overflow → escape pipeline as comment bodies (see - // `docs/forge.md::Body excerpt + truncation + heading escape`). + // 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 raw_excerpt = truncate(raw, BODY_TRUNCATE); - let truncated = extract_truncated_mention_lines(raw, &raw_excerpt); - let mentions = render_truncated_mentions(&truncated); - let excerpt = escape_md_headings(&raw_excerpt); + let (excerpt, mentions) = render_body_excerpt(raw); format!("\n\n{excerpt}{mentions}") }) .unwrap_or_default(); @@ -712,8 +714,15 @@ fn parse_rfc3339_secs(s: &str) -> Option { if !(rest.is_empty() || rest.starts_with('Z')) { let sign = rest.as_bytes()[0]; let off = &rest[1..]; - let oh: i64 = off.get(0..2)?.parse().ok()?; - let om: i64 = off.get(3..5).unwrap_or("00").parse().ok()?; + // Accept `HH:MM` (Forgejo's form) and bare `HHMM`; minutes optional. + // Both fields fail the same way — a present-but-unparseable component + // returns `None` rather than one silently defaulting. + let (hh, mm) = match off.split_once(':') { + Some((h, m)) => (h, m), + None => (off.get(0..2)?, off.get(2..4).unwrap_or("00")), + }; + let oh: i64 = hh.parse().ok()?; + let om: i64 = if mm.is_empty() { 0 } else { mm.parse().ok()? }; let offset = oh * 3_600 + om * 60; match sign { b'+' => epoch -= offset, @@ -885,11 +894,11 @@ fn should_deliver(delivered: &HashMap, id: u64, updated_at: &str) - } /// Mark a notification thread as read. Best-effort — logs on failure but -/// does not abort the poll loop. Called only on the self-echo and -/// drop-listed paths (the agent's own writes / explicitly-suppressed -/// reasons) — delivered threads are deliberately left unread for the -/// read-before-comment guard, and a failed delivery is left unread + out -/// of the dedupe cursor so it resurfaces on the next poll tick. +/// does not abort the poll loop. Called only on the self-echo path (the +/// agent's own comment/review/creation writes) — delivered threads are +/// deliberately left unread for the read-before-comment guard, and a failed +/// delivery is left unread + out of the dedupe cursor so it resurfaces on the +/// next poll tick. async fn mark_read(client: &reqwest::Client, forge_url: &str, token: &str, id: u64) { let mark_url = format!("{forge_url}/api/v1/notifications/threads/{id}"); match client