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

listFleet's compat overloads default callerIsPrimary to true, so the coordinator gate fails open #463

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

Follow-up to #439, which landed in 92a96fc (PR #462). Not a live exposure. Filed so the fail-open default is a decision on the record rather than an accident.

What is there

FleetMcp.listFleet has 7 declarations. Measured on 92a96fc:

line  1314  declares callerIsPrimary: false   passes on: selfTerm
line  1320  declares callerIsPrimary: false   passes on: CoordinationSource.none()
line  1328  declares callerIsPrimary: false   passes on: CoordinationSource.none()
line  1345  declares callerIsPrimary: false   passes on: coordination
line  1354  declares callerIsPrimary: false   passes on: coordination
line  1373  declares callerIsPrimary: false   passes on: true      <-- the default is written here
line  1395  declares callerIsPrimary: true    (the canonical method)

So the default is injected in exactly one place, FleetMcp.java:1373, and the other five overloads inherit it by delegating up the chain. An earlier note of mine said "six overloads default it to true"; six overloads reach the default, but only one writes it.

Why it is worth changing

true means "show the caller the coordinator row". A future call site that forgets the argument gets the permissive answer, and nothing says so: no test breaks, no log line appears, and the row is simply disclosed. false fails closed — a forgotten argument means a missing row, which is a visible bug someone fixes, not a silent disclosure.

Nothing is exposed today. There is one production call site, the fleet_list handler at FleetMcp.java:426, and it passes the computed value. That is now pinned by FleetMcpAuthzTest.theFleetListHandlerActuallyConsultsCoordinatorVisibleTo, a source-reading test — I checked it is not vacuous by renaming its anchor (listHandler -> listHandlerX), and it failed loudly rather than passing on an empty scrape.

The work

Change FleetMcp.java:1373 to pass false, and give the tests that need the row an explicit true.

The #439 worker measured the blast radius: exactly 6 FleetMcpTest methods rely on the default to see the coordinator row at all —

  • listReportsThisDaemonsOwnCoordIdWhenLeadCoordinationIsOn
  • ...AnUnresolvedSelfProbeAsUnknown...
  • ...HeldMessagesWithATruncatedPreview...
  • ...AnHonestHeldCountAndDurability...
  • ...HeldDurableFalseWhenTheChannelSays...
  • ...EachDeclaredPeersLiveReachability

I have not re-run that count myself; it is the worker's number and it is a straight compile-and-fix, not a design change.

Acceptance criteria

  1. FleetMcp.java:1373 passes false.
  2. Every test that needs the coordinator row passes true explicitly. Whole suite green, with the totals and the exit code checked separately from the output.
  3. One test pins the new default: call a compat overload with no boolean and assert the result has no coordinator key. Break it by putting true back and paste the failure; restore and show green. Without that step the default is unpinned again and this ticket has only moved the problem.
Follow-up to #439, which landed in `92a96fc` (PR #462). Not a live exposure. Filed so the fail-open default is a decision on the record rather than an accident. ## What is there `FleetMcp.listFleet` has 7 declarations. Measured on `92a96fc`: ``` line 1314 declares callerIsPrimary: false passes on: selfTerm line 1320 declares callerIsPrimary: false passes on: CoordinationSource.none() line 1328 declares callerIsPrimary: false passes on: CoordinationSource.none() line 1345 declares callerIsPrimary: false passes on: coordination line 1354 declares callerIsPrimary: false passes on: coordination line 1373 declares callerIsPrimary: false passes on: true <-- the default is written here line 1395 declares callerIsPrimary: true (the canonical method) ``` So the default is injected in exactly **one** place, `FleetMcp.java:1373`, and the other five overloads inherit it by delegating up the chain. An earlier note of mine said "six overloads default it to true"; six overloads *reach* the default, but only one writes it. ## Why it is worth changing `true` means "show the caller the coordinator row". A future call site that forgets the argument gets the permissive answer, and nothing says so: no test breaks, no log line appears, and the row is simply disclosed. `false` fails closed — a forgotten argument means a missing row, which is a visible bug someone fixes, not a silent disclosure. Nothing is exposed today. There is one production call site, the `fleet_list` handler at `FleetMcp.java:426`, and it passes the computed value. That is now pinned by `FleetMcpAuthzTest.theFleetListHandlerActuallyConsultsCoordinatorVisibleTo`, a source-reading test — I checked it is not vacuous by renaming its anchor (`listHandler` -> `listHandlerX`), and it failed loudly rather than passing on an empty scrape. ## The work Change `FleetMcp.java:1373` to pass `false`, and give the tests that need the row an explicit `true`. The #439 worker measured the blast radius: exactly 6 `FleetMcpTest` methods rely on the default to see the coordinator row at all — - `listReportsThisDaemonsOwnCoordIdWhenLeadCoordinationIsOn` - `...AnUnresolvedSelfProbeAsUnknown...` - `...HeldMessagesWithATruncatedPreview...` - `...AnHonestHeldCountAndDurability...` - `...HeldDurableFalseWhenTheChannelSays...` - `...EachDeclaredPeersLiveReachability` I have not re-run that count myself; it is the worker's number and it is a straight compile-and-fix, not a design change. ## Acceptance criteria 1. `FleetMcp.java:1373` passes `false`. 2. Every test that needs the coordinator row passes `true` explicitly. Whole suite green, with the totals and the exit code checked separately from the output. 3. One test pins the new default: call a compat overload with no boolean and assert the result has **no** `coordinator` key. Break it by putting `true` back and paste the failure; restore and show green. Without that step the default is unpinned again and this ticket has only moved the problem.
ltms commented 2026-09-10 14:07:20 +02:00
Author
Owner
Copy Link
Copy Source

Merged as 235644c (PR #467). Main source change is one word plus javadoc.

 return listFleet(workers, sessions, messages, capacity, healthCoverage, quarantine, outage,
-        leadSeats, leads, selfTerm, coordination, true);
+        leadSeats, leads, selfTerm, coordination, false);

The funnel, measured on the merge commit

I walked every declaration and every call rather than grepping for a shape I expected.

  • listFleet has 7 declarations.
  • There are 8 listFleet( matches outside the declarations, but one is a javadoc {@link} at :1371. So 7 real calls.
  • Exactly one call writes a literal for the new boolean: :1380, and it writes false.
  • Exactly one call writes the real predicate: the MCP handler at :426, coordinatorVisibleTo(principal(exchange)).
  • No call writes true.

So the default is written in one place, and it now fails closed.

Mutations, all on the merge commit

The merged tree is befc4ec, byte-for-byte the tree the battery ran on, so these results are about the code that is now on main.

Cell What it changes Result
CONTROL nothing 1584 green
M1 the fix reverted: coordination, false) → true) KILLED — listCompatOverloadWithNoCallerIsPrimaryArgumentOmitsTheCoordinatorKey
M2 the primary's coordinator row silently loses heldCount KILLED — listIsByteForByteUnchangedForThePrimaryCaller and listReportsAnHonestHeldCountAndDurabilityNotJustPendingZero
M3 #439's own gate: if (callerIsPrimary) → if (true) KILLED — 3 tests

M2 is the cell that mattered, and it is worth explaining why. The worker rewrote listIsByteForByteUnchangedForThePrimaryCaller and dropped its byte-for-byte equality assertion. That was the right call: the assertion compared the canonical overload against the implicit-default overload, and the implicit default is exactly the path this ticket closes. Keeping the comparison would have pinned the bug. But dropping it weakened the test, so the question I had to answer was whether anything still notices when the primary's row loses a field. It does.

What is no longer pinned — stating it plainly

The primary's fleet_list output is now checked field by field, not as one whole string. Every field the tests name is pinned, and M2 proves that. A field that no test names could disappear without failing anything. That is a real, small reduction, and the whole-output answer belongs to #460 (nothing exercises fleetd over its real transport). Not filing a separate ticket for it.

A control in my own battery was wrong

I wrote a control that counted ', true);' in FleetMcp.java and labelled it "must be 0". It printed 2: row.put("configured", true); at :1475 and m.put("self", true); at :1715. Neither is a listFleet delegation — my pattern was simply too wide to answer the question I asked it.

Recording it because the failure is instructive in the opposite direction to the usual one. The trap I write down most often is a narrow pattern giving a confident zero. This was a wide pattern giving a confident non-zero, and a wide pattern is more dangerous when the control is written as "must be 0", because a non-zero then looks like a defect on the branch instead of a defect in the control. The real counts above came from walking each declaration and joining multi-line calls.

Branch staleness, checked before merging

git diff origin/main..branch showed plans/fleet01-standup/plan.md | 369 ----- and CLAUDE.md | 5 +-, which reads at a glance like the worker deleted my work. It did not. The branch is based on 92a96fc and main had moved three commits (29e7a06, f5e02fe, eccd054). Against its own merge-base the worker touched 2 files. I confirmed the merge restored both: plan.md is 369 lines and CLAUDE.md carries the new invariant 5.

Merged as `235644c` (PR #467). Main source change is one word plus javadoc. ```java return listFleet(workers, sessions, messages, capacity, healthCoverage, quarantine, outage, - leadSeats, leads, selfTerm, coordination, true); + leadSeats, leads, selfTerm, coordination, false); ``` ## The funnel, measured on the merge commit I walked every declaration and every call rather than grepping for a shape I expected. - `listFleet` has **7 declarations**. - There are **8 `listFleet(` matches** outside the declarations, but one is a javadoc `{@link}` at :1371. So **7 real calls**. - Exactly **one** call writes a literal for the new boolean: `:1380`, and it writes `false`. - Exactly **one** call writes the real predicate: the MCP handler at `:426`, `coordinatorVisibleTo(principal(exchange))`. - **No call writes `true`.** So the default is written in one place, and it now fails closed. ## Mutations, all on the merge commit The merged tree is `befc4ec`, byte-for-byte the tree the battery ran on, so these results are about the code that is now on `main`. | Cell | What it changes | Result | |---|---|---| | CONTROL | nothing | 1584 green | | **M1** | the fix reverted: `coordination, false)` → `true)` | **KILLED** — `listCompatOverloadWithNoCallerIsPrimaryArgumentOmitsTheCoordinatorKey` | | **M2** | the primary's coordinator row silently loses `heldCount` | **KILLED** — `listIsByteForByteUnchangedForThePrimaryCaller` and `listReportsAnHonestHeldCountAndDurabilityNotJustPendingZero` | | **M3** | #439's own gate: `if (callerIsPrimary)` → `if (true)` | **KILLED** — 3 tests | **M2 is the cell that mattered**, and it is worth explaining why. The worker rewrote `listIsByteForByteUnchangedForThePrimaryCaller` and dropped its byte-for-byte equality assertion. That was the right call: the assertion compared the canonical overload against the *implicit-default* overload, and the implicit default is exactly the path this ticket closes. Keeping the comparison would have pinned the bug. But dropping it weakened the test, so the question I had to answer was whether anything still notices when the primary's row loses a field. It does. ## What is no longer pinned — stating it plainly The primary's `fleet_list` output is now checked **field by field**, not as one whole string. Every field the tests name is pinned, and M2 proves that. A field that **no test names** could disappear without failing anything. That is a real, small reduction, and the whole-output answer belongs to #460 (nothing exercises fleetd over its real transport). Not filing a separate ticket for it. ## A control in my own battery was wrong I wrote a control that counted `', true);'` in `FleetMcp.java` and labelled it "must be 0". It printed **2**: `row.put("configured", true);` at :1475 and `m.put("self", true);` at :1715. Neither is a `listFleet` delegation — my pattern was simply too wide to answer the question I asked it. Recording it because the failure is instructive in the opposite direction to the usual one. The trap I write down most often is a **narrow** pattern giving a confident zero. This was a **wide** pattern giving a confident non-zero, and a wide pattern is more dangerous when the control is written as "must be 0", because a non-zero then looks like a defect on the branch instead of a defect in the control. The real counts above came from walking each declaration and joining multi-line calls. ## Branch staleness, checked before merging `git diff origin/main..branch` showed `plans/fleet01-standup/plan.md | 369 -----` and `CLAUDE.md | 5 +-`, which reads at a glance like the worker deleted my work. It did not. The branch is based on `92a96fc` and `main` had moved three commits (`29e7a06`, `f5e02fe`, `eccd054`). Against its own merge-base the worker touched **2 files**. I confirmed the merge restored both: `plan.md` is 369 lines and `CLAUDE.md` carries the new invariant 5.
ltms closed this issue 2026-09-10 14:17:13 +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#463
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?