diff --git a/cli/__tests__/adapters.codex.test.mjs b/cli/__tests__/adapters.codex.test.mjs index 8dd6326917..d162675a54 100644 --- a/cli/__tests__/adapters.codex.test.mjs +++ b/cli/__tests__/adapters.codex.test.mjs @@ -134,6 +134,20 @@ describe('codex adapter — detect()', () => { }); describe('codex adapter — spawn()', () => { + test('an aborted signal before the child is created spawns nothing', async () => { + const { impl, calls } = makeSpawnImpl({ outputContents: 'ok' }); + const controller = new AbortController(); + const reason = new Error('stalled during preparation'); + controller.abort(reason); + await expect(codex.spawn('hi', { + environment: { model: 'test-model' }, + sessionId: null, + signal: controller.signal, + _spawnImpl: impl, + })).rejects.toBe(reason); + expect(calls).toHaveLength(0); + }); + test('BYO without fleet opt-in retains the CLI default on fresh and resumed turns', async () => { const prior = process.env.COMMONLY_REQUIRE_SEAT_MODEL; delete process.env.COMMONLY_REQUIRE_SEAT_MODEL; diff --git a/cli/__tests__/supervise-runs-flag.test.mjs b/cli/__tests__/supervise-runs-flag.test.mjs index c1e16a87cc..0ca1e2d0c1 100644 --- a/cli/__tests__/supervise-runs-flag.test.mjs +++ b/cli/__tests__/supervise-runs-flag.test.mjs @@ -41,7 +41,7 @@ await jest.unstable_mockModule('../src/lib/pod-focus.js', () => ({ })); const { createClient } = await import('../src/lib/api.js'); -const { performRun } = await import('../src/commands/agent.js'); +const { performRun, superviseRunsByDefault } = await import('../src/commands/agent.js'); const stubAdapter = (await import('../src/lib/adapters/stub.js')).default; const drain = async (depth = 40) => { @@ -274,13 +274,34 @@ const filesUnder = (dir) => { return found; }; -describe('COMMONLY_SUPERVISE_RUNS=1 with the real codex adapter', () => { +describe('superviseRunsByDefault: codex on by default, others only on =1', () => { + test.each([ + ['codex', undefined, true], + ['codex', '1', true], + ['codex', '0', false], + ['codex', 'true', true], + ['claude', undefined, false], + ['claude', '1', true], + ['claude', '0', false], + ['stub', undefined, false], + ['stub', '', false], + [undefined, undefined, false], + ])('adapter %s with COMMONLY_SUPERVISE_RUNS=%s -> %s', (adapterName, value, expected) => { + const env = value === undefined ? {} : { COMMONLY_SUPERVISE_RUNS: value }; + expect(superviseRunsByDefault(adapterName, env)).toBe(expected); + }); +}); + +describe('COMMONLY_SUPERVISE_RUNS with the real codex adapter', () => { // Each turn spawns a real child and makes real loopback calls; under a // parallel suite on a loaded host that takes longer than jest's 5 s default. jest.setTimeout(60_000); const TURN_KEY = 'sk-turn-run-loop-4c1d9e7a'; - test('a clean reply is posted, the metric line says supervised and delivered, and nothing else reaches disk', async () => { + test.each([ + ['unset (codex default)', undefined], + ['=1', '1'], + ])('flag %s: a clean reply is posted, the metric line says supervised and delivered, and nothing else reaches disk', async (_label, flag) => { const { startFakeProvider } = await import('../../packages/seats/test-support/fake-provider.js'); const codex = (await import('../src/lib/adapters/codex.js')).default; const provider = await startFakeProvider({ defaultReply: 'review: PASS' }); @@ -294,14 +315,15 @@ describe('COMMONLY_SUPERVISE_RUNS=1 with the real codex adapter', () => { fs.writeFileSync(path.join(bin, 'codex'), STUB_CODEX, { mode: 0o755 }); const workspace = fs.mkdtempSync(path.join(home, 'ws-')); const env = { - COMMONLY_SUPERVISE_RUNS: '1', CODEX_HOME: codexHome, FAKE_PROVIDER_KEY: TURN_KEY, STUB_CODEX_DIRECT_BASE: provider.baseURL, PATH: `${bin}${path.delimiter}${path.dirname(process.execPath)}${path.delimiter}${process.env.PATH}`, }; // The env must stay set while the turn runs: the adapter reads it at spawn. - const saved = Object.fromEntries(Object.keys(env).map((k) => [k, process.env[k]])); + const saved = Object.fromEntries(['COMMONLY_SUPERVISE_RUNS', ...Object.keys(env)].map((k) => [k, process.env[k]])); + delete process.env.COMMONLY_SUPERVISE_RUNS; + if (flag !== undefined) process.env.COMMONLY_SUPERVISE_RUNS = flag; Object.assign(process.env, env); try { const run = harness({ diff --git a/cli/src/commands/agent.js b/cli/src/commands/agent.js index 209da246c5..091a5d0e82 100644 --- a/cli/src/commands/agent.js +++ b/cli/src/commands/agent.js @@ -1386,6 +1386,15 @@ const extractPrompt = (event) => { // cancel grace, and the wall-clock backstop past the adapter's own timeout. export { supervisedRunPolicy }; +// Codex turns are supervised by default (D211): a turn that stops making +// progress is named and killed instead of holding the seat. COMMONLY_SUPERVISE_RUNS=0 +// opts out. Every other adapter stays off unless COMMONLY_SUPERVISE_RUNS=1. +export const superviseRunsByDefault = (adapterName, env = process.env) => { + if (env.COMMONLY_SUPERVISE_RUNS === '0') return false; + if (adapterName === 'codex') return true; + return env.COMMONLY_SUPERVISE_RUNS === '1'; +}; + /** * Start the run loop. Polls /events, spawns the adapter per event, posts the * result into the pod, acks. @@ -1461,14 +1470,15 @@ export const performRun = ({ resolveOwnProviderKey = null, runtimeReportClient = null, refusalHandoff = null, - // Off unless COMMONLY_SUPERVISE_RUNS=1: each spawn runs under the seats - // supervisor (stall window, wall-clock backstop, a named failure on every - // settle). The CLI still talks to its provider directly, and one metadata - // line per turn goes to ~/.commonly/state/turns/.jsonl once - // delivery settles. Off, the spawn path is the plain adapter call it has - // always been, and nothing is written. - superviseRuns = process.env.COMMONLY_SUPERVISE_RUNS === '1', + // Each spawn runs under the seats supervisor (stall window, wall-clock + // backstop, a named failure on every settle) when superviseRuns is true. + // Unset, the default comes from superviseRunsByDefault. The CLI still talks + // to its provider directly, and one metadata line per turn goes to + // ~/.commonly/state/turns/.jsonl once delivery settles. Off, the spawn + // path is the plain adapter call it has always been, and nothing is written. + superviseRuns, }) => { + const supervising = superviseRuns ?? superviseRunsByDefault(adapter?.name); reportRuntime = reportRuntime && WALKED_HARNESSES.includes(adapter?.name); if (environment?.provider !== undefined && !adapter?.supportsProvider) { throw new Error('environment.provider requires a provider-capable adapter'); @@ -1480,9 +1490,9 @@ export const performRun = ({ // The supervised run keeps its trace in memory, metadata only; no record of // a run is written to disk (lib/supervised-turn.js). const turnRunner = createTurnRunner({ - seat: agentName, turnTimeoutMs: effectiveTurnTimeoutMs, supervise: superviseRuns, log, + seat: agentName, turnTimeoutMs: effectiveTurnTimeoutMs, supervise: supervising, log, }); - if (superviseRuns) log(turnRunner.describe(adapter?.name)); + if (supervising) log(turnRunner.describe(adapter?.name)); const runtimeReporter = createRuntimeReporter({ client: runtimeReportClient || { post: async () => {} }, harness: adapter.name, detect: () => adapter.detect(), @@ -1895,7 +1905,7 @@ export const performRun = ({ // The spawn → post-decision half of a turn, split out so the claim release // above is a plain finally instead of threading through every return path. - // With superviseRuns, the turn's log line is written once delivery has + // With supervising, the turn's log line is written once delivery has // settled; anything thrown after a spawn that resolved means the reply never // reached the pod. Off, `turn` writes nothing. const runTurn = async (args) => { diff --git a/cli/src/lib/adapters/codex.js b/cli/src/lib/adapters/codex.js index 4ae97d260e..b6cc54930d 100644 --- a/cli/src/lib/adapters/codex.js +++ b/cli/src/lib/adapters/codex.js @@ -897,6 +897,13 @@ export default { await writePublicCodexInstructions(childEnv.CODEX_HOME, ctx); } + // The awaits above can outlast a supervisor stall window. Once it has + // aborted, the attempt has already settled and a retry may be running, + // so no process is created for this one. Checked synchronously before + // runCodex, so the spawn itself cannot race the abort. + if (ctx.signal?.aborted) { + throw ctx.signal.reason || new Error('codex spawn cancelled before the process started'); + } const { threadId } = await runCodex({ args, cwd: ctx.cwd, diff --git a/cli/src/lib/supervised-turn.js b/cli/src/lib/supervised-turn.js index 651286a34b..420f0fb126 100644 --- a/cli/src/lib/supervised-turn.js +++ b/cli/src/lib/supervised-turn.js @@ -3,7 +3,8 @@ * path also writes one metadata line per turn (the turn log), once the * turn's outcome has reached the pod or failed to. * - * COMMONLY_SUPERVISE_RUNS=1 (read by performRun as `superviseRuns`) + * codex by default, or COMMONLY_SUPERVISE_RUNS=1 (superviseRunsByDefault in + * commands/agent.js; COMMONLY_SUPERVISE_RUNS=0 opts codex out) * the turn runs under `superviseRun` from @commonlyai/seats: a stall * window, a wall-clock backstop, and a guarantee the turn settles with a * named failure. No provider relay: the CLI talks to its provider directly,