mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(notifications): clarify morning briefing failures
## Summary - Rewrites failed `morning_briefing` cron alert bodies to a concrete recovery message instead of the generic agent fallback. - Strips `<openhuman-link>` 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 `<openhuman-link>` 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 `<openhuman-link>` 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/tinyhumansai/openhuman/pull/2296?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: aqilaziz <gonzes7@gmail.com> Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
This commit is contained in:
co-authored by
Steven Enamakel
parent
fbc7ef3e16
commit
5a0a5a7e02
@@ -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> = {}): 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 <openhuman-link path="community/discord">Report on Discord</openhuman-link>.'
|
||||
)
|
||||
).toBe('You can Report on Discord.');
|
||||
});
|
||||
});
|
||||
|
||||
describe('NotificationCard', () => {
|
||||
it('does not render raw openhuman-link markup in the body preview', () => {
|
||||
const rendered = render(
|
||||
<NotificationCard
|
||||
notification={notification({
|
||||
body: 'Something went wrong.\n<openhuman-link path="community/discord">Report on Discord</openhuman-link>',
|
||||
})}
|
||||
onMarkRead={vi.fn()}
|
||||
/>
|
||||
);
|
||||
|
||||
expect(screen.getByText(/Report on Discord/)).toBeInTheDocument();
|
||||
expect(rendered.container.textContent).not.toContain('<openhuman-link');
|
||||
expect(rendered.container.textContent).not.toContain('</openhuman-link>');
|
||||
});
|
||||
});
|
||||
@@ -44,6 +44,17 @@ function scoreBadgeClass(score: number): string {
|
||||
return 'bg-sage-500/20 text-green-700 border-green-200';
|
||||
}
|
||||
|
||||
const OPENHUMAN_LINK_TAG =
|
||||
/<openhuman-link\s+path=(?:"[^"]*"|'[^']*')\s*>([\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 }
|
||||
</p>
|
||||
|
||||
{/* Body preview */}
|
||||
{n.body && (
|
||||
{bodyPreview && (
|
||||
<p className="text-xs text-stone-500 dark:text-neutral-400 mt-0.5 line-clamp-2">
|
||||
{n.body}
|
||||
{bodyPreview}
|
||||
</p>
|
||||
)}
|
||||
</button>
|
||||
|
||||
@@ -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.\n<openhuman-link path=\"community/discord\">Report on Discord</openhuman-link>";
|
||||
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 = "<openhuman-link";
|
||||
const CLOSE_TAG: &str = "</openhuman-link>";
|
||||
|
||||
let mut output = String::with_capacity(input.len());
|
||||
let mut rest = input;
|
||||
|
||||
while let Some(start) = rest.find(OPEN_TAG) {
|
||||
output.push_str(&rest[..start]);
|
||||
let tag_and_after = &rest[start..];
|
||||
|
||||
let Some(open_end) = tag_and_after.find('>') 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"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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("<openhuman-link"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cron_alert_body_strips_openhuman_link_markup() {
|
||||
let job = test_job("");
|
||||
let body = cron_alert_body(
|
||||
&job,
|
||||
"Read <openhuman-link path=\"settings/notifications\">notification settings</openhuman-link> before tomorrow.",
|
||||
);
|
||||
|
||||
assert_eq!(body, "Read notification settings before tomorrow.");
|
||||
assert!(!body.contains("<openhuman-link"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn push_cron_alert_deduplicates_repeated_morning_briefing_failures() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let config = test_config(&tmp).await;
|
||||
let mut job = test_job("");
|
||||
job.job_type = JobType::Agent;
|
||||
job.name = Some("morning_briefing".into());
|
||||
job.agent_id = Some("morning_briefing".into());
|
||||
|
||||
push_cron_alert(&config, &job, AGENT_JOB_USER_FAILURE_MESSAGE);
|
||||
push_cron_alert(&config, &job, AGENT_JOB_USER_FAILURE_MESSAGE);
|
||||
|
||||
let items =
|
||||
crate::openhuman::notifications::store::list(&config, 10, 0, Some("cron"), None).unwrap();
|
||||
assert_eq!(items.len(), 1);
|
||||
assert_eq!(items[0].body, MORNING_BRIEFING_FAILURE_NOTIFICATION);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn agent_session_target_tag_matches_expected_values() {
|
||||
assert_eq!(agent_session_target_tag(&SessionTarget::Main), "main");
|
||||
|
||||
Reference in New Issue
Block a user