From 36870836aa5caf9d6892dd24d67f8230d62c05fe Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 12 Sep 2026 10:27:42 +0700 Subject: [PATCH] fleetd #505: a herdr error during the pane scan must not read as a clean negative MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A transient herdr error on pane.process_info during PaneLocator's pid→pane scan used to be swallowed into a plain "does not own it", so a real worker whose owning pane errored mid-scan resolved with a null terminal but a resolved (real) pid — exactly what CallerResolver's loopback-trust fallback reads as the primary. That is a worker→primary privilege escalation through the door fleetd #317 did not close: #317 guards a failed lsof lookup (c.resolved()), not a failed herdr pane scan. Fix: add a third state to the scan instead of widening Caller.resolved() (which stays centralised next to the lsof sentinel it tests, per #505's explicit instruction not to reopen that decision). PaneLocator.terminalForPid now returns a Lookup(terminal, complete) record: a HerdrException on one pane marks that pane's ownership UNKNOWN, not DOES_NOT_OWN, and the scan is complete only if every pane was either matched or confirmed not to own the pid. A definite match found elsewhere in the same scan still short-circuits as complete — a pane that genuinely vanished mid-scan without being the caller's own does not turn into a refusal. ConnectionIdentity.Caller carries the new scanComplete flag alongside the unchanged resolved(). CallerResolver's loopback-trust fallback now requires both resolved() and scanComplete() before promoting to Principal.primary(); an incomplete scan resolves anonymous, which fails toward the recoverable error (a refused primary retries loudly; a promoted worker would not). Logs a warning naming the pane and which herdr client (of how many) failed, so the incomplete-scan path is diagnosable rather than silent (fleetd #317's own lesson). --- .../dev/ltms/fleet/auth/CallerResolver.java | 10 ++- .../dev/ltms/fleet/herdr/PaneLocator.java | 86 +++++++++++++++---- .../ltms/fleet/mcp/ConnectionIdentity.java | 14 ++- .../ltms/fleet/auth/CallerResolverTest.java | 37 ++++++++ .../java/dev/ltms/fleet/herdr/FakeHerdr.java | 20 +++++ .../fleet/herdr/PaneLocatorContractTest.java | 2 +- .../dev/ltms/fleet/herdr/PaneLocatorTest.java | 62 ++++++++++--- .../fleet/mcp/ConnectionIdentityTest.java | 17 ++++ 8 files changed, 209 insertions(+), 39 deletions(-) diff --git a/fleetd/src/main/java/dev/ltms/fleet/auth/CallerResolver.java b/fleetd/src/main/java/dev/ltms/fleet/auth/CallerResolver.java index a358be9..8658cc4 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/auth/CallerResolver.java +++ b/fleetd/src/main/java/dev/ltms/fleet/auth/CallerResolver.java @@ -244,7 +244,15 @@ public final class CallerResolver { // already names what happens if that case is handed the primary role: a worker→primary // escalation. So an unresolved caller is refused (ANONYMOUS — the same clean, already-tested // "authenticated as nothing" outcome used everywhere else in this method), never promoted. - return isLoopback(remoteAddr) && c.resolved() ? Principal.primary(c.pid()) : Principal.anonymous(); + // + // fleetd #505: the OTHER way a real pid can wrongly reach here with a null terminal — not a + // failed lsof lookup, but a herdr error partway through PaneLocator's pane scan. c.resolved() + // says nothing about that; it only tests the lsof sentinel (by design — see + // ConnectionIdentity.Caller#resolved). c.scanComplete() is the separate signal: a scan that + // could not check every pane must not be read as "checked everywhere, no match" — the pane it + // could not check might have been the caller's own. So both must hold before this promotes. + return isLoopback(remoteAddr) && c.resolved() && c.scanComplete() + ? Principal.primary(c.pid()) : Principal.anonymous(); } private boolean presentedTokenMatches(String authorizationHeader) { diff --git a/fleetd/src/main/java/dev/ltms/fleet/herdr/PaneLocator.java b/fleetd/src/main/java/dev/ltms/fleet/herdr/PaneLocator.java index 7c5c89f..ca9fcb7 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/herdr/PaneLocator.java +++ b/fleetd/src/main/java/dev/ltms/fleet/herdr/PaneLocator.java @@ -1,6 +1,8 @@ package dev.ltms.fleet.herdr; import com.fasterxml.jackson.databind.JsonNode; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.LinkedHashSet; import java.util.List; @@ -36,6 +38,8 @@ import java.util.Set; */ public final class PaneLocator { + private static final Logger log = LoggerFactory.getLogger(PaneLocator.class); + /** * Bound on how many ancestor generations {@link #ancestorsOf} walks. This runs on every MCP * call, so a cycle or a pathologically deep process tree must not hang identity resolution; @@ -73,22 +77,46 @@ public final class PaneLocator { } /** - * The {@code terminal_id} of the agent pane whose process tree contains {@code pid}, or - * {@code null} if no agent pane on any searched daemon owns it (e.g. the caller is the - * primary, or off-host). + * The outcome of a {@link #terminalForPid} scan: the {@code terminal_id} of the agent pane + * whose process tree contains the pid ({@link #terminal} is {@code null} if none matched), + * and whether the scan that produced that answer ran to completion on every daemon searched. + * + *

{@link #complete} is {@code false} exactly when some {@code pane.process_info} call + * failed and, despite that, no pane was ever found to own the pid. In that case a {@code null} + * {@link #terminal} means "could not tell", not "definitely not a worker" — fleetd #505: a + * transient herdr error on the very pane that does own the caller's pid must not read + * as a clean negative and fall through to {@code Principal.primary}, the same way #317's + * {@code Caller.resolved()} already guards a failed lsof lookup. Callers ({@code + * ConnectionIdentity}, {@code CallerResolver}) must refuse rather than promote on an incomplete + * scan. + * + *

When a pane genuinely owns the pid, {@link #complete} is {@code true} regardless of + * whether some other, unrelated pane failed to answer earlier in the same scan — a positive + * match is definitive and does not need every pane to have been checked (a pane that "vanished + * mid-scan" but was never the match is still a clean, complete result). */ - public String terminalForPid(long pid) { + public record Lookup(String terminal, boolean complete) { + private static final Lookup NOT_FOUND = new Lookup(null, true); + } + + /** + * Resolve {@code pid} to the agent pane whose process tree contains it, across every searched + * herdr daemon. See {@link Lookup} for how to read a {@code null} terminal. + */ + public Lookup terminalForPid(long pid) { if (pid <= 0) { - return null; + return Lookup.NOT_FOUND; } Set ancestry = ancestorsOf(pid); - for (HerdrClient herdr : herdrs) { - String terminal = terminalForPid(herdr, ancestry); - if (terminal != null) { - return terminal; + boolean complete = true; + for (int i = 0; i < herdrs.size(); i++) { + Lookup outcome = scan(herdrs.get(i), i, herdrs.size(), ancestry); + if (outcome.terminal() != null) { + return outcome; // a definite match — no need to finish checking other clients } + complete = complete && outcome.complete(); } - return null; + return new Lookup(null, complete); } /** @@ -117,31 +145,51 @@ public final class PaneLocator { return ancestry; } - private static String terminalForPid(HerdrClient herdr, Set ancestry) { + /** Whether a pane owns one of the scanned pid's ancestors, or the check of it failed outright. */ + private enum Ownership { OWNS, DOES_NOT_OWN, UNKNOWN } + + private static Lookup scan(HerdrClient herdr, int clientIndex, int clientCount, Set ancestry) { + boolean complete = true; for (JsonNode pane : herdr.call("pane.list", Map.of()).path("panes")) { String paneId = pane.path("pane_id").asText(null); - if (paneId != null && paneOwnsAnyOf(herdr, paneId, ancestry)) { - return pane.path("terminal_id").asText(null); + if (paneId == null) { + continue; + } + Ownership owns = paneOwnsAnyOf(herdr, clientIndex, clientCount, paneId, ancestry); + if (owns == Ownership.OWNS) { + return new Lookup(pane.path("terminal_id").asText(null), true); + } + if (owns == Ownership.UNKNOWN) { + complete = false; } } - return null; + return new Lookup(null, complete); } - private static boolean paneOwnsAnyOf(HerdrClient herdr, String paneId, Set ancestry) { + private static Ownership paneOwnsAnyOf(HerdrClient herdr, int clientIndex, int clientCount, + String paneId, Set ancestry) { JsonNode info; try { info = herdr.call("pane.process_info", Map.of("pane_id", paneId)).path("process_info"); } catch (HerdrException e) { - return false; // pane vanished mid-scan — just skip it + // fleetd #505: this used to be read as a clean "does not own it" (the pane vanished + // mid-scan, just skip it) — one boolean carrying two different facts. It is UNKNOWN + // now: if THIS pane is the one that owns the pid, the caller must not be told "no pane + // owns it", because that reads as a real primary and is promoted under loopback-trust. + log.warn("pane.process_info failed for pane {} on herdr client {} of {} during a " + + "pid-owner scan — treating it as \"could not tell\", not a clean " + + "negative (fleetd #505): {}", + paneId, clientIndex + 1, clientCount, e.getMessage()); + return Ownership.UNKNOWN; } if (ancestry.contains(info.path("shell_pid").asLong(-1))) { - return true; + return Ownership.OWNS; } for (JsonNode p : info.path("foreground_processes")) { if (ancestry.contains(p.path("pid").asLong(-1))) { - return true; + return Ownership.OWNS; } } - return false; + return Ownership.DOES_NOT_OWN; } } diff --git a/fleetd/src/main/java/dev/ltms/fleet/mcp/ConnectionIdentity.java b/fleetd/src/main/java/dev/ltms/fleet/mcp/ConnectionIdentity.java index 3813e12..01a88a6 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/mcp/ConnectionIdentity.java +++ b/fleetd/src/main/java/dev/ltms/fleet/mcp/ConnectionIdentity.java @@ -33,9 +33,10 @@ public final class ConnectionIdentity { /** * The caller resolved from the connection: its worker {@code terminal} (or {@code null} for the - * primary / an off-host client) and its {@code pid} (or {@code -1} if not resolvable). + * primary / an off-host client), its {@code pid} (or {@code -1} if not resolvable), and whether + * the pane scan behind {@code terminal} ran to completion ({@link #scanComplete}). */ - public record Caller(String terminal, long pid) { + public record Caller(String terminal, long pid, boolean scanComplete) { /** * Whether the OS peer-PID lookup actually succeeded — {@code false} means {@code pid} is @@ -51,6 +52,10 @@ public final class ConnectionIdentity { * {@link ConnectionIdentity#isLoopback} is centralised rather than left for each caller to * reimplement: a raw {@code pid > 0} check duplicated at every call site is precisely the * "one rule, two copies" shape that let #305 drift. + * + *

This method is deliberately NOT widened for fleetd #505's failure (a herdr error + * during the pane scan, not a failed lsof lookup) — it still tests only the sentinel it is + * named for. #505 is a different axis, carried separately in {@link #scanComplete}. */ public boolean resolved() { return pid > 0; @@ -60,10 +65,11 @@ public final class ConnectionIdentity { /** Resolve the caller's terminal and PID from one peer-PID lookup. */ public Caller resolve(String remoteAddr, int remotePort) { if (!isLoopback(remoteAddr)) { - return new Caller(null, -1); // only same-host callers can be workers + return new Caller(null, -1, true); // only same-host callers can be workers } long pid = pids.pidForLocalPort(remotePort); - return new Caller(panes.terminalForPid(pid), pid); + PaneLocator.Lookup lookup = panes.terminalForPid(pid); + return new Caller(lookup.terminal(), pid, lookup.complete()); } /** diff --git a/fleetd/src/test/java/dev/ltms/fleet/auth/CallerResolverTest.java b/fleetd/src/test/java/dev/ltms/fleet/auth/CallerResolverTest.java index e8d7016..5d574fe 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/auth/CallerResolverTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/auth/CallerResolverTest.java @@ -186,6 +186,43 @@ class CallerResolverTest { assertEquals(Role.PRIMARY, r.resolve("127.0.0.1", 99, "BEARER s3cret").role()); } + // ── fleetd #505: a herdr error DURING THE SCAN must not be conflated with "not a worker" ────── + // #317 (above) covers a failed lsof lookup. This is the other input to the same decision: the + // lsof lookup succeeds (a real pid), but PaneLocator's own pane scan hits a herdr error on the + // pane that owns that pid — so c.resolved() is true and c.terminal() is null, exactly like a + // real primary. c.scanComplete() is what tells them apart. + + /** + * The discriminating case named in the ticket: the error must land on the pane that DOES own + * the caller's pid, or the test proves nothing (any other pane's failure is invisible to the + * scan's outcome, since a match found elsewhere is definitive regardless). + */ + @Test + void aHerdrErrorOnTheOwningPaneDuringTheScanIsRefusedNotPromotedToPrimary() { + FakeHerdr failing = new FakeHerdr().processInfoFailsForPane("w2:p7", "transient"); + ConnectionIdentity incomplete = new ConnectionIdentity(new PaneLocator(failing), _ -> FakeHerdr.WORKER_PID); + + Principal p = new CallerResolver(incomplete).resolve("127.0.0.1", 55555, null); + + assertEquals(Role.ANONYMOUS, p.role(), + "an incomplete pane scan must never be read as a clean negative and promoted to primary"); + } + + /** + * The companion invariant: a herdr error on a DIFFERENT, non-owning pane must not turn every + * mid-scan teardown into a refusal — the real match is still found and resolves as a worker. + */ + @Test + void aHerdrErrorOnANonOwningPaneStillResolvesTheRealWorker() { + FakeHerdr vanishedElsewhere = new FakeHerdr().processInfoFailsForPane("w2:p9", "pane_not_found"); + ConnectionIdentity id = new ConnectionIdentity(new PaneLocator(vanishedElsewhere), _ -> FakeHerdr.WORKER_PID); + + Principal p = new CallerResolver(id).resolve("127.0.0.1", 55555, null); + + assertEquals(Role.WORKER, p.role()); + assertEquals("term_a", p.terminal()); + } + @Test void aNonLoopbackCallerIsNeverThePrimaryUnderLoopbackTrust() { // Defence in depth: startup already refuses this pairing (validateAuthExposure), but if a 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 3ba4441..5d3fbc4 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/herdr/FakeHerdr.java +++ b/fleetd/src/test/java/dev/ltms/fleet/herdr/FakeHerdr.java @@ -44,6 +44,7 @@ public final class FakeHerdr implements HerdrClient { private int workerTabPaneCount = 1; private String paneCloseErrorCode = null; private final Map paneCloseErrorCodeFor = new ConcurrentHashMap<>(); + private final Map processInfoErrorCodeFor = new ConcurrentHashMap<>(); private String tabCloseErrorCode = null; private final Map tabCloseErrorCodeFor = new ConcurrentHashMap<>(); private String agentSendErrorCode = null; @@ -144,6 +145,18 @@ public final class FakeHerdr implements HerdrClient { return this; } + /** + * Make {@code pane.process_info} fail with this herdr error code, but only for the given + * {@code pane_id} — every other pane's {@code pane.process_info} still succeeds. Models a + * transient herdr failure partway through a {@link PaneLocator} pid→pane scan (fleetd #505): + * the scan must be able to tell "this pane does not own the pid" apart from "the scan could + * not check this pane at all", instead of collapsing both into one {@code false}. + */ + public FakeHerdr processInfoFailsForPane(String paneId, String code) { + this.processInfoErrorCodeFor.put(paneId, code); + return this; + } + /** Set the {@code agent_status} that {@code agent.get} reports (drives the injector). */ public FakeHerdr agentStatus(String status) { this.agentStatus = status; @@ -407,6 +420,13 @@ public final class FakeHerdr implements HerdrClient { {"pane_id":"w2:p9","terminal_id":"term_shell","workspace_id":"w2","tab_id":"w2:t8"}]}"""); case "pane.process_info" -> { Object paneId = params instanceof java.util.Map m ? m.get("pane_id") : null; + String failCode = paneId == null ? null + : processInfoErrorCodeFor.get(String.valueOf(paneId)); + if (failCode != null) { + throw new HerdrException( + "herdr error [" + failCode + "]: pane.process_info failed", + failCode, null); + } yield "w2:p7".equals(paneId) ? mapper.readTree((""" {"type":"pane_process_info","process_info":{"pane_id":"w2:p7","shell_pid":%d, diff --git a/fleetd/src/test/java/dev/ltms/fleet/herdr/PaneLocatorContractTest.java b/fleetd/src/test/java/dev/ltms/fleet/herdr/PaneLocatorContractTest.java index 0a8ea4d..964a2b8 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/herdr/PaneLocatorContractTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/herdr/PaneLocatorContractTest.java @@ -37,7 +37,7 @@ class PaneLocatorContractTest { .path("pane").path("terminal_id").asText(null); assertNotNull(terminalId, "seed pane should carry a terminal_id"); - assertEquals(terminalId, new PaneLocator(herdr).terminalForPid(shellPid), + assertEquals(terminalId, new PaneLocator(herdr).terminalForPid(shellPid).terminal(), "a real PID must resolve back to its own pane's terminal_id"); } finally { spaces.closeTab(tab.tab().tabId()); diff --git a/fleetd/src/test/java/dev/ltms/fleet/herdr/PaneLocatorTest.java b/fleetd/src/test/java/dev/ltms/fleet/herdr/PaneLocatorTest.java index 7208f05..9818fae 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/herdr/PaneLocatorTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/herdr/PaneLocatorTest.java @@ -15,18 +15,20 @@ class PaneLocatorTest { @Test void resolvesTerminalForAForegroundPid() { - assertEquals("term_a", loc.terminalForPid(FakeHerdr.WORKER_PID)); + assertEquals("term_a", loc.terminalForPid(FakeHerdr.WORKER_PID).terminal()); } @Test void nullForAPidInNoPane() { - assertNull(loc.terminalForPid(999_999)); + PaneLocator.Lookup outcome = loc.terminalForPid(999_999); + assertNull(outcome.terminal()); + assertTrue(outcome.complete(), "a full, error-free scan that finds no match is complete"); } @Test void nullForNonPositivePid() { - assertNull(loc.terminalForPid(0)); - assertNull(loc.terminalForPid(-1)); + assertNull(loc.terminalForPid(0).terminal()); + assertNull(loc.terminalForPid(-1).terminal()); } // --- two-daemon fallback (CB-185) ----------------------------------------- @@ -38,7 +40,7 @@ class PaneLocatorTest { HerdrClient lead = new FakeHerdr().withNoPanes(); HerdrClient member = new FakeHerdr(); PaneLocator two = new PaneLocator(lead, member); - assertEquals("term_a", two.terminalForPid(FakeHerdr.WORKER_PID)); + assertEquals("term_a", two.terminalForPid(FakeHerdr.WORKER_PID).terminal()); } @Test @@ -48,13 +50,13 @@ class PaneLocatorTest { HerdrClient lead = new FakeHerdr(); HerdrClient member = new FakeHerdr().withNoPanes(); PaneLocator two = new PaneLocator(lead, member); - assertEquals("term_a", two.terminalForPid(FakeHerdr.WORKER_PID)); + assertEquals("term_a", two.terminalForPid(FakeHerdr.WORKER_PID).terminal()); } @Test void nullWhenNeitherClientHasTheMatch() { PaneLocator two = new PaneLocator(new FakeHerdr().withNoPanes(), new FakeHerdr().withNoPanes()); - assertNull(two.terminalForPid(FakeHerdr.WORKER_PID)); + assertNull(two.terminalForPid(FakeHerdr.WORKER_PID).terminal()); } @Test @@ -63,7 +65,7 @@ class PaneLocatorTest { // must behave exactly like the one-arg constructor, including making only one herdr call. FakeHerdr shared = new FakeHerdr(); PaneLocator two = new PaneLocator(shared, shared); - assertEquals("term_a", two.terminalForPid(FakeHerdr.WORKER_PID)); + assertEquals("term_a", two.terminalForPid(FakeHerdr.WORKER_PID).terminal()); long paneListCalls = shared.calls.stream().filter(c -> c.method().equals("pane.list")).count(); assertEquals(1, paneListCalls, "same-object lead/member must scan exactly once, not twice"); } @@ -75,7 +77,7 @@ class PaneLocatorTest { // Regression: a pid with no parent chain at all — no ancestry walk is needed to match it. OnePaneHerdr pane = new OnePaneHerdr("term_x", "pX", 5000, 6000); PaneLocator loc = new PaneLocator(pane, new FakeParentResolver()); - assertEquals("term_x", loc.terminalForPid(5000)); + assertEquals("term_x", loc.terminalForPid(5000).terminal()); } @Test @@ -83,7 +85,7 @@ class PaneLocatorTest { // Regression: same as above, but matching via the foreground-processes list. OnePaneHerdr pane = new OnePaneHerdr("term_x", "pX", 5000, 6000); PaneLocator loc = new PaneLocator(pane, new FakeParentResolver()); - assertEquals("term_x", loc.terminalForPid(6000)); + assertEquals("term_x", loc.terminalForPid(6000).terminal()); } @Test @@ -97,7 +99,7 @@ class PaneLocatorTest { .parent(7002, 7001) // grandchild -> child .parent(7001, 5000); // child -> shell (the pane's shell_pid) PaneLocator loc = new PaneLocator(pane, parents); - assertEquals("term_x", loc.terminalForPid(7002)); + assertEquals("term_x", loc.terminalForPid(7002).terminal()); } @Test @@ -110,7 +112,7 @@ class PaneLocatorTest { .parent(9002, 9001) .parent(9001, 9000); // chain never reaches 5000 or 6000 PaneLocator loc = new PaneLocator(pane, parents); - assertNull(loc.terminalForPid(9002)); + assertNull(loc.terminalForPid(9002).terminal()); } @Test @@ -122,7 +124,7 @@ class PaneLocatorTest { .parent(100, 101) .parent(101, 100); // cycle, never reaches the pane's pids PaneLocator loc = new PaneLocator(pane, parents); - assertNull(loc.terminalForPid(100)); + assertNull(loc.terminalForPid(100).terminal()); } @Test @@ -141,11 +143,43 @@ class PaneLocatorTest { }; HerdrClient noPanes = new FakeHerdr().withNoPanes(); PaneLocator two = new PaneLocator(noPanes, pane, counting); - assertEquals("term_x", two.terminalForPid(7002)); + assertEquals("term_x", two.terminalForPid(7002).terminal()); assertEquals(3, calls.get(), "ancestry must be walked once (3 lookups: 7002, 7001, 5000), " + "not re-walked per herdr client"); } + // --- fleetd #505: a herdr error during the scan must not read as a clean negative --------- + + @Test + void anErrorOnThePaneThatOwnsThePidMakesTheScanIncompleteNotAClearNegative() { + // The discriminating case: pane.process_info fails for exactly the pane that DOES own the + // caller's pid ("w2:p7", term_a). Before the fix, that failure was swallowed into a plain + // "does not own it" and the scan finished with a clean-looking null — indistinguishable + // from a real primary. It must now report incomplete, not a definite null. + FakeHerdr herdr = new FakeHerdr().processInfoFailsForPane("w2:p7", "transient"); + PaneLocator loc = new PaneLocator(herdr); + + PaneLocator.Lookup outcome = loc.terminalForPid(FakeHerdr.WORKER_PID); + + assertNull(outcome.terminal(), "the failing pane's ownership could not be confirmed"); + assertFalse(outcome.complete(), + "a scan that could not check the owning pane must not report as complete"); + } + + @Test + void aVanishedPaneThatIsNotTheMatchLeavesAnOtherwiseSuccessfulScanComplete() { + // The companion invariant: a DIFFERENT pane (not the caller's own) failing mid-scan must + // not turn every mid-scan teardown into a refusal — the real match is still found, and the + // scan is still reported complete. + FakeHerdr herdr = new FakeHerdr().processInfoFailsForPane("w2:p9", "pane_not_found"); + PaneLocator loc = new PaneLocator(herdr); + + PaneLocator.Lookup outcome = loc.terminalForPid(FakeHerdr.WORKER_PID); + + assertEquals("term_a", outcome.terminal()); + assertTrue(outcome.complete(), "a positive match elsewhere in the scan is definitive"); + } + /** Minimal single-pane {@link HerdrClient} fake, purpose-built for the ancestry tests above. */ private static final class OnePaneHerdr implements HerdrClient { private final ObjectMapper mapper = new ObjectMapper(); diff --git a/fleetd/src/test/java/dev/ltms/fleet/mcp/ConnectionIdentityTest.java b/fleetd/src/test/java/dev/ltms/fleet/mcp/ConnectionIdentityTest.java index af4139c..32e716c 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/mcp/ConnectionIdentityTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/mcp/ConnectionIdentityTest.java @@ -57,6 +57,23 @@ class ConnectionIdentityTest { // must read as "resolved" — the distinction #317 turns on. ConnectionIdentity.Caller c = with(_ -> 999_999).resolve("127.0.0.1", 55555); assertTrue(c.resolved()); + assertTrue(c.scanComplete(), "no herdr error happened, so the scan is complete"); + } + + @Test + void scanIsIncompleteWhenHerdrErrorsOnThePaneThatOwnsThePid() { + // fleetd #505: a transient herdr error on exactly the pane that DOES own the caller's pid + // must be visible as an incomplete scan, distinct from a real primary (resolved(), null + // terminal, complete scan). Both have pid > 0 and a null terminal — scanComplete is the + // only thing that tells them apart. + FakeHerdr failing = new FakeHerdr().processInfoFailsForPane("w2:p7", "transient"); + ConnectionIdentity id = new ConnectionIdentity(new PaneLocator(failing), _ -> FakeHerdr.WORKER_PID); + + ConnectionIdentity.Caller c = id.resolve("127.0.0.1", 55555); + + assertTrue(c.resolved(), "the pid itself resolved fine — this is not #317's failure"); + assertNull(c.terminal(), "the owning pane could not be confirmed"); + assertFalse(c.scanComplete(), "the scan could not check the pane that owns this pid"); } @Test