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

fleetd #248: prove main() wires CompletionResolver arguments, not just the class #251

Closed
agent wants to merge 0 commits from worker/cb248-composition-root-b-9acdf7-15 into main
pull from: worker/cb248-composition-root-b-9acdf7-15
merge into: fleet:main
Conversation 0 Commits - Files Changed -
agent commented 2026-09-03 07:21:01 +02:00
Member
Copy Link
Copy Source

fleetd #248: composition root is untested — CompletionResolver wiring

The measurement (reproduced first, before any change)

Fleetd.main builds CompletionResolver with an 8-argument constructor. I mutated that one
statement twice, ran the full build each time, then reverted:

Mutation Compile errors Result
Drop the worktree/branch lookup — pass _ -> null as the last argument 0 GREEN — Tests run: 1216, Failures: 0
Drop the backend-error pair — pass BackendErrorPatternLookup.legacy() / BackendErrorSink.none() 0 GREEN — Tests run: 1216, Failures: 0

Both confirmed the ticket's claim: either feature can be silently unwired at the composition
root and the build stays green, because every existing test constructs its own
CompletionResolver and proves the class, never main's wiring.

The fix

Extended the pattern Fleetd.deliverableTo already uses (a package-private static factory
main calls, tested directly):

  • Fleetd.worktreeBranchLookup(Supplier<List<MemberSession>> roster) — package-private,
    replaces the anonymous lambda that used to be built inline in the CompletionResolver
    constructor call. Takes a roster Supplier rather than a SessionManager so it is directly
    testable with a hand-built session list.
  • Fleetd.backendErrorPatternLookup(Supplier<List<MemberSession>> roster, Map<String, Pattern> errorPatternsByProfile)
    — package-private, replaces the local lambda that built backendErrorPatterns.
  • Fleetd.backendErrorSink(SessionManager sessions, Supplier<Map<String, FleetConfig.Profile>> profiles, BackendOutagePolicy outagePolicy, Supplier<ReplyPushLoop> pushLoop)
    — public, replacing the long local lambda that built backendErrorSink. Made public
    (unlike the other two) so dev.ltms.fleet.inject.BackendOutageFlowTest — whose own class doc
    said it "mirrors Fleetd.main's backendErrorSink lambda line-for-line" — could be pointed at
    the real production factory in a follow-up if desired; for this PR it's covered by a new,
    equivalent behavioural test (FleetdBackendErrorSinkTest) that calls the real factory directly.

Each factory has its own behaviour test:

  • FleetdWorktreeBranchLookupTest
  • FleetdBackendErrorPatternLookupTest
  • FleetdBackendErrorSinkTest (includes a real two-distinct-target incident flow through the
    actual production object, not a copy)

And the test that was actually missing — that main really passes these into
CompletionResolver — is FleetdCompletionResolverWiringTest: a source-text assertion on
Fleetd.java, explicitly named and worded [SOURCE TEXT] so it reads honestly as a source
check, not a behaviour check (mirrors the existing FleetdFleetAppConstructionTest pattern).

Verifying the fix — both mutations re-run against the fixed code

Mutation Compile errors Result
Drop worktree/branch lookup (_ -> null) 0 RED — Tests run: 1228, Failures: 1 (FleetdCompletionResolverWiringTest.worktreeBranchLookupIsStillPassedAtTheCallSite)
Drop backend-error pair (.legacy()/.none()) 0 RED — Tests run: 1228, Failures: 1 (FleetdCompletionResolverWiringTest.backendErrorArgumentsAreStillNamedAtTheCallSite)

Both mutations reverted and confirmed identical to the fixed source via diff -q before the
final build.

Build

mvn clean install from fleetd/, unpiped, full output read:

[INFO] Tests run: 1228, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS

(1216 pre-existing + 12 new tests across the 4 new test files.)

Scope / caveats

  • No production behaviour changes — mechanical extraction only (confirmed by diff review: the
    extracted lambda bodies are byte-for-byte the same logic, just moved and parameterized).
  • Scope was intentionally limited to CompletionResolver's construction and its arguments, per
    the ticket. exhaustedPatterns and exhaustionSink (2 of the 8 constructor arguments) were
    not touched — they were not part of the ticket's measured mutations or its named steps, and
    are pre-existing local variables, not lambdas built inline at the call site.
  • dev.ltms.fleet.inject.BackendOutageFlowTest itself was left untouched (still a hand-mirrored
    copy of the sink's logic) to avoid touching a test outside the assigned scope; the new
    FleetdBackendErrorSinkTest provides equivalent coverage against the real factory instead. A
    follow-up could rewire BackendOutageFlowTest to call Fleetd.backendErrorSink(...) directly
    now that it's public — noted here, not done, per the "note anything else, don't fix it" rule.

Ticket: fleetd #248

## fleetd #248: composition root is untested — CompletionResolver wiring ### The measurement (reproduced first, before any change) `Fleetd.main` builds `CompletionResolver` with an 8-argument constructor. I mutated that one statement twice, ran the full build each time, then reverted: | Mutation | Compile errors | Result | |---|---|---| | Drop the worktree/branch lookup — pass `_ -> null` as the last argument | 0 | GREEN — `Tests run: 1216, Failures: 0` | | Drop the backend-error pair — pass `BackendErrorPatternLookup.legacy()` / `BackendErrorSink.none()` | 0 | GREEN — `Tests run: 1216, Failures: 0` | Both confirmed the ticket's claim: either feature can be silently unwired at the composition root and the build stays green, because every existing test constructs its own `CompletionResolver` and proves the class, never `main`'s wiring. ### The fix Extended the pattern `Fleetd.deliverableTo` already uses (a package-private `static` factory `main` calls, tested directly): - **`Fleetd.worktreeBranchLookup(Supplier<List<MemberSession>> roster)`** — package-private, replaces the anonymous lambda that used to be built inline in the `CompletionResolver` constructor call. Takes a roster `Supplier` rather than a `SessionManager` so it is directly testable with a hand-built session list. - **`Fleetd.backendErrorPatternLookup(Supplier<List<MemberSession>> roster, Map<String, Pattern> errorPatternsByProfile)`** — package-private, replaces the local lambda that built `backendErrorPatterns`. - **`Fleetd.backendErrorSink(SessionManager sessions, Supplier<Map<String, FleetConfig.Profile>> profiles, BackendOutagePolicy outagePolicy, Supplier<ReplyPushLoop> pushLoop)`** — **public**, replacing the long local lambda that built `backendErrorSink`. Made public (unlike the other two) so `dev.ltms.fleet.inject.BackendOutageFlowTest` — whose own class doc said it "mirrors `Fleetd.main`'s `backendErrorSink` lambda line-for-line" — could be pointed at the real production factory in a follow-up if desired; for this PR it's covered by a new, equivalent behavioural test (`FleetdBackendErrorSinkTest`) that calls the real factory directly. Each factory has its own behaviour test: - `FleetdWorktreeBranchLookupTest` - `FleetdBackendErrorPatternLookupTest` - `FleetdBackendErrorSinkTest` (includes a real two-distinct-target incident flow through the actual production object, not a copy) And the test that was actually missing — that `main` really **passes** these into `CompletionResolver` — is `FleetdCompletionResolverWiringTest`: a source-text assertion on `Fleetd.java`, explicitly named and worded `[SOURCE TEXT]` so it reads honestly as a source check, not a behaviour check (mirrors the existing `FleetdFleetAppConstructionTest` pattern). ### Verifying the fix — both mutations re-run against the fixed code | Mutation | Compile errors | Result | |---|---|---| | Drop worktree/branch lookup (`_ -> null`) | 0 | **RED** — `Tests run: 1228, Failures: 1` (`FleetdCompletionResolverWiringTest.worktreeBranchLookupIsStillPassedAtTheCallSite`) | | Drop backend-error pair (`.legacy()`/`.none()`) | 0 | **RED** — `Tests run: 1228, Failures: 1` (`FleetdCompletionResolverWiringTest.backendErrorArgumentsAreStillNamedAtTheCallSite`) | Both mutations reverted and confirmed identical to the fixed source via `diff -q` before the final build. ### Build `mvn clean install` from `fleetd/`, unpiped, full output read: ``` [INFO] Tests run: 1228, Failures: 0, Errors: 0, Skipped: 0 [INFO] BUILD SUCCESS ``` (1216 pre-existing + 12 new tests across the 4 new test files.) ### Scope / caveats - No production behaviour changes — mechanical extraction only (confirmed by diff review: the extracted lambda bodies are byte-for-byte the same logic, just moved and parameterized). - Scope was intentionally limited to `CompletionResolver`'s construction and its arguments, per the ticket. `exhaustedPatterns` and `exhaustionSink` (2 of the 8 constructor arguments) were **not** touched — they were not part of the ticket's measured mutations or its named steps, and are pre-existing local variables, not lambdas built inline at the call site. - `dev.ltms.fleet.inject.BackendOutageFlowTest` itself was left untouched (still a hand-mirrored copy of the sink's logic) to avoid touching a test outside the assigned scope; the new `FleetdBackendErrorSinkTest` provides equivalent coverage against the real factory instead. A follow-up could rewire `BackendOutageFlowTest` to call `Fleetd.backendErrorSink(...)` directly now that it's public — noted here, not done, per the "note anything else, don't fix it" rule. Ticket: fleetd #248
agent added 1 commit 2026-09-03 07:21:02 +02:00
fleetd #248: prove main() wires CompletionResolver's arguments, not just the class
CI / contract (pull_request) Successful in 48s
Details
CI / build (pull_request) Failing after 1m40s
Details
d895f02bc1 
Fleetd.main built three of CompletionResolver's 8 constructor arguments inline
(a worktree/branch lookup lambda, and the backend-error pattern lookup + sink
locals). Dropping any of them at the call site compiled clean and left every
existing test green, because every existing test constructs its own
CompletionResolver and only ever proves the class, never main's wiring.

Extract each into a static factory on Fleetd (worktreeBranchLookup,
backendErrorPatternLookup, backendErrorSink — the same static-factory pattern
Fleetd.deliverableTo already uses), test each factory's own behaviour, and add
a source-text assertion (FleetdCompletionResolverWiringTest) proving main's
CompletionResolver call still passes all three. backendErrorSink is public so
BackendOutageFlowTest can exercise the real production sink directly instead
of the hand-mirrored copy its own class doc used to describe.

No production behaviour changes — mechanical extraction only.
ltms closed this pull request 2026-09-03 07:25:16 +02:00
Some checks are pending
CI / contract (pull_request) Successful in 48s
Details
CI / build (pull_request) Failing after 1m40s
Details

Pull request closed

Please reopen this pull request to perform a merge.
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
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#251
Delete Branch "worker/cb248-composition-root-b-9acdf7-15"

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?