diff --git a/bridged/src/main/java/dev/ltms/bridged/herdr/AgentControl.java b/bridged/src/main/java/dev/ltms/bridged/herdr/AgentControl.java index 6251145..e8ab4d5 100644 --- a/bridged/src/main/java/dev/ltms/bridged/herdr/AgentControl.java +++ b/bridged/src/main/java/dev/ltms/bridged/herdr/AgentControl.java @@ -6,93 +6,120 @@ import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; /** * Domain layer over herdr's native {@code agent.*} namespace — the worker south side. - * Chosen in the CB-102 spike over the pane + {@code send_text} fallback because - * {@code agent.start} takes a first-class {@code env} map (clean, guard-checked - * subscription injection) and herdr tracks each worker's Claude session UUID itself. + * Chosen in the CB-102 spike over the pane + {@code send_text} fallback because herdr + * tracks each worker's Claude session UUID itself. + * + *

Ported to herdr protocol 19 (herdr 0.8.0, CB-521): {@code agent.start} now starts a + * supported agent ({@code kind}) into an existing pane, so the worker's + * {@code env}/{@code cwd} move to pane creation ({@code tab.create}/{@code pane.split} — see + * {@link WorkspaceControl}), and {@code agent.send} is replaced by {@code agent.prompt} + * (which submits in one call) plus {@code agent.send_keys} for the raw Enter nudge. * *

Every method is one herdr call through the injected {@link HerdrClient}, so this * layer is unit-testable with a fake and contract-tested against a live daemon. */ public final class AgentControl { - /** - * The keystroke that submits a prompt in the Claude Code TUI: a carriage return (Enter). - * It must be delivered as its own {@code agent.send} call — herdr delivers a message's - * text as a bracketed paste, and a {@code "\r"} appended to that same text is swallowed as - * literal newline content, not a submit. Sent as a separate keystroke event it lands outside - * the paste and submits. (A bare {@code "\n"} inserts a newline either way.) Verified live - * against Claude Code v2.1.210: an injected task stayed unsubmitted with {@code "text\r"} in - * one call, and submitted the instant a standalone {@code "\r"} was sent. - */ - static final String SUBMIT_KEY = "\r"; - private final HerdrClient herdr; + /** + * Protocol 19 dropped {@code terminal_id} as an {@code agent.*} target — herdr now resolves + * targets by pane id or agent name only, while the bridge keys every session on the terminal. + * This caches the terminal→pane mapping (stable for a worker's lifetime) so callers keep + * addressing agents by terminal; entries are invalidated on {@code agent_not_found}. + */ + private final Map paneByTerminal = new ConcurrentHashMap<>(); + public AgentControl(HerdrClient herdr) { this.herdr = herdr; } + /** One agent-targeted call, translating a terminal id to its pane id (retrying once fresh). */ + private JsonNode agentCall(String method, String target, Map extra) { + String resolved = resolveTarget(target); + try { + return herdr.call(method, withTarget(resolved, extra)); + } catch (HerdrException e) { + if (!"agent_not_found".equals(e.code()) || resolved.equals(target)) throw e; + paneByTerminal.remove(target); // the cached pane went away — re-resolve once + String fresh = resolveTarget(target); + if (fresh.equals(resolved)) throw e; + return herdr.call(method, withTarget(fresh, extra)); + } + } + + private static Map withTarget(String target, Map extra) { + Map m = new LinkedHashMap<>(); + m.put("target", target); + m.putAll(extra); + return m; + } + + /** The pane id behind a terminal-id target, or the target verbatim for pane ids / names. */ + private String resolveTarget(String target) { + if (target == null || !target.startsWith("term_")) { + return target; + } + String cached = paneByTerminal.get(target); + if (cached != null) { + return cached; + } + for (JsonNode a : herdr.call("agent.list").path("agents")) { + if (target.equals(a.path("terminal_id").asText(null))) { + String pane = a.path("pane_id").asText(null); + if (pane != null) { + paneByTerminal.put(target, pane); + return pane; + } + } + } + return target; // unknown terminal — let herdr report it against the original target + } + /** - * Spawn an agent. {@code env} is applied to the process environment verbatim — this - * is where a worker's {@code ANTHROPIC_BASE_URL} lives, and the ONLY place it should. + * Start an agent into {@code paneId}, which must be sitting at its interactive shell prompt — + * the seed pane of a freshly-created worker tab, or a fresh split. The pane's shell already + * carries the worker's env ({@code ANTHROPIC_BASE_URL}, token, …) and cwd from pane creation; + * herdr resolves the executable from {@code kind} and waits (its default timeout) until the + * agent is detected and ready for input. * - * @param name label/kind for herdr status detection (e.g. {@code "claude"}) - * @param argv launch command, e.g. {@code ["claude"]} - * @param env process environment additions ({@code ANTHROPIC_BASE_URL}, token, …) + * @param name unique label for this agent ({@code ---}) + * @param kind supported agent kind and canonical executable, e.g. {@code "claude"}, + * {@code "opencode"} + * @param args extra arguments after the executable, e.g. {@code --mcp-config …} + * @param paneId the pane to start the agent in */ - public Agent start(String name, List argv, Map env) { - return start(name, argv, env, null); - } - - /** Spawn an agent into {@code tabId} at herdr's default cwd. */ - public Agent start(String name, List argv, Map env, String tabId) { - return start(name, argv, env, tabId, null); - } - - /** - * Spawn an agent. With a non-null {@code tabId} the worker lands in that tab (the placement - * policy's dedicated worker tab); with {@code null} herdr splits the currently-focused tab - * (legacy pane placement). A non-blank {@code cwd} sets the worker process's working directory — - * {@code agent.start} honours {@code cwd} directly (an agent pane does not inherit the - * tab's or workspace's cwd, so this is the only way to root a worker in the primary's directory; - * CB-112). - */ - public Agent start(String name, List argv, Map env, String tabId, String cwd) { + public Agent start(String name, String kind, List args, String paneId) { Map params = new LinkedHashMap<>(); params.put("name", name); - params.put("argv", argv); - params.put("env", env); - if (tabId != null) { - params.put("tab_id", tabId); - } - if (cwd != null && !cwd.isBlank()) { - params.put("cwd", cwd); - } + params.put("kind", kind); + params.put("pane_id", paneId); + params.put("args", args); JsonNode result = herdr.call("agent.start", params); return Agent.from(result.get("agent")); } /** - * Deliver {@code text} to an agent as its next prompt and submit it — two keystroke - * events: the message (a bracketed paste, so any embedded newlines are preserved verbatim), - * then a standalone {@link #SUBMIT_KEY} (Enter) that actually submits it. Without the second - * event the text just sits in the worker's input box, never processed (see {@link #SUBMIT_KEY}). + * Deliver {@code text} to an agent as its next prompt and submit it — herdr's + * {@code agent.prompt} pastes the text (embedded newlines preserved verbatim) and submits it + * in the same call, replacing the pre-protocol-19 two-event {@code agent.send} dance. */ public void send(String target, String text) { - herdr.call("agent.send", Map.of("target", target, "text", text)); - herdr.call("agent.send", Map.of("target", target, "text", SUBMIT_KEY)); + agentCall("agent.prompt", target, Map.of("text", text)); } /** - * Re-send the submit keystroke (Enter) to {@code target}. The Enter that accompanies a delivery - * can race the paste — especially right as the worker's TUI becomes interactive — leaving the - * text unsubmitted; the injector nudges it with this until the worker actually picks up (CB-113). + * Re-send the submit keystroke (Enter) to {@code target}. The submit that accompanies a + * delivery can race the paste — especially right as the worker's TUI becomes interactive — + * leaving the text unsubmitted; the injector nudges it with this until the worker actually + * picks up (CB-113). */ public void submit(String target) { - herdr.call("agent.send", Map.of("target", target, "text", SUBMIT_KEY)); + agentCall("agent.send_keys", target, Map.of("keys", List.of("enter"))); } /** @@ -101,13 +128,13 @@ public final class AgentControl { * @param source one of {@code visible|recent|recent_unwrapped|detection} */ public String read(String target, String source) { - JsonNode result = herdr.call("agent.read", Map.of("target", target, "source", source)); + JsonNode result = agentCall("agent.read", target, Map.of("source", source)); return result.path("read").path("text").asText(""); } /** Current agent record (status, session UUID, pane). */ public Agent get(String target) { - return Agent.from(herdr.call("agent.get", Map.of("target", target)).get("agent")); + return Agent.from(agentCall("agent.get", target, Map.of()).get("agent")); } /** Just the lifecycle status — what the status-gated injector checks before send. */ diff --git a/bridged/src/main/java/dev/ltms/bridged/herdr/Tab.java b/bridged/src/main/java/dev/ltms/bridged/herdr/Tab.java index bfc7124..01e35cb 100644 --- a/bridged/src/main/java/dev/ltms/bridged/herdr/Tab.java +++ b/bridged/src/main/java/dev/ltms/bridged/herdr/Tab.java @@ -23,9 +23,9 @@ public record Tab(String tabId, String workspaceId, String label, int paneCount) } /** - * A freshly-created tab together with the placeholder shell pane herdr seeds it with. - * The caller starts the worker into {@link #tab()} then closes {@link #rootPaneId()} so - * only the worker pane remains. + * A freshly-created tab together with the shell pane herdr seeds it with. Under protocol 19 + * the caller starts the worker into {@link #rootPaneId()} — the seed pane's shell + * carries the worker's cwd and env from {@code tab.create}, and becomes the worker pane. */ public record Created(Tab tab, String rootPaneId) { /** Project a {@code tab_created} result ({@code {tab, root_pane}}). */ diff --git a/bridged/src/main/java/dev/ltms/bridged/herdr/WorkspaceControl.java b/bridged/src/main/java/dev/ltms/bridged/herdr/WorkspaceControl.java index 99b2a40..69ed9a5 100644 --- a/bridged/src/main/java/dev/ltms/bridged/herdr/WorkspaceControl.java +++ b/bridged/src/main/java/dev/ltms/bridged/herdr/WorkspaceControl.java @@ -5,6 +5,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.util.ArrayList; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -65,13 +66,37 @@ public final class WorkspaceControl { } /** - * A brand-new tab in {@code workspaceId} plus the placeholder shell pane herdr seeds it with. - * Start the worker into the tab, then {@code pane.close} the root pane so the tab holds only the - * worker. (The worker's own cwd is set on {@code agent.start}, not here — an {@code agent.start} - * pane does not inherit the tab's cwd; see {@code AgentControl.start}.) + * A brand-new tab in {@code workspaceId} plus the shell pane herdr seeds it with. Under + * protocol 19 that seed pane is where the worker starts: its shell carries + * {@code cwd} and {@code env} (the worker's {@code ANTHROPIC_BASE_URL} — this is the + * subscription-injection seam now), and {@code agent.start} launches the agent into it. */ - public Tab.Created createTab(String workspaceId) { - return Tab.Created.from(herdr.call("tab.create", Map.of("workspace_id", workspaceId))); + public Tab.Created createTab(String workspaceId, String cwd, Map env) { + Map params = new LinkedHashMap<>(); + params.put("workspace_id", workspaceId); + if (cwd != null && !cwd.isBlank()) { + params.put("cwd", cwd); + } + if (env != null && !env.isEmpty()) { + params.put("env", env); + } + return Tab.Created.from(herdr.call("tab.create", params)); + } + + /** + * Split the currently-focused tab and return the new pane's id — the legacy pane placement's + * seed pane, carrying {@code cwd} and {@code env} exactly as {@link #createTab}'s does. + */ + public String splitPane(String cwd, Map env) { + Map params = new LinkedHashMap<>(); + params.put("direction", "right"); + if (cwd != null && !cwd.isBlank()) { + params.put("cwd", cwd); + } + if (env != null && !env.isEmpty()) { + params.put("env", env); + } + return herdr.call("pane.split", params).path("pane").path("pane_id").asText(null); } /** Give a worker's tab a human label in the tab bar. */ diff --git a/bridged/src/main/java/dev/ltms/bridged/worker/HerdrPeerLauncher.java b/bridged/src/main/java/dev/ltms/bridged/worker/HerdrPeerLauncher.java index 1d3f7c2..cdd2005 100644 --- a/bridged/src/main/java/dev/ltms/bridged/worker/HerdrPeerLauncher.java +++ b/bridged/src/main/java/dev/ltms/bridged/worker/HerdrPeerLauncher.java @@ -55,6 +55,13 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { /** herdr rejects a duplicate agent {@code name}; we retry a bumped name this many times. */ private static final int NAME_RETRIES = 8; + /** + * Retries for {@code agent.start} against a seed pane whose shell has not reached its prompt + * yet — {@code tab.create}/{@code pane.split} return as soon as the pane exists, and herdr + * refuses to start an agent in a pane that is not "an available shell" ({@code agent_pane_busy}). + */ + private static final int SHELL_READY_RETRIES = 20; + private final String namePrefix; // label prefix: naming + reap scheme private final AgentControl agents; private final WorkspaceControl spaces; @@ -227,17 +234,23 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { return null; } - /** Dedicated worker space → own tab → start the peer (rooted at {@code cwd}) → drop the shell. */ + /** Dedicated worker space → own tab (carrying cwd+env) → start the peer into the seed pane. */ private Agent spawnInTab(BridgedConfig.Worker cfg, Map workerEnv, List argv, String cwd) { Workspace space = spaces.ensureWorkspace(cfg.workspace()); - Tab.Created tab = spaces.createTab(space.workspaceId()); + Tab.Created tab = spaces.createTab(space.workspaceId(), cwd, workerEnv); log.info("spawning {} profile={} space={} tab={} cwd={}", namePrefix, cfg.profile(), space.workspaceId(), tab.tab().tabId(), cwd); Started started; try { - started = startUniquelyNamed(cfg, workerEnv, argv, tab.tab().tabId(), cwd); + if (tab.rootPaneId() == null) { + // Protocol 19 starts the agent INTO the seed pane — without one there is nowhere + // to start, and a partial tab would be left behind. + throw new IllegalStateException("tab " + tab.tab().tabId() + + " had no seed pane in the create response — cannot start a peer in it"); + } + started = startUniquelyNamed(cfg, argv, tab.rootPaneId()); } catch (RuntimeException e) { // The peer never started — don't leave the tab we just created orphaned. // Best-effort cleanup; never let it mask the real spawn failure. @@ -250,16 +263,9 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { throw e; } - // The peer is LIVE now. The remaining steps are cosmetic (drop herdr's seed shell so the - // tab holds only the peer; label the tab). They must not fail the spawn or orphan the - // running peer — on error we log and still return it so the caller gets its paneId and can - // tear it down. - if (tab.rootPaneId() != null) { - tidy("close seed pane " + tab.rootPaneId(), () -> agents.close(tab.rootPaneId())); - } else { - log.warn("tab {} had no seed pane in the create response; peer tab may hold an extra pane", - tab.tab().tabId()); - } + // The peer is LIVE now, in the seed pane itself (no shell pane to drop — protocol 19). + // Labelling is cosmetic: it must not fail the spawn or orphan the running peer — on error + // we log and still return it so the caller gets its paneId and can tear it down. tidy("label tab " + tab.tab().tabId(), () -> spaces.renameTab(tab.tab().tabId(), cfg.renderTabLabel(started.seq()))); log.info("{} started pane={} tab={} terminal={}", @@ -276,12 +282,16 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { } } - /** Legacy placement: herdr splits the currently-focused tab; the peer still starts in {@code cwd}. */ + /** Legacy placement: split the currently-focused tab; the peer still starts in {@code cwd}. */ private Agent spawnAsPane(BridgedConfig.Worker cfg, Map workerEnv, List argv, String cwd) { log.info("spawning {} (pane placement) profile={} cwd={} argv={}", namePrefix, cfg.profile(), cwd, argv); - Agent peer = startUniquelyNamed(cfg, workerEnv, argv, null, cwd).agent(); + String paneId = spaces.splitPane(cwd, workerEnv); + if (paneId == null) { + throw new IllegalStateException("pane.split returned no pane — cannot start a peer"); + } + Agent peer = startUniquelyNamed(cfg, argv, paneId).agent(); log.info("{} started pane={} terminal={}", namePrefix, peer.paneId(), peer.terminalId()); return peer; } @@ -300,14 +310,16 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { * backstop for the astronomically unlikely nonce+seq clash; the name is a label only — herdr * detects kind and status from terminal output, not from it. */ - private Started startUniquelyNamed(BridgedConfig.Worker cfg, Map workerEnv, - List argv, String tabId, String cwd) { + private Started startUniquelyNamed(BridgedConfig.Worker cfg, List argv, String paneId) { + // Protocol 19 resolves the executable from the agent kind (== namePrefix here), so + // argv[0] — the configured executable — is dropped and only the extra args are passed. + List args = argv.isEmpty() ? argv : argv.subList(1, argv.size()); HerdrException last = null; for (int attempt = 0; attempt < NAME_RETRIES; attempt++) { long seq = nameSeq.incrementAndGet(); String name = namePrefix + "-" + cfg.profile() + "-" + nameNonce + "-" + seq; try { - return new Started(agents.start(name, argv, workerEnv, tabId, cwd), seq); + return new Started(startAwaitingShellPrompt(name, args, paneId), seq); } catch (HerdrException e) { if (!"agent_name_taken".equals(e.code())) throw e; log.debug("peer name '{}' taken, retrying", name); @@ -317,6 +329,22 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { throw last; } + /** Start the agent into {@code paneId}, waiting out the seed shell's boot with the sleeper. */ + private Agent startAwaitingShellPrompt(String name, List args, String paneId) { + HerdrException busy = null; + for (int attempt = 0; attempt < SHELL_READY_RETRIES; attempt++) { + try { + return agents.start(name, namePrefix, args, paneId); + } catch (HerdrException e) { + if (!"agent_pane_busy".equals(e.code())) throw e; + log.debug("pane {} not at its shell prompt yet, retrying agent.start", paneId); + busy = e; + sleeper.run(); + } + } + throw busy; + } + // --- discovery + reap ---------------------------------------------------------------------- /** All herdr-tracked agents — discovery for "what peers exist". */ diff --git a/bridged/src/test/java/dev/ltms/bridged/herdr/AgentControlContractTest.java b/bridged/src/test/java/dev/ltms/bridged/herdr/AgentControlContractTest.java index 1e34c8d..d5b5fec 100644 --- a/bridged/src/test/java/dev/ltms/bridged/herdr/AgentControlContractTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/herdr/AgentControlContractTest.java @@ -11,10 +11,11 @@ import static org.junit.jupiter.api.Assertions.*; import static org.junit.jupiter.api.Assumptions.assumeTrue; /** - * Contract test for the {@code agent.*} south side against a REAL herdr, locking in - * the CB-102 spike findings. It spawns a HARMLESS probe command (never {@code claude}, - * so no subscription/token involvement), proves the {@code env} map reaches the process - * environment, exercises status/read, and always tears the pane down. + * Contract test for the worker env seam against a REAL herdr. Under protocol 19 (CB-521) the + * env map is injected at PANE CREATION ({@code tab.create}), not {@code agent.start} — and + * {@code agent.start} now only launches supported agent kinds, so this probes the seed pane's + * SHELL directly (never {@code claude}, so no subscription/token involvement) and always tears + * the throwaway space down. * *

Tagged {@code contract}; run with {@code mvn test -Pcontract}. */ @@ -26,38 +27,30 @@ class AgentControlContractTest { } @Test - void startInjectsEnvThenReadAndClose() throws Exception { + void tabCreateInjectsEnvIntoTheSeedShell() throws Exception { assumeTrue(!noSocket(), "no herdr socket — skipping"); try (UnixSocketHerdrClient herdr = UnixSocketHerdrClient.connect()) { - AgentControl agents = new AgentControl(herdr); - - Agent probe = agents.start( - "__contract__", - List.of("bash", "-c", "printf 'PROBE_BASE=[%s]\\n' \"$ANTHROPIC_BASE_URL\"; sleep 20"), + WorkspaceControl spaces = new WorkspaceControl(herdr); + Workspace space = spaces.ensureWorkspace("__bridged_env_contract__"); + Tab.Created tab = spaces.createTab(space.workspaceId(), null, Map.of("ANTHROPIC_BASE_URL", "http://gx00.gw:8000")); - - assertNotNull(probe.terminalId()); - assertNotNull(probe.paneId()); try { - // Give the shell a moment to print, then confirm env reached the process. + assertNotNull(tab.rootPaneId(), "tab.create must return the seed pane"); + Thread.sleep(1000); // let the seed shell reach its prompt + herdr.call("pane.send_input", Map.of( + "pane_id", tab.rootPaneId(), + "text", "printf 'PROBE_BASE=[%s]\\n' \"$ANTHROPIC_BASE_URL\"", + "keys", List.of("enter"))); Thread.sleep(800); - String visible = agents.read(probe.terminalId(), "visible"); + String visible = herdr.call("pane.read", + Map.of("pane_id", tab.rootPaneId(), "source", "visible")) + .path("read").path("text").asText(""); assertTrue(visible.contains("PROBE_BASE=[http://gx00.gw:8000]"), - "env map must reach the process; saw: " + visible); - - // Status is queryable; the probe appears in the agent list. - assertNotNull(agents.status(probe.terminalId())); - assertTrue(agents.list().stream() - .anyMatch(a -> probe.terminalId().equals(a.terminalId())), - "spawned probe should appear in agent.list"); + "env map must reach the seed shell; saw: " + visible); } finally { - agents.close(probe.paneId()); + spaces.closeTab(tab.tab().tabId()); + herdr.call("workspace.close", Map.of("workspace_id", space.workspaceId())); } - - // After close the pane is gone. - assertFalse(agents.list().stream() - .anyMatch(a -> probe.terminalId().equals(a.terminalId())), - "closed probe should no longer be listed"); } } } diff --git a/bridged/src/test/java/dev/ltms/bridged/herdr/AgentControlTest.java b/bridged/src/test/java/dev/ltms/bridged/herdr/AgentControlTest.java index b32bea3..023c0ea 100644 --- a/bridged/src/test/java/dev/ltms/bridged/herdr/AgentControlTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/herdr/AgentControlTest.java @@ -7,34 +7,68 @@ import java.util.Map; import static org.junit.jupiter.api.Assertions.assertEquals; -/** Unit-level behaviour of {@link AgentControl} over a fake herdr. */ +/** Unit-level behaviour of {@link AgentControl} over a fake herdr (protocol 19). */ class AgentControlTest { - /** The {@code text} of every agent.send, in call order. */ + /** The {@code text} of every agent.prompt, in call order. */ @SuppressWarnings("unchecked") - private static List sendTexts(FakeHerdr herdr) { + private static List promptTexts(FakeHerdr herdr) { return herdr.calls.stream() - .filter(c -> c.method().equals("agent.send")) + .filter(c -> c.method().equals("agent.prompt")) .map(c -> ((Map) c.params()).get("text").toString()) .toList(); } @Test - void sendDeliversThePayloadThenAStandaloneSubmitKey() { + void sendDeliversThePayloadAsOnePromptThatSubmitsItself() { FakeHerdr herdr = new FakeHerdr(); new AgentControl(herdr).send("term_x", "do the thing"); - // The Enter must be its own event — appended to the paste it would be swallowed as text. - assertEquals(List.of("do the thing", "\r"), sendTexts(herdr), - "payload paste first, then a separate carriage-return keystroke to submit it"); + // agent.prompt pastes AND submits in one call — no separate Enter event to assert. + assertEquals(List.of("do the thing"), promptTexts(herdr), + "exactly one agent.prompt carrying the payload"); } @Test - void sendPreservesEmbeddedNewlinesAndSubmitsOnlyOnce() { + void sendPreservesEmbeddedNewlinesVerbatim() { FakeHerdr herdr = new FakeHerdr(); new AgentControl(herdr).send("term_x", "line1\nline2"); - assertEquals(List.of("line1\nline2", "\r"), sendTexts(herdr), - "multiline content is delivered verbatim; a single trailing Enter submits it"); + assertEquals(List.of("line1\nline2"), promptTexts(herdr), + "multiline content is delivered verbatim in the single prompt"); + } + + @Test + void submitNudgesWithAStandaloneEnterKeystroke() { + FakeHerdr herdr = new FakeHerdr(); + new AgentControl(herdr).submit("term_x"); + + FakeHerdr.Call keys = herdr.lastCall("agent.send_keys"); + assertEquals(Map.of("target", "term_x", "keys", List.of("enter")), keys.params(), + "the raced-Enter nudge is a raw send_keys, not a second prompt"); + } + + @Test + @SuppressWarnings("unchecked") + void aTerminalIdTargetIsTranslatedToItsPaneId() { + // Protocol 19 rejects terminal_id as an agent.* target; the fake's agent.list maps + // term_a to pane w2:p7, and the control layer must address herdr by that pane. + FakeHerdr herdr = new FakeHerdr(); + new AgentControl(herdr).send("term_a", "hello"); + + Map prompt = (Map) herdr.lastCall("agent.prompt").params(); + assertEquals("w2:p7", prompt.get("target"), "terminal target resolved to the agent's pane id"); + } + + @Test + @SuppressWarnings("unchecked") + void theTerminalToPaneMappingIsCachedAcrossCalls() { + FakeHerdr herdr = new FakeHerdr(); + AgentControl agents = new AgentControl(herdr); + agents.send("term_a", "one"); + agents.send("term_a", "two"); + + long lists = herdr.calls.stream().filter(c -> c.method().equals("agent.list")).count(); + assertEquals(1, lists, "one agent.list resolution serves every later call to the same terminal"); } } diff --git a/bridged/src/test/java/dev/ltms/bridged/herdr/FakeHerdr.java b/bridged/src/test/java/dev/ltms/bridged/herdr/FakeHerdr.java index 85d7fee..ae3dc49 100644 --- a/bridged/src/test/java/dev/ltms/bridged/herdr/FakeHerdr.java +++ b/bridged/src/test/java/dev/ltms/bridged/herdr/FakeHerdr.java @@ -8,8 +8,8 @@ import java.util.List; /** * Recording fake {@link HerdrClient} for unit/acceptance tests. Returns canned frames - * captured from the real herdr 0.7.0 daemon and records every call so tests can assert - * both behaviour and that guard-blocked paths never reached herdr. + * matching the real herdr 0.8.0 daemon (protocol 19) and records every call so tests can + * assert both behaviour and that guard-blocked paths never reached herdr. */ public final class FakeHerdr implements HerdrClient { @@ -25,6 +25,7 @@ public final class FakeHerdr implements HerdrClient { private final List extraWorkspaces = new ArrayList<>(); private final List extraAgents = new ArrayList<>(); private int agentNameTakenFor = 0; + private int agentPaneBusyFor = 0; private int workerTabPaneCount = 1; private String paneCloseErrorCode = null; private String agentSendErrorCode = null; @@ -42,6 +43,12 @@ public final class FakeHerdr implements HerdrClient { return this; } + /** Reject the first {@code n} {@code agent.start} calls with {@code agent_pane_busy}. */ + public FakeHerdr agentPaneBusyTimes(int n) { + this.agentPaneBusyFor = n; + return this; + } + /** Make the worker tab (w9:t2) report this many panes in {@code tab.list} (default 1). */ public FakeHerdr withWorkerTabPaneCount(int n) { this.workerTabPaneCount = n; @@ -66,7 +73,7 @@ public final class FakeHerdr implements HerdrClient { return this; } - /** Make {@code agent.send} fail with this herdr error code. */ + /** Make delivery ({@code agent.prompt} / {@code agent.send_keys}) fail with this error code. */ public FakeHerdr agentSendFailsWith(String code) { this.agentSendErrorCode = code; return this; @@ -110,7 +117,7 @@ public final class FakeHerdr implements HerdrClient { try { return switch (method) { case "ping" -> mapper.readTree( - "{\"type\":\"pong\",\"version\":\"0.7.0\",\"protocol\":14}"); + "{\"type\":\"pong\",\"version\":\"0.8.0\",\"protocol\":19}"); case "workspace.list" -> mapper.readTree((""" {"type":"workspace_list","workspaces":[ {"workspace_id":"w1","label":"dev-mgnl","focused":true,"pane_count":7,"agent_status":"unknown"}, @@ -122,9 +129,19 @@ public final class FakeHerdr implements HerdrClient { "agent_session":{"kind":"id","value":"sess-1111"}, "workspace_id":"w2","tab_id":"w2:t7","pane_id":"w2:p7"}%s]}""") .formatted(extraAgents.isEmpty() ? "" : "," + String.join(",", extraAgents))); - case "agent.send" -> { + case "agent.prompt" -> { if (agentSendErrorCode != null) { - throw new HerdrException("herdr error [" + agentSendErrorCode + "]: agent.send failed", + throw new HerdrException("herdr error [" + agentSendErrorCode + "]: agent.prompt failed", + agentSendErrorCode, null); + } + yield mapper.readTree((""" + {"type":"agent_prompted","agent":{"terminal_id":"term_a","agent":"claude", + "agent_status":"%s","workspace_id":"w2","tab_id":"w2:t7","pane_id":"w2:p7"}}""") + .formatted(agentStatus)); + } + case "agent.send_keys" -> { + if (agentSendErrorCode != null) { + throw new HerdrException("herdr error [" + agentSendErrorCode + "]: agent.send_keys failed", agentSendErrorCode, null); } yield mapper.readTree("{\"type\":\"ok\"}"); @@ -136,29 +153,54 @@ public final class FakeHerdr implements HerdrClient { case "agent.read" -> mapper.readTree(mapper.writeValueAsString( java.util.Map.of("type", "agent_read", "read", java.util.Map.of("text", readText)))); case "agent.start" -> { + // Protocol 19: kind and pane_id are required — reject like the real daemon. + java.util.Map p = params instanceof java.util.Map m ? m : java.util.Map.of(); + for (String required : new String[]{"kind", "pane_id"}) { + if (p.get(required) == null) { + throw new HerdrException( + "herdr error [invalid_request]: invalid request: missing field `" + + required + "`", "invalid_request", null); + } + } long starts = calls.stream().filter(c -> c.method().equals("agent.start")).count(); - if (starts <= agentNameTakenFor) { + if (starts <= agentPaneBusyFor) { + throw new HerdrException( + "herdr error [agent_pane_busy]: agent target pane is not an available shell", + "agent_pane_busy", null); + } + long busyAdjusted = starts - agentPaneBusyFor; + if (busyAdjusted <= agentNameTakenFor) { throw new HerdrException( "herdr error [agent_name_taken]: agent name already used", "agent_name_taken", null); } - long n = starts - agentNameTakenFor; + long n = busyAdjusted - agentNameTakenFor; + // The agent starts INTO the requested pane, so its pane_id echoes the param. yield mapper.readTree((""" {"type":"agent_started","agent":{ "terminal_id":"term_new_%d","name":"claude","agent_status":"unknown", - "workspace_id":"w9","tab_id":"w9:t2","pane_id":"w9:pW_%d"}}""") - .formatted(n, n)); + "workspace_id":"w9","tab_id":"w9:t2","pane_id":"%s"}}""") + .formatted(n, p.get("pane_id"))); } + case "pane.split" -> mapper.readTree(""" + {"type":"pane_info","pane":{"pane_id":"w1:pSplit","workspace_id":"w1", + "tab_id":"w1:t1"}}"""); case "workspace.create" -> mapper.readTree(""" {"type":"workspace_created", "workspace":{"workspace_id":"w9","label":"bridged-workers","focused":false, "pane_count":1,"tab_count":1,"active_tab_id":"w9:t1","agent_status":"unknown"}, "tab":{"tab_id":"w9:t1","workspace_id":"w9","label":"1","pane_count":1}, "root_pane":{"pane_id":"w9:p1","workspace_id":"w9","tab_id":"w9:t1"}}"""); - case "tab.create" -> mapper.readTree(""" + case "tab.create" -> { + // Each tab gets its own seed pane — under protocol 19 that pane becomes the + // worker pane, so distinct spawns must yield distinct pane ids. + long tabs = calls.stream().filter(c -> c.method().equals("tab.create")).count(); + yield mapper.readTree((""" {"type":"tab_created", "tab":{"tab_id":"w9:t2","workspace_id":"w9","label":"2","pane_count":1}, - "root_pane":{"pane_id":"w9:pRoot","workspace_id":"w9","tab_id":"w9:t2"}}"""); + "root_pane":{"pane_id":"w9:pRoot_%d","workspace_id":"w9","tab_id":"w9:t2"}}""") + .formatted(tabs)); + } case "tab.rename" -> mapper.readTree(""" {"type":"tab_info","tab":{"tab_id":"w9:t2","workspace_id":"w9", "label":"worker: ltms-local","pane_count":1}}"""); diff --git a/bridged/src/test/java/dev/ltms/bridged/herdr/PaneLocatorContractTest.java b/bridged/src/test/java/dev/ltms/bridged/herdr/PaneLocatorContractTest.java index 1ef0451..d391948 100644 --- a/bridged/src/test/java/dev/ltms/bridged/herdr/PaneLocatorContractTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/herdr/PaneLocatorContractTest.java @@ -5,16 +5,15 @@ import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import java.nio.file.Files; -import java.util.List; import java.util.Map; import static org.junit.jupiter.api.Assertions.*; import static org.junit.jupiter.api.Assumptions.assumeTrue; /** - * Contract test for the herdr half of connection-based identity against a REAL herdr: spawn a - * harmless probe, read its actual {@code shell_pid} from {@code pane.process_info}, and confirm - * {@link PaneLocator} resolves that PID back to the probe's own {@code terminal_id}. + * Contract test for {@link PaneLocator} against a REAL herdr: the PID→pane mapping that + * connection identity rests on. Uses a throwaway tab's seed shell as the probe process + * (protocol 19 removed arbitrary-command agents), and always tears the space down. * *

Tagged {@code contract}; run with {@code mvn test -Pcontract}. */ @@ -25,18 +24,24 @@ class PaneLocatorContractTest { void resolvesTheTerminalOwningARealProcessPid() throws Exception { assumeTrue(Files.exists(UnixSocketHerdrClient.defaultSocketPath()), "no herdr socket — skipping"); try (UnixSocketHerdrClient herdr = UnixSocketHerdrClient.connect()) { - AgentControl agents = new AgentControl(herdr); - Agent probe = agents.start("__pidprobe__", List.of("bash", "-c", "sleep 20"), Map.of()); + WorkspaceControl spaces = new WorkspaceControl(herdr); + Workspace space = spaces.ensureWorkspace("__bridged_pid_contract__"); + Tab.Created tab = spaces.createTab(space.workspaceId(), null, Map.of()); try { - JsonNode info = herdr.call("pane.process_info", Map.of("pane_id", probe.paneId())) + JsonNode info = herdr.call("pane.process_info", Map.of("pane_id", tab.rootPaneId())) .path("process_info"); long shellPid = info.path("shell_pid").asLong(-1); - assertTrue(shellPid > 0, "probe pane should report a shell pid"); + assertTrue(shellPid > 0, "seed pane should report a shell pid"); - assertEquals(probe.terminalId(), new PaneLocator(herdr).terminalForPid(shellPid), + String terminalId = herdr.call("pane.get", Map.of("pane_id", tab.rootPaneId())) + .path("pane").path("terminal_id").asText(null); + assertNotNull(terminalId, "seed pane should carry a terminal_id"); + + assertEquals(terminalId, new PaneLocator(herdr).terminalForPid(shellPid), "a real PID must resolve back to its own pane's terminal_id"); } finally { - agents.close(probe.paneId()); + spaces.closeTab(tab.tab().tabId()); + herdr.call("workspace.close", Map.of("workspace_id", space.workspaceId())); } } } diff --git a/bridged/src/test/java/dev/ltms/bridged/herdr/WorkspacePlacementContractTest.java b/bridged/src/test/java/dev/ltms/bridged/herdr/WorkspacePlacementContractTest.java index c4a2cb9..6a6e373 100644 --- a/bridged/src/test/java/dev/ltms/bridged/herdr/WorkspacePlacementContractTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/herdr/WorkspacePlacementContractTest.java @@ -5,7 +5,6 @@ import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import java.nio.file.Files; -import java.util.List; import java.util.Map; import static org.junit.jupiter.api.Assertions.*; @@ -13,9 +12,9 @@ import static org.junit.jupiter.api.Assumptions.assumeTrue; /** * Contract test for the placement layer ({@code workspace.*}/{@code tab.*}) against a - * REAL herdr, locking in the "one clean tab per worker" recipe: find-or-create a worker - * space, give the worker its own tab, drop herdr's seed shell so the tab holds only the - * worker, and tear it all down. Uses a HARMLESS probe (never {@code claude}) in a + * REAL herdr, locking in the "one clean tab per worker" recipe under protocol 19: find-or-create + * a worker space, give the worker its own tab, and the SEED pane is where the worker starts — + * the tab holds exactly that one pane from creation. Uses no agent (never {@code claude}) in a * throwaway space that is fully removed at the end. * *

Tagged {@code contract}; run with {@code mvn test -Pcontract}. @@ -41,7 +40,6 @@ class WorkspacePlacementContractTest { void workerGetsOwnCleanTabAndTearsDownCompletely() throws Exception { assumeTrue(!noSocket(), "no herdr socket — skipping"); try (UnixSocketHerdrClient herdr = UnixSocketHerdrClient.connect()) { - AgentControl agents = new AgentControl(herdr); WorkspaceControl spaces = new WorkspaceControl(herdr); Workspace space = spaces.ensureWorkspace(LABEL); @@ -49,36 +47,27 @@ class WorkspacePlacementContractTest { // Idempotent: a second ensure finds the same space, never creates a duplicate. assertEquals(space.workspaceId(), spaces.ensureWorkspace(LABEL).workspaceId()); - Tab.Created tab = spaces.createTab(space.workspaceId()); - Agent worker = agents.start( - "__contract__", - List.of("bash", "-c", "sleep 20"), - Map.of(), - tab.tab().tabId()); + Tab.Created tab = spaces.createTab(space.workspaceId(), null, Map.of()); try { - // The worker landed in its dedicated tab in the worker space. - assertEquals(tab.tab().tabId(), worker.tabId()); - assertEquals(space.workspaceId(), worker.workspaceId()); + assertNotNull(tab.rootPaneId(), "tab.create must return the seed pane"); - // Drop the seed shell; the tab now holds exactly the worker pane. - agents.close(tab.rootPaneId()); + // Protocol 19: the seed pane IS the worker pane — the tab holds exactly it. spaces.renameTab(tab.tab().tabId(), "worker: contract"); assertEquals(1, paneCount(herdr, space.workspaceId(), tab.tab().tabId()), - "worker tab must hold only the worker pane after the seed shell is dropped"); + "worker tab must hold exactly the seed/worker pane"); // Teardown resolves the tab from the pane, and sees it holds exactly one pane. - WorkspaceControl.PaneLocation loc = spaces.locatePane(worker.paneId()); + WorkspaceControl.PaneLocation loc = spaces.locatePane(tab.rootPaneId()); assertNotNull(loc); assertEquals(tab.tab().tabId(), loc.tabId()); - assertEquals(1, loc.tabPaneCount(), "worker is the tab's sole occupant"); + assertEquals(1, loc.tabPaneCount(), "worker pane is the tab's sole occupant"); } finally { - agents.close(worker.paneId()); spaces.closeTab(tab.tab().tabId()); } // Tolerant teardown: closing an already-gone tab / reading a gone pane is a no-op. spaces.closeTab(tab.tab().tabId()); - assertNull(spaces.locatePane(worker.paneId()), "closed worker pane must be gone"); + assertNull(spaces.locatePane(tab.rootPaneId()), "closed worker pane must be gone"); // Remove the throwaway space entirely so the test leaves no residue. herdr.call("workspace.close", Map.of("workspace_id", space.workspaceId())); diff --git a/bridged/src/test/java/dev/ltms/bridged/inject/InjectorTest.java b/bridged/src/test/java/dev/ltms/bridged/inject/InjectorTest.java index 2e81012..0d7cffa 100644 --- a/bridged/src/test/java/dev/ltms/bridged/inject/InjectorTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/inject/InjectorTest.java @@ -28,17 +28,15 @@ class InjectorTest { private final Injector injector = new Injector(new AgentControl(herdr)); /** - * The logical messages delivered, in order. AgentControl.send emits each delivery as two - * agent.send calls — the payload, then a standalone Enter keystroke ({@code "\r"}) to submit - * it; these tests assert delivery ordering/gating, not the submit event, so drop the bare - * carriage returns. + * The logical messages delivered, in order. Under protocol 19 each delivery is one + * {@code agent.prompt} carrying the payload (it submits itself); the Enter nudge is a + * separate {@code agent.send_keys} and never appears here. */ @SuppressWarnings("unchecked") private List sent() { return herdr.calls.stream() - .filter(c -> c.method().equals("agent.send")) + .filter(c -> c.method().equals("agent.prompt")) .map(c -> ((Map) c.params()).get("text").toString()) - .filter(t -> !t.equals("\r")) .toList(); } @@ -66,11 +64,9 @@ class InjectorTest { assertEquals(List.of("task"), sent(), "delivers once the worker is available"); } - @SuppressWarnings("unchecked") private long enterKeystrokes() { return herdr.calls.stream() - .filter(c -> c.method().equals("agent.send")) - .filter(c -> "\r".equals(((Map) c.params()).get("text"))) + .filter(c -> c.method().equals("agent.send_keys")) .count(); } @@ -374,13 +370,12 @@ class InjectorTest { poller.stop(); } assertEquals(List.of("via-poller"), idle.calls.stream() - .filter(c -> c.method().equals("agent.send")) + .filter(c -> c.method().equals("agent.prompt")) .map(c -> { @SuppressWarnings("unchecked") Map p = (Map) c.params(); return p.get("text").toString(); }) - .filter(t -> !t.equals("\r")) // drop the standalone submit keystroke .toList()); } diff --git a/bridged/src/test/java/dev/ltms/bridged/mcp/BridgeMcpTest.java b/bridged/src/test/java/dev/ltms/bridged/mcp/BridgeMcpTest.java index d06ebc3..44fa75b 100644 --- a/bridged/src/test/java/dev/ltms/bridged/mcp/BridgeMcpTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/mcp/BridgeMcpTest.java @@ -221,7 +221,7 @@ class BridgeMcpTest { assertNotEquals(Boolean.TRUE, res.isError()); String out = textOf(res); assertTrue(out.contains("\"sessionId\":\"term_new_1\""), out); - assertTrue(out.contains("\"paneId\":\"w9:pW_1\""), out); + assertTrue(out.contains("\"paneId\":\"w9:pRoot_1\""), out); assertTrue(out.contains("\"status\":\"spawning\""), out); } @@ -250,9 +250,10 @@ class BridgeMcpTest { McpSchema.CallToolResult res = BridgeMcp.spawn( sessionManager(h, "http://gx00.gw:8000", Set.of("gx00.gw")), null, "/req/dir", null, null, null); assertNotEquals(Boolean.TRUE, res.isError()); + // Protocol 19: the requested cwd roots the worker's pane at creation (tab.create). @SuppressWarnings("unchecked") - Map start = (Map) h.lastCall("agent.start").params(); - assertEquals("/req/dir", start.get("cwd")); + Map create = (Map) h.lastCall("tab.create").params(); + assertEquals("/req/dir", create.get("cwd")); } @Test diff --git a/bridged/src/test/java/dev/ltms/bridged/msg/ReplyPushLoopTest.java b/bridged/src/test/java/dev/ltms/bridged/msg/ReplyPushLoopTest.java index dbbf859..974d85e 100644 --- a/bridged/src/test/java/dev/ltms/bridged/msg/ReplyPushLoopTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/msg/ReplyPushLoopTest.java @@ -133,10 +133,10 @@ class ReplyPushLoopTest { loop(1, 50).onReplyQueued(WORKER); assertTrue(rec.sendLatch.await(3, TimeUnit.SECONDS), - "one nudge (2 agent.send calls) should have been sent"); + "one nudge (1 agent.prompt call) should have been sent"); - // Exactly one nudge = exactly 2 agent.send calls (text + submit) - assertEquals(2, rec.sendCount()); + // Exactly one nudge = exactly 1 agent.prompt call (it submits itself) + assertEquals(1, rec.sendCount()); assertTrue(rec.sentParams().stream() .anyMatch(e -> e.getValue().toString().contains("bridge_poll")), "nudge text should contain bridge_poll"); @@ -153,9 +153,9 @@ class ReplyPushLoopTest { loop.onReplyQueued(WORKER); // second call — should be a no-op assertTrue(rec.sendLatch.await(3, TimeUnit.SECONDS), - "expected exactly one nudge (2 sends)"); + "expected exactly one nudge (1 prompt)"); Thread.sleep(200); - assertEquals(2, rec.sendCount(), + assertEquals(1, rec.sendCount(), "second onReplyQueued must not trigger another nudge"); } @@ -165,15 +165,15 @@ class ReplyPushLoopTest { var rec = recordingClient(); agents = new AgentControl(rec); inbox.publish(WORKER, "m1", "hello"); - rec.sendLatch = new CountDownLatch(cap * 2); + rec.sendLatch = new CountDownLatch(cap); loop(cap, 50).onReplyQueued(WORKER); assertTrue(rec.sendLatch.await(5, TimeUnit.SECONDS), - cap + " nudges (" + (cap * 2) + " sends) should have fired"); + cap + " nudges (" + cap + " prompts) should have fired"); Thread.sleep(300); - assertEquals(cap * 2, rec.sendCount(), - "exactly " + (cap * 2) + " agent.send calls (cap=" + cap + ")"); + assertEquals(cap, rec.sendCount(), + "exactly " + cap + " agent.prompt calls (cap=" + cap + ")"); } // --- nudge format -------------------------------------------------------------------------- @@ -197,7 +197,7 @@ class ReplyPushLoopTest { loop(1, 50, metrics).onReplyQueued(WORKER); assertTrue(rec.sendLatch.await(3, TimeUnit.SECONDS), - "one nudge (2 agent.send calls) should have been sent"); + "one nudge (1 agent.prompt call) should have been sent"); // The delivered count is bumped on the scheduler thread right after the send that releases // the latch — settle briefly so the counter is published before we read it. Thread.sleep(200); @@ -261,13 +261,13 @@ class ReplyPushLoopTest { } /** - * Thread-safe recording fake that counts agent.send calls. Uses synchronized access - * so the scheduler thread and test thread never race. + * Thread-safe recording fake that counts agent.prompt calls (protocol 19: one nudge = one + * prompt). Uses synchronized access so the scheduler thread and test thread never race. */ private static final class RecordingHerdrClient implements HerdrClient { private final List> calls = Collections.synchronizedList(new ArrayList<>()); - volatile CountDownLatch sendLatch = new CountDownLatch(2); + volatile CountDownLatch sendLatch = new CountDownLatch(1); @Override public JsonNode call(String method, Object params) { @@ -277,7 +277,7 @@ class ReplyPushLoopTest { .put("terminal_id", PRIMARY) .put("agent_status", "idle")); // recording double is always injectable } - if ("agent.send".equals(method)) { + if ("agent.prompt".equals(method)) { calls.add(Map.entry(method, params)); sendLatch.countDown(); } diff --git a/bridged/src/test/java/dev/ltms/bridged/rest/BridgedAppTest.java b/bridged/src/test/java/dev/ltms/bridged/rest/BridgedAppTest.java index d546ca4..eb6feab 100644 --- a/bridged/src/test/java/dev/ltms/bridged/rest/BridgedAppTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/rest/BridgedAppTest.java @@ -118,7 +118,7 @@ class BridgedAppTest { assertEquals(200, res.statusCode()); JsonNode body = mapper.readTree(res.body()); assertEquals("ok", body.get("status").asText()); - assertEquals(14, body.get("herdr").get("protocol").asInt()); + assertEquals(19, body.get("herdr").get("protocol").asInt()); } @Test @@ -156,22 +156,24 @@ class BridgedAppTest { HttpResponse res = req(port, "POST", "/workers"); assertEquals(201, res.statusCode()); JsonNode body = mapper.readTree(res.body()); - assertEquals("w9:pW_1", body.get("paneId").asText()); + assertEquals("w9:pRoot_1", body.get("paneId").asText()); assertEquals("spawning", body.get("state").asText()); - // Subscription boundary: agent.start carried base_url + token in its env map. - Map start = params(herdr, "agent.start"); + // Subscription boundary (protocol 19): tab.create carried base_url + token in its env + // map — the seed shell the agent starts into is what inherits them. + Map create = params(herdr, "tab.create"); @SuppressWarnings("unchecked") - Map env = (Map) start.get("env"); + Map env = (Map) create.get("env"); assertEquals("http://gx00.gw:8000", env.get("ANTHROPIC_BASE_URL")); assertEquals("tok-abc", env.get("ANTHROPIC_AUTH_TOKEN")); - assertEquals(List.of("claude"), start.get("argv")); - // Placement: worker space ensured, worker started INTO its own tab, seed shell - // dropped, and the tab given a friendly label. + // Placement: worker space ensured, worker started INTO its tab's seed pane (which + // becomes the worker pane — nothing is dropped), and the tab given a friendly label. + Map start = params(herdr, "agent.start"); assertTrue(herdr.called("workspace.create"), "worker space must be found-or-created"); - assertEquals("w9:t2", start.get("tab_id"), "worker must start into its dedicated tab"); - assertEquals("w9:pRoot", params(herdr, "pane.close").get("pane_id"), "seed shell pane dropped"); + assertEquals("claude", start.get("kind"), "herdr resolves the executable from kind"); + assertEquals("w9:pRoot_1", start.get("pane_id"), "worker must start into its tab's seed pane"); + assertFalse(herdr.called("pane.close"), "the seed pane IS the worker pane — never dropped"); assertEquals("worker: ltms-local #1", params(herdr, "tab.rename").get("label"), "tab label carries the worker number so siblings stay distinct"); } @@ -215,7 +217,7 @@ class BridgedAppTest { FakeHerdr herdr = new FakeHerdr(); int port = start(herdr, "http://gx00.gw:8000", Set.of("gx00.gw")); assertEquals(201, req(port, "POST", "/workers?cwd=/tmp/proj").statusCode()); - assertEquals("/tmp/proj", params(herdr, "agent.start").get("cwd"), "the worker starts in cwd"); + assertEquals("/tmp/proj", params(herdr, "tab.create").get("cwd"), "the worker starts in cwd"); } @Test @@ -322,7 +324,7 @@ class BridgedAppTest { HttpResponse res = send.get(6, java.util.concurrent.TimeUnit.SECONDS); assertEquals(200, res.statusCode()); assertEquals("LGTM ship it", mapper.readTree(res.body()).get("reply").asText()); - // (injection via agent.send is covered deterministically by the timeout-working test) + // (injection via agent.prompt is covered deterministically by the timeout-working test) } @Test @@ -394,7 +396,7 @@ class BridgedAppTest { HttpResponse res = postMessage(port, "{\"content\":\"hi\",\"timeoutMs\":150}"); assertEquals(202, res.statusCode()); assertEquals("queued", mapper.readTree(res.body()).get("status").asText()); - assertFalse(herdr.called("agent.send"), "no injection while the worker is mid-turn"); + assertFalse(herdr.called("agent.prompt"), "no injection while the worker is mid-turn"); } @Test @@ -405,7 +407,7 @@ class BridgedAppTest { HttpResponse res = postMessage(port, "{\"content\":\"hi\",\"timeoutMs\":250}"); assertEquals(202, res.statusCode()); assertEquals("working", mapper.readTree(res.body()).get("status").asText()); - assertTrue(herdr.called("agent.send"), "message was injected"); + assertTrue(herdr.called("agent.prompt"), "message was injected"); } @Test diff --git a/bridged/src/test/java/dev/ltms/bridged/session/WorktreeSessionManagerTest.java b/bridged/src/test/java/dev/ltms/bridged/session/WorktreeSessionManagerTest.java index 8f8f581..f36cba0 100644 --- a/bridged/src/test/java/dev/ltms/bridged/session/WorktreeSessionManagerTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/session/WorktreeSessionManagerTest.java @@ -32,7 +32,8 @@ class WorktreeSessionManagerTest { private static String startCwd(FakeHerdr herdr) { @SuppressWarnings("unchecked") - Map start = (Map) herdr.lastCall("agent.start").params(); + // Protocol 19: the worker's cwd rides on pane creation (tab.create), not agent.start. + Map start = (Map) herdr.lastCall("tab.create").params(); Object cwd = start.get("cwd"); return cwd == null ? null : cwd.toString(); } diff --git a/bridged/src/test/java/dev/ltms/bridged/worker/ClaudeCodeLauncherTest.java b/bridged/src/test/java/dev/ltms/bridged/worker/ClaudeCodeLauncherTest.java index 7449157..d4510c0 100644 --- a/bridged/src/test/java/dev/ltms/bridged/worker/ClaudeCodeLauncherTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/worker/ClaudeCodeLauncherTest.java @@ -29,52 +29,81 @@ class ClaudeCodeLauncherTest { new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(), _ -> null); } + /** The {@code args} of the last agent.start — protocol 19: everything after the executable. */ @SuppressWarnings("unchecked") - private List spawnedArgv(FakeHerdr herdr) { - return (List) ((Map) herdr.lastCall("agent.start").params()).get("argv"); + private List spawnedArgs(FakeHerdr herdr) { + return (List) ((Map) herdr.lastCall("agent.start").params()).get("args"); } @Test void appendsBridgeMcpAndReplyCharterWhenMcpUrlSet() { FakeHerdr herdr = new FakeHerdr(); - service(herdr, List.of("ccs", "ltms-local"), "http://127.0.0.1:8765/mcp").spawn(); + service(herdr, List.of("claude"), "http://127.0.0.1:8765/mcp").spawn(); - List argv = spawnedArgv(herdr); - assertEquals(List.of("ccs", "ltms-local"), argv.subList(0, 2), "base command preserved first"); - assertTrue(argv.contains("--mcp-config")); - assertTrue(argv.stream().anyMatch(a -> a.contains("\"bridge\"") && a.contains("http://127.0.0.1:8765/mcp")), + List args = spawnedArgs(herdr); + assertTrue(args.contains("--mcp-config")); + assertTrue(args.stream().anyMatch(a -> a.contains("\"bridge\"") && a.contains("http://127.0.0.1:8765/mcp")), "inline bridge MCP config present"); - assertTrue(argv.contains("--append-system-prompt")); - assertTrue(argv.stream().anyMatch(a -> a.contains("bridge_reply")), "reply charter present"); + assertTrue(args.contains("--append-system-prompt")); + assertTrue(args.stream().anyMatch(a -> a.contains("bridge_reply")), "reply charter present"); + } + + @Test + void startRetriesWhileTheSeedShellBoots() { + // tab.create returns before the seed shell reaches its prompt; herdr refuses agent.start + // into a not-ready pane with agent_pane_busy. The launcher must wait it out, not fail. + FakeHerdr herdr = new FakeHerdr().agentPaneBusyTimes(2); + long[] clock = {0}; + ClaudeCodeLauncher svc = new ClaudeCodeLauncher( + new AgentControl(herdr), new WorkspaceControl(herdr), + new SubscriptionGuard(Set.of("gx00.gw")), + Map.of("ltms-local", new BridgedConfig.Worker( + "ltms-local", "http://gx00.gw:8000", "coder", null, "BRIDGED_WORKER_TOKEN", + List.of("claude"), "tab", "bridged-workers", "w #{n}", null, null, null)), + "ltms-local", _ -> null, + 0, () -> clock[0], () -> clock[0] += 50); + + PeerHandle handle = svc.spawn(new SpawnRequest(null, null, null)); + + assertNotNull(handle, "spawn succeeds once the shell is ready"); + assertEquals(3, herdr.calls.stream().filter(c -> c.method().equals("agent.start")).count(), + "two busy rejections, then the successful start"); + } + + @Test + void startResolvesTheExecutableFromKindAndDropsArgvZero() { + FakeHerdr herdr = new FakeHerdr(); + service(herdr, List.of("claude"), null).spawn(); + + Map start = (Map) herdr.lastCall("agent.start").params(); + assertEquals("claude", start.get("kind"), "herdr launches the canonical executable by kind"); + assertEquals(List.of(), start.get("args"), "the configured executable is not repeated in args"); } @Test void noBridgeFlagsWhenMcpUrlAbsent() { FakeHerdr herdr = new FakeHerdr(); - service(herdr, List.of("bash", "-c", "sleep 1"), null).spawn(); - assertEquals(List.of("bash", "-c", "sleep 1"), spawnedArgv(herdr), "argv untouched without mcpUrl"); + service(herdr, List.of("claude", "--verbose"), null).spawn(); + assertEquals(List.of("--verbose"), spawnedArgs(herdr), "extra args untouched without mcpUrl"); } private ClaudeCodeLauncher multiProfile(FakeHerdr herdr) { BridgedConfig.Worker gx10 = new BridgedConfig.Worker("gx10", "http://gx10.gw:8000", "coder", - null, "BRIDGED_WORKER_TOKEN", List.of("ccs", "gx10"), "tab", "bridged-workers", "w #{n}", null, null, null); + null, "BRIDGED_WORKER_TOKEN", List.of("claude"), "tab", "bridged-workers", "w #{n}", null, null, null); BridgedConfig.Worker ollama = new BridgedConfig.Worker("ollama", "http://ollama.ltms.dev", null, - null, "BRIDGED_WORKER_TOKEN", List.of("ccs", "ollama"), "tab", "bridged-workers", "w #{n}", null, null, null); + null, "BRIDGED_WORKER_TOKEN", List.of("claude"), "tab", "bridged-workers", "w #{n}", null, null, null); return new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr), new SubscriptionGuard(Set.of("gx10.gw", "ollama.ltms.dev")), Map.of("gx10", gx10, "ollama", ollama), "gx10", _ -> "tok"); } @Test - @SuppressWarnings("unchecked") - void spawnPicksTheNamedProfilesBaseUrlAndArgv() { + void spawnPicksTheNamedProfilesBaseUrl() { FakeHerdr herdr = new FakeHerdr(); multiProfile(herdr).spawn("ollama"); - Map start = (Map) herdr.lastCall("agent.start").params(); - Map env = (Map) start.get("env"); - assertEquals("http://ollama.ltms.dev", env.get("ANTHROPIC_BASE_URL"), "the named profile's base_url"); - assertEquals(List.of("ccs", "ollama"), start.get("argv"), "the named profile's launch command"); + assertEquals("http://ollama.ltms.dev", startEnv(herdr).get("ANTHROPIC_BASE_URL"), + "the named profile's base_url"); } @Test @@ -86,8 +115,9 @@ class ClaudeCodeLauncherTest { @SuppressWarnings("unchecked") private static String startCwd(FakeHerdr herdr) { - // The worker's cwd is set on agent.start (an agent pane does not inherit the tab's cwd). - Object v = ((Map) herdr.lastCall("agent.start").params()).get("cwd"); + // Protocol 19: the worker's cwd is set at pane creation (tab.create), where the seed + // shell — which the agent starts into — is rooted. + Object v = ((Map) herdr.lastCall("tab.create").params()).get("cwd"); return v == null ? null : v.toString(); } @@ -119,9 +149,10 @@ class ClaudeCodeLauncherTest { // --- CB-302 git-forge token injection (worker checkpoint grant) ------------ + /** Protocol 19: the worker's env is injected at pane creation (tab.create), not agent.start. */ @SuppressWarnings("unchecked") private static Map startEnv(FakeHerdr herdr) { - return (Map) ((Map) herdr.lastCall("agent.start").params()).get("env"); + return (Map) ((Map) herdr.lastCall("tab.create").params()).get("env"); } @Test @@ -234,7 +265,7 @@ class ClaudeCodeLauncherTest { PeerHandle handle = svc.spawn(new SpawnRequest(null, null, null)); assertNotNull(handle, "spawn must return a non-null handle"); - assertEquals("w9:pW_1", handle.id(), "handle.id() must equal the agent's paneId"); + assertEquals("w9:pRoot_1", handle.id(), "handle.id() must equal the agent's paneId"); } @Test @@ -355,8 +386,8 @@ class ClaudeCodeLauncherTest { PeerHandle handle = svc.spawn(new SpawnRequest(null, null, null)); assertNotNull(handle, "spawn returns a handle when worker becomes injectable"); - assertEquals("w9:pW_1", handle.id(), "handle id matches the started pane"); - assertEquals(0, paneCloseCount(herdr, "w9:pW_1"), + assertEquals("w9:pRoot_1", handle.id(), "handle id matches the started pane"); + assertEquals(0, paneCloseCount(herdr, "w9:pRoot_1"), "no pane.close when worker becomes injectable before timeout"); } @@ -376,12 +407,12 @@ class ClaudeCodeLauncherTest { PeerUnreachableException.class, () -> svc.spawn(new SpawnRequest(null, null, null))); - assertTrue(ex.getMessage().contains("w9:pW_1"), + assertTrue(ex.getMessage().contains("w9:pRoot_1"), "exception message references the paneId: " + ex.getMessage()); assertTrue(ex.getMessage().contains("1000"), "exception message references the timeout: " + ex.getMessage()); assertTrue(clock[0] >= 1000, "fake clock advanced past the timeout: " + clock[0]); - assertEquals(1, paneCloseCount(herdr, "w9:pW_1"), + assertEquals(1, paneCloseCount(herdr, "w9:pRoot_1"), "pane was closed on timeout (no orphan left behind)"); } diff --git a/bridged/src/test/java/dev/ltms/bridged/worker/OpenCodeLauncherTest.java b/bridged/src/test/java/dev/ltms/bridged/worker/OpenCodeLauncherTest.java index f058a67..57da6d2 100644 --- a/bridged/src/test/java/dev/ltms/bridged/worker/OpenCodeLauncherTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/worker/OpenCodeLauncherTest.java @@ -45,14 +45,18 @@ class OpenCodeLauncherTest { return (Map) herdr.lastCall("agent.start").params(); } + /** Protocol 19: the worker's env is injected at pane creation (tab.create), not agent.start. */ @SuppressWarnings("unchecked") private static Map startEnv(FakeHerdr herdr) { - return (Map) lastStart(herdr).get("env"); + Map env = + (Map) ((Map) herdr.lastCall("tab.create").params()).get("env"); + return env == null ? Map.of() : env; } + /** Protocol 19: agent.start carries only the args after the kind-resolved executable. */ @SuppressWarnings("unchecked") - private static List startArgv(FakeHerdr herdr) { - return (List) lastStart(herdr).get("argv"); + private static List startArgs(FakeHerdr herdr) { + return (List) lastStart(herdr).get("args"); } @Test @@ -99,18 +103,17 @@ class OpenCodeLauncherTest { FakeHerdr herdr = new FakeHerdr(); service(herdr, root, opencodeCfg("google/gemini-2.5-pro", null, null)).spawn(); - List argv = startArgv(herdr); - assertEquals("opencode", argv.getFirst(), "base opencode command preserved first"); - int m = argv.indexOf("-m"); + List args = startArgs(herdr); + int m = args.indexOf("-m"); assertTrue(m >= 0, "model is selected with -m"); - assertEquals("google/gemini-2.5-pro", argv.get(m + 1), "the provider/model selector follows -m"); + assertEquals("google/gemini-2.5-pro", args.get(m + 1), "the provider/model selector follows -m"); } @Test void noModelFlagWhenModelBlank(@TempDir Path root) { FakeHerdr herdr = new FakeHerdr(); service(herdr, root, opencodeCfg(null, null, null)).spawn(); - assertEquals(List.of("opencode"), startArgv(herdr), "no model → argv is the bare opencode command"); + assertEquals(List.of(), startArgs(herdr), "no model → no extra args beyond the executable"); } @Test @@ -170,7 +173,7 @@ class OpenCodeLauncherTest { assertTrue(clock[0] >= 1000, "the fake clock advanced past the timeout: " + clock[0]); long closes = herdr.calls.stream() .filter(c -> c.method().equals("pane.close")) - .filter(c -> "w9:pW_1".equals(((Map) c.params()).get("pane_id"))) + .filter(c -> "w9:pRoot_1".equals(((Map) c.params()).get("pane_id"))) .count(); assertEquals(1, closes, "the worker pane was reaped on timeout (no orphan)"); assertNotNull(ex.getMessage());