From 1fcda74596de9c1a1eda05da7334c9f121660d4b Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Mon, 31 Aug 2026 10:50:40 +0700 Subject: [PATCH 1/2] #164: classify a backend-error scrape as WORKER_FAILED, not a completed reply main already ships fleetd#164 points 1 and 2 (MIN_TURN_NANOS floor + hard-fail on an empty/unreadable scrape, commit 3bfa828, already an ancestor of main). The rescued worker branch worker/cb-164-empty-scrape-false-success-1a80af-3 (851ebca) reimplemented the same two behaviours via a separate, more invasive mechanism (elapsedNanos threaded through TurnListener/Injector/Fleetd) that would conflict with main's already-shipped clock-injection design if merged wholesale. Port forward only what main is genuinely missing: the narrow BACKEND_ERROR pattern (`(?i)\bAPI Error\s*:`) that classifies a cleanly-scraped-but-backend-rejected turn as a failure naming the member, instead of letting the error text resolve as a completed reply. See REPORT-cb164.md for the full reasoning, what was deliberately not ported, and the sabotage proof for the new tests. --- REPORT-cb164.md | 148 ++++++++++++++++++ .../ltms/fleet/inject/CompletionResolver.java | 17 ++ .../fleet/inject/CompletionResolverTest.java | 46 ++++++ .../ltms/fleet/msg/MessageServiceTest.java | 22 +++ 4 files changed, 233 insertions(+) create mode 100644 REPORT-cb164.md diff --git a/REPORT-cb164.md b/REPORT-cb164.md new file mode 100644 index 0000000..3e8d7d4 --- /dev/null +++ b/REPORT-cb164.md @@ -0,0 +1,148 @@ +# CB-164 report — worker/cb-164-rebase-885863-8 + +## Headline finding — please read this before the diff + +**`main` already ships a full fix for issue #164 points 1 and 2, done separately from the +rescued branch.** Commit `3bfa828` ("fleetd#164: an empty or suspiciously fast scrape must +fail, never resolve as a success") is already an ancestor of `origin/main` (verified with +`git merge-base --is-ancestor 3bfa828 origin/main`). It was written the same day as the +rescued commit (2026-08-28), by the same author, but on `main` directly. No later commit +touches `CompletionResolver.java` after it. + +What `main` already has, before any change of mine: + +- `CompletionResolver.MIN_TURN_NANOS` = 2 seconds — the exact "too fast to be a real + completion" floor the brief described, just under a different name and a different + mechanism (an injectable `LongSupplier nowNanos` clock read at delivery and again at + resolution, stored on the `InFlight` record — no interface or caller changes needed). +- A hard fail on any empty or unreadable scrape, via the existing `fail()` → + `Rendezvous.resolveFailure()` → `MessageService.Outcome.WORKER_FAILED` path — the same + pattern already used for the CB-109 wedge case. `FleetMcp.formatReply` already renders + `WORKER_FAILED` as `"[worker failed — turn ended in an unrecoverable state]\n" + text`, + never as blank content. I read this chain end to end and confirmed it (not just from the + commit message). + +So **"Part 2" as described in the brief — the one called out as the most important — was +already done.** What I did **not** find on `main`: the narrow `BACKEND_ERROR` pattern +(`(?i)\bAPI Error\s*:`) from the rescued branch. That is genuinely new. + +### Why I did not cherry-pick 851ebca, and what I did instead + +`851ebca` implements the same floor-check idea a second time, but via a structurally +different, more invasive mechanism: it threads a real measured `elapsedNanos` through +`TurnListener.onTurnComplete`, `Injector` (a new `turnStartedAtNanos` field, a new +constructor, a new `LongSupplier` clock), and `Fleetd`'s anonymous `TurnListener`. Cherry- +picking that wholesale onto current `main` would have created **two competing timing +mechanisms for the same floor check** in the same class — main's already-shipped +delivery-clock read, and a second, parameter-threaded one from the rescued branch — with +no clear answer for which one should win if they ever disagreed. Reintroducing the +Injector/TurnListener/Fleetd signature changes for that also seemed hard to justify: main's +already-tested approach measures essentially the same interval (delivery to resolution) +with no interface changes at all. + +Given that, I read the instruction *"if you cannot tell which behaviour a hunk is meant to +have, keep both behaviours and say so"* as pointing at genuinely uncertain hunks — not at +knowingly wiring in two redundant implementations of the identical check. So instead of a +mechanical cherry-pick, I hand-ported only the parts of `851ebca` that are **not** already +on `main`: + +1. Added `CompletionResolver.BACKEND_ERROR` — the exact narrow pattern from the brief, + `(?i)\bAPI Error\s*:`, with the same "keep it narrow" comment. +2. Added a classification check for it, placed right after the existing `BACKEND_EXHAUSTED` + pattern-match block (same position, same shape: `firstMatchingLine` against + `assistantBlock`, then `fail(target, turn, reason)` naming the member and the matched + line) and before the success path (`fleetd/src/main/java/dev/ltms/fleet/inject/ + CompletionResolver.java`). + +I did **not** port: +- `MIN_COMPLETED_TURN_NANOS` / the `elapsedNanos` parameter threading through + `TurnListener`/`Injector`/`Fleetd` — redundant with `main`'s already-shipped + `MIN_TURN_NANOS`/`LongSupplier` mechanism, and touching those three extra files for no + behavioural gain seemed like avoidable risk. +- The rescued branch's `visibleTurn = assistantBlock.isBlank() ? raw : assistantBlock` + fallback (falls back to the raw pane when the parsed assistant block is blank, so a + TUI-hidden error/exhausted line still classifies). This is a real, separate improvement, + but on `main`'s current check order the empty-tail fail already fires *before* the + exhausted/backend-error pattern checks run, so the fallback would be dead code unless I + also reordered those checks ahead of the empty-tail fail. That reorder is a bigger, + separate behavioural change than either "Part 1" or "Part 2" asked for, so I left it out + and am flagging it here rather than making that call silently. **Out-of-scope note for + the lead**, not fixed: a raw-screen-only exhausted/backend-error line (no `⏺` marker, so + `lastAssistantBlock` returns blank) is still swallowed into the generic "empty scrape" + failure rather than being classified specifically. + +I did not use `fleet_ask` for this: the brief's own instruction for exactly this kind of +conflict ("keep both, say so") already pre-authorized me to decide and document rather than +block, and a live `fleet_ask` has only a ~55s window per prior fleet notes, so I judged +documenting clearly here was more reliable than gambling on that window. If this call is +wrong, it's easy to undo — the diff is 3 files, +85/-0 lines. + +## Files changed (worktree-relative) + +- `fleetd/src/main/java/dev/ltms/fleet/inject/CompletionResolver.java` — added the + `BACKEND_ERROR` pattern constant and the classification check (+17 lines). +- `fleetd/src/test/java/dev/ltms/fleet/inject/CompletionResolverTest.java` — 3 new unit + tests for the `BACKEND_ERROR` classification (+46 lines). +- `fleetd/src/test/java/dev/ltms/fleet/msg/MessageServiceTest.java` — 1 new end-to-end test + driving the same scenario through `MessageService` (+22 lines). + +No changes to `Fleetd.java`, `Injector.java`, or `TurnListener.java` — see above for why. + +## Build — verbatim result + +Full unpiped `mvn clean install` from `fleetd/`, exit code `0`: + +``` +[INFO] Tests run: 1036, Failures: 0, Errors: 0, Skipped: 0 +[INFO] BUILD SUCCESS +``` + +No `[ERROR]` lines anywhere in the full log (checked with `grep -n "\[ERROR\]"` over the +whole log, not a piped/truncated view). + +## Sabotage proof for the new tests + +Commented out the new check in `CompletionResolver.java`: + +```java +// String backendError = firstMatchingLine(assistantBlock, BACKEND_ERROR); +// if (backendError != null) { +// fail(target, turn, "member " + target + " ended on a backend error: " + backendError); +// return; +// } +``` + +Ran just the 4 new tests: + +``` +mvn -q test -Dtest='CompletionResolverTest#classifiesABackendErrorLineAsAFailureInsteadOfACompletedReply+theBackendErrorReasonNamesTheMemberAndCarriesTheMatchedLine+aCaseInsensitiveApiErrorLineIsStillClassifiedAsABackendError,MessageServiceTest#backendErrorScrapeThroughMessageServiceFailsInsteadOfBecomingReplyText' +``` + +All 4 failed, exactly as expected without the fix: + +``` +[ERROR] Tests run: 4, Failures: 4, Errors: 0, Skipped: 0 +[ERROR] CompletionResolverTest.aCaseInsensitiveApiErrorLineIsStillClassifiedAsABackendError:612 + the pattern is case-insensitive ==> expected: but was: +[ERROR] CompletionResolverTest.classifiesABackendErrorLineAsAFailureInsteadOfACompletedReply:582 + a backend rejection is a failure, not a completed reply ==> expected: but was: +[ERROR] CompletionResolverTest.theBackendErrorReasonNamesTheMemberAndCarriesTheMatchedLine:597 + the failure names the member: API Error: 400 invalid request body ==> expected: but was: +[ERROR] MessageServiceTest.backendErrorScrapeThroughMessageServiceFailsInsteadOfBecomingReplyText:104 + a backend rejection must use the caller's failure outcome, not a completed reply + ==> expected: but was: +``` + +Then restored the fix (put the block back exactly as committed) and re-ran the full +`mvn clean install` — green again, see above (`Tests run: 1036, Failures: 0`). + +## What I could not confirm + +- I have no IDE tooling and no way to run the daemon live — only `mvn` in this worktree. + Everything reported above is from that command, nothing else. +- I did not attempt the "visibleTurn/raw-fallback" reorder described above — flagged as an + open, separate item, not fixed, not tested. +- Points 3 (broader backend-error surfacing) and 4 (spawn-time profile quarantine) of the + issue are untouched, as instructed. +- I have not verified this against a live daemon or a real crashed backend — only against + the unit/integration test fixtures in this repo. 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 c1dd7af..417219e 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/inject/CompletionResolver.java +++ b/fleetd/src/main/java/dev/ltms/fleet/inject/CompletionResolver.java @@ -74,6 +74,14 @@ public final class CompletionResolver implements TurnListener { */ public static final long MIN_TURN_NANOS = Duration.ofSeconds(2).toNanos(); + /** + * fleetd#164 (part 2): one stable, explicit backend-failure marker seen on a Claude Code pane + * when the backend itself rejected the turn (e.g. {@code "API Error: 400 invalid request body"}). + * Kept deliberately narrow — a growing list of ad-hoc error strings rots as backends change their + * wording; broader backend-error surfacing is out of scope here (fleetd#164 point 3). + */ + private static final Pattern BACKEND_ERROR = Pattern.compile("(?i)\\bAPI Error\\s*:"); + private static final String CLIPPED_PANE_TAIL_MARKER = "[Pane tail clipped: member did not call fleet_reply.]"; @@ -278,6 +286,15 @@ public final class CompletionResolver implements TurnListener { } return; } + // fleetd#164 (part 2): a scrape that read cleanly and produced content still isn't a real + // reply when that content is the backend's own rejection (e.g. an HTTP 400 before the worker + // did any work). Classify it as a failure naming the member, rather than handing the caller a + // scrape that reads like a completed answer. + String backendError = firstMatchingLine(assistantBlock, BACKEND_ERROR); + if (backendError != null) { + fail(target, turn, "member " + target + " ended on a backend error: " + backendError); + return; + } String completion = clipped ? tail + "\n" + CLIPPED_PANE_TAIL_MARKER : tail; if (rendezvous.resolveCompletion(waiter, completion)) { inFlight.remove(target, turn); 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 407e163..9b150ef 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/inject/CompletionResolverTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/inject/CompletionResolverTest.java @@ -566,6 +566,52 @@ class CompletionResolverTest { assertEquals("The usage limit has been reached.", waiter.getNow(null).text()); } + // --- fleetd#164 (part 2 addendum): narrow BACKEND_ERROR pattern classification --------- + + @Test + void classifiesABackendErrorLineAsAFailureInsteadOfACompletedReply() { + String block = "⏺ API Error: 400 invalid request body\n❯ "; + FakeHerdr herdr = new FakeHerdr().readText(block); + Rendezvous rendezvous = new Rendezvous(); + CompletionResolver resolver = new CompletionResolver(new AgentControl(herdr), rendezvous, ExhaustedPatternLookup.none(), ExhaustionSink.none()); + + var waiter = rendezvous.open("term_a"); + resolver.resolve("term_a", new CompletionResolver.InFlight(waiter, null)); + + assertTrue(waiter.isDone(), "a backend-error scrape still resolves the blocked send"); + assertEquals(Rendezvous.Kind.FAILED, waiter.getNow(null).kind(), + "a backend rejection is a failure, not a completed reply"); + } + + @Test + void theBackendErrorReasonNamesTheMemberAndCarriesTheMatchedLine() { + String block = "⏺ API Error: 400 invalid request body\n❯ "; + FakeHerdr herdr = new FakeHerdr().readText(block); + Rendezvous rendezvous = new Rendezvous(); + CompletionResolver resolver = new CompletionResolver(new AgentControl(herdr), rendezvous, ExhaustedPatternLookup.none(), ExhaustionSink.none()); + + var waiter = rendezvous.open("term_a"); + resolver.resolve("term_a", new CompletionResolver.InFlight(waiter, null)); + + String reason = waiter.getNow(null).text(); + assertTrue(reason.contains("term_a"), "the failure names the member: " + reason); + assertTrue(reason.contains("API Error: 400 invalid request body"), + "the failure carries the matched backend-error line: " + reason); + } + + @Test + void aCaseInsensitiveApiErrorLineIsStillClassifiedAsABackendError() { + String block = "⏺ api error: rate limited\n❯ "; + FakeHerdr herdr = new FakeHerdr().readText(block); + Rendezvous rendezvous = new Rendezvous(); + CompletionResolver resolver = new CompletionResolver(new AgentControl(herdr), rendezvous, ExhaustedPatternLookup.none(), ExhaustionSink.none()); + + var waiter = rendezvous.open("term_a"); + resolver.resolve("term_a", new CompletionResolver.InFlight(waiter, null)); + + assertEquals(Rendezvous.Kind.FAILED, waiter.getNow(null).kind(), "the pattern is case-insensitive"); + } + @Test void coverageIsOffWhenNoProfileHasAPatternConfigured() { assertEquals("off (no profile has an exhaustedPattern configured; profiles: [terra])", diff --git a/fleetd/src/test/java/dev/ltms/fleet/msg/MessageServiceTest.java b/fleetd/src/test/java/dev/ltms/fleet/msg/MessageServiceTest.java index dde5d87..eca9721 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/msg/MessageServiceTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/msg/MessageServiceTest.java @@ -86,6 +86,28 @@ class MessageServiceTest { assertTrue(reply.completed(), "a scraped completion still counts as completed"); } + @Test + void backendErrorScrapeThroughMessageServiceFailsInsteadOfBecomingReplyText() throws Exception { + // fleetd#164 (part 2 addendum): a scrape that reads cleanly but is only the backend's own + // rejection (e.g. an HTTP 400) must reach the caller as WORKER_FAILED, not as a completed + // reply whose text happens to be the error line. + CompletableFuture send = sendAsync(); + awaitWaiting(); + + herdr.readText("$ prompt"); + injector.onStatus(T, AgentStatus.IDLE); + injector.onStatus(T, AgentStatus.WORKING); + herdr.readText("⏺ API Error: 400 invalid request body"); + injector.onStatus(T, AgentStatus.IDLE); + + MessageService.Reply reply = send.get(5, TimeUnit.SECONDS); + assertEquals(MessageService.Outcome.WORKER_FAILED, reply.outcome(), + "a backend rejection must use the caller's failure outcome, not a completed reply"); + assertFalse(reply.completed(), "plain backend errors are never fallback reply content"); + assertTrue(reply.text().contains("API Error: 400 invalid request body"), + "the visible backend error is carried as the failure reason: " + reply.text()); + } + @Test void explicitFleetReplyResolvesAsReplied() throws Exception { CompletableFuture send = sendAsync(); -- 2.52.0 From 8aaf1f7e447cff9f015bf31ecd5842ccb3ffec86 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Mon, 31 Aug 2026 10:56:15 +0700 Subject: [PATCH 2/2] #164: a backend-error failure must carry the whole scrape, not just the matched line The BACKEND_ERROR pattern is a heuristic. It also matches a member that forgot fleet_reply while reporting *about* a backend error. Failing is still right -- the caller must never read a scrape as an answer -- but the reason carried only the matched line, so the rest of the report was thrown away. That is the same defect #164 exists to fix: information destroyed on the way to the caller. Carry the full pane tail alongside the classification, so a genuine backend error reads the same as before and a false positive keeps its report. --- .../ltms/fleet/inject/CompletionResolver.java | 7 ++++++- .../fleet/inject/CompletionResolverTest.java | 21 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) 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 417219e..94adcc9 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/inject/CompletionResolver.java +++ b/fleetd/src/main/java/dev/ltms/fleet/inject/CompletionResolver.java @@ -292,7 +292,12 @@ public final class CompletionResolver implements TurnListener { // scrape that reads like a completed answer. String backendError = firstMatchingLine(assistantBlock, BACKEND_ERROR); if (backendError != null) { - fail(target, turn, "member " + target + " ended on a backend error: " + backendError); + // Carry the whole scrape, not just the matched line. The pattern is a heuristic: a member + // that forgot fleet_reply while reporting *about* a backend error matches it too. Failing + // is still right — the caller must not read a scrape as an answer — but dropping the rest + // of the pane would destroy the report, which is the same defect fleetd#164 is about. + fail(target, turn, "member " + target + " ended on a backend error: " + backendError + + "\n--- pane tail ---\n" + tail); return; } String completion = clipped ? tail + "\n" + CLIPPED_PANE_TAIL_MARKER : tail; 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 9b150ef..3f7e3c0 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/inject/CompletionResolverTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/inject/CompletionResolverTest.java @@ -612,6 +612,27 @@ class CompletionResolverTest { assertEquals(Rendezvous.Kind.FAILED, waiter.getNow(null).kind(), "the pattern is case-insensitive"); } + @Test + void aBackendErrorFailureStillCarriesTheRestOfTheScrape() { + // The pattern is a heuristic: a member that forgot fleet_reply while *reporting on* a backend + // error matches it too. Failing is still correct, but the report itself must survive — losing + // it would be the same information-destroying defect fleetd#164 exists to fix. + String block = "\u23fa I looked into the gateway problem.\n" + + "The log line was: API Error: 400 invalid request body\n" + + "The cause is a missing content-type header.\n\u276f "; + FakeHerdr herdr = new FakeHerdr().readText(block); + Rendezvous rendezvous = new Rendezvous(); + CompletionResolver resolver = new CompletionResolver(new AgentControl(herdr), rendezvous, ExhaustedPatternLookup.none(), ExhaustionSink.none()); + + var waiter = rendezvous.open("term_a"); + resolver.resolve("term_a", new CompletionResolver.InFlight(waiter, null)); + + String reason = waiter.getNow(null).text(); + assertEquals(Rendezvous.Kind.FAILED, waiter.getNow(null).kind()); + assertTrue(reason.contains("The cause is a missing content-type header."), + "the failure carries the rest of the pane, not only the matched line: " + reason); + } + @Test void coverageIsOffWhenNoProfileHasAPatternConfigured() { assertEquals("off (no profile has an exhaustedPattern configured; profiles: [terra])", -- 2.52.0