fleet/fleetd
3
0
Fork 0
Code Issues 73 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Completion fallback can hand the lead back its own brief as the member's "report" #241

New Issue
Closed
opened 2026-09-03 05:42:40 +02:00 by ltms · 3 comments
ltms commented 2026-09-03 05:42:40 +02:00
Owner
Copy Link
Copy Source

What happened

A worker (terra, opencode backend) finished its task, committed bbf68f3 and pushed it. It then ended its turn without a fleet_reply. The turn-done fallback scraped the pane and returned that scrape as the report.

The scrape was the lead's own brief, rendered in the member's pane. The "report" I collected from fleet_poll was several hundred words of my own instructions read back to me, ending in the member's idle status bar:

[done — worker finished without a structured fleet_reply; transcript tail follows]
  ┃  **It has no test.** I reverted that one hunk to the old fall-through form ...
  ┃  ## Acceptance
  ┃  - The new test passes on your current code.
  ...
  ┃  Dev auto · GPT-5.6 Terra OpenAI

Not one word of it came from the member.

Why this matters

The fallback exists so a lost reply still yields something. Here it yields something actively misleading. Three specific harms:

  1. It looks like a report. It is prose, it is on-topic, and it is long. Nothing in it signals "this is your own text". A lead skimming it can easily conclude the worker refused, stalled, or did nothing.
  2. It hides a successful delivery. The work was done, committed and pushed. I only found that out because I went and ran git -C <worktree> log. A lead that trusted the report would have re-delegated finished work, or written the member off as broken.
  3. It is worse than an empty result. An empty or explicitly-failed scrape would have sent me straight to the worktree. The echo sent me looking for a problem that did not exist.

Suggested fix

When the fallback scrape is substantially the text that was injected for this turn, do not return it as a report. The daemon already holds the injected prompt for the turn, so the comparison is local and cheap — normalised containment or a similarity ratio over the scrape versus the injected content.

On a match, return an explicit outcome instead, something like:

[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's worktree and branch
for committed work before re-delegating.]

That last sentence is the part that would have saved the round trip.

Worth considering alongside it: when a member's session has a known worktree and branch, the fallback could name them in that message, since that is exactly where a lead has to look next.

Not proposing

Do not make the fallback go and read the worktree itself — that is the lead's call and a different layer's job. This ticket is only about not presenting the lead's own words back as the member's answer.

Evidence

  • Member: profile terra, role dev, pane 5bd048f0-7795-4c09-9ec3-8faea70c44c7, worktree /Users/dai.ha/LTMS/.bridged-worktrees/c9351c-7, branch worker/cb201-unit4-outcome-a13bfa-7.
  • Work that the report failed to mention: commit bbf68f3, pushed to origin, 115 lines of new test.
  • Observed 2026-09-03 on daemon pid 45674, jar ecb9645758b3.
## What happened A worker (`terra`, opencode backend) finished its task, committed `bbf68f3` and pushed it. It then ended its turn **without** a `fleet_reply`. The turn-done fallback scraped the pane and returned that scrape as the report. The scrape was **the lead's own brief**, rendered in the member's pane. The "report" I collected from `fleet_poll` was several hundred words of my own instructions read back to me, ending in the member's idle status bar: ``` [done — worker finished without a structured fleet_reply; transcript tail follows] ┃ **It has no test.** I reverted that one hunk to the old fall-through form ... ┃ ## Acceptance ┃ - The new test passes on your current code. ... ┃ Dev auto · GPT-5.6 Terra OpenAI ``` Not one word of it came from the member. ## Why this matters The fallback exists so a lost reply still yields *something*. Here it yields something actively misleading. Three specific harms: 1. **It looks like a report.** It is prose, it is on-topic, and it is long. Nothing in it signals "this is your own text". A lead skimming it can easily conclude the worker refused, stalled, or did nothing. 2. **It hides a successful delivery.** The work was done, committed and pushed. I only found that out because I went and ran `git -C <worktree> log`. A lead that trusted the report would have re-delegated finished work, or written the member off as broken. 3. **It is worse than an empty result.** An empty or explicitly-failed scrape would have sent me straight to the worktree. The echo sent me looking for a problem that did not exist. ## Suggested fix When the fallback scrape is substantially the text that was injected for this turn, do not return it as a report. The daemon already holds the injected prompt for the turn, so the comparison is local and cheap — normalised containment or a similarity ratio over the scrape versus the injected content. On a match, return an explicit outcome instead, something like: ``` [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's worktree and branch for committed work before re-delegating.] ``` That last sentence is the part that would have saved the round trip. Worth considering alongside it: when a member's session has a known worktree and branch, the fallback could name them in that message, since that is exactly where a lead has to look next. ## Not proposing Do not make the fallback go and read the worktree itself — that is the lead's call and a different layer's job. This ticket is only about not presenting the lead's own words back as the member's answer. ## Evidence - Member: profile `terra`, role `dev`, pane `5bd048f0-7795-4c09-9ec3-8faea70c44c7`, worktree `/Users/dai.ha/LTMS/.bridged-worktrees/c9351c-7`, branch `worker/cb201-unit4-outcome-a13bfa-7`. - Work that the report failed to mention: commit `bbf68f3`, pushed to origin, 115 lines of new test. - Observed 2026-09-03 on daemon pid 45674, jar `ecb9645758b3`.
ltms commented 2026-09-03 06:20:58 +02:00
Author
Owner
Copy Link
Copy Source

Merge note to myself — a way this fix could ship dead

Recording this before I forget it, because it is the #234 trap wearing different clothes.

Fleetd.java:393 is one statement:

CompletionResolver completion = new CompletionResolver(agents, rendezvous, exhaustedPatterns, exhaustionSink);

Two in-flight units must both change that same line:

  • #201/#227 Unit 5 adds the real backendErrorPatterns and backendErrorSink.
  • #241 adds the worktreeBranches lookup that lets the no-report message name where the lead should look.

#241 also gives the 6-argument constructor a _ -> null default for that lookup. So if I resolve the merge by keeping Unit 5's call and simply letting it compile, the lookup silently defaults to null, the worktree and branch are never named, and #241's most useful sentence is dead in production while every test passes.

That is exactly what happened in #234: a defaulted parameter, a call site that never passed it, a green suite, and a fix that never ran.

The resolution is the 8-argument constructor, which #241 already added:

new CompletionResolver(agents, rendezvous, exhaustedPatterns, exhaustionSink,
        backendErrorPatterns, backendErrorSink, System::nanoTime, worktreeLookup)

And the check that proves it: after merging, mutate Fleetd.java's call site to drop the lookup argument. A test must go red. If nothing fails, the production wiring is untested and the merge is not done — no matter what the suite says.

Merge order stays: Unit 5 first (it owns Fleetd.java by design and is the larger change), then #241 resolving onto it.

## Merge note to myself — a way this fix could ship dead Recording this before I forget it, because it is the #234 trap wearing different clothes. `Fleetd.java:393` is one statement: ```java CompletionResolver completion = new CompletionResolver(agents, rendezvous, exhaustedPatterns, exhaustionSink); ``` Two in-flight units must both change that same line: - **#201/#227 Unit 5** adds the real `backendErrorPatterns` and `backendErrorSink`. - **#241** adds the `worktreeBranches` lookup that lets the no-report message name where the lead should look. #241 also gives the 6-argument constructor a `_ -> null` default for that lookup. So if I resolve the merge by keeping Unit 5's call and simply letting it compile, **the lookup silently defaults to null, the worktree and branch are never named, and #241's most useful sentence is dead in production while every test passes.** That is exactly what happened in #234: a defaulted parameter, a call site that never passed it, a green suite, and a fix that never ran. **The resolution is the 8-argument constructor**, which #241 already added: ```java new CompletionResolver(agents, rendezvous, exhaustedPatterns, exhaustionSink, backendErrorPatterns, backendErrorSink, System::nanoTime, worktreeLookup) ``` **And the check that proves it:** after merging, mutate `Fleetd.java`'s call site to drop the lookup argument. A test must go red. If nothing fails, the production wiring is untested and the merge is not done — no matter what the suite says. Merge order stays: Unit 5 first (it owns `Fleetd.java` by design and is the larger change), then #241 resolving onto it.
ltms commented 2026-09-03 06:57:14 +02:00
Author
Owner
Copy Link
Copy Source

Fixed and merged to main as 5cf3ca9 (PR #245).

CompletionResolver.echoesInjectedBrief now refuses a scrape that is just the injected brief read back, so a silent member no longer looks like a member that reported.

Round 1 was sent back, and the reason is worth keeping. It used plain containment in both directions. That destroys real reports: a genuine report which quotes its brief contains the brief, so it was classified as an echo and thrown away. I proved it with a probe before sending it back. Round 2 keeps the safe direction unbounded — the brief containing the scrape can only be an echo — and bounds the other direction at MAX_ECHO_EXCESS_CHARS (160), so a scrape counts as an echo only when it adds almost nothing to the brief. ECHO_MIN_CHARS is 400.

Verified by the lead: 1215 tests, 0 failures, 0 compile errors, BUILD SUCCESS.

Attribution: the worker's backend died during round 2 — two turns, no commit, no reply, empty inbox. I took its working diff over, verified it, and committed it with attribution rather than discarding the work.

One thing this merge exposed and did not fix. Fleetd.java's new CompletionResolver(...) was changed by both this ticket and #201/#227 Unit 5, so I resolved the conflict onto the full 8-argument constructor. Then I tried to prove the resolution had kept both features, and could not: dropping this ticket's lookup at the call site leaves all 1215 tests green, with 0 compile errors. The composition root is untested. Filed as #248.

Fixed and merged to `main` as `5cf3ca9` (PR #245). `CompletionResolver.echoesInjectedBrief` now refuses a scrape that is just the injected brief read back, so a silent member no longer looks like a member that reported. **Round 1 was sent back, and the reason is worth keeping.** It used plain containment in both directions. That destroys real reports: a genuine report which quotes its brief *contains* the brief, so it was classified as an echo and thrown away. I proved it with a probe before sending it back. Round 2 keeps the safe direction unbounded — the brief containing the scrape can only be an echo — and bounds the other direction at `MAX_ECHO_EXCESS_CHARS` (160), so a scrape counts as an echo only when it adds almost nothing to the brief. `ECHO_MIN_CHARS` is 400. Verified by the lead: 1215 tests, 0 failures, 0 compile errors, `BUILD SUCCESS`. **Attribution:** the worker's backend died during round 2 — two turns, no commit, no reply, empty inbox. I took its working diff over, verified it, and committed it with attribution rather than discarding the work. **One thing this merge exposed and did not fix.** `Fleetd.java`'s `new CompletionResolver(...)` was changed by both this ticket and #201/#227 Unit 5, so I resolved the conflict onto the full 8-argument constructor. Then I tried to prove the resolution had kept both features, and could not: dropping this ticket's lookup at the call site leaves all 1215 tests green, with 0 compile errors. The composition root is untested. Filed as #248.
ltms closed this issue 2026-09-03 06:57:19 +02:00
ltms commented 2026-09-03 06:57:44 +02:00
Author
Owner
Copy Link
Copy Source

Correction to my comment above: the PR for this work is #243, not #245. #245 is the #134 tool-surface PR. The merge commit 5cf3ca9 carries the same wrong number in its message; it is pushed, and I am not rewriting published history over a wrong reference. This comment is the correction of record.

Everything else in that comment stands — the fix, the round-1 rejection, the 1215-test verification, and the #248 gap.

Also correcting one thing I said there: the worker's backend died during round 2 after it had opened #243, not before. So its work was recoverable from the PR branch, not only from the worktree. That is exactly why every brief now says to commit and push before writing the report.

Correction to my comment above: the PR for this work is **#243**, not #245. #245 is the #134 tool-surface PR. The merge commit `5cf3ca9` carries the same wrong number in its message; it is pushed, and I am not rewriting published history over a wrong reference. This comment is the correction of record. Everything else in that comment stands — the fix, the round-1 rejection, the 1215-test verification, and the #248 gap. Also correcting one thing I said there: the worker's backend died during round 2 **after** it had opened #243, not before. So its work was recoverable from the PR branch, not only from the worktree. That is exactly why every brief now says to commit and push before writing the report.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
blocked
Cannot start until something else lands. The body says what.
 needs-live-proof
Merged and green, but never shown working on the running daemon. Not the same as done.
 ready-to-delegate
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
 silent-default
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent kevin ltms
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#241
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?