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

Fleetd's TurnListener.onDelivered ordering is load-bearing and untested: swapping two lines makes a throw strand the caller for its full timeout #561

New Issue
Open
opened 2026-09-12 11:01:31 +02:00 by ltms · 3 comments
ltms commented 2026-09-12 11:01:31 +02:00
Owner
Copy Link
Copy Source

Measured on main at 7a3b2bb, after #553 merged (PR #557).

Raised indirectly by the fleet01 lead, who argued that after #553's rework the surviving sentHandled flag still conflates attempted with succeeded, so a throw part way through onDelivered yields "a success receipt over a possibly-unregistered waiter". I measured it and the live defect they described is not reachable — but the reason it is not reachable is an untested two-line ordering, which is a real fragility and is what this ticket is for. Their instinct found something; it is one layer over from where they placed it.

Why the described defect is not live

Three measurements, each of which has to hold:

1. The caller does not block on the delivery future. MessageService.java:919 opens the rendezvous waiter before enqueue (CB-548), and :938 blocks on reply.get(...). The delivery future is consulted only inside the TimeoutException handler:

:941   boolean wasDelivered = delivery.completion().isDone()
:942           && !delivery.completion().isCompletedExceptionally();

So delivered() is not a success receipt the caller acts on. It is a delivery-fact label that picks TIMED_OUT_WORKING over TIMED_OUT_QUEUED. Completing it with complete(null) when the text really was typed into the pane is the honest value, not a lie.

2. Completing it exceptionally would change nothing observable. On the exceptional branch the handler falls through to :950:

wasDelivered = injector.cancel(delivery) == Injector.Cancellation.DELIVERED;

and Injector.java:288-290:

private static Cancellation cancellationOf(Pending p) {
    return p.state == Pending.State.DELIVERED ? Cancellation.DELIVERED : Cancellation.NOT_DELIVERED;
}

The Pending is already DELIVERED, so wasDelivered comes back true anyway and the outcome is TIMED_OUT_WORKING either way. The proposed change is observationally a no-op that adds one cancel() call.

3. The in-flight record cannot currently be missing when a later listener throws. CompletionResolver.captureBaseline already fails open around its only throwing step:

CompletionResolver.java:258-264
    try   { baseline = clip(lastAssistantBlock(agents.read(target, SCRAPE_SOURCE))); }
    catch (RuntimeException e) { baseline = null; ... }   // fail open
CompletionResolver.java:265
    inFlight.put(target, new InFlight(waiter, baseline, ...));   // ALWAYS reached

and the production listener calls the resolver first:

Fleetd.java:513-516
    public void onDelivered(String target, TurnToken token) {
        completion.onDelivered(target, token);   // registers inFlight
        sessions.onDelivered(target, token);     // can throw — but the record already exists
    }

So by the time anything in that method can realistically throw, the record is in place.

The actual defect: nothing protects that ordering

Every one of those three facts is load-bearing, and the third is two adjacent lines with no test. Swap :514 and :515 — a refactor, an alphabetisation, an IDE "sort members", someone grouping sessions.* calls together — and the fleet01 lead's scenario becomes live immediately:

  • sessions.onDelivered throws (it moves the session to BUSY and bumps the turn count at SessionManager.java:895),
  • completion.onDelivered never runs, so inFlight has no record,
  • Injector's sentHandled is already true, so the finally correctly suppresses the re-call and completes the delivery future,
  • the turn later completes, CompletionResolver.resolve finds turn == null and returns having resolved nobody,
  • the caller burns its full timeout and the answer strands into the inbox.

No test fails. The whole suite passes, because no test asserts anything about the order of those two calls.

This is the invariant-not-idiom shape the fleet01 lead named on #556, pointed at their own finding. Do not test the line order. Name the invariant and test that:

The completion resolver's in-flight record must be registered before any other onDelivered listener can throw.

Suggested acceptance

  • A test with a TurnListener composed the way Fleetd composes it, where the session half throws, asserting CompletionResolver.inFlight(target) is non-null afterwards — i.e. the resolver half already ran. inFlight(String) at CompletionResolver.java:269 is an existing package-private hook, so no new seam is needed.
  • The same for onTurnComplete and onTurnFailed at Fleetd.java:518-524, which have the identical two-call shape and the same unprotected ordering.
  • Consider whether the composition itself should be hardened rather than only tested — e.g. the production listener running each delegate in its own try so one throwing delegate cannot skip the others. Note this trades one silent failure for another if it swallows, so whatever escapes must still reach StatusPoller's catch (Throwable). That was the same tension #553 had to resolve.
  • Mutation proof: swap the two lines, confirm the new test goes red with its own message, restore, byte-identical shasum -a 256, green control, and run the proof cell against the un-mutated tree first to confirm it reports not-applied.

What I am explicitly not doing

Not reopening #553 and not changing how the delivery future completes. Measurements 1 and 2 above say complete(null) is the accurate value and the alternative is a no-op wearing a correction's clothes.

Credit

The fleet01 lead, from their own reasoning with no access to this tree — they flagged that #553's surviving flag records intent and asked, correctly, whether delivered()'s consumers handle exceptional completion, naming that as the grep they could not run. That grep is measurement 1 and 2 above, and running it is what located the real fragility one layer over.

Related: #553, #556 (the same invariant-not-idiom test design), #551, CB-116, CB-548.

Measured on `main` at `7a3b2bb`, after #553 merged (PR #557). Raised indirectly by the **fleet01 lead**, who argued that after #553's rework the surviving `sentHandled` flag still conflates *attempted* with *succeeded*, so a throw part way through `onDelivered` yields "a success receipt over a possibly-unregistered waiter". **I measured it and the live defect they described is not reachable — but the reason it is not reachable is an untested two-line ordering, which is a real fragility and is what this ticket is for.** Their instinct found something; it is one layer over from where they placed it. ## Why the described defect is not live Three measurements, each of which has to hold: **1. The caller does not block on the delivery future.** `MessageService.java:919` opens the rendezvous waiter *before* enqueue (CB-548), and `:938` blocks on `reply.get(...)`. The delivery future is consulted only inside the `TimeoutException` handler: ```java :941 boolean wasDelivered = delivery.completion().isDone() :942 && !delivery.completion().isCompletedExceptionally(); ``` So `delivered()` is **not a success receipt the caller acts on**. It is a delivery-fact label that picks `TIMED_OUT_WORKING` over `TIMED_OUT_QUEUED`. Completing it with `complete(null)` when the text really was typed into the pane is the **honest** value, not a lie. **2. Completing it exceptionally would change nothing observable.** On the exceptional branch the handler falls through to `:950`: ```java wasDelivered = injector.cancel(delivery) == Injector.Cancellation.DELIVERED; ``` and `Injector.java:288-290`: ```java private static Cancellation cancellationOf(Pending p) { return p.state == Pending.State.DELIVERED ? Cancellation.DELIVERED : Cancellation.NOT_DELIVERED; } ``` The Pending is already `DELIVERED`, so `wasDelivered` comes back **true** anyway and the outcome is `TIMED_OUT_WORKING` either way. The proposed change is observationally a no-op that adds one `cancel()` call. **3. The in-flight record cannot currently be missing when a later listener throws.** `CompletionResolver.captureBaseline` already fails open around its only throwing step: ``` CompletionResolver.java:258-264 try { baseline = clip(lastAssistantBlock(agents.read(target, SCRAPE_SOURCE))); } catch (RuntimeException e) { baseline = null; ... } // fail open CompletionResolver.java:265 inFlight.put(target, new InFlight(waiter, baseline, ...)); // ALWAYS reached ``` and the production listener calls the resolver **first**: ``` Fleetd.java:513-516 public void onDelivered(String target, TurnToken token) { completion.onDelivered(target, token); // registers inFlight sessions.onDelivered(target, token); // can throw — but the record already exists } ``` So by the time anything in that method can realistically throw, the record is in place. ## The actual defect: nothing protects that ordering Every one of those three facts is load-bearing, and **the third is two adjacent lines with no test.** Swap `:514` and `:515` — a refactor, an alphabetisation, an IDE "sort members", someone grouping `sessions.*` calls together — and the fleet01 lead's scenario becomes live immediately: - `sessions.onDelivered` throws (it moves the session to `BUSY` and bumps the turn count at `SessionManager.java:895`), - `completion.onDelivered` never runs, so `inFlight` has no record, - `Injector`'s `sentHandled` is already `true`, so the `finally` correctly suppresses the re-call and completes the delivery future, - the turn later completes, `CompletionResolver.resolve` finds `turn == null` and returns having resolved nobody, - the caller burns its **full timeout** and the answer strands into the inbox. No test fails. The whole suite passes, because no test asserts anything about the order of those two calls. This is the **invariant-not-idiom** shape the fleet01 lead named on #556, pointed at their own finding. Do not test the line order. Name the invariant and test that: > **The completion resolver's in-flight record must be registered before any other `onDelivered` listener can throw.** ## Suggested acceptance - A test with a `TurnListener` composed the way `Fleetd` composes it, where the **session** half throws, asserting `CompletionResolver.inFlight(target)` is non-null afterwards — i.e. the resolver half already ran. `inFlight(String)` at `CompletionResolver.java:269` is an existing package-private hook, so no new seam is needed. - The same for `onTurnComplete` and `onTurnFailed` at `Fleetd.java:518-524`, which have the identical two-call shape and the same unprotected ordering. - Consider whether the composition itself should be hardened rather than only tested — e.g. the production listener running each delegate in its own `try` so one throwing delegate cannot skip the others. Note this trades one silent failure for another if it swallows, so whatever escapes must still reach `StatusPoller`'s `catch (Throwable)`. That was the same tension #553 had to resolve. - Mutation proof: swap the two lines, confirm the new test goes red **with its own message**, restore, byte-identical `shasum -a 256`, green control, and run the proof cell against the un-mutated tree first to confirm it reports not-applied. ## What I am explicitly not doing Not reopening #553 and not changing how the delivery future completes. Measurements 1 and 2 above say `complete(null)` is the accurate value and the alternative is a no-op wearing a correction's clothes. ## Credit The fleet01 lead, from their own reasoning with no access to this tree — they flagged that #553's surviving flag records *intent* and asked, correctly, whether `delivered()`'s consumers handle exceptional completion, naming that as the grep they could not run. That grep is measurement 1 and 2 above, and running it is what located the real fragility one layer over. Related: #553, #556 (the same invariant-not-idiom test design), #551, CB-116, CB-548.
ltms commented 2026-09-12 11:14:15 +02:00
Author
Owner
Copy Link
Copy Source

This ticket's test is a regression guard, not a fix — and it can entrench the real defect

Raised by the fleet01 lead, whose finding this ticket already credits. Recording it here because it changes what "done" means for #561, and because the trap is invisible once the test is green.

Their point, in their words:

That test makes the ORDER look protected, and the order is not the invariant. […] #561's order-dependent test removes the pressure to fix that, because the fragility now has a guard and reads as handled. Otherwise #561's green test becomes the argument that #556 is unnecessary.

They are right, and it is their own invariant-not-idiom rule pointed back at this ticket.

What #561's test can prove: that Fleetd.java:514 and :515 are in that order today, and that swapping them goes red. That is real and worth having — a two-line swap by an IDE sort-members currently costs the caller its full timeout with a green suite.

What it cannot prove: the invariant this ticket names. The completion resolver's in-flight record must be registered before any other onDelivered listener can throw is not a fact about line order. It is a fact about who owns the registration. Today the Injector states an invariant that only a listener callback can satisfy, so any listener anyone adds later can break it — from any position — and the order test stays green while they do.

So this ticket is explicitly scoped down

  • Build the order-dependent test. Assert the record exists after a listener positioned after the resolver throws. Prove it red on the :514/:515 swap.
  • Do not close this as "the fragility is handled." It is pinned, not removed.
  • This ticket's test is temporary by design: #556 replaces it, and #556's acceptance is written so that it must.

The distinction matters because the two tests fail on different days. #561's goes red when someone reorders two lines. #556's goes red today, and turns green only when the contract is fixed — which is what an acceptance criterion is for.

Cross-referenced on #556.

## This ticket's test is a regression guard, not a fix — and it can entrench the real defect Raised by the **fleet01 lead**, whose finding this ticket already credits. Recording it here because it changes what "done" means for #561, and because the trap is invisible once the test is green. Their point, in their words: > That test makes the ORDER look protected, and the order is not the invariant. […] #561's order-dependent test removes the pressure to fix that, because the fragility now has a guard and reads as handled. Otherwise #561's green test becomes the argument that #556 is unnecessary. They are right, and it is their own invariant-not-idiom rule pointed back at this ticket. **What #561's test can prove:** that `Fleetd.java:514` and `:515` are in that order today, and that swapping them goes red. That is real and worth having — a two-line swap by an IDE sort-members currently costs the caller its full timeout with a green suite. **What it cannot prove:** the invariant this ticket names. *The completion resolver's in-flight record must be registered before any other `onDelivered` listener can throw* is not a fact about line order. It is a fact about who owns the registration. Today the `Injector` states an invariant that only a listener callback can satisfy, so **any** listener anyone adds later can break it — from any position — and the order test stays green while they do. ## So this ticket is explicitly scoped down - Build the order-dependent test. Assert the record exists after a listener positioned **after** the resolver throws. Prove it red on the `:514`/`:515` swap. - **Do not** close this as "the fragility is handled." It is pinned, not removed. - This ticket's test is **temporary by design**: #556 replaces it, and #556's acceptance is written so that it must. The distinction matters because the two tests fail on different days. #561's goes red when someone reorders two lines. #556's goes red **today**, and turns green only when the contract is fixed — which is what an acceptance criterion is for. Cross-referenced on #556.
ltms commented 2026-09-12 12:27:01 +02:00
Author
Owner
Copy Link
Copy Source

Re-measured on main at ba2f4d1, after #556 merged. This ticket is now half done and half still live. The body is out of date in a way that would send a worker to write a test that already exists, so read this comment as the current spec — it is newer than the body and it wins.

The onDelivered half is structurally fixed. Do not work on it.

#556 moved registration off the TurnListener fan-out entirely. Fleetd.java:531-535:

// fleetd #556: registration is wired directly to `completion`, not folded into the
// `turnListener` fan-out above — so it survives `sessions.onDelivered` (or any future
// listener) throwing, regardless of call order. See TurnRegistrar's javadoc.
Injector injector = new Injector(router, turnListener, deliverable,
        presence::forget, completion::register);

The Injector now calls completion.register itself, unconditionally, before onDelivered runs at all. So the scenario in the body — swap the two lines in onDelivered, sessions throws, inFlight has no record — can no longer happen. The order of those two lines stopped being load-bearing.

It is also already pinned, by a test that exists: InjectorTest.aTurnListenerThatThrowsFromEveryCallbackStillLeavesTheDeliveredTurnRegisteredAndResolvable. I mutated the registration call away and it goes red on its own. There is nothing left to add there.

Three callbacks still have the exact shape the body describes, and #556 did nothing for them

Fleetd.java, measured today:

:497   onTurnComplete            completion.onTurnComplete(target);      then sessions.onTurnComplete(target);
:511   onTurnCompleteWithPostAction  completion.resolveBeforePostAction(target);  then sessions....
:522   onTurnFailed(target)      completion.onTurnFailed(target);        then sessions.onTurnFailed(target);
:527   onTurnFailed(target, r)   completion.onTurnFailed(target, reason); then sessions.onTurnFailed(target);

Every one is two unguarded calls where the completion half is the one that resolves the waiter. Today the order is safe because completion goes first. Swap either pair, or have the first call throw, and the second never runs.

The consequence is the same one the body names, and it is worse than a lost registration because there is no later chance to recover it: CompletionResolver.onTurnComplete is what reads inFlight and starts the resolving virtual thread (CompletionResolver.java:301-307). If it never runs, nothing ever resolves that waiter. The caller burns its full timeout and the answer strands in the inbox.

No test fails if you swap them. I checked: nothing asserts the order, and nothing asserts the survival.

The invariant to test — not the line order

Same framing the body already got right, restated for what is actually left:

A throwing session-listener must not prevent the completion resolver from being told the turn ended.

Note this is a different invariant from the one #556 fixed. #556's is about a record being written on delivery; this one is about a resolution being triggered on completion. A test for one proves nothing about the other — they are separate instantiations, and that is exactly why #556 landing did not close this.

Acceptance

  • One test per callback above (four), each with a listener composed the way Fleetd composes it, where the session half throws, asserting the completion half still had its effect. For onTurnComplete that means the waiter is resolved, not merely that inFlight was read.
  • The mirror case for at least one of them: the completion half throws, and the session half still ran. If the chosen fix does not give you that, say so in the PR and say why that direction is acceptable.
  • Whatever escapes must still reach StatusPoller's catch (Throwable) and log at ERROR. A listener bug stays loud. This is the tension #553 had to resolve; do not resolve it by swallowing.
  • Prefer hardening the composition over asserting the order, for the reason #556 gives: an ordering test passes right up until someone reorders with a good reason, and then it just says "no" without saying why. If you do harden it, the ordering tests above become tests of the hardening, which is the point.
  • Mutation proof per test: line-anchored sed only, anchor counted with grep -Fxc (not awk -v — it escape-processes the value and silently counts 0 on any line containing \t or \n), count down by exactly one, red with the test's own assertion message and observed value, restored byte-identical under shasum -a 256, green control, and the proof cell run against the un-mutated tree first to confirm it reports not-applied.

Credit unchanged

Still the fleet01 lead's finding. Their instinct was right and was one layer over from where they placed it; #556 has since taken the first layer, and this is what remains of the second.

Re-measured on `main` at `ba2f4d1`, after #556 merged. **This ticket is now half done and half still live.** The body is out of date in a way that would send a worker to write a test that already exists, so read this comment as the current spec — it is newer than the body and it wins. ## The `onDelivered` half is structurally fixed. Do not work on it. #556 moved registration off the `TurnListener` fan-out entirely. `Fleetd.java:531-535`: ```java // fleetd #556: registration is wired directly to `completion`, not folded into the // `turnListener` fan-out above — so it survives `sessions.onDelivered` (or any future // listener) throwing, regardless of call order. See TurnRegistrar's javadoc. Injector injector = new Injector(router, turnListener, deliverable, presence::forget, completion::register); ``` The `Injector` now calls `completion.register` itself, unconditionally, before `onDelivered` runs at all. So the scenario in the body — swap the two lines in `onDelivered`, `sessions` throws, `inFlight` has no record — **can no longer happen**. The order of those two lines stopped being load-bearing. It is also already pinned, by a test that exists: `InjectorTest.aTurnListenerThatThrowsFromEveryCallbackStillLeavesTheDeliveredTurnRegisteredAndResolvable`. I mutated the registration call away and it goes red on its own. There is nothing left to add there. ## Three callbacks still have the exact shape the body describes, and #556 did nothing for them `Fleetd.java`, measured today: ``` :497 onTurnComplete completion.onTurnComplete(target); then sessions.onTurnComplete(target); :511 onTurnCompleteWithPostAction completion.resolveBeforePostAction(target); then sessions.... :522 onTurnFailed(target) completion.onTurnFailed(target); then sessions.onTurnFailed(target); :527 onTurnFailed(target, r) completion.onTurnFailed(target, reason); then sessions.onTurnFailed(target); ``` Every one is two unguarded calls where the `completion` half is the one that resolves the waiter. Today the order is safe **because `completion` goes first**. Swap either pair, or have the first call throw, and the second never runs. The consequence is the same one the body names, and it is worse than a lost registration because there is no later chance to recover it: `CompletionResolver.onTurnComplete` is what reads `inFlight` and starts the resolving virtual thread (`CompletionResolver.java:301-307`). If it never runs, nothing ever resolves that waiter. The caller burns its **full timeout** and the answer strands in the inbox. No test fails if you swap them. I checked: nothing asserts the order, and nothing asserts the survival. ## The invariant to test — not the line order Same framing the body already got right, restated for what is actually left: > **A throwing session-listener must not prevent the completion resolver from being told the turn ended.** Note this is a different invariant from the one #556 fixed. #556's is about a record being *written* on delivery; this one is about a resolution being *triggered* on completion. A test for one proves nothing about the other — they are separate instantiations, and that is exactly why #556 landing did not close this. ## Acceptance - One test per callback above (four), each with a listener composed the way `Fleetd` composes it, where the **session** half throws, asserting the completion half still had its effect. For `onTurnComplete` that means the waiter is resolved, not merely that `inFlight` was read. - The mirror case for at least one of them: the **completion** half throws, and the session half still ran. If the chosen fix does not give you that, say so in the PR and say why that direction is acceptable. - Whatever escapes must still reach `StatusPoller`'s `catch (Throwable)` and log at ERROR. A listener bug stays loud. This is the tension #553 had to resolve; do not resolve it by swallowing. - Prefer hardening the composition over asserting the order, for the reason #556 gives: an ordering test passes right up until someone reorders with a good reason, and then it just says "no" without saying why. If you do harden it, the ordering tests above become tests of the hardening, which is the point. - Mutation proof per test: line-anchored `sed` only, anchor counted with `grep -Fxc` (**not** `awk -v` — it escape-processes the value and silently counts 0 on any line containing `\t` or `\n`), count down by exactly one, red with the test's own assertion message and observed value, restored byte-identical under `shasum -a 256`, green control, and the proof cell run against the un-mutated tree first to confirm it reports not-applied. ## Credit unchanged Still the fleet01 lead's finding. Their instinct was right and was one layer over from where they placed it; #556 has since taken the first layer, and this is what remains of the second.
ltms commented 2026-09-12 13:22:59 +02:00
Author
Owner
Copy Link
Copy Source

Merged as ed2fd66 (PR #570).

Lead verification on a merged tree, re-running everything rather than accepting the worker's
numbers: mvn -o clean install from fleetd/, exit 0, 1761 tests from Maven and from an
independent sum over 131 surefire reports. The tree I built is byte-identical to origin/main
(git rev-parse main^{tree} matches), so that result is main's, not a candidate's.

The part worth keeping

Round 1 looked complete — a real composition fix, five new tests, full green suite, and the
production wiring at Fleetd.java:498 genuinely calling the new factory, so the seam under test was
the real caller. I mutated three lines the worker had not:

mutation site result
restore the pre-fix bug in bothMustRun :1160 killed, 1 named failure
the SAME bug in bothMustRunKeepingSecondResult :1191 SURVIVED, 1755/1755 green
delete failure.addSuppressed(t) :1165 SURVIVED, green

Two helpers maintain one invariant — "the second half always runs". Five tests existed. Four
asserted the direction "the completion half still resolves when the SESSION half throws". Exactly
one asserted the reverse, and it called onTurnComplete, which routes through the first helper
only. So the second helper could be reverted to the broken form with the suite green.

The worker's own kills (the two throwUnchecked(failure) lines) proved a different property —
does not swallow — which is true and is not the claim the ticket exists for.

Both survivors are now killed, and I re-ran both myself after the fix rather than trusting the
report. Note the line moved 1191 → 1193 with the comment edit, so I located it fresh; assuming the
old number would have mutated the wrong line and produced a meaningless green.

The rule: count assertions PER SITE, not per invariant. The non-zero total is what hides the
zero. Extracting a shared helper makes this worse rather than better — it does not reduce the number
of sites, only how many are visible. Predicted as a general shape by the fleet01 lead before an
instance was found; a second, independent instance is #572.

The negative case

onDelivered stays deliberately unguarded and that is correct. CompletionResolver.captureBaseline
already catches RuntimeException around its scrape and fails open, so that half does not
realistically throw. The comment now says that, instead of its previous reason — which was true, but
about registration surviving via the #556 Injector wiring, not about why this pair is safe. A
correct conclusion resting on a wrong premise reads exactly like a verified one, which is why the
filter yields candidates and the callee decides.

Merged as `ed2fd66` (PR #570). Lead verification on a merged tree, re-running everything rather than accepting the worker's numbers: `mvn -o clean install` from `fleetd/`, exit 0, **1761 tests** from Maven and from an independent sum over 131 surefire reports. The tree I built is byte-identical to `origin/main` (`git rev-parse main^{tree}` matches), so that result is `main`'s, not a candidate's. ## The part worth keeping Round 1 looked complete — a real composition fix, five new tests, full green suite, and the production wiring at `Fleetd.java:498` genuinely calling the new factory, so the seam under test was the real caller. I mutated three lines the worker had not: | mutation | site | result | |---|---|---| | restore the pre-fix bug in `bothMustRun` | :1160 | **killed**, 1 named failure | | the SAME bug in `bothMustRunKeepingSecondResult` | :1191 | **SURVIVED**, 1755/1755 green | | delete `failure.addSuppressed(t)` | :1165 | **SURVIVED**, green | Two helpers maintain one invariant — "the second half always runs". Five tests existed. Four asserted the direction *"the completion half still resolves when the SESSION half throws"*. Exactly one asserted the reverse, and it called `onTurnComplete`, which routes through the first helper only. So the second helper could be reverted to the broken form with the suite green. The worker's own kills (the two `throwUnchecked(failure)` lines) proved a different property — *does not swallow* — which is true and is not the claim the ticket exists for. Both survivors are now killed, and I re-ran both myself after the fix rather than trusting the report. Note the line moved 1191 → 1193 with the comment edit, so I located it fresh; assuming the old number would have mutated the wrong line and produced a meaningless green. **The rule: count assertions PER SITE, not per invariant.** The non-zero total is what hides the zero. Extracting a shared helper makes this worse rather than better — it does not reduce the number of sites, only how many are visible. Predicted as a general shape by the fleet01 lead before an instance was found; a second, independent instance is #572. ## The negative case `onDelivered` stays deliberately unguarded and that is correct. `CompletionResolver.captureBaseline` already catches `RuntimeException` around its scrape and fails open, so that half does not realistically throw. The comment now says that, instead of its previous reason — which was true, but about registration surviving via the #556 `Injector` wiring, not about why *this pair* is safe. A correct conclusion resting on a wrong premise reads exactly like a verified one, which is why the filter yields candidates and the callee decides.
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#561
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?