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.
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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.
|
||||
*
|
||||
* <p>A null {@code terminalId} is a normal input, not a caller bug: every lifecycle hook here is
|
||||
* fed from the MCP transport, where the <em>primary</em> 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;
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in New Issue
Block a user