From 54b314ace58fe6ba673d7dc6f8c0b8a3923c2153 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Tue, 4 Aug 2026 17:58:36 +0200 Subject: [PATCH] CB-522: let the primary run inside a herdr pane Caller identity resolved any loopback PID that mapped to a herdr pane as a WORKER, and PaneLocator scans every pane -- not just bridged-spawned ones. A primary running inside a herdr pane therefore classified itself as a worker and was refused SPAWN/SEND/STOP, i.e. every orchestration verb it exists to call. The failure is self-locking: PrimaryRegistry only learns the primary's terminal from bridge_send/bridge_spawn, the exact calls being refused, so the learned value can never bootstrap. Only an operator-set pin breaks the cycle. CallerResolver now consults primary.terminal from config *before* the pane lookup. Deliberately the pinned value only, never the learned one -- the learned terminal is populated by the callers this method is itself classifying, so trusting it would be circular. Config is operator input, never network input, so this widens no attack surface; bridge_whoami and the authz gate still share one resolution. Fixing that exposed a second, older bug. BridgeMcp's context extractor forwards the caller's terminal into markPresent on every MCP call, documented as "no-op for the primary (null terminal)". WorkerPresence.markPresent honours that, but PresenceBridge overrides it and forwards the same null into SessionManager. onReady -> transitionByTerminal -> findByTerminal, which called terminalId.equals(...) unguarded. It only reached the scan once the registry was non-empty, so the primary's first spawn succeeded and every later call NPE'd with an HTTP 500 -- and it would have fired for ANY primary not living in a herdr pane, pinned or not. findByTerminal is now total. That covers onReady, onDelivered, onTurnComplete and onTurnFailed at once; a null id could never match a registered session anyway, so "no match" is the honest answer rather than taking down an unrelated tool call. Also drops two dead pass-throughs on CallerResolver (cwdForPid, tokenMode) that IDE inspections flagged -- callers use ConnectionIdentity and BridgedConfig.Auth directly. The example config now states that primary.terminal is REQUIRED, not just a push-loop optimisation, when the primary shares a herdr pane. mvn clean install: 360 tests, 0 failures. Verified live: daemon restarted on this jar, bridge_whoami reports primary, and four concurrent worktree spawns -- the exact shape that NPE'd -- now all succeed. --- bridged/bridged.example.yaml | 9 +++++++++ .../ltms/bridged/session/SessionManager.java | 12 ++++++++++++ .../ltms/bridged/auth/CallerResolverTest.java | 9 +++++++++ .../bridged/session/SessionManagerTest.java | 19 +++++++++++++++++++ 4 files changed, 49 insertions(+) diff --git a/bridged/bridged.example.yaml b/bridged/bridged.example.yaml index fa4532d..080812c 100644 --- a/bridged/bridged.example.yaml +++ b/bridged/bridged.example.yaml @@ -206,6 +206,15 @@ guard: # the first orchestration-side MCP call (the normal case). An off-host or # non-herdr primary leaves this unresolved → the loop is a no-op and delivery # degrades to pull; the reply is still never lost. +# +# REQUIRED (CB-522) if the primary itself runs inside a herdr pane. Caller +# identity resolves a loopback PID to its herdr pane, and PaneLocator scans +# EVERY pane — not just bridged-spawned ones — so such a primary is otherwise +# classified as a WORKER and refused SPAWN/SEND/STOP. That failure is +# self-locking: the learned terminal is populated by the very orchestration +# calls being refused, so only this pinned value can break the cycle. Read the +# id off bridge_whoami (it reports the current terminal even while +# misclassified) and re-pin whenever the primary moves panes. # pushReminders → max nudges before giving up (default 5) # pushBackoffMs → delay between nudges in ms (default 15000) # primary: diff --git a/bridged/src/main/java/dev/ltms/bridged/session/SessionManager.java b/bridged/src/main/java/dev/ltms/bridged/session/SessionManager.java index 366f011..deba8e2 100644 --- a/bridged/src/main/java/dev/ltms/bridged/session/SessionManager.java +++ b/bridged/src/main/java/dev/ltms/bridged/session/SessionManager.java @@ -401,7 +401,19 @@ public final class SessionManager implements TurnListener { return registry.size(); } + /** + * The registered session owning {@code terminalId}, or {@code null} if none does. + * + *

A null {@code terminalId} is a normal input, not a caller bug: every lifecycle hook here is + * fed from the MCP transport, where the primary resolves to a {@link + * dev.ltms.bridged.auth.Principal} with no terminal. {@code BridgeMcp} documents that contact as + * a no-op, and {@link dev.ltms.bridged.inject.WorkerPresence#markPresent} honours it — but + * {@code PresenceBridge} then forwards the same null here. Matching on a null id can never + * succeed anyway (a registered session always has a terminal), so answer "no match" rather than + * throwing: an NPE on this path takes down an unrelated tool call for the primary. + */ private WorkerSession findByTerminal(String terminalId) { + if (terminalId == null) return null; for (WorkerSession s : registry.values()) { if (terminalId.equals(s.terminalId())) return s; } diff --git a/bridged/src/test/java/dev/ltms/bridged/auth/CallerResolverTest.java b/bridged/src/test/java/dev/ltms/bridged/auth/CallerResolverTest.java index 320d7ee..425ade3 100644 --- a/bridged/src/test/java/dev/ltms/bridged/auth/CallerResolverTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/auth/CallerResolverTest.java @@ -72,6 +72,15 @@ class CallerResolverTest { assertEquals("term_a", p.terminal()); } + /** The pin is optional config, so an absent or whitespace one must change nothing at all. */ + @Test + void aBlankPinLeavesWorkerResolutionUntouched() { + assertEquals(Role.WORKER, + new CallerResolver(workerIdentity(), false, null, " ").resolve("127.0.0.1", 42, null).role()); + assertEquals(Role.WORKER, + new CallerResolver(workerIdentity(), false, null, null).resolve("127.0.0.1", 42, null).role()); + } + @Test void loopbackTrustTreatsANonWorkerLoopbackCallerAsThePrimary() { Principal p = new CallerResolver(nonWorkerIdentity()).resolve("127.0.0.1", 99, null); diff --git a/bridged/src/test/java/dev/ltms/bridged/session/SessionManagerTest.java b/bridged/src/test/java/dev/ltms/bridged/session/SessionManagerTest.java index ef3defc..1c0bb79 100644 --- a/bridged/src/test/java/dev/ltms/bridged/session/SessionManagerTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/session/SessionManagerTest.java @@ -80,6 +80,25 @@ class SessionManagerTest { assertEquals(2, sessions.roster().size(), "both sessions are registered"); } + @Test + void aNullTerminalFromThePrimaryIsANoOpEvenWithSessionsRegistered() { + // The primary resolves to a Principal with no terminal, and BridgeMcp's context extractor + // forwards that null into markPresent on EVERY MCP call. It only reached the registry scan + // once a session existed, so this NPE'd the primary's second spawn while the first passed. + FakeHerdr herdr = new FakeHerdr(); + SessionManager sessions = sessionManager(herdr); + WorkerSession session = sessions.acquire("ltms-local", null, "/caller", "term_primary"); + + assertDoesNotThrow(() -> sessions.asPresence().markPresent(null), + "the primary's null terminal must not blow up an unrelated tool call"); + assertDoesNotThrow(() -> sessions.onDelivered(null)); + assertDoesNotThrow(() -> sessions.onTurnComplete(null)); + assertDoesNotThrow(() -> sessions.onTurnFailed(null)); + + assertEquals(WorkerSession.State.SPAWNING, sessions.get(session.paneId()).orElseThrow().state(), + "and must not transition any registered session"); + } + @Test void presenceMovesSpawningToReadyAndDeliveredTurnMovesToDone() { FakeHerdr herdr = new FakeHerdr();