fleetd #553: split sentHandled's two meanings so onDelivered's own throw still completes the future

Lead review of PR #557 (ticket comment 16916) found one path left open: sentHandled
is set to true BEFORE onDelivered() runs (correctly, per the earlier fix), so when
onDelivered() itself throws on the normal path, the finally's 'if (sent != null &&
!sentHandled)' guard skipped the whole recovery -- completion included -- and left
sent.delivered() pending forever for a message that really was delivered.

sentHandled must guard only the onDelivered RE-CALL (the permanent-suppression
hazard), never the future completion, since CompletableFuture.complete/
completeExceptionally are idempotent and a no-op on the already-handled path.
Split the one flag's two jobs: the outer 'if (sent != null)' now always runs the
recovery block, and '!sentHandled' moved onto just the onDelivered call inside it.

Added anOnDeliveredThrowOnTheNormalPathStillCompletesTheDeliveryFuture, proven with
the lead's own mutation (reverting !sentHandled onto the outer if): the new test
goes red while anOnDeliveredThrowAfterItsOwnRegistrationDoesNotRunASecondTime stays
green, showing the two concerns are genuinely separate.

fleetd/src/main/java/dev/ltms/fleet/inject/Injector.java

@@ -615,13 +615,26 @@ public final class Injector {
             // like success. onDelivered() runs only when sendError == null: nothing was delivered on
             // the error path, so there is nothing to register.
             //
+            // fleetd #553 (lead review, comment 16916): `sentHandled` guards ONLY the onDelivered
+            // re-call below, never the future completion outside this inner try. One boolean cannot
+            // carry both meanings — "onDelivered was called" and "the future has been dealt with" —
+            // because they come apart exactly when onDelivered throws PART WAY THROUGH: sentHandled
+            // is already true (set before the call, correctly — see the `if (sent != null)` block
+            // above), so a guard on the outer `if` here would skip this whole recovery, including the
+            // completion, and leave sent.delivered() pending forever even though the message really
+            // was typed into the pane and taken off the queue. So `sent != null` alone gates whether
+            // this target has anything to finish; `!sentHandled` gates only the onDelivered re-call
+            // inside. CompletableFuture.complete/completeExceptionally are idempotent — on the
+            // ordinary path (sentHandled == true, no throw) the `if (sent != null)` block above has
+            // already completed this future, so the calls below are a no-op returning false.
+            //
             // This recovery is wrapped in its own try/catch(Throwable) that swallows only ITS OWN
             // throwable and logs at WARN — the future is still completed either way — while the
             // ORIGINAL throwable from the try block above is left alone to keep unwinding out of
             // this method to StatusPoller's catch (Throwable), so a listener bug stays loud.
-            if (sent != null && !sentHandled) {
+            if (sent != null) {
                 try {
-                    if (sendError == null) {
+                    if (!sentHandled && sendError == null) {
                         turnListener.onDelivered(target, sent.token());
                     }
                 } catch (Throwable recoveryError) {
@@ -630,6 +643,8 @@ public final class Injector {
                                     + "anyway: {}",
                             target, recoveryError.getMessage());
                 } finally {
+                    // No-op (returns false) on the ordinary path, where the `if (sent != null)` block
+                    // above already completed this future — see the comment above this block.
                     if (sendError != null) {
                         sent.delivered().completeExceptionally(sendError);
                     } else {

fleetd/src/test/java/dev/ltms/fleet/inject/InjectorTest.java

@@ -1153,4 +1153,48 @@ class InjectorTest {
                         + "AFTER the call (rather than before) would leave it false here and the "
                         + "finally backstop would call onDelivered a second time");
     }
+
+    @Test
+    void anOnDeliveredThrowOnTheNormalPathStillCompletesTheDeliveryFuture() {
+        // fleetd #553, lead review of PR #557 (ticket comment 16916): `sentHandled` must guard ONLY
+        // the onDelivered RE-CALL in the finally, never the future completion alongside it. Gating
+        // BOTH behind `!sentHandled` (the shape this test is red against) misses the one path the
+        // flag's own correct placement creates: `sentHandled` is set to true FIRST, before the
+        // onDelivered() call, inside the `if (sent != null)` block above (see the previous test —
+        // that placement is right and must not change). So when onDelivered() itself throws on that
+        // NORMAL path, `sentHandled` already reads true by the time control reaches the finally, and
+        // a `sent != null && !sentHandled` guard around the WHOLE recovery — completion included —
+        // skips it entirely. The message was typed into the target's pane and taken off the queue
+        // inside the monitor, same as any other delivery, so its future is left pending forever: the
+        // exact defect this ticket exists to close, just reached from a different throwing call.
+        //
+        // The fix splits the one flag's two jobs: `!sentHandled` keeps gating only the onDelivered
+        // call (so the exactly-once guarantee in the test above still holds — CompletableFuture.
+        // complete/completeExceptionally are idempotent, so completing unconditionally here is a
+        // no-op on the ordinary path, where the `if (sent != null)` block already completed it.
+        RuntimeException boom = new RuntimeException("boom from onDelivered on the normal path");
+        TurnListener listener = new TurnListener() {
+            @Override
+            public void onTurnComplete(String target) {
+            }
+
+            @Override
+            public void onDelivered(String target, TurnToken token) {
+                throw boom;
+            }
+        };
+        Injector inj = new Injector(new AgentControl(herdr), listener);
+        CompletableFuture<Void> delivered = inj.enqueue(T, "task", TestTurnTokens.inert(T)).completion();
+
+        RuntimeException thrown = assertThrows(RuntimeException.class,
+                () -> inj.onStatus(T, AgentStatus.IDLE), // delivers "task"; onDelivered throws
+                "the original throwable from onDelivered must still escape onStatus");
+        assertSame(boom, thrown);
+
+        assertTrue(delivered.isDone(),
+                "fleetd #553: \"task\" was actually delivered — typed into the pane and taken off "
+                        + "the queue inside the monitor — so its delivery future must be completed on "
+                        + "every path out of onStatus, including the one where onDelivered itself is "
+                        + "what threw. Leaving it pending here is a hang, not a fix");
+    }
 }