diff --git a/frontend/src/lib/api/types.ts b/frontend/src/lib/api/types.ts index f6459876..4758bf7c 100644 --- a/frontend/src/lib/api/types.ts +++ b/frontend/src/lib/api/types.ts @@ -718,6 +718,14 @@ export interface JobSummary { * for this job. Distinct from `running` — a paused run is * resumable via the same trigger endpoint. */ paused_run?: PausedRunBrief; + /** Status of this job's most recent run row (recoverable jobs only). + * + * **Prefer this over `last_outcome` wherever they could disagree.** + * `last_outcome` is the backend's in-memory record of the last + * dispatch, so anything that changes a run row without running the + * handler leaves it stale — cancelling a Paused run is a direct SQL + * flip, and the panel went on rendering the pause it replaced. */ + last_run_status?: RunStatus; /** Present iff `OXICLOUD_STARTUP_JOBS` names this job — the flags it * is dispatched with at every boot. Worth showing: a job configured * with `repair: true` deletes on every restart, and the row would diff --git a/frontend/src/lib/components/AdminJobsPanel.svelte b/frontend/src/lib/components/AdminJobsPanel.svelte index 6360f5fc..d30f1367 100644 --- a/frontend/src/lib/components/AdminJobsPanel.svelte +++ b/frontend/src/lib/components/AdminJobsPanel.svelte @@ -524,8 +524,23 @@ // green "ok" hides the thing worth acting on. A paused // `backend_migration` is still holding `migration_readonly` and // refusing writes across the whole app; the row has to say so. - function stoppedOnBackendFailure(job: JobSummary): boolean { - return job.last_outcome?.outcome === 'ok' && job.last_outcome.extra?.retryable === true; + /// Lifecycle label for the State column. Lower-cased to match the + /// existing `running` pill rather than shouting the DB's PascalCase. + function runStatusLabel(status: RunStatus): string { + switch (status) { + case 'Running': + return t('admin.jobs.state_running', 'running'); + case 'Paused': + return t('admin.jobs.state_paused', 'paused'); + case 'CancelRequested': + return t('admin.jobs.state_cancelling', 'cancelling'); + case 'Cancelled': + return t('admin.jobs.state_cancelled', 'cancelled'); + case 'Completed': + return t('admin.jobs.state_completed', 'completed'); + case 'Failed': + return t('admin.jobs.state_failed', 'failed'); + } } function backendFailureReason(job: JobSummary): string | undefined { @@ -534,13 +549,24 @@ return typeof reason === 'string' ? reason : undefined; } + /// How the last dispatch turned out. NOT where the run is in its + /// lifecycle — that is the State column, driven by + /// `last_run_status`. A paused run legitimately has no outcome yet, + /// and saying so is the honest answer. function outcomeLabel(job: JobSummary): string { - if (!job.last_outcome) return t('admin.jobs.never', 'never'); - // Checked before the findings branches: a run that never finished - // has nothing meaningful to say about findings, and "0 issues" on - // an aborted scan is a worse answer than "blocked". - if (stoppedOnBackendFailure(job)) { - return t('admin.jobs.outcome_blocked', 'blocked'); + if (!job.last_outcome) { + // "never" means never ran. A job with a run row DID run — the + // outcome simply is not in memory, because `last_outcome` is + // populated per dispatch and a restart empties it. Saying + // "never" there is a lie the run history immediately + // contradicts: Ed saw it on a job whose last run was 8h ago. + // + // "—" is the honest answer: no outcome recorded. The State + // column still shows what the run did, and the drawer has + // the history. + return job.last_run_status + ? t('admin.jobs.outcome_unknown', '—') + : t('admin.jobs.never', 'never'); } if (job.last_outcome.outcome === 'ok') { // `ok` on the wire = dispatch completed. If any actionable @@ -563,14 +589,6 @@ if (job.last_outcome.outcome !== 'ok') { return 'jobs-panel__pill jobs-panel__pill--err'; } - // Amber, not red: nothing is broken and no data was lost — the - // run is waiting for the backend to come back and a Resume - // continues it. Red would read as "this job is failing" and - // invite a cancel, which for a migration also throws away the - // copy already done. - if (stoppedOnBackendFailure(job)) { - return 'jobs-panel__pill jobs-panel__pill--paused'; - } if (actionableFindingCount(job) > 0) { return 'jobs-panel__pill jobs-panel__pill--paused'; } @@ -944,14 +962,7 @@ {timeAgo(job.last_run_at)}
- - {outcomeLabel(job)} + {outcomeLabel(job)} {#if actionableFindingCount(job) > 0} {@const findings = actionableFindingCount(job)} + {#if isRunning(job)} {t('admin.jobs.state_running', 'running')} + {:else if job.last_run_status} + + + {runStatusLabel(job.last_run_status)} + {:else} — {/if} diff --git a/src/infrastructure/scheduler/registry.rs b/src/infrastructure/scheduler/registry.rs index 951c23c8..7692d982 100644 --- a/src/infrastructure/scheduler/registry.rs +++ b/src/infrastructure/scheduler/registry.rs @@ -249,11 +249,12 @@ impl JobRegistry { last_outcome, running: state.current_run_start.is_some(), recoverable: entry.handler.is_recoverable(), - // Both populated in the `list_jobs` handler — one + // All populated in the `list_jobs` handler — two // from a DB round-trip, one from AppConfig. Kept // out of the registry snapshot so the in-memory // scheduler state pulls in neither dependency. paused_run: None, + last_run_status: None, startup: None, } }) @@ -349,6 +350,18 @@ pub struct JobSummary { /// job, most of which the job ignored with no way to tell. #[serde(skip_serializing_if = "<[_]>::is_empty")] pub parameters: &'static [JobParam], + /// Status of this job's most recent run row, for recoverable jobs. + /// + /// Populated by the `list_jobs` handler from the DB, and it exists + /// because [`Self::last_outcome`] cannot answer this: that field is + /// in-memory, written when a dispatch completes through the engine, + /// so anything changing a run row without running the handler leaves + /// it stale. Cancelling a Paused run is exactly that — a direct SQL + /// flip — and the panel went on showing the pause's outcome. + /// + /// Prefer this over `last_outcome` wherever the two could disagree. + #[serde(skip_serializing_if = "Option::is_none")] + pub last_run_status: Option, #[serde(skip_serializing_if = "Option::is_none")] pub interval_ms: Option, #[serde(skip_serializing_if = "Option::is_none")] diff --git a/src/interfaces/api/handlers/admin_handler.rs b/src/interfaces/api/handlers/admin_handler.rs index 15de53a1..20b03f45 100644 --- a/src/interfaces/api/handlers/admin_handler.rs +++ b/src/interfaces/api/handlers/admin_handler.rs @@ -2570,41 +2570,94 @@ pub async fn list_jobs(State(state): State>) -> impl IntoResponse // failures fall back to the pre-enrichment shape so the endpoint // stays useful when the jobs DB is temporarily unreachable. if let Some(pool) = state.db_pool.as_ref() { - let paused_rows: Vec<(String, uuid::Uuid, Option, Option)> = sqlx::query_as( + // The LATEST run per job, whatever its status — not just the + // paused ones. + // + // `last_outcome` is in-memory, written when a dispatch finishes + // through the engine. Anything that changes a run row WITHOUT + // running the handler leaves it stale: cancelling a Paused run + // is a direct SQL flip to `Cancelled`, so the panel kept + // rendering the outcome of the run that pause belonged to — a + // cancelled job still showing "blocked". + // + // `DISTINCT ON` is safe as "the current run": the + // `one_active_run_per_job` partial unique index allows only one + // non-terminal row per job, and a resume reuses it rather than + // starting a new one, so a non-terminal row is always the newest. + /// `(job_name, status, run_id, started_at, scanned, total)` — the + /// enrichment row shape, named so the query's type stays legible. + type LatestRunRow = ( + String, + String, + uuid::Uuid, + chrono::DateTime, + Option, + Option, + ); + let latest_rows: Vec = sqlx::query_as( r#" - SELECT + SELECT DISTINCT ON (job_name) job_name, + status::TEXT, id, + started_at, (stats ->> 'scanned_count')::BIGINT AS scanned, (params ->> 'total_rows')::BIGINT AS total FROM jobs.recoverable_runs - WHERE status = 'Paused' + ORDER BY job_name, started_at DESC "#, ) .fetch_all(pool.as_ref()) .await .unwrap_or_default(); - let by_name: std::collections::HashMap = paused_rows + type LatestRun = (String, chrono::DateTime, PausedRunBrief); + let by_name: std::collections::HashMap = latest_rows .into_iter() - .map(|(name, id, scanned, total)| { + .map(|(name, status, id, started_at, scanned, total)| { ( name, - PausedRunBrief { - id, - scanned: scanned.unwrap_or(0).max(0) as u64, - total: total.filter(|t| *t > 0).map(|t| t as u64), - }, + ( + status, + started_at, + PausedRunBrief { + id, + scanned: scanned.unwrap_or(0).max(0) as u64, + total: total.filter(|t| *t > 0).map(|t| t as u64), + }, + ), ) }) .collect(); for job in summary.iter_mut() { - if job.recoverable - && !job.running - && let Some(paused) = by_name.get(&job.name) - { - job.paused_run = Some(paused.clone()); + if !job.recoverable { + continue; + } + let Some((status, started_at, brief)) = by_name.get(&job.name) else { + continue; + }; + // Always reported, so the panel can prefer the row's truth + // over the in-memory outcome rather than guessing which is + // fresher. + job.last_run_status = Some(status.clone()); + // Fill the timestamp too when memory has none. + // + // `last_outcome` and `last_run_at` are both in-memory, so a + // restart empties them and the row read "never" for a job + // with real runs in the DB — the opposite failure to the + // stale-outcome one, and just as misleading. The row is + // authoritative for "did this ever run"; memory only adds + // the richer outcome detail when it happens to be warm. + // + // Only when absent: a warm `last_run_at` describes the last + // DISPATCH, which for a non-recoverable tick is finer-grained + // than any run row. + if job.last_run_at.is_none() { + job.last_run_at = Some(*started_at); + } + if !job.running && status == "Paused" { + job.paused_run = Some(brief.clone()); } } }