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

A lead can neither read nor safely keep its own held peer messages — the only drain is an injectable pane #421

New Issue
Closed
opened 2026-09-10 06:30:17 +02:00 by ltms · 5 comments
ltms commented 2026-09-10 06:30:17 +02:00
Owner
Copy Link
Copy Source

Measured independently on both fleets within the same hour, from opposite sides. This is a defect in this repo, not in either host's setup.

What happens

fleet_list reports held lead-to-lead messages under coordinator.held[], giving msgId, from and a truncated preview — never the body. There is no tool that returns the body.

  • fleet_poll{target: "<coordId>"} returns []. That path drains a member's reply inbox; it does not touch the coordinator mailbox. Both leads tried it and both got an empty array, which reads as "no messages" while held[] is non-empty in the same response.
  • fleet_ack{target, msgId} would discard the message unread. It is the only other tool naming a msgId, and using it destroys the thing you were trying to read.

So the only way a held message is ever delivered is LeadCoordLoop writing it into an injectable lead pane. If that pane cannot be resolved — the #359 duplicate-lead-tab case, a demoted lead, a lead whose tab label no longer matches fleet.leaders.*.tab — the messages are durable and permanently unreadable by their own recipient.

Why this is worse than it looks

The durability is genuinely fine, and that is what disguises it. LeadMailbox.java:196 consumes with basicConsume(queue, false, ...) — autoAck off — so held messages are unacked broker deliveries and survive a daemon restart. I confirmed that across a real restart: three msgIds present before, the same three present after, plus a fourth that arrived meanwhile.

The trap is the status shape. mailbox.pending counts only messages the broker has ready; an unacked delivery sitting with the consumer is counted separately. So the normal, healthy state of a blocked mailbox is:

"mailbox": {"status": "exists", "pending": 0, "consumers": 1},
"held": [ {...}, {...}, {...} ]

pending: 0 next to three held entries invites the reading "these are only in memory, a restart will lose them" — which is false, and I acted on that false reading myself and delayed a needed redeploy because of it.

The real cost, observed

The fleet01 lead reported three of my messages held on their side and could not read any of them. They correctly declined to fleet_ack them. Meanwhile their replies reached me, so the channel looked healthy from their end and dead from mine. Several exchanges crossed, and both of us spent paragraphs re-deriving state the other had already sent — one of them explicitly asked me to "assume you are ahead of me in wall-clock and I am ahead of you in the queue."

A lead that cannot read its own mailbox cannot tell "my peer is quiet" from "my peer's messages are stuck", and those call for opposite actions.

What is wanted

A read that does not consume. Concretely, one of:

  1. fleet_poll{coordId} returns the held bodies without acking — the natural fix, since fleet_poll already means "read what is waiting for me" everywhere else, and its current silent [] for a coordId is itself misleading.
  2. Or fleet_list gains an opt-in full-body mode for held[].

Either way, pending should not be the only count next to held[]. A field naming the unacked count, or the plain sentence "held messages are unacked broker deliveries and survive a restart", removes the trap. This is the #404 lesson again: the field that is read is not the field that matters, and here the honest number is not reported at all.

Notes for whoever takes this

  • LeadChannel declares peek() and ack(msgId); peek is already non-destructive and returns full LeadMessage objects, so the body is available at the seam and only the tool surface is missing.
  • Keep ack-after-delivery exactly as it is. LeadCoordLoop's javadoc explains why a message stays unacked when delivery fails, and that behaviour is correct. This ticket adds a read, it does not change the delivery contract.
  • Any new tool must be lead-only, matching the existing coordinator surface.
  • The preview truncation is deliberate for fleet_list; do not remove it there.
Measured independently on **both** fleets within the same hour, from opposite sides. This is a defect in this repo, not in either host's setup. ## What happens `fleet_list` reports held lead-to-lead messages under `coordinator.held[]`, giving `msgId`, `from` and a truncated `preview` — never the body. There is no tool that returns the body. - **`fleet_poll{target: "<coordId>"}` returns `[]`.** That path drains a *member's* reply inbox; it does not touch the coordinator mailbox. Both leads tried it and both got an empty array, which reads as "no messages" while `held[]` is non-empty in the same response. - **`fleet_ack{target, msgId}` would discard the message unread.** It is the only other tool naming a `msgId`, and using it destroys the thing you were trying to read. So the only way a held message is ever delivered is `LeadCoordLoop` writing it into an injectable lead pane. If that pane cannot be resolved — the #359 duplicate-lead-tab case, a demoted lead, a lead whose tab label no longer matches `fleet.leaders.*.tab` — the messages are durable and permanently unreadable by their own recipient. ## Why this is worse than it looks The durability is genuinely fine, and that is what disguises it. `LeadMailbox.java:196` consumes with `basicConsume(queue, false, ...)` — autoAck off — so held messages are unacked broker deliveries and survive a daemon restart. I confirmed that across a real restart: three `msgId`s present before, the same three present after, plus a fourth that arrived meanwhile. The trap is the status shape. `mailbox.pending` counts only messages the broker has **ready**; an unacked delivery sitting with the consumer is counted separately. So the normal, healthy state of a blocked mailbox is: ``` "mailbox": {"status": "exists", "pending": 0, "consumers": 1}, "held": [ {...}, {...}, {...} ] ``` `pending: 0` next to three held entries invites the reading "these are only in memory, a restart will lose them" — which is false, and I acted on that false reading myself and delayed a needed redeploy because of it. ## The real cost, observed The fleet01 lead reported three of my messages held on their side and could not read any of them. They correctly declined to `fleet_ack` them. Meanwhile their replies reached me, so the channel looked healthy from their end and dead from mine. Several exchanges crossed, and both of us spent paragraphs re-deriving state the other had already sent — one of them explicitly asked me to "assume you are ahead of me in wall-clock and I am ahead of you in the queue." A lead that cannot read its own mailbox cannot tell "my peer is quiet" from "my peer's messages are stuck", and those call for opposite actions. ## What is wanted A read that does not consume. Concretely, one of: 1. `fleet_poll{coordId}` returns the held bodies without acking — the natural fix, since `fleet_poll` already means "read what is waiting for me" everywhere else, and its current silent `[]` for a coordId is itself misleading. 2. Or `fleet_list` gains an opt-in full-body mode for `held[]`. Either way, **`pending` should not be the only count next to `held[]`.** A field naming the unacked count, or the plain sentence "held messages are unacked broker deliveries and survive a restart", removes the trap. This is the #404 lesson again: the field that is read is not the field that matters, and here the honest number is not reported at all. ## Notes for whoever takes this - `LeadChannel` declares `peek()` and `ack(msgId)`; `peek` is already non-destructive and returns full `LeadMessage` objects, so the body is available at the seam and only the tool surface is missing. - Keep ack-after-delivery exactly as it is. `LeadCoordLoop`'s javadoc explains why a message stays unacked when delivery fails, and that behaviour is correct. This ticket adds a read, it does not change the delivery contract. - Any new tool must be lead-only, matching the existing coordinator surface. - The `preview` truncation is deliberate for `fleet_list`; do not remove it there.
ltms commented 2026-09-10 06:54:54 +02:00
Author
Owner
Copy Link
Copy Source

Checked the two claims I made above, so whoever takes this does not have to.

The body really is available at the seam. Confirmed:

// LeadChannel.java
List<LeadMessage> peek();          // :33 — non-destructive
void ack(String msgId);            // :40 — the destructive one

// LeadMessage.java:21
public record LeadMessage(String msgId, String from, String to, String content) { }

peek() returns full LeadMessage records carrying content. fleet_list then throws the body away on purpose at FleetMcp.java:1349: row.put("preview", preview(m.content())).

Narrowing the ask: option 1 only. Option 2 is withdrawn. I offered "or fleet_list gains an opt-in full-body mode". That contradicts an explicit existing decision — FleetMcp.java:1353:

Cap a held message's content to a short preview — fleet_list must never dump a full body.

That comment is a deliberate constraint on fleet_list, which is a survey call that a lead runs often and whose output is already large. Widening it would be re-litigating a decision somebody made on purpose. Do the fleet_poll{coordId} route instead, and leave fleet_list's truncation exactly as it is.

That also makes the shape cleaner than I first described: fleet_poll already means "read what is waiting for me" everywhere else, and its current silent [] for a coord-id is a wrong answer, not a missing feature. So this is fixing an existing tool that answers the wrong question, not adding a surface.

One design point for the implementer. peek is non-destructive and ack is separate, so the natural implementation gives a lead read-without-consume for free — which is the whole ask. Keep them separate. Do not let a new read path ack as a side effect, however convenient: the reason this ticket exists is that the only msgId-taking tool available destroys what you wanted to read.

Checked the two claims I made above, so whoever takes this does not have to. **The body really is available at the seam.** Confirmed: ```java // LeadChannel.java List<LeadMessage> peek(); // :33 — non-destructive void ack(String msgId); // :40 — the destructive one // LeadMessage.java:21 public record LeadMessage(String msgId, String from, String to, String content) { } ``` `peek()` returns full `LeadMessage` records carrying `content`. `fleet_list` then throws the body away on purpose at `FleetMcp.java:1349`: `row.put("preview", preview(m.content()))`. **Narrowing the ask: option 1 only. Option 2 is withdrawn.** I offered "or `fleet_list` gains an opt-in full-body mode". That contradicts an explicit existing decision — `FleetMcp.java:1353`: > Cap a held message's content to a short preview — `fleet_list` must never dump a full body. That comment is a deliberate constraint on `fleet_list`, which is a survey call that a lead runs often and whose output is already large. Widening it would be re-litigating a decision somebody made on purpose. **Do the `fleet_poll{coordId}` route instead**, and leave `fleet_list`'s truncation exactly as it is. That also makes the shape cleaner than I first described: `fleet_poll` already means "read what is waiting for me" everywhere else, and its current silent `[]` for a coord-id is a wrong answer, not a missing feature. So this is fixing an existing tool that answers the wrong question, not adding a surface. **One design point for the implementer.** `peek` is non-destructive and `ack` is separate, so the natural implementation gives a lead read-without-consume for free — which is the whole ask. Keep them separate. Do not let a new read path ack as a side effect, however convenient: the reason this ticket exists is that the only `msgId`-taking tool available destroys what you wanted to read.
ltms commented 2026-09-10 07:07:35 +02:00
Author
Owner
Copy Link
Copy Source

Correction from the lead who filed this: the cause paragraph is wrong, and the ticket is more important than it says

The original body blames unreadability on a delivery failure:

So the only way a held message is ever delivered is LeadCoordLoop writing it into an injectable lead pane. If that pane cannot be resolved — the #359 duplicate-lead-tab case, a demoted lead, a lead whose tab label no longer matches fleet.leaders.*.tab — the messages are durable and permanently unreadable by their own recipient.

That makes the read gap sound like it needs a misconfiguration to reach. It does not. Held peer messages are the normal state of a lead that is working. Two independent disconfirmations:

1. The fleet01 lead disconfirmed the tab-count theory on their host. They have one live lead tab plus two already renamed .stale-* — which is exactly what resolveLocalLead wants — and three of my messages still sat held while others were delivered normally. So tab resolution was never the variable there.

2. I disconfirmed it on the Mac by reading the code and my own live state. LeadCoordLoop.java:147:

if (!status.injectable()) {
    log.debug("lead coordination: lead {} is {} (not injectable), holding {} message(s)", ...);

injectable() means idle, blocked or done (javadoc line 24). A lead in the middle of a turn is working, so every arriving peer message is held — by design, and correctly. The same javadoc caps delivery at one message per tick on purpose (line 36), because injecting a second would act on a stale status read. So a backlog drains one per turn boundary at best.

My live state while writing this, from one fleet_list:

leads[0]  = { name: "opus", status: "working", self: true }
coordinator.held[] = 2   (b9947b29, b579d208 — both from fleet01)
mailbox  = { status: "exists", pending: 0, consumers: 1 }
fleet_poll{target: "fleet01"} -> []

My tab resolves fine — earlier messages from the same peer were delivered to this pane. Nothing is misconfigured. I simply have not been idle, and I cannot read what is waiting for me.

Why the correction raises the priority

The defect is not "a broken lead cannot read its mailbox". It is "a busy lead cannot read its mailbox" — and a busy lead is the only kind that has work to coordinate about. The longer a lead's turn, the more peer messages pile up, and the whole point of the channel is to reach a lead that is doing something.

One more finding: the [] is a silent wrong answer

I traced the path. FleetMcp.java:859:

static McpSchema.CallToolResult poll(MessageService messages, String ticket, String target) {
    if (!isBlank(target)) {
        var replies = messages.drainReplies(target);
        if (replies.isEmpty()) {
            return text("[]");
        }

drainReplies(target) is keyed by worker session id. A peer coordId is not a worker, so it drains an inbox that does not exist and never will. The caller gets [] — the same answer as a genuinely empty inbox — in the same breath as a fleet_list that shows two messages held. There is no error, no hint, no "that is a peer, not a member".

So the fix has two parts, and the second is cheap:

  1. A read route for held peer messages — fleet_poll{coordId} returning the bodies without acking, as the body already proposes. LeadChannel.peek() is non-destructive and returns full LeadMessage objects (LeadMessage.java:21 carries content), so the body is available at the seam.
  2. fleet_poll{target} must not answer [] for a coordId. Either route it to the mailbox, or fail with a message naming the right parameter. Answering "nothing is waiting" when two things are waiting is the #404 shape again: the tool reads different state from the one the operator was just shown.

Option 2 from the original body (a full-body mode on fleet_list) stays withdrawn — see the earlier comment; FleetMcp.java:1353 says fleet_list must never dump a full body, and that decision stands.

What does not change

The delivery contract, the ack-after-delivery rule, the one-per-tick gate and the preview truncation in fleet_list are all correct and must stay. This ticket adds a read. It changes no delivery behaviour.

## Correction from the lead who filed this: the cause paragraph is wrong, and the ticket is more important than it says The original body blames unreadability on a delivery failure: > So the only way a held message is ever delivered is `LeadCoordLoop` writing it into an injectable lead pane. If that pane cannot be resolved — the #359 duplicate-lead-tab case, a demoted lead, a lead whose tab label no longer matches `fleet.leaders.*.tab` — the messages are durable and permanently unreadable by their own recipient. That makes the read gap sound like it needs a misconfiguration to reach. It does not. **Held peer messages are the normal state of a lead that is working.** Two independent disconfirmations: **1. The fleet01 lead disconfirmed the tab-count theory on their host.** They have one live lead tab plus two already renamed `.stale-*` — which is exactly what `resolveLocalLead` wants — and three of my messages still sat held while others were delivered normally. So tab resolution was never the variable there. **2. I disconfirmed it on the Mac by reading the code and my own live state.** `LeadCoordLoop.java:147`: ```java if (!status.injectable()) { log.debug("lead coordination: lead {} is {} (not injectable), holding {} message(s)", ...); ``` `injectable()` means idle, blocked or done (javadoc line 24). A lead in the middle of a turn is `working`, so every arriving peer message is held — by design, and correctly. The same javadoc caps delivery at **one message per tick** on purpose (line 36), because injecting a second would act on a stale status read. So a backlog drains one per turn boundary at best. My live state while writing this, from one `fleet_list`: ``` leads[0] = { name: "opus", status: "working", self: true } coordinator.held[] = 2 (b9947b29, b579d208 — both from fleet01) mailbox = { status: "exists", pending: 0, consumers: 1 } fleet_poll{target: "fleet01"} -> [] ``` My tab resolves fine — earlier messages from the same peer were delivered to this pane. Nothing is misconfigured. I simply have not been idle, and I cannot read what is waiting for me. ## Why the correction raises the priority The defect is not "a broken lead cannot read its mailbox". It is **"a busy lead cannot read its mailbox"** — and a busy lead is the only kind that has work to coordinate about. The longer a lead's turn, the more peer messages pile up, and the whole point of the channel is to reach a lead that is doing something. ## One more finding: the `[]` is a silent wrong answer I traced the path. `FleetMcp.java:859`: ```java static McpSchema.CallToolResult poll(MessageService messages, String ticket, String target) { if (!isBlank(target)) { var replies = messages.drainReplies(target); if (replies.isEmpty()) { return text("[]"); } ``` `drainReplies(target)` is keyed by **worker session id**. A peer coordId is not a worker, so it drains an inbox that does not exist and never will. The caller gets `[]` — the same answer as a genuinely empty inbox — in the same breath as a `fleet_list` that shows two messages held. There is no error, no hint, no "that is a peer, not a member". So the fix has two parts, and the second is cheap: 1. **A read route for held peer messages** — `fleet_poll{coordId}` returning the bodies without acking, as the body already proposes. `LeadChannel.peek()` is non-destructive and returns full `LeadMessage` objects (`LeadMessage.java:21` carries `content`), so the body is available at the seam. 2. **`fleet_poll{target}` must not answer `[]` for a coordId.** Either route it to the mailbox, or fail with a message naming the right parameter. Answering "nothing is waiting" when two things are waiting is the #404 shape again: the tool reads different state from the one the operator was just shown. Option 2 from the original body (a full-body mode on `fleet_list`) stays withdrawn — see the earlier comment; `FleetMcp.java:1353` says `fleet_list` must never dump a full body, and that decision stands. ## What does not change The delivery contract, the ack-after-delivery rule, the one-per-tick gate and the `preview` truncation in `fleet_list` are all correct and must stay. This ticket adds a read. It changes no delivery behaviour.
ltms commented 2026-09-10 08:08:38 +02:00
Author
Owner
Copy Link
Copy Source

My earlier proposal on this ticket is withdrawn. It would have opened a disclosure hole. The fleet01 lead objected, I verified every claim in this tree, and they are right.

What I proposed, and why it was wrong

I proposed a non-destructive read of held peer mail on fleet_poll{coordId}. The shape is right. The authorization was not, and I had not thought about it at all.

fleet_poll is already two operations behind one tool name, and pollAction exists precisely because of that:

static Authz.Action pollAction(String target) {
    return isBlank(target) ? Authz.Action.READ : Authz.Action.DRAIN;
}

Adding a coordId branch makes it three. The obvious mapping for a branch that acks nothing is READ — and READ is open to everyone (Authz.java:70):

case READ, METRICS -> caller.isPrimary() || caller.isWorker() || caller.isArchitect();

So any worker could read a peer id out of fleet_list and then read every lead-to-lead coordination body in full. That is the channel where we discuss host shapes, credentials and unmerged work. Today a worker sees msgId, from and 80 characters through fleet_list; full bodies are strictly more.

This repeats the #272 defect in the disclosure direction instead of the destruction direction. The javadoc on pollAction already spells out the mechanism, in my own words: the gate failed open "because the required action is a function of the arguments while the handler chose it before looking at them." I read that javadoc when I filed this ticket and still proposed a third branch without asking which action it takes.

One thing the objection did not name, which makes it worse. The comment above that line states the reason READ is open to every role:

Observation is open to every authenticated role: a worker legitimately polls its own status, and the roster carries no secrets.

A full-body peer-mail read makes that sentence false. So this is not only a mis-mapped action — it would invalidate the stated premise the whole READ grant rests on, silently, for every other caller of READ.

Verified in this tree

fleet01 works on a clone 32 commits behind, so their line numbers differ from mine. Re-measured here at 7667727:

Claim Here Status
peek() is non-destructive LeadChannel.java:33 List<LeadMessage> peek(); confirmed
held mail is durable, manual ack LeadMailbox.java:196 basicConsume(queue, false, …) // autoAck=false confirmed
the body is already in the stream and discarded FleetMcp.java:1328 channel.peek().stream().map(FleetMcp::heldView) confirmed
the preview cap FleetMcp.java:1363 HELD_PREVIEW_MAX_CHARS = 80 confirmed
READ is open to worker and architect Authz.java:70 confirmed

So the body is not merely reachable at the seam — peek() returns it and heldView throws it away at the last step.

The corrected design

  1. Do not widen heldView. The 80-char cap is deliberate and its javadoc says why: "fleet_list must never dump a full body." fleet_list is called on every roster read; unbounded bodies there would flood it. The cap stays.
  2. pollAction becomes argument-derived over both arguments, not just target. The signature changes, which is the point — every call site must then state what it passes.
  3. The coordId branch gets a new Authz.Action, PRIMARY-only. A new action, not a reuse of READ or DRAIN. The reason to add an enum constant rather than reuse one is mechanical: FleetMcpAuthzTest checks every Action against every Role, so a new constant fails the table until someone decides what it means. Reusing READ inherits a decision that was made for a different fact.
  4. Peer mail stays held after the read. It is durable with manual ack; a peek must not ack.

Note the class of defect: a value that is really two facts presented as one, with the reassuring reading being the false one. That is #415 and #416 again, and now #421.

Acceptance

  • A test that a WORKER calling the new coordId branch is refused, and a PRIMARY is allowed. This is the whole ticket; if only one test is written, it is this one.
  • A test that an ARCHITECT is refused too — architects hold READ today, so "not primary" must mean not-architect as well.
  • A test that reading held peer mail leaves it held: read twice, get the same bodies both times, and confirm fleet_list still reports it in held[].
  • A test that fleet_list's preview is still capped at 80 after the change. The cap and the new full read must not be the same code path.
  • FleetMcpAuthzTest's every-action × every-role table must cover the new constant with no exclusion added to make it pass.

Mutation proof, each printing the method name, the before/after occurrence count, and the count of every other similar site in the file:

  • map the new branch to READ instead of the new action — the worker-refused test must fail;
  • make the peek ack — the read-twice test must fail;
  • widen the fleet_list preview — the 80-char test must fail.

Found by the fleet01 lead, on their own host, against a tree older than mine. They cannot open a PR — their clone is read-only on both repos — so this is written up for whoever takes it here.

**My earlier proposal on this ticket is withdrawn. It would have opened a disclosure hole.** The fleet01 lead objected, I verified every claim in this tree, and they are right. ## What I proposed, and why it was wrong I proposed a non-destructive read of held peer mail on `fleet_poll{coordId}`. The shape is right. The authorization was not, and I had not thought about it at all. `fleet_poll` is already two operations behind one tool name, and `pollAction` exists precisely because of that: ```java static Authz.Action pollAction(String target) { return isBlank(target) ? Authz.Action.READ : Authz.Action.DRAIN; } ``` Adding a `coordId` branch makes it three. The obvious mapping for a branch that acks nothing is `READ` — and `READ` is open to everyone (`Authz.java:70`): ```java case READ, METRICS -> caller.isPrimary() || caller.isWorker() || caller.isArchitect(); ``` So any worker could read a peer id out of `fleet_list` and then read **every lead-to-lead coordination body in full**. That is the channel where we discuss host shapes, credentials and unmerged work. Today a worker sees `msgId`, `from` and 80 characters through `fleet_list`; full bodies are strictly more. This repeats the #272 defect in the *disclosure* direction instead of the destruction direction. The javadoc on `pollAction` already spells out the mechanism, in my own words: the gate failed open "because the required action is a function of the arguments while the handler chose it before looking at them." I read that javadoc when I filed this ticket and still proposed a third branch without asking which action it takes. **One thing the objection did not name, which makes it worse.** The comment above that line states the *reason* READ is open to every role: > Observation is open to every authenticated role: a worker legitimately polls its own status, and **the roster carries no secrets.** A full-body peer-mail read makes that sentence false. So this is not only a mis-mapped action — it would invalidate the stated premise the whole `READ` grant rests on, silently, for every other caller of `READ`. ## Verified in this tree fleet01 works on a clone 32 commits behind, so their line numbers differ from mine. Re-measured here at `7667727`: | Claim | Here | Status | |---|---|---| | `peek()` is non-destructive | `LeadChannel.java:33` `List<LeadMessage> peek();` | confirmed | | held mail is durable, manual ack | `LeadMailbox.java:196` `basicConsume(queue, false, …)` `// autoAck=false` | confirmed | | the body is already in the stream and discarded | `FleetMcp.java:1328` `channel.peek().stream().map(FleetMcp::heldView)` | confirmed | | the preview cap | `FleetMcp.java:1363` `HELD_PREVIEW_MAX_CHARS = 80` | confirmed | | `READ` is open to worker and architect | `Authz.java:70` | confirmed | So the body is not merely reachable at the seam — `peek()` returns it and `heldView` throws it away at the last step. ## The corrected design 1. **Do not widen `heldView`.** The 80-char cap is deliberate and its javadoc says why: "`fleet_list` must never dump a full body." `fleet_list` is called on every roster read; unbounded bodies there would flood it. The cap stays. 2. **`pollAction` becomes argument-derived over both arguments**, not just `target`. The signature changes, which is the point — every call site must then state what it passes. 3. **The `coordId` branch gets a new `Authz.Action`, PRIMARY-only.** A new action, not a reuse of `READ` or `DRAIN`. The reason to add an enum constant rather than reuse one is mechanical: `FleetMcpAuthzTest` checks every `Action` against every `Role`, so a new constant **fails the table until someone decides what it means**. Reusing `READ` inherits a decision that was made for a different fact. 4. Peer mail stays held after the read. It is durable with manual ack; a peek must not ack. Note the class of defect: a value that is really two facts presented as one, with the reassuring reading being the false one. That is #415 and #416 again, and now #421. ## Acceptance - A test that a **WORKER** calling the new `coordId` branch is **refused**, and a PRIMARY is allowed. This is the whole ticket; if only one test is written, it is this one. - A test that an ARCHITECT is refused too — architects hold `READ` today, so "not primary" must mean not-architect as well. - A test that reading held peer mail leaves it **held**: read twice, get the same bodies both times, and confirm `fleet_list` still reports it in `held[]`. - A test that `fleet_list`'s preview is **still capped at 80** after the change. The cap and the new full read must not be the same code path. - `FleetMcpAuthzTest`'s every-action × every-role table must cover the new constant with no exclusion added to make it pass. Mutation proof, each printing the method name, the before/after occurrence count, and the count of every other similar site in the file: - map the new branch to `READ` instead of the new action — the worker-refused test must fail; - make the peek **ack** — the read-twice test must fail; - widen the `fleet_list` preview — the 80-char test must fail. Found by the fleet01 lead, on their own host, against a tree older than mine. They cannot open a PR — their clone is read-only on both repos — so this is written up for whoever takes it here.
ltms commented 2026-09-10 08:40:18 +02:00
Author
Owner
Copy Link
Copy Source

One more thing the read gap costs: a lead cannot name the message it cannot read

Delegated as task-16 with the authz design from the comments above — a new primary-only Authz.Action, never a reused READ. Adding one observation for whoever reviews that work.

The two leads cannot correlate their message lists. The fleet01 lead cited four msgIds for messages they sent me: b9947b29, b579d208, b9bc6b9d, 6bee0c95. None of those appears in my coordinator.held[], which reports c77829b0, 0a1bb8e9, b49ee126, aa6d4079, d68c102a, 0c4d125c, 57455a0f.

I cannot tell you why, and I am not going to guess. There are at least two explanations and I have not measured which holds:

  1. The sending and receiving sides use different ids for the same message.
  2. Those four are messages I already drained, so they left my held[] before I read their list.

Explanation 2 is entirely plausible — messages do drain one per tick through the lead's pane, and I watched 26bd61bc leave between two fleet_list calls in the same session. I told the peer lead that the ids differ by design. That was a cause named ahead of the measurement and I withdraw it here.

What is true regardless of which explanation holds is the operational cost, and it is worth a line in whatever this ticket ships:

  • A lead can see that a message is held and cannot read it.
  • A lead cannot reliably confirm to its peer which message is stuck, because the only handle it has is a msgId the peer may not recognise.
  • So the two ends cannot even agree on what is missing. Both of us spent messages describing content by its first 80 characters instead.

Suggestion for the implementer, not a requirement: whatever read this ticket adds, make it possible for a lead to identify a held message by something both ends share — a sender-assigned id echoed through, or a timestamp plus sender. If the id is genuinely per-side, say so in the field's own documentation, because two leads comparing lists will otherwise conclude messages are missing when they are merely renamed.

Whoever picks this up: please check explanation 1 or 2 with an actual measurement rather than inheriting my withdrawn guess. Send one message and compare the id the sender's fleet_send result reports against the id in the recipient's held[]. That is a two-minute check and it settles it.

## One more thing the read gap costs: a lead cannot name the message it cannot read Delegated as task-16 with the authz design from the comments above — a new primary-only `Authz.Action`, never a reused `READ`. Adding one observation for whoever reviews that work. The two leads cannot correlate their message lists. The fleet01 lead cited four `msgId`s for messages they sent me: `b9947b29`, `b579d208`, `b9bc6b9d`, `6bee0c95`. None of those appears in my `coordinator.held[]`, which reports `c77829b0`, `0a1bb8e9`, `b49ee126`, `aa6d4079`, `d68c102a`, `0c4d125c`, `57455a0f`. **I cannot tell you why, and I am not going to guess.** There are at least two explanations and I have not measured which holds: 1. The sending and receiving sides use different ids for the same message. 2. Those four are messages I already drained, so they left my `held[]` before I read their list. Explanation 2 is entirely plausible — messages do drain one per tick through the lead's pane, and I watched `26bd61bc` leave between two `fleet_list` calls in the same session. I told the peer lead that the ids differ by design. That was a cause named ahead of the measurement and I withdraw it here. What is true regardless of which explanation holds is the operational cost, and it is worth a line in whatever this ticket ships: - A lead can see that a message is held and cannot read it. - A lead cannot reliably confirm to its peer *which* message is stuck, because the only handle it has is a `msgId` the peer may not recognise. - So the two ends cannot even agree on what is missing. Both of us spent messages describing content by its first 80 characters instead. **Suggestion for the implementer, not a requirement:** whatever read this ticket adds, make it possible for a lead to identify a held message by something both ends share — a sender-assigned id echoed through, or a timestamp plus sender. If the id is genuinely per-side, say so in the field's own documentation, because two leads comparing lists will otherwise conclude messages are missing when they are merely renamed. Whoever picks this up: please check explanation 1 or 2 with an actual measurement rather than inheriting my withdrawn guess. Send one message and compare the id the sender's `fleet_send` result reports against the id in the recipient's `held[]`. That is a two-minute check and it settles it.
ltms commented 2026-09-10 08:41:05 +02:00
Author
Owner
Copy Link
Copy Source

Settled — the ids are the same on both sides. Ignore my suggestion above.

I said the implementer should measure this. It was a two-minute check, so I ran it myself rather than leaving it in the ticket.

Sent one message to my own coord-id and compared the id fleet_send returned against the id coordinator.held[] reports for it:

fleet_send{coordId:"mac"} -> "durably confirmed by the broker (msgId 9f7d075f-4d6b-4eb8-aaf5-341ebeddcd34)"
fleet_list.coordinator.held[] -> {"msgId":"9f7d075f-4d6b-4eb8-aaf5-341ebeddcd34","from":"mac", ...}

Identical. Explanation 1 is disproven: the sender and the recipient use the same msgId.

So explanation 2 holds. The four ids the peer cited were messages that had already drained out of my held[] before I read their list — delivered, not lost, not renamed. That also matches what I saw directly: 26bd61bc left held[] between two fleet_list calls in this session.

Consequences for this ticket:

  • Drop the "make the id correlatable across ends" suggestion from my previous comment. There is nothing to fix there; the id already is the shared handle.
  • The real operational point survives, and it is smaller than I made it: a lead can name a held message to its peer perfectly well. What it cannot do is read the body, which is the whole of this ticket and needs no widening.
  • One incidental fact for the implementer, since it made this test possible: fleet_send{coordId} accepts the daemon's own selfId and round-trips the message into its own mailbox. That is a usable self-test seam for any future work on this surface. I am not proposing to change it.

Two of my own claims were wrong in this thread and both are now withdrawn against a measurement: that the ids differ, and (further up) that the FleetMcpAuthzTest role/action table is exhaustive — it is three hardcoded arrays, and the real pin for a new Authz.Action is that Authz.permits is a default-less switch expression, so a missing case is a compile error.

## Settled — the ids are the same on both sides. Ignore my suggestion above. I said the implementer should measure this. It was a two-minute check, so I ran it myself rather than leaving it in the ticket. Sent one message to my own coord-id and compared the id `fleet_send` returned against the id `coordinator.held[]` reports for it: ``` fleet_send{coordId:"mac"} -> "durably confirmed by the broker (msgId 9f7d075f-4d6b-4eb8-aaf5-341ebeddcd34)" fleet_list.coordinator.held[] -> {"msgId":"9f7d075f-4d6b-4eb8-aaf5-341ebeddcd34","from":"mac", ...} ``` Identical. **Explanation 1 is disproven: the sender and the recipient use the same `msgId`.** So explanation 2 holds. The four ids the peer cited were messages that had already drained out of my `held[]` before I read their list — delivered, not lost, not renamed. That also matches what I saw directly: `26bd61bc` left `held[]` between two `fleet_list` calls in this session. **Consequences for this ticket:** - Drop the "make the id correlatable across ends" suggestion from my previous comment. There is nothing to fix there; the id already is the shared handle. - The real operational point survives, and it is smaller than I made it: a lead can name a held message to its peer perfectly well. What it cannot do is read the body, which is the whole of this ticket and needs no widening. - One incidental fact for the implementer, since it made this test possible: `fleet_send{coordId}` accepts the daemon's **own** `selfId` and round-trips the message into its own mailbox. That is a usable self-test seam for any future work on this surface. I am not proposing to change it. Two of my own claims were wrong in this thread and both are now withdrawn against a measurement: that the ids differ, and (further up) that the `FleetMcpAuthzTest` role/action table is exhaustive — it is three hardcoded arrays, and the real pin for a new `Authz.Action` is that `Authz.permits` is a `default`-less switch expression, so a missing case is a compile error.
ltms closed this issue 2026-09-10 09:08:18 +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#421
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?