feat(public-server): U1: build reporting schemas per group and version

[dannash100]

Reporting schemas are built against a database nobody records, so groups sit minors behind the schema they run and nothing says which one an application is on.

• Canopy derives the (group, version) pairs from what applications report running and the open plan, dispatches each as a restore replica, and takes the result in as a group-scoped artifact it offers back.
• once is keyed to the pair, not the snapshot, so a newer backup doesn't rebuild a schema the pair has. A failed build settles it too; an operator asking reinstates it.
• The builder is the restore consumer, authorised for the groups its own declaration covers. This is the registrar ART deliberately left out.

QA

• Reporting schemas local basic checks

🦸 Review Hero

• Run Review Hero

[dannash100]

🤖 What's waiting on this merging and deploying:

• pgro can't report a build outcome yet. Its wire types are generated from live canopy's OpenAPI at compile time, so VerificationArgs.reporting_schema exists for pgro only once this is deployed. Until then no pair settles, the worklist re-dispatches every pair on every poll, and a failed build raises no check at all, since the alerting sweep filters Awaiting out. Noted on pgro#135.
• The builder in the middle doesn't exist. build_reporting_assets.py in tamanu-source-dbt needs a headless mode and there's no PR for it anywhere. Noted on deployment-scoped artifacts and reporting epic #551; wants a card in MAUI.

Order: #552, then this, deploy, rebuild pgro, then one line on its report builder.

[review-hero]

🦸 Review Hero (could not post inline comments — showing here instead)

crates/database/src/reporting_schemas.rs:478

[Bugs & Correctness] suggestion

An open reporting-schema warning can never be cleared once a group leaves the sweep. Both early exits — !group_builds_schemas (line 477) and no canonical central (line 482) — continue before any filing, so an issue already active on the group's central stays active forever. Failure scenario: a build fails, the sweep files the Warning on central, then an operator disables the declaration (or clears publishes_schemas, or retires the builder). Every later sweep skips the group, and the operator is left with a standing warning about a schema Canopy no longer owes and no action that can clear it. The same happens if the canonical central changes (a higher-ranked central is added, or the old one is archived): the issue is filed against the new central's id, and the old one's issue is orphaned. file_restore_check's instances.is_empty() branch — with the gone message this code already defines — exists exactly for this; the group should be swept with empty instances rather than skipped. The PR's the_check_closes_once_the_group_owes_no_schema test only covers the build row being deleted, which still goes through the filing path.

---

crates/private-server/src/fns/restore_replicas.rs:317

[Bugs & Correctness] suggestion

The three publishes_schemas guards are unreachable when the consumer hasn't advertised the intent: normalized_params_for_intent returns at line 297-299 (let Some(desc) = ... else { return Ok(params.clone()) }) before them. Failure scenario: an operator creates a declaration for an intent the consumer no longer advertises (an intent gap, which this codebase treats as a normal transient state) with machine_id set and publishes_schemas: true; the write succeeds, the row is stored, and to_views shows the "publishes schema" chip. Nothing is ever dispatched or authorised for it, because authorizes_schema_artifacts independently requires the semantic and machine_id IS NULL — so the operator reads a grant that does not exist. Move the machine_id.is_some() and redacts checks (which don't depend on the descriptor) above the early return, and refuse publishes_schemas outright when the intent is not advertised.

---

crates/database/src/reporting_schemas.rs:104

[Bugs & Correctness] suggestion

ReportingSchemaBuild::record clears the operator's ask on any successful-restore build report for the pair, including one dispatched before the ask was made. Concretely: pair is unbuilt, a build is dispatched at T0 and takes an hour; the operator changes the group's configuration and asks for a build at T1; the in-flight build (restored from the pre-change state) reports at T2 and both records a build and deletes the request — the pair settles and the schema the operator asked for is never built, with nothing on screen to say so. reporting_schema_requests.requested_at is already stored, so clear only asks older than the run that answered them (e.g. compare against the report's observed_at, or the restore's start), rather than unconditionally.

---

crates/database/src/restore.rs:379

[Design & Architecture] suggestion

run_claimed_elsewhere hangs off RestoreReplica but never touches restore_replicas — it queries backup_restore_checks and backup_runs, and its subject is a run's provenance, not a replica declaration. It belongs alongside the run/report models (crates/database/src/backups.rs or restore's check types) where the tables it reads live; on RestoreReplica it's discoverable only by whoever already knows the artifacts handler calls it.

---

crates/public-server/src/artifacts.rs:180

[Design & Architecture] suggestion

create now serves two different resources through one path: a releaser POSTing a plain-text URL, and a builder POSTing the artifact's raw bytes. The meaning of the request body flips on the presence of ?group=, and the handler grew a ~60-line preamble that hand-rolls role policy (matches!(role, Releaser | Admin), role == Admin || authorizes_schema_artifacts(..)) after dropping the ReleaserDevice extractor. That abandons the codebase's convention of encoding "who may call this" in the extractor type (device_role_struct! in commons-servers/src/device_auth/mod.rs), so the authorisation for this endpoint is no longer visible in its signature and can't be reused or audited alongside the others. Consider a second route for the bytes-carrying, group-scoped registration (adding a path is compatible under the API rules) with its own extractor, leaving create as the releaser's URL registration it already was; or at minimum extract the group branch into a named authorise_group_registration(...) so the handler body reads as one thing.

---

crates/public-server/src/restore.rs:322

[Design & Architecture] suggestion

"This declaration may build/publish a group's schema" is now spelled out three times in three shapes: as SQL in RestoreReplica::authorizes_schema_artifacts (enabled + publishes_schemas + !redacts + machine_id IS NULL + advertised semantic), as inline continue guards in the worklist (!d.publishes_schemas, d.redacts, d.machine_id.is_some()), and as request validation in normalized_params_for_intent. The comments assert dispatch and acceptance must agree — "Canopy asks for no build it would refuse the result of" — but nothing structurally enforces that; adding a fourth condition to one copy silently diverges the others. Give RestoreReplica a single predicate over a loaded row (e.g. fn may_publish_schema(&self, descriptor: &IntentDescriptor) -> bool) and have the SQL query, the worklist, and the validator all funnel through it.

---

crates/public-server/src/restore.rs:316

[Design & Architecture] suggestion

The if builds_schema { ... continue; } block is ~110 lines of a distinct dispatch strategy (per-pair, group-central, version-list) inlined into the middle of the per-declaration loop of an already long worklist handler, and it introduces three more function-scope caches (pairs, version_cache, settlement_cache) alongside the existing per-machine ones. version_cache and settlement_cache are always populated and read together, so they're one value split across two maps kept in sync by hand (settlement_cache[&d.group_id] panics if that invariant ever breaks). Extracting async fn schema_build_entries(conn, &d, &cfg, &snapshots, &mut cache) -> Result<Vec<WorklistEntry>> with a single HashMap<Uuid, GroupPairs { versions, settlement }> would make the per-machine path readable again and put the pair-dispatch rules in one place.

---

crates/database/src/artifacts.rs:330

[Design & Architecture] suggestion

newest_change_for_version (singular) has no caller anywhere in the tree — it exists only to wrap the plural form, and does so at the cost of an extra Version::get_by_id round trip. Similarly ReportingSchemaBuild::is_settled and latest_for_pair are public API exercised only by tests (production code goes through Settlement and latest_by_version_for_group). Generalising ahead of a second caller means three surfaces to keep correct where one is used; drop the singular wrapper, and either make the two test-only helpers pub(crate)/test helpers or let the tests assert through Settlement as the worklist does.

---

crates/public-server/src/restore.rs:356

[Performance] suggestion

In the schema-build branch the per-group work is only half cached: version_cache/settlement_cache are keyed by group, but Application::list_live_in_group and Machine::get_by_id run once per declaration, so a group covered by two schema declarations pays both queries twice even though pairs then discards the duplicate entries (exactly the case a_second_declaration_dispatches_no_second_build covers). Worse, the machines resolution above (line ~270-283, Machine::list_for_group for a group-wide declaration) has already run and its result is thrown away for a schema declaration. Move the if builds_schema block above the machines/snapshot_cache resolution, and cache the resolved (central, machine) per group alongside version_cache.

---

crates/database/src/artifacts.rs:366

[Performance] suggestion

The range half of newest_change_for_versions loads every unscoped range artifact in the table (no LIMIT, no per-version predicate) and re-parses each version_range_pattern with node_semver::Range::parse, then tests it against every requested version. The result is group-independent, yet Settlement::for_group calls this once per group, so a worklist poll for a consumer covering N groups repeats the same full scan and the same N×R semver parses N times — and R grows monotonically with every range artifact ever registered. Either hoist the range lookup out of Settlement::for_group (compute MAX(updated_at) per pattern once per worklist call and pass it in), or collapse it in SQL to SELECT version_range_pattern, MAX(updated_at) GROUP BY version_range_pattern so at least the parse count is per distinct pattern rather than per row.

---

crates/public-server/src/artifacts.rs:105

[Performance] suggestion

DefaultBodyLimit::max(MAX_HELD_ARTIFACT_BYTES + 64 KiB) is applied to the whole route, so the releaser path — whose body is a plain download URL — now also accepts and fully buffers a 32 MiB body before the handler ever looks at named.group, raising the per-request memory ceiling for unscoped registrations from axum's 2 MiB to 32 MiB with no bound on concurrent registrations. On the held path the 32 MiB Bytes is then copied again via Vec::from(body) and hashed with digest_of inline on the async task (a synchronous ~100 ms SHA-256 over 32 MiB, which stalls the runtime worker). Consider limiting the generous body budget to the group-scoped case (e.g. a smaller default limit plus a RequestBodyLimit/manual read for the held path) and moving the digest into spawn_blocking.

---

crates/database/src/reporting_schemas.rs:476

[Performance] critical

sweep is an N+1 (really N×M) over the whole fleet on every backup sweep. For each of ServerGroup::list_all, group_builds_schemas runs one list_for_group query plus two more per distinct consumer (list_for_consumer + the count in authorizes_schema_artifacts), and a group that does have a builder then pays another ~6 queries (list_live_in_group, last_versions, get_by_versions, planned_target, latest_by_version_for_group, pending_for_group) plus the filing. With a few hundred groups each carrying a handful of restore declarations that is well over a thousand round-trips per sweep, and every group pays the probe even though almost none have a builder. Resolve the publishing groups up front with one join (restore_replicas × restore_consumer_capabilities filtered on enabled AND publishes_schemas AND NOT redacts AND machine_id IS NULL and the semantics containment), then iterate only those groups and batch the per-group lookups by group id.

---

crates/public-server/src/artifacts.rs:343

[Security] suggestion

run_id is now accepted from the query string on both registration paths, but run_claimed_elsewhere is only consulted inside the Some(group) arm (line 226). An unscoped registration (releaser or admin credential) therefore writes run_id: named.run with no check at all, so it can stamp a releaser artifact with a run id that belongs to a backup-restore consumer — exactly the forged provenance the guard exists to prevent ("provenance a party can forge for itself is worth nothing to the operator reading it"). A releaser owns no runs, so any run it names is someone else's. Either run the same run_claimed_elsewhere check before the insert regardless of scope, or refuse run outright on the unscoped path the way content_type is refused there.

---

crates/public-server/src/artifacts.rs:167

[Security] suggestion

The route's body limit was raised from axum's 2 MiB default to ~32 MiB for every caller, and body: Bytes is the last extractor, so the whole body is buffered into memory before the handler's role/declaration checks run. The extractor is now AuthDevice rather than ReleaserDevice, so any enrolled device — including the low-privilege machine credential every monitored box holds — can force a 32 MiB allocation per concurrent request and only then receive a 403. Previously the same credential was rejected by the extractor with at most 2 MiB read. Consider gating on scope before buffering (e.g. take the body via Request/body::to_bytes after the authorization branch, or split the group-scoped registration onto its own route where the large DefaultBodyLimit and a BackupRestoreDevice-style extractor apply together) so the large limit is only reachable by a caller already authorised to publish for the group.

---

crates/private-server/src/fns/restore_replicas.rs:298

[Security] suggestion

The early return for an unadvertised intent (let Some(desc) = … else { return Ok(params.clone()) }) skips every check below it, including the new publishes_schemas validation. So create/update will happily store publishes_schemas: true on a machine-scoped or redacting declaration whose intent the consumer hasn't advertised — the three refusals just added are bypassed by naming an intent that isn't in restore_consumer_capabilities. Nothing is currently exploitable because authorizes_schema_artifacts re-checks the semantic, machine_id IS NULL and redacts = false at register/report time, but this is the operator-granted privilege flag over every machine in the group, and the persisted row is now inconsistent with its own invariants. Move the publishes_schemas (and redacts) validation above the descriptor lookup, refusing the flag outright when there is no descriptor to check it against.

[review-hero]

🦸 Review Hero Summary
12 agents reviewed this PR | 1 critical | 14 suggestions | 0 nitpicks | Filtering: consensus 3 voters, 12 below threshold, 2 suppressed

Below consensus threshold (12 unique issues not confirmed by majority)

Location	Agent	Severity	Comment
crates/database/src/reporting_schemas.rs:44	Design & Architecture	nitpick	artifact_ids: Vec<Option<Uuid>> on a type deriving Serialize + utoipa::ToSchema leaks diesel's Array<Nullable<Uuid>> representation into the wire shape — anything consuming this sees `(uuid...
crates/database/src/reporting_schemas.rs:535	Bugs & Correctness	nitpick	total is pairs.len(), which includes Awaiting pairs that were deliberately filtered out of instances, so the headline reads e.g. "No reporting schema for 2 of 5 versions kamaka runs" when o...
crates/database/src/restore.rs:379	Security	suggestion	run_claimed_elsewhere only looks in backup_restore_checks and backup_runs, but the doc comment itself notes an artifact is registered mid-restore, before the report of that run lands — so a...
crates/database/src/restore.rs:388	Performance	critical	run_claimed_elsewhere runs SELECT COUNT(*) FROM backup_restore_checks WHERE run_id = $1 AND (...), but run_id was added as a bare column (migrations/2026-07-05-113113-0000_add_run_id_to_backu...
crates/public-server/src/artifacts.rs:110	Security	suggestion	The 32 MiB + 64 KiB body limit is a route-level layer, so it applies to every caller of POST /artifacts/{version}/{type}/{platform} on the internet-exposed public server — including a plain `mach...
crates/public-server/src/artifacts.rs:185	Bugs & Correctness	suggestion	The group-scoped branch refuses any artifact_type other than reporting-schema with a 403, but RPT (reporting-schemas.md, unchanged by this PR) says the builder "may register further artifacts b...
crates/public-server/src/artifacts.rs:315	Bugs & Correctness	suggestion	On the group-scoped path the caller-supplied digest is parsed into named_digest and then silently discarded; Canopy digests whatever bytes arrived. A truncated or otherwise corrupted upload is ...
crates/public-server/src/restore.rs:245	Bugs & Correctness	suggestion	The pairs dedup only prevents duplicate build entries within one consumer's worklist — worklist is scoped to consumer_device_id, and nothing (neither authorizes_schema_artifacts nor the n...
crates/public-server/src/restore.rs:248	Design & Architecture	suggestion	version_cache and settlement_cache are two parallel maps keyed by the same group, always written together in the Entry::Vacant block and always read together — with a `version_cache[&d.group_...
crates/public-server/src/restore.rs:381	Performance	nitpick	for version in version_cache[&d.group_id].clone() deep-clones the group's whole Vec<Version> on every schema declaration purely to satisfy the borrow checker; nothing in the loop body needs `&m...
crates/public-server/src/restore.rs:875	Design & Architecture	suggestion	ReportingSchemaArgs ships with two mutually-exclusive ways to name the same version (target_version semver and target_version_id), both optional, with resolution preferring one and erroring o...
private-web/src/components/ReportingSchemasSection.tsx:18	Design & Architecture	nitpick	type PairState = "awaiting" | "built" | "failed" is hand-declared here and then applied with pair.state as PairState. Wire shapes come from the generated api-types.ts via ../types (see AGEN...

Local fix prompt (copy to your coding agent)

Fix these issues identified on the pull request. One commit per issue fixed.

-------

`crates/database/src/reporting_schemas.rs:478`: An open reporting-schema warning can never be cleared once a group leaves the sweep. Both early exits — `!group_builds_schemas` (line 477) and no canonical central (line 482) — `continue` before any filing, so an issue already active on the group's central stays active forever. Failure scenario: a build fails, the sweep files the Warning on `central`, then an operator disables the declaration (or clears `publishes_schemas`, or retires the builder). Every later sweep skips the group, and the operator is left with a standing warning about a schema Canopy no longer owes and no action that can clear it. The same happens if the canonical central changes (a higher-ranked central is added, or the old one is archived): the issue is filed against the *new* central's id, and the old one's issue is orphaned. `file_restore_check`'s `instances.is_empty()` branch — with the `gone` message this code already defines — exists exactly for this; the group should be swept with empty instances rather than skipped. The PR's `the_check_closes_once_the_group_owes_no_schema` test only covers the build row being deleted, which still goes through the filing path.

-------

`crates/private-server/src/fns/restore_replicas.rs:317`: The three `publishes_schemas` guards are unreachable when the consumer hasn't advertised the intent: `normalized_params_for_intent` returns at line 297-299 (`let Some(desc) = ... else { return Ok(params.clone()) }`) before them. Failure scenario: an operator creates a declaration for an intent the consumer no longer advertises (an intent gap, which this codebase treats as a normal transient state) with `machine_id` set and `publishes_schemas: true`; the write succeeds, the row is stored, and `to_views` shows the "publishes schema" chip. Nothing is ever dispatched or authorised for it, because `authorizes_schema_artifacts` independently requires the semantic and `machine_id IS NULL` — so the operator reads a grant that does not exist. Move the `machine_id.is_some()` and `redacts` checks (which don't depend on the descriptor) above the early return, and refuse `publishes_schemas` outright when the intent is not advertised.

-------

`crates/database/src/reporting_schemas.rs:104`: `ReportingSchemaBuild::record` clears the operator's ask on *any* successful-restore build report for the pair, including one dispatched before the ask was made. Concretely: pair is unbuilt, a build is dispatched at T0 and takes an hour; the operator changes the group's configuration and asks for a build at T1; the in-flight build (restored from the pre-change state) reports at T2 and both records a build and deletes the request — the pair settles and the schema the operator asked for is never built, with nothing on screen to say so. `reporting_schema_requests.requested_at` is already stored, so clear only asks older than the run that answered them (e.g. compare against the report's `observed_at`, or the restore's start), rather than unconditionally.

-------

`crates/database/src/restore.rs:379`: `run_claimed_elsewhere` hangs off `RestoreReplica` but never touches `restore_replicas` — it queries `backup_restore_checks` and `backup_runs`, and its subject is a run's provenance, not a replica declaration. It belongs alongside the run/report models (crates/database/src/backups.rs or restore's check types) where the tables it reads live; on `RestoreReplica` it's discoverable only by whoever already knows the artifacts handler calls it.

-------

`crates/public-server/src/artifacts.rs:180`: `create` now serves two different resources through one path: a releaser POSTing a plain-text URL, and a builder POSTing the artifact's raw bytes. The meaning of the request body flips on the presence of `?group=`, and the handler grew a ~60-line preamble that hand-rolls role policy (`matches!(role, Releaser | Admin)`, `role == Admin || authorizes_schema_artifacts(..)`) after dropping the `ReleaserDevice` extractor. That abandons the codebase's convention of encoding "who may call this" in the extractor type (`device_role_struct!` in commons-servers/src/device_auth/mod.rs), so the authorisation for this endpoint is no longer visible in its signature and can't be reused or audited alongside the others. Consider a second route for the bytes-carrying, group-scoped registration (adding a path is compatible under the API rules) with its own extractor, leaving `create` as the releaser's URL registration it already was; or at minimum extract the group branch into a named `authorise_group_registration(...)` so the handler body reads as one thing.

-------

`crates/public-server/src/restore.rs:322`: "This declaration may build/publish a group's schema" is now spelled out three times in three shapes: as SQL in `RestoreReplica::authorizes_schema_artifacts` (enabled + publishes_schemas + !redacts + machine_id IS NULL + advertised semantic), as inline `continue` guards in the worklist (`!d.publishes_schemas`, `d.redacts`, `d.machine_id.is_some()`), and as request validation in `normalized_params_for_intent`. The comments assert dispatch and acceptance must agree — "Canopy asks for no build it would refuse the result of" — but nothing structurally enforces that; adding a fourth condition to one copy silently diverges the others. Give `RestoreReplica` a single predicate over a loaded row (e.g. `fn may_publish_schema(&self, descriptor: &IntentDescriptor) -> bool`) and have the SQL query, the worklist, and the validator all funnel through it.

-------

`crates/public-server/src/restore.rs:316`: The `if builds_schema { ... continue; }` block is ~110 lines of a distinct dispatch strategy (per-pair, group-central, version-list) inlined into the middle of the per-declaration loop of an already long `worklist` handler, and it introduces three more function-scope caches (`pairs`, `version_cache`, `settlement_cache`) alongside the existing per-machine ones. `version_cache` and `settlement_cache` are always populated and read together, so they're one value split across two maps kept in sync by hand (`settlement_cache[&d.group_id]` panics if that invariant ever breaks). Extracting `async fn schema_build_entries(conn, &d, &cfg, &snapshots, &mut cache) -> Result<Vec<WorklistEntry>>` with a single `HashMap<Uuid, GroupPairs { versions, settlement }>` would make the per-machine path readable again and put the pair-dispatch rules in one place.

-------

`crates/database/src/artifacts.rs:330`: `newest_change_for_version` (singular) has no caller anywhere in the tree — it exists only to wrap the plural form, and does so at the cost of an extra `Version::get_by_id` round trip. Similarly `ReportingSchemaBuild::is_settled` and `latest_for_pair` are public API exercised only by tests (production code goes through `Settlement` and `latest_by_version_for_group`). Generalising ahead of a second caller means three surfaces to keep correct where one is used; drop the singular wrapper, and either make the two test-only helpers `pub(crate)`/test helpers or let the tests assert through `Settlement` as the worklist does.

-------

`crates/public-server/src/restore.rs:356`: In the schema-build branch the per-group work is only half cached: `version_cache`/`settlement_cache` are keyed by group, but `Application::list_live_in_group` and `Machine::get_by_id` run once per *declaration*, so a group covered by two schema declarations pays both queries twice even though `pairs` then discards the duplicate entries (exactly the case `a_second_declaration_dispatches_no_second_build` covers). Worse, the `machines` resolution above (line ~270-283, `Machine::list_for_group` for a group-wide declaration) has already run and its result is thrown away for a schema declaration. Move the `if builds_schema` block above the `machines`/`snapshot_cache` resolution, and cache the resolved `(central, machine)` per group alongside `version_cache`.

-------

`crates/database/src/artifacts.rs:366`: The range half of `newest_change_for_versions` loads *every* unscoped range artifact in the table (no `LIMIT`, no per-version predicate) and re-parses each `version_range_pattern` with `node_semver::Range::parse`, then tests it against every requested version. The result is group-independent, yet `Settlement::for_group` calls this once per group, so a worklist poll for a consumer covering N groups repeats the same full scan and the same N×R semver parses N times — and R grows monotonically with every range artifact ever registered. Either hoist the range lookup out of `Settlement::for_group` (compute `MAX(updated_at)` per pattern once per worklist call and pass it in), or collapse it in SQL to `SELECT version_range_pattern, MAX(updated_at) GROUP BY version_range_pattern` so at least the parse count is per distinct pattern rather than per row.

-------

`crates/public-server/src/artifacts.rs:105`: `DefaultBodyLimit::max(MAX_HELD_ARTIFACT_BYTES + 64 KiB)` is applied to the whole route, so the *releaser* path — whose body is a plain download URL — now also accepts and fully buffers a 32 MiB body before the handler ever looks at `named.group`, raising the per-request memory ceiling for unscoped registrations from axum's 2 MiB to 32 MiB with no bound on concurrent registrations. On the held path the 32 MiB `Bytes` is then copied again via `Vec::from(body)` and hashed with `digest_of` inline on the async task (a synchronous ~100 ms SHA-256 over 32 MiB, which stalls the runtime worker). Consider limiting the generous body budget to the group-scoped case (e.g. a smaller default limit plus a `RequestBodyLimit`/manual read for the held path) and moving the digest into `spawn_blocking`.

-------

`crates/database/src/reporting_schemas.rs:476`: `sweep` is an N+1 (really N×M) over the whole fleet on every backup sweep. For each of `ServerGroup::list_all`, `group_builds_schemas` runs one `list_for_group` query plus two more per distinct consumer (`list_for_consumer` + the count in `authorizes_schema_artifacts`), and a group that does have a builder then pays another ~6 queries (`list_live_in_group`, `last_versions`, `get_by_versions`, `planned_target`, `latest_by_version_for_group`, `pending_for_group`) plus the filing. With a few hundred groups each carrying a handful of restore declarations that is well over a thousand round-trips per sweep, and every group pays the probe even though almost none have a builder. Resolve the publishing groups up front with one join (`restore_replicas` × `restore_consumer_capabilities` filtered on `enabled AND publishes_schemas AND NOT redacts AND machine_id IS NULL` and the semantics containment), then iterate only those groups and batch the per-group lookups by group id.

-------

`crates/public-server/src/artifacts.rs:343`: `run_id` is now accepted from the query string on both registration paths, but `run_claimed_elsewhere` is only consulted inside the `Some(group)` arm (line 226). An unscoped registration (releaser or admin credential) therefore writes `run_id: named.run` with no check at all, so it can stamp a releaser artifact with a run id that belongs to a backup-restore consumer — exactly the forged provenance the guard exists to prevent ("provenance a party can forge for itself is worth nothing to the operator reading it"). A releaser owns no runs, so any run it names is someone else's. Either run the same `run_claimed_elsewhere` check before the insert regardless of scope, or refuse `run` outright on the unscoped path the way `content_type` is refused there.

-------

`crates/public-server/src/artifacts.rs:167`: The route's body limit was raised from axum's 2 MiB default to ~32 MiB for every caller, and `body: Bytes` is the last extractor, so the whole body is buffered into memory before the handler's role/declaration checks run. The extractor is now `AuthDevice` rather than `ReleaserDevice`, so any enrolled device — including the low-privilege `machine` credential every monitored box holds — can force a 32 MiB allocation per concurrent request and only then receive a 403. Previously the same credential was rejected by the extractor with at most 2 MiB read. Consider gating on scope before buffering (e.g. take the body via `Request`/`body::to_bytes` after the authorization branch, or split the group-scoped registration onto its own route where the large `DefaultBodyLimit` and a `BackupRestoreDevice`-style extractor apply together) so the large limit is only reachable by a caller already authorised to publish for the group.

-------

`crates/private-server/src/fns/restore_replicas.rs:298`: The early return for an unadvertised intent (`let Some(desc) = … else { return Ok(params.clone()) }`) skips every check below it, including the new `publishes_schemas` validation. So `create`/`update` will happily store `publishes_schemas: true` on a machine-scoped or redacting declaration whose intent the consumer hasn't advertised — the three refusals just added are bypassed by naming an intent that isn't in `restore_consumer_capabilities`. Nothing is currently exploitable because `authorizes_schema_artifacts` re-checks the semantic, `machine_id IS NULL` and `redacts = false` at register/report time, but this is the operator-granted privilege flag over every machine in the group, and the persisted row is now inconsistent with its own invariants. Move the `publishes_schemas` (and `redacts`) validation above the descriptor lookup, refusing the flag outright when there is no descriptor to check it against.

[review-hero]

🦸 Review Hero Summary
12 agents reviewed this PR | 2 critical | 11 suggestions | 0 nitpicks | Filtering: consensus 3 voters, 13 below threshold, 1 suppressed

Below consensus threshold (13 unique issues not confirmed by majority)

Location	Agent	Severity	Comment
crates/database/src/reporting_schemas.rs:38	Design & Architecture	suggestion	artifact_ids: Vec<Option<Uuid>> leaks diesel's Array<Nullable<Uuid>> convention into a type that derives Serialize and utoipa::ToSchema, so the wire/OpenAPI shape becomes `(string | null)[]...
crates/database/tests/it/reporting_schemas.rs:76	Design & Architecture	nitpick	report_for(outcome, healthy, error) already exists as the report builder, yet record_build and a_failed_restore_leaves_the_pair_unsettled each re-inline the full 25-field `NewBackupRestoreChe...
crates/private-server/src/fns/reporting_schemas.rs:88	Bugs & Correctness	nitpick	build enqueues whatever group_id/version_id the body names with no validation. A group or version id that doesn't exist trips the FK constraint, which surfaces as AppError::DatabaseQuery → ...
crates/private-server/src/fns/restore_replicas.rs:288	Design & Architecture	suggestion	normalized_params_for_intent now takes seven arguments and does two unrelated jobs: resolving/validating parameter values, and rejecting declaration shapes (publishes_schemas against the semant...
crates/private-server/src/fns/restore_replicas.rs:297	Security	nitpick	normalized_params_for_intent returns early when the consumer has not advertised the named intent, which skips the whole publishes_schemas validation block added below it (l.317-333). A declarat...
crates/public-server/src/artifacts.rs:257	Design & Architecture	suggestion	The doc comment (which ships in openapi.json and the generated bes-canopy-api client) says the endpoint "Requires a device certificate whose restore declaration for the named group advertises t...
crates/public-server/src/artifacts.rs:297	Security	suggestion	register_for_group takes body: axum::body::Bytes, so axum buffers the entire request body (up to the 32 MiB + 64 KiB MAX_UPLOAD_BODY_BYTES layer) into memory before the handler runs any aut...
crates/public-server/src/artifacts.rs:315	Security	suggestion	An admin-role device certificate bypasses the operator's publishing mark entirely on this internet-exposed endpoint: device.0.role == DeviceRole::Admin || short-circuits before `authorizes_sche...
crates/public-server/src/restore.rs:381	Performance	nitpick	for version in version_cache[&d.group_id].clone() deep-clones the group's whole Vec<Version> on every schema-building declaration purely to dodge the borrow against conn. Nothing inside the l...
crates/public-server/src/restore.rs:382	Bugs & Correctness	suggestion	The pairs dedup is inserted before the once/settled gate, so a settled pair suppresses dispatch for every other declaration too. With two group-wide publishing declarations over one group — one...
crates/public-server/src/restore.rs:1047	Security	suggestion	built: true is taken entirely on the builder's word and is never cross-checked against an artifact Canopy actually holds. Recording a build with built: true settles the pair (drops it off the w...
crates/public-server/src/restore.rs:1049	Security	suggestion	build.artifacts is stored verbatim as the build's provenance with no check that those artifact ids exist, are group-scoped to args.group, or were registered by this consumer. This is the same f...
crates/public-server/src/restore.rs:1097	Design & Architecture	suggestion	resolve_build_target is a character-for-character copy of resolve_migration_target (semver wins, else id, else 400) differing only in the argument type, and ReportingSchemaArgs duplicates `Mi...

Local fix prompt (copy to your coding agent)

Fix these issues identified on the pull request. One commit per issue fixed.

-------

`crates/database/src/reporting_schemas.rs:205`: Settling compares the version's newest artifact change against `built_at`, which is `NOW()` at the moment the *report* is recorded — not the moment the build read the version's artifacts. An artifact registered while a build is in flight is therefore treated as already incorporated. Failure scenario: a build is dispatched at T0 and restores/migrates for an hour; a releaser re-registers the version's migrations artifact at T1; the builder's report lands at T2 > T1, so `built_at = T2 >= T1` and `settled()` returns true — the pair keeps a schema built from the superseded artifacts and is never re-dispatched until an operator asks, which is exactly what this rule exists to prevent. Anchor the comparison to when the build started (e.g. carry the worklist dispatch/restore time on the report, or have the builder echo the artifact digests it read) rather than to the report timestamp.

-------

`crates/private-server/src/fns/restore_replicas.rs:316`: Nothing constrains a group to a single publishing declaration: the new column has no partial unique index (`publishes_schemas = true AND enabled` per group), and this validation only rejects redacting/machine-scoped/unadvertised cases. The worklist dedupes pairs only within one consumer's response (`pairs` in public-server `worklist`), so two devices each holding a marked declaration for the same group are each dispatched the group's whole pair list — a duplicated restore + migrate per pair — and both are authorised to register the same `(group, version, reporting-schema)` artifact, which upserts, so which builder's bytes the group's machines run is a race. The spec speaks of *the* declaration that publishes a group's schema; either enforce that on create/update (and with a DB index) or dedupe dispatch by pair across consumers.

-------

`crates/database/src/reporting_schemas.rs:477`: An already-open reporting-schema warning can never be cleared once the group leaves the sweep. `file_restore_check` is the only thing that regrades or recovers this check, and both early `continue`s here skip it: if an operator disables the builder declaration, deletes it, or unsets `publishes_schemas` after a failed build has been filed, `group_builds_schemas` goes false and the sweep returns before filing anything — the warning stays active against the group's central forever, and no build will ever land to recover it. The same applies at line 482 when the group loses its central. This is the exact failure the sibling test `the_check_closes_once_the_group_owes_no_schema` guards against for vanished pairs ("or it stands forever against a group that owes nothing"), just reached by a different route — and disabling a declaration is the ordinary operator action. Fix: when the group no longer builds schemas, still resolve its central and call `file_restore_check` with an empty `instances` vec so an open check is recovered with the `gone` message; only skip entirely when there is no central to file against.

-------

`crates/database/src/artifacts.rs:355`: `newest_change_for_version` (singular) has no callers anywhere — production goes through `Settlement::for_group` → `newest_change_for_versions`. `ReportingSchemaBuild::is_settled` is likewise only reached from tests, and both wrappers pay an extra `Version::get_by_id` round trip to reshape a single id into the slice the batch API wants. Two public entry points that exist to make assertions terser is API surface that will be maintained forever; drop the singular helper and have the tests build a `Settlement` (or keep `is_settled` and mark it clearly as the test-facing convenience it is).

-------

`crates/public-server/src/restore.rs:313`: The "is this declaration the group's schema publisher" rule is now written out four times, in four crates' worth of code, with no shared definition: here in the worklist branch (`builds_schema && d.publishes_schemas && !d.redacts && d.machine_id.is_none()`, plus the enabled filter upstream), in SQL in `RestoreReplica::authorizes_schema_artifacts` (semantic + enabled + publishes_schemas + !redacts + machine_id IS NULL), in Rust-side validation in `private-server/src/fns/restore_replicas.rs::normalized_params_for_intent`, and once more as a per-consumer loop in `database::reporting_schemas::group_builds_schemas`. The comments themselves state the invariant that matters ("Canopy asks for no build it would refuse the result of"), and four independent encodings are exactly how that invariant drifts — add one axis (say, a type or rank constraint) and three of the four sites silently disagree. Extract a single predicate over `(&RestoreReplica, &IntentDescriptor)` in the database crate and have dispatch, registration authorisation, the pair derivation, and the admin validation all call it (the SQL variant then becomes that predicate applied to the rows `list_for_group` already returns).

-------

`crates/public-server/src/restore.rs:312`: The `builds_schema` branch adds ~110 lines inline to `worklist`, which is now ~315 lines and holds five per-group caches. This branch is a self-contained sub-dispatcher (its own guards, its own key set, its own entry shape, ending in `continue`) and reads much better as `schema_entries_for_group(&mut conn, d, &cfg, ...) -> Result<Vec<WorklistEntry>>`. While extracting, fold `version_cache` and `settlement_cache` into one map: they are keyed identically, written together in the same `Entry::Vacant` arm, and the split forces the `settlement_cache[&group]` index plus a `.clone()` of the version vector per declaration.

-------

`crates/database/src/restore.rs:372`: `RestoreReplica::run_claimed_elsewhere` doesn't touch `restore_replicas` at all — it queries `backup_restore_checks` and `backup_runs` only. Hanging it off the replica model means the artifact-registration path reaches for the restore-declaration type to answer a question about run provenance. It belongs next to the run/report models (e.g. on `BackupRun`, or a small provenance helper in `backup`), which is also where a future caller would look for it.

-------

`crates/public-server/src/artifacts.rs:300`: `register_for_group` takes `{artifact_type}` in the path but refuses anything other than `reporting-schema`, so the segment is an inert dimension that a caller can only get wrong — and the refusal is a 403 for what is really an unsupported path. Given the spec now states the schema is the only type publishable this way, either bake it into the route (`/groups/{group}/{version}/reporting-schema/{platform}`) and name the handler for what it is, or keep the parameter and return 400. Note the path shape is public API, so it's cheaper to settle before the client crate is generated against it.

-------

`crates/database/src/versions.rs:171`: `get_by_versions` builds a hand-rolled `BoxableExpression` OR-chain (~25 lines plus a type alias) to express "any of these (major, minor, patch) triples". Diesel supports tuple membership directly on Postgres — `filter((major, minor, patch).eq_any(triples))` — which is a three-line body; failing that, `major.eq_any(distinct_majors)` plus the in-memory triple match the only caller (`versions_and_applications`) already performs is simpler than a boxed predicate tree. As written it's a lot of machinery for one call site.

-------

`crates/database/src/reporting_schemas.rs:476`: `sweep()` runs every minute from the monitor loop (`database::backup::sweep` → here, `crates/jobs/src/bin/monitor.rs:267`) and walks *every* group in the fleet, calling `group_builds_schemas` for each. That helper issues one `RestoreReplica::list_for_group` query plus, for each distinct consumer on the group, a `list_for_consumer` query and a `COUNT(*)` on `restore_replicas` (`authorizes_schema_artifacts`). With N groups and C consumers per group that's N·(1 + 2C) round trips per minute, paid in full even for the overwhelming majority of groups that have no restore replicas at all and will be `continue`d one line later. The whole predicate is one query: `SELECT DISTINCT group_id FROM restore_replicas r JOIN restore_consumer_capabilities c USING (consumer_device_id, intent) WHERE r.enabled AND r.publishes_schemas AND NOT r.redacts AND r.machine_id IS NULL AND c.semantics ? 'reporting-schema'` — then iterate only that (small) set of groups instead of `ServerGroup::list_all`.

-------

`crates/database/src/artifacts.rs:391`: `newest_change_for_versions` loads *every* unscoped range artifact row in the table (no version filter, no limit) and re-parses each `version_range_pattern` with `node_semver::Range::parse`, then does an O(ranges × versions) `satisfies` sweep. This is on the hot path: `Settlement::for_group` calls it once per group on every `/restore-worklist` poll, and every restore consumer polls on a schedule, so the cost is (consumers × groups × range-artifact rows) semver parses per polling interval, growing unboundedly with every release that registers a range artifact. The new `artifacts_version_updated` index only covers the exact-version half. Consider narrowing the range query (e.g. `HAVING max(updated_at) > <oldest build_at among the versions asked about>`, or aggregating `max(updated_at)` per pattern in SQL so one row is parsed per distinct pattern rather than per artifact) and/or caching the parsed ranges across the request.

-------

`crates/public-server/src/restore.rs:355`: The schema-build branch resolves the group's membership per *declaration*, not per group: `Application::list_live_in_group` (line 355) and `Machine::get_by_id` (line 364) both run before the `version_cache`/`settlement_cache` lookup, so the second and subsequent declarations covering one group re-query them even though the comment above claims "a group covered by several declarations is resolved once" (and `a_second_declaration_dispatches_no_second_build` exercises exactly that shape). Move the members/central/machine resolution inside the same `Entry::Vacant` block that fills the version cache — or cache central+machine per group alongside it — so the whole per-group resolution is paid once per worklist request.

-------

`crates/public-server/src/artifacts.rs:309`: `artifact_type` is pinned to `reporting-schema`, but `platform` is taken from the path unvalidated and is part of the upsert conflict key `(artifact_type, platform, version_id, version_range_pattern, group_id)`. A schema is specified to be published on platform `any`, so every other platform value creates a *new* row instead of replacing the group's one schema: an authorised builder can persist unbounded distinct 32 MiB blobs for its own group (and have several conflicting `reporting-schema` artifacts offered to the same machines) rather than being capped at one. Reject any platform other than `any` here, the same way the type is rejected.

[review-hero]

🦸 Review Hero Summary (round 6)
9 agents reviewed this PR | 0 critical | 7 suggestions | 0 nitpicks | Filtering: consensus 3 voters, 12 below threshold, 1 suppressed

Below consensus threshold (12 unique issues not confirmed by majority)

Location	Agent	Severity	Comment
crates/database/src/reporting_schemas.rs:64	Bugs & Correctness	critical	record decides whether to store the build from the restore outcome alone (restore_failed = report.outcome != RunOutcome::Success) and discards the whole NewReportingSchemaBuild when it is s...
crates/database/src/reporting_schemas.rs:331	Performance	nitpick	pairs_for_group asks the fleet-wide groups_building_schemas (join over all restore_replicas × restore_consumer_capabilities with DISTINCT) only to test membership of a single group, and i...
crates/database/src/reporting_schemas.rs:483	Bugs & Correctness	suggestion	The sweep continues before filing when a group has no canonical central, so a warning already open against a former central is never regraded. The branch below deliberately handles the analogous ...
crates/database/src/reporting_schemas.rs:560	Bugs & Correctness	nitpick	total is pairs.len(), which counts pairs still Awaiting a build, while the numerator many.len() only counts graded (built/failed) instances. The headline therefore reports a denominator tha...
crates/database/src/restore.rs:397	Performance	suggestion	run_claimed_elsewhere does COUNT(*) over backup_restore_checks filtered on run_id, but there is no index on that column (the table only has group_type, machine_type, snapshot, `replic...
crates/private-server/src/fns/reporting_schemas.rs:91	Bugs & Correctness	suggestion	build enqueues straight from the request body with no validation that the version (or the pair) exists. reporting_schema_requests.version_id is NOT NULL REFERENCES versions(id), so an unknown...
crates/public-server/src/artifacts.rs:286	Security	suggestion	The 32 MiB body is fully buffered before any authorisation runs. body: axum::body::Bytes is the last extractor, so axum collects the whole request body (up to MAX_UPLOAD_BODY_BYTES = 32 MiB + 6...
crates/public-server/src/restore.rs:419	Bugs & Correctness	critical	A schema-building declaration emits one entry per pair, but every entry carries the same replica_id, machine_id and name — only target_version/target_version_id differ. The contract docum...
crates/public-server/src/restore.rs:1068	Security	suggestion	A build report's artifacts: Vec<Uuid> is stored verbatim into reporting_schema_builds.artifact_ids with no check that the ids exist, are group-scoped to args.group, or were registered by this...
crates/public-server/src/restore.rs:1123	Security	nitpick	resolve_build_target passes Version::get_by_version's error straight through, so a device-supplied target_version that parses as semver but names no release row surfaces as `AppError::Databas...
crates/public-server/src/restore.rs:1133	Bugs & Correctness	suggestion	resolve_build_target returns build.target_version_id without checking Canopy holds that version, and ReportingSchemaBuild::record writes the restore report first and the build row second with...
private-web/src/components/ReportingSchemasSection.tsx:118	Bugs & Correctness	suggestion	The "Build sooner"/"Build again" button is rendered unconditionally, but reporting_schemas/build takes TailscaleAdmin, so a non-admin operator gets a button that always 403s (surfaced as a red ...

Local fix prompt (copy to your coding agent)

Fix these issues identified on the pull request. One commit per issue fixed.

-------

`crates/database/src/reporting_schemas.rs:90`: `record` clears the operator's ask for the pair unconditionally, with no comparison against when the ask was made. A build is dispatched from a worklist poll and lands minutes later; an ask entered inside that window is answered by a build that started before it and cannot reflect whatever configuration change prompted the ask. Compare `requested_at` with the report (e.g. delete only where `requested_at < report.observed_at`) so an in-flight build cannot swallow a later ask.

-------

`crates/private-server/src/fns/restore_replicas.rs:317`: The `publishes_schemas` guards sit after the early return for an unadvertised intent (`let Some(desc) = … else { return Ok(params.clone()) }`), so a declaration created against an intent the consumer hasn't advertised yet skips all three checks. That stores a machine-scoped or redacting declaration with `publishes_schemas = true`, which authorises nothing (both `authorizes_schema_artifacts` and `groups_building_schemas` filter `machine_id IS NULL` and `redacts = false`) but does occupy the group's one slot in `restore_replicas_one_schema_publisher`. The operator then cannot declare the real group-wide publisher: they get "another enabled declaration already publishes this group's reporting schema" pointing at a declaration that publishes nothing. Move the `redacts`/`machine_id` refusals ahead of the descriptor lookup (they don't depend on it), or refuse `publishes_schemas` outright when the intent isn't advertised.

-------

`crates/database/src/reporting_schemas.rs:481`: `sweep()` walks every group in the fleet and issues per-group queries on the one-minute monitor cadence (`database::backup::sweep` → here). Each iteration costs at least `list_live_in_group` plus the `open_server_issue_active` probe inside `file_restore_check`, and for a group in `builders` it costs ~6 more (`last_versions`, `get_by_versions`, `planned_target`'s two, `latest_by_version_for_group`, `pending_for_group`). That is a textbook N+1 that scales with fleet size for work that is a no-op for most groups. The header comment says the non-builder path "costs one query and writes nothing", but it is two, and the member load is only needed to name the central for a recovery filing. Narrow the loop instead: one fleet-wide query for applications with an open `reporting-schema` issue, then iterate `builders ∪ those_groups` only, and batch the member load for that set rather than one query per group.

-------

`crates/database/src/artifacts.rs:416`: The range half of `newest_change_for_versions` is unfiltered by version: it aggregates over every unscoped range artifact in the table and then parses each pattern and runs `range.satisfies` for every requested version (O(patterns × versions) semver work). Because it lives behind `Settlement::for_group`, a consumer whose worklist covers N groups repeats this identical whole-table scan and cross product N times per poll — the pattern set does not vary per group. Hoist the range lookup to one call per request (or per sweep) and pass the resulting pattern → timestamp map into `Settlement`, so only the per-group exact-version query stays in the loop.

-------

`crates/database/src/versions.rs:188`: `get_by_versions` narrows only on `major` and then sifts the triple in memory. In practice the whole fleet is on major 2, so this loads *every* 2.x release row from `versions` on each call — and it is called once per group per `reporting_schemas::sweep` (fleet-wide, on the backup sweep cadence) and again per group on every restore-worklist poll. Adding the other two components as set predicates keeps it one query while letting Postgres do the narrowing: `.filter(major.eq_any(majors)).filter(minor.eq_any(minors)).filter(patch.eq_any(patches))`, keeping the in-memory triple filter to drop the cross-product leftovers.

-------

`crates/public-server/src/restore.rs:212`: `resolve_schema_group` already has the group's live applications in `members`, then calls `versions_for_group`, which runs `Application::list_live_in_group` for the same group a second time. That is a duplicate query per schema-building group on every worklist poll, and every restore consumer polls on a schedule. `reporting_schemas` already has the members-in-hand split for exactly this (`pairs_of_members`); expose the same for versions (e.g. `versions_of_members(db, group, &members)`) and pass `members` through.

-------

`crates/public-server/src/artifacts.rs:307`: The admin-device branch grants blanket group-scoped publishing on an internet-exposed endpoint, with no declaration, no operator mark, and no `restore_replicas_one_schema_publisher` uniqueness applying to it. The specs this change writes say the opposite: RPT states "the mark is the operator's alone, and is the whole of the authorisation" and that a builder may register "for a group whose enabled declaration an operator has marked ... and for no other", and ART says a releaser "carries no authorisation for any group" — neither reserves an exception for `DeviceRole::Admin`. Since a group-scoped `reporting-schema` artifact is fetched and applied by every machine in the group, one admin device certificate becomes an arbitrary-SQL-to-the-whole-fleet credential that the group's own operator cannot see or revoke through the declaration. Either drop the branch (the private server already has an operator-authenticated upload path) or make the exception explicit in ART/RPT and scope it, e.g. still requiring the type/platform pin plus an audit record naming the admin device.