From ed28b51f12a8909418c3eed8dfdc21f34c32234a Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 12 Sep 2026 16:32:36 +0700 Subject: [PATCH 1/2] =?UTF-8?q?fleetd=20#513:=20fix=20TIMED=5FOUT=5FQUEUED?= =?UTF-8?q?=20javadoc=20=E2=80=94=20cancelled,=20not=20queued?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two javadoc blocks (queuedDeliveries field, hasQueuedDelivery) said a timed-out message is still sitting in the injector's per-target queue. CB-640 made send() cancel it via Injector.cancel() instead, so the message is gone and will never arrive. Rewrote both to describe cancellation. Also fixed a third instance of the same stale claim in hasOrphanedDelegation's javadoc, and added a line to the TIMED_OUT_QUEUED enum constant's own comment clarifying the name is kept but no longer means the message stays queued. Per the ticket's follow-up comment: no rename (TIMED_OUT_QUEUED reaches FleetApp.java REST mapping and FleetMcp.java — out of scope here) and no behavior change; comments only. --- .../dev/ltms/fleet/msg/MessageService.java | 32 ++++++++++++------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/fleetd/src/main/java/dev/ltms/fleet/msg/MessageService.java b/fleetd/src/main/java/dev/ltms/fleet/msg/MessageService.java index b76c5cf..0c445bb 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/msg/MessageService.java +++ b/fleetd/src/main/java/dev/ltms/fleet/msg/MessageService.java @@ -92,7 +92,13 @@ public final class MessageService { QUESTION, /** Timed out after the message was delivered — the worker is still working. */ TIMED_OUT_WORKING, - /** Timed out before delivery — the message is still queued for the worker. */ + /** + * Timed out before delivery. Despite the name, the message is not left queued: {@link + * #send} cancels the pending entry ({@link Injector#cancel}) before returning this + * outcome, so it will not arrive later — the target never saw a word of it. The name + * records that the message was still queued at the moment the caller gave up, not that it + * stays queued afterward. + */ TIMED_OUT_QUEUED, /** Another send to this session was in flight for the whole window. */ BUSY, @@ -301,10 +307,12 @@ public final class MessageService { /** * Targets whose last send timed out with {@link Outcome#TIMED_OUT_QUEUED} (CB-640) — the * message never reached the {@link Injector} delivery window before the caller's deadline, so - * it is still sitting in the injector's own per-target queue. Set where {@link #send} already - * computes {@code wasDelivered} for that outcome; no new queue is kept here, only the fact. - * Cleared the same way as {@link #strandedReplies}: the next accepted delivery for the target - * ({@link #send} opening a fresh waiter) or a teardown ({@link #abandon}). + * {@link #send} cancelled it via {@link Injector#cancel} before it could ever be delivered. The + * message is gone, not pending: it will not arrive later, and the target never saw it. Set + * where {@link #send} already computes {@code wasDelivered} for that outcome; no queue is kept + * here, only the fact that a delivery was cancelled. Cleared the same way as + * {@link #strandedReplies}: the next accepted delivery for the target ({@link #send} opening a + * fresh waiter) or a teardown ({@link #abandon}). */ private final ConcurrentHashMap queuedDeliveries = new ConcurrentHashMap<>(); private final AtomicLong ticketSeq = new AtomicLong(); @@ -393,10 +401,12 @@ public final class MessageService { * Read-only delegation fact for fleet views (CB-640): {@code target}'s last send timed out * before the {@link Injector} ever delivered it — the caller saw * {@link Outcome#TIMED_OUT_QUEUED} (see the {@code TimeoutException} branch of {@link #send}), - * and the message is still sitting in the injector's per-target queue waiting for the worker - * to go idle. Distinct from {@link Outcome#TIMED_OUT_WORKING}, where delivery already happened - * and only the reply is outstanding. Cleared the next time this target's delivery is accepted - * or the target is abandoned — see {@link #queuedDeliveries}. + * and {@link Injector#cancel} removed the pending entry before it could be delivered. Despite + * the method's name, the message is not queued and will not arrive later — the target never + * saw a word of it; this reports a cancellation, not a pending delivery. Distinct from + * {@link Outcome#TIMED_OUT_WORKING}, where delivery already happened and only the reply is + * outstanding. Cleared the next time this target's delivery is accepted or the target is + * abandoned — see {@link #queuedDeliveries}. */ public boolean hasQueuedDelivery(String target) { return target != null && queuedDeliveries.containsKey(target); @@ -416,8 +426,8 @@ public final class MessageService { /** * Read-only delegation fact for fleet views (CB-640): an async ticket is still * {@link Phase#PENDING} against {@code target}, yet nothing is actually in flight for it — no - * open rendezvous waiter ({@link #hasAcceptedDelivery}) and no message still sitting in the - * injector's queue ({@link #hasQueuedDelivery}). A healthy PENDING ticket can briefly look this + * open rendezvous waiter ({@link #hasAcceptedDelivery}) and no record of a cancelled, + * undelivered send ({@link #hasQueuedDelivery}). A healthy PENDING ticket can briefly look this * way while its virtual thread has not yet been scheduled or is blocked on the session lock * behind another send to the same target, so this is a snapshot fact for the health classifier * to weigh across ticks, not proof on its own that the ticket is stuck. It also genuinely -- 2.52.0 From cb64bc8157214bce20b19788d45d628585524f80 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 12 Sep 2026 16:45:35 +0700 Subject: [PATCH 2/2] =?UTF-8?q?fleetd=20#513:=20rework=20=E2=80=94=20TIMED?= =?UTF-8?q?=5FOUT=5FQUEUED=20has=20four=20routes,=20not=20one?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Comment 16984 on the ticket showed my first pass (ed28b51) replaced one wrong invariant with a narrower one: it described injector.cancel() cancelling a queued entry as THE mechanism, when three of its four routes (Injector.java: the send-to-terminal call throwing, the readiness grace expiring, or the target being torn down) never cancel anything — cancel() just reports a state a different code path already set. It also claimed the target never saw a word of the message, which is not established when the herdr agent.prompt call throws after already pasting. Rewrote all four comments (the TIMED_OUT_QUEUED enum constant, queuedDeliveries, hasQueuedDelivery, hasOrphanedDelegation) plus the pre-existing inline comment that seeded the original bad wording, to state only what holds on every route: the message will not arrive later and is not sitting in a queue. The CANCELLED case is called out as the only one where the target is known to have seen nothing; the NOT_DELIVERED case (including the herdr-send-threw route) is flagged as leaving that open. Comment-only; no behavior change. --- .../dev/ltms/fleet/msg/MessageService.java | 78 +++++++++++-------- 1 file changed, 47 insertions(+), 31 deletions(-) diff --git a/fleetd/src/main/java/dev/ltms/fleet/msg/MessageService.java b/fleetd/src/main/java/dev/ltms/fleet/msg/MessageService.java index 0c445bb..9ce814d 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/msg/MessageService.java +++ b/fleetd/src/main/java/dev/ltms/fleet/msg/MessageService.java @@ -93,11 +93,17 @@ public final class MessageService { /** Timed out after the message was delivered — the worker is still working. */ TIMED_OUT_WORKING, /** - * Timed out before delivery. Despite the name, the message is not left queued: {@link - * #send} cancels the pending entry ({@link Injector#cancel}) before returning this - * outcome, so it will not arrive later — the target never saw a word of it. The name - * records that the message was still queued at the moment the caller gave up, not that it - * stays queued afterward. + * Timed out with no confirmed delivery. Despite the name, this does not mean the message + * is sitting in a queue — on every route it will not arrive later. {@link #send} reaches + * this outcome through {@link Injector#cancel}, whose result tells the routes apart: + * {@link Injector.Cancellation#CANCELLED} means the message was still queued and this call + * removed it, so the target saw nothing; {@link Injector.Cancellation#NOT_DELIVERED} means + * an earlier attempt already decided the message's fate — the injector's call to the + * target's terminal ({@link dev.ltms.fleet.herdr.AgentControl#send}) threw, or the queue + * was cleared because the target never became ready or was abandoned — and {@code cancel} + * is only reporting that pre-existing state. On the failed-attempt route the terminal may + * already hold a partial paste from before the call threw, so only the {@code CANCELLED} + * case establishes that the target saw nothing. */ TIMED_OUT_QUEUED, /** Another send to this session was in flight for the whole window. */ @@ -305,14 +311,18 @@ public final class MessageService { */ private final ConcurrentHashMap strandedReplies = new ConcurrentHashMap<>(); /** - * Targets whose last send timed out with {@link Outcome#TIMED_OUT_QUEUED} (CB-640) — the - * message never reached the {@link Injector} delivery window before the caller's deadline, so - * {@link #send} cancelled it via {@link Injector#cancel} before it could ever be delivered. The - * message is gone, not pending: it will not arrive later, and the target never saw it. Set - * where {@link #send} already computes {@code wasDelivered} for that outcome; no queue is kept - * here, only the fact that a delivery was cancelled. Cleared the same way as - * {@link #strandedReplies}: the next accepted delivery for the target ({@link #send} opening a - * fresh waiter) or a teardown ({@link #abandon}). + * Targets whose last send timed out with {@link Outcome#TIMED_OUT_QUEUED} (CB-640) — {@link + * #send} called {@link Injector#cancel} and got back something other than {@code DELIVERED}. + * That covers more than one history: the message may have still been queued and {@code cancel} + * removed it right there, or an earlier attempt may already have failed (the call to the + * target's terminal threw) or been abandoned (the target never became ready, or was torn + * down). What holds on every route: the message will not arrive later, and it is not sitting + * in any queue. What does NOT hold on every route: that the target saw nothing — a failed + * delivery attempt can leave a partial paste behind. Set where {@link #send} already computes + * {@code wasDelivered} for that outcome; no queue is kept here, only the fact that the send + * ended with no confirmed delivery. Cleared the same way as {@link #strandedReplies}: the next + * accepted delivery for the target ({@link #send} opening a fresh waiter) or a teardown + * ({@link #abandon}). */ private final ConcurrentHashMap queuedDeliveries = new ConcurrentHashMap<>(); private final AtomicLong ticketSeq = new AtomicLong(); @@ -399,14 +409,17 @@ public final class MessageService { /** * Read-only delegation fact for fleet views (CB-640): {@code target}'s last send timed out - * before the {@link Injector} ever delivered it — the caller saw - * {@link Outcome#TIMED_OUT_QUEUED} (see the {@code TimeoutException} branch of {@link #send}), - * and {@link Injector#cancel} removed the pending entry before it could be delivered. Despite - * the method's name, the message is not queued and will not arrive later — the target never - * saw a word of it; this reports a cancellation, not a pending delivery. Distinct from - * {@link Outcome#TIMED_OUT_WORKING}, where delivery already happened and only the reply is - * outstanding. Cleared the next time this target's delivery is accepted or the target is - * abandoned — see {@link #queuedDeliveries}. + * with no confirmed delivery — the caller saw {@link Outcome#TIMED_OUT_QUEUED} (see the + * {@code TimeoutException} branch of {@link #send}). Despite the method's name, this is not + * proof that a message is sitting in a queue: {@link Injector#cancel} reports this outcome + * either because the message was still queued and got removed right there, or because an + * earlier attempt already failed (the call to the target's terminal threw) or was abandoned + * (the target never became ready, or was torn down). Either way the message will not arrive + * later. It does NOT follow that the target saw nothing — on the failed-attempt route the + * terminal may already hold a partial paste. Distinct from {@link Outcome#TIMED_OUT_WORKING}, + * where delivery already happened and only the reply is outstanding. Cleared the next time + * this target's delivery is accepted or the target is abandoned — see + * {@link #queuedDeliveries}. */ public boolean hasQueuedDelivery(String target) { return target != null && queuedDeliveries.containsKey(target); @@ -426,15 +439,15 @@ public final class MessageService { /** * Read-only delegation fact for fleet views (CB-640): an async ticket is still * {@link Phase#PENDING} against {@code target}, yet nothing is actually in flight for it — no - * open rendezvous waiter ({@link #hasAcceptedDelivery}) and no record of a cancelled, - * undelivered send ({@link #hasQueuedDelivery}). A healthy PENDING ticket can briefly look this - * way while its virtual thread has not yet been scheduled or is blocked on the session lock - * behind another send to the same target, so this is a snapshot fact for the health classifier - * to weigh across ticks, not proof on its own that the ticket is stuck. It also genuinely - * persists — not just as a passing race — once an async {@code fleet_ask} lapses unanswered: - * {@link #ask} clears the ticket's question and returns it to {@code PENDING}, but {@link #send} - * already closed the forward waiter the instant the question surfaced, so the target has - * neither an accepted nor a queued delivery left to show for it. + * open rendezvous waiter ({@link #hasAcceptedDelivery}) and no record of a send that ended with + * no confirmed delivery ({@link #hasQueuedDelivery}). A healthy PENDING ticket can briefly look + * this way while its virtual thread has not yet been scheduled or is blocked on the session + * lock behind another send to the same target, so this is a snapshot fact for the health + * classifier to weigh across ticks, not proof on its own that the ticket is stuck. It also + * genuinely persists — not just as a passing race — once an async {@code fleet_ask} lapses + * unanswered: {@link #ask} clears the ticket's question and returns it to {@code PENDING}, but + * {@link #send} already closed the forward waiter the instant the question surfaced, so the + * target has neither an accepted nor a queued delivery left to show for it. * *

Deliberately still {@code question == null} only (fleetd #275). This * method must not also report a still-{@link Phase#ASKING} task as orphaned: the worker may @@ -962,7 +975,10 @@ public final class MessageService { log.debug("send to {} timed out (delivered={})", target, wasDelivered); if (!wasDelivered) { // CB-640: record that delivery did not happen for fleet health (see - // queuedDeliveries). The exact Pending was cancelled, so it cannot arrive later. + // queuedDeliveries). Whatever injector.cancel() reported above — this call + // removed a still-queued Pending, or an earlier attempt already failed or + // was abandoned — the send ends with no confirmed delivery and will not + // arrive later. queuedDeliveries.put(target, Boolean.TRUE); } return recorded(new Reply( -- 2.52.0