From 63c22cce9db421397550beda72a194d5264847e3 Mon Sep 17 00:00:00 2001 From: JuliaEdom Date: Sun, 19 Jul 2026 11:13:02 +0300 Subject: [PATCH] fix(health): keep `message` in the public health view (SDK contract) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The S-3 sanitizer dropped `message` from each /v1/health component, but the SDK `HealthComponent` model requires a string `message` — so `client.health()`'s `HealthStatus.model_validate` raised 7 missing-field errors. This only surfaced in the push-only E2E and Staging lanes (not PR CI): both failed `test_ops_agent_journey` identically. The leak was the message *content* (`f"Kafka unavailable: {exc}"`, counts, backend identity), not the field. Blank the message to "" instead of dropping it — shape preserved, content gone. The unit suite now validates the view against the SDK `HealthStatus` model so this class of break fails here, not only in the push-only lanes; my earlier test wrongly asserted the message-less shape. Co-Authored-By: Claude Fable 5 --- src/serving/api/main.py | 17 +++++++++++------ tests/unit/test_health_public_view.py | 21 ++++++++++++++++++--- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/serving/api/main.py b/src/serving/api/main.py index 3a2e217..2325f19 100644 --- a/src/serving/api/main.py +++ b/src/serving/api/main.py @@ -740,12 +740,14 @@ def _public_health_view(payload: dict) -> dict: `/v1/health` is auth-exempt, yet each component's `message`/`metrics` carry recon: `f"Kafka unavailable: {exc}"` (internal hostnames/URLs), broker/topic - counts, cluster sizes, and the serving backend's identity. Keep the overall - status, each component's name/status/source (the documented contract), and - only the allowlisted operational gauges (pool utilization, data freshness); - drop `message` and every non-allowlisted metric. Agents still get the - per-component status they use to caveat freshness, and pool-utilization - observability (its only exposure — not on /metrics) is preserved. + counts, cluster sizes, and the serving backend's identity. Neutralise the + `message` text (it is where every `{exc}` and count lives) and keep only the + allowlisted operational gauges (pool utilization, data freshness); keep the + overall status and each component's name/status/source. The field *shape* is + unchanged — `message` stays a string and `metrics` a dict — because the SDK's + `HealthComponent` model requires them; only the sensitive *content* is + dropped. Agents still get per-component status to caveat freshness, and + pool-utilization observability (its only exposure — not on /metrics) is kept. """ components = [] for c in payload.get("components", []): @@ -754,6 +756,9 @@ def _public_health_view(payload: dict) -> dict: { "name": c.get("name"), "status": c.get("status"), + # Blanked, not dropped: the SDK HealthComponent requires a string + # `message`, but its content (error text, counts) is the leak. + "message": "", "source": c.get("source"), "metrics": metrics, } diff --git a/tests/unit/test_health_public_view.py b/tests/unit/test_health_public_view.py index 40c4612..4aa66ce 100644 --- a/tests/unit/test_health_public_view.py +++ b/tests/unit/test_health_public_view.py @@ -9,6 +9,8 @@ from __future__ import annotations +from agentflow.models import HealthStatus + from src.serving.api.main import _public_health_view _RAW_PAYLOAD = { @@ -40,10 +42,23 @@ } -def test_view_keeps_only_name_status_source_per_component() -> None: +def test_view_component_fields_match_sdk_contract() -> None: view = _public_health_view(_RAW_PAYLOAD) for component in view["components"]: - assert set(component) == {"name", "status", "source", "metrics"} + assert set(component) == {"name", "status", "message", "source", "metrics"} + + +def test_view_still_validates_against_sdk_health_model() -> None: + # The SDK's HealthComponent requires string `message`/`source` and the + # top-level HealthStatus requires `checked_at`. The push-only E2E lane + # validates the /v1/health payload through this model — dropping `message` + # broke it. Pin the contract here so it fails in unit tests, not only e2e. + HealthStatus.model_validate(_public_health_view(_RAW_PAYLOAD)) + + +def test_view_blanks_message_but_keeps_it_a_string() -> None: + for component in _public_health_view(_RAW_PAYLOAD)["components"]: + assert component["message"] == "" def test_view_drops_error_strings_counts_and_backend_identity() -> None: @@ -95,5 +110,5 @@ def test_view_tolerates_missing_optional_keys() -> None: view = _public_health_view({"status": "healthy", "components": [{"name": "x", "status": "ok"}]}) assert view == { "status": "healthy", - "components": [{"name": "x", "status": "ok", "source": None, "metrics": {}}], + "components": [{"name": "x", "status": "ok", "message": "", "source": None, "metrics": {}}], }