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

fleetd #463: default listFleet callerIsPrimary to false, fail closed #467

Closed
agent wants to merge 0 commits from worker/463-listfleet-default-fails-open-f1c76c-11 into main
pull from: worker/463-listfleet-default-fails-open-f1c76c-11
merge into: fleet:main
Conversation 1 Commits - Files Changed -
agent commented 2026-09-10 13:55:41 +02:00
Member
Copy Link
Copy Source

fleetd #463 — listFleet's compat overloads defaulted callerIsPrimary to true (a fail-open default), exposing the lead-to-lead coordinator row to any caller that forgot the argument.

1. FleetMcp.java:1373 (the last compat overload, before the canonical one) now passes false instead of true at its delegation to the canonical listFleet overload. Updated the two javadoc comments on that method and on the canonical method that described the old true-everywhere assumption.

2. Whole suite green. Ran in fleetd/:

mvn -B clean test
rc=0

Final totals: Tests run: 1584, Failures: 0, Errors: 0, Skipped: 0 — BUILD SUCCESS.

Six FleetMcpTest methods relied on the implicit true default to see the coordinator row at all, and now pass true explicitly through the canonical (12-arg, boolean-taking) overload:

  • listReportsThisDaemonsOwnCoordIdWhenLeadCoordinationIsOn
  • listReportsAnUnresolvedSelfProbeAsUnknownNeverAsAMeasuredZero
  • listReportsHeldMessagesWithATruncatedPreviewNeverTheFullBody
  • listReportsAnHonestHeldCountAndDurabilityNotJustPendingZero
  • listReportsHeldDurableFalseWhenTheChannelSaysMailIsNotDurable
  • listReportsEachDeclaredPeersLiveReachability

A 7th test also broke that the ticket's hint list did not name: listIsByteForByteUnchangedForThePrimaryCaller. Its own premise was comparing the implicit-default call path (a "pre-#439 overload") against an explicit-true call path, asserting they were byte-for-byte equal. That implicit-default path is exactly the shape of the bug this ticket fixes, so the comparison could no longer hold by design — the pre-#439 overload no longer stands in for "primary caller" once the default fails closed. I rewrote the test to construct both sides through the canonical overload with an explicit true, dropped the now-meaningless equality assertion, and kept the content assertions (updated the javadoc to explain why).

3. New test, broken and restored to prove it pins the default:
listCompatOverloadWithNoCallerIsPrimaryArgumentOmitsTheCoordinatorKey calls a compat overload with no boolean argument at all, against a fully-configured lead channel (so the row is capable of being assembled), and asserts the coordinator key is absent — not present-and-empty.

Broke it by putting true back at FleetMcp.java:1373 and ran just that test:

mvn -B -Dtest=FleetMcpTest#listCompatOverloadWithNoCallerIsPrimaryArgumentOmitsTheCoordinatorKey test
rc=1

org.opentest4j.AssertionFailedError: no callerIsPrimary argument must fail closed (absent), not open (present): {"leads":[],"members":[],"healthCoverage":"off","coordinator":{"selfId":"mac-opus",...}} ==> expected: <false> but was: <true>

Restored false at FleetMcp.java:1373 and re-ran the whole suite: green again, Tests run: 1584, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS, rc=0.

Out of scope, noted not fixed: grepped FleetMcp.java for other boolean-defaulting overload chains of this shape (a compat overload silently defaulting a caller-identity/visibility flag) — found none besides this listFleet chain. Did not touch Authz, REST paths, coordinatorVisibleTo, or heldView's 80-char cap.

fleetd #463 — `listFleet`'s compat overloads defaulted `callerIsPrimary` to `true` (a fail-open default), exposing the lead-to-lead `coordinator` row to any caller that forgot the argument. **1. `FleetMcp.java:1373`** (the last compat overload, before the canonical one) now passes `false` instead of `true` at its delegation to the canonical `listFleet` overload. Updated the two javadoc comments on that method and on the canonical method that described the old `true`-everywhere assumption. **2. Whole suite green.** Ran in `fleetd/`: ``` mvn -B clean test rc=0 ``` Final totals: `Tests run: 1584, Failures: 0, Errors: 0, Skipped: 0` — `BUILD SUCCESS`. Six `FleetMcpTest` methods relied on the implicit `true` default to see the coordinator row at all, and now pass `true` explicitly through the canonical (12-arg, boolean-taking) overload: - `listReportsThisDaemonsOwnCoordIdWhenLeadCoordinationIsOn` - `listReportsAnUnresolvedSelfProbeAsUnknownNeverAsAMeasuredZero` - `listReportsHeldMessagesWithATruncatedPreviewNeverTheFullBody` - `listReportsAnHonestHeldCountAndDurabilityNotJustPendingZero` - `listReportsHeldDurableFalseWhenTheChannelSaysMailIsNotDurable` - `listReportsEachDeclaredPeersLiveReachability` A 7th test also broke that the ticket's hint list did not name: `listIsByteForByteUnchangedForThePrimaryCaller`. Its own premise was comparing the implicit-default call path (a "pre-#439 overload") against an explicit-`true` call path, asserting they were byte-for-byte equal. That implicit-default path is exactly the shape of the bug this ticket fixes, so the comparison could no longer hold by design — the pre-#439 overload no longer stands in for "primary caller" once the default fails closed. I rewrote the test to construct both sides through the canonical overload with an explicit `true`, dropped the now-meaningless equality assertion, and kept the content assertions (updated the javadoc to explain why). **3. New test, broken and restored to prove it pins the default:** `listCompatOverloadWithNoCallerIsPrimaryArgumentOmitsTheCoordinatorKey` calls a compat overload with no boolean argument at all, against a fully-configured lead channel (so the row is capable of being assembled), and asserts the `coordinator` key is absent — not present-and-empty. Broke it by putting `true` back at `FleetMcp.java:1373` and ran just that test: ``` mvn -B -Dtest=FleetMcpTest#listCompatOverloadWithNoCallerIsPrimaryArgumentOmitsTheCoordinatorKey test rc=1 ``` ``` org.opentest4j.AssertionFailedError: no callerIsPrimary argument must fail closed (absent), not open (present): {"leads":[],"members":[],"healthCoverage":"off","coordinator":{"selfId":"mac-opus",...}} ==> expected: <false> but was: <true> ``` Restored `false` at `FleetMcp.java:1373` and re-ran the whole suite: green again, `Tests run: 1584, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`, `rc=0`. **Out of scope, noted not fixed:** grepped `FleetMcp.java` for other boolean-defaulting overload chains of this shape (a compat overload silently defaulting a caller-identity/visibility flag) — found none besides this `listFleet` chain. Did not touch `Authz`, REST paths, `coordinatorVisibleTo`, or `heldView`'s 80-char cap.
agent added 1 commit 2026-09-10 13:55:41 +02:00
fleetd #463: default listFleet's callerIsPrimary to false, fail closed
CI / contract (pull_request) Successful in 1m16s
Details
CI / build (pull_request) Successful in 2m9s
Details
7df503dfc2 
The compat overload at FleetMcp.java:1373 defaulted callerIsPrimary to a
literal true, so a caller that forgot the argument silently got the
coordinator row (this daemon's coord-id, mailbox state, held-mail previews,
peer reachability) -- lead-to-lead state fleetd #439 just gated. Flip the
default to false: a forgotten argument now yields a missing row instead of
a leaked one.

Six FleetMcpTest methods relied on the implicit true to see the coordinator
row at all; they now pass true explicitly through the canonical overload.
listIsByteForByteUnchangedForThePrimaryCaller's own premise (comparing the
implicit-default path against an explicit-true path) was the shape of the
bug, so it now only exercises the explicit-true path.

Added listCompatOverloadWithNoCallerIsPrimaryArgumentOmitsTheCoordinatorKey
to pin the new default: a compat overload called with no callerIsPrimary
argument, against a fully-configured lead channel, must produce a result
with the coordinator key absent -- not empty, not redacted, absent.
ltms commented 2026-09-10 14:19:57 +02:00
Owner
Copy Link
Copy Source

Merged as 235644c. Closing by hand — a local --no-ff merge plus push did not trip the forge's auto-close.

Proof the branch is in main: git merge-base --is-ancestor origin/worker/463-listfleet-default-fails-open-f1c76c-11 origin/main succeeds, branch tip 7df503d.

Full verification is on #463: the funnel walked declaration by declaration (7 declarations, 7 real calls, exactly one writing a literal and it writes false), three mutations all killed, and 1584 green. The rewrite of listIsByteForByteUnchangedForThePrimaryCaller was the right call and M2 proves it did not open a hole.

Merged as **`235644c`**. Closing by hand — a local `--no-ff` merge plus push did not trip the forge's auto-close. Proof the branch is in `main`: `git merge-base --is-ancestor origin/worker/463-listfleet-default-fails-open-f1c76c-11 origin/main` succeeds, branch tip `7df503d`. Full verification is on #463: the funnel walked declaration by declaration (7 declarations, 7 real calls, exactly one writing a literal and it writes `false`), three mutations all killed, and 1584 green. The rewrite of `listIsByteForByteUnchangedForThePrimaryCaller` was the right call and M2 proves it did not open a hole.
ltms closed this pull request 2026-09-10 14:20:17 +02:00
Some checks are pending
CI / contract (pull_request) Successful in 1m16s
Details
CI / build (pull_request) Successful in 2m9s
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
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#467
Delete Branch "worker/463-listfleet-default-fails-open-f1c76c-11"

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?