diff --git a/packages/agent-driver/src/codex.LOGIC.md b/packages/agent-driver/src/codex.LOGIC.md index d1a60c2ab..a3e282d67 100644 --- a/packages/agent-driver/src/codex.LOGIC.md +++ b/packages/agent-driver/src/codex.LOGIC.md @@ -26,7 +26,7 @@ Drives Codex as a driver [1]: each turn [2] is one non-interactive invocation of ## Business logic — TL;DR - **Starting and prompting Codex** - every turn [2] spawns `codex` in its non-interactive mode with streamed JSON output, pointed at the driver session's [3] directory, with the prompt over standard input. -- **Sandboxed to the directory** - Codex runs under its `workspace-write` sandbox unless the driver [1] was configured with `read-only` or `danger-full-access`; the flag that bypasses Codex's approvals and sandbox is never passed. +- **Sandboxed to the directory** - Codex runs under its `workspace-write` sandbox unless the driver [1] was configured with `read-only` or `danger-full-access`; under `workspace-write` the directory's git repository data is writable too, so the coding agent [4] can commit; the flag that bypasses Codex's approvals and sandbox is never passed. - **Framing rides ahead of the prompt** - Codex has no system prompt flag, so the driver session's framing [10] and the turn's extra framing are placed in front of the prompt, as their own block. - **Model pass-through** - the model the caller names is passed to Codex as is; without one, Codex's own default runs. - **Every turn starts fresh** - the driver never resumes a Codex conversation: a request to continue the previous turn, and an earlier session id to continue, are ignored and the turn runs fresh. @@ -57,6 +57,8 @@ Every turn [2] spawns the `codex` command, found on `PATH` unless the driver [1] Unless told otherwise, Codex runs with its `workspace-write` sandbox, so the coding agent [4] can edit the directory it was pointed at and nothing else on the machine; this is the counterpart of Claude Code's `acceptEdits` permission mode. The driver [1] can be configured with `read-only` or with `danger-full-access` instead. Codex's flag that bypasses both its approvals and its sandbox is never passed, whatever the configuration. +Under `workspace-write`, Codex keeps a `.git` directory at the root of the directory read-only. In a plain clone, as opposed to a git worktree such as an agent's checkout [8], that makes the coding agent's [4] first commit or branch rename fail. So on every turn [2] the driver asks git for the absolute path of the directory's git repository data (the common git directory, which for a worktree is the main checkout's `.git`) and tells Codex to make that path writable as well. The path is resolved per turn, not once, because the coding agent may turn the directory into a git repository during an earlier turn. When the directory is not in a git repository, or git is not installed, nothing is added and the turn runs as before. Nothing is added under `read-only` or `danger-full-access`. A writable git directory also lets the coding agent change the repository's hooks and configuration, which run later outside the sandbox; this is accepted, since committing is the coding agent's job and a worktree's git data was already writable. Extra command-line arguments the driver was configured with come after, so one that sets Codex's writable paths itself replaces this path rather than adding to it. + ### Framing rides ahead of the prompt #### Context diff --git a/packages/agent-driver/src/codex.test.LOGIC.md b/packages/agent-driver/src/codex.test.LOGIC.md index 78f933af4..97c1b6cdd 100644 --- a/packages/agent-driver/src/codex.test.LOGIC.md +++ b/packages/agent-driver/src/codex.test.LOGIC.md @@ -3,7 +3,8 @@ What the tests cover, against the streamed output of a real Codex run: - **Reading the streamed output** - the last message Codex sends is the turn's answer, not the first, and the thread id is the turn's session id; every message surfaces as streamed text and every started work item as a tool use named by its kind only; a banner line, an empty line, a JSON `null` and the line opening the turn are ignored; a turn with no message answers with an empty final message. - **Usage** - the token counts are reported and the price is absent, never zero; the cached tokens are split out of Codex's inclusive input total; reasoning tokens are not added to the output count a second time; a missing or malformed usage payload yields no usage; absent counts read as zero; a cached count above the input count cannot push the uncached input negative. - **Running a turn** - a turn through the command resolves with the last message and the session id, reports the work item's kind as an action and closes with a `result` progress event. -- **The command line** - Codex runs in its non-interactive mode with JSON output, under the `workspace-write` sandbox, pointed at the driver session's directory, with its git repository check skipped, and never with the flag that bypasses its approvals and sandbox. +- **The command line** - Codex runs in its non-interactive mode with JSON output, under the `workspace-write` sandbox, pointed at the driver session's directory, with its git repository check skipped, and never with the flag that bypasses its approvals and sandbox; in a directory that is no git repository, nothing more is passed. +- **A writable git directory** - in a freshly initialized git repository, the repository's absolute `.git` path is passed to Codex as a writable path under `workspace-write`, and no writable path is passed under `read-only`. - **The prompt over standard input** - the prompt is fed to Codex over standard input and never appears as a command-line argument. - **Framing ahead of the prompt** - the driver session's framing, then the turn's extra framing, then the prompt, as blank-line separated blocks. - **Model pass-through** - the model the caller names reaches Codex's command line. diff --git a/packages/agent-driver/src/codex.test.ts b/packages/agent-driver/src/codex.test.ts index c16007ec2..2fbfc972b 100644 --- a/packages/agent-driver/src/codex.test.ts +++ b/packages/agent-driver/src/codex.test.ts @@ -1,4 +1,8 @@ import { strict as assert } from 'node:assert' +import { execFileSync } from 'node:child_process' +import { mkdtemp, realpath, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' import { test } from 'node:test' import { Readable, Writable } from 'node:stream' import { CodexDriver, CodexJsonParser, parseCodexUsage } from './codex.js' @@ -142,6 +146,38 @@ test('CodexDriver runs sandboxed in the workspace, never with the bypass (#539)' assert.ok(seen.includes('--skip-git-repo-check')) }) +test('CodexDriver makes the git dir writable, so a plain checkout can commit (#1747)', async () => { + const dir = await realpath(await mkdtemp(join(tmpdir(), 'codex-git-'))) + try { + execFileSync('git', ['init', '-q'], { cwd: dir }) + let seen: readonly string[] = [] + const driver = new CodexDriver({ spawn: fakeSpawn(REAL_RUN, args => (seen = args)) }) + const session = await driver.start({ cwd: dir }) + await session.prompt('go') + // `workspace-write` keeps a root `.git/` read-only; without this the commit fails on `.git/index.lock`. + assert.deepEqual([...seen], [ + 'exec', '--json', '--skip-git-repo-check', '--sandbox', 'workspace-write', '-C', dir, + '-c', `sandbox_workspace_write.writable_roots=["${join(dir, '.git')}"]`, + ]) + } finally { + await rm(dir, { recursive: true, force: true }) + } +}) + +test('CodexDriver widens no read-only sandbox, even in a repository (#1747)', async () => { + const dir = await realpath(await mkdtemp(join(tmpdir(), 'codex-git-'))) + try { + execFileSync('git', ['init', '-q'], { cwd: dir }) + let seen: readonly string[] = [] + const driver = new CodexDriver({ sandbox: 'read-only', spawn: fakeSpawn(REAL_RUN, args => (seen = args)) }) + const session = await driver.start({ cwd: dir }) + await session.prompt('go') + assert.ok(!seen.includes('-c')) + } finally { + await rm(dir, { recursive: true, force: true }) + } +}) + test('CodexDriver sends the prompt over stdin, not as an argument (#539)', async () => { let stdin = '' let seen: readonly string[] = [] diff --git a/packages/agent-driver/src/codex.ts b/packages/agent-driver/src/codex.ts index 5330f5f21..b95bf3af9 100644 --- a/packages/agent-driver/src/codex.ts +++ b/packages/agent-driver/src/codex.ts @@ -1,4 +1,4 @@ -import { spawn as nodeSpawn } from 'node:child_process' +import { execFile, spawn as nodeSpawn } from 'node:child_process' import { runCliSession, type AgentCliParser, type SpawnLike } from './cli-session.js' import { combineFraming, combineSignals, makeEmit, readWorkspaceFile } from './session-support.js' import type { Driver, DriverEvent, DriverPromptOptions, DriverSession, DriverStartOptions, DriverTurn, DriverUsage } from './types.js' @@ -18,7 +18,11 @@ export interface CodexDriverOptions { bin?: string /** Sandbox policy. Default `"workspace-write"`. */ sandbox?: CodexSandbox - /** Extra CLI args appended verbatim (escape hatch). */ + /** + * Extra CLI args appended verbatim (escape hatch). Appended last, so a + * `-c sandbox_workspace_write.writable_roots=...` here replaces the git dir + * the driver makes writable rather than adding to it. + */ extraArgs?: string[] /** Environment for the child process. Default `process.env`. */ env?: NodeJS.ProcessEnv @@ -69,14 +73,16 @@ export class CodexSession implements DriverSession { this.id = `codex-${++sessionCounter}` } - prompt(text: string, opts: DriverPromptOptions = {}): Promise { + async prompt(text: string, opts: DriverPromptOptions = {}): Promise { // Codex takes no system-prompt flag, so the framing rides in front of the // prompt. Blank-line separated, so it reads as its own block. const framing = combineFraming(this.startOpts.system, opts.system) const prompt = framing ? `${framing}\n\n${text}` : text + // Resolved every turn, not at start: the agent may `git init` in turn 1. + const gitDir = await gitCommonDir(this.cwd) return runCliSession({ bin: this.config.bin ?? 'codex', - args: this.buildArgs(), + args: this.buildArgs(gitDir), cwd: this.cwd, env: this.config.env ?? process.env, prompt, @@ -97,17 +103,34 @@ export class CodexSession implements DriverSession { return Promise.resolve() } - private buildArgs(): string[] { + private buildArgs(gitDir: string | undefined): string[] { // No prompt argument: it goes over stdin, so a long one never hits the // arg-length limit. `--skip-git-repo-check` because Codex otherwise refuses // to run outside a git repo, and a workspace may legitimately not be one yet. - const args = ['exec', '--json', '--skip-git-repo-check', '--sandbox', this.config.sandbox ?? 'workspace-write', '-C', this.cwd] + const sandbox = this.config.sandbox ?? 'workspace-write' + const args = ['exec', '--json', '--skip-git-repo-check', '--sandbox', sandbox, '-C', this.cwd] + // `workspace-write` keeps a `.git/` directory at the workspace root read-only, + // so in a plain checkout (not a worktree) the agent's commit fails on + // `.git/index.lock` (verified on codex-cli 0.144.4, #1747). The git dir is made + // writable: committing is the agent's job, and a worktree's git dir already is. + // The `-c` value is TOML; a JSON string array is a valid TOML array. + if (sandbox === 'workspace-write' && gitDir) args.push('-c', `sandbox_workspace_write.writable_roots=${JSON.stringify([gitDir])}`) if (this.startOpts.model) args.push('-m', this.startOpts.model) if (this.config.extraArgs) args.push(...this.config.extraArgs) return args } } +/** The absolute git common dir of `cwd`, or `undefined` when it is not a repository or git is missing. */ +function gitCommonDir(cwd: string): Promise { + return new Promise(resolve => { + execFile('git', ['rev-parse', '--path-format=absolute', '--git-common-dir'], { cwd }, (err, stdout) => { + const dir = String(stdout).trim() + resolve(err || !dir ? undefined : dir) + }) + }) +} + /** * Parses Codex's `exec --json` output: one JSON event per line. *