fix(cron): seed morning briefing disabled by default (opt-in) (#4028)

src/openhuman/cron/seed.rs

@@ -1,7 +1,8 @@
 //! Seed default proactive agent cron jobs.
 //!
 //! Called once after onboarding completes to create:
-//! - A recurring daily morning briefing job (7 AM, user's local time or UTC)
+//! - A recurring daily morning briefing job (7 AM, user's local time or UTC),
+//!   seeded disabled until the user opts in
 //!
 //! The morning briefing uses `mode: "proactive"` delivery so the
 //! channels module's
@@ -73,7 +74,7 @@ pub fn seed_proactive_agents(config: &Config) -> Result<()> {
     prune_legacy_welcome(config, &existing);
 
     if !has(MORNING_BRIEFING_JOB_NAME) {
-        tracing::info!("[cron::seed] creating morning_briefing daily cron job");
+        tracing::info!("[cron::seed] creating morning_briefing daily cron job (disabled — opt-in)");
         seed_morning_briefing(config)?;
     } else {
         tracing::debug!("[cron::seed] morning_briefing job already exists — skipping");
@@ -164,7 +165,12 @@ fn prune_legacy_welcome(config: &Config, existing: &[crate::openhuman::cron::Cro
 /// (unless a timezone is later set explicitly).
 /// The cron expression `0 7 * * *` fires once per day. Users can later
 /// adjust the schedule or time zone via `cron.update_job`.
+///
+/// Created disabled in a single insert. The briefing is a full proactive agent
+/// turn, so it must not start billing inference until the user explicitly
+/// enables it from Settings/Routines (`cron.update_job → enabled=true`).
 fn seed_morning_briefing(config: &Config) -> Result<()> {
+    tracing::debug!("[cron::seed] seed_morning_briefing start");
     let schedule = Schedule::Cron {
         expr: "0 7 * * *".to_string(),
         tz: None,
@@ -178,7 +184,7 @@ fn seed_morning_briefing(config: &Config) -> Result<()> {
         "efficient briefing they can scan in 30 seconds over coffee."
     );
 
-    add_agent_job_with_definition(
+    let job = add_agent_job_with_definition(
         config,
         Some(MORNING_BRIEFING_JOB_NAME.to_string()),
         schedule,
@@ -188,9 +194,14 @@ fn seed_morning_briefing(config: &Config) -> Result<()> {
         Some(proactive_delivery()),
         false, // recurring — do not delete after run
         Some(MORNING_BRIEFING_JOB_NAME.to_string()),
-        true, // enabled
+        false, // enabled=false — opt-in, created disabled atomically
     )?;
 
+    tracing::debug!(
+        job_id = %job.id,
+        enabled = job.enabled,
+        "[cron::seed] seed_morning_briefing done — created disabled (opt-in)"
+    );
     Ok(())
 }
 
@@ -328,6 +339,54 @@ mod tests {
         );
     }
 
+    #[test]
+    fn seeds_morning_briefing_disabled_and_idempotent() {
+        let tmp = TempDir::new().unwrap();
+        let config = test_config(&tmp);
+
+        seed_proactive_agents(&config).expect("first seed");
+        let jobs = list_jobs(&config).unwrap();
+        assert!(
+            jobs.iter()
+                .filter(|j| matches!(j.job_type, crate::openhuman::cron::JobType::Agent))
+                .all(|j| !j.enabled),
+            "fresh onboarding seed must not create enabled billable agent cron jobs: {jobs:?}"
+        );
+        let briefings: Vec<_> = jobs
+            .iter()
+            .filter(|j| j.name.as_deref() == Some(MORNING_BRIEFING_JOB_NAME))
+            .collect();
+        assert_eq!(
+            briefings.len(),
+            1,
+            "exactly one morning_briefing job, got {briefings:?}"
+        );
+        let briefing = briefings[0];
+        assert!(
+            !briefing.enabled,
+            "morning_briefing must be seeded disabled until explicit opt-in"
+        );
+        assert_eq!(
+            briefing.agent_id.as_deref(),
+            Some(MORNING_BRIEFING_JOB_NAME)
+        );
+        assert!(matches!(
+            briefing.schedule,
+            Schedule::Cron { ref expr, .. } if expr == "0 7 * * *"
+        ));
+
+        seed_proactive_agents(&config).expect("second seed");
+        let after = list_jobs(&config).unwrap();
+        assert_eq!(
+            after
+                .iter()
+                .filter(|j| j.name.as_deref() == Some(MORNING_BRIEFING_JOB_NAME))
+                .count(),
+            1,
+            "second seed must not duplicate the morning_briefing job"
+        );
+    }
+
     #[test]
     fn boot_seed_is_noop_until_onboarded() {
         let tmp = TempDir::new().unwrap();

src/openhuman/cron/store.rs

@@ -304,6 +304,7 @@ pub fn due_jobs(config: &Config, now: DateTime<Utc>) -> Result<Vec<CronJob>> {
 
 pub fn update_job(config: &Config, job_id: &str, patch: CronJobPatch) -> Result<CronJob> {
     let mut job = get_job(config, job_id)?;
+    let was_enabled = job.enabled;
     let mut schedule_changed = false;
 
     if let Some(schedule) = patch.schedule {
@@ -342,6 +343,24 @@ pub fn update_job(config: &Config, job_id: &str, patch: CronJobPatch) -> Result<
 
     if schedule_changed {
         job.next_run = next_run_for_schedule(&job.schedule, Utc::now())?;
+    } else if job.enabled && !was_enabled {
+        // Disabled→enabled transition (e.g. opting into a seeded morning
+        // briefing). A job that sat disabled past its originally computed
+        // next_run would otherwise fire immediately on opt-in, because the
+        // scheduler selects `enabled = 1 AND next_run <= now`. Refresh a stale
+        // next_run so the first run lands on the next scheduled occurrence
+        // rather than firing the instant the user flips the switch.
+        let now = Utc::now();
+        if job.next_run <= now {
+            let refreshed = next_run_for_schedule(&job.schedule, now)?;
+            tracing::debug!(
+                job_id = %job.id,
+                stale_next_run = %job.next_run.to_rfc3339(),
+                next_run = %refreshed.to_rfc3339(),
+                "[cron::update_job] refreshed stale next_run on disabled→enabled transition"
+            );
+            job.next_run = refreshed;
+        }
     }
 
     with_connection(config, |conn| {

src/openhuman/cron/store_tests.rs

@@ -103,6 +103,102 @@ fn due_jobs_filters_by_timestamp_and_enabled() {
     assert!(due_after_disable.is_empty());
 }
 
+#[test]
+fn enabling_stale_disabled_job_refreshes_next_run() {
+    let tmp = TempDir::new().unwrap();
+    let config = test_config(&tmp);
+
+    // Daily 7 AM job, then disable it (mimics a seeded opt-in morning briefing).
+    let job = add_job(&config, "0 7 * * *", "echo briefing").unwrap();
+    update_job(
+        &config,
+        &job.id,
+        CronJobPatch {
+            enabled: Some(false),
+            ..CronJobPatch::default()
+        },
+    )
+    .unwrap();
+
+    // Force a stale next_run in the past, as if the user onboarded before the
+    // job's first scheduled fire and only opted in later (hours or days after).
+    let stale = Utc::now() - ChronoDuration::hours(2);
+    with_connection(&config, |conn| {
+        conn.execute(
+            "UPDATE cron_jobs SET next_run = ?1 WHERE id = ?2",
+            params![stale.to_rfc3339(), job.id],
+        )?;
+        Ok(())
+    })
+    .unwrap();
+
+    // Opt in: disabled -> enabled, with the schedule unchanged.
+    let enabled = update_job(
+        &config,
+        &job.id,
+        CronJobPatch {
+            enabled: Some(true),
+            ..CronJobPatch::default()
+        },
+    )
+    .unwrap();
+
+    assert!(enabled.enabled);
+    assert!(
+        enabled.next_run > Utc::now(),
+        "enabling a job with a stale next_run must refresh it to the future, got {}",
+        enabled.next_run
+    );
+    assert!(
+        due_jobs(&config, Utc::now()).unwrap().is_empty(),
+        "freshly opted-in job must not fire immediately on enable"
+    );
+}
+
+#[test]
+fn enabling_job_with_future_next_run_preserves_it() {
+    let tmp = TempDir::new().unwrap();
+    let config = test_config(&tmp);
+
+    let job = add_job(&config, "0 7 * * *", "echo briefing").unwrap();
+    update_job(
+        &config,
+        &job.id,
+        CronJobPatch {
+            enabled: Some(false),
+            ..CronJobPatch::default()
+        },
+    )
+    .unwrap();
+
+    // A future next_run is still valid and must be left untouched on enable.
+    let future = Utc::now() + ChronoDuration::hours(3);
+    with_connection(&config, |conn| {
+        conn.execute(
+            "UPDATE cron_jobs SET next_run = ?1 WHERE id = ?2",
+            params![future.to_rfc3339(), job.id],
+        )?;
+        Ok(())
+    })
+    .unwrap();
+
+    let enabled = update_job(
+        &config,
+        &job.id,
+        CronJobPatch {
+            enabled: Some(true),
+            ..CronJobPatch::default()
+        },
+    )
+    .unwrap();
+
+    assert_eq!(
+        enabled.next_run.to_rfc3339(),
+        future.to_rfc3339(),
+        "enabling a job whose next_run is in the future must not reschedule it"
+    );
+}
+
 #[test]
 fn due_jobs_respects_scheduler_max_tasks_limit() {
     let tmp = TempDir::new().unwrap();