-
Notifications
You must be signed in to change notification settings - Fork 64
Add headless CLI for one-shot agent runs #3798
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
haacked
wants to merge
3
commits into
main
Choose a base branch
from
posthog-code/headless-cli-mode
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 }); | ||
| } | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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" | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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)); | ||
| }); | ||
| }); |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Injecting the caller's logger reroutes ClaudeAcpAgent's internal debug logging into Desktop's persisted log sink and Cloud's SSE/log-writer stream, contradicting the PR's "no behavior change" claim
Why we think it's a valid issue
logger.ts:38-42(emitLogskips the level/debugEnabledgate wheneveronLogis set) and72-79(child()copiesonLog); the PR diff (git show 988523a0) confirming acp-connection.ts:122 and claude-agent.ts:279-281 are both new; the Desktop caller chain (agent.ts:29-33 builds the logger withonLog:config.onLog, agent.ts:133 forwards it, workspace-server agent.ts:842-850 always setsonLog:this.onAgentLog, makeOnAgentLog:235-244 forwards unconditionally); the Cloud chain (agent-server.ts:1615 passesthis.logger, 1822-1828 reassigns it withonLog:emitConsoleLogafter the createAcpConnection call, cleanupSession:4452-4521 never resetsthis.logger, emitConsoleLog:455-482 broadcasts via SSE +logWriter.appendRawLinewith no level filter).onLog, and ClaudeAcpAgent's internal logs (previously console-only, discarded) now route to those sinks. Cloud is confirmed: on any session re-init on the same instance, the child logger inheritsemitConsoleLog, so all-level chatter (including debug) is rebroadcast over SSE and persisted.should_fix: (1) the Desktop file transport level isisDev ? "debug" : "info"(apps/code/src/main/utils/logger.ts:63-64), so in production the 8debugsites — including the per-loop "Idle without an active turn" at claude-agent.ts:991 the finding relies on — are filtered; only ~4info+ warn/error persist at per-session frequency into a rotated 10MB×3 log, making "regardless of isDevBuild()" and "every consumer-loop iteration" overstated. (2) The AcpConnection-level child logger already routed info logs to these same sinks pre-PR, so the change is incremental. Cloud SSE/log noise is real but bounded and only on the resume path.Issue description
ClaudeAcpAgentnow usesoptions?.logger ?? new Logger({ debug: true, prefix: "[ClaudeAcpAgent]" }), andcreateClaudeConnectionin acp-connection.ts (line 122) passesconfig.logger?.child("ClaudeAcpAgent")whenever the caller supplied a logger. The PR description asserts "Desktop and cloud behavior is unchanged since they don't pass one" — but both existing hosts DO pass a logger:Agent.run()(packages/agent/src/agent.ts:133) forwardslogger: this.logger, andAgentServer._doInitializeSession(packages/agent/src/server/agent-server.ts:1614) forwardslogger: this.loggertoo.Logger.emitLog(utils/logger.ts) forwards unconditionally toonLogwhen one is set, bypassing thedebugEnabledcheck entirely, andLogger.child()copies the parent'sonLog. Concretely: on Desktop,Agentis always constructed withonLog: this.onAgentLog(packages/workspace-server/src/services/agent/agent.ts:850), which forwards every call straight into RootLogger — so every one of ClaudeAcpAgent's ~30 internal debug/info call sites (fired on essentially every consumer-loop iteration: idle detection, compaction, MCP reconnects, turn activation, etc.) now gets persisted to the on-disk desktop log file on every single task run, regardless ofisDevBuild(), where previously they only reachedconsole.*and were discarded. On the cloud sandbox,AgentServer's ownthis.loggeronly gains anonLogofemitConsoleLogpartway through_doInitializeSession(agent-server.ts:1822-1828, AFTER thecreateAcpConnectioncall at line 1609) — but that mutated field is never reset incleanupSession(), so on any session re-init on the same instance (the code explicitly supports "warm reconnects... sandbox restart with snapshot resume"), the nextcreateAcpConnectioncall reuses the already-mutated logger, and ClaudeAcpAgent's full internal debug chatter then gets broadcast as_posthog/consoleSSE notifications to the client AND persisted viasession.logWriter.appendRawLinefor every session after the first on that instance.Suggested fix
Verify and, if unintended, avoid coupling ClaudeAcpAgent's own internal logger to the caller-supplied
onLogsink. Either keep ClaudeAcpAgent's console-only isolation by default and have the CLI opt in with its own dedicated logger construction (not by threading the host's general-purpose logger throughAcpConnectionConfig.logger), or explicitly measure/bound the log volume impact on Desktop (disk usage per run) and Cloud (SSE bandwidth + logWriter storage per session-resume) before merging, since neither is covered by the tests described in the PR ("39 unit tests... Full @posthog/agent suite" — none of which exercise Desktop'sonAgentLogwiring or Cloud's session-resumecreateAcpConnectionreuse path).Prompt to fix with AI (copy-paste)