fix(#2565): lenient local notification struct in forge_notify so forgejo schema drift can't drop notifications

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> {
     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<i64>,
+    #[serde(default, deserialize_with = "de_opt_rfc3339")]
+    updated_at: Option<OffsetDateTime>,
+    #[serde(default)]
+    subject: Option<NotificationSubject>,
+    #[serde(default)]
+    repository: Option<NotificationRepo>,
+}
+
+#[derive(Debug, Deserialize)]
+struct NotificationSubject {
+    #[serde(default)]
+    title: Option<String>,
+    #[serde(rename = "type", default, deserialize_with = "de_opt_subject_type")]
+    r#type: Option<NotifySubjectType>,
+    #[serde(default, deserialize_with = "de_opt_url")]
+    html_url: Option<url::Url>,
+    #[serde(default, deserialize_with = "de_opt_url")]
+    url: Option<url::Url>,
+    #[serde(default, deserialize_with = "de_opt_url")]
+    latest_comment_url: Option<url::Url>,
+    #[serde(default, deserialize_with = "de_opt_url")]
+    latest_comment_html_url: Option<url::Url>,
+}
+
+#[derive(Debug, Deserialize)]
+struct NotificationRepo {
+    #[serde(default)]
+    full_name: Option<String>,
+}
+
+/// 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<Option<NotifySubjectType>, D::Error>
+where
+    D: Deserializer<'de>,
+{
+    Ok(
+        Option::<String>::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<Url>` would reject).
+fn de_opt_url<'de, D>(d: D) -> Result<Option<url::Url>, D::Error>
+where
+    D: Deserializer<'de>,
+{
+    Ok(Option::<String>::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<Option<OffsetDateTime>, D::Error>
+where
+    D: Deserializer<'de>,
+{
+    Ok(Option::<String>::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<PolledNotification> {
+/// 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<PolledNotification> {
     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::<NotificationThread>(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> {