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 aefa43f..1e6d830 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java +++ b/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java @@ -231,7 +231,9 @@ public final class LeadRollover { * #status} could wrongly answer {@link #UNKNOWN} ("nothing was ever requested") for a roll * that is, in fact, actively running. This is not sticky: the deferred continuation * overwrites this same entry with a terminal state ({@link #ROLLED}, {@link - * #TURN_NEVER_SETTLED}, or {@link #CLEAR_NEVER_SETTLED}) once it finishes. + * #TURN_NEVER_SETTLED}, {@link #CLEAR_NEVER_SETTLED}, or {@link #FAILED}) once it finishes + * — including by throwing, which fleetd #615's catch in {@link #runRollover} now turns into + * {@link #FAILED} instead of leaving this entry stuck forever. */ IN_PROGRESS, /** @@ -253,6 +255,19 @@ public final class LeadRollover { * within {@code clearSettleSeconds} — {@code bootstrapText} was never sent. */ CLEAR_NEVER_SETTLED, + /** + * fleetd #615: the deferred continuation threw a {@link RuntimeException} — most likely a + * {@link dev.ltms.fleet.herdr.HerdrException} out of one of the two unwrapped {@code + * agents.send} calls in {@link #runRollover} — and the continuation thread died with it. + * Before this state existed, that throw left {@link #outcomes} holding {@link #IN_PROGRESS} + * forever, because the production {@code continuationRunner} is a bare virtual thread with + * no uncaught-exception handler and nothing downstream of the throw ever ran to write a + * terminal outcome. {@code detail} names the exception, so a reader has something to act on + * — the same diagnostic style as {@link #TURN_NEVER_SETTLED} and {@link + * #CLEAR_NEVER_SETTLED}. The roll is dead at this point and does not retry itself; a stuck + * lead must {@link #open} a fresh request. + */ + FAILED, /** * {@code token} names nothing this instance currently knows about: never issued by {@link * #open}, dropped by {@link #cancel}, or aged out of {@link #outcomes}'s bounded history. @@ -493,8 +508,42 @@ public final class LeadRollover { * entirely after {@link #confirm} has returned to its caller — see this class's javadoc for the * four-step order. There is no result to return to by this point, so every outcome is logged * only. + * + *
fleetd #615 — the whole body is wrapped in one {@code try}. The two {@code + * agents.send} calls below are not wrapped individually: {@code send} → {@code agentCall} → + * {@code herdr.call} can throw an unchecked {@link dev.ltms.fleet.herdr.HerdrException} (see + * {@code AgentControl.java}), and the production {@code continuationRunner} is a bare virtual + * thread with no uncaught-exception handler (see this class's public constructor). Before this + * fix, either throw killed the continuation thread silently, leaving the {@link + * RollState#IN_PROGRESS} entry {@link #confirm} wrote at hand-off stuck forever — {@link + * #status} had no way to tell a dead roll from one still genuinely running. The {@code catch} + * below is scoped to the method body rather than to each {@code send} call individually, so it + * also covers anything else added to this continuation later, not just today's two call sites — + * the same reasoning that put the write-a-terminal-outcome step at each of this method's other + * exits (see the {@link RollState#TURN_NEVER_SETTLED} and {@link RollState#CLEAR_NEVER_SETTLED} + * branches below) rather than inside the helpers that detect them.
+ * + *Only {@link RuntimeException} is caught, matching the local convention {@link + * #waitUntilAtTurnBoundary} already set around its own {@code agents.status} call — not the + * broader {@link Exception} or {@link Throwable}, which would also swallow something like an + * {@link OutOfMemoryError} this continuation has no business handling.
*/ private void runRollover(PendingRollover p, FleetConfig.LeadRollover cfg) { + try { + runRolloverUnguarded(p, cfg); + } catch (RuntimeException e) { + log.warn("lead-rollover: continuation for token={} lead={} threw {} — the roll is dead; " + + "no further step in this continuation will run", + p.token(), p.leadTerminal(), e.toString(), e); + outcomes.put(p.token(), new RollStatus(RollState.FAILED, + "the roll's continuation threw " + e.toString() + " — the roll is dead and will " + + "not retry itself; check the daemon log for the stack trace, then open() " + + "a fresh rollover request")); + } + } + + /** The actual body of {@link #runRollover}, unwrapped — see that method's javadoc for the catch. */ + private void runRolloverUnguarded(PendingRollover p, FleetConfig.LeadRollover cfg) { String lead = p.leadTerminal(); long rollStartMillis = nowMillis.getAsLong(); TurnSettleResult turnResult = waitUntilAtTurnBoundary(lead, cfg.turnSettleSeconds()); 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 07d5bb8..dbf410d 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java @@ -1359,4 +1359,92 @@ class LeadRolloverTest { + "it were the measured wait duration: " + message); } } + + // ---- fleetd #615: a HerdrException out of either unwrapped agents.send call must leave a ---- + // ---- TERMINAL FAILED outcome, never a stuck IN_PROGRESS --------------------------------------- + + @Test + @DisplayName("[fleetd #615 — 1] send() throwing on the /clear call leaves status(token) " + + "reporting FAILED, not stuck at IN_PROGRESS") + void sendThrowingOnClearLeavesStatusReportingFailed() throws IOException { + FakeHerdr fake = new FakeHerdr(); // default idle — the turn-settle wait passes immediately + HerdrClient throwsOnClear = new HerdrClient() { + @Override + public JsonNode call(String method, Object params) throws HerdrException { + if ("agent.prompt".equals(method) && String.valueOf(params).contains("/clear")) { + throw new HerdrException("simulated herdr transport failure sending /clear"); + } + 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(throwsOnClear, 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(), "every synchronous gate passes; the throw happens only " + + "inside the deferred continuation, which this test's synchronous runner has " + + "already run to completion by the time confirm() returns"); + + LeadRollover.RollStatus status = rollover.status(pending.token()); + assertEquals(LeadRollover.RollState.FAILED, status.state(), + "a HerdrException out of the /clear send must leave a TERMINAL FAILED outcome — " + + "before fleetd #615's fix, the continuation thread died silently and " + + "status() was stuck reporting the IN_PROGRESS confirm() wrote at hand-off, " + + "forever: got " + status.state() + " / " + status.detail()); + assertNotEquals(LeadRollover.RollState.IN_PROGRESS, status.state()); + assertTrue(status.detail().contains("HerdrException"), "the detail must name the exception " + + "so an operator reading status() has something to act on: " + status.detail()); + } + + @Test + @DisplayName("[fleetd #615 — 2] send() throwing on the bootstrap-text call (after /clear " + + "succeeded and the pane settled) also leaves status(token) reporting FAILED — a " + + "DIFFERENT exit from the /clear-throw case above") + void sendThrowingOnBootstrapTextLeavesStatusReportingFailed() throws IOException { + FakeHerdr fake = new FakeHerdr(); // default idle throughout — both settle waits pass promptly + HerdrClient throwsOnBootstrapText = new HerdrClient() { + @Override + public JsonNode call(String method, Object params) throws HerdrException { + if ("agent.prompt".equals(method) && String.valueOf(params).contains("read the handover file")) { + throw new HerdrException("simulated herdr transport failure sending bootstrapText"); + } + 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(throwsOnBootstrapText, 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(), "every synchronous gate passes; the throw happens only " + + "inside the deferred continuation, which this test's synchronous runner has " + + "already run to completion by the time confirm() returns"); + assertEquals(1, promptCallCount(fake), "sanity: /clear was sent and settled — only the " + + "SECOND agent.prompt call (bootstrapText) threw"); + + LeadRollover.RollStatus status = rollover.status(pending.token()); + assertEquals(LeadRollover.RollState.FAILED, status.state(), + "a HerdrException out of the bootstrapText send — a DIFFERENT exit from the /clear " + + "throw, reached only after /clear already succeeded and the pane already " + + "settled — must also leave a TERMINAL FAILED outcome, not a stuck " + + "IN_PROGRESS: got " + status.state() + " / " + status.detail()); + assertNotEquals(LeadRollover.RollState.IN_PROGRESS, status.state()); + assertTrue(status.detail().contains("HerdrException"), "the detail must name the exception " + + "so an operator reading status() has something to act on: " + status.detail()); + } }