Conversation
`agents` is keyed by the heartbeat's `device`, which every agent mints with
one shared TURMA_AGENT_TOKEN — so the number of RETAINED records was
unbounded. AGENT_RECORD_MAX bounds one record and prune() only reclaims at
seven days, so 512 beats of 0.9 MiB under 512 names OOM-killed a 256 MiB hub
while the same 512 beats under ONE name peaked at 169 MiB. The per-record
ceiling was never the aggregate.
Two budgets, bounding different things:
* AGENTS_TOTAL_MAX — aggregate record bytes, derived from the container's
own cgroup limit (an eighth, clamped 8-64 MiB) rather than picked, since
a ceiling above the limit the kernel kills on is not a ceiling.
* AGENTS_MAX — record count (64). Not redundant: the byte budget measures
what agentRecordSize measures, which excludes the on-demand caches, so
only a cap on hosts bounds their multiple.
A host already in the registry is always admitted — turning the cap into a
wall for the fleet's own hosts is the same outage from the other side. A NEW
device gets a slot only if there is room, or one can be reclaimed from a host
unseen for AGENT_EVICT_IDLE_MS (1h); otherwise it is refused 429 and logged.
A record holds an offline host's last known sessions, PR chips and usage, so
a rebooting or updating host is never displaced by a newcomer, and eviction
never runs when it could not satisfy the request anyway.
The state.json restore enforces the same budget (keep-newest): a flood that
landed before a restart is on disk, and a bound the loading path skips is not
a bound — the same reason normalizeRecord runs on the restore too.
Byte accounting is a side map, never a field on the record, and re-measures
unknown keys and forgets dead ones so the many `delete agents[key]` sites
need not remember it.
Tests: turma/tests/registry-cap.test.js (own process, tiny caps).
Five defects, all from the adversarial pass on the first commit. A full registry must not read as an outage. The aggregate gate refused whichever host beat next, and rolling a known host back to its previous record rolls back `lastSeen` with it — so under sustained pressure a live, normal-sized host was refused every beat, aged past OFFLINE_AFTER_MS and showed offline while it was up, indistinguishable from a network failure and invisible to the operator (measured: 57/57 refused over 85s). Only a host OVER its share (AGENT_FAIR_SHARE = total/count, 512 KiB deployed, against a measured largest-real-record of 0.30 MiB) is refused now. The cost is a bounded overshoot (~2x the budget) instead of a hard total; the claim that a refusal was "contention, not a wall" was wrong and is gone from the docs. The restore trim could not protect a restore it never reached: readFileSync + JSON.parse materialize the whole file, so a 264 MiB state.json left by a flood killed a 256 MiB hub at init — before any log line, every boot, forever under `restart: unless-stopped`. The file is now measured with statSync first, moved to .oversized, and the hub boots empty and says so. The refusal log was one line per refused beat, so surviving a flood cost the host its disk instead: throttled to one a minute carrying the suppressed count. `device` is agent-supplied and unvalidated for content, so a newline in it forged a line reading exactly like the hub's own — every log naming a host goes through logName now. The new env knobs went through `Number(x) || default`, which silently obeys a negative and would refuse the whole fleet on its first beat; positiveEnv announces and ignores a bad value, and the derived budget is printed at boot. Also: eviction now spends no record on a request it could not satisfy anyway, and releases the evicted host's recordSizeWarned entry. Filed XERK-292 for the one finding out of scope here: `historyResults` and the five other on-demand caches are capped by COUNT and never by bytes, so ONE device name and no concurrency at all still OOM-kills the hub (4 sequential 30 MiB beats). Pre-existing, kills origin/main identically. Tests: registry-cap.test.js (share exemption, throttle, log forging, env knobs), registry-restore.test.js (the oversized state.json, own process).
The exemption admits an at-or-under-share host regardless of the budget, so the aggregate is a soft total. Two things bound it: a fat beat lands only while the whole registry fits, and a NEW device is admitted only while the registry is inside the budget — so the flood path cannot reach the exemption at all, and only an already-seated host can overshoot, by at most its share. Worst case is 2x the budget, held by a test that actually drives it.
XERK-268 landed per-agent tokens, which bind the heartbeat's `device` to the credential. That shrinks this ticket's threat model from any-token-holder to a compromised or buggy host plus the `legacy` master a mid-rollover fleet still accepts — it does not bound the registry, so the cap stands and its rationale is corrected in both the code and the rules file. Admission control is ordered AFTER the new binding, so an unbound beat can never spend a registry slot.
… for
QA pass 2 cleared D2-D6 and found one blocker plus two smaller defects.
F1 (blocker). The overshoot bound is an identity — worst-case retained is
AGENTS_TOTAL_MAX + AGENTS_MAX * AGENT_FAIR_SHARE — so a FLOOR under the share
makes the second term unbounded in AGENTS_MAX. And raising AGENTS_MAX is
exactly what the comment and the rules file tell an operator with a growing
fleet to do. At AGENTS_MAX=2000 against the deployed 32 MiB budget the 64 KiB
floor gave 3.9x the budget and the hub was OOM-killed at -m 256m, exit 137,
/healthz dead. The share is now derived and never floored, so the bound is 2x
at any AGENTS_MAX; a share too small to be sane warns at load instead, since
the config — not the bound — is what is wrong there. The warning moved to
module scope so it is reachable without binding a port.
F2. On a read-only /data the rename fails and the message still sent the
operator to a .oversized that was never created. It now says which happened.
D4 was only half fixed: logName() went on the two new 429 lines while
refuseOversized (ONE request, no registry pressure) and sanitizeHeartbeat's
drop line (rides a 200 beat) stayed injectable — both easier to reach than
the path that was fixed. All four heartbeat log sites now go through it.
F3 was a coverage gap, and QA's mutation run proved it: the fair-share floor,
the recordSizeWarned eviction sweep, and the restore's agents={} clear could
all be deleted with the suite still green. All three are now caught — the
floor needs the degenerate config, so registry-restore.test.js carries
AGENTS_MAX=2000, and the half-built restore needs a file that PARSES and then
throws, so it boots a child process on a poisoned state.json.
Filed, not fixed here: XERK-298 (no heartbeat refusal — 413 or 429 — reaches
the operator; the host just freezes or vanishes, and closing it spans web +
Android), XERK-299 (hub-agent post() clears a delivered beat's staged work
inside the transport try, so a late exception loses it and discards the
reply; latent).
The verdict was PASS with four mutations escaping, all new surface from the
previous commit — including the read-only-volume message, which was itself the
fix for the finding before it and had nothing pinning it.
* The share is now `fairShare(total, max)`, a function, so the extremes are
reachable to a test without a process pinned to a degenerate config. A
share of 0 (reachable when AGENTS_MAX passes the budget in BYTES, which
positiveEnv accepts) makes every host over-share and brings back the
silent-offline regression; nothing was watching that.
* The oversized-state message's two branches are both driven, on a real
read-only directory in a child process.
* All four converted log sites are driven — the earlier test only reached
two of them, so the over-half warn and the coercion-failure line could
have reverted to interpolating the raw device name.
* `logName` also strips C1 now: JSON.stringify escapes none of that block,
and NEL (U+0085) reads as a line break to some log viewers.
Also QA's answer to whether the budget should be a larger fraction of the
container: no — the gap is the silence, not the size. The per-record ceiling
warns at 4 MiB while a share is 512 KiB, so a record drifts past its share,
starts being refused, and the first thing the operator sees is the host
vanishing, with the older warning still eight times away. So a host now gets
the same crossing-edge warning against HALF ITS SHARE. That is what makes the
eighth-of-the-container default defensible; raising the fraction would reserve
half the container, since the 193.8 MiB flood peak is mostly serving overhead
that scales with the registry rather than the registry itself.
QA verdict: PASS (pass 3). 512 distinct devices survive at -m 256m in both
strict and legacy auth (36x200/476x429, peak 193.8 MiB); the AGENTS_MAX=2000
config that was OOM-killed now lands at 54.8 MiB against a 64 MiB bound;
dashboard/sessions/board/usage byte-identical to origin/main.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes XERK-272.
The bug
agentsis keyed by the heartbeat'sdevicename and nothing capped how many keys it could hold.AGENT_RECORD_MAX(XERK-235) bounds ONE record;prune()only reclaims at seven days. So the retained set was unbounded, and 512 beats of 0.9 MiB under 512 names OOM-killed a 256 MiB hub while the same 512 beats under one name peaked at 169 MiB — sockets, body sizes and concurrency identical. The per-record bound was never the aggregate.XERK-268 landed per-agent tokens while this was in flight, which binds
deviceto the credential. That changes who can do this — a compromised or buggy host, or thelegacymaster a mid-rollover fleet still accepts — but it bounds nothing, and a host deriving its name from something unstable grows records with no attacker at all. The two are complementary.The fix
Two budgets, bounding different things:
AGENTS_TOTAL_MAX— aggregate record bytes, derived from the container's own cgroup limit (an eighth, clamped 8–64 MiB). Per the ticket's invariant: a ceiling above the limit the kernel kills on is not a ceiling, so this is read from the container rather than picked.AGENTS_MAX— record count (64). Not redundant: the byte budget measures whatagentRecordSizemeasures, which excludes the on-demand caches, so only a cap on hosts bounds their multiple.The policy (chosen by the reporter, from the ticket's three options):
devicegets a slot only if there is room, or one can be reclaimed from a host unseen forAGENT_EVICT_IDLE_MS(1h, ≫OFFLINE_AFTER_MS); otherwise 429. A record holds an offline host's last known sessions, PR chips and usage, so a host rebooting or updating is never displaced by a newcomer, and nothing is evicted when eviction could not satisfy the request anyway.AGENT_FAIR_SHARE= total/count, derived and never floored). A host inside its share is not why the registry is full, so it never pays.statSyncbefore it is opened.Accepted cost, stated plainly: a flood of names can squat slots and block onboarding a genuinely new host until it stops or the caps are raised. Per-agent tokens (XERK-268) are what make that hard to reach.
What QA found, and what it cost
Three adversarial passes (
~/.claude/agents/qa.md), each re-running the ticket's own container reproduction. Two of them failed me, and both findings were real:Pass 1 — FAIL. Under sustained pressure the aggregate gate refused whichever host beat next, and rolling a known host back to its previous record rolls back
lastSeenwith it — so a live, normal-sized host was refused every beat, aged pastOFFLINE_AFTER_MS, and read offline while it was up, indistinguishable from a network failure (measured 57/57 refused over 85 s). My claim that this was "contention, not a wall" was simply wrong. Fixed with the per-host share. Also from that pass: the restore trim could not protect a restore it never reached (a 264 MiBstate.jsonkilled the hub at init, every boot, with no log output — a permanent crash loop underrestart: unless-stopped); the refusal log was one line per refused beat, so surviving a flood cost the host its disk;devicecould forge a hub log line via a newline; andNumber(x) || defaultsilently obeyed a negative cap, which would take the whole fleet offline on a compose typo.Pass 2 — FAIL. The 64 KiB floor I had put under the per-host share broke the very bound the share exists to provide. The bound is an identity —
AGENTS_TOTAL_MAX + AGENTS_MAX × AGENT_FAIR_SHARE— so a floor makes the second term unbounded inAGENTS_MAX, and raisingAGENTS_MAXis exactly what my own docs told an operator with a growing fleet to do. AtAGENTS_MAX=2000that is 3.9× the budget: OOMKilled, exit 137,/healthzdead. The share is now derived and never floored; a degenerate config warns at load instead of the two numbers contradicting each other silently.Pass 3 — PASS.
Verification
All numbers below are QA's, re-measured on this branch in
node:24-alpineatdocker run -m 256m --memory-swap 256m, with per-host tokens derived for the merged XERK-268 code and the A/B run in both strict and legacy auth modes./healthzorigin/main, 512 distinctorigin/main, 512 sharedPeak of the surviving flood container: 193.8 MiB of 256. The budget is enforced exactly — 36 admitted × 928 570 B = 33 428 520 ≤ 33 554 432; the 37th would have crossed.
AGENTS_MAX=2000retains 54.8 MiB against a 64 MiB bound (was OOM-killed at 125 MiB).55 accepted / 0 refused,online=true.state.jsonboots clean 3/3,.oversizedpreserved byte-identical; read-only/data, corrupt JSON, andstate.jsonas a directory all boot.origin/mainunder identical beats, zero console errors./api/agentsgains no field, so there is no new Android decode surface — the Web ⇄ Android parity rule does not apply.hub-agent.pyalready treats any non-200 as a failed beat, retains its staged work and retries in 20 s. Verified by driving the realpost().Gates:
node --test1173 pass,python3 -m unittest1301 OK, Semgrep 0 findings with CI's exact rule set and exclusions. Instruction files under the 40k limit.Not verified: Android/glasses/veiller (untouched, no new wire surface);
AGENT_EVICT_IDLE_MSat its real 1h default (every test wound it to seconds); the cgroup v1 branch ofcontainerMemoryLimit()(unreachable on this cgroup-v2 host); a real agent container through a full 429 → recovery cycle with live sessions.Findings routed elsewhere
Every defect found is accounted for. Three are filed rather than fixed here:
historyResultsand five sibling caches are capped by count, never bytes, so one device name and four sequential 30 MiB beats OOM-kill the hub. Pre-existing; killsorigin/mainidentically. Deliberately out of the byte budget here (XERK-235 excluded the caches so a legitimate ~5 MiB/historydelivery cannot cost a host its heartbeat), so bounding them is its own policy call.hub-agent.py'spost()clears a delivered beat's staged work inside the transporttry, so a late exception loses the PR links and discards the reply while reporting the beat as failed. Latent (the attributes exist in production).Notes for review
AGENT_CACHE_KEYSandagentRecordSizemoved to sit above thestate.jsonrestore, byte-identical, because the restore now enforces the budget at module init and aconstdeclared later is in its temporal dead zone there — the failure theLIVE_AGENTS_MAXcomment already documents.server.test.jssetsAGENTS_MAX=1000: it invents ~100 synthetic host names in one process and is not a fleet. The cap's own behaviour lives inregistry-cap.test.jsandregistry-restore.test.js, each pinning caps in its own process — the latter also carries the degenerateAGENTS_MAX=2000config, since the bound only breaks when the derived share falls below what a floor would impose.AGENTS_MAX,AGENTS_TOTAL_MAX,AGENT_EVICT_IDLE_MS,STATE_FILE_MAX) all default sensibly, so no DockerOps compose change is required.