diff --git a/bridged/bridged.example.yaml b/bridged/bridged.example.yaml index 187e084..87b5748 100644 --- a/bridged/bridged.example.yaml +++ b/bridged/bridged.example.yaml @@ -18,6 +18,12 @@ worker: profile: ltms-local baseUrl: http://gx00.gw:8000 # the gx00 vLLM (models: coder / deepseek-v4-flash) model: coder + # Placement: each worker lands in its OWN tab inside a dedicated worker space, so it + # never splits or clutters your real work spaces. Use `pane` for the legacy behaviour + # (split the currently-focused tab). + placement: tab # tab | pane + workspace: bridged-workers # the dedicated worker space (found-or-created, shared) + tabLabel: "worker: {profile} #{n}" # {profile}/{model}/{n} substituted; {n} keeps sibling tabs distinct # Subscription boundary. A worker's base_url host MUST be one of these; the primary # must carry none. Grounded in ltms-local's real endpoints. diff --git a/bridged/src/main/java/dev/ltms/bridged/Bridged.java b/bridged/src/main/java/dev/ltms/bridged/Bridged.java index 5276070..c87e43e 100644 --- a/bridged/src/main/java/dev/ltms/bridged/Bridged.java +++ b/bridged/src/main/java/dev/ltms/bridged/Bridged.java @@ -4,6 +4,7 @@ import dev.ltms.bridged.config.BridgedConfig; import dev.ltms.bridged.guard.SubscriptionGuard; import dev.ltms.bridged.herdr.AgentControl; import dev.ltms.bridged.herdr.UnixSocketHerdrClient; +import dev.ltms.bridged.herdr.WorkspaceControl; import dev.ltms.bridged.rest.BridgedApp; import dev.ltms.bridged.worker.WorkerService; import io.javalin.Javalin; @@ -37,7 +38,8 @@ public final class Bridged { Runtime.getRuntime().addShutdownHook(new Thread(herdr::close)); AgentControl agents = new AgentControl(herdr); - WorkerService workers = new WorkerService(agents, guard, cfg.worker(), System::getenv); + WorkspaceControl spaces = new WorkspaceControl(herdr); + WorkerService workers = new WorkerService(agents, spaces, guard, cfg.worker(), System::getenv); Javalin app = new BridgedApp(herdr, workers).build(); app.start(cfg.bind().host(), cfg.bind().port()); diff --git a/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java b/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java index e63906d..fd064c3 100644 --- a/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java +++ b/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java @@ -45,13 +45,41 @@ public record BridgedConfig( * @param tokenEnv name of the host env var holding the worker's auth token; its value * is injected as {@code ANTHROPIC_AUTH_TOKEN} (never stored in config) * @param argv launch command; defaults to {@code ["claude"]} + * @param placement where a worker lands: {@code "tab"} (default — its own tab in the + * worker space) or {@code "pane"} (legacy — split the focused tab) + * @param workspace label of the dedicated worker space; found-or-created on first + * spawn (default {@code "bridged-workers"}). A future per-session + * layout is just a distinct label here — the shared space is default. + * @param tabLabel template for a worker tab's label; {@code {profile}}/{@code {model}} + * and {@code {n}} (per-worker number, to keep sibling tabs distinct) + * are substituted (default {@code "worker: {profile} #{n}"}) */ @JsonIgnoreProperties(ignoreUnknown = true) public record Worker(String profile, String baseUrl, String model, - String configDir, String tokenEnv, List argv) { + String configDir, String tokenEnv, List argv, + String placement, String workspace, String tabLabel) { public Worker { argv = (argv == null || argv.isEmpty()) ? List.of("claude") : List.copyOf(argv); tokenEnv = (tokenEnv == null || tokenEnv.isBlank()) ? "BRIDGED_WORKER_TOKEN" : tokenEnv; + placement = (placement == null || placement.isBlank()) ? "tab" : placement.toLowerCase(); + workspace = (workspace == null || workspace.isBlank()) ? "bridged-workers" : workspace; + tabLabel = (tabLabel == null || tabLabel.isBlank()) ? "worker: {profile} #{n}" : tabLabel; + } + + /** True when workers should land in their own tab in the worker space. */ + public boolean tabPlacement() { + return "tab".equals(placement); + } + + /** + * Render {@link #tabLabel} for the {@code n}-th worker (substitutes + * {@code {profile}}/{@code {model}}/{@code {n}}), so sibling worker tabs are distinct. + */ + public String renderTabLabel(long n) { + return tabLabel + .replace("{profile}", profile == null ? "" : profile) + .replace("{model}", model == null ? "" : model) + .replace("{n}", Long.toString(n)); } } diff --git a/bridged/src/main/java/dev/ltms/bridged/herdr/Agent.java b/bridged/src/main/java/dev/ltms/bridged/herdr/Agent.java index 9b26b0f..b84efb1 100644 --- a/bridged/src/main/java/dev/ltms/bridged/herdr/Agent.java +++ b/bridged/src/main/java/dev/ltms/bridged/herdr/Agent.java @@ -9,15 +9,18 @@ import com.fasterxml.jackson.databind.JsonNode; * @param terminalId herdr's stable handle — the {@code target} for send/read/get * @param paneId pane handle — the argument to {@code pane.close} * @param workspaceId owning workspace + * @param tabId owning tab (each worker gets its own; {@code null} in pane placement) * @param sessionId the agent's own session id (Claude's session UUID), or {@code null} * before it has registered one (e.g. immediately after start) - * @param agentType agent kind, e.g. {@code "claude"} (the launch label for spawned probes) + * @param agentType detected agent kind, e.g. {@code "claude"}, or {@code null} before herdr + * has detected it (the start-time shape) * @param status current lifecycle state */ public record Agent( String terminalId, String paneId, String workspaceId, + String tabId, String sessionId, String agentType, AgentStatus status) { @@ -28,13 +31,15 @@ public record Agent( String sessionId = session != null && session.hasNonNull("value") ? session.get("value").asText() : null; - // start returns "name" (the launch label); list/get return "agent" (the kind). - String type = a.hasNonNull("agent") ? a.get("agent").asText() - : a.path("name").asText(null); + // list/get carry the detected kind in "agent"; at start-time it is absent and the + // kind is unknown. Do NOT fall back to "name" — that is now a unique worker label + // (claude---), not a kind, and would diverge from agent.list. + String type = a.hasNonNull("agent") ? a.get("agent").asText() : null; return new Agent( a.path("terminal_id").asText(null), a.path("pane_id").asText(null), a.path("workspace_id").asText(null), + a.path("tab_id").asText(null), sessionId, type, AgentStatus.fromWire(a.path("agent_status").asText(null))); 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 36bfeca..3104545 100644 --- a/bridged/src/main/java/dev/ltms/bridged/herdr/AgentControl.java +++ b/bridged/src/main/java/dev/ltms/bridged/herdr/AgentControl.java @@ -3,6 +3,7 @@ package dev.ltms.bridged.herdr; import com.fasterxml.jackson.databind.JsonNode; import java.util.ArrayList; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -32,10 +33,23 @@ public final class AgentControl { * @param env process environment additions ({@code ANTHROPIC_BASE_URL}, token, …) */ public Agent start(String name, List argv, Map env) { - JsonNode result = herdr.call("agent.start", Map.of( - "name", name, - "argv", argv, - "env", env)); + return start(name, argv, env, null); + } + + /** + * Spawn an agent into a specific tab. 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). + */ + public Agent start(String name, List argv, Map env, String tabId) { + Map params = new LinkedHashMap<>(); + params.put("name", name); + params.put("argv", argv); + params.put("env", env); + if (tabId != null) { + params.put("tab_id", tabId); + } + JsonNode result = herdr.call("agent.start", params); return Agent.from(result.get("agent")); } diff --git a/bridged/src/main/java/dev/ltms/bridged/herdr/Tab.java b/bridged/src/main/java/dev/ltms/bridged/herdr/Tab.java new file mode 100644 index 0000000..bfc7124 --- /dev/null +++ b/bridged/src/main/java/dev/ltms/bridged/herdr/Tab.java @@ -0,0 +1,38 @@ +package dev.ltms.bridged.herdr; + +import com.fasterxml.jackson.databind.JsonNode; + +/** + * A herdr tab within a workspace — one worker gets one dedicated tab. Projected from + * {@code tab.list}/{@code tab.get}/{@code tab.rename}. + * + * @param tabId herdr's stable id (e.g. {@code "w4:t2"}) + * @param workspaceId owning workspace + * @param label display label in the tab bar + * @param paneCount number of panes in the tab (a finished worker tab holds exactly 1) + */ +public record Tab(String tabId, String workspaceId, String label, int paneCount) { + + /** Project a herdr {@code tab} node. */ + public static Tab from(JsonNode t) { + return new Tab( + t.path("tab_id").asText(null), + t.path("workspace_id").asText(null), + t.path("label").asText(null), + t.path("pane_count").asInt(0)); + } + + /** + * 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. + */ + public record Created(Tab tab, String rootPaneId) { + /** Project a {@code tab_created} result ({@code {tab, root_pane}}). */ + public static Created from(JsonNode result) { + return new Created( + Tab.from(result.path("tab")), + result.path("root_pane").path("pane_id").asText(null)); + } + } +} diff --git a/bridged/src/main/java/dev/ltms/bridged/herdr/Workspace.java b/bridged/src/main/java/dev/ltms/bridged/herdr/Workspace.java new file mode 100644 index 0000000..9dbfcab --- /dev/null +++ b/bridged/src/main/java/dev/ltms/bridged/herdr/Workspace.java @@ -0,0 +1,22 @@ +package dev.ltms.bridged.herdr; + +import com.fasterxml.jackson.databind.JsonNode; + +/** + * A herdr workspace ("space") — the top of the placement hierarchy. Workers live in a + * dedicated worker space so they never split or clutter the user's real work spaces. + * + * @param workspaceId herdr's stable id (e.g. {@code "w4"}) + * @param label display label shown in herdr's UI (e.g. {@code "bridged-workers"}) + * @param activeTabId the workspace's currently-focused tab, or {@code null} + */ +public record Workspace(String workspaceId, String label, String activeTabId) { + + /** Project a herdr {@code workspace} node (from workspace.list/create/get). */ + public static Workspace from(JsonNode w) { + return new Workspace( + w.path("workspace_id").asText(null), + w.path("label").asText(null), + w.path("active_tab_id").asText(null)); + } +} diff --git a/bridged/src/main/java/dev/ltms/bridged/herdr/WorkspaceControl.java b/bridged/src/main/java/dev/ltms/bridged/herdr/WorkspaceControl.java new file mode 100644 index 0000000..f5b6c99 --- /dev/null +++ b/bridged/src/main/java/dev/ltms/bridged/herdr/WorkspaceControl.java @@ -0,0 +1,139 @@ +package dev.ltms.bridged.herdr; + +import com.fasterxml.jackson.databind.JsonNode; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Optional; + +/** + * Domain layer over herdr's {@code workspace.*} and {@code tab.*} namespaces — the + * placement side of the worker south side. Where {@link AgentControl} runs a worker + * process, this decides where it lands: a dedicated worker space, one tab per + * worker, so workers never split or clutter the user's real work spaces. + * + *

Lifecycle is handled defensively: {@link #ensureWorkspace(String)} is a + * synchronized find-or-create so concurrent spawns never double-create the + * shared space, and teardown ({@link #closeTab(String)}) tolerates an already-gone tab + * so a double DELETE or a crashed worker never surfaces an error. + */ +public final class WorkspaceControl { + + private static final Logger log = LoggerFactory.getLogger(WorkspaceControl.class); + + private final HerdrClient herdr; + + public WorkspaceControl(HerdrClient herdr) { + this.herdr = herdr; + } + + /** Every workspace herdr knows about. */ + public List listWorkspaces() { + JsonNode result = herdr.call("workspace.list"); + List out = new ArrayList<>(); + for (JsonNode w : result.path("workspaces")) { + out.add(Workspace.from(w)); + } + return out; + } + + /** The first workspace with this exact label, if any. */ + public Optional findByLabel(String label) { + return listWorkspaces().stream() + .filter(w -> label.equals(w.label())) + .findFirst(); + } + + /** + * The dedicated worker space for {@code label}, creating it if absent. Idempotent and + * synchronized so two concurrent spawns share one space rather than racing to create + * two. The freshly-created space keeps its seed tab as a stable anchor — the shared + * space is persistent infra and is never auto-destroyed under running workers. + */ + public synchronized Workspace ensureWorkspace(String label) { + Optional existing = findByLabel(label); + if (existing.isPresent()) { + return existing.get(); + } + JsonNode result = herdr.call("workspace.create", Map.of("label", label)); + Workspace created = Workspace.from(result.path("workspace")); + log.info("created worker space '{}' -> {}", label, created.workspaceId()); + return created; + } + + /** + * 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. + */ + public Tab.Created createTab(String workspaceId) { + JsonNode result = herdr.call("tab.create", Map.of("workspace_id", workspaceId)); + return Tab.Created.from(result); + } + + /** Give a worker's tab a human label in the tab bar. */ + public void renameTab(String tabId, String label) { + herdr.call("tab.rename", Map.of("tab_id", tabId, "label", label)); + } + + /** + * Close a worker's tab. Tolerant only of an already-gone tab (double teardown, + * crashed worker) — a {@code tab_not_found} is success. Any other failure is real and + * propagates, so a genuinely failed close is never silently reported as done. + */ + public void closeTab(String tabId) { + try { + herdr.call("tab.close", Map.of("tab_id", tabId)); + } catch (HerdrException e) { + if (!isAlreadyGone(e)) throw e; + log.debug("tab.close({}) ignored — already gone: {}", tabId, e.getMessage()); + } + } + + /** + * Where a pane lives, for safe teardown: its tab, that tab's workspace, and how many + * panes the tab holds. The pane count lets the caller close a tab only when the + * worker is its sole occupant — never a shared user tab. Returns {@code null} if the pane + * is already gone. + * + * @param tabPaneCount panes in the tab, or {@code -1} if it could not be determined + */ + public record PaneLocation(String tabId, String workspaceId, int tabPaneCount) { + } + + /** Locate a pane's tab (with the tab's pane count), or {@code null} if the pane is gone. */ + public PaneLocation locatePane(String paneId) { + JsonNode pane; + try { + pane = herdr.call("pane.get", Map.of("pane_id", paneId)).path("pane"); + } catch (HerdrException e) { + log.debug("pane.get({}) — pane already gone: {}", paneId, e.getMessage()); + return null; + } + String tabId = pane.path("tab_id").asText(null); + String workspaceId = pane.path("workspace_id").asText(null); + if (tabId == null || tabId.isBlank()) return null; + return new PaneLocation(tabId, workspaceId, tabPaneCount(workspaceId, tabId)); + } + + private int tabPaneCount(String workspaceId, String tabId) { + if (workspaceId == null || workspaceId.isBlank()) return -1; + try { + for (JsonNode t : herdr.call("tab.list", Map.of("workspace_id", workspaceId)).path("tabs")) { + if (tabId.equals(t.path("tab_id").asText())) return t.path("pane_count").asInt(-1); + } + } catch (HerdrException e) { + log.debug("tab.list({}) failed while sizing tab {}: {}", workspaceId, tabId, e.getMessage()); + } + return -1; + } + + /** True when a herdr error means the target no longer exists (safe to treat as done). */ + private static boolean isAlreadyGone(HerdrException e) { + String code = e.code(); + return code != null && code.endsWith("_not_found"); + } +} diff --git a/bridged/src/main/java/dev/ltms/bridged/rest/BridgedApp.java b/bridged/src/main/java/dev/ltms/bridged/rest/BridgedApp.java index a6f240b..4f08fda 100644 --- a/bridged/src/main/java/dev/ltms/bridged/rest/BridgedApp.java +++ b/bridged/src/main/java/dev/ltms/bridged/rest/BridgedApp.java @@ -103,6 +103,7 @@ public final class BridgedApp { m.put("terminalId", a.terminalId()); m.put("paneId", a.paneId()); m.put("workspaceId", a.workspaceId()); + m.put("tabId", a.tabId()); m.put("sessionId", a.sessionId()); m.put("agentType", a.agentType()); m.put("status", a.status().name().toLowerCase()); diff --git a/bridged/src/main/java/dev/ltms/bridged/worker/WorkerService.java b/bridged/src/main/java/dev/ltms/bridged/worker/WorkerService.java index 43d53c6..194c897 100644 --- a/bridged/src/main/java/dev/ltms/bridged/worker/WorkerService.java +++ b/bridged/src/main/java/dev/ltms/bridged/worker/WorkerService.java @@ -4,12 +4,18 @@ import dev.ltms.bridged.config.BridgedConfig; import dev.ltms.bridged.guard.SubscriptionGuard; import dev.ltms.bridged.herdr.Agent; import dev.ltms.bridged.herdr.AgentControl; +import dev.ltms.bridged.herdr.HerdrException; +import dev.ltms.bridged.herdr.Tab; +import dev.ltms.bridged.herdr.Workspace; +import dev.ltms.bridged.herdr.WorkspaceControl; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.security.SecureRandom; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicLong; import java.util.function.Function; /** @@ -21,19 +27,33 @@ import java.util.function.Function; * touching herdr, and only then {@code agent.start}. A worker's base_url lives in the * env map handed to herdr and nowhere else; {@code bridged}'s own environment is never * mutated. + * + *

Placement: in the default {@code tab} policy a worker lands in its own tab inside a + * dedicated worker space (found-or-created once, then shared), so workers never split or + * clutter the user's real work spaces. Teardown removes the worker's pane and its + * now-empty tab, tolerating an already-gone worker so a repeated DELETE is harmless. */ public final class WorkerService { private static final Logger log = LoggerFactory.getLogger(WorkerService.class); + /** herdr rejects a duplicate agent {@code name}; we retry a bumped name this many times. */ + private static final int NAME_RETRIES = 8; + private final AgentControl agents; + private final WorkspaceControl spaces; private final SubscriptionGuard guard; private final BridgedConfig.Worker cfg; private final Function env; // host env lookup (injectable for tests) + private final AtomicLong nameSeq = new AtomicLong(); // per-worker counter (also the tab #) + // Per-process token mixed into each worker name so a fresh process (nameSeq back at 0) + // cannot collide with same-profile workers that outlived a restart. See startUniquelyNamed. + private final String nameNonce = String.format("%06x", new SecureRandom().nextInt(1 << 24)); - public WorkerService(AgentControl agents, SubscriptionGuard guard, + public WorkerService(AgentControl agents, WorkspaceControl spaces, SubscriptionGuard guard, BridgedConfig.Worker cfg, Function env) { this.agents = agents; + this.spaces = spaces; this.guard = guard; this.cfg = cfg; this.env = env; @@ -51,22 +71,131 @@ public final class WorkerService { String token = env.apply(cfg.tokenEnv()); putIfPresent(workerEnv, "ANTHROPIC_AUTH_TOKEN", token); - String name = "claude"; - List argv = cfg.argv(); - log.info("spawning worker profile={} base_url={} argv={}", cfg.profile(), baseUrl, argv); - Agent worker = agents.start(name, argv, workerEnv); + return cfg.tabPlacement() ? spawnInTab(workerEnv) : spawnAsPane(workerEnv); + } + + /** Dedicated worker space → own tab → drop the placeholder shell so only the worker remains. */ + private Agent spawnInTab(Map workerEnv) { + Workspace space = spaces.ensureWorkspace(cfg.workspace()); + Tab.Created tab = spaces.createTab(space.workspaceId()); + log.info("spawning worker profile={} base_url={} space={} tab={}", + cfg.profile(), cfg.baseUrl(), space.workspaceId(), tab.tab().tabId()); + + Started started; + try { + started = startUniquelyNamed(workerEnv, tab.tab().tabId()); + } catch (RuntimeException e) { + // The worker never started — don't leave the tab we just created orphaned. + // Best-effort cleanup; never let it mask the real spawn failure. + try { + spaces.closeTab(tab.tab().tabId()); + } catch (RuntimeException cleanup) { + log.warn("failed to close orphaned tab {} after spawn error: {}", + tab.tab().tabId(), cleanup.getMessage()); + } + throw e; + } + + // The worker is LIVE now. The remaining steps are cosmetic (drop herdr's seed shell + // so the tab holds only the worker; label the tab). They must not fail the spawn or + // orphan the running worker — 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; worker tab may hold an extra pane", + tab.tab().tabId()); + } + tidy("label tab " + tab.tab().tabId(), + () -> spaces.renameTab(tab.tab().tabId(), cfg.renderTabLabel(started.seq()))); + log.info("worker started pane={} tab={} terminal={}", + started.agent().paneId(), started.agent().tabId(), started.agent().terminalId()); + return started.agent(); + } + + /** Run a best-effort post-start cleanup step, logging (not throwing) on failure. */ + private void tidy(String what, Runnable step) { + try { + step.run(); + } catch (RuntimeException e) { + log.warn("post-start step failed ({}) — worker is running regardless: {}", what, e.getMessage()); + } + } + + /** Legacy placement: herdr splits the currently-focused tab. */ + private Agent spawnAsPane(Map workerEnv) { + log.info("spawning worker (pane placement) profile={} base_url={} argv={}", + cfg.profile(), cfg.baseUrl(), cfg.argv()); + Agent worker = startUniquelyNamed(workerEnv, null).agent(); log.info("worker started pane={} terminal={}", worker.paneId(), worker.terminalId()); return worker; } + /** A started worker together with the sequence its unique name/label used. */ + private record Started(Agent agent, long seq) { + } + + /** + * Start the worker under a unique herdr agent name. herdr requires each running + * agent's {@code name} to be distinct (a 2nd {@code name:"claude"} fails + * {@code agent_name_taken}) — the exact case that makes multiple workers useful. The name + * is {@code claude---}: {@code seq} distinguishes workers within this + * process, and the per-process {@code nonce} keeps a fresh process (whose {@code seq} + * restarts at 0) from colliding with same-profile workers that outlived a restart. The + * retry is a belt-and-braces backstop for the astronomically unlikely nonce+seq clash; + * the name is a label only — herdr detects kind and status from terminal output, not it. + */ + private Started startUniquelyNamed(Map workerEnv, String tabId) { + HerdrException last = null; + for (int attempt = 0; attempt < NAME_RETRIES; attempt++) { + long seq = nameSeq.incrementAndGet(); + String name = "claude-" + cfg.profile() + "-" + nameNonce + "-" + seq; + try { + return new Started(agents.start(name, cfg.argv(), workerEnv, tabId), seq); + } catch (HerdrException e) { + if (!"agent_name_taken".equals(e.code())) throw e; + log.debug("worker name '{}' taken, retrying", name); + last = e; + } + } + throw last; + } + /** All herdr-tracked agents — discovery for "what workers exist". */ public List list() { return agents.list(); } - /** Tear a worker down by pane id. */ + /** + * Tear a worker down by pane id: close the pane, and close its tab only when the + * worker is that tab's sole occupant. The single-pane check is what makes this safe + * regardless of how the worker was placed (or a placement-config change across a restart): + * a pane-placement worker sitting in one of the user's shared tabs has siblings, so its + * tab is never closed — we only ever remove a tab we created to hold one worker. + * + *

Resolves the tab from the pane before closing it. An already-gone pane/tab + * (repeated DELETE, crashed worker) is treated as success; any other failure propagates so + * a genuinely failed teardown is not reported as done. + */ public void stop(String paneId) { - agents.close(paneId); + WorkspaceControl.PaneLocation loc = cfg.tabPlacement() ? spaces.locatePane(paneId) : null; + try { + agents.close(paneId); + } catch (HerdrException e) { + if (!isAlreadyGone(e)) throw e; + log.debug("pane.close({}) ignored — already gone: {}", paneId, e.getMessage()); + } + if (loc != null && loc.tabPaneCount() == 1) { + spaces.closeTab(loc.tabId()); + } else if (loc != null) { + log.debug("not closing tab {} — it holds {} panes (not a dedicated worker tab)", + loc.tabId(), loc.tabPaneCount()); + } + } + + /** True when a herdr error means the target is already gone (safe to treat as done). */ + private static boolean isAlreadyGone(HerdrException e) { + return e.code() != null && e.code().endsWith("_not_found"); } private static void putIfPresent(Map m, String k, String v) { 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 b0ed532..1e1b548 100644 --- a/bridged/src/test/java/dev/ltms/bridged/herdr/FakeHerdr.java +++ b/bridged/src/test/java/dev/ltms/bridged/herdr/FakeHerdr.java @@ -19,12 +19,46 @@ public final class FakeHerdr implements HerdrClient { private final ObjectMapper mapper = new ObjectMapper(); public final List calls = new ArrayList<>(); private boolean healthy = true; + private final List extraWorkspaces = new ArrayList<>(); + private int agentNameTakenFor = 0; + private int workerTabPaneCount = 1; + private String paneCloseErrorCode = null; public FakeHerdr healthy(boolean h) { this.healthy = h; return this; } + /** Reject the first {@code n} {@code agent.start} calls with {@code agent_name_taken}. */ + public FakeHerdr agentNameTakenTimes(int n) { + this.agentNameTakenFor = 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; + return this; + } + + /** Make {@code pane.close} fail with this herdr error code. */ + public FakeHerdr paneCloseFailsWith(String code) { + this.paneCloseErrorCode = code; + return this; + } + + private long callCount(String method) { + return calls.stream().filter(c -> c.method().equals(method)).count(); + } + + /** Seed an additional workspace into {@code workspace.list} (e.g. a pre-existing worker space). */ + public FakeHerdr withWorkspace(String id, String label) { + extraWorkspaces.add(("{\"workspace_id\":\"%s\",\"label\":\"%s\",\"focused\":false," + + "\"pane_count\":1,\"active_tab_id\":\"%s:t1\",\"agent_status\":\"unknown\"}") + .formatted(id, label, id)); + return this; + } + public boolean called(String method) { return calls.stream().anyMatch(c -> c.method().equals(method)); } @@ -42,22 +76,60 @@ public final class FakeHerdr implements HerdrClient { return switch (method) { case "ping" -> mapper.readTree( "{\"type\":\"pong\",\"version\":\"0.7.0\",\"protocol\":14}"); - case "workspace.list" -> mapper.readTree(""" + case "workspace.list" -> mapper.readTree((""" {"type":"workspace_list","workspaces":[ {"workspace_id":"w1","label":"dev-mgnl","focused":true,"pane_count":7,"agent_status":"unknown"}, - {"workspace_id":"w2","label":"ltms","focused":false,"pane_count":5,"agent_status":"done"}]}"""); + {"workspace_id":"w2","label":"ltms","focused":false,"pane_count":5,"agent_status":"done"}%s]}""") + .formatted(extraWorkspaces.isEmpty() ? "" : "," + String.join(",", extraWorkspaces))); case "agent.list" -> mapper.readTree(""" {"type":"agent_list","agents":[ {"terminal_id":"term_a","agent":"claude","agent_status":"idle", "agent_session":{"kind":"id","value":"sess-1111"}, "workspace_id":"w2","tab_id":"w2:t7","pane_id":"w2:p7"}]}"""); - case "agent.start" -> mapper.readTree(""" + case "agent.start" -> { + if (callCount("agent.start") <= agentNameTakenFor) { + throw new HerdrException( + "herdr error [agent_name_taken]: agent name already used", + "agent_name_taken", null); + } + yield mapper.readTree(""" {"type":"agent_started","agent":{ "terminal_id":"term_new","name":"claude","agent_status":"unknown", - "workspace_id":"w2","tab_id":"w2:t9","pane_id":"w2:pZ"}}"""); - case "pane.close" -> mapper.readTree("{\"type\":\"ok\"}"); + "workspace_id":"w9","tab_id":"w9:t2","pane_id":"w9:pW"}}"""); + } + 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(""" + {"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"}}"""); + case "tab.rename" -> mapper.readTree(""" + {"type":"tab_info","tab":{"tab_id":"w9:t2","workspace_id":"w9", + "label":"worker: ltms-local","pane_count":1}}"""); + case "tab.list" -> mapper.readTree((""" + {"type":"tab_list","tabs":[ + {"tab_id":"w9:t1","workspace_id":"w9","label":"1","pane_count":1}, + {"tab_id":"w9:t2","workspace_id":"w9","label":"worker: ltms-local","pane_count":%d}]}""") + .formatted(workerTabPaneCount)); + case "tab.close" -> mapper.readTree("{\"type\":\"ok\"}"); + case "pane.get" -> mapper.readTree(""" + {"type":"pane_info","pane":{"pane_id":"w9:pW","workspace_id":"w9", + "tab_id":"w9:t2","agent_status":"idle"}}"""); + case "pane.close" -> { + if (paneCloseErrorCode != null) { + throw new HerdrException("herdr error [" + paneCloseErrorCode + "]: pane.close failed", + paneCloseErrorCode, null); + } + yield mapper.readTree("{\"type\":\"ok\"}"); + } default -> throw new HerdrException("fake has no canned response for " + method); }; + } catch (HerdrException e) { + throw e; // intentional protocol errors (e.g. agent_name_taken) propagate with their code } catch (Exception e) { throw new HerdrException("fake decode failed for " + method, e); } diff --git a/bridged/src/test/java/dev/ltms/bridged/herdr/WorkspacePlacementContractTest.java b/bridged/src/test/java/dev/ltms/bridged/herdr/WorkspacePlacementContractTest.java new file mode 100644 index 0000000..c4a2cb9 --- /dev/null +++ b/bridged/src/test/java/dev/ltms/bridged/herdr/WorkspacePlacementContractTest.java @@ -0,0 +1,88 @@ +package dev.ltms.bridged.herdr; + +import com.fasterxml.jackson.databind.JsonNode; +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 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 + * throwaway space that is fully removed at the end. + * + *

Tagged {@code contract}; run with {@code mvn test -Pcontract}. + */ +@Tag("contract") +class WorkspacePlacementContractTest { + + private static final String LABEL = "__bridged_contract__"; + + private boolean noSocket() { + return !Files.exists(UnixSocketHerdrClient.defaultSocketPath()); + } + + private int paneCount(HerdrClient herdr, String workspaceId, String tabId) { + JsonNode tabs = herdr.call("tab.list", Map.of("workspace_id", workspaceId)).path("tabs"); + for (JsonNode t : tabs) { + if (tabId.equals(t.path("tab_id").asText())) return t.path("pane_count").asInt(-1); + } + return -1; + } + + @Test + 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); + assertNotNull(space.workspaceId()); + // 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()); + try { + // The worker landed in its dedicated tab in the worker space. + assertEquals(tab.tab().tabId(), worker.tabId()); + assertEquals(space.workspaceId(), worker.workspaceId()); + + // Drop the seed shell; the tab now holds exactly the worker pane. + agents.close(tab.rootPaneId()); + 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"); + + // Teardown resolves the tab from the pane, and sees it holds exactly one pane. + WorkspaceControl.PaneLocation loc = spaces.locatePane(worker.paneId()); + assertNotNull(loc); + assertEquals(tab.tab().tabId(), loc.tabId()); + assertEquals(1, loc.tabPaneCount(), "worker 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"); + + // Remove the throwaway space entirely so the test leaves no residue. + herdr.call("workspace.close", Map.of("workspace_id", space.workspaceId())); + assertTrue(spaces.findByLabel(LABEL).isEmpty(), "throwaway worker space must be gone"); + } + } +} 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 25221ab..7517420 100644 --- a/bridged/src/test/java/dev/ltms/bridged/rest/BridgedAppTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/rest/BridgedAppTest.java @@ -6,6 +6,7 @@ import dev.ltms.bridged.config.BridgedConfig; import dev.ltms.bridged.guard.SubscriptionGuard; import dev.ltms.bridged.herdr.AgentControl; import dev.ltms.bridged.herdr.FakeHerdr; +import dev.ltms.bridged.herdr.WorkspaceControl; import dev.ltms.bridged.worker.WorkerService; import io.javalin.Javalin; import org.junit.jupiter.api.AfterEach; @@ -24,7 +25,8 @@ import static org.junit.jupiter.api.Assertions.*; /** * REST acceptance tests — the feature contract over plain HTTP with a fake herdr, no * live daemon and no Claude in the loop. This is the surface later MCP tools match by - * parity, and where the subscription boundary is proven at the API edge. + * parity, and where the subscription boundary and worker placement are proven at the + * API edge. */ class BridgedAppTest { @@ -38,10 +40,15 @@ class BridgedAppTest { } private int start(FakeHerdr herdr, String workerBaseUrl, Set allow) { + return start(herdr, workerBaseUrl, allow, "tab"); + } + + private int start(FakeHerdr herdr, String workerBaseUrl, Set allow, String placement) { BridgedConfig.Worker wcfg = new BridgedConfig.Worker( - "ltms-local", workerBaseUrl, "coder", null, "BRIDGED_WORKER_TOKEN", null); + "ltms-local", workerBaseUrl, "coder", null, "BRIDGED_WORKER_TOKEN", null, + placement, "bridged-workers", "worker: {profile} #{n}"); WorkerService workers = new WorkerService( - new AgentControl(herdr), new SubscriptionGuard(allow), wcfg, + new AgentControl(herdr), new WorkspaceControl(herdr), new SubscriptionGuard(allow), wcfg, k -> "BRIDGED_WORKER_TOKEN".equals(k) ? "tok-abc" : null); app = new BridgedApp(herdr, workers).build().start("127.0.0.1", 0); return app.port(); @@ -61,6 +68,11 @@ class BridgedAppTest { return http.send(b.build(), HttpResponse.BodyHandlers.ofString()); } + @SuppressWarnings("unchecked") + private Map params(FakeHerdr herdr, String method) { + return (Map) herdr.lastCall(method).params(); + } + @Test void healthzOkWhenHerdrAnswers() throws Exception { int port = startHealthy(); @@ -98,22 +110,69 @@ class BridgedAppTest { } @Test - void spawnWorkerInjectsBaseUrlAndReturns201() throws Exception { + void spawnWorkerLandsInOwnTabInWorkerSpaceAndInjectsBaseUrl() throws Exception { FakeHerdr herdr = new FakeHerdr(); int port = start(herdr, "http://gx00.gw:8000", Set.of("gx00.gw")); HttpResponse res = req(port, "POST", "/workers"); assertEquals(201, res.statusCode()); - assertEquals("w2:pZ", mapper.readTree(res.body()).get("paneId").asText()); + JsonNode body = mapper.readTree(res.body()); + assertEquals("w9:pW", body.get("paneId").asText()); + assertEquals("w9:t2", body.get("tabId").asText()); - // The proof: agent.start carried ANTHROPIC_BASE_URL in its env map. + // Subscription boundary: agent.start carried base_url + token in its env map. + Map start = params(herdr, "agent.start"); @SuppressWarnings("unchecked") - Map params = (Map) herdr.lastCall("agent.start").params(); - @SuppressWarnings("unchecked") - Map env = (Map) params.get("env"); + Map env = (Map) start.get("env"); assertEquals("http://gx00.gw:8000", env.get("ANTHROPIC_BASE_URL")); assertEquals("tok-abc", env.get("ANTHROPIC_AUTH_TOKEN")); - assertEquals(List.of("claude"), params.get("argv")); + 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. + 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("worker: ltms-local #1", params(herdr, "tab.rename").get("label"), + "tab label carries the worker number so siblings stay distinct"); + } + + @Test + void spawnWorkerReusesExistingWorkerSpace() throws Exception { + // A space labelled "bridged-workers" already exists → no second workspace.create. + FakeHerdr herdr = new FakeHerdr().withWorkspace("w9", "bridged-workers"); + int port = start(herdr, "http://gx00.gw:8000", Set.of("gx00.gw")); + + assertEquals(201, req(port, "POST", "/workers").statusCode()); + assertFalse(herdr.called("workspace.create"), "existing worker space must be reused, not recreated"); + assertTrue(herdr.called("tab.create"), "a fresh tab is still created for the worker"); + } + + @Test + @SuppressWarnings("unchecked") + void spawnRetriesUnderAFreshNameWhenAgentNameTaken() throws Exception { + // herdr rejects a duplicate agent name; the service must bump and retry. + FakeHerdr herdr = new FakeHerdr().agentNameTakenTimes(2); + int port = start(herdr, "http://gx00.gw:8000", Set.of("gx00.gw")); + + assertEquals(201, req(port, "POST", "/workers").statusCode()); + List names = herdr.calls.stream() + .filter(c -> c.method().equals("agent.start")) + .map(c -> ((Map) c.params()).get("name").toString()) + .toList(); + assertEquals(3, names.size(), "2 rejected + 1 success"); + assertEquals(3, Set.copyOf(names).size(), "each attempt must use a distinct name"); + } + + @Test + void spawnClosesTheCreatedTabWhenTheWorkerNeverStarts() throws Exception { + // Every agent.start attempt is rejected → spawn fails; the tab we created must not leak. + FakeHerdr herdr = new FakeHerdr().agentNameTakenTimes(99); + int port = start(herdr, "http://gx00.gw:8000", Set.of("gx00.gw")); + + assertEquals(500, req(port, "POST", "/workers").statusCode()); + assertTrue(herdr.called("tab.create"), "a tab was created before the failed start"); + assertEquals("w9:t2", params(herdr, "tab.close").get("tab_id"), "orphaned tab must be closed"); } @Test @@ -126,13 +185,71 @@ class BridgedAppTest { assertEquals(403, res.statusCode()); assertEquals("subscription_boundary", mapper.readTree(res.body()).get("error").asText()); assertFalse(herdr.called("agent.start"), "guard must stop the spawn before herdr"); + assertFalse(herdr.called("workspace.create"), "guard must stop before provisioning a space"); } @Test - void stopWorkerClosesPane() throws Exception { + void panePlacementSplitsFocusedTabWithoutADedicatedSpace() throws Exception { + FakeHerdr herdr = new FakeHerdr(); + int port = start(herdr, "http://gx00.gw:8000", Set.of("gx00.gw"), "pane"); + + assertEquals(201, req(port, "POST", "/workers").statusCode()); + Map start = params(herdr, "agent.start"); + assertFalse(start.containsKey("tab_id"), "pane placement must not target a tab"); + assertFalse(herdr.called("workspace.create"), "pane placement uses no dedicated space"); + assertFalse(herdr.called("tab.create")); + } + + @Test + void stopWorkerClosesPaneAndItsTab() throws Exception { FakeHerdr herdr = new FakeHerdr(); int port = start(herdr, "http://gx00.gw:8000", Set.of("gx00.gw")); - assertEquals(204, req(port, "DELETE", "/workers/w2:pZ").statusCode()); + + assertEquals(204, req(port, "DELETE", "/workers/w9:pW").statusCode()); assertTrue(herdr.called("pane.close")); + // Tab resolved from the pane (pane.get), then closed. + assertEquals("w9:t2", params(herdr, "tab.close").get("tab_id")); + } + + @Test + void stopWorkerInPanePlacementClosesOnlyThePane() throws Exception { + FakeHerdr herdr = new FakeHerdr(); + int port = start(herdr, "http://gx00.gw:8000", Set.of("gx00.gw"), "pane"); + + assertEquals(204, req(port, "DELETE", "/workers/w9:pW").statusCode()); + assertTrue(herdr.called("pane.close")); + assertFalse(herdr.called("tab.close"), "pane placement owns no tab to close"); + assertFalse(herdr.called("pane.get"), "no tab resolution in pane placement"); + } + + @Test + void stopNeverClosesATabThatHoldsOtherPanes() throws Exception { + // The worker's tab has 2 panes (e.g. a pane-placement worker sharing a user tab). + FakeHerdr herdr = new FakeHerdr().withWorkerTabPaneCount(2); + int port = start(herdr, "http://gx00.gw:8000", Set.of("gx00.gw")); + + assertEquals(204, req(port, "DELETE", "/workers/w9:pW").statusCode()); + assertTrue(herdr.called("pane.close"), "the worker's own pane is still closed"); + assertFalse(herdr.called("tab.close"), "must not close a tab that holds the user's other panes"); + } + + @Test + void stopReportsFailureWhenPaneCloseFailsForARealReason() throws Exception { + FakeHerdr herdr = new FakeHerdr().paneCloseFailsWith("herdr_busy"); + int port = start(herdr, "http://gx00.gw:8000", Set.of("gx00.gw")); + + // A genuine teardown failure must surface, not be reported as a successful 204. + assertEquals(500, req(port, "DELETE", "/workers/w9:pW").statusCode()); + assertFalse(herdr.called("tab.close"), "tab is not removed when the pane close failed"); + } + + @Test + void stopToleratesAnAlreadyGonePane() throws Exception { + FakeHerdr herdr = new FakeHerdr().paneCloseFailsWith("pane_not_found"); + int port = start(herdr, "http://gx00.gw:8000", Set.of("gx00.gw")); + + // Already-gone is success; the (now-empty) tab is still cleaned up. + assertEquals(204, req(port, "DELETE", "/workers/w9:pW").statusCode()); + assertTrue(herdr.called("tab.close")); } } diff --git a/wiki b/wiki index aa93ed5..8c37bb7 160000 --- a/wiki +++ b/wiki @@ -1 +1 @@ -Subproject commit aa93ed51c555c19f002df48444fd41894cb5d6e2 +Subproject commit 8c37bb71c1289efa6d277b9b8e1b7b89f949b986