Merge #484: fleetd #480 Unit E — a BLOCKED pane is not a settled pane
CI / contract (push) Successful in 55s
CI / build (push) Successful in 1m35s

LeadRollover's two settle waits used AgentStatus#injectable(), which is
IDLE || BLOCKED || DONE. That is the right rule for Injector ("may I deliver a message
without stepping on a live turn") and the wrong one here ("has the turn actually
ended"), because BLOCKED is a live turn that is merely paused — a pane sitting on an
approval prompt.

Path in: the lead calls confirm(); its turn carries on and hits anything needing
approval; herdr reports blocked; within 250ms the deferred continuation reads that as
settled; /clear is typed into an open prompt. That destroys the lead's live context
mid-turn, which is exactly what the turnSettleSeconds gate added in #483 exists to
prevent. The window is turnSettleSeconds, default 20s.

Both waits now require a real turn boundary — IDLE or DONE. AgentStatus#injectable() is
untouched: it is correct for Injector, LeadHeartbeatLoop, LeadCoordLoop, ReplyPushLoop
and HerdrPeerLauncher's two readiness checks, all of which are delivery gates.

Verified by the lead before merge:
- CI run 1726 green on head e2a91e8.
- Mutation on the half the worker did NOT mutate: dropped the DONE arm, leaving
  `if (status == AgentStatus.IDLE)`. Mutant proven applied with two unrelated proofs
  using different search strings (MUT-DROP-DONE present -> 1; 'IDLE || status' gone
  -> 0). Same single test, same 100s cap: unmutated passes in 0.172s; mutated is still
  running at 100s and does not pass. So doneStatusStillCompletesTheFullRoll really does
  pin the DONE arm, and the fix did not over-tighten to IDLE-only.

Why the earlier #483 battery could not have caught this: it proved the guard FIRES, not
that the guard's predicate was tight enough. LeadRolloverTest drove the fake status with
only "idle" and "working"; neither value discriminates this defect, only "blocked" does.

Correction round applied before merge: both warning lines said "never went idle" and
"did not become injectable". Retiring that word is the whole point of the change, so a
future reader debugging from those lines would have been handed back the wrong mental
model. Both now say "did not reach a turn boundary (IDLE or DONE)".

Filed separately as #486, deliberately not fixed here: with the DONE arm removed the
test hangs rather than fails, because waitUntilAtTurnBoundary is bounded only by an
injected clock that the tests hold fixed. Not a production bug — nowMillis is
System::currentTimeMillis there — but it turns a future regression into a CI hang
instead of a red test.
This commit was merged in pull request #484.
This commit is contained in:
2026-09-11 02:21:06 +02:00
2 changed files with 125 additions and 16 deletions
@@ -281,11 +281,12 @@ public final class LeadRollover {
*/
private void runRollover(PendingRollover p, FleetConfig.LeadRollover cfg) {
String lead = p.leadTerminal();
boolean turnSettled = waitUntilInjectable(lead, cfg.turnSettleSeconds());
boolean turnSettled = waitUntilAtTurnBoundary(lead, cfg.turnSettleSeconds());
if (!turnSettled) {
log.warn("lead-rollover: pane {} never went idle within {}s after confirm() — refusing "
+ "to send /clear at all; the calling lead's own turn is still live and "
+ "clearing it now would destroy live context (token={})",
log.warn("lead-rollover: pane {} did not reach a turn boundary (IDLE or DONE) within {}s "
+ "after confirm() — refusing to send /clear at all; the calling lead's "
+ "own turn is still live and clearing it now would destroy live context "
+ "(token={})",
lead, cfg.turnSettleSeconds(), p.token());
return;
}
@@ -294,10 +295,10 @@ public final class LeadRollover {
// /clear is housekeeping, not a delegated turn, and routing it through Injector wedges the
// pane forever (see this class's javadoc).
agents.send(lead, "/clear");
boolean clearSettled = waitUntilInjectable(lead, cfg.clearSettleSeconds());
boolean clearSettled = waitUntilAtTurnBoundary(lead, cfg.clearSettleSeconds());
if (!clearSettled) {
log.warn("lead-rollover: pane {} did not become injectable within {}s after /clear — "
+ "NOT sending bootstrapText (token={})",
log.warn("lead-rollover: pane {} did not reach a turn boundary (IDLE or DONE) within {}s "
+ "after /clear — NOT sending bootstrapText (token={})",
lead, cfg.clearSettleSeconds(), p.token());
return;
}
@@ -350,15 +351,27 @@ public final class LeadRollover {
}
/**
* Poll {@link AgentControl#status} until {@code target} reports an injectable state, bounded by
* {@code settleSeconds}. Used twice by {@link #runRollover}: once to wait for the CALLING
* turn's own pane to settle (the {@code turnSettleSeconds} gate that makes this correction
* safe), and once to wait for the pane to re-settle after {@code /clear}. A failed status read
* degrades to "not yet settled" and is retried on the next poll, the same posture {@code
* LeadHeartbeatLoop} and {@code HerdrPeerLauncher}'s readiness gate already take toward an
* unreadable status.
* Poll {@link AgentControl#status} until {@code target} reports a real turn boundary — {@link
* AgentStatus#IDLE} or {@link AgentStatus#DONE} — bounded by {@code settleSeconds}. Used twice
* by {@link #runRollover}: once to wait for the CALLING turn's own pane to settle (the {@code
* turnSettleSeconds} gate that makes this correction safe), and once to wait for the pane to
* re-settle after {@code /clear}. A failed status read degrades to "not yet settled" and is
* retried on the next poll, the same posture {@code LeadHeartbeatLoop} and {@code
* HerdrPeerLauncher}'s readiness gate already take toward an unreadable status.
*
* <p><strong>Deliberately not {@link AgentStatus#injectable()}.</strong> {@code injectable()}
* answers the {@code Injector}'s question — "may I deliver a message without stepping on a live
* turn" — and it accepts {@link AgentStatus#BLOCKED} for that purpose, because a pane paused on
* an approval prompt is safe to queue a message behind. This class asks a stricter question —
* "has the turn actually ended" — and {@code BLOCKED} answers no: it is a live turn that is
* merely paused, not one that has finished. Reusing {@code injectable()} here would let both
* waits fire into an open approval prompt mid-turn (the first wait would send {@code /clear}
* while the lead's own {@code confirm()}-calling turn is still live and paused on a prompt; the
* second would send {@code bootstrapText} the same way after {@code /clear}) — exactly the
* live-context-destroying failure the {@code turnSettleSeconds} gate exists to prevent. Do not
* "simplify" this back to {@code injectable()}.
*/
private boolean waitUntilInjectable(String target, int settleSeconds) {
private boolean waitUntilAtTurnBoundary(String target, int settleSeconds) {
long deadline = nowMillis.getAsLong() + TimeUnit.SECONDS.toMillis(settleSeconds);
while (nowMillis.getAsLong() < deadline) {
AgentStatus status;
@@ -369,7 +382,7 @@ public final class LeadRollover {
target, e.toString());
status = null;
}
if (status != null && status.injectable()) {
if (status == AgentStatus.IDLE || status == AgentStatus.DONE) {
return true;
}
settleSleeper.run();
@@ -250,6 +250,102 @@ class LeadRolloverTest {
+ "same live context");
}
// ---- fleetd #480 Unit E: BLOCKED is not a settled turn boundary ------------------------
@Test
@DisplayName("[UNIT E] the calling lead's turn reporting BLOCKED the whole window is NOT settled — /clear is never sent")
void blockedTheWholeTurnSettleWindowSendsNoClearAtAll() throws IOException {
FakeHerdr herdr = new FakeHerdr();
herdr.agentStatus("blocked"); // the calling lead's own pane — paused on a prompt the whole window
Path handover = writeHandover("handover contents");
FleetConfig.LeadRollover config =
new FleetConfig.LeadRollover(handover.toString(), true, 3600, 1 /*turnSettleSeconds*/, 20, "text");
// The clock must ADVANCE across waitUntilAtTurnBoundary's poll loop, or a bounded loop
// against a frozen clock never reaches its own deadline.
AtomicLong clock = new AtomicLong(1_000);
LeadRollover rollover = newRollover(herdr, config, () -> clock.addAndGet(500));
LeadRollover.PendingRollover pending = rollover.open(LEAD, "context is full");
LeadRollover.RollDecision decision = rollover.confirm(LEAD, pending.token(), true);
assertTrue(decision.accepted(), "every synchronous gate should pass; the refusal happens "
+ "only inside the deferred continuation, which this test's synchronous runner has "
+ "already run to completion by the time confirm() returns");
assertEquals(0, promptCallCount(herdr),
"BLOCKED means the calling lead's turn is paused on a prompt, not ended — it is NOT "
+ "a turn boundary, so /clear must never be sent while the pane sits on an "
+ "open prompt (that is exactly what injectable() would wrongly allow, since "
+ "it treats BLOCKED as safe to inject into)");
}
@Test
@DisplayName("[UNIT E] a pane that goes BLOCKED after /clear is NOT settled — bootstrapText is never sent")
void blockedAfterClearNeverSendsBootstrapText() throws IOException {
// Idle until /clear is sent, then permanently blocked (paused on a prompt) — isolates the
// SECOND wait (clearSettleSeconds) from the first (turnSettleSeconds), which passes
// immediately here.
FakeHerdr fake = new FakeHerdr();
HerdrClient blocksAfterClear = new HerdrClient() {
@Override
public JsonNode call(String method, Object params) throws HerdrException {
JsonNode result = fake.call(method, params);
if ("agent.prompt".equals(method) && String.valueOf(params).contains("/clear")) {
fake.agentStatus("blocked");
}
return result;
}
@Override
public void close() {
fake.close();
}
};
Path handover = writeHandover("handover contents");
FleetConfig.LeadRollover config =
new FleetConfig.LeadRollover(handover.toString(), true, 3600, 20, 1 /*clearSettleSeconds*/, "boot text");
AtomicLong clock = new AtomicLong(1_000);
LeadRollover rollover = newRollover(blocksAfterClear, config, () -> clock.addAndGet(500));
LeadRollover.PendingRollover pending = rollover.open(LEAD, "context is full");
LeadRollover.RollDecision decision = rollover.confirm(LEAD, pending.token(), true);
assertTrue(decision.accepted(), "every synchronous gate passes; the refusal is logged only, "
+ "deep inside the deferred continuation");
assertEquals(1, promptCallCount(fake), "exactly one agent.prompt call — the /clear — and "
+ "nothing else: a freshly-cleared pane sitting on a prompt must not be typed into");
long bootstrapSends = fake.calls.stream()
.filter(c -> "agent.prompt".equals(c.method()))
.filter(c -> String.valueOf(c.params()).contains("boot text"))
.count();
assertEquals(0, bootstrapSends,
"bootstrapText must never be sent while the freshly-cleared pane reports BLOCKED — "
+ "typing into an open prompt after /clear is exactly as destructive as "
+ "typing /clear into one");
}
@Test
@DisplayName("[UNIT E] DONE still counts as a settled turn boundary — the fix must not over-tighten to IDLE-only")
void doneStatusStillCompletesTheFullRoll() throws IOException {
FakeHerdr herdr = new FakeHerdr();
herdr.agentStatus("done"); // both waits must accept DONE as much as IDLE
Path handover = writeHandover("handover contents");
AtomicLong clock = new AtomicLong(1_000);
FleetConfig.LeadRollover config = cfg(handover.toString());
LeadRollover rollover = newRollover(herdr, config, fixedClock(clock));
LeadRollover.PendingRollover pending = rollover.open(LEAD, "context is full");
LeadRollover.RollDecision decision = rollover.confirm(LEAD, pending.token(), true);
assertTrue(decision.accepted(), "expected approval; got: " + decision.reason() + " / " + decision.detail());
var prompts = herdr.calls.stream().filter(c -> "agent.prompt".equals(c.method())).toList();
assertEquals(2, prompts.size(),
"a pane reporting DONE the whole time must complete the full roll — /clear then "
+ "bootstrapText — exactly like IDLE; DONE is a real turn-boundary equivalent "
+ "to IDLE (see AgentStatus#DONE), not merely 'injectable'");
assertTrue(prompts.get(0).params().toString().contains("/clear"));
assertTrue(prompts.get(1).params().toString().contains("read the handover file"));
}
// ---- Correction 2: only the terminal that opened a request may confirm it -------------
@Test