From 1cc592bef6f6fa01ec4aa809f5dd3b0fcbb32e10 Mon Sep 17 00:00:00 2001 From: agent_coder Date: Sun, 5 Jul 2026 10:47:29 +0300 Subject: [PATCH] =?UTF-8?q?fix(stability):=20stage=206=20review=20round=20?= =?UTF-8?q?1=20=E2=80=94=20/ping=20degrades=20to=20pure=20connectivity=20w?= =?UTF-8?q?hen=20watchdog=20disabled?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding (low, real): the comment claimed /ping degenerates to a pure connectivity check when TG_WATCHDOG_ENABLED=false, but _wd_last_ok_monotonic is also stamped by _restart_client (on the disconnect-flap path, which runs before the watchdog-enabled gate), so with the watchdog off one flap sets age and nothing ever refreshes it — age grows unbounded past the threshold and /ping returns 503 on a live connection, spuriously failing the container healthcheck and triggering an autoheal restart after every flap. Fix: gate the staleness branch on the watchdog being enabled — healthy = connected and (not Config["tg_watchdog_enabled"] or age is None or age < threshold) so with the watchdog disabled /ping is a pure connectivity check (matching the intent), and correct the comment to note a flap-restart can stamp age even when the watchdog is off. New test test_ping_watchdog_disabled_stale_age_still_healthy: watchdog off + connected + stale age => 200 ok. Adversarially validated — reverting the gate reds the new test (503) while the watchdog-ON stale-probe test stays green. Co-Authored-By: Claude Opus 4.8 (1M context) --- api_server.py | 16 ++++++++++++---- tests/test_stage6_healthcheck.py | 15 +++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/api_server.py b/api_server.py index bba4f43..6fbe2ea 100644 --- a/api_server.py +++ b/api_server.py @@ -1083,10 +1083,18 @@ async def ping() -> JSONResponse: # age is None right after boot: the watchdog hasn't run its first probe yet. Treat that # as healthy (gate on connected only) so a freshly-started container is not killed before # its first probe cycle — otherwise start_period would have to cover a full watchdog interval. - # Note: when the watchdog is DISABLED (TG_WATCHDOG_ENABLED=false) age stays None forever, - # so /ping degenerates to a pure connectivity check and cannot detect a stale-but-connected - # ("zombie") session — that TG-liveness signal only exists while the watchdog runs. - healthy = connected and (age is None or age < threshold) + # + # The staleness branch (age >= threshold => degraded) is only meaningful while the watchdog + # is running to refresh age. With the watchdog DISABLED (TG_WATCHDOG_ENABLED=false) nothing + # refreshes age — yet a disconnect-flap restart can still stamp it once (see _restart_client, + # which runs before the watchdog-enabled gate), after which age only grows. Letting that + # stale age drive /ping to 503 would spuriously fail the container healthcheck on a live + # connection and trigger an autoheal restart. So gate staleness on the watchdog being on; + # with it off, /ping is a pure connectivity check (no zombie-session detection — that + # TG-liveness signal only exists while the watchdog runs). + healthy = connected and ( + not Config["tg_watchdog_enabled"] or age is None or age < threshold + ) return JSONResponse( { "status": "ok" if healthy else "degraded", diff --git a/tests/test_stage6_healthcheck.py b/tests/test_stage6_healthcheck.py index 8f54b63..2a1d5ff 100644 --- a/tests/test_stage6_healthcheck.py +++ b/tests/test_stage6_healthcheck.py @@ -99,6 +99,21 @@ def test_ping_degraded_stale_probe(patch_client): assert fake.client.get_me_calls == 0 +def test_ping_watchdog_disabled_stale_age_still_healthy(patch_client, monkeypatch): + # With the watchdog OFF, nothing refreshes age (a disconnect-flap restart can stamp it + # once, then it only grows). A stale age must NOT drive /ping to 503 on a live connection + # — that would spuriously fail the healthcheck and trigger an autoheal restart. So with the + # watchdog disabled, /ping is a pure connectivity check: connected + stale age => healthy. + monkeypatch.setitem(api_server.Config, "tg_watchdog_enabled", False) + fake, tc = patch_client(age=THRESHOLD + 100, is_connected=True) + r = tc.get("/ping") + assert r.status_code == 200 + body = r.json() + assert body["status"] == "ok" + assert body["connected"] is True + assert fake.client.get_me_calls == 0 + + def test_ping_degraded_disconnected(patch_client): # Even with a fresh probe age, a disconnected client is unhealthy. fake, tc = patch_client(age=1.0, is_connected=False)