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 29cbb0c..b2054cd 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java +++ b/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java @@ -44,15 +44,18 @@ import java.util.function.Supplier; * {@code continuationRunner} before returning. That continuation is what actually touches the pane, * once the calling turn has ended, in this order: *
Deliberately not {@link AgentStatus#injectable()}. {@code injectable()} @@ -452,12 +469,12 @@ public final class LeadRollover { * 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()}. + * merely paused, not one that has finished. Reusing {@code injectable()} here would let this + * wait fire {@code /clear} while the lead's own {@code confirm()}-calling turn is still live and + * paused on a prompt — exactly the live-context-destroying failure the {@code turnSettleSeconds} + * gate exists to prevent. Do not "simplify" this back to {@code injectable()}. ({@link + * #waitForClearPickupAndSettle} keeps the same exclusion of {@code BLOCKED}, for the same + * reason, on the second wait.) */ private boolean waitUntilAtTurnBoundary(String target, int settleSeconds) { long deadline = nowMillis.getAsLong() + TimeUnit.SECONDS.toMillis(settleSeconds); @@ -477,4 +494,95 @@ public final class LeadRollover { } return false; } + + /** + * The SECOND wait in {@link #runRollover} — after {@code /clear} has been sent, waits for it to + * settle, bounded by {@code settleSeconds}. fleetd #489 — the paste-race fix. + * {@code /clear} does not start a real turn of its own, so a pane with no submit race simply + * stays {@link AgentStatus#IDLE} the whole time: {@link #waitUntilAtTurnBoundary} would (wrongly) + * call that "settled" on its very first poll, whether or not the {@code /clear} Enter actually + * landed. That was Fault 1, measured live on 2026-09-12 — the second gate was a no-op, so a + * {@code bootstrapText} send followed immediately, racing Fault 2: {@link AgentControl#submit}'s + * own javadoc already records that the submit accompanying a delivery "can race the paste — + * especially right as the worker's TUI becomes interactive — leaving the text unsubmitted" + * (CB-113). Because {@code runRollover} deliberately bypasses {@code Injector} for {@code + * /clear} (see this class's javadoc), it inherited none of {@code Injector}'s nudging — so the + * lost {@code /clear} Enter sat in the input box and {@code bootstrapText} was typed right after + * it, landing as one concatenated line. + * + *
This method copies the pickup-nudge pattern {@link dev.ltms.fleet.inject.Injector} already + * ships for exactly this, on its own post-turn {@code /clear} housekeeping (fleetd #306; see + * {@code Injector.java:288-340} and {@code Injector.java:437-442}): + *
{@link AgentStatus#BLOCKED} is deliberately excluded from both the nudge and the + * boundary check — the same reasoning as {@link #waitUntilAtTurnBoundary}'s own + * javadoc: a paused live turn is not a settled one, and re-sending Enter into an open approval + * prompt could wrongly answer it. A {@code BLOCKED} sample (or an unreadable/{@link + * AgentStatus#UNKNOWN} one) simply keeps this polling, with no nudge and no release, until either + * a real boundary is reached or {@code settleSeconds} runs out. + * + *
{@link AgentControl#submit} can itself throw; a {@link RuntimeException} from it is + * swallowed and logged at {@code debug}, exactly like {@code Injector.java:437-442} — a failed + * nudge must not abort the roll. + * + * @return {@code true} once {@code /clear} has settled, or once the nudge budget was exhausted + * with no pickup ever observed (released rather than wedged); {@code false} if {@code + * settleSeconds} elapses first — the caller must NOT send {@code bootstrapText} in that + * case, exactly as before this fix + */ + private boolean waitForClearPickupAndSettle(String target, int settleSeconds) { + long deadline = nowMillis.getAsLong() + TimeUnit.SECONDS.toMillis(settleSeconds); + boolean pickedUp = false; // a WORKING sample has been observed since /clear was sent + int idlePollsAwaitingPickup = 0; + while (nowMillis.getAsLong() < deadline) { + AgentStatus status; + try { + status = agents.status(target); + } catch (RuntimeException e) { + log.debug("lead-rollover: status check failed while waiting for {} to settle after " + + "/clear: {}", target, e.toString()); + status = null; + } + if (status == AgentStatus.WORKING) { + pickedUp = true; + } else if (status == AgentStatus.IDLE || status == AgentStatus.DONE) { + if (pickedUp) { + return true; // a real WORKING -> IDLE/DONE completion boundary + } + if (++idlePollsAwaitingPickup >= PICKUP_GRACE_POLLS) { + log.info("lead-rollover: /clear on {} was never observed as WORKING after {} " + + "consecutive IDLE/DONE polls ({} of those were nudged) — " + + "releasing rather than wedging the roll", + target, PICKUP_GRACE_POLLS, PICKUP_GRACE_POLLS - 1); + return true; + } + try { + agents.submit(target); // nudge a raced Enter (CB-113) so /clear actually submits + } catch (RuntimeException e) { + log.debug("lead-rollover: resubmit to {} failed (will retry next poll): {}", + target, e.getMessage()); + } + } + // AgentStatus.BLOCKED or UNKNOWN (or an unreadable status, above): neither a pickup + // signal nor a boundary — keep polling without nudging or releasing. + settleSleeper.run(); + } + return false; + } } 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 8d69d92..6d85796 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java @@ -458,6 +458,184 @@ class LeadRolloverTest { assertEquals(0, bootstrapSends, "bootstrapText must never be sent when /clear did not settle"); } + // ---- fleetd #489: the second wait nudges the /clear pickup instead of being a no-op -------- + + /** Every {@code agent.send_keys} call {@code herdr} recorded — the submit-keystroke nudge. */ + private static long sendKeysCallCount(FakeHerdr herdr) { + return herdr.calls.stream().filter(c -> "agent.send_keys".equals(c.method())).count(); + } + + @Test + @DisplayName("[fleetd #489] a pane that stays IDLE the whole time (the paste-race case, " + + "measured live 2026-09-12) is nudged between /clear and bootstrapText, never lets " + + "them concatenate into one line") + void clearPickupIsNudgedBeforeBootstrapTextWhenPaneStaysIdle() throws IOException { + FakeHerdr herdr = new FakeHerdr(); // default agentStatus is "idle" throughout — no WORKING sample ever + Path handover = writeHandover("handover contents"); + AtomicLong clock = new AtomicLong(1_000); + LeadRollover rollover = newRollover(herdr, cfg(handover.toString()), 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()); + + int clearIdx = -1; + int bootstrapIdx = -1; + int firstNudgeIdx = -1; + for (int i = 0; i < herdr.calls.size(); i++) { + FakeHerdr.Call c = herdr.calls.get(i); + if ("agent.prompt".equals(c.method()) && String.valueOf(c.params()).contains("/clear") && clearIdx < 0) { + clearIdx = i; + } else if ("agent.prompt".equals(c.method()) && String.valueOf(c.params()).contains("read the handover file")) { + bootstrapIdx = i; + } else if ("agent.send_keys".equals(c.method()) && firstNudgeIdx < 0) { + firstNudgeIdx = i; + } + } + assertTrue(clearIdx >= 0, "/clear must have been sent"); + assertTrue(bootstrapIdx >= 0, "bootstrapText must have been sent"); + assertTrue(firstNudgeIdx >= 0, "at least one agent.send_keys nudge must go out — the pane " + + "never reported WORKING, so the /clear Enter may have raced the paste, and only a " + + "re-sent Enter proves the clear rather than concatenating bootstrapText onto " + + "whatever sits unsubmitted in the input box"); + assertTrue(firstNudgeIdx > clearIdx, "the nudge must happen AFTER /clear was sent, got call " + + "order: " + herdr.calls); + assertTrue(firstNudgeIdx < bootstrapIdx, "the nudge must happen BEFORE bootstrapText is " + + "sent — never concatenated onto the same input line, got call order: " + herdr.calls); + } + + @Test + @DisplayName("[fleetd #489] once a WORKING sample confirms /clear was picked up, nudging stops " + + "and bootstrapText is still sent after the pane returns to IDLE") + void pickupSeenStopsNudgingAndBootstrapTextIsSent() throws IOException { + FakeHerdr fake = new FakeHerdr(); + // Scripts the SECOND wait only: idle (default) until /clear is sent, then the first status + // poll after /clear reports WORKING (a confirmed pickup), and every poll after that reports + // IDLE (the completion boundary). The first wait (turnSettleSeconds) never sees this + // sequence — it passes on its own first poll, before /clear is ever sent, on the default + // "idle" status. + AtomicLong postClearGetCalls = new AtomicLong(0); + HerdrClient scriptsPickupThenIdle = new HerdrClient() { + private volatile boolean clearSent = false; + + @Override + public JsonNode call(String method, Object params) throws HerdrException { + if ("agent.prompt".equals(method) && String.valueOf(params).contains("/clear")) { + clearSent = true; + } + if (clearSent && "agent.get".equals(method)) { + long n = postClearGetCalls.incrementAndGet(); + fake.agentStatus(n == 1 ? "working" : "idle"); + } + return fake.call(method, params); + } + + @Override + public void close() { + fake.close(); + } + }; + Path handover = writeHandover("handover contents"); + AtomicLong clock = new AtomicLong(1_000); + LeadRollover rollover = newRollover(scriptsPickupThenIdle, cfg(handover.toString()), 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()); + assertEquals(2, promptCallCount(fake), "a confirmed WORKING pickup followed by IDLE must " + + "still complete the full roll — /clear then bootstrapText"); + assertEquals(0, sendKeysCallCount(fake), "once WORKING was observed, nudging must stop " + + "immediately — no agent.send_keys call should ever have been needed or sent"); + assertTrue(postClearGetCalls.get() >= 2, "the pane's status must have been polled AGAIN " + + "after the WORKING sample, before bootstrapText was sent — this is what proves " + + "the method actually waited for the WORKING -> IDLE completion boundary instead " + + "of returning as soon as pickup was seen (or worse, without polling at all, as a " + + "stub that just returns true would); got " + postClearGetCalls.get() + + " agent.get call(s) after /clear"); + } + + @Test + @DisplayName("[fleetd #489] a pane that never reaches a turn boundary after /clear (stuck at " + + "UNKNOWN, never WORKING either) lets clearSettleSeconds expire — bootstrapText is " + + "never sent") + void clearPickupNeverSettlesWhenStatusNeverReachesABoundary() throws IOException { + FakeHerdr fake = new FakeHerdr(); + HerdrClient stuckUnknownAfterClear = new HerdrClient() { + @Override + public JsonNode call(String method, Object params) throws HerdrException { + if ("agent.prompt".equals(method) && String.valueOf(params).contains("/clear")) { + // "wedged" maps to AgentStatus.UNKNOWN (see AgentStatus#fromWire) — neither a + // pickup signal (WORKING) nor a boundary (IDLE/DONE), and distinct from the + // already-covered BLOCKED case below. + fake.agentStatus("wedged"); + } + return fake.call(method, params); + } + + @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(stuckUnknownAfterClear, 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"); + 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 when the pane never " + + "reaches a turn boundary after /clear, whether WORKING was ever observed or not"); + assertEquals(0, sendKeysCallCount(fake), "an UNKNOWN status is neither a pickup signal nor " + + "a boundary — it must never be nudged"); + } + + @Test + @DisplayName("[fleetd #489] a submit() nudge that throws does not abort the roll — /clear and " + + "bootstrapText are both still sent") + void submitThatThrowsDoesNotAbortTheRoll() throws IOException { + FakeHerdr fake = new FakeHerdr(); // default idle throughout — nudging will be attempted + HerdrClient throwsOnSubmit = new HerdrClient() { + @Override + public JsonNode call(String method, Object params) throws HerdrException { + if ("agent.send_keys".equals(method)) { + throw new RuntimeException("simulated herdr transport failure on submit"); + } + return fake.call(method, params); + } + + @Override + public void close() { + fake.close(); + } + }; + Path handover = writeHandover("handover contents"); + AtomicLong clock = new AtomicLong(1_000); + LeadRollover rollover = newRollover(throwsOnSubmit, cfg(handover.toString()), 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 = fake.calls.stream().filter(c -> "agent.prompt".equals(c.method())).toList(); + assertEquals(2, prompts.size(), "a throwing submit() must be swallowed, not abort the roll " + + "— /clear and bootstrapText must both still be sent"); + assertTrue(prompts.get(0).params().toString().contains("/clear")); + assertTrue(prompts.get(1).params().toString().contains("read the handover file")); + } + @Test @DisplayName("a full successful roll sends /clear then bootstrapText, in order, and consumes the token") void successfulRollSendsClearThenBootstrapTextAndConsumesTheToken() throws IOException {