diff --git a/docs/findings/13-acp-agents.md b/docs/findings/13-acp-agents.md index 9e1f13d..680a896 100644 --- a/docs/findings/13-acp-agents.md +++ b/docs/findings/13-acp-agents.md @@ -191,3 +191,89 @@ Both agents produced only AG-UI types the SPA already folds (`RUN_*`, `TEXT_MESS `CLAUDE_CODE_SESSION_ID`, … Conclusions → [`adr/0013-agents-over-acp.md`](../adr/0013-agents-over-acp.md). + +## Addendum 2: host settings, verified with a prompt (#63, same day) + +The production adapter (`src/runtime/acp-adapter.ts`, definitions in `acp-agents.ts`) run +through `bun spikes/13-acp-agents.ts isolation [ignore|include]`. The scratch +repo carries **project** settings: `CLAUDE.md` and `AGENTS.md` with a codeword each, and a skill +in `.claude/skills` and one in `.agents/skills`. One structured step asks the agent which skills +it has and which codewords its instructions hold, then runs `echo $XDG_CONFIG_HOME`, +`gh auth status`, `git config --global --list --show-origin` and `env` through its own shell. +The host has 36 entries in `~/.claude/skills` (49 `SKILL.md` files below them) plus the +`anthropic-skills` plugin, 34 in `~/.agents/skills`, `gh-stack` in `~/.config/opencode/skills`, +and no `~/.claude/CLAUDE.md`. For +the user-level `CLAUDE.md`, Claude ran with `CLAUDE_CONFIG_DIR` pointing at a copy of `~/.claude` +made of symlinks plus a `CLAUDE.md` holding a codeword, so the real `~/.claude` stayed untouched. + +### Claude Code: `settingSources: ["project", "local"]` keeps skills and `CLAUDE.md` out + +| `hostSettings` | skills listed | host skills among them | user `CLAUDE.md` codeword | project `CLAUDE.md` codeword | project skill | +|---|---|---|---|---|---| +| `include` (SDK default `user, project, local`) | 47 | 21 user + 13 `anthropic-skills:*` | seen | seen | seen | +| `ignore` (`project, local`) | 14 | none | **not seen** | seen | seen | + +The other 13 are Claude Code's bundled skills (`simplify`, `loop`, `claude-api`, …). The open +question of the first addendum is settled: `settingSources` keeps the user's skills, plugins and +`CLAUDE.md` out of a run. The residue stays as described there: claude-agent-acp still reads the +user's `model`, `permissions.defaultMode` and `availableModels` itself. Factory always sends a +model and answers every ask, so only an `availableModels` allowlist could still narrow a run. + +### opencode: `OPENCODE_CONFIG_DIR` does not hide anything + +opencode 1.18.31 (read from the bundled source, then measured) builds its config directories as +`[$XDG_CONFIG_HOME/opencode, project .opencode dirs, ~/.opencode, $OPENCODE_CONFIG_DIR]` and loads +the global config file from the first unconditionally. `OPENCODE_CONFIG_DIR` **adds** a +directory: with it set to an empty dir, `session/new` still offered 715 models including the +host's `omniroute` provider. `XDG_CONFIG_HOME` is read once, at start, into opencode's paths. The +bash tool spawns with `{...process.env, ...shell.env hook output}`. That makes the mechanism: + +- `XDG_CONFIG_HOME` → an empty factory-owned dir hides `~/.config/opencode`; +- a factory plugin (`src/runtime/acp-opencode-plugin.ts`, loaded by file URL through + `OPENCODE_CONFIG_CONTENT`) uses opencode's `shell.env` hook to hand the host's + `XDG_CONFIG_HOME` (or `~/.config`) back to every shell command, so `gh` and git keep their + config. + +That still left a second leak. opencode also reads **home-level** files of its own accord: +`~/.claude/skills`, `~/.agents/skills`, `~/.claude/CLAUDE.md` and `~/.opencode`. They hang off +its notion of home, which `OPENCODE_TEST_HOME` overrides. The variable changes neither `HOME` +nor the data dir, so provider logins are kept. + +| opencode env | skills listed | host skills | project skills | models offered | shell `XDG_CONFIG_HOME` | `gh auth status` | git global config | +|---|---|---|---|---|---|---|---| +| `include` (host's own) | 51 | 48 | both | 715 | unset | logged in | read | +| `XDG_CONFIG_HOME` + plugin | 51 | 48 | both | 39 | `/home/karl/.config` | logged in | read | +| … + `OPENCODE_DISABLE_CLAUDE_CODE_SKILLS=1` | 36 | 34 (`~/.agents`) | `.agents` only | 39 | `/home/karl/.config` | logged in | read | +| … + `OPENCODE_TEST_HOME` → empty dir (**shipped**) | 3 | none | both | 39 | `/home/karl/.config` | logged in | read | + +The 3 skills are opencode's built-in `customize-opencode` and the two project skills. In every +variant opencode took the project's `AGENTS.md` codeword; it reads `CLAUDE.md` only when there is +no `AGENTS.md`. The 39 models are the auth-based `opencode` and `opencode-go` providers. Hiding the +host's skill listing shrank opencode's prompt: the `implement` step's cached input went from +~13.9k to ~7.7k tokens. + +- `OPENCODE_DISABLE_CLAUDE_CODE` / `_SKILLS` / `_PROMPT` are not an alternative. They also drop the + project's `.claude/skills` and `CLAUDE.md`, and leave `~/.agents/skills` in. +- `OPENCODE_TEST_HOME` is undocumented, a test hook by its name. If opencode drops it, the host's + skills reappear in runs; nothing breaks. Other effects, read from the source: `~/` in a + project's opencode config (permission patterns, `skills.paths`, `instructions`, MCP paths) + resolves against the empty dir; the TUI's path display is not used under ACP. + +### Environment scrub + +The adapter's agent environment drops `CLAUDECODE` and every `CLAUDE_*` variable except +`CLAUDE_CONFIG_DIR`, `CLAUDE_CODE_OAUTH_TOKEN`, `CLAUDE_CODE_USE_BEDROCK` and +`CLAUDE_CODE_USE_VERTEX` (where the credentials are, or which provider pays). Run from this Claude +Code session: 12 `CLAUDE*` variables in the parent, none passed on. With the parent's +`CLAUDE_EFFORT`, `CLAUDE_CODE_SESSION_ID` and `CLAUDE_CODE_SUBAGENT_MODEL` set to a sentinel, the +sentinel appeared 0 times in either agent's shell. Claude's shell still shows `CLAUDECODE`, +`CLAUDE_CODE_SESSION_ID`, `CLAUDE_EFFORT`, … — those belong to the agent's own Claude Code +session. opencode's shell shows none. + +### Availability + +`checkAgents(["claude", "opencode"])` reports `opencode` missing from PATH (`Bun.which`) and +Claude not logged in. The second comes from the bundled CLI's `auth status --json` +(`bun …/claude-agent-acp/dist/index.js --cli auth status --json`, ~0.25 s): logged out means no +claude.ai login, no API key source and the first-party backend. On this host both pass. With +`HOME` pointed at an empty dir and opencode off PATH, both are reported. diff --git a/spikes/13-acp-agents.ts b/spikes/13-acp-agents.ts index 422a962..cc776fa 100644 --- a/spikes/13-acp-agents.ts +++ b/spikes/13-acp-agents.ts @@ -1,12 +1,18 @@ /** - * SPIKE (finding 13): run a hello-shaped workflow through factory's real - * runtime (`startRun`, `AgentRuntime`, `agent-step.ts`) on the ACP adapter, - * against opencode and Claude Code, and measure what the ADR needs to know: - * model selection, usage, structured output, permission asks, cancel. + * SPIKE (finding 13), kept as the live check of the production ACP adapter + * (#63): run a hello-shaped workflow through factory's real runtime + * (`startRun`, `AgentRuntime`, `agent-step.ts`) on opencode or Claude Code, + * and measure model selection, usage, structured output, permission asks, + * cancel, and what the agent can see of the host's settings. * * bun spikes/13-acp-agents.ts run * bun spikes/13-acp-agents.ts cancel [afterMs] * bun spikes/13-acp-agents.ts bad-model + * bun spikes/13-acp-agents.ts isolation [ignore|include] + * + * `isolation` asks the agent which skills and instructions it has and runs + * `gh auth status`, git's global config and `env` through its shell, with the + * host's agent settings ignored (default) or included. * * Writes `spikes/out/--/{events,diagnostics}.ndjson`. */ @@ -15,19 +21,22 @@ import { mkdir, mkdtemp, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import type { RunEvent } from "../src/events"; +import { acpAdapter, type AcpDiagnostic } from "../src/runtime/acp-adapter"; import { - ACP_AGENTS, - acpAdapter, + ACP_AGENT_KINDS, + acpAgent, + agentEnv, type AcpAgentKind, - type AcpDiagnostic, -} from "../src/runtime/acp-adapter"; + type HostSettings, +} from "../src/runtime/acp-agents"; import { makeAgentRuntime } from "../src/runtime/agent-runtime"; import { startRun } from "../src/runtime/run"; import { defineWorkflow, Schema } from "../src/workflow"; const [mode = "run", agentArg = "opencode", modelArg, afterArg] = process.argv.slice(2); const agent = agentArg as AcpAgentKind; -if (!(agent in ACP_AGENTS)) throw new Error(`unknown agent ${agentArg}`); +if (!ACP_AGENT_KINDS.includes(agent)) throw new Error(`unknown agent ${agentArg}`); +const hostSettings: HostSettings = afterArg === "include" ? "include" : "ignore"; const model = mode === "bad-model" ? "no-such-model" : modelArg; if (model === undefined) throw new Error("model required"); @@ -46,6 +55,21 @@ async function scratchRepo(): Promise { "export function add(a: number, b: number): number {\n return a + b;\n}\n", ); await writeFile(join(dir, "README.md"), "# math\n\nTiny arithmetic helpers.\n"); + if (mode === "isolation") { + // Project-level settings, which a run must keep seeing. + await writeFile(join(dir, "CLAUDE.md"), "Project codeword: PROJ-CLAUDE-7731\n"); + await writeFile(join(dir, "AGENTS.md"), "Project codeword: PROJ-AGENTS-4410\n"); + for (const [root, name] of [ + [".claude", "project-claude-probe"], + [".agents", "project-agents-probe"], + ] as const) { + await mkdir(join(dir, root, "skills", name), { recursive: true }); + await writeFile( + join(dir, root, "skills", name, "SKILL.md"), + `---\nname: ${name}\ndescription: Probe skill; use when asked about probes.\n---\n\nProbe.\n`, + ); + } + } await sh(dir, "git", "init", "-q", "-b", "main"); await sh(dir, "git", "add", "."); await sh( @@ -104,6 +128,40 @@ Make the change. When you are done, stop — do not run git commands and do not }, }); +const Visible = Schema.Struct({ + skills: Schema.Array(Schema.String), + codewords: Schema.Array(Schema.String), + xdgConfigHome: Schema.String, + ghAuth: Schema.String, + gitUser: Schema.String, + claudeVars: Schema.String, + parentSentinels: Schema.String, +}); + +const isolation = defineWorkflow("acp-isolation", { + input: Schema.Struct({ long: Schema.Boolean }), + output: Visible, + agent: { model }, + run: async (ctx) => { + const seen = await ctx.agent( + "visible", + `Report what you can see. Do not guess and do not invent anything. + +1. skills: the name of every skill available to you, exactly as your skill tool or skill listing names them (all of them). +2. codewords: every "codeword" value that appears in instructions you were given (CLAUDE.md, AGENTS.md or similar), verbatim. Do not search files for them; only what is already in your context. +3. Run these shell commands, each separately, and report their output trimmed: + - xdgConfigHome: \`echo "XDG=$XDG_CONFIG_HOME"\` + - ghAuth: \`gh auth status 2>&1 | head -2\` + - gitUser: \`git config --global --list --show-origin | head -1\` + - claudeVars: \`env | grep -o '^CLAUDE[A-Z_]*' | sort | tr '\\n' ' '\` + - parentSentinels: \`env | grep -c factory-parent-sentinel || true\``, + { output: Visible }, + ); + if (seen.output === undefined) throw new Error("no structured output"); + return seen.output; + }, +}); + const stamp = new Date().toISOString().replace(/[:.]/g, "-"); const out = join(import.meta.dir, "out", `${mode}-${agent}-${stamp}`); await mkdir(out, { recursive: true }); @@ -112,19 +170,28 @@ const diagnostics: Array = []; const pids: number[] = []; let step = -1; -const adapter = acpAdapter(ACP_AGENTS[agent], (event) => { - if (event.kind === "spawned") { - step += 1; - pids.push(event.pid); - } - diagnostics.push({ ...event, step }); +const definition = acpAgent(agent, { hostSettings }); +const leaked = Object.keys(agentEnv(definition)).filter( + (k) => k === "CLAUDECODE" || k.startsWith("CLAUDE_"), +); +console.log( + `parent CLAUDE* vars: ${Object.keys(process.env).filter((k) => k.startsWith("CLAUDE")).length}, passed to the agent: [${leaked.join(", ")}]`, +); +const adapter = acpAdapter(definition, { + onDiagnostic: (event) => { + if (event.kind === "spawned") { + step += 1; + pids.push(event.pid); + } + diagnostics.push({ ...event, step }); + }, }); const dir = await scratchRepo(); console.log(`${mode} ${agent} model=${model} dir=${dir}`); const runtime = makeAgentRuntime(adapter); const t0 = Date.now(); -const handle = startRun(hello, runtime, { +const handle = startRun(mode === "isolation" ? isolation : hello, runtime, { runId: `spike-${stamp}`, dir, input: { long: mode === "cancel" }, diff --git a/src/runtime/acp-adapter.test.ts b/src/runtime/acp-adapter.test.ts new file mode 100644 index 0000000..a2a2511 --- /dev/null +++ b/src/runtime/acp-adapter.test.ts @@ -0,0 +1,285 @@ +/** + * The ACP adapter against a fake agent over stdio (`test/fixtures/ + * fake-acp-agent.ts`, built on the SDK's `AgentSideConnection`): the real + * protocol, process and pipes, with the agent's behaviour scripted. + */ + +import { describe, expect, test } from "bun:test"; +import { join, relative } from "node:path"; +import { acpAdapter, AcpAgentError, type AcpDiagnostic } from "./acp-adapter"; +import type { AcpAgentDefinition } from "./acp-agents"; +import type { AgentAdapterYield, AgentSignal } from "./agent-adapter"; +import type { RunEvent } from "../events"; +import { defineWorkflow, Schema } from "../workflow"; +import { makeAgentRuntime } from "./agent-runtime"; +import { startRun } from "./run"; + +const FAKE = join(import.meta.dir, "../../test/fixtures/fake-acp-agent.ts"); + +function fake(env: Record = {}): AcpAgentDefinition { + return { agent: "fake", command: [process.execPath, FAKE], env }; +} + +interface Collected { + readonly yields: AgentAdapterYield[]; + readonly diagnostics: AcpDiagnostic[]; + readonly error: unknown; +} + +async function collect( + prompt: string, + options: { + model?: string; + dir?: string; + definition?: AcpAgentDefinition; + outputSchema?: unknown; + abortAfter?: (y: AgentAdapterYield) => boolean; + cancelGraceMs?: number; + } = {}, +): Promise { + const diagnostics: AcpDiagnostic[] = []; + const adapter = acpAdapter(options.definition ?? fake(), { + onDiagnostic: (d) => diagnostics.push(d), + cancelGraceMs: options.cancelGraceMs ?? 300, + }); + const abortController = new AbortController(); + const yields: AgentAdapterYield[] = []; + let error: unknown; + try { + for await (const y of adapter.stream({ + threadId: "thread", + dir: options.dir ?? import.meta.dir, + model: options.model ?? "fake/fast", + prompt, + outputSchema: options.outputSchema, + abortController, + })) { + yields.push(y); + if (options.abortAfter?.(y)) abortController.abort(); + } + } catch (cause) { + error = cause; + } + return { yields, diagnostics, error, abortController }; +} + +const signals = (c: Collected, tag: AgentSignal["_tag"]) => + c.yields.flatMap((y) => (y.signal?._tag === tag ? [y.signal] : [])); +const types = (c: Collected) => c.yields.map((y) => (y.chunk as { type: string }).type); +const text = (c: Collected) => + c.yields + .map((y) => y.chunk as { type: string; delta?: string }) + .filter((ch) => ch.type === "TEXT_MESSAGE_CONTENT") + .map((ch) => ch.delta) + .join(""); +const pidOf = (c: Collected) => + c.diagnostics.flatMap((d) => (d.kind === "spawned" ? [d.pid] : []))[0]!; +const alive = (pid: number) => { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +}; + +describe("acpAdapter", () => { + test("selects the requested model before prompting", async () => { + const c = await collect("hello", { model: "fake/smart" }); + expect(c.error).toBeUndefined(); + expect(text(c)).toBe("hello from fake/smart"); + const configured = c.diagnostics.find((d) => d.kind === "configured"); + expect(configured).toMatchObject({ configId: "model", value: "fake/smart" }); + expect(types(c)[0]).toBe("RUN_STARTED"); + expect(types(c).at(-1)).toBe("RUN_FINISHED"); + expect(signals(c, "sessionId")).toHaveLength(1); + }); + + test("runs in a relative dir: ACP gets it as an absolute cwd", async () => { + // The default workspace root, `.factory/workspaces`, is relative. + const dir = relative(process.cwd(), import.meta.dir) || "."; + const c = await collect("hello", { dir }); + expect(c.error).toBeUndefined(); + expect(text(c)).toBe("hello from fake/fast"); + }); + + test("sets the model even when the agent reports it as current", async () => { + // claude-agent-acp reports the user's `model` setting as current even + // when host settings are ignored and the SDK runs another model. + const c = await collect("hello", { model: "fake/default" }); + expect(text(c)).toBe("hello from fake/default"); + const configured = c.diagnostics.find((d) => d.kind === "configured"); + expect(configured).toMatchObject({ configId: "model", value: "fake/default" }); + }); + + test("fails before the prompt on a model the agent does not offer", async () => { + const c = await collect("hello", { model: "nope/model" }); + expect(c.error).toBeInstanceOf(AcpAgentError); + expect((c.error as Error).message).toBe( + 'fake has no model "nope/model" (3 offered, e.g. fake/default, fake/fast, fake/smart)', + ); + expect(c.yields).toHaveLength(0); + expect(c.diagnostics.some((d) => d.kind === "update")).toBe(false); + }); + + test("fails when the agent offers no model option", async () => { + const c = await collect("hello", { definition: fake({ FAKE_ACP_NO_MODEL: "1" }) }); + expect((c.error as Error).message).toContain("fake offers no model option"); + }); + + test("fails without a model", async () => { + const c = await collect("hello", { model: "" }); + expect((c.error as Error).message).toContain("no model given"); + expect(c.diagnostics).toHaveLength(0); + }); + + test("answers a permission ask with allow_always", async () => { + const c = await collect("permission"); + expect(text(c)).toBe("permission: always"); + expect(c.diagnostics.find((d) => d.kind === "permission")).toMatchObject({ + title: "Edit math.ts", + chosen: "always", + }); + }); + + test("forwards each usage_update as an acp.usage chunk with a usage signal, in order", async () => { + const c = await collect("hello"); + expect(signals(c, "usage")).toEqual([ + { _tag: "usage", value: { context: { used: 1000, size: 200_000 } } }, + { + _tag: "usage", + value: { context: { used: 1200, size: 200_000 }, cost: { amount: 0.25, currency: "USD" } }, + }, + ]); + const names = c.yields.map((y) => { + const chunk = y.chunk as { type: string; name?: string }; + return chunk.type === "CUSTOM" ? chunk.name : chunk.type; + }); + // usage, then the text, then usage with cost, then the end of the run. + expect(names.indexOf("acp.usage")).toBeLessThan(names.indexOf("TEXT_MESSAGE_CONTENT")); + expect(names.lastIndexOf("acp.usage")).toBeGreaterThan(names.indexOf("TEXT_MESSAGE_CONTENT")); + expect(names.lastIndexOf("acp.usage")).toBeLessThan(names.indexOf("RUN_FINISHED")); + const finished = c.yields.at(-1)!.chunk as { usage?: unknown }; + expect(finished.usage).toMatchObject({ + promptTokens: 10, + completionTokens: 5, + promptTokensDetails: { cachedTokens: 990 }, + }); + }); + + test("parses structured output from the last message and yields it before RUN_FINISHED", async () => { + const c = await collect("json", { outputSchema: { type: "object" } }); + expect(signals(c, "structuredOutput")).toEqual([ + { _tag: "structuredOutput", value: { summary: "done", count: 2 } }, + ]); + const last = c.yields + .slice(-2) + .map( + (y) => + (y.chunk as { type: string; name?: string }).name ?? (y.chunk as { type: string }).type, + ); + expect(last).toEqual(["structured-output.complete", "RUN_FINISHED"]); + }); + + test("a refusal becomes RUN_ERROR with a runError signal", async () => { + const c = await collect("refuse"); + expect(signals(c, "runError")).toHaveLength(1); + expect(types(c)).toContain("RUN_ERROR"); + }); + + test("the agent does not inherit the parent's Claude Code session variables", async () => { + const before = { ...process.env }; + process.env.CLAUDECODE = "1"; + process.env.CLAUDE_EFFORT = "high"; + process.env.CLAUDE_CODE_SESSION_ID = "parent"; + process.env.CLAUDE_CONFIG_DIR = "/tmp/claude-config"; + try { + const seen = text(await collect("env")).split(","); + expect(seen).toContain("CLAUDE_CONFIG_DIR"); + expect(seen).not.toContain("CLAUDECODE"); + expect(seen).not.toContain("CLAUDE_EFFORT"); + expect(seen).not.toContain("CLAUDE_CODE_SESSION_ID"); + } finally { + for (const key of [ + "CLAUDECODE", + "CLAUDE_EFFORT", + "CLAUDE_CODE_SESSION_ID", + "CLAUDE_CONFIG_DIR", + ]) + if (before[key] === undefined) delete process.env[key]; + else process.env[key] = before[key]; + } + }); + + test("cancel sends session/cancel, the turn settles as cancelled, and the process is gone", async () => { + const c = await collect("hang", { + abortAfter: (y) => (y.chunk as { type: string }).type === "TEXT_MESSAGE_CONTENT", + }); + expect(c.error).toBeUndefined(); + expect(c.diagnostics.find((d) => d.kind === "done")).toMatchObject({ stopReason: "cancelled" }); + expect(alive(pidOf(c))).toBe(false); + }); + + test("an agent that ignores cancel is killed after the grace period", async () => { + const started = Date.now(); + const c = await collect("deaf", { + cancelGraceMs: 200, + abortAfter: (y) => (y.chunk as { type: string }).type === "TEXT_MESSAGE_CONTENT", + }); + expect(Date.now() - started).toBeLessThan(5000); + expect(c.diagnostics.some((d) => d.kind === "exited")).toBe(true); + expect(alive(pidOf(c))).toBe(false); + }); + + test("an agent that exits mid-turn fails the step with its exit code and stderr tail", async () => { + const c = await collect("crash"); + expect(c.error).toBeInstanceOf(AcpAgentError); + const message = (c.error as Error).message; + expect(message).toContain("fake exited (code 3)"); + expect(message).toContain("fatal: the fake agent fell over"); + // The translator closed the open text message before the failure surfaced. + expect(types(c)).toContain("TEXT_MESSAGE_END"); + }); + + test("an agent that cannot be started fails with its command", async () => { + const c = await collect("hello", { + definition: { agent: "ghost", command: ["/nonexistent/acp-agent"] }, + }); + expect((c.error as Error).message).toContain("ghost did not start (/nonexistent/acp-agent)"); + }); + + test("the process is gone after a normal turn", async () => { + const c = await collect("hello"); + expect(c.error).toBeUndefined(); + expect(alive(pidOf(c))).toBe(false); + }); + + test("through the runtime: a cancelled run cancels the step and leaves no agent process", async () => { + const diagnostics: AcpDiagnostic[] = []; + const runtime = makeAgentRuntime( + acpAdapter(fake(), { onDiagnostic: (d) => diagnostics.push(d), cancelGraceMs: 300 }), + ); + const workflow = defineWorkflow("acp-cancel", { + input: Schema.Struct({}), + run: async (ctx) => { + await ctx.agent("hang", "hang until cancelled", { model: "fake/fast" }); + }, + }); + const events: RunEvent[] = []; + const handle = startRun(workflow, runtime, { + runId: "acp-cancel", + dir: import.meta.dir, + input: {}, + onEvent: (event) => events.push(event), + }); + while (!events.some((e) => e.payload._tag === "AgentChunk")) await Bun.sleep(10); + await handle.cancel(); + expect((await handle.result).outcome).toBe("cancelled"); + await handle.settled; + await runtime.dispose(); + const step = events.find((e) => e.payload._tag === "AgentStepFinished")?.payload; + expect(step).toMatchObject({ outcome: "cancelled" }); + const pid = diagnostics.flatMap((d) => (d.kind === "spawned" ? [d.pid] : []))[0]!; + expect(alive(pid)).toBe(false); + }); +}); diff --git a/src/runtime/acp-adapter.ts b/src/runtime/acp-adapter.ts index e54e577..cea628e 100644 --- a/src/runtime/acp-adapter.ts +++ b/src/runtime/acp-adapter.ts @@ -1,19 +1,30 @@ /** - * SPIKE (finding 13): an `AgentAdapter` that drives any ACP-speaking coding - * agent over stdio — opencode (`opencode acp`) and Claude Code - * (`@agentclientprotocol/claude-agent-acp`) — in place of `@tanstack/ai-opencode`. + * An `AgentAdapter` that drives any ACP-speaking coding agent over stdio + * (ADR 0013): opencode (`opencode acp`) and Claude Code + * (`@agentclientprotocol/claude-agent-acp`). What differs per agent lives in + * its definition (`acp-agents.ts`); this module knows no agent by name. * - * Ported from canvas's `src/server/agents.ts` (canvas findings 01 and 04): - * `@agentclientprotocol/sdk` speaks ACP directly, because `chat()` + - * `acpCompatible` drops `session/new`'s `configOptions` and cannot call - * `session/set_config_option` — and that is the only way to pick a model in - * either agent. AG-UI chunks still come from `@tanstack/ai-acp`'s + * Ported from canvas's `src/server/agents.ts` (canvas findings 01, 04) and + * proven in finding 13. `@agentclientprotocol/sdk` speaks ACP directly, + * because `chat()` + `acpCompatible` drops `session/new`'s `configOptions` + * and cannot call `session/set_config_option` — the only way to pick a model + * in either agent. AG-UI chunks come from `@tanstack/ai-acp`'s * `translateAcpStream`, so stored chunks and the SPA stay AG-UI (ADR 0003 §2). * - * One agent process per `stream()` call: a factory step is one fresh session - * and one prompt, so canvas's idle timer and reconnect logic have no job here. - * Headless permission is answered here, by the client, for either agent: - * every ask is allowed (the #24 policy, without writing `opencode.json`). + * One agent process per `stream()` call (ADR 0013 §6): spawn, `initialize`, + * `session/new`, set the model, `session/prompt`, kill. A step is one fresh + * session and one prompt, so canvas's idle timer and reconnect have no job. + * + * - **Model** (§2): required. A model the agent does not offer fails the step + * before the prompt, naming the agent and some of the ids it does offer. + * - **Permission** (§4): every ask is allowed, `allow_always` before + * `allow_once` — #24's policy, without writing a file into the tree. + * - **Usage** (§5): each ACP `usage_update` becomes a `CUSTOM` `acp.usage` + * chunk carrying a `usage` signal, in stream order. + * - **Cancel** (§6): ACP `session/cancel`, then a kill after a grace period; + * always a kill in `finally`. + * - **Exit mid-turn**: the step fails with the agent's exit code and the tail + * of its stderr. */ import { @@ -36,19 +47,25 @@ import { type AcpSessionUpdate, type AcpStreamEvent, } from "@tanstack/ai-acp"; -import type { AgentAdapter, AgentAdapterOptions, AgentAdapterYield } from "./agent-adapter"; - -export type AcpAgentKind = "claude" | "opencode"; +import { resolve } from "node:path"; +import { agentEnv, type AcpAgentDefinition } from "./acp-agents"; +import type { + AgentAdapter, + AgentAdapterOptions, + AgentAdapterYield, + AgentUsage, +} from "./agent-adapter"; -export interface AcpAgentDefinition { - readonly kind: AcpAgentKind; - readonly command: ReadonlyArray; - readonly env?: Record; - /** Agent-specific `_meta` for `session/new`. */ - readonly sessionMeta?: Record; -} +/** CUSTOM chunk names this adapter emits. */ +export const ACP_CHUNK = { + sessionId: "acp.session-id", + content: "acp.message-content", + plan: "acp.plan", + usage: "acp.usage", + structuredOutput: "structured-output.complete", +} as const; -/** What the spike measures; a production adapter would not have this. */ +/** What happened on the wire, for the spike runner and tests; not persisted. */ export type AcpDiagnostic = | { readonly kind: "spawned"; readonly pid: number; readonly at: number } | { readonly kind: "initialized"; readonly at: number; readonly agentInfo: unknown } @@ -81,18 +98,23 @@ export type AcpDiagnostic = } | { readonly kind: "exited"; readonly at: number; readonly code: number | null }; -const claudeAcpBin = Bun.resolveSync( - "@agentclientprotocol/claude-agent-acp/dist/index.js", - import.meta.dir, -); +export interface AcpAdapterOptions { + /** Called for every `AcpDiagnostic`. */ + readonly onDiagnostic?: (event: AcpDiagnostic) => void; + /** How long a cancelled agent gets to settle its turn before it is killed. Default 2000. */ + readonly cancelGraceMs?: number; +} -export const ACP_AGENTS: Record = { - claude: { kind: "claude", command: ["bun", claudeAcpBin] }, - opencode: { kind: "opencode", command: ["opencode", "acp"] }, -}; +/** A failure the adapter itself detected: bad model, agent exit, protocol gap. */ +export class AcpAgentError extends Error { + override readonly name = "AcpAgentError"; +} + +const STDERR_KEEP = 8192; +const STDERR_TAIL_LINES = 12; /** Model choices an option offers, flattened out of its groups. */ -function choicesOf(option: SessionConfigOption): string[] { +export function choicesOf(option: SessionConfigOption): string[] { if (option.type !== "select") return []; return option.options.flatMap((entry) => "group" in entry ? entry.options.map((c) => c.value) : [entry.value], @@ -100,7 +122,7 @@ function choicesOf(option: SessionConfigOption): string[] { } /** Allow every ask, preferring "always" so the agent stops asking (#24). */ -function headlessAllow(request: RequestPermissionRequest): string | null { +export function headlessAllow(request: Pick): string | null { const pick = request.options.find((o) => o.kind === "allow_always") ?? request.options.find((o) => o.kind === "allow_once"); @@ -112,53 +134,143 @@ function headlessAllow(request: RequestPermissionRequest): string | null { * `plan` update; read it as one, so every agent has one kind of plan * (canvas `todo-plan.ts`). */ -function todoPlan(update: SessionUpdate): SessionUpdate | null { +export function todoPlan(update: SessionUpdate): SessionUpdate | null { if (update.sessionUpdate !== "tool_call" && update.sessionUpdate !== "tool_call_update") return null; const input = update.rawInput as { todos?: unknown } | null | undefined; if (!Array.isArray(input?.todos)) return null; const entries = input.todos.flatMap((todo: unknown): PlanEntry[] => { - const { content, status } = (todo ?? {}) as Record; + const { content, status, priority } = (todo ?? {}) as Record; if (typeof content !== "string") return []; if (status !== "pending" && status !== "in_progress" && status !== "completed") return []; - return [{ content, status, priority: "medium" }]; + return [ + { + content, + status, + priority: priority === "high" || priority === "low" ? priority : "medium", + }, + ]; }); return entries.length ? { sessionUpdate: "plan", entries } : null; } +/** An ACP `usage_update` as factory's `AgentUsage`. */ +export function usageOf(update: { + used: number; + size: number; + cost?: { amount: number; currency: string } | null; +}): AgentUsage { + return { + context: { used: update.used, size: update.size }, + ...(update.cost != null && { + cost: { amount: update.cost.amount, currency: update.cost.currency }, + }), + }; +} + +/** What the adapter's own queue carries: ACP events, plus usage, which AG-UI has no event for. */ +type Incoming = AcpStreamEvent | { readonly kind: "usage"; readonly usage: AgentUsage }; + +/** + * Drive `translateAcpStream` one input event at a time, so the adapter can + * put its own chunks (usage) between the translated ones in arrival order. + * `feed` hands the translator one event and returns what it yielded before + * asking for the next; `fail` and `end` close its input. + */ +function stepTranslator(translate: (input: AsyncIterable) => AsyncIterable) { + let give: { resolve: (r: IteratorResult) => void; reject: (e: unknown) => void }; + let asked!: () => void; + let askedNext = new Promise((resolve) => (asked = resolve)); + const source: AsyncIterable = { + [Symbol.asyncIterator]: () => ({ + next: () => + new Promise>((resolve, reject) => { + give = { resolve, reject }; + asked(); + }), + }), + }; + const output = translate(source)[Symbol.asyncIterator](); + let pending: Promise> = output.next(); + + /** + * Collect what the translator yields until it asks for input again, ends, + * or throws — keeping what it yielded before throwing (it closes open text + * and tool calls on the way out). + */ + async function drain(): Promise<{ chunks: O[]; done: boolean; error?: unknown }> { + const chunks: O[] = []; + for (;;) { + let result: IteratorResult | "asked"; + try { + result = await Promise.race([pending, askedNext.then(() => "asked" as const)]); + } catch (error) { + return { chunks, done: true, error }; + } + if (result === "asked") return { chunks, done: false }; + if (result.done === true) return { chunks, done: true }; + chunks.push(result.value); + pending = output.next(); + } + } + const handOver = async (send: () => void) => { + // Wait until the translator is actually waiting on its input. + await Promise.race([askedNext, pending]).catch(() => undefined); + askedNext = new Promise((resolve) => (asked = resolve)); + send(); + return drain(); + }; + return { + feed: (event: AcpStreamEvent) => handOver(() => give.resolve({ value: event, done: false })), + end: () => handOver(() => give.resolve({ value: undefined, done: true })), + fail: (cause: unknown) => handOver(() => give.reject(cause)), + }; +} + export function acpAdapter( definition: AcpAgentDefinition, - observe: (event: AcpDiagnostic) => void = () => {}, + adapterOptions: AcpAdapterOptions = {}, ): AgentAdapter { + const observe = adapterOptions.onDiagnostic ?? (() => {}); + const cancelGraceMs = adapterOptions.cancelGraceMs ?? 2000; + const name = definition.agent; + return { // Permission is answered by the client (`headlessAllow`); nothing to write. async prepareWorkspace(): Promise {}, async *stream(options: AgentAdapterOptions): AsyncIterable { - const child = Bun.spawn([...definition.command], { - cwd: options.dir, - env: { ...process.env, ...definition.env }, - stdin: "pipe", - stdout: "pipe", - stderr: "pipe", - }); + if (options.model.trim() === "") + throw new AcpAgentError(`${name}: no model given; factory always sends one`); + const signal = options.abortController.signal; + if (signal.aborted) return; + // ACP requires an absolute `cwd`; a workspace root may be relative + // (`.factory/workspaces`, the default), resolved against the daemon's. + const cwd = resolve(options.dir); + + let child: Bun.Subprocess<"pipe", "pipe", "pipe">; + try { + child = Bun.spawn([...definition.command], { + cwd, + env: agentEnv(definition), + stdin: "pipe", + stdout: "pipe", + stderr: "pipe", + }); + } catch (cause) { + throw new AcpAgentError( + `${name} did not start (${definition.command[0]}): ${String(cause)}`, + ); + } observe({ kind: "spawned", pid: child.pid, at: Date.now() }); let stderr = ""; - void (async () => { + const stderrDone = (async () => { for await (const chunk of child.stderr.pipeThrough(new TextDecoderStream())) - stderr = (stderr + chunk).slice(-4096); + stderr = (stderr + chunk).slice(-STDERR_KEEP); })().catch(() => undefined); - const queue = new AsyncQueue(); - const exited = child.exited.then((code) => { - observe({ kind: "exited", at: Date.now(), code }); - const tail = stderr.trim().split("\n").at(-1); - throw new Error(`${definition.kind} exited (code ${code})${tail ? `: ${tail}` : ""}`); - }); - exited.catch((cause: unknown) => queue.fail(cause)); - const race = (work: Promise) => Promise.race([work, exited]); - + const incoming = new AsyncQueue(); const acp = new ClientSideConnection( () => ({ requestPermission: async (request): Promise => { @@ -180,10 +292,13 @@ export function acpAdapter( sessionUpdate: async ({ update }) => { observe({ kind: "update", at: Date.now(), update }); if (update.sessionUpdate === "config_option_update") return; - if (update.sessionUpdate === "usage_update") return; - queue.push({ kind: "update", update: update as AcpSessionUpdate }); + if (update.sessionUpdate === "usage_update") { + incoming.push({ kind: "usage", usage: usageOf(update) }); + return; + } + incoming.push({ kind: "update", update: update as AcpSessionUpdate }); const plan = todoPlan(update); - if (plan) queue.push({ kind: "update", update: plan as AcpSessionUpdate }); + if (plan) incoming.push({ kind: "update", update: plan as AcpSessionUpdate }); }, }), ndJsonStream( @@ -198,19 +313,44 @@ export function acpAdapter( ), ); + // The agent is gone once it has exited and its stdout is read to the + // end — so a reply it wrote just before exiting still lands first. + let exitedCode: number | null | undefined; + const died: Promise = Promise.all([ + child.exited, + acp.closed.catch(() => undefined), + ]).then(async ([code]) => { + exitedCode = code; + observe({ kind: "exited", at: Date.now(), code }); + await Promise.race([stderrDone, Bun.sleep(200)]); + const tail = stderr.trim().split("\n").slice(-STDERR_TAIL_LINES).join("\n"); + return new AcpAgentError( + `${name} exited (code ${code ?? "signal"})${tail ? `; stderr:\n${tail}` : ""}`, + ); + }); + void died.then((error) => incoming.fail(error)); + const race = (work: Promise) => + Promise.race([work, died.then((error) => Promise.reject(error))]); + let sessionId: string | undefined; + let killTimer: ReturnType | undefined; const onAbort = () => { - if (sessionId !== undefined) void acp.cancel({ sessionId }).catch(() => undefined); + if (sessionId === undefined) { + child.kill(); + return; + } + void acp.cancel({ sessionId }).catch(() => undefined); // Give the agent a moment to settle the turn as `cancelled`, then make sure. - setTimeout(() => child.kill(), 2000).unref(); + killTimer = setTimeout(() => child.kill(), cancelGraceMs); }; - options.abortController.signal.addEventListener("abort", onAbort, { once: true }); + signal.addEventListener("abort", onAbort, { once: true }); try { const init = await race( acp.initialize({ protocolVersion: PROTOCOL_VERSION, clientInfo: { name: "factory", version: "0.0.0" }, + // The agent works in the tree directly; it never needs our fs. clientCapabilities: { fs: { readTextFile: false, writeTextFile: false } }, }), ); @@ -218,48 +358,54 @@ export function acpAdapter( const created = await race( acp.newSession({ - cwd: options.dir, + cwd, mcpServers: [], - ...(definition.sessionMeta && { _meta: definition.sessionMeta }), + ...(definition.sessionMeta && { _meta: { ...definition.sessionMeta } }), }), ); sessionId = created.sessionId; - let configOptions = created.configOptions ?? []; + const configOptions = created.configOptions ?? []; observe({ kind: "session", at: Date.now(), sessionId, configOptions }); + if (signal.aborted) return; - // The model decides which other options exist, so it goes first and - // fails loudly: a workflow asked for it, and running on another - // model silently is worse than not running. + // The model decides which other options exist, so it goes first, and + // it fails loudly: running silently on another model is worse than not + // running (ADR 0013 §2). const modelOption = configOptions.find((o) => o.category === "model" || o.id === "model"); - if (modelOption === undefined) throw new Error(`${definition.kind} offers no model option`); - if (modelOption.currentValue !== options.model) { - const choices = choicesOf(modelOption); - if (!choices.includes(options.model)) - throw new Error( - `${definition.kind} has no model "${options.model}" (${choices.length} offered, e.g. ${choices.slice(0, 8).join(", ")})`, - ); - const response = await race( - acp.setSessionConfigOption({ - sessionId, - configId: modelOption.id, - value: options.model, - }), + if (modelOption === undefined) + throw new AcpAgentError( + `${name} offers no model option; cannot select "${options.model}"`, ); - configOptions = response.configOptions; - observe({ - kind: "configured", - at: Date.now(), + // Set even when `currentValue` already matches: with host settings + // ignored, claude-agent-acp still reports the user's `model` setting + // as current while the Agent SDK runs its own default, so a match + // proves nothing. Only an explicit set pins the session. + const choices = choicesOf(modelOption); + if (!choices.includes(options.model) && modelOption.currentValue !== options.model) + throw new AcpAgentError( + `${name} has no model "${options.model}" (${choices.length} offered, e.g. ${choices.slice(0, 8).join(", ")})`, + ); + const response = await race( + acp.setSessionConfigOption({ + sessionId, configId: modelOption.id, value: options.model, - configOptions, - }); - } + }), + ); + observe({ + kind: "configured", + at: Date.now(), + configId: modelOption.id, + value: options.model, + configOptions: response.configOptions, + }); + if (signal.aborted) return; const prompt = options.outputSchema !== undefined ? appendOutputSchemaInstruction(options.prompt, options.outputSchema) : options.prompt; - queue.push({ kind: "session", sessionId }); + incoming.push({ kind: "session", sessionId }); void acp .prompt({ sessionId, prompt: [{ type: "text", text: prompt }] }) .then((response) => { @@ -269,46 +415,94 @@ export function acpAdapter( stopReason: response.stopReason, usage: response.usage, }); - queue.push({ + incoming.push({ kind: "done", stopReason: response.stopReason, ...(response.usage && { usage: response.usage }), }); - queue.end(); + incoming.end(); }) - .catch((cause: unknown) => queue.fail(cause)); + .catch(async (cause: unknown) => { + // A dying agent rejects the pending prompt first; prefer the exit + // (with its stderr) when that is what happened. + const exit = await Promise.race([died, Bun.sleep(500).then(() => undefined)]); + incoming.fail(exit ?? cause); + }); - const runId = crypto.randomUUID(); - const chunks = translateAcpStream(queue, { - model: options.model, - runId, - threadId: options.threadId, - genId: () => crypto.randomUUID(), - labels: { - sessionIdEvent: "acp.session-id", - contentEvent: "acp.message-content", - planEvent: "acp.plan", - }, - }); + const translator = stepTranslator((input) => + translateAcpStream(input, { + model: options.model, + runId: crypto.randomUUID(), + threadId: options.threadId, + genId: () => crypto.randomUUID(), + labels: { + sessionIdEvent: ACP_CHUNK.sessionId, + contentEvent: ACP_CHUNK.content, + planEvent: ACP_CHUNK.plan, + }, + }), + ); let lastText = ""; - let finished: unknown; - for await (const chunk of chunks) { - const record = chunk as { type: string; delta?: unknown; message?: unknown }; - if (record.type === "TEXT_MESSAGE_START") lastText = ""; - else if (record.type === "TEXT_MESSAGE_CONTENT" && typeof record.delta === "string") - lastText += record.delta; - if (record.type === "RUN_FINISHED" && options.outputSchema !== undefined) { - // Hold it: the structured output goes before the run's end. - finished = chunk; - continue; + let held: unknown; + const handle = function* (chunks: ReadonlyArray): Generator { + for (const chunk of chunks) { + const record = chunk as { + type: string; + name?: unknown; + delta?: unknown; + message?: unknown; + }; + if (record.type === "TEXT_MESSAGE_START") lastText = ""; + else if (record.type === "TEXT_MESSAGE_CONTENT" && typeof record.delta === "string") + lastText += record.delta; + if (record.type === "RUN_FINISHED" && options.outputSchema !== undefined) { + // Hold it: the structured output goes before the run's end. + held = chunk; + } else if (record.type === "CUSTOM" && record.name === ACP_CHUNK.sessionId) { + yield { chunk, signal: { _tag: "sessionId", value: sessionId! } }; + } else if (record.type === "RUN_ERROR") { + const message = typeof record.message === "string" ? record.message : "agent error"; + yield { chunk, signal: { _tag: "runError", value: message } }; + } else yield { chunk }; } - if (record.type === "CUSTOM" && (chunk as { name?: string }).name === "acp.session-id") { - yield { chunk, signal: { _tag: "sessionId", value: sessionId } }; - } else if (record.type === "RUN_ERROR") { - const message = typeof record.message === "string" ? record.message : "agent error"; - yield { chunk, signal: { _tag: "runError", value: message } }; - } else yield { chunk }; + }; + + let done = false; + try { + for await (const event of incoming) { + if (event.kind === "usage") { + yield { + chunk: { + type: "CUSTOM", + name: ACP_CHUNK.usage, + value: event.usage, + model: options.model, + timestamp: Date.now(), + }, + signal: { _tag: "usage", value: event.usage }, + }; + continue; + } + const step = await translator.feed(event); + yield* handle(step.chunks); + if (step.error !== undefined) throw step.error; + if (step.done) { + done = true; + break; + } + } + if (!done) { + const step = await translator.end(); + yield* handle(step.chunks); + if (step.error !== undefined) throw step.error; + } + } catch (cause) { + if (done) throw cause; + done = true; + // Let the translator close what it opened (text, tool calls), then fail. + yield* handle((await translator.fail(cause)).chunks); + throw cause; } if (options.outputSchema !== undefined) { @@ -317,8 +511,9 @@ export function acpAdapter( yield { chunk: { type: "CUSTOM", - name: "structured-output.complete", + name: ACP_CHUNK.structuredOutput, value: { object, raw: lastText }, + model: options.model, timestamp: Date.now(), }, signal: { _tag: "structuredOutput", value: object }, @@ -327,11 +522,19 @@ export function acpAdapter( // No JSON in the last message: the runtime's tier-2 re-parse of // `finalText` gets the same text and reports the miss. } - if (finished !== undefined) yield { chunk: finished }; + if (held !== undefined) yield { chunk: held }; } } finally { - options.abortController.signal.removeEventListener("abort", onAbort); - child.kill(); + signal.removeEventListener("abort", onAbort); + clearTimeout(killTimer); + if (exitedCode === undefined) { + child.kill(); + const gone = await Promise.race([ + child.exited.then(() => true), + Bun.sleep(cancelGraceMs).then(() => false), + ]); + if (!gone) child.kill("SIGKILL"); + } } }, }; diff --git a/src/runtime/acp-agents.test.ts b/src/runtime/acp-agents.test.ts new file mode 100644 index 0000000..cebc586 --- /dev/null +++ b/src/runtime/acp-agents.test.ts @@ -0,0 +1,161 @@ +import { describe, expect, test } from "bun:test"; +import { homedir } from "node:os"; +import { join } from "node:path"; +import { FactoryHostShellEnv } from "./acp-opencode-plugin"; +import { + agentEnv, + checkAgents, + claudeAgent, + claudeAuthProblem, + HOST_XDG_CONFIG_HOME, + OPENCODE_HOST_ENV_PLUGIN, + opencodeAgent, + opencodeIsolatedDirs, + scrubAgentEnv, +} from "./acp-agents"; + +describe("scrubAgentEnv", () => { + test("drops the parent's Claude Code session variables and keeps credentials", () => { + expect( + scrubAgentEnv({ + PATH: "/bin", + CLAUDECODE: "1", + CLAUDE_EFFORT: "high", + CLAUDE_CODE_SESSION_ID: "s", + CLAUDE_CODE_SUBAGENT_MODEL: "sonnet", + CLAUDE_CODE_MESSAGING_TOKEN: "t", + CLAUDE_CONFIG_DIR: "/c", + CLAUDE_CODE_OAUTH_TOKEN: "o", + ANTHROPIC_API_KEY: "k", + UNSET: undefined, + }), + ).toEqual({ + PATH: "/bin", + CLAUDE_CONFIG_DIR: "/c", + CLAUDE_CODE_OAUTH_TOKEN: "o", + ANTHROPIC_API_KEY: "k", + }); + }); +}); + +describe("agentEnv", () => { + test("scrubs, drops the definition's variables, then adds its own", () => { + const env = agentEnv( + { agent: "x", command: ["x"], dropEnv: ["OPENCODE_CONFIG"], env: { A: "new" } }, + { A: "old", OPENCODE_CONFIG: "/host.json", CLAUDECODE: "1", PATH: "/bin" }, + ); + expect(env).toEqual({ A: "new", PATH: "/bin" }); + }); +}); + +describe("claudeAgent", () => { + test("ignores host settings by default: settingSources without user", () => { + const agent = claudeAgent(); + expect(agent.agent).toBe("claude"); + expect(agent.command[0]).toBe(process.execPath); + expect(agent.command[1]).toEndWith("claude-agent-acp/dist/index.js"); + expect(agent.sessionMeta).toEqual({ + claudeCode: { options: { settingSources: ["project", "local"] } }, + }); + }); + + test("includes host settings when asked: no _meta, the agent's own defaults", () => { + expect(claudeAgent({ hostSettings: "include" }).sessionMeta).toBeUndefined(); + }); +}); + +describe("opencodeAgent", () => { + test("ignores host settings by default: config home and home hidden, plugin loaded", () => { + const agent = opencodeAgent(); + const { configHome, home } = opencodeIsolatedDirs(); + expect(agent.command).toEqual(["opencode", "acp"]); + expect(agent.env).toMatchObject({ + XDG_CONFIG_HOME: configHome, + OPENCODE_TEST_HOME: home, + [HOST_XDG_CONFIG_HOME]: process.env.XDG_CONFIG_HOME ?? join(homedir(), ".config"), + }); + expect(JSON.parse(agent.env!.OPENCODE_CONFIG_CONTENT!)).toEqual({ + plugin: [OPENCODE_HOST_ENV_PLUGIN], + }); + expect(agent.dropEnv).toEqual([ + "OPENCODE_CONFIG", + "OPENCODE_CONFIG_DIR", + "OPENCODE_CONFIG_CONTENT", + ]); + }); + + test("includes host settings when asked: plain `opencode acp`", () => { + expect(opencodeAgent({ hostSettings: "include" })).toEqual({ + agent: "opencode", + command: ["opencode", "acp"], + }); + }); + + test("the plugin file exists where the definition points", async () => { + expect(await Bun.file(new URL(OPENCODE_HOST_ENV_PLUGIN)).exists()).toBe(true); + }); +}); + +describe("the opencode host-env plugin", () => { + test("hands the host's XDG_CONFIG_HOME to shell commands", async () => { + const before = process.env[HOST_XDG_CONFIG_HOME]; + process.env[HOST_XDG_CONFIG_HOME] = "/home/someone/.config"; + try { + const hooks = await FactoryHostShellEnv(); + const output = { env: {} as Record }; + await hooks["shell.env"]({}, output); + expect(output.env).toEqual({ XDG_CONFIG_HOME: "/home/someone/.config" }); + } finally { + if (before === undefined) delete process.env[HOST_XDG_CONFIG_HOME]; + else process.env[HOST_XDG_CONFIG_HOME] = before; + } + }); + + test("leaves the shell alone outside factory", async () => { + const before = process.env[HOST_XDG_CONFIG_HOME]; + delete process.env[HOST_XDG_CONFIG_HOME]; + try { + const output = { env: {} as Record }; + await (await FactoryHostShellEnv())["shell.env"]({}, output); + expect(output.env).toEqual({}); + } finally { + if (before !== undefined) process.env[HOST_XDG_CONFIG_HOME] = before; + } + }); +}); + +describe("claudeAuthProblem", () => { + test("logged in, API key, or an external provider: no problem", () => { + expect(claudeAuthProblem('{"loggedIn":true,"apiProvider":"firstParty"}')).toBeUndefined(); + expect( + claudeAuthProblem( + '{"loggedIn":false,"apiKeySource":"ANTHROPIC_API_KEY","apiProvider":"firstParty"}', + ), + ).toBeUndefined(); + expect(claudeAuthProblem('{"loggedIn":false,"apiProvider":"bedrock"}')).toBeUndefined(); + }); + + test("logged out on the first-party backend is a problem", () => { + expect( + claudeAuthProblem('{"loggedIn":false,"authMethod":"none","apiProvider":"firstParty"}'), + ).toContain("Claude is not logged in"); + }); + + test("output it cannot read is not reported", () => { + expect(claudeAuthProblem("")).toBeUndefined(); + expect(claudeAuthProblem("not json")).toBeUndefined(); + expect(claudeAuthProblem('{"something":"else"}')).toBeUndefined(); + }); +}); + +describe("checkAgents", () => { + test("reports opencode missing from PATH", async () => { + expect(await checkAgents(["opencode"], { PATH: "/nonexistent" })).toEqual([ + { agent: "opencode", problem: "`opencode` is not on PATH" }, + ]); + }); + + test("an API key in the environment is enough for Claude", async () => { + expect(await checkAgents(["claude"], { ANTHROPIC_API_KEY: "sk-test", PATH: "" })).toEqual([]); + }); +}); diff --git a/src/runtime/acp-agents.ts b/src/runtime/acp-agents.ts new file mode 100644 index 0000000..8ca8b98 --- /dev/null +++ b/src/runtime/acp-agents.ts @@ -0,0 +1,257 @@ +/** + * The ACP agents factory knows how to launch (ADR 0013 §1, §3). Every + * agent-specific detail lives in a definition: the command, the environment, + * the `_meta` for `session/new`, and how the host's own agent settings are + * kept out of a run. The adapter (`acp-adapter.ts`) is generic; a third ACP + * agent is a third definition. + * + * Host settings (ADR 0013 §3, finding 13 addendum): with `hostSettings: + * "ignore"` (the default) a run reads the project's agent configuration and + * not the operator's. Credentials are not settings and stay available. + * + * - claude: `settingSources: ["project", "local"]` drops `~/.claude`'s + * settings, skills and `CLAUDE.md` from the Agent SDK. claude-agent-acp + * still reads the user's `model`, `permissions.defaultMode` and + * `availableModels` itself, which is why factory always sends a model and + * answers every permission ask. + * - opencode: see `opencodeAgent`. In short, its global config dir and its + * home-level reads point at empty factory-owned directories, and a factory + * plugin hands the host's `XDG_CONFIG_HOME` back to the shell commands it + * runs, so `gh`, git and other tools keep their config. + */ + +import { mkdirSync } from "node:fs"; +import { homedir, tmpdir, userInfo } from "node:os"; +import { join } from "node:path"; +import { pathToFileURL } from "node:url"; + +/** The agents factory ships definitions for. */ +export const ACP_AGENT_KINDS = ["claude", "opencode"] as const; +export type AcpAgentKind = (typeof ACP_AGENT_KINDS)[number]; + +/** Whether a run sees the operator's own agent settings (ADR 0013 §3). */ +export type HostSettings = "ignore" | "include"; + +export interface AcpAgentDefinition { + /** The agent's name in errors and logs. */ + readonly agent: string; + /** The launch command; the agent speaks ACP on stdin/stdout. */ + readonly command: ReadonlyArray; + /** Added to the (scrubbed) environment. */ + readonly env?: Readonly>; + /** Removed from the inherited environment, beside the `CLAUDE_*` scrub. */ + readonly dropEnv?: ReadonlyArray; + /** Agent-specific `_meta` for `session/new`. */ + readonly sessionMeta?: Readonly>; +} + +export interface AcpAgentOptions { + /** Default `"ignore"`. */ + readonly hostSettings?: HostSettings; +} + +/** + * Inherited variables an agent must not see: a daemon started from inside a + * Claude Code session would otherwise pass its effort, subagent model, + * session ids and messaging socket down to every agent it runs (finding 13 + * addendum). Kept: the ones that say where Claude's credentials are, or which + * provider pays — credentials are not settings. + */ +const CLAUDE_ENV_KEEP = new Set([ + "CLAUDE_CONFIG_DIR", + "CLAUDE_CODE_OAUTH_TOKEN", + "CLAUDE_CODE_USE_BEDROCK", + "CLAUDE_CODE_USE_VERTEX", +]); + +/** `env` without the parent's Claude Code session variables. */ +export function scrubAgentEnv( + env: Readonly>, +): Record { + const out: Record = {}; + for (const [key, value] of Object.entries(env)) { + if (value === undefined) continue; + if (key === "CLAUDECODE") continue; + if (key.startsWith("CLAUDE_") && !CLAUDE_ENV_KEEP.has(key)) continue; + out[key] = value; + } + return out; +} + +/** The environment an agent process starts with. */ +export function agentEnv( + definition: AcpAgentDefinition, + parent: Readonly> = process.env, +): Record { + const env = scrubAgentEnv(parent); + for (const key of definition.dropEnv ?? []) delete env[key]; + return { ...env, ...definition.env }; +} + +let claudeAcpEntry: string | undefined; +/** claude-agent-acp's entry point, bundled as a dependency. */ +export function claudeAcpBin(): string { + claudeAcpEntry ??= Bun.resolveSync( + "@agentclientprotocol/claude-agent-acp/dist/index.js", + import.meta.dir, + ); + return claudeAcpEntry; +} + +/** Claude Code through Anthropic's claude-agent-acp (bundled). */ +export function claudeAgent(options: AcpAgentOptions = {}): AcpAgentDefinition { + const ignore = (options.hostSettings ?? "ignore") === "ignore"; + return { + agent: "claude", + // Run it on the daemon's own bun: `bun` need not be on PATH. + command: [process.execPath, claudeAcpBin()], + ...(ignore && { + sessionMeta: { claudeCode: { options: { settingSources: ["project", "local"] } } }, + }), + }; +} + +/** + * The empty, factory-owned directories opencode sees as its config home and + * its home when the host's are hidden. opencode creates `opencode/` under the + * config home at start; nothing writes settings into either. + */ +export function opencodeIsolatedDirs(): { readonly configHome: string; readonly home: string } { + const root = join(tmpdir(), `factory-${userInfo().username}`, "opencode-isolated"); + return { configHome: join(root, "config"), home: join(root, "home") }; +} + +/** The opencode plugin that hands the host's `XDG_CONFIG_HOME` back to shell commands. */ +export const OPENCODE_HOST_ENV_PLUGIN = pathToFileURL( + join(import.meta.dir, "acp-opencode-plugin.ts"), +).href; + +/** The variable the plugin reads the host's value from. */ +export const HOST_XDG_CONFIG_HOME = "FACTORY_HOST_XDG_CONFIG_HOME"; + +/** + * opencode's own ACP server (`opencode acp`), from PATH. + * + * With host settings ignored (finding 13, addendum 2): + * - `XDG_CONFIG_HOME` → an empty directory hides `~/.config/opencode` + * (config, providers, plugins, global `AGENTS.md`, skills, agents, + * commands). opencode reads the variable once at start. Provider logins + * live in the data dir (`~/.local/share/opencode/auth.json`) and stay. + * `OPENCODE_CONFIG_DIR` cannot do this: it adds a directory beside the + * global one. + * - the host-env plugin (`acp-opencode-plugin.ts`, through + * `OPENCODE_CONFIG_CONTENT`) restores the host's `XDG_CONFIG_HOME` (or its + * default, `~/.config`) for every shell command, so `gh` keeps its auth and + * git its global config. + * - `OPENCODE_TEST_HOME` → an empty directory hides what opencode reads from + * the home directory itself: `~/.claude/skills`, `~/.agents/skills`, + * `~/.claude/CLAUDE.md` and `~/.opencode`. It changes only opencode's own + * notion of home (not `HOME`, not the data dir); the project's + * `.claude/skills`, `.agents/skills`, `AGENTS.md` and `CLAUDE.md` stay. It + * is an undocumented (test) variable: should opencode drop it, the host's + * skills reappear in runs, nothing breaks. `OPENCODE_DISABLE_CLAUDE_CODE*` + * is no substitute: it also drops the project's Claude files and keeps + * `~/.agents/skills`. + * - the host's `OPENCODE_CONFIG*` variables are dropped: they are settings. + */ +export function opencodeAgent(options: AcpAgentOptions = {}): AcpAgentDefinition { + const ignore = (options.hostSettings ?? "ignore") === "ignore"; + if (!ignore) return { agent: "opencode", command: ["opencode", "acp"] }; + const { configHome, home } = opencodeIsolatedDirs(); + mkdirSync(configHome, { recursive: true }); + mkdirSync(home, { recursive: true }); + return { + agent: "opencode", + command: ["opencode", "acp"], + dropEnv: ["OPENCODE_CONFIG", "OPENCODE_CONFIG_DIR", "OPENCODE_CONFIG_CONTENT"], + env: { + XDG_CONFIG_HOME: configHome, + OPENCODE_TEST_HOME: home, + [HOST_XDG_CONFIG_HOME]: process.env.XDG_CONFIG_HOME ?? join(homedir(), ".config"), + OPENCODE_CONFIG_CONTENT: JSON.stringify({ plugin: [OPENCODE_HOST_ENV_PLUGIN] }), + }, + }; +} + +/** The definition for one of factory's agents. */ +export function acpAgent(kind: AcpAgentKind, options: AcpAgentOptions = {}): AcpAgentDefinition { + return kind === "claude" ? claudeAgent(options) : opencodeAgent(options); +} + +/** Something that keeps a configured agent from running on this host. */ +export interface AgentProblem { + readonly agent: AcpAgentKind; + readonly problem: string; +} + +/** + * Which of `agents` cannot run on this host, so the daemon can say so at + * start rather than fail the first step (ADR 0013 Consequences). opencode + * needs `opencode` on PATH; Claude needs a login or an API key, which the + * bundled CLI's `auth status` reports. When the status cannot be read the + * agent is not reported: only a known problem is one. + */ +export async function checkAgents( + agents: ReadonlyArray, + env: Readonly> = process.env, +): Promise { + const problems: AgentProblem[] = []; + for (const agent of new Set(agents)) { + const problem = agent === "opencode" ? checkOpencode(env) : await checkClaude(env); + if (problem !== undefined) problems.push({ agent, problem }); + } + return problems; +} + +function checkOpencode(env: Readonly>): string | undefined { + return Bun.which("opencode", { PATH: env.PATH ?? "" }) === null + ? "`opencode` is not on PATH" + : undefined; +} + +async function checkClaude( + env: Readonly>, +): Promise { + let bin: string; + try { + bin = claudeAcpBin(); + } catch { + return "@agentclientprotocol/claude-agent-acp is not installed"; + } + if (env.ANTHROPIC_API_KEY) return undefined; + const child = Bun.spawn([process.execPath, bin, "--cli", "auth", "status", "--json"], { + env: scrubAgentEnv(env), + stdin: "ignore", + stdout: "pipe", + stderr: "ignore", + }); + const timer = setTimeout(() => child.kill(), 10_000); + try { + const stdout = await new Response(child.stdout).text(); + await child.exited; + return claudeAuthProblem(stdout); + } catch { + return undefined; + } finally { + clearTimeout(timer); + } +} + +/** + * Read `claude auth status --json`. Logged out means: no claude.ai login, no + * API key from any source, and the first-party backend (Bedrock, Vertex, … + * keep their credentials outside Claude, so `loggedIn` is false there). + */ +export function claudeAuthProblem(stdout: string): string | undefined { + let status: { loggedIn?: unknown; apiKeySource?: unknown; apiProvider?: unknown }; + try { + status = JSON.parse(stdout) as typeof status; + } catch { + return undefined; + } + if (typeof status !== "object" || status === null || typeof status.loggedIn !== "boolean") + return undefined; + const external = typeof status.apiProvider === "string" && status.apiProvider !== "firstParty"; + if (status.loggedIn || status.apiKeySource || external) return undefined; + return "Claude is not logged in (run `claude auth login`, or set ANTHROPIC_API_KEY)"; +} diff --git a/src/runtime/acp-opencode-plugin.ts b/src/runtime/acp-opencode-plugin.ts new file mode 100644 index 0000000..a7edf69 --- /dev/null +++ b/src/runtime/acp-opencode-plugin.ts @@ -0,0 +1,23 @@ +/** + * An opencode plugin factory loads into every opencode agent whose host + * settings are ignored (`acp-agents.ts`, `opencodeAgent`). + * + * opencode is started with `XDG_CONFIG_HOME` pointing at an empty directory, + * which hides `~/.config/opencode`. Every process opencode spawns would + * inherit that, and `gh` (auth in `~/.config/gh`), git and other XDG tools + * would lose their config with it. opencode reads `XDG_CONFIG_HOME` once at + * start, so this hook can hand the host's value back to each shell command + * (opencode's `shell.env` plugin hook) without un-hiding opencode's own + * config. + * + * Loaded by opencode from its file URL; it must not import anything. + */ + +type ShellEnvOutput = { env: Record }; + +export const FactoryHostShellEnv = async () => ({ + "shell.env": async (_input: unknown, output: ShellEnvOutput) => { + const host = process.env.FACTORY_HOST_XDG_CONFIG_HOME; + if (host) output.env.XDG_CONFIG_HOME = host; + }, +}); diff --git a/src/runtime/agent-adapter.ts b/src/runtime/agent-adapter.ts index fe8776b..64f297f 100644 --- a/src/runtime/agent-adapter.ts +++ b/src/runtime/agent-adapter.ts @@ -27,11 +27,25 @@ export interface AgentAdapterOptions { * - `sessionId` → `AgentStepFinished.sessionId` * - `structuredOutput` → `AgentStepFinished.output` (via `resolveOutput`) * - `runError` → `AgentStepFinished.error` + * - `usage` → `AgentStepFinished.context` / `.cost` (ADR 0013 §5; recorded + * from #65 on — until then the runtime reads past it) */ export type AgentSignal = | { readonly _tag: "sessionId"; readonly value: string } | { readonly _tag: "structuredOutput"; readonly value: unknown } - | { readonly _tag: "runError"; readonly value: string }; + | { readonly _tag: "runError"; readonly value: string } + | { readonly _tag: "usage"; readonly value: AgentUsage }; + +/** + * What an agent reports about its context window and spend (ACP + * `usage_update`), as the agent measured it. The last one of a step stands. + */ +export interface AgentUsage { + /** Tokens in context now, and the window size. */ + readonly context: { readonly used: number; readonly size: number }; + /** What the session has cost so far; absent when the agent does not say. */ + readonly cost?: { readonly amount: number; readonly currency: string }; +} /** * One item yielded by an adapter: the opaque AG-UI chunk (unchanged) plus diff --git a/src/runtime/agent-step.ts b/src/runtime/agent-step.ts index 4df2d64..4c6ea27 100644 --- a/src/runtime/agent-step.ts +++ b/src/runtime/agent-step.ts @@ -68,7 +68,12 @@ function readUsage(value: unknown): AgentStepUsage | undefined { export class AgentStepChunkError extends Schema.TaggedError()( "AgentStepChunkError", { cause: Schema.Defect() }, -) {} +) { + /** The adapter's own message, so a failed step's `error` leads with it. */ + override get message(): string { + return this.cause instanceof Error ? this.cause.message : String(this.cause); + } +} export interface AgentStepEffectOptions { readonly threadId: string; @@ -237,6 +242,9 @@ export function buildAgentStepEffect(options: AgentStepEffectOptions): AgentStep case "runError": runError = yieldItem.signal.value; break; + case "usage": + // No field yet: `AgentStepFinished.context` / `.cost` arrive with #65. + break; } } diff --git a/test/fixtures/fake-acp-agent.ts b/test/fixtures/fake-acp-agent.ts new file mode 100644 index 0000000..9c60dcb --- /dev/null +++ b/test/fixtures/fake-acp-agent.ts @@ -0,0 +1,175 @@ +/** + * A fake ACP agent over stdio for `acp-adapter.test.ts`, built on the SDK's + * `AgentSideConnection` — the same protocol opencode and claude-agent-acp + * speak, with behaviour chosen by the prompt's first word: + * + * - `hello`: usage, a text reply naming the session's model, usage with cost. + * - `permission`: asks to edit a file and replies with the option it got. + * - `json`: replies with a JSON object (structured output). + * - `env`: replies with the names of its `CLAUDE*` environment variables. + * - `hang`: works until cancelled, then ends the turn as `cancelled`. + * - `deaf`: works forever and ignores `session/cancel`. + * - `crash`: writes to stderr and exits with code 3 mid-turn. + * - `refuse`: ends the turn with `stopReason: "refusal"`. + * + * `FAKE_ACP_NO_MODEL=1` offers no model option. + */ + +import { + AgentSideConnection, + ndJsonStream, + PROTOCOL_VERSION, + RequestError, + type SessionConfigOption, +} from "@agentclientprotocol/sdk"; +import { isAbsolute } from "node:path"; +import { Writable } from "node:stream"; + +const MODELS = ["fake/default", "fake/fast", "fake/smart"]; + +const sessions = new Map void; wasCancelled: boolean }>(); + +function modelOption(current: string): SessionConfigOption { + return { + id: "model", + name: "Model", + category: "model", + type: "select", + currentValue: current, + options: [ + { group: "fake", name: "Fake", options: MODELS.map((value) => ({ value, name: value })) }, + ], + }; +} + +const connection = new AgentSideConnection( + (conn) => { + const say = (sessionId: string, text: string) => + conn.sessionUpdate({ + sessionId, + update: { sessionUpdate: "agent_message_chunk", content: { type: "text", text } }, + }); + const usage = (sessionId: string, used: number, cost?: number) => + conn.sessionUpdate({ + sessionId, + update: { + sessionUpdate: "usage_update", + used, + size: 200_000, + ...(cost !== undefined && { cost: { amount: cost, currency: "USD" } }), + }, + }); + return { + initialize: async () => ({ + protocolVersion: PROTOCOL_VERSION, + agentCapabilities: { loadSession: false }, + agentInfo: { name: "fake-acp-agent", version: "0.0.0" }, + }), + newSession: async ({ cwd }) => { + // As claude-agent-acp does: ACP's `cwd` is an absolute path. + if (!isAbsolute(cwd)) + throw RequestError.invalidParams({ cwd }, `\`cwd\` must be an absolute path, but received: ${cwd}`); + const sessionId = crypto.randomUUID(); + sessions.set(sessionId, { model: MODELS[0]!, cancelled: () => {}, wasCancelled: false }); + return { + sessionId, + configOptions: process.env.FAKE_ACP_NO_MODEL ? [] : [modelOption(MODELS[0]!)], + }; + }, + setSessionConfigOption: async (params) => { + const session = sessions.get(params.sessionId)!; + if (params.configId === "model" && typeof params.value === "string") + session.model = params.value; + return { configOptions: [modelOption(session.model)] }; + }, + authenticate: async () => ({}), + cancel: async ({ sessionId }) => { + const session = sessions.get(sessionId); + if (session) { + session.wasCancelled = true; + session.cancelled(); + } + }, + prompt: async ({ sessionId, prompt }) => { + const session = sessions.get(sessionId)!; + const text = prompt.map((block) => (block.type === "text" ? block.text : "")).join(""); + const verb = text.trim().split(/\s+/)[0]; + switch (verb) { + case "hello": + await usage(sessionId, 1000); + await say(sessionId, `hello from ${session.model}`); + await usage(sessionId, 1200, 0.25); + return { + stopReason: "end_turn", + usage: { inputTokens: 10, outputTokens: 5, cachedReadTokens: 990, totalTokens: 1005 }, + }; + case "permission": { + await conn.sessionUpdate({ + sessionId, + update: { + sessionUpdate: "tool_call", + toolCallId: "t1", + title: "Edit math.ts", + kind: "edit", + status: "pending", + }, + }); + const answer = await conn.requestPermission({ + sessionId, + toolCall: { toolCallId: "t1", title: "Edit math.ts" }, + options: [ + { optionId: "once", name: "Allow once", kind: "allow_once" }, + { optionId: "always", name: "Always allow", kind: "allow_always" }, + { optionId: "no", name: "Reject", kind: "reject_once" }, + ], + }); + const chosen = + answer.outcome.outcome === "selected" ? answer.outcome.optionId : "cancelled"; + await conn.sessionUpdate({ + sessionId, + update: { sessionUpdate: "tool_call_update", toolCallId: "t1", status: "completed" }, + }); + await say(sessionId, `permission: ${chosen}`); + return { stopReason: "end_turn" }; + } + case "json": + await say(sessionId, '{"summary":"done","count":2}'); + return { stopReason: "end_turn" }; + case "env": + await say( + sessionId, + Object.keys(process.env) + .filter((k) => k.startsWith("CLAUDE")) + .sort() + .join(","), + ); + return { stopReason: "end_turn" }; + case "hang": + await say(sessionId, "working"); + await new Promise((resolve) => { + session.cancelled = resolve; + if (session.wasCancelled) resolve(); + }); + return { stopReason: "cancelled" }; + case "deaf": + await say(sessionId, "working"); + await new Promise(() => {}); + return { stopReason: "end_turn" }; + case "crash": + await say(sessionId, "about to fail"); + process.stderr.write("starting up\nfatal: the fake agent fell over\n"); + setTimeout(() => process.exit(3), 20); + await new Promise(() => {}); + return { stopReason: "end_turn" }; + case "refuse": + return { stopReason: "refusal" }; + default: + throw new Error(`fake agent: unknown prompt ${JSON.stringify(verb)}`); + } + }, + }; + }, + ndJsonStream(Writable.toWeb(process.stdout) as WritableStream, Bun.stdin.stream()), +); + +await connection.closed;