diff --git a/src/core/acpx-resolve-agent.test.ts b/src/core/acpx-resolve-agent.test.ts new file mode 100644 index 0000000..57227a2 --- /dev/null +++ b/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 { + 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"); + }); +}); diff --git a/src/core/acpx-runtime.test.ts b/src/core/acpx-runtime.test.ts index 5661530..5ff6ab6 100644 --- a/src/core/acpx-runtime.test.ts +++ b/src/core/acpx-runtime.test.ts @@ -8,6 +8,16 @@ const config: AgentConfig = { cwd: "/tmp", }; +/** Check if acpx is available (cached for the test run). */ +let _acpxAvailable: boolean | undefined; +async function isAcpxAvailable(): Promise { + if (_acpxAvailable === undefined) { + const rt = new AcpxRuntime(); + _acpxAvailable = await rt.isAvailable(); + } + return _acpxAvailable; +} + describe("AcpxRuntime", () => { test("isAvailable returns boolean", async () => { const rt = new AcpxRuntime(); @@ -15,12 +25,11 @@ describe("AcpxRuntime", () => { 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--- 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); }); diff --git a/src/core/acpx-runtime.ts b/src/core/acpx-runtime.ts index abb196e..76dfa9c 100644 --- a/src/core/acpx-runtime.ts +++ b/src/core/acpx-runtime.ts @@ -9,12 +9,15 @@ */ import { execSync, spawn as nodeSpawn } from "node:child_process"; -import { appendFileSync, mkdirSync } from "node:fs"; +import { createWriteStream, mkdirSync } from "node:fs"; import { join } from "node:path"; import type { AgentConfig, AgentRuntime, AgentSession } from "./agent-runtime.js"; +import { shellEscape } from "./shell-utils.js"; +import type { AgentPlatformType } from "./topology.js"; function appendLog(msg: string): void { try { + const { appendFileSync } = require("node:fs") as typeof import("node:fs"); appendFileSync("/tmp/grove-debug.log", `[${new Date().toISOString()}] ${msg}\n`); } catch { /* ignore */ @@ -24,6 +27,23 @@ function appendLog(msg: string): void { /** Default agent backend used by acpx when none is specified. */ const DEFAULT_AGENT = "codex"; +/** Known acpx agent subcommands. */ +export const KNOWN_ACPX_AGENTS: Set = new Set([ + "claude", + "codex", + "gemini", + "pi", + "openclaw", +]); + +/** Maps AgentPlatformType to the acpx agent subcommand. */ +export const PLATFORM_TO_AGENT: Record = { + "claude-code": "claude", + codex: "codex", + gemini: "gemini", + custom: "codex", // custom falls back to default +}; + interface AcpxSessionEntry { session: AgentSession; agent: string; @@ -35,6 +55,8 @@ interface AcpxSessionEntry { idleTimer: ReturnType | null; /** Active child process for the current prompt (null when idle). */ activeProc: ReturnType | null; + /** Log write stream (opened at session creation, closed on session close). */ + logStream: import("node:fs").WriteStream | null; /** Log file path for agent output (debug/streaming). */ logFile: string | null; } @@ -52,6 +74,12 @@ export class AcpxRuntime implements AgentRuntime { /** Directory for per-agent log files. */ private readonly logDir: string | undefined; + /** Cached availability result (acpx doesn't get uninstalled mid-session). */ + private cachedAvailable: boolean | undefined; + + /** Set of distinct agent backends that have been spawned (for multi-agent listSessions). */ + private readonly spawnedAgents: Set = new Set(); + constructor(options?: { agent?: string; idlePollMs?: number; logDir?: string }) { this.agent = options?.agent ?? DEFAULT_AGENT; this.idlePollMs = options?.idlePollMs ?? 5000; @@ -66,30 +94,61 @@ export class AcpxRuntime implements AgentRuntime { } async isAvailable(): Promise { + if (this.cachedAvailable !== undefined) return this.cachedAvailable; + try { const out = execSync("acpx --version", { encoding: "utf-8", stdio: "pipe" }).trim(); // acpx >=0.5.3 is required: 0.3.x uses the buggy @zed-industries/claude-agent-acp // adapter that fails session/new with "Query closed before response received". // 0.5.x switched to @agentclientprotocol/claude-agent-acp which works. const match = /^(\d+)\.(\d+)\.(\d+)/.exec(out); - if (!match) return true; // unparseable — don't block + if (!match) { + this.cachedAvailable = true; + return true; // unparseable — don't block + } const [, maj, min, patch] = match; const major = Number(maj); const minor = Number(min); const patchNum = Number(patch); - if (major > 0) return true; - if (minor > 5) return true; - if (minor === 5 && patchNum >= 3) return true; + if (major > 0 || minor > 5 || (minor === 5 && patchNum >= 3)) { + this.cachedAvailable = true; + return true; + } process.stderr.write( `[acpx-runtime] acpx ${out} is too old — grove requires acpx >=0.5.3. ` + `Upgrade with: npm install -g acpx@latest\n`, ); + this.cachedAvailable = false; return false; } catch { + this.cachedAvailable = false; return false; } } + /** + * Resolve the acpx agent backend for a spawn request. + * + * Priority: config.platform → parsed config.command → constructor default. + */ + resolveAgent(config: AgentConfig): string { + // 1. Explicit platform takes precedence + if (config.platform) { + const mapped = PLATFORM_TO_AGENT[config.platform]; + if (mapped && KNOWN_ACPX_AGENTS.has(mapped)) return mapped; + } + + // 2. Fallback: parse agent name from command string (backward compat) + if (config.command) { + const stripped = config.command.replace(/^rm\s+[^;]+;\s*/, ""); // drop leading "rm -f ~/..." hooks + const first = stripped.trim().split(/\s+/)[0] ?? ""; + if (KNOWN_ACPX_AGENTS.has(first)) return first; + } + + // 3. Constructor-level default + return this.agent; + } + async spawn(role: string, config: AgentConfig): Promise { if (!(await this.isAvailable())) { throw new Error("acpx is not installed or not in PATH"); @@ -122,20 +181,15 @@ export class AcpxRuntime implements AgentRuntime { } const mergedEnv = config.env ? { ...baseEnv, ...config.env } : baseEnv; - // Extract the agent binary name from config.command (e.g. "claude --flag" → "claude"). - // acpx takes the agent name as a subcommand; flags and the initial prompt go through - // the session creation path, not the `acpx ` argument. - // - // Only known acpx subcommands are accepted (claude/codex/gemini/pi/openclaw). Any - // other first token (including shell builtins like `echo` used in tests) falls back - // to the runtime-level default so we never pass acpx an unknown agent name. - const KNOWN_ACPX_AGENTS = new Set(["claude", "codex", "gemini", "pi", "openclaw"]); - const agent = (() => { - if (!config.command) return this.agent; - const stripped = config.command.replace(/^rm\s+[^;]+;\s*/, ""); // drop leading "rm -f ~/..." hooks - const first = stripped.trim().split(/\s+/)[0] ?? ""; - return KNOWN_ACPX_AGENTS.has(first) ? first : this.agent; - })(); + // Export platform/model as env vars so agent identity resolution picks them up + if (config.platform) mergedEnv.GROVE_AGENT_PLATFORM = config.platform; + if (config.model) mergedEnv.GROVE_AGENT_MODEL = config.model; + + // Resolve agent backend via platform → command → default chain + const agent = this.resolveAgent(config); + + // Track this agent type for multi-backend listSessions + this.spawnedAgents.add(agent); // Create a new acpx session with --approve-all (layer 1: acpx client-side auto-approve) const createCmd = `acpx --approve-all ${shellEscape(agent)} sessions new --name ${shellEscape(sessionName)}`; @@ -156,8 +210,23 @@ export class AcpxRuntime implements AgentRuntime { // Non-fatal — some agents may not support set-mode (claude, gemini) } - const session: AgentSession = { id, role, status: "running" }; + const session: AgentSession = { + id, + role, + status: "running", + platform: config.platform, + model: config.model, + agent, + }; const logFile = this.logDir ? join(this.logDir, `${role}-${counter}.log`) : null; + let logStream: import("node:fs").WriteStream | null = null; + if (logFile) { + logStream = createWriteStream(logFile, { flags: "a" }); + logStream.write( + `[${new Date().toISOString()}] === Session ${sessionName} (role: ${role}, agent: ${agent}, platform: ${config.platform ?? "unset"}, model: ${config.model ?? "unset"}) ===\n`, + ); + } + const entry: AcpxSessionEntry = { session, agent, @@ -168,20 +237,11 @@ export class AcpxRuntime implements AgentRuntime { outputCallbacks: [], idleTimer: null, activeProc: null, + logStream, logFile, }; this.sessions.set(id, entry); - // Write initial log header - if (logFile) { - const header = `[${new Date().toISOString()}] === Session ${sessionName} (role: ${role}) ===\n`; - try { - appendFileSync(logFile, header); - } catch { - /* ignore */ - } - } - // Send initial prompt unless this role waits for push (e.g., reviewer waits for coder) if (!config.waitForPush) { const initialMessage = config.goal ?? config.prompt; @@ -228,16 +288,9 @@ RULES ABOUT grove_done: ${message}`; // Log the outgoing prompt - if (entry.logFile) { + if (entry.logStream) { const ts = new Date().toISOString(); - try { - appendFileSync( - entry.logFile, - `\n[${ts}] >>> PROMPT >>>\n${message}\n[${ts}] <<< END PROMPT <<<\n`, - ); - } catch { - /* ignore */ - } + entry.logStream.write(`\n[${ts}] >>> PROMPT >>>\n${message}\n[${ts}] <<< END PROMPT <<<\n`); } appendLog( @@ -265,18 +318,12 @@ ${message}`; ); }); - // Stream stdout to output callbacks + log file + // Stream stdout to output callbacks + log stream child.stdout?.on("data", (chunk: Buffer) => { const text = chunk.toString(); - // Write to log file - if (entry.logFile) { - try { - appendFileSync(entry.logFile, text); - } catch { - /* ignore */ - } + if (entry.logStream) { + entry.logStream.write(text); } - // Forward to output callbacks for (const cb of entry.outputCallbacks) { try { cb(text); @@ -286,14 +333,10 @@ ${message}`; } }); - // Capture stderr to log file + // Capture stderr to log stream child.stderr?.on("data", (chunk: Buffer) => { - if (entry.logFile) { - try { - appendFileSync(entry.logFile, `[stderr] ${chunk.toString()}`); - } catch { - /* ignore */ - } + if (entry.logStream) { + entry.logStream.write(`[stderr] ${chunk.toString()}`); } }); @@ -303,12 +346,8 @@ ${message}`; const ts = new Date().toISOString(); if (code === 0) { entry.session = { ...entry.session, status: "idle" }; - if (entry.logFile) { - try { - appendFileSync(entry.logFile, `\n[${ts}] === IDLE (exit 0) ===\n`); - } catch { - /* ignore */ - } + if (entry.logStream) { + entry.logStream.write(`\n[${ts}] === IDLE (exit 0) ===\n`); } for (const cb of entry.idleCallbacks) { try { @@ -319,12 +358,8 @@ ${message}`; } } else { entry.session = { ...entry.session, status: "crashed" }; - if (entry.logFile) { - try { - appendFileSync(entry.logFile, `\n[${ts}] === CRASHED (exit ${code}) ===\n`); - } catch { - /* ignore */ - } + if (entry.logStream) { + entry.logStream.write(`\n[${ts}] === CRASHED (exit ${code}) ===\n`); } } }); @@ -332,12 +367,8 @@ ${message}`; child.on("error", (err) => { entry.activeProc = null; entry.session = { ...entry.session, status: "crashed" }; - if (entry.logFile) { - try { - appendFileSync(entry.logFile, `\n[ERROR] ${err.message}\n`); - } catch { - /* ignore */ - } + if (entry.logStream) { + entry.logStream.write(`\n[ERROR] ${err.message}\n`); } }); } @@ -352,9 +383,12 @@ ${message}`; appendLog( `[acpx.send] sessionId=${session.id} NOT in sessions map → creating reattach entry`, ); + // Use session metadata if available, fall back to runtime default + const agent = session.agent ?? this.agent; + this.spawnedAgents.add(agent); entry = { session, - agent: this.agent, + agent, sessionName: session.id, cwd: process.cwd(), env: { ...process.env }, @@ -362,8 +396,12 @@ ${message}`; outputCallbacks: [], idleTimer: null, activeProc: null, + logStream: null, logFile: this.logDir ? join(this.logDir, `${session.role}-reattach.log`) : null, }; + if (entry.logFile) { + entry.logStream = createWriteStream(entry.logFile, { flags: "a" }); + } this.sessions.set(session.id, entry); } else { appendLog( @@ -400,6 +438,11 @@ ${message}`; if (entry) { entry.session = { ...entry.session, status: "stopped" }; + // Close log stream + if (entry.logStream) { + entry.logStream.end(); + entry.logStream = null; + } } this.sessions.delete(session.id); } @@ -423,39 +466,66 @@ ${message}`; entry.outputCallbacks.push(callback); } + /** + * List sessions tracked in-memory. For discovering orphaned sessions + * from previous runs, use `discoverSessions()`. + */ async listSessions(): Promise { + // Short-circuit: return in-memory sessions if we have any tracked + if (this.sessions.size > 0) { + return [...this.sessions.values()].map((e) => e.session); + } + + // No in-memory sessions — fall back to discovery + return this.discoverSessions(); + } + + /** + * Discover sessions across all known agent backends via the acpx CLI. + * Used for reattach scenarios where sessions may exist from a prior run. + */ + async discoverSessions(): Promise { if (!(await this.isAvailable())) { return []; } - try { - const output = execSync(`acpx ${shellEscape(this.agent)} sessions list`, { - encoding: "utf-8", - stdio: "pipe", - }); + // Query all agents: the default + any that have been spawned + const agentsToQuery = new Set([this.agent, ...this.spawnedAgents, ...KNOWN_ACPX_AGENTS]); + const result: AgentSession[] = []; + const seenIds = new Set(); - const lines = output.trim().split("\n").filter(Boolean); - const result: AgentSession[] = []; - - for (const entry of this.sessions.values()) { - result.push(entry.session); - } + // Include all in-memory sessions first + for (const entry of this.sessions.values()) { + result.push(entry.session); + seenIds.add(entry.session.id); + } - for (const line of lines) { - // acpx output: UUID\tname\tpath\ttimestamp (tab-separated) - const fields = line.split("\t"); - const name = (fields[1] ?? line).trim(); - const isClosed = line.includes("[closed]"); - if (name.startsWith("grove-") && !this.sessions.has(name) && !isClosed) { - const role = name.replace(/^grove-/, "").replace(/-\d+-.*$/, ""); - result.push({ id: name, role, status: "idle" }); + // Query each agent backend for grove sessions + for (const agentName of agentsToQuery) { + try { + const output = execSync(`acpx ${shellEscape(agentName)} sessions list`, { + encoding: "utf-8", + stdio: "pipe", + }); + + const lines = output.trim().split("\n").filter(Boolean); + for (const line of lines) { + // acpx output: UUID\tname\tpath\ttimestamp (tab-separated) + const fields = line.split("\t"); + const name = (fields[1] ?? line).trim(); + const isClosed = line.includes("[closed]"); + if (name.startsWith("grove-") && !seenIds.has(name) && !isClosed) { + const role = name.replace(/^grove-/, "").replace(/-\d+-.*$/, ""); + result.push({ id: name, role, status: "idle", agent: agentName }); + seenIds.add(name); + } } + } catch { + // Agent backend may not exist or have no sessions — continue } - - return result; - } catch { - return [...this.sessions.values()].map((e) => e.session); } + + return result; } /** Poll-based idle detection fallback. */ @@ -475,8 +545,3 @@ ${message}`; } } } - -/** Escape a string for safe use in shell commands. */ -function shellEscape(s: string): string { - return `'${s.replace(/'/g, "'\\''")}'`; -} diff --git a/src/core/agent-runtime.test.ts b/src/core/agent-runtime.test.ts index d47b8e5..2b894ed 100644 --- a/src/core/agent-runtime.test.ts +++ b/src/core/agent-runtime.test.ts @@ -87,4 +87,50 @@ describe("AgentRuntime contract (MockRuntime)", () => { const s2 = await rt.spawn("coder", config); expect(s1.id).not.toBe(s2.id); }); + + // --- Platform/model metadata propagation --- + + test("spawn propagates platform from config to session", async () => { + const rt = new MockRuntime(); + const session = await rt.spawn("coder", { ...config, platform: "claude-code" }); + expect(session.platform).toBe("claude-code"); + }); + + test("spawn propagates model from config to session", async () => { + const rt = new MockRuntime(); + const session = await rt.spawn("coder", { ...config, model: "claude-opus-4-6" }); + expect(session.model).toBe("claude-opus-4-6"); + }); + + test("spawn leaves platform/model undefined when not in config", async () => { + const rt = new MockRuntime(); + const session = await rt.spawn("coder", config); + expect(session.platform).toBeUndefined(); + expect(session.model).toBeUndefined(); + }); + + test("getSessionMetadata returns platform and model", async () => { + const rt = new MockRuntime(); + const session = await rt.spawn("coder", { + ...config, + platform: "codex", + model: "gpt-4.1", + }); + const meta = rt.getSessionMetadata(session.id); + expect(meta).toBeDefined(); + expect(meta!.platform).toBe("codex"); + expect(meta!.model).toBe("gpt-4.1"); + }); + + test("getSessionMetadata returns undefined for unknown session", () => { + const rt = new MockRuntime(); + expect(rt.getSessionMetadata("nonexistent")).toBeUndefined(); + }); + + test("spawnCalls captures platform and model in config", async () => { + const rt = new MockRuntime(); + await rt.spawn("coder", { ...config, platform: "gemini", model: "gemini-2.5" }); + expect(rt.spawnCalls[0]!.config.platform).toBe("gemini"); + expect(rt.spawnCalls[0]!.config.model).toBe("gemini-2.5"); + }); }); diff --git a/src/core/agent-runtime.ts b/src/core/agent-runtime.ts index 6c5b49a..622caa2 100644 --- a/src/core/agent-runtime.ts +++ b/src/core/agent-runtime.ts @@ -4,6 +4,8 @@ * Decouples grove from any specific agent CLI (acpx, tmux, subprocess). */ +import type { AgentPlatformType } from "./topology.js"; + /** Configuration for spawning an agent. */ export interface AgentConfig { readonly role: string; @@ -14,6 +16,10 @@ export interface AgentConfig { readonly prompt?: string | undefined; /** If true, don't send initial prompt — agent waits for push via IPC. */ readonly waitForPush?: boolean | undefined; + /** Agent platform identifier — determines which backend CLI to invoke. */ + readonly platform?: AgentPlatformType | undefined; + /** Model identifier, e.g. "claude-opus-4-6". Passed through to the runtime. */ + readonly model?: string | undefined; } /** A running agent session. */ @@ -22,6 +28,12 @@ export interface AgentSession { readonly role: string; readonly pid?: number | undefined; readonly status: "running" | "idle" | "stopped" | "crashed"; + /** Agent platform used for this session (e.g. "claude-code", "codex"). */ + readonly platform?: AgentPlatformType | undefined; + /** Model identifier used for this session. */ + readonly model?: string | undefined; + /** Agent backend name used by the runtime (e.g. "claude", "codex", "gemini"). */ + readonly agent?: string | undefined; } /** Runtime for managing agent lifecycle. */ diff --git a/src/core/index.ts b/src/core/index.ts index a3a434d..826fe26 100644 --- a/src/core/index.ts +++ b/src/core/index.ts @@ -207,7 +207,7 @@ export type { AgentSessionInfo, SessionConfig, } from "./session-orchestrator.js"; -export { SessionOrchestrator } from "./session-orchestrator.js"; +export { mergeRuntimeConfig, SessionOrchestrator } from "./session-orchestrator.js"; export type { ActiveClaimFilter, ClaimStore, diff --git a/src/core/merge-runtime-config.test.ts b/src/core/merge-runtime-config.test.ts new file mode 100644 index 0000000..b20cfe4 --- /dev/null +++ b/src/core/merge-runtime-config.test.ts @@ -0,0 +1,137 @@ +import { describe, expect, test } from "bun:test"; +import type { AgentProfile } from "./agent-profile.js"; +import { mergeRuntimeConfig } from "./session-orchestrator.js"; +import type { AgentRole } from "./topology.js"; + +function makeRole(overrides?: Partial): AgentRole { + return { name: "coder", description: "Write code", ...overrides }; +} + +function makeProfile(overrides?: Partial): AgentProfile { + return { name: "@coder", role: "coder", platform: "codex", ...overrides }; +} + +describe("mergeRuntimeConfig", () => { + // --- command --- + + test("defaults command to 'claude' when neither role nor profile set it", () => { + const result = mergeRuntimeConfig(makeRole(), undefined); + expect(result.command).toBe("claude"); + }); + + test("uses role.command when no profile", () => { + const result = mergeRuntimeConfig(makeRole({ command: "codex" }), undefined); + expect(result.command).toBe("codex"); + }); + + test("profile.command overrides role.command", () => { + const result = mergeRuntimeConfig( + makeRole({ command: "claude" }), + makeProfile({ command: "gemini" }), + ); + expect(result.command).toBe("gemini"); + }); + + test("falls back to role.command when profile has no command", () => { + const result = mergeRuntimeConfig( + makeRole({ command: "codex" }), + makeProfile({ command: undefined }), + ); + expect(result.command).toBe("codex"); + }); + + // --- platform --- + + test("platform is undefined when neither role nor profile set it", () => { + const result = mergeRuntimeConfig(makeRole(), undefined); + expect(result.platform).toBeUndefined(); + }); + + test("uses role.platform when no profile", () => { + const result = mergeRuntimeConfig(makeRole({ platform: "gemini" }), undefined); + expect(result.platform).toBe("gemini"); + }); + + test("profile.platform overrides role.platform", () => { + const result = mergeRuntimeConfig( + makeRole({ platform: "claude-code" }), + makeProfile({ platform: "codex" }), + ); + expect(result.platform).toBe("codex"); + }); + + test("falls back to role.platform when profile has no platform override", () => { + // AgentProfile.platform is required, so we test with a profile that has + // a different platform value — this tests the override path. + const result = mergeRuntimeConfig( + makeRole({ platform: "claude-code" }), + makeProfile({ platform: "claude-code" }), + ); + expect(result.platform).toBe("claude-code"); + }); + + // --- model --- + + test("model is undefined when neither role nor profile set it", () => { + const result = mergeRuntimeConfig(makeRole(), undefined); + expect(result.model).toBeUndefined(); + }); + + test("uses role.model when no profile", () => { + const result = mergeRuntimeConfig(makeRole({ model: "claude-opus-4-6" }), undefined); + expect(result.model).toBe("claude-opus-4-6"); + }); + + test("profile.model overrides role.model", () => { + const result = mergeRuntimeConfig( + makeRole({ model: "claude-opus-4-6" }), + makeProfile({ model: "gpt-4.1" }), + ); + expect(result.model).toBe("gpt-4.1"); + }); + + test("falls back to role.model when profile has no model", () => { + const result = mergeRuntimeConfig( + makeRole({ model: "claude-opus-4-6" }), + makeProfile({ model: undefined }), + ); + expect(result.model).toBe("claude-opus-4-6"); + }); + + // --- combined precedence --- + + test("full precedence chain: profile > role > default", () => { + const role = makeRole({ + command: "claude", + platform: "claude-code", + model: "claude-opus-4-6", + }); + const profile = makeProfile({ + command: "codex", + platform: "codex", + model: "gpt-4.1", + }); + const result = mergeRuntimeConfig(role, profile); + expect(result.command).toBe("codex"); + expect(result.platform).toBe("codex"); + expect(result.model).toBe("gpt-4.1"); + }); + + test("partial profile overlay: only overrides fields that are set", () => { + const role = makeRole({ + command: "claude", + platform: "claude-code", + model: "claude-opus-4-6", + }); + // Profile only sets platform, everything else falls back to role + const profile = makeProfile({ + command: undefined, + platform: "codex", + model: undefined, + }); + const result = mergeRuntimeConfig(role, profile); + expect(result.command).toBe("claude"); + expect(result.platform).toBe("codex"); + expect(result.model).toBe("claude-opus-4-6"); + }); +}); diff --git a/src/core/mock-runtime.ts b/src/core/mock-runtime.ts index 03b9e73..b029411 100644 --- a/src/core/mock-runtime.ts +++ b/src/core/mock-runtime.ts @@ -22,7 +22,13 @@ export class MockRuntime implements AgentRuntime { async spawn(role: string, config: AgentConfig): Promise { this.spawnCalls.push({ role, config }); const id = `mock-${role}-${this.nextId++}`; - const session: AgentSession = { id, role, status: "running" }; + const session: AgentSession = { + id, + role, + status: "running", + platform: config.platform, + model: config.model, + }; this.sessions.set(id, session); this.idleCallbacks.set(id, []); return session; @@ -67,6 +73,15 @@ export class MockRuntime implements AgentRuntime { if (s) this.sessions.set(sessionId, { ...s, status }); } + /** Get session metadata (platform, model, agent) for test assertions. */ + getSessionMetadata( + sessionId: string, + ): Pick | undefined { + const s = this.sessions.get(sessionId); + if (!s) return undefined; + return { platform: s.platform, model: s.model, agent: s.agent }; + } + /** Reset all recorded calls. */ reset(): void { this.spawnCalls.length = 0; diff --git a/src/core/platform-integration.test.ts b/src/core/platform-integration.test.ts new file mode 100644 index 0000000..98a1826 --- /dev/null +++ b/src/core/platform-integration.test.ts @@ -0,0 +1,286 @@ +/** + * Integration test: validates the complete platform/model flow from + * topology parsing through orchestrator to runtime session metadata. + * + * This is the end-to-end validation for issue #207. + */ + +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"; +import { LocalEventBus } from "./local-event-bus.js"; +import { MockRuntime } from "./mock-runtime.js"; +import { mergeRuntimeConfig, SessionOrchestrator } from "./session-orchestrator.js"; +import { shellEscape } from "./shell-utils.js"; +import type { AgentTopology } from "./topology.js"; +import { AgentTopologySchema, wireToTopology } from "./topology.js"; + +describe("Issue 207 — full platform/model integration", () => { + // ── Topology parsing → TypeScript types ───────────────────────────────── + + test("topology schema accepts platform and model fields", () => { + const wire = { + structure: "flat" as const, + roles: [ + { + name: "coder", + description: "Write code", + command: "codex", + platform: "codex" as const, + model: "gpt-4.1", + }, + { + name: "reviewer", + description: "Review code", + command: "claude", + platform: "claude-code" as const, + model: "claude-opus-4-6", + }, + ], + }; + + const result = AgentTopologySchema.safeParse(wire); + expect(result.success).toBe(true); + }); + + test("wireToTopology preserves platform and model", () => { + const wire = { + structure: "flat" as const, + roles: [ + { + name: "coder", + description: "Write code", + command: "codex", + platform: "codex" as const, + model: "gpt-4.1", + }, + ], + }; + + const topology = wireToTopology(wire); + expect(topology.roles[0]!.platform).toBe("codex"); + expect(topology.roles[0]!.model).toBe("gpt-4.1"); + expect(topology.roles[0]!.command).toBe("codex"); + }); + + // ── mergeRuntimeConfig ───────────────────────────────────────────────── + + test("merge produces correct config for mixed topology", () => { + const coderRole = { + name: "coder", + command: "codex", + platform: "codex" as const, + model: "gpt-4.1", + }; + const reviewerRole = { + name: "reviewer", + command: "claude", + platform: "claude-code" as const, + model: "claude-opus-4-6", + }; + + const coderConfig = mergeRuntimeConfig(coderRole, undefined); + const reviewerConfig = mergeRuntimeConfig(reviewerRole, undefined); + + expect(coderConfig.command).toBe("codex"); + expect(coderConfig.platform).toBe("codex"); + expect(coderConfig.model).toBe("gpt-4.1"); + + expect(reviewerConfig.command).toBe("claude"); + expect(reviewerConfig.platform).toBe("claude-code"); + expect(reviewerConfig.model).toBe("claude-opus-4-6"); + }); + + // ── AcpxRuntime.resolveAgent ─────────────────────────────────────────── + + test("resolveAgent picks correct backend for each platform", () => { + const rt = new AcpxRuntime(); + + const coderConfig: AgentConfig = { + role: "coder", + command: "codex", + platform: "codex", + model: "gpt-4.1", + cwd: "/tmp", + }; + const reviewerConfig: AgentConfig = { + role: "reviewer", + command: "claude", + platform: "claude-code", + model: "claude-opus-4-6", + cwd: "/tmp", + }; + + expect(rt.resolveAgent(coderConfig)).toBe("codex"); + expect(rt.resolveAgent(reviewerConfig)).toBe("claude"); + }); + + // ── SessionOrchestrator → MockRuntime ────────────────────────────────── + + test("orchestrator passes distinct platform/model per role", async () => { + const runtime = new MockRuntime(); + const bus = new LocalEventBus(); + + const topology: AgentTopology = { + structure: "flat", + roles: [ + { + name: "coder", + description: "Write code", + command: "codex", + platform: "codex", + model: "gpt-4.1", + }, + { + name: "reviewer", + description: "Review code", + command: "claude", + platform: "claude-code", + model: "claude-opus-4-6", + }, + ], + }; + + const orchestrator = new SessionOrchestrator({ + goal: "Build auth module", + contract: { contractVersion: 2, name: "test", topology }, + topology, + runtime, + eventBus: bus, + projectRoot: "/tmp", + workspaceBaseDir: "/tmp/workspaces", + workspaceIsolationPolicy: "allow-fallback", + }); + + await orchestrator.start(); + + // Verify each role got its own platform/model in AgentConfig + const coderCall = runtime.spawnCalls.find((c) => c.role === "coder"); + const reviewerCall = runtime.spawnCalls.find((c) => c.role === "reviewer"); + + expect(coderCall).toBeDefined(); + expect(reviewerCall).toBeDefined(); + + // Coder: codex platform + expect(coderCall!.config.platform).toBe("codex"); + expect(coderCall!.config.model).toBe("gpt-4.1"); + expect(coderCall!.config.command).toBe("codex"); + + // Reviewer: claude-code platform + expect(reviewerCall!.config.platform).toBe("claude-code"); + expect(reviewerCall!.config.model).toBe("claude-opus-4-6"); + expect(reviewerCall!.config.command).toBe("claude"); + + bus.close(); + }); + + test("MockRuntime session metadata matches spawn config", async () => { + const runtime = new MockRuntime(); + const bus = new LocalEventBus(); + + const topology: AgentTopology = { + structure: "flat", + roles: [{ name: "coder", command: "codex", platform: "codex", model: "gpt-4.1" }], + }; + + const orchestrator = new SessionOrchestrator({ + goal: "Build it", + contract: { contractVersion: 2, name: "test", topology }, + topology, + runtime, + eventBus: bus, + projectRoot: "/tmp", + workspaceBaseDir: "/tmp/workspaces", + workspaceIsolationPolicy: "allow-fallback", + }); + + const status = await orchestrator.start(); + const session = status.agents[0]!.session; + + // Session carries metadata from config + expect(session.platform).toBe("codex"); + expect(session.model).toBe("gpt-4.1"); + + // getSessionMetadata helper works + const meta = runtime.getSessionMetadata(session.id); + expect(meta!.platform).toBe("codex"); + expect(meta!.model).toBe("gpt-4.1"); + + bus.close(); + }); + + // ── Profile overlay through orchestrator ─────────────────────────────── + + test("profile overlay changes platform mid-flight", async () => { + const runtime = new MockRuntime(); + const bus = new LocalEventBus(); + + const topology: AgentTopology = { + structure: "flat", + roles: [ + { name: "coder", command: "claude", platform: "claude-code", model: "claude-opus-4-6" }, + ], + }; + + const orchestrator = new SessionOrchestrator({ + goal: "Build it", + contract: { contractVersion: 2, name: "test", topology }, + topology, + runtime, + eventBus: bus, + projectRoot: "/tmp", + workspaceBaseDir: "/tmp/workspaces", + workspaceIsolationPolicy: "allow-fallback", + profiles: [ + { + name: "@coder-fast", + role: "coder", + platform: "codex", + command: "codex", + model: "gpt-4.1", + }, + ], + }); + + await orchestrator.start(); + + const call = runtime.spawnCalls[0]!; + // Profile overrides role + expect(call.config.platform).toBe("codex"); + expect(call.config.model).toBe("gpt-4.1"); + expect(call.config.command).toBe("codex"); + + // Session metadata reflects the override + const session = (await runtime.listSessions())[0]!; + expect(session.platform).toBe("codex"); + expect(session.model).toBe("gpt-4.1"); + + bus.close(); + }); + + // ── Shell utils shared correctly ─────────────────────────────────────── + + test("shellEscape is the same function imported by all runtimes", () => { + // Verify it handles the edge case that matters most: single quotes in session names + const name = "grove-coder-0-l'abc"; + const escaped = shellEscape(name); + expect(escaped).toContain("'\\''"); + expect(escaped.startsWith("'")).toBe(true); + expect(escaped.endsWith("'")).toBe(true); + }); + + // ── Constants consistency ────────────────────────────────────────────── + + test("every PLATFORM_TO_AGENT value is in KNOWN_ACPX_AGENTS", () => { + for (const agent of Object.values(PLATFORM_TO_AGENT)) { + expect(KNOWN_ACPX_AGENTS.has(agent)).toBe(true); + } + }); + + test("all topology platform enum values have a mapping", () => { + const topologyPlatforms = ["claude-code", "codex", "gemini", "custom"] as const; + for (const p of topologyPlatforms) { + expect(PLATFORM_TO_AGENT[p]).toBeDefined(); + } + }); +}); diff --git a/src/core/session-orchestrator.test.ts b/src/core/session-orchestrator.test.ts index 222c3e8..fb39385 100644 --- a/src/core/session-orchestrator.test.ts +++ b/src/core/session-orchestrator.test.ts @@ -357,6 +357,253 @@ describe("SessionOrchestrator", () => { }); }); +// --------------------------------------------------------------------------- +// Platform/model passthrough (Issue 207) +// --------------------------------------------------------------------------- + +describe("SessionOrchestrator — platform/model passthrough", () => { + test("passes role.platform to AgentConfig", async () => { + const runtime = new MockRuntime(); + const bus = new LocalEventBus(); + const contract = makeContract({ + topology: { + structure: "flat", + roles: [ + { + name: "coder", + description: "Write code", + command: "codex", + platform: "codex", + model: "gpt-4.1", + }, + ], + }, + }); + + const orchestrator = new SessionOrchestrator({ + goal: "Build it", + contract, + topology: contract.topology!, + runtime, + eventBus: bus, + projectRoot: "/tmp", + workspaceBaseDir: "/tmp/workspaces", + workspaceIsolationPolicy: "allow-fallback", + }); + + await orchestrator.start(); + + expect(runtime.spawnCalls[0]!.config.platform).toBe("codex"); + expect(runtime.spawnCalls[0]!.config.model).toBe("gpt-4.1"); + bus.close(); + }); + + test("platform and model are undefined when role doesn't set them", async () => { + const runtime = new MockRuntime(); + const bus = new LocalEventBus(); + const contract = makeContract({ + topology: { + structure: "flat", + roles: [{ name: "worker", description: "Do work", command: "echo worker" }], + }, + }); + + const orchestrator = new SessionOrchestrator({ + goal: "Work", + contract, + topology: contract.topology!, + runtime, + eventBus: bus, + projectRoot: "/tmp", + workspaceBaseDir: "/tmp/workspaces", + workspaceIsolationPolicy: "allow-fallback", + }); + + await orchestrator.start(); + + expect(runtime.spawnCalls[0]!.config.platform).toBeUndefined(); + expect(runtime.spawnCalls[0]!.config.model).toBeUndefined(); + bus.close(); + }); + + test("profile overlays role — platform and command override", async () => { + const runtime = new MockRuntime(); + const bus = new LocalEventBus(); + const contract = makeContract({ + topology: { + structure: "flat", + roles: [ + { + name: "coder", + description: "Write code", + command: "claude", + platform: "claude-code", + }, + ], + }, + }); + + const orchestrator = new SessionOrchestrator({ + goal: "Build it", + contract, + topology: contract.topology!, + runtime, + eventBus: bus, + projectRoot: "/tmp", + workspaceBaseDir: "/tmp/workspaces", + workspaceIsolationPolicy: "allow-fallback", + profiles: [ + { + name: "@coder", + role: "coder", + platform: "codex", + command: "codex", + model: "gpt-4.1", + }, + ], + }); + + await orchestrator.start(); + + // Profile overrides role + expect(runtime.spawnCalls[0]!.config.command).toBe("codex"); + expect(runtime.spawnCalls[0]!.config.platform).toBe("codex"); + expect(runtime.spawnCalls[0]!.config.model).toBe("gpt-4.1"); + bus.close(); + }); + + test("profile without command falls back to role command", async () => { + const runtime = new MockRuntime(); + const bus = new LocalEventBus(); + const contract = makeContract({ + topology: { + structure: "flat", + roles: [ + { + name: "coder", + description: "Write code", + command: "claude", + platform: "claude-code", + model: "claude-opus-4-6", + }, + ], + }, + }); + + const orchestrator = new SessionOrchestrator({ + goal: "Build it", + contract, + topology: contract.topology!, + runtime, + eventBus: bus, + projectRoot: "/tmp", + workspaceBaseDir: "/tmp/workspaces", + workspaceIsolationPolicy: "allow-fallback", + profiles: [ + { + name: "@coder", + role: "coder", + platform: "gemini", // override platform only + }, + ], + }); + + await orchestrator.start(); + + // Profile overrides platform, role provides command and model + expect(runtime.spawnCalls[0]!.config.command).toBe("claude"); + expect(runtime.spawnCalls[0]!.config.platform).toBe("gemini"); + expect(runtime.spawnCalls[0]!.config.model).toBe("claude-opus-4-6"); + bus.close(); + }); + + test("mixed topology with different platforms per role", async () => { + const runtime = new MockRuntime(); + const bus = new LocalEventBus(); + const contract = makeContract({ + topology: { + structure: "graph", + roles: [ + { + name: "coder", + description: "Write code", + command: "codex", + platform: "codex", + edges: [{ target: "reviewer", edgeType: "delegates" as const }], + }, + { + name: "reviewer", + description: "Review code", + command: "claude", + platform: "claude-code", + model: "claude-opus-4-6", + edges: [{ target: "coder", edgeType: "feedback" as const }], + }, + ], + }, + }); + + const orchestrator = new SessionOrchestrator({ + goal: "Build + review", + contract, + topology: contract.topology!, + runtime, + eventBus: bus, + projectRoot: "/tmp", + workspaceBaseDir: "/tmp/workspaces", + workspaceIsolationPolicy: "allow-fallback", + }); + + await orchestrator.start(); + + const coderCall = runtime.spawnCalls.find((c) => c.role === "coder"); + const reviewerCall = runtime.spawnCalls.find((c) => c.role === "reviewer"); + + expect(coderCall!.config.platform).toBe("codex"); + expect(reviewerCall!.config.platform).toBe("claude-code"); + expect(reviewerCall!.config.model).toBe("claude-opus-4-6"); + bus.close(); + }); + + test("uses role.goal when available", async () => { + const runtime = new MockRuntime(); + const bus = new LocalEventBus(); + const contract = makeContract({ + topology: { + structure: "flat", + roles: [ + { + name: "writer", + description: "A writer agent", + prompt: "System instructions for writing", + goal: "Write excellent documentation", + command: "echo writer", + }, + ], + }, + }); + + const orchestrator = new SessionOrchestrator({ + goal: "Document the API", + contract, + topology: contract.topology!, + runtime, + eventBus: bus, + projectRoot: "/tmp", + workspaceBaseDir: "/tmp/workspaces", + workspaceIsolationPolicy: "allow-fallback", + }); + + await orchestrator.start(); + + // goal takes precedence over prompt and description + expect(runtime.sendCalls[0]!.message).toContain("Write excellent documentation"); + expect(runtime.sendCalls[0]!.message).not.toContain("System instructions for writing"); + expect(runtime.sendCalls[0]!.message).not.toContain("A writer agent"); + bus.close(); + }); +}); + // --------------------------------------------------------------------------- // Workspace isolation policy tests // --------------------------------------------------------------------------- diff --git a/src/core/session-orchestrator.ts b/src/core/session-orchestrator.ts index 9030adf..108e481 100644 --- a/src/core/session-orchestrator.ts +++ b/src/core/session-orchestrator.ts @@ -6,11 +6,12 @@ */ import { join } from "node:path"; +import type { AgentProfile } from "./agent-profile.js"; import type { AgentConfig, AgentRuntime, AgentSession } from "./agent-runtime.js"; import type { GroveContract } from "./contract.js"; import type { EventBus, GroveEvent } from "./event-bus.js"; import { resolveMcpServePath } from "./resolve-mcp-serve-path.js"; -import type { AgentRole, AgentTopology } from "./topology.js"; +import type { AgentPlatformType, AgentRole, AgentTopology } from "./topology.js"; import { resolveRoleWorkspaceStrategies, topologicalSortRoles } from "./topology.js"; import { TopologyRouter } from "./topology-router.js"; import { bootstrapWorkspace } from "./workspace-bootstrap.js"; @@ -60,6 +61,8 @@ export interface SessionConfig { readonly contributionStore?: | { list(query?: { limit?: number }): Promise } | undefined; + /** Optional agent profiles — overlay role defaults with per-agent runtime config. */ + readonly profiles?: readonly AgentProfile[] | undefined; } /** Status of a running session. */ @@ -81,6 +84,23 @@ export interface AgentSessionInfo { readonly workspaceMode: WorkspaceMode; } +/** + * Merge runtime selection fields from role + profile. + * Precedence: profile > role > default. + * + * Exported for testability — pure function, no side effects. + */ +export function mergeRuntimeConfig( + role: AgentRole, + profile: AgentProfile | undefined, +): { command: string; platform: AgentPlatformType | undefined; model: string | undefined } { + return { + command: profile?.command ?? role.command ?? "claude", + platform: profile?.platform ?? role.platform, + model: profile?.model ?? role.model, + }; +} + export class SessionOrchestrator { private readonly config: SessionConfig; private readonly sessionId: string; @@ -341,7 +361,7 @@ export class SessionOrchestrator { signal?: AbortSignal, workspace?: { cwd: string; workspaceMode: WorkspaceMode }, ): Promise { - const roleGoal = role.prompt ?? role.description ?? `Fulfill role: ${role.name}`; + const roleGoal = role.goal ?? role.prompt ?? role.description ?? `Fulfill role: ${role.name}`; const fullGoal = `Session goal: ${this.config.goal}\n\nYour role (${role.name}): ${roleGoal}`; const { cwd, workspaceMode } = workspace ?? { @@ -355,9 +375,15 @@ export class SessionOrchestrator { if (signal?.aborted) throw new Error(`Spawn aborted for role '${role.name}'`); + // Merge profile overlay (profile > role > default) + const profile = this.config.profiles?.find((p) => p.role === role.name); + const resolved = mergeRuntimeConfig(role, profile); + const agentConfig: AgentConfig = { role: role.name, - command: role.command ?? "claude", + command: resolved.command, + platform: resolved.platform, + model: resolved.model, cwd, goal: fullGoal, env: { diff --git a/src/core/shell-utils.test.ts b/src/core/shell-utils.test.ts new file mode 100644 index 0000000..b497142 --- /dev/null +++ b/src/core/shell-utils.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, test } from "bun:test"; +import { shellEscape } from "./shell-utils.js"; + +describe("shellEscape", () => { + test("wraps simple string in single quotes", () => { + expect(shellEscape("hello")).toBe("'hello'"); + }); + + test("escapes single quotes", () => { + expect(shellEscape("it's")).toBe("'it'\\''s'"); + }); + + test("handles empty string", () => { + expect(shellEscape("")).toBe("''"); + }); + + test("handles string with spaces", () => { + expect(shellEscape("hello world")).toBe("'hello world'"); + }); + + test("handles string with special characters", () => { + expect(shellEscape("$HOME && rm -rf /")).toBe("'$HOME && rm -rf /'"); + }); + + test("handles multiple single quotes", () => { + expect(shellEscape("it's a 'test'")).toBe("'it'\\''s a '\\''test'\\'''"); + }); +}); diff --git a/src/core/shell-utils.ts b/src/core/shell-utils.ts new file mode 100644 index 0000000..3a4aee5 --- /dev/null +++ b/src/core/shell-utils.ts @@ -0,0 +1,6 @@ +/** Shared shell utility functions for runtime implementations. */ + +/** Escape a string for safe use in shell commands (single-quote wrapping). */ +export function shellEscape(s: string): string { + return `'${s.replace(/'/g, "'\\''")}'`; +} diff --git a/src/core/subprocess-runtime.ts b/src/core/subprocess-runtime.ts index 2b10df3..1a131c8 100644 --- a/src/core/subprocess-runtime.ts +++ b/src/core/subprocess-runtime.ts @@ -23,15 +23,29 @@ export class SubprocessRuntime implements AgentRuntime { throw new Error(`Empty command for role "${role}"`); } + const spawnEnv: Record = { + ...process.env, + ...config.env, + } as Record; + if (config.platform) spawnEnv.GROVE_AGENT_PLATFORM = config.platform; + if (config.model) spawnEnv.GROVE_AGENT_MODEL = config.model; + const proc = Bun.spawn([cmd, ...args], { cwd: config.cwd, - env: { ...process.env, ...config.env }, + env: spawnEnv, stdin: "pipe", stdout: "pipe", stderr: "pipe", }); - const session: AgentSession = { id, role, pid: proc.pid, status: "running" }; + const session: AgentSession = { + id, + role, + pid: proc.pid, + status: "running", + platform: config.platform, + model: config.model, + }; this.sessions.set(id, { proc, session, idleCallbacks: [] }); // Monitor for exit diff --git a/src/core/tmux-runtime.ts b/src/core/tmux-runtime.ts index fa96eb7..4ac06b8 100644 --- a/src/core/tmux-runtime.ts +++ b/src/core/tmux-runtime.ts @@ -8,6 +8,7 @@ import { execSync } from "node:child_process"; import type { AgentConfig, AgentRuntime, AgentSession } from "./agent-runtime.js"; +import { shellEscape } from "./shell-utils.js"; /** Prefix for all grove tmux session names. */ const SESSION_PREFIX = "grove-"; @@ -44,13 +45,21 @@ export class TmuxRuntime implements AgentRuntime { const sessionName = `${SESSION_PREFIX}${role}-${counter}`; const id = sessionName; + // Pass platform/model as env vars so the agent process can read them + const spawnEnv: Record = { + ...process.env, + ...config.env, + } as Record; + if (config.platform) spawnEnv.GROVE_AGENT_PLATFORM = config.platform; + if (config.model) spawnEnv.GROVE_AGENT_MODEL = config.model; + try { execSync( `tmux -L grove new-session -d -s ${shellEscape(sessionName)} -c ${shellEscape(config.cwd)} ${shellEscape(config.command)}`, { encoding: "utf-8", stdio: "pipe", - env: config.env ? { ...process.env, ...config.env } : process.env, + env: spawnEnv, }, ); } catch (err) { @@ -59,7 +68,13 @@ export class TmuxRuntime implements AgentRuntime { ); } - const session: AgentSession = { id, role, status: "running" }; + const session: AgentSession = { + id, + role, + status: "running", + platform: config.platform, + model: config.model, + }; const entry: TmuxSessionEntry = { session, idleCallbacks: [], @@ -195,8 +210,3 @@ export class TmuxRuntime implements AgentRuntime { } } } - -/** Escape a string for safe use in shell commands. */ -function shellEscape(s: string): string { - return `'${s.replace(/'/g, "'\\''")}'`; -} diff --git a/src/tui/app.tsx b/src/tui/app.tsx index a75a40a..0e529c2 100644 --- a/src/tui/app.tsx +++ b/src/tui/app.tsx @@ -711,11 +711,14 @@ export function App({ const depthCheck = checkSpawnDepth(topology, depth); if (!depthCheck.allowed) return; - // Look up role prompt/description from topology to inject as agent context. + // Look up role config from topology to inject as agent context. const role = topology?.roles.find((r) => r.name === agentId); const context: Record = {}; if (role?.prompt) context.rolePrompt = role.prompt; if (role?.description) context.roleDescription = role.description; + if (role?.goal) context.roleGoal = role.goal; + if (role?.platform) context.platform = role.platform; + if (role?.model) context.model = role.model; if (topology) context.topology = topology; spawnManager.spawn(agentId, command, parentAgentId, depth, context).catch((err) => { diff --git a/src/tui/screens/screen-manager.tsx b/src/tui/screens/screen-manager.tsx index e69f161..49ef8d7 100644 --- a/src/tui/screens/screen-manager.tsx +++ b/src/tui/screens/screen-manager.tsx @@ -510,12 +510,18 @@ export const ScreenManager: React.NamedExoticComponent = Rea void (async () => { const orderedRoles = topologicalSortRoles(topology); for (const role of orderedRoles) { - const command = roleMapping.get(role.name) ?? role.command ?? "codex"; + const userOverrideCmd = roleMapping.get(role.name); + const command = userOverrideCmd ?? role.command ?? "codex"; const context: Record = {}; const editedPrompt = rolePromptsRef.current.get(role.name); context.rolePrompt = editedPrompt ?? role.prompt ?? ""; if (role.description) context.roleDescription = role.description; if (role.goal) context.roleGoal = role.goal; + // If the user explicitly changed the CLI in launch preview, don't pass + // the topology's platform — it would override the user's choice in + // resolveAgent(). Let resolveAgent fall back to command parsing instead. + if (!userOverrideCmd && role.platform) context.platform = role.platform; + if (role.model) context.model = role.model; if (topology) context.topology = topology; // Mark as spawning diff --git a/src/tui/spawn-manager.ts b/src/tui/spawn-manager.ts index 7eded81..ea0ccfc 100644 --- a/src/tui/spawn-manager.ts +++ b/src/tui/spawn-manager.ts @@ -304,7 +304,9 @@ export class SpawnManager { initialPrompt = parts.join(". "); } - // Compose agent command with auto-approve flags + // Compose agent command with auto-approve flags. + // The prompt is passed via AgentConfig.prompt, NOT embedded in the command + // string — avoids shell injection and decouples "what to run" from "what to say". let agentCommand = command; const baseCmd = command.split(/\s+/)[0] ?? command; if (baseCmd === "claude") { @@ -312,9 +314,6 @@ export class SpawnManager { } else if (baseCmd === "codex") { agentCommand = `${command} --full-auto`; } - if (initialPrompt) { - agentCommand = `${agentCommand} "${initialPrompt.replace(/"/g, '\\"')}"`; - } // For codex roles, require successful codex MCP registration before // spawning. writeMcpConfig above runs ensureCodexMcpRegistered in a @@ -345,14 +344,24 @@ export class SpawnManager { const rolePromptText = String(context?.rolePrompt ?? "").toLowerCase(); const waitForPush = context?.waitForPush === true || rolePromptText.includes("wait for"); + // Extract platform/model from context (set by topology role or profile overlay) + const platform = context?.platform as + | import("../core/topology.js").AgentPlatformType + | undefined; + const model = context?.model as string | undefined; + const agentConfig: AgentConfig = { role: roleId, command: agentCommand, cwd: workspacePath, env: { ...roleEnv, ...prEnv }, - goal: this.sessionGoal, + // initialPrompt already contains the session goal + role details + "Read CLAUDE.md". + // Use it as goal so runtimes send the complete message (not just the bare session goal). + goal: initialPrompt ?? this.sessionGoal, prompt: initialPrompt, waitForPush, + platform, + model, }; const session = await this.agentRuntime.spawn(roleId, agentConfig); this.agentSessions.set(spawnId, session);