fleetd #561: harden the completion/session TurnListener fan-out #570

Merged

ltms merged 2 commits from worker/561-listener-fanout-survives-a-throw-61d538-2 into main 2026-09-12 13:20:56 +02:00

Conversation 0 Commits 2 Files Changed 2

+442 -36

agent commented 2026-09-12 12:48:23 +02:00

Member

Copy Link

Copy Source

Fixes fleetd #561.

What changed

Fleetd.java's TurnListener fan-out (the composed listener that hands each
lifecycle event to both CompletionResolver and SessionManager) had four
callbacks — onTurnComplete, onTurnCompleteWithPostAction, and both
onTurnFailed overloads — built from two bare, unguarded statements each.
onDelivered's registration hazard was already fixed structurally by #556
(moved off the fan-out entirely); these four callbacks have the identical
shape and were still untested per comment 17041 on #561 (which supersedes the
ticket body — the body's onDelivered example is already fixed and already
pinned by InjectorTest, not touched here).

Extracted the composition into a package-private static factory,
Fleetd.turnListener(CompletionResolver completion, TurnListener sessions),
and hardened it (rather than only asserting call order) with two small
helpers: bothMustRun and bothMustRunKeepingSecondResult. Both callback
halves are always attempted regardless of whether the other one throws;
whatever escapes (from one half or both) is rethrown once both have run —
never swallowed — so it still reaches StatusPoller's catch (Throwable)
and logs at ERROR.

onTurnCompleteWithPostAction is documented as the one exception: order
there is a functional requirement (completion.resolveBeforePostAction must
run before the session half's context-reset housekeeping can erase the
pane), not just fault tolerance, so it is not reorder-symmetric like the
other three — see the javadoc on Fleetd.turnListener.

Tests

New FleetdTurnListenerCompositionTest (5 tests) builds the real production
composition from a real CompletionResolver and a throwing fake sessions
half:

• one test per callback (4): the session half throws, and the completion
  half's effect (the captured Rendezvous waiter resolving/failing) still
  happened;
• one mirror test: the completion half throws synchronously (forced via
  ConcurrentHashMap.get(null), a real code path, not a contrived one — the
  production CompletionResolver is otherwise too defensive to throw
  synchronously in normal operation), and the session half still ran.

Mutation proof

• Two "kill" mutations (swallow the rethrow in each helper) turned the
  relevant tests red with their own assertion messages, confirming the
  tests are not vacuous.
• Three "survivor" mutations (swap which half is passed first/second at each
  of the three symmetric call sites) left the full suite green —
  confirming the hardening actually decouples the invariant from call
  order, as intended. Reported as intentional successes, not gaps.
• Every mutation was restored and shasum -a 256 on Fleetd.java matched
  the pristine file byte-for-byte before moving to the next.

Build

mvn -o clean install from fleetd/: BUILD SUCCESS, exit 0.
Maven's own count: Tests run: 1755, Failures: 0, Errors: 0, Skipped: 0.
Independent sum over fleetd/target/surefire-reports/*.txt: 1755 tests, 0
failures/errors — agrees with Maven's count. 131 report files (baseline 130

• 1 new test class).

Out of scope

Injector.java is being edited by another worker for #551 — untouched here.

Fixes fleetd #561. ## What changed `Fleetd.java`'s `TurnListener` fan-out (the composed listener that hands each lifecycle event to both `CompletionResolver` and `SessionManager`) had four callbacks — `onTurnComplete`, `onTurnCompleteWithPostAction`, and both `onTurnFailed` overloads — built from two bare, unguarded statements each. `onDelivered`'s registration hazard was already fixed structurally by #556 (moved off the fan-out entirely); these four callbacks have the identical shape and were still untested per comment 17041 on #561 (which supersedes the ticket body — the body's `onDelivered` example is already fixed and already pinned by `InjectorTest`, not touched here). Extracted the composition into a package-private static factory, `Fleetd.turnListener(CompletionResolver completion, TurnListener sessions)`, and hardened it (rather than only asserting call order) with two small helpers: `bothMustRun` and `bothMustRunKeepingSecondResult`. Both callback halves are always attempted regardless of whether the other one throws; whatever escapes (from one half or both) is rethrown once both have run — never swallowed — so it still reaches `StatusPoller`'s `catch (Throwable)` and logs at ERROR. `onTurnCompleteWithPostAction` is documented as the one exception: order there is a functional requirement (`completion.resolveBeforePostAction` must run before the session half's context-reset housekeeping can erase the pane), not just fault tolerance, so it is not reorder-symmetric like the other three — see the javadoc on `Fleetd.turnListener`. ## Tests New `FleetdTurnListenerCompositionTest` (5 tests) builds the real production composition from a real `CompletionResolver` and a throwing fake `sessions` half: - one test per callback (4): the session half throws, and the completion half's effect (the captured `Rendezvous` waiter resolving/failing) still happened; - one mirror test: the completion half throws synchronously (forced via `ConcurrentHashMap.get(null)`, a real code path, not a contrived one — the production `CompletionResolver` is otherwise too defensive to throw synchronously in normal operation), and the session half still ran. ## Mutation proof - Two "kill" mutations (swallow the rethrow in each helper) turned the relevant tests red with their own assertion messages, confirming the tests are not vacuous. - Three "survivor" mutations (swap which half is passed first/second at each of the three symmetric call sites) left the **full suite** green — confirming the hardening actually decouples the invariant from call order, as intended. Reported as intentional successes, not gaps. - Every mutation was restored and `shasum -a 256` on `Fleetd.java` matched the pristine file byte-for-byte before moving to the next. ## Build `mvn -o clean install` from `fleetd/`: BUILD SUCCESS, exit 0. Maven's own count: `Tests run: 1755, Failures: 0, Errors: 0, Skipped: 0`. Independent sum over `fleetd/target/surefire-reports/*.txt`: 1755 tests, 0 failures/errors — agrees with Maven's count. 131 report files (baseline 130 + 1 new test class). ## Out of scope `Injector.java` is being edited by another worker for #551 — untouched here.

agent added 1 commit 2026-09-12 12:48:23 +02:00

fleetd #561: harden the completion/session TurnListener fan-out ... e20ccab1eb

Fleetd's turnListener composition had four callbacks (onTurnComplete,
onTurnCompleteWithPostAction, and both onTurnFailed overloads) built from two
bare, unguarded statements each. onDelivered's registration was already fixed
structurally by #556; these four had the identical fragility and were still
untested: nothing enforced that the completion resolver's half ran before the
session half beyond call order in the source, so a future reorder (or a
throwing session listener sequenced first) could silently skip the
completion resolver's effect and strand a caller for its full timeout.

Extracted the composition to a package-private static factory,
Fleetd.turnListener(completion, sessions), and hardened it with
bothMustRun/bothMustRunKeepingSecondResult: both callback halves are always
attempted regardless of whether the other throws, and whatever escapes is
rethrown afterward (never swallowed) so it still reaches StatusPoller's
catch (Throwable) and logs at ERROR.

FleetdTurnListenerCompositionTest builds this real composition from a real
CompletionResolver and a throwing fake sessions half, and asserts the
completion resolver's effect (the waiter resolving) survives the session
half throwing, for all four callbacks, plus a mirror case showing the
session half still runs when the completion half throws first.

onTurnCompleteWithPostAction keeps completion-before-session as a functional
requirement (resolveBeforePostAction must run before the context-reset
housekeeping can erase the pane), not just fault tolerance, so it is not
reorder-symmetric like the other three — documented in Fleetd.turnListener's
javadoc.

agent added 1 commit 2026-09-12 13:10:38 +02:00

fleetd #561 follow-up: pin the session half of bothMustRunKeepingSecondResult ... 034e17bb32

Two helpers maintain one invariant (the second callback half always runs,
even when the first throws): bothMustRun and bothMustRunKeepingSecondResult.
Only bothMustRun's "session half still runs" direction was asserted
(sessionHalfStillRunsWhenTheCompletionHalfThrowsSynchronously, via
onTurnComplete). bothMustRunKeepingSecondResult — the helper
onTurnCompleteWithPostAction uses — could be reverted to the pre-#561
broken shape and the suite stayed green.

Adds two tests to FleetdTurnListenerCompositionTest:
- sessionHalfStillRunsWhenTheCompletionHalfThrowsSynchronouslyForPostAction:
  mirrors the existing onTurnComplete case for onTurnCompleteWithPostAction/
  bothMustRunKeepingSecondResult.
- bothFailuresEscapeWhenBothHalvesThrowDistinctExceptions: proves a second,
  distinct failure from the session half is preserved via addSuppressed
  rather than silently dropped when both halves of bothMustRun throw.

Also rewords the onDelivered comment in Fleetd.turnListener: it previously
said this pair is safe because registration survives a throw via #556's
Injector wiring, which is true but is not why THIS pair is unguarded.
CompletionResolver.captureBaseline already catches RuntimeException around
its scrape read and fails open, so completion.onDelivered does not
realistically throw. Comment text only, no logic change.

ltms merged commit ed2fd6646a into main 2026-09-12 13:20:56 +02:00

ltms referenced this pull request 2026-09-12 13:22:59 +02:00

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

Sign in to join this conversation.

Reviewers

No Reviewers

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

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: fleet/fleetd#570