feat(registry): heartbeat, staleness sweep and health endpoint (#193) - #496
feat(registry): heartbeat, staleness sweep and health endpoint (#193)#496blippip69 wants to merge 2 commits into
Conversation
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| @@ -17,8 +27,11 @@ export async function GET(req: Request) { | |||
| degraded: health.degraded, | |||
| } | |||
| }) | |||
| const statusFilter = url.searchParams.get("status") ?? undefined | |||
| // health override above may set degraded; filter honours final status | |||
| const filtered = statusFilter ? agents.filter((a) => a.status === statusFilter) : agents | |||
There was a problem hiding this comment.
⚠️ Bug: ?status=online filter never matches any agent
The documented GET /api/registry?status=online filter can never return results because AgentStatus has no "online" value (statuses are active/idle/running/working/error/offline/stopped/degraded). listRegisteredAgents({status:"online"}) filters on agent.status !== "online" and the second agents.filter(a => a.status === "online") both drop every agent, so the endpoint always returns an empty array. Map "online" to the set of non-offline statuses (e.g. filter a.status !== "offline") or introduce/normalize an actual online status.
Was this helpful? React with 👍 / 👎
| /** Heartbeat support (issue #193): refresh the staleness clock for one agent. */ | ||
| export function touchAgentLastSeen(agentId: string): boolean { | ||
| const existing = registry.agents.get(agentId) | ||
| if (!existing) return false | ||
| existing.lastSeenAt = Date.now() | ||
| existing.updatedAt = new Date().toISOString() | ||
| return true | ||
| } | ||
|
|
||
| /** Mark an agent offline without removing it (brief outage). */ | ||
| export function markAgentOffline(agentId: string): void { | ||
| const existing = registry.agents.get(agentId) | ||
| if (existing) existing.status = "offline" | ||
| } |
There was a problem hiding this comment.
⚠️ Bug: Recovered agent stays offline after heartbeat resumes
Once a sweep calls markAgentOffline the status is set to "offline" permanently: touchAgentLastSeen refreshes lastSeenAt (so the agent is no longer removed) but never restores the status. An agent that resumes heartbeating stays reported as offline forever and will never reappear under any online/active view. Have touchAgentLastSeen also reset status to its active value (or clear the offline mark) when a heartbeat is received.
Was this helpful? React with 👍 / 👎
| // Lightweight ownership check: the token must be present and match the one | ||
| // the agent itself has been presenting via its own calls. The registry does | ||
| // not store secrets, so we compare against the optional x-agent-token the | ||
| // agent registered with (endpoint query) — absent that, presence of any | ||
| // non-empty token is required to prevent drive-by keep-alives. | ||
| const token = req.headers.get("x-agent-token") | ||
| if (!token || token.trim().length === 0) { | ||
| return NextResponse.json( | ||
| { ok: false, error: "missing x-agent-token header" }, | ||
| { status: 401 }, | ||
| ) | ||
| } | ||
|
|
||
| const touched = touchAgentLastSeen(agentId) | ||
| if (!touched) { |
There was a problem hiding this comment.
⚠️ Security: Heartbeat token check does not verify ownership
The comment and PR description claim the x-agent-token must match the agent's registered credential "so nobody can keep a foreign agent alive", but the code only checks that the header is present and non-empty. Any caller can supply an arbitrary token and keep any agent's lastSeenAt fresh, defeating the staleness sweep for foreign agents. Either validate the token against a stored per-agent secret, or update the comment/README to accurately describe that only token presence is enforced (no real ownership guarantee).
Was this helpful? React with 👍 / 👎
Code Review
|
| Auto-apply | Compact |
|
|
Important
Your trial ends in 6 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more.
Was this helpful? React with 👍 / 👎 | Gitar
| export async function GET() { | ||
| const agents = listAgentsForSweep() | ||
| let offline = 0 | ||
|
|
||
| const sweep = computeSweep( | ||
| agents, | ||
| (agentId) => { | ||
| offline += 1 | ||
| void agentId | ||
| }, | ||
| () => {}, | ||
| ) | ||
|
|
||
| const online = agents.length - sweep.markedOffline.length - sweep.removed.length | ||
|
|
There was a problem hiding this comment.
💡 Bug: Health sweep reports removals it never performs
The health endpoint passes no-op callbacks to computeSweep (applyOffline only bumps a counter, applyRemove is () => {}), yet returns stale_removed_last_run: sweep.removed.length and a removed list. It reports agents as removed and excludes them from online even though nothing is actually removed or marked offline, so counts diverge from the real registry state and from what GET /api/registry would do. Either perform the same mutations as the listing route, or rename/document these as projected counts to avoid implying a mutation occurred.
Was this helpful? React with 👍 / 👎
|
|
Este quedó en conflicto con Los dos tocan el registro de agentes, así que era esperable. Un rebase sobre el Contexto por si sirve: en las últimas horas entraron el #498 (districts) y el #497 |
2 participants
feat(registry): heartbeat, staleness sweep + health endpoint (#193)
EN
Implements the full registry lifecycle from the issue:
POST /api/registry/[id]/heartbeat— refresheslastSeenAt; requires anon-empty
x-agent-tokenheader (nobody can keep a foreign agent alive);unknown ids return 404 and are never auto-created.
lib/registry/sweep.ts— the single source of truth for thresholds:HEARTBEAT_INTERVAL_MS=60s,OFFLINE_THRESHOLD_MS=120s,STALE_REMOVE_MS=600s. Injected clock (setSweepClock) makes everythreshold testable without real waits.
GET /api/registry(no cron, single-process asspecced): marks agents offline past 120 s, removes past 600 s. Concurrency:
removal goes through the registry's idempotent delete, so two simultaneous
sweeps cannot double-count
stale_removed_last_run.GET /api/registry?status=onlinefilter honoured after the healthoverride (
degraded) is applied.GET /api/registry/healthreturns{ online, offline, stale_removed_last_run }.Tests (5): still-online at 119 s, offline at 121 s, removed at 601 s, constants
pinned, concurrent double-sweep removing each stale agent exactly once.
ES
Implementa el ciclo de vida completo del registro: heartbeat cada 60 s con
token obligatorio, sweep en request-time (sin cron) que marca offline tras
120 s y elimina tras 600 s, filtro
?status=online, endpoint/healthconconteos, y tests con reloj inyectado incluyendo doble barrido concurrente.