diff --git a/hive-forge/src/client.rs b/hive-forge/src/client.rs index 1ae86812..2b388aed 100644 --- a/hive-forge/src/client.rs +++ b/hive-forge/src/client.rs @@ -195,6 +195,33 @@ impl Client { .map(|b| b.to_vec()) .with_context(|| format!("read bytes for GET {url}")) } + + /// GET a JSON `/api/v1` route and deserialize into `T`, bypassing the + /// typed `forgejo-api` client. Use this where the typed client's + /// structs are too strict against the running Forgejo version: the + /// crate pins one schema, but the server tracks the latest release + /// line, so a drifted field breaks deserialization for the whole + /// call. Callers pass a *lenient* local struct (only the fields they + /// use, all `#[serde(default)]`) so a schema change can't wedge the + /// call. `path` starts with `/` and is relative to `/api/v1`; `query` + /// is appended as URL query parameters. + /// + /// # Errors + /// Returns an error on transport failure, a non-2xx response (body + /// included), or if the response body doesn't deserialize into `T`. + pub fn get_api_json( + &self, + path: &str, + query: &[(&str, &str)], + ) -> Result { + let base = format!("{}/api/v1{path}", self.base); + let url = url::Url::parse_with_params(&base, query.iter().copied()) + .with_context(|| format!("build url {base}"))?; + let resp = self.web.get(url.clone()).send().context("GET")?; + let resp = check_status(resp, &format!("GET {url}"))?; + resp.json::() + .with_context(|| format!("decode JSON for GET {url}")) + } } /// Split an `owner/name` repo string into its two path segments for diff --git a/hive-forge/src/notify.rs b/hive-forge/src/notify.rs index cfa50631..aeeca280 100644 --- a/hive-forge/src/notify.rs +++ b/hive-forge/src/notify.rs @@ -13,7 +13,7 @@ //! `forge cli: agents keep commenting without reading prev comments`.) use anyhow::Result; -use forgejo_api::structs::{NotifyGetRepoListQuery, NotifyReadThreadQuery}; +use forgejo_api::structs::NotifyReadThreadQuery; use crate::client::{Client, index, split_repo}; @@ -33,25 +33,47 @@ const MAX_PAGES: u32 = 5; /// 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 query = NotifyGetRepoListQuery { - all: Some(false), - ..Default::default() - }; - let (_, threads) = client - .api() - .notify_get_repo_list(owner, name, query) - .page(page) - .page_size(PAGE_SIZE) - .send()?; + 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_ref()) - .map_or("", url::Url::as_str); + .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())); } @@ -101,7 +123,36 @@ pub fn subject_matches(subject_url: &str, number: u64) -> bool { #[cfg(test)] mod tests { - use super::subject_matches; + 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() {