fleetd #556: make turn registration structural, independent of any TurnListener #566

Merged

ltms merged 2 commits from worker/556-injector-owns-registration-e027a5-1 into main 2026-09-12 12:18:16 +02:00

Conversation 1 Commits 2 Files Changed 6

+430 -37

agent commented 2026-09-12 11:50:47 +02:00

Member

Copy Link

Copy Source

fleetd #556: the Injector owns the invariant "every delivered turn has a registered waiter," but the only thing that satisfied it was CompletionResolver.captureBaseline, called from inside a TurnListener.onDelivered callback wired in Fleetd.java. Any TurnListener that throws (from onDelivered or elsewhere) could break the invariant with no way for the Injector to detect it. #553 only made the one reachable listener behave via a try/finally backstop; it did not remove this structural dependency.

Design

Added a narrow TurnRegistrar functional interface, deliberately decoupled from TurnListener (its own javadoc explains why). CompletionResolver now implements it via a new register() method extracted from captureBaseline's registration half; captureBaseline keeps its own full body unchanged (only javadoc updated), so every existing direct caller/test of captureBaseline is untouched. Injector gets an explicit registrar field and a constructor family that auto-derives the registrar from the TurnListener via instanceof where that still works, plus new explicit-registrar overloads where it can't (Fleetd.java's anonymous fan-out listener can't implement two interfaces via new Type(){}). The delivery path calls registrar.register(target, sent.token()) directly and unconditionally — in both the ordinary if (sent != null) block and the #553 finally backstop — before turnListener.onDelivered(...), so registration no longer depends on that notification callback succeeding. Fleetd.java wires completion::register explicitly as the registrar, bypassing the turnListener fan-out for registration purposes.

CB-116 ordering (onTurnComplete reads the PREVIOUS turn's inFlight entry before the new turn's registrar.register() runs) and the two-arg inFlight.remove(target, turn) vs one-arg distinction on the completion path are both preserved unchanged — see the updated comments around Injector.java's delivery method.

#561's order test

Per comment 16966: checked git log, git branch -a, and grepped the repo at this branch's point off main (4f9aba4) — #561's order-dependent test (asserting Fleetd.java:514-515's completion.onDelivered -> sessions.onDelivered call order) does not exist anywhere in this repository. Nothing to delete. The two new acceptance tests below (a throws-from-every-callback listener, and the CB-116 order pin) are the order-independent replacement comment 16966 asked for.

Tests added

• InjectorTest.aTurnListenerThatThrowsFromEveryCallbackStillLeavesTheDeliveredTurnRegisteredAndResolvable — acceptance criterion 1: a TurnListener that throws from every callback still leaves a delivered turn registered and its waiter resolvable; red against the pre-fix shape (registration used to live inside onDelivered, one of the throwing callbacks).

• InjectorTest.theNewTurnsRegistrationRunsAfterThePreviousTurnsCompletionIsRead — acceptance criterion 2 (CB-116 guard): pins the call order between onTurnComplete (previous turn) and register (new turn) within one onStatus cycle.

• CompletionResolverTest.aSupersededTurnsPlainCompletionMustNotEvictItsSuccessorsRegistration

• CompletionResolverTest.aSupersededTurnsEchoedNoReportSubPathMustNotEvictItsSuccessorsRegistration

• CompletionResolverTest.aSupersededTurnsFailMustNotEvictItsSuccessorsRegistration

  These three (per comment 16908) name and test the invariant the two-arg remove(key, value) idiom maintains — "a superseded turn's terminal handling must not evict its successor's registration" — across resolve()'s plain-completion branch, its echoed-noReportMessage sub-path, and fail(), rather than testing the idiom literally.

Every new test was proven by mutation: a line-anchored sed edit on the exact production line each test protects (confirmed red with that test's own assertion message and observed value), restored and verified byte-identical via shasum -a 256, then re-run green as a control. Full detail is in the worker's handoff.

Build

mvn -o clean install: exit 0, Tests run: 1749, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS.

Scope

Only #556. #553's ordering decision was not reopened; LoopWatchdog's package placement was not touched.

fleetd #556: the Injector owns the invariant "every delivered turn has a registered waiter," but the only thing that satisfied it was `CompletionResolver.captureBaseline`, called from inside a `TurnListener.onDelivered` callback wired in `Fleetd.java`. Any `TurnListener` that throws (from `onDelivered` or elsewhere) could break the invariant with no way for the `Injector` to detect it. #553 only made the one reachable listener behave via a try/finally backstop; it did not remove this structural dependency. ## Design Added a narrow `TurnRegistrar` functional interface, deliberately decoupled from `TurnListener` (its own javadoc explains why). `CompletionResolver` now implements it via a new `register()` method extracted from `captureBaseline`'s registration half; `captureBaseline` keeps its own full body unchanged (only javadoc updated), so every existing direct caller/test of `captureBaseline` is untouched. `Injector` gets an explicit `registrar` field and a constructor family that auto-derives the registrar from the `TurnListener` via `instanceof` where that still works, plus new explicit-registrar overloads where it can't (`Fleetd.java`'s anonymous fan-out listener can't implement two interfaces via `new Type(){}`). The delivery path calls `registrar.register(target, sent.token())` directly and unconditionally — in both the ordinary `if (sent != null)` block and the #553 `finally` backstop — before `turnListener.onDelivered(...)`, so registration no longer depends on that notification callback succeeding. `Fleetd.java` wires `completion::register` explicitly as the registrar, bypassing the `turnListener` fan-out for registration purposes. CB-116 ordering (`onTurnComplete` reads the PREVIOUS turn's `inFlight` entry before the new turn's `registrar.register()` runs) and the two-arg `inFlight.remove(target, turn)` vs one-arg distinction on the completion path are both preserved unchanged — see the updated comments around `Injector.java`'s delivery method. ## #561's order test Per comment 16966: checked `git log`, `git branch -a`, and grepped the repo at this branch's point off `main` (`4f9aba4`) — #561's order-dependent test (asserting `Fleetd.java:514-515`'s `completion.onDelivered` -> `sessions.onDelivered` call order) does not exist anywhere in this repository. Nothing to delete. The two new acceptance tests below (a throws-from-every-callback listener, and the CB-116 order pin) are the order-independent replacement comment 16966 asked for. ## Tests added - `InjectorTest.aTurnListenerThatThrowsFromEveryCallbackStillLeavesTheDeliveredTurnRegisteredAndResolvable` — acceptance criterion 1: a `TurnListener` that throws from every callback still leaves a delivered turn registered and its waiter resolvable; red against the pre-fix shape (registration used to live inside `onDelivered`, one of the throwing callbacks). - `InjectorTest.theNewTurnsRegistrationRunsAfterThePreviousTurnsCompletionIsRead` — acceptance criterion 2 (CB-116 guard): pins the call order between `onTurnComplete` (previous turn) and `register` (new turn) within one `onStatus` cycle. - `CompletionResolverTest.aSupersededTurnsPlainCompletionMustNotEvictItsSuccessorsRegistration` - `CompletionResolverTest.aSupersededTurnsEchoedNoReportSubPathMustNotEvictItsSuccessorsRegistration` - `CompletionResolverTest.aSupersededTurnsFailMustNotEvictItsSuccessorsRegistration` These three (per comment 16908) name and test the invariant the two-arg `remove(key, value)` idiom maintains — "a superseded turn's terminal handling must not evict its successor's registration" — across `resolve()`'s plain-completion branch, its echoed-`noReportMessage` sub-path, and `fail()`, rather than testing the idiom literally. Every new test was proven by mutation: a line-anchored `sed` edit on the exact production line each test protects (confirmed red with that test's own assertion message and observed value), restored and verified byte-identical via `shasum -a 256`, then re-run green as a control. Full detail is in the worker's handoff. ## Build `mvn -o clean install`: exit 0, `Tests run: 1749, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`. ## Scope Only #556. #553's ordering decision was not reopened; `LoopWatchdog`'s package placement was not touched.

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

fleetd #556: make turn registration structural, independent of any TurnListener ... a46e4058ac

The Injector owns the invariant "every delivered turn has a registered
waiter," but before this the only thing that satisfied it was
CompletionResolver.captureBaseline, called from inside a TurnListener
callback wired in Fleetd.java. Any TurnListener that throws (from
onDelivered or elsewhere) could break the invariant with no way for the
Injector to detect it. #553 only made the one reachable listener behave
via a try/finally backstop; it did not remove this structural dependency.

Add a narrow TurnRegistrar functional interface, decoupled from
TurnListener, whose only job is registering a delivered turn's waiter.
CompletionResolver now implements it via a new register() method
(extracted from captureBaseline's registration half; captureBaseline
keeps its own full body unchanged, so existing direct callers/tests are
untouched). Injector gets an explicit registrar field/constructor family
(auto-derived from the TurnListener via instanceof where that still
works, explicit where Fleetd's anonymous fan-out listener can't
implement two interfaces at once) and calls registrar.register(...)
directly and unconditionally in both the ordinary delivery path and the
#553 finally backstop, before turnListener.onDelivered(...) — so
registration no longer depends on that notification callback succeeding.
Fleetd.java wires completion::register explicitly as the registrar,
bypassing the turnListener fan-out for registration purposes.

CB-116 ordering (onTurnComplete reads the PREVIOUS turn's inFlight entry
before the new turn's registrar.register() runs) and the two-arg
inFlight.remove(target, turn) vs one-arg distinction on the completion
path are both preserved unchanged.

#561's order-dependent test (asserting Fleetd.java's
completion.onDelivered -> sessions.onDelivered call order) does not
exist anywhere in this repo at this branch point — nothing to delete.

Adds 5 tests: a TurnListener that throws from every callback still
leaves the delivered turn registered and resolvable; the new turn's
registration still runs after the previous turn's completion is read
(CB-116 guard, pinned as a call-order assertion); and three tests naming
the two-arg-remove invariant directly (a superseded turn's terminal
handling must not evict its successor's registration) across resolve()'s
plain-completion branch, its echoed-noReportMessage sub-path, and
fail().

ltms referenced this pull request 2026-09-12 12:04:54 +02:00

TurnListener implementations are the only thing maintaining the Injector's own invariant #556

agent added 1 commit 2026-09-12 12:08:52 +02:00

fleetd #556 rework: pin registration on the #553 finally backstop path ... 738d34a609

Comment 17009: there are TWO registrar.register(target, sent.token())
call sites in Injector's delivery method — the ordinary path inside
`if (sent != null)`, and the fleetd #553 finally backstop, reached only
when an earlier block throws before the ordinary path ever runs. The
lead's mutation on Injector.java:703 (the backstop call) survived the
full suite: the existing #553 regression test for this exact scenario
(aRuntimeExceptionFromOnTurnCompleteStillCompletesTheNextDelivery)
asserts only that the delivered future completes, never that the turn
is registered with CompletionResolver — so a redesign that dropped
registration from the backstop would reopen this ticket's own defect on
precisely the path #553 exists for, with every existing test green.

Adds aRuntimeExceptionFromOnTurnCompleteStillLeavesTheNextDeliveryRegisteredOnTheRecoveryPath:
drives the same construction as the existing #553 test (onTurnComplete
throws for a previous turn, forcing the next turn's delivery down the
finally backstop) and additionally asserts the new turn is registered
with CompletionResolver and carries the correct waiter — the same
assertion the ordinary-path test makes, now made on the recovery path.

Proven by mutation: removing Injector.java:703 alone (exact-line anchor
1 -> 0) turns the new test red with its own assertion message; restored
and confirmed byte-identical (sha256 8fcb698afccc254b0c99d3a4bf9e960c0e7c85e542024e870c1e815dce6d62a9,
matching the pre-mutation tree); re-run green as a control. Full suite
after restore: 1750 tests, 0 failures, 0 errors, 0 skipped (Maven's own
summary and an independent sum over surefire-reports/*.txt agree),
BUILD SUCCESS.

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

Author

Member

Copy Link

Copy Source

Rework (comment 17009): the #553 finally backstop was unpinned

Confirmed the gap as reported: Injector.java:703 (the fleetd #553 finally backstop's own registrar.register(target, sent.token()) call) had no test asserting registration on that path — only the delivered-future completion was pinned there (by the pre-existing aRuntimeExceptionFromOnTurnCompleteStillCompletesTheNextDelivery). Mutating :703 alone (comment out, exact-line anchor 1 -> 0) left the full suite green (1749/1749 before this rework), because nothing exercised the invariant on that specific path.

Fix

Added InjectorTest.aRuntimeExceptionFromOnTurnCompleteStillLeavesTheNextDeliveryRegisteredOnTheRecoveryPath: same construction as the existing #553 test (onTurnComplete throws for the previous turn, forcing the next turn's delivery down the finally backstop, since the ordinary if (sent != null) path never runs for it), plus the assertion the ordinary-path test already makes — completion.inFlight(target) non-null, carrying the correct waiter — now checked on the recovery path.

Mutation proof (Injector.java:703)

• Pristine exact-line anchor count: 1 -> mutated: 0 (comment-out on the single line, indentation makes the two registrar.register sites textually distinct so this is unambiguous)
• RED: InjectorTest.aRuntimeExceptionFromOnTurnCompleteStillLeavesTheNextDeliveryRegisteredOnTheRecoveryPath, exit 1:

  org.opentest4j.AssertionFailedError: fleetd #556: "second" must be registered by the fleetd #553
  finally backstop even though onTurnComplete threw for "first"'s completion before the ordinary
  registration path ever ran for "second" ==> expected: not <null>
  

• Restored from the pristine copy; shasum -a 256 on Injector.java returns to 8fcb698afccc254b0c99d3a4bf9e960c0e7c85e542024e870c1e815dce6d62a9 (matches the tree before the mutation, and the sha the lead independently confirmed on 52f018e).
• Green control after restore: Tests run: 1, Failures: 0, Errors: 0, Skipped: 0.

Full suite after the rework

mvn -o clean install: exit 0. Maven's own summary line and an independent sum over target/surefire-reports/*.txt agree: Tests run: 1750, Failures: 0, Errors: 0, Skipped: 0. BUILD SUCCESS.

Criterion-5 caveat (restated, per item 4)

The literal form of criterion 5 — compile and run the new order-independent test against the unfixed tree at the branch point — is not executable: aTurnListenerThatThrowsFromEveryCallbackStillLeavesTheDeliveredTurnRegisteredAndResolvable calls the new 5-arg Injector constructor (with the explicit TurnRegistrar parameter), which does not exist on origin/main before this ticket's TurnRegistrar/constructor changes — the test cannot even compile against that tree. The faithful equivalent I ran instead (and reported in the original hand-off, independently reproduced by the lead on 52f018e): with the fix's production code present, comment out the ordinary-path registration call (Injector.java:660, exact-line anchor 1 -> 0) so the tree reaches the same end state (no registration) that the pre-fix code produced whenever the sole listener threw. Both new tests went red against that mutation with their own assertion messages. This is stated here explicitly rather than implying the literal unfixed-tree check was run.

Commit: 738d34a on worker/556-injector-owns-registration-e027a5-1.

## Rework (comment 17009): the #553 finally backstop was unpinned Confirmed the gap as reported: `Injector.java:703` (the fleetd #553 `finally` backstop's own `registrar.register(target, sent.token())` call) had no test asserting registration on that path — only the delivered-future completion was pinned there (by the pre-existing `aRuntimeExceptionFromOnTurnCompleteStillCompletesTheNextDelivery`). Mutating `:703` alone (comment out, exact-line anchor 1 -> 0) left the full suite green (1749/1749 before this rework), because nothing exercised the invariant on that specific path. ### Fix Added `InjectorTest.aRuntimeExceptionFromOnTurnCompleteStillLeavesTheNextDeliveryRegisteredOnTheRecoveryPath`: same construction as the existing #553 test (`onTurnComplete` throws for the previous turn, forcing the next turn's delivery down the finally backstop, since the ordinary `if (sent != null)` path never runs for it), plus the assertion the ordinary-path test already makes — `completion.inFlight(target)` non-null, carrying the correct waiter — now checked on the recovery path. ### Mutation proof (Injector.java:703) - Pristine exact-line anchor count: 1 -> mutated: 0 (comment-out on the single line, indentation makes the two `registrar.register` sites textually distinct so this is unambiguous) - RED: `InjectorTest.aRuntimeExceptionFromOnTurnCompleteStillLeavesTheNextDeliveryRegisteredOnTheRecoveryPath`, exit 1: ``` org.opentest4j.AssertionFailedError: fleetd #556: "second" must be registered by the fleetd #553 finally backstop even though onTurnComplete threw for "first"'s completion before the ordinary registration path ever ran for "second" ==> expected: not <null> ``` - Restored from the pristine copy; `shasum -a 256` on `Injector.java` returns to `8fcb698afccc254b0c99d3a4bf9e960c0e7c85e542024e870c1e815dce6d62a9` (matches the tree before the mutation, and the sha the lead independently confirmed on `52f018e`). - Green control after restore: `Tests run: 1, Failures: 0, Errors: 0, Skipped: 0`. ### Full suite after the rework `mvn -o clean install`: exit 0. Maven's own summary line and an independent sum over `target/surefire-reports/*.txt` agree: `Tests run: 1750, Failures: 0, Errors: 0, Skipped: 0`. `BUILD SUCCESS`. ### Criterion-5 caveat (restated, per item 4) The literal form of criterion 5 — compile and run the new order-independent test against the unfixed tree at the branch point — is not executable: `aTurnListenerThatThrowsFromEveryCallbackStillLeavesTheDeliveredTurnRegisteredAndResolvable` calls the new 5-arg `Injector` constructor (with the explicit `TurnRegistrar` parameter), which does not exist on `origin/main` before this ticket's `TurnRegistrar`/constructor changes — the test cannot even compile against that tree. The faithful equivalent I ran instead (and reported in the original hand-off, independently reproduced by the lead on `52f018e`): with the fix's production code present, comment out the ordinary-path registration call (`Injector.java:660`, exact-line anchor 1 -> 0) so the tree reaches the same end state (no registration) that the pre-fix code produced whenever the sole listener threw. Both new tests went red against that mutation with their own assertion messages. This is stated here explicitly rather than implying the literal unfixed-tree check was run. Commit: `738d34a` on `worker/556-injector-owns-registration-e027a5-1`.

ltms merged commit db4c98ac60 into main 2026-09-12 12:18:16 +02:00

ltms referenced this pull request 2026-09-12 12:20:41 +02:00

TurnListener implementations are the only thing maintaining the Injector's own invariant #556

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#566