fix(parser): surface task-notification wrapped in system-reminder (v2.1.234+)

specs/01-parser-pipeline.md

@@ -177,6 +177,7 @@ flowchart TD
 | A "diagnostics attachment" content block can appear in `message.content`, distinct from the `image`/`document` attachment blocks handled since v2.1.97. The exact `type` discriminant isn't documented; a message made up only of such a block previously fell through every known-type check (`extract_text`, `has_user_content`) and was dropped entirely rather than rendered, mirroring the upstream CLI bug this release fixed on its own history-loading path.                                                                                                                                                                                                                                   | v2.1.223 (issue #235)                                       | `sanitize::is_attachment_block_type()` recognises `image`/`document` plus any `type` string containing `"attachment"` or `"diagnostic"`, so a future attachment shape is caught without a hardcoded literal. `extract_text()` renders a generic `[Attachment: <type>]` placeholder when no other block in the array produced text; `has_user_content()`, `has_user_content_raw()`, and `scan_ongoing_user()`'s continuation-content check all reuse the same predicate so the message is classified instead of silently dropped, and pending-tool-id/liveness tracking stays consistent with what actually renders.                                                                                                                                                                                                                                                                                                                                                                                               |
 | Cross-session `SendMessage` lets sessions on different machines message each other, discovered via `ListAgents`; new settings `crossSessionInbound` (`accept`/`hold`/`refuse`) and `dialogExpiry` hold inbound messages for approval or auto-deliver them — an approved/auto-accepted message is delivered as a `type:"user"` entry whose content is wrapped in `<cross-session-message from="..." from-name="...">...</cross-session-message>` (confirmed by inspecting the shipped v2.1.226 CLI binary; a held message that expires without approval is simply never delivered, so it never reaches the transcript at all).                                                                          | v2.1.224+ / v2.1.225+                                       | `CROSS_SESSION_MESSAGE_RE` / `CROSS_SESSION_FROM_NAME_RE` in `patterns.rs` detect the wrapper (mirroring the existing `<teammate-message teammate_id="..." color="...">` mechanism). `sanitize::extract_cross_session_message` unwraps it and prefixes the text with `[from-name]:` so sender attribution is not lost; `sanitize_content` calls it before any other tag handling so the raw markup never leaks into the UI. `SendMessage`'s tool-call summary (`summary_send_message`) already handles the `recipient` field pointing at a Remote Control/cloud session name via its existing generic branch — no per-`type` allowlist to fall through. Regression tests: `parse_entry_preserves_cross_session_message_wrapper_verbatim` (`entry.rs`), `classify_unwraps_cross_session_message_with_sender_attribution` (`classify.rs`), `extract_cross_session_message_*` / `sanitize_cross_session_message_unwraps_tag_with_attribution` (`sanitize.rs`), `summary_send_message_unknown_type_*` (`summary.rs`). |
 | On the non-streaming fallback path (typically via third-party gateways), the API response can contain a `text` content block missing its `text` field, or a `thinking` block missing its `thinking` field — Claude Code itself used to crash on this and now handles it gracefully.                                                                                                                                                                                                                                                                                                                                                                                                                    | v2.1.234 (issue #260)                                       | Every call site that reads these fields (`extract_assistant_details` in `classify.rs`, `extract_text` in `sanitize.rs`, plus `session.rs`/`subagent.rs`) already reads via `serde_json`'s `.get("text"/"thinking").and_then(\|v\| v.as_str()).unwrap_or("")`, which defaults to an empty string when the field is absent — no struct requires the field. Regression tests pin this: `classify_text_block_missing_text_field_does_not_panic` / `classify_thinking_block_missing_thinking_field_does_not_panic` (`classify.rs`), `extract_text_block_missing_text_field_does_not_panic` / `extract_text_block_missing_text_field_alongside_real_text` (`sanitize.rs`).                                                                                                                                                                                                                                                                                                                                              |
+| Between-turn background-task notifications are now delivered fully wrapped in `<system-reminder>...</system-reminder>`, matching the wrapping mid-turn delivery already used. Previously a `<task-notification>` entry arrived unwrapped and was routed to `ClassifiedMsg::System` via the `TASK_NOTIFICATION_TAG` check; wrapped, it instead matched `is_user_noise()`'s pure-reminder rule (starts _and_ ends with `<system-reminder>`) and was dropped entirely, silently losing the task's completed/failed/killed status.                                                                                                                                                                         | v2.1.234+ (issue #262)                                      | `classify::unwrap_system_reminder()` strips a full `<system-reminder>...</system-reminder>` wrapper when present. `is_user_noise()` now checks the unwrapped inner content for a `<task-notification>` prefix before treating a `<system-reminder>`-wrapped entry as noise; the `TASK_NOTIFICATION_TAG` dispatch in `classify()` does the same check so the wrapped notification still produces a `System` message with the correct `is_error` status. Plain reminders with no task-notification inside are unaffected. Regression tests: `classify_task_notification_wrapped_in_system_reminder_is_system_msg`, `classify_task_notification_wrapped_in_system_reminder_failed_is_error`, `classify_returns_none_for_plain_system_reminder_still_dropped` (`classify.rs`).                                                                                                                                                                                                                                        |
 
 ---
 

src-tauri/src/parser/classify.rs

@@ -184,6 +184,18 @@ const AUTO_MODE_DENIAL_DATA_TYPES: &[&str] = &[
 
 const HARD_NOISE_TAGS: &[&str] = &["<local-command-caveat>", "<system-reminder>"];
 
+/// Returns the inner content of a `<system-reminder>...</system-reminder>` wrapper when the
+/// entire trimmed string is exactly that wrapper. Returns the input unchanged otherwise.
+fn unwrap_system_reminder(trimmed: &str) -> &str {
+    const OPEN: &str = "<system-reminder>";
+    const CLOSE: &str = "</system-reminder>";
+    if trimmed.starts_with(OPEN) && trimmed.ends_with(CLOSE) {
+        trimmed[OPEN.len()..trimmed.len() - CLOSE.len()].trim()
+    } else {
+        trimmed
+    }
+}
+
 const EMPTY_STDOUT: &str = "<local-command-stdout></local-command-stdout>";
 const EMPTY_STDERR: &str = "<local-command-stderr></local-command-stderr>";
 
@@ -501,7 +513,11 @@ pub fn classify(e: Entry) -> Option<ClassifiedMsg> {
                 is_error: !stderr_content.is_empty(),
             }));
         }
-        if trimmed.starts_with(TASK_NOTIFICATION_TAG) {
+        // v2.1.234+: between-turn background-task notifications may arrive fully wrapped in
+        // <system-reminder>, matching mid-turn delivery (issue #262).
+        if trimmed.starts_with(TASK_NOTIFICATION_TAG)
+            || unwrap_system_reminder(trimmed).starts_with(TASK_NOTIFICATION_TAG)
+        {
             let status = RE_TASK_NOTIFY_STATUS
                 .captures(&content_str)
                 .and_then(|c| c.get(1))
@@ -677,6 +693,15 @@ fn is_user_noise(raw: &Option<Value>, content_str: &str) -> bool {
     for tag in HARD_NOISE_TAGS {
         let close_tag = tag.replace('<', "</");
         if trimmed.starts_with(tag) && trimmed.ends_with(&close_tag) {
+            // v2.1.234+: between-turn background-task notifications are now delivered fully
+            // wrapped in <system-reminder>, matching mid-turn delivery (issue #262). A wrapped
+            // <task-notification> still carries a real task completion/failure status and must
+            // surface as a System message, not be swallowed as reminder noise.
+            if *tag == "<system-reminder>"
+                && unwrap_system_reminder(trimmed).starts_with(TASK_NOTIFICATION_TAG)
+            {
+                continue;
+            }
             return true;
         }
     }
@@ -1407,6 +1432,48 @@ mod tests {
         }
     }
 
+    // --- Issue #262: v2.1.234+ between-turn task notifications wrapped in <system-reminder> ---
+
+    #[test]
+    fn classify_task_notification_wrapped_in_system_reminder_is_system_msg() {
+        // v2.1.234+: between-turn background-task notifications are now delivered fully
+        // wrapped in <system-reminder>, matching mid-turn delivery. The wrapper must not
+        // cause the notification to be dropped as reminder noise.
+        let content = "<system-reminder><task-notification><summary>Task done</summary><status>completed</status></task-notification></system-reminder>";
+        let e = make_entry("user", Some(json!(content)));
+        match classify(e) {
+            Some(ClassifiedMsg::System(s)) => {
+                assert!(!s.is_error);
+                assert!(s.output.contains("Task done"), "got: {:?}", s.output);
+            }
+            other => panic!("Expected System for wrapped task-notification, got {other:?}"),
+        }
+    }
+
+    #[test]
+    fn classify_task_notification_wrapped_in_system_reminder_failed_is_error() {
+        let content = "<system-reminder><task-notification><summary>Background command failed</summary><status>failed</status></task-notification></system-reminder>";
+        let e = make_entry("user", Some(json!(content)));
+        match classify(e) {
+            Some(ClassifiedMsg::System(s)) => {
+                assert!(
+                    s.is_error,
+                    "failed status should be an error even when wrapped"
+                );
+            }
+            other => panic!("Expected System with is_error, got {other:?}"),
+        }
+    }
+
+    #[test]
+    fn classify_returns_none_for_plain_system_reminder_still_dropped() {
+        // Regression guard: a genuine reminder (no task-notification inside) wrapped in
+        // <system-reminder> must still be dropped as noise, not surfaced as a System message.
+        let content = "<system-reminder>some reminder</system-reminder>";
+        let e = make_entry("user", Some(json!(content)));
+        assert!(classify(e).is_none());
+    }
+
     // --- Hook event compat tests (v2.1.84+) ---
 
     #[test]