From 693759f2bbef2aaff4cb8a3b22f558ffd8136992 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 23 Sep 2026 14:52:36 -0400 Subject: [PATCH] fix: an idle GPU agent could not check in, so the roster called it stopped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Operator, 2026-09-23: *"I'm running the gpu agent on my device and it currently reads as 'offline' but it's running and has checked in recently."* It had checked in — twelve minutes ago. Two cadences that never agreed: idle lease poll ceiling 900s agent/fc_agent/worker.py (sleep mode) heartbeat while idle never gated on holding leases roster "stopped" after 300s api/system_health.py The roster records an agent check-in on `lease` and `heartbeat`. The heartbeat loop was gated on `if ids:`, so an agent holding no leases sent nothing at all — leaving the lease poll as the only check-in, and sleep mode backs that off exponentially to a 900s ceiling. 900 against 300: an IDLE agent was structurally guaranteed to read as stopped. Nothing was broken; nothing was misconfigured; the two halves simply disagreed. Not a recent regression. Sleep mode landed 2026-07-02; the roster adopted the lease as its check-in on 2026-09-02 — *"A lease IS the check-in … Recorded on the call that was already happening"* — without noticing that the call it was piggybacking on had been deliberately slowed ten weeks earlier. The heartbeat now sends whether or not it holds leases. An empty one extends nothing (`id.in_([])` matches no rows) and costs one small POST every 45s — against the 6/min lease poll sleep mode exists to avoid, that is not a cadence worth protecting, and it is what makes "is the agent alive" answerable at all. Still gated on `self._running`: a worker that has been stopped is not checking in for work, and reporting it as present would be a different lie. Two things I could NOT determine from the code, both needing the live table: whether a stale `agent:agent` row exists from an older build that omitted `agent_id` (the server defaults it), and whether changing `AGENT_ID` has ever stranded an abandoned row — nothing prunes `service_seen`, so either would sit there reading "stopped" forever. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR --- agent/fc_agent/worker.py | 39 ++++++++++++++++++++---- tests/test_api_gpu.py | 64 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 5 deletions(-) diff --git a/agent/fc_agent/worker.py b/agent/fc_agent/worker.py index e0c4cc5..8683ad2 100644 --- a/agent/fc_agent/worker.py +++ b/agent/fc_agent/worker.py @@ -342,15 +342,44 @@ class Worker: # --- background loops --------------------------------------------------- def _heartbeat_loop(self) -> None: - """Keep every held lease alive so buffered jobs waiting on the GPU aren't - reclaimed by curator's 180s TTL. Errors are swallowed by client.heartbeat; - a reclaimed lease just re-leases elsewhere — never fatal.""" + """Keep every held lease alive, and say we are here even when holding none. + + Leases: buffered jobs waiting on the GPU would otherwise be reclaimed by + curator's 180s TTL. Errors are swallowed by client.heartbeat; a reclaimed + lease just re-leases elsewhere — never fatal. + + ## Why this sends with an EMPTY list rather than skipping + + Curator's roster records a check-in on this call (and on `lease`), and + calls an agent stopped after 300s of silence. This loop used to be + gated on `if ids:` — so an agent holding no leases sent nothing at all, + and the only check-in left was the lease poll, which sleep mode backs + off exponentially to a 900s ceiling (see IDLE_POLL_MAX_SECONDS). + + 900 against 300: an IDLE agent was structurally guaranteed to read as + stopped. Operator, 2026-09-23: *"I'm running the gpu agent on my device + and it currently reads as 'offline' but it's running and has checked in + recently."* It had — twelve minutes ago, partway up the backoff ladder. + + The two halves were written ten weeks apart and never reconciled: sleep + mode landed 2026-07-02, and the roster adopted the lease as its + check-in on 2026-09-02 without noticing the call it was piggybacking on + had been deliberately slowed. + + An empty heartbeat extends nothing (`id.in_([])` matches no rows) and + costs one small POST every 45s — against the 6/min lease poll sleep + mode exists to avoid, that is not a cadence worth protecting, and it is + what makes "is the agent alive" answerable at all. + + Still gated on `self._running`: a worker that has been stopped is not + checking in for work, and reporting it as present would be a different + lie. + """ while True: if self._running: with self._held_lock: ids = list(self._held) - if ids: - self.client.heartbeat(ids) + self.client.heartbeat(ids) time.sleep(HEARTBEAT_INTERVAL) def _queue_poll_loop(self): diff --git a/tests/test_api_gpu.py b/tests/test_api_gpu.py index 25f0a7b..db51353 100644 --- a/tests/test_api_gpu.py +++ b/tests/test_api_gpu.py @@ -314,3 +314,67 @@ async def test_cpu_embed_never_blocks_gpu_crop_backfills(db): select(GpuJob.task).where(GpuJob.image_record_id == img.id) )).scalars().all()) assert tasks == {"ccip", "siglip"} + + +# --- an idle agent still checks in ------------------------------------------- + + +@pytest.mark.asyncio +async def test_a_heartbeat_with_no_jobs_still_records_the_check_in(client, db): + """Operator, 2026-09-23: *"I'm running the gpu agent on my device and it + currently reads as 'offline' but it's running and has checked in + recently."* + + It had — twelve minutes ago. The roster takes its agent check-in from the + `lease` and `heartbeat` calls, and calls an agent stopped after 300s of + silence. The agent's heartbeat loop was gated on holding leases, so an + IDLE agent sent none; the only check-in left was the lease poll, which + sleep mode backs off to a 900s ceiling. 900 against 300 — an idle agent + was structurally guaranteed to read as stopped. + + So the empty heartbeat has to be a real check-in on the server side, not + merely tolerated. Asserted on `last_seen_at` moving, because "it returned + 200" would pass against an endpoint that recorded nothing. + """ + from backend.app.models import ServiceSeen + + token = (await (await client.post("/api/gpu/token/rotate")).get_json())["token"] + hdr = {"Authorization": f"Bearer {token}"} + + resp = await client.post( + "/api/gpu/jobs/heartbeat", + json={"agent_id": "desktop-agent", "job_ids": []}, headers=hdr, + ) + + assert resp.status_code == 200 + assert (await resp.get_json())["extended"] == 0, "it extends no lease" + + row = (await db.execute( + select(ServiceSeen).where(ServiceSeen.key == "agent:desktop-agent") + )).scalar_one() + assert row.kind == "agent" + assert row.display_name == "GPU agent (desktop-agent)" + assert row.last_seen_at is not None + + +def test_the_agent_heartbeats_whether_or_not_it_holds_a_lease(): + """The other half, in the agent itself — the half that was actually wrong. + + Read from the source rather than by running the loop: it is a `while True` + with a sleep, so exercising it means threads and timing, and the property + is simply that the call is not behind a `if ids:`. + """ + from pathlib import Path + + src = ( + Path(__file__).resolve().parents[1] / "agent" / "fc_agent" / "worker.py" + ).read_text() + loop = src[src.index("def _heartbeat_loop"):] + loop = loop[:loop.index("\n def ")] + + assert "self.client.heartbeat(ids)" in loop + assert "if ids:" not in loop, ( + "the heartbeat is gated on holding leases again; an idle agent then " + "reads as stopped after 300s while sleep mode backs its lease poll " + "off to 900s" + )