diff --git a/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java b/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java index 979956f..c22ee8e 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java +++ b/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java @@ -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. + * + *

Deliberately not {@link AgentStatus#injectable()}. {@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(); diff --git a/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java b/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java index 7f6eec4..22213cb 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java @@ -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