fleetd #421: let a lead peek its own held peer mail, primary-only #438

Merged

ltms merged 2 commits from worker/421-lead-peek-held-msgs-cdbad2-10 into main 2026-09-10 09:08:18 +02:00

Conversation 0 Commits 2 Files Changed 6

+271 -25

agent commented 2026-09-10 08:56:33 +02:00

Member

Copy Link

Copy Source

Closes #421.

The defect

fleet_list reported held lead-to-lead messages under coordinator.held[] with only a truncated 80-char preview — never the full body. fleet_poll{target: coordId} returned [] silently (it drains a worker's reply inbox, not the coordinator mailbox — the wrong route entirely). fleet_ack would have destroyed the message unread. A lead had no way to read its own held peer mail safely.

Tool shape chosen, and why

Added a non-destructive read via fleet_poll{coordId} — not a fleet_list full-body mode. A fleet_list full-body mode was raised and withdrawn in the ticket's own comments: it would contradict fleet_list's deliberate truncation (HELD_PREVIEW_MAX_CHARS), which exists specifically so a roster scan never dumps a coordination body. Routing the full read through fleet_poll instead keeps fleet_list a cheap, always-safe scan and makes the one place that can return a full body an explicit, separately-authorized call.

fleet_poll(target, coordId) now branches on two independent arguments:

• coordId set → peek (never ack) this daemon's own held lead-to-lead mail
• target set (no coordId) → drain a worker's reply inbox (unchanged, DRAIN)
• neither → poll a ticket (unchanged, READ)

pollAction(String target, String coordId) got an actual signature change, not an additive overload — every one of its 9 call sites (production + tests) now states explicitly what it passes. This was deliberate per the ticket: an overload would let a caller keep passing the old single argument and silently miss the new branch.

coordId must equal the caller's own leadChannel.selfCoordId(). Passing a peer's coord-id is refused with a reason (coordId "X" is not this daemon's own coord-id...) instead of repeating the original bug's silent wrong-inbox behavior — this directly refuses the confusion that caused the ticket in the first place (a lead passing a peer's id expecting to read that peer's mail, which this system has no route for).

The new Authz action

/**
 * Read (never ack) this daemon's own held lead-to-lead coordination mail (fleetd #421).
 *
 * <p>Deliberately <strong>not</strong> folded into {@link #READ}. {@code READ}'s grant
 * rests on "the roster carries no secrets" (see its case below) — a lead-to-lead body is
 * not the roster; it is where leads discuss host shapes, credentials and unmerged work.
 * Mapping this to {@code READ} would let any worker read every peer lead's mail in full
 * and would silently falsify that comment for every other {@code READ} caller.
 */
COORD_READ,

Its switch row in Authz.permits:

// fleetd #421: reading held lead-to-lead mail is the primary's alone. An architect
// holds READ today (CB-548), so "not primary" must mean not-architect here too — this
// is coordination between leads, not observation of the roster.
case COORD_READ -> caller.isPrimary();

The switch has no default — this is a forcing function: the build does not compile until every Authz.Action has an explicit case, so COORD_READ could not be silently left unhandled.

The "pending: 0" trap

mailbox.pending only counts broker-ready (unacked-but-deliverable) messages — a held, durably-queued message doesn't show there, so a perfectly healthy lead with real mail waiting reads as pending: 0, inviting the wrong conclusion that nothing is there or that it's in-memory-only. coordinatorView now also reports heldCount (the real count from leadChannel.peek()) and heldDurable: true (AMQP consumption here is manual-ack, LeadMailbox.java:196, so held mail survives a daemon restart) as siblings of mailbox, not folded into it — mailboxView is shared with peerView, where a held-count concept doesn't apply (you can't peek() a peer's mailbox).

Tests added

• AuthzTest.coordReadIsThePrimarysAloneNotAWidenedRead — primary allowed, worker and architect refused.
• FleetMcpAuthzTest.pollingByCoordIdIsACoordReadNeverAPlainRead — pins pollAction's actual branch selection over both arguments.
• FleetMcpAuthzTest.aWorkerAndAnArchitectMayNotReadHeldPeerMailOnlyThePrimaryMay — the ticket's most important test: worker refused, architect refused, primary allowed, through the real deny/handler path.
• FleetMcpAuthzTest.everyRegisteredToolHasItsHandlerActionPinned updated to cover the new fleet_poll{coordId} mapping — no exclusion added.
• FleetMcpTest.pollWithCoordIdReturnsTheFullBodyWithoutAckingAndLeavesItHeld — reads twice, same full bodies both times, message still in held[] afterward, channel.acked() stays empty.
• FleetMcpTest.pollWithCoordIdRefusesAPeersCoordIdInsteadOfReturningTheWrongMailOrNothing — passing a peer's coord-id is refused, and the body is never leaked into the error text.
• FleetMcpTest.pollWithCoordIdErrorsHonestlyWhenLeadCoordinationIsNotConfigured.
• FleetMcpTest.listReportsAnHonestHeldCountAndDurabilityNotJustPendingZero.
• FleetMcpTest.listReportsHeldMessagesWithATruncatedPreviewNeverTheFullBody — strengthened (see mutation proof below).

Build — full, unpiped, run myself

mvn -f /Users/dai.ha/LTMS/.bridged-worktrees/12fa88-10/fleetd/pom.xml clean install
...
Tests run: 1546, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

(main's baseline before this branch was 1539 — +7 matches the 7 new test methods: 1 in AuthzTest, 2 in FleetMcpAuthzTest, 4 new + 1 strengthened in FleetMcpTest.)

Mutation proof

Each mutation applied one at a time to a clean checkout of this branch, targeted tests run, exact failing test(s) recorded, then reverted from a backup before the next. Restored state confirmed byte-identical to the backup after each revert; final git diff --stat against this PR's intended 6 files shows no leftovers.

1. FleetMcp.java:849 — pollAction's coordId branch mapped to Authz.Action.READ instead of COORD_READ:

   return Authz.Action.READ; // MUTATION fleetd #421: should be Authz.Action.COORD_READ
   

   Failed exactly: FleetMcpAuthzTest.everyRegisteredToolHasItsHandlerActionPinned, FleetMcpAuthzTest.aWorkerAndAnArchitectMayNotReadHeldPeerMailOnlyThePrimaryMay, FleetMcpAuthzTest.pollingByCoordIdIsACoordReadNeverAPlainRead. Nothing else in the 111-test authz run moved.

2. FleetMcp.java, pollHeldPeerMail — made the peek ack:

   List<LeadMessage> held = leadChannel.peek();
   held.forEach(m -> leadChannel.ack(m.msgId())); // MUTATION fleetd #421: a read must never ack
   return text(json(held.stream().map(FleetMcp::heldMailView).toList()));
   

   Failed exactly: FleetMcpTest.pollWithCoordIdReturnsTheFullBodyWithoutAckingAndLeavesItHeld:748 (expected: <[...one held message...]> but was: <[]> on the second read). 80 other FleetMcpTest tests stayed green.

3. FleetMcp.java:1445 — widened the preview cap:

   private static final int HELD_PREVIEW_MAX_CHARS = 81; // MUTATION fleetd #421: preview must stay capped at 80
   

   First attempt at this test was too weak to catch it — it used a homogeneous "x".repeat(200) body, and out.contains("x".repeat(80) + "…") still matched the widened 81-char preview one character later, because every character was 'x' (a mutation that applies and passes looks exactly like one that never applied). Fixed by putting a sentinel character ("Y") exactly at index 80 — the first character a widened cap would leak — and asserting it never appears. Re-ran against the mutation: failed exactly FleetMcpTest.listReportsHeldMessagesWithATruncatedPreviewNeverTheFullBody:701, with the sentinel visible in the actual output. Re-ran against the unmutated code first to confirm the strengthened test is still green there.

4. Authz.java:85 — the authz-case-level mutation requested for this report, widening COORD_READ to include workers:

   case COORD_READ -> caller.isPrimary() || caller.isWorker(); // MUTATION fleetd #421: must be primary-only
   

   Failed exactly: AuthzTest.coordReadIsThePrimarysAloneNotAWidenedRead:124 (a worker must not read held lead-to-lead mail ==> expected: <false> but was: <true>) and FleetMcpAuthzTest.aWorkerAndAnArchitectMayNotReadHeldPeerMailOnlyThePrimaryMay:306. All other Authz/FleetMcpAuthz tests (28 of 30) stayed green.

All four mutations reverted; git diff --stat confirms the working tree matches the 6 committed files with no MUTATION markers remaining; final full mvn clean install above is post-revert.

CLAUDE.md / wiki note (for the lead to carry over)

Added a row to the primary's intent→tool table in CLAUDE.md (worker cannot touch wiki/ — it's a submodule):

Intent	Tool
Read your own held lead-to-lead mail (no ack)	fleet_poll{coordId: <your own coord-id, from fleet_list's coordinator.selfId>} — primary-only; never acks, so fleet_list's held[] still shows it after. fleet_list's held[] gives only a truncated preview — this is the only way to read the full body

This desyncs CLAUDE.md's canonical block from wiki/7-Use-Cases.md's template copy (verified by the python snippet CLAUDE.md itself documents). Please carry this row into the wiki template manually.

Suggested wiki/11-Features.md-style entry (also not committable from this worktree):

Read a lead's own held peer mail. fleet_poll{coordId} — primary-only. The knob: pass your own coord-id (from fleet_list's coordinator.selfId) as fleet_poll's coordId argument. Why it exists: fleet_list's held[] only ever shows an 80-char preview, by design, and fleet_ack would destroy the message before you'd read it — this is the one non-destructive path to the full body. The gotcha: passing a peer's coord-id (expecting to read their mail) is refused, not silently empty — there is no such route; you can only ever read your own held mail.

One thing noticed, not investigated (per the brief, left alone)

Authz.Action.READ's case comment ("the roster carries no secrets") is the exact claim this ticket had to carve COORD_READ out from for message bodies — but fleet_list's coordinator row, still gated on plain READ, already hands every worker cross-host topology (peer coord-ids, mailbox existence/pending state, and now this PR's heldCount/heldDurable) that is coordination metadata, not roster data. Noting only, per scope — not fixing.

Out of scope (noted, not investigated)

None encountered beyond the above.

Closes #421. ## The defect `fleet_list` reported held lead-to-lead messages under `coordinator.held[]` with only a truncated 80-char `preview` — never the full body. `fleet_poll{target: coordId}` returned `[]` silently (it drains a worker's reply inbox, not the coordinator mailbox — the wrong route entirely). `fleet_ack` would have destroyed the message unread. A lead had no way to read its own held peer mail safely. ## Tool shape chosen, and why Added a **non-destructive read via `fleet_poll{coordId}`** — not a `fleet_list` full-body mode. A `fleet_list` full-body mode was raised and withdrawn in the ticket's own comments: it would contradict `fleet_list`'s deliberate truncation (`HELD_PREVIEW_MAX_CHARS`), which exists specifically so a roster scan never dumps a coordination body. Routing the full read through `fleet_poll` instead keeps `fleet_list` a cheap, always-safe scan and makes the one place that *can* return a full body an explicit, separately-authorized call. `fleet_poll(target, coordId)` now branches on two independent arguments: - `coordId` set → peek (never ack) this daemon's own held lead-to-lead mail - `target` set (no `coordId`) → drain a worker's reply inbox (unchanged, `DRAIN`) - neither → poll a ticket (unchanged, `READ`) `pollAction(String target, String coordId)` got an actual **signature change**, not an additive overload — every one of its 9 call sites (production + tests) now states explicitly what it passes. This was deliberate per the ticket: an overload would let a caller keep passing the old single argument and silently miss the new branch. `coordId` must equal the caller's own `leadChannel.selfCoordId()`. Passing a peer's coord-id is refused with a reason (`coordId "X" is not this daemon's own coord-id...`) instead of repeating the original bug's silent wrong-inbox behavior — this directly refuses the confusion that caused the ticket in the first place (a lead passing a peer's id expecting to read that peer's mail, which this system has no route for). ## The new Authz action ```java /** * Read (never ack) this daemon's own held lead-to-lead coordination mail (fleetd #421). * * <p>Deliberately <strong>not</strong> folded into {@link #READ}. {@code READ}'s grant * rests on "the roster carries no secrets" (see its case below) — a lead-to-lead body is * not the roster; it is where leads discuss host shapes, credentials and unmerged work. * Mapping this to {@code READ} would let any worker read every peer lead's mail in full * and would silently falsify that comment for every other {@code READ} caller. */ COORD_READ, ``` Its switch row in `Authz.permits`: ```java // fleetd #421: reading held lead-to-lead mail is the primary's alone. An architect // holds READ today (CB-548), so "not primary" must mean not-architect here too — this // is coordination between leads, not observation of the roster. case COORD_READ -> caller.isPrimary(); ``` The switch has no `default` — this is a forcing function: the build does not compile until every `Authz.Action` has an explicit case, so `COORD_READ` could not be silently left unhandled. ## The "pending: 0" trap `mailbox.pending` only counts broker-ready (unacked-but-deliverable) messages — a held, durably-queued message doesn't show there, so a perfectly healthy lead with real mail waiting reads as `pending: 0`, inviting the wrong conclusion that nothing is there or that it's in-memory-only. `coordinatorView` now also reports `heldCount` (the real count from `leadChannel.peek()`) and `heldDurable: true` (AMQP consumption here is manual-ack, `LeadMailbox.java:196`, so held mail survives a daemon restart) as siblings of `mailbox`, not folded into it — `mailboxView` is shared with `peerView`, where a held-count concept doesn't apply (you can't `peek()` a peer's mailbox). ## Tests added - `AuthzTest.coordReadIsThePrimarysAloneNotAWidenedRead` — primary allowed, worker and architect refused. - `FleetMcpAuthzTest.pollingByCoordIdIsACoordReadNeverAPlainRead` — pins `pollAction`'s actual branch selection over both arguments. - `FleetMcpAuthzTest.aWorkerAndAnArchitectMayNotReadHeldPeerMailOnlyThePrimaryMay` — the ticket's most important test: worker refused, architect refused, primary allowed, through the real `deny`/handler path. - `FleetMcpAuthzTest.everyRegisteredToolHasItsHandlerActionPinned` updated to cover the new `fleet_poll{coordId}` mapping — no exclusion added. - `FleetMcpTest.pollWithCoordIdReturnsTheFullBodyWithoutAckingAndLeavesItHeld` — reads twice, same full bodies both times, message still in `held[]` afterward, `channel.acked()` stays empty. - `FleetMcpTest.pollWithCoordIdRefusesAPeersCoordIdInsteadOfReturningTheWrongMailOrNothing` — passing a peer's coord-id is refused, and the body is never leaked into the error text. - `FleetMcpTest.pollWithCoordIdErrorsHonestlyWhenLeadCoordinationIsNotConfigured`. - `FleetMcpTest.listReportsAnHonestHeldCountAndDurabilityNotJustPendingZero`. - `FleetMcpTest.listReportsHeldMessagesWithATruncatedPreviewNeverTheFullBody` — strengthened (see mutation proof below). ## Build — full, unpiped, run myself ``` mvn -f /Users/dai.ha/LTMS/.bridged-worktrees/12fa88-10/fleetd/pom.xml clean install ... Tests run: 1546, Failures: 0, Errors: 0, Skipped: 0 BUILD SUCCESS ``` (main's baseline before this branch was 1539 — +7 matches the 7 new test methods: 1 in `AuthzTest`, 2 in `FleetMcpAuthzTest`, 4 new + 1 strengthened in `FleetMcpTest`.) ## Mutation proof Each mutation applied one at a time to a clean checkout of this branch, targeted tests run, exact failing test(s) recorded, then reverted from a backup before the next. Restored state confirmed byte-identical to the backup after each revert; final `git diff --stat` against this PR's intended 6 files shows no leftovers. 1. **`FleetMcp.java:849`** — `pollAction`'s `coordId` branch mapped to `Authz.Action.READ` instead of `COORD_READ`: ```java return Authz.Action.READ; // MUTATION fleetd #421: should be Authz.Action.COORD_READ ``` Failed exactly: `FleetMcpAuthzTest.everyRegisteredToolHasItsHandlerActionPinned`, `FleetMcpAuthzTest.aWorkerAndAnArchitectMayNotReadHeldPeerMailOnlyThePrimaryMay`, `FleetMcpAuthzTest.pollingByCoordIdIsACoordReadNeverAPlainRead`. Nothing else in the 111-test authz run moved. 2. **`FleetMcp.java`, `pollHeldPeerMail`** — made the peek ack: ```java List<LeadMessage> held = leadChannel.peek(); held.forEach(m -> leadChannel.ack(m.msgId())); // MUTATION fleetd #421: a read must never ack return text(json(held.stream().map(FleetMcp::heldMailView).toList())); ``` Failed exactly: `FleetMcpTest.pollWithCoordIdReturnsTheFullBodyWithoutAckingAndLeavesItHeld:748` (`expected: <[...one held message...]> but was: <[]>` on the second read). 80 other `FleetMcpTest` tests stayed green. 3. **`FleetMcp.java:1445`** — widened the preview cap: ```java private static final int HELD_PREVIEW_MAX_CHARS = 81; // MUTATION fleetd #421: preview must stay capped at 80 ``` **First attempt at this test was too weak to catch it** — it used a homogeneous `"x".repeat(200)` body, and `out.contains("x".repeat(80) + "…")` still matched the widened 81-char preview one character later, because every character was `'x'` (a mutation that applies and passes looks exactly like one that never applied). Fixed by putting a sentinel character (`"Y"`) exactly at index 80 — the first character a widened cap would leak — and asserting it never appears. Re-ran against the mutation: failed exactly `FleetMcpTest.listReportsHeldMessagesWithATruncatedPreviewNeverTheFullBody:701`, with the sentinel visible in the actual output. Re-ran against the unmutated code first to confirm the strengthened test is still green there. 4. **`Authz.java:85`** — the authz-case-level mutation requested for this report, widening `COORD_READ` to include workers: ```java case COORD_READ -> caller.isPrimary() || caller.isWorker(); // MUTATION fleetd #421: must be primary-only ``` Failed exactly: `AuthzTest.coordReadIsThePrimarysAloneNotAWidenedRead:124` (`a worker must not read held lead-to-lead mail ==> expected: <false> but was: <true>`) and `FleetMcpAuthzTest.aWorkerAndAnArchitectMayNotReadHeldPeerMailOnlyThePrimaryMay:306`. All other Authz/FleetMcpAuthz tests (28 of 30) stayed green. All four mutations reverted; `git diff --stat` confirms the working tree matches the 6 committed files with no `MUTATION` markers remaining; final full `mvn clean install` above is post-revert. ## CLAUDE.md / wiki note (for the lead to carry over) Added a row to the primary's intent→tool table in `CLAUDE.md` (worker cannot touch `wiki/` — it's a submodule): | Intent | Tool | |---|---| | Read your own held lead-to-lead mail (no ack) | `fleet_poll{coordId: <your own coord-id, from fleet_list's coordinator.selfId>}` — primary-only; never acks, so `fleet_list`'s `held[]` still shows it after. `fleet_list`'s `held[]` gives only a truncated preview — this is the only way to read the full body | This desyncs `CLAUDE.md`'s canonical block from `wiki/7-Use-Cases.md`'s template copy (verified by the python snippet `CLAUDE.md` itself documents). Please carry this row into the wiki template manually. Suggested wiki/11-Features.md-style entry (also not committable from this worktree): > **Read a lead's own held peer mail.** `fleet_poll{coordId}` — primary-only. The knob: pass your own coord-id (from `fleet_list`'s `coordinator.selfId`) as `fleet_poll`'s `coordId` argument. Why it exists: `fleet_list`'s `held[]` only ever shows an 80-char preview, by design, and `fleet_ack` would destroy the message before you'd read it — this is the one non-destructive path to the full body. The gotcha: passing a peer's coord-id (expecting to read *their* mail) is refused, not silently empty — there is no such route; you can only ever read your own held mail. ## One thing noticed, not investigated (per the brief, left alone) `Authz.Action.READ`'s case comment ("the roster carries no secrets") is the exact claim this ticket had to carve `COORD_READ` out from for message *bodies* — but `fleet_list`'s `coordinator` row, still gated on plain `READ`, already hands every worker cross-host topology (peer coord-ids, mailbox existence/pending state, and now this PR's `heldCount`/`heldDurable`) that is coordination metadata, not roster data. Noting only, per scope — not fixing. ## Out of scope (noted, not investigated) None encountered beyond the above.

agent added 1 commit 2026-09-10 08:56:34 +02:00

fleetd #421: let a lead peek its own held peer mail, primary-only ... 1e9b2c9b7e

fleet_list truncated held lead-to-lead messages to an 80-char preview with
no way to read the full body, and fleet_poll{target} drained the wrong
inbox (a worker's reply queue, not the coordinator mailbox) -- it silently
returned []. fleet_ack would have destroyed the message unread.

Add a non-destructive read: fleet_poll{coordId} peeks (never acks) this
daemon's own held mail via LeadChannel.peek(). The coordId must equal the
caller's own selfCoordId -- passing a peer's id is refused with a reason,
instead of repeating the original silent-[] confusion.

This is authorization-sensitive: mapping it to the existing READ action
would let any worker read every peer lead's mail in full. READ's openness
rests on "the roster carries no secrets" (Authz.java), which does not hold
for lead-to-lead coordination bodies. Added Authz.Action.COORD_READ,
primary-only (not even the architect, which holds READ today), and made
pollAction's signature depend on both target and coordId so every call
site states explicitly what it passes.

Also fixes fleet_list's "pending: 0" trap: mailbox.pending only counts
broker-ready messages, so a healthy held mailbox reads as empty. Added
heldCount/heldDurable beside held[] so the durability fact isn't implied
only by reading the code.

Mutation-tested: pollAction's COORD_READ->READ mapping, the peek->ack
substitution, the 80-char preview cap widened to 81, and the Authz case
widened to include caller.isWorker() -- each breaks exactly its matching
test and nothing else. The first attempt at the preview-cap test used a
homogeneous "x"*200 body, which a widened cap slipped through unnoticed
(contains() found a shifted match); replaced with a sentinel character at
index 80 to actually pin the boundary.

Updates CLAUDE.md's intent->tool table for fleet_poll's new coordId
semantics, per this repo's own "prompt is part of the product" rule.
wiki/ is a submodule and not committable from a worker's worktree --
wiki-bound content is in the PR body instead.

ltms referenced this pull request 2026-09-10 09:07:48 +02:00

fleet_list hands every worker the coordinator row: peer coord-ids and 80-char previews of lead-to-lead bodies #439

agent added 1 commit 2026-09-10 09:08:04 +02:00

Merge main into #421 branch (brings #434 model-gate observability and #436 fixed-placement cap) 77a6a7142e

ltms merged commit 12cff28abb into main 2026-09-10 09:08:18 +02:00

ltms referenced this pull request 2026-09-10 09:21:36 +02:00

coordinatorView reports heldDurable as a hardcoded true, so it cannot go false when durability breaks #440

agent referenced this issue from a commit 2026-09-10 09:24:21 +02:00

Merge main into #425 rework branch (brings #438 held-peer-mail read)

ltms referenced this issue from a commit 2026-09-10 11:21:42 +02:00

Merge #443: derive coordinator.heldDurable from queue durability + ack mode (fleetd #440)

ltms referenced this pull request 2026-09-12 04:09:08 +02:00

LeadRollover's failure logs print the CONFIGURED budget, not the measured wait — the exact thing that hid #489 #494

ltms referenced this pull request 2026-09-12 04:09:31 +02:00

MessageServiceTest.anAlreadyCollectedTicketProducesNoNudge races the push loop's 300ms tick — reproduced on both hosts, under load #477

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

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: fleet/fleetd#438