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

fleetd #710: gate fleet_list's leads/members arrays by caller role #717

Closed
agent wants to merge 0 commits from worker/task-16-50a702-13 into main
pull from: worker/task-16-50a702-13
merge into: fleet:main
Conversation 2 Commits - Files Changed -
agent commented 2026-10-04 07:26:21 +02:00
Member
Copy Link
Copy Source

Fixes fleetd #710 finding 1: fleet_list put the leads and members keys into its JSON result unconditionally, so any worker holding only READ could read every session on the daemon (sessionId, paneId, profile, role, state, worktree, branch, agentSessionId, charter facts, live status, owner) -- including sessions belonging to other leads.

Follows the two existing precedents in FleetMcp exactly: a named static predicate (leadsVisibleTo, membersVisibleTo), consulted before the row arrays are assembled, with the key omitted entirely (never an empty array) when the predicate is false. Primary and architect callers see both arrays exactly as before; a worker and a collaborator see neither key at all.

Only the canonical listFleet implementation and the one overload that already carries caller-identity (the callerIsPrimary overload from fleetd #439) gained the two new boolean parameters. The other 8 forwarding overloads keep their existing signatures and hardcode true,true when calling the canonical overload, preserving their previous default-visible behavior and keeping the test diff scoped.

Verification run in this worktree:

  • Targeted suite (FleetMcpTest, FleetMcpAuthzTest, FleetMcpLeadContextGaugeWiringTest): 135 tests, 0 failures.
  • Manual mutation test: replaced leadsVisibleTo(principal(exchange)), membersVisibleTo(principal(exchange)) in the handler with literal true, true. mvn -q -o compile stayed green (the mutant compiles and is live). Running the targeted suite against the mutant turned exactly 2 tests red: theFleetListHandlerActuallyConsultsLeadsVisibleTo and theFleetListHandlerActuallyConsultsMembersVisibleTo. No other test moved, and no survivor. Reverted, reconfirmed 135/135 green.
  • Full project build: mvn -o clean install -> BUILD SUCCESS, Tests run: 2013, Failures: 0, Errors: 0, Skipped: 0.
  • I added 5 new @Test methods (confirmed by diff: git diff | grep -c '^+.*@Test' = 5 net new), which is consistent with the ticket's stated baseline of 2008 tests on main (2008 + 5 = 2013), though I have not independently run the build on main itself to confirm that baseline number.

Updated fleet_list's tool description to document the new gating for a worker caller.

Caveat for review: this was verified by unit test and compilation only. Nobody, including me, has probed this from a live worker MCP session end-to-end -- no runtime claim is made beyond what the tests above actually exercised.

Fixes fleetd #710 finding 1: fleet_list put the leads and members keys into its JSON result unconditionally, so any worker holding only READ could read every session on the daemon (sessionId, paneId, profile, role, state, worktree, branch, agentSessionId, charter facts, live status, owner) -- including sessions belonging to other leads. Follows the two existing precedents in FleetMcp exactly: a named static predicate (leadsVisibleTo, membersVisibleTo), consulted before the row arrays are assembled, with the key omitted entirely (never an empty array) when the predicate is false. Primary and architect callers see both arrays exactly as before; a worker and a collaborator see neither key at all. Only the canonical listFleet implementation and the one overload that already carries caller-identity (the callerIsPrimary overload from fleetd #439) gained the two new boolean parameters. The other 8 forwarding overloads keep their existing signatures and hardcode true,true when calling the canonical overload, preserving their previous default-visible behavior and keeping the test diff scoped. Verification run in this worktree: - Targeted suite (FleetMcpTest, FleetMcpAuthzTest, FleetMcpLeadContextGaugeWiringTest): 135 tests, 0 failures. - Manual mutation test: replaced `leadsVisibleTo(principal(exchange)), membersVisibleTo(principal(exchange))` in the handler with literal `true, true`. `mvn -q -o compile` stayed green (the mutant compiles and is live). Running the targeted suite against the mutant turned exactly 2 tests red: theFleetListHandlerActuallyConsultsLeadsVisibleTo and theFleetListHandlerActuallyConsultsMembersVisibleTo. No other test moved, and no survivor. Reverted, reconfirmed 135/135 green. - Full project build: `mvn -o clean install` -> BUILD SUCCESS, `Tests run: 2013, Failures: 0, Errors: 0, Skipped: 0`. - I added 5 new @Test methods (confirmed by diff: `git diff | grep -c '^+.*@Test'` = 5 net new), which is consistent with the ticket's stated baseline of 2008 tests on main (2008 + 5 = 2013), though I have not independently run the build on main itself to confirm that baseline number. Updated fleet_list's tool description to document the new gating for a worker caller. Caveat for review: this was verified by unit test and compilation only. Nobody, including me, has probed this from a live worker MCP session end-to-end -- no runtime claim is made beyond what the tests above actually exercised.
agent added 1 commit 2026-10-04 07:26:21 +02:00
fleetd #710: gate fleet_list's leads and members arrays by caller role
CI / shell-tests (pull_request) Failing after 9s
Details
CI / contract (pull_request) Successful in 51s
Details
CI / build (pull_request) Failing after 1m50s
Details
10ab58e4fc 
Omit the leads and members keys entirely (never an empty array) from
fleet_list's result for a worker, matching the existing
coordinatorVisibleTo/collaboratorsVisibleTo pattern: two new named
predicates (leadsVisibleTo, membersVisibleTo) are consulted before
assembling either array, so a worker holding only READ can no longer
read every session on the daemon through this tool. Primary and
architect callers are unaffected.
ltms commented 2026-10-04 07:31:41 +02:00
Owner
Copy Link
Copy Source

Lead review — membersVisibleTo is right, leadsVisibleTo is wrong. One change, then I merge.

Good work, and thank you for flagging your own unverified scope decision rather than presenting it
as checked. I verified the part you could not.

What I checked for you

The 8 forwarding overloads that hardcode true, true are safe. You said they looked test-only
but that you had not verified each caller. I enumerated them:

$ git grep -n "listFleet(" origin/worker/task-16-50a702-13 -- fleetd/src/main/java

Every hit is either a declaration, an internal forward between overloads, or the one handler call at
FleetMcp.java:560. No other file under src/main/java calls listFleet at all. So the handler is
the only production path, and it passes both predicates. Your scope decision was correct.

Your control move is correct. The listOmits...Coordinator... control is now
assertTrue(out.contains("\"healthCoverage\"")), moved and not deleted. That is what I wanted.

Your widened regex is still sound. I was ready to find it vacuous, because widening an anchor
usually makes it stop pinning anything. It does not here: the alternation still only accepts
coordinatorVisibleTo(principal(exchange)), true or false, and the assertEquals on group(1)
is what fails if a literal is substituted. The [,)] ending matches the coordinator argument and not
an earlier one, because no earlier argument in that call is one of the three alternatives.

main's baseline is 2008. You were right not to repeat that number as your own. I built
origin/main myself in a clean worktree: Tests run: 2008, Failures: 0, BUILD SUCCESS. So your
2008 + 5 = 2013 arithmetic holds, and the baseline half of it is now measured rather than quoted.

The one thing that must change: a collaborator must still see leads

leadsVisibleTo and membersVisibleTo are both caller.isPrimary() || caller.isArchitect(). For
members that is right. For leads it is wrong, and it breaks a shipped feature.

The reason is a rule this project has already written down, in wiki/11-Features.md, about the
sibling collaborators array:

Visible to the primary, an architect, and a collaborator — never a worker. That is exactly the
set of roles that may SEND to a named peer, so the rule is "you may list what you could address".

A collaborator may send to a lead. That is its one granted action. And leads is the only place
the bridge tells it a lead's sessionId — a collaborator's own fleet_whoami carries its registry
name and its own sessionId, and no lead address. So hiding leads from a collaborator leaves it
authorized to send to a lead with no way to learn where to send. That is a grant at one gate that
dies at another.

Applying "you may list what you could address" gives:

array visible to
leads primary, architect, collaborator
members primary, architect — a collaborator may never address a spawned member
collaborators primary, architect, collaborator (unchanged)
coordinator primary (unchanged)

So members keeps your current predicate, and the asymmetry is the point: it is what stops a
collaborator reaching a worker.

This does not reopen the settled #710 finding 1 decision. That decision is about a worker,
and a worker still sees neither array. A collaborator is not a worker.

Scope of the change — this and nothing else

  1. Make leadsVisibleTo also true for a collaborator. Leave membersVisibleTo exactly as it is.
  2. Update the two truth-table tests so the collaborator row asserts the new answer for leads and
    the unchanged answer for members. Those two rows are the whole point — do not drop them.
  3. Add or extend one behavioural test proving a collaborator's fleet_list output contains
    leads and does not contain members, with a control assertion on a key that is always
    present so it cannot pass on an empty payload.
  4. Update listTool()'s description so it matches the new rule, including the collaborator case.
  5. Re-run your mutation on the handler arguments and confirm it still fails. Then mutate
    leadsVisibleTo's body to drop the collaborator clause, mvn -o compile green first, and confirm
    a test goes red. Restore and confirm byte-identical.

Report back

Real mvn clean install numbers from fleetd/, written to a file and not piped. State the
arithmetic against 2013. End your turn with exactly one fleet_reply carrying the whole report.

## Lead review — `membersVisibleTo` is right, `leadsVisibleTo` is wrong. One change, then I merge. Good work, and thank you for flagging your own unverified scope decision rather than presenting it as checked. I verified the part you could not. ### What I checked for you **The 8 forwarding overloads that hardcode `true, true` are safe.** You said they looked test-only but that you had not verified each caller. I enumerated them: ``` $ git grep -n "listFleet(" origin/worker/task-16-50a702-13 -- fleetd/src/main/java ``` Every hit is either a declaration, an internal forward between overloads, or the one handler call at `FleetMcp.java:560`. No other file under `src/main/java` calls `listFleet` at all. So the handler is the only production path, and it passes both predicates. Your scope decision was correct. **Your control move is correct.** The `listOmits...Coordinator...` control is now `assertTrue(out.contains("\"healthCoverage\""))`, moved and not deleted. That is what I wanted. **Your widened regex is still sound.** I was ready to find it vacuous, because widening an anchor usually makes it stop pinning anything. It does not here: the alternation still only accepts `coordinatorVisibleTo(principal(exchange))`, `true` or `false`, and the `assertEquals` on `group(1)` is what fails if a literal is substituted. The `[,)]` ending matches the coordinator argument and not an earlier one, because no earlier argument in that call is one of the three alternatives. **main's baseline is 2008.** You were right not to repeat that number as your own. I built `origin/main` myself in a clean worktree: `Tests run: 2008, Failures: 0`, `BUILD SUCCESS`. So your 2008 + 5 = 2013 arithmetic holds, and the baseline half of it is now measured rather than quoted. ### The one thing that must change: a collaborator must still see `leads` `leadsVisibleTo` and `membersVisibleTo` are both `caller.isPrimary() || caller.isArchitect()`. For `members` that is right. For `leads` it is wrong, and it breaks a shipped feature. The reason is a rule this project has already written down, in `wiki/11-Features.md`, about the sibling `collaborators` array: > **Visible to the primary, an architect, and a collaborator — never a worker.** That is exactly the > set of roles that may `SEND` to a named peer, so the rule is "you may list what you could address". A collaborator **may** send to a lead. That is its one granted action. And `leads` is the only place the bridge tells it a lead's `sessionId` — a collaborator's own `fleet_whoami` carries its registry name and its own `sessionId`, and no lead address. So hiding `leads` from a collaborator leaves it authorized to send to a lead with no way to learn where to send. That is a grant at one gate that dies at another. Applying "you may list what you could address" gives: | array | visible to | |---|---| | `leads` | primary, architect, **collaborator** | | `members` | primary, architect — a collaborator may never address a spawned member | | `collaborators` | primary, architect, collaborator (unchanged) | | `coordinator` | primary (unchanged) | So `members` keeps your current predicate, and the asymmetry is the point: it is what stops a collaborator reaching a worker. **This does not reopen the settled #710 finding 1 decision.** That decision is about a **worker**, and a worker still sees neither array. A collaborator is not a worker. ### Scope of the change — this and nothing else 1. Make `leadsVisibleTo` also true for a collaborator. Leave `membersVisibleTo` exactly as it is. 2. Update the two truth-table tests so the collaborator row asserts the new answer for `leads` and the unchanged answer for `members`. Those two rows are the whole point — do not drop them. 3. Add or extend one behavioural test proving a collaborator's `fleet_list` output **contains** `leads` and **does not contain** `members`, with a control assertion on a key that is always present so it cannot pass on an empty payload. 4. Update `listTool()`'s description so it matches the new rule, including the collaborator case. 5. Re-run your mutation on the handler arguments and confirm it still fails. Then mutate `leadsVisibleTo`'s body to drop the collaborator clause, `mvn -o compile` green first, and confirm a test goes red. Restore and confirm byte-identical. ### Report back Real `mvn clean install` numbers from `fleetd/`, written to a file and not piped. State the arithmetic against 2013. End your turn with exactly one `fleet_reply` carrying the whole report.
agent added 1 commit 2026-10-04 07:38:01 +02:00
fleetd #710 PR #717 review: leadsVisibleTo must also admit a collaborator
CI / shell-tests (pull_request) Failing after 8s
Details
CI / contract (pull_request) Successful in 50s
Details
CI / build (pull_request) Failing after 1m47s
Details
d2db8c7dc9 
A collaborator may SEND to a lead, and fleet_list's leads array is the
only place this tool gives it a lead's sessionId -- its own
fleet_whoami carries no lead address. leadsVisibleTo now returns true
for caller.isCollaborator() as well as primary and architect, matching
the existing rule for the sibling collaborators array (every role that
may SEND to a named peer). membersVisibleTo is unchanged: a
collaborator may never SEND to a spawned member.

Updated the truth-table tests for both predicates, added a behavioural
test proving a collaborator's fleet_list output contains leads and not
members, and updated fleet_list's tool description.
ltms commented 2026-10-04 07:41:56 +02:00
Owner
Copy Link
Copy Source

Merged to main as f4e0ca4. Closing by hand — a local merge does not close a PR here.

The collaborator change is right, and I verified the whole thing myself rather than taking the
report.

The four rows that matter, read off d2db8c7:

predicate collaborator worker
leadsVisibleTo assertTrue assertFalse
membersVisibleTo assertFalse assertFalse

So a collaborator keeps the one address it needs, can never address a spawned member, and the
settled #710 finding 1 decision about a worker is untouched.

My build, in a throwaway worktree off main:

Tests run: 2014, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

That is 2008 + 5 + 1. The 2008 baseline is my own measurement of origin/main, not a number carried
over from a report.

My own mutation, not the one in the report. I dropped || caller.isCollaborator() from
leadsVisibleTo with a line-anchored sed, confirmed mvn -o compile exit 0 first so the mutation
was live and not a compile error, and got exactly two failures:

FleetMcpTest.listShowsLeadsButNotMembersToACollaborator
FleetMcpAuthzTest.primaryArchitectAndCollaboratorMaySeeTheLeadsArray

Then restored and confirmed the file byte-identical. Two kills, the truth-table row and the
behavioural test, which is the pair I wanted: one pins the predicate, one pins what a caller
actually receives.

I also confirmed the merge carried nothing unexpected. The merged tree is byte-identical to the
tree I built (fd3ad87), and the only delta between the merged tree and the branch tip is the
CLAUDE.md correction that landed on main separately as 95311c6.

Still owed, and it is mine, not yours

wiki/11-Features.md's Leads are visible in fleet_list entry is now wrong in three ways, and a
worker cannot fix it because wiki/ is a submodule it never gets. I am doing it:

  1. it says leads sits alongside workers — the live payload key is members
  2. it does not mention the role gating this PR adds
  3. its Why ends by saying both halves are always reported, "even when a half is empty". That is now
    deliberately false for a worker, where the key is absent — so the entry needs the hazard spelled
    out: an absent key means "you may not see this", not "there are none".

One thing you were right to leave alone

You did not re-run the CLAUDE.md ↔ wiki sync check, and you said so plainly instead of reporting it
as passed. That is correct: it is unsatisfiable in a worker worktree. I ran it in the main clone and
it prints True.

## Merged to `main` as `f4e0ca4`. Closing by hand — a local merge does not close a PR here. The collaborator change is right, and I verified the whole thing myself rather than taking the report. **The four rows that matter, read off `d2db8c7`:** | predicate | collaborator | worker | |---|---|---| | `leadsVisibleTo` | `assertTrue` | `assertFalse` | | `membersVisibleTo` | `assertFalse` | `assertFalse` | So a collaborator keeps the one address it needs, can never address a spawned member, and the settled #710 finding 1 decision about a worker is untouched. **My build, in a throwaway worktree off `main`:** ``` Tests run: 2014, Failures: 0, Errors: 0, Skipped: 0 BUILD SUCCESS ``` That is 2008 + 5 + 1. The 2008 baseline is my own measurement of `origin/main`, not a number carried over from a report. **My own mutation, not the one in the report.** I dropped `|| caller.isCollaborator()` from `leadsVisibleTo` with a line-anchored `sed`, confirmed `mvn -o compile` exit 0 first so the mutation was live and not a compile error, and got exactly two failures: ``` FleetMcpTest.listShowsLeadsButNotMembersToACollaborator FleetMcpAuthzTest.primaryArchitectAndCollaboratorMaySeeTheLeadsArray ``` Then restored and confirmed the file byte-identical. Two kills, the truth-table row and the behavioural test, which is the pair I wanted: one pins the predicate, one pins what a caller actually receives. **I also confirmed the merge carried nothing unexpected.** The merged tree is byte-identical to the tree I built (`fd3ad87`), and the only delta between the merged tree and the branch tip is the `CLAUDE.md` correction that landed on `main` separately as `95311c6`. ### Still owed, and it is mine, not yours `wiki/11-Features.md`'s *Leads are visible in fleet_list* entry is now wrong in three ways, and a worker cannot fix it because `wiki/` is a submodule it never gets. I am doing it: 1. it says `leads` sits alongside **`workers`** — the live payload key is `members` 2. it does not mention the role gating this PR adds 3. its *Why* ends by saying both halves are always reported, "even when a half is empty". That is now deliberately false for a worker, where the key is absent — so the entry needs the hazard spelled out: an absent key means "you may not see this", not "there are none". ### One thing you were right to leave alone You did not re-run the CLAUDE.md ↔ wiki sync check, and you said so plainly instead of reporting it as passed. That is correct: it is unsatisfiable in a worker worktree. I ran it in the main clone and it prints `True`.
ltms closed this pull request 2026-10-04 07:42:01 +02:00
Some checks are pending
CI / shell-tests (pull_request) Failing after 8s
Details
CI / contract (pull_request) Successful in 50s
Details
CI / build (pull_request) Failing after 1m47s
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#717
Delete Branch "worker/task-16-50a702-13"

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?