hyperhive/hive-forge/src/notify.rs
atlas 156ca70b1e feat(#1888): read-before-comment guard via forge notification read-state
Rebuild on forge's own notification read-state instead of the local
seen-cursor mirror (operator-nacked: mirrors state forge owns + drifts
on restart).

- comment refuses to post when forge has an unread notification for the
  thread (someone commented since you last read it); --force overrides.
  Degrades open if the notification check itself fails.
- comments / view mark the thread's notification read (the "I've seen
  it" signal), clearing the guard for a subsequent reply.
- new crate::notify (no local file): unread_thread_id pages repo-scoped
  unread notifications (newest-first, capped) + subject-url number match;
  mark_thread_read via the new client.patch_no_content.

Pairs with the forge_notify harness change (#1895, merged) that leaves
delivered notifications unread until the agent actually reads. Covers
pr/issue comment too (they delegate to comment::run).
2026-06-22 18:54:40 +02:00

138 lines
4.8 KiB
Rust

//! Read-before-comment guard backed by forge's own notification
//! read-state — no local mirror (which drifts across restarts).
//!
//! When someone comments on an issue/PR the caller participates in,
//! forge raises an unread notification for that thread; once the agent
//! actually reads the thread (`comments` / `view`) we mark it read via
//! the notifications API. So "is there unread activity here?" is just
//! "does forge still have an unread notification for this thread?" —
//! forge is the single source of truth, nothing to lose on restart.
//!
//! (This pairs with the `forge_notify` harness change that stops
//! marking notifications read on wake *delivery* — issue tracker
//! `forge cli: agents keep commenting without reading prev comments`.)
use anyhow::Result;
use serde_json::Value;
use crate::client::Client;
/// Forgejo's per-page notification cap.
const PAGE_SIZE: u64 = 50;
/// How many pages of unread notifications to scan for the thread.
/// Notifications come newest-first and a thread the caller is about to
/// comment on was just active, so it sits near the top; this cap keeps
/// the guard cheap even when the unread list is large (an uncontrolled
/// firehose), at the cost of not detecting a match buried past
/// `MAX_PAGES * PAGE_SIZE` unread items (degrade-open — acceptable for
/// a courtesy guard).
const MAX_PAGES: u64 = 5;
/// The notification thread id of an UNREAD notification on
/// `<repo>#<number>`, or `None` when the thread has no unread
/// notification (nothing to read / already read). Repo-scoped so a bare
/// number can't false-match another repo. Errors only on transport /
/// non-2xx — callers degrade open on `Err`.
pub fn unread_thread_id(client: &Client, repo: &str, number: u64) -> Result<Option<u64>> {
for page in 1..=MAX_PAGES {
let v = client.get_json(&format!(
"/repos/{repo}/notifications?all=false&page={page}&limit={PAGE_SIZE}"
))?;
let arr = v.as_array().cloned().unwrap_or_default();
let len = arr.len() as u64;
for n in &arr {
let subject_url = n
.get("subject")
.and_then(|s| s.get("url"))
.and_then(Value::as_str)
.unwrap_or("");
if subject_matches(subject_url, number) {
return Ok(n.get("id").and_then(Value::as_u64));
}
}
// Last (short) page reached — stop.
if len < PAGE_SIZE {
break;
}
}
Ok(None)
}
/// Mark a notification thread read (best-effort; the caller ignores the
/// result). Clears the unread signal so a subsequent comment isn't
/// blocked after the agent has read the thread.
pub fn mark_thread_read(client: &Client, thread_id: u64) -> Result<()> {
client.patch_no_content(&format!("/notifications/threads/{thread_id}"))
}
/// Reading a thread (`comments` / `view`) is the "I've seen it" signal:
/// clear its unread notification so the read-before-comment guard lets a
/// subsequent comment through. Fully best-effort — never fails or noises
/// up the read verb (a thread with no unread notification is a no-op).
pub fn mark_read_best_effort(client: &Client, repo: &str, number: u64) {
if let Ok(Some(id)) = unread_thread_id(client, repo, number) {
let _ = mark_thread_read(client, id);
}
}
/// True when a notification `subject.url` refers to issue/PR `number` in
/// the (already repo-scoped) query. Forgejo subject URLs end in the
/// issue/PR number (`…/repos/o/r/issues/42`, or `…/pulls/42`); match the
/// trailing path segment. Tolerates a trailing slash.
#[must_use]
pub fn subject_matches(subject_url: &str, number: u64) -> bool {
subject_url
.trim_end_matches('/')
.rsplit('/')
.next()
.and_then(|seg| seg.parse::<u64>().ok())
== Some(number)
}
#[cfg(test)]
mod tests {
use super::subject_matches;
#[test]
fn matches_issue_subject_url() {
assert!(subject_matches(
"https://forge/api/v1/repos/o/r/issues/42",
42
));
}
#[test]
fn matches_pull_subject_url() {
assert!(subject_matches("https://forge/api/v1/repos/o/r/pulls/7", 7));
}
#[test]
fn tolerates_trailing_slash() {
assert!(subject_matches(
"https://forge/api/v1/repos/o/r/issues/9/",
9
));
}
#[test]
fn rejects_different_number() {
assert!(!subject_matches(
"https://forge/api/v1/repos/o/r/issues/42",
7
));
}
#[test]
fn rejects_non_numeric_tail() {
assert!(!subject_matches("https://forge/api/v1/repos/o/r", 42));
}
#[test]
fn rejects_substring_number() {
// 142 must not match 42 — full-segment parse, not substring.
assert!(!subject_matches(
"https://forge/api/v1/repos/o/r/issues/142",
42
));
}
}