diff --git a/docs/plan/job-registry.md b/docs/plan/job-registry.md index ebc30e6e..98c40617 100644 --- a/docs/plan/job-registry.md +++ b/docs/plan/job-registry.md @@ -171,6 +171,58 @@ Native services implement this trait on an existing service type (no new wrapper) and register a single `Arc` with the scheduler. +### Self-description — `description` / `mutates` / `repair_description` + +Three defaulted methods on both `JobHandler` and `RecoverableJobHandler` +let a job tell the admin UI what it is. `RecoverableAdapter` forwards +them, since the registry only ever holds `dyn JobHandler`. + +```rust +fn description(&self) -> &'static str { "" } +fn mutates(&self) -> Mutates { Mutates::Never } +fn repair_description(&self) -> Option<&'static str> { None } + +pub enum Mutates { Never, Always, OnRepairOnly } +``` + +They surface on `JobSummary` (`GET /api/admin/jobs`) and drive the +panel: `Never` earns a read-only badge and triggers straight through, +`Always` confirms first, `OnRepairOnly` is safe to run and confirms only +when the repair variant is picked. `repair_description.is_some()` is +what renders the repair toggle at all, and its text is the confirmation +copy. + +**Why three values and not a boolean.** A job can be read-only by +default and destructive under `?repair=true`; a boolean has to answer +wrongly for one of those two modes, and `false` on something that +deletes files is the dangerous direction to be wrong in. It is also +where the recovery framework is heading — discovery-only default, +mutation behind an opt-in — so a tenant that later grows a repair arm +changes this one value and nothing else. + +**Why `Option<&str>` and not `supports_repair: bool` + prose.** +Presence gates the toggle, content supplies the wording. Split across +two methods they can disagree; and the frontend cannot invent the +wording itself, because correcting a counter and unlinking files off +disk are not the same warning. The two are independent, not derived +from each other: the thumbnail imports are `Always` *and* +repair-capable. + +`OnRepairOnly` with no `repair_description` is rejected at registration +— it claims to mutate only under a flag it does not support, and would +render as safe with no reachable mutating path. + +**Why English in the trait, not `locales/*.json`.** A description that +lives away from the behaviour rots the moment a job changes, invisibly, +and a translator cannot know what `manifests_consistency` reconciles. +i18n can layer on later keyed by job name with these as the fallback, +matching the frontend's `t(key, params, fallback)` — a missing +translation then degrades to English from code rather than to a blank +panel. No rework needed to get there. + +Defaults exist so the methods could be added without touching every +job at once; every registered job declares all three today. + ### `JobOutcome` ```rust diff --git a/frontend/src/lib/api/types.ts b/frontend/src/lib/api/types.ts index 9c967495..e3757566 100644 --- a/frontend/src/lib/api/types.ts +++ b/frontend/src/lib/api/types.ts @@ -618,8 +618,32 @@ export interface PausedRunBrief { total?: number; } +/** + * When a job changes state — `RecoverableJobHandler::mutates()` on the + * backend. Three values rather than a boolean because the interesting + * case is conditional: a job can be read-only by default and destructive + * under `?repair=true`. + * + * - `never` — read-only under every flag. Render a read-only badge; no + * confirmation needed to trigger. + * - `always` — changes state on a plain run. Confirm before triggering. + * - `on_repair_only` — safe to trigger; confirm only when the repair + * toggle is on. + */ +export type Mutates = 'never' | 'always' | 'on_repair_only'; + export interface JobSummary { name: string; + /** One or two sentences on what the job does, in English, authored + * next to the handler. Absent for jobs that haven't declared one — + * omit the line rather than rendering an empty block. */ + description?: string; + mutates: Mutates; + /** Present iff `?repair=true` does something beyond a default run; + * describes what it ADDS. Presence is what gates the repair toggle; + * the text is the confirmation copy. Independent of `mutates` — the + * thumbnail import jobs are `always` AND repair-capable. */ + repair_description?: string; interval_ms?: number; next_run_at?: string; last_run_at?: string; diff --git a/frontend/src/lib/components/AdminJobsPanel.svelte b/frontend/src/lib/components/AdminJobsPanel.svelte index d247ba58..e7e0a778 100644 --- a/frontend/src/lib/components/AdminJobsPanel.svelte +++ b/frontend/src/lib/components/AdminJobsPanel.svelte @@ -167,7 +167,7 @@ .slice() // `consistency_batch` is served by the top-bar // action buttons; hiding it here removes the - // duplicate table row. `hasBatch` still checks the + // duplicate table row. `batchJob` still reads from the // full fetched list so the top buttons only render // when the coordinator is actually registered. .filter((j) => j.name !== 'consistency_batch') @@ -180,7 +180,7 @@ // Track whether the coordinator is registered so the // top-bar buttons can gate on it without checking `jobs` // (which now filters it out). - hasBatch = fetched.some((j) => j.name === 'consistency_batch'); + batchJob = fetched.find((j) => j.name === 'consistency_batch') ?? null; loadError = null; } catch (e) { loadError = errorMessage(e); @@ -250,13 +250,15 @@ // ─── Expansion toggles ───────────────────────────────────────────── - function toggleJob(name: string) { - if (expandedJob === name) { + function toggleJob(job: JobSummary) { + if (expandedJob === job.name) { expandedJob = null; } else { - expandedJob = name; - // Lazy-load on first open, refresh on subsequent opens. - void loadRuns(name); + expandedJob = job.name; + // Lazy-load on first open, refresh on subsequent opens. Only + // recoverable jobs have runs to load — the others expand purely + // to show their description. + if (isRecoverable(job)) void loadRuns(job.name); } } @@ -458,8 +460,9 @@ * Per-severity finding counts from `last_outcome.extra.severity_counts` * (a JSON object populated by `run_or_resume`). Missing / older * runs return an empty record — callers should tolerate absent keys. - * The three severity values are the ones consistency tenants emit - * today: `data_loss`, `inconsistent`, `anomaly`. + * Severity values emitted today: `data_loss`, `inconsistent`, + * `anomaly`. The set is open (the column is TEXT), so unknown keys + * must degrade rather than throw. */ function lastSeverityCounts(job: JobSummary): Record { if (!job.last_outcome || job.last_outcome.outcome !== 'ok') return {}; @@ -480,6 +483,13 @@ return (s.data_loss ?? 0) + (s.inconsistent ?? 0); } + /** + * Informational findings. `anomaly` is the wire value; "notice" is + * what the panel calls it — there is no separate `notice` severity. + * A job that acted on what it found (a repair run deleting an + * orphaned sidecar) records the same severity and says so in the + * finding's `detail`. + */ function anomalyFindingCount(job: JobSummary): number { return lastSeverityCounts(job).anomaly ?? 0; } @@ -636,29 +646,65 @@ return name === 'consistency_batch' || name === 'blobs_consistency'; } - // Jobs whose handler consults `args.repair` and applies a - // corrective UPDATE against the finding it just emitted. Only the - // two ref_count tenants today; `consistency_batch` also accepts - // the flag (fans out to both) and is surfaced separately as the - // top-bar "Repair ref_counts" button. Keep this list narrow — - // adding a job here without a matching backend handler produces a - // silently no-op button that confuses operators. - function supportsRepair(name: string): boolean { - return name === 'blobs_consistency' || name === 'manifests_consistency'; + // Whether `?repair=true` does anything for this job — declared by the + // handler itself via `repair_description()`, not by a name allowlist + // here. The allowlist this replaces named only the two ref_count + // tenants and silently omitted every repair-capable job added since, + // so the thumbnail imports could not be run in repair mode from the + // panel at all despite supporting it. + function supportsRepair(job: JobSummary): boolean { + return !!job.repair_description; } - async function onTriggerWithRepairConfirm(name: string) { + // What the repair adds, in the handler's own words. The backend owns + // this string precisely because the wording differs per job: correcting + // a counter and unlinking files off disk are not the same warning, and + // the frontend has no way to tell them apart. + async function onTriggerWithRepairConfirm(job: JobSummary) { const ok = await confirmDialog({ - title: t('admin.jobs.run_repair_confirm_title', 'Repair drifted ref_counts?'), - message: t( - 'admin.jobs.run_repair_confirm_body_scoped', - { name }, - 'Runs {{name}} and applies a corrective UPDATE to any counter that disagrees with its live reference count. Content-safe: only counters change; blob content and file rows are untouched.' + title: t( + 'admin.jobs.run_repair_confirm_title_scoped', + { name: job.name }, + 'Run {{name}} in repair mode?' ), + message: job.repair_description ?? '', confirmText: t('admin.jobs.run_repair_confirm', 'Repair'), danger: true }); - if (ok) await onTrigger(name, { repair: true }); + if (ok) await onTrigger(job.name, { repair: true }); + } + + // Confirmation before a plain run of a job that writes. `never` jobs + // trigger straight through — that is the point of the flag — and + // `on_repair_only` jobs are read-only until the repair variant is + // picked, which carries its own confirm. + async function onTriggerGuarded(job: JobSummary) { + if (job.mutates === 'always') { + const ok = await confirmDialog({ + title: t('admin.jobs.run_mutating_confirm_title', { name: job.name }, 'Run {{name}}?'), + message: + job.description || + t('admin.jobs.run_mutating_confirm_body', 'This job changes stored state when it runs.'), + confirmText: t('admin.jobs.run', 'Run'), + danger: true + }); + if (!ok) return; + } + await onTrigger(job.name); + } + + // Row badge. `never` is the one worth stating outright — it is the + // answer to "is it safe to click this on production?", and it is the + // question an operator asks before every trigger. + function mutatesLabel(job: JobSummary): string | null { + switch (job.mutates) { + case 'never': + return t('admin.jobs.mutates_never', 'read-only'); + case 'on_repair_only': + return t('admin.jobs.mutates_on_repair_only', 'read-only unless repaired'); + default: + return null; + } } function isRunning(job: JobSummary): boolean { @@ -679,11 +725,13 @@ // coordinator is registered (should always be true post-Slice 5, // but check defensively so the button doesn't appear on an old // deployment before this component is upgraded). - // Coordinator registration flag — set imperatively in - // `loadJobs` because `jobs` no longer contains the - // `consistency_batch` row (filtered out to avoid duplicating the - // top-bar action buttons). - let hasBatch = $state(false); + // Held as the whole summary rather than a boolean because the + // top-bar buttons need its `repair_description` — the coordinator + // describes its own repair semantics, same as every table row. + // Set imperatively in `loadJobs` because `jobs` no longer contains + // the `consistency_batch` row (filtered out to avoid duplicating + // the top-bar action buttons). + let batchJob = $state(null);
@@ -697,10 +745,12 @@

- {#if hasBatch} + {#if batchJob} + {@const batch = batchJob} - - + + {#if batch.repair_description} + + {/if} {/if} {/if} - {#if supportsRepair(job.name)} + {#if supportsRepair(job)}