LeadRollover refuses to touch a BLOCKED lead and documents why; LeadHeartbeatLoop:161 gates on injectable() and accepts one — the same question answered both ways in one codebase #594

Open
opened 2026-09-19 10:00:19 +02:00 by ltms · 0 comments
Owner

The inconsistency

Two code paths both decide "is it safe to type into the lead's pane now?". They give opposite
answers, and only one of them has thought about it.

LeadRollover — correct, and deliberately so (:479-488):

Deliberately not AgentStatus#injectable(). injectable() [asks] "can a message be pasted
into this pane mid-turn" — and it accepts BLOCKED for that purpose, because a pane paused on
an approval prompt can still receive text. This gate asks a different question — "has the turn
actually ended" — and BLOCKED answers no: it is a live turn that is merely paused, not one
that has finished. Reusing injectable() here would let this [proceed on a pane] this gate
exists to prevent. Do not "simplify" this back to injectable().

Both of its gates implement that (:508, :593-595): IDLE || DONE, never BLOCKED.

LeadHeartbeatLoop — the opposite, at :161:

if (status == null || !status.injectable()) {

and its own class javadoc states the intent plainly:

Status-gated — a WORKING lead is making progress and is never touched; only an injectable
(idle/done/blocked) lead is even considered (constraint 2).

So the heartbeat treats "blocked" as a safe moment to prompt a lead onward. It is not one. A
BLOCKED pane is sitting at an approval prompt, and text pushed into it lands in the prompt,
not on a fresh input line.

This is not theoretical — it is the fleet01 incident

Measured 2026-09-14. A lead-to-lead coordination message was delivered to fleet01's lead through
LeadCoordLoop:147, which gates on the same injectable(). /sessions reported that lead as
agentStatus: "blocked", and its claude process had been up 9 days with no restart. The message
was typed into a pane waiting on a human. It was never acted on, and the receipt said delivered.

LeadCoordLoop is therefore a third site with the same gate. AgentStatus.injectable() has call
sites in LeadHeartbeatLoop, ReplyPushLoop, MessageService, inject/StatusRefiner and
inject/Injector — each needs the same question asked of it: is this "can text be pasted" or
"has the turn ended"?
They are different questions and the enum offers one answer.

Consequence, and why it is worse for the heartbeat than for a message

A coordination message typed into an approval prompt is lost text. A heartbeat nudge is worse:
its whole purpose is to start a new turn in an idle lead. Fired at a BLOCKED pane it cannot
start a turn, so the debounce and the quietNudgeCap are counting events that never happened,
and the loop's four stated invariants are all reasoning about a state it misread.

Current exposure

leadHeartbeat: is not configured on the Mac, so the loop is never constructed here and the
defect is latent on this host. It is live on any host that turns the block on. I have not checked
fleet01's config for it.

Fix direction

Do not widen injectable(). Give the enum a second predicate that answers the other question —
LeadRollover has effectively hand-rolled it twice already — and move every call site onto
whichever one it actually meant. LeadRollover's javadoc is the specification; it is the only
place that states the distinction.

Acceptance criteria

  1. A blocked lead is not nudged. With the heartbeat enabled and the lead BLOCKED past
    idleAfterSeconds, no injection happens. Flip the status to IDLE and one does. A test that
    only drives IDLE stays green while the defect is present.
  2. The two predicates cannot be confused by a later edit. Replace the turn-ended predicate
    with injectable() at any call site that needs it and a test must fail, naming the site.
    LeadRollover's comment is currently the only thing preventing this, and a comment is not a test.
  3. Every existing injectable() call site is classified. For each, say which question it
    means and why. A site left unexamined is the defect surviving the fix.
  4. LeadRollover's behaviour does not change. It is already correct; the refactor must not
    quietly relax it.

Related: #340 (classify() can call an active pane IDLE — the other direction of the same
misread), #383, #590. Found while measuring whether a lead could be restarted on a timer (#595).

## The inconsistency Two code paths both decide "is it safe to type into the lead's pane now?". They give opposite answers, and only one of them has thought about it. **`LeadRollover` — correct, and deliberately so** (`:479-488`): > **Deliberately not `AgentStatus#injectable()`.** `injectable()` [asks] "can a message be pasted > into this pane mid-turn" — and it accepts `BLOCKED` for that purpose, because a pane paused on > an approval prompt can still receive text. This gate asks a different question — "has the turn > actually ended" — and `BLOCKED` answers no: it is a live turn that is merely paused, not one > that has finished. Reusing `injectable()` here would let this [proceed on a pane] this gate > exists to prevent. **Do not "simplify" this back to `injectable()`.** Both of its gates implement that (`:508`, `:593-595`): `IDLE || DONE`, never `BLOCKED`. **`LeadHeartbeatLoop` — the opposite**, at `:161`: ```java if (status == null || !status.injectable()) { ``` and its own class javadoc states the intent plainly: > **Status-gated** — a `WORKING` lead is making progress and is never touched; only an injectable > (idle/done/**blocked**) lead is even considered (constraint 2). So the heartbeat treats "blocked" as a safe moment to prompt a lead onward. It is not one. A `BLOCKED` pane is sitting at an approval prompt, and text pushed into it lands **in the prompt**, not on a fresh input line. ## This is not theoretical — it is the fleet01 incident Measured 2026-09-14. A lead-to-lead coordination message was delivered to fleet01's lead through `LeadCoordLoop:147`, which gates on the same `injectable()`. `/sessions` reported that lead as `agentStatus: "blocked"`, and its claude process had been up 9 days with no restart. The message was typed into a pane waiting on a human. It was never acted on, and the receipt said delivered. `LeadCoordLoop` is therefore a third site with the same gate. `AgentStatus.injectable()` has call sites in `LeadHeartbeatLoop`, `ReplyPushLoop`, `MessageService`, `inject/StatusRefiner` and `inject/Injector` — each needs the same question asked of it: *is this "can text be pasted" or "has the turn ended"?* They are different questions and the enum offers one answer. ## Consequence, and why it is worse for the heartbeat than for a message A coordination message typed into an approval prompt is lost text. A **heartbeat nudge** is worse: its whole purpose is to start a new turn in an idle lead. Fired at a `BLOCKED` pane it cannot start a turn, so the debounce and the `quietNudgeCap` are counting events that never happened, and the loop's four stated invariants are all reasoning about a state it misread. ## Current exposure `leadHeartbeat:` is **not configured on the Mac**, so the loop is never constructed here and the defect is latent on this host. It is live on any host that turns the block on. I have not checked fleet01's config for it. ## Fix direction Do not widen `injectable()`. Give the enum a second predicate that answers the other question — `LeadRollover` has effectively hand-rolled it twice already — and move every call site onto whichever one it actually meant. `LeadRollover`'s javadoc is the specification; it is the only place that states the distinction. ## Acceptance criteria 1. **A blocked lead is not nudged.** With the heartbeat enabled and the lead `BLOCKED` past `idleAfterSeconds`, no injection happens. Flip the status to `IDLE` and one does. A test that only drives `IDLE` stays green while the defect is present. 2. **The two predicates cannot be confused by a later edit.** Replace the turn-ended predicate with `injectable()` at any call site that needs it and a test must fail, naming the site. `LeadRollover`'s comment is currently the only thing preventing this, and a comment is not a test. 3. **Every existing `injectable()` call site is classified.** For each, say which question it means and why. A site left unexamined is the defect surviving the fix. 4. **`LeadRollover`'s behaviour does not change.** It is already correct; the refactor must not quietly relax it. Related: #340 (`classify()` can call an active pane IDLE — the other direction of the same misread), #383, #590. Found while measuring whether a lead could be restarted on a timer (#595).
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#594