fleetd #241: suppress echoed fallback briefs #243

Closed
agent wants to merge 0 commits from worker/cb241-fallback-echo-1175e9-11 into main
Member

Fixes fleetd #241.

The completion fallback keeps the injected prompt with its turn token. It normalises both values and returns no-report only when one contains the other and both have at least 400 normalised characters. This catches a whole echoed brief while keeping a real report that quotes an acceptance section.

Known location:
[no report — the member ended its turn without fleet_reply, and the pane still shows the injected brief. Nothing was produced on the pane. Check the member worktree and branch for committed work before re-delegating. worktree=/tmp/member-worktree branch=worker/cb241]

Unknown location:
[no report — the member ended its turn without fleet_reply, and the pane still shows the injected brief. Nothing was produced on the pane. Check the member worktree and branch for committed work before re-delegating.]

Tests use the real MessageService -> Injector -> CompletionResolver fallback path. They cover echoed briefs with and without a known location, a real report that quotes part of the brief and returns unchanged, and an echoed brief clipped at 4,000 characters that keeps the existing partial marker.

Mutation proof:

Mutation Compile errors Tests red
Replaced the production echo condition with false 0 completionFallbackReplacesAnEchoedInjectedBriefWithNoReportOutcome; completionFallbackNamesTheKnownWorktreeAndBranchForAnEchoedBrief

grep -cE checked the mutation log before the failure result.

Checks run:

  • mvn -Dtest=MessageServiceTest test: 67 tests, 0 failures.
  • mvn clean install: Tests run: 1167, Failures: 0, Errors: 0, Skipped: 0; BUILD SUCCESS.

No worktree reads were added.

Fixes fleetd #241. The completion fallback keeps the injected prompt with its turn token. It normalises both values and returns no-report only when one contains the other and both have at least 400 normalised characters. This catches a whole echoed brief while keeping a real report that quotes an acceptance section. Known location: [no report — the member ended its turn without fleet_reply, and the pane still shows the injected brief. Nothing was produced on the pane. Check the member worktree and branch for committed work before re-delegating. worktree=/tmp/member-worktree branch=worker/cb241] Unknown location: [no report — the member ended its turn without fleet_reply, and the pane still shows the injected brief. Nothing was produced on the pane. Check the member worktree and branch for committed work before re-delegating.] Tests use the real MessageService -> Injector -> CompletionResolver fallback path. They cover echoed briefs with and without a known location, a real report that quotes part of the brief and returns unchanged, and an echoed brief clipped at 4,000 characters that keeps the existing partial marker. Mutation proof: | Mutation | Compile errors | Tests red | | --- | ---: | --- | | Replaced the production echo condition with false | 0 | completionFallbackReplacesAnEchoedInjectedBriefWithNoReportOutcome; completionFallbackNamesTheKnownWorktreeAndBranchForAnEchoedBrief | grep -cE checked the mutation log before the failure result. Checks run: - mvn -Dtest=MessageServiceTest test: 67 tests, 0 failures. - mvn clean install: Tests run: 1167, Failures: 0, Errors: 0, Skipped: 0; BUILD SUCCESS. No worktree reads were added.
agent added 1 commit 2026-09-03 06:11:45 +02:00
fleetd #241: suppress echoed fallback briefs
CI / contract (pull_request) Successful in 46s
CI / build (pull_request) Successful in 1m20s
321d8dcbb5
ltms added 1 commit 2026-09-03 06:39:15 +02:00
fleetd #241: bound the echo match so a real report is never swallowed
CI / contract (pull_request) Successful in 1m9s
CI / build (pull_request) Successful in 1m25s
3437d6313d
Round 1 used plain bidirectional containment. The direction that catches the real bug --
the pane holds the brief plus a status bar, so the scrape contains the brief -- also fires
when a member restates the whole brief and then writes a genuine report under it. That
threw the report away and told the lead nothing was produced, which is worse than the bug
being fixed: it destroys a delivery instead of merely obscuring one.

The safe direction (the scrape is a fragment of the brief) stays unbounded, because a
fragment of the brief is by definition not a report. The dangerous direction now requires
the scrape to add at most MAX_ECHO_EXCESS_CHARS beyond the brief, which is the amount of
TUI chrome a real echo carries.

Work by the cb241 worker, committed by the lead: its backend stopped answering after the
fix was written, so two turns ended with no commit and no reply. Verified by the lead:
1169 tests, 0 failures; removing the bound turns pinsTheMaximumTuiChromeExcess and
completionFallbackKeepsARealReportThatRestatesTheWholeBrief red with 0 compile errors.
Owner

Round 2 pushed as 3437d63. Verified, but not yet merged — it must land after #201/#227 Unit 5, see the collision note below.

The round-1 defect and the fix

Round 1 used plain bidirectional containment. The direction that catches the real bug — pane holds the brief plus a status bar, so scrape ⊇ brief — also fired when a member restated the whole brief and then wrote a genuine report. That threw the report away and told the lead nothing was produced. Strictly worse than the original bug: it destroys a delivery rather than obscuring one.

The fix keeps the asymmetry that matters:

if (normalInjected.contains(normalScrape)) {
    return true;                                    // safe: a fragment of the brief is not a report
}
return normalScrape.contains(normalInjected)
        && normalScrape.length() - normalInjected.length() <= MAX_ECHO_EXCESS_CHARS;

MAX_ECHO_EXCESS_CHARS = 160, on top of the existing ECHO_MIN_CHARS = 400.

Verification (lead)

Baseline 1169 tests, 0 failures, 0 compile errors. Removing the bound — back to plain containment — gives 2 reds, 0 compile errors:

CompletionResolverTest.pinsTheMaximumTuiChromeExcess:215
  one character beyond the excess must preserve the scrape as a real report

MessageServiceTest.completionFallbackKeepsARealReportThatRestatesTheWholeBrief:128
  a report that restates the whole brief must survive unchanged

The second is the case I originally reproduced, and it runs through the real MessageService path rather than calling the helper.

Why I committed this rather than the worker

The worker wrote the fix correctly, then its opencode backend stopped answering. Two consecutive turns ended with no commit, no push, and no reply — I found the work only by reading its worktree. So I took the diff, verified it in my own worktree, and committed it with attribution.

There is some irony worth recording: this ticket's own failure mode is what hid its fix. A member finished real work and the lead saw nothing. That is the third independent occurrence today, and it is the strongest argument for landing this.

Merge order — do not merge this first

Fleetd.java:393 is a single statement that both this PR and #201/#227 Unit 5 must change. This PR also gives the 6-argument CompletionResolver constructor a _ -> null default for the worktree lookup, so resolving the merge by keeping Unit 5's call site would compile cleanly, silently default the lookup to null, and leave the worktree/branch never named — the fix dead in production with a green suite. Exactly the #234 shape.

Resolution is the 8-argument constructor, and the check that proves it is a mutation at the Fleetd.java call site: drop the lookup argument and a test must go red.

Round 2 pushed as `3437d63`. **Verified, but not yet merged** — it must land after #201/#227 Unit 5, see the collision note below. ### The round-1 defect and the fix Round 1 used plain bidirectional containment. The direction that catches the real bug — pane holds the brief plus a status bar, so scrape ⊇ brief — also fired when a member restated the whole brief and *then* wrote a genuine report. That threw the report away and told the lead nothing was produced. Strictly worse than the original bug: it destroys a delivery rather than obscuring one. The fix keeps the asymmetry that matters: ```java if (normalInjected.contains(normalScrape)) { return true; // safe: a fragment of the brief is not a report } return normalScrape.contains(normalInjected) && normalScrape.length() - normalInjected.length() <= MAX_ECHO_EXCESS_CHARS; ``` `MAX_ECHO_EXCESS_CHARS = 160`, on top of the existing `ECHO_MIN_CHARS = 400`. ### Verification (lead) Baseline **1169 tests, 0 failures, 0 compile errors**. Removing the bound — back to plain containment — gives **2 reds, 0 compile errors**: ``` CompletionResolverTest.pinsTheMaximumTuiChromeExcess:215 one character beyond the excess must preserve the scrape as a real report MessageServiceTest.completionFallbackKeepsARealReportThatRestatesTheWholeBrief:128 a report that restates the whole brief must survive unchanged ``` The second is the case I originally reproduced, and it runs through the real `MessageService` path rather than calling the helper. ### Why I committed this rather than the worker The worker wrote the fix correctly, then its opencode backend stopped answering. Two consecutive turns ended with no commit, no push, and no reply — I found the work only by reading its worktree. So I took the diff, verified it in my own worktree, and committed it with attribution. There is some irony worth recording: **this ticket's own failure mode is what hid its fix.** A member finished real work and the lead saw nothing. That is the third independent occurrence today, and it is the strongest argument for landing this. ### Merge order — do not merge this first `Fleetd.java:393` is a single statement that both this PR and #201/#227 Unit 5 must change. This PR also gives the 6-argument `CompletionResolver` constructor a `_ -> null` default for the worktree lookup, so resolving the merge by keeping Unit 5's call site would compile cleanly, silently default the lookup to null, and leave the worktree/branch never named — the fix dead in production with a green suite. Exactly the #234 shape. Resolution is the 8-argument constructor, and the check that proves it is a mutation at the `Fleetd.java` call site: drop the lookup argument and a test must go red.
ltms closed this pull request 2026-09-03 06:57:47 +02:00
Some checks are pending
CI / contract (pull_request) Successful in 1m9s
CI / build (pull_request) Successful in 1m25s

Pull request closed

Sign in to join this conversation.