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

fleet_reply cannot answer a peer lead — the charter documents a path that does not exist #391

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

The defect

The canonical CLAUDE.md block tells a lead to answer a peer lead with fleet_reply:

Intent Tool
Answer a peer lead that messaged you fleet_reply{content}

and again in prose: "Being messaged by a peer does not make you its worker: answer with fleet_reply".

That path does not exist in the code. Every such call fails.

Reproduced

Mac lead, after a fleet_send{coordId: "fleet01"} message arrived from the fleet01 lead, no restart on either side:

fleet_reply{content: "..."}
-> reply 12b8c9e2-882f-4339-aa72-9db772417af7 was returned as unroutable (queue not declared/owned)

Both durable mailboxes were healthy at that moment, confirmed independently from both hosts:

coordinator selfId=mac      mailbox: exists pending=0 consumers=1
peers: fleet01 -> status=exists pending=0 consumers=1

coordinator selfId=fleet01  mailbox: exists pending=0 consumers=1
peers: mac -> status=exists pending=0 consumers=1

So the coordination transport is fine in both directions. fleet_send{coordId} works and has carried every message between the two leads all day.

Why it cannot work

  1. FleetMcp.reply (FleetMcp.java:879) resolves callerTerminal from the connection, never from an argument. For a lead that is the lead's own terminal id.
  2. MessageService.reply (MessageService.java:491) tries a rendezvous, then falls through to the session inbox. There is no peer branch: grep -c "coord\|LeadMailbox" MessageService.java returns 0.
  3. AmqpReplyInbox.queueName (AmqpReplyInbox.java:556) builds agent.<target>.inbox.

So a lead's fleet_reply publishes to agent.<its own terminal>.inbox — the member namespace. A lead is not a member, nothing ever declares that queue, and the mandatory publish comes straight back as unroutable. It never had a route to lead.<peer>.inbox.

This is not a restart-lifetime problem and not a transient reply-to queue. The fleet01 lead proposed that hypothesis; the reproduction above, with no restart between the incoming message and the reply, falsifies it.

Why it went unnoticed

fleet_send{coordId} works, so both leads reached for it and coordination never actually stalled. The broken row only fires when a lead follows the charter literally. The failure is loud and non-destructive — the sender simply never hears back, and the reply is lost.

The two fixes are separate, and one is urgent

a) The instruction surface is wrong today. CLAUDE.md and the wiki template (7-Use-Cases → The portable CLAUDE.md block) must stay byte-identical, and both currently tell every lead to use a tool that cannot work. Whatever happens to the code, that row should say fleet_send{coordId}. Per "The prompt is part of the product", this is the incomplete half of CB-637.

b) Whether fleet_reply should learn the peer path is a design call. Two options, and I do not think it should be decided in this ticket:

  • Route a lead's fleet_reply to the mailbox of the peer whose message it is answering. Needs the daemon to remember which peer a lead is replying to — there is no turn id on the coordination path today.
  • Refuse it with a clear error naming fleet_send{coordId} instead. Cheaper, honest, and matches how both leads already work.

The second is my recommendation. fleet_reply's whole contract is "resolve the sender's blocked fleet_send", and a coordId send is durable and non-blocking — there is no waiter to resolve, so the semantics do not carry over.

Acceptance

  • A lead calling fleet_reply gets either a working delivery or an error that names the tool to use instead — never unroutable (queue not declared/owned).
  • The canonical block in CLAUDE.md and the wiki template both name the working tool, and the byte-identical check in CLAUDE.md still passes.
  • A test covers a lead calling fleet_reply; today the reply path is only tested for workers.

Found while testing a hypothesis from the fleet01 lead. Observation and diagnosis: mac lead.

## The defect The canonical `CLAUDE.md` block tells a lead to answer a peer lead with `fleet_reply`: | Intent | Tool | |---|---| | Answer a peer lead that messaged you | `fleet_reply{content}` | and again in prose: *"Being messaged by a peer does not make you its worker: answer with `fleet_reply`"*. **That path does not exist in the code.** Every such call fails. ## Reproduced Mac lead, after a `fleet_send{coordId: "fleet01"}` message arrived from the fleet01 lead, no restart on either side: ``` fleet_reply{content: "..."} -> reply 12b8c9e2-882f-4339-aa72-9db772417af7 was returned as unroutable (queue not declared/owned) ``` Both durable mailboxes were healthy at that moment, confirmed independently from both hosts: ``` coordinator selfId=mac mailbox: exists pending=0 consumers=1 peers: fleet01 -> status=exists pending=0 consumers=1 ``` ``` coordinator selfId=fleet01 mailbox: exists pending=0 consumers=1 peers: mac -> status=exists pending=0 consumers=1 ``` So the coordination transport is fine in both directions. `fleet_send{coordId}` works and has carried every message between the two leads all day. ## Why it cannot work 1. `FleetMcp.reply` (`FleetMcp.java:879`) resolves `callerTerminal` **from the connection**, never from an argument. For a lead that is the lead's own terminal id. 2. `MessageService.reply` (`MessageService.java:491`) tries a rendezvous, then falls through to the session inbox. There is no peer branch: `grep -c "coord\|LeadMailbox" MessageService.java` returns **0**. 3. `AmqpReplyInbox.queueName` (`AmqpReplyInbox.java:556`) builds `agent.<target>.inbox`. So a lead's `fleet_reply` publishes to `agent.<its own terminal>.inbox` — the **member** namespace. A lead is not a member, nothing ever declares that queue, and the `mandatory` publish comes straight back as unroutable. It never had a route to `lead.<peer>.inbox`. This is not a restart-lifetime problem and not a transient reply-to queue. The fleet01 lead proposed that hypothesis; the reproduction above, with no restart between the incoming message and the reply, falsifies it. ## Why it went unnoticed `fleet_send{coordId}` works, so both leads reached for it and coordination never actually stalled. The broken row only fires when a lead follows the charter literally. The failure is loud and non-destructive — the sender simply never hears back, and the reply is lost. ## The two fixes are separate, and one is urgent **a) The instruction surface is wrong today.** `CLAUDE.md` and the wiki template (7-Use-Cases → *The portable CLAUDE.md block*) must stay byte-identical, and both currently tell every lead to use a tool that cannot work. Whatever happens to the code, that row should say `fleet_send{coordId}`. Per *"The prompt is part of the product"*, this is the incomplete half of CB-637. **b) Whether `fleet_reply` should learn the peer path is a design call.** Two options, and I do not think it should be decided in this ticket: - Route a lead's `fleet_reply` to the mailbox of the peer whose message it is answering. Needs the daemon to remember which peer a lead is replying to — there is no turn id on the coordination path today. - Refuse it with a clear error naming `fleet_send{coordId}` instead. Cheaper, honest, and matches how both leads already work. The second is my recommendation. `fleet_reply`'s whole contract is "resolve the sender's blocked `fleet_send`", and a `coordId` send is durable and non-blocking — there is no waiter to resolve, so the semantics do not carry over. ## Acceptance - A lead calling `fleet_reply` gets either a working delivery or an error that names the tool to use instead — never `unroutable (queue not declared/owned)`. - The canonical block in `CLAUDE.md` and the wiki template both name the working tool, and the byte-identical check in `CLAUDE.md` still passes. - A test covers a lead calling `fleet_reply`; today the reply path is only tested for workers. Found while testing a hypothesis from the fleet01 lead. Observation and diagnosis: mac lead.
ltms commented 2026-09-10 04:18:59 +02:00
Author
Owner
Copy Link
Copy Source

Merged as PR #402, in two rounds. The second round is the one worth reading.

Round 1 — the refusal itself

fleet_reply now refuses a PRIMARY caller and names the two routes that do work
(fleet_send{coordId} for another daemon, fleet_send{sessionId} on this host), plus why there
is nothing for reply to resolve: a peer's coord-id message is durable and non-blocking.

Round 2 — the mutation that was NOT caught, and how it got closed

Round 1 kept a permissive test helper:

/** Test helper for worker replies that bypass the MCP transport context. */
static McpSchema.CallToolResult reply(MessageService messages, String callerTerminal, String content) {
    return reply(messages, callerTerminal, Role.WORKER, content);   // permissive default
}

I mutated the handler to call it instead of the real 4-arg form:

349: return reply(messages, self, str(req.arguments(), "content")); // MUTANT D
→ Tests run: 1473, Failures: 0   BUILD SUCCESS

Not caught. Every one of the 10 test call sites used the 3-arg form, so all of them walked around
the gate the ticket had just added. The refusal was tested; the caller was not. That is the
[a test on the seam does not prove the caller] shape for the third time this week, and here the seam
had a default value that silently restored the old behaviour.

The follow-up deletes the overload. FleetMcp now has one reply(...) and it requires a Role.
The same mutation now fails at compile time:

method reply in class dev.ltms.fleet.mcp.FleetMcp cannot be applied to given types;
  required: MessageService, String, dev.ltms.fleet.auth.Role, String
  found:    MessageService, String, String
  reason: actual and formal argument lists differ in length

A compile error is a stronger kill than a failing test: it cannot be skipped, ignored, or made
flaky. This is the outcome to aim for whenever a security-relevant argument has a plausible default
— delete the default rather than test around it.

What I checked in the diff myself

  • The refusal text, the guard condition and the guard order are unchanged from round 1. Only the
    overload is gone and the 10 call sites now pass Role.WORKER explicitly.
  • replyFromLeadIsRefusedBeforeItCanPublishToTheWorkerInbox injects a fake ReplyInbox whose
    publish calls fail(...). So it pins the behaviour — the refusal happens before any publish —
    not just the message string. That is the right kind of test here.
  • replyFromUnidentifiedCallerKeepsItsOwnError passes (null, Role.PRIMARY) and asserts the
    "workers only" text. I did not ask for this, and it is the better of the two: it pins that the
    null-terminal check wins over the role check. A future reorder of the two guards would give a lead
    with no terminal the wrong message, and this is the only test that would notice.
  • The worker also swept the other FleetMcp overloads for the same shape and reported that the
    lead-channel and observation-source ones disable optional features rather than widening caller
    authority. I agree with that reading — none of them supplies a default for an identity or a role.

Production reachability, confirmed

CALLER_ROLE is written in exactly one place (FleetMcp.java:295-299), by a single transport-context
extractor that always sets all four keys together. principalFrom never returns null. So the guard
fires on every real connection — this is not a defence that only exists in tests.

Merged as PR #402, in two rounds. The second round is the one worth reading. ## Round 1 — the refusal itself `fleet_reply` now refuses a `PRIMARY` caller and names the two routes that do work (`fleet_send{coordId}` for another daemon, `fleet_send{sessionId}` on this host), plus *why* there is nothing for reply to resolve: a peer's coord-id message is durable and non-blocking. ## Round 2 — the mutation that was NOT caught, and how it got closed Round 1 kept a permissive test helper: ```java /** Test helper for worker replies that bypass the MCP transport context. */ static McpSchema.CallToolResult reply(MessageService messages, String callerTerminal, String content) { return reply(messages, callerTerminal, Role.WORKER, content); // permissive default } ``` I mutated the handler to call it instead of the real 4-arg form: ``` 349: return reply(messages, self, str(req.arguments(), "content")); // MUTANT D → Tests run: 1473, Failures: 0 BUILD SUCCESS ``` **Not caught.** Every one of the 10 test call sites used the 3-arg form, so all of them walked around the gate the ticket had just added. The refusal was tested; the caller was not. That is the [a test on the seam does not prove the caller] shape for the third time this week, and here the seam had a *default value* that silently restored the old behaviour. The follow-up deletes the overload. `FleetMcp` now has one `reply(...)` and it requires a `Role`. The same mutation now fails at **compile time**: ``` method reply in class dev.ltms.fleet.mcp.FleetMcp cannot be applied to given types; required: MessageService, String, dev.ltms.fleet.auth.Role, String found: MessageService, String, String reason: actual and formal argument lists differ in length ``` A compile error is a stronger kill than a failing test: it cannot be skipped, ignored, or made flaky. This is the outcome to aim for whenever a security-relevant argument has a plausible default — **delete the default rather than test around it.** ## What I checked in the diff myself - The refusal text, the guard condition and the **guard order** are unchanged from round 1. Only the overload is gone and the 10 call sites now pass `Role.WORKER` explicitly. - `replyFromLeadIsRefusedBeforeItCanPublishToTheWorkerInbox` injects a fake `ReplyInbox` whose `publish` calls `fail(...)`. So it pins the *behaviour* — the refusal happens before any publish — not just the message string. That is the right kind of test here. - `replyFromUnidentifiedCallerKeepsItsOwnError` passes `(null, Role.PRIMARY)` and asserts the "workers only" text. I did not ask for this, and it is the better of the two: it pins that the null-terminal check wins over the role check. A future reorder of the two guards would give a lead with no terminal the wrong message, and this is the only test that would notice. - The worker also swept the other `FleetMcp` overloads for the same shape and reported that the lead-channel and observation-source ones disable optional features rather than widening caller authority. I agree with that reading — none of them supplies a default for an identity or a role. ## Production reachability, confirmed `CALLER_ROLE` is written in exactly one place (`FleetMcp.java:295-299`), by a single transport-context extractor that always sets all four keys together. `principalFrom` never returns null. So the guard fires on every real connection — this is not a defence that only exists in tests.
ltms closed this issue 2026-09-10 04:21:39 +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#391
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?