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

LeadTabScanner's excludedWorkspaceLabels argument is unpinned: mutating FleetdAssembly.java:265 leaves the suite green #670

New Issue
Closed
opened 2026-10-03 20:02:24 +02:00 by ltms · 1 comment
ltms commented 2026-10-03 20:02:24 +02:00
Owner
Copy Link
Copy Source

Split out of #661. The main fix there landed in b4b7cf5; this is the one acceptance criterion it did not meet, and I confirmed the gap by measurement rather than by reading.

The gap

LeadTabScanner's third constructor parameter, excludedWorkspaceLabels, is supplied by production at FleetdAssembly.java:265 as Set.of(). No test pins that argument. The only test that exercises the parameter supplies its own set (LeadTabScannerTest.java:163 passes Set.of("fleetd-workers")), so it tests the seam and says nothing about the producer.

Measured on b4b7cf5

Mutation run in a throwaway worktree, not the main clone.

# mutate line 265 only
perl -i -pe 's/Set\.of\(\)/Set.of("fleet")/ if $. == 265' \
    fleetd/src/main/java/dev/ltms/fleet/FleetdAssembly.java

git diff --numstat   ->  1  1  fleetd/src/main/java/dev/ltms/fleet/FleetdAssembly.java
mvn -o clean install ->  MUTANT_EXIT=0
                         [INFO] Tests run: 1927, Failures: 0, Errors: 0, Skipped: 0
                         [INFO] BUILD SUCCESS
                         report files: 170

The mutant survives. I then reverted and confirmed the worktree was byte-identical to HEAD and that the suite was green at the same 1927 / 170 before and after, so the survival is not an artefact of a dirty tree.

This is exactly what #661's acceptance criterion 3 predicted: "today you can change it to Set.of("fleet") and the suite stays green, because the only test exercising it supplies its own set." I closed #661 without running that check and am filing the remainder here rather than leaving the criterion quietly unmet.

Why it is worth pinning even though the value is empty

The empty set is not an accident; it is the correct production value, and the comment above it explains why — scanning member tabs would demote the lead to a worker. That is the point. A future edit that "hardens" the scanner by putting a label back in would reintroduce the demotion bug and no test would object. The argument encodes a real decision, so it needs an assertion.

Note the asymmetry with the hazard #661 fixed. There, a non-empty set would have been a defence. Here, a non-empty set is a defect. Anyone reading the parameter name alone will guess wrong about which direction is safe, which is the strongest reason to write the test.

Acceptance criteria

  1. A test asserts the exclusion set that FleetdAssembly's production boot path actually passes to LeadTabScanner. Mutating Set.of() to any non-empty set at FleetdAssembly.java:265 must turn that test red. Report the mutant output, not only the green run.
  2. The test must instantiate or observe the production producer, not supply its own set. A test that constructs LeadTabScanner directly repeats the existing blind spot at LeadTabScannerTest.java:163.
  3. Report the complement too: confirm the new test is the one that goes red, by naming the failing test class and method from the mutant run. A mutation that reddens some unrelated test is not evidence this site is pinned.
  4. Do not change FleetdAssembly.java:265 itself. Set.of() is correct. This ticket adds a test, not a behaviour change.

Not in scope

The wiki/11-Features.md:678-680 correction and the feature entry for the new validator are still owed from #661 and are lead-only, because wiki/ is a submodule. Also still open: #668, the missing validateAll reachability case for validateLeadRollover.

Split out of #661. The main fix there landed in **b4b7cf5**; this is the one acceptance criterion it did not meet, and I confirmed the gap by measurement rather than by reading. ## The gap `LeadTabScanner`'s third constructor parameter, `excludedWorkspaceLabels`, is supplied by production at `FleetdAssembly.java:265` as `Set.of()`. **No test pins that argument.** The only test that exercises the parameter supplies its own set (`LeadTabScannerTest.java:163` passes `Set.of("fleetd-workers")`), so it tests the seam and says nothing about the producer. ## Measured on b4b7cf5 Mutation run in a throwaway worktree, not the main clone. ``` # mutate line 265 only perl -i -pe 's/Set\.of\(\)/Set.of("fleet")/ if $. == 265' \ fleetd/src/main/java/dev/ltms/fleet/FleetdAssembly.java git diff --numstat -> 1 1 fleetd/src/main/java/dev/ltms/fleet/FleetdAssembly.java mvn -o clean install -> MUTANT_EXIT=0 [INFO] Tests run: 1927, Failures: 0, Errors: 0, Skipped: 0 [INFO] BUILD SUCCESS report files: 170 ``` **The mutant survives.** I then reverted and confirmed the worktree was byte-identical to `HEAD` and that the suite was green at the same 1927 / 170 before and after, so the survival is not an artefact of a dirty tree. This is exactly what #661's acceptance criterion 3 predicted: *"today you can change it to `Set.of("fleet")` and the suite stays green, because the only test exercising it supplies its own set."* I closed #661 without running that check and am filing the remainder here rather than leaving the criterion quietly unmet. ## Why it is worth pinning even though the value is empty The empty set is not an accident; it is the correct production value, and the comment above it explains why — scanning member tabs would demote the lead to a worker. That is the point. A future edit that "hardens" the scanner by putting a label back in would **reintroduce the demotion bug** and no test would object. The argument encodes a real decision, so it needs an assertion. Note the asymmetry with the hazard #661 fixed. There, a non-empty set would have been a *defence*. Here, a non-empty set is a *defect*. Anyone reading the parameter name alone will guess wrong about which direction is safe, which is the strongest reason to write the test. ## Acceptance criteria 1. A test asserts the exclusion set that `FleetdAssembly`'s production boot path actually passes to `LeadTabScanner`. Mutating `Set.of()` to any non-empty set at `FleetdAssembly.java:265` must turn that test **red**. Report the mutant output, not only the green run. 2. The test must instantiate or observe the production producer, not supply its own set. A test that constructs `LeadTabScanner` directly repeats the existing blind spot at `LeadTabScannerTest.java:163`. 3. Report the complement too: confirm the new test is the one that goes red, by naming the failing test class and method from the mutant run. A mutation that reddens some unrelated test is not evidence this site is pinned. 4. Do not change `FleetdAssembly.java:265` itself. `Set.of()` is correct. This ticket adds a test, not a behaviour change. ## Not in scope The `wiki/11-Features.md:678-680` correction and the feature entry for the new validator are still owed from #661 and are lead-only, because `wiki/` is a submodule. Also still open: #668, the missing `validateAll` reachability case for `validateLeadRollover`.
ltms commented 2026-10-03 20:29:43 +02:00
Author
Owner
Copy Link
Copy Source

Merged locally as 7f9a9c0 and pushed. PR 671 closed by hand.

What I re-ran myself rather than taking from the report

  • Green: Tests run: 1928, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS, 171 surefire report files — one class more than main's 170, which is the control proving the new test actually ran.
  • Kill: same one-line mutation at FleetdAssembly.java:265, git diff --numstat showing 1 file / 1 line. Result Tests run: 1928, Failures: 1, BUILD FAILURE, and grepping every <<< FAILURE/<<< ERROR line gives exactly one test: FleetdAssemblyLeadTabScannerExclusionTest.productionBootPathPassesNoExcludedWorkspaceLabels. Nothing unrelated reddened, so the kill is attributable to this test alone — criterion 3 satisfied.
  • Revert: worktree byte-identical to HEAD, line 265 back to Set.of().

The worker's control run on clean main (1927 green) matches the number I measured when filing this ticket, from a separate run. That is a genuine second data point, because the two runs differ in the thing being trusted.

On the design: reflection was the right call here

The test reflects two private fields — LeadCoordLoop.leads and LeadTabScanner.excludedWorkspaceLabels. I checked the failure mode before accepting that, because a reflective test that cannot find its field is only useful if it fails loudly. It does: the method declares throws Exception, there is no catch anywhere in the file, so a renamed field raises NoSuchFieldException and the test errors. It cannot silently pass. (Read from the code; I did not run a rename to confirm.)

The alternative was adding production accessors purely so a test could read them, which this project has paid for before. Two loud control assertions carry the soundness instead: LeadCoordLoop must be non-null, and the supplier must be a real LeadTabScanner rather than the Map::of fallback — without that second one the test would pass for the wrong reason on any config that skips the lead-scan branch.

It also never binds a real port, so it cannot collide with the live daemon on 8765.

The reported shape instance was real, and worse than this one

The brief asked for other instances of the same shape, reported and not fixed. The worker named FleetdAssembly.java:481, which passes FleetMcp.AuthorizationMode.ENFORCED as a literal.

I measured it: flipping it to UNENFORCED leaves all 1928 tests green, BUILD SUCCESS, not one test red. That literal is the on/off switch for the entire role table (FleetMcp.java:410), so the whole primary-only capability set can be disabled in production with no test objecting.

Filed as #672. That asking-for-the-shape line earned its place in the brief — the follow-on finding is more serious than the gap this ticket closed.

Closing.

Merged locally as **7f9a9c0** and pushed. PR 671 closed by hand. ## What I re-ran myself rather than taking from the report - **Green**: `Tests run: 1928, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`, **171** surefire report files — one class more than main's 170, which is the control proving the new test actually ran. - **Kill**: same one-line mutation at `FleetdAssembly.java:265`, `git diff --numstat` showing 1 file / 1 line. Result `Tests run: 1928, Failures: 1`, `BUILD FAILURE`, and grepping every `<<< FAILURE`/`<<< ERROR` line gives **exactly one** test: `FleetdAssemblyLeadTabScannerExclusionTest.productionBootPathPassesNoExcludedWorkspaceLabels`. Nothing unrelated reddened, so the kill is attributable to this test alone — criterion 3 satisfied. - **Revert**: worktree byte-identical to `HEAD`, line 265 back to `Set.of()`. The worker's control run on clean `main` (1927 green) matches the number I measured when filing this ticket, from a separate run. That is a genuine second data point, because the two runs differ in the thing being trusted. ## On the design: reflection was the right call here The test reflects two private fields — `LeadCoordLoop.leads` and `LeadTabScanner.excludedWorkspaceLabels`. I checked the failure mode before accepting that, because a reflective test that cannot find its field is only useful if it fails loudly. It does: the method declares `throws Exception`, there is no `catch` anywhere in the file, so a renamed field raises `NoSuchFieldException` and the test errors. It cannot silently pass. (Read from the code; I did not run a rename to confirm.) The alternative was adding production accessors purely so a test could read them, which this project has paid for before. Two loud control assertions carry the soundness instead: `LeadCoordLoop` must be non-null, and the supplier must be a real `LeadTabScanner` rather than the `Map::of` fallback — without that second one the test would pass for the wrong reason on any config that skips the lead-scan branch. It also never binds a real port, so it cannot collide with the live daemon on 8765. ## The reported shape instance was real, and worse than this one The brief asked for other instances of the same shape, reported and not fixed. The worker named `FleetdAssembly.java:481`, which passes `FleetMcp.AuthorizationMode.ENFORCED` as a literal. I measured it: flipping it to `UNENFORCED` leaves **all 1928 tests green**, `BUILD SUCCESS`, not one test red. That literal is the on/off switch for the entire role table (`FleetMcp.java:410`), so the whole primary-only capability set can be disabled in production with no test objecting. Filed as **#672**. That asking-for-the-shape line earned its place in the brief — the follow-on finding is more serious than the gap this ticket closed. Closing.
ltms closed this issue 2026-10-03 20:29:46 +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#670
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?