fleetd #176 fix 1: {@code agents.get} was previously unguarded here, so a herdr + * {@code *_not_found} answer — which is what happens when the backend process EXITED rather + * than being slow — propagated as a raw {@link HerdrException} instead of the + * {@link PeerUnreachableException} every other failure path of this gate produces, and skipped + * teardown ({@link #stop}) entirely, leaking the pane/tab. {@link #failFastOnGoneBackend} closes + * that gap: it stops waiting immediately (never burns the rest of the timeout), runs the same + * teardown the timeout path below runs, and throws with a message that says the backend exited + * rather than that the pane was slow. Any other {@link HerdrException} still propagates + * unchanged — this gate does not know how to recover from it. */ private void waitUntilInjectableOrThrow(String paneId) { - long deadline = nowMillis.getAsLong() + spawnReadyTimeoutMs; - Object lastStatus = null; + long start = nowMillis.getAsLong(); + long deadline = start + spawnReadyTimeoutMs; + AgentStatus lastStatus = null; while (nowMillis.getAsLong() < deadline) { - var status = agents.status(paneId); - lastStatus = status; - if (status.injectable()) { + Agent sample; + try { + sample = agents.get(paneId); + } catch (HerdrException e) { + if (isAlreadyGone(e)) { + failFastOnGoneBackend(paneId, e, nowMillis.getAsLong() - start); + } + throw e; // any other herdr failure is not ours to interpret — let it propagate + } + lastStatus = sample.status(); + if (lastStatus.injectable() || refinedInjectable(paneId, sample)) { log.debug("peer pane={} reached injectable state", paneId); return; } @@ -937,6 +967,66 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { + spawnReadyTimeoutMs + "ms"); } + /** + * fleetd #176 fix 1: the backend process exited while the gate was still waiting — herdr + * answered {@code *_not_found} instead of ever reporting an injectable status. Fails + * immediately (never burns the rest of {@link #spawnReadyTimeoutMs}), runs the exact same + * teardown {@link #waitUntilInjectableOrThrow}'s timeout path runs, and throws a + * {@link PeerUnreachableException} whose message says the process exited rather than that the + * pane was slow. + * + * @throws PeerUnreachableException always — this method never returns normally + */ + private void failFastOnGoneBackend(String paneId, HerdrException cause, long elapsedMs) { + String tail = readPaneQuietly(paneId); // read before stop() closes the pane + log.warn("peer pane={} backend process exited after {}ms while waiting for injectable " + + "state (herdr: {}) — closing. Pane tail:\n{}", + paneId, elapsedMs, cause.getMessage(), tail); + stop(paneId); + throw new PeerUnreachableException( + "worker pane " + paneId + " backend process exited after " + elapsedMs + + "ms while waiting to become injectable (herdr reported: " + cause.getMessage() + + "). Pane tail:\n" + tail); + } + + /** + * fleetd #176 fix 2: resolve a raw {@link AgentStatus#UNKNOWN} sample into a trustworthy + * injectable state via pane content, the same refinement {@code StatusPoller} applies during a + * peer's working life — but corroborated, because the trap this gate is exposed to that the + * poller is not: a pane whose backend has already exited settles at a plain shell prompt, and + * that prompt commonly contains the same {@code ❯} glyph {@link StatusRefiner#classify} treats + * as "idle at the Claude Code TUI prompt". Naively wiring the refiner in would turn "the backend + * died" into "ready to inject" — strictly worse than today's timeout. + * + *
Two guards, both required: + *
Called only when {@code sample.status() == UNKNOWN}, so a healthy spawn — which never sees + * {@code UNKNOWN} — triggers zero extra herdr calls; only a persistently-{@code unknown} pane + * pays the one extra {@code agent.read} {@link StatusRefiner#refine} performs. + */ + private boolean refinedInjectable(String paneId, Agent sample) { + if (sample.status() != AgentStatus.UNKNOWN) { + return false; + } + if (!"claude".equals(namePrefix)) { + return false; // StatusRefiner.classify reads a Claude Code TUI prompt specifically + } + if (sample.agentType() == null) { + return false; // no corroborating liveness signal — could be a dead pane's bare shell + } + return statusRefiner.refine(paneId, AgentStatus.UNKNOWN, agents).injectable(); + } + /** * fleetd #220: the pane's recent output, clipped, for the readiness-gate timeout log — or a * short note when it cannot be read. Best-effort by construction: this runs on a path that is diff --git a/fleetd/src/test/java/dev/ltms/fleet/herdr/FakeHerdr.java b/fleetd/src/test/java/dev/ltms/fleet/herdr/FakeHerdr.java index b1ff5f2..99357cd 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/herdr/FakeHerdr.java +++ b/fleetd/src/test/java/dev/ltms/fleet/herdr/FakeHerdr.java @@ -44,10 +44,13 @@ public final class FakeHerdr implements HerdrClient { private String agentSendErrorCode = null; private boolean noPanes = false; private volatile String agentStatus = "idle"; // steady-state agent.get status + private volatile String agentType = "claude"; // detected agent kind on agent.get; null = undetected private volatile String readText = "worker transcript tail"; // canned agent.read output private int pinnedStarts = 0; // how many upcoming agent.start calls report a fixed pane private String pinnedStartTerminal; private String pinnedStartPane; + private volatile int agentGetOkCalls = Integer.MAX_VALUE; // how many agent.get calls succeed first + private volatile String agentGetFailCode = null; // error code every agent.get call after that reports public FakeHerdr healthy(boolean h) { this.healthy = h; @@ -103,6 +106,27 @@ public final class FakeHerdr implements HerdrClient { return this; } + /** + * Set the detected agent kind ({@code "agent"} field) that {@code agent.get} reports — + * {@code null} models herdr not (or no longer) detecting a supported backend in the pane, e.g. + * a bare shell prompt (fleetd #176 fix 2 corroboration test). + */ + public FakeHerdr agentType(String type) { + this.agentType = type; + return this; + } + + /** + * Make {@code agent.get} succeed normally for its first {@code okCalls} invocations, then fail + * every call after that with {@code code} — fleetd #176 fix 1's "backend exited mid-wait" + * fixture. {@code okCalls == 0} fails from the very first call. + */ + public FakeHerdr agentGetFailsWithAfter(int okCalls, String code) { + this.agentGetOkCalls = okCalls; + this.agentGetFailCode = code; + return this; + } + /** The text {@code agent.read} returns (the CB-106 completion scrape). */ public FakeHerdr readText(String text) { this.readText = text; @@ -208,10 +232,21 @@ public final class FakeHerdr implements HerdrClient { } yield mapper.readTree("{\"type\":\"ok\"}"); } - case "agent.get" -> mapper.readTree((""" - {"type":"agent_info","agent":{"terminal_id":"term_a","agent":"claude", + case "agent.get" -> { + if (agentGetFailCode != null) { + long getCalls = calls.stream().filter(c -> c.method().equals("agent.get")).count(); + if (getCalls > agentGetOkCalls) { + throw new HerdrException( + "herdr error [" + agentGetFailCode + "]: agent target not found", + agentGetFailCode, null); + } + } + String agentField = agentType == null ? "null" : "\"" + agentType + "\""; + yield mapper.readTree((""" + {"type":"agent_info","agent":{"terminal_id":"term_a","agent":%s, "agent_status":"%s","workspace_id":"w2","tab_id":"w2:t7","pane_id":"w2:p7"}}""") - .formatted(agentStatus)); + .formatted(agentField, agentStatus)); + } case "agent.read" -> mapper.readTree(mapper.writeValueAsString( java.util.Map.of("type", "agent_read", "read", java.util.Map.of("text", readText)))); case "agent.start" -> { diff --git a/fleetd/src/test/java/dev/ltms/fleet/member/ClaudeCodeLauncherTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/ClaudeCodeLauncherTest.java index 255e947..f6127ee 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/ClaudeCodeLauncherTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/ClaudeCodeLauncherTest.java @@ -963,6 +963,152 @@ class ClaudeCodeLauncherTest { assertDoesNotThrow(() -> UUID.fromString(handle.id())); } + // --- fleetd #176 fix 1: fail fast when the backend process exits mid-wait ------------------- + + @Test + void spawnFailsFastWhenBackendProcessExitsMidWaitInsteadOfBurningTheTimeout() { + // First agent.get sees UNKNOWN (one normal tick); the second reports the pane gone, exactly + // what herdr answers when the backend process has already exited. The timeout is generous + // (60s) so a test that wrongly falls through to the old unguarded call — and therefore + // waits out the whole window — is unambiguously distinguishable from one that fails fast. + FakeHerdr herdr = new FakeHerdr(); + herdr.agentStatus("unknown"); + herdr.agentGetFailsWithAfter(1, "pane_not_found"); + long[] clock = {0}; + + ClaudeCodeLauncher svc = new ClaudeCodeLauncher( + new AgentControl(herdr), new WorkspaceControl(herdr), + new SubscriptionGuard(Set.of("gx00.gw")), + workerConfigMap("ltms-local", null), "ltms-local", _ -> null, + 60_000, () -> clock[0], () -> clock[0] += 50); + + PeerUnreachableException ex = assertThrows( + PeerUnreachableException.class, + () -> svc.spawn(new SpawnRequest(null, null, null))); + + assertTrue(ex.getMessage().contains("w9:pRoot_1"), + "exception message references the paneId: " + ex.getMessage()); + assertTrue(ex.getMessage().toLowerCase().contains("exited"), + "exception message says the backend exited, not that the pane was slow: " + + ex.getMessage()); + assertTrue(clock[0] < 60_000, + "the gate must not burn the rest of the 60s timeout: clock only reached " + clock[0]); + long getCalls = herdr.calls.stream().filter(c -> c.method().equals("agent.get")).count(); + assertEquals(2, getCalls, + "exactly one normal poll then the not_found answer — no further polling after that: " + + getCalls); + assertEquals(1, paneCloseCount(herdr, "w9:pRoot_1"), + "the pane is torn down (no orphan) even on the fail-fast path"); + } + + @Test + void spawnLetsAnUnrelatedHerdrErrorPropagateUnchanged() { + // Fix 1 must only special-case a "*_not_found" answer. Any other herdr failure keeps + // propagating as-is — this gate does not know how to recover from it. + FakeHerdr herdr = new FakeHerdr(); + herdr.agentStatus("unknown"); + herdr.agentGetFailsWithAfter(0, "internal_error"); + long[] clock = {0}; + + ClaudeCodeLauncher svc = new ClaudeCodeLauncher( + new AgentControl(herdr), new WorkspaceControl(herdr), + new SubscriptionGuard(Set.of("gx00.gw")), + workerConfigMap("ltms-local", null), "ltms-local", _ -> null, + 60_000, () -> clock[0], () -> clock[0] += 50); + + dev.ltms.fleet.herdr.HerdrException ex = assertThrows( + dev.ltms.fleet.herdr.HerdrException.class, + () -> svc.spawn(new SpawnRequest(null, null, null))); + + assertEquals("internal_error", ex.code()); + assertEquals(0, paneCloseCount(herdr, "w9:pRoot_1"), + "an error this gate does not recognize is not this gate's teardown to run"); + } + + // --- fleetd #176 fix 2: corroborated UNKNOWN refinement -------------------------------------- + + @Test + void refinedIdleIsNotAcceptedWhenAgentTypeIsNull() { + // The trap fix 2 must close: a pane sitting at a bare shell prompt after its backend exited + // still contains the same "❯" glyph StatusRefiner.classify treats as "idle at the Claude + // Code TUI prompt". Without the agentType corroboration this would be misread as injectable + // and the gate would hand back a peer that never started. herdr reports no agentType for + // that bare shell. + FakeHerdr herdr = new FakeHerdr(); + herdr.agentStatus("unknown"); // always UNKNOWN + herdr.agentType(null); // no supported backend detected — could be a bare shell + herdr.readText("some-host:~ user$ ❯ "); // looks exactly like an idle Claude Code prompt + long[] clock = {0}; + + ClaudeCodeLauncher svc = new ClaudeCodeLauncher( + new AgentControl(herdr), new WorkspaceControl(herdr), + new SubscriptionGuard(Set.of("gx00.gw")), + workerConfigMap("ltms-local", null), "ltms-local", _ -> null, + 500, () -> clock[0], () -> clock[0] += 50); + + PeerUnreachableException ex = assertThrows( + PeerUnreachableException.class, + () -> svc.spawn(new SpawnRequest(null, null, null))); + + assertTrue(clock[0] >= 500, + "a null agentType must not let the '❯' prompt refine to injectable — the gate has " + + "to wait out the full timeout: clock only reached " + clock[0]); + assertEquals(1, paneCloseCount(herdr, "w9:pRoot_1"), + "the pane is torn down on timeout, same as any other never-injectable spawn"); + } + + @Test + void refinedIdleIsAcceptedWhenAgentTypeCorroboratesLiveness() { + // The positive case: a genuinely live claude pane that herdr misreports as UNKNOWN (CB-115) + // still resolves to injectable once agentType corroborates that a supported backend is + // detected in the same sample. + FakeHerdr herdr = new FakeHerdr(); + herdr.agentStatus("unknown"); // always UNKNOWN at the raw level + herdr.agentType("claude"); // herdr still detects a live claude backend + herdr.readText("? for shortcuts"); // StatusRefiner.classify's idle footer marker + long[] clock = {0}; + + ClaudeCodeLauncher svc = new ClaudeCodeLauncher( + new AgentControl(herdr), new WorkspaceControl(herdr), + new SubscriptionGuard(Set.of("gx00.gw")), + workerConfigMap("ltms-local", null), "ltms-local", _ -> null, + 60_000, () -> clock[0], () -> clock[0] += 50); + + PeerHandle handle = svc.spawn(new SpawnRequest(null, null, null)); + + assertNotNull(handle, "a corroborated refined-IDLE sample lets the spawn succeed"); + assertTrue(clock[0] < 60_000, + "refinement must resolve well before the timeout: clock reached " + clock[0]); + assertEquals(0, paneCloseCount(herdr, "w9:pRoot_1"), + "no pane close — the peer is genuinely injectable"); + } + + @Test + void refinementNeverFiresWhenRawStatusIsAlreadyInjectable() { + // Acceptance criterion 4: refinement must trigger only on a raw UNKNOWN sample. Seed the + // pane content with an active-generation marker that StatusRefiner.classify would read as + // WORKING (never injectable) if refine() were wrongly invoked here — proving that a raw + // IDLE status short-circuits before refine() (and its extra agent.read call) ever runs. + FakeHerdr herdr = new FakeHerdr(); + herdr.agentStatus("idle"); // already injectable at the raw level + herdr.agentType("claude"); + herdr.readText("esc to interrupt"); // would classify as WORKING if refine() ran anyway + + long[] clock = {0}; + ClaudeCodeLauncher gated = new ClaudeCodeLauncher( + new AgentControl(herdr), new WorkspaceControl(herdr), + new SubscriptionGuard(Set.of("gx00.gw")), + workerConfigMap("ltms-local", null), "ltms-local", _ -> null, + 5000, () -> clock[0], () -> clock[0] += 300); + + PeerHandle handle = gated.spawn(new SpawnRequest(null, null, null)); + + assertNotNull(handle, "an already-injectable raw status succeeds without ever refining"); + assertFalse(herdr.called("agent.read"), + "refine() must never run (and so never call agent.read) when the raw status is " + + "already injectable"); + } + // --- CB-511: worker environment seeding ----------------------------------------------------- @Test diff --git a/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java index df70228..3cb9745 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java @@ -396,6 +396,46 @@ class OpenCodeLauncherTest { assertNotNull(ex.getMessage()); } + /** + * fleetd #176 fix 2's per-adapter guard: {@code StatusRefiner.classify} is written for the + * Claude Code TUI only, and the spawn-readiness gate must never run it against an opencode + * pane. {@code agentType("opencode")} deliberately satisfies the OTHER guard (the corroborating + * liveness check) so it cannot be what makes this test pass — only the {@code namePrefix} + * check can be. If that check were ever removed, this pane's {@code ❯} content would refine + * straight to IDLE and the gate would report ready before the backend actually was. + */ + @Test + void opencodePaneIsNeverRefinedEvenWhenItsContentLooksLikeAnIdleClaudePrompt(@TempDir Path root) { + FakeHerdr herdr = new FakeHerdr(); + herdr.agentStatus("unknown"); // always UNKNOWN + herdr.agentType("opencode"); // non-null — satisfies the liveness guard on its own + herdr.readText("some-host:~ user$ ❯ "); // content StatusRefiner.classify reads as IDLE + long[] clock = {0}; + OpenCodeLauncher svc = new OpenCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr), + Map.of("gemini", opencodeCfg(null, null, null)), "gemini", _ -> null, + 1000, () -> clock[0], () -> clock[0] += 50, root, root); + + assertThrows(PeerUnreachableException.class, + () -> svc.spawn(new SpawnRequest(null, null, null))); + + assertTrue(clock[0] >= 1000, + "an opencode pane must never refine to injectable, however its content reads — " + + "the gate has to wait out the full timeout: clock only reached " + clock[0]); + // A blanket "agent.read is never called" does not hold here: the timeout path itself reads + // the pane tail (source=recent) for its own log message, on every timeout, regardless of + // adapter — see HerdrPeerLauncher.readPaneQuietly. So assert on the refiner's OWN probe + // source (StatusRefiner.PROBE_SOURCE = "detection") instead — that call happens only inside + // StatusRefiner.refine, so its absence proves the refiner itself was never reached for this + // opencode pane, not merely that its answer was discarded. + long detectionReads = herdr.calls.stream() + .filter(c -> c.method().equals("agent.read")) + .filter(c -> "detection".equals(((Map) c.params()).get("source"))) + .count(); + assertEquals(0, detectionReads, + "the refiner's own pane probe (source=detection) must never run against an " + + "opencode pane — the namePrefix guard has to stop it before that call"); + } + @Test void spawnReturnsHandleWhenGateDisabled(@TempDir Path root) { FakeHerdr herdr = new FakeHerdr();