From 411a7d2d81b8ca4244c5310247798906e7899e74 Mon Sep 17 00:00:00 2001 From: Daniel Nash <38335330+dannash100@users.noreply.github.com> Date: Tue, 7 Jul 2026 12:22:46 +1200 Subject: [PATCH 1/7] docs(spec): add STA multi-client status reporting Establish the contract for more than one agent (bestool, seedling) reporting status under one server identity. A heartbeat is attributed to a named client (default `bestool` for back-compat); status is kept per (server, client) so streams and reachability don't collide; the response is client-scoped so Canopy can direct each agent over the same return channel. Foundation for the Seedling x Canopy channel (TAM-6867). --- .workhorse/specs/public-server/status.md | 43 ++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 .workhorse/specs/public-server/status.md diff --git a/.workhorse/specs/public-server/status.md b/.workhorse/specs/public-server/status.md new file mode 100644 index 00000000..2a8d14be --- /dev/null +++ b/.workhorse/specs/public-server/status.md @@ -0,0 +1,43 @@ +--- +id: STA +--- + +# Status reporting + +A server device sends Canopy periodic status heartbeats: a self-reported health flag, named health checks, and free-form host facts (versions, uptime, and the like). +Canopy records each, derives incidents from failing checks, and infers reachability from how recently a device reported. + +More than one agent may report for one host: on a Tamanu server the bestool alert daemon and Seedling run against the same device identity, each with its own view. +This spec covers how Canopy keeps those reports distinct and replies to each. + +## Scope + +This spec covers the device-facing status contract: how a heartbeat is attributed to a reporting agent, how Canopy keeps concurrent agents under one server distinct, and what it returns in response. + +It does not cover how a device enrols or authenticates (see [DTR](../private-server/device-trust.md)), nor what a device backs up (see [BAK](backup.md)). + +## Identity and clients + +A heartbeat authenticates as a server device over either transport Canopy accepts, exactly as every device request does; identity is never taken from the request body. + +Under one authenticated server, a heartbeat is attributed to a named **client**, the agent that produced it. +The set of client names is open; the two Canopy expects are `bestool` (the alert daemon) and `seedling`. + +A heartbeat that names no client is attributed to `bestool`, so agents already deployed, which name none, keep reporting unchanged until they are rebuilt. + +The client is a label, not a second identity: authorisation stays the server binding, so a device reports only for the server it is bound to, and an admin for any. + +## Independent streams + +Canopy records status per `(server, client)`, so two agents reporting for one server never overwrite each other: each keeps its own latest heartbeat, health checks, and history. + +A server is not treated as down while any of its clients is still reporting, so one agent going quiet does not by itself make the server look down. + +## Response + +Canopy knows the reporting client from the request, so it returns only what that client needs, and does not give one client another's concerns. +A client acts on the parts of the response it understands and ignores the rest; it is sent nothing meant only for another client, and relies on receiving nothing beyond its own. + +The backup types due now go to the client that runs backups (`bestool`), not to one such as Seedling that does not. +A client that reports health checks is told the severities Canopy applies to them. +Each reporting client gets its own recorded status back. From 5e2e0fc2a6d05befcb16de63465fffe221308fd1 Mon Sep 17 00:00:00 2001 From: Daniel Nash <38335330+dannash100@users.noreply.github.com> Date: Thu, 9 Jul 2026 07:45:56 +1200 Subject: [PATCH 2/7] feat(public): record status per (server, client) --- crates/database/src/bin/seed.rs | 1 + crates/database/src/schema.rs | 1 + crates/database/src/statuses.rs | 8 ++++++ crates/database/tests/status_health_state.rs | 1 + crates/public-server/src/statuses.rs | 27 ++++++++++++++++--- .../down.sql | 4 +++ .../up.sql | 11 ++++++++ 7 files changed, 49 insertions(+), 4 deletions(-) create mode 100644 migrations/2026-07-08-193059-0000_add_client_to_statuses/down.sql create mode 100644 migrations/2026-07-08-193059-0000_add_client_to_statuses/up.sql diff --git a/crates/database/src/bin/seed.rs b/crates/database/src/bin/seed.rs index efc62d69..82686156 100644 --- a/crates/database/src/bin/seed.rs +++ b/crates/database/src/bin/seed.rs @@ -864,6 +864,7 @@ async fn seed_statuses( extra, healthy, health, + client: "bestool".to_owned(), }) .execute(conn) .await?; diff --git a/crates/database/src/schema.rs b/crates/database/src/schema.rs index 7ec4304f..8ab00ba6 100644 --- a/crates/database/src/schema.rs +++ b/crates/database/src/schema.rs @@ -538,6 +538,7 @@ diesel::table! { device_id -> Nullable, healthy -> Bool, health -> Jsonb, + client -> Text, } } diff --git a/crates/database/src/statuses.rs b/crates/database/src/statuses.rs index 99935a72..f890661b 100644 --- a/crates/database/src/statuses.rs +++ b/crates/database/src/statuses.rs @@ -91,6 +91,9 @@ pub struct Status { /// `check` name and a result; any extra per-check fields are passed /// through verbatim. pub health: serde_json::Value, + /// The reporting agent this status is attributed to; `bestool` when the + /// report named none. + pub client: String, } #[derive(Debug, Insertable)] @@ -104,6 +107,7 @@ pub struct NewStatus { pub extra: serde_json::Value, pub healthy: bool, pub health: serde_json::Value, + pub client: String, } impl Default for NewStatus { @@ -115,6 +119,7 @@ impl Default for NewStatus { extra: serde_json::Value::Object(Default::default()), healthy: true, health: serde_json::Value::Array(Default::default()), + client: "bestool".to_owned(), } } } @@ -162,6 +167,9 @@ impl Status { // to avoid false-positive unhealthy events from this path. healthy: true, health: serde_json::Value::Array(Default::default()), + // Canopy-generated reachability status; attributed to the + // default client stream. + client: "bestool".to_owned(), }) } Err(err) => { diff --git a/crates/database/tests/status_health_state.rs b/crates/database/tests/status_health_state.rs index 0f40c453..55c8e454 100644 --- a/crates/database/tests/status_health_state.rs +++ b/crates/database/tests/status_health_state.rs @@ -16,6 +16,7 @@ fn status(healthy: bool, health: serde_json::Value) -> Status { extra: serde_json::json!({}), healthy, health, + client: "bestool".to_owned(), } } diff --git a/crates/public-server/src/statuses.rs b/crates/public-server/src/statuses.rs index b6ef8576..1f9f2a42 100644 --- a/crates/public-server/src/statuses.rs +++ b/crates/public-server/src/statuses.rs @@ -42,6 +42,13 @@ use crate::state::AppState; /// status data. #[derive(Debug, Deserialize, ToSchema)] pub struct StatusPayload { + /// The reporting agent this status is attributed to (e.g. `bestool`, + /// `seedling`). **Absent means `bestool`**, so agents that predate this + /// field keep reporting under their existing stream. Status is recorded + /// per (server, client), so concurrent agents on one host never overwrite + /// each other. + pub client: Option, + /// Overall self-reported health of the server. **Absent means `true`**, /// so senders that predate this field are never treated as unhealthy by /// omission. Recorded for historical analysis and display, but **not @@ -223,7 +230,7 @@ async fn create( }; let raw = body.map(|j| j.0).unwrap_or(serde_json::Value::Null); - let (healthy, health, extra) = split_health_from_extra(raw)?; + let (healthy, client, health, extra) = split_health_from_extra(raw)?; // The server version canopy tracks (and compares against the published // version catalog) is the Tamanu version. Prefer the payload's @@ -238,7 +245,7 @@ async fn create( if !server.allow_legacy_status { return Err(AppError::BadRequest("`health` array is required".into())); } - let status = create_legacy_status(&mut db, server_id, id, extra, version).await?; + let status = create_legacy_status(&mut db, server_id, id, extra, version, client).await?; let check_severities = effective_check_severities(&mut db, server_id, server.group_id).await?; return Ok(Json(StatusResponse { @@ -269,6 +276,7 @@ async fn create( extra, healthy, health, + client, } .save(conn) .await?; @@ -381,6 +389,7 @@ async fn create_legacy_status( device_id: Uuid, extra: serde_json::Value, version: Option, + client: String, ) -> Result { let (healthy, health) = match Status::latest_for_server(db, server_id).await? { Some(prior) => (prior.healthy, prior.health), @@ -393,6 +402,7 @@ async fn create_legacy_status( extra, healthy, health, + client, } .save(db) .await @@ -651,7 +661,7 @@ fn per_check_description( /// verbatim. fn split_health_from_extra( raw: serde_json::Value, -) -> Result<(bool, Option, serde_json::Value)> { +) -> Result<(bool, String, Option, serde_json::Value)> { let mut obj = match raw { serde_json::Value::Null => serde_json::Map::new(), serde_json::Value::Object(m) => m, @@ -668,12 +678,20 @@ fn split_health_from_extra( Some(_) => return Err(AppError::BadRequest("`healthy` must be a boolean".into())), }; + // The reporting agent, defaulting to `bestool` so reporters that name none + // keep their existing stream. + let client = match obj.remove("client") { + None => "bestool".to_owned(), + Some(serde_json::Value::String(s)) => s, + Some(_) => return Err(AppError::BadRequest("`client` must be a string".into())), + }; + // A push without a `health` key is the retired legacy format. We don't // reject it here — the caller decides, per the server's // `allow_legacy_status` flag, whether to accept it (reachability-only, // carrying prior healthchecks forward) or 400 it. let Some(health_value) = obj.remove("health") else { - return Ok((healthy, None, serde_json::Value::Object(obj))); + return Ok((healthy, client, None, serde_json::Value::Object(obj))); }; let health_arr = match health_value { serde_json::Value::Array(a) => a, @@ -727,6 +745,7 @@ fn split_health_from_extra( Ok(( healthy, + client, Some(serde_json::Value::Array(health_arr)), serde_json::Value::Object(obj), )) diff --git a/migrations/2026-07-08-193059-0000_add_client_to_statuses/down.sql b/migrations/2026-07-08-193059-0000_add_client_to_statuses/down.sql new file mode 100644 index 00000000..29d43f39 --- /dev/null +++ b/migrations/2026-07-08-193059-0000_add_client_to_statuses/down.sql @@ -0,0 +1,4 @@ +drop index if exists statuses_server_client_created_at_idx; + +alter table statuses + drop column client; diff --git a/migrations/2026-07-08-193059-0000_add_client_to_statuses/up.sql b/migrations/2026-07-08-193059-0000_add_client_to_statuses/up.sql new file mode 100644 index 00000000..111a4ff0 --- /dev/null +++ b/migrations/2026-07-08-193059-0000_add_client_to_statuses/up.sql @@ -0,0 +1,11 @@ +-- A status is attributed to a named client (the reporting agent). Existing +-- agents name none and keep reporting as `bestool`, so their streams are +-- unchanged; a second agent (e.g. seedling) reports under its own name and is +-- kept distinct per (server, client). +alter table statuses + add column client text not null default 'bestool'; + +-- Latest-per-stream lookups key off (server, client); the descending +-- created_at lets "most recent for this client" be an index-only range scan. +create index statuses_server_client_created_at_idx + on statuses (server_id, client, created_at desc); From a6ca4013511a6faa21a3f435c77a1509f26aeac9 Mon Sep 17 00:00:00 2001 From: Daniel Nash <38335330+dannash100@users.noreply.github.com> Date: Thu, 9 Jul 2026 07:56:07 +1200 Subject: [PATCH 3/7] feat(public): scope the status response to the reporting client --- crates/public-server/src/statuses.rs | 45 ++++++++++------- crates/public-server/tests/statuses.rs | 70 ++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 18 deletions(-) diff --git a/crates/public-server/src/statuses.rs b/crates/public-server/src/statuses.rs index 1f9f2a42..9009df1b 100644 --- a/crates/public-server/src/statuses.rs +++ b/crates/public-server/src/statuses.rs @@ -144,9 +144,11 @@ pub struct StatusResponse { /// one-offs plus scheduled backups that are due. Each serializes as a /// plain string (e.g. `"tamanu-postgres"`). The device should run each /// listed type, then report via `POST /backup-report`; an empty list - /// means nothing to do. - #[schema(value_type = Vec)] - pub backup_now: Vec, + /// means nothing to do. Sent only to the client that runs backups + /// (`bestool`); omitted for other reporting clients. + #[serde(skip_serializing_if = "Option::is_none")] + #[schema(value_type = Option>)] + pub backup_now: Option>, /// The effective handling of every healthcheck canopy knows about, keyed /// by check name (as reported in `health[].check`): `skip` (silenced for /// this server, or classified below warning), `warn` (warning), or `fail` @@ -175,11 +177,13 @@ pub fn routes() -> OpenApiRouter { /// /// The calling device must be the one enrolled for this exact server (or /// hold the admin role). The response echoes back the stored status -/// record, plus a `backup_now` list of backup types the server should -/// back up immediately — devices should treat a non-empty list as a -/// prompt to run those backups and report them afterwards — and a -/// `check_severities` map describing how canopy classifies each known -/// healthcheck for this server (`skip`/`warn`/`fail`). +/// record and a `check_severities` map describing how canopy classifies +/// each known healthcheck for this server (`skip`/`warn`/`fail`). The +/// `bestool` client (the agent that runs backups) additionally gets a +/// `backup_now` list of backup types the server should back up +/// immediately — it should treat a non-empty list as a prompt to run +/// those backups and report them afterwards; other clients are sent no +/// `backup_now` at all. #[utoipa::path( post, path = "/{server_id}", @@ -219,19 +223,24 @@ async fn create( )); } - // Tell the device which backup types to run now (operator one-offs + - // schedule-due), riding the heartbeat response. Empty for an ungrouped - // server or one whose group has no `ready` backup config. - let backup_now = match server.group_id { - Some(group_id) => { - backups_due_now_for_server(&mut db, server_id, group_id, Timestamp::now()).await? - } - None => Vec::new(), - }; - let raw = body.map(|j| j.0).unwrap_or(serde_json::Value::Null); let (healthy, client, health, extra) = split_health_from_extra(raw)?; + // Tell the device which backup types to run now (operator one-offs + + // schedule-due), riding the heartbeat response. Only the client that runs + // backups is told; others get no `backup_now` at all. Empty for an + // ungrouped server or one whose group has no `ready` backup config. + let backup_now = if client == "bestool" { + Some(match server.group_id { + Some(group_id) => { + backups_due_now_for_server(&mut db, server_id, group_id, Timestamp::now()).await? + } + None => Vec::new(), + }) + } else { + None + }; + // The server version canopy tracks (and compares against the published // version catalog) is the Tamanu version. Prefer the payload's // `tamanuVersion` extra; fall back to the legacy `X-Version` header for diff --git a/crates/public-server/tests/statuses.rs b/crates/public-server/tests/statuses.rs index 2b679cdd..1bdda3bc 100644 --- a/crates/public-server/tests/statuses.rs +++ b/crates/public-server/tests/statuses.rs @@ -2643,3 +2643,73 @@ async fn submit_status_versionless_when_neither_present() { ) .await } + +#[derive(QueryableByName)] +struct ClientRow { + #[diesel(sql_type = sql_types::Text)] + client: String, +} + +#[tokio::test(flavor = "multi_thread")] +async fn status_is_recorded_and_scoped_per_client() { + commons_tests::server::run_with_device_auth( + "server", + async |mut conn, cert, device_id, public, _| { + let server_id = insert_health_test_server(&mut conn, device_id).await; + + // A push naming no client is attributed to bestool and gets the + // backup list. + let response = public + .post(&format!("/status/{server_id}")) + .add_header("mtls-certificate", &cert) + .json(&serde_json::json!({ "health": [] })) + .await; + response.assert_status_ok(); + let body: serde_json::Value = response.json(); + assert_eq!(body.get("client").and_then(|v| v.as_str()), Some("bestool")); + assert!( + body.get("backup_now").is_some(), + "bestool gets a backup_now list" + ); + assert!(body.get("check_severities").is_some()); + + // A seedling push is kept as its own stream and is sent no + // backup_now at all. + let response = public + .post(&format!("/status/{server_id}")) + .add_header("mtls-certificate", &cert) + .json(&serde_json::json!({ "client": "seedling", "health": [] })) + .await; + response.assert_status_ok(); + let body: serde_json::Value = response.json(); + assert_eq!( + body.get("client").and_then(|v| v.as_str()), + Some("seedling") + ); + assert!( + body.get("backup_now").is_none(), + "seedling is sent no backup_now" + ); + assert!(body.get("check_severities").is_some()); + + // Both streams are stored distinctly for the one server. + let rows: Vec = + sql_query("SELECT client FROM statuses WHERE server_id = $1 ORDER BY client") + .bind::(server_id) + .get_results(&mut conn) + .await + .expect("fetch status clients"); + let clients: Vec<&str> = rows.iter().map(|r| r.client.as_str()).collect(); + assert_eq!(clients, ["bestool", "seedling"]); + + // A non-string client is rejected. + let response = public + .post(&format!("/status/{server_id}")) + .add_header("mtls-certificate", &cert) + .json(&serde_json::json!({ "client": 42, "health": [] })) + .await; + response.assert_status_bad_request(); + }, + ) + .await +} From a3f688d4ff12508b4be4b2830048e3bfc0c5341f Mon Sep 17 00:00:00 2001 From: Daniel Nash <38335330+dannash100@users.noreply.github.com> Date: Thu, 9 Jul 2026 08:04:18 +1200 Subject: [PATCH 4/7] feat(database): any-client freshness, default-client content reads --- crates/database/src/server_groups.rs | 8 +-- crates/database/src/statuses.rs | 43 +++++++++++++- crates/database/tests/reachability_sweep.rs | 66 +++++++++++++++++++++ 3 files changed, 112 insertions(+), 5 deletions(-) diff --git a/crates/database/src/server_groups.rs b/crates/database/src/server_groups.rs index 8f9b0e83..445f559d 100644 --- a/crates/database/src/server_groups.rs +++ b/crates/database/src/server_groups.rs @@ -235,10 +235,10 @@ impl ServerGroup { .map_err(AppError::from)?; if !member_ids.is_empty() { - // A member is "gone" iff it's absent from `latest_for_servers` - // (no status in the last 7 days). Allow the cascade only when - // every live member is gone; any recent reporter blocks it. - let recent = Status::latest_for_servers(conn, &member_ids).await?; + // A member is "gone" iff no client of it has reported in the + // last 7 days. Allow the cascade only when every live member + // is gone; any recent reporter blocks it. + let recent = Status::last_report_for_servers(conn, &member_ids).await?; if !recent.is_empty() { return Err(AppError::Conflict(format!( "group {group_id} has {} server(s) that reported within the last \ diff --git a/crates/database/src/statuses.rs b/crates/database/src/statuses.rs index f890661b..3a263e9a 100644 --- a/crates/database/src/statuses.rs +++ b/crates/database/src/statuses.rs @@ -31,6 +31,12 @@ pub const CANOPY_SOURCE: &str = "canopy"; /// same issue row. pub const REACHABILITY_REF: &str = "reachability"; +/// The client stream canopy's own views read: `bestool` is the agent whose +/// heartbeats carry the authoritative health checks and version. Other +/// clients' streams are stored and answered per client, but do not (yet) +/// feed the status board, version tracking, or health issues. +pub const DEFAULT_CLIENT: &str = "bestool"; + fn server_label(s: &Server) -> String { s.name .clone() @@ -245,7 +251,9 @@ impl Status { } let server_ids: Vec = monitored.iter().map(|s| s.id).collect(); - let statuses = Self::latest_for_servers(db, &server_ids).await?; + // Any-client freshness: a server is reachable while any of its + // clients is still reporting. + let statuses = Self::last_report_for_servers(db, &server_ids).await?; let status_map: std::collections::HashMap = statuses.into_iter().map(|s| (s.server_id, s)).collect(); let existing_issues = @@ -368,6 +376,7 @@ impl Status { .filter( server_id .eq(server) + .and(client.eq(DEFAULT_CLIENT)) .and(created_at.ge(diesel::dsl::sql("NOW() - INTERVAL '7 days'"))) .and(id.ne(Uuid::nil())), ) @@ -393,6 +402,7 @@ impl Status { .filter( server_id .eq(server) + .and(client.eq(DEFAULT_CLIENT)) .and(created_at.le(jiff_diesel::Timestamp::from(at))) .and(id.ne(Uuid::nil())), ) @@ -418,6 +428,7 @@ impl Status { .filter( server_id .eq(server) + .and(client.eq(DEFAULT_CLIENT)) .and(version.is_not_null()) .and(id.ne(Uuid::nil())), ) @@ -441,6 +452,36 @@ impl Status { // status row in the 7-day window for each server. The LIMIT 1 under // the lateral join reads one row per weekly partition through the // (server_id, created_at DESC) composite index. + let query = diesel::sql_query( + "SELECT st.* FROM unnest($1) AS s(id) \ + CROSS JOIN LATERAL ( \ + SELECT * FROM statuses \ + WHERE server_id = s.id \ + AND client = $2 \ + AND created_at >= NOW() - INTERVAL '7 days' \ + AND id != '00000000-0000-0000-0000-000000000000' \ + ORDER BY created_at DESC LIMIT 1 \ + ) st", + ) + .bind::, _>(server_ids) + .bind::(DEFAULT_CLIENT); + + query.load::(db).await.map_err(AppError::from) + } + + /// Most recent report per server from **any** client, for freshness + /// decisions only: a server is reporting while any of its clients is, + /// so one agent going quiet does not by itself make the server look + /// down. The returned row belongs to whichever client reported last — + /// read its timestamp, not its content. + pub async fn last_report_for_servers( + db: &mut AsyncPgConnection, + server_ids: &[Uuid], + ) -> Result> { + if server_ids.is_empty() { + return Ok(Vec::new()); + } + let query = diesel::sql_query( "SELECT st.* FROM unnest($1) AS s(id) \ CROSS JOIN LATERAL ( \ diff --git a/crates/database/tests/reachability_sweep.rs b/crates/database/tests/reachability_sweep.rs index e39e8778..cb6d68ba 100644 --- a/crates/database/tests/reachability_sweep.rs +++ b/crates/database/tests/reachability_sweep.rs @@ -232,3 +232,69 @@ async fn sweep_closes_issue_when_server_returns() { }) .await } + +async fn insert_status_for_client( + conn: &mut diesel_async::AsyncPgConnection, + server_id: Uuid, + minutes_ago: i32, + client: &str, +) { + sql_query( + r#" + INSERT INTO statuses (server_id, created_at, extra, client) + VALUES ($1, NOW() - ($2 || ' minutes')::INTERVAL, '{}'::jsonb, $3) + "#, + ) + .bind::(server_id) + .bind::(minutes_ago.to_string()) + .bind::(client) + .execute(conn) + .await + .expect("insert status"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn any_client_reporting_keeps_server_up() { + commons_tests::db::TestDb::run(async |mut conn, _| { + // 10-min threshold; bestool has gone quiet (15 min) but seedling is + // still reporting (1 min) — the server is not down. + let id = insert_server(&mut conn, "http://busy.invalid/", 600).await; + insert_status_for_client(&mut conn, id, 15, "bestool").await; + insert_status_for_client(&mut conn, id, 1, "seedling").await; + let filed = Status::sweep_reachability(&mut conn).await.expect("sweep"); + assert_eq!(filed, 0); + assert!(issue_for(&mut conn, id).await.is_none()); + }) + .await +} + +#[tokio::test(flavor = "multi_thread")] +async fn content_reads_stay_on_the_default_client_stream() { + commons_tests::db::TestDb::run(async |mut conn, _| { + // A newer seedling heartbeat must not become the "latest status" that + // canopy's views read health and version from. + let id = insert_server(&mut conn, "http://mixed.invalid/", 600).await; + insert_status_for_client(&mut conn, id, 5, "bestool").await; + insert_status_for_client(&mut conn, id, 1, "seedling").await; + + let latest = Status::latest_for_server(&mut conn, id) + .await + .expect("query") + .expect("has a bestool status"); + assert_eq!(latest.client, "bestool"); + + let latest_many = Status::latest_for_servers(&mut conn, &[id]) + .await + .expect("query"); + assert_eq!(latest_many.len(), 1); + assert_eq!(latest_many[0].client, "bestool"); + + // Freshness, by contrast, sees the most recent report of any client. + let last_report = Status::last_report_for_servers(&mut conn, &[id]) + .await + .expect("query"); + assert_eq!(last_report.len(), 1); + assert_eq!(last_report[0].client, "seedling"); + }) + .await +} From 29545762d9706d995048d9f2e4c05bab3ca683a7 Mon Sep 17 00:00:00 2001 From: Daniel Nash <38335330+dannash100@users.noreply.github.com> Date: Thu, 9 Jul 2026 08:11:12 +1200 Subject: [PATCH 5/7] feat(public): derive health issues from the bestool stream only --- crates/public-server/src/statuses.rs | 7 ++++++- crates/public-server/tests/statuses.rs | 11 +++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/crates/public-server/src/statuses.rs b/crates/public-server/src/statuses.rs index 9009df1b..3c94828f 100644 --- a/crates/public-server/src/statuses.rs +++ b/crates/public-server/src/statuses.rs @@ -290,7 +290,12 @@ async fn create( .save(conn) .await?; - file_health_events(conn, server_id, Some(id), &status, &tags).await?; + // Health issues derive from the authoritative client's checks + // only: another client's stream is stored and answered, but its + // checks must not open or close issues on the same refs. + if status.client == database::statuses::DEFAULT_CLIENT { + file_health_events(conn, server_id, Some(id), &status, &tags).await?; + } Ok(status) }) diff --git a/crates/public-server/tests/statuses.rs b/crates/public-server/tests/statuses.rs index 1bdda3bc..8eb7729f 100644 --- a/crates/public-server/tests/statuses.rs +++ b/crates/public-server/tests/statuses.rs @@ -2674,11 +2674,15 @@ async fn status_is_recorded_and_scoped_per_client() { assert!(body.get("check_severities").is_some()); // A seedling push is kept as its own stream and is sent no - // backup_now at all. + // backup_now at all. Its failing check must not open an issue: + // health issues derive from the bestool stream only. let response = public .post(&format!("/status/{server_id}")) .add_header("mtls-certificate", &cert) - .json(&serde_json::json!({ "client": "seedling", "health": [] })) + .json(&serde_json::json!({ + "client": "seedling", + "health": [{ "check": "seedling_proxy", "result": "failed" }], + })) .await; response.assert_status_ok(); let body: serde_json::Value = response.json(); @@ -2702,6 +2706,9 @@ async fn status_is_recorded_and_scoped_per_client() { let clients: Vec<&str> = rows.iter().map(|r| r.client.as_str()).collect(); assert_eq!(clients, ["bestool", "seedling"]); + // The seedling stream's failing check filed nothing. + assert_eq!(count_issues_for_server(&mut conn, server_id).await, 0); + // A non-string client is rejected. let response = public .post(&format!("/status/{server_id}")) From 930736d894bcb3606be8ad04507c85264f62bf80 Mon Sep 17 00:00:00 2001 From: Daniel Nash <38335330+dannash100@users.noreply.github.com> Date: Thu, 9 Jul 2026 09:54:52 +1200 Subject: [PATCH 6/7] feat(database): flag the authoritative client going quiet --- .workhorse/specs/public-server/status.md | 2 + crates/database/src/bin/seed.rs | 2 +- crates/database/src/statuses.rs | 92 +++++++++++++++++-- .../database/tests/it/reachability_sweep.rs | 52 ++++++++++- crates/public-server/src/statuses.rs | 4 +- 5 files changed, 136 insertions(+), 16 deletions(-) diff --git a/.workhorse/specs/public-server/status.md b/.workhorse/specs/public-server/status.md index f5a6f722..90c00fbc 100644 --- a/.workhorse/specs/public-server/status.md +++ b/.workhorse/specs/public-server/status.md @@ -32,6 +32,8 @@ The client is a label, not a second identity: authorisation stays the server bin Canopy records status per `(server, client)`, so two agents reporting for one server never overwrite each other: each keeps its own latest heartbeat, health checks, and history. A server is not treated as down while any of its clients is still reporting, so one agent going quiet does not by itself make the server look down. +A quiet agent is still surfaced: when the server is reporting but its `bestool` client — the stream Canopy's health view reads — has gone quiet past the server's down threshold, Canopy raises an issue for that client, distinct from server reachability, and resolves it when the client reports again. +A server that has never had a `bestool` client raises nothing. ## Response diff --git a/crates/database/src/bin/seed.rs b/crates/database/src/bin/seed.rs index 82686156..f5a50d8e 100644 --- a/crates/database/src/bin/seed.rs +++ b/crates/database/src/bin/seed.rs @@ -864,7 +864,7 @@ async fn seed_statuses( extra, healthy, health, - client: "bestool".to_owned(), + client: database::statuses::DEFAULT_CLIENT.to_owned(), }) .execute(conn) .await?; diff --git a/crates/database/src/statuses.rs b/crates/database/src/statuses.rs index 077b63e6..cbeb99e4 100644 --- a/crates/database/src/statuses.rs +++ b/crates/database/src/statuses.rs @@ -37,6 +37,14 @@ pub const REACHABILITY_REF: &str = "reachability"; /// feed the status board, version tracking, or health issues. pub const DEFAULT_CLIENT: &str = "bestool"; +/// Ref value for the one authoritative-client-quiet issue per server. Filed +/// when the server is still reporting (some client is fresh) but the +/// [`DEFAULT_CLIENT`] stream has gone quiet past the server's down threshold, +/// so a dead health reporter isn't masked by another agent keeping the server +/// "up". Distinct from [`REACHABILITY_REF`], which covers the fully quiet +/// server. +pub const CLIENT_STALE_REF: &str = "client-stale/bestool"; + fn server_label(s: &Server) -> String { s.name .clone() @@ -125,7 +133,7 @@ impl Default for NewStatus { extra: serde_json::Value::Object(Default::default()), healthy: true, health: serde_json::Value::Array(Default::default()), - client: "bestool".to_owned(), + client: DEFAULT_CLIENT.to_owned(), } } } @@ -175,7 +183,7 @@ impl Status { health: serde_json::Value::Array(Default::default()), // Canopy-generated reachability status; attributed to the // default client stream. - client: "bestool".to_owned(), + client: DEFAULT_CLIENT.to_owned(), }) } Err(err) => { @@ -265,6 +273,20 @@ impl Status { .filter_map(|i| i.server_id.map(|sid| (sid, i))) .collect(); + // The authoritative client's own freshness, for the client-stale pass + // below (`latest_for_servers` is pinned to [`DEFAULT_CLIENT`]). + let bestool_statuses = Self::latest_for_servers(db, &server_ids).await?; + let bestool_map: std::collections::HashMap = bestool_statuses + .into_iter() + .map(|s| (s.server_id, s)) + .collect(); + let stale_issues = + Issue::list_by_source_ref(db, CANOPY_SOURCE, CLIENT_STALE_REF, &server_ids).await?; + let stale_issue_map: std::collections::HashMap = stale_issues + .iter() + .filter_map(|i| i.server_id.map(|sid| (sid, i))) + .collect(); + let now = Timestamp::now(); let mut filed = 0usize; for server in &monitored { @@ -281,9 +303,9 @@ impl Status { let existing = issue_map.get(&server.id).copied(); let event = match (down, existing) { - (false, None) => continue, - (false, Some(issue)) if !issue.active => continue, - (false, Some(_)) => NewEvent { + (false, None) => None, + (false, Some(issue)) if !issue.active => None, + (false, Some(_)) => Some(NewEvent { source: CANOPY_SOURCE.into(), r#ref: REACHABILITY_REF.into(), severity: Some(Severity::Info), @@ -291,8 +313,8 @@ impl Status { message: format!("Server {} is reachable again", server_label(server)), active: Some(false), occurred_at: Some(now), - }, - (true, _) => NewEvent { + }), + (true, _) => Some(NewEvent { source: CANOPY_SOURCE.into(), r#ref: REACHABILITY_REF.into(), severity: Some(Severity::Error), @@ -312,10 +334,60 @@ impl Status { }, active: Some(true), occurred_at: Some(now), - }, + }), }; - event.save(db, server.id, None).await?; - filed += 1; + if let Some(event) = event { + event.save(db, server.id, None).await?; + filed += 1; + } + + // Client-stale pass: the server-down issue above covers a fully + // quiet server; this covers the authoritative stream going quiet + // while another client keeps the server up. + if !down { + let bestool_elapsed: Option = bestool_map + .get(&server.id) + .map(|s| now.duration_since(s.created_at).abs()); + let existing_stale = stale_issue_map.get(&server.id).copied(); + let stale_event = match (bestool_elapsed, existing_stale) { + // Nothing in the window: either the client never existed + // (nothing went quiet) or it's been gone so long its rows + // aged out — in which case the already-open issue stays + // open until the client actually reports again. + (None, _) => None, + (Some(e), _) if e >= threshold => Some(NewEvent { + source: CANOPY_SOURCE.into(), + r#ref: CLIENT_STALE_REF.into(), + severity: Some(Severity::Error), + description: None, + message: format!( + "Client {DEFAULT_CLIENT} on server {} has not reported for {} (threshold {}), though the server is still reporting", + server_label(server), + format_secs(e.as_secs()), + format_secs(threshold.as_secs()), + ), + active: Some(true), + occurred_at: Some(now), + }), + (Some(_), Some(issue)) if issue.active => Some(NewEvent { + source: CANOPY_SOURCE.into(), + r#ref: CLIENT_STALE_REF.into(), + severity: Some(Severity::Info), + description: None, + message: format!( + "Client {DEFAULT_CLIENT} on server {} is reporting again", + server_label(server), + ), + active: Some(false), + occurred_at: Some(now), + }), + (Some(_), _) => None, + }; + if let Some(event) = stale_event { + event.save(db, server.id, None).await?; + filed += 1; + } + } } Ok(filed) } diff --git a/crates/database/tests/it/reachability_sweep.rs b/crates/database/tests/it/reachability_sweep.rs index cb6d68ba..e822e76d 100644 --- a/crates/database/tests/it/reachability_sweep.rs +++ b/crates/database/tests/it/reachability_sweep.rs @@ -1,7 +1,7 @@ use commons_types::issue::Severity; use database::{ issues::Issue, - statuses::{CANOPY_SOURCE, REACHABILITY_REF, Status}, + statuses::{CANOPY_SOURCE, CLIENT_STALE_REF, REACHABILITY_REF, Status}, }; use diesel::{QueryableByName, sql_query, sql_types}; use diesel_async::RunQueryDsl; @@ -71,6 +71,17 @@ async fn issue_for(conn: &mut diesel_async::AsyncPgConnection, server_id: Uuid) .next() } +async fn stale_issue_for( + conn: &mut diesel_async::AsyncPgConnection, + server_id: Uuid, +) -> Option { + Issue::list_by_source_ref(conn, CANOPY_SOURCE, CLIENT_STALE_REF, &[server_id]) + .await + .expect("list issues") + .into_iter() + .next() +} + #[tokio::test(flavor = "multi_thread")] async fn sweep_files_error_when_threshold_crossed() { commons_tests::db::TestDb::run(async |mut conn, _| { @@ -254,16 +265,51 @@ async fn insert_status_for_client( } #[tokio::test(flavor = "multi_thread")] -async fn any_client_reporting_keeps_server_up() { +async fn any_client_reporting_keeps_server_up_but_flags_the_quiet_client() { commons_tests::db::TestDb::run(async |mut conn, _| { // 10-min threshold; bestool has gone quiet (15 min) but seedling is - // still reporting (1 min) — the server is not down. + // still reporting (1 min) — the server is not down, but the quiet + // authoritative client gets its own issue. let id = insert_server(&mut conn, "http://busy.invalid/", 600).await; insert_status_for_client(&mut conn, id, 15, "bestool").await; insert_status_for_client(&mut conn, id, 1, "seedling").await; let filed = Status::sweep_reachability(&mut conn).await.expect("sweep"); + assert_eq!(filed, 1); + assert!(issue_for(&mut conn, id).await.is_none()); + let stale = stale_issue_for(&mut conn, id).await.expect("stale issue"); + assert_eq!(stale.severity, Severity::Error); + assert!(stale.active); + }) + .await +} + +#[tokio::test(flavor = "multi_thread")] +async fn client_stale_issue_resolves_when_the_client_reports_again() { + commons_tests::db::TestDb::run(async |mut conn, _| { + let id = insert_server(&mut conn, "http://revived.invalid/", 600).await; + insert_status_for_client(&mut conn, id, 15, "bestool").await; + insert_status_for_client(&mut conn, id, 1, "seedling").await; + Status::sweep_reachability(&mut conn).await.expect("sweep"); + assert!(stale_issue_for(&mut conn, id).await.expect("filed").active); + + insert_status_for_client(&mut conn, id, 0, "bestool").await; + Status::sweep_reachability(&mut conn).await.expect("sweep"); + let stale = stale_issue_for(&mut conn, id).await.expect("still exists"); + assert!(!stale.active); + }) + .await +} + +#[tokio::test(flavor = "multi_thread")] +async fn client_that_never_reported_raises_nothing() { + commons_tests::db::TestDb::run(async |mut conn, _| { + // A seedling-only host has no bestool stream to go quiet. + let id = insert_server(&mut conn, "http://seedling-only.invalid/", 600).await; + insert_status_for_client(&mut conn, id, 1, "seedling").await; + let filed = Status::sweep_reachability(&mut conn).await.expect("sweep"); assert_eq!(filed, 0); assert!(issue_for(&mut conn, id).await.is_none()); + assert!(stale_issue_for(&mut conn, id).await.is_none()); }) .await } diff --git a/crates/public-server/src/statuses.rs b/crates/public-server/src/statuses.rs index f3a51741..35adcbe2 100644 --- a/crates/public-server/src/statuses.rs +++ b/crates/public-server/src/statuses.rs @@ -234,7 +234,7 @@ async fn create( // schedule-due), riding the heartbeat response. Only the client that runs // backups is told; others get no `backup_now` at all. Empty for an // ungrouped server or one whose group has no `ready` backup config. - let backup_now = if client == "bestool" { + let backup_now = if client == database::statuses::DEFAULT_CLIENT { Some(match server.group_id { Some(group_id) => { backups_due_now_for_server(&mut db, server_id, group_id, Timestamp::now()).await? @@ -701,7 +701,7 @@ fn split_health_from_extra( // The reporting agent, defaulting to `bestool` so reporters that name none // keep their existing stream. let client = match obj.remove("client") { - None => "bestool".to_owned(), + None => database::statuses::DEFAULT_CLIENT.to_owned(), Some(serde_json::Value::String(s)) => s, Some(_) => return Err(AppError::BadRequest("`client` must be a string".into())), }; From 67bcd8f67a4432ca314f594c59f88829c9716482 Mon Sep 17 00:00:00 2001 From: Daniel Nash <38335330+dannash100@users.noreply.github.com> Date: Thu, 9 Jul 2026 12:08:39 +1200 Subject: [PATCH 7/7] chore(public): refresh committed OpenAPI snapshot --- crates/public-server/openapi.json | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/crates/public-server/openapi.json b/crates/public-server/openapi.json index 2bb8f05f..157f7078 100644 --- a/crates/public-server/openapi.json +++ b/crates/public-server/openapi.json @@ -739,7 +739,7 @@ "statuses" ], "summary": "Submit a status heartbeat for a server.", - "description": "Records a periodic status push against the server identified in the\npath: overall self-reported health, a per-check breakdown, and any\nfree-form extra data. Each failed or warning check opens (or keeps\nopen) an issue at that check's operator-configured severity, and each\npassed check closes any issue it previously opened; the server's\ntracked software version is also updated from the payload.\n\nThe calling device must be the one enrolled for this exact server (or\nhold the admin role). The response carries only return-path\ninstructions: a `backup_now` list of backup types the server should\nback up immediately — devices should treat a non-empty list as a\nprompt to run those backups and report them afterwards — a\n`check_severities` map describing how canopy classifies each known\nhealthcheck for this server (`skip`/`warn`/`fail`), and the server's\neffective `tags` (as served by `GET /tags`). The stored status record\nis not echoed back.", + "description": "Records a periodic status push against the server identified in the\npath: overall self-reported health, a per-check breakdown, and any\nfree-form extra data. Each failed or warning check opens (or keeps\nopen) an issue at that check's operator-configured severity, and each\npassed check closes any issue it previously opened; the server's\ntracked software version is also updated from the payload.\n\nThe calling device must be the one enrolled for this exact server (or\nhold the admin role). The response carries only return-path\ninstructions, scoped to the reporting client: a `backup_now` list of\nbackup types the server should back up immediately — sent only to the\n`bestool` client (the agent that runs backups), which should treat a\nnon-empty list as a prompt to run those backups and report them\nafterwards; other clients get no `backup_now` at all — a\n`check_severities` map describing how canopy classifies each known\nhealthcheck for this server (`skip`/`warn`/`fail`), and the server's\neffective `tags` (as served by `GET /tags`). The stored status record\nis not echoed back.", "operationId": "submit_status", "parameters": [ { @@ -2045,6 +2045,13 @@ "health" ], "properties": { + "client": { + "type": [ + "string", + "null" + ], + "description": "The reporting agent this status is attributed to (e.g. `bestool`,\n`seedling`). **Absent means `bestool`**, so agents that predate this\nfield keep reporting under their existing stream. Status is recorded\nper (server, client), so concurrent agents on one host never overwrite\neach other." + }, "health": { "type": "array", "items": { @@ -2068,17 +2075,19 @@ "type": "object", "description": "The status-push response: only the return-path instructions the device\ncan act on. The stored status record is deliberately not echoed back —\nthe device already has everything it sent.", "required": [ - "backup_now", "check_severities", "tags" ], "properties": { "backup_now": { - "type": "array", + "type": [ + "array", + "null" + ], "items": { "type": "string" }, - "description": "Backup types the server should back up now: operator-requested\none-offs plus scheduled backups that are due. Each serializes as a\nplain string (e.g. `\"tamanu-postgres\"`). The device should run each\nlisted type, then report via `POST /backup-report`; an empty list\nmeans nothing to do." + "description": "Backup types the server should back up now: operator-requested\none-offs plus scheduled backups that are due. Each serializes as a\nplain string (e.g. `\"tamanu-postgres\"`). The device should run each\nlisted type, then report via `POST /backup-report`; an empty list\nmeans nothing to do. Sent only to the client that runs backups\n(`bestool`); omitted for other reporting clients." }, "check_severities": { "type": "object",