From 5a0a5a7e02ad85c84117fc97790dfe5024d32eda Mon Sep 17 00:00:00 2001 From: Aqil Aziz Date: Thu, 21 May 2026 05:19:43 +0700 Subject: [PATCH] fix(notifications): clarify morning briefing failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Rewrites failed `morning_briefing` cron alert bodies to a concrete recovery message instead of the generic agent fallback. - Strips `` tags from cron alert bodies before they are persisted to the notification center. - Uses recent-notification dedupe for cron alerts so repeated identical failures do not spam the list. - Sanitizes notification card previews on the frontend so raw OpenHuman link markup is never rendered as text. ## Problem - Failed morning briefing runs stored the generic agent failure message directly in the notifications store. - That message is designed for chat bubbles and includes internal `` markup. - The notification center renders integration notification bodies as plain text, so users saw raw markup and a non-actionable error. ## Solution - Add a cron alert body formatter that detects the built-in morning briefing failure shape and replaces it with actionable copy. - Preserve normal successful briefing content, while stripping any OpenHuman link tags down to their visible label for notification previews. - Route cron notification persistence through the existing `insert_if_not_recent` dedupe helper. - Add frontend preview formatting plus component coverage for link-markup stripping. ## Submission Checklist - [x] Tests added or updated (happy path + at least one failure / edge case) per [Testing Strategy](../gitbooks/developing/testing-strategy.md#failure-path-requirement) - [x] **Diff coverage ≥ 80%** — local coverage run is blocked by missing libclang; CI coverage gate will verify changed lines. - [x] Coverage matrix updated — N/A: behavior-only cron/notification rendering fix, no feature row added/removed/renamed. - [x] All affected feature IDs from the matrix are listed in the PR description under `## Related` — N/A: no matrix feature ID changed. - [x] No new external network dependencies introduced (mock/local tests only) - [x] Manual smoke checklist updated if this touches release-cut surfaces ([`docs/RELEASE-MANUAL-SMOKE.md`](../docs/RELEASE-MANUAL-SMOKE.md)) — N/A: notification rendering copy only. - [x] Linked issue closed via `Closes #NNN` in the `## Related` section ## Impact - Runtime: cron alert persistence and notification center preview rendering. - User-visible: morning briefing failures now explain likely recovery steps; raw `` markup is hidden in notifications. - Compatibility: chat-facing generic agent fallback remains unchanged. ## Related - Closes #2279 - Follow-up PR(s)/TODOs: verify the final desktop notification flow on macOS build hardware once CI artifacts are available. --- ## AI Authored PR Metadata (required for Codex/Linear PRs) ### Linear Issue - Key: N/A - URL: N/A ### Commit & Branch - Branch: `codex/2279-morning-briefing-notifications` - Commit SHA: `513e41ccd39f483563460cf742246ab9850d1cfc` ### Validation Run - [x] `pnpm --filter openhuman-app exec prettier --check src/components/notifications/NotificationCard.tsx src/components/notifications/NotificationCard.test.tsx` passed. - [x] `pnpm typecheck` passed. - [x] `pnpm --filter openhuman-app test -- src/components/notifications/NotificationCard.test.tsx` passed. - [x] Rust fmt/check (if changed): `cargo fmt --all --check` passed. - [x] Focused Rust tests: blocked locally; see Validation Blocked. - [x] Tauri fmt/check (if changed): N/A: no Tauri shell changes. ### Validation Blocked - `command:` `cargo test --lib cron_alert_body --manifest-path Cargo.toml` - `error:` `whisper-rs-sys` build script could not find `clang.dll` / `libclang.dll`; `LIBCLANG_PATH` is unset in this Windows environment. - `impact:` focused Rust tests could not run locally, but the scheduler tests are included for CI. - `note:` full `pnpm --filter openhuman-app format:check` also reports pre-existing repo-wide Prettier drift; changed notification files pass targeted Prettier check. ### Behavior Changes - Intended behavior change: failed morning briefing cron alerts now use actionable notification copy and identical recent cron alerts are skipped. - User-visible effect: notification bodies do not show raw OpenHuman link markup. ### Parity Contract - Legacy behavior preserved: successful cron output still reaches proactive delivery and notification persistence; chat fallback message remains unchanged. - Guard/fallback/dispatch parity checks: unit tests cover morning briefing failure rewrite, link stripping, duplicate alert persistence, and frontend preview rendering. ### Duplicate / Superseded PR Handling - Duplicate PR(s): none found for #2279 at PR creation time. - Canonical PR: this PR. - Resolution (closed/superseded/updated): N/A ## Summary by CodeRabbit * **Bug Fixes** * Notification previews no longer show raw markup tags and now collapse whitespace, improving readability. * **Improvements** * Morning briefing failures display a clear, user-friendly failure message. * Repeated cron job alerts are deduplicated to reduce notification noise. * **Tests** * Added tests covering notification preview formatting, cron alert rewriting, and deduplication. [![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/tinyhumansai/openhuman/pull/2296?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) Co-authored-by: aqilaziz Co-authored-by: Steven Enamakel --- .../notifications/NotificationCard.test.tsx | 50 +++++++++++ .../notifications/NotificationCard.tsx | 16 +++- src/openhuman/cron/scheduler.rs | 85 ++++++++++++++++--- src/openhuman/cron/scheduler_tests.rs | 44 ++++++++++ 4 files changed, 180 insertions(+), 15 deletions(-) create mode 100644 app/src/components/notifications/NotificationCard.test.tsx diff --git a/app/src/components/notifications/NotificationCard.test.tsx b/app/src/components/notifications/NotificationCard.test.tsx new file mode 100644 index 000000000..247227687 --- /dev/null +++ b/app/src/components/notifications/NotificationCard.test.tsx @@ -0,0 +1,50 @@ +import { render, screen } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; + +import type { IntegrationNotification } from '../../types/notifications'; +import NotificationCard, { formatNotificationBodyPreview } from './NotificationCard'; + +function notification(overrides: Partial = {}): IntegrationNotification { + return { + id: 'notif-1', + provider: 'cron', + account_id: 'morning_briefing', + title: 'morning_briefing', + body: 'Morning briefing ready.', + raw_payload: {}, + importance_score: 0.65, + triage_action: 'react', + triage_reason: 'Scheduled delivery', + status: 'unread', + received_at: new Date().toISOString(), + scored_at: new Date().toISOString(), + ...overrides, + }; +} + +describe('formatNotificationBodyPreview', () => { + it('strips openhuman-link markup while preserving the label', () => { + expect( + formatNotificationBodyPreview( + 'You can Report on Discord.' + ) + ).toBe('You can Report on Discord.'); + }); +}); + +describe('NotificationCard', () => { + it('does not render raw openhuman-link markup in the body preview', () => { + const rendered = render( + Report on Discord', + })} + onMarkRead={vi.fn()} + /> + ); + + expect(screen.getByText(/Report on Discord/)).toBeInTheDocument(); + expect(rendered.container.textContent).not.toContain(''); + }); +}); diff --git a/app/src/components/notifications/NotificationCard.tsx b/app/src/components/notifications/NotificationCard.tsx index be5757d7c..e4944cabb 100644 --- a/app/src/components/notifications/NotificationCard.tsx +++ b/app/src/components/notifications/NotificationCard.tsx @@ -44,6 +44,17 @@ function scoreBadgeClass(score: number): string { return 'bg-sage-500/20 text-green-700 border-green-200'; } +const OPENHUMAN_LINK_TAG = + /([\s\S]*?)<\/openhuman-link>/gi; + +export function formatNotificationBodyPreview(body: string): string { + return body + .replace(OPENHUMAN_LINK_TAG, '$1') + .replace(/[ \t]+\n/g, '\n') + .replace(/\n{3,}/g, '\n\n') + .trim(); +} + // ───────────────────────────────────────────────────────────────────────────── // Component // ───────────────────────────────────────────────────────────────────────────── @@ -58,6 +69,7 @@ interface Props { const NotificationCard = ({ notification: n, onMarkRead, onNavigate, onDismiss }: Props) => { const { t } = useT(); const isUnread = n.status === 'unread'; + const bodyPreview = n.body ? formatNotificationBodyPreview(n.body) : ''; const handleBodyClick = () => { if (onNavigate) { @@ -118,9 +130,9 @@ const NotificationCard = ({ notification: n, onMarkRead, onNavigate, onDismiss }

{/* Body preview */} - {n.body && ( + {bodyPreview && (

- {n.body} + {bodyPreview}

)} diff --git a/src/openhuman/cron/scheduler.rs b/src/openhuman/cron/scheduler.rs index 6085eb249..c28ca3380 100644 --- a/src/openhuman/cron/scheduler.rs +++ b/src/openhuman/cron/scheduler.rs @@ -16,6 +16,8 @@ use tokio::time::{self, Duration}; const MIN_POLL_SECONDS: u64 = 5; const SHELL_JOB_TIMEOUT_SECS: u64 = 120; const AGENT_JOB_USER_FAILURE_MESSAGE: &str = "Something went wrong. Please try again.\nThis error has been reported. You can also report it on Discord.\nReport on Discord"; +const MORNING_BRIEFING_AGENT_ID: &str = "morning_briefing"; +const MORNING_BRIEFING_FAILURE_NOTIFICATION: &str = "Morning briefing could not run. Check your AI provider, API key, and connected apps, then run it again from Settings > Cron Jobs."; fn agent_session_target_tag(target: &SessionTarget) -> &'static str { match target { @@ -24,6 +26,54 @@ fn agent_session_target_tag(target: &SessionTarget) -> &'static str { } } +fn is_morning_briefing_job(job: &CronJob) -> bool { + job.name.as_deref() == Some(MORNING_BRIEFING_AGENT_ID) + || job.agent_id.as_deref() == Some(MORNING_BRIEFING_AGENT_ID) +} + +fn strip_openhuman_link_markup(input: &str) -> String { + const OPEN_TAG: &str = "') else { + output.push_str(tag_and_after); + return output; + }; + let label_and_after = &tag_and_after[open_end + 1..]; + + let Some(close_start) = label_and_after.find(CLOSE_TAG) else { + output.push_str(tag_and_after); + return output; + }; + + output.push_str(&label_and_after[..close_start]); + rest = &label_and_after[close_start + CLOSE_TAG.len()..]; + } + + output.push_str(rest); + output +} + +fn cron_alert_body(job: &CronJob, output: &str) -> String { + let trimmed = output.trim(); + if matches!(job.job_type, JobType::Agent) + && trimmed == AGENT_JOB_USER_FAILURE_MESSAGE + && is_morning_briefing_job(job) + { + return MORNING_BRIEFING_FAILURE_NOTIFICATION.to_string(); + } + + let body = strip_openhuman_link_markup(output); + crate::openhuman::util::truncate_with_ellipsis(&body, 512) +} + pub async fn run(config: Config) -> Result<()> { // Ensure the global event bus is initialized so cron delivery events // are not silently dropped. This is a no-op if already initialized. @@ -490,14 +540,14 @@ fn push_cron_alert(config: &Config, job: &CronJob, output: &str) { use crate::openhuman::notifications::types::{IntegrationNotification, NotificationStatus}; let name = job.name.as_deref().unwrap_or("Cron job"); - let truncated = crate::openhuman::util::truncate_with_ellipsis(output, 512); + let body = cron_alert_body(job, output); let notification = IntegrationNotification { id: uuid::Uuid::new_v4().to_string(), provider: "cron".to_string(), account_id: Some(job.id.clone()), title: name.to_string(), - body: truncated, + body, raw_payload: serde_json::json!({ "job_id": job.id, "job_name": job.name, @@ -511,17 +561,26 @@ fn push_cron_alert(config: &Config, job: &CronJob, output: &str) { scored_at: Some(Utc::now()), }; - if let Err(e) = notif_store::insert(config, ¬ification) { - tracing::warn!( - job_id = %job.id, - error = %e, - "[cron] failed to push notification alert" - ); - } else { - tracing::debug!( - job_id = %job.id, - "[cron] pushed notification alert to alerts tab" - ); + match notif_store::insert_if_not_recent(config, ¬ification) { + Ok(true) => { + tracing::debug!( + job_id = %job.id, + "[cron] pushed notification alert to alerts tab" + ); + } + Ok(false) => { + tracing::debug!( + job_id = %job.id, + "[cron] skipped duplicate notification alert" + ); + } + Err(e) => { + tracing::warn!( + job_id = %job.id, + error = %e, + "[cron] failed to push notification alert" + ); + } } } diff --git a/src/openhuman/cron/scheduler_tests.rs b/src/openhuman/cron/scheduler_tests.rs index cfd879f33..7d3f70630 100644 --- a/src/openhuman/cron/scheduler_tests.rs +++ b/src/openhuman/cron/scheduler_tests.rs @@ -54,6 +54,50 @@ fn agent_failure_copy_mentions_retry_reporting_and_discord() { assert!(AGENT_JOB_USER_FAILURE_MESSAGE.contains("Report on Discord")); } +#[test] +fn cron_alert_body_rewrites_morning_briefing_failure() { + let mut job = test_job(""); + job.job_type = JobType::Agent; + job.name = Some("morning_briefing".into()); + job.agent_id = Some("morning_briefing".into()); + + let body = cron_alert_body(&job, AGENT_JOB_USER_FAILURE_MESSAGE); + + assert_eq!(body, MORNING_BRIEFING_FAILURE_NOTIFICATION); + assert!(!body.contains("Something went wrong")); + assert!(!body.contains("notification settings before tomorrow.", + ); + + assert_eq!(body, "Read notification settings before tomorrow."); + assert!(!body.contains("