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

CB-588: the CB-307 push loop never fires for an async ticket — the exact mode the charter tells leads to use #72

New Issue
Closed
opened 2026-08-15 15:59:52 +02:00 by ltms · 1 comment
ltms commented 2026-08-15 15:59:52 +02:00
Owner
Copy Link
Copy Source

Reported by the operator after the lead sat on three finished deliveries without collecting them: "that's why I don't trust your poll". The diagnosis is right, and the fault is structural rather than a lapse of discipline. Polling is not a mechanism; it is the lead remembering.

The gap

MessageService.reply has two paths, and only the second one nudges:

public boolean reply(String session, String content) {
    if (rendezvous.resolve(session, content)) {
        count(BridgedMetrics.REPLIES, "path", "rendezvous");
        return true;                       // a live send took it — unchanged fast path
    }
    inbox.publish(session, UUID.randomUUID().toString(), content);
    count(BridgedMetrics.REPLIES, "path", "inbox");
    if (pushLoop != null) {
        pushLoop.onReplyQueued(session);   // <-- the ONLY nudge
    }
    ...
}

ReplyPushLoop's own javadoc states the scope plainly: it nudges "when a worker reply lands with no live bridge_send to resolve it."

But sendAsync runs the same blocking send on a background virtual thread — see asyncTasksByWaiter, keyed by the CompletableFuture<Rendezvous.Resolution> the send registered. So an async ticket is a live waiter. The worker's reply takes the rendezvous fast path, returns at the first if, and the push loop is never told anything happened.

The ticket flips to a terminal phase and waits, silently, for a bridge_poll that depends entirely on the lead remembering to make it.

Why this is the worst possible case to miss

CLAUDE.md instructs leads to prefer exactly this mode:

prefer wait:false + bridge_poll for anything non-trivial: a blocking bridge_send is capped by your own MCP client call timeout (~60s), well below the task's real runtime.

So the push loop covers the incidental case (nobody waiting) and misses the case the charter makes standard. The busier and better-behaved the lead, the more likely it is to be bitten: a lead that fans out four workers and then does its own work has four silent completions queued behind its attention.

Observed twice on 2026-08-15, both times with three workers finished and reports unread. The second time the operator had to say so.

Compounding factor — the idle reaper

lifecycle.idleTtlSeconds is 1800. A DONE member holds its report only until the reaper takes it, and then the ticket, the pane and the report are gone with no scrape fallback. So a missed nudge is not merely a delay: past 30 minutes it is data loss. On the run that prompted this, one worker was at 1258s of that 1800s budget when the lead finally polled.

Scope

Make an async ticket reaching a terminal phase notify the lead the same way an inbox reply does.

  1. Fire a nudge when an async ticket becomes terminal — DONE and the failure phases. A ticket that failed is more urgent to surface, not less.
  2. Reuse ReplyPushLoop rather than growing a second notifier. It already solves the parts that are actually hard: status gating (only inject when the primary is injectable), bounded reminders with backoff, and metrics. Prefer a new entry point (onTicketTerminal(ticket, target)) over widening onReplyQueued, so the nudge text can name the ticket.
  3. The nudge must name what to run. ReplyPushLoop.NUDGE_FORMAT currently says run bridge_poll(target=%s). For a ticket the correct call is bridge_poll(ticket=...), so the existing string is wrong for this path and must not be reused verbatim.
  4. Coalesce. Four tickets finishing together must not produce four separate injections into the lead's pane — one nudge naming the count, or the existing per-target bounding extended to cover it.
  5. Do not nudge for a ticket the lead has already polled. The trigger is the ticket becoming terminal, not its remaining unread forever.

Acceptance criteria

  1. An async bridge_send{wait:false} whose worker replies produces a nudge in the lead's pane without the lead calling anything first.
  2. The nudge names the ticket and the exact call that collects it.
  3. A ticket that reaches a failure phase nudges too, and its nudge says the outcome was a failure.
  4. Several tickets finishing at once produce one coalesced nudge, not one per ticket.
  5. A ticket already collected produces no nudge.
  6. The existing inbox-path nudge (CB-307) is unchanged — same trigger, same text, same bound. Regression tests for it must still pass untouched.
  7. Nudges stay status-gated and bounded exactly as today: never injected into a pane that is not injectable, and never unbounded.
  8. A fleet with no push loop configured behaves exactly as today.

Note on the silent-default trap

Read issue #50's note before starting. This repo has shipped a feature switched off nine times because a new collaborator got a default so existing wiring still compiled. pushLoop is already nullable in MessageService (if (pushLoop != null)), which is precisely that shape — do not add a second optional dependency beside it. Any new collaborator is required, with an explicit inert value for tests that omits the fact rather than inventing one.

Out of scope

Changing the reaper's TTL, and any change to bridge_ask (issue #61 covers that separately — questions are interactive and must never be queued).

Reported by the operator after the lead sat on three finished deliveries without collecting them: *"that's why I don't trust your poll"*. The diagnosis is right, and the fault is structural rather than a lapse of discipline. Polling is not a mechanism; it is the lead remembering. ## The gap `MessageService.reply` has two paths, and only the second one nudges: ```java public boolean reply(String session, String content) { if (rendezvous.resolve(session, content)) { count(BridgedMetrics.REPLIES, "path", "rendezvous"); return true; // a live send took it — unchanged fast path } inbox.publish(session, UUID.randomUUID().toString(), content); count(BridgedMetrics.REPLIES, "path", "inbox"); if (pushLoop != null) { pushLoop.onReplyQueued(session); // <-- the ONLY nudge } ... } ``` `ReplyPushLoop`'s own javadoc states the scope plainly: it nudges *"when a worker reply lands with **no live `bridge_send`** to resolve it."* But `sendAsync` runs the same blocking `send` on a background virtual thread — see `asyncTasksByWaiter`, keyed by the `CompletableFuture<Rendezvous.Resolution>` the send registered. So an async ticket **is** a live waiter. The worker's reply takes the rendezvous fast path, returns at the first `if`, and the push loop is never told anything happened. The ticket flips to a terminal phase and waits, silently, for a `bridge_poll` that depends entirely on the lead remembering to make it. ## Why this is the worst possible case to miss `CLAUDE.md` instructs leads to prefer exactly this mode: > prefer `wait:false` + `bridge_poll` for anything non-trivial: a blocking `bridge_send` is capped by *your own* MCP client call timeout (~60s), well below the task's real runtime. So the push loop covers the incidental case (nobody waiting) and misses the case the charter makes standard. The busier and better-behaved the lead, the more likely it is to be bitten: a lead that fans out four workers and then does its own work has four silent completions queued behind its attention. Observed twice on 2026-08-15, both times with three workers finished and reports unread. The second time the operator had to say so. ## Compounding factor — the idle reaper `lifecycle.idleTtlSeconds` is 1800. A DONE member holds its report only until the reaper takes it, and then the ticket, the pane and the report are gone with no scrape fallback. So a missed nudge is not merely a delay: past 30 minutes it is data loss. On the run that prompted this, one worker was at 1258s of that 1800s budget when the lead finally polled. ## Scope Make an async ticket reaching a terminal phase notify the lead the same way an inbox reply does. 1. Fire a nudge when an async ticket becomes terminal — DONE **and** the failure phases. A ticket that failed is more urgent to surface, not less. 2. Reuse `ReplyPushLoop` rather than growing a second notifier. It already solves the parts that are actually hard: status gating (only inject when the primary is injectable), bounded reminders with backoff, and metrics. Prefer a new entry point (`onTicketTerminal(ticket, target)`) over widening `onReplyQueued`, so the nudge text can name the ticket. 3. The nudge must name **what to run**. `ReplyPushLoop.NUDGE_FORMAT` currently says `run bridge_poll(target=%s)`. For a ticket the correct call is `bridge_poll(ticket=...)`, so the existing string is wrong for this path and must not be reused verbatim. 4. Coalesce. Four tickets finishing together must not produce four separate injections into the lead's pane — one nudge naming the count, or the existing per-target bounding extended to cover it. 5. Do not nudge for a ticket the lead has already polled. The trigger is the ticket becoming terminal, not its remaining unread forever. ## Acceptance criteria 1. An async `bridge_send{wait:false}` whose worker replies produces a nudge in the lead's pane without the lead calling anything first. 2. The nudge names the ticket and the exact call that collects it. 3. A ticket that reaches a **failure** phase nudges too, and its nudge says the outcome was a failure. 4. Several tickets finishing at once produce one coalesced nudge, not one per ticket. 5. A ticket already collected produces no nudge. 6. The existing inbox-path nudge (CB-307) is unchanged — same trigger, same text, same bound. Regression tests for it must still pass untouched. 7. Nudges stay status-gated and bounded exactly as today: never injected into a pane that is not injectable, and never unbounded. 8. A fleet with no push loop configured behaves exactly as today. ## Note on the silent-default trap Read issue #50's note before starting. This repo has shipped a feature switched off **nine** times because a new collaborator got a default so existing wiring still compiled. `pushLoop` is already nullable in `MessageService` (`if (pushLoop != null)`), which is precisely that shape — do not add a second optional dependency beside it. Any new collaborator is required, with an explicit inert value for tests that omits the fact rather than inventing one. ## Out of scope Changing the reaper's TTL, and any change to `bridge_ask` (issue #61 covers that separately — questions are interactive and must never be queued).
ltms added the ready-to-delegate label 2026-08-15 16:00:03 +02:00
ltms commented 2026-08-15 17:07:54 +02:00
Author
Owner
Copy Link
Copy Source

Merged to main as 5206679 (PR #73, head ac044e7). Verified on my own unpiped
mvn clean install: 802 tests, 0 failures, BUILD SUCCESS.

Three defects were found in review and fixed before merge — the middle one matters most:

  1. A pendingTickets entry outlived the ticket it named. poll() returns null at the top once
    pruneTerminalTickets drops a ticket, so ticketCollected was never reached. The entry leaked for
    the daemon's whole life and rode along on every later nudge to that lead, sending it after a ticket
    bridge_poll can no longer find. Fixed by reclaiming on prune, so an entry lives exactly as long as
    the ticket is pollable.
  2. A lost nudge. A ticket landing between decideTickets returning STOP and activeLeads.remove
    coalesced onto a schedule that was about to die, and no nudge was ever scheduled. That is the exact
    failure this ticket exists to remove, reintroduced in a narrow window.
  3. Success direction unpinned in tests, and the comment listing the paths that complete the future was
    short by several (it omitted the timeout/BUSY/BACKEND_EXHAUSTED outcomes, completeExceptionally,
    and the answer() path).

The obvious fix for #2 was wrong, and the worker caught it rather than following the brief.
Restarting the schedule on any pending ticket defeats the reminder cap: when STOP is reached because
the cap was hit rather than the backlog draining, the never-collected ticket is expected to still be
sitting there, so an unconditional restart nudges forever. Two existing tests failed on that version.
The merged fix diffs the pending set against a snapshot taken just before the decision, so only a
ticket that genuinely arrived during the window restarts the schedule.

Not fixed here, split out deliberately: #75 (CB-590) — activeTargets and activeLeads do not
gate each other, so two loops can inject into one lead pane. Narrow, because both gate on
status.injectable(), but real.

Still true and worth recording: a ticket that overruns ASYNC_TIMEOUT_MS (30 min) goes FAILED, and the
worker's eventual reply then has no rendezvous waiter — so it falls through to the durable inbox and is
covered by the CB-307 path instead, collected with bridge_poll{target} rather than by ticket.
Both modes are now covered, by different mechanisms.

Merged to `main` as `5206679` (PR #73, head `ac044e7`). Verified on my own unpiped `mvn clean install`: **802 tests, 0 failures, BUILD SUCCESS**. Three defects were found in review and fixed before merge — the middle one matters most: 1. **A `pendingTickets` entry outlived the ticket it named.** `poll()` returns `null` at the top once `pruneTerminalTickets` drops a ticket, so `ticketCollected` was never reached. The entry leaked for the daemon's whole life and rode along on every later nudge to that lead, sending it after a ticket `bridge_poll` can no longer find. Fixed by reclaiming on prune, so an entry lives exactly as long as the ticket is pollable. 2. **A lost nudge.** A ticket landing between `decideTickets` returning STOP and `activeLeads.remove` coalesced onto a schedule that was about to die, and no nudge was ever scheduled. That is the exact failure this ticket exists to remove, reintroduced in a narrow window. 3. Success direction unpinned in tests, and the comment listing the paths that complete the future was short by several (it omitted the timeout/BUSY/BACKEND_EXHAUSTED outcomes, `completeExceptionally`, and the `answer()` path). **The obvious fix for #2 was wrong, and the worker caught it rather than following the brief.** Restarting the schedule on any pending ticket defeats the reminder cap: when STOP is reached because the cap was hit rather than the backlog draining, the never-collected ticket is *expected* to still be sitting there, so an unconditional restart nudges forever. Two existing tests failed on that version. The merged fix diffs the pending set against a snapshot taken just before the decision, so only a ticket that genuinely arrived during the window restarts the schedule. Not fixed here, split out deliberately: **#75 (CB-590)** — `activeTargets` and `activeLeads` do not gate each other, so two loops can inject into one lead pane. Narrow, because both gate on `status.injectable()`, but real. Still true and worth recording: a ticket that overruns `ASYNC_TIMEOUT_MS` (30 min) goes FAILED, and the worker's eventual reply then has no rendezvous waiter — so it falls through to the durable inbox and is covered by the **CB-307** path instead, collected with `bridge_poll{target}` rather than by ticket. Both modes are now covered, by different mechanisms.
ltms closed this issue 2026-08-15 17:07:59 +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 ready-to-delegate
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#72
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?