fix(scale-to-zero): give a departed dashboard client the full idle_timeout grace
Review finding: dashboard_client_last_seen() discarded the marker once it was >= 45s old, so after the client disconnected the gateway fell back to its own (much older) _last_inbound_at and suspended ~45-75s later, not idle_timeout later as the PR claimed. The staging release leg had in fact shown 46s. The marker mtime is a timestamp of real inbound; is_idle already judges recency. Drop the staleness cutoff entirely and always fold the raw mtime into the inbound clock (max with _last_inbound_at). An old marker is harmless: it is outside idle_timeout just like an old _last_inbound_at. Also: - touch the marker immediately after ws.accept(), before the ready/skin setup, so a client waiting on a slow ready frame is still visible - make the newer-message-wins test discriminating (timeout 10s, chat 5s, marker 40s: choosing the marker would read idle) - add < idle_timeout / >= idle_timeout / ancient-marker boundary tests Re-validated live on hermes-agent-stg-test-6698: last client frame 03:18:11Z -> going dormant 03:20:17Z (126s = 120s timeout + watcher tick) while the gateway's own inbound clock was 368s stale. Mutation check vs origin/main files: run.py hunk reverted -> 4 fail, ws.py reverted -> 3 fail, "prefer marker over max" -> 1 fail.
This commit is contained in:
@@ -9488,7 +9488,8 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
||||
# process refreshes on every WS frame (gateway/scale_to_zero.py). Fold
|
||||
# it into the inbound clock rather than adding a conjunct: the client
|
||||
# then gets the same idle_timeout grace after it disconnects as a chat
|
||||
# message does, and a lingering marker cannot pin the box (stale -> None).
|
||||
# message does, and a lingering marker cannot pin the box (an old mtime
|
||||
# is outside idle_timeout just like an old _last_inbound_at).
|
||||
last_inbound = self._last_inbound_at
|
||||
try:
|
||||
from gateway.scale_to_zero import dashboard_client_last_seen
|
||||
|
||||
@@ -167,15 +167,19 @@ def is_idle(
|
||||
# Dashboard-client liveness marker. The dashboard process (tui_gateway/ws.py,
|
||||
# a DIFFERENT process from the gateway on hosted instances) touches this file
|
||||
# on every /api/ws connect and inbound frame — the desktop app, web dashboard
|
||||
# and TUI all send `gateway.ping` every 15s and treat 45s of silence as dead
|
||||
# (apps/shared/src/json-rpc-gateway.ts, ui-tui/src/gatewayClient.ts). The
|
||||
# gateway reads the mtime and counts a fresh marker as inbound activity, so an
|
||||
# open client holds the box awake exactly like a chat message does. Without
|
||||
# this the box suspends under the open client, the client's reconnect loop
|
||||
# re-pokes the Fly-proxied hostname, autostart resumes it, and the instance
|
||||
# flaps every ~60s (13 of 72 active opted-in prod instances, 2026-09-02).
|
||||
# and TUI all send `gateway.ping` every 15s (apps/shared/src/json-rpc-gateway.ts,
|
||||
# ui-tui/src/gatewayClient.ts). The gateway folds the mtime into its inbound
|
||||
# clock, so an open client holds the box awake exactly like a chat message does
|
||||
# and gets the same idle_timeout grace after it disconnects. Without this the
|
||||
# box suspends under the open client, the client's reconnect loop re-pokes the
|
||||
# Fly-proxied hostname, autostart resumes it, and the instance flaps every ~60s
|
||||
# (13 of 72 active opted-in prod instances, 2026-09-02).
|
||||
#
|
||||
# There is deliberately NO staleness cutoff here: the mtime is a timestamp of
|
||||
# real inbound, and is_idle already decides whether it is recent enough. A
|
||||
# lingering marker cannot pin the box — once it is older than idle_timeout it
|
||||
# no longer counts, exactly like an old _last_inbound_at.
|
||||
DASHBOARD_CLIENT_HEARTBEAT_REL = os.path.join("state", "dashboard_clients.heartbeat")
|
||||
DASHBOARD_CLIENT_STALE_SECONDS = 45.0
|
||||
|
||||
|
||||
def dashboard_client_heartbeat_path(hermes_home: Optional[os.PathLike | str] = None):
|
||||
@@ -207,29 +211,23 @@ def dashboard_client_last_seen(
|
||||
path: Optional[os.PathLike | str] = None,
|
||||
*,
|
||||
now: Optional[float] = None,
|
||||
stale_seconds: float = DASHBOARD_CLIENT_STALE_SECONDS,
|
||||
) -> Optional[float]:
|
||||
"""Epoch seconds a dashboard client was last seen, or None when no live client.
|
||||
"""Epoch seconds a dashboard client last sent a WS frame, or None if never.
|
||||
|
||||
Missing marker -> None (the steady state on a box nobody has the dashboard
|
||||
open on — NOT fail-awake, or every instance would never sleep). A marker
|
||||
older than ``stale_seconds`` -> None (client gone; the file just lingers).
|
||||
An unreadable marker -> ``now`` (fail-awake: an unreadable source counts as
|
||||
open on — NOT fail-awake, or every instance would never sleep). An
|
||||
unreadable marker -> ``now`` (fail-awake: an unreadable source counts as
|
||||
activity, same rule as the work counters in ``is_idle``).
|
||||
"""
|
||||
import time
|
||||
|
||||
current = time.time() if now is None else now
|
||||
p = dashboard_client_heartbeat_path() if path is None else path
|
||||
try:
|
||||
mtime = os.stat(p).st_mtime
|
||||
return os.stat(p).st_mtime
|
||||
except FileNotFoundError:
|
||||
return None
|
||||
except OSError:
|
||||
return current
|
||||
if current - mtime >= stale_seconds:
|
||||
return None
|
||||
return mtime
|
||||
return time.time() if now is None else now
|
||||
|
||||
|
||||
def self_suspend_available(environ: Optional[dict] = None) -> bool:
|
||||
|
||||
@@ -64,15 +64,15 @@ def test_touch_creates_state_dir_and_marker(hermes_home):
|
||||
assert seen is not None and abs(seen - time.time()) < 5
|
||||
|
||||
|
||||
def test_last_seen_fresh_then_stale(hermes_home):
|
||||
def test_last_seen_returns_raw_mtime_without_staleness_cutoff(hermes_home):
|
||||
# No liveness cutoff here on purpose: is_idle decides recency. A 1h-old
|
||||
# marker still reports its mtime; the gateway then finds it outside
|
||||
# idle_timeout, same as an old _last_inbound_at.
|
||||
s2z.touch_dashboard_client_heartbeat()
|
||||
p = s2z.dashboard_client_heartbeat_path()
|
||||
mtime = os.stat(p).st_mtime
|
||||
assert s2z.dashboard_client_last_seen(now=mtime + 10) == mtime
|
||||
assert s2z.dashboard_client_last_seen(now=mtime + 44.9) == mtime
|
||||
# A client that stopped pinging 45s ago is gone (client heartbeat deadline).
|
||||
assert s2z.dashboard_client_last_seen(now=mtime + 45) is None
|
||||
assert s2z.dashboard_client_last_seen(now=mtime + 3600) is None
|
||||
assert s2z.dashboard_client_last_seen(now=mtime + 3600) == mtime
|
||||
|
||||
|
||||
def test_last_seen_unreadable_marker_fails_awake(hermes_home, monkeypatch):
|
||||
@@ -116,20 +116,40 @@ def test_attached_dashboard_client_blocks_idle(hermes_home, monkeypatch):
|
||||
assert r._scale_to_zero_is_idle() is False
|
||||
|
||||
|
||||
def test_stale_marker_hands_back_to_the_gateway_clock(hermes_home, monkeypatch):
|
||||
"""Marker 100s old (> 45s staleness) => the client is gone; a lingering file
|
||||
must NOT extend the inbound clock. The gateway's own _last_inbound_at (600s
|
||||
ago) decides, so the box is idle."""
|
||||
def test_client_gets_the_same_idle_grace_as_a_message(hermes_home, monkeypatch):
|
||||
"""Last WS frame 100s ago with a 120s idle_timeout => still inside the
|
||||
window => NOT idle. This is the 2-minute-after-the-app-closes contract; an
|
||||
earlier draft cut the marker off at 45s and suspended ~50s after
|
||||
disconnect (observed live on staging)."""
|
||||
r = _runner(monkeypatch, last_inbound_at=time.time() - 600)
|
||||
s2z.touch_dashboard_client_heartbeat()
|
||||
p = s2z.dashboard_client_heartbeat_path()
|
||||
old = time.time() - 100
|
||||
os.utime(p, (old, old))
|
||||
assert r._scale_to_zero_is_idle() is False
|
||||
|
||||
|
||||
def test_client_gone_longer_than_idle_timeout_is_idle(hermes_home, monkeypatch):
|
||||
r = _runner(monkeypatch, last_inbound_at=time.time() - 600)
|
||||
s2z.touch_dashboard_client_heartbeat()
|
||||
p = s2z.dashboard_client_heartbeat_path()
|
||||
old = time.time() - 121
|
||||
os.utime(p, (old, old))
|
||||
assert r._scale_to_zero_is_idle() is True
|
||||
|
||||
|
||||
def test_marker_predating_gateway_inbound_does_not_matter(hermes_home, monkeypatch):
|
||||
# Ancient marker from a client that left hours ago, gateway idle 600s.
|
||||
r = _runner(monkeypatch, last_inbound_at=time.time() - 600)
|
||||
s2z.touch_dashboard_client_heartbeat()
|
||||
p = s2z.dashboard_client_heartbeat_path()
|
||||
old = time.time() - 7200
|
||||
os.utime(p, (old, old))
|
||||
assert r._scale_to_zero_is_idle() is True
|
||||
|
||||
|
||||
def test_dashboard_client_seen_recently_extends_inbound_clock(hermes_home, monkeypatch):
|
||||
# Marker 30s old (fresh, < 45s): inbound clock moves to 30s ago, which is
|
||||
# Marker 30s old: inbound clock moves to 30s ago, which is
|
||||
# inside the 120s window => not idle, even though the gateway's own
|
||||
# _last_inbound_at is ancient.
|
||||
r = _runner(monkeypatch, last_inbound_at=time.time() - 600)
|
||||
@@ -141,13 +161,13 @@ def test_dashboard_client_seen_recently_extends_inbound_clock(hermes_home, monke
|
||||
|
||||
|
||||
def test_newer_gateway_inbound_wins_over_older_marker(hermes_home, monkeypatch):
|
||||
# Marker fresh at 40s ago, but a chat message landed 5s ago: max() keeps the
|
||||
# message; either way the box stays awake. Pin no regression on the path.
|
||||
r = _runner(monkeypatch, last_inbound_at=time.time() - 5)
|
||||
monkeypatch.setattr(r, "_scale_to_zero_idle_timeout_seconds", lambda: 10.0, raising=False)
|
||||
s2z.touch_dashboard_client_heartbeat()
|
||||
p = s2z.dashboard_client_heartbeat_path()
|
||||
t = time.time() - 40
|
||||
os.utime(p, (t, t))
|
||||
# Picking the marker (40s > 10s) would read idle; the chat message (5s) wins.
|
||||
assert r._scale_to_zero_is_idle() is False
|
||||
# _last_inbound_at itself is not mutated by the read.
|
||||
assert time.time() - r._last_inbound_at < 10
|
||||
|
||||
@@ -370,6 +370,9 @@ async def handle_ws(
|
||||
else:
|
||||
await ws.accept()
|
||||
disconnect_reason = "connected"
|
||||
# A client is attached from the moment the upgrade is accepted — mark it
|
||||
# before the (possibly slow) ready/skin setup so scale-to-zero sees it.
|
||||
_note_dashboard_client_activity(force=True)
|
||||
# Push small streamed frames out immediately instead of letting Nagle
|
||||
# batch them — keeps the live token cadence intact for GUI clients.
|
||||
_disable_nagle(ws)
|
||||
@@ -417,7 +420,6 @@ async def handle_ws(
|
||||
# Track this peer for session-less global broadcasts (skin.changed
|
||||
# from the background watcher) — write_json can't route those.
|
||||
server.register_live_transport(transport)
|
||||
_note_dashboard_client_activity(force=True)
|
||||
# Cross-backend liveness (#94895): register a heartbeat row so
|
||||
# the startup orphan sweep can distinguish "row owned by a live
|
||||
# but idle backend" from "row truly orphaned". The stdio TUI's
|
||||
|
||||
Reference in New Issue
Block a user