fix(routines): normalize notification summaries with truncation and metadata (#1321)

- Capitalize status labels in notifications (ok -> Completed, attention -> Needs attention)
- Sanitize and truncate long summaries to 500 chars with UTF-8-safe ellipsis
- Include job_id in notification metadata for full-job routines
- Move sanitize_summary/strip_html_tags out of #[cfg(test)] for production use
- Add regression tests for truncation, status labels, job_id metadata

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

src/agent/routine_engine.rs

@@ -715,6 +715,7 @@ impl RoutineEngine {
             status,
             Some(summary),
             thread_id.as_deref(),
+            run.job_id,
         )
         .await;
 
@@ -1218,6 +1219,7 @@ async fn execute_routine(ctx: EngineContext, routine: Routine, run: RoutineRun)
         status,
         summary.as_deref(),
         thread_id.as_deref(),
+        run.job_id,
     )
     .await;
 }
@@ -1827,7 +1829,18 @@ async fn execute_routine_tool(
     Ok(result_str)
 }
 
+/// Human-readable label for a run status, suitable for user-facing notifications.
+fn status_display_label(status: RunStatus) -> &'static str {
+    match status {
+        RunStatus::Ok => "Completed",
+        RunStatus::Attention => "Needs attention",
+        RunStatus::Failed => "Failed",
+        RunStatus::Running => "Running",
+    }
+}
+
 /// Send a notification based on the routine's notify config and run status.
+#[allow(clippy::too_many_arguments)]
 async fn send_notification(
     tx: &mpsc::Sender<OutgoingResponse>,
     notify: &NotifyConfig,
@@ -1836,6 +1849,7 @@ async fn send_notification(
     status: RunStatus,
     summary: Option<&str>,
     thread_id: Option<&str>,
+    job_id: Option<Uuid>,
 ) {
     let should_notify = match status {
         RunStatus::Ok => notify.on_success,
@@ -1855,23 +1869,34 @@ async fn send_notification(
         RunStatus::Running => "⏳",
     };
 
+    let label = status_display_label(status);
+
     let message = match summary {
-        Some(s) => format!("{} *Routine '{}'*: {}\n\n{}", icon, routine_name, status, s),
-        None => format!("{} *Routine '{}'*: {}", icon, routine_name, status),
+        Some(s) => {
+            let sanitized = sanitize_summary(s);
+            format!("{} *Routine '{}'*: {}\n\n{}", icon, routine_name, label, sanitized)
+        }
+        None => format!("{} *Routine '{}'*: {}", icon, routine_name, label),
     };
 
+    let mut metadata = serde_json::json!({
+        "source": "routine",
+        "routine_name": routine_name,
+        "status": status.to_string(),
+        "owner_id": owner_id,
+        "notify_user": notify.user,
+        "notify_channel": notify.channel,
+    });
+
+    if let Some(jid) = job_id {
+        metadata["job_id"] = serde_json::json!(jid.to_string());
+    }
+
     let response = OutgoingResponse {
         content: message,
         thread_id: thread_id.map(String::from),
         attachments: Vec::new(),
-        metadata: serde_json::json!({
-            "source": "routine",
-            "routine_name": routine_name,
-            "status": status.to_string(),
-            "owner_id": owner_id,
-            "notify_user": notify.user,
-            "notify_channel": notify.channel,
-        }),
+        metadata,
     };
 
     if let Err(e) = tx.send(response).await {
@@ -1934,7 +1959,6 @@ fn truncate(s: &str, max: usize) -> String {
 /// 2. Strip HTML tags to prevent injection in web-rendered notifications
 /// 3. Collapse multiple whitespace/newlines to single spaces for cleaner output
 /// 4. Truncate to 500 chars to prevent oversized notifications
-#[cfg(test)]
 fn sanitize_summary(s: &str) -> String {
     // Strip control characters (keep newline for now, collapse later)
     let no_control: String = s
@@ -1962,7 +1986,6 @@ fn sanitize_summary(s: &str) -> String {
 }
 
 /// Remove HTML/XML tags from a string.
-#[cfg(test)]
 fn strip_html_tags(s: &str) -> String {
     let mut result = String::with_capacity(s.len());
     let mut in_tag = false;
@@ -2558,4 +2581,162 @@ mod tests {
         assert!(result.len() <= 503);
         assert!(result.ends_with("..."));
     }
+
+    #[test]
+    fn test_sanitize_summary_truncates_long_text() {
+        use super::sanitize_summary;
+
+        let short = "This is a short summary.";
+        assert_eq!(sanitize_summary(short), short);
+
+        let long = "x".repeat(600);
+        let result = sanitize_summary(&long);
+        assert!(
+            result.len() <= 503,
+            "Truncated summary should be at most 503 bytes (500 + '...')"
+        );
+        assert!(result.ends_with("..."), "Truncated summary should end with ellipsis");
+    }
+
+    #[test]
+    fn test_status_display_label_readable() {
+        use super::status_display_label;
+
+        assert_eq!(status_display_label(RunStatus::Ok), "Completed");
+        assert_eq!(status_display_label(RunStatus::Failed), "Failed");
+        assert_eq!(status_display_label(RunStatus::Attention), "Needs attention");
+        assert_eq!(status_display_label(RunStatus::Running), "Running");
+    }
+
+    #[tokio::test]
+    async fn test_notification_message_uses_readable_status() {
+        use tokio::sync::mpsc;
+
+        let (tx, mut rx) = mpsc::channel(1);
+        let notify = NotifyConfig {
+            on_success: true,
+            on_failure: true,
+            on_attention: true,
+            ..Default::default()
+        };
+
+        super::send_notification(
+            &tx,
+            &notify,
+            "user-1",
+            "my-routine",
+            RunStatus::Ok,
+            Some("All good"),
+            None,
+            None,
+        )
+        .await;
+
+        let msg = rx.recv().await.expect("should receive notification");
+        assert!(
+            msg.content.contains("Completed"),
+            "Notification should use readable label 'Completed', got: {}",
+            msg.content
+        );
+        assert!(
+            !msg.content.contains(": ok"),
+            "Notification should not contain raw lowercase status"
+        );
+    }
+
+    #[tokio::test]
+    async fn test_notification_includes_job_id_in_metadata() {
+        use tokio::sync::mpsc;
+
+        let (tx, mut rx) = mpsc::channel(1);
+        let notify = NotifyConfig {
+            on_failure: true,
+            ..Default::default()
+        };
+        let job_id = uuid::Uuid::new_v4();
+
+        super::send_notification(
+            &tx,
+            &notify,
+            "user-1",
+            "my-routine",
+            RunStatus::Failed,
+            Some("something broke"),
+            None,
+            Some(job_id),
+        )
+        .await;
+
+        let msg = rx.recv().await.expect("should receive notification");
+        let meta_job_id = msg.metadata["job_id"]
+            .as_str()
+            .expect("metadata should contain job_id");
+        assert_eq!(meta_job_id, job_id.to_string());
+    }
+
+    #[tokio::test]
+    async fn test_notification_omits_job_id_when_none() {
+        use tokio::sync::mpsc;
+
+        let (tx, mut rx) = mpsc::channel(1);
+        let notify = NotifyConfig {
+            on_success: true,
+            ..Default::default()
+        };
+
+        super::send_notification(
+            &tx,
+            &notify,
+            "user-1",
+            "my-routine",
+            RunStatus::Ok,
+            Some("done"),
+            None,
+            None,
+        )
+        .await;
+
+        let msg = rx.recv().await.expect("should receive notification");
+        assert!(
+            msg.metadata.get("job_id").is_none(),
+            "metadata should not contain job_id when None"
+        );
+    }
+
+    #[tokio::test]
+    async fn test_notification_truncates_long_summary() {
+        use tokio::sync::mpsc;
+
+        let (tx, mut rx) = mpsc::channel(1);
+        let notify = NotifyConfig {
+            on_failure: true,
+            ..Default::default()
+        };
+
+        let long_summary = "z".repeat(1000);
+        super::send_notification(
+            &tx,
+            &notify,
+            "user-1",
+            "my-routine",
+            RunStatus::Failed,
+            Some(&long_summary),
+            None,
+            None,
+        )
+        .await;
+
+        let msg = rx.recv().await.expect("should receive notification");
+        // The sanitized summary should be truncated to ~500 chars + "..."
+        // The full message includes icon + routine name + label, so just check
+        // it doesn't contain the full 1000-char string.
+        assert!(
+            !msg.content.contains(&long_summary),
+            "Notification should truncate long summaries"
+        );
+        assert!(
+            msg.content.contains("..."),
+            "Truncated notification should contain ellipsis"
+        );
+    }
 }