diff --git a/hive-agent/src/forge_notify.rs b/hive-agent/src/forge_notify.rs index f4794bbf..d6c1de32 100644 --- a/hive-agent/src/forge_notify.rs +++ b/hive-agent/src/forge_notify.rs @@ -24,10 +24,9 @@ use std::fmt::Write as _; use std::path::{Path, PathBuf}; use std::time::Duration; -use forgejo_api::structs::{ - NotificationThread, NotifyGetListQuery, NotifyReadThreadQuery, NotifySubjectType, -}; +use forgejo_api::structs::{NotifyGetListQuery, NotifyReadThreadQuery, NotifySubjectType}; use forgejo_api::{Auth, Forgejo, ForgejoError}; +use serde::{Deserialize, Deserializer}; use time::OffsetDateTime; use time::format_description::well_known::Rfc3339; use tracing::{debug, info, warn}; @@ -870,15 +869,97 @@ fn parse_rfc3339(s: &str) -> Option { OffsetDateTime::parse(s, &Rfc3339).ok() } -/// One notification from the poll page: the typed thread plus two raw -/// fields the typed structs can't carry faithfully. +/// Minimal, drift-tolerant view of a Forgejo notification thread — +/// deliberately NOT `forgejo_api::structs::NotificationThread`. That crate +/// (0.11.0) lags the running `pkgs.forgejo` release, and one field +/// type/shape drift in the upstream struct fails the WHOLE parse, so every +/// notification is dropped, never dedup'd / marked-read, and redelivered +/// forever. This local struct carries only the fields `forge_notify` reads, +/// each optional + lenient, so unknown or reshaped upstream fields can't +/// break notification read-state again. +#[derive(Debug, Deserialize)] +struct NotificationThread { + #[serde(default)] + id: Option, + #[serde(default, deserialize_with = "de_opt_rfc3339")] + updated_at: Option, + #[serde(default)] + subject: Option, + #[serde(default)] + repository: Option, +} + +#[derive(Debug, Deserialize)] +struct NotificationSubject { + #[serde(default)] + title: Option, + #[serde(rename = "type", default, deserialize_with = "de_opt_subject_type")] + r#type: Option, + #[serde(default, deserialize_with = "de_opt_url")] + html_url: Option, + #[serde(default, deserialize_with = "de_opt_url")] + url: Option, + #[serde(default, deserialize_with = "de_opt_url")] + latest_comment_url: Option, + #[serde(default, deserialize_with = "de_opt_url")] + latest_comment_html_url: Option, +} + +#[derive(Debug, Deserialize)] +struct NotificationRepo { + #[serde(default)] + full_name: Option, +} + +/// Deserialize an optional Forgejo subject `type`, mapping any unknown or +/// missing value to `None` instead of failing the parse — so a new Forgejo +/// subject type can never break notification read-state. +fn de_opt_subject_type<'de, D>(d: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + Ok( + Option::::deserialize(d)?.and_then(|s| match s.as_str() { + "Pull" => Some(NotifySubjectType::Pull), + "Issue" => Some(NotifySubjectType::Issue), + "Commit" => Some(NotifySubjectType::Commit), + "Repository" => Some(NotifySubjectType::Repository), + _ => None, + }), + ) +} + +/// Deserialize an optional URL, mapping a blank or unparseable value to +/// `None` (Forgejo marshals empty strings for absent URLs, which a plain +/// `Option` would reject). +fn de_opt_url<'de, D>(d: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + Ok(Option::::deserialize(d)? + .filter(|s| !s.is_empty()) + .and_then(|s| url::Url::parse(&s).ok())) +} + +/// Deserialize an optional RFC 3339 timestamp, mapping any unrecognised +/// shape to `None` via the module's tolerant [`parse_rfc3339`]. +fn de_opt_rfc3339<'de, D>(d: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + Ok(Option::::deserialize(d)? + .as_deref() + .and_then(parse_rfc3339)) +} + +/// One notification from the poll page: the lenient thread plus two raw +/// fields the typed struct can't carry faithfully. struct PolledNotification { thread: NotificationThread, - /// Raw `subject.state`. Forgejo reports `"merged"` for merged PRs - /// (`services/convert/notification.go`), which forgejo-api's - /// `StateType` (open/closed only) rejects at deserialization — so - /// the state is extracted verbatim before the typed parse and - /// matched as a string, exactly like the pre-typed code. + /// Raw `subject.state`, kept as a string. Forgejo reports `"merged"` + /// for merged PRs (`services/convert/notification.go`) alongside + /// open/closed, and the state is matched as a string throughout, so + /// it's extracted verbatim rather than typed into an enum. state: String, /// Raw `updated_at` string, byte-identical to what Forgejo sent, so /// the persisted delivery-dedupe cursor keeps its exact format @@ -888,16 +969,12 @@ struct PolledNotification { } /// Parse one notification JSON object: pull out the raw `subject.state` -/// and `updated_at` (see [`PolledNotification`]), null the state so the -/// closed `StateType` enum can't reject it, and deserialize the rest -/// into the typed [`NotificationThread`]. Returns `None` (with a warn) -/// for an item the typed struct can't represent — the rest of the page -/// still delivers. -fn parse_notification(mut value: serde_json::Value) -> Option { +/// and `updated_at` (see [`PolledNotification`]) as strings, then +/// deserialize the rest into the lenient local [`NotificationThread`]. +/// Returns `None` (with a warn) only for a fundamentally malformed item +/// (e.g. a non-object) — the rest of the page still delivers. +fn parse_notification(value: serde_json::Value) -> Option { let state = value["subject"]["state"].as_str().unwrap_or("").to_owned(); - if let Some(s) = value.get_mut("subject").and_then(|s| s.get_mut("state")) { - *s = serde_json::Value::Null; - } let updated_at = value["updated_at"].as_str().unwrap_or("").to_owned(); match serde_json::from_value::(value) { Ok(thread) => Some(PolledNotification { @@ -1445,6 +1522,48 @@ mod tests { assert!(parse_notification(serde_json::json!("nonsense")).is_none()); } + #[test] + fn parse_notification_survives_schema_drift() { + // The whole point of the local lenient struct: a notification + // carrying fields the upstream forgejo-api struct didn't expect — + // an unknown top-level key, an unknown subject `type`, a blank + // url — must still parse so the item can be dedup'd + marked-read. + // Otherwise the parse fails, the item is dropped, and it + // redelivers forever. + let polled = parse_notification(serde_json::json!({ + "id": 42, + "updated_at": "2026-07-17T12:00:00Z", + "url": "", + "some_new_forgejo_field": { "nested": true }, + "subject": { + "title": "t", + "type": "SomeBrandNewType", + "html_url": "", + "url": "http://forge/api/v1/repos/o/r/issues/9", + "another_unknown": 123, + }, + })) + .expect("drifted notification must still parse"); + assert_eq!(polled.thread.id, Some(42)); + // Unknown subject type degrades to None instead of failing. + assert!( + polled + .thread + .subject + .as_ref() + .and_then(|s| s.r#type) + .is_none() + ); + // Blank html_url is None, not a parse error. + assert!( + polled + .thread + .subject + .as_ref() + .is_some_and(|s| s.html_url.is_none()) + ); + } + /// Build a `NotifMeta` for the state-change formatter tests. The `&str` /// fields borrow `'static` literals so the value is self-contained. fn state_change_meta(subject: serde_json::Value) -> NotifMeta<'static> {