fix(agentbox): never shrink a pool on an unanswered replica count

[jacoblee-io]

The defect

agents.replicas arrives over RPC (config.getAgent on the control-plane WS). The old code read a failed lookup as 1:

} catch (err) {
  // Fail to ONE, never to many: a config lookup blip must not scale an agent up.
  console.warn(`replicas lookup failed for agent=${agentId}; using 1:`, err);
  return 1;
}

That comment shows the trap: the author considered a failed lookup and reasoned about one direction — it must not scale an agent up. The other direction is worse. A few lines later:

if (accepting.length <= replicas) continue;   // 5 boxes > 1 → drain 4 of them

So one failed RPC shrinks a live multi-box pool to a single box. A failed lookup carries no information about the pool's correct size, and the two directions of a wrong guess are not symmetric: guessing high costs a pod, guessing low costs a running pool.

Why it fires on a deploy

Every one of these produces the same rejected promise, and none needs a second Runtime:

• The WS is not connected. FrontendWsClient.request rejects immediately — it does not queue:

  if (!this._connected || !this.ws) {
    return Promise.reject(new Error("FrontendWsClient is not connected"));
  }

• The RPC times out against a slow, restarting or partitioned control plane.
• The control plane refuses the agent — record removed, renamed, or bound elsewhere.

And the timing lines up with a restart. The drain reaper is armed in setBoxStatusProbe and does not wait for the control plane, so its first tick is 10 s after the Runtime process starts — while the WS may still be connecting, or the control plane may itself be rolling. AgentBox pods are independent of the Runtime and are adopted on restart, so the previous generation's full pool is already there to be judged.

The fix — three rules, and the first two are not sufficient

1. lookupReplicas answers undefined for "cannot establish". Only resolveReplicas — the serving path — may read that as one box: a request naming the agent is evidence this Runtime serves it, and one box is the safe shape for work that has to happen. Destruction requires an answer; service does not.

2. The check is the first statement of the reconcile loop body, ahead of healCrashedBoxes / markStaleBoxesDraining / advanceRoll / shrink. It previously sat after the shrink's own accepting.length <= 1 short-circuit — that micro-optimisation is what put the question in the wrong place.

3. A DrainMark now records WHY, and an excess mark is withdrawn once the pool is not over its count. This is the rule that covers the deploy, and neither of the first two can: a mark is acted on ticks after it is made, so a lookup that fails for one tick and recovers leaves a shrink queued against a pool that was never surplus — and nothing revisited it.

   The withdrawal compares boxes.length, never accepting.length: the latter has already had the marks subtracted, so it can only ever answer "no". stale and unresponsive marks are judged from what the box itself presents and are re-reached on re-examination, so withdrawing them would only churn the drain budget. As a bonus this also fixes an operator raising replicas back up mid-shrink, which previously did not cancel it.

Also: reapDrainedBoxes re-confirms before stop() and drops a mark whose agent became unresolvable — holding it would keep the box out of its own pool's accepting set forever, since every later tick answers unknown too.

A failed lookup deliberately stays uncached: a remembered unknown would hold a multi-replica agent at one box for the whole TTL and send every new session to instance 0. Its two costs are paid separately instead — the log line is rate-limited (REPLICAS_UNKNOWN_LOG_INTERVAL_MS), and the repeated RPC absorbed by a memo (UNOWNED_MEMO_MS) that reconcilePoolSizes alone reads.

Why this never reproduced in a test namespace

The shrink path is gated on accepting.length <= 1, which is checked before the lookup. A single-box agent never reaches it at all. Across every namespace in the test cluster there is exactly one multi-replica pool, and it was created by hand to exercise pooling:

siclaw-inner   agentbox-1ad8e24a-...-0 … -4     ← 5 replicas, created manually for pool testing
sicore-test    agentbox-e067a1fc-...-0          ← single box
every other ns                                  ← single box

Not "low probability" — unreachable. Pooling is only used in production, so production is the only place this path executes.

Tests

Nine cases in manager.test.ts, each reverse-verified by reverting the corresponding rule and confirming failure.

• the loop body is skipped entirely when the count cannot be established (no drain, no corpse collection, no roll, no respawn-on-a-guess), using the real FrontendWsClient is not connected error
• the control: the same pool is acted on once the count is known
• a turn is still served for an agent whose count is unavailable
• the withdrawal: a shrink queued by a failed tick is withdrawn once the count comes back
• the control: a shrink the count still justifies survives re-examination — otherwise scaling down would be impossible
• a stale mark is not withdrawn (re-examination reaches it again; withdrawing churns the budget)
• a queued drain is dropped when the agent can no longer be resolved
• the unknown-count line is reported once a window while the lookup keeps retrying
• the reconciler stops re-asking, and asks again once the memo lapses

Fixtures now mark boxes through markDraining rather than eight literal copies of the mark shape — which is exactly how the reason field was missed on the first pass.

Full suite: 294 files, 6579 passed.

Known gaps, documented rather than half-fixed

• Nothing here re-fills a wrongly shrunk pool. reconcilePoolSizes heals crashes and advances rolls but never re-fills a shrink, so recovery waits for a request to reach getOrCreatePooled. A quiet agent stays short.
• sweepOrphans' capability-box branch asks this Runtime's isLive about a run. Its oracle already fails safe on a thrown store lookup (catch { return true }); the remaining gap is a store answering null for a run it does not own. Its chat-box branch removes terminal pods only.
• spawner.list() is scoped to the namespace and the app=agentbox label, and pods carry no runtime identity. If several Runtimes ever share one AgentBox namespace, the rules above are what make that safe — there is no label to filter on, and adding one cannot be retroactive.

Docs

New invariants.md §1.5, and a correction to §1.3's CA-fingerprint bullet: a singleton Runtime Deployment makes a second writer unlikely but does not establish it, since nothing in the pod labels identifies one.

🤖 Generated with Claude Code