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

Deleting any of the four startup report calls from Fleetd.main leaves the suite green #442

New Issue
Closed
opened 2026-09-10 09:29:37 +02:00 by ltms · 1 comment
ltms commented 2026-09-10 09:29:37 +02:00
Owner
Copy Link
Copy Source

Split out of #441 (closed as a duplicate). The Features page named this gap; my own grep did not find it.

The gap

Fleetd.main makes four startup report calls:

:131  reportGitHostShape(cfg);
:132  reportMemberTrustModel(cfg);
:137  reportMemberCredentialsGap(cfg);
:141  reportExhaustedPatternGap(cfg);

Each has a test for its own behaviour, and each of those tests calls the method directly.
Measured: exactly one test file references each of the four names.

reportGitHostShape:        1 test file
reportMemberTrustModel:    1 test file
reportMemberCredentialsGap:1 test file
reportExhaustedPatternGap: 1 test file

Compare validateAll, which is referenced by 4.

So nothing proves main still calls any of them. Delete a line and the suite stays green.
wiki/11-Features.md records this being measured at the #395 merge: 1472 tests, 0 failures with
reportExhaustedPatternGap(cfg) deleted.

Why it matters

All four reports exist to make a silent misconfiguration audible — an unprotected credential
name, a member trust model the operator did not intend, a profile that can never detect its own
usage limit. Every one of them fails in the same direction: if the call goes missing, nothing
throws, nothing is logged, and the fleet runs on looking healthy. That is the exact failure the
reports were written to prevent, so an unpinned call site removes the whole value of the feature
while every test still passes.

This is a known recurring shape in this repo — a test on the seam that does not prove the caller.

The precedent, and it is a close one

The same defect was already found and fixed for the validators. FleetdStartupValidationTest's
javadoc records it:

Mutation testing found that deleting cfg.validateAll(); (née six individual
cfg.validateXxx(); calls) from Fleetd.main left the full 1478-test suite green; every
existing test called a validator directly and none exercised Fleetd.main as the caller.

The fix was two-part: collapse the six calls into one cfg.validateAll(), then pin that one call
by invoking the real Fleetd.main with a config that fails validation and asserting it throws.

Why the same fix does not transfer directly

validateAll() throws, so "main refuses" is an observable outcome. The four reports only
log. There is nothing to assert by return value or exception.

The seam that makes it possible anyway

The ordering is favourable, and this is the key fact for whoever takes this:

:131  reportGitHostShape(cfg);          <- all four reports
:132  reportMemberTrustModel(cfg);
:137  reportMemberCredentialsGap(cfg);
:141  reportExhaustedPatternGap(cfg);
:150  guard.assertPrimaryClean(...);
:161  cfg.validateAll();                <- throws

All four reports run before validateAll(). So a config that (a) triggers all four reports
and (b) fails one validator makes the real Fleetd.main emit all four report lines and then throw
— before it opens the herdr socket, binds Javalin, or causes any other side effect. That is the
same safe-abort property FleetdStartupValidationTest already documents and relies on.

A test can therefore attach a log capture, call the real Fleetd.main, assert it throws, and
assert all four report lines were emitted.

Acceptance criteria

  1. A test calls the real Fleetd.main and fails if any of the four report calls is missing.
    One test covering all four is fine and preferred.
  2. It must not open a socket, bind a port, or start the daemon. Rely on the pre-validateAll
    ordering above, and assert main throws.
  3. Every fixture uses @TempDir. No test may write a real .claude.json or a real fleetd.yaml.
  4. The mutations that must fail — all four, one at a time. Delete each of the four report
    calls from Fleetd.main in turn and show at least one test going red each time. Paste four
    red runs with the failing test method names, plus the restored green run. Four separate
    mutations, not one.
  5. If pinning them separately is unreasonably awkward, collapsing the four into one aggregate
    call (reportStartupShape(cfg) or similar) and pinning that is acceptable — it is exactly
    what validateAll() did. If you take that route, criterion 4 still applies: deleting the one
    aggregate call must go red, and each of the four reports must still be individually covered by
    its existing behaviour test.

Out of scope

  • Do not change what any report logs, or its wording. This ticket pins the call, nothing else.
  • Do not touch validateAll() or FleetdStartupValidationTest.
  • The model gate coverage line at Fleetd.java:241 has the same unpinned shape, but it sits
    after validateAll() and after workers is constructed, so the safe-abort trick above does
    not reach it. Leave it alone; it needs a different approach and its own ticket.

Also to fix, one line

wiki/11-Features.md, under "Startup says which profiles have usage-limit detection turned off",
says "Same shape as the six FleetConfig.validateXxx() startup calls — fleetd #398 owns closing
it." That is now stale twice over: the validateXxx half was fixed by validateAll(), and #398
is the closed models allow-list PR. I will correct that line myself, since the wiki is a submodule
workers cannot usefully commit to.

Split out of #441 (closed as a duplicate). The Features page named this gap; my own grep did not find it. ## The gap `Fleetd.main` makes four startup report calls: ``` :131 reportGitHostShape(cfg); :132 reportMemberTrustModel(cfg); :137 reportMemberCredentialsGap(cfg); :141 reportExhaustedPatternGap(cfg); ``` Each has a test for its **own behaviour**, and each of those tests calls the method directly. Measured: exactly one test file references each of the four names. ``` reportGitHostShape: 1 test file reportMemberTrustModel: 1 test file reportMemberCredentialsGap:1 test file reportExhaustedPatternGap: 1 test file ``` Compare `validateAll`, which is referenced by 4. So nothing proves `main` still **calls** any of them. Delete a line and the suite stays green. `wiki/11-Features.md` records this being measured at the #395 merge: 1472 tests, 0 failures with `reportExhaustedPatternGap(cfg)` deleted. ## Why it matters All four reports exist to make a silent misconfiguration audible — an unprotected credential name, a member trust model the operator did not intend, a profile that can never detect its own usage limit. Every one of them fails in the same direction: if the call goes missing, nothing throws, nothing is logged, and the fleet runs on looking healthy. That is the exact failure the reports were written to prevent, so an unpinned call site removes the whole value of the feature while every test still passes. This is a known recurring shape in this repo — a test on the seam that does not prove the caller. ## The precedent, and it is a close one The same defect was already found and fixed for the validators. `FleetdStartupValidationTest`'s javadoc records it: > Mutation testing found that deleting `cfg.validateAll();` (née six individual > `cfg.validateXxx();` calls) from `Fleetd.main` left the full 1478-test suite green; every > existing test called a validator directly and none exercised `Fleetd.main` as the caller. The fix was two-part: collapse the six calls into one `cfg.validateAll()`, then pin that one call by invoking the real `Fleetd.main` with a config that fails validation and asserting it throws. ## Why the same fix does not transfer directly `validateAll()` **throws**, so "main refuses" is an observable outcome. The four reports only **log**. There is nothing to assert by return value or exception. ## The seam that makes it possible anyway The ordering is favourable, and this is the key fact for whoever takes this: ``` :131 reportGitHostShape(cfg); <- all four reports :132 reportMemberTrustModel(cfg); :137 reportMemberCredentialsGap(cfg); :141 reportExhaustedPatternGap(cfg); :150 guard.assertPrimaryClean(...); :161 cfg.validateAll(); <- throws ``` All four reports run **before** `validateAll()`. So a config that (a) triggers all four reports and (b) fails one validator makes the real `Fleetd.main` emit all four report lines and then throw — before it opens the herdr socket, binds Javalin, or causes any other side effect. That is the same safe-abort property `FleetdStartupValidationTest` already documents and relies on. A test can therefore attach a log capture, call the real `Fleetd.main`, assert it throws, and assert all four report lines were emitted. ## Acceptance criteria 1. A test calls the real `Fleetd.main` and fails if any of the four report calls is missing. One test covering all four is fine and preferred. 2. It must not open a socket, bind a port, or start the daemon. Rely on the pre-`validateAll` ordering above, and assert `main` throws. 3. Every fixture uses `@TempDir`. No test may write a real `.claude.json` or a real `fleetd.yaml`. 4. **The mutations that must fail — all four, one at a time.** Delete each of the four report calls from `Fleetd.main` in turn and show at least one test going red each time. Paste four red runs with the failing test method names, plus the restored green run. Four separate mutations, not one. 5. If pinning them separately is unreasonably awkward, collapsing the four into one aggregate call (`reportStartupShape(cfg)` or similar) and pinning that is acceptable — it is exactly what `validateAll()` did. If you take that route, criterion 4 still applies: deleting the one aggregate call must go red, and each of the four reports must still be individually covered by its existing behaviour test. ## Out of scope - Do not change what any report logs, or its wording. This ticket pins the call, nothing else. - Do not touch `validateAll()` or `FleetdStartupValidationTest`. - The model gate coverage line at `Fleetd.java:241` has the same unpinned shape, but it sits **after** `validateAll()` and after `workers` is constructed, so the safe-abort trick above does not reach it. Leave it alone; it needs a different approach and its own ticket. ## Also to fix, one line `wiki/11-Features.md`, under "Startup says which profiles have usage-limit detection turned off", says "Same shape as the six `FleetConfig.validateXxx()` startup calls — fleetd #398 owns closing it." That is now stale twice over: the `validateXxx` half was fixed by `validateAll()`, and #398 is the closed models allow-list PR. I will correct that line myself, since the wiki is a submodule workers cannot usefully commit to.
ltms commented 2026-09-10 11:31:00 +02:00
Author
Owner
Copy Link
Copy Source

Fixed and merged as PR #445, on main at 82fae94.

FleetdStartupReportTest now pins all four startup reports in Fleetd.main. The seam is that the reports run before cfg.validateAll() throws, so a config with a non-loopback bind.host in a @TempDir lets the test capture every report line without main ever opening the herdr socket or binding a port.

Evidence, run by me against the tree that was merged (the worker that wrote the test lost its backend mid-turn and its worktree was reaped, so its own run is gone):

  • merge of current main: 0 conflicts
  • full build: Tests run: 1574, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS, 0 compile errors
  • CONTROL green
  • deleting each call from main() — reportGitHostShape, reportMemberTrustModel, reportMemberCredentialsGap, reportExhaustedPatternGap — is KILLED by mainReportsEveryStartupGapBeforeValidationAborts, each deletion proven applied (occurrences driven to 0) and restored

I also removed an unused java.util.List import the test carried (b1f34c2); it is a warning in this repo, and the file only ever names ListAppender.

What this closes, in plain terms. Each report* method already had a unit test, but nothing checked that main still called it — each method was referenced by exactly one test file, while validateAll had 4. So deleting a report call from startup was a green-build change. It is not any more.

Fixed and merged as PR #445, on `main` at `82fae94`. `FleetdStartupReportTest` now pins all four startup reports in `Fleetd.main`. The seam is that the reports run *before* `cfg.validateAll()` throws, so a config with a non-loopback `bind.host` in a `@TempDir` lets the test capture every report line without `main` ever opening the herdr socket or binding a port. **Evidence, run by me against the tree that was merged** (the worker that wrote the test lost its backend mid-turn and its worktree was reaped, so its own run is gone): - merge of current `main`: **0 conflicts** - full build: `Tests run: 1574, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`, 0 compile errors - CONTROL green - deleting each call from `main()` — `reportGitHostShape`, `reportMemberTrustModel`, `reportMemberCredentialsGap`, `reportExhaustedPatternGap` — is **KILLED** by `mainReportsEveryStartupGapBeforeValidationAborts`, each deletion proven applied (occurrences driven to 0) and restored I also removed an unused `java.util.List` import the test carried (`b1f34c2`); it is a warning in this repo, and the file only ever names `ListAppender`. **What this closes, in plain terms.** Each `report*` method already had a unit test, but nothing checked that `main` still *called* it — each method was referenced by exactly one test file, while `validateAll` had 4. So deleting a report call from startup was a green-build change. It is not any more.
ltms closed this issue 2026-09-10 11:31:17 +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
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#442
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?