fix: honor per-role command/platform/model when spawning through AgentRuntime

src/core/acpx-resolve-agent.test.ts

@@ -0,0 +1,103 @@
+import { describe, expect, test } from "bun:test";
+import { AcpxRuntime, KNOWN_ACPX_AGENTS, PLATFORM_TO_AGENT } from "./acpx-runtime.js";
+import type { AgentConfig } from "./agent-runtime.js";
+
+function makeConfig(overrides?: Partial<AgentConfig>): AgentConfig {
+  return {
+    role: "coder",
+    command: "echo hello",
+    cwd: "/tmp",
+    ...overrides,
+  };
+}
+
+describe("PLATFORM_TO_AGENT mapping", () => {
+  test("claude-code maps to claude", () => {
+    expect(PLATFORM_TO_AGENT["claude-code"]).toBe("claude");
+  });
+
+  test("codex maps to codex", () => {
+    expect(PLATFORM_TO_AGENT.codex).toBe("codex");
+  });
+
+  test("gemini maps to gemini", () => {
+    expect(PLATFORM_TO_AGENT.gemini).toBe("gemini");
+  });
+
+  test("custom falls back to codex", () => {
+    expect(PLATFORM_TO_AGENT.custom).toBe("codex");
+  });
+
+  test("all platform values map to known agents", () => {
+    for (const [_platform, agent] of Object.entries(PLATFORM_TO_AGENT)) {
+      expect(KNOWN_ACPX_AGENTS.has(agent)).toBe(true);
+    }
+  });
+});
+
+describe("KNOWN_ACPX_AGENTS", () => {
+  test("contains expected agents", () => {
+    expect(KNOWN_ACPX_AGENTS.has("claude")).toBe(true);
+    expect(KNOWN_ACPX_AGENTS.has("codex")).toBe(true);
+    expect(KNOWN_ACPX_AGENTS.has("gemini")).toBe(true);
+    expect(KNOWN_ACPX_AGENTS.has("pi")).toBe(true);
+    expect(KNOWN_ACPX_AGENTS.has("openclaw")).toBe(true);
+  });
+
+  test("does not contain unknown agents", () => {
+    expect(KNOWN_ACPX_AGENTS.has("gpt")).toBe(false);
+    expect(KNOWN_ACPX_AGENTS.has("echo")).toBe(false);
+  });
+});
+
+describe("AcpxRuntime.resolveAgent", () => {
+  test("uses platform when provided — claude-code", () => {
+    const rt = new AcpxRuntime();
+    expect(rt.resolveAgent(makeConfig({ platform: "claude-code" }))).toBe("claude");
+  });
+
+  test("uses platform when provided — codex", () => {
+    const rt = new AcpxRuntime();
+    expect(rt.resolveAgent(makeConfig({ platform: "codex" }))).toBe("codex");
+  });
+
+  test("uses platform when provided — gemini", () => {
+    const rt = new AcpxRuntime();
+    expect(rt.resolveAgent(makeConfig({ platform: "gemini" }))).toBe("gemini");
+  });
+
+  test("platform takes precedence over command", () => {
+    const rt = new AcpxRuntime();
+    // platform says gemini, command says claude — platform wins
+    const agent = rt.resolveAgent(makeConfig({ platform: "gemini", command: "claude --flag" }));
+    expect(agent).toBe("gemini");
+  });
+
+  test("falls back to command parsing when no platform", () => {
+    const rt = new AcpxRuntime();
+    expect(rt.resolveAgent(makeConfig({ command: "claude --flag" }))).toBe("claude");
+  });
+
+  test("falls back to command parsing — strips rm prefix", () => {
+    const rt = new AcpxRuntime();
+    const config = makeConfig({
+      command: "rm -f ~/.claude/remote-settings.json; claude --dangerously-skip-permissions",
+    });
+    expect(rt.resolveAgent(config)).toBe("claude");
+  });
+
+  test("falls back to constructor default for unknown command", () => {
+    const rt = new AcpxRuntime({ agent: "codex" });
+    expect(rt.resolveAgent(makeConfig({ command: "echo hello" }))).toBe("codex");
+  });
+
+  test("falls back to constructor default when no platform and no command", () => {
+    const rt = new AcpxRuntime({ agent: "claude" });
+    expect(rt.resolveAgent(makeConfig({ command: "" }))).toBe("claude");
+  });
+
+  test("uses DEFAULT_AGENT (codex) when no constructor override and no config", () => {
+    const rt = new AcpxRuntime();
+    expect(rt.resolveAgent(makeConfig({ command: "unknown-binary" }))).toBe("codex");
+  });
+});

src/core/acpx-runtime.test.ts

@@ -8,19 +8,28 @@ const config: AgentConfig = {
   cwd: "/tmp",
 };
 
+/** Check if acpx is available (cached for the test run). */
+let _acpxAvailable: boolean | undefined;
+async function isAcpxAvailable(): Promise<boolean> {
+  if (_acpxAvailable === undefined) {
+    const rt = new AcpxRuntime();
+    _acpxAvailable = await rt.isAvailable();
+  }
+  return _acpxAvailable;
+}
+
 describe("AcpxRuntime", () => {
   test("isAvailable returns boolean", async () => {
     const rt = new AcpxRuntime();
     const available = await rt.isAvailable();
     expect(typeof available).toBe("boolean");
   });
 
-  test("isAvailable returns false when acpx not installed", async () => {
-    // acpx is unlikely to be in PATH in CI/test environments
+  test("isAvailable caches its result", async () => {
     const rt = new AcpxRuntime();
-    const available = await rt.isAvailable();
-    // We just verify it doesn't throw — the value depends on environment
-    expect(typeof available).toBe("boolean");
+    const first = await rt.isAvailable();
+    const second = await rt.isAvailable();
+    expect(first).toBe(second);
   });
 
   test("implements AgentRuntime interface", () => {
@@ -31,21 +40,30 @@ describe("AcpxRuntime", () => {
     expect(typeof rt.onIdle).toBe("function");
     expect(typeof rt.listSessions).toBe("function");
     expect(typeof rt.isAvailable).toBe("function");
+    expect(typeof rt.resolveAgent).toBe("function");
+    expect(typeof rt.discoverSessions).toBe("function");
   });
 
   test("spawn throws when acpx is not available", async () => {
-    const rt = new AcpxRuntime();
-    const skip = await rt.isAvailable();
-    if (skip) return; // acpx is actually installed, skip this test
+    const available = await isAcpxAvailable();
+    if (available) {
+      // Skip — acpx is installed in this environment
+      console.log("[SKIP] acpx is installed, skipping unavailable test");
+      return;
+    }
 
+    const rt = new AcpxRuntime();
     await expect(rt.spawn("coder", config)).rejects.toThrow("acpx is not installed");
   });
 
   test("listSessions returns empty array when unavailable", async () => {
-    const rt = new AcpxRuntime();
-    const skip = await rt.isAvailable();
-    if (skip) return;
+    const available = await isAcpxAvailable();
+    if (available) {
+      console.log("[SKIP] acpx is installed, skipping unavailable test");
+      return;
+    }
 
+    const rt = new AcpxRuntime();
     const sessions = await rt.listSessions();
     expect(sessions).toEqual([]);
   });
@@ -64,12 +82,28 @@ describe("AcpxRuntime", () => {
     });
   });
 
-  test("send does nothing for unknown session", async () => {
+  test("send does nothing for unknown session (creates reattach entry)", async () => {
     const rt = new AcpxRuntime();
-    // Should not throw when session doesn't exist (no-op)
+    // Should not throw when session doesn't exist — creates reattach entry
     await rt.send({ id: "nonexistent", role: "test", status: "running" }, "hello");
   });
 
+  test("send uses session.agent for reattach entry", async () => {
+    const rt = new AcpxRuntime({ agent: "codex" });
+    const session = {
+      id: "test-session",
+      role: "test",
+      status: "running" as const,
+      agent: "claude",
+    };
+    // This will create a reattach entry using session.agent ("claude") not the default ("codex")
+    await rt.send(session, "hello");
+    // We can't directly inspect the entry, but the session is now tracked
+    const sessions = await rt.listSessions();
+    expect(sessions).toHaveLength(1);
+    expect(sessions[0]!.id).toBe("test-session");
+  });
+
   test("constructor accepts agent option", () => {
     const rt = new AcpxRuntime({ agent: "codex" });
     expect(rt).toBeDefined();
@@ -78,10 +112,13 @@ describe("AcpxRuntime", () => {
   // Timeout bumped to 30s because acpx cold-start can take >5s when the
   // agent adapter has to be fetched via npx or when the host is under load.
   test("spawn and close work when acpx is available", async () => {
-    const rt = new AcpxRuntime();
-    const skip = !(await rt.isAvailable());
-    if (skip) return;
+    const available = await isAcpxAvailable();
+    if (!available) {
+      console.log("[SKIP] acpx not installed, skipping integration test");
+      return;
+    }
 
+    const rt = new AcpxRuntime();
     // Session names now carry a per-spawn counter and a base36 timestamp:
     // grove-<role>-<counter>-<timestamp>
     const session = await rt.spawn("test", config);
@@ -91,4 +128,43 @@ describe("AcpxRuntime", () => {
 
     await rt.close(session);
   }, 30_000);
+
+  test("spawn returns platform/model metadata when provided", async () => {
+    const available = await isAcpxAvailable();
+    if (!available) {
+      console.log("[SKIP] acpx not installed, skipping integration test");
+      return;
+    }
+
+    const rt = new AcpxRuntime();
+    const session = await rt.spawn("test", {
+      ...config,
+      platform: "codex",
+      model: "gpt-4.1",
+    });
+
+    expect(session.platform).toBe("codex");
+    expect(session.model).toBe("gpt-4.1");
+    expect(session.agent).toBe("codex");
+
+    await rt.close(session);
+  }, 30_000);
+
+  test("spawn returns agent field matching resolved backend", async () => {
+    const available = await isAcpxAvailable();
+    if (!available) {
+      console.log("[SKIP] acpx not installed, skipping integration test");
+      return;
+    }
+
+    const rt = new AcpxRuntime();
+    const session = await rt.spawn("test", {
+      ...config,
+      platform: "claude-code",
+    });
+
+    expect(session.agent).toBe("claude");
+
+    await rt.close(session);
+  }, 30_000);
 });