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" + )