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

t365: fleet_reply/REST reply distinguish resolved vs queued; rename nudge metric #379

Closed
agent wants to merge 0 commits from worker/t365-3920c5-3 into main
pull from: worker/t365-3920c5-3
merge into: fleet:main
Conversation 0 Commits - Files Changed -
agent commented 2026-09-09 02:24:39 +02:00
Member
Copy Link
Copy Source

fleetd #365 — three tool results claim "delivered" for something weaker than delivery

Step 1 — item 3 verified first, as the ticket asked

I read countNudge and its only two call sites (LeadHeartbeatLoop.injectNudge,
ReplyPushLoop.injectNudge) plus AgentControl.send (fleetd/src/main/java/dev/ltms/fleet/herdr/AgentControl.java:116-118),
which is what both loops call before counting.

Finding: it records a nudge SENT, not a nudge ARRIVED. agents.send(target, text) is one herdr
agent.prompt call — a one-way paste-and-submit into the pane. It has no confirmation that the
lead's Claude session actually read or processed the text; there is no read-receipt concept at
this layer at all. countNudge("delivered") in both loops fires right after that call returns
without throwing — i.e. it counts "the herdr RPC succeeded," not "the lead saw it."

What reads the counter: only GET /metrics (FleetApp.java:267-271, wired from
FleetMetrics.HEARTBEAT_NUDGES / PUSH_NUDGES) — a Prometheus-style scrape for an operator's
dashboard. Nothing in the codebase branches on the counter's value.

Since nothing else claims more than "sent" either, this is the honest-rename case the ticket
described, not a counting bug — I renamed the outcome label "delivered" → "sent" in both
loops, updated FleetMetrics's describe() text for both series to say what "sent" actually
means, and updated the three ReplyPushLoopTest assertions that checked the old label
(successfulNudgeIncrementsDelivered → successfulNudgeIncrementsSent,
successfulTicketNudgeIncrementsDelivered → successfulTicketNudgeIncrementsSent, plus the
zero-count assertion in reminderCapIncrementsExhausted). LeadHeartbeatLoopTest had no
assertions on the metric label, so nothing there needed changing. The count itself is
unchanged — only the word.

Step 2 — items 1 and 2 fixed with one shared mechanism

MessageService.reply(String, String) used to return an always-true boolean even though it
internally already distinguishes three landing paths (visible in its own metrics: path=rendezvous
/ path=async-recovered / path=inbox). Both callers discarded that distinction:

  • FleetMcp.reply (fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java) always returned the
    literal text "delivered".
  • FleetApp.replyMessage (fleetd/src/main/java/dev/ltms/fleet/rest/FleetApp.java) always
    returned {"delivered": true}.

Fix: added MessageService.ReplyOutcome (three values — RESOLVED_SEND,
RESOLVED_ASYNC_TICKET, QUEUED — each carrying a wireName(), a delivered() boolean, and a
shared description() string), and changed reply() to return it instead of boolean. Both call
sites now read the same enum:

  • FleetMcp.reply returns outcome.description() — e.g.
    "delivered — resolved the fleet_send that was waiting for it" vs.
    "queued — no send or ticket was waiting; held in the inbox for a later drain".
  • FleetApp.replyMessage returns {"sessionId", "delivered": outcome.delivered(), "outcome": outcome.wireName()}
    — delivered is now true only for the two resolved cases and false for QUEUED; outcome
    names which of the three actually happened.

New production code is the ReplyOutcome enum (~40 lines) plus the two small call-site edits —
well under the 80-line ceiling, so I did not build anything more elaborate; two call sites shared
one enum, nothing framework-shaped.

Tests that fail against the old code (acceptance criterion 2):

  • FleetMcpTest.replyWithNoPendingSendIsQueuedNotError now asserts the QUEUED description
    instead of "delivered" — fails against the old code, which always returned "delivered".
  • FleetMcpTest.sendThenReplyRoundTrips / asyncSendReturnsATicketThenPollReportsTheReply /
    askThenAnswerRoundTrips now assert the RESOLVED_SEND description specifically.
  • FleetAppTest.messageReturnsTheWorkersStructuredReply now asserts
    delivered=true, outcome="resolved_send" on the REST response body.
  • FleetAppTest.replyWithNoPendingSendQueuesInsteadOfConflict now asserts
    delivered=false, outcome="queued" — fails against the old code, which always sent delivered: true.
  • MessageServiceTest.replyQueuesInInboxWhenNoSendIsOpen / replyResolvesOpenSendDoesNotQueue now
    assert the specific ReplyOutcome (this is the pair the ticket asked for: one send-waiting case,
    one not).

I also had to update every other assertTrue(messages.reply(...)) call site in
MessageServiceTest.java to compile against the new return type (~13 sites). Where the scenario's
own comments/setup made the expected path unambiguous (a live forward waiter open vs. an
already-closed one vs. two ambiguous candidates falling to the inbox), I asserted the specific
ReplyOutcome rather than just dropping the assertion — this doubles as regression coverage for
those already-subtle async/ask-timeout races. All of them passed on the first mvn clean install
run, which is the strongest evidence I have that I read each scenario correctly.

Wording surfaces I found documenting the old "delivered" behavior

  • fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java — reply()'s return text("delivered") (fixed).
  • fleetd/src/main/java/dev/ltms/fleet/rest/FleetApp.java — replyMessage()'s unconditional
    "delivered": true (fixed).
  • Their own tests (FleetMcpTest.java, FleetAppTest.java) asserted the old literal wording
    (fixed, listed above).
  • docs/MCP-Contract.md — grepped for delivered/fleet_reply: only generic sequence-diagram
    references to the fleet_reply{content} call and unrelated AgentStatus transition labels
    ("message delivered, picked up" / "input delivered" in the state diagram) — no literal claim
    about the tool's return text. Nothing to change there.
  • CLAUDE.md (this repo's own canonical block / addendum) — grepped, no mention of the
    fleet_reply/REST reply "delivered" wording.
  • wiki/ — not checked out in this worktree (worker worktrees don't get it; per this repo's own
    addendum I should not read/edit it even if it were). I could not check it myself — please check
    whether any wiki page quotes the old fleet_reply → "delivered" wording.

What else has this shape (found, NOT fixed, per the ticket)

  • FleetMcp.ack (fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java:892-898,
    messages.ackReply → ReplyInbox.ack): always returns "acknowledged " + msgId even when the
    msgId never existed. Both InMemoryReplyInbox.ack (InMemoryReplyInbox.java:62-67) and
    AmqpReplyInbox.ack (AmqpReplyInbox.java:397-401) silently no-op and return void when the
    target isn't owned or the id isn't found — the caller has no way to tell "removed" from "was
    never there."
  • FleetMcp.stop (fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java:1523-1534): returns
    "stopped " + paneId" once sessions.release(paneId) returns without throwing. That only proves
    herdr's pane.close RPC was accepted, the same "attempt vs. confirmed" gap as the nudge
    counters — I did not trace whether herdr's pane.close itself waits for the process to actually
    exit, so I'm reporting this with the same "most likely, unverified further" confidence the
    ticket used for item 3, not as a confirmed defect.

Build

cd fleetd && mvn clean install, full output read (not piped):

[INFO] Tests run: 1439, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS

McpContractDocTest: Tests run: 3, Failures: 0, Errors: 0, Skipped: 0 — passes.

Relevant suites individually: MessageServiceTest 82/82, FleetMcpTest 75/75, FleetAppTest
37/37, ReplyPushLoopTest 62/62, LeadHeartbeatLoopTest 14/14 — all green.

Out of scope, not investigated

FleetMcp.ack/FleetMcp.stop findings above are reported only, per the ticket's instruction not
to fix what the "also find" sweep turns up.

## fleetd #365 — three tool results claim "delivered" for something weaker than delivery ### Step 1 — item 3 verified first, as the ticket asked I read `countNudge` and its only two call sites (`LeadHeartbeatLoop.injectNudge`, `ReplyPushLoop.injectNudge`) plus `AgentControl.send` (`fleetd/src/main/java/dev/ltms/fleet/herdr/AgentControl.java:116-118`), which is what both loops call before counting. **Finding: it records a nudge SENT, not a nudge ARRIVED.** `agents.send(target, text)` is one herdr `agent.prompt` call — a one-way paste-and-submit into the pane. It has no confirmation that the lead's Claude session actually read or processed the text; there is no read-receipt concept at this layer at all. `countNudge("delivered")` in both loops fires right after that call returns without throwing — i.e. it counts "the herdr RPC succeeded," not "the lead saw it." **What reads the counter:** only `GET /metrics` (`FleetApp.java:267-271`, wired from `FleetMetrics.HEARTBEAT_NUDGES` / `PUSH_NUDGES`) — a Prometheus-style scrape for an operator's dashboard. Nothing in the codebase branches on the counter's value. Since nothing else claims more than "sent" either, this is the honest-rename case the ticket described, not a counting bug — **I renamed the outcome label `"delivered"` → `"sent"`** in both loops, updated `FleetMetrics`'s `describe()` text for both series to say what "sent" actually means, and updated the three `ReplyPushLoopTest` assertions that checked the old label (`successfulNudgeIncrementsDelivered` → `successfulNudgeIncrementsSent`, `successfulTicketNudgeIncrementsDelivered` → `successfulTicketNudgeIncrementsSent`, plus the zero-count assertion in `reminderCapIncrementsExhausted`). `LeadHeartbeatLoopTest` had no assertions on the metric label, so nothing there needed changing. **The count itself is unchanged** — only the word. ### Step 2 — items 1 and 2 fixed with one shared mechanism `MessageService.reply(String, String)` used to return an always-`true` `boolean` even though it internally already distinguishes three landing paths (visible in its own metrics: `path=rendezvous` / `path=async-recovered` / `path=inbox`). Both callers discarded that distinction: - `FleetMcp.reply` (`fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java`) always returned the literal text `"delivered"`. - `FleetApp.replyMessage` (`fleetd/src/main/java/dev/ltms/fleet/rest/FleetApp.java`) always returned `{"delivered": true}`. **Fix:** added `MessageService.ReplyOutcome` (three values — `RESOLVED_SEND`, `RESOLVED_ASYNC_TICKET`, `QUEUED` — each carrying a `wireName()`, a `delivered()` boolean, and a shared `description()` string), and changed `reply()` to return it instead of `boolean`. Both call sites now read the same enum: - `FleetMcp.reply` returns `outcome.description()` — e.g. `"delivered — resolved the fleet_send that was waiting for it"` vs. `"queued — no send or ticket was waiting; held in the inbox for a later drain"`. - `FleetApp.replyMessage` returns `{"sessionId", "delivered": outcome.delivered(), "outcome": outcome.wireName()}` — `delivered` is now `true` only for the two resolved cases and `false` for `QUEUED`; `outcome` names which of the three actually happened. New production code is the `ReplyOutcome` enum (~40 lines) plus the two small call-site edits — well under the 80-line ceiling, so I did not build anything more elaborate; two call sites shared one enum, nothing framework-shaped. **Tests that fail against the old code (acceptance criterion 2):** - `FleetMcpTest.replyWithNoPendingSendIsQueuedNotError` now asserts the `QUEUED` description instead of `"delivered"` — fails against the old code, which always returned `"delivered"`. - `FleetMcpTest.sendThenReplyRoundTrips` / `asyncSendReturnsATicketThenPollReportsTheReply` / `askThenAnswerRoundTrips` now assert the `RESOLVED_SEND` description specifically. - `FleetAppTest.messageReturnsTheWorkersStructuredReply` now asserts `delivered=true, outcome="resolved_send"` on the REST response body. - `FleetAppTest.replyWithNoPendingSendQueuesInsteadOfConflict` now asserts `delivered=false, outcome="queued"` — fails against the old code, which always sent `delivered: true`. - `MessageServiceTest.replyQueuesInInboxWhenNoSendIsOpen` / `replyResolvesOpenSendDoesNotQueue` now assert the specific `ReplyOutcome` (this is the pair the ticket asked for: one send-waiting case, one not). I also had to update every other `assertTrue(messages.reply(...))` call site in `MessageServiceTest.java` to compile against the new return type (~13 sites). Where the scenario's own comments/setup made the expected path unambiguous (a live forward waiter open vs. an already-closed one vs. two ambiguous candidates falling to the inbox), I asserted the specific `ReplyOutcome` rather than just dropping the assertion — this doubles as regression coverage for those already-subtle async/ask-timeout races. All of them passed on the first `mvn clean install` run, which is the strongest evidence I have that I read each scenario correctly. ### Wording surfaces I found documenting the old "delivered" behavior - `fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java` — `reply()`'s `return text("delivered")` (fixed). - `fleetd/src/main/java/dev/ltms/fleet/rest/FleetApp.java` — `replyMessage()`'s unconditional `"delivered": true` (fixed). - Their own tests (`FleetMcpTest.java`, `FleetAppTest.java`) asserted the old literal wording (fixed, listed above). - `docs/MCP-Contract.md` — grepped for `delivered`/`fleet_reply`: only generic sequence-diagram references to the `fleet_reply{content}` call and unrelated `AgentStatus` transition labels ("message delivered, picked up" / "input delivered" in the state diagram) — no literal claim about the tool's return text. Nothing to change there. - `CLAUDE.md` (this repo's own canonical block / addendum) — grepped, no mention of the `fleet_reply`/REST reply "delivered" wording. - `wiki/` — not checked out in this worktree (worker worktrees don't get it; per this repo's own addendum I should not read/edit it even if it were). I could not check it myself — please check whether any wiki page quotes the old `fleet_reply` → `"delivered"` wording. ### What else has this shape (found, NOT fixed, per the ticket) - `FleetMcp.ack` (`fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java:892-898`, `messages.ackReply` → `ReplyInbox.ack`): always returns `"acknowledged " + msgId` even when the msgId never existed. Both `InMemoryReplyInbox.ack` (`InMemoryReplyInbox.java:62-67`) and `AmqpReplyInbox.ack` (`AmqpReplyInbox.java:397-401`) silently no-op and return `void` when the target isn't owned or the id isn't found — the caller has no way to tell "removed" from "was never there." - `FleetMcp.stop` (`fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java:1523-1534`): returns `"stopped " + paneId"` once `sessions.release(paneId)` returns without throwing. That only proves herdr's `pane.close` RPC was accepted, the same "attempt vs. confirmed" gap as the nudge counters — I did not trace whether herdr's `pane.close` itself waits for the process to actually exit, so I'm reporting this with the same "most likely, unverified further" confidence the ticket used for item 3, not as a confirmed defect. ### Build `cd fleetd && mvn clean install`, full output read (not piped): ``` [INFO] Tests run: 1439, Failures: 0, Errors: 0, Skipped: 0 [INFO] BUILD SUCCESS ``` `McpContractDocTest`: `Tests run: 3, Failures: 0, Errors: 0, Skipped: 0` — passes. Relevant suites individually: `MessageServiceTest` 82/82, `FleetMcpTest` 75/75, `FleetAppTest` 37/37, `ReplyPushLoopTest` 62/62, `LeadHeartbeatLoopTest` 14/14 — all green. ### Out of scope, not investigated `FleetMcp.ack`/`FleetMcp.stop` findings above are reported only, per the ticket's instruction not to fix what the "also find" sweep turns up.
agent added 1 commit 2026-09-09 02:24:39 +02:00
t365: fleet_reply/REST reply distinguish resolved vs queued; rename nudge metric outcome
CI / contract (pull_request) Successful in 1m17s
Details
CI / build (pull_request) Successful in 1m51s
Details
a9a3c12232 
fleetd #365. fleet_reply always returned the literal "delivered" and
POST /sessions/{id}/reply always returned {"delivered": true}, whether
the reply resolved a live waiting send/ticket or was merely queued in
the inbox for a later drain (CB-307) — both are successes, but not the
same fact.

MessageService.reply() now returns a ReplyOutcome (RESOLVED_SEND,
RESOLVED_ASYNC_TICKET, or QUEUED) instead of an always-true boolean.
FleetMcp.reply and FleetApp.replyMessage both read it: the MCP tool
result names which happened, and the REST body's "delivered" field is
now accurate, with an added "outcome" field.

Also renames the heartbeat/push-loop nudge metric's "delivered" outcome
to "sent" (LeadHeartbeatLoop, ReplyPushLoop, FleetMetrics): it only
records that the herdr agent.prompt paste-and-submit call succeeded,
never that the lead's pane actually read it — there is no read-receipt
concept at that layer, so "delivered" overclaimed there too.

Tests: MessageServiceTest/FleetMcpTest/FleetAppTest strengthened to
assert the specific outcome per case (a resolved send, a resolved async
ticket, and a queued reply); ReplyPushLoopTest updated for the outcome
rename.
ltms closed this pull request 2026-09-09 02:38:15 +02:00
Some checks are pending
CI / contract (pull_request) Successful in 1m17s
Details
CI / build (pull_request) Successful in 1m51s
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
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#379
Delete Branch "worker/t365-3920c5-3"

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?