diff --git a/fleetd/src/main/java/dev/ltms/fleet/inject/CompletionResolver.java b/fleetd/src/main/java/dev/ltms/fleet/inject/CompletionResolver.java index 50755fc..7ad8406 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/inject/CompletionResolver.java +++ b/fleetd/src/main/java/dev/ltms/fleet/inject/CompletionResolver.java @@ -548,8 +548,19 @@ public final class CompletionResolver implements TurnListener { * fast completion. Runs the same backend-error classification the normal and raw-scrape paths * apply, against whatever is on screen right now: a match together with the too-fast crash * signature notifies {@link #backendErrorSink} (only on the resolution that wins the race). A - * non-match stays the original generic too-fast failure, naming the member and both timings, - * with whatever the pane shows appended so the caller sees the cause, not just "it failed". + * non-match stays the generic too-fast failure, naming the member and both timings, with + * whatever the pane shows appended so the caller sees the evidence, not just "it failed". + * + *

fleetd#376: this path must never resolve a completion. A fast backend can + * genuinely answer inside the floor, so the failure is sometimes wrong — but it is wrong in the + * loud direction, and the fix for that is honest wording, not a guess at the pane's meaning. + * Reclassifying from the scrape was tried and rejected: there is no reliable positive marker for + * "this is a real reply" across backends. {@link #lastAssistantBlock} falls back to the entire + * pane when it finds no {@code ⏺} marker, so on a crash the candidate "reply" is the whole + * screen; and {@code ⏺} itself is a Claude Code marker that an opencode pane never carries — the + * very backend whose speed raised this ticket. Any weaker test (non-blank, or "contains sentence + * punctuation") passes on almost every crash, because a pane holding a file path, a version + * number or a hostname contains a full stop. That trades a loud wrong answer for a silent one. */ private void failTooFast(String target, InFlight turn, CompletableFuture waiter, long elapsedNanos) { @@ -560,13 +571,13 @@ public final class CompletionResolver implements TurnListener { scrape = ""; } String clippedScrape = clip(scrape); - String baseReason = String.format( - "member %s went BUSY -> DONE in %dms (floor %dms) — too fast to be real work, most " - + "likely a backend error before any work started", + String timing = String.format( + "member %s went BUSY -> DONE in %dms (floor %dms)", target, elapsedNanos / 1_000_000, MIN_TURN_NANOS / 1_000_000); String backendError = firstMatchingLine(scrape, backendErrorPatternOrFallback(target)); if (backendError != null) { - String reason = baseReason + ": " + clippedScrape; + String reason = timing + " — too fast to be real work, and the pane carries a backend " + + "error: " + clippedScrape; if (rendezvous.resolveFailure(waiter, reason)) { inFlight.remove(target, turn); log.warn("failing send to {} via turn-stall fallback: {}", target, reason); @@ -575,7 +586,14 @@ public final class CompletionResolver implements TurnListener { } return; } - fail(target, turn, clippedScrape.isBlank() ? baseReason : baseReason + ": " + clippedScrape); + // fleetd#376: no pattern matched, so the cause is genuinely unknown. Say that, rather than + // asserting a backend error the way this message used to — a fast backend really can finish + // inside the floor, and a reader who trusts a wrong cause stops looking at the pane. + String reason = timing + " — inside the floor. That is usually a backend error before any " + + "work started, but a fast backend can answer inside it too, and nothing here can " + + "tell those apart, so the turn is reported failed rather than guessed. Read the " + + "pane below before deciding which it was"; + fail(target, turn, clippedScrape.isBlank() ? reason : reason + ": " + clippedScrape); } /** diff --git a/fleetd/src/test/java/dev/ltms/fleet/inject/CompletionResolverTest.java b/fleetd/src/test/java/dev/ltms/fleet/inject/CompletionResolverTest.java index e1aaab2..1d17799 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/inject/CompletionResolverTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/inject/CompletionResolverTest.java @@ -357,6 +357,54 @@ class CompletionResolverTest { "the failure carries whatever was on screen: " + waiter.getNow(null).text()); } + @Test + void aPlausibleLookingReplyInsideTheFloorStillFails() { + // fleetd#376 guard. A fix was attempted that inspected the pane inside the floor and resolved + // a COMPLETION when the text "looked like a real reply". Every cheap test for that is unsafe: + // lastAssistantBlock falls back to the WHOLE pane when there is no ⏺ marker, and a crash pane + // almost always contains sentence punctuation — in a file path, a version, or a hostname. + // This pane is the trap: it reads like a finished answer and it is a backend failure. + FakeHerdr herdr = new FakeHerdr().readText( + "Error: connection reset while loading src/main/java/Foo.java v1.2.3\n❯ "); + Rendezvous rendezvous = new Rendezvous(); + long[] clock = {10_000_000_000L}; + CompletionResolver resolver = new CompletionResolver(new AgentControl(herdr), rendezvous, + ExhaustedPatternLookup.none(), ExhaustionSink.none(), () -> clock[0]); + + var waiter = rendezvous.open("term_a"); + var turn = new CompletionResolver.InFlight(waiter, null, clock[0]); + clock[0] += CompletionResolver.MIN_TURN_NANOS - 1; // inside the floor + + resolver.resolve("term_a", turn); + + assertEquals(Rendezvous.Kind.FAILED, waiter.getNow(null).kind(), + "inside the floor the verdict is always FAILED — never guess a completion from pane text"); + } + + @Test + void theTooFastFailureDoesNotAssertACauseItCannotKnow() { + // fleetd#376: the message used to say "most likely a backend error before any work started". + // When no error pattern matches, that cause is a guess, and a reader who believes it stops + // looking at the pane. The verdict stays FAILED; only the claim about WHY is withdrawn. + FakeHerdr herdr = new FakeHerdr().readText("I am running on opencode/mimo-v2.5-free.\n❯ "); + Rendezvous rendezvous = new Rendezvous(); + long[] clock = {10_000_000_000L}; + CompletionResolver resolver = new CompletionResolver(new AgentControl(herdr), rendezvous, + ExhaustedPatternLookup.none(), ExhaustionSink.none(), () -> clock[0]); + + var waiter = rendezvous.open("term_a"); + var turn = new CompletionResolver.InFlight(waiter, null, clock[0]); + clock[0] += CompletionResolver.MIN_TURN_NANOS - 1; // inside the floor + + resolver.resolve("term_a", turn); + + String text = waiter.getNow(null).text(); + assertEquals(Rendezvous.Kind.FAILED, waiter.getNow(null).kind(), "still fails, still loud"); + assertFalse(text.contains("most likely a backend error"), + "an unmatched fast turn must not assert a backend error: " + text); + assertTrue(text.contains("mimo-v2.5-free"), "the pane is still carried: " + text); + } + @Test void aBusyToDoneTransitionJustOutsideTheFloorResolvesNormally() { FakeHerdr herdr = new FakeHerdr().readText("⏺ a real, if quick, answer\n❯ "); @@ -1117,8 +1165,16 @@ class CompletionResolverTest { assertTrue(waiter.isDone()); assertEquals(Rendezvous.Kind.FAILED, waiter.getNow(null).kind(), "still a failure — the floor itself, not the pattern, is why"); - assertTrue(waiter.getNow(null).text().contains("too fast to be real work"), - "a non-match inside the floor stays the generic too-fast reason: " + waiter.getNow(null).text()); + // fleetd#376: this used to assert the phrase "too fast to be real work", which carried the + // claim "most likely a backend error before any work started". With no pattern matched that + // cause is a guess, so the wording was withdrawn. What this test really guards is unchanged: + // the floor alone still fails the turn, it stays generic, and it never notifies the sink. + String reason = waiter.getNow(null).text(); + assertTrue(reason.contains("inside the floor"), + "a non-match inside the floor stays the generic floor reason: " + reason); + assertFalse(reason.contains("most likely a backend error"), + "a non-match must not assert a cause it did not establish: " + reason); + assertTrue(reason.contains("still starting up"), "the pane is still carried: " + reason); assertTrue(notified.isEmpty(), "a non-match must never notify the typed sink"); } }