From 112f470a9a6c4c1bb3a29af46948913ae4adb210 Mon Sep 17 00:00:00 2001 From: reyanthony062001-ops Date: Fri, 10 Jul 2026 06:53:26 +0000 Subject: [PATCH] feat(miner-hands): resolve concrete CLI/SDK providers in the CodingAgentDriver factory --- packages/gittensory-engine/src/index.ts | 2 + .../src/miner/cli-subprocess-driver.ts | 6 +- .../src/miner/driver-factory.ts | 116 ++++++++++++- .../test/driver-factory.test.ts | 48 ++++++ test/unit/coding-agent-miner.test.ts | 153 +++++++++++++++++- 5 files changed, 317 insertions(+), 8 deletions(-) diff --git a/packages/gittensory-engine/src/index.ts b/packages/gittensory-engine/src/index.ts index 7b525c74c8..2f180f6bd9 100644 --- a/packages/gittensory-engine/src/index.ts +++ b/packages/gittensory-engine/src/index.ts @@ -194,6 +194,7 @@ export { } from "./miner/coding-agent-driver.js"; export { createCliSubprocessCodingAgentDriver, + defaultCliSubprocessArgs, type CliSubprocessDriverOptions, type CliSubprocessSpawnFn, } from "./miner/cli-subprocess-driver.js"; @@ -232,6 +233,7 @@ export { createFakeCodingAgentDriverForFactory, isConfiguredCodingAgentDriver, resolveConfiguredCodingAgentDriverNames, + resolveFirstConfiguredCodingAgentDriverName, runCodingAgentAttempt, type CodingAgentDriverName, type CreateCodingAgentDriverOptions, diff --git a/packages/gittensory-engine/src/miner/cli-subprocess-driver.ts b/packages/gittensory-engine/src/miner/cli-subprocess-driver.ts index 07660b9cc8..60828998c1 100644 --- a/packages/gittensory-engine/src/miner/cli-subprocess-driver.ts +++ b/packages/gittensory-engine/src/miner/cli-subprocess-driver.ts @@ -50,7 +50,9 @@ const DEFAULT_TIMEOUT_MS = 120_000; const MAX_TRANSCRIPT_CHARS = 8000; const MAX_ERROR_DETAIL_CHARS = 500; -function defaultBuildArgs(task: CodingAgentDriverTask): string[] { +/** 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), @@ -67,7 +69,7 @@ function defaultBuildArgs(task: CodingAgentDriverTask): string[] { */ export function createCliSubprocessCodingAgentDriver(options: CliSubprocessDriverOptions): CodingAgentDriver { const timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS; - const buildArgs = options.buildArgs ?? defaultBuildArgs; + const buildArgs = options.buildArgs ?? defaultCliSubprocessArgs; const knownSecrets = options.knownSecrets ?? []; return { async run(task: CodingAgentDriverTask): Promise { diff --git a/packages/gittensory-engine/src/miner/driver-factory.ts b/packages/gittensory-engine/src/miner/driver-factory.ts index 352b471e6b..338e706af1 100644 --- a/packages/gittensory-engine/src/miner/driver-factory.ts +++ b/packages/gittensory-engine/src/miner/driver-factory.ts @@ -17,18 +17,51 @@ import { } from "./coding-agent-mode.js"; import type { CodingAgentDriverResult, CodingAgentDriverTask } from "./coding-agent-driver.js"; import { guardCodingAgentDriverResult, type LintGuardOptions, type LintGuardResult } from "./lint-guard.js"; +import { + createCliSubprocessCodingAgentDriver, + defaultCliSubprocessArgs, + type CliSubprocessSpawnFn, +} from "./cli-subprocess-driver.js"; +import { + createAgentSdkCodingAgentDriver, + type AgentSdkHooks, + type AgentSdkQueryFn, +} from "./agent-sdk-driver.js"; -/** Provider names the factory knows how to resolve today. Concrete CLI/SDK drivers land in #4266/#4267. */ -export const CODING_AGENT_DRIVER_NAMES = Object.freeze(["noop"] as const); +/** Provider names the factory resolves: the two concrete drivers from #4266/#4267 (`claude-cli`/`codex-cli` + * spawn the respective CLI; `agent-sdk` runs in-process via the Agent SDK) plus the `noop` stub. All are + * locally-authenticated (no API-key env requirement), mirroring how `isConfiguredSelfHostProvider` treats + * `claude-code`/`codex` as always-configured. */ +export const CODING_AGENT_DRIVER_NAMES = Object.freeze(["noop", "claude-cli", "codex-cli", "agent-sdk"] as const); export type CodingAgentDriverName = (typeof CODING_AGENT_DRIVER_NAMES)[number]; -/** Per-provider env keys for coding-agent configuration (mirrors `SELF_HOST_REVIEWER_MODEL_ENV`). */ -export const CODING_AGENT_DRIVER_CONFIG_ENV: Readonly> = +/** Per-provider env keys for coding-agent configuration (mirrors `SELF_HOST_REVIEWER_MODEL_ENV`). Every key + * declared here is CONSUMED by `createCodingAgentDriver` below — a declared-but-unread entry is dead, + * misleading config-as-code surface. Deliberately NOT declared: a max-turns key (the turn budget is task-level + * input — `CodingAgentDriverTask.maxTurns` — set by the orchestrator per attempt, not per-provider config) and + * an agent-sdk model key (the SDK session uses the account/CLI default; it exposes no model option on the + * driver today). */ +export const CODING_AGENT_DRIVER_CONFIG_ENV: Readonly> = Object.freeze({ noop: {}, + "claude-cli": { model: "MINER_CODING_AGENT_CLAUDE_MODEL", timeoutMs: "MINER_CODING_AGENT_TIMEOUT_MS" }, + "codex-cli": { model: "MINER_CODING_AGENT_CODEX_MODEL", timeoutMs: "MINER_CODING_AGENT_TIMEOUT_MS" }, + "agent-sdk": {}, }); +/** `firstConfigured` (src/selfhost/ai.ts:117-134) pattern: a set-and-non-empty env value, else undefined. */ +function firstConfiguredEnvValue(value: string | undefined): string | undefined { + const trimmed = value?.trim(); + return trimmed ? trimmed : undefined; +} + +/** Positive-integer env parse for the CLI wall-clock ceiling; anything else defers to the driver default. */ +function configuredTimeoutMs(env: Record): number | undefined { + const raw = Number(firstConfiguredEnvValue(env.MINER_CODING_AGENT_TIMEOUT_MS)); + return Number.isFinite(raw) && Number.isInteger(raw) && raw > 0 ? raw : undefined; +} + function parseDriverNames(env: Record): string[] { return (env.MINER_CODING_AGENT_PROVIDER ?? "") .split(",") @@ -43,6 +76,9 @@ export function isConfiguredCodingAgentDriver( ): boolean { switch (name) { case "noop": + case "claude-cli": + case "codex-cli": + case "agent-sdk": return true; default: return false; @@ -55,13 +91,65 @@ export function resolveConfiguredCodingAgentDriverNames( return parseDriverNames(env).filter((name) => isConfiguredCodingAgentDriver(name, env)); } +/** Primary-then-fallback resolution over `MINER_CODING_AGENT_PROVIDER`'s comma-separated list (the same + * fallback-chain semantic `AiRunOptions.fallback` gives reviewers): the FIRST configured name wins; unknown + * names are skipped (deny-by-default), and an all-unknown/empty list resolves to undefined so the caller + * fails closed rather than falling through to some implicit default driver. */ +export function resolveFirstConfiguredCodingAgentDriverName( + env: Record, +): string | undefined { + return resolveConfiguredCodingAgentDriverNames(env)[0]; +} + export type CreateCodingAgentDriverOptions = { providerName: string; env?: Record | undefined; /** Test seam — inject a fake driver instead of constructing the named provider. */ driver?: CodingAgentDriver | undefined; + /** Subprocess runner for the CLI providers (`claude-cli`/`codex-cli`). REQUIRED for those providers — the + * engine package ships no default spawn, so constructing a CLI driver without one fails closed rather than + * producing a driver that can never run. */ + spawn?: CliSubprocessSpawnFn | undefined; + /** Optional injected `query()` loop for the `agent-sdk` provider (defaults to the real SDK import). */ + query?: AgentSdkQueryFn | undefined; + /** Forwarded to the `agent-sdk` provider's session (#2343's PreToolUse interception point). */ + hooks?: AgentSdkHooks | undefined; + /** Known secret values the CLI providers strip from surfaced output, on top of the token-shape patterns. */ + 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 { + if (model === undefined) return undefined; + return (task) => ["--model", model, ...defaultCliSubprocessArgs(task)]; +} + +function createCliProvider( + command: "claude" | "codex", + modelEnvKey: string, + options: CreateCodingAgentDriverOptions, + env: Record, +): CodingAgentDriver { + if (!options.spawn) { + // Fail-closed (resolveAutonomy's deny-by-default precedent): a CLI provider without a spawn dependency is + // unconfigured in the way that matters — never hand back a driver whose every run() would throw. + throw new Error(`unconfigured_coding_agent_driver_missing_spawn:${command}-cli`); + } + const model = firstConfiguredEnvValue(env[modelEnvKey]); + const timeoutMs = configuredTimeoutMs(env); + const buildArgs = buildCliArgsWithConfiguredModel(model); + return createCliSubprocessCodingAgentDriver({ + command, + spawn: options.spawn, + parentEnv: env, + ...(timeoutMs !== undefined ? { timeoutMs } : {}), + ...(buildArgs !== undefined ? { buildArgs } : {}), + ...(options.knownSecrets !== undefined ? { knownSecrets: options.knownSecrets } : {}), + }); +} + /** Resolve a concrete driver for `providerName`. Throws on unknown/unconfigured providers (fail-closed). */ export function createCodingAgentDriver(options: CreateCodingAgentDriverOptions): CodingAgentDriver { if (options.driver) return options.driver; @@ -73,7 +161,16 @@ export function createCodingAgentDriver(options: CreateCodingAgentDriverOptions) switch (name) { case "noop": return createNoopCodingAgentDriver(); - /* v8 ignore next -- isConfiguredCodingAgentDriver already rejects unknown names before this switch. */ + case "claude-cli": + return createCliProvider("claude", "MINER_CODING_AGENT_CLAUDE_MODEL", options, env); + case "codex-cli": + return createCliProvider("codex", "MINER_CODING_AGENT_CODEX_MODEL", options, env); + case "agent-sdk": + return createAgentSdkCodingAgentDriver({ + ...(options.query !== undefined ? { query: options.query } : {}), + ...(options.hooks !== undefined ? { hooks: options.hooks } : {}), + }); + /* v8 ignore next 2 -- isConfiguredCodingAgentDriver already rejects unknown names before this switch. */ default: throw new Error(`unconfigured_coding_agent_driver:${name}`); } @@ -87,6 +184,11 @@ export type RunCodingAgentAttemptOptions = { task: CodingAgentDriverTask; log?: AttemptLogSink | undefined; driver?: CodingAgentDriver | undefined; + /** Provider dependencies, forwarded to `createCodingAgentDriver` (see `CreateCodingAgentDriverOptions`). */ + spawn?: CliSubprocessSpawnFn | undefined; + query?: AgentSdkQueryFn | undefined; + hooks?: AgentSdkHooks | undefined; + knownSecrets?: readonly string[] | undefined; /** When supplied, the driver result is run through the lint guard (#4276) before being returned, so a * live coding-agent edit that fails its own package's typecheck/node --check never reads as `ok: true`. */ lintGuard?: LintGuardOptions | undefined; @@ -109,6 +211,10 @@ export async function runCodingAgentAttempt( providerName: options.providerName, env: options.env, driver: options.driver, + spawn: options.spawn, + query: options.query, + hooks: options.hooks, + knownSecrets: options.knownSecrets, }); const result = await invokeCodingAgentDriver(driver, mode, options.task, options.log); if (!options.lintGuard) return { mode, result }; diff --git a/packages/gittensory-engine/test/driver-factory.test.ts b/packages/gittensory-engine/test/driver-factory.test.ts index 09accf277e..caec5e161e 100644 --- a/packages/gittensory-engine/test/driver-factory.test.ts +++ b/packages/gittensory-engine/test/driver-factory.test.ts @@ -6,6 +6,7 @@ import { createCodingAgentDriver, isConfiguredCodingAgentDriver, resolveConfiguredCodingAgentDriverNames, + resolveFirstConfiguredCodingAgentDriverName, runCodingAgentAttempt, type CodingAgentDriverTask, } from "../dist/index.js"; @@ -58,3 +59,50 @@ test("runCodingAgentAttempt wires mode + driver + attempt log end-to-end", async assert.equal(live.mode, "live"); assert.equal(fake.lastTask, task); }); + +// ── #4289: concrete provider resolution (mirrors the root vitest suite's key cases) ──────────────────────── + +test("all concrete provider names are configured; unknown stays denied (#4289)", () => { + for (const name of ["claude-cli", "codex-cli", "agent-sdk"]) { + assert.equal(isConfiguredCodingAgentDriver(name, {}), true); + } + assert.equal(isConfiguredCodingAgentDriver("mystery", {}), false); +}); + +test("claude-cli consumes its declared model env key into the argv (#4289)", async () => { + const calls: Array<{ cmd: string; args: readonly string[] }> = []; + const driver = createCodingAgentDriver({ + providerName: "claude-cli", + env: { MINER_CODING_AGENT_CLAUDE_MODEL: "claude-sonnet-5" }, + spawn: async (cmd, args) => { + calls.push({ cmd, args }); + return { stdout: "done", code: 0 }; + }, + }); + const task = { + attemptId: "a1", + workingDirectory: "/tmp/w", + acceptanceCriteriaPath: "/tmp/w/AC.md", + instructions: "fix", + maxTurns: 2, + }; + const result = await driver.run(task); + assert.equal(result.ok, true); + assert.equal(calls[0]!.cmd, "claude"); + assert.deepEqual([...calls[0]!.args].slice(0, 2), ["--model", "claude-sonnet-5"]); +}); + +test("a CLI provider without a spawn dependency fails closed (#4289)", () => { + assert.throws( + () => createCodingAgentDriver({ providerName: "codex-cli" }), + /unconfigured_coding_agent_driver_missing_spawn:codex-cli/, + ); +}); + +test("resolveFirstConfiguredCodingAgentDriverName is primary-then-fallback over the provider list (#4289)", () => { + assert.equal( + resolveFirstConfiguredCodingAgentDriverName({ MINER_CODING_AGENT_PROVIDER: "mystery, agent-sdk" }), + "agent-sdk", + ); + assert.equal(resolveFirstConfiguredCodingAgentDriverName({}), undefined); +}); diff --git a/test/unit/coding-agent-miner.test.ts b/test/unit/coding-agent-miner.test.ts index e0a0a8f5b6..43b5523c42 100644 --- a/test/unit/coding-agent-miner.test.ts +++ b/test/unit/coding-agent-miner.test.ts @@ -20,6 +20,7 @@ import { resolveCodingAgentExecutionMode, resolveCodingAgentModeFromConfig, resolveConfiguredCodingAgentDriverNames, + resolveFirstConfiguredCodingAgentDriverName, runCodingAgentAttempt, type CodingAgentDriverResult, type CodingAgentDriverTask, @@ -298,8 +299,10 @@ describe("invokeCodingAgentDriver (#4313)", () => { describe("coding-agent driver factory (#4289)", () => { it("exposes the noop provider registry", () => { - expect([...CODING_AGENT_DRIVER_NAMES]).toEqual(["noop"]); + expect([...CODING_AGENT_DRIVER_NAMES]).toEqual(["noop", "claude-cli", "codex-cli", "agent-sdk"]); expect(CODING_AGENT_DRIVER_CONFIG_ENV.noop).toEqual({}); + expect(CODING_AGENT_DRIVER_CONFIG_ENV["claude-cli"]).toEqual({ model: "MINER_CODING_AGENT_CLAUDE_MODEL", timeoutMs: "MINER_CODING_AGENT_TIMEOUT_MS" }); + expect(CODING_AGENT_DRIVER_CONFIG_ENV["agent-sdk"]).toEqual({}); }); it("isConfiguredCodingAgentDriver is deny-by-default for unknown names", () => { @@ -581,3 +584,151 @@ describe("lint-guarded edit wrapper (#4276)", () => { expect(decorated.lintGuard.ok).toBe(true); }); }); + +// ── #4289: concrete provider resolution (claude-cli / codex-cli / agent-sdk) ─────────────────────────────── + +describe("createCodingAgentDriver provider resolution (#4289)", () => { + const cliTask: CodingAgentDriverTask = { + attemptId: "attempt-factory-1", + workingDirectory: "/tmp/worktrees/attempt-factory-1", + acceptanceCriteriaPath: "/tmp/worktrees/attempt-factory-1/ACCEPTANCE-CRITERIA.md", + instructions: "Apply the fix.", + maxTurns: 4, + }; + + function recordingSpawn() { + const calls: Array<{ cmd: string; args: readonly string[]; opts: { cwd: string; env: Record; timeoutMs: number } }> = []; + const spawn = async (cmd: string, args: readonly string[], opts: { cwd: string; env: Record; timeoutMs: number }) => { + calls.push({ cmd, args, opts }); + return { stdout: "done", code: 0 }; + }; + return { spawn, calls }; + } + + it("accepts every concrete provider name (locally-authenticated, always configured)", () => { + for (const name of ["claude-cli", "codex-cli", "agent-sdk"]) { + expect(isConfiguredCodingAgentDriver(name, {})).toBe(true); + } + }); + + it("claude-cli spawns the claude command with the driver's default argv when no model is configured", async () => { + const { spawn, calls } = recordingSpawn(); + const driver = createCodingAgentDriver({ providerName: "claude-cli", spawn, env: {} }); + const result = await driver.run(cliTask); + expect(result.ok).toBe(true); + expect(calls[0]!.cmd).toBe("claude"); + expect(calls[0]!.args).not.toContain("--model"); + expect(calls[0]!.args).toContain("--max-turns"); + expect(calls[0]!.opts.cwd).toBe(cliTask.workingDirectory); + }); + + it("CONSUMES the declared model env key: MINER_CODING_AGENT_CLAUDE_MODEL lands in the claude argv", async () => { + const { spawn, calls } = recordingSpawn(); + const driver = createCodingAgentDriver({ + providerName: "claude-cli", + spawn, + env: { MINER_CODING_AGENT_CLAUDE_MODEL: "claude-sonnet-5" }, + }); + await driver.run(cliTask); + const args = [...calls[0]!.args]; + expect(args.slice(0, 2)).toEqual(["--model", "claude-sonnet-5"]); + // The default argv contract still follows the prefix. + expect(args).toContain("--max-turns"); + }); + + it("codex-cli reads ITS OWN model key and ignores claude's", async () => { + const { spawn, calls } = recordingSpawn(); + const driver = createCodingAgentDriver({ + providerName: "codex-cli", + spawn, + env: { MINER_CODING_AGENT_CODEX_MODEL: "gpt-5.1-codex", MINER_CODING_AGENT_CLAUDE_MODEL: "ignored" }, + }); + await driver.run(cliTask); + expect(calls[0]!.cmd).toBe("codex"); + expect([...calls[0]!.args].slice(0, 2)).toEqual(["--model", "gpt-5.1-codex"]); + }); + + it("CONSUMES the declared timeout env key when it is a positive integer, else defers to the driver default", async () => { + const { spawn, calls } = recordingSpawn(); + await createCodingAgentDriver({ providerName: "claude-cli", spawn, env: { MINER_CODING_AGENT_TIMEOUT_MS: "90000" } }).run(cliTask); + expect(calls[0]!.opts.timeoutMs).toBe(90_000); + for (const bad of ["not-a-number", "-5", "0", "1.5", " "]) { + const rec = recordingSpawn(); + await createCodingAgentDriver({ providerName: "claude-cli", spawn: rec.spawn, env: { MINER_CODING_AGENT_TIMEOUT_MS: bad } }).run(cliTask); + expect(rec.calls[0]!.opts.timeoutMs).toBe(120_000); + } + }); + + it("a whitespace-only model env value is treated as unset", async () => { + const { spawn, calls } = recordingSpawn(); + await createCodingAgentDriver({ providerName: "claude-cli", spawn, env: { MINER_CODING_AGENT_CLAUDE_MODEL: " " } }).run(cliTask); + expect(calls[0]!.args).not.toContain("--model"); + }); + + it("fails closed when a CLI provider has no spawn dependency", () => { + expect(() => createCodingAgentDriver({ providerName: "claude-cli" })).toThrowError( + "unconfigured_coding_agent_driver_missing_spawn:claude-cli", + ); + expect(() => createCodingAgentDriver({ providerName: "codex-cli", env: {} })).toThrowError( + "unconfigured_coding_agent_driver_missing_spawn:codex-cli", + ); + }); + + it("forwards knownSecrets to the CLI driver's redaction", async () => { + const secretValue = ["long-injected", "auth-value"].join("-"); + const spawn = async () => ({ stdout: `echoed ${secretValue}`, code: 0 }); + const driver = createCodingAgentDriver({ providerName: "claude-cli", spawn, knownSecrets: [secretValue] }); + const result = await driver.run(cliTask); + expect(result.transcript).not.toContain(secretValue); + expect(result.transcript).toContain("[redacted]"); + }); + + it("agent-sdk resolves with an injected query loop and forwards hooks to the session", async () => { + let captured: { options: { hooks?: unknown } } | undefined; + const hooks = { PreToolUse: [{ hooks: ["policy"] }] }; + const driver = createCodingAgentDriver({ + providerName: "agent-sdk", + hooks, + query: (input) => { + captured = input; + return (async function* (): AsyncGenerator> { + yield { type: "result", subtype: "success", is_error: false, num_turns: 1, result: "ok" }; + })(); + }, + }); + const result = await driver.run(cliTask); + expect(result.ok).toBe(true); + expect(captured!.options.hooks).toBe(hooks); + }); + + it("agent-sdk constructs without any injected deps (real-SDK default) without invoking it", () => { + const driver = createCodingAgentDriver({ providerName: "agent-sdk" }); + expect(typeof driver.run).toBe("function"); + }); + + it("normalizes provider-name case and whitespace", async () => { + const { spawn, calls } = recordingSpawn(); + const driver = createCodingAgentDriver({ providerName: " Claude-CLI ", spawn }); + await driver.run(cliTask); + expect(calls[0]!.cmd).toBe("claude"); + }); + + it("resolveFirstConfiguredCodingAgentDriverName skips unknown names (primary-then-fallback) and fails closed on none", () => { + expect(resolveFirstConfiguredCodingAgentDriverName({ MINER_CODING_AGENT_PROVIDER: "mystery, agent-sdk, noop" })).toBe("agent-sdk"); + expect(resolveFirstConfiguredCodingAgentDriverName({ MINER_CODING_AGENT_PROVIDER: "mystery,unknown" })).toBeUndefined(); + expect(resolveFirstConfiguredCodingAgentDriverName({})).toBeUndefined(); + }); + + it("runCodingAgentAttempt threads provider deps end-to-end (claude-cli under live mode)", async () => { + const { spawn, calls } = recordingSpawn(); + const { mode, result } = await runCodingAgentAttempt({ + providerName: "claude-cli", + env: { MINER_CODING_AGENT_CLAUDE_MODEL: "claude-sonnet-5" }, + spawn, + task: cliTask, + }); + expect(mode).toBe("live"); + expect(result.ok).toBe(true); + expect([...calls[0]!.args].slice(0, 2)).toEqual(["--model", "claude-sonnet-5"]); + }); +});