diff --git a/src/core/jsonrpc.rs b/src/core/jsonrpc.rs index 8af8a33ff..5b47190bd 100644 --- a/src/core/jsonrpc.rs +++ b/src/core/jsonrpc.rs @@ -1056,35 +1056,50 @@ pub(super) fn with_cors_headers(mut response: Response, origin: Option<&str>) -> } /// Handler for the health check endpoint. +/// +/// Liveness is granular (#3312): a single degraded *background* component +/// (scheduler, channels, update_checker, …) no longer 503s the whole container. +/// `/health` returns 503 only when a *critical* component is unhealthy (see +/// `health::CRITICAL_COMPONENTS`); otherwise it returns 200 — with a `degraded` +/// flag and per-component buckets in the body so readiness probes and operators +/// can still see partial failures. async fn health_handler() -> impl IntoResponse { let snapshot = crate::openhuman::health::snapshot(); - let unhealthy: Vec<&str> = snapshot - .components - .iter() - .filter_map(|(name, c)| { - if c.status == "ok" || c.status == "starting" { - None - } else { - Some(name.as_str()) - } - }) - .collect(); - let is_ok = unhealthy.is_empty(); + let verdict = crate::openhuman::health::verdict(&snapshot); - let status = if is_ok { + let status = if verdict.healthy { StatusCode::OK } else { StatusCode::SERVICE_UNAVAILABLE }; + // Augment the snapshot body with the verdict so the components map stays + // backward-compatible while exposing overall liveness/readiness. + let mut body = serde_json::to_value(&snapshot).unwrap_or_else(|_| serde_json::json!({})); + if let Some(obj) = body.as_object_mut() { + obj.insert("healthy".to_string(), serde_json::json!(verdict.healthy)); + obj.insert("degraded".to_string(), serde_json::json!(verdict.degraded)); + obj.insert( + "critical_unhealthy".to_string(), + serde_json::json!(verdict.critical_unhealthy), + ); + obj.insert( + "degraded_components".to_string(), + serde_json::json!(verdict.degraded_components), + ); + } + tracing::debug!( - "[health] status={} components={} unhealthy={:?}", + "[health] status={} components={} healthy={} degraded={} critical_unhealthy={:?} degraded_components={:?}", status.as_u16(), snapshot.components.len(), - unhealthy + verdict.healthy, + verdict.degraded, + verdict.critical_unhealthy, + verdict.degraded_components, ); - (status, Json(snapshot)) + (status, Json(body)) } /// Handler for the schema discovery endpoint. diff --git a/src/core/jsonrpc_tests.rs b/src/core/jsonrpc_tests.rs index be17c546e..993e7889f 100644 --- a/src/core/jsonrpc_tests.rs +++ b/src/core/jsonrpc_tests.rs @@ -1195,20 +1195,43 @@ async fn test_http_health_handler_returns_correct_status() { .as_object() .expect("components should be an object"); - // Derive the expected HTTP status solely from the response body so the - // test asserts internal consistency of the handler rather than racing on - // live component state. - let body_says_ok = components.values().all(|c| { - let s = c["status"].as_str().unwrap_or(""); - s == "ok" || s == "starting" - }); - let expected_status = if body_says_ok { + // Granular liveness (#3312): the HTTP status is driven by the `healthy` + // verdict (no *critical* component unhealthy), not by all-components-ok. + // Derive the expectation from the body so the test asserts the handler's + // internal consistency rather than racing on live component state. + let body_healthy = snapshot["healthy"] + .as_bool() + .expect("body exposes a `healthy` verdict flag"); + let expected_status = if body_healthy { StatusCode::OK } else { StatusCode::SERVICE_UNAVAILABLE }; - assert_eq!(status, expected_status); + + // `healthy` must mean "no critical component is unhealthy", and any + // unhealthy component must be bucketed as either critical or degraded. + let critical_unhealthy = snapshot["critical_unhealthy"] + .as_array() + .expect("body exposes critical_unhealthy"); + assert_eq!(body_healthy, critical_unhealthy.is_empty()); + + let unhealthy_count = components + .values() + .filter(|c| { + let s = c["status"].as_str().unwrap_or(""); + s != "ok" && s != "starting" + }) + .count(); + let degraded_count = snapshot["degraded_components"] + .as_array() + .expect("body exposes degraded_components") + .len(); + assert_eq!( + unhealthy_count, + critical_unhealthy.len() + degraded_count, + "every unhealthy component is bucketed as critical or degraded" + ); } #[tokio::test] diff --git a/src/openhuman/health/README.md b/src/openhuman/health/README.md index fe7f1471b..08ef3f921 100644 --- a/src/openhuman/health/README.md +++ b/src/openhuman/health/README.md @@ -7,6 +7,7 @@ In-process health registry for the OpenHuman core. Tracks per-component liveness - Maintain a process-global registry of named components, each with `status`, `updated_at`, `last_ok`, `last_error`, `restart_count`. - Provide mutators: `mark_component_ok`, `mark_component_error`, `bump_component_restart`. - Produce a point-in-time `HealthSnapshot` (PID, uptime seconds, components map) and its JSON form. +- Classify a snapshot into a `HealthVerdict` (`verdict()`): a single degraded *non-critical* component no longer makes the container unhealthy — `/health` returns 503 only when a *critical* component (`CRITICAL_COMPONENTS` = `core`, `memory_tree_db`) is unhealthy; non-critical components (scheduler, channels, update_checker, …) return 200 + a `degraded` flag (#3312). - Drive component state automatically from `DomainEvent`s via an event-bus subscriber. - Expose `health.snapshot` and `health.system_info` RPC/CLI controllers. @@ -23,8 +24,10 @@ In-process health registry for the OpenHuman core. Tracks per-component liveness ## Public surface From `core.rs` (re-exported via `pub use core::*`): -- Types: `ComponentHealth`, `HealthSnapshot`. -- Functions: `mark_component_ok(component)`, `mark_component_error(component, error)`, `bump_component_restart(component)`, `snapshot() -> HealthSnapshot`, `snapshot_json() -> serde_json::Value`. +- Types: `ComponentHealth`, `HealthSnapshot`, `HealthVerdict`. +- Functions: `mark_component_ok(component)`, `mark_component_error(component, error)`, `bump_component_restart(component)`, `snapshot() -> HealthSnapshot`, `snapshot_json() -> serde_json::Value`, `verdict(&HealthSnapshot) -> HealthVerdict`, `is_critical_component(name) -> bool`. + +The HTTP `GET /health` handler (`core::jsonrpc::health_handler`) uses `verdict()` for its status code (200 unless a critical component is unhealthy) and adds `healthy` / `degraded` / `critical_unhealthy` / `degraded_components` fields alongside the `components` map in the body. The `components` map shape is unchanged — the new fields are additive. From `ops.rs` (re-exported via `pub use ops::*`, also aliased `pub use ops as rpc`): - `health_snapshot() -> RpcOutcome`, `system_info() -> RpcOutcome`, and the `SystemInfo` struct. diff --git a/src/openhuman/health/core.rs b/src/openhuman/health/core.rs index 7811e883c..d6a920603 100644 --- a/src/openhuman/health/core.rs +++ b/src/openhuman/health/core.rs @@ -109,6 +109,77 @@ pub fn snapshot_json() -> serde_json::Value { }) } +/// Components whose sustained failure means the whole container should be +/// recycled — and the **only** ones whose unhealth makes `/health` return 503. +/// +/// Everything else is a degradable background service whose failure must NOT +/// flip the container `unhealthy` (#3312): a single cron-job timeout marked the +/// `scheduler` component `error` and 503'd the container for 7h43m even though +/// the core RPC was serving fine the whole time. `scheduler`, `channels`, and +/// `update_checker` are therefore intentionally **non-critical**. +/// +/// - `core` — the core process / RPC serving capability itself. +/// - `memory_tree_db` — the memory database. Its health signal is a *debounced* +/// circuit breaker that only trips after several consecutive schema-init +/// failures (a genuine, restart-worthy data-layer fault), so unlike the +/// scheduler case it does not false-trip on a transient blip. +/// +/// New components default to **non-critical**: add a name here deliberately when +/// its failure should recycle the container. +const CRITICAL_COMPONENTS: &[&str] = &["core", "memory_tree_db"]; + +/// Whether `name` is a critical component (see [`CRITICAL_COMPONENTS`]). +pub fn is_critical_component(name: &str) -> bool { + CRITICAL_COMPONENTS.contains(&name) +} + +/// A component status counts as healthy for liveness purposes when it is `ok` +/// or still `starting` (boot grace — a component that hasn't reported yet must +/// not 503 the container). +fn is_healthy_status(status: &str) -> bool { + status == "ok" || status == "starting" +} + +/// Liveness/readiness verdict derived from a [`HealthSnapshot`]. Pure function +/// of the snapshot so it is unit-testable without the global registry. +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] +pub struct HealthVerdict { + /// True when no *critical* component is unhealthy → `/health` returns 200. + pub healthy: bool, + /// True when at least one *non-critical* component is unhealthy. The + /// container stays live (200) but is degraded — surfaced for readiness / + /// observability, not for the liveness 503. + pub degraded: bool, + /// Names of unhealthy critical components — these drive the 503. + pub critical_unhealthy: Vec, + /// Names of unhealthy non-critical components (informational). + pub degraded_components: Vec, +} + +/// Classify a snapshot into a [`HealthVerdict`]: a single degraded background +/// component no longer makes the whole container unhealthy — only an unhealthy +/// *critical* component does (#3312). +pub fn verdict(snapshot: &HealthSnapshot) -> HealthVerdict { + let mut critical_unhealthy = Vec::new(); + let mut degraded_components = Vec::new(); + for (name, component) in &snapshot.components { + if is_healthy_status(&component.status) { + continue; + } + if is_critical_component(name) { + critical_unhealthy.push(name.clone()); + } else { + degraded_components.push(name.clone()); + } + } + HealthVerdict { + healthy: critical_unhealthy.is_empty(), + degraded: !degraded_components.is_empty(), + critical_unhealthy, + degraded_components, + } +} + #[cfg(test)] mod tests { use super::*; @@ -188,4 +259,91 @@ mod tests { assert!(component_json["last_ok"].as_str().is_some()); assert!(json["uptime_seconds"].as_u64().is_some()); } + + // ── Critical-component verdict (#3312) ──────────────────────────────── + + fn component(status: &str) -> ComponentHealth { + ComponentHealth { + status: status.to_string(), + updated_at: "2026-06-10T00:00:00Z".to_string(), + last_ok: None, + last_error: None, + restart_count: 0, + } + } + + /// Build a synthetic snapshot from `(name, status)` pairs — lets the + /// verdict be tested without mutating the process-global registry. + fn snapshot_of(components: &[(&str, &str)]) -> HealthSnapshot { + HealthSnapshot { + pid: 1, + updated_at: "2026-06-10T00:00:00Z".to_string(), + uptime_seconds: 0, + components: components + .iter() + .map(|(n, s)| ((*n).to_string(), component(s))) + .collect(), + } + } + + #[test] + fn critical_set_membership() { + assert!(is_critical_component("core")); + assert!(is_critical_component("memory_tree_db")); + assert!(!is_critical_component("scheduler")); + assert!(!is_critical_component("channels")); + assert!(!is_critical_component("update_checker")); + } + + #[test] + fn all_ok_is_healthy_and_not_degraded() { + let v = verdict(&snapshot_of(&[("core", "ok"), ("scheduler", "ok")])); + assert!(v.healthy); + assert!(!v.degraded); + assert!(v.critical_unhealthy.is_empty()); + assert!(v.degraded_components.is_empty()); + } + + #[test] + fn noncritical_failure_stays_healthy_but_degraded() { + // The exact #3312 case: scheduler in error must NOT 503 the container. + let v = verdict(&snapshot_of(&[("core", "ok"), ("scheduler", "error")])); + assert!(v.healthy, "a degraded background service must not 503"); + assert!(v.degraded); + assert_eq!(v.degraded_components, vec!["scheduler".to_string()]); + assert!(v.critical_unhealthy.is_empty()); + } + + #[test] + fn critical_failure_is_unhealthy() { + let v = verdict(&snapshot_of(&[("memory_tree_db", "error")])); + assert!( + !v.healthy, + "a critical component failure 503s the container" + ); + assert_eq!(v.critical_unhealthy, vec!["memory_tree_db".to_string()]); + } + + #[test] + fn mixed_failures_report_both_buckets_and_503() { + let v = verdict(&snapshot_of(&[ + ("core", "error"), + ("scheduler", "error"), + ("channels", "ok"), + ])); + assert!(!v.healthy); + assert_eq!(v.critical_unhealthy, vec!["core".to_string()]); + assert_eq!(v.degraded_components, vec!["scheduler".to_string()]); + } + + #[test] + fn starting_status_is_treated_as_healthy() { + // Boot grace: a not-yet-reported component must not 503 nor degrade. + let v = verdict(&snapshot_of(&[ + ("core", "starting"), + ("scheduler", "starting"), + ])); + assert!(v.healthy); + assert!(!v.degraded); + } }