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

A worker's report is stranded when its fleet_ask timed out, and the lead is told the worker never replied #307

New Issue
Closed
opened 2026-09-04 08:08:37 +02:00 by ltms · 1 comment
ltms commented 2026-09-04 08:08:37 +02:00
Owner
Copy Link
Copy Source

Found by a delegated hunter. I verified every line of this myself, and the asymmetry is sharper than the report put it.

The two timeouts, and only one is covered

reply() has a recovery path for a reply that arrives with no live rendezvous waiter:

List<Task> candidates = askAnsweredAsyncTasks(session);
if (candidates.size() == 1) { ... complete the ticket directly ... }

and the filter behind it requires a live turnId:

private List<Task> askAnsweredAsyncTasks(String target) {
    ...
    if (target.equals(task.target) && task.question == null && task.turnId != null
            && !task.future.isDone()) {

Now compare what the two timeout paths do to turnId:

what timed out call turnId after recovery works?
answer() — the lead answered, its own bounded wait for the onward reply expired clearAsyncQuestion(turnId, false) kept yes
ask() — the lead never answered at all clearAsyncQuestion(ticket.turnId(), true) nulled no

forgetTurn=true nulls task.turnId and drops the task from asyncTasksByTurn, so the task can never again match the filter. The worker's later fleet_reply falls through to inbox.publish.

The comment above the recovery path names only the first case ("answer()'s own bounded wait … can time out"). The sibling was never considered.

Why this matters more than it looks

The report is not destroyed — it reaches the inbox, strandedReplies is set, and the push loop is nudged. State that plainly.

What breaks is what the lead is told. fleet_poll{ticket} keeps saying PENDING after the worker has replied, and the ticket is eventually forced FAILED by abandon() with the reason "session released before it replied". That reason is false. The lead is not merely left uninformed; it is told the opposite of what happened, and this project already has a standing note that a lead must not believe "the member did nothing" without checking the worktree.

Reachability — this is the common case here, not an edge

An unanswered fleet_ask is routine on this fleet. The ask window is about 55 seconds and is invisible to fleet_poll, which is exactly why the charter says "never brief a worker to ask me — decide before you delegate, or give it an explicit default." When a worker asks anyway and the lead is mid-turn elsewhere, the ask times out, the worker follows the timeout instruction, resumes, finishes, and ends its turn with fleet_reply. That is the ordinary path, and it lands in the broken half of the table above.

So of everything found in this batch, this is the one most likely to have already happened.

The obvious fix is wrong — do not take it

The tempting change is to pass forgetTurn=false on the ask timeout so the turnId survives and the filter matches. That breaks a different thing, and I checked it before writing this:

private boolean hasAsyncQuestion(String target) {
    return asyncTasksByTurn.values().stream().anyMatch(task -> target.equals(task.target));
}

hasAsyncQuestion is what keeps a target BUSY and refuses to open a new waiter on it. Leaving the turnId stamped after a timed-out ask would keep the worker BUSY indefinitely, so every later fleet_send to it would be refused. forgetTurn=true on timeout is deliberate and correct. Leave it alone.

What I want

Goal: after an ask() timeout, a worker's genuine fleet_reply must complete its own async ticket, so fleet_poll{ticket} returns the real report instead of PENDING and then a false FAILED.

Invariants that must survive:

  1. A target whose ask timed out must not stay BUSY. A later fleet_send to it must still be accepted.
  2. The ambiguity guard stays. Where more than one candidate matches, the code must still refuse to guess and fall back to the inbox — completing the wrong ticket hands the lead a plausible answer to a delegation the worker never touched, which is worse than a failure because the lead acts on it. Read the long javadoc above reply() before you touch the filter; it explains why that branch is defence in depth and not dead code.
  3. A reply that arrives with a live rendezvous waiter must keep taking the existing fast path, untouched.

The obvious candidate mechanism — and it is a candidate, not an instruction: mark the Task when its ask times out ("asked, timed out, still awaiting the worker's real reply") and let askAnsweredAsyncTasks accept that marker in place of a live turnId. That keeps the task out of asyncTasksByTurn, so invariant 1 holds, while still distinguishing it from an async task that never asked at all.

Decide it yourself and justify it in your report. If you find a better place or a reason the marker is wrong, say so and do that instead — a reported and tested deviation is a good outcome here, not a problem. What I do not want is the mechanism above implemented without thought because I named it.

Rules

  • Prove it with a test that fails without the fix: an async wait:false send, a fleet_ask that is never answered and times out, then a fleet_reply — and fleet_poll{ticket} must return the reply, not PENDING.
  • Add the negative test too: two open async tasks on one target must still fall back to the inbox rather than guess.
  • Mutation proof required: revert the fix, quote the real failure output, restore it.
  • Do not run git stash — the stash is shared with other worktrees and you will take someone else's work.
  • Run cd fleetd && mvn clean install unpiped and quote the real Tests run: and BUILD lines. Never pipe maven through tail or head, and never read $? after a pipe — it is the pipe's last command's status, not Maven's. I shipped a "green" run earlier today that had not compiled, for exactly that reason.

Shape check

When done, look for the same shape elsewhere in MessageService.java only: a cleanup that runs on one timeout path but not its sibling. One line each, do not fix any of it.

Found by a delegated hunter. **I verified every line of this myself**, and the asymmetry is sharper than the report put it. ## The two timeouts, and only one is covered `reply()` has a recovery path for a reply that arrives with no live rendezvous waiter: ```java List<Task> candidates = askAnsweredAsyncTasks(session); if (candidates.size() == 1) { ... complete the ticket directly ... } ``` and the filter behind it requires a live `turnId`: ```java private List<Task> askAnsweredAsyncTasks(String target) { ... if (target.equals(task.target) && task.question == null && task.turnId != null && !task.future.isDone()) { ``` Now compare what the two timeout paths do to `turnId`: | what timed out | call | `turnId` after | recovery works? | |---|---|---|---| | `answer()` — the lead answered, its own bounded wait for the onward reply expired | `clearAsyncQuestion(turnId, false)` | kept | **yes** | | `ask()` — the lead never answered at all | `clearAsyncQuestion(ticket.turnId(), true)` | **nulled** | **no** | `forgetTurn=true` nulls `task.turnId` and drops the task from `asyncTasksByTurn`, so the task can never again match the filter. The worker's later `fleet_reply` falls through to `inbox.publish`. The comment above the recovery path names only the first case ("answer()'s own bounded wait … can time out"). The sibling was never considered. ## Why this matters more than it looks The report is **not destroyed** — it reaches the inbox, `strandedReplies` is set, and the push loop is nudged. State that plainly. What breaks is what the lead is told. `fleet_poll{ticket}` keeps saying `PENDING` after the worker has replied, and the ticket is eventually forced `FAILED` by `abandon()` with the reason *"session released before it replied"*. That reason is false. The lead is not merely left uninformed; it is told the opposite of what happened, and this project already has a standing note that a lead must not believe "the member did nothing" without checking the worktree. ## Reachability — this is the common case here, not an edge An unanswered `fleet_ask` is routine on this fleet. The ask window is about 55 seconds and is invisible to `fleet_poll`, which is exactly why the charter says *"never brief a worker to ask me — decide before you delegate, or give it an explicit default."* When a worker asks anyway and the lead is mid-turn elsewhere, the ask times out, the worker follows the timeout instruction, resumes, finishes, and ends its turn with `fleet_reply`. That is the ordinary path, and it lands in the broken half of the table above. So of everything found in this batch, this is the one most likely to have already happened. ## The obvious fix is wrong — do not take it The tempting change is to pass `forgetTurn=false` on the ask timeout so the `turnId` survives and the filter matches. **That breaks a different thing**, and I checked it before writing this: ```java private boolean hasAsyncQuestion(String target) { return asyncTasksByTurn.values().stream().anyMatch(task -> target.equals(task.target)); } ``` `hasAsyncQuestion` is what keeps a target BUSY and refuses to open a new waiter on it. Leaving the `turnId` stamped after a timed-out ask would keep the worker BUSY indefinitely, so every later `fleet_send` to it would be refused. `forgetTurn=true` on timeout is deliberate and correct. Leave it alone. ## What I want **Goal:** after an `ask()` timeout, a worker's genuine `fleet_reply` must complete its own async ticket, so `fleet_poll{ticket}` returns the real report instead of `PENDING` and then a false `FAILED`. **Invariants that must survive:** 1. A target whose ask timed out must **not** stay BUSY. A later `fleet_send` to it must still be accepted. 2. The ambiguity guard stays. Where more than one candidate matches, the code must still refuse to guess and fall back to the inbox — completing the *wrong* ticket hands the lead a plausible answer to a delegation the worker never touched, which is worse than a failure because the lead acts on it. Read the long javadoc above `reply()` before you touch the filter; it explains why that branch is defence in depth and not dead code. 3. A reply that arrives with a live rendezvous waiter must keep taking the existing fast path, untouched. **The obvious candidate mechanism** — and it is a candidate, not an instruction: mark the Task when its ask times out ("asked, timed out, still awaiting the worker's real reply") and let `askAnsweredAsyncTasks` accept that marker in place of a live `turnId`. That keeps the task out of `asyncTasksByTurn`, so invariant 1 holds, while still distinguishing it from an async task that never asked at all. **Decide it yourself and justify it in your report.** If you find a better place or a reason the marker is wrong, say so and do that instead — a reported and tested deviation is a good outcome here, not a problem. What I do not want is the mechanism above implemented without thought because I named it. ## Rules - Prove it with a test that fails without the fix: an async `wait:false` send, a `fleet_ask` that is never answered and times out, then a `fleet_reply` — and `fleet_poll{ticket}` must return the reply, not `PENDING`. - Add the negative test too: two open async tasks on one target must still fall back to the inbox rather than guess. - Mutation proof required: revert the fix, quote the real failure output, restore it. - Do not run `git stash` — the stash is shared with other worktrees and you will take someone else's work. - Run `cd fleetd && mvn clean install` unpiped and quote the real `Tests run:` and `BUILD` lines. **Never pipe maven through `tail` or `head`, and never read `$?` after a pipe** — it is the pipe's last command's status, not Maven's. I shipped a "green" run earlier today that had not compiled, for exactly that reason. ## Shape check When done, look for the same shape elsewhere in `MessageService.java` only: **a cleanup that runs on one timeout path but not its sibling.** One line each, do **not** fix any of it.
ltms commented 2026-09-04 08:34:22 +02:00
Author
Owner
Copy Link
Copy Source

Merged to main in b9c2cf6. This is the one in the batch I judged most likely to have already cost us real work.

What I checked myself

The worker did not use the candidate mechanism blindly. It added a second, independent field Task.askTimedOut and left clearAsyncQuestion(turnId, true) alone, so the BUSY release is untouched. That is the right call: loosening forgetTurn would have changed the shared NO_WAITER call site too.

I ran two mutations, both different from the worker's whole-file revert.

D — the ordering. The fix depends on markAskTimedOut running before clearAsyncQuestion erases the turnId it looks up. I swapped the two lines and changed nothing else:

[ERROR] MessageServiceTest.aReplyAfterAnAskTimeoutStillCompletesTheAsyncTicket:988
        expected: <DONE> but was: <PENDING>
[ERROR] FleetMcpTest.unansweredAsyncAskReturnsTheTicketToPendingThenAWorkersLateReplyStillCompletesIt:212
        expected: <finished after timeout> but was: <[pending — worker idle]>

The ordering is load-bearing and pinned at both layers.

E — the ambiguity branch. This is the part I cared about most, because the fix creates the reachable two-candidate state. I made reply() guess (candidates.size() == 1 → !candidates.isEmpty()):

[ERROR] MessageServiceTest.twoAskTimedOutTicketsOnOneTargetFallBackToTheInboxRatherThanGuess:1022
        an ambiguous reply must not guess ticket2 ==> expected: <PENDING> but was: <DONE>

So the new test really does exercise the branch, and the safe fallback is pinned.

The finding beyond the ticket is the valuable half

askAnsweredAsyncTasks' javadoc said its ambiguity branch was unreachable — "returns at most one entry today — verified, not assumed" — and that the ambiguity handling was defence in depth. This fix makes it reachable, exactly as the worker argued: a lapsed ask frees the target, a fresh independent sendAsync can land on it, ask, and lapse too, leaving two open tasks on one target.

The worker rewrote both javadoc blocks to say so and added a test for the state. I confirmed the reasoning by reading the code and by mutation E above. A stale "verified, not assumed" comment is worse than no comment — it is the thing that stops the next reader from checking.

Trade recorded plainly: the fix converts "one stranded ticket, force-failed later with a false 'session released before it replied'" into "the ticket completes; or, in the two-open-tasks case, the reply lands in the inbox and abandon resolves it deterministically". The second case is not perfect, but it never completes the wrong ticket, which was the only outcome worth avoiding at any cost.

Shape check the worker reported

answer()'s TimeoutException catch does not call clearAsyncQuestion the way its success path does. The worker checked it and found it correct — clearAsyncQuestion(turnId, false) already ran earlier in answer(), so turnId stays live on purpose. Reported rather than assumed. Right outcome.

Build

cd fleetd && mvn clean install, unpiped: Tests run: 1323, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS.

Nothing in my ticket was wrong this time

The worker read reply(), askAnsweredAsyncTasks, ask(), answer(), hasAsyncQuestion and abandon() in full and said the ticket's table, trap and three invariants all matched the code. First ticket in this batch where that was true.

Merged to `main` in `b9c2cf6`. This is the one in the batch I judged most likely to have already cost us real work. ## What I checked myself The worker did not use the candidate mechanism blindly. It added a second, independent field `Task.askTimedOut` and left `clearAsyncQuestion(turnId, true)` alone, so the BUSY release is untouched. That is the right call: loosening `forgetTurn` would have changed the shared `NO_WAITER` call site too. I ran two mutations, both different from the worker's whole-file revert. **D — the ordering.** The fix depends on `markAskTimedOut` running *before* `clearAsyncQuestion` erases the `turnId` it looks up. I swapped the two lines and changed nothing else: ``` [ERROR] MessageServiceTest.aReplyAfterAnAskTimeoutStillCompletesTheAsyncTicket:988 expected: <DONE> but was: <PENDING> [ERROR] FleetMcpTest.unansweredAsyncAskReturnsTheTicketToPendingThenAWorkersLateReplyStillCompletesIt:212 expected: <finished after timeout> but was: <[pending — worker idle]> ``` The ordering is load-bearing and pinned at both layers. **E — the ambiguity branch.** This is the part I cared about most, because the fix *creates* the reachable two-candidate state. I made `reply()` guess (`candidates.size() == 1` → `!candidates.isEmpty()`): ``` [ERROR] MessageServiceTest.twoAskTimedOutTicketsOnOneTargetFallBackToTheInboxRatherThanGuess:1022 an ambiguous reply must not guess ticket2 ==> expected: <PENDING> but was: <DONE> ``` So the new test really does exercise the branch, and the safe fallback is pinned. ## The finding beyond the ticket is the valuable half `askAnsweredAsyncTasks`' javadoc said its ambiguity branch was unreachable — "returns at most one entry today — verified, not assumed" — and that the ambiguity handling was defence in depth. **This fix makes it reachable**, exactly as the worker argued: a lapsed ask frees the target, a fresh independent `sendAsync` can land on it, ask, and lapse too, leaving two open tasks on one target. The worker rewrote both javadoc blocks to say so and added a test for the state. I confirmed the reasoning by reading the code and by mutation E above. A stale "verified, not assumed" comment is worse than no comment — it is the thing that stops the next reader from checking. **Trade recorded plainly:** the fix converts "one stranded ticket, force-failed later with a false 'session released before it replied'" into "the ticket completes; or, in the two-open-tasks case, the reply lands in the inbox and `abandon` resolves it deterministically". The second case is not perfect, but it never completes the wrong ticket, which was the only outcome worth avoiding at any cost. ## Shape check the worker reported `answer()`'s `TimeoutException` catch does not call `clearAsyncQuestion` the way its success path does. The worker checked it and found it correct — `clearAsyncQuestion(turnId, false)` already ran earlier in `answer()`, so `turnId` stays live on purpose. Reported rather than assumed. Right outcome. ## Build `cd fleetd && mvn clean install`, unpiped: `Tests run: 1323, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`. ## Nothing in my ticket was wrong this time The worker read `reply()`, `askAnsweredAsyncTasks`, `ask()`, `answer()`, `hasAsyncQuestion` and `abandon()` in full and said the ticket's table, trap and three invariants all matched the code. First ticket in this batch where that was true.
ltms closed this issue 2026-09-04 08:34:28 +02:00
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#307
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?