//! 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 forgejo_api::structs::NotifyReadThreadQuery; use crate::client::{Client, index, split_repo}; /// Forgejo's per-page notification cap. const PAGE_SIZE: u32 = 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: u32 = 5; /// The notification thread id of an UNREAD notification on /// `#`, 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`. /// Minimal, drift-tolerant view of a Forgejo notification thread — only /// the two fields the read-before-comment guard needs: `id` (to mark the /// thread read) and `subject.url` (to match the thread to an issue/PR /// number). Both `#[serde(default)]` so a schema change anywhere else in /// the notification object can't break this deserialize. This is the /// deliberate replacement for `forgejo-api`'s strict `NotificationThread`, /// whose schema drift vs the running (latest-line) Forgejo was breaking /// the whole notifications call — silently disabling the guard and /// wedging notification read-state (so wakes kept redelivering). #[derive(serde::Deserialize)] struct Notification { #[serde(default)] id: Option, #[serde(default)] subject: Option, } /// The one notification-subject field the guard reads: the API `url`, /// which ends in the issue/PR number (see [`subject_matches`]). #[derive(serde::Deserialize)] struct Subject { #[serde(default)] url: Option, } pub fn unread_thread_id(client: &Client, repo: &str, number: u64) -> Result> { let (owner, name) = split_repo(repo)?; let limit = PAGE_SIZE.to_string(); let path = format!("/repos/{owner}/{name}/notifications"); for page in 1..=MAX_PAGES { let page_str = page.to_string(); let threads: Vec = client.get_api_json( &path, &[("all", "false"), ("page", &page_str), ("limit", &limit)], )?; for n in &threads { let subject_url = n .subject .as_ref() .and_then(|s| s.url.as_deref()) .unwrap_or(""); if subject_matches(subject_url, number) { return Ok(n.id.and_then(|id| u64::try_from(id).ok())); } } // Last (short) page reached — stop. if threads.len() < PAGE_SIZE as usize { 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 .api() .notify_read_thread(index(thread_id)?, NotifyReadThreadQuery::default()) .send()?; Ok(()) } /// 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::().ok()) == Some(number) } #[cfg(test)] mod tests { use super::{Notification, subject_matches}; #[test] fn notification_deserializes_leniently_from_drifted_json() { // A payload carrying fields the guard doesn't model (which would // break `forgejo-api`'s strict struct) plus a missing subject — // must still yield `id` + `subject.url` and never fail to parse. let json = serde_json::json!([ { "id": 42, "unread": true, "pinned": false, "subject": { "title": "whatever", "url": "https://forge/api/v1/repos/o/r/issues/7", "type": "SomeFutureSubjectTypeWeDoNotKnow" }, "some_new_field": { "nested": 1 } }, { "id": 43 } ]); let ns: Vec = serde_json::from_value(json).expect("lenient deser"); assert_eq!(ns[0].id, Some(42)); assert_eq!( ns[0].subject.as_ref().and_then(|s| s.url.as_deref()), Some("https://forge/api/v1/repos/o/r/issues/7") ); assert_eq!(ns[1].id, Some(43)); assert!(ns[1].subject.is_none()); } #[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 )); } }