fleetd #368 review: only agent_not_found may forget a lead binding

isLive treated any RuntimeException from the liveness probe as "the lead is
gone", which forgetDelegation then acted on destructively and permanently.
That made a transient herdr hiccup (socket blip, decode error) on a perfectly
live lead indistinguishable from the lead actually being dead — the same
one-bad-reading mistake #359 shipped a guard against for lead-tab liveness.

Narrow isLive to match AgentControl.agentCall's own rule: only an affirmative
HerdrException("agent_not_found") counts as gone. Every other failure is
treated as still live and the binding is left alone.

Adds aTransientLivenessFailureMustNotForgetABindingToAStillLiveLead, which
fails with the bare RuntimeException catch and passes with the narrowed one.

fleetd/src/main/java/dev/ltms/fleet/msg/ReplyPushLoop.java

@@ -2,6 +2,7 @@ package dev.ltms.fleet.msg;
 
 import dev.ltms.fleet.herdr.AgentControl;
 import dev.ltms.fleet.herdr.AgentStatus;
+import dev.ltms.fleet.herdr.HerdrException;
 import dev.ltms.fleet.mcp.PrimaryRegistry;
 import dev.ltms.fleet.metrics.FleetMetrics;
 import dev.ltms.fleet.metrics.Metrics;
@@ -398,6 +399,18 @@ public final class ReplyPushLoop {
      * {@code AgentControl.paneByTerminal} already does on {@code agent_not_found}) and resolution is
      * retried, which now reaches the fallback {@code nudgeTargetFor} was built to reach — the same
      * empty-map state its javadoc already argues is correct.
+     *
+     * <p><strong>fleetd #368 review — only a positive "gone" reading forgets the binding.</strong>
+     * The first version of this method treated <em>any</em> {@code RuntimeException} from the probe
+     * as death, which is the #359 mistake repeated: a transient socket blip or a codec error on a
+     * perfectly live lead would silently and permanently unbind it, with no re-record ever coming.
+     * That is destructive on one bad reading, exactly what #359 shipped a two-reading guard to avoid
+     * for the analogous lead-tab-liveness question. {@link #isLive} now matches
+     * {@code AgentControl.agentCall}'s own narrower rule (see its {@code agent_not_found} check): only
+     * that specific, affirmative "herdr has no such agent" signal counts as gone. Every other failure
+     * — timeout, transport error, a decode error — is treated as still live and the binding is left
+     * alone, because guessing wrong here is unrecoverable while guessing "live" merely costs one more
+     * retry on the next tick, which {@link #decide} already tolerates.
      */
     private Optional<String> resolveLiveLead(String target) {
         Optional<String> lead = primaryRegistry.nudgeTargetFor(target);
@@ -410,14 +423,31 @@ public final class ReplyPushLoop {
         return primaryRegistry.nudgeTargetFor(target);
     }
 
-    /** Whether herdr still reports a status for {@code lead} — false for a closed/dead terminal. */
+    /**
+     * Whether {@code lead} should still be trusted: {@code false} only when herdr affirmatively
+     * reports the terminal gone ({@code agent_not_found}), never on a merely inconclusive failure.
+     *
+     * <p>fleetd #368 review: an earlier version returned {@code false} for any {@code RuntimeException},
+     * which made a transient herdr hiccup on a live lead indistinguishable from the lead actually
+     * being dead — and the caller's response to {@code false} ({@code forgetDelegation}) is
+     * destructive and permanent. Narrowed to the one code {@code AgentControl.agentCall} itself
+     * already treats as a genuine, resolvable absence (see its {@code agent_not_found} handling) —
+     * every other {@code RuntimeException} is treated as "still live" and the binding survives to be
+     * probed again next time, which costs nothing worse than one more retry.
+     */
     private boolean isLive(String lead) {
         try {
             agents.status(lead);
             return true;
         } catch (RuntimeException e) {
-            log.debug("push: liveness check failed for lead {}: {}", lead, e.toString());
-            return false;
+            boolean gone = e instanceof HerdrException he && "agent_not_found".equals(he.code());
+            if (gone) {
+                log.debug("push: lead {} no longer exists ({})", lead, e.toString());
+            } else {
+                log.debug("push: liveness check for lead {} was inconclusive ({}); treating as live "
+                        + "rather than risk destroying a live binding", lead, e.toString());
+            }
+            return !gone;
         }
     }
 

fleetd/src/test/java/dev/ltms/fleet/msg/ReplyPushLoopTest.java

@@ -271,6 +271,39 @@ class ReplyPushLoopTest {
                 "the stale binding must be forgotten even when there is no fallback to hand back");
     }
 
+    /**
+     * fleetd #368 review, must-fix: the first version of {@code isLive} treated <em>any</em>
+     * {@code RuntimeException} from the liveness probe as "the lead is gone" — indistinguishable
+     * from a transient herdr hiccup (a socket blip, a decode error) on a lead that is actually
+     * still live. The consequence of that misdiagnosis is destructive and permanent
+     * ({@code forgetDelegation}), which is the exact #359 mistake repeated two days later: a single
+     * bad reading must never destroy a live binding. This pins the narrower rule — only an
+     * affirmative {@code agent_not_found} may forget a binding; a merely inconclusive failure must
+     * leave the binding alone, and the lead must still be nudged once the probe recovers.
+     */
+    @Test
+    void aTransientLivenessFailureMustNotForgetABindingToAStillLiveLead() throws Exception {
+        // OTHER_PRIMARY is delegated to and genuinely live — its FIRST agent.get call fails with a
+        // transient, non-agent_not_found HerdrException (a transport-level failure, code null,
+        // exactly what a socket blip looks like), then succeeds on every call after.
+        registry.recordDelegation(WORKER, OTHER_PRIMARY);
+
+        var rec = new FlakyThenLiveHerdrClient(OTHER_PRIMARY);
+        agents = new AgentControl(rec);
+        inbox.publish(WORKER, "m1", "hello");
+
+        loop(2, 50).onReplyQueued(WORKER);
+
+        assertTrue(rec.sendLatch.await(3, TimeUnit.SECONDS),
+                "the nudge must still reach the live lead once the transient failure clears");
+        assertEquals(List.of(OTHER_PRIMARY), rec.promptTargets(),
+                "the nudge must go to the lead that was only transiently unreachable, not the "
+                        + "unrelated pinned primary");
+        assertEquals(OTHER_PRIMARY, registry.nudgeTargetFor(WORKER).orElseThrow(),
+                "a merely transient failure must not forget the binding to a lead that is actually "
+                        + "still live");
+    }
+
     // --- nudge format --------------------------------------------------------------------------
 
     @Test
@@ -1212,4 +1245,51 @@ class ReplyPushLoopTest {
         public void close() {
         }
     }
+
+    /**
+     * Fake herdr client for fleetd #368 review: {@code flakyTarget}'s FIRST {@code agent.get} call
+     * fails with a transient, non-{@code agent_not_found} {@code HerdrException} — a transport-level
+     * failure (code {@code null}), exactly what a socket blip or a decode error on a perfectly live
+     * lead looks like — then succeeds ({@code idle}) on every call after. Used to prove a merely
+     * inconclusive failure must not be treated as the lead being gone.
+     */
+    private static final class FlakyThenLiveHerdrClient implements HerdrClient {
+        private final String flakyTarget;
+        private final AtomicInteger getCalls = new AtomicInteger();
+        private final List<String> promptTargets = Collections.synchronizedList(new ArrayList<>());
+        volatile CountDownLatch sendLatch = new CountDownLatch(1);
+
+        FlakyThenLiveHerdrClient(String flakyTarget) {
+            this.flakyTarget = flakyTarget;
+        }
+
+        @Override
+        @SuppressWarnings("unchecked")
+        public JsonNode call(String method, Object params) {
+            Map<String, Object> p = params instanceof Map ? (Map<String, Object>) params : Map.of();
+            if ("agent.get".equals(method)) {
+                String target = String.valueOf(p.get("target"));
+                if (flakyTarget.equals(target) && getCalls.getAndIncrement() == 0) {
+                    throw new HerdrException("herdr socket read timed out"); // transport failure, code == null
+                }
+                return MAPPER.createObjectNode()
+                        .set("agent", MAPPER.createObjectNode()
+                                .put("terminal_id", target)
+                                .put("agent_status", "idle"));
+            }
+            if ("agent.prompt".equals(method)) {
+                promptTargets.add(String.valueOf(p.get("target")));
+                sendLatch.countDown();
+            }
+            return MAPPER.createObjectNode();
+        }
+
+        List<String> promptTargets() {
+            return List.copyOf(promptTargets);
+        }
+
+        @Override
+        public void close() {
+        }
+    }
 }