From 1ea382666032b8daa02e365f1eec10d9cbffa6cf Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Sat, 29 Aug 2026 00:04:12 +0200 Subject: [PATCH] =?UTF-8?q?feat(jobs):=20jobs=20describe=20themselves=20?= =?UTF-8?q?=E2=80=94=20description,=20mutates,=20repair=5Fdescription?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The admin panel had no repair toggle wired to anything but a hardcoded name list naming the two refcount tenants, so `thumb_derived_import` and `thumb_attached_import` could not be run in repair mode from the UI at all despite supporting it. And nothing in the job list said what any given job does or whether clicking Run on production writes anything. Three defaulted methods on `JobHandler` and `RecoverableJobHandler`: fn description(&self) -> &'static str fn mutates(&self) -> Mutates // Never | Always | OnRepairOnly fn repair_description(&self) -> Option<&'static str> `RecoverableAdapter` forwards them — the registry only holds `dyn JobHandler`, so a tenant's metadata is invisible otherwise, and falling back to the defaults would report every recoverable job as read-only, including the ones that delete files. Three values rather than a boolean because a job can be read-only by default and destructive under `?repair=true`; a boolean answers wrongly for one of its two modes, and `false` on something that unlinks files is the dangerous direction to be wrong in. `repair_description` returning `Option` collapses "does it repair" and "what does repair do" into one method: presence gates the toggle, content is the confirmation text — which the frontend cannot invent, since correcting a counter and deleting sidecars are not the same warning. `OnRepairOnly` with no `repair_description` is rejected at registration: it claims to mutate only under a flag it does not support. All 17 registered jobs declare all three. The panel now renders the description under each name, badges read-only jobs, confirms before a plain run of a mutating one, and offers the repair variant off the backend flag instead of the name list. Descriptions are English in the trait, next to the behaviour: one in `locales/*.json` rots invisibly the moment a job changes, and a translator cannot know what `manifests_consistency` reconciles. i18n can layer on later keyed by job name with these as the fallback. Co-Authored-By: Claude Opus 5 (1M context) --- docs/plan/job-registry.md | 52 ++++ frontend/src/lib/api/types.ts | 24 ++ .../src/lib/components/AdminJobsPanel.svelte | 242 ++++++++++++------ frontend/static/locales/en.json | 8 +- .../services/storage_usage_service.rs | 15 +- src/infrastructure/scheduler/handler.rs | 38 ++- src/infrastructure/scheduler/mod.rs | 2 +- src/infrastructure/scheduler/recoverable.rs | 109 +++++++- src/infrastructure/scheduler/registry.rs | 100 +++++++- src/infrastructure/scheduler/types.rs | 42 +++ .../services/backend_consistency_service.rs | 8 + .../services/backend_migration_service.rs | 18 +- .../services/backend_rotate_service.rs | 18 +- .../services/blobs_consistency_service.rs | 27 +- .../services/consistency_batch_service.rs | 24 +- src/infrastructure/services/dedup_service.rs | 14 + .../services/drives_consistency_service.rs | 7 + .../services/files_consistency_service.rs | 7 + .../services/folders_consistency_service.rs | 8 + .../services/grant_cleanup_service.rs | 14 +- .../services/manifests_consistency_service.rs | 24 +- .../satellites_consistency_service.rs | 9 + .../services/session_cleanup_service.rs | 13 +- .../services/thumb_attached_import_service.rs | 82 ++++-- .../services/thumb_derived_import_service.rs | 88 +++++-- .../services/trash_cleanup_service.rs | 13 +- tests/api/admin_jobs.hurl | 34 +++ 27 files changed, 906 insertions(+), 134 deletions(-) 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)}