fix(miner): use claude/codex's real non-interactive CLI argv

packages/gittensory-engine/src/index.ts

@@ -277,7 +277,8 @@ export {
 } from "./miner/coding-agent-driver.js";
 export {
   createCliSubprocessCodingAgentDriver,
-  defaultCliSubprocessArgs,
+  defaultClaudeCliArgs,
+  defaultCodexCliArgs,
   type CliSubprocessDriverOptions,
   type CliSubprocessSpawnFn,
 } from "./miner/cli-subprocess-driver.js";

packages/gittensory-engine/src/miner/cli-subprocess-driver.ts

@@ -59,8 +59,8 @@ export type CliSubprocessDriverOptions = {
   /** Known secret values (e.g. an injected auth token) to strip from any surfaced output, on top of the well-known
    *  token-shape patterns. */
   knownSecrets?: readonly string[];
-  /** Build the CLI argv from a task. Default is a generic max-turns / acceptance-criteria / instructions argv that a
-   *  caller overrides to match the real CLI's flags. */
+  /** Build the CLI argv from a task. Defaults to `defaultClaudeCliArgs`/`defaultCodexCliArgs` based on
+   *  `command` -- a caller overrides this to point at a different real CLI's own flags. */
   buildArgs?: (task: CodingAgentDriverTask) => readonly string[];
 };
 
@@ -84,8 +84,88 @@ const CODING_AGENT_ENV_ALLOWLIST = [
   "no_proxy",
 ] as const;
 
-function defaultBuildArgs(task: CodingAgentDriverTask): string[] {
-  return defaultCliSubprocessArgs(task);
+/** Real, verified `claude -p` non-interactive argv (confirmed against `claude --help` and a live invocation,
+ *  #5135 follow-up -- the previous default argv here used `--max-turns`/`--acceptance-criteria`, neither of
+ *  which is a real `claude` CLI flag, and never passed `-p`/`--print` at all, meaning a spawned `claude`
+ *  process would start an INTERACTIVE session against a subprocess whose stdin is closed).
+ *  - `-p`/`--print` is REQUIRED for non-interactive use; without it `claude` starts an interactive session.
+ *  - `--output-format json` produces a single parseable JSON result on stdout, matching `claudeErrorStatus`'s
+ *    own `JSON.parse(stdout.trim())` assumption (which was already written for this shape, just never
+ *    actually triggered because this flag was never passed).
+ *  - `--permission-mode acceptEdits` is the same edit-permission scope the Agent-SDK driver already uses
+ *    (#4267) -- file edits run unattended inside the scoped worktree, nothing broader.
+ *  There is no turn-budget flag on the real CLI (verified via `claude --help`) and no acceptance-criteria
+ *  flag either -- the coding agent discovers `task.acceptanceCriteriaPath` itself via its own Read tool
+ *  inside the scoped working directory, exactly like the Agent-SDK driver already does (agent-sdk-driver.ts
+ *  never passes it as a distinct option either; only `task.instructions` is forwarded as the prompt there
+ *  too). The wall-clock `timeoutMs` (already implemented) is this provider's only real turn/cost ceiling. */
+export function defaultClaudeCliArgs(task: CodingAgentDriverTask): string[] {
+  return ["--print", "--output-format", "json", "--permission-mode", "acceptEdits", task.instructions];
+}
+
+/** Real, verified `codex exec` non-interactive argv (confirmed against `codex exec --help`, #5135
+ *  follow-up). `exec` is a REQUIRED subcommand -- without it `codex` starts an interactive session, the same
+ *  class of bug the missing `-p` had for claude. `--json` emits the JSONL event stream
+ *  `codexErrorFromStdout` already parses line-by-line. `--sandbox workspace-write` is codex's own equivalent
+ *  of claude's `acceptEdits` -- the model may write within the workspace, nothing broader. Same "no
+ *  turn-budget flag" and "no acceptance-criteria flag" gaps as claude apply here too. */
+export function defaultCodexCliArgs(task: CodingAgentDriverTask): string[] {
+  return ["exec", "--json", "--sandbox", "workspace-write", task.instructions];
+}
+
+/** Resolve the default argv builder for a known production command; throws for anything else so an
+ *  unrecognized `command` fails at driver-construction time (never silently invokes a binary with no known
+ *  real argv shape) unless the caller supplies an explicit `options.buildArgs` override. */
+function resolveDefaultBuildArgs(command: string): (task: CodingAgentDriverTask) => readonly string[] {
+  if (command === "claude") return defaultClaudeCliArgs;
+  if (command === "codex") return defaultCodexCliArgs;
+  throw new Error(`unsupported_cli_subprocess_command:${command}`);
+}
+
+/** Best-effort real dollar-cost extraction from a CLI's own stdout. Mirrors src/selfhost/ai.ts's
+ *  `extractCliUsage`/`COST_KEYS` (redeclared here, not imported, per this file's own no-src-import
+ *  convention) but narrowed to just the cost field this driver's `CodingAgentDriverResult` surfaces -- tokens
+ *  and model aren't part of that shape. Tries the whole trimmed stdout as one JSON object first (claude's
+ *  `--output-format json` shape, empirically confirmed to carry `total_cost_usd`, the exact same field name
+ *  the Agent-SDK's own result message carries), then scans line by line (codex's `--json` JSONL stream,
+ *  "still evolving" per src/selfhost/ai.ts's own comment, so multiple real key spellings are tolerated). A
+ *  missing/malformed field means "no cost signal", never an error -- never fabricated. */
+const COST_KEYS = ["total_cost_usd", "totalCostUsd", "cost_usd", "costUsd"] as const;
+
+function finiteNonNegativeNumber(value: unknown): number | undefined {
+  const n = typeof value === "number" ? value : typeof value === "string" && value.trim() ? Number(value) : NaN;
+  return Number.isFinite(n) && n >= 0 ? n : undefined;
+}
+
+function costUsdFromRecord(record: Record<string, unknown>): number | undefined {
+  let best: number | undefined;
+  for (const key of COST_KEYS) {
+    const n = finiteNonNegativeNumber(record[key]);
+    if (n !== undefined) best = Math.max(best ?? 0, n);
+  }
+  return best;
+}
+
+function extractCostUsd(stdout: string): number | undefined {
+  const trimmed = stdout.trim();
+  if (!trimmed) return undefined;
+  let best: number | undefined;
+  const tryLine = (text: string): void => {
+    try {
+      const parsed = JSON.parse(text) as unknown;
+      if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) {
+        const n = costUsdFromRecord(parsed as Record<string, unknown>);
+        if (n !== undefined) best = Math.max(best ?? 0, n);
+      }
+    } catch {
+      /* not JSON -- best-effort only */
+    }
+  };
+  tryLine(trimmed);
+  for (const line of trimmed.split(/\r?\n/)) {
+    if (line.trim()) tryLine(line);
+  }
+  return best;
 }
 
 /** Claude Code's `--output-format json` sometimes exits non-zero while still emitting a structured
@@ -131,26 +211,14 @@ function codexErrorFromStdout(stdout: string): string | null {
   return null;
 }
 
-/** The default argv contract, exported so the factory (#4289) can PREFIX provider config (e.g. a configured
- *  model flag) without re-inventing — and silently drifting from — this baseline argv shape. */
-export function defaultCliSubprocessArgs(task: CodingAgentDriverTask): string[] {
-  return [
-    "--max-turns",
-    String(task.maxTurns),
-    "--acceptance-criteria",
-    task.acceptanceCriteriaPath,
-    task.instructions,
-  ];
-}
-
 /**
  * Create a {@link CodingAgentDriver} that runs the coding agent as a CLI subprocess. A non-zero or absent exit
  * code, or a timeout, yields `ok: false` with a redacted error; exit `0` yields `ok: true`. Any subprocess output
  * kept as a transcript or folded into an error is redacted first.
  */
 export function createCliSubprocessCodingAgentDriver(options: CliSubprocessDriverOptions): CodingAgentDriver {
   const timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS;
-  const buildArgs = options.buildArgs ?? defaultBuildArgs;
+  const buildArgs = options.buildArgs ?? resolveDefaultBuildArgs(options.command);
   const knownSecrets = options.knownSecrets ?? [];
   return {
     async run(task: CodingAgentDriverTask): Promise<CodingAgentDriverResult> {
@@ -241,11 +309,32 @@ export function createCliSubprocessCodingAgentDriver(options: CliSubprocessDrive
           error: `${options.command}_exit_${spawned.code}: ${detail}`,
         };
       }
+      // Claude Code's own documented behavior: `--output-format json` "sometimes exits non-zero", implying
+      // it can ALSO exit 0 while still reporting `is_error: true` in its structured JSON envelope (confirmed
+      // empirically: a live `claude -p --output-format json` invocation returns a real `is_error`/`subtype`
+      // field on every result, success or not). claudeErrorStatus was already written to detect exactly this
+      // shape but, before this fix, was only ever checked on the `code !== 0` branch above -- a code-0 error
+      // envelope silently read as `ok: true`. Checked here, not folded into that branch, since it's a
+      // genuinely different condition (exit code 0).
+      if (options.command === "claude") {
+        const errStatus = claudeErrorStatus(spawned.stdout);
+        if (errStatus) {
+          return {
+            ok: false,
+            changedFiles: [],
+            summary: `${options.command} exited 0 but reported an error envelope`,
+            transcript,
+            error: redactSecrets(`claude_code_error_${errStatus}`, knownSecrets),
+          };
+        }
+      }
+      const costUsd = extractCostUsd(spawned.stdout);
       return {
         ok: true,
         changedFiles: [],
         summary: `${options.command} completed for ${task.attemptId}`,
         transcript,
+        ...(costUsd !== undefined ? { costUsd } : {}),
       };
     },
   };

packages/gittensory-engine/src/miner/driver-factory.ts

@@ -20,7 +20,8 @@ import type { CodingAgentDriverResult, CodingAgentDriverTask } from "./coding-ag
 import { guardCodingAgentDriverResult, type LintGuardOptions, type LintGuardResult } from "./lint-guard.js";
 import {
   createCliSubprocessCodingAgentDriver,
-  defaultCliSubprocessArgs,
+  defaultClaudeCliArgs,
+  defaultCodexCliArgs,
   type CliSubprocessSpawnFn,
 } from "./cli-subprocess-driver.js";
 import {
@@ -119,12 +120,24 @@ export type CreateCodingAgentDriverOptions = {
   knownSecrets?: readonly string[] | undefined;
 };
 
-/** Build a CLI provider's argv: the driver's own default argv contract, prefixed with the CONFIGURED model
- *  flag when the provider's `CODING_AGENT_DRIVER_CONFIG_ENV` model key is set — this is where that declared
- *  config is actually consumed. */
-function buildCliArgsWithConfiguredModel(model: string | undefined): ((task: CodingAgentDriverTask) => readonly string[]) | undefined {
+/** Build a CLI provider's argv: the driver's own real default argv contract, with the CONFIGURED model
+ *  flag inserted at the RIGHT position for that command — this is where that declared config is actually
+ *  consumed. claude's `--model` is a top-level flag (prefixed before everything else is fine); codex's
+ *  `-m`/`--model` is scoped to the `exec` subcommand (per `codex exec --help`) and must be inserted AFTER
+ *  the leading `"exec"` token, not before it — a single shared prefix-everything scheme would silently
+ *  misparse for codex. */
+function buildCliArgsWithConfiguredModel(
+  command: "claude" | "codex",
+  model: string | undefined,
+): ((task: CodingAgentDriverTask) => readonly string[]) | undefined {
   if (model === undefined) return undefined;
-  return (task) => ["--model", model, ...defaultCliSubprocessArgs(task)];
+  if (command === "claude") {
+    return (task) => ["--model", model, ...defaultClaudeCliArgs(task)];
+  }
+  return (task) => {
+    const [subcommand, ...rest] = defaultCodexCliArgs(task);
+    return [subcommand!, "--model", model, ...rest];
+  };
 }
 
 function createCliProvider(
@@ -146,7 +159,7 @@ function createCliProvider(
   }
   const model = firstConfiguredEnvValue(env[modelEnvKey]);
   const timeoutMs = configuredTimeoutMs(env);
-  const buildArgs = buildCliArgsWithConfiguredModel(model);
+  const buildArgs = buildCliArgsWithConfiguredModel(command, model);
   return createCliSubprocessCodingAgentDriver({
     command,
     spawn: options.spawn,

test/unit/cli-subprocess-driver.test.ts

@@ -36,14 +36,39 @@ describe("createCliSubprocessCodingAgentDriver (#4266)", () => {
     expect(calls[0]?.opts.cwd).toBe("/work/attempt-1");
     expect(calls[0]?.opts.timeoutMs).toBe(120_000);
     expect(calls[0]?.args).toEqual([
-      "--max-turns",
-      "6",
-      "--acceptance-criteria",
-      "/work/attempt-1/acceptance-criteria.json",
+      "--print",
+      "--output-format",
+      "json",
+      "--permission-mode",
+      "acceptEdits",
       "Fix the pagination bug.",
     ]);
   });
 
+  it("uses codex's own real default argv (the exec subcommand, not claude's flags)", async () => {
+    const { spawn, calls } = fakeSpawn({ stdout: "done", code: 0 });
+    const driver = createCliSubprocessCodingAgentDriver({ command: "codex", spawn });
+    const result = await driver.run(TASK);
+    expect(result.ok).toBe(true);
+    expect(calls[0]?.cmd).toBe("codex");
+    expect(calls[0]?.args).toEqual(["exec", "--json", "--sandbox", "workspace-write", "Fix the pagination bug."]);
+  });
+
+  it("fails closed at construction time for an unrecognized command with no explicit buildArgs override", () => {
+    const { spawn } = fakeSpawn({ stdout: "", code: 0 });
+    expect(() => createCliSubprocessCodingAgentDriver({ command: "some-other-cli", spawn })).toThrow(
+      "unsupported_cli_subprocess_command:some-other-cli",
+    );
+  });
+
+  it("an explicit buildArgs override still works for an unrecognized command (the fail-closed default is bypassable on purpose)", async () => {
+    const { spawn, calls } = fakeSpawn({ stdout: "done", code: 0 });
+    const driver = createCliSubprocessCodingAgentDriver({ command: "some-other-cli", spawn, buildArgs: () => ["--custom"] });
+    const result = await driver.run(TASK);
+    expect(result.ok).toBe(true);
+    expect(calls[0]?.args).toEqual(["--custom"]);
+  });
+
   it("returns a redacted error on a non-zero exit code", async () => {
     const { spawn } = fakeSpawn({
       stdout: "",
@@ -332,4 +357,120 @@ describe("createCliSubprocessCodingAgentDriver (#4266)", () => {
       expect(result.error).toContain("[redacted]");
     });
   });
+
+  describe("REGRESSION: claude exits 0 while still reporting an error envelope (#5135 follow-up)", () => {
+    it("reports ok:false when --output-format json's envelope carries is_error:true even on exit code 0", async () => {
+      const { spawn } = fakeSpawn({
+        stdout: JSON.stringify({ type: "result", subtype: "success", is_error: true, api_error_status: "invalid_api_key", total_cost_usd: 0 }),
+        code: 0,
+      });
+      const driver = createCliSubprocessCodingAgentDriver({ command: "claude", spawn });
+      const result = await driver.run(TASK);
+      expect(result.ok).toBe(false);
+      expect(result.error).toBe("claude_code_error_invalid_api_key");
+    });
+
+    it("still reports ok:true on a genuine exit-0 success envelope with is_error:false", async () => {
+      const { spawn } = fakeSpawn({
+        stdout: JSON.stringify({ type: "result", subtype: "success", is_error: false, result: "done", total_cost_usd: 0.02 }),
+        code: 0,
+      });
+      const driver = createCliSubprocessCodingAgentDriver({ command: "claude", spawn });
+      const result = await driver.run(TASK);
+      expect(result.ok).toBe(true);
+    });
+
+    it("never inspects the envelope for a non-claude command on exit 0 either", async () => {
+      const { spawn } = fakeSpawn({
+        stdout: JSON.stringify({ is_error: true, api_error_status: "invalid_api_key" }),
+        code: 0,
+      });
+      const driver = createCliSubprocessCodingAgentDriver({ command: "codex", spawn });
+      const result = await driver.run(TASK);
+      expect(result.ok).toBe(true);
+    });
+  });
+
+  describe("REGRESSION: real dollar-cost extraction (#5135 follow-up)", () => {
+    it("extracts claude's real total_cost_usd from its single JSON result on success", async () => {
+      const { spawn } = fakeSpawn({
+        stdout: JSON.stringify({ type: "result", subtype: "success", is_error: false, result: "done", total_cost_usd: 0.1234 }),
+        code: 0,
+      });
+      const driver = createCliSubprocessCodingAgentDriver({ command: "claude", spawn });
+      const result = await driver.run(TASK);
+      expect(result.costUsd).toBe(0.1234);
+    });
+
+    it("extracts codex's real cost from its JSONL event stream, tolerating either key spelling", async () => {
+      const { spawn } = fakeSpawn({
+        stdout: '{"type":"start"}\n{"type":"turn","total_cost_usd":0.05}\n{"type":"end"}',
+        code: 0,
+      });
+      const driver = createCliSubprocessCodingAgentDriver({ command: "codex", spawn });
+      const result = await driver.run(TASK);
+      expect(result.costUsd).toBe(0.05);
+    });
+
+    it("stays undefined (never fabricated) when stdout carries no cost field at all", async () => {
+      const { spawn } = fakeSpawn({ stdout: "plain text output, no JSON", code: 0 });
+      const driver = createCliSubprocessCodingAgentDriver({ command: "claude", spawn });
+      const result = await driver.run(TASK);
+      expect(result.ok).toBe(true);
+      expect(result.costUsd).toBeUndefined();
+    });
+
+    it("takes the largest cost value seen across a multi-event codex stream (cumulative, matches src/selfhost/ai.ts's own convention)", async () => {
+      const { spawn } = fakeSpawn({
+        stdout: '{"total_cost_usd":0.01}\n{"total_cost_usd":0.07}\n{"total_cost_usd":0.03}',
+        code: 0,
+      });
+      const driver = createCliSubprocessCodingAgentDriver({ command: "codex", spawn });
+      const result = await driver.run(TASK);
+      expect(result.costUsd).toBe(0.07);
+    });
+
+    it("ignores a non-numeric cost field value instead of throwing or fabricating a number", async () => {
+      const { spawn } = fakeSpawn({
+        stdout: JSON.stringify({ total_cost_usd: "not-a-number" }),
+        code: 0,
+      });
+      const driver = createCliSubprocessCodingAgentDriver({ command: "claude", spawn });
+      const result = await driver.run(TASK);
+      expect(result.ok).toBe(true);
+      expect(result.costUsd).toBeUndefined();
+    });
+
+    it("ignores a negative cost value (a real number, but not a valid cost) instead of surfacing it", async () => {
+      const { spawn } = fakeSpawn({
+        stdout: JSON.stringify({ total_cost_usd: -1 }),
+        code: 0,
+      });
+      const driver = createCliSubprocessCodingAgentDriver({ command: "claude", spawn });
+      const result = await driver.run(TASK);
+      expect(result.costUsd).toBeUndefined();
+    });
+
+    it("ignores a JSONL line that parses to a non-object (array or primitive) instead of throwing", async () => {
+      const { spawn } = fakeSpawn({
+        stdout: '[1,2,3]\n"just a string"\n{"total_cost_usd":0.09}',
+        code: 0,
+      });
+      const driver = createCliSubprocessCodingAgentDriver({ command: "codex", spawn });
+      const result = await driver.run(TASK);
+      expect(result.ok).toBe(true);
+      expect(result.costUsd).toBe(0.09);
+    });
+
+    it("skips a blank interior line in the JSONL stream rather than treating it as malformed JSON", async () => {
+      const { spawn } = fakeSpawn({
+        stdout: '{"total_cost_usd":0.06}\n\n{"type":"end"}',
+        code: 0,
+      });
+      const driver = createCliSubprocessCodingAgentDriver({ command: "codex", spawn });
+      const result = await driver.run(TASK);
+      expect(result.ok).toBe(true);
+      expect(result.costUsd).toBe(0.06);
+    });
+  });
 });