feat(jobs): jobs describe themselves — description, mutates, repair_description

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) <noreply@anthropic.com>
This commit is contained in:
Edouard Vanbelle
2026-08-29 00:04:12 +02:00
parent b485db46fa
commit 1ea3826660
27 changed files with 906 additions and 134 deletions
+37 -1
View File
@@ -9,7 +9,7 @@
use async_trait::async_trait;
use super::types::{JobOutcome, JobRunArgs};
use super::types::{JobOutcome, JobRunArgs, Mutates};
/// Implemented by every service that wants to run on a fixed interval
/// through the periodic scheduler.
@@ -92,4 +92,40 @@ pub trait JobHandler: Send + Sync {
fn is_recoverable(&self) -> bool {
false
}
/// What this job does, in one or two sentences, for the admin UI.
///
/// English, in the trait, beside the behaviour it describes — not in
/// `locales/*.json`. A description that lives away from the code 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 this as the fallback, so a missing
/// translation degrades to English rather than to a blank panel.
///
/// Defaulted to `""` so adding it to the existing jobs is incremental
/// rather than one breaking change; the UI omits the line when empty.
fn description(&self) -> &'static str {
""
}
/// Whether a run changes state, and under what conditions. See
/// [`Mutates`] for why this is not a boolean.
fn mutates(&self) -> Mutates {
Mutates::Never
}
/// `Some(..)` when `?repair=true` does something beyond a default run,
/// describing what it ADDS; `None` when the flag is inert.
///
/// One method rather than a `supports_repair` boolean plus prose: its
/// presence drives whether the UI offers the toggle, its content drives
/// the confirmation text. A boolean would leave the frontend to invent
/// wording for a destructive action it does not understand.
///
/// Independent of [`Self::mutates`], not derived from it — the thumbnail
/// import jobs are [`Mutates::Always`] *and* repair-capable, inserting
/// rows on a plain run and additionally unlinking sidecars under repair.
fn repair_description(&self) -> Option<&'static str> {
None
}
}
+1 -1
View File
@@ -38,4 +38,4 @@ pub use recoverable::{
record_or_log, run_or_resume,
};
pub use registry::{JobEntry, JobRegistry, JobSummary, PausedRunBrief, RegisterError};
pub use types::{ErrCause, JobOutcome, JobRunArgs};
pub use types::{ErrCause, JobOutcome, JobRunArgs, Mutates};
+107 -2
View File
@@ -48,7 +48,7 @@ use uuid::Uuid;
use crate::common::errors::DomainError;
use super::handler::JobHandler;
use super::types::{JobOutcome, JobRunArgs};
use super::types::{JobOutcome, JobRunArgs, Mutates};
// ─── Run status ─────────────────────────────────────────────────────────────
@@ -238,6 +238,47 @@ pub trait RecoverableJobHandler: Send + Sync {
/// URL fragment: `POST /api/admin/jobs/{name}/trigger`.
fn name(&self) -> &str;
/// What this job does, for the admin UI.
///
/// English, in the trait, beside the behaviour it describes — not in
/// `locales/*.json`. A description that lives away from the code 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 this as the fallback, so a missing translation degrades
/// to English rather than a blank panel.
///
/// Defaulted so adding it to ~15 existing jobs is incremental rather than
/// one breaking change.
fn description(&self) -> &'static str {
""
}
/// Whether a run changes state, and under what conditions.
///
/// Three values rather than a boolean because there are three cases, and
/// the interesting one is conditional: a job can be read-only by default
/// and destructive under `?repair=true`. A boolean forces that job to
/// answer wrongly for one of its two modes — `false` on something that
/// can delete files is actively misleading.
fn mutates(&self) -> Mutates {
Mutates::Never
}
/// `Some(..)` when `?repair=true` does something beyond a default run,
/// describing what it ADDS; `None` when the flag is inert.
///
/// One method rather than a `supports_repair` boolean plus prose: its
/// presence drives whether the UI offers the toggle, its content drives
/// the confirmation text. A boolean would leave the frontend to invent
/// wording for a destructive action it does not understand.
///
/// Independent of [`Self::mutates`], not derived from it — the import
/// jobs are [`Mutates::Always`] *and* repair-capable, inserting rows on a
/// plain run and additionally unlinking files under repair.
fn repair_description(&self) -> Option<&'static str> {
None
}
/// Long-running scan. See trait-level doc for the contract.
///
/// `store` — bound to THIS run (a single row in
@@ -361,7 +402,15 @@ pub trait JobStore: Send + Sync {
/// `"stale_used_bytes"`, `"missing_blob"`). Never rename across
/// releases; new failure modes get new values.
///
/// `severity` — one of `"data_loss"`, `"inconsistent"`, `"anomaly"`.
/// `severity` — one of:
/// - `"data_loss"` — bytes / rows unreachable or gone.
/// - `"inconsistent"` — counters or materialised values wrong,
/// content intact.
/// - `"anomaly"` — surprising state worth surfacing, no known impact.
/// This is the level the admin panel labels "notices"; there is no
/// separate `notice` severity, and a job that acted on what it found
/// says so in `detail` rather than in a fourth severity that would
/// render identically.
///
/// `resource_id` — the file / folder / drive / blob the finding
/// pertains to. `None` for run-wide findings (e.g. "backend
@@ -993,6 +1042,21 @@ impl JobHandler for RecoverableAdapter {
// downstream.
true
}
// The registry only ever sees `dyn JobHandler`, so the tenant's own
// metadata has to be forwarded through the wrapper or it is invisible
// to `GET /api/admin/jobs`. Silently returning the JobHandler defaults
// here would leave every recoverable job undescribed and reported as
// read-only — including ones that delete files.
fn description(&self) -> &'static str {
self.inner.description()
}
fn mutates(&self) -> Mutates {
self.inner.mutates()
}
fn repair_description(&self) -> Option<&'static str> {
self.inner.repair_description()
}
}
// ─── Ergonomics: JobRegistry extension for recoverable jobs ─────────────────
@@ -1504,6 +1568,47 @@ mod tests {
// ─── Tests ─────────────────────────────────────────────────────────────
/// The registry only ever sees `dyn JobHandler`, so a recoverable
/// tenant's metadata reaches `GET /api/admin/jobs` only if the adapter
/// forwards it. Falling back to the `JobHandler` defaults here would
/// report every recoverable job as undescribed and read-only —
/// including the imports, which delete files under repair.
#[tokio::test]
async fn adapter_forwards_job_metadata_from_inner_handler() {
struct Annotated;
#[async_trait]
impl RecoverableJobHandler for Annotated {
fn name(&self) -> &str {
"annotated"
}
async fn run_resumable(
&self,
_store: &dyn JobStore,
_args: &JobRunArgs,
_resume_cursor: Option<Vec<u8>>,
) -> RunOutcome {
RunOutcome::completed()
}
fn description(&self) -> &'static str {
"walks a thing"
}
fn mutates(&self) -> Mutates {
Mutates::OnRepairOnly
}
fn repair_description(&self) -> Option<&'static str> {
Some("fixes the thing")
}
}
let provider: Arc<dyn JobStoreProvider> = Arc::new(MemProvider::new());
let adapter = RecoverableAdapter::new(Arc::new(Annotated), provider);
let as_handler: &dyn JobHandler = &adapter;
assert_eq!(as_handler.description(), "walks a thing");
assert_eq!(as_handler.mutates(), Mutates::OnRepairOnly);
assert_eq!(as_handler.repair_description(), Some("fixes the thing"));
}
#[tokio::test]
async fn fresh_run_completes_and_marks_status_completed() {
let provider = Arc::new(MemProvider::new());
+99 -1
View File
@@ -20,7 +20,7 @@ use serde::Serialize;
use tokio::sync::{RwLock, Semaphore};
use super::handler::JobHandler;
use super::types::{JobOutcome, JobRunArgs};
use super::types::{JobOutcome, JobRunArgs, Mutates};
/// A registered job plus its runtime state. Held as `Arc<JobEntry>`
/// inside the registry so the engine can hold a snapshot across an
@@ -135,6 +135,13 @@ impl JobRegistry {
timeout: Option<Duration>,
) -> Result<(), RegisterError> {
let name = handler.name().to_string();
// A job declaring it mutates only under a flag it does not support
// is self-contradictory, and the UI would render it as safe with no
// way to reach the mutating path. Cheap to catch here, invisible
// otherwise.
if handler.mutates() == Mutates::OnRepairOnly && handler.repair_description().is_none() {
return Err(RegisterError::RepairOnlyWithoutRepair(name));
}
let mut guard = self.entries.write().await;
if guard.contains_key(&name) {
return Err(RegisterError::DuplicateName(name));
@@ -220,6 +227,9 @@ impl JobRegistry {
};
JobSummary {
name,
description: entry.handler.description(),
mutates: entry.handler.mutates(),
repair_description: entry.handler.repair_description(),
interval_ms: entry.interval.map(|d| d.as_millis() as u64),
next_run_at: state.next_run_at,
last_run_at,
@@ -283,6 +293,11 @@ impl Default for JobRegistry {
pub enum RegisterError {
#[error("job name already registered: {0}")]
DuplicateName(String),
#[error(
"job {0} declares mutates = OnRepairOnly but no repair_description() — \
it claims to mutate only under a flag it does not support"
)]
RepairOnlyWithoutRepair(String),
}
/// Per-job row in the `GET /api/admin/jobs` response.
@@ -304,6 +319,17 @@ pub enum RegisterError {
#[derive(Debug, Clone, Serialize)]
pub struct JobSummary {
pub name: String,
/// One or two sentences on what the job does. Empty for jobs that
/// haven't declared one yet — the UI omits the line rather than
/// rendering a blank block.
#[serde(skip_serializing_if = "str::is_empty")]
pub description: &'static str,
pub mutates: Mutates,
/// `Some` iff the job does something extra under `?repair=true`.
/// Presence is what gates the repair toggle in the UI; the string
/// is the confirmation text.
#[serde(skip_serializing_if = "Option::is_none")]
pub repair_description: Option<&'static str>,
#[serde(skip_serializing_if = "Option::is_none")]
pub interval_ms: Option<u64>,
#[serde(skip_serializing_if = "Option::is_none")]
@@ -393,6 +419,78 @@ mod tests {
assert!(matches!(err, RegisterError::DuplicateName(_)));
}
/// A job declaring `OnRepairOnly` without a `repair_description` has
/// no reachable mutating path — the UI gates the repair toggle on
/// that string's presence, so the job would render as safe and stay
/// read-only forever. Catch it at wiring time rather than let it read
/// as a working configuration.
#[tokio::test]
async fn repair_only_without_repair_description_rejected() {
struct Contradictory;
#[async_trait]
impl JobHandler for Contradictory {
fn name(&self) -> &str {
"contradictory"
}
async fn run(&self, _args: &JobRunArgs) -> JobOutcome {
JobOutcome::ok(0)
}
fn mutates(&self) -> Mutates {
Mutates::OnRepairOnly
}
// repair_description() left at its `None` default — the bug.
}
let reg = JobRegistry::new();
let err = reg
.try_register(Arc::new(Contradictory), None, None)
.await
.expect_err("OnRepairOnly without a repair_description must be rejected");
assert!(matches!(err, RegisterError::RepairOnlyWithoutRepair(_)));
}
/// The registry hands `dyn JobHandler` to the admin snapshot, so a
/// tenant's own metadata is only visible if it survives that erasure.
#[tokio::test]
async fn snapshot_carries_job_metadata() {
struct Described;
#[async_trait]
impl JobHandler for Described {
fn name(&self) -> &str {
"described"
}
async fn run(&self, _args: &JobRunArgs) -> JobOutcome {
JobOutcome::ok(0)
}
fn description(&self) -> &'static str {
"does a thing"
}
fn mutates(&self) -> Mutates {
Mutates::Always
}
fn repair_description(&self) -> Option<&'static str> {
Some("also deletes the thing")
}
}
let reg = JobRegistry::new();
reg.register(Arc::new(Described), None, None).await;
let snap = reg.snapshot().await;
let row = snap.iter().find(|j| j.name == "described").unwrap();
assert_eq!(row.description, "does a thing");
assert_eq!(row.mutates, Mutates::Always);
assert_eq!(row.repair_description, Some("also deletes the thing"));
// Undeclared jobs stay at the safe defaults so the panel can tell
// "read-only" from "not yet described" — empty string, not prose.
reg.register(handler("bare"), None, None).await;
let snap = reg.snapshot().await;
let bare = snap.iter().find(|j| j.name == "bare").unwrap();
assert_eq!(bare.description, "");
assert_eq!(bare.mutates, Mutates::Never);
assert!(bare.repair_description.is_none());
}
#[tokio::test]
#[should_panic(expected = "DI wiring bug")]
async fn register_panics_on_duplicate() {
+42
View File
@@ -167,10 +167,52 @@ impl fmt::Display for ErrCause {
}
}
/// When a job changes state.
///
/// Drives how the admin UI presents a trigger: `Never` earns a read-only
/// badge, `OnRepairOnly` is safe to run and warns only when the toggle is on,
/// `Always` warns regardless.
///
/// Three values rather than a boolean because there are three cases, and the
/// interesting one is conditional. `false` on a job that can delete files
/// under `?repair=true` is actively misleading; `true` on one that is
/// read-only by default is equally wrong. `OnRepairOnly` names the case a
/// boolean cannot, and it is where the recovery framework is heading —
/// discovery-only by default, mutation behind an explicit opt-in — so a
/// consistency tenant that later grows a repair arm changes this one value
/// and nothing else.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)]
#[serde(rename_all = "snake_case")]
pub enum Mutates {
/// Read-only under every flag. All consistency tenants.
Never,
/// Changes state on a plain run. GC, janitors, the import jobs.
Always,
/// Read-only by default; mutates only under `?repair=true`. Pairing this
/// with `repair_description() == None` is contradictory — a job claiming
/// it mutates only under a flag it does not support.
OnRepairOnly,
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn mutates_serialises_snake_case() {
// The admin UI switches on these strings — a rename is a breaking
// change to the panel, not just to Rust callers.
assert_eq!(serde_json::to_string(&Mutates::Never).unwrap(), "\"never\"");
assert_eq!(
serde_json::to_string(&Mutates::Always).unwrap(),
"\"always\""
);
assert_eq!(
serde_json::to_string(&Mutates::OnRepairOnly).unwrap(),
"\"on_repair_only\""
);
}
#[test]
fn joboutcome_kind_label() {
assert_eq!(JobOutcome::ok(0).kind(), "ok");