//! Core cron scheduling engine.
//!
//! `CronScheduler` manages a persistent SQLite job store, runs a background
//! tick loop, and fires agents whose `next_run` has arrived. It follows the
//! same `Notify`-based shutdown pattern used by `DefaultAgentScheduler`.
//!
//! v1.0.0 enhancements:
//! - Per-job concurrency guards (`max_concurrent`)
//! - Random jitter to prevent thundering-herd
//! - Dead-letter queue for jobs exceeding `max_retries`
//! - Session isolation via `HeartbeatContextMode`
//! - AgentPin identity verification before each run
//! - Comprehensive `CronMetrics` for observability

use std::collections::HashMap;
use std::str::FromStr;
use std::sync::Arc;
use std::time::Duration;

use chrono::{DateTime, Utc};
use cron::Schedule;
use parking_lot::RwLock;
use rand::Rng;
use tokio::sync::Notify;
use tokio::time::interval;

use super::cron_types::*;
use super::job_store::{JobStore, JobStoreError, SqliteJobStore};
use super::{AgentScheduler, DefaultAgentScheduler};
use crate::types::ExecutionMode;

/// Configuration for the CronScheduler.
#[derive(Debug, Clone)]
pub struct CronSchedulerConfig {
    /// How often the scheduler checks for due jobs.
    pub tick_interval: Duration,
    /// Global cap on concurrent cron-triggered agent runs.
    pub max_concurrent_cron_jobs: usize,
    /// Path to the SQLite job store. `None` = default path.
    pub job_store_path: Option<std::path::PathBuf>,
    /// Whether to catch up on runs that were missed while the process was down.
    pub enable_missed_run_catchup: bool,
}

impl Default for CronSchedulerConfig {
    fn default() -> Self {
        Self {
            tick_interval: Duration::from_secs(1),
            max_concurrent_cron_jobs: 100,
            job_store_path: None,
            enable_missed_run_catchup: true,
        }
    }
}

/// Errors produced by the CronScheduler.
#[derive(Debug, thiserror::Error)]
pub enum CronSchedulerError {
    #[error("invalid cron expression (expected 6-field: sec min hour day month weekday): {0}")]
    InvalidCron(String),
    #[error("invalid timezone: {0}")]
    InvalidTimezone(String),
    #[error("job store error: {0}")]
    Store(#[from] JobStoreError),
    #[error("scheduler error: {0}")]
    Scheduler(String),
    #[error("job not found: {0}")]
    NotFound(CronJobId),
    #[error("identity verification failed for job {0}: {1}")]
    IdentityVerificationFailed(CronJobId, String),
}

/// Live metrics for the cron scheduler (thread-safe counters).
#[derive(Debug, Clone, Default)]
pub struct CronMetrics {
    pub jobs_total: u64,
    pub jobs_active: u64,
    pub jobs_paused: u64,
    pub jobs_dead_letter: u64,
    pub runs_total: u64,
    pub runs_succeeded: u64,
    pub runs_failed: u64,
    pub runs_skipped_concurrency: u64,
    pub runs_skipped_identity: u64,
    pub average_execution_time_ms: f64,
    pub longest_run_ms: u64,
}

/// The core cron scheduling engine.
pub struct CronScheduler {
    store: Arc<SqliteJobStore>,
    agent_scheduler: Arc<DefaultAgentScheduler>,
    config: CronSchedulerConfig,
    shutdown_notify: Arc<Notify>,
    is_running: Arc<RwLock<bool>>,
    active_runs: Arc<RwLock<usize>>,
    /// Per-job active run counters for concurrency limiting.
    per_job_active: Arc<RwLock<HashMap<CronJobId, usize>>>,
    /// Observable metrics snapshot (updated each tick).
    metrics: Arc<RwLock<CronMetrics>>,
    /// Optional AgentPin verifier. When present, every cron-triggered
    /// run verifies `job.agentpin_jwt` before invoking the agent scheduler.
    /// When the verifier is present but the job carries no JWT, the run
    /// is refused so "AgentPin enabled at the scheduler" cannot be
    /// bypassed by leaving the JWT column null.
    agentpin_verifier: Option<Arc<dyn crate::integrations::AgentPinVerifier>>,
}

impl CronScheduler {
    /// Create and start a new CronScheduler.
    pub async fn new(
        config: CronSchedulerConfig,
        agent_scheduler: Arc<DefaultAgentScheduler>,
    ) -> Result<Self, CronSchedulerError> {
        let path = config
            .job_store_path
            .clone()
            .unwrap_or_else(SqliteJobStore::default_path);
        let store = Arc::new(SqliteJobStore::open(&path)?);

        let scheduler = Self {
            store,
            agent_scheduler,
            config,
            shutdown_notify: Arc::new(Notify::new()),
            is_running: Arc::new(RwLock::new(true)),
            active_runs: Arc::new(RwLock::new(0)),
            per_job_active: Arc::new(RwLock::new(HashMap::new())),
            metrics: Arc::new(RwLock::new(CronMetrics::default())),
            agentpin_verifier: None,
        };

        scheduler.start_tick_loop();
        Ok(scheduler)
    }

    /// Install an AgentPin verifier. After this call, every cron run
    /// requires `job.agentpin_jwt` to verify successfully — jobs without
    /// a JWT are refused (see AP-3). Returns the scheduler builder-style
    /// for chaining.
    pub fn with_agentpin_verifier(
        mut self,
        verifier: Arc<dyn crate::integrations::AgentPinVerifier>,
    ) -> Self {
        self.agentpin_verifier = Some(verifier);
        self
    }

    /// Verify that a job's AgentPin credential is valid before firing.
    ///
    /// Returns `Ok(())` when no verifier is configured. When a verifier
    /// IS configured, both absence of `agentpin_jwt` and verification
    /// failure produce an `IdentityVerificationFailed` error and the run
    /// is skipped (and counted in `runs_skipped_identity`).
    async fn verify_job_credential(
        &self,
        job: &CronJobDefinition,
    ) -> Result<(), CronSchedulerError> {
        let Some(ref verifier) = self.agentpin_verifier else {
            return Ok(());
        };
        let jwt = job.agentpin_jwt.as_deref().ok_or_else(|| {
            CronSchedulerError::IdentityVerificationFailed(
                job.job_id,
                "AgentPin verification is enabled but job has no agentpin_jwt".to_string(),
            )
        })?;
        let result = verifier.verify_credential(jwt).await.map_err(|e| {
            CronSchedulerError::IdentityVerificationFailed(job.job_id, e.to_string())
        })?;
        if !result.valid {
            return Err(CronSchedulerError::IdentityVerificationFailed(
                job.job_id,
                result
                    .error_message
                    .unwrap_or_else(|| "invalid credential".to_string()),
            ));
        }
        // The job's agent_config.id must be covered by the JWT's subject.
        // This prevents a JWT for one agent from being reused to run a
        // cron job that claims a different agent identity.
        let expected = job.agent_config.id.0.to_string();
        let sub_matches = result
            .agent_id
            .as_deref()
            .map(|sub| sub == expected)
            .unwrap_or(false);
        if !sub_matches {
            return Err(CronSchedulerError::IdentityVerificationFailed(
                job.job_id,
                format!(
                    "JWT subject {:?} does not match job agent {}",
                    result.agent_id, expected
                ),
            ));
        }
        Ok(())
    }

    /// Create a CronScheduler with an in-memory store (for tests).
    #[cfg(test)]
    pub async fn new_in_memory(
        config: CronSchedulerConfig,
        agent_scheduler: Arc<DefaultAgentScheduler>,
    ) -> Result<Self, CronSchedulerError> {
        let store = Arc::new(SqliteJobStore::open_in_memory()?);
        let scheduler = Self {
            store,
            agent_scheduler,
            config,
            shutdown_notify: Arc::new(Notify::new()),
            is_running: Arc::new(RwLock::new(true)),
            active_runs: Arc::new(RwLock::new(0)),
            per_job_active: Arc::new(RwLock::new(HashMap::new())),
            metrics: Arc::new(RwLock::new(CronMetrics::default())),
            agentpin_verifier: None,
        };
        scheduler.start_tick_loop();
        Ok(scheduler)
    }

    // ── Public API ────────────────────────────────────────────────────

    /// Register a new cron job. Returns the assigned `CronJobId`.
    pub async fn add_job(
        &self,
        mut job: CronJobDefinition,
    ) -> Result<CronJobId, CronSchedulerError> {
        // Validate cron expression.
        Self::parse_cron(&job.cron_expression)?;
        // Validate timezone.
        Self::validate_timezone(&job.timezone)?;
        // Compute first next_run.
        job.next_run = self.compute_next_run(&job.cron_expression, &job.timezone, None)?;
        self.store.save_job(&job).await?;
        tracing::info!(
            "Added cron job {} ({}) — next run: {:?}",
            job.job_id,
            job.name,
            job.next_run
        );
        Ok(job.job_id)
    }

    /// Remove a cron job.
    pub async fn remove_job(&self, job_id: CronJobId) -> Result<(), CronSchedulerError> {
        if !self.store.delete_job(job_id).await? {
            return Err(CronSchedulerError::NotFound(job_id));
        }
        tracing::info!("Removed cron job {}", job_id);
        Ok(())
    }

    /// Pause a cron job (keeps it in the store but stops firing).
    pub async fn pause_job(&self, job_id: CronJobId) -> Result<(), CronSchedulerError> {
        let mut job = self
            .store
            .get_job(job_id)
            .await?
            .ok_or(CronSchedulerError::NotFound(job_id))?;
        job.status = CronJobStatus::Paused;
        job.enabled = false;
        job.updated_at = Utc::now();
        self.store.save_job(&job).await?;
        tracing::info!("Paused cron job {}", job_id);
        Ok(())
    }

    /// Resume a paused or dead-lettered cron job.
    pub async fn resume_job(&self, job_id: CronJobId) -> Result<(), CronSchedulerError> {
        let mut job = self
            .store
            .get_job(job_id)
            .await?
            .ok_or(CronSchedulerError::NotFound(job_id))?;
        job.status = CronJobStatus::Active;
        job.enabled = true;
        job.failure_count = 0; // Reset on resume
        job.next_run = self.compute_next_run(&job.cron_expression, &job.timezone, None)?;
        job.updated_at = Utc::now();
        self.store.save_job(&job).await?;
        tracing::info!("Resumed cron job {} — next run: {:?}", job_id, job.next_run);
        Ok(())
    }

    /// Update a job definition in-place.
    pub async fn update_job(&self, mut job: CronJobDefinition) -> Result<(), CronSchedulerError> {
        Self::parse_cron(&job.cron_expression)?;
        Self::validate_timezone(&job.timezone)?;
        job.next_run = self.compute_next_run(&job.cron_expression, &job.timezone, None)?;
        job.updated_at = Utc::now();
        self.store.save_job(&job).await?;
        Ok(())
    }

    /// Get a single job.
    pub async fn get_job(
        &self,
        job_id: CronJobId,
    ) -> Result<CronJobDefinition, CronSchedulerError> {
        self.store
            .get_job(job_id)
            .await?
            .ok_or(CronSchedulerError::NotFound(job_id))
    }

    /// List all jobs.
    pub async fn list_jobs(&self) -> Result<Vec<CronJobDefinition>, CronSchedulerError> {
        Ok(self.store.list_jobs(None).await?)
    }

    /// Compute the next N fire times for a job.
    pub fn get_next_runs(
        &self,
        cron_expression: &str,
        timezone: &str,
        count: usize,
    ) -> Result<Vec<DateTime<Utc>>, CronSchedulerError> {
        let schedule = Self::parse_cron(cron_expression)?;
        let tz: chrono_tz::Tz = timezone
            .parse()
            .map_err(|_| CronSchedulerError::InvalidTimezone(timezone.to_string()))?;
        let now = Utc::now().with_timezone(&tz);
        let runs: Vec<DateTime<Utc>> = schedule
            .after(&now)
            .take(count)
            .map(|dt| dt.with_timezone(&Utc))
            .collect();
        Ok(runs)
    }

    /// Force-trigger a job immediately, regardless of its schedule.
    pub async fn trigger_now(&self, job_id: CronJobId) -> Result<(), CronSchedulerError> {
        let job = self
            .store
            .get_job(job_id)
            .await?
            .ok_or(CronSchedulerError::NotFound(job_id))?;
        tracing::info!("Force-triggering cron job {} ({})", job_id, job.name);

        // AgentPin identity check BEFORE scheduling. If verification fails we
        // do not invoke the agent scheduler and we still persist a failed
        // run record so the operator can see the identity-rejection in
        // history.
        if let Err(verify_err) = self.verify_job_credential(&job).await {
            tracing::warn!(
                "Refusing to trigger job {} ({}): {}",
                job_id,
                job.name,
                verify_err
            );
            {
                let mut m = self.metrics.write();
                m.runs_skipped_identity = m.runs_skipped_identity.saturating_add(1);
            }
            let record = JobRunRecord {
                run_id: uuid::Uuid::new_v4(),
                job_id,
                agent_id: job.agent_config.id,
                started_at: Utc::now(),
                completed_at: Some(Utc::now()),
                status: JobRunStatus::Failed,
                error: Some(verify_err.to_string()),
                execution_time_ms: Some(0),
            };
            let _ = self.store.save_run_record(&record).await;
            return Err(verify_err);
        }

        let mut run_config = job.agent_config.clone();
        run_config.execution_mode = ExecutionMode::CronScheduled {
            cron_expression: job.cron_expression.clone(),
            timezone: job.timezone.clone(),
        };
        run_config
            .metadata
            .insert("trigger".to_string(), "cron_manual".to_string());
        run_config
            .metadata
            .insert("cron_job_id".to_string(), job.job_id.to_string());
        // Session isolation metadata
        run_config
            .metadata
            .insert("session_id".to_string(), uuid::Uuid::new_v4().to_string());
        run_config.metadata.insert(
            "session_mode".to_string(),
            format!("{:?}", job.session_mode),
        );

        let started_at = Utc::now();
        let run_id = uuid::Uuid::new_v4();
        let agent_id = job.agent_config.id;

        let result = self.agent_scheduler.schedule_agent(run_config).await;

        let (status, error) = match &result {
            Ok(_) => (JobRunStatus::Succeeded, None),
            Err(e) => (JobRunStatus::Failed, Some(e.to_string())),
        };
        let completed_at = Utc::now();
        let exec_ms = (completed_at - started_at).num_milliseconds().max(0) as u64;

        let record = JobRunRecord {
            run_id,
            job_id: job.job_id,
            agent_id,
            started_at,
            completed_at: Some(completed_at),
            status,
            error,
            execution_time_ms: Some(exec_ms),
        };
        self.store.save_run_record(&record).await?;

        result
            .map(|_| ())
            .map_err(|e| CronSchedulerError::Scheduler(e.to_string()))
    }

    /// Graceful shutdown — stops the tick loop.
    pub async fn shutdown(&self) {
        {
            let is_running = self.is_running.read();
            if !*is_running {
                return;
            }
        }
        *self.is_running.write() = false;
        self.shutdown_notify.notify_waiters();
        tracing::info!("CronScheduler shutdown complete");
    }

    /// Get run history for a specific job.
    pub async fn get_run_history(
        &self,
        job_id: CronJobId,
        limit: usize,
    ) -> Result<Vec<JobRunRecord>, CronSchedulerError> {
        Ok(self.store.get_run_history(job_id, limit).await?)
    }

    /// Return a snapshot of current metrics.
    pub fn metrics(&self) -> CronMetrics {
        self.metrics.read().clone()
    }

    /// Check whether the store is accessible and return health info.
    pub async fn check_health(&self) -> Result<CronSchedulerHealth, CronSchedulerError> {
        // Probe the store with a cheap query.
        let jobs = self.store.list_jobs(None).await?;
        let active = jobs
            .iter()
            .filter(|j| j.status == CronJobStatus::Active)
            .count();
        let paused = jobs
            .iter()
            .filter(|j| j.status == CronJobStatus::Paused)
            .count();
        let dead = jobs
            .iter()
            .filter(|j| j.status == CronJobStatus::DeadLetter)
            .count();

        Ok(CronSchedulerHealth {
            is_running: *self.is_running.read(),
            store_accessible: true,
            jobs_total: jobs.len(),
            jobs_active: active,
            jobs_paused: paused,
            jobs_dead_letter: dead,
            global_active_runs: *self.active_runs.read(),
            max_concurrent: self.config.max_concurrent_cron_jobs,
        })
    }

    // ── Internals ─────────────────────────────────────────────────────

    fn start_tick_loop(&self) {
        let store = self.store.clone();
        let agent_scheduler = self.agent_scheduler.clone();
        let shutdown = self.shutdown_notify.clone();
        let is_running = self.is_running.clone();
        let tick = self.config.tick_interval;
        let max_concurrent = self.config.max_concurrent_cron_jobs;
        let active_runs = self.active_runs.clone();
        let per_job_active = self.per_job_active.clone();
        let metrics = self.metrics.clone();
        let agentpin_verifier = self.agentpin_verifier.clone();

        tokio::spawn(async move {
            let mut ticker = interval(tick);

            loop {
                tokio::select! {
                    _ = ticker.tick() => {
                        if !*is_running.read() {
                            break;
                        }

                        let current_active = *active_runs.read();
                        if current_active >= max_concurrent {
                            tracing::debug!("CronScheduler: at capacity ({}/{}), skipping tick",
                                current_active, max_concurrent);
                            continue;
                        }

                        let now = Utc::now();
                        let due_jobs = match store.get_due_jobs(now).await {
                            Ok(jobs) => jobs,
                            Err(e) => {
                                tracing::error!("CronScheduler: failed to query due jobs: {}", e);
                                continue;
                            }
                        };

                        for job in due_jobs {
                            let remaining = max_concurrent.saturating_sub(*active_runs.read());
                            if remaining == 0 {
                                break;
                            }

                            // ── Per-job concurrency guard ─────────────
                            {
                                let pja = per_job_active.read();
                                let job_active = pja.get(&job.job_id).copied().unwrap_or(0);
                                if job_active >= job.max_concurrent as usize {
                                    tracing::debug!(
                                        "CronScheduler: job {} at per-job concurrency limit ({}/{}), skipping",
                                        job.job_id, job_active, job.max_concurrent
                                    );
                                    metrics.write().runs_skipped_concurrency += 1;
                                    continue;
                                }
                            }

                            // ── Compute next run (DST-aware via chrono-tz) ──
                            let next_run = compute_next_run_static(
                                &job.cron_expression,
                                &job.timezone,
                                Some(now),
                            );
                            let new_status = if job.one_shot {
                                CronJobStatus::Completed
                            } else {
                                CronJobStatus::Active
                            };
                            let enabled = !job.one_shot;

                            // Update run state.
                            if let Err(e) = store
                                .update_run_state(
                                    job.job_id,
                                    now,
                                    next_run,
                                    job.run_count + 1,
                                    new_status,
                                    enabled,
                                )
                                .await
                            {
                                tracing::error!("CronScheduler: failed to update run state for {}: {}", job.job_id, e);
                                continue;
                            }

                            // ── Increment counters ──
                            let store_c = store.clone();
                            let sched_c = agent_scheduler.clone();
                            let active_c = active_runs.clone();
                            let pja_c = per_job_active.clone();
                            let metrics_c = metrics.clone();
                            let verifier_c = agentpin_verifier.clone();

                            *active_c.write() += 1;
                            {
                                let mut pja = pja_c.write();
                                *pja.entry(job.job_id).or_insert(0) += 1;
                            }

                            tokio::spawn(async move {
                                // ── AgentPin identity check ───────────
                                // When a verifier is installed we must refuse
                                // to invoke the agent scheduler without a
                                // valid JWT whose `sub` covers this job's
                                // agent. A failed check is recorded as a
                                // skipped run with JobRunStatus::Failed so
                                // operators see it in history.
                                if let Some(verifier) = verifier_c.as_ref() {
                                    let ident_err = match job.agentpin_jwt.as_deref() {
                                        None => Some(
                                            "AgentPin verification is enabled but \
                                             job has no agentpin_jwt"
                                                .to_string(),
                                        ),
                                        Some(jwt) => match verifier.verify_credential(jwt).await {
                                            Ok(r) if r.valid => {
                                                let expected =
                                                    job.agent_config.id.0.to_string();
                                                if r.agent_id
                                                    .as_deref()
                                                    .map(|s| s == expected)
                                                    .unwrap_or(false)
                                                {
                                                    None
                                                } else {
                                                    Some(format!(
                                                        "JWT subject {:?} does not match \
                                                         job agent {}",
                                                        r.agent_id, expected
                                                    ))
                                                }
                                            }
                                            Ok(r) => Some(
                                                r.error_message
                                                    .unwrap_or_else(|| {
                                                        "invalid credential".to_string()
                                                    }),
                                            ),
                                            Err(e) => Some(e.to_string()),
                                        },
                                    };
                                    if let Some(reason) = ident_err {
                                        tracing::warn!(
                                            job_id = %job.job_id,
                                            "Cron run refused by AgentPin: {}",
                                            reason
                                        );
                                        let now = Utc::now();
                                        let record = JobRunRecord {
                                            run_id: uuid::Uuid::new_v4(),
                                            job_id: job.job_id,
                                            agent_id: job.agent_config.id,
                                            started_at: now,
                                            completed_at: Some(now),
                                            status: JobRunStatus::Failed,
                                            error: Some(format!("identity: {}", reason)),
                                            execution_time_ms: Some(0),
                                        };
                                        let _ = store_c.save_run_record(&record).await;
                                        {
                                            let mut m = metrics_c.write();
                                            m.runs_skipped_identity =
                                                m.runs_skipped_identity.saturating_add(1);
                                        }
                                        *active_c.write() = active_c.read().saturating_sub(1);
                                        {
                                            let mut pja = pja_c.write();
                                            if let Some(c) = pja.get_mut(&job.job_id) {
                                                *c = c.saturating_sub(1);
                                            }
                                        }
                                        return;
                                    }
                                }

                                // ── Jitter ────────────────────────────
                                if job.jitter_max_secs > 0 {
                                    let jitter_ms = {
                                        let mut rng = rand::thread_rng();
                                        rng.gen_range(0..=(job.jitter_max_secs as u64 * 1000))
                                    };
                                    tokio::time::sleep(Duration::from_millis(jitter_ms)).await;
                                }

                                let started_at = Utc::now();
                                let run_id = uuid::Uuid::new_v4();
                                let agent_id = job.agent_config.id;
                                let session_id = uuid::Uuid::new_v4();

                                // Build an ephemeral agent config for the run.
                                let mut run_config = job.agent_config.clone();
                                run_config.execution_mode = ExecutionMode::CronScheduled {
                                    cron_expression: job.cron_expression.clone(),
                                    timezone: job.timezone.clone(),
                                };
                                run_config.metadata.insert(
                                    "trigger".to_string(),
                                    "cron".to_string(),
                                );
                                run_config.metadata.insert(
                                    "cron_job_id".to_string(),
                                    job.job_id.to_string(),
                                );
                                run_config.metadata.insert(
                                    "cron_expression".to_string(),
                                    job.cron_expression.clone(),
                                );
                                // Session isolation metadata
                                run_config.metadata.insert(
                                    "session_id".to_string(),
                                    session_id.to_string(),
                                );
                                run_config.metadata.insert(
                                    "session_mode".to_string(),
                                    format!("{:?}", job.session_mode),
                                );

                                let result: Result<crate::types::AgentId, crate::types::SchedulerError> =
                                    sched_c.schedule_agent(run_config).await;

                                let (status, error) = match &result {
                                    Ok(_) => (JobRunStatus::Succeeded, None),
                                    Err(e) => (JobRunStatus::Failed, Some(e.to_string())),
                                };

                                let completed_at = Utc::now();
                                let exec_ms = (completed_at - started_at).num_milliseconds().max(0) as u64;

                                // Update metrics
                                {
                                    let mut m = metrics_c.write();
                                    m.runs_total += 1;
                                    match status {
                                        JobRunStatus::Succeeded => m.runs_succeeded += 1,
                                        JobRunStatus::Failed => m.runs_failed += 1,
                                        _ => {}
                                    }
                                    if exec_ms > m.longest_run_ms {
                                        m.longest_run_ms = exec_ms;
                                    }
                                    // Rolling average
                                    if m.runs_total > 0 {
                                        m.average_execution_time_ms = m.average_execution_time_ms
                                            + (exec_ms as f64 - m.average_execution_time_ms)
                                                / m.runs_total as f64;
                                    }
                                }

                                let record = JobRunRecord {
                                    run_id,
                                    job_id: job.job_id,
                                    agent_id,
                                    started_at,
                                    completed_at: Some(completed_at),
                                    status,
                                    error: error.clone(),
                                    execution_time_ms: Some(exec_ms),
                                };

                                if let Err(e) = store_c.save_run_record(&record).await {
                                    tracing::error!("CronScheduler: failed to save run record: {}", e);
                                }

                                // ── Dead-letter handling on failure ──
                                if result.is_err() {
                                    let new_fail = job.failure_count + 1;
                                    let fail_status = if new_fail >= job.max_retries as u64 {
                                        tracing::warn!(
                                            "CronScheduler: job {} exceeded max_retries ({}), moving to dead letter",
                                            job.job_id, job.max_retries
                                        );
                                        CronJobStatus::DeadLetter
                                    } else {
                                        CronJobStatus::Active
                                    };
                                    let _ = store_c.record_failure(job.job_id, new_fail, fail_status).await;
                                }

                                // ── Decrement counters ──
                                *active_c.write() -= 1;
                                {
                                    let mut pja = pja_c.write();
                                    if let Some(count) = pja.get_mut(&job.job_id) {
                                        *count = count.saturating_sub(1);
                                        if *count == 0 {
                                            pja.remove(&job.job_id);
                                        }
                                    }
                                }
                            });
                        }
                    }
                    _ = shutdown.notified() => {
                        tracing::info!("CronScheduler tick loop shutting down");
                        break;
                    }
                }
            }
        });
    }

    fn parse_cron(expr: &str) -> Result<Schedule, CronSchedulerError> {
        Schedule::from_str(expr)
            .map_err(|e| CronSchedulerError::InvalidCron(format!("{expr}: {e}")))
    }

    fn validate_timezone(tz: &str) -> Result<chrono_tz::Tz, CronSchedulerError> {
        tz.parse::<chrono_tz::Tz>()
            .map_err(|_| CronSchedulerError::InvalidTimezone(tz.to_string()))
    }

    fn compute_next_run(
        &self,
        cron_expression: &str,
        timezone: &str,
        after: Option<DateTime<Utc>>,
    ) -> Result<Option<DateTime<Utc>>, CronSchedulerError> {
        Ok(compute_next_run_static(cron_expression, timezone, after))
    }
}

/// Health snapshot for the cron scheduler subsystem.
#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)]
pub struct CronSchedulerHealth {
    pub is_running: bool,
    pub store_accessible: bool,
    pub jobs_total: usize,
    pub jobs_active: usize,
    pub jobs_paused: usize,
    pub jobs_dead_letter: usize,
    pub global_active_runs: usize,
    pub max_concurrent: usize,
}

/// Standalone helper so the spawned task can call it without `&self`.
fn compute_next_run_static(
    cron_expression: &str,
    timezone: &str,
    after: Option<DateTime<Utc>>,
) -> Option<DateTime<Utc>> {
    let schedule = Schedule::from_str(cron_expression).ok()?;
    let tz: chrono_tz::Tz = timezone.parse().ok()?;
    let reference = after.unwrap_or_else(Utc::now).with_timezone(&tz);
    schedule
        .after(&reference)
        .next()
        .map(|dt| dt.with_timezone(&Utc))
}

#[cfg(test)]
mod tests {
    use super::*;
    use crate::scheduler::heartbeat::HeartbeatContextMode;
    use crate::scheduler::SchedulerConfig;
    use crate::types::{AgentConfig, AgentId, Priority, ResourceLimits, SecurityTier};
    use std::collections::HashMap;

    fn test_agent_config() -> AgentConfig {
        AgentConfig {
            id: AgentId::new(),
            name: "cron_agent".to_string(),
            dsl_source: "agent cron_agent {}".to_string(),
            execution_mode: ExecutionMode::Ephemeral,
            security_tier: SecurityTier::Tier1,
            resource_limits: ResourceLimits::default(),
            capabilities: vec![],
            policies: vec![],
            metadata: HashMap::new(),
            priority: Priority::Normal,
        }
    }

    async fn make_scheduler() -> (CronScheduler, Arc<DefaultAgentScheduler>) {
        let sched = Arc::new(
            DefaultAgentScheduler::new(SchedulerConfig::default())
                .await
                .unwrap(),
        );
        let config = CronSchedulerConfig {
            tick_interval: Duration::from_millis(100),
            ..Default::default()
        };
        let store = Arc::new(SqliteJobStore::open_in_memory().unwrap());
        let cron = CronScheduler {
            store,
            agent_scheduler: sched.clone(),
            config,
            shutdown_notify: Arc::new(Notify::new()),
            is_running: Arc::new(RwLock::new(true)),
            active_runs: Arc::new(RwLock::new(0)),
            per_job_active: Arc::new(RwLock::new(HashMap::new())),
            metrics: Arc::new(RwLock::new(CronMetrics::default())),
            agentpin_verifier: None,
        };
        cron.start_tick_loop();
        (cron, sched)
    }

    #[test]
    fn parse_valid_seven_field_cron_expressions() {
        // 7-field: sec min hour dom month dow year
        assert!(CronScheduler::parse_cron("0 * * * * * *").is_ok());
        // Every 5 minutes (7-field)
        assert!(CronScheduler::parse_cron("0 */5 * * * * *").is_ok());
        // Specific year
        assert!(CronScheduler::parse_cron("0 0 12 * * Mon 2027").is_ok());
    }

    #[test]
    fn parse_valid_six_field_cron_expressions() {
        // 6-field: sec min hour dom month dow (no year)
        assert!(CronScheduler::parse_cron("0 * * * * *").is_ok());
        // Every 5 minutes (6-field)
        assert!(CronScheduler::parse_cron("0 */5 * * * *").is_ok());
        // Every 30 seconds
        assert!(CronScheduler::parse_cron("*/30 * * * * *").is_ok());
        // Weekdays at 9 AM
        assert!(CronScheduler::parse_cron("0 0 9 * * Mon-Fri").is_ok());
        // First of month at midnight
        assert!(CronScheduler::parse_cron("0 0 0 1 * *").is_ok());
    }

    #[test]
    fn reject_invalid_cron() {
        assert!(CronScheduler::parse_cron("not a cron").is_err());
    }

    #[test]
    fn reject_five_field_unix_cron() {
        // Standard 5-field Unix cron (min hour dom month dow) is NOT supported
        // because the `cron` crate requires a leading seconds field.
        assert!(CronScheduler::parse_cron("*/5 * * * *").is_err());
        assert!(CronScheduler::parse_cron("0 12 * * Mon").is_err());
    }

    #[test]
    fn reject_empty_and_whitespace_cron() {
        assert!(CronScheduler::parse_cron("").is_err());
        assert!(CronScheduler::parse_cron("   ").is_err());
    }

    #[test]
    fn error_message_includes_format_hint() {
        let err = CronScheduler::parse_cron("bad").unwrap_err();
        let msg = err.to_string();
        assert!(
            msg.contains("6-field"),
            "error should mention 6-field format, got: {msg}"
        );
        assert!(
            msg.contains("sec min hour"),
            "error should include field names, got: {msg}"
        );
    }

    #[test]
    fn validate_timezone() {
        assert!(CronScheduler::validate_timezone("UTC").is_ok());
        assert!(CronScheduler::validate_timezone("America/New_York").is_ok());
        assert!(CronScheduler::validate_timezone("Asia/Kathmandu").is_ok());
        assert!(CronScheduler::validate_timezone("Bogus/Zone").is_err());
    }

    #[test]
    fn compute_next_run_returns_future_time_seven_field() {
        let next = compute_next_run_static("0 * * * * * *", "UTC", None);
        assert!(next.is_some());
        assert!(next.unwrap() > Utc::now());
    }

    #[test]
    fn compute_next_run_returns_future_time_six_field() {
        let next = compute_next_run_static("0 * * * * *", "UTC", None);
        assert!(next.is_some());
        assert!(next.unwrap() > Utc::now());
    }

    #[test]
    fn compute_next_run_respects_after_parameter() {
        let reference = Utc::now() + chrono::Duration::hours(1);
        let next = compute_next_run_static("0 * * * * *", "UTC", Some(reference));
        assert!(next.is_some());
        assert!(next.unwrap() > reference);
    }

    #[test]
    fn compute_next_run_with_different_timezones() {
        let utc_next = compute_next_run_static("0 0 12 * * *", "UTC", None);
        let eastern_next = compute_next_run_static("0 0 12 * * *", "America/New_York", None);
        assert!(utc_next.is_some());
        assert!(eastern_next.is_some());
        // Same cron expression in different timezones should produce different UTC times
        // (unless we happen to be at exactly the boundary, which is astronomically unlikely).
        assert_ne!(utc_next.unwrap(), eastern_next.unwrap());
    }

    #[test]
    fn compute_next_run_returns_none_for_invalid_expression() {
        let next = compute_next_run_static("bad", "UTC", None);
        assert!(next.is_none());
    }

    #[test]
    fn compute_next_run_returns_none_for_invalid_timezone() {
        let next = compute_next_run_static("0 * * * * *", "Mars/Olympus", None);
        assert!(next.is_none());
    }

    #[test]
    fn get_next_runs_returns_multiple() {
        let runs = {
            let schedule = Schedule::from_str("0 * * * * *").unwrap();
            let tz: chrono_tz::Tz = "UTC".parse().unwrap();
            let now = Utc::now().with_timezone(&tz);
            schedule
                .after(&now)
                .take(5)
                .map(|dt| dt.with_timezone(&Utc))
                .collect::<Vec<_>>()
        };
        assert_eq!(runs.len(), 5);
        for pair in runs.windows(2) {
            assert!(pair[1] > pair[0]);
        }
    }

    #[test]
    fn six_and_seven_field_produce_equivalent_schedules() {
        // "0 */5 * * * *" (6-field) and "0 */5 * * * * *" (7-field) should
        // produce the same next fire time.
        let now = Some(Utc::now());
        let six = compute_next_run_static("0 */5 * * * *", "UTC", now);
        let seven = compute_next_run_static("0 */5 * * * * *", "UTC", now);
        assert_eq!(six, seven);
    }

    #[tokio::test]
    async fn add_and_list_jobs() {
        let (cron, _sched) = make_scheduler().await;

        let job = CronJobDefinition::new(
            "test_job".to_string(),
            "0 * * * * *".to_string(), // 6-field: every minute
            "UTC".to_string(),
            test_agent_config(),
        );
        let id = cron.add_job(job).await.unwrap();

        let jobs = cron.list_jobs().await.unwrap();
        assert_eq!(jobs.len(), 1);
        assert_eq!(jobs[0].job_id, id);
        assert!(jobs[0].next_run.is_some());
        cron.shutdown().await;
    }

    #[tokio::test]
    async fn pause_and_resume() {
        let (cron, _sched) = make_scheduler().await;

        let job = CronJobDefinition::new(
            "pause_test".to_string(),
            "0 * * * * *".to_string(), // 6-field
            "UTC".to_string(),
            test_agent_config(),
        );
        let id = cron.add_job(job).await.unwrap();

        cron.pause_job(id).await.unwrap();
        let paused = cron.get_job(id).await.unwrap();
        assert_eq!(paused.status, CronJobStatus::Paused);
        assert!(!paused.enabled);

        cron.resume_job(id).await.unwrap();
        let resumed = cron.get_job(id).await.unwrap();
        assert_eq!(resumed.status, CronJobStatus::Active);
        assert!(resumed.enabled);
        assert!(resumed.next_run.is_some());
        cron.shutdown().await;
    }

    #[tokio::test]
    async fn remove_job() {
        let (cron, _sched) = make_scheduler().await;

        let job = CronJobDefinition::new(
            "remove_test".to_string(),
            "0 * * * * *".to_string(), // 6-field
            "UTC".to_string(),
            test_agent_config(),
        );
        let id = cron.add_job(job).await.unwrap();
        cron.remove_job(id).await.unwrap();

        assert!(matches!(
            cron.get_job(id).await,
            Err(CronSchedulerError::NotFound(_))
        ));
        cron.shutdown().await;
    }

    #[tokio::test]
    async fn one_shot_lifecycle() {
        let (cron, _sched) = make_scheduler().await;

        let mut job = CronJobDefinition::new(
            "one_shot".to_string(),
            // Fire every second (7-field cron: sec min hour dom mon dow year).
            "* * * * * * *".to_string(),
            "UTC".to_string(),
            test_agent_config(),
        );
        job.one_shot = true;
        let id = cron.add_job(job).await.unwrap();

        // Force next_run into the past so the tick loop picks it up immediately.
        cron.store
            .update_run_state(
                id,
                Utc::now(),
                Some(Utc::now() - chrono::Duration::seconds(5)),
                0,
                CronJobStatus::Active,
                true,
            )
            .await
            .unwrap();

        // Wait for the tick loop to fire it (tick interval=100ms, allow plenty of time).
        tokio::time::sleep(Duration::from_secs(2)).await;

        let loaded = cron.get_job(id).await.unwrap();
        assert_eq!(loaded.status, CronJobStatus::Completed);
        assert!(!loaded.enabled);
        cron.shutdown().await;
    }

    #[tokio::test]
    async fn trigger_now_fires_immediately() {
        let (cron, _sched) = make_scheduler().await;

        let job = CronJobDefinition::new(
            "trigger_now".to_string(),
            "0 0 0 1 1 * 2099".to_string(), // Far future — won't fire normally.
            "UTC".to_string(),
            test_agent_config(),
        );
        let id = cron.add_job(job).await.unwrap();
        cron.trigger_now(id).await.unwrap();

        // Should have a run record.
        tokio::time::sleep(Duration::from_millis(200)).await;
        let history = cron.get_run_history(id, 10).await.unwrap();
        assert!(!history.is_empty());
        cron.shutdown().await;
    }

    #[tokio::test]
    async fn reject_invalid_cron_on_add() {
        let (cron, _sched) = make_scheduler().await;

        let job = CronJobDefinition::new(
            "bad_cron".to_string(),
            "invalid".to_string(),
            "UTC".to_string(),
            test_agent_config(),
        );
        assert!(cron.add_job(job).await.is_err());
        cron.shutdown().await;
    }

    #[tokio::test]
    async fn reject_five_field_cron_on_add() {
        let (cron, _sched) = make_scheduler().await;

        let job = CronJobDefinition::new(
            "five_field".to_string(),
            "*/5 * * * *".to_string(), // 5-field Unix cron — not supported
            "UTC".to_string(),
            test_agent_config(),
        );
        assert!(cron.add_job(job).await.is_err());
        cron.shutdown().await;
    }

    #[tokio::test]
    async fn reject_invalid_timezone_on_add() {
        let (cron, _sched) = make_scheduler().await;

        let job = CronJobDefinition::new(
            "bad_tz".to_string(),
            "0 * * * * *".to_string(),
            "Mars/Olympus".to_string(),
            test_agent_config(),
        );
        assert!(cron.add_job(job).await.is_err());
        cron.shutdown().await;
    }

    #[tokio::test]
    async fn shutdown_is_idempotent() {
        let (cron, _sched) = make_scheduler().await;
        cron.shutdown().await;
        cron.shutdown().await; // Should not panic.
    }

    #[tokio::test]
    async fn metrics_increment_on_runs() {
        let (cron, _sched) = make_scheduler().await;
        let m = cron.metrics();
        assert_eq!(m.runs_total, 0);
        assert_eq!(m.runs_succeeded, 0);
        cron.shutdown().await;
    }

    #[tokio::test]
    async fn health_check_returns_valid() {
        let (cron, _sched) = make_scheduler().await;
        let health = cron.check_health().await.unwrap();
        assert!(health.is_running);
        assert!(health.store_accessible);
        assert_eq!(health.jobs_total, 0);
        cron.shutdown().await;
    }

    #[tokio::test]
    async fn session_mode_persists() {
        let (cron, _sched) = make_scheduler().await;

        let mut job = CronJobDefinition::new(
            "session_test".to_string(),
            "0 * * * * * *".to_string(),
            "UTC".to_string(),
            test_agent_config(),
        );
        job.session_mode = HeartbeatContextMode::FullyEphemeral;
        let id = cron.add_job(job).await.unwrap();

        let loaded = cron.get_job(id).await.unwrap();
        assert_eq!(loaded.session_mode, HeartbeatContextMode::FullyEphemeral);
        cron.shutdown().await;
    }

    #[tokio::test]
    async fn dead_letter_status_roundtrip() {
        let (cron, _sched) = make_scheduler().await;

        let job = CronJobDefinition::new(
            "dl_test".to_string(),
            "0 * * * * * *".to_string(),
            "UTC".to_string(),
            test_agent_config(),
        );
        let id = cron.add_job(job).await.unwrap();

        // Manually move to dead letter
        cron.store
            .record_failure(id, 10, CronJobStatus::DeadLetter)
            .await
            .unwrap();
        let loaded = cron.get_job(id).await.unwrap();
        assert_eq!(loaded.status, CronJobStatus::DeadLetter);

        // Resume from dead letter
        cron.resume_job(id).await.unwrap();
        let resumed = cron.get_job(id).await.unwrap();
        assert_eq!(resumed.status, CronJobStatus::Active);
        assert_eq!(resumed.failure_count, 0);
        cron.shutdown().await;
    }

    #[tokio::test]
    async fn jitter_field_persists() {
        let (cron, _sched) = make_scheduler().await;

        let mut job = CronJobDefinition::new(
            "jitter_test".to_string(),
            "0 * * * * * *".to_string(),
            "UTC".to_string(),
            test_agent_config(),
        );
        job.jitter_max_secs = 5;
        let id = cron.add_job(job).await.unwrap();

        let loaded = cron.get_job(id).await.unwrap();
        assert_eq!(loaded.jitter_max_secs, 5);
        cron.shutdown().await;
    }

    // ── 6-field end-to-end tests ─────────────────────────────────────

    #[tokio::test]
    async fn add_job_with_six_field_cron() {
        let (cron, _sched) = make_scheduler().await;

        let job = CronJobDefinition::new(
            "six_field_job".to_string(),
            "0 */5 * * * *".to_string(), // 6-field: every 5 minutes
            "UTC".to_string(),
            test_agent_config(),
        );
        let id = cron.add_job(job).await.unwrap();

        let loaded = cron.get_job(id).await.unwrap();
        assert_eq!(loaded.cron_expression, "0 */5 * * * *");
        assert!(loaded.next_run.is_some());
        cron.shutdown().await;
    }

    #[tokio::test]
    async fn trigger_now_with_six_field_cron() {
        let (cron, _sched) = make_scheduler().await;

        let job = CronJobDefinition::new(
            "trigger_six".to_string(),
            "0 0 0 1 1 *".to_string(), // Far future — 6-field
            "UTC".to_string(),
            test_agent_config(),
        );
        let id = cron.add_job(job).await.unwrap();
        cron.trigger_now(id).await.unwrap();

        let history = cron.get_run_history(id, 10).await.unwrap();
        assert!(!history.is_empty());
        assert_eq!(history[0].status, JobRunStatus::Succeeded);
        cron.shutdown().await;
    }

    // ── update_job tests ─────────────────────────────────────────────

    #[tokio::test]
    async fn update_job_changes_cron_expression() {
        let (cron, _sched) = make_scheduler().await;

        let job = CronJobDefinition::new(
            "update_cron".to_string(),
            "0 * * * * *".to_string(),
            "UTC".to_string(),
            test_agent_config(),
        );
        let id = cron.add_job(job).await.unwrap();
        let original = cron.get_job(id).await.unwrap();

        let mut updated = original.clone();
        updated.cron_expression = "0 */10 * * * *".to_string();
        cron.update_job(updated).await.unwrap();

        let loaded = cron.get_job(id).await.unwrap();
        assert_eq!(loaded.cron_expression, "0 */10 * * * *");
        // next_run should have been recomputed
        assert!(loaded.next_run.is_some());
        assert!(loaded.updated_at > original.updated_at);
        cron.shutdown().await;
    }

    #[tokio::test]
    async fn update_job_rejects_invalid_cron() {
        let (cron, _sched) = make_scheduler().await;

        let job = CronJobDefinition::new(
            "update_bad".to_string(),
            "0 * * * * *".to_string(),
            "UTC".to_string(),
            test_agent_config(),
        );
        let id = cron.add_job(job).await.unwrap();
        let mut bad = cron.get_job(id).await.unwrap();
        bad.cron_expression = "nope".to_string();

        assert!(cron.update_job(bad).await.is_err());
        // Original should be unchanged
        let loaded = cron.get_job(id).await.unwrap();
        assert_eq!(loaded.cron_expression, "0 * * * * *");
        cron.shutdown().await;
    }

    #[tokio::test]
    async fn update_job_rejects_invalid_timezone() {
        let (cron, _sched) = make_scheduler().await;

        let job = CronJobDefinition::new(
            "update_bad_tz".to_string(),
            "0 * * * * *".to_string(),
            "UTC".to_string(),
            test_agent_config(),
        );
        let id = cron.add_job(job).await.unwrap();
        let mut bad = cron.get_job(id).await.unwrap();
        bad.timezone = "Fake/Zone".to_string();

        assert!(cron.update_job(bad).await.is_err());
        cron.shutdown().await;
    }

    #[tokio::test]
    async fn update_job_changes_timezone() {
        let (cron, _sched) = make_scheduler().await;

        let job = CronJobDefinition::new(
            "tz_update".to_string(),
            "0 0 12 * * *".to_string(),
            "UTC".to_string(),
            test_agent_config(),
        );
        let id = cron.add_job(job).await.unwrap();
        let original_next = cron.get_job(id).await.unwrap().next_run;

        let mut updated = cron.get_job(id).await.unwrap();
        updated.timezone = "America/New_York".to_string();
        cron.update_job(updated).await.unwrap();

        let loaded = cron.get_job(id).await.unwrap();
        assert_eq!(loaded.timezone, "America/New_York");
        // next_run should differ because timezone changed
        assert_ne!(loaded.next_run, original_next);
        cron.shutdown().await;
    }

    // ── get_next_runs API tests ──────────────────────────────────────

    #[tokio::test]
    async fn get_next_runs_api_six_field() {
        let (cron, _sched) = make_scheduler().await;
        let runs = cron.get_next_runs("0 * * * * *", "UTC", 3).unwrap();
        assert_eq!(runs.len(), 3);
        for pair in runs.windows(2) {
            assert!(pair[1] > pair[0]);
        }
        cron.shutdown().await;
    }

    #[tokio::test]
    async fn get_next_runs_api_rejects_bad_expression() {
        let (cron, _sched) = make_scheduler().await;
        assert!(cron.get_next_runs("bad", "UTC", 3).is_err());
        cron.shutdown().await;
    }

    #[tokio::test]
    async fn get_next_runs_api_rejects_bad_timezone() {
        let (cron, _sched) = make_scheduler().await;
        assert!(cron.get_next_runs("0 * * * * *", "Bogus/Tz", 3).is_err());
        cron.shutdown().await;
    }

    // ── Multiple jobs / timezone isolation ────────────────────────────

    #[tokio::test]
    async fn multiple_jobs_coexist() {
        let (cron, _sched) = make_scheduler().await;

        let job_a = CronJobDefinition::new(
            "job_a".to_string(),
            "0 * * * * *".to_string(),
            "UTC".to_string(),
            test_agent_config(),
        );
        let job_b = CronJobDefinition::new(
            "job_b".to_string(),
            "0 */10 * * * *".to_string(),
            "America/Chicago".to_string(),
            test_agent_config(),
        );
        let job_c = CronJobDefinition::new(
            "job_c".to_string(),
            "0 0 9 * * Mon-Fri".to_string(),
            "Europe/London".to_string(),
            test_agent_config(),
        );

        let id_a = cron.add_job(job_a).await.unwrap();
        let id_b = cron.add_job(job_b).await.unwrap();
        let id_c = cron.add_job(job_c).await.unwrap();

        let all = cron.list_jobs().await.unwrap();
        assert_eq!(all.len(), 3);

        // Removing one doesn't affect others
        cron.remove_job(id_b).await.unwrap();
        let remaining = cron.list_jobs().await.unwrap();
        assert_eq!(remaining.len(), 2);
        let ids: Vec<_> = remaining.iter().map(|j| j.job_id).collect();
        assert!(ids.contains(&id_a));
        assert!(ids.contains(&id_c));
        cron.shutdown().await;
    }

    // ── Health check with jobs ───────────────────────────────────────

    #[tokio::test]
    async fn health_check_reflects_job_states() {
        let (cron, _sched) = make_scheduler().await;

        // Add an active job
        let active_job = CronJobDefinition::new(
            "active".to_string(),
            "0 * * * * *".to_string(),
            "UTC".to_string(),
            test_agent_config(),
        );
        cron.add_job(active_job).await.unwrap();

        // Add and pause a job
        let pause_job = CronJobDefinition::new(
            "paused".to_string(),
            "0 * * * * *".to_string(),
            "UTC".to_string(),
            test_agent_config(),
        );
        let pid = cron.add_job(pause_job).await.unwrap();
        cron.pause_job(pid).await.unwrap();

        // Add and dead-letter a job
        let dl_job = CronJobDefinition::new(
            "dead_letter".to_string(),
            "0 * * * * *".to_string(),
            "UTC".to_string(),
            test_agent_config(),
        );
        let did = cron.add_job(dl_job).await.unwrap();
        cron.store
            .record_failure(did, 10, CronJobStatus::DeadLetter)
            .await
            .unwrap();

        let health = cron.check_health().await.unwrap();
        assert_eq!(health.jobs_total, 3);
        assert_eq!(health.jobs_active, 1);
        assert_eq!(health.jobs_paused, 1);
        assert_eq!(health.jobs_dead_letter, 1);
        cron.shutdown().await;
    }

    // ── Pause / resume / remove edge cases ───────────────────────────

    #[tokio::test]
    async fn pause_nonexistent_job_returns_not_found() {
        let (cron, _sched) = make_scheduler().await;
        let bogus_id = CronJobId::new();
        assert!(matches!(
            cron.pause_job(bogus_id).await,
            Err(CronSchedulerError::NotFound(_))
        ));
        cron.shutdown().await;
    }

    #[tokio::test]
    async fn resume_nonexistent_job_returns_not_found() {
        let (cron, _sched) = make_scheduler().await;
        let bogus_id = CronJobId::new();
        assert!(matches!(
            cron.resume_job(bogus_id).await,
            Err(CronSchedulerError::NotFound(_))
        ));
        cron.shutdown().await;
    }

    #[tokio::test]
    async fn remove_nonexistent_job_returns_not_found() {
        let (cron, _sched) = make_scheduler().await;
        let bogus_id = CronJobId::new();
        assert!(matches!(
            cron.remove_job(bogus_id).await,
            Err(CronSchedulerError::NotFound(_))
        ));
        cron.shutdown().await;
    }

    #[tokio::test]
    async fn trigger_now_nonexistent_job_returns_not_found() {
        let (cron, _sched) = make_scheduler().await;
        let bogus_id = CronJobId::new();
        assert!(matches!(
            cron.trigger_now(bogus_id).await,
            Err(CronSchedulerError::NotFound(_))
        ));
        cron.shutdown().await;
    }

    #[tokio::test]
    async fn double_pause_is_idempotent() {
        let (cron, _sched) = make_scheduler().await;
        let job = CronJobDefinition::new(
            "double_pause".to_string(),
            "0 * * * * *".to_string(),
            "UTC".to_string(),
            test_agent_config(),
        );
        let id = cron.add_job(job).await.unwrap();
        cron.pause_job(id).await.unwrap();
        cron.pause_job(id).await.unwrap(); // Should not panic or error
        let loaded = cron.get_job(id).await.unwrap();
        assert_eq!(loaded.status, CronJobStatus::Paused);
        cron.shutdown().await;
    }

    // ── Metrics after trigger ────────────────────────────────────────

    #[tokio::test]
    async fn metrics_update_after_trigger_now() {
        let (cron, _sched) = make_scheduler().await;

        let job = CronJobDefinition::new(
            "metrics_trigger".to_string(),
            "0 0 0 1 1 * 2099".to_string(),
            "UTC".to_string(),
            test_agent_config(),
        );
        let id = cron.add_job(job).await.unwrap();

        let before = cron.metrics();
        assert_eq!(before.runs_total, 0);

        cron.trigger_now(id).await.unwrap();
        // trigger_now records directly, but metrics are updated by the tick loop.
        // The trigger_now path records in the store but the tick-loop path
        // updates metrics. Since trigger_now bypasses the tick loop, we verify
        // the run record exists instead.
        let history = cron.get_run_history(id, 10).await.unwrap();
        assert_eq!(history.len(), 1);
        assert_eq!(history[0].status, JobRunStatus::Succeeded);
        assert!(history[0].execution_time_ms.is_some());
        cron.shutdown().await;
    }

    // ── Health check after shutdown ──────────────────────────────────

    #[tokio::test]
    async fn health_check_after_shutdown() {
        let (cron, _sched) = make_scheduler().await;
        cron.shutdown().await;
        let health = cron.check_health().await.unwrap();
        assert!(!health.is_running);
        assert!(health.store_accessible);
    }

    // ── AP-3: AgentPin verification on cron runs ──────────────────────

    #[tokio::test]
    async fn trigger_now_refused_when_verifier_installed_without_jwt() {
        let (cron, _sched) = make_scheduler().await;
        let cron = cron.with_agentpin_verifier(std::sync::Arc::new(
            crate::integrations::MockAgentPinVerifier::new_success(),
        ));
        let id = cron
            .add_job(CronJobDefinition::new(
                "needs-jwt".to_string(),
                "0 * * * * *".to_string(),
                "UTC".to_string(),
                test_agent_config(),
            ))
            .await
            .unwrap();
        let err = cron.trigger_now(id).await.expect_err("must refuse");
        assert!(
            matches!(err, CronSchedulerError::IdentityVerificationFailed(_, _)),
            "expected IdentityVerificationFailed, got {:?}",
            err
        );
        let metrics = cron.metrics();
        assert!(metrics.runs_skipped_identity >= 1);
        cron.shutdown().await;
    }

    #[tokio::test]
    async fn trigger_now_refused_when_verifier_rejects() {
        let (cron, _sched) = make_scheduler().await;
        let cron = cron.with_agentpin_verifier(std::sync::Arc::new(
            crate::integrations::MockAgentPinVerifier::new_failure(),
        ));
        let mut job = CronJobDefinition::new(
            "bad-jwt".to_string(),
            "0 * * * * *".to_string(),
            "UTC".to_string(),
            test_agent_config(),
        );
        job.agentpin_jwt = Some("any.jwt.here".to_string());
        let id = cron.add_job(job).await.unwrap();
        let err = cron.trigger_now(id).await.expect_err("must refuse");
        assert!(matches!(
            err,
            CronSchedulerError::IdentityVerificationFailed(_, _)
        ));
        cron.shutdown().await;
    }

    #[tokio::test]
    async fn trigger_now_allowed_when_verifier_subject_matches_job_agent() {
        let (cron, _sched) = make_scheduler().await;
        let mut job = CronJobDefinition::new(
            "good-jwt".to_string(),
            "0 * * * * *".to_string(),
            "UTC".to_string(),
            test_agent_config(),
        );
        job.agentpin_jwt = Some("any.jwt.here".to_string());
        let expected_sub = job.agent_config.id.0.to_string();
        let cron = cron.with_agentpin_verifier(std::sync::Arc::new(
            crate::integrations::MockAgentPinVerifier::with_identity(
                expected_sub,
                "test.example.com".to_string(),
                vec![],
            ),
        ));
        let id = cron.add_job(job).await.unwrap();
        cron.trigger_now(id).await.expect("must succeed");
        cron.shutdown().await;
    }

    #[tokio::test]
    async fn trigger_now_refused_when_subject_does_not_match_job_agent() {
        let (cron, _sched) = make_scheduler().await;
        let mut job = CronJobDefinition::new(
            "wrong-sub".to_string(),
            "0 * * * * *".to_string(),
            "UTC".to_string(),
            test_agent_config(),
        );
        job.agentpin_jwt = Some("any.jwt.here".to_string());
        let cron = cron.with_agentpin_verifier(std::sync::Arc::new(
            crate::integrations::MockAgentPinVerifier::with_identity(
                "someone-else".to_string(),
                "test.example.com".to_string(),
                vec![],
            ),
        ));
        let id = cron.add_job(job).await.unwrap();
        let err = cron.trigger_now(id).await.expect_err("must refuse");
        assert!(matches!(
            err,
            CronSchedulerError::IdentityVerificationFailed(_, _)
        ));
        cron.shutdown().await;
    }
}
