From 4badbbf006e74dd2b0f305f14e3aaf0b4761c06f Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Fri, 24 Jul 2026 13:39:53 -0700 Subject: [PATCH 1/4] feat(cli): add headless CLI for one-shot agent runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds packages/cli (@posthog/code-cli, bin posthog-code-cli): run one agent turn against a local repo from the shell — prompt in, assistant output on stdout, exit code out. Supports --permission-mode auto (default) and bypassPermissions, --output text|json, --model, --system-prompt, and stdin prompts. Auth comes from ANTHROPIC_* env vars; question tool calls are parked with the same no-user-available contract as cloud background runs. ClaudeAcpAgent now accepts an injected logger (falling back to its console logger), letting hosts route adapter diagnostics off stdout. Generated-By: PostHog Code Task-Id: 97a6265f-b609-4775-9949-887aca895991 --- AGENTS.md | 2 +- knip.json | 4 + packages/agent/src/adapters/acp-connection.ts | 1 + .../agent/src/adapters/claude/claude-agent.ts | 6 +- packages/cli/e2e/cli.e2e.test.ts | 82 ++++++++ packages/cli/package.json | 33 ++++ packages/cli/src/args.test.ts | 145 ++++++++++++++ packages/cli/src/args.ts | 143 ++++++++++++++ packages/cli/src/cli.ts | 54 ++++++ packages/cli/src/output.test.ts | 183 ++++++++++++++++++ packages/cli/src/output.ts | 61 ++++++ packages/cli/src/permission-policy.test.ts | 148 ++++++++++++++ packages/cli/src/permission-policy.ts | 41 ++++ packages/cli/src/run.ts | 118 +++++++++++ packages/cli/tsconfig.json | 17 ++ packages/cli/tsup.config.ts | 11 ++ packages/cli/vitest.config.ts | 12 ++ packages/cli/vitest.e2e.config.ts | 18 ++ pnpm-lock.yaml | 28 +++ 19 files changed, 1105 insertions(+), 2 deletions(-) create mode 100644 packages/cli/e2e/cli.e2e.test.ts create mode 100644 packages/cli/package.json create mode 100644 packages/cli/src/args.test.ts create mode 100644 packages/cli/src/args.ts create mode 100644 packages/cli/src/cli.ts create mode 100644 packages/cli/src/output.test.ts create mode 100644 packages/cli/src/output.ts create mode 100644 packages/cli/src/permission-policy.test.ts create mode 100644 packages/cli/src/permission-policy.ts create mode 100644 packages/cli/src/run.ts create mode 100644 packages/cli/tsconfig.json create mode 100644 packages/cli/tsup.config.ts create mode 100644 packages/cli/vitest.config.ts create mode 100644 packages/cli/vitest.e2e.config.ts diff --git a/AGENTS.md b/AGENTS.md index 9ec5ba9418..0bdd711ada 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -37,7 +37,7 @@ Hosts: - `apps/code`: Electron desktop host. - `apps/web`: web host and portability smoke test. - `apps/mobile`: React Native host. -- `apps/cli`: thin shell over `@posthog/cli`. +- `packages/cli`: headless CLI (`@posthog/code-cli`, bin `posthog-code-cli`) — one-shot agent runs over the in-process ACP connection. ## Rules diff --git a/knip.json b/knip.json index 6dc2881c40..677f408831 100644 --- a/knip.json +++ b/knip.json @@ -34,6 +34,10 @@ "project": ["src/**/*.ts", "bin/**/*.ts"], "includeEntryExports": true }, + "packages/cli": { + "project": ["src/**/*.ts"], + "includeEntryExports": true + }, "packages/agent": { "project": ["src/**/*.ts"], "ignore": ["src/templates/**"], diff --git a/packages/agent/src/adapters/acp-connection.ts b/packages/agent/src/adapters/acp-connection.ts index d04c1fef8e..8e13611f8f 100644 --- a/packages/agent/src/adapters/acp-connection.ts +++ b/packages/agent/src/adapters/acp-connection.ts @@ -119,6 +119,7 @@ function createClaudeConnection(config: AcpConnectionConfig): AcpConnection { onStructuredOutput: config.onStructuredOutput, posthogApiConfig: resolveEnricherApiConfig(config), gatewayEnv: config.claudeGatewayEnv, + logger: config.logger?.child("ClaudeAcpAgent"), }); return agent; }, agentStream); diff --git a/packages/agent/src/adapters/claude/claude-agent.ts b/packages/agent/src/adapters/claude/claude-agent.ts index e07a2c404f..c9b26e99ca 100644 --- a/packages/agent/src/adapters/claude/claude-agent.ts +++ b/packages/agent/src/adapters/claude/claude-agent.ts @@ -256,6 +256,8 @@ export interface ClaudeAcpAgentOptions { posthogApiConfig?: PostHogAPIConfig; /** Explicit gateway config — avoids global process.env mutation across concurrent sessions. */ gatewayEnv?: GatewayEnv; + /** Injected logger; defaults to a console logger with debug enabled. */ + logger?: Logger; } export class ClaudeAcpAgent extends BaseAcpAgent { @@ -279,7 +281,9 @@ export class ClaudeAcpAgent extends BaseAcpAgent { this.toolUseCache = {}; this.emittedToolCalls = new Set(); this.toolUseStreamCache = new Map(); - this.logger = new Logger({ debug: true, prefix: "[ClaudeAcpAgent]" }); + this.logger = + options?.logger ?? + new Logger({ debug: true, prefix: "[ClaudeAcpAgent]" }); this.enrichment = createEnrichment(options?.posthogApiConfig, this.logger); } diff --git a/packages/cli/e2e/cli.e2e.test.ts b/packages/cli/e2e/cli.e2e.test.ts new file mode 100644 index 0000000000..7a06446881 --- /dev/null +++ b/packages/cli/e2e/cli.e2e.test.ts @@ -0,0 +1,82 @@ +import { execFile, execFileSync } from "node:child_process"; +import { + existsSync, + mkdtempSync, + realpathSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; +import { promisify } from "node:util"; +import { describe, expect, it } from "vitest"; + +const execFileAsync = promisify(execFile); + +// Same env contract as packages/agent/e2e/config.ts: a local llm-gateway plus +// a personal API key. Without the token the suite self-skips (never silent). +const GATEWAY_URL = + process.env.POSTHOG_CODE_E2E_GATEWAY_URL || "http://localhost:3308/ci"; +const TOKEN = process.env.POSTHOG_CODE_E2E_GATEWAY_PERSONAL_API_KEY ?? ""; +const MODEL = process.env.POSTHOG_CODE_E2E_CLAUDE_MODEL || "claude-haiku-4-5"; + +const CLI_PATH = resolve(__dirname, "../dist/cli.js"); + +function setupRepo(): string { + // realpath: on macOS os.tmpdir() is a symlink and the SDK keys its session + // store by resolved path. + const repo = realpathSync(mkdtempSync(join(tmpdir(), "code-cli-e2e-"))); + writeFileSync(join(repo, "hello.txt"), "hello\n"); + execFileSync("git", ["init", "-q"], { cwd: repo }); + execFileSync("git", ["add", "-A"], { cwd: repo }); + execFileSync( + "git", + [ + "-c", + "commit.gpgsign=false", + "-c", + "user.email=e2e@posthog.dev", + "-c", + "user.name=e2e", + "commit", + "-qm", + "init", + ], + { cwd: repo }, + ); + return repo; +} + +describe.skipIf(!TOKEN)("posthog-code-cli", () => { + it("runs one turn and prints the answer", async () => { + expect( + existsSync(CLI_PATH), + `CLI not built at ${CLI_PATH} — run pnpm --filter @posthog/code-cli build first`, + ).toBe(true); + const repo = setupRepo(); + try { + const { stdout } = await execFileAsync( + process.execPath, + [ + CLI_PATH, + "Reply with exactly the word OK and nothing else.", + "--cwd", + repo, + "--model", + MODEL, + ], + { + env: { + ...process.env, + ANTHROPIC_BASE_URL: GATEWAY_URL, + ANTHROPIC_AUTH_TOKEN: TOKEN, + }, + timeout: 240_000, + }, + ); + expect(stdout).toContain("OK"); + } finally { + rmSync(repo, { recursive: true, force: true }); + } + }); +}); diff --git a/packages/cli/package.json b/packages/cli/package.json new file mode 100644 index 0000000000..1cd93ed393 --- /dev/null +++ b/packages/cli/package.json @@ -0,0 +1,33 @@ +{ + "name": "@posthog/code-cli", + "version": "0.0.0-dev", + "private": true, + "description": "Headless CLI for PostHog Code — one-shot agent runs over the in-process ACP connection", + "type": "module", + "bin": { + "posthog-code-cli": "./dist/cli.js" + }, + "scripts": { + "build": "node ../../scripts/rimraf.mjs dist && tsup", + "typecheck": "pnpm exec tsc --noEmit", + "test": "vitest run", + "test:watch": "vitest", + "test:e2e": "vitest run --config vitest.e2e.config.ts", + "clean": "node ../../scripts/rimraf.mjs dist .turbo" + }, + "engines": { + "node": ">=20.0.0" + }, + "dependencies": { + "@agentclientprotocol/sdk": "1.1.0", + "@posthog/agent": "workspace:*", + "@posthog/shared": "workspace:*", + "commander": "^14.0.2" + }, + "devDependencies": { + "@types/node": "catalog:", + "tsup": "catalog:", + "typescript": "catalog:", + "vitest": "^4.1.8" + } +} diff --git a/packages/cli/src/args.test.ts b/packages/cli/src/args.test.ts new file mode 100644 index 0000000000..00096764a0 --- /dev/null +++ b/packages/cli/src/args.test.ts @@ -0,0 +1,145 @@ +import { mkdtempSync, realpathSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import { parseCliArgs } from "./args"; + +// argv passed to parseCliArgs mirrors process.argv: [node, script, ...args]. +function argv(...args: string[]): string[] { + return ["/usr/bin/node", "/path/to/posthog-code-cli", ...args]; +} + +describe("parseCliArgs", () => { + describe("defaults", () => { + it("returns default options when no flags or prompt are given", () => { + const result = parseCliArgs(argv()); + + expect("error" in result).toBe(false); + if ("error" in result) return; + expect(result.prompt).toBeUndefined(); + expect(result.cwd).toBe(realpathSync(process.cwd())); + expect(result.permissionMode).toBe("auto"); + expect(result.output).toBe("text"); + expect(result.debug).toBe(false); + }); + }); + + it("parses the prompt positional", () => { + const result = parseCliArgs(argv("Fix the failing test")); + + expect("error" in result).toBe(false); + if ("error" in result) return; + expect(result.prompt).toBe("Fix the failing test"); + }); + + describe("individual flags", () => { + it("parses --cwd to the realpath of a valid directory", () => { + const tmp = mkdtempSync(join(tmpdir(), "cli-args-test-")); + const result = parseCliArgs(argv("--cwd", tmp)); + + expect("error" in result).toBe(false); + if ("error" in result) return; + expect(result.cwd).toBe(realpathSync(tmp)); + }); + + it("parses --permission-mode bypassPermissions", () => { + const result = parseCliArgs( + argv("--permission-mode", "bypassPermissions"), + ); + + expect("error" in result).toBe(false); + if ("error" in result) return; + expect(result.permissionMode).toBe("bypassPermissions"); + }); + + it("parses a valid --model id", () => { + const result = parseCliArgs(argv("--model", "claude-opus-4")); + + expect("error" in result).toBe(false); + if ("error" in result) return; + expect(result.model).toBe("claude-opus-4"); + }); + + it("parses --system-prompt as-is", () => { + const result = parseCliArgs( + argv("--system-prompt", "Respond only in haiku"), + ); + + expect("error" in result).toBe(false); + if ("error" in result) return; + expect(result.systemPrompt).toBe("Respond only in haiku"); + }); + + it("parses --output json", () => { + const result = parseCliArgs(argv("--output", "json")); + + expect("error" in result).toBe(false); + if ("error" in result) return; + expect(result.output).toBe("json"); + }); + + it("parses --debug as true when present", () => { + const result = parseCliArgs(argv("--debug")); + + expect("error" in result).toBe(false); + if ("error" in result) return; + expect(result.debug).toBe(true); + }); + }); + + describe("invalid --permission-mode values", () => { + it.each(["default", "plan", "acceptEdits", "yolo"])( + "rejects %j with an error result", + (mode) => { + const result = parseCliArgs(argv("--permission-mode", mode)); + + expect("error" in result).toBe(true); + if (!("error" in result)) return; + expect(typeof result.error).toBe("string"); + expect(result.error.length).toBeGreaterThan(0); + }, + ); + }); + + describe("invalid --model prefixes", () => { + it.each(["gpt-4", "llama-3"])( + "rejects %j since it does not start with claude-", + (model) => { + const result = parseCliArgs(argv("--model", model)); + + expect("error" in result).toBe(true); + if (!("error" in result)) return; + expect(typeof result.error).toBe("string"); + expect(result.error.length).toBeGreaterThan(0); + }, + ); + }); + + it("rejects an invalid --output value", () => { + const result = parseCliArgs(argv("--output", "yaml")); + + expect("error" in result).toBe(true); + if (!("error" in result)) return; + expect(typeof result.error).toBe("string"); + expect(result.error.length).toBeGreaterThan(0); + }); + + it("rejects a --cwd path that does not exist", () => { + const bogusPath = join(tmpdir(), "definitely-does-not-exist-cli-args-test"); + const result = parseCliArgs(argv("--cwd", bogusPath)); + + expect("error" in result).toBe(true); + if (!("error" in result)) return; + expect(typeof result.error).toBe("string"); + expect(result.error.length).toBeGreaterThan(0); + }); + + it("accepts a --cwd path pointing to a real directory", () => { + const tmp = mkdtempSync(join(tmpdir(), "cli-args-test-")); + const result = parseCliArgs(argv("--cwd", tmp)); + + expect("error" in result).toBe(false); + if ("error" in result) return; + expect(result.cwd).toBe(realpathSync(tmp)); + }); +}); diff --git a/packages/cli/src/args.ts b/packages/cli/src/args.ts new file mode 100644 index 0000000000..8c487150ed --- /dev/null +++ b/packages/cli/src/args.ts @@ -0,0 +1,143 @@ +import { realpathSync, statSync } from "node:fs"; +import type { CodeExecutionMode } from "@posthog/agent/execution-mode"; +import { Command, CommanderError } from "commander"; + +const CLI_PERMISSION_MODES = [ + "auto", + "bypassPermissions", +] as const satisfies readonly CodeExecutionMode[]; + +export type CliPermissionMode = (typeof CLI_PERMISSION_MODES)[number]; + +export const OUTPUT_MODES = ["text", "json"] as const; + +export type OutputMode = (typeof OUTPUT_MODES)[number]; + +export interface CliOptions { + prompt?: string; + cwd: string; + permissionMode: CliPermissionMode; + model?: string; + systemPrompt?: string; + output: OutputMode; + debug: boolean; +} + +export interface ParseError { + error: string; + exitCode: number; +} + +export type ParseResult = CliOptions | ParseError; + +function buildProgram(): Command { + return new Command() + .name("posthog-code-cli") + .description( + "Run one PostHog Code agent turn against a local repository and print the result", + ) + .argument("[prompt]", "prompt for the agent (read from stdin when piped)") + .option("--cwd ", "repository to run against", process.cwd()) + .option( + "--permission-mode ", + `permission mode: ${CLI_PERMISSION_MODES.join(" | ")}`, + "auto", + ) + .option("--model ", 'Claude model id (must start with "claude-")') + .option("--system-prompt ", "replace the default system prompt") + .option( + "--output ", + `output format: ${OUTPUT_MODES.join(" | ")}`, + "text", + ) + .option("--debug", "verbose diagnostics on stderr", false) + .exitOverride(); +} + +export function parseCliArgs(argv: string[]): ParseResult { + const program = buildProgram(); + try { + program.parse(argv); + } catch (err) { + if (err instanceof CommanderError) { + // Help/version output was already written by commander itself. + const informational = + err.code === "commander.helpDisplayed" || + err.code === "commander.version"; + return { + error: informational ? "" : err.message, + exitCode: informational ? 0 : 1, + }; + } + throw err; + } + + const opts = program.opts<{ + cwd: string; + permissionMode: string; + model?: string; + systemPrompt?: string; + output: string; + debug: boolean; + }>(); + + const permissionMode = parseChoice( + "--permission-mode", + opts.permissionMode, + CLI_PERMISSION_MODES, + "; interactive modes (default, acceptEdits, plan) need a UI to answer permission prompts", + ); + if (typeof permissionMode === "object") return permissionMode; + + const output = parseChoice("--output", opts.output, OUTPUT_MODES); + if (typeof output === "object") return output; + + // Non-claude ids are silently coerced to the default model downstream, so + // reject them here where the user can see why. + if (opts.model && !opts.model.startsWith("claude-")) { + return { + error: + `Invalid --model "${opts.model}". Pass a full Claude model id starting with ` + + `"claude-" (e.g. "claude-sonnet-4-5").`, + exitCode: 1, + }; + } + + let cwd: string; + try { + // realpath: the agent SDK keys its session store by resolved path, and on + // macOS common paths like /tmp are symlinks. + cwd = realpathSync(opts.cwd); + if (!statSync(cwd).isDirectory()) { + return { error: `--cwd is not a directory: ${opts.cwd}`, exitCode: 1 }; + } + } catch { + return { error: `--cwd does not exist: ${opts.cwd}`, exitCode: 1 }; + } + + return { + prompt: program.args[0], + cwd, + permissionMode, + model: opts.model, + systemPrompt: opts.systemPrompt, + output, + debug: opts.debug, + }; +} + +function parseChoice( + flag: string, + value: string, + allowed: readonly T[], + hint = "", +): T | ParseError { + if ((allowed as readonly string[]).includes(value)) { + return value as T; + } + const choices = allowed.map((v) => `"${v}"`).join(" or "); + return { + error: `Unsupported ${flag} "${value}". Use ${choices}${hint}.`, + exitCode: 1, + }; +} diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts new file mode 100644 index 0000000000..5576b769b0 --- /dev/null +++ b/packages/cli/src/cli.ts @@ -0,0 +1,54 @@ +#!/usr/bin/env node +import { parseCliArgs } from "./args"; +import { run } from "./run"; + +async function readStdin(): Promise { + const parts: Buffer[] = []; + for await (const part of process.stdin) { + parts.push(part as Buffer); + } + return Buffer.concat(parts).toString("utf8"); +} + +async function main(): Promise { + const parsed = parseCliArgs(process.argv); + if ("error" in parsed) { + if (parsed.error) { + process.stderr.write(`${parsed.error}\n`); + } + return parsed.exitCode; + } + + let prompt = parsed.prompt; + if (!prompt && !process.stdin.isTTY) { + prompt = (await readStdin()).trim(); + } + if (!prompt) { + process.stderr.write( + "No prompt given. Pass one as an argument or pipe it on stdin.\n", + ); + return 1; + } + + if (!process.env.ANTHROPIC_API_KEY && !process.env.ANTHROPIC_AUTH_TOKEN) { + process.stderr.write( + "Warning: neither ANTHROPIC_API_KEY nor ANTHROPIC_AUTH_TOKEN is set; " + + "relying on a stored claude login credential.\n", + ); + } + + return run({ ...parsed, prompt }); +} + +// The agent subprocess can leave handles open past cleanup; force-exit once +// output has flushed. unref lets a clean event loop exit naturally first. +function exitAfterFlush(code: number): void { + process.exitCode = code; + setTimeout(() => process.exit(code), 500).unref(); +} + +main().then(exitAfterFlush, (err) => { + const message = err instanceof Error ? err.message : String(err); + process.stderr.write(`posthog-code-cli: ${message}\n`); + exitAfterFlush(1); +}); diff --git a/packages/cli/src/output.test.ts b/packages/cli/src/output.test.ts new file mode 100644 index 0000000000..401792f5d2 --- /dev/null +++ b/packages/cli/src/output.test.ts @@ -0,0 +1,183 @@ +import { describe, expect, it } from "vitest"; +import { createOutputSink } from "./output"; + +function makeFakeStdout() { + const chunks: string[] = []; + return { + chunks, + write(s: string): boolean { + chunks.push(s); + return true; + }, + get output(): string { + return chunks.join(""); + }, + }; +} + +function textUpdate(sessionId: string, text: string) { + return { + sessionId, + update: { + sessionUpdate: "agent_message_chunk", + content: { type: "text", text }, + }, + }; +} + +function ignoredUpdate(sessionId: string) { + return [ + { + label: "agent_thought_chunk", + update: { + sessionId, + update: { + sessionUpdate: "agent_thought_chunk", + content: { type: "text", text: "thinking…" }, + }, + }, + }, + { + label: "tool_call", + update: { + sessionId, + update: { sessionUpdate: "tool_call", toolCallId: "tc1" }, + }, + }, + { + label: "tool_call_update", + update: { + sessionId, + update: { sessionUpdate: "tool_call_update", toolCallId: "tc1" }, + }, + }, + { + label: "plan", + update: { + sessionId, + update: { sessionUpdate: "plan", entries: [] }, + }, + }, + { + label: "non-text content on agent_message_chunk", + update: { + sessionId, + update: { + sessionUpdate: "agent_message_chunk", + content: { type: "image", data: "base64...", mimeType: "image/png" }, + }, + }, + }, + ]; +} + +describe("createOutputSink", () => { + describe("text mode", () => { + it("streams agent_message_chunk text to stdout immediately, in order", () => { + const stdout = makeFakeStdout(); + const sink = createOutputSink("text", stdout); + + sink.onSessionUpdate(textUpdate("s1", "Hello, ")); + sink.onSessionUpdate(textUpdate("s1", "world")); + sink.onSessionUpdate(textUpdate("s1", "!")); + + expect(stdout.output).toBe("Hello, world!"); + }); + + it.each(ignoredUpdate("s1"))("ignores $label updates", ({ update }) => { + const stdout = makeFakeStdout(); + const sink = createOutputSink("text", stdout); + + sink.onSessionUpdate(textUpdate("s1", "kept")); + sink.onSessionUpdate(update); + sink.onSessionUpdate(textUpdate("s1", " text")); + + expect(stdout.output).toBe("kept text"); + }); + + it("does not emit a JSON document from finish()", () => { + const stdout = makeFakeStdout(); + const sink = createOutputSink("text", stdout); + + sink.onSessionUpdate(textUpdate("s1", "some streamed text")); + sink.finish({ stopReason: "end_turn", sessionId: "s1" }); + + // Text mode must never buffer the streamed text into a finish-result + // JSON document. Unparseable output is fine — that IS plain text. + let parsed: unknown; + try { + parsed = JSON.parse(stdout.output); + } catch { + parsed = undefined; + } + const isFinishDocument = + typeof parsed === "object" && + parsed !== null && + "text" in parsed && + "stopReason" in parsed; + expect(isFinishDocument).toBe(false); + expect(stdout.output).toContain("some streamed text"); + }); + }); + + describe("json mode", () => { + it("buffers session updates and writes nothing until finish()", () => { + const stdout = makeFakeStdout(); + const sink = createOutputSink("json", stdout); + + sink.onSessionUpdate(textUpdate("s1", "Hello, ")); + sink.onSessionUpdate(textUpdate("s1", "world")); + + expect(stdout.output).toBe(""); + }); + + it("emits exactly one parseable JSON document with the concatenated text and finish() fields", () => { + const stdout = makeFakeStdout(); + const sink = createOutputSink("json", stdout); + + sink.onSessionUpdate(textUpdate("s1", "Hello, ")); + sink.onSessionUpdate(textUpdate("s1", "world")); + sink.finish({ + stopReason: "end_turn", + usage: { totalTokens: 42 }, + sessionId: "s1", + }); + + const parsed = JSON.parse(stdout.output); + expect(parsed).toEqual({ + text: "Hello, world", + stopReason: "end_turn", + usage: { totalTokens: 42 }, + sessionId: "s1", + }); + }); + + it.each(ignoredUpdate("s1"))( + "excludes $label updates from the concatenated text", + ({ update }) => { + const stdout = makeFakeStdout(); + const sink = createOutputSink("json", stdout); + + sink.onSessionUpdate(textUpdate("s1", "kept")); + sink.onSessionUpdate(update); + sink.onSessionUpdate(textUpdate("s1", " text")); + sink.finish({ stopReason: "end_turn", sessionId: "s1" }); + + const parsed = JSON.parse(stdout.output); + expect(parsed.text).toBe("kept text"); + }, + ); + + it('produces text: "" for an empty run with no session updates', () => { + const stdout = makeFakeStdout(); + const sink = createOutputSink("json", stdout); + + sink.finish({ stopReason: "end_turn", sessionId: "s1" }); + + const parsed = JSON.parse(stdout.output); + expect(parsed.text).toBe(""); + expect(parsed.stopReason).toBe("end_turn"); + expect(parsed.sessionId).toBe("s1"); + }); + }); +}); diff --git a/packages/cli/src/output.ts b/packages/cli/src/output.ts new file mode 100644 index 0000000000..816c9aa50b --- /dev/null +++ b/packages/cli/src/output.ts @@ -0,0 +1,61 @@ +import type { OutputMode } from "./args"; + +export interface FinishResult { + stopReason: string; + usage?: unknown; + sessionId: string; +} + +export interface OutputSink { + onSessionUpdate(notification: unknown): void; + finish(result: FinishResult): void; +} + +interface WritableLike { + write(chunk: string): unknown; +} + +function extractChunkText(notification: unknown): string | undefined { + const update = (notification as { update?: unknown } | undefined)?.update as + | { sessionUpdate?: string; content?: { type?: string; text?: unknown } } + | undefined; + if (update?.sessionUpdate !== "agent_message_chunk") return undefined; + if (update.content?.type !== "text") return undefined; + return typeof update.content.text === "string" + ? update.content.text + : undefined; +} + +/** + * Routes assistant output to stdout. "text" streams each agent_message_chunk + * as it arrives; "json" buffers and emits one JSON document on finish. + */ +export function createOutputSink( + mode: OutputMode, + stdout: WritableLike, +): OutputSink { + const chunks: string[] = []; + let streamedText = false; + + return { + onSessionUpdate(notification) { + const text = extractChunkText(notification); + if (text === undefined) return; + if (mode === "text") { + stdout.write(text); + streamedText = true; + } else { + chunks.push(text); + } + }, + finish({ stopReason, usage, sessionId }) { + if (mode === "json") { + stdout.write( + `${JSON.stringify({ text: chunks.join(""), stopReason, usage, sessionId })}\n`, + ); + } else if (streamedText) { + stdout.write("\n"); + } + }, + }; +} diff --git a/packages/cli/src/permission-policy.test.ts b/packages/cli/src/permission-policy.test.ts new file mode 100644 index 0000000000..9309e60bc2 --- /dev/null +++ b/packages/cli/src/permission-policy.test.ts @@ -0,0 +1,148 @@ +import type { RequestPermissionRequest } from "@agentclientprotocol/sdk"; +import { describe, expect, it } from "vitest"; +import { resolvePermissionRequest } from "./permission-policy"; + +// These option/toolCall shapes mirror the relevant fields of +// @agentclientprotocol/sdk's RequestPermissionRequest/RequestPermissionResponse, +// trimmed to the fields the pure policy function reads. + +interface Option { + optionId: string; + name: string; + kind: "allow_once" | "allow_always" | "reject_once" | "reject_always"; +} + +function makeParams( + options: Option[], + codeToolKind?: string, +): RequestPermissionRequest { + return { + sessionId: "session-1", + toolCall: { + toolCallId: "tool-1", + ...(codeToolKind ? { _meta: { codeToolKind } } : {}), + }, + options, + } as RequestPermissionRequest; +} + +describe("resolvePermissionRequest", () => { + describe("question tool calls", () => { + it("cancels with an actionable message instead of auto-approving", () => { + const params = makeParams( + [{ optionId: "opt-1", name: "Allow", kind: "allow_once" }], + "question", + ); + + const result = resolvePermissionRequest(params); + + expect(result.outcome.outcome).toBe("cancelled"); + const message = result._meta?.message as string; + expect(typeof message).toBe("string"); + expect(message.length).toBeGreaterThan(0); + expect(message).toMatch(/no user/i); + }); + + it("parks the question even when both allow_once and allow_always options exist", () => { + const params = makeParams( + [ + { optionId: "opt-1", name: "Allow always", kind: "allow_always" }, + { optionId: "opt-2", name: "Allow once", kind: "allow_once" }, + ], + "question", + ); + + const result = resolvePermissionRequest(params); + + expect(result.outcome.outcome).toBe("cancelled"); + expect(result._meta?.message).toMatch(/no user/i); + }); + }); + + describe("auto-approval", () => { + it.each([ + { + label: "allow_always listed first, allow_once second", + options: [ + { + optionId: "always-1", + name: "Allow always", + kind: "allow_always" as const, + }, + { + optionId: "once-1", + name: "Allow once", + kind: "allow_once" as const, + }, + ], + expectedOptionId: "once-1", + }, + { + label: "allow_once listed first, allow_always second", + options: [ + { + optionId: "once-1", + name: "Allow once", + kind: "allow_once" as const, + }, + { + optionId: "always-1", + name: "Allow always", + kind: "allow_always" as const, + }, + ], + expectedOptionId: "once-1", + }, + ])( + "prefers allow_once over allow_always regardless of order ($label)", + ({ options, expectedOptionId }) => { + const result = resolvePermissionRequest(makeParams(options)); + + expect(result.outcome).toEqual({ + outcome: "selected", + optionId: expectedOptionId, + }); + }, + ); + + it("selects allow_always when no allow_once option exists", () => { + const options: Option[] = [ + { optionId: "reject-1", name: "Reject once", kind: "reject_once" }, + { optionId: "always-1", name: "Allow always", kind: "allow_always" }, + ]; + + const result = resolvePermissionRequest(makeParams(options)); + + expect(result.outcome).toEqual({ + outcome: "selected", + optionId: "always-1", + }); + }); + + it("falls back to the first option when only reject kinds are present", () => { + const options: Option[] = [ + { optionId: "reject-once-1", name: "Reject once", kind: "reject_once" }, + { + optionId: "reject-always-1", + name: "Reject always", + kind: "reject_always", + }, + ]; + + const result = resolvePermissionRequest(makeParams(options)); + + expect(result.outcome).toEqual({ + outcome: "selected", + optionId: "reject-once-1", + }); + }); + }); + + describe("empty options", () => { + it("cancels when the options array is empty", () => { + const result = resolvePermissionRequest(makeParams([])); + + expect(result.outcome).toEqual({ outcome: "cancelled" }); + }); + }); +}); diff --git a/packages/cli/src/permission-policy.ts b/packages/cli/src/permission-policy.ts new file mode 100644 index 0000000000..abe08ac3fc --- /dev/null +++ b/packages/cli/src/permission-policy.ts @@ -0,0 +1,41 @@ +import type { + RequestPermissionRequest, + RequestPermissionResponse, +} from "@agentclientprotocol/sdk"; + +/** + * Mirrors the cloud background-mode reply: the model must surface the question + * as regular assistant text and end its turn instead of guessing an answer. + */ +const UNATTENDED_QUESTION_MESSAGE = + "No user is available to answer this question. Do NOT pick an answer yourself " + + "and do NOT re-ask via this tool. Restate the question and its options in your " + + "response, then end your turn."; + +/** + * Unattended permission policy. Question tool calls are parked (no user to + * answer); everything else that reaches the client is auto-approved. Prefers + * allow_once so a one-shot run never persists allow-always rules into the + * target repository's settings. + */ +export function resolvePermissionRequest( + params: RequestPermissionRequest, +): RequestPermissionResponse { + const meta = params.toolCall?._meta as { codeToolKind?: string } | undefined; + if (meta?.codeToolKind === "question") { + return { + outcome: { outcome: "cancelled" }, + _meta: { message: UNATTENDED_QUESTION_MESSAGE }, + }; + } + + const options = params.options ?? []; + const chosen = + options.find((o) => o.kind === "allow_once") ?? + options.find((o) => o.kind === "allow_always") ?? + options[0]; + if (!chosen) { + return { outcome: { outcome: "cancelled" } }; + } + return { outcome: { outcome: "selected", optionId: chosen.optionId } }; +} diff --git a/packages/cli/src/run.ts b/packages/cli/src/run.ts new file mode 100644 index 0000000000..29088f8e7b --- /dev/null +++ b/packages/cli/src/run.ts @@ -0,0 +1,118 @@ +import { randomUUID } from "node:crypto"; +import { format } from "node:util"; +import { + ClientSideConnection, + ndJsonStream, + type RequestPermissionRequest, +} from "@agentclientprotocol/sdk"; +import { Agent } from "@posthog/agent/agent"; +import { withTimeout } from "@posthog/shared"; +import type { CliOptions } from "./args"; +import { createOutputSink } from "./output"; +import { resolvePermissionRequest } from "./permission-policy"; + +const STOP_REASON_EXIT_CODES: Record = { + end_turn: 0, + refusal: 2, + max_tokens: 3, + max_turn_requests: 3, +}; + +export type RunOptions = CliOptions & { prompt: string }; + +export async function run(options: RunOptions): Promise { + const debugLog = options.debug + ? (message: string) => + process.stderr.write(`[posthog-code-cli] ${message}\n`) + : () => {}; + + // stdout must carry only assistant output. onLog bypasses the adapter's + // console logging entirely, routing diagnostics to stderr behind --debug. + // No posthog config: gateway resolution is skipped and the agent subprocess + // inherits ANTHROPIC_* auth from process.env. + const agent = new Agent({ + debug: options.debug, + onLog: (level, scope, message, data) => + debugLog( + `${scope} [${level}] ${message}${data === undefined ? "" : ` ${format(data)}`}`, + ), + }); + const acp = await agent.run("cli", `cli-${randomUUID()}`, { + adapter: "claude", + }); + + const sink = createOutputSink(options.output, process.stdout); + + const client = { + async sessionUpdate(notification: unknown): Promise { + sink.onSessionUpdate(notification); + }, + async requestPermission(params: RequestPermissionRequest) { + const response = resolvePermissionRequest(params); + debugLog( + `permission "${params.toolCall?.title ?? "unknown"}" -> ${JSON.stringify(response.outcome)}`, + ); + return response; + }, + // No readTextFile/writeTextFile and empty clientCapabilities below: the + // adapter runs file tools in-process instead of proxying through the host. + async extNotification(): Promise {}, + }; + + const conn = new ClientSideConnection( + () => client, + ndJsonStream(acp.clientStreams.writable, acp.clientStreams.readable), + ); + + const cleanup = async () => { + // Bounded: a wedged adapter cleanup must never hang the process. + await withTimeout( + agent.cleanup().catch(() => undefined), + 8000, + ); + }; + + let sessionId: string | undefined; + const onSigint = () => { + void (async () => { + if (sessionId) { + await conn.cancel({ sessionId }).catch(() => undefined); + } + await cleanup(); + process.exit(130); + })(); + }; + process.once("SIGINT", onSigint); + + try { + await conn.initialize({ protocolVersion: 1, clientCapabilities: {} }); + const session = await conn.newSession({ + cwd: options.cwd, + mcpServers: [], + _meta: { + permissionMode: options.permissionMode, + ...(options.model ? { model: options.model } : {}), + ...(options.systemPrompt ? { systemPrompt: options.systemPrompt } : {}), + }, + }); + sessionId = session.sessionId; + debugLog(`session ${sessionId} started in ${options.cwd}`); + + const result = await conn.prompt({ + sessionId, + prompt: [{ type: "text", text: options.prompt }], + }); + const stopReason = result.stopReason ?? "unknown"; + debugLog(`turn finished: ${stopReason}`); + + sink.finish({ + stopReason, + usage: (result as { usage?: unknown }).usage, + sessionId, + }); + return STOP_REASON_EXIT_CODES[stopReason] ?? 1; + } finally { + process.removeListener("SIGINT", onSigint); + await cleanup(); + } +} diff --git a/packages/cli/tsconfig.json b/packages/cli/tsconfig.json new file mode 100644 index 0000000000..03bcd96837 --- /dev/null +++ b/packages/cli/tsconfig.json @@ -0,0 +1,17 @@ +{ + "compilerOptions": { + "lib": ["ESNext"], + "target": "ES2022", + "module": "ESNext", + "moduleDetection": "force", + "types": ["node"], + "moduleResolution": "bundler", + "verbatimModuleSyntax": true, + "noEmit": true, + "strict": true, + "skipLibCheck": true, + "noFallthroughCasesInSwitch": true + }, + "include": ["src/**/*", "e2e/**/*", "*.ts"], + "exclude": ["node_modules", "dist"] +} diff --git a/packages/cli/tsup.config.ts b/packages/cli/tsup.config.ts new file mode 100644 index 0000000000..d0a9833099 --- /dev/null +++ b/packages/cli/tsup.config.ts @@ -0,0 +1,11 @@ +import { defineConfig } from "tsup"; + +export default defineConfig({ + entry: ["src/cli.ts"], + format: "esm", + platform: "node", + target: "node20", + dts: false, + sourcemap: false, + clean: false, +}); diff --git a/packages/cli/vitest.config.ts b/packages/cli/vitest.config.ts new file mode 100644 index 0000000000..407c36b953 --- /dev/null +++ b/packages/cli/vitest.config.ts @@ -0,0 +1,12 @@ +import { defineConfig } from "vitest/config"; +import { trunkTestOptions } from "../../vitest.config.base"; + +export default defineConfig({ + test: { + globals: true, + ...trunkTestOptions, + environment: "node", + include: ["src/**/*.test.ts"], + exclude: ["**/node_modules/**", "**/dist/**"], + }, +}); diff --git a/packages/cli/vitest.e2e.config.ts b/packages/cli/vitest.e2e.config.ts new file mode 100644 index 0000000000..64a59c9b96 --- /dev/null +++ b/packages/cli/vitest.e2e.config.ts @@ -0,0 +1,18 @@ +import { defineConfig } from "vitest/config"; + +// Live, opt-in smoke test for the built CLI binary. Separate from +// vitest.config.ts so it never runs under `pnpm test` — only via +// `pnpm test:e2e` with a gateway token set (same env contract as +// packages/agent/e2e). Each test spawns a real agent turn end to end. +export default defineConfig({ + test: { + globals: true, + environment: "node", + include: ["e2e/**/*.e2e.test.ts"], + exclude: ["**/node_modules/**", "**/dist/**"], + isolate: true, + fileParallelism: false, + testTimeout: 300_000, + hookTimeout: 120_000, + }, +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 28baa3b7d4..d3e9762b5e 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -898,6 +898,34 @@ importers: specifier: ^4.1.8 version: 4.1.8(@opentelemetry/api@1.9.1)(@types/node@25.2.0)(@vitest/ui@4.1.8)(jsdom@26.1.0)(msw@2.12.8(@types/node@25.2.0)(typescript@5.9.3))(vite@7.3.5(@types/node@25.2.0)(jiti@2.7.0)(lightningcss@1.32.0)(terser@5.46.0)(tsx@4.22.4)(yaml@2.9.0)) + packages/cli: + dependencies: + '@agentclientprotocol/sdk': + specifier: 1.1.0 + version: 1.1.0(zod@4.4.3) + '@posthog/agent': + specifier: workspace:* + version: link:../agent + '@posthog/shared': + specifier: workspace:* + version: link:../shared + commander: + specifier: ^14.0.2 + version: 14.0.3 + devDependencies: + '@types/node': + specifier: 'catalog:' + version: 20.19.41 + tsup: + specifier: 'catalog:' + version: 8.5.1(@swc/core@1.15.43)(jiti@2.7.0)(postcss@8.5.15)(tsx@4.22.4)(typescript@5.9.3)(yaml@2.9.0) + typescript: + specifier: 'catalog:' + version: 5.9.3 + vitest: + specifier: ^4.1.8 + version: 4.1.8(@opentelemetry/api@1.9.1)(@types/node@20.19.41)(@vitest/ui@4.1.8)(jsdom@26.1.0)(msw@2.12.8(@types/node@20.19.41)(typescript@5.9.3))(vite@7.3.5(@types/node@20.19.41)(jiti@2.7.0)(lightningcss@1.32.0)(terser@5.46.0)(tsx@4.22.4)(yaml@2.9.0)) + packages/core: dependencies: '@json-render/core': From 53d8c0038903eb7f2e566f5e37cc9b3b6c0ea667 Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Thu, 30 Jul 2026 17:05:25 -0700 Subject: [PATCH 2/4] fix(cli): harden exit paths, error surfacing, and arg validation Review follow-ups: adapter error-level diagnostics now reach stderr without --debug (except expected teardown noise); SIGINT cancels the turn and exits 130 deterministically instead of racing the main path; stdout is flushed before the force-exit timer so piped JSON can't truncate; commander choices()/argParser replace hand-rolled validation (and the double error print); session updates and prompt results use the ACP SDK types instead of unknown casts; reject-only permission requests resolve to reject_once; bypassPermissions as non-sandboxed root fails fast with a clear message. Generated-By: PostHog Code Task-Id: 97a6265f-b609-4775-9949-887aca895991 --- packages/cli/src/args.ts | 154 +++++++++++++------------- packages/cli/src/cli.ts | 34 ++++-- packages/cli/src/output.test.ts | 11 +- packages/cli/src/output.ts | 18 ++- packages/cli/src/permission-policy.ts | 8 +- packages/cli/src/run.ts | 58 ++++++---- 6 files changed, 154 insertions(+), 129 deletions(-) diff --git a/packages/cli/src/args.ts b/packages/cli/src/args.ts index 8c487150ed..8bc7abe87b 100644 --- a/packages/cli/src/args.ts +++ b/packages/cli/src/args.ts @@ -1,6 +1,11 @@ import { realpathSync, statSync } from "node:fs"; import type { CodeExecutionMode } from "@posthog/agent/execution-mode"; -import { Command, CommanderError } from "commander"; +import { + Command, + CommanderError, + InvalidArgumentError, + Option, +} from "commander"; const CLI_PERMISSION_MODES = [ "auto", @@ -9,7 +14,7 @@ const CLI_PERMISSION_MODES = [ export type CliPermissionMode = (typeof CLI_PERMISSION_MODES)[number]; -export const OUTPUT_MODES = ["text", "json"] as const; +const OUTPUT_MODES = ["text", "json"] as const; export type OutputMode = (typeof OUTPUT_MODES)[number]; @@ -30,28 +35,71 @@ export interface ParseError { export type ParseResult = CliOptions | ParseError; +function parseModel(value: string): string { + // Non-claude ids are silently coerced to the default model downstream, so + // reject them here where the user can see why. + if (!value.startsWith("claude-")) { + throw new InvalidArgumentError( + 'Pass a full Claude model id starting with "claude-" (e.g. "claude-sonnet-4-5").', + ); + } + return value; +} + +function parseCwd(value: string): string { + // realpath: the agent SDK keys its session store by resolved path, and on + // macOS common paths like /tmp are symlinks. + let cwd: string; + try { + cwd = realpathSync(value); + } catch { + throw new InvalidArgumentError("No such directory."); + } + if (!statSync(cwd).isDirectory()) { + throw new InvalidArgumentError("Not a directory."); + } + return cwd; +} + function buildProgram(): Command { - return new Command() - .name("posthog-code-cli") - .description( - "Run one PostHog Code agent turn against a local repository and print the result", - ) - .argument("[prompt]", "prompt for the agent (read from stdin when piped)") - .option("--cwd ", "repository to run against", process.cwd()) - .option( - "--permission-mode ", - `permission mode: ${CLI_PERMISSION_MODES.join(" | ")}`, - "auto", - ) - .option("--model ", 'Claude model id (must start with "claude-")') - .option("--system-prompt ", "replace the default system prompt") - .option( - "--output ", - `output format: ${OUTPUT_MODES.join(" | ")}`, - "text", - ) - .option("--debug", "verbose diagnostics on stderr", false) - .exitOverride(); + return ( + new Command() + .name("posthog-code-cli") + .description( + "Run one PostHog Code agent turn against a local repository and print the result", + ) + .argument("[prompt]", "prompt for the agent (read from stdin when piped)") + .option( + "--cwd ", + "repository to run against", + parseCwd, + realpathSync(process.cwd()), + ) + .addOption( + new Option( + "--permission-mode ", + "unattended permission mode (interactive modes need a UI to answer prompts)", + ) + .choices(CLI_PERMISSION_MODES) + .default("auto"), + ) + .option( + "--model ", + 'Claude model id (must start with "claude-")', + parseModel, + ) + .option("--system-prompt ", "replace the default system prompt") + .addOption( + new Option("--output ", "output format") + .choices(OUTPUT_MODES) + .default("text"), + ) + .option("--debug", "verbose diagnostics on stderr", false) + .exitOverride() + // Errors are returned as ParseError and printed once by the caller; + // without this, commander writes them to stderr itself first. + .configureOutput({ writeErr: () => {} }) + ); } export function parseCliArgs(argv: string[]): ParseResult { @@ -74,70 +122,20 @@ export function parseCliArgs(argv: string[]): ParseResult { const opts = program.opts<{ cwd: string; - permissionMode: string; + permissionMode: CliPermissionMode; model?: string; systemPrompt?: string; - output: string; + output: OutputMode; debug: boolean; }>(); - const permissionMode = parseChoice( - "--permission-mode", - opts.permissionMode, - CLI_PERMISSION_MODES, - "; interactive modes (default, acceptEdits, plan) need a UI to answer permission prompts", - ); - if (typeof permissionMode === "object") return permissionMode; - - const output = parseChoice("--output", opts.output, OUTPUT_MODES); - if (typeof output === "object") return output; - - // Non-claude ids are silently coerced to the default model downstream, so - // reject them here where the user can see why. - if (opts.model && !opts.model.startsWith("claude-")) { - return { - error: - `Invalid --model "${opts.model}". Pass a full Claude model id starting with ` + - `"claude-" (e.g. "claude-sonnet-4-5").`, - exitCode: 1, - }; - } - - let cwd: string; - try { - // realpath: the agent SDK keys its session store by resolved path, and on - // macOS common paths like /tmp are symlinks. - cwd = realpathSync(opts.cwd); - if (!statSync(cwd).isDirectory()) { - return { error: `--cwd is not a directory: ${opts.cwd}`, exitCode: 1 }; - } - } catch { - return { error: `--cwd does not exist: ${opts.cwd}`, exitCode: 1 }; - } - return { prompt: program.args[0], - cwd, - permissionMode, + cwd: opts.cwd, + permissionMode: opts.permissionMode, model: opts.model, systemPrompt: opts.systemPrompt, - output, + output: opts.output, debug: opts.debug, }; } - -function parseChoice( - flag: string, - value: string, - allowed: readonly T[], - hint = "", -): T | ParseError { - if ((allowed as readonly string[]).includes(value)) { - return value as T; - } - const choices = allowed.map((v) => `"${v}"`).join(" or "); - return { - error: `Unsupported ${flag} "${value}". Use ${choices}${hint}.`, - exitCode: 1, - }; -} diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index 5576b769b0..c03cf4ed34 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -1,15 +1,8 @@ #!/usr/bin/env node +import { text } from "node:stream/consumers"; import { parseCliArgs } from "./args"; import { run } from "./run"; -async function readStdin(): Promise { - const parts: Buffer[] = []; - for await (const part of process.stdin) { - parts.push(part as Buffer); - } - return Buffer.concat(parts).toString("utf8"); -} - async function main(): Promise { const parsed = parseCliArgs(process.argv); if ("error" in parsed) { @@ -21,7 +14,7 @@ async function main(): Promise { let prompt = parsed.prompt; if (!prompt && !process.stdin.isTTY) { - prompt = (await readStdin()).trim(); + prompt = (await text(process.stdin)).trim(); } if (!prompt) { process.stderr.write( @@ -30,6 +23,19 @@ async function main(): Promise { return 1; } + // The Claude subprocess refuses bypass for root outside a sandbox; fail + // here with a clear message instead of a cryptic session error. + if ( + parsed.permissionMode === "bypassPermissions" && + process.getuid?.() === 0 && + !process.env.IS_SANDBOX + ) { + process.stderr.write( + "--permission-mode bypassPermissions is unavailable when running as root unless IS_SANDBOX=1 is set.\n", + ); + return 1; + } + if (!process.env.ANTHROPIC_API_KEY && !process.env.ANTHROPIC_AUTH_TOKEN) { process.stderr.write( "Warning: neither ANTHROPIC_API_KEY nor ANTHROPIC_AUTH_TOKEN is set; " + @@ -40,11 +46,15 @@ async function main(): Promise { return run({ ...parsed, prompt }); } -// The agent subprocess can leave handles open past cleanup; force-exit once -// output has flushed. unref lets a clean event loop exit naturally first. +// process.exit does not wait for pending pipe writes, so flush stdout first +// (the write callback fires once earlier writes are accepted by the OS). +// The agent subprocess can leave handles open past cleanup; the unref'd timer +// force-exits then, while a clean event loop still exits naturally. function exitAfterFlush(code: number): void { process.exitCode = code; - setTimeout(() => process.exit(code), 500).unref(); + process.stdout.write("", () => { + setTimeout(() => process.exit(code), 500).unref(); + }); } main().then(exitAfterFlush, (err) => { diff --git a/packages/cli/src/output.test.ts b/packages/cli/src/output.test.ts index 401792f5d2..bc906ffc8b 100644 --- a/packages/cli/src/output.test.ts +++ b/packages/cli/src/output.test.ts @@ -1,3 +1,4 @@ +import type { SessionNotification } from "@agentclientprotocol/sdk"; import { describe, expect, it } from "vitest"; import { createOutputSink } from "./output"; @@ -15,7 +16,7 @@ function makeFakeStdout() { }; } -function textUpdate(sessionId: string, text: string) { +function textUpdate(sessionId: string, text: string): SessionNotification { return { sessionId, update: { @@ -25,7 +26,11 @@ function textUpdate(sessionId: string, text: string) { }; } -function ignoredUpdate(sessionId: string) { +// Some entries are deliberately partial protocol shapes (the sink must ignore +// them at runtime), so they are cast rather than fully constructed. +function ignoredUpdate( + sessionId: string, +): { label: string; update: SessionNotification }[] { return [ { label: "agent_thought_chunk", @@ -42,7 +47,7 @@ function ignoredUpdate(sessionId: string) { update: { sessionId, update: { sessionUpdate: "tool_call", toolCallId: "tc1" }, - }, + } as SessionNotification, }, { label: "tool_call_update", diff --git a/packages/cli/src/output.ts b/packages/cli/src/output.ts index 816c9aa50b..88dffa43df 100644 --- a/packages/cli/src/output.ts +++ b/packages/cli/src/output.ts @@ -1,3 +1,4 @@ +import type { SessionNotification } from "@agentclientprotocol/sdk"; import type { OutputMode } from "./args"; export interface FinishResult { @@ -7,7 +8,7 @@ export interface FinishResult { } export interface OutputSink { - onSessionUpdate(notification: unknown): void; + onSessionUpdate(notification: SessionNotification): void; finish(result: FinishResult): void; } @@ -15,15 +16,12 @@ interface WritableLike { write(chunk: string): unknown; } -function extractChunkText(notification: unknown): string | undefined { - const update = (notification as { update?: unknown } | undefined)?.update as - | { sessionUpdate?: string; content?: { type?: string; text?: unknown } } - | undefined; - if (update?.sessionUpdate !== "agent_message_chunk") return undefined; - if (update.content?.type !== "text") return undefined; - return typeof update.content.text === "string" - ? update.content.text - : undefined; +function extractChunkText( + notification: SessionNotification, +): string | undefined { + const { update } = notification; + if (update.sessionUpdate !== "agent_message_chunk") return undefined; + return update.content.type === "text" ? update.content.text : undefined; } /** diff --git a/packages/cli/src/permission-policy.ts b/packages/cli/src/permission-policy.ts index abe08ac3fc..415e5863e7 100644 --- a/packages/cli/src/permission-policy.ts +++ b/packages/cli/src/permission-policy.ts @@ -16,12 +16,13 @@ const UNATTENDED_QUESTION_MESSAGE = * Unattended permission policy. Question tool calls are parked (no user to * answer); everything else that reaches the client is auto-approved. Prefers * allow_once so a one-shot run never persists allow-always rules into the - * target repository's settings. + * target repository's settings; likewise a reject-only request resolves to + * reject_once rather than whatever option happens to be listed first. */ export function resolvePermissionRequest( params: RequestPermissionRequest, ): RequestPermissionResponse { - const meta = params.toolCall?._meta as { codeToolKind?: string } | undefined; + const meta = params.toolCall._meta as { codeToolKind?: string } | undefined; if (meta?.codeToolKind === "question") { return { outcome: { outcome: "cancelled" }, @@ -29,10 +30,11 @@ export function resolvePermissionRequest( }; } - const options = params.options ?? []; + const { options } = params; const chosen = options.find((o) => o.kind === "allow_once") ?? options.find((o) => o.kind === "allow_always") ?? + options.find((o) => o.kind === "reject_once") ?? options[0]; if (!chosen) { return { outcome: { outcome: "cancelled" } }; diff --git a/packages/cli/src/run.ts b/packages/cli/src/run.ts index 29088f8e7b..874edb6780 100644 --- a/packages/cli/src/run.ts +++ b/packages/cli/src/run.ts @@ -4,6 +4,7 @@ import { ClientSideConnection, ndJsonStream, type RequestPermissionRequest, + type SessionNotification, } from "@agentclientprotocol/sdk"; import { Agent } from "@posthog/agent/agent"; import { withTimeout } from "@posthog/shared"; @@ -21,21 +22,27 @@ const STOP_REASON_EXIT_CODES: Record = { export type RunOptions = CliOptions & { prompt: string }; export async function run(options: RunOptions): Promise { - const debugLog = options.debug - ? (message: string) => - process.stderr.write(`[posthog-code-cli] ${message}\n`) - : () => {}; + const writeDiagnostic = (message: string) => + process.stderr.write(`[posthog-code-cli] ${message}\n`); + const debugLog = options.debug ? writeDiagnostic : () => {}; // stdout must carry only assistant output. onLog bypasses the adapter's - // console logging entirely, routing diagnostics to stderr behind --debug. + // console logging entirely; errors during the turn always reach stderr (a + // failed run must say why), everything else only with --debug. Teardown + // errors are expected noise (the adapter reports the closing connection). // No posthog config: gateway resolution is skipped and the agent subprocess // inherits ANTHROPIC_* auth from process.env. + let tearingDown = false; const agent = new Agent({ debug: options.debug, - onLog: (level, scope, message, data) => - debugLog( - `${scope} [${level}] ${message}${data === undefined ? "" : ` ${format(data)}`}`, - ), + onLog: (level, scope, message, data) => { + const line = `${scope} [${level}] ${message}${data === undefined ? "" : ` ${format(data)}`}`; + if (level === "error" && !tearingDown) { + writeDiagnostic(line); + } else { + debugLog(line); + } + }, }); const acp = await agent.run("cli", `cli-${randomUUID()}`, { adapter: "claude", @@ -44,13 +51,13 @@ export async function run(options: RunOptions): Promise { const sink = createOutputSink(options.output, process.stdout); const client = { - async sessionUpdate(notification: unknown): Promise { + async sessionUpdate(notification: SessionNotification): Promise { sink.onSessionUpdate(notification); }, async requestPermission(params: RequestPermissionRequest) { const response = resolvePermissionRequest(params); debugLog( - `permission "${params.toolCall?.title ?? "unknown"}" -> ${JSON.stringify(response.outcome)}`, + `permission "${params.toolCall.title ?? "unknown"}" -> ${JSON.stringify(response.outcome)}`, ); return response; }, @@ -73,14 +80,18 @@ export async function run(options: RunOptions): Promise { }; let sessionId: string | undefined; + let interrupted = false; const onSigint = () => { - void (async () => { - if (sessionId) { - await conn.cancel({ sessionId }).catch(() => undefined); - } - await cleanup(); - process.exit(130); - })(); + interrupted = true; + if (sessionId) { + // Cancel resolves the in-flight prompt() with stopReason "cancelled"; + // the normal path then finishes output and exits 130. A second Ctrl-C + // (the handler is `once`) falls through to Node's default kill. + void conn.cancel({ sessionId }).catch(() => undefined); + } else { + // Nothing to cancel yet — exit directly. + void cleanup().then(() => process.exit(130)); + } }; process.once("SIGINT", onSigint); @@ -102,16 +113,17 @@ export async function run(options: RunOptions): Promise { sessionId, prompt: [{ type: "text", text: options.prompt }], }); - const stopReason = result.stopReason ?? "unknown"; - debugLog(`turn finished: ${stopReason}`); + debugLog(`turn finished: ${result.stopReason}`); sink.finish({ - stopReason, - usage: (result as { usage?: unknown }).usage, + stopReason: result.stopReason, + usage: result.usage, sessionId, }); - return STOP_REASON_EXIT_CODES[stopReason] ?? 1; + if (interrupted) return 130; + return STOP_REASON_EXIT_CODES[result.stopReason] ?? 1; } finally { + tearingDown = true; process.removeListener("SIGINT", onSigint); await cleanup(); } From f1f7a9e472f23404622db7763d5b702a9cc6dc41 Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Fri, 24 Jul 2026 15:41:17 -0700 Subject: [PATCH 3/4] fix(cli): close hang and interrupt gaps in exit handling The force-exit backstop now arms unconditionally so a stalled stdout consumer cannot hang the process, and a SIGINT that lands during session setup exits 130 instead of being reset when the turn activates. Generated-By: PostHog Code Task-Id: 97a6265f-b609-4775-9949-887aca895991 --- packages/cli/src/cli.ts | 5 ++++- packages/cli/src/run.ts | 6 +++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index c03cf4ed34..a847528e32 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -49,9 +49,12 @@ async function main(): Promise { // process.exit does not wait for pending pipe writes, so flush stdout first // (the write callback fires once earlier writes are accepted by the OS). // The agent subprocess can leave handles open past cleanup; the unref'd timer -// force-exits then, while a clean event loop still exits naturally. +// force-exits then, while a clean event loop still exits naturally. The +// unconditional backstop covers a stalled consumer that never drains the pipe, +// which would otherwise keep the flush callback from ever firing. function exitAfterFlush(code: number): void { process.exitCode = code; + setTimeout(() => process.exit(code), 10_000).unref(); process.stdout.write("", () => { setTimeout(() => process.exit(code), 500).unref(); }); diff --git a/packages/cli/src/run.ts b/packages/cli/src/run.ts index 874edb6780..c3f33d244a 100644 --- a/packages/cli/src/run.ts +++ b/packages/cli/src/run.ts @@ -84,7 +84,7 @@ export async function run(options: RunOptions): Promise { const onSigint = () => { interrupted = true; if (sessionId) { - // Cancel resolves the in-flight prompt() with stopReason "cancelled"; + // Cancel resolves an in-flight prompt() with stopReason "cancelled"; // the normal path then finishes output and exits 130. A second Ctrl-C // (the handler is `once`) falls through to Node's default kill. void conn.cancel({ sessionId }).catch(() => undefined); @@ -109,6 +109,10 @@ export async function run(options: RunOptions): Promise { sessionId = session.sessionId; debugLog(`session ${sessionId} started in ${options.cwd}`); + // A cancel sent before the turn activates is reset by the adapter, so a + // SIGINT that landed during session setup must be honored here instead. + if (interrupted) return 130; + const result = await conn.prompt({ sessionId, prompt: [{ type: "text", text: options.prompt }], From eb72cfe0de8697770e414d89c7cc82dbf0192bde Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Fri, 24 Jul 2026 19:41:59 -0700 Subject: [PATCH 4/4] fix(cli): close a content leak, harden signals and piping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on the headless CLI, plus two fixes in @posthog/agent that it depends on. Adapter diagnostics are no longer forwarded to a host logger by default. Forwarding them routed lines that carry whole payloads (an expanded slash-command's output, raw tool inputs, subprocess stderr) into desktop's electron-log file and OTLP transports, and into cloud's task feed on a session re-init. `Logger.emitLog` calls `onLog` for every level before the debug gate, so `debug: false` did not hold them back. A new `forwardAdapterLogs` opt-in carries them, and only the CLI sets it, where the sink is the operator's own stderr. A plan approval no longer downgrades the session. The unattended policy ranks options by kind, but a plan approval's options are session modes and its only `allow_once` is "manually approve edits", so approving a plan switched an unattended run into an interactive mode and persisted that into the target repo's settings. Plan approvals now take array order, which is the mode the adapter puts first, matching the cloud client. Also: - SIGTERM is handled alongside SIGINT (exit 143), so a supervisor no longer skips teardown and orphans the claude subprocess. - EPIPE on stdout is swallowed, so `… | head -1` stops quietly instead of crashing with a stack trace mid-teardown. - A whitespace-only positional prompt is rejected like a piped one. - The root/bypass guard reads the effective uid, matching `IS_ROOT`. - Reading a prompt from a stdin that never closes is bounded. - The `usage` key stays in the JSON document as null when a turn settles without token counts, and `stopReason`/`usage` use the SDK's types. - An unavailable `--model` id now warns instead of being swapped silently. - `--debug` keeps the stack on an unexpected throw. The unattended permission policy moves to `@posthog/agent/unattended-permission-policy`. It is the third copy in the repo and the only one that ranks by kind; giving it a home is a step toward cloud and desktop adopting it. `runTurn` is split behind a structural connection interface so the signal and exit-code paths are testable, and `ParseResult` is a tagged union so `--help` stops travelling as an error. Adds a README, drops the stale `apps/cli` knip entry, and separates hosts from executable packages in AGENTS.md. Claude-Session: https://claude.ai/code/session_013Gyfq8R69DNpPSV2DBicK5 --- AGENTS.md | 6 +- knip.json | 5 - packages/agent/package.json | 4 + .../agent/src/adapters/acp-connection.test.ts | 73 +++++ packages/agent/src/adapters/acp-connection.ts | 13 +- .../agent/src/adapters/claude/claude-agent.ts | 11 + packages/agent/src/agent.ts | 3 + packages/agent/src/types.ts | 7 + .../src/unattended-permission-policy.test.ts | 256 ++++++++++++++++ .../agent/src/unattended-permission-policy.ts | 63 ++++ packages/agent/tsup.config.ts | 1 + packages/cli/README.md | 77 +++++ packages/cli/e2e/cli.e2e.test.ts | 12 +- packages/cli/src/args.test.ts | 150 +++++----- packages/cli/src/args.ts | 40 +-- packages/cli/src/cli.ts | 68 +++-- packages/cli/src/output.test.ts | 60 ++-- packages/cli/src/output.ts | 15 +- packages/cli/src/permission-policy.test.ts | 148 ---------- packages/cli/src/permission-policy.ts | 43 --- packages/cli/src/run.test.ts | 273 ++++++++++++++++++ packages/cli/src/run.ts | 258 ++++++++++++----- 22 files changed, 1177 insertions(+), 409 deletions(-) create mode 100644 packages/agent/src/adapters/acp-connection.test.ts create mode 100644 packages/agent/src/unattended-permission-policy.test.ts create mode 100644 packages/agent/src/unattended-permission-policy.ts create mode 100644 packages/cli/README.md delete mode 100644 packages/cli/src/permission-policy.test.ts delete mode 100644 packages/cli/src/permission-policy.ts create mode 100644 packages/cli/src/run.test.ts diff --git a/AGENTS.md b/AGENTS.md index 0bdd711ada..3859bc1f86 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -37,7 +37,11 @@ Hosts: - `apps/code`: Electron desktop host. - `apps/web`: web host and portability smoke test. - `apps/mobile`: React Native host. -- `packages/cli`: headless CLI (`@posthog/code-cli`, bin `posthog-code-cli`) — one-shot agent runs over the in-process ACP connection. + +Executable packages own a `bin` rather than an `apps/*` host shell. They boot the same packages a host does, without a UI: + +- `packages/cli`: headless CLI (`@posthog/code-cli`, bin `posthog-code-cli`) for one-shot agent runs over the in-process ACP connection. +- `packages/harness`: `@posthog/harness` (bin `harness`, `hog`), which spawns the pi.dev coding agent against the PostHog LLM gateway. ## Rules diff --git a/knip.json b/knip.json index 677f408831..a7d8741c1a 100644 --- a/knip.json +++ b/knip.json @@ -29,11 +29,6 @@ "@vitest/coverage-v8" ] }, - "apps/cli": { - "entry": ["src/cli.ts"], - "project": ["src/**/*.ts", "bin/**/*.ts"], - "includeEntryExports": true - }, "packages/cli": { "project": ["src/**/*.ts"], "includeEntryExports": true diff --git a/packages/agent/package.json b/packages/agent/package.json index c4e12b6365..d040125584 100644 --- a/packages/agent/package.json +++ b/packages/agent/package.json @@ -104,6 +104,10 @@ "types": "./dist/execution-mode.d.ts", "import": "./dist/execution-mode.js" }, + "./unattended-permission-policy": { + "types": "./dist/unattended-permission-policy.d.ts", + "import": "./dist/unattended-permission-policy.js" + }, "./resume": { "types": "./dist/resume.d.ts", "import": "./dist/resume.js" diff --git a/packages/agent/src/adapters/acp-connection.test.ts b/packages/agent/src/adapters/acp-connection.test.ts new file mode 100644 index 0000000000..0a9cb89ef4 --- /dev/null +++ b/packages/agent/src/adapters/acp-connection.test.ts @@ -0,0 +1,73 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const claudeAgentConstructor = vi.fn(); + +vi.mock("./claude/claude-agent", () => ({ + ClaudeAcpAgent: class { + constructor(...args: unknown[]) { + claudeAgentConstructor(...args); + } + // Called by the connection's cleanup. + async closeSession(): Promise {} + }, +})); + +const { createAcpConnection } = await import("./acp-connection"); +const { Logger } = await import("../utils/logger"); + +/** + * The Claude adapter's own diagnostics include whole payloads: the expanded body + * of a slash command, raw tool inputs, the subprocess's stderr. A host logger + * usually carries an `onLog` that persists or transmits what it receives (on + * desktop, electron-log's file and OTLP transports; on cloud, the run log and + * the user's task feed), so forwarding is opt-in. + */ +describe("createAcpConnection adapter log forwarding", () => { + beforeEach(() => { + claudeAgentConstructor.mockClear(); + }); + + function adapterOptions(): { logger?: unknown } { + // AgentSideConnection builds the agent eagerly, so one call is recorded by + // the time createAcpConnection returns. + expect(claudeAgentConstructor).toHaveBeenCalledTimes(1); + return claudeAgentConstructor.mock.calls[0][1] as { logger?: unknown }; + } + + it("withholds the host logger from the adapter by default", async () => { + const connection = createAcpConnection({ + adapter: "claude", + logger: new Logger({ debug: true, onLog: () => {} }), + }); + try { + expect(adapterOptions().logger).toBeUndefined(); + } finally { + await connection.cleanup(); + } + }); + + it("passes a scoped child logger when the host opts in", async () => { + const connection = createAcpConnection({ + adapter: "claude", + logger: new Logger({ debug: true, onLog: () => {} }), + forwardAdapterLogs: true, + }); + try { + expect(adapterOptions().logger).toBeInstanceOf(Logger); + } finally { + await connection.cleanup(); + } + }); + + it("passes no logger when the host opts in without supplying one", async () => { + const connection = createAcpConnection({ + adapter: "claude", + forwardAdapterLogs: true, + }); + try { + expect(adapterOptions().logger).toBeUndefined(); + } finally { + await connection.cleanup(); + } + }); +}); diff --git a/packages/agent/src/adapters/acp-connection.ts b/packages/agent/src/adapters/acp-connection.ts index 8e13611f8f..7c29a92db0 100644 --- a/packages/agent/src/adapters/acp-connection.ts +++ b/packages/agent/src/adapters/acp-connection.ts @@ -23,6 +23,15 @@ export type AcpConnectionConfig = { /** Deployment environment - "local" for desktop, "cloud" for cloud sandbox */ deviceType?: "local" | "cloud"; logger?: Logger; + /** + * Route the Claude adapter's own diagnostics through `logger` instead of its + * console fallback. Off by default: adapter-internal logging includes whole + * payloads (expanded slash-command output, tool inputs, subprocess stderr), + * and a host `logger` typically carries an `onLog` that persists or transmits + * what it receives. Only a host whose sink is the operator's own terminal + * should turn this on. + */ + forwardAdapterLogs?: boolean; processCallbacks?: ProcessSpawnedCallback; codexOptions?: CodexOptions; codexModels?: ReadonlyArray; @@ -119,7 +128,9 @@ function createClaudeConnection(config: AcpConnectionConfig): AcpConnection { onStructuredOutput: config.onStructuredOutput, posthogApiConfig: resolveEnricherApiConfig(config), gatewayEnv: config.claudeGatewayEnv, - logger: config.logger?.child("ClaudeAcpAgent"), + logger: config.forwardAdapterLogs + ? config.logger?.child("ClaudeAcpAgent") + : undefined, }); return agent; }, agentStream); diff --git a/packages/agent/src/adapters/claude/claude-agent.ts b/packages/agent/src/adapters/claude/claude-agent.ts index c9b26e99ca..8e6a24c1c2 100644 --- a/packages/agent/src/adapters/claude/claude-agent.ts +++ b/packages/agent/src/adapters/claude/claude-agent.ts @@ -2282,6 +2282,17 @@ export class ClaudeAcpAgent extends BaseAcpAgent { settingsManager.getSettings().model, ]); modelOptions.currentModelId = resolvedModelId; + // A requested id that isn't available falls back silently, which reads as + // "the model I asked for" to a caller that can't see the allowed set (a + // headless CLI run, a scripted session). Say so. + const requestedModelId = meta?.model?.trim(); + if (requestedModelId && requestedModelId !== resolvedModelId) { + this.logger.warn("Requested model is unavailable; using another", { + requested: requestedModelId, + resolved: resolvedModelId, + available: modelOptions.options.map((opt) => opt.value), + }); + } session.modelId = resolvedModelId; session.lastContextWindowSize = meta?.contextWindow === "200k" diff --git a/packages/agent/src/agent.ts b/packages/agent/src/agent.ts index 63b3830849..13e113d986 100644 --- a/packages/agent/src/agent.ts +++ b/packages/agent/src/agent.ts @@ -24,6 +24,7 @@ export class Agent { private sessionLogWriter?: SessionLogWriter; private posthogApiConfig?: AgentConfig["posthog"]; private enricherEnabled: boolean; + private forwardAdapterLogs: boolean; constructor(config: AgentConfig) { this.logger = new Logger({ @@ -37,6 +38,7 @@ export class Agent { this.posthogApiConfig = config.posthog; } this.enricherEnabled = config.enricher?.enabled !== false; + this.forwardAdapterLogs = config.forwardAdapterLogs === true; if (config.posthog && !config.skipLogPersistence) { this.sessionLogWriter = new SessionLogWriter({ @@ -131,6 +133,7 @@ export class Agent { taskId, deviceType: "local", logger: this.logger, + forwardAdapterLogs: this.forwardAdapterLogs, processCallbacks: options.processCallbacks, onStructuredOutput: options.onStructuredOutput, codexModels, diff --git a/packages/agent/src/types.ts b/packages/agent/src/types.ts index 8bbffe5e70..39228e1018 100644 --- a/packages/agent/src/types.ts +++ b/packages/agent/src/types.ts @@ -108,6 +108,13 @@ export interface AgentConfig { enricher?: { enabled?: boolean }; debug?: boolean; onLog?: OnLogCallback; + /** + * Send the adapter's own diagnostics to `onLog` too. Off by default because + * those lines carry whole payloads (expanded slash-command output, tool + * inputs, subprocess stderr) and most hosts persist or transmit what `onLog` + * receives. Turn it on only when the sink is the operator's own terminal. + */ + forwardAdapterLogs?: boolean; } // Device info for tracking where work happens diff --git a/packages/agent/src/unattended-permission-policy.test.ts b/packages/agent/src/unattended-permission-policy.test.ts new file mode 100644 index 0000000000..8a419a2826 --- /dev/null +++ b/packages/agent/src/unattended-permission-policy.test.ts @@ -0,0 +1,256 @@ +import type { RequestPermissionRequest } from "@agentclientprotocol/sdk"; +import { describe, expect, it } from "vitest"; +import { resolveUnattendedPermissionRequest } from "./unattended-permission-policy"; + +// These option/toolCall shapes mirror the relevant fields of +// @agentclientprotocol/sdk's RequestPermissionRequest/RequestPermissionResponse, +// trimmed to the fields the pure policy function reads. + +interface Option { + optionId: string; + name: string; + kind: "allow_once" | "allow_always" | "reject_once" | "reject_always"; +} + +function makeParams( + options: Option[], + codeToolKind?: string, + kind?: string, +): RequestPermissionRequest { + return { + sessionId: "session-1", + toolCall: { + toolCallId: "tool-1", + ...(kind ? { kind } : {}), + ...(codeToolKind ? { _meta: { codeToolKind } } : {}), + }, + options, + } as RequestPermissionRequest; +} + +describe("resolveUnattendedPermissionRequest", () => { + describe("question tool calls", () => { + it("cancels with an actionable message instead of auto-approving", () => { + const params = makeParams( + [{ optionId: "opt-1", name: "Allow", kind: "allow_once" }], + "question", + ); + + const result = resolveUnattendedPermissionRequest(params); + + expect(result.outcome.outcome).toBe("cancelled"); + const message = result._meta?.message as string; + expect(typeof message).toBe("string"); + expect(message.length).toBeGreaterThan(0); + expect(message).toMatch(/no user/i); + // The operative half: without these the model loops on a tool nobody + // can answer. + expect(message).toMatch(/do NOT re-ask/i); + expect(message).toMatch(/end your turn/i); + }); + + it("parks the question even when both allow_once and allow_always options exist", () => { + const params = makeParams( + [ + { optionId: "opt-1", name: "Allow always", kind: "allow_always" }, + { optionId: "opt-2", name: "Allow once", kind: "allow_once" }, + ], + "question", + ); + + const result = resolveUnattendedPermissionRequest(params); + + expect(result.outcome.outcome).toBe("cancelled"); + expect(result._meta?.message).toMatch(/no user/i); + }); + }); + + describe("auto-approval", () => { + it.each([ + { + label: "allow_always listed first, allow_once second", + options: [ + { + optionId: "always-1", + name: "Allow always", + kind: "allow_always" as const, + }, + { + optionId: "once-1", + name: "Allow once", + kind: "allow_once" as const, + }, + ], + expectedOptionId: "once-1", + }, + { + label: "allow_once listed first, allow_always second", + options: [ + { + optionId: "once-1", + name: "Allow once", + kind: "allow_once" as const, + }, + { + optionId: "always-1", + name: "Allow always", + kind: "allow_always" as const, + }, + ], + expectedOptionId: "once-1", + }, + ])( + "prefers allow_once over allow_always regardless of order ($label)", + ({ options, expectedOptionId }) => { + const result = resolveUnattendedPermissionRequest(makeParams(options)); + + expect(result.outcome).toEqual({ + outcome: "selected", + optionId: expectedOptionId, + }); + }, + ); + + it("selects allow_always when no allow_once option exists", () => { + const options: Option[] = [ + { optionId: "reject-1", name: "Reject once", kind: "reject_once" }, + { optionId: "always-1", name: "Allow always", kind: "allow_always" }, + ]; + + const result = resolveUnattendedPermissionRequest(makeParams(options)); + + expect(result.outcome).toEqual({ + outcome: "selected", + optionId: "always-1", + }); + }); + + // reject_always first, so passing this requires ranking by kind rather + // than taking options[0]. + it("prefers reject_once over reject_always when only reject kinds are present", () => { + const options: Option[] = [ + { + optionId: "reject-always-1", + name: "Reject always", + kind: "reject_always", + }, + { optionId: "reject-once-1", name: "Reject once", kind: "reject_once" }, + ]; + + const result = resolveUnattendedPermissionRequest(makeParams(options)); + + expect(result.outcome).toEqual({ + outcome: "selected", + optionId: "reject-once-1", + }); + }); + + it("falls back to the first option when no reject_once exists either", () => { + const options: Option[] = [ + { + optionId: "reject-always-1", + name: "Reject always", + kind: "reject_always", + }, + { + optionId: "reject-always-2", + name: "Reject always too", + kind: "reject_always", + }, + ]; + + const result = resolveUnattendedPermissionRequest(makeParams(options)); + + expect(result.outcome).toEqual({ + outcome: "selected", + optionId: "reject-always-1", + }); + }); + }); + + // A plan approval's options are session modes, and buildExitPlanModePermissionOptions + // puts the mode to continue in at index 0. Ranking by kind would pick the sole + // allow_once ("default"), downgrading an unattended run to interactive and + // persisting that mode into the repo's local settings. + describe("plan approval (switch_mode)", () => { + const exitPlanModeOptions: Option[] = [ + { + optionId: "auto", + name: 'Yes, continue in "auto" mode', + kind: "allow_always", + }, + { + optionId: "acceptEdits", + name: "Yes, and auto-accept edits", + kind: "allow_always", + }, + { + optionId: "default", + name: "Yes, and manually approve edits", + kind: "allow_once", + }, + { + optionId: "reject_with_feedback", + name: "No, and tell the agent what to do differently", + kind: "reject_once", + }, + ]; + + it("continues in the mode the run started in rather than switching to default", () => { + const result = resolveUnattendedPermissionRequest( + makeParams(exitPlanModeOptions, undefined, "switch_mode"), + ); + + expect(result.outcome).toEqual({ + outcome: "selected", + optionId: "auto", + }); + }); + + it("continues in bypassPermissions when that is the mode in front", () => { + const result = resolveUnattendedPermissionRequest( + makeParams( + [ + { + optionId: "bypassPermissions", + name: "Yes, continue bypassing all permissions", + kind: "allow_always", + }, + ...exitPlanModeOptions, + ], + undefined, + "switch_mode", + ), + ); + + expect(result.outcome).toEqual({ + outcome: "selected", + optionId: "bypassPermissions", + }); + }); + + it("still parks a question even when the tool call is a mode switch", () => { + const result = resolveUnattendedPermissionRequest( + makeParams(exitPlanModeOptions, "question", "switch_mode"), + ); + + expect(result.outcome.outcome).toBe("cancelled"); + }); + }); + + describe("empty options", () => { + it("cancels when the options array is empty", () => { + const result = resolveUnattendedPermissionRequest(makeParams([])); + + expect(result.outcome).toEqual({ outcome: "cancelled" }); + }); + + it("cancels a plan approval with no options", () => { + const result = resolveUnattendedPermissionRequest( + makeParams([], undefined, "switch_mode"), + ); + + expect(result.outcome).toEqual({ outcome: "cancelled" }); + }); + }); +}); diff --git a/packages/agent/src/unattended-permission-policy.ts b/packages/agent/src/unattended-permission-policy.ts new file mode 100644 index 0000000000..f7379019b2 --- /dev/null +++ b/packages/agent/src/unattended-permission-policy.ts @@ -0,0 +1,63 @@ +import type { + RequestPermissionRequest, + RequestPermissionResponse, +} from "@agentclientprotocol/sdk"; + +/** + * The no-user-available contract: the model must surface the question as regular + * assistant text and end its turn instead of guessing an answer. + * + * AgentServer's background-mode branch has its own copy of this that ends "so the + * user can answer when they are back", which fits a cloud task someone returns + * to. Keep them in sync when the instructions change. + */ +const UNATTENDED_QUESTION_MESSAGE = + "No user is available to answer this question. Do NOT pick an answer yourself " + + "and do NOT re-ask via this tool. Restate the question and its options in your " + + "response, then end your turn."; + +/** + * Resolves a permission request for a host with no user to ask: question tool + * calls are parked, everything else that reaches the client is auto-approved. + * + * Prefers `allow_once` so an unattended run never persists allow-always rules + * into the target repository's settings, and a reject-only request resolves to + * `reject_once` rather than whatever option happens to be listed first. Plan + * approvals are the exception, see below. + * + * Two other unattended policies exist and behave differently: AgentServer's + * cloud branch and `buildAutoApproveOutcome` in `@posthog/workspace-server` both + * take the first allow option in array order, and neither prefers `reject_once`. + * This is the intended behavior for all three; adopting it there is a follow-up. + */ +export function resolveUnattendedPermissionRequest( + params: RequestPermissionRequest, +): RequestPermissionResponse { + if (params.toolCall._meta?.codeToolKind === "question") { + return { + outcome: { outcome: "cancelled" }, + _meta: { message: UNATTENDED_QUESTION_MESSAGE }, + }; + } + + const { options } = params; + + // A plan approval's options are session modes, not degrees of consent, and its + // only allow_once is "manually approve edits". Preferring kind there would + // switch an unattended run into an interactive mode and persist that into the + // repo's local settings. The adapter puts the current mode first, so take + // array order and carry on in the mode the caller asked for. + const chosen = + params.toolCall.kind === "switch_mode" + ? (options.find( + (o) => o.kind === "allow_once" || o.kind === "allow_always", + ) ?? options[0]) + : (options.find((o) => o.kind === "allow_once") ?? + options.find((o) => o.kind === "allow_always") ?? + options.find((o) => o.kind === "reject_once") ?? + options[0]); + if (!chosen) { + return { outcome: { outcome: "cancelled" } }; + } + return { outcome: { outcome: "selected", optionId: chosen.optionId } }; +} diff --git a/packages/agent/tsup.config.ts b/packages/agent/tsup.config.ts index 55bfd03fea..43e6d93f48 100644 --- a/packages/agent/tsup.config.ts +++ b/packages/agent/tsup.config.ts @@ -148,6 +148,7 @@ export default defineConfig([ "src/adapters/claude/mcp/tool-metadata.ts", "src/adapters/reasoning-effort.ts", "src/execution-mode.ts", + "src/unattended-permission-policy.ts", "src/server/schemas.ts", "src/server/agent-server.ts", "src/server/bin.ts", diff --git a/packages/cli/README.md b/packages/cli/README.md new file mode 100644 index 0000000000..40c5fc533b --- /dev/null +++ b/packages/cli/README.md @@ -0,0 +1,77 @@ +# @posthog/code-cli + +Headless CLI for PostHog Code: one agent turn against a local repository, prompt in, answer on stdout, exit code out. Drives the same in-process ACP connection the desktop app uses. + +```bash +posthog-code-cli "Fix the failing test" --cwd ~/src/myrepo +``` + +The prompt can also come from stdin: + +```bash +echo "Summarize the last commit" | posthog-code-cli --cwd ~/src/myrepo +``` + +## Flags + +| Flag | Default | Notes | +| --- | --- | --- | +| `[prompt]` | reads stdin when piped | Positional. Quote it; multiple words without quotes are an error. | +| `--cwd ` | current directory | Must exist and be a directory. Resolved to its realpath. | +| `--permission-mode ` | `auto` | `auto` or `bypassPermissions`. Interactive modes need a UI to answer prompts, so they are rejected. | +| `--model ` | session default | Must start with `claude-`. An id that isn't available is substituted, and the adapter warns (visible with `--debug`). | +| `--system-prompt ` | preset | Replaces the default system prompt. | +| `--output ` | `text` | `text` streams as it arrives; `json` emits one document. | +| `--debug` | off | Verbose diagnostics on stderr. | + +## Output + +stdout carries only assistant output. Every diagnostic goes to stderr, so `--output json` is safe to pipe. + +`--output text` streams each assistant chunk as it arrives and terminates with a newline. `--output json` buffers and emits a single document: + +```json +{ "text": "…", "stopReason": "end_turn", "usage": null, "sessionId": "…" } +``` + +`usage` is `null` rather than absent when the turn settled without token counts, so `.usage` is always safe to read. Nothing is written on a hard mid-turn failure, so check the exit code before parsing. + +## Exit codes + +| Code | Meaning | +| --- | --- | +| 0 | `end_turn` | +| 1 | Bad arguments, no prompt, or an unexpected failure (reason on stderr) | +| 2 | `refusal` | +| 3 | `max_tokens` or `max_turn_requests` | +| 130 | SIGINT | +| 143 | SIGTERM | + +Both signals cancel the turn and tear the agent subprocess down before exiting. + +## Authentication + +Auth comes from the environment and is passed to the agent subprocess: + +- `ANTHROPIC_API_KEY` or `ANTHROPIC_AUTH_TOKEN` +- `ANTHROPIC_BASE_URL` to point at a gateway + +With neither key set, the CLI warns and falls back to a stored `claude` login credential. + +## Unattended behavior + +There is no user to answer prompts, so: + +- Tool permissions that reach the client are auto-approved, preferring `allow_once` so a run never persists allow-always rules into the target repo's settings. See `resolveUnattendedPermissionRequest` in `@posthog/agent`. +- `AskUserQuestion` calls are parked: the model is told to restate the question as text and end its turn rather than answer on the user's behalf. +- `bypassPermissions` as root outside a sandbox fails fast, matching what the agent subprocess would refuse anyway. Set `IS_SANDBOX=1` if that is genuinely the situation. + +## Development + +```bash +pnpm --filter @posthog/code-cli build # tsup -> dist/cli.js +pnpm --filter @posthog/code-cli test # vitest, src/**/*.test.ts +pnpm --filter @posthog/code-cli typecheck +``` + +`pnpm --filter @posthog/code-cli test:e2e` runs a live turn against the built binary. It needs `POSTHOG_CODE_E2E_GATEWAY_PERSONAL_API_KEY` (same env contract as `packages/agent/e2e`) and fails rather than skipping when it is unset. diff --git a/packages/cli/e2e/cli.e2e.test.ts b/packages/cli/e2e/cli.e2e.test.ts index 7a06446881..35290b3f4f 100644 --- a/packages/cli/e2e/cli.e2e.test.ts +++ b/packages/cli/e2e/cli.e2e.test.ts @@ -14,7 +14,8 @@ import { describe, expect, it } from "vitest"; const execFileAsync = promisify(execFile); // Same env contract as packages/agent/e2e/config.ts: a local llm-gateway plus -// a personal API key. Without the token the suite self-skips (never silent). +// a personal API key. Without the token the suite self-skips, so the guard test +// below fails rather than letting the run go green having tested nothing. const GATEWAY_URL = process.env.POSTHOG_CODE_E2E_GATEWAY_URL || "http://localhost:3308/ci"; const TOKEN = process.env.POSTHOG_CODE_E2E_GATEWAY_PERSONAL_API_KEY ?? ""; @@ -47,6 +48,15 @@ function setupRepo(): string { return repo; } +// Outside the skipIf, mirroring packages/agent/e2e/guard.e2e.test.ts: without +// this, an unset token turns the whole suite into a pass that ran nothing. +it("requires POSTHOG_CODE_E2E_GATEWAY_PERSONAL_API_KEY", () => { + expect( + !!TOKEN, + "POSTHOG_CODE_E2E_GATEWAY_PERSONAL_API_KEY is not set, so the CLI e2e suite would skip and the run would pass without exercising the binary.", + ).toBe(true); +}); + describe.skipIf(!TOKEN)("posthog-code-cli", () => { it("runs one turn and prints the answer", async () => { expect( diff --git a/packages/cli/src/args.test.ts b/packages/cli/src/args.test.ts index 00096764a0..0c41fd9853 100644 --- a/packages/cli/src/args.test.ts +++ b/packages/cli/src/args.test.ts @@ -1,102 +1,110 @@ -import { mkdtempSync, realpathSync } from "node:fs"; +import { mkdtempSync, realpathSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; -import { parseCliArgs } from "./args"; +import { type CliOptions, type ParseResult, parseCliArgs } from "./args"; // argv passed to parseCliArgs mirrors process.argv: [node, script, ...args]. function argv(...args: string[]): string[] { return ["/usr/bin/node", "/path/to/posthog-code-cli", ...args]; } +// Assert and narrow in one step, so an unexpected parse failure reports the +// message that caused it rather than "expected true to be false". +function expectOptions(result: ParseResult): CliOptions { + if (result.kind !== "options") { + throw new Error( + `expected parsed options, got ${result.kind}: ${JSON.stringify(result)}`, + ); + } + return result.options; +} + +function expectError(result: ParseResult): string { + if (result.kind !== "error") { + throw new Error( + `expected a parse error, got ${result.kind}: ${JSON.stringify(result)}`, + ); + } + return result.message; +} + describe("parseCliArgs", () => { describe("defaults", () => { it("returns default options when no flags or prompt are given", () => { - const result = parseCliArgs(argv()); - - expect("error" in result).toBe(false); - if ("error" in result) return; - expect(result.prompt).toBeUndefined(); - expect(result.cwd).toBe(realpathSync(process.cwd())); - expect(result.permissionMode).toBe("auto"); - expect(result.output).toBe("text"); - expect(result.debug).toBe(false); + const options = expectOptions(parseCliArgs(argv())); + + expect(options.prompt).toBeUndefined(); + expect(options.cwd).toBe(realpathSync(process.cwd())); + expect(options.permissionMode).toBe("auto"); + expect(options.output).toBe("text"); + expect(options.debug).toBe(false); }); }); it("parses the prompt positional", () => { - const result = parseCliArgs(argv("Fix the failing test")); + const options = expectOptions(parseCliArgs(argv("Fix the failing test"))); - expect("error" in result).toBe(false); - if ("error" in result) return; - expect(result.prompt).toBe("Fix the failing test"); + expect(options.prompt).toBe("Fix the failing test"); }); describe("individual flags", () => { it("parses --cwd to the realpath of a valid directory", () => { const tmp = mkdtempSync(join(tmpdir(), "cli-args-test-")); - const result = parseCliArgs(argv("--cwd", tmp)); + const options = expectOptions(parseCliArgs(argv("--cwd", tmp))); - expect("error" in result).toBe(false); - if ("error" in result) return; - expect(result.cwd).toBe(realpathSync(tmp)); + expect(options.cwd).toBe(realpathSync(tmp)); }); it("parses --permission-mode bypassPermissions", () => { - const result = parseCliArgs( - argv("--permission-mode", "bypassPermissions"), + const options = expectOptions( + parseCliArgs(argv("--permission-mode", "bypassPermissions")), ); - expect("error" in result).toBe(false); - if ("error" in result) return; - expect(result.permissionMode).toBe("bypassPermissions"); + expect(options.permissionMode).toBe("bypassPermissions"); }); it("parses a valid --model id", () => { - const result = parseCliArgs(argv("--model", "claude-opus-4")); + const options = expectOptions( + parseCliArgs(argv("--model", "claude-opus-4")), + ); - expect("error" in result).toBe(false); - if ("error" in result) return; - expect(result.model).toBe("claude-opus-4"); + expect(options.model).toBe("claude-opus-4"); }); it("parses --system-prompt as-is", () => { - const result = parseCliArgs( - argv("--system-prompt", "Respond only in haiku"), + const options = expectOptions( + parseCliArgs(argv("--system-prompt", "Respond only in haiku")), ); - expect("error" in result).toBe(false); - if ("error" in result) return; - expect(result.systemPrompt).toBe("Respond only in haiku"); + expect(options.systemPrompt).toBe("Respond only in haiku"); }); it("parses --output json", () => { - const result = parseCliArgs(argv("--output", "json")); + const options = expectOptions(parseCliArgs(argv("--output", "json"))); - expect("error" in result).toBe(false); - if ("error" in result) return; - expect(result.output).toBe("json"); + expect(options.output).toBe("json"); }); it("parses --debug as true when present", () => { - const result = parseCliArgs(argv("--debug")); + const options = expectOptions(parseCliArgs(argv("--debug"))); - expect("error" in result).toBe(false); - if ("error" in result) return; - expect(result.debug).toBe(true); + expect(options.debug).toBe(true); }); }); describe("invalid --permission-mode values", () => { it.each(["default", "plan", "acceptEdits", "yolo"])( - "rejects %j with an error result", + "rejects %j and names the allowed modes", (mode) => { - const result = parseCliArgs(argv("--permission-mode", mode)); - - expect("error" in result).toBe(true); - if (!("error" in result)) return; - expect(typeof result.error).toBe("string"); - expect(result.error.length).toBeGreaterThan(0); + const message = expectError( + parseCliArgs(argv("--permission-mode", mode)), + ); + + // Message content, not just non-emptiness: an "unknown option" error + // would otherwise pass as if the value had been rejected. + expect(message).toMatch(/auto/); + expect(message).toMatch(/bypassPermissions/); }, ); }); @@ -105,41 +113,47 @@ describe("parseCliArgs", () => { it.each(["gpt-4", "llama-3"])( "rejects %j since it does not start with claude-", (model) => { - const result = parseCliArgs(argv("--model", model)); - - expect("error" in result).toBe(true); - if (!("error" in result)) return; - expect(typeof result.error).toBe("string"); - expect(result.error.length).toBeGreaterThan(0); + expect(expectError(parseCliArgs(argv("--model", model)))).toMatch( + /claude-/, + ); }, ); }); it("rejects an invalid --output value", () => { - const result = parseCliArgs(argv("--output", "yaml")); + const message = expectError(parseCliArgs(argv("--output", "yaml"))); - expect("error" in result).toBe(true); - if (!("error" in result)) return; - expect(typeof result.error).toBe("string"); - expect(result.error.length).toBeGreaterThan(0); + expect(message).toMatch(/text/); + expect(message).toMatch(/json/); }); it("rejects a --cwd path that does not exist", () => { const bogusPath = join(tmpdir(), "definitely-does-not-exist-cli-args-test"); - const result = parseCliArgs(argv("--cwd", bogusPath)); - expect("error" in result).toBe(true); - if (!("error" in result)) return; - expect(typeof result.error).toBe("string"); - expect(result.error.length).toBeGreaterThan(0); + expect(expectError(parseCliArgs(argv("--cwd", bogusPath)))).toMatch( + /No such directory/, + ); + }); + + it("rejects a --cwd path pointing to a file rather than a directory", () => { + const file = join(mkdtempSync(join(tmpdir(), "cli-args-test-")), "f.txt"); + writeFileSync(file, "x"); + + expect(expectError(parseCliArgs(argv("--cwd", file)))).toMatch( + /Not a directory/, + ); }); it("accepts a --cwd path pointing to a real directory", () => { const tmp = mkdtempSync(join(tmpdir(), "cli-args-test-")); - const result = parseCliArgs(argv("--cwd", tmp)); + const options = expectOptions(parseCliArgs(argv("--cwd", tmp))); + + expect(options.cwd).toBe(realpathSync(tmp)); + }); - expect("error" in result).toBe(false); - if ("error" in result) return; - expect(result.cwd).toBe(realpathSync(tmp)); + // Commander writes help to stdout itself, so the result carries no message + // and exit 0. Inverting this would break any script that probes usage. + it("maps --help to a silent success", () => { + expect(parseCliArgs(argv("--help"))).toEqual({ kind: "exit", exitCode: 0 }); }); }); diff --git a/packages/cli/src/args.ts b/packages/cli/src/args.ts index 8bc7abe87b..3febef395b 100644 --- a/packages/cli/src/args.ts +++ b/packages/cli/src/args.ts @@ -28,12 +28,11 @@ export interface CliOptions { debug: boolean; } -export interface ParseError { - error: string; - exitCode: number; -} - -export type ParseResult = CliOptions | ParseError; +export type ParseResult = + | { kind: "options"; options: CliOptions } + /** Commander already wrote help or version to stdout; exit quietly. */ + | { kind: "exit"; exitCode: number } + | { kind: "error"; message: string }; function parseModel(value: string): string { // Non-claude ids are silently coerced to the default model downstream, so @@ -109,13 +108,13 @@ export function parseCliArgs(argv: string[]): ParseResult { } catch (err) { if (err instanceof CommanderError) { // Help/version output was already written by commander itself. - const informational = + if ( err.code === "commander.helpDisplayed" || - err.code === "commander.version"; - return { - error: informational ? "" : err.message, - exitCode: informational ? 0 : 1, - }; + err.code === "commander.version" + ) { + return { kind: "exit", exitCode: 0 }; + } + return { kind: "error", message: err.message }; } throw err; } @@ -130,12 +129,15 @@ export function parseCliArgs(argv: string[]): ParseResult { }>(); return { - prompt: program.args[0], - cwd: opts.cwd, - permissionMode: opts.permissionMode, - model: opts.model, - systemPrompt: opts.systemPrompt, - output: opts.output, - debug: opts.debug, + kind: "options", + options: { + prompt: program.args[0], + cwd: opts.cwd, + permissionMode: opts.permissionMode, + model: opts.model, + systemPrompt: opts.systemPrompt, + output: opts.output, + debug: opts.debug, + }, }; } diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index a847528e32..1c8a2821c4 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -1,20 +1,41 @@ #!/usr/bin/env node import { text } from "node:stream/consumers"; +import { withTimeout } from "@posthog/shared"; import { parseCliArgs } from "./args"; import { run } from "./run"; +// A reader that exits early (`… | head -1`) closes the pipe under us. Without a +// listener the next write raises an unhandled 'error' event, which crashes with +// a raw stack trace before cleanup can run. Swallowing it lets the run unwind +// through its normal teardown, so the agent subprocess still gets torn down; +// further writes are discarded, and the force-exit backstop covers the flush +// callback that a closed pipe will never fire. Registered before any output. +process.stdout.on("error", (err: NodeJS.ErrnoException) => { + if (err.code !== "EPIPE") throw err; +}); + +// A non-TTY stdin that never reaches EOF (an inherited but idle stdin under a +// supervisor or CI step) must not hang a one-shot run. +const STDIN_READ_TIMEOUT_MS = 30_000; + async function main(): Promise { const parsed = parseCliArgs(process.argv); - if ("error" in parsed) { - if (parsed.error) { - process.stderr.write(`${parsed.error}\n`); - } - return parsed.exitCode; + if (parsed.kind === "exit") return parsed.exitCode; + if (parsed.kind === "error") { + process.stderr.write(`${parsed.message}\n`); + return 1; } + const { options } = parsed; - let prompt = parsed.prompt; + let prompt = options.prompt?.trim(); if (!prompt && !process.stdin.isTTY) { - prompt = (await text(process.stdin)).trim(); + const piped = await withTimeout(text(process.stdin), STDIN_READ_TIMEOUT_MS); + if (piped.result === "success") { + prompt = piped.value.trim(); + } else { + // The read is still pending and holds the event loop open. + process.stdin.destroy(); + } } if (!prompt) { process.stderr.write( @@ -24,10 +45,11 @@ async function main(): Promise { } // The Claude subprocess refuses bypass for root outside a sandbox; fail - // here with a clear message instead of a cryptic session error. + // here with a clear message instead of a cryptic session error. Effective uid + // first, matching the IS_ROOT check in @posthog/agent that actually gates it. if ( - parsed.permissionMode === "bypassPermissions" && - process.getuid?.() === 0 && + options.permissionMode === "bypassPermissions" && + (process.geteuid?.() ?? process.getuid?.()) === 0 && !process.env.IS_SANDBOX ) { process.stderr.write( @@ -43,25 +65,33 @@ async function main(): Promise { ); } - return run({ ...parsed, prompt }); + return run({ ...options, prompt }); } +// The agent subprocess can leave handles open past cleanup, so force-exit once +// stdout has drained; a clean event loop still exits naturally before then. +const POST_FLUSH_GRACE_MS = 500; +// Backstop for a consumer that never drains the pipe, which would otherwise +// keep the flush callback from ever firing. +const STALLED_CONSUMER_TIMEOUT_MS = 10_000; + // process.exit does not wait for pending pipe writes, so flush stdout first // (the write callback fires once earlier writes are accepted by the OS). -// The agent subprocess can leave handles open past cleanup; the unref'd timer -// force-exits then, while a clean event loop still exits naturally. The -// unconditional backstop covers a stalled consumer that never drains the pipe, -// which would otherwise keep the flush callback from ever firing. function exitAfterFlush(code: number): void { process.exitCode = code; - setTimeout(() => process.exit(code), 10_000).unref(); + setTimeout(() => process.exit(code), STALLED_CONSUMER_TIMEOUT_MS).unref(); process.stdout.write("", () => { - setTimeout(() => process.exit(code), 500).unref(); + setTimeout(() => process.exit(code), POST_FLUSH_GRACE_MS).unref(); }); } main().then(exitAfterFlush, (err) => { - const message = err instanceof Error ? err.message : String(err); - process.stderr.write(`posthog-code-cli: ${message}\n`); + // parsed.debug is out of scope here, and an unexpected throw is exactly when + // the stack is worth having. + const detail = + err instanceof Error + ? (process.argv.includes("--debug") && err.stack) || err.message + : String(err); + process.stderr.write(`posthog-code-cli: ${detail}\n`); exitAfterFlush(1); }); diff --git a/packages/cli/src/output.test.ts b/packages/cli/src/output.test.ts index bc906ffc8b..22c7b5fac7 100644 --- a/packages/cli/src/output.test.ts +++ b/packages/cli/src/output.test.ts @@ -28,7 +28,7 @@ function textUpdate(sessionId: string, text: string): SessionNotification { // Some entries are deliberately partial protocol shapes (the sink must ignore // them at runtime), so they are cast rather than fully constructed. -function ignoredUpdate( +function ignoredUpdates( sessionId: string, ): { label: string; update: SessionNotification }[] { return [ @@ -89,7 +89,7 @@ describe("createOutputSink", () => { expect(stdout.output).toBe("Hello, world!"); }); - it.each(ignoredUpdate("s1"))("ignores $label updates", ({ update }) => { + it.each(ignoredUpdates("s1"))("ignores $label updates", ({ update }) => { const stdout = makeFakeStdout(); const sink = createOutputSink("text", stdout); @@ -100,28 +100,25 @@ describe("createOutputSink", () => { expect(stdout.output).toBe("kept text"); }); - it("does not emit a JSON document from finish()", () => { + it("emits the streamed text and a terminating newline, and nothing else", () => { const stdout = makeFakeStdout(); const sink = createOutputSink("text", stdout); sink.onSessionUpdate(textUpdate("s1", "some streamed text")); sink.finish({ stopReason: "end_turn", sessionId: "s1" }); - // Text mode must never buffer the streamed text into a finish-result - // JSON document. Unparseable output is fine — that IS plain text. - let parsed: unknown; - try { - parsed = JSON.parse(stdout.output); - } catch { - parsed = undefined; - } - const isFinishDocument = - typeof parsed === "object" && - parsed !== null && - "text" in parsed && - "stopReason" in parsed; - expect(isFinishDocument).toBe(false); - expect(stdout.output).toContain("some streamed text"); + // Exact equality, so text mode can't regress into appending a + // finish-result JSON document after the streamed text. + expect(stdout.output).toBe("some streamed text\n"); + }); + + it("writes nothing at all when the turn produced no assistant text", () => { + const stdout = makeFakeStdout(); + const sink = createOutputSink("text", stdout); + + sink.finish({ stopReason: "end_turn", sessionId: "s1" }); + + expect(stdout.output).toBe(""); }); }); @@ -142,22 +139,35 @@ describe("createOutputSink", () => { sink.onSessionUpdate(textUpdate("s1", "Hello, ")); sink.onSessionUpdate(textUpdate("s1", "world")); - sink.finish({ - stopReason: "end_turn", - usage: { totalTokens: 42 }, - sessionId: "s1", - }); + const usage = { totalTokens: 42, inputTokens: 30, outputTokens: 12 }; + sink.finish({ stopReason: "end_turn", usage, sessionId: "s1" }); const parsed = JSON.parse(stdout.output); expect(parsed).toEqual({ text: "Hello, world", stopReason: "end_turn", - usage: { totalTokens: 42 }, + usage, + sessionId: "s1", + }); + }); + + // Several adapter settle paths carry no usage. The key has to stay in the + // document so a consumer reading usage.totalTokens sees null, not a crash. + it("emits usage: null when the turn settled without usage", () => { + const stdout = makeFakeStdout(); + const sink = createOutputSink("json", stdout); + + sink.finish({ stopReason: "cancelled", sessionId: "s1" }); + + expect(JSON.parse(stdout.output)).toEqual({ + text: "", + stopReason: "cancelled", + usage: null, sessionId: "s1", }); }); - it.each(ignoredUpdate("s1"))( + it.each(ignoredUpdates("s1"))( "excludes $label updates from the concatenated text", ({ update }) => { const stdout = makeFakeStdout(); diff --git a/packages/cli/src/output.ts b/packages/cli/src/output.ts index 88dffa43df..bb5991f63a 100644 --- a/packages/cli/src/output.ts +++ b/packages/cli/src/output.ts @@ -1,9 +1,13 @@ -import type { SessionNotification } from "@agentclientprotocol/sdk"; +import type { + PromptResponse, + SessionNotification, + StopReason, +} from "@agentclientprotocol/sdk"; import type { OutputMode } from "./args"; export interface FinishResult { - stopReason: string; - usage?: unknown; + stopReason: StopReason; + usage?: PromptResponse["usage"]; sessionId: string; } @@ -48,8 +52,11 @@ export function createOutputSink( }, finish({ stopReason, usage, sessionId }) { if (mode === "json") { + // usage ?? null: several adapter settle paths carry no usage, and + // JSON.stringify drops an undefined value, which would change the + // document's shape out from under a consumer reading usage.totalTokens. stdout.write( - `${JSON.stringify({ text: chunks.join(""), stopReason, usage, sessionId })}\n`, + `${JSON.stringify({ text: chunks.join(""), stopReason, usage: usage ?? null, sessionId })}\n`, ); } else if (streamedText) { stdout.write("\n"); diff --git a/packages/cli/src/permission-policy.test.ts b/packages/cli/src/permission-policy.test.ts deleted file mode 100644 index 9309e60bc2..0000000000 --- a/packages/cli/src/permission-policy.test.ts +++ /dev/null @@ -1,148 +0,0 @@ -import type { RequestPermissionRequest } from "@agentclientprotocol/sdk"; -import { describe, expect, it } from "vitest"; -import { resolvePermissionRequest } from "./permission-policy"; - -// These option/toolCall shapes mirror the relevant fields of -// @agentclientprotocol/sdk's RequestPermissionRequest/RequestPermissionResponse, -// trimmed to the fields the pure policy function reads. - -interface Option { - optionId: string; - name: string; - kind: "allow_once" | "allow_always" | "reject_once" | "reject_always"; -} - -function makeParams( - options: Option[], - codeToolKind?: string, -): RequestPermissionRequest { - return { - sessionId: "session-1", - toolCall: { - toolCallId: "tool-1", - ...(codeToolKind ? { _meta: { codeToolKind } } : {}), - }, - options, - } as RequestPermissionRequest; -} - -describe("resolvePermissionRequest", () => { - describe("question tool calls", () => { - it("cancels with an actionable message instead of auto-approving", () => { - const params = makeParams( - [{ optionId: "opt-1", name: "Allow", kind: "allow_once" }], - "question", - ); - - const result = resolvePermissionRequest(params); - - expect(result.outcome.outcome).toBe("cancelled"); - const message = result._meta?.message as string; - expect(typeof message).toBe("string"); - expect(message.length).toBeGreaterThan(0); - expect(message).toMatch(/no user/i); - }); - - it("parks the question even when both allow_once and allow_always options exist", () => { - const params = makeParams( - [ - { optionId: "opt-1", name: "Allow always", kind: "allow_always" }, - { optionId: "opt-2", name: "Allow once", kind: "allow_once" }, - ], - "question", - ); - - const result = resolvePermissionRequest(params); - - expect(result.outcome.outcome).toBe("cancelled"); - expect(result._meta?.message).toMatch(/no user/i); - }); - }); - - describe("auto-approval", () => { - it.each([ - { - label: "allow_always listed first, allow_once second", - options: [ - { - optionId: "always-1", - name: "Allow always", - kind: "allow_always" as const, - }, - { - optionId: "once-1", - name: "Allow once", - kind: "allow_once" as const, - }, - ], - expectedOptionId: "once-1", - }, - { - label: "allow_once listed first, allow_always second", - options: [ - { - optionId: "once-1", - name: "Allow once", - kind: "allow_once" as const, - }, - { - optionId: "always-1", - name: "Allow always", - kind: "allow_always" as const, - }, - ], - expectedOptionId: "once-1", - }, - ])( - "prefers allow_once over allow_always regardless of order ($label)", - ({ options, expectedOptionId }) => { - const result = resolvePermissionRequest(makeParams(options)); - - expect(result.outcome).toEqual({ - outcome: "selected", - optionId: expectedOptionId, - }); - }, - ); - - it("selects allow_always when no allow_once option exists", () => { - const options: Option[] = [ - { optionId: "reject-1", name: "Reject once", kind: "reject_once" }, - { optionId: "always-1", name: "Allow always", kind: "allow_always" }, - ]; - - const result = resolvePermissionRequest(makeParams(options)); - - expect(result.outcome).toEqual({ - outcome: "selected", - optionId: "always-1", - }); - }); - - it("falls back to the first option when only reject kinds are present", () => { - const options: Option[] = [ - { optionId: "reject-once-1", name: "Reject once", kind: "reject_once" }, - { - optionId: "reject-always-1", - name: "Reject always", - kind: "reject_always", - }, - ]; - - const result = resolvePermissionRequest(makeParams(options)); - - expect(result.outcome).toEqual({ - outcome: "selected", - optionId: "reject-once-1", - }); - }); - }); - - describe("empty options", () => { - it("cancels when the options array is empty", () => { - const result = resolvePermissionRequest(makeParams([])); - - expect(result.outcome).toEqual({ outcome: "cancelled" }); - }); - }); -}); diff --git a/packages/cli/src/permission-policy.ts b/packages/cli/src/permission-policy.ts deleted file mode 100644 index 415e5863e7..0000000000 --- a/packages/cli/src/permission-policy.ts +++ /dev/null @@ -1,43 +0,0 @@ -import type { - RequestPermissionRequest, - RequestPermissionResponse, -} from "@agentclientprotocol/sdk"; - -/** - * Mirrors the cloud background-mode reply: the model must surface the question - * as regular assistant text and end its turn instead of guessing an answer. - */ -const UNATTENDED_QUESTION_MESSAGE = - "No user is available to answer this question. Do NOT pick an answer yourself " + - "and do NOT re-ask via this tool. Restate the question and its options in your " + - "response, then end your turn."; - -/** - * Unattended permission policy. Question tool calls are parked (no user to - * answer); everything else that reaches the client is auto-approved. Prefers - * allow_once so a one-shot run never persists allow-always rules into the - * target repository's settings; likewise a reject-only request resolves to - * reject_once rather than whatever option happens to be listed first. - */ -export function resolvePermissionRequest( - params: RequestPermissionRequest, -): RequestPermissionResponse { - const meta = params.toolCall._meta as { codeToolKind?: string } | undefined; - if (meta?.codeToolKind === "question") { - return { - outcome: { outcome: "cancelled" }, - _meta: { message: UNATTENDED_QUESTION_MESSAGE }, - }; - } - - const { options } = params; - const chosen = - options.find((o) => o.kind === "allow_once") ?? - options.find((o) => o.kind === "allow_always") ?? - options.find((o) => o.kind === "reject_once") ?? - options[0]; - if (!chosen) { - return { outcome: { outcome: "cancelled" } }; - } - return { outcome: { outcome: "selected", optionId: chosen.optionId } }; -} diff --git a/packages/cli/src/run.test.ts b/packages/cli/src/run.test.ts new file mode 100644 index 0000000000..d74f1fb6fa --- /dev/null +++ b/packages/cli/src/run.test.ts @@ -0,0 +1,273 @@ +import type { StopReason } from "@agentclientprotocol/sdk"; +import { describe, expect, it, vi } from "vitest"; +import { createOutputSink } from "./output"; +import { + exitCodeFor, + type RunOptions, + runTurn, + type TurnConnection, +} from "./run"; + +// The exit code is the CLI's machine-facing contract: a script branching on $? +// has no other signal, so every stop reason's mapping is pinned here. +describe("exitCodeFor", () => { + it.each([ + { stopReason: "end_turn", expected: 0 }, + { stopReason: "refusal", expected: 2 }, + { stopReason: "max_tokens", expected: 3 }, + { stopReason: "max_turn_requests", expected: 3 }, + // No signal arrived, so a cancelled turn is a failure the caller should see. + { stopReason: "cancelled", expected: 1 }, + ] satisfies { stopReason: StopReason; expected: number }[])( + "maps $stopReason to $expected", + ({ stopReason, expected }) => { + expect(exitCodeFor(stopReason)).toBe(expected); + }, + ); + + it("maps an unrecognized stop reason to 1", () => { + expect(exitCodeFor("something_new" as StopReason)).toBe(1); + }); + + describe("when a signal interrupted the turn", () => { + it.each([ + { label: "SIGINT", interruptedBy: 130 }, + { label: "SIGTERM", interruptedBy: 143 }, + ])( + "reports $interruptedBy for $label whatever the adapter settled on", + ({ interruptedBy }) => { + // Including end_turn: the adapter can finish the turn cleanly in the + // window between the signal and the cancel taking effect. + for (const stopReason of [ + "cancelled", + "end_turn", + "refusal", + ] satisfies StopReason[]) { + expect(exitCodeFor(stopReason, interruptedBy)).toBe(interruptedBy); + } + }, + ); + }); +}); + +function makeOptions(overrides: Partial = {}): RunOptions { + return { + prompt: "do the thing", + cwd: "/repo", + permissionMode: "auto", + output: "json", + debug: false, + ...overrides, + }; +} + +function makeSink() { + const chunks: string[] = []; + const sink = createOutputSink("json", { + write(s: string) { + chunks.push(s); + return true; + }, + }); + return { + sink, + document: (): unknown => + chunks.length ? JSON.parse(chunks.join("")) : undefined, + }; +} + +/** A connection whose prompt() stays pending until the test resolves it. */ +function makeConnection( + overrides: Partial = {}, +): TurnConnection & { + cancels: number; + settlePrompt: (stopReason: StopReason) => void; +} { + let settle: (result: { stopReason: StopReason }) => void = () => {}; + const conn = { + cancels: 0, + initialize: vi.fn().mockResolvedValue(undefined), + newSession: vi.fn().mockResolvedValue({ sessionId: "sess-1" }), + prompt: vi.fn( + () => + new Promise<{ stopReason: StopReason }>((resolve) => { + settle = resolve; + }), + ), + async cancel() { + conn.cancels += 1; + }, + settlePrompt: (stopReason: StopReason) => settle({ stopReason }), + ...overrides, + }; + return conn as TurnConnection & { + cancels: number; + settlePrompt: (stopReason: StopReason) => void; + }; +} + +function makeHooks() { + const exits: number[] = []; + const hooks = { + debugLog: vi.fn(), + markTearingDown: vi.fn(), + cleanup: vi.fn().mockResolvedValue(undefined), + exit: (code: number) => { + exits.push(code); + }, + }; + return { exits, hooks }; +} + +describe("runTurn", () => { + it("returns the mapped exit code and emits the document on a clean turn", async () => { + const conn = makeConnection(); + const { sink, document } = makeSink(); + const { hooks } = makeHooks(); + + const pending = runTurn(conn, makeOptions(), sink, hooks); + await vi.waitFor(() => expect(conn.prompt).toHaveBeenCalled()); + conn.settlePrompt("end_turn"); + + expect(await pending).toBe(0); + expect(document()).toEqual({ + text: "", + stopReason: "end_turn", + usage: null, + sessionId: "sess-1", + }); + expect(hooks.cleanup).toHaveBeenCalledTimes(1); + expect(hooks.markTearingDown).toHaveBeenCalled(); + }); + + it("passes the session _meta the flags asked for", async () => { + const conn = makeConnection(); + const { sink } = makeSink(); + const { hooks } = makeHooks(); + + const pending = runTurn( + conn, + makeOptions({ + permissionMode: "bypassPermissions", + model: "claude-sonnet-4-5", + systemPrompt: "be terse", + }), + sink, + hooks, + ); + await vi.waitFor(() => expect(conn.prompt).toHaveBeenCalled()); + conn.settlePrompt("end_turn"); + await pending; + + expect(conn.newSession).toHaveBeenCalledWith( + expect.objectContaining({ + cwd: "/repo", + _meta: { + permissionMode: "bypassPermissions", + model: "claude-sonnet-4-5", + systemPrompt: "be terse", + }, + }), + ); + }); + + it("omits model and systemPrompt from _meta when unset", async () => { + const conn = makeConnection(); + const { sink } = makeSink(); + const { hooks } = makeHooks(); + + const pending = runTurn(conn, makeOptions(), sink, hooks); + await vi.waitFor(() => expect(conn.prompt).toHaveBeenCalled()); + conn.settlePrompt("end_turn"); + await pending; + + expect(conn.newSession).toHaveBeenCalledWith( + expect.objectContaining({ _meta: { permissionMode: "auto" } }), + ); + }); + + it("removes both signal handlers on the way out", async () => { + const before = { + sigint: process.listenerCount("SIGINT"), + sigterm: process.listenerCount("SIGTERM"), + }; + const conn = makeConnection(); + const { sink } = makeSink(); + const { hooks } = makeHooks(); + + const pending = runTurn(conn, makeOptions(), sink, hooks); + await vi.waitFor(() => expect(conn.prompt).toHaveBeenCalled()); + expect(process.listenerCount("SIGTERM")).toBe(before.sigterm + 1); + conn.settlePrompt("end_turn"); + await pending; + + expect(process.listenerCount("SIGINT")).toBe(before.sigint); + expect(process.listenerCount("SIGTERM")).toBe(before.sigterm); + }); + + describe.each([ + { signal: "SIGINT" as const, expected: 130 }, + { signal: "SIGTERM" as const, expected: 143 }, + ])("$signal during a turn", ({ signal, expected }) => { + it(`cancels the session and returns ${expected}`, async () => { + const conn = makeConnection(); + const { sink } = makeSink(); + const { hooks } = makeHooks(); + + const pending = runTurn(conn, makeOptions(), sink, hooks); + await vi.waitFor(() => expect(conn.prompt).toHaveBeenCalled()); + + process.emit(signal); + await vi.waitFor(() => expect(conn.cancels).toBeGreaterThan(0)); + // The adapter resolves a cancelled turn rather than rejecting it. + conn.settlePrompt("cancelled"); + + expect(await pending).toBe(expected); + expect(hooks.cleanup).toHaveBeenCalledTimes(1); + }); + + it("reports the signal even when the turn completed anyway", async () => { + const conn = makeConnection(); + const { sink } = makeSink(); + const { hooks } = makeHooks(); + + const pending = runTurn(conn, makeOptions(), sink, hooks); + await vi.waitFor(() => expect(conn.prompt).toHaveBeenCalled()); + + process.emit(signal); + await vi.waitFor(() => expect(conn.cancels).toBeGreaterThan(0)); + conn.settlePrompt("end_turn"); + + expect(await pending).toBe(expected); + }); + }); + + // The adapter discards a cancel that arrives before the turn activates, so the + // guard after newSession is the only thing that stops the turn opening at all. + it("stops before opening a turn when a signal landed during session setup", async () => { + let releaseSession: (v: { sessionId: string }) => void = () => {}; + const conn = makeConnection({ + newSession: vi.fn( + () => + new Promise<{ sessionId: string }>((resolve) => { + releaseSession = resolve; + }), + ), + }); + const { sink } = makeSink(); + const { hooks, exits } = makeHooks(); + + const pending = runTurn(conn, makeOptions(), sink, hooks); + await vi.waitFor(() => expect(conn.newSession).toHaveBeenCalled()); + + process.emit("SIGINT"); + // No sessionId yet, so the handler tears down and exits directly. + await vi.waitFor(() => expect(exits).toEqual([130])); + expect(hooks.markTearingDown).toHaveBeenCalled(); + + releaseSession({ sessionId: "sess-1" }); + + expect(await pending).toBe(130); + expect(conn.prompt).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/cli/src/run.ts b/packages/cli/src/run.ts index c3f33d244a..b7bda77a91 100644 --- a/packages/cli/src/run.ts +++ b/packages/cli/src/run.ts @@ -5,95 +5,148 @@ import { ndJsonStream, type RequestPermissionRequest, type SessionNotification, + type StopReason, } from "@agentclientprotocol/sdk"; import { Agent } from "@posthog/agent/agent"; +import type { OnLogCallback } from "@posthog/agent/types"; +import { resolveUnattendedPermissionRequest } from "@posthog/agent/unattended-permission-policy"; import { withTimeout } from "@posthog/shared"; import type { CliOptions } from "./args"; -import { createOutputSink } from "./output"; -import { resolvePermissionRequest } from "./permission-policy"; +import { createOutputSink, type OutputSink } from "./output"; -const STOP_REASON_EXIT_CODES: Record = { +const STOP_REASON_EXIT_CODES: Partial> = { end_turn: 0, refusal: 2, max_tokens: 3, max_turn_requests: 3, }; -export type RunOptions = CliOptions & { prompt: string }; +/** 128 + signal number, the shell convention. */ +const EXIT_SIGINT = 130; +const EXIT_SIGTERM = 143; -export async function run(options: RunOptions): Promise { +/** Bounded: a wedged adapter cleanup must never hang the process. */ +const CLEANUP_TIMEOUT_MS = 8000; + +/** + * A turn cut short by a signal reports that signal's code whatever the adapter + * settled on; anything unmapped (including "cancelled" with no signal) is a + * failure the caller should notice. + */ +export function exitCodeFor( + stopReason: StopReason, + interruptedBy?: number, +): number { + return interruptedBy ?? STOP_REASON_EXIT_CODES[stopReason] ?? 1; +} + +/** + * stdout carries only assistant output, so every diagnostic goes to stderr. + * Errors during the turn always show (a failed run must say why); everything + * else needs --debug. Teardown errors are expected noise, the adapter reports + * the connection it is closing, so they drop to --debug once teardown starts. + */ +function createDiagnostics(debug: boolean) { const writeDiagnostic = (message: string) => process.stderr.write(`[posthog-code-cli] ${message}\n`); - const debugLog = options.debug ? writeDiagnostic : () => {}; - - // stdout must carry only assistant output. onLog bypasses the adapter's - // console logging entirely; errors during the turn always reach stderr (a - // failed run must say why), everything else only with --debug. Teardown - // errors are expected noise (the adapter reports the closing connection). - // No posthog config: gateway resolution is skipped and the agent subprocess - // inherits ANTHROPIC_* auth from process.env. + const debugLog = debug ? writeDiagnostic : () => {}; let tearingDown = false; - const agent = new Agent({ - debug: options.debug, - onLog: (level, scope, message, data) => { + + return { + debugLog, + markTearingDown: () => { + tearingDown = true; + }, + onLog: ((level, scope, message, data) => { const line = `${scope} [${level}] ${message}${data === undefined ? "" : ` ${format(data)}`}`; if (level === "error" && !tearingDown) { writeDiagnostic(line); } else { debugLog(line); } - }, - }); - const acp = await agent.run("cli", `cli-${randomUUID()}`, { - adapter: "claude", - }); + }) satisfies OnLogCallback, + }; +} - const sink = createOutputSink(options.output, process.stdout); +export type RunOptions = CliOptions & { prompt: string }; - const client = { - async sessionUpdate(notification: SessionNotification): Promise { - sink.onSessionUpdate(notification); - }, - async requestPermission(params: RequestPermissionRequest) { - const response = resolvePermissionRequest(params); - debugLog( - `permission "${params.toolCall.title ?? "unknown"}" -> ${JSON.stringify(response.outcome)}`, - ); - return response; - }, - // No readTextFile/writeTextFile and empty clientCapabilities below: the - // adapter runs file tools in-process instead of proxying through the host. - async extNotification(): Promise {}, - }; +/** + * The subset of ClientSideConnection the turn drives. Structural, so the real + * connection satisfies it without a cast and a test can supply a stub. + */ +export interface TurnConnection { + initialize(params: { + protocolVersion: number; + clientCapabilities: Record; + }): Promise; + newSession(params: { + cwd: string; + mcpServers: never[]; + _meta: Record; + }): Promise<{ sessionId: string }>; + prompt(params: { + sessionId: string; + prompt: { type: "text"; text: string }[]; + }): Promise<{ stopReason: StopReason; usage?: unknown }>; + cancel(params: { sessionId: string }): Promise; +} - const conn = new ClientSideConnection( - () => client, - ndJsonStream(acp.clientStreams.writable, acp.clientStreams.readable), - ); +export interface TurnHooks { + debugLog: (message: string) => void; + markTearingDown: () => void; + cleanup: () => Promise; + /** Injectable so a test can drive the no-session signal path. */ + exit?: (code: number) => void; +} - const cleanup = async () => { - // Bounded: a wedged adapter cleanup must never hang the process. - await withTimeout( - agent.cleanup().catch(() => undefined), - 8000, - ); - }; +/** + * Opens a session, runs one turn, and maps the outcome to an exit code. Owns the + * signal handlers for the turn's lifetime and removes them on the way out. + */ +export async function runTurn( + conn: TurnConnection, + options: RunOptions, + sink: OutputSink, + hooks: TurnHooks, +): Promise { + const { debugLog, markTearingDown, cleanup } = hooks; + const exit = hooks.exit ?? ((code: number) => process.exit(code)); let sessionId: string | undefined; - let interrupted = false; - const onSigint = () => { - interrupted = true; - if (sessionId) { - // Cancel resolves an in-flight prompt() with stopReason "cancelled"; - // the normal path then finishes output and exits 130. A second Ctrl-C - // (the handler is `once`) falls through to Node's default kill. - void conn.cancel({ sessionId }).catch(() => undefined); - } else { - // Nothing to cancel yet — exit directly. - void cleanup().then(() => process.exit(130)); + let interruptedBy: number | undefined; + let settled = false; + + // SIGTERM as well as SIGINT: a supervisor (CI step, cron, container runtime) + // terminates with SIGTERM, and Node's default disposition for it skips the + // finally below, leaving the agent's claude subprocess orphaned. + const onSignal = (exitCode: number) => () => { + interruptedBy = exitCode; + const id = sessionId; + if (!id) { + // Nothing to cancel yet, so exit directly. Flag teardown first, or the + // adapter's closing-connection errors print to stderr without --debug. + markTearingDown(); + void cleanup().then(() => exit(exitCode)); + return; } + // Cancel resolves an in-flight prompt() with stopReason "cancelled"; the + // normal path then finishes output and exits with this code. A second signal + // (the handlers are `once`) falls through to Node's default kill. + const sendCancel = () => + void conn.cancel({ sessionId: id }).catch(() => undefined); + sendCancel(); + // A cancel that lands before the turn activates is discarded, so re-send + // until the turn settles. See the guard after newSession for the rest. + const retry = setInterval(() => { + if (settled) clearInterval(retry); + else sendCancel(); + }, 500); + retry.unref(); }; + const onSigint = onSignal(EXIT_SIGINT); + const onSigterm = onSignal(EXIT_SIGTERM); process.once("SIGINT", onSigint); + process.once("SIGTERM", onSigterm); try { await conn.initialize({ protocolVersion: 1, clientCapabilities: {} }); @@ -109,26 +162,81 @@ export async function run(options: RunOptions): Promise { sessionId = session.sessionId; debugLog(`session ${sessionId} started in ${options.cwd}`); - // A cancel sent before the turn activates is reset by the adapter, so a - // SIGINT that landed during session setup must be honored here instead. - if (interrupted) return 130; + // A signal that landed during session setup has nothing to cancel yet and + // its handler already started teardown, so stop before opening a turn. + if (interruptedBy) return interruptedBy; - const result = await conn.prompt({ - sessionId, - prompt: [{ type: "text", text: options.prompt }], - }); - debugLog(`turn finished: ${result.stopReason}`); + try { + const result = await conn.prompt({ + sessionId, + prompt: [{ type: "text", text: options.prompt }], + }); + debugLog(`turn finished: ${result.stopReason}`); - sink.finish({ - stopReason: result.stopReason, - usage: result.usage, - sessionId, - }); - if (interrupted) return 130; - return STOP_REASON_EXIT_CODES[result.stopReason] ?? 1; + sink.finish({ + stopReason: result.stopReason, + usage: result.usage as Parameters[0]["usage"], + sessionId, + }); + return exitCodeFor(result.stopReason, interruptedBy); + } finally { + settled = true; + } } finally { - tearingDown = true; + markTearingDown(); process.removeListener("SIGINT", onSigint); + process.removeListener("SIGTERM", onSigterm); await cleanup(); } } + +export async function run(options: RunOptions): Promise { + const { debugLog, markTearingDown, onLog } = createDiagnostics(options.debug); + + // No posthog config: gateway resolution is skipped and the agent subprocess + // inherits ANTHROPIC_* auth from process.env. + const agent = new Agent({ + debug: options.debug, + // Safe to opt in here, unlike desktop and cloud: this sink is the operator's + // own stderr, and nothing persists or transmits it. + forwardAdapterLogs: true, + onLog, + }); + const acp = await agent.run("cli", `cli-${randomUUID()}`, { + adapter: "claude", + }); + + const sink = createOutputSink(options.output, process.stdout); + + const client = { + async sessionUpdate(notification: SessionNotification): Promise { + sink.onSessionUpdate(notification); + }, + async requestPermission(params: RequestPermissionRequest) { + const response = resolveUnattendedPermissionRequest(params); + debugLog( + `permission "${params.toolCall.title ?? "unknown"}" -> ${JSON.stringify(response.outcome)}`, + ); + return response; + }, + // No readTextFile/writeTextFile and empty clientCapabilities below: the + // adapter runs file tools in-process instead of proxying through the host. + async extNotification(): Promise {}, + }; + + const conn = new ClientSideConnection( + () => client, + ndJsonStream(acp.clientStreams.writable, acp.clientStreams.readable), + ); + + return runTurn(conn, options, sink, { + debugLog, + markTearingDown, + cleanup: async () => { + await withTimeout( + agent.cleanup().catch(() => undefined), + CLEANUP_TIMEOUT_MS, + ); + }, + }); +}