diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 62cb5c36..6eba6c6f 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -5,13 +5,13 @@ }, "metadata": { "description": "Codex plugins to use in Claude Code for delegation and code review.", - "version": "1.0.6-fork.1" + "version": "1.0.6-fork.2" }, "plugins": [ { "name": "codex", "description": "Use Codex from Claude Code to review code or delegate tasks.", - "version": "1.0.6-fork.1", + "version": "1.0.6-fork.2", "author": { "name": "OpenAI" }, diff --git a/README.md b/README.md index e4f4196a..ee16ff99 100644 --- a/README.md +++ b/README.md @@ -86,15 +86,15 @@ Use it when you want: - a review of your current uncommitted changes - a review of your branch compared to a base branch like `main` -Use `--base ` for branch review. It also supports `--wait`, `--background`, `--model `, and `--effort `. It is not steerable and does not take custom focus text. Use [`/codex:adversarial-review`](#codexadversarial-review) when you want to challenge a specific decision or risk area. +Use `--base ` for branch review. It also supports `--wait`, `--background`, `--model`, and `--effort`. It is not steerable and does not take custom focus text. Use [`/codex:adversarial-review`](#codexadversarial-review) when you want to challenge a specific decision or risk area. Examples: ```bash /codex:review /codex:review --base main -/codex:review --model spark --effort medium /codex:review --background +/codex:review --model gpt-5.6-sol --effort max ``` This command is read-only and will not perform any changes. When run in the background you can use [`/codex:status`](#codexstatus) to check on the progress and [`/codex:cancel`](#codexcancel) to cancel the ongoing task. @@ -106,7 +106,7 @@ Runs a **steerable** review that questions the chosen implementation and design. It can be used to pressure-test assumptions, tradeoffs, failure modes, and whether a different approach would have been safer or simpler. It uses the same review target selection as `/codex:review`, including `--base ` for branch review. -It also supports `--wait`, `--background`, `--model `, and `--effort `. Unlike `/codex:review`, it can take extra focus text after the flags. +It also supports `--wait`, `--background`, `--model`, and `--effort`. Unlike `/codex:review`, it can take extra focus text after the flags. Use it when you want: @@ -119,8 +119,8 @@ Examples: ```bash /codex:adversarial-review /codex:adversarial-review --base main challenge whether this was the right caching and retry design -/codex:adversarial-review --model gpt-5.4-mini --effort high look for race conditions and question the chosen approach /codex:adversarial-review --background look for race conditions and question the chosen approach +/codex:adversarial-review --model gpt-5.6-terra --effort xhigh challenge the retry design ``` This command is read-only. It does not fix code. @@ -162,6 +162,8 @@ Ask Codex to redesign the database connection to be more resilient. - if you do not pass `--model` or `--effort`, Codex chooses its own defaults. - if you say `spark`, the plugin maps that to `gpt-5.3-codex-spark` +- reasoning efforts are `none`, `minimal`, `low`, `medium`, `high`, `xhigh`, `max`, and `ultra`; the current Codex model catalog is used to reject unsupported combinations such as `gpt-5.6-luna` with `ultra` +- model names are otherwise passed through, so custom providers and newly released models are not blocked by a plugin allowlist - follow-up rescue requests can continue the latest Codex task in the repo ### `/codex:transfer` diff --git a/package-lock.json b/package-lock.json index 0c919c3d..86aa59a1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@openai/codex-plugin-cc", - "version": "1.0.6", + "version": "1.0.7", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@openai/codex-plugin-cc", - "version": "1.0.6", + "version": "1.0.7", "license": "Apache-2.0", "devDependencies": { "@types/node": "^25.5.0", diff --git a/package.json b/package.json index b1d984d1..26ebc91d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@openai/codex-plugin-cc", - "version": "1.0.6", + "version": "1.0.7", "private": true, "type": "module", "description": "Use Codex from Claude Code to review code or delegate tasks.", diff --git a/plugins/codex/.claude-plugin/plugin.json b/plugins/codex/.claude-plugin/plugin.json index 7186a01a..d65a87e4 100644 --- a/plugins/codex/.claude-plugin/plugin.json +++ b/plugins/codex/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "codex", - "version": "1.0.6-fork.1", + "version": "1.0.6-fork.2", "description": "Use Codex from Claude Code to review code or delegate tasks.", "author": { "name": "OpenAI" diff --git a/plugins/codex/CHANGELOG.md b/plugins/codex/CHANGELOG.md index d647561b..bcd5d5c6 100644 --- a/plugins/codex/CHANGELOG.md +++ b/plugins/codex/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## Unreleased + +- Stop an orphaned shared Codex app-server broker after 15 minutes with no connected clients. Active foreground and background jobs keep their broker connection open, and the timeout can be configured with `CODEX_COMPANION_BROKER_IDLE_TIMEOUT_MS` (`0` disables the safety timer). +- Close the broker listener before asynchronous child cleanup and safely reject reconnects already queued during shutdown. + ## 1.0.0 - Initial version of the Codex plugin for Claude Code diff --git a/plugins/codex/commands/adversarial-review.md b/plugins/codex/commands/adversarial-review.md index 867ac973..94e86177 100644 --- a/plugins/codex/commands/adversarial-review.md +++ b/plugins/codex/commands/adversarial-review.md @@ -1,6 +1,6 @@ --- description: Run a Codex review that challenges the implementation approach and design choices -argument-hint: '[--wait|--background] [--base ] [--scope auto|working-tree|branch] [--model ] [--effort ] [focus ...]' +argument-hint: '[--wait|--background] [--base ] [--scope auto|working-tree|branch] [--model ] [--effort ] [focus ...]' disable-model-invocation: true allowed-tools: Read, Glob, Grep, Bash(node:*), Bash(git:*), AskUserQuestion --- @@ -36,9 +36,9 @@ Execution mode rules: Argument handling: - Preserve the user's arguments exactly. +- `--model` and `--effort` select the Codex runtime and must not become part of the focus text. - Do not strip `--wait` or `--background` yourself. - Do not weaken the adversarial framing or rewrite the user's focus text. -- `--model` and `--effort` are runtime-selection flags. Preserve them for the forwarded `adversarial-review` call, but do not treat them as part of the review instructions. - The companion script parses `--wait` and `--background`, but Claude Code's `Bash(..., run_in_background: true)` is what actually detaches the run. - `/codex:adversarial-review` uses the same review target selection as `/codex:review`. - It supports working-tree review, branch review, and `--base `. diff --git a/plugins/codex/commands/rescue.md b/plugins/codex/commands/rescue.md index 56de9555..2d610e7b 100644 --- a/plugins/codex/commands/rescue.md +++ b/plugins/codex/commands/rescue.md @@ -1,6 +1,6 @@ --- description: Delegate investigation, an explicit fix request, or follow-up rescue work to the Codex rescue subagent -argument-hint: "[--background|--wait] [--resume|--fresh] [--model ] [--effort ] [what Codex should investigate, solve, or continue]" +argument-hint: "[--background|--wait] [--resume|--fresh] [--model ] [--effort ] [what Codex should investigate, solve, or continue]" allowed-tools: Bash(node:*), AskUserQuestion, Agent --- diff --git a/plugins/codex/commands/review.md b/plugins/codex/commands/review.md index 665aae65..8568ce02 100644 --- a/plugins/codex/commands/review.md +++ b/plugins/codex/commands/review.md @@ -1,6 +1,6 @@ --- description: Run a Codex code review against local git state -argument-hint: '[--wait|--background] [--base ] [--scope auto|working-tree|branch] [--model ] [--effort ]' +argument-hint: '[--wait|--background] [--base ] [--scope auto|working-tree|branch] [--model ] [--effort ]' disable-model-invocation: true allowed-tools: Read, Glob, Grep, Bash(node:*), Bash(git:*), AskUserQuestion --- @@ -33,9 +33,9 @@ Execution mode rules: Argument handling: - Preserve the user's arguments exactly. +- `--model` and `--effort` select the Codex runtime for the review and are not focus text. - Do not strip `--wait` or `--background` yourself. - Do not add extra review instructions or rewrite the user's intent. -- `--model` and `--effort` are runtime-selection flags. Preserve them for the forwarded `review` call, but do not treat them as part of the review instructions. - The companion script parses `--wait` and `--background`, but Claude Code's `Bash(..., run_in_background: true)` is what actually detaches the run. - `/codex:review` is native-review only. It does not support staged-only review, unstaged-only review, or extra focus text. - If the user needs custom review instructions or more adversarial framing, they should use `/codex:adversarial-review`. diff --git a/plugins/codex/scripts/app-server-broker.mjs b/plugins/codex/scripts/app-server-broker.mjs index c8b58118..4b6cc8a0 100644 --- a/plugins/codex/scripts/app-server-broker.mjs +++ b/plugins/codex/scripts/app-server-broker.mjs @@ -10,6 +10,21 @@ import { BROKER_BUSY_RPC_CODE, CodexAppServerClient } from "./lib/app-server.mjs import { parseBrokerEndpoint } from "./lib/broker-endpoint.mjs"; const STREAMING_METHODS = new Set(["turn/start", "review/start", "thread/compact/start"]); +const BROKER_IDLE_TIMEOUT_ENV = "CODEX_COMPANION_BROKER_IDLE_TIMEOUT_MS"; +const DEFAULT_BROKER_IDLE_TIMEOUT_MS = 15 * 60 * 1000; + +function resolveIdleTimeoutMs(env = process.env) { + const rawValue = env[BROKER_IDLE_TIMEOUT_ENV]; + if (rawValue == null || rawValue === "") { + return DEFAULT_BROKER_IDLE_TIMEOUT_MS; + } + + const parsed = Number(rawValue); + if (!Number.isFinite(parsed) || parsed < 0) { + return DEFAULT_BROKER_IDLE_TIMEOUT_MS; + } + return Math.floor(parsed); +} function buildStreamThreadIds(method, params, result) { const threadIds = new Set(); @@ -70,6 +85,16 @@ async function main() { let activeStreamSocket = null; let activeStreamThreadIds = null; const sockets = new Set(); + const idleTimeoutMs = resolveIdleTimeoutMs(); + let idleTimer = null; + let shuttingDown = false; + + function cancelIdleShutdown() { + if (idleTimer) { + clearTimeout(idleTimer); + idleTimer = null; + } + } function clearSocketOwnership(socket) { if (activeRequestSocket === socket) { @@ -100,11 +125,20 @@ async function main() { } async function shutdown(server) { + if (shuttingDown) { + return; + } + shuttingDown = true; + cancelIdleShutdown(); + // Stop accepting connections before awaiting child cleanup. Otherwise a + // reconnect can slip into the async shutdown window and inherit a closing + // app-server client. + const serverClosed = new Promise((resolve) => server.close(resolve)); for (const socket of sockets) { socket.end(); } await appClient.close().catch(() => {}); - await new Promise((resolve) => server.close(resolve)); + await serverClosed; if (listenTarget.kind === "unix" && fs.existsSync(listenTarget.path)) { fs.unlinkSync(listenTarget.path); } @@ -113,9 +147,33 @@ async function main() { } } + function scheduleIdleShutdown(server) { + cancelIdleShutdown(); + if (shuttingDown || idleTimeoutMs === 0 || sockets.size > 0 || activeRequestSocket || activeStreamSocket) { + return; + } + + idleTimer = setTimeout(async () => { + idleTimer = null; + if (sockets.size > 0 || activeRequestSocket || activeStreamSocket) { + return; + } + await shutdown(server); + process.exit(0); + }, idleTimeoutMs); + } + appClient.setNotificationHandler(routeNotification); const server = net.createServer((socket) => { + if (shuttingDown) { + // An already-accepted connection event can be delivered after + // server.close(). Reject it decisively and absorb a simultaneous reset. + socket.on("error", () => {}); + socket.destroy(); + return; + } + cancelIdleShutdown(); sockets.add(socket); socket.setEncoding("utf8"); let buffer = ""; @@ -158,6 +216,13 @@ async function main() { } if (message.id !== undefined && message.method === "broker/shutdown") { + if (activeRequestSocket || activeStreamSocket) { + send(socket, { + id: message.id, + error: buildJsonRpcError(BROKER_BUSY_RPC_CODE, "Shared Codex broker is busy.") + }); + continue; + } send(socket, { id: message.id, result: {} }); await shutdown(server); process.exit(0); @@ -225,11 +290,13 @@ async function main() { socket.on("close", () => { sockets.delete(socket); clearSocketOwnership(socket); + scheduleIdleShutdown(server); }); socket.on("error", () => { sockets.delete(socket); clearSocketOwnership(socket); + scheduleIdleShutdown(server); }); }); @@ -243,7 +310,9 @@ async function main() { process.exit(0); }); - server.listen(listenTarget.path); + server.listen(listenTarget.path, () => { + scheduleIdleShutdown(server); + }); } main().catch((error) => { diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index c3077c9f..f3d4f0c9 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -8,19 +8,17 @@ import { fileURLToPath } from "node:url"; import { parseArgs, splitRawArgumentString } from "./lib/args.mjs"; import { - buildPersistentTaskThreadName, - DEFAULT_CONTINUE_PROMPT, - findLatestTaskThread, - getCodexAuthStatus, - getCodexAvailability, - getSessionRuntimeStatus, - importExternalAgentSession, - interruptAppServerTurn, - parseStructuredOutput, - readOutputSchema, - runAppServerReview, - runAppServerTurn - } from "./lib/codex.mjs"; + findLatestTaskThread, + getCodexAuthStatus, + getCodexAvailability, + getSessionRuntimeStatus, + importExternalAgentSession, + interruptAppServerTurn, + runAppServerReview, + runAppServerTurn +} from "./lib/codex.mjs"; +import { parseStructuredOutput, readOutputSchema } from "./lib/structured-output.mjs"; +import { buildPersistentTaskThreadName, DEFAULT_CONTINUE_PROMPT } from "./lib/task-thread.mjs"; import { resolveClaudeSessionPath } from "./lib/claude-session-transfer.mjs"; import { readStdinIfPiped } from "./lib/fs.mjs"; import { collectReviewContext, ensureGitRepository, resolveReviewTarget } from "./lib/git.mjs"; @@ -68,7 +66,8 @@ const ROOT_DIR = path.resolve(fileURLToPath(new URL("..", import.meta.url))); const REVIEW_SCHEMA = path.join(ROOT_DIR, "schemas", "review-output.schema.json"); const DEFAULT_STATUS_WAIT_TIMEOUT_MS = 240000; const DEFAULT_STATUS_POLL_INTERVAL_MS = 2000; -const VALID_REASONING_EFFORTS = new Set(["none", "minimal", "low", "medium", "high", "xhigh"]); +const REASONING_EFFORTS = ["none", "minimal", "low", "medium", "high", "xhigh", "max", "ultra"]; +const VALID_REASONING_EFFORTS = new Set(REASONING_EFFORTS); const MODEL_ALIASES = new Map([["spark", "gpt-5.3-codex-spark"]]); const STOP_REVIEW_TASK_MARKER = "Run a stop-gate review of the previous Claude turn."; @@ -77,9 +76,9 @@ function printUsage() { [ "Usage:", " node scripts/codex-companion.mjs setup [--enable-review-gate|--disable-review-gate] [--json]", - " node scripts/codex-companion.mjs review [--wait|--background] [--base ] [--scope ]", - " node scripts/codex-companion.mjs adversarial-review [--wait|--background] [--base ] [--scope ] [focus text]", - " node scripts/codex-companion.mjs task [--background] [--write] [--resume-last|--resume|--fresh] [--model ] [--effort ] [prompt]", + " node scripts/codex-companion.mjs review [--wait|--background] [--base ] [--scope ] [--model ] [--effort ]", + " node scripts/codex-companion.mjs adversarial-review [--wait|--background] [--base ] [--scope ] [--model ] [--effort ] [focus text]", + " node scripts/codex-companion.mjs task [--background] [--write] [--resume-last|--resume|--fresh] [--model ] [--effort ] [prompt]", " node scripts/codex-companion.mjs transfer [--source ] [--json]", " node scripts/codex-companion.mjs status [job-id] [--all] [--json]", " node scripts/codex-companion.mjs result [job-id] [--json]", @@ -121,7 +120,7 @@ function normalizeReasoningEffort(effort) { } if (!VALID_REASONING_EFFORTS.has(normalized)) { throw new Error( - `Unsupported reasoning effort "${effort}". Use one of: none, minimal, low, medium, high, xhigh.` + `Unsupported reasoning effort "${effort}". Use one of: ${REASONING_EFFORTS.join(", ")}.` ); } return normalized; @@ -398,6 +397,7 @@ async function executeReviewRun(request) { exitStatus: result.status, threadId: result.threadId, turnId: result.turnId, + resolved: result.resolved, payload, rendered, summary: firstMeaningfulLine(result.reviewText, `${reviewName} completed.`), @@ -446,6 +446,7 @@ async function executeReviewRun(request) { exitStatus: result.status, threadId: result.threadId, turnId: result.turnId, + resolved: result.resolved, payload, rendered: renderReviewResult(parsed, { reviewLabel: reviewName, @@ -522,6 +523,7 @@ async function executeTaskRun(request) { exitStatus: result.status, threadId: result.threadId, turnId: result.turnId, + resolved: result.resolved, payload, rendered, summary: firstMeaningfulLine(rawOutput, firstMeaningfulLine(failureMessage, `${taskMetadata.title} finished.`)), diff --git a/plugins/codex/scripts/lib/app-server-protocol.d.ts b/plugins/codex/scripts/lib/app-server-protocol.d.ts index f61a4588..e66d19a6 100644 --- a/plugins/codex/scripts/lib/app-server-protocol.d.ts +++ b/plugins/codex/scripts/lib/app-server-protocol.d.ts @@ -8,6 +8,8 @@ import type { import type { ExternalAgentConfigImportParams, ExternalAgentConfigImportResponse, + ConfigReadParams, + ConfigReadResponse, ReviewStartParams, ReviewStartResponse, ReviewTarget, @@ -15,6 +17,8 @@ import type { ThreadItem, ThreadListParams, ThreadListResponse, + ModelListParams, + ModelListResponse, ThreadResumeParams as RawThreadResumeParams, ThreadResumeResponse, ThreadSetNameParams, @@ -58,11 +62,13 @@ export interface CodexAppServerClientOptions { export interface AppServerMethodMap { initialize: { params: InitializeParams; result: InitializeResponse }; + "config/read": { params: ConfigReadParams; result: ConfigReadResponse }; "externalAgentConfig/import": { params: ExternalAgentConfigImportParams; result: ExternalAgentConfigImportResponse }; "thread/start": { params: ThreadStartParams; result: ThreadStartResponse }; "thread/resume": { params: ThreadResumeParams; result: ThreadResumeResponse }; "thread/name/set": { params: ThreadSetNameParams; result: ThreadSetNameResponse }; "thread/list": { params: ThreadListParams; result: ThreadListResponse }; + "model/list": { params: ModelListParams; result: ModelListResponse }; "review/start": { params: ReviewStartParams; result: ReviewStartResponse }; "turn/start": { params: TurnStartParams; result: TurnStartResponse }; "turn/interrupt": { params: TurnInterruptParams; result: TurnInterruptResponse }; diff --git a/plugins/codex/scripts/lib/app-server.mjs b/plugins/codex/scripts/lib/app-server.mjs index e0349d63..af88bc15 100644 --- a/plugins/codex/scripts/lib/app-server.mjs +++ b/plugins/codex/scripts/lib/app-server.mjs @@ -13,7 +13,7 @@ import process from "node:process"; import { spawn } from "node:child_process"; import readline from "node:readline"; import { parseBrokerEndpoint } from "./broker-endpoint.mjs"; -import { ensureBrokerSession, loadBrokerSession } from "./broker-lifecycle.mjs"; +import { ensureBrokerSession, loadReusableBrokerSession } from "./broker-lifecycle.mjs"; import { terminateProcessTree } from "./process.mjs"; const PLUGIN_MANIFEST_URL = new URL("../../.claude-plugin/plugin.json", import.meta.url); @@ -187,14 +187,7 @@ class SpawnedCodexAppServerClient extends AppServerClientBase { } async initialize() { - const args = ["app-server"]; - if (this.options.model) { - args.push("-c", `model=${JSON.stringify(String(this.options.model))}`); - } - if (this.options.effort) { - args.push("-c", `model_reasoning_effort=${JSON.stringify(String(this.options.effort))}`); - } - this.proc = spawn("codex", args, { + this.proc = spawn("codex", ["app-server"], { cwd: this.cwd, env: this.options.env ?? process.env, stdio: ["pipe", "pipe", "pipe"], @@ -342,13 +335,17 @@ class BrokerCodexAppServerClient extends AppServerClientBase { export class CodexAppServerClient { static async connect(cwd, options = {}) { let brokerEndpoint = null; + const brokerOptions = { + env: options.env, + allowBusyStaleBroker: options.allowBusyStaleBroker + }; if (!options.disableBroker) { brokerEndpoint = options.brokerEndpoint ?? options.env?.[BROKER_ENDPOINT_ENV] ?? process.env[BROKER_ENDPOINT_ENV] ?? null; if (!brokerEndpoint && options.reuseExistingBroker) { - brokerEndpoint = loadBrokerSession(cwd)?.endpoint ?? null; + brokerEndpoint = (await loadReusableBrokerSession(cwd, brokerOptions))?.endpoint ?? null; } if (!brokerEndpoint && !options.reuseExistingBroker) { - const brokerSession = await ensureBrokerSession(cwd, { env: options.env, model: options.model, effort: options.effort }); + const brokerSession = await ensureBrokerSession(cwd, brokerOptions); brokerEndpoint = brokerSession?.endpoint ?? null; } } diff --git a/plugins/codex/scripts/lib/broker-lifecycle.mjs b/plugins/codex/scripts/lib/broker-lifecycle.mjs index 7237064c..003110f7 100644 --- a/plugins/codex/scripts/lib/broker-lifecycle.mjs +++ b/plugins/codex/scripts/lib/broker-lifecycle.mjs @@ -6,12 +6,31 @@ import process from "node:process"; import { spawn } from "node:child_process"; import { fileURLToPath } from "node:url"; import { createBrokerEndpoint, parseBrokerEndpoint } from "./broker-endpoint.mjs"; +import { withBrokerLock } from "./broker-lock.mjs"; +import { probeBroker } from "./broker-probe.mjs"; +import { binaryAvailable, terminateProcessTree } from "./process.mjs"; import { resolveStateDir } from "./state.mjs"; -import { terminateProcessTree } from "./process.mjs"; export const PID_FILE_ENV = "CODEX_COMPANION_APP_SERVER_PID_FILE"; export const LOG_FILE_ENV = "CODEX_COMPANION_APP_SERVER_LOG_FILE"; const BROKER_STATE_FILE = "broker.json"; +const PLUGIN_MANIFEST_URL = new URL("../../.claude-plugin/plugin.json", import.meta.url); +const PLUGIN_MANIFEST = JSON.parse(fs.readFileSync(PLUGIN_MANIFEST_URL, "utf8")); + +export function resolveBrokerRuntimeIdentity(cwd, env = process.env) { + const codex = binaryAvailable("codex", ["--version"], { cwd, env }); + return { + pluginVersion: PLUGIN_MANIFEST.version ?? "0.0.0", + codexVersion: codex.available ? codex.detail : null + }; +} + +export function isBrokerRuntimeCurrent(session, runtime) { + return ( + session?.runtime?.pluginVersion === runtime.pluginVersion && + session?.runtime?.codexVersion === runtime.codexVersion + ); +} export function createBrokerSessionDir(prefix = "cxc-") { return fs.mkdtempSync(path.join(os.tmpdir(), prefix)); @@ -42,50 +61,60 @@ export async function waitForBrokerEndpoint(endpoint, timeoutMs = 2000) { } export async function sendBrokerShutdown(endpoint, timeoutMs = 2000) { - await new Promise((resolve) => { + return new Promise((resolve) => { let settled = false; - const finish = () => { + const socket = connectToEndpoint(endpoint); + let buffer = ""; + socket.setEncoding("utf8"); + + // Bound the wait so a wedged broker (connect accepted, but no data/close/error + // ever arrives) can't hang the caller indefinitely — e.g. a runtime-identity + // respawn that calls sendBrokerShutdown before teardown would otherwise stall + // every subsequent command. Resolve false (treat as "broker not responsive") + // and let the caller fall through to its teardown/kill path. + const timer = setTimeout(() => { + if (settled) { + return; + } + settled = true; + socket.destroy(); + resolve(false); + }, timeoutMs); + + const finish = (value) => { if (settled) { return; } settled = true; clearTimeout(timer); - resolve(); + resolve(value); }; - const socket = connectToEndpoint(endpoint); - const timer = setTimeout(() => { - // A wedged broker may accept the connection but never respond or close. - // Bound the wait so the caller can fall back to the process-tree kill. - try { - socket.destroy(); - } catch { - // Ignore destroy errors on an already-closed socket. - } - finish(); - }, timeoutMs); - socket.setEncoding("utf8"); + socket.on("connect", () => { socket.write(`${JSON.stringify({ id: 1, method: "broker/shutdown", params: {} })}\n`); }); - socket.on("data", () => { + socket.on("data", (chunk) => { + buffer += chunk; + const newlineIndex = buffer.indexOf("\n"); + if (newlineIndex === -1) { + return; + } + const line = buffer.slice(0, newlineIndex); socket.end(); - finish(); + try { + finish(!JSON.parse(line.trim()).error); + } catch { + finish(false); + } }); - socket.on("error", finish); - socket.on("close", finish); + socket.on("error", () => finish(false)); + socket.on("close", () => finish(false)); }); } -export function spawnBrokerProcess({ scriptPath, cwd, endpoint, pidFile, logFile, model, effort, env = process.env }) { +export function spawnBrokerProcess({ scriptPath, cwd, endpoint, pidFile, logFile, env = process.env }) { const logFd = fs.openSync(logFile, "a"); - const args = [scriptPath, "serve", "--endpoint", endpoint, "--cwd", cwd, "--pid-file", pidFile]; - if (model) { - args.push("--model", String(model)); - } - if (effort) { - args.push("--effort", String(effort)); - } - const child = spawn(process.execPath, args, { + const child = spawn(process.execPath, [scriptPath, "serve", "--endpoint", endpoint, "--cwd", cwd, "--pid-file", pidFile], { cwd, env, detached: true, @@ -137,96 +166,114 @@ async function isBrokerEndpointReady(endpoint) { } } -export async function ensureBrokerSession(cwd, options = {}) { +function teardownExistingBroker(cwd, existing, killProcess) { + teardownBrokerSession({ + endpoint: existing.endpoint ?? null, + pidFile: existing.pidFile ?? null, + logFile: existing.logFile ?? null, + sessionDir: existing.sessionDir ?? null, + pid: existing.pid ?? null, + killProcess + }); + clearBrokerSession(cwd); +} + +async function loadReusableBrokerSessionUnlocked(cwd, options = {}) { const existing = loadBrokerSession(cwd); - const existingReady = existing && (await isBrokerEndpointReady(existing.endpoint)); - if (existing && existingReady) { - // Reuse the warm broker only when it was spawned with the same model/effort - // override the caller is requesting now. model/effort are baked in at spawn - // (they reach `codex app-server` via `-c` argv), so a differing override - // cannot take effect on an already-running broker — respawn instead. - const sameModel = (existing.model ?? null) === (options.model ?? null); - const sameEffort = (existing.effort ?? null) === (options.effort ?? null); - if (sameModel && sameEffort) { - return existing; - } + const runtime = resolveBrokerRuntimeIdentity(cwd, options.env); + if ( + existing && + isBrokerRuntimeCurrent(existing, runtime) && + (await isBrokerEndpointReady(existing.endpoint)) + ) { + return existing; } if (existing) { - // The existing broker may still be live (this path now also runs on the - // override-differ respawn branch above). Ask it to shut down gracefully so - // it closes its codex app-server child, then tear down its files and - // belt-and-suspenders tree-kill in case the RPC doesn't land. Without this - // the detached+unref'd broker process (and its app-server child) would be - // orphaned indefinitely — only its socket file got unlinked before. - if (existing.endpoint) { - try { - await sendBrokerShutdown(existing.endpoint); - } catch { - // Broker may already be gone; the tree-kill below is the fallback. + // Only trust the recorded pid for tree-kill when the endpoint probe confirmed + // the broker was actually live. A stale session whose endpoint is not ready + // likely points at a dead broker whose pid the OS may have recycled into an + // unrelated process — tree-killing there risks killing the wrong process, so + // just drop the files and let any survivor exit on its own. + const existingReady = await isBrokerEndpointReady(existing.endpoint); + if (existingReady) { + const brokerStatus = await probeBroker(existing.endpoint, cwd); + if (brokerStatus === "busy" && options.allowBusyStaleBroker) { + return existing; + } + if (brokerStatus !== "idle") { + options.deferBrokerReplacement = true; + return null; + } + if (!(await sendBrokerShutdown(existing.endpoint))) { + options.deferBrokerReplacement = true; + return null; } } - // Only tree-kill by pid when we confirmed the broker endpoint was live, so - // the pid still belongs to our broker. A stale session whose endpoint is - // not ready likely points at a dead broker whose pid the OS may have - // reused for an unrelated process — in that case just drop the files and - // let any survivor exit on its own. - const killProcess = existingReady ? (options.killProcess ?? terminateProcessTree) : (options.killProcess ?? null); - teardownBrokerSession({ - endpoint: existing.endpoint ?? null, - pidFile: existing.pidFile ?? null, - logFile: existing.logFile ?? null, - sessionDir: existing.sessionDir ?? null, - pid: existing.pid ?? null, - killProcess - }); - clearBrokerSession(cwd); + const killProcess = existingReady + ? (options.killProcess ?? terminateProcessTree) + : (options.killProcess ?? null); + teardownExistingBroker(cwd, existing, killProcess); } - const sessionDir = createBrokerSessionDir(); - const endpointFactory = options.createBrokerEndpoint ?? createBrokerEndpoint; - const endpoint = endpointFactory(sessionDir, options.platform); - const pidFile = path.join(sessionDir, "broker.pid"); - const logFile = path.join(sessionDir, "broker.log"); - const scriptPath = - options.scriptPath ?? - fileURLToPath(new URL("../app-server-broker.mjs", import.meta.url)); - - const child = spawnBrokerProcess({ - scriptPath, - cwd, - endpoint, - pidFile, - logFile, - model: options.model, - effort: options.effort, - env: options.env ?? process.env - }); + return null; +} + +export async function loadReusableBrokerSession(cwd, options = {}) { + return withBrokerLock(cwd, options, () => loadReusableBrokerSessionUnlocked(cwd, options)); +} + +export async function ensureBrokerSession(cwd, options = {}) { + return withBrokerLock(cwd, options, async () => { + const existing = await loadReusableBrokerSessionUnlocked(cwd, options); + if (existing || options.deferBrokerReplacement) { + return existing; + } + + const runtime = resolveBrokerRuntimeIdentity(cwd, options.env); - const ready = await waitForBrokerEndpoint(endpoint, options.timeoutMs ?? 2000); - if (!ready) { - teardownBrokerSession({ + const sessionDir = createBrokerSessionDir(); + const endpointFactory = options.createBrokerEndpoint ?? createBrokerEndpoint; + const endpoint = endpointFactory(sessionDir, options.platform); + const pidFile = path.join(sessionDir, "broker.pid"); + const logFile = path.join(sessionDir, "broker.log"); + const scriptPath = + options.scriptPath ?? + fileURLToPath(new URL("../app-server-broker.mjs", import.meta.url)); + + const child = spawnBrokerProcess({ + scriptPath, + cwd, endpoint, pidFile, logFile, - sessionDir, - pid: child.pid ?? null, - killProcess: options.killProcess ?? null + env: options.env ?? process.env }); - return null; - } - const session = { - endpoint, - pidFile, - logFile, - sessionDir, - pid: child.pid ?? null, - model: options.model ?? null, - effort: options.effort ?? null - }; - saveBrokerSession(cwd, session); - return session; + const ready = await waitForBrokerEndpoint(endpoint, options.timeoutMs ?? 2000); + if (!ready) { + teardownBrokerSession({ + endpoint, + pidFile, + logFile, + sessionDir, + pid: child.pid ?? null, + killProcess: options.killProcess ?? terminateProcessTree + }); + return null; + } + + const session = { + endpoint, + pidFile, + logFile, + sessionDir, + pid: child.pid ?? null, + runtime + }; + saveBrokerSession(cwd, session); + return session; + }); } export function teardownBrokerSession({ endpoint = null, pidFile, logFile, sessionDir = null, pid = null, killProcess = null }) { diff --git a/plugins/codex/scripts/lib/broker-lock.mjs b/plugins/codex/scripts/lib/broker-lock.mjs new file mode 100644 index 00000000..842c2a84 --- /dev/null +++ b/plugins/codex/scripts/lib/broker-lock.mjs @@ -0,0 +1,100 @@ +import fs from "node:fs"; +import path from "node:path"; +import process from "node:process"; + +import { resolveStateDir } from "./state.mjs"; + +const BROKER_LOCK_FILE = "broker.lock"; +const BROKER_LOCK_TIMEOUT_MS = 5000; +const BROKER_LOCK_STALE_MS = 30000; + +function sleep(ms) { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +function brokerLockPath(cwd) { + const stateDir = resolveStateDir(cwd); + fs.mkdirSync(stateDir, { recursive: true }); + return path.join(stateDir, BROKER_LOCK_FILE); +} + +function isProcessAlive(pid) { + try { + process.kill(pid, 0); + return true; + } catch (error) { + return error?.code !== "ESRCH"; + } +} + +function removeAbandonedBrokerLock(lockFile, staleMs) { + try { + const stat = fs.statSync(lockFile); + const ownerPid = Number.parseInt(fs.readFileSync(lockFile, "utf8").split(":", 1)[0], 10); + if (Number.isFinite(ownerPid) && isProcessAlive(ownerPid)) { + return false; + } + if (!Number.isFinite(ownerPid) && Date.now() - stat.mtimeMs <= staleMs) { + return false; + } + fs.unlinkSync(lockFile); + return true; + } catch (error) { + if (error?.code === "ENOENT") { + return true; + } + throw error; + } +} + +async function acquireBrokerLock(cwd, options = {}) { + const lockFile = brokerLockPath(cwd); + const timeoutMs = options.lockTimeoutMs ?? BROKER_LOCK_TIMEOUT_MS; + const staleMs = options.lockStaleMs ?? BROKER_LOCK_STALE_MS; + const deadline = Date.now() + timeoutMs; + const token = `${process.pid}:${Date.now()}:${Math.random()}`; + + while (true) { + try { + const fd = fs.openSync(lockFile, "wx"); + fs.writeFileSync(fd, token, "utf8"); + return { fd, lockFile, token }; + } catch (error) { + if (error?.code !== "EEXIST") { + throw error; + } + if (removeAbandonedBrokerLock(lockFile, staleMs)) { + continue; + } + if (Date.now() >= deadline) { + throw new Error(`Timed out waiting for the shared Codex broker lock at ${lockFile}.`); + } + await sleep(25); + } + } +} + +function releaseBrokerLock(lock) { + try { + fs.closeSync(lock.fd); + } finally { + try { + if (fs.readFileSync(lock.lockFile, "utf8") === lock.token) { + fs.unlinkSync(lock.lockFile); + } + } catch (error) { + if (error?.code !== "ENOENT") { + throw error; + } + } + } +} + +export async function withBrokerLock(cwd, options, action) { + const lock = await acquireBrokerLock(cwd, options); + try { + return await action(); + } finally { + releaseBrokerLock(lock); + } +} diff --git a/plugins/codex/scripts/lib/broker-probe.mjs b/plugins/codex/scripts/lib/broker-probe.mjs new file mode 100644 index 00000000..4cebb26e --- /dev/null +++ b/plugins/codex/scripts/lib/broker-probe.mjs @@ -0,0 +1,91 @@ +import net from "node:net"; +import { parseBrokerEndpoint } from "./broker-endpoint.mjs"; + +const BROKER_BUSY_RPC_CODE = -32001; +const BROKER_PROBE_TIMEOUT_MS = 500; + +function connectToEndpoint(endpoint) { + const target = parseBrokerEndpoint(endpoint); + return net.createConnection({ path: target.path }); +} + +function sendJsonLine(socket, message) { + socket.write(`${JSON.stringify(message)}\n`); +} + +function handleProbeMessage(message, phase, socket, cwd) { + if (message.error) { + return { status: message.error.code === BROKER_BUSY_RPC_CODE ? "busy" : "unknown" }; + } + if (phase === "initialize" && message.id === 1) { + sendJsonLine(socket, { method: "initialized", params: {} }); + sendJsonLine(socket, { id: 2, method: "thread/list", params: { cwd, limit: 1 } }); + return { phase: "probe" }; + } + if (phase === "probe" && message.id === 2) { + return { status: "idle" }; + } + return {}; +} + +function consumeProbeData(buffer, chunk, phase, socket, cwd, finish) { + buffer += chunk; + let newlineIndex = buffer.indexOf("\n"); + while (newlineIndex !== -1) { + const line = buffer.slice(0, newlineIndex); + buffer = buffer.slice(newlineIndex + 1); + newlineIndex = buffer.indexOf("\n"); + if (!line.trim()) { + continue; + } + let message; + try { + message = JSON.parse(line); + } catch { + finish("unknown"); + return { buffer, phase }; + } + const next = handleProbeMessage(message, phase, socket, cwd); + if (next.status) { + finish(next.status); + return { buffer, phase }; + } + phase = next.phase ?? phase; + } + return { buffer, phase }; +} + +export async function probeBroker(endpoint, cwd, timeoutMs = BROKER_PROBE_TIMEOUT_MS) { + return new Promise((resolve) => { + const socket = connectToEndpoint(endpoint); + let buffer = ""; + let phase = "initialize"; + let settled = false; + const timer = setTimeout(() => finish("unknown"), timeoutMs); + + function finish(status) { + if (settled) { + return; + } + settled = true; + clearTimeout(timer); + socket.destroy(); + resolve(status); + } + + socket.setEncoding("utf8"); + socket.on("connect", () => sendJsonLine(socket, { + id: 1, + method: "initialize", + params: { + clientInfo: { title: "Codex Plugin Broker Probe", name: "Claude Code", version: "0.0.0" }, + capabilities: { experimentalApi: false, requestAttestation: false } + } + })); + socket.on("data", (chunk) => { + ({ buffer, phase } = consumeProbeData(buffer, chunk, phase, socket, cwd, finish)); + }); + socket.on("error", () => finish("unknown")); + socket.on("close", () => finish("unknown")); + }); +} diff --git a/plugins/codex/scripts/lib/codex.mjs b/plugins/codex/scripts/lib/codex.mjs index f30fb73c..f22a05df 100644 --- a/plugins/codex/scripts/lib/codex.mjs +++ b/plugins/codex/scripts/lib/codex.mjs @@ -43,11 +43,10 @@ import { readJsonFile } from "./fs.mjs"; import { BROKER_BUSY_RPC_CODE, BROKER_ENDPOINT_ENV, CodexAppServerClient } from "./app-server.mjs"; import { loadBrokerSession } from "./broker-lifecycle.mjs"; import { binaryAvailable } from "./process.mjs"; +import { validateExplicitReasoningSelection, validateReasoningSelection } from "./model-catalog.mjs"; +import { TASK_THREAD_PREFIX } from "./task-thread.mjs"; const SERVICE_NAME = "claude_code_codex_plugin"; -const TASK_THREAD_PREFIX = "Codex Companion Task"; -const DEFAULT_CONTINUE_PROMPT = - "Continue from the current thread state. Pick the next highest-value step and follow through until the task is resolved."; const EXTERNAL_AGENT_IMPORT_COMPLETED = "externalAgentConfig/import/completed"; const EXTERNAL_AGENT_IMPORT_TIMEOUT_MS = 2 * 60 * 1000; @@ -66,6 +65,7 @@ function buildThreadParams(cwd, options = {}) { model: options.model ?? null, approvalPolicy: options.approvalPolicy ?? "never", sandbox: options.sandbox ?? "read-only", + config: options.effort ? { model_reasoning_effort: options.effort } : null, serviceName: SERVICE_NAME, ephemeral: options.ephemeral ?? true }; @@ -104,11 +104,6 @@ function looksLikeVerificationCommand(command) { ); } -function buildTaskThreadName(prompt) { - const excerpt = shorten(prompt, 56); - return excerpt ? `${TASK_THREAD_PREFIX}: ${excerpt}` : TASK_THREAD_PREFIX; -} - function extractThreadId(message) { return message?.params?.threadId ?? null; } @@ -263,6 +258,8 @@ function describeStartedItem(state, item) { } case "webSearch": return { message: `Searching: ${shorten(item.query, 96)}`, phase: "investigating" }; + case "reasoning": + return { message: "Thinking.", phase: null }; default: return null; } @@ -979,7 +976,10 @@ export async function interruptAppServerTurn(cwd, { threadId, turnId }) { let client = null; try { - client = await CodexAppServerClient.connect(cwd, { reuseExistingBroker: true }); + client = await CodexAppServerClient.connect(cwd, { + reuseExistingBroker: true, + allowBusyStaleBroker: true + }); await client.request("turn/interrupt", { threadId, turnId }); return { attempted: true, @@ -1006,16 +1006,30 @@ export async function runAppServerReview(cwd, options = {}) { } return withAppServer(cwd, async (client) => { + await validateExplicitReasoningSelection(client, cwd, options); emitProgress(options.onProgress, "Starting Codex review thread.", "starting"); - const thread = await startThread(client, cwd, { + const response = await startThread(client, cwd, { model: options.model, + effort: options.effort, sandbox: "read-only", ephemeral: true, threadName: options.threadName }); - const sourceThreadId = thread.thread.id; + const sourceThreadId = response.thread.id; + const resolved = { + model: response.model, + modelProvider: response.modelProvider, + reasoningEffort: response.reasoningEffort, + sandbox: response.sandbox + }; + await validateReasoningSelection(client, { + model: options.model ?? response.model, + effort: options.effort ?? response.reasoningEffort, + modelProvider: response.modelProvider + }); emitProgress(options.onProgress, `Thread ready (${sourceThreadId}).`, "starting", { - threadId: sourceThreadId + threadId: sourceThreadId, + resolved }); const delivery = options.delivery ?? "inline"; @@ -1046,6 +1060,7 @@ export async function runAppServerReview(cwd, options = {}) { threadId: turnState.threadId, sourceThreadId, turnId: turnState.turnId, + resolved, reviewText: turnState.reviewText, reasoningSummary: turnState.reasoningSummary, turn: turnState.finalTurn, @@ -1099,29 +1114,51 @@ export async function runAppServerTurn(cwd, options = {}) { } return withAppServer(cwd, async (client) => { - let threadId; + let response; + let threadSelection; + + if (!options.resumeThreadId) { + await validateExplicitReasoningSelection(client, cwd, options, { + includeInherited: options.persistThread === true + }); + } if (options.resumeThreadId) { emitProgress(options.onProgress, `Resuming thread ${options.resumeThreadId}.`, "starting"); - const response = await resumeThread(client, options.resumeThreadId, cwd, { + response = await resumeThread(client, options.resumeThreadId, cwd, { model: options.model, sandbox: options.sandbox, ephemeral: false }); - threadId = response.thread.id; + threadSelection = response; } else { emitProgress(options.onProgress, "Starting Codex task thread.", "starting"); - const response = await startThread(client, cwd, { + response = await startThread(client, cwd, { model: options.model, sandbox: options.sandbox, ephemeral: options.persistThread ? false : true, threadName: options.persistThread ? options.threadName : options.threadName ?? null }); - threadId = response.thread.id; + threadSelection = response; } + const threadId = response.thread.id; + let resolved = { + model: response.model, + modelProvider: response.modelProvider, + reasoningEffort: response.reasoningEffort, + sandbox: response.sandbox + }; + await validateReasoningSelection(client, { + model: options.model ?? threadSelection.model, + effort: options.effort ?? threadSelection.reasoningEffort, + modelProvider: threadSelection.modelProvider + }); + + emitProgress(options.onProgress, `Thread ready (${threadId}).`, "starting", { - threadId + threadId, + resolved }); const prompt = options.prompt?.trim() || options.defaultPrompt || ""; @@ -1140,13 +1177,23 @@ export async function runAppServerTurn(cwd, options = {}) { effort: options.effort ?? null, outputSchema: options.outputSchema ?? null }), - { onProgress: options.onProgress } + { + onProgress: options.onProgress, + onResponse() { + if (!options.effort) { + return; + } + resolved = { ...resolved, reasoningEffort: options.effort }; + options.onProgress?.({ message: "", resolved }); + } + } ); return { status: buildResultStatus(turnState), threadId, turnId: turnState.turnId, + resolved, finalMessage: turnState.lastAgentMessage, reasoningSummary: turnState.reasoningSummary, turn: turnState.finalTurn, @@ -1180,40 +1227,3 @@ export async function findLatestTaskThread(cwd) { ); }); } - -export function buildPersistentTaskThreadName(prompt) { - return buildTaskThreadName(prompt); -} - -export function parseStructuredOutput(rawOutput, fallback = {}) { - if (!rawOutput) { - return { - parsed: null, - parseError: fallback.failureMessage ?? "Codex did not return a final structured message.", - rawOutput: rawOutput ?? "", - ...fallback - }; - } - - try { - return { - parsed: JSON.parse(rawOutput), - parseError: null, - rawOutput, - ...fallback - }; - } catch (error) { - return { - parsed: null, - parseError: error.message, - rawOutput, - ...fallback - }; - } -} - -export function readOutputSchema(schemaPath) { - return readJsonFile(schemaPath); -} - -export { DEFAULT_CONTINUE_PROMPT, TASK_THREAD_PREFIX }; diff --git a/plugins/codex/scripts/lib/model-catalog.mjs b/plugins/codex/scripts/lib/model-catalog.mjs new file mode 100644 index 00000000..441e52ee --- /dev/null +++ b/plugins/codex/scripts/lib/model-catalog.mjs @@ -0,0 +1,92 @@ +function isUnsupportedMethodError(error) { + if (error?.rpcCode === -32601) { + return true; + } + return /unknown (variant|method)|unsupported method|method not found/i.test( + String(error?.message ?? error ?? "") + ); +} + +async function readModelCatalog(client) { + const models = []; + let cursor = null; + + try { + do { + const response = await client.request("model/list", { + cursor, + limit: 100, + includeHidden: true + }); + models.push(...(response.data ?? [])); + cursor = response.nextCursor ?? null; + } while (cursor); + } catch (error) { + if (isUnsupportedMethodError(error)) { + return null; + } + throw error; + } + + return models; +} + +function supportedEfforts(model) { + return (model.supportedReasoningEfforts ?? []) + .map((option) => String(option.reasoningEffort ?? "").trim().toLowerCase()) + .filter(Boolean); +} + +export async function validateReasoningSelection(client, selection = {}) { + const modelName = String(selection.model ?? "").trim(); + const effort = String(selection.effort ?? "").trim().toLowerCase(); + const provider = String(selection.modelProvider ?? "").trim().toLowerCase(); + if (!effort || provider !== "openai") { + return; + } + + const catalog = await readModelCatalog(client); + if (!catalog) { + return; + } + + const model = modelName + ? catalog.find((candidate) => candidate.model === modelName || candidate.id === modelName) + : catalog.find((candidate) => candidate.isDefault === true); + if (!model) { + return; + } + const selectedModelName = model.model ?? model.id ?? modelName; + + const efforts = supportedEfforts(model); + if (efforts.length === 0 || efforts.includes(effort)) { + return; + } + + throw new Error( + `Reasoning effort "${effort}" is not supported by model "${selectedModelName}". Supported efforts: ${efforts.join(", ")}.` + ); +} + +export async function validateExplicitReasoningSelection(client, cwd, selection = {}, options = {}) { + if (!selection.model && !selection.effort && !options.includeInherited) { + return; + } + + let config; + try { + const response = await client.request("config/read", { cwd, includeLayers: false }); + config = response.config ?? {}; + } catch (error) { + if (isUnsupportedMethodError(error)) { + return; + } + throw error; + } + + await validateReasoningSelection(client, { + model: selection.model ?? config.model, + effort: selection.effort ?? config.model_reasoning_effort, + modelProvider: config.model_provider ?? "openai" + }); +} diff --git a/plugins/codex/scripts/lib/structured-output.mjs b/plugins/codex/scripts/lib/structured-output.mjs new file mode 100644 index 00000000..317c915b --- /dev/null +++ b/plugins/codex/scripts/lib/structured-output.mjs @@ -0,0 +1,32 @@ +import { readJsonFile } from "./fs.mjs"; + +export function parseStructuredOutput(rawOutput, fallback = {}) { + if (!rawOutput) { + return { + parsed: null, + parseError: fallback.failureMessage ?? "Codex did not return a final structured message.", + rawOutput: rawOutput ?? "", + ...fallback + }; + } + + try { + return { + parsed: JSON.parse(rawOutput), + parseError: null, + rawOutput, + ...fallback + }; + } catch (error) { + return { + parsed: null, + parseError: error.message, + rawOutput, + ...fallback + }; + } +} + +export function readOutputSchema(schemaPath) { + return readJsonFile(schemaPath); +} diff --git a/plugins/codex/scripts/lib/task-thread.mjs b/plugins/codex/scripts/lib/task-thread.mjs new file mode 100644 index 00000000..f0f01e09 --- /dev/null +++ b/plugins/codex/scripts/lib/task-thread.mjs @@ -0,0 +1,16 @@ +export const TASK_THREAD_PREFIX = "Codex Companion Task"; +export const DEFAULT_CONTINUE_PROMPT = + "Continue from the current thread state. Pick the next highest-value step and follow through until the task is resolved."; + +function shorten(text, limit) { + const normalized = String(text ?? "").trim().replace(/\s+/g, " "); + if (!normalized || normalized.length <= limit) { + return normalized; + } + return `${normalized.slice(0, limit - 3)}...`; +} + +export function buildPersistentTaskThreadName(prompt) { + const excerpt = shorten(prompt, 56); + return excerpt ? `${TASK_THREAD_PREFIX}: ${excerpt}` : TASK_THREAD_PREFIX; +} diff --git a/plugins/codex/scripts/lib/tracked-jobs.mjs b/plugins/codex/scripts/lib/tracked-jobs.mjs index 90286901..0c7d56d3 100644 --- a/plugins/codex/scripts/lib/tracked-jobs.mjs +++ b/plugins/codex/scripts/lib/tracked-jobs.mjs @@ -16,6 +16,7 @@ function normalizeProgressEvent(value) { phase: typeof value.phase === "string" && value.phase.trim() ? value.phase.trim() : null, threadId: typeof value.threadId === "string" && value.threadId.trim() ? value.threadId.trim() : null, turnId: typeof value.turnId === "string" && value.turnId.trim() ? value.turnId.trim() : null, + resolved: value.resolved && typeof value.resolved === "object" && !Array.isArray(value.resolved) ? value.resolved : null, stderrMessage: value.stderrMessage == null ? null : String(value.stderrMessage).trim(), logTitle: typeof value.logTitle === "string" && value.logTitle.trim() ? value.logTitle.trim() : null, logBody: value.logBody == null ? null : String(value.logBody).trimEnd() @@ -27,6 +28,7 @@ function normalizeProgressEvent(value) { phase: null, threadId: null, turnId: null, + resolved: null, stderrMessage: String(value ?? "").trim(), logTitle: null, logBody: null @@ -71,6 +73,7 @@ export function createJobProgressUpdater(workspaceRoot, jobId) { let lastPhase = null; let lastThreadId = null; let lastTurnId = null; + let lastResolved = null; return (event) => { const normalized = normalizeProgressEvent(event); @@ -95,6 +98,12 @@ export function createJobProgressUpdater(workspaceRoot, jobId) { changed = true; } + if (normalized.resolved && normalized.resolved !== lastResolved) { + lastResolved = normalized.resolved; + patch.resolved = normalized.resolved; + changed = true; + } + if (!changed) { return; } @@ -160,6 +169,7 @@ export async function runTrackedJob(job, runner, options = {}) { status: completionStatus, threadId: execution.threadId ?? null, turnId: execution.turnId ?? null, + resolved: execution.resolved ?? null, pid: null, phase: completionStatus === "completed" ? "done" : "failed", completedAt, @@ -171,6 +181,7 @@ export async function runTrackedJob(job, runner, options = {}) { status: completionStatus, threadId: execution.threadId ?? null, turnId: execution.turnId ?? null, + resolved: execution.resolved ?? null, summary: execution.summary, phase: completionStatus === "completed" ? "done" : "failed", pid: null, diff --git a/plugins/codex/skills/codex-cli-runtime/SKILL.md b/plugins/codex/skills/codex-cli-runtime/SKILL.md index 0e91bfb5..a3eabab8 100644 --- a/plugins/codex/skills/codex-cli-runtime/SKILL.md +++ b/plugins/codex/skills/codex-cli-runtime/SKILL.md @@ -32,7 +32,8 @@ Command selection: - If the forwarded request includes `--fresh`, strip that token from the task text and do not add `--resume-last`. - `--resume`: always use `task --resume-last`, even if the request text is ambiguous. - `--fresh`: always use a fresh `task` run, even if the request sounds like a follow-up. -- `--effort`: accepted values are `none`, `minimal`, `low`, `medium`, `high`, `xhigh`. +- `--effort`: accepted values are `none`, `minimal`, `low`, `medium`, `high`, `xhigh`, `max`, `ultra`. +- The runtime checks known OpenAI models against the current Codex model catalog. For example, `gpt-5.6-luna` supports up to `max`, while `gpt-5.6-sol` and `gpt-5.6-terra` also support `ultra` in Codex 0.144.x. - `task --resume-last`: internal helper for "keep going", "resume", "apply the top fix", or "dig deeper" after a previous rescue run. Safety rules: diff --git a/tests/broker-idle.test.mjs b/tests/broker-idle.test.mjs new file mode 100644 index 00000000..6f3860d4 --- /dev/null +++ b/tests/broker-idle.test.mjs @@ -0,0 +1,126 @@ +import fs from "node:fs"; +import net from "node:net"; +import path from "node:path"; +import test from "node:test"; +import assert from "node:assert/strict"; +import { once } from "node:events"; +import { spawn } from "node:child_process"; +import { fileURLToPath } from "node:url"; + +import { buildEnv, installFakeCodex } from "./fake-codex-fixture.mjs"; +import { makeTempDir } from "./helpers.mjs"; +import { sendBrokerShutdown, waitForBrokerEndpoint } from "../plugins/codex/scripts/lib/broker-lifecycle.mjs"; + +const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); +const BROKER_SCRIPT = path.join(ROOT, "plugins", "codex", "scripts", "app-server-broker.mjs"); +const IDLE_TIMEOUT_ENV = "CODEX_COMPANION_BROKER_IDLE_TIMEOUT_MS"; + +function spawnTestBroker({ idleTimeoutMs }) { + const workspace = makeTempDir(); + const binDir = makeTempDir(); + const sessionDir = makeTempDir("codex-plugin-broker-"); + const socketPath = path.join(sessionDir, "broker.sock"); + const endpoint = `unix:${socketPath}`; + const pidFile = path.join(sessionDir, "broker.pid"); + installFakeCodex(binDir); + + const child = spawn( + process.execPath, + [BROKER_SCRIPT, "serve", "--endpoint", endpoint, "--cwd", workspace, "--pid-file", pidFile], + { + cwd: workspace, + env: { + ...buildEnv(binDir), + [IDLE_TIMEOUT_ENV]: String(idleTimeoutMs) + }, + stdio: ["ignore", "pipe", "pipe"] + } + ); + + return { child, endpoint, pidFile, socketPath }; +} + +async function waitForExit(child, timeoutMs = 3000) { + if (child.exitCode != null || child.signalCode != null) { + return; + } + await Promise.race([ + once(child, "exit"), + new Promise((_, reject) => setTimeout(() => reject(new Error("Timed out waiting for broker to exit.")), timeoutMs)) + ]); +} + +function terminateBroker(child) { + if (child.exitCode == null && child.signalCode == null) { + child.kill("SIGTERM"); + } +} + +test("broker exits and removes runtime files after its last client stays disconnected", async (t) => { + const broker = spawnTestBroker({ idleTimeoutMs: 150 }); + t.after(() => terminateBroker(broker.child)); + + assert.equal(await waitForBrokerEndpoint(broker.endpoint), true); + await waitForExit(broker.child); + + assert.equal(broker.child.exitCode, 0); + assert.equal(fs.existsSync(broker.socketPath), false); + assert.equal(fs.existsSync(broker.pidFile), false); +}); + +test("broker idle shutdown waits until a connected client disconnects", async (t) => { + const broker = spawnTestBroker({ idleTimeoutMs: 150 }); + t.after(() => terminateBroker(broker.child)); + + assert.equal(await waitForBrokerEndpoint(broker.endpoint), true); + const socket = net.createConnection({ path: broker.socketPath }); + await once(socket, "connect"); + + await new Promise((resolve) => setTimeout(resolve, 350)); + assert.equal(broker.child.exitCode, null); + assert.equal(broker.child.signalCode, null); + + socket.end(); + await once(socket, "close"); + await waitForExit(broker.child); + assert.equal(broker.child.exitCode, 0); +}); + +test("broker idle timeout can be disabled", async (t) => { + const broker = spawnTestBroker({ idleTimeoutMs: 0 }); + t.after(() => terminateBroker(broker.child)); + + assert.equal(await waitForBrokerEndpoint(broker.endpoint), true); + await new Promise((resolve) => setTimeout(resolve, 350)); + assert.equal(broker.child.exitCode, null); + assert.equal(broker.child.signalCode, null); + + terminateBroker(broker.child); + await waitForExit(broker.child); +}); + +test("broker shuts down cleanly while new clients race to connect", async (t) => { + const broker = spawnTestBroker({ idleTimeoutMs: 0 }); + t.after(() => terminateBroker(broker.child)); + + assert.equal(await waitForBrokerEndpoint(broker.endpoint), true); + const reconnects = Array.from( + { length: 50 }, + () => + new Promise((resolve) => { + const socket = net.createConnection({ path: broker.socketPath }); + const timer = setTimeout(() => socket.destroy(), 1000); + const finish = () => { + clearTimeout(timer); + resolve(); + }; + socket.on("connect", () => socket.end()); + socket.on("error", finish); + socket.on("close", finish); + }) + ); + + await Promise.all([sendBrokerShutdown(broker.endpoint), ...reconnects]); + await waitForExit(broker.child); + assert.equal(broker.child.exitCode, 0); +}); diff --git a/tests/broker-lifecycle.test.mjs b/tests/broker-lifecycle.test.mjs new file mode 100644 index 00000000..dcbd18b7 --- /dev/null +++ b/tests/broker-lifecycle.test.mjs @@ -0,0 +1,173 @@ +import fs from "node:fs"; +import net from "node:net"; +import path from "node:path"; +import test from "node:test"; +import assert from "node:assert/strict"; +import { spawn } from "node:child_process"; +import { fileURLToPath } from "node:url"; + +import { buildEnv, installFakeCodex } from "./fake-codex-fixture.mjs"; +import { initGitRepo, makeTempDir } from "./helpers.mjs"; +import { withBrokerLock } from "../plugins/codex/scripts/lib/broker-lock.mjs"; +import { + loadBrokerSession, + loadReusableBrokerSession, + sendBrokerShutdown +} from "../plugins/codex/scripts/lib/broker-lifecycle.mjs"; +import { resolveStateDir } from "../plugins/codex/scripts/lib/state.mjs"; + +const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); +const BROKER_LIFECYCLE = path.join(ROOT, "plugins", "codex", "scripts", "lib", "broker-lifecycle.mjs"); + +function startEnsureProcess(cwd, env) { + const source = [ + `import { ensureBrokerSession } from ${JSON.stringify(BROKER_LIFECYCLE)};`, + "const session = await ensureBrokerSession(process.cwd());", + "console.log(JSON.stringify(session));" + ].join("\n"); + const child = spawn(process.execPath, ["--input-type=module", "-e", source], { + cwd, + env, + stdio: ["ignore", "pipe", "pipe"] + }); + let stdout = ""; + let stderr = ""; + child.stdout.setEncoding("utf8").on("data", (chunk) => (stdout += chunk)); + child.stderr.setEncoding("utf8").on("data", (chunk) => (stderr += chunk)); + return new Promise((resolve) => { + child.on("close", (code) => resolve({ code, stdout, stderr })); + }); +} + +async function startProbeBroker(socketPath, { busy, shutdownResponseChunks = ['{"id":1,"result":{}}\n'] }) { + const requests = []; + const server = net.createServer((socket) => { + socket.setEncoding("utf8"); + let buffer = ""; + socket.on("data", (chunk) => { + buffer += chunk; + let newlineIndex = buffer.indexOf("\n"); + while (newlineIndex !== -1) { + const line = buffer.slice(0, newlineIndex); + buffer = buffer.slice(newlineIndex + 1); + newlineIndex = buffer.indexOf("\n"); + if (!line.trim()) { + continue; + } + const message = JSON.parse(line); + requests.push(message.method); + if (message.method === "initialize") { + socket.write('{"id":1,"result":{"userAgent":"probe"}}\n'); + } else if (message.method === "thread/list") { + socket.write( + busy + ? '{"id":2,"error":{"code":-32001,"message":"Shared Codex broker is busy."}}\n' + : '{"id":2,"result":{"data":[],"nextCursor":null}}\n' + ); + } else if (message.method === "broker/shutdown") { + for (const chunk of shutdownResponseChunks) { + socket.write(chunk); + } + } + } + }); + }); + await new Promise((resolve) => server.listen(socketPath, resolve)); + return { + requests, + close: async () => { + await new Promise((resolve) => server.close(resolve)); + fs.rmSync(socketPath, { force: true }); + } + }; +} + +test("concurrent startup creates and records only one shared broker", async () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + initGitRepo(repo); + const env = buildEnv(binDir); + + const [first, second] = await Promise.all([ + startEnsureProcess(repo, env), + startEnsureProcess(repo, env) + ]); + + assert.equal(first.code, 0, first.stderr); + assert.equal(second.code, 0, second.stderr); + assert.equal(JSON.parse(first.stdout).endpoint, JSON.parse(second.stdout).endpoint); + assert.equal(JSON.parse(fs.readFileSync(statePath, "utf8")).appServerStarts, 1); + + const broker = loadBrokerSession(repo); + await sendBrokerShutdown(broker.endpoint); +}); + +test("broker lock recovers immediately when its owner process is gone", async () => { + const repo = makeTempDir(); + const stateDir = resolveStateDir(repo); + fs.mkdirSync(stateDir, { recursive: true }); + fs.writeFileSync(path.join(stateDir, "broker.lock"), "2147483647:0:abandoned", "utf8"); + + const result = await withBrokerLock(repo, { lockTimeoutMs: 100 }, async () => "acquired"); + + assert.equal(result, "acquired"); + assert.equal(fs.existsSync(path.join(stateDir, "broker.lock")), false); +}); + +test("stale reachable brokers are preserved when the broker reports an active turn", async () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const sessionDir = makeTempDir(); + const socketPath = path.join(sessionDir, "broker.sock"); + const probeBroker = await startProbeBroker(socketPath, { busy: true }); + installFakeCodex(binDir, "review-ok", "codex-cli 0.144.0"); + initGitRepo(repo); + + const stateDir = resolveStateDir(repo); + fs.mkdirSync(stateDir, { recursive: true }); + fs.writeFileSync( + path.join(stateDir, "broker.json"), + `${JSON.stringify( + { + endpoint: `unix:${socketPath}`, + pidFile: path.join(sessionDir, "broker.pid"), + logFile: path.join(sessionDir, "broker.log"), + sessionDir, + pid: 999999, + runtime: { pluginVersion: "1.0.6", codexVersion: "codex-cli 0.143.0" } + }, + null, + 2 + )}\n` + ); + + let killed = false; + const options = { + env: buildEnv(binDir), + killProcess: () => { + killed = true; + } + }; + const result = await loadReusableBrokerSession(repo, options); + + assert.equal(result, null); + assert.equal(killed, false); + assert.equal(options.deferBrokerReplacement, true); + assert.equal(probeBroker.requests.includes("broker/shutdown"), false); + assert.equal(fs.existsSync(path.join(stateDir, "broker.json")), true); + await probeBroker.close(); +}); + +test("broker shutdown accepts a response split across socket chunks", async () => { + const sessionDir = makeTempDir(); + const socketPath = path.join(sessionDir, "broker.sock"); + const probeBroker = await startProbeBroker(socketPath, { + busy: false, + shutdownResponseChunks: ['{"id":1,"result":', '{}', '}\n'] + }); + + assert.equal(await sendBrokerShutdown(`unix:${socketPath}`), true); + await probeBroker.close(); +}); diff --git a/tests/commands.test.mjs b/tests/commands.test.mjs index 6075667d..b5abd4a8 100644 --- a/tests/commands.test.mjs +++ b/tests/commands.test.mjs @@ -22,11 +22,12 @@ test("review command uses AskUserQuestion and background Bash while staying revi assert.match(source, /```typescript/); assert.match(source, /review "\$ARGUMENTS"/); assert.match(source, /\[--scope auto\|working-tree\|branch\]/); + assert.match(source, /--model /); + assert.match(source, /--effort /); + assert.match(source, /are not focus text/i); assert.match(source, /run_in_background:\s*true/); assert.match(source, /command:\s*`node "\$\{CLAUDE_PLUGIN_ROOT\}\/scripts\/codex-companion\.mjs" review "\$ARGUMENTS"`/); assert.match(source, /description:\s*"Codex review"/); - assert.match(source, /\[--model \] \[--effort \]/); - assert.match(source, /`--model` and `--effort` are runtime-selection flags/i); assert.match(source, /Do not call `BashOutput`/); assert.match(source, /Return the command stdout verbatim, exactly as-is/i); assert.match(source, /git status --short --untracked-files=all/); @@ -51,12 +52,13 @@ test("adversarial review command uses AskUserQuestion and background Bash while assert.match(source, /```bash/); assert.match(source, /```typescript/); assert.match(source, /adversarial-review "\$ARGUMENTS"/); - assert.match(source, /\[--scope auto\|working-tree\|branch\].*\[focus \.\.\.\]/s); + assert.match(source, /\[--scope auto\|working-tree\|branch\].*\[focus \.\.\.\]/); + assert.match(source, /--model /); + assert.match(source, /--effort /); + assert.match(source, /must not become part of the focus text/i); assert.match(source, /run_in_background:\s*true/); assert.match(source, /command:\s*`node "\$\{CLAUDE_PLUGIN_ROOT\}\/scripts\/codex-companion\.mjs" adversarial-review "\$ARGUMENTS"`/); assert.match(source, /description:\s*"Codex adversarial review"/); - assert.match(source, /\[--model \] \[--effort \] \[focus \.\.\.\]/); - assert.match(source, /`--model` and `--effort` are runtime-selection flags/i); assert.match(source, /Do not call `BashOutput`/); assert.match(source, /Return the command stdout verbatim, exactly as-is/i); assert.match(source, /git status --short --untracked-files=all/); @@ -108,7 +110,7 @@ test("rescue command absorbs continue semantics", () => { assert.match(rescue, /--background\|--wait/); assert.match(rescue, /--resume\|--fresh/); assert.match(rescue, /--model /); - assert.match(rescue, /--effort /); + assert.match(rescue, /--effort /); assert.match(rescue, /task-resume-candidate --json/); assert.match(rescue, /AskUserQuestion/); assert.match(rescue, /Continue current Codex thread/); @@ -154,7 +156,7 @@ test("rescue command absorbs continue semantics", () => { assert.match(runtimeSkill, /Map `spark` to `--model gpt-5\.3-codex-spark`/i); assert.match(runtimeSkill, /If the forwarded request includes `--background` or `--wait`, treat that as Claude-side execution control only/i); assert.match(runtimeSkill, /Strip it before calling `task`/i); - assert.match(runtimeSkill, /`--effort`: accepted values are `none`, `minimal`, `low`, `medium`, `high`, `xhigh`/i); + assert.match(runtimeSkill, /`--effort`: accepted values are `none`, `minimal`, `low`, `medium`, `high`, `xhigh`, `max`, `ultra`/i); assert.match(runtimeSkill, /Do not inspect the repository, read files, grep, monitor progress, poll status, fetch results, cancel jobs, summarize output, or do any follow-up work of your own/i); assert.match(runtimeSkill, /If the Bash call fails or Codex cannot be invoked, return nothing/i); assert.match(readme, /`codex:codex-rescue` subagent/i); diff --git a/tests/fake-codex-fixture.mjs b/tests/fake-codex-fixture.mjs index 313e6808..4c647fa1 100644 --- a/tests/fake-codex-fixture.mjs +++ b/tests/fake-codex-fixture.mjs @@ -4,7 +4,7 @@ import process from "node:process"; import { writeExecutable } from "./helpers.mjs"; -export function installFakeCodex(binDir, behavior = "review-ok") { +export function installFakeCodex(binDir, behavior = "review-ok", version = "codex-cli test") { const statePath = path.join(binDir, "fake-codex-state.json"); const scriptPath = path.join(binDir, "codex"); const source = `#!/usr/bin/env node @@ -14,7 +14,8 @@ const path = require("node:path"); const readline = require("node:readline"); const STATE_PATH = ${JSON.stringify(statePath)}; - const BEHAVIOR = ${JSON.stringify(behavior)}; + const BEHAVIOR = ${JSON.stringify(behavior)}; + const VERSION = ${JSON.stringify(version)}; const interruptibleTurns = new Map(); function loadState() { @@ -71,7 +72,7 @@ function buildAccountReadResult() { case "auth-run-fails": return { account: null, requiresOpenaiAuth: true }; case "provider-no-auth": - case "env-key-provider": + case "env-key-provider": return { account: null, requiresOpenaiAuth: false }; case "api-key-account-only": return { account: { type: "apiKey" }, requiresOpenaiAuth: true }; @@ -88,8 +89,33 @@ function buildConfigReadResult() { case "provider-no-auth": return { config: { model_provider: "ollama" }, - origins: {} - }; + origins: {} + }; + case "custom-provider": + return { + config: { model_provider: "custom" }, + origins: {} + }; + case "inherited-sol-max": + return { + config: { model_provider: "openai", model: "gpt-5.6-sol", model_reasoning_effort: "max" }, + origins: {} + }; + case "inherited-luna-ultra": + return { + config: { model_provider: "openai", model: "gpt-5.6-luna", model_reasoning_effort: "ultra" }, + origins: {} + }; + case "inherited-default-luna-ultra": + return { + config: { model_provider: "openai", model_reasoning_effort: "ultra" }, + origins: {} + }; + case "config-luna": + return { + config: { model_provider: "openai", model: "gpt-5.6-luna", model_reasoning_effort: "high" }, + origins: {} + }; case "env-key-provider": return { config: { @@ -249,7 +275,7 @@ function taskPayload(prompt, resume) { const args = process.argv.slice(2); if (args[0] === "--version") { - console.log("codex-cli test"); + console.log(VERSION); process.exit(0); } if (args[0] === "app-server" && args[1] === "--help") { @@ -313,8 +339,29 @@ rl.on("line", (line) => { if (requiresExperimental("persistExtendedHistory", message, state) || requiresExperimental("persistFullHistory", message, state)) { throw new Error("thread/start.persistFullHistory requires experimentalApi capability"); } - const thread = nextThread(state, message.params.cwd, message.params.ephemeral); - send({ id: message.id, result: { thread: buildThread(thread), model: message.params.model || "gpt-5.4", modelProvider: "openai", serviceTier: null, cwd: thread.cwd, approvalPolicy: "never", sandbox: { type: "readOnly", access: { type: "fullAccess" }, networkAccess: false }, reasoningEffort: null } }); + const thread = nextThread(state, message.params.cwd, message.params.ephemeral); + const inheritedSelection = BEHAVIOR === "inherited-sol-max" + ? { model: "gpt-5.6-sol", effort: "max" } + : BEHAVIOR === "inherited-luna-ultra" + ? { model: "gpt-5.6-luna", effort: "ultra" } + : BEHAVIOR === "inherited-default-luna-ultra" + ? { model: "gpt-5.6-luna", effort: "ultra" } + : null; + const selectedModel = message.params.model || inheritedSelection?.model || "gpt-5.4"; + const selectedEffort = BEHAVIOR === "resolved-effort" + ? "medium" + : message.params.config?.model_reasoning_effort || inheritedSelection?.effort || null; + const modelProvider = BEHAVIOR === "custom-provider" ? "custom" : "openai"; + thread.model = selectedModel; + thread.reasoningEffort = selectedEffort; + state.lastThreadStart = { + model: selectedModel, + effort: selectedEffort, + config: message.params.config ?? null, + sandbox: message.params.sandbox ?? null + }; + saveState(state); + send({ id: message.id, result: { thread: buildThread(thread), model: selectedModel, modelProvider, serviceTier: null, cwd: thread.cwd, approvalPolicy: "never", sandbox: { type: "readOnly", access: { type: "fullAccess" }, networkAccess: false }, reasoningEffort: selectedEffort } }); send({ method: "thread/started", params: { thread: { id: thread.id } } }); break; } @@ -348,10 +395,44 @@ rl.on("line", (line) => { const thread = ensureThread(state, message.params.threadId); thread.updatedAt = now(); saveState(state); - send({ id: message.id, result: { thread: buildThread(thread), model: message.params.model || "gpt-5.4", modelProvider: "openai", serviceTier: null, cwd: thread.cwd, approvalPolicy: "never", sandbox: { type: "readOnly", access: { type: "fullAccess" }, networkAccess: false }, reasoningEffort: null } }); - break; + const selectedModel = message.params.model || thread.model || "gpt-5.4"; + const selectedEffort = BEHAVIOR === "inherited-sol-max" ? "max" : thread.reasoningEffort || null; + state.lastThreadResume = { + model: selectedModel, + effort: selectedEffort, + sandbox: message.params.sandbox ?? null + }; + saveState(state); + send({ id: message.id, result: { thread: buildThread(thread), model: selectedModel, modelProvider: "openai", serviceTier: null, cwd: thread.cwd, approvalPolicy: "never", sandbox: { type: "readOnly", access: { type: "fullAccess" }, networkAccess: false }, reasoningEffort: selectedEffort } }); + break; } + case "model/list": { + if (BEHAVIOR === "model-list-unsupported") { + send({ id: message.id, error: { code: -32601, message: "Unsupported method: model/list" } }); + break; + } + const model = (name, efforts) => ({ + id: name, + model: name, + isDefault: BEHAVIOR === "inherited-default-luna-ultra" && name === "gpt-5.6-luna", + hidden: false, + supportedReasoningEfforts: efforts.map((reasoningEffort) => ({ reasoningEffort, description: reasoningEffort })) + }); + send({ + id: message.id, + result: { + data: [ + model("gpt-5.6-sol", ["low", "medium", "high", "xhigh", "max", "ultra"]), + model("gpt-5.6-terra", ["low", "medium", "high", "xhigh", "max", "ultra"]), + model("gpt-5.6-luna", ["low", "medium", "high", "xhigh", "max"]) + ], + nextCursor: null + } + }); + break; + } + case "externalAgentConfig/import": { if (BEHAVIOR === "external-import-unsupported") { send({ id: message.id, error: { code: -32601, message: "Unsupported method: externalAgentConfig/import" } }); @@ -446,13 +527,28 @@ rl.on("line", (line) => { } case "turn/start": { + if (BEHAVIOR === "turn-start-fails") { + throw new Error("turn/start failed after thread resolution"); + } + if (BEHAVIOR === "reject-gpt-5.6" && String(message.params.model || "").startsWith("gpt-5.6-")) { + send({ + id: message.id, + error: { + code: -32000, + message: "The '" + message.params.model + "' model requires a newer version of Codex." + } + }); + break; + } const thread = ensureThread(state, message.params.threadId); const prompt = (message.params.input || []) .filter((item) => item.type === "text") .map((item) => item.text) .join("\\n"); - const turnId = nextTurnId(state); - thread.updatedAt = now(); + const turnId = nextTurnId(state); + thread.updatedAt = now(); + thread.model = message.params.model ?? thread.model ?? null; + thread.reasoningEffort = message.params.effort ?? thread.reasoningEffort ?? null; state.lastTurnStart = { threadId: message.params.threadId, turnId, @@ -662,6 +758,9 @@ export function buildEnv(binDir) { const sep = process.platform === "win32" ? ";" : ":"; return { ...process.env, - PATH: `${binDir}${sep}${process.env.PATH}` + PATH: `${binDir}${sep}${process.env.PATH}`, + // Production keeps an idle broker warm for 15 minutes. Tests only need a + // brief reuse window and should not leave dozens of detached helpers. + CODEX_COMPANION_BROKER_IDLE_TIMEOUT_MS: "2000" }; } diff --git a/tests/model-catalog.test.mjs b/tests/model-catalog.test.mjs new file mode 100644 index 00000000..af6046d1 --- /dev/null +++ b/tests/model-catalog.test.mjs @@ -0,0 +1,95 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { validateReasoningSelection } from "../plugins/codex/scripts/lib/model-catalog.mjs"; + +function clientWith(models) { + return { + async request(method) { + assert.equal(method, "model/list"); + return { data: models, nextCursor: null }; + } + }; +} + +function model(name, efforts, isDefault = false) { + return { + id: name, + model: name, + isDefault, + supportedReasoningEfforts: efforts.map((reasoningEffort) => ({ reasoningEffort })) + }; +} + +test("catalog accepts Ultra for Sol and Terra", async () => { + const client = clientWith([ + model("gpt-5.6-sol", ["high", "max", "ultra"]), + model("gpt-5.6-terra", ["high", "max", "ultra"]) + ]); + + await validateReasoningSelection(client, { + model: "gpt-5.6-sol", + effort: "ultra", + modelProvider: "openai" + }); + await validateReasoningSelection(client, { + model: "gpt-5.6-terra", + effort: "ultra", + modelProvider: "openai" + }); +}); + +test("catalog rejects Luna with Ultra and lists supported efforts", async () => { + const client = clientWith([model("gpt-5.6-luna", ["low", "medium", "high", "xhigh", "max"])]); + + await assert.rejects( + validateReasoningSelection(client, { + model: "gpt-5.6-luna", + effort: "ultra", + modelProvider: "openai" + }), + /Reasoning effort "ultra" is not supported by model "gpt-5\.6-luna".*low, medium, high, xhigh, max/i + ); +}); + +test("catalog validates effort against the default model when no model is selected", async () => { + const client = clientWith([ + model("gpt-5.6-luna", ["low", "medium", "high", "xhigh", "max"], true) + ]); + + await assert.rejects( + validateReasoningSelection(client, { effort: "ultra", modelProvider: "openai" }), + /Reasoning effort "ultra" is not supported by model "gpt-5\.6-luna"/i + ); +}); + +test("catalog fallback allows older CLIs without model/list", async () => { + const client = { + async request() { + const error = new Error("Unsupported method: model/list"); + error.rpcCode = -32601; + throw error; + } + }; + + await validateReasoningSelection(client, { + model: "gpt-5.6-sol", + effort: "ultra", + modelProvider: "openai" + }); +}); + +test("catalog does not block custom providers or unknown models", async () => { + const client = clientWith([model("gpt-5.6-luna", ["high"])]); + + await validateReasoningSelection(client, { + model: "gpt-5.6-luna", + effort: "ultra", + modelProvider: "custom" + }); + await validateReasoningSelection(client, { + model: "custom-model", + effort: "ultra", + modelProvider: "openai" + }); +}); diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index f26fba5a..037e3244 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -7,7 +7,7 @@ import { fileURLToPath } from "node:url"; import { buildEnv, installFakeCodex } from "./fake-codex-fixture.mjs"; import { initGitRepo, makeTempDir, run } from "./helpers.mjs"; -import { loadBrokerSession, saveBrokerSession } from "../plugins/codex/scripts/lib/broker-lifecycle.mjs"; +import { loadBrokerSession } from "../plugins/codex/scripts/lib/broker-lifecycle.mjs"; import { resolveStateDir } from "../plugins/codex/scripts/lib/state.mjs"; const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); @@ -15,6 +15,16 @@ const PLUGIN_ROOT = path.join(ROOT, "plugins", "codex"); const SCRIPT = path.join(PLUGIN_ROOT, "scripts", "codex-companion.mjs"); const STOP_HOOK = path.join(PLUGIN_ROOT, "scripts", "stop-review-gate-hook.mjs"); const SESSION_HOOK = path.join(PLUGIN_ROOT, "scripts", "session-lifecycle-hook.mjs"); +const FAKE_RESOLVED_SETTINGS = { + model: "gpt-5.4", + modelProvider: "openai", + reasoningEffort: null, + sandbox: { + type: "readOnly", + access: { type: "fullAccess" }, + networkAccess: false + } +}; async function waitFor(predicate, { timeoutMs = 5000, intervalMs = 50 } = {}) { const start = Date.now(); @@ -28,6 +38,13 @@ async function waitFor(predicate, { timeoutMs = 5000, intervalMs = 50 } = {}) { throw new Error("Timed out waiting for condition."); } +function readPersistedJob(workspaceRoot, jobId = null) { + const stateDir = resolveStateDir(workspaceRoot); + const state = JSON.parse(fs.readFileSync(path.join(stateDir, "state.json"), "utf8")); + const resolvedJobId = jobId ?? state.jobs[0].id; + return JSON.parse(fs.readFileSync(path.join(stateDir, "jobs", `${resolvedJobId}.json`), "utf8")); +} + test("setup reports ready when fake codex is installed and authenticated", () => { const binDir = makeTempDir(); installFakeCodex(binDir); @@ -155,34 +172,7 @@ test("review renders a no-findings result from app-server review/start", () => { assert.equal(result.status, 0); assert.match(result.stdout, /Reviewed uncommitted changes/); assert.match(result.stdout, /No material issues found/); -}); - -test("review --model --effort reach the codex app-server spawn argv as -c overrides", () => { - const repo = makeTempDir(); - const binDir = makeTempDir(); - const statePath = path.join(binDir, "fake-codex-state.json"); - installFakeCodex(binDir); - initGitRepo(repo); - fs.mkdirSync(path.join(repo, "src")); - fs.writeFileSync(path.join(repo, "src", "app.js"), "export const value = 1;\n"); - run("git", ["add", "src/app.js"], { cwd: repo }); - run("git", ["commit", "-m", "init"], { cwd: repo }); - fs.writeFileSync(path.join(repo, "src", "app.js"), "export const value = 2;\n"); - - const result = run("node", [SCRIPT, "review", "--model", "spark", "--effort", "high"], { - cwd: repo, - env: buildEnv(binDir) - }); - - assert.equal(result.status, 0, result.stderr); - const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); - assert.ok(fakeState.lastAppServerSpawnArgs, "spawn args were recorded"); - const argv = fakeState.lastAppServerSpawnArgs.join(" "); - // model/effort only take effect via -c config overrides at codex app-server spawn - // (review/start RPC params are dropped by the server), so the load-bearing assertion - // is that they reach the spawn argv. - assert.match(argv, /model="gpt-5\.3-codex-spark"/); - assert.match(argv, /model_reasoning_effort="high"/); + assert.deepEqual(readPersistedJob(repo).resolved, FAKE_RESOLVED_SETTINGS); }); test("task runs when the active provider does not require OpenAI login", () => { @@ -412,6 +402,7 @@ test("adversarial review renders structured findings over app-server turn/start" assert.equal(result.status, 0); assert.match(result.stdout, /Missing empty-state guard/); + assert.deepEqual(readPersistedJob(repo).resolved, FAKE_RESOLVED_SETTINGS); }); test("adversarial review accepts the same base-branch targeting as review", () => { @@ -529,6 +520,7 @@ test("task --resume-last resumes the latest persisted task thread", () => { assert.equal(result.status, 0, result.stderr); assert.equal(result.stdout, "Resumed the prior run.\nFollow-up prompt accepted.\n"); + assert.deepEqual(readPersistedJob(repo).resolved, FAKE_RESOLVED_SETTINGS); }); test("task-resume-candidate returns the latest rescue thread from the current session", () => { @@ -744,6 +736,46 @@ test("write task output focuses on the Codex result without generic follow-up hi assert.equal(result.stdout, "Handled the requested task.\nTask prompt accepted.\n"); }); +test("task --write starts Codex with workspace-write sandbox", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + initGitRepo(repo); + + const result = run("node", [SCRIPT, "task", "--write", "capture the page"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + const state = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(state.lastThreadStart.sandbox, "workspace-write"); +}); + +test("resuming task --write upgrades the thread to workspace-write sandbox", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + initGitRepo(repo); + + const firstRun = run("node", [SCRIPT, "task", "initial task"], { + cwd: repo, + env: buildEnv(binDir) + }); + assert.equal(firstRun.status, 0, firstRun.stderr); + + const result = run("node", [SCRIPT, "task", "--write", "--resume-last", "capture the page"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + const state = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(state.lastThreadResume.sandbox, "workspace-write"); +}); + test("task --resume acts like --resume-last without leaking the flag into the prompt", () => { const repo = makeTempDir(); const binDir = makeTempDir(); @@ -760,17 +792,75 @@ test("task --resume acts like --resume-last without leaking the flag into the pr }); assert.equal(firstRun.status, 0, firstRun.stderr); - const result = run("node", [SCRIPT, "task", "--resume", "follow up"], { - cwd: repo, - env: buildEnv(binDir) - }); + const result = run( + "node", + [SCRIPT, "task", "--resume", "--model", "gpt-5.6-terra", "--effort", "max", "follow up"], + { + cwd: repo, + env: buildEnv(binDir) + } + ); assert.equal(result.status, 0, result.stderr); const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); assert.equal(fakeState.lastTurnStart.threadId, "thr_1"); + assert.equal(fakeState.lastThreadResume.model, "gpt-5.6-terra"); + assert.equal(fakeState.lastTurnStart.model, "gpt-5.6-terra"); + assert.equal(fakeState.lastTurnStart.effort, "max"); assert.equal(fakeState.lastTurnStart.prompt, "follow up"); }); +test("resume validates the persisted thread provider even when current config uses a custom provider", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir, "custom-provider"); + initGitRepo(repo); + + const first = run("node", [SCRIPT, "task", "initial task"], { + cwd: repo, + env: buildEnv(binDir) + }); + assert.equal(first.status, 0, first.stderr); + const initialTurnId = JSON.parse(fs.readFileSync(statePath, "utf8")).lastTurnStart.turnId; + + const resumed = run( + "node", + [SCRIPT, "task", "--resume", "--model", "gpt-5.6-luna", "--effort", "ultra", "follow up"], + { cwd: repo, env: buildEnv(binDir) } + ); + + assert.notEqual(resumed.status, 0); + assert.match(resumed.stderr, /not supported by model "gpt-5\.6-luna"/i); + assert.equal(JSON.parse(fs.readFileSync(statePath, "utf8")).lastTurnStart.turnId, initialTurnId); +}); + +test("resume validates effort against the persisted thread model instead of current config", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir, "config-luna"); + initGitRepo(repo); + + const first = run( + "node", + [SCRIPT, "task", "--model", "gpt-5.6-sol", "--effort", "high", "initial task"], + { cwd: repo, env: buildEnv(binDir) } + ); + assert.equal(first.status, 0, first.stderr); + + const resumed = run("node", [SCRIPT, "task", "--resume", "--effort", "ultra", "follow up"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(resumed.status, 0, resumed.stderr); + const state = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(state.lastThreadResume.model, "gpt-5.6-sol"); + assert.equal(state.lastTurnStart.model, null); + assert.equal(state.lastTurnStart.effort, "ultra"); +}); + test("task --fresh is treated as routing control and does not leak into the prompt", () => { const repo = makeTempDir(); const binDir = makeTempDir(); @@ -795,7 +885,7 @@ test("task forwards model selection and reasoning effort to app-server turn/star const repo = makeTempDir(); const binDir = makeTempDir(); const statePath = path.join(binDir, "fake-codex-state.json"); - installFakeCodex(binDir); + installFakeCodex(binDir, "resolved-effort"); initGitRepo(repo); fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); run("git", ["add", "README.md"], { cwd: repo }); @@ -810,6 +900,212 @@ test("task forwards model selection and reasoning effort to app-server turn/star const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); assert.equal(fakeState.lastTurnStart.model, "gpt-5.3-codex-spark"); assert.equal(fakeState.lastTurnStart.effort, "low"); + assert.deepEqual(readPersistedJob(repo).resolved, { + ...FAKE_RESOLVED_SETTINGS, + model: "gpt-5.3-codex-spark", + reasoningEffort: "low" + }); +}); + +test("task preserves resolved settings when turn/start fails", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir, "turn-start-fails"); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + + const result = run("node", [SCRIPT, "task", "--effort", "xhigh", "diagnose the failing test"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.notEqual(result.status, 0); + assert.match(result.stderr, /turn\/start failed after thread resolution/); + const storedJob = readPersistedJob(repo); + assert.equal(storedJob.status, "failed"); + assert.deepEqual(storedJob.resolved, FAKE_RESOLVED_SETTINGS); + const stateDir = resolveStateDir(repo); + const state = JSON.parse(fs.readFileSync(path.join(stateDir, "state.json"), "utf8")); + assert.deepEqual(state.jobs[0].resolved, FAKE_RESOLVED_SETTINGS); +}); + +test("task supports max and ultra while rejecting unsupported model combinations locally", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + initGitRepo(repo); + + const max = run("node", [SCRIPT, "task", "--model", "gpt-5.6-luna", "--effort", "max", "check"], { + cwd: repo, + env: buildEnv(binDir) + }); + assert.equal(max.status, 0, max.stderr); + assert.equal(JSON.parse(fs.readFileSync(statePath, "utf8")).lastTurnStart.effort, "max"); + + const ultra = run("node", [SCRIPT, "task", "--model", "gpt-5.6-sol", "--effort", "ultra", "check"], { + cwd: repo, + env: buildEnv(binDir) + }); + assert.equal(ultra.status, 0, ultra.stderr); + assert.equal(JSON.parse(fs.readFileSync(statePath, "utf8")).lastTurnStart.effort, "ultra"); + + const invalid = run("node", [SCRIPT, "task", "--model", "gpt-5.6-luna", "--effort", "ultra", "check"], { + cwd: repo, + env: buildEnv(binDir) + }); + assert.notEqual(invalid.status, 0); + assert.match(invalid.stderr, /not supported by model "gpt-5\.6-luna"/i); + assert.equal(JSON.parse(fs.readFileSync(statePath, "utf8")).lastThreadStart.model, "gpt-5.6-sol"); +}); + +test("task validates model and effort inherited from Codex config", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir, "inherited-sol-max"); + initGitRepo(repo); + + const result = run("node", [SCRIPT, "task", "check inherited selection"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + const state = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(state.lastThreadStart.model, "gpt-5.6-sol"); + assert.equal(state.lastThreadStart.effort, "max"); + assert.equal(state.lastTurnStart.model, null); + assert.equal(state.lastTurnStart.effort, null); +}); + +test("task rejects an unsupported model and effort inherited from Codex config", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir, "inherited-luna-ultra"); + initGitRepo(repo); + + const result = run("node", [SCRIPT, "task", "check inherited selection"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.notEqual(result.status, 0); + assert.match(result.stderr, /Reasoning effort "ultra" is not supported by model "gpt-5\.6-luna"/i); + assert.equal(JSON.parse(fs.readFileSync(statePath, "utf8")).threads.length, 0); +}); + +test("task prevalidates a partial explicit selection against Codex config", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir, "inherited-luna-ultra"); + initGitRepo(repo); + + const result = run("node", [SCRIPT, "task", "--effort", "ultra", "check selection"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.notEqual(result.status, 0); + assert.match(result.stderr, /not supported by model "gpt-5\.6-luna"/i); + assert.equal(JSON.parse(fs.readFileSync(statePath, "utf8")).threads.length, 0); +}); + +test("task prevalidates effort against the catalog default before creating a persistent thread", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir, "inherited-default-luna-ultra"); + initGitRepo(repo); + + const result = run("node", [SCRIPT, "task", "--effort", "ultra", "check default selection"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.notEqual(result.status, 0); + assert.match(result.stderr, /not supported by model "gpt-5\.6-luna"/i); + assert.equal(JSON.parse(fs.readFileSync(statePath, "utf8")).threads.length, 0); +}); + +test("task falls back cleanly when an older Codex CLI does not expose model/list", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir, "model-list-unsupported"); + initGitRepo(repo); + + const result = run("node", [SCRIPT, "task", "--model", "gpt-5.6-sol", "--effort", "max", "check"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); +}); + +test("task does not apply the OpenAI effort matrix to a custom provider", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir, "custom-provider"); + initGitRepo(repo); + + const result = run("node", [SCRIPT, "task", "--model", "gpt-5.6-luna", "--effort", "ultra", "check"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); +}); + +test("review rejects an unsupported explicit selection before creating a thread", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); + + const result = run("node", [SCRIPT, "review", "--model", "gpt-5.6-luna", "--effort", "ultra"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.notEqual(result.status, 0); + assert.match(result.stderr, /not supported by model "gpt-5\.6-luna"/i); + assert.equal(JSON.parse(fs.readFileSync(statePath, "utf8")).threads.length, 0); +}); + +test("review and adversarial-review consume model and effort flags instead of leaking them into focus text", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); + + const review = run("node", [SCRIPT, "review", "--model", "gpt-5.6-sol", "--effort", "max"], { + cwd: repo, + env: buildEnv(binDir) + }); + assert.equal(review.status, 0, review.stderr); + let state = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(state.lastThreadStart.model, "gpt-5.6-sol"); + assert.equal(state.lastThreadStart.effort, "max"); + + const adversarial = run( + "node", + [SCRIPT, "adversarial-review", "--model", "gpt-5.6-terra", "--effort", "xhigh", "challenge retries"], + { cwd: repo, env: buildEnv(binDir) } + ); + assert.equal(adversarial.status, 0, adversarial.stderr); + state = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(state.lastTurnStart.model, "gpt-5.6-terra"); + assert.equal(state.lastTurnStart.effort, "xhigh"); + assert.doesNotMatch(state.lastTurnStart.prompt, /--model|--effort/); + assert.match(state.lastTurnStart.prompt, /challenge retries/); }); test("task logs reasoning summaries and assistant messages to the job log", () => { @@ -957,16 +1253,39 @@ test("task --background enqueues a detached worker and exposes per-job status", run("git", ["add", "README.md"], { cwd: repo }); run("git", ["commit", "-m", "init"], { cwd: repo }); - const launched = run("node", [SCRIPT, "task", "--background", "--json", "investigate the failing test"], { - cwd: repo, - env: buildEnv(binDir) - }); + const launched = run( + "node", + [ + SCRIPT, + "task", + "--background", + "--json", + "--model", + "gpt-5.6-luna", + "--effort", + "max", + "investigate the failing test" + ], + { cwd: repo, env: buildEnv(binDir) } + ); assert.equal(launched.status, 0, launched.stderr); const launchPayload = JSON.parse(launched.stdout); assert.equal(launchPayload.status, "queued"); assert.match(launchPayload.jobId, /^task-/); + const runningJob = await waitFor(() => { + try { + const storedJob = readPersistedJob(repo, launchPayload.jobId); + return storedJob.status === "running" && storedJob.resolved ? storedJob : null; + } catch { + return null; + } + }); + assert.deepEqual(runningJob.resolved, FAKE_RESOLVED_SETTINGS); + const runningState = JSON.parse(fs.readFileSync(path.join(resolveStateDir(repo), "state.json"), "utf8")); + assert.deepEqual(runningState.jobs.find((job) => job.id === launchPayload.jobId).resolved, FAKE_RESOLVED_SETTINGS); + const waitedStatus = run( "node", [SCRIPT, "status", launchPayload.jobId, "--wait", "--timeout-ms", "15000", "--json"], @@ -994,7 +1313,12 @@ test("task --background enqueues a detached worker and exposes per-job status", assert.equal(resultPayload.job.id, launchPayload.jobId); assert.equal(resultPayload.job.status, "completed"); + assert.deepEqual(resultPayload.job.resolved, FAKE_RESOLVED_SETTINGS); + assert.deepEqual(resultPayload.storedJob.resolved, FAKE_RESOLVED_SETTINGS); assert.match(resultPayload.storedJob.rendered, /Handled the requested task/); + const fakeState = JSON.parse(fs.readFileSync(path.join(binDir, "fake-codex-state.json"), "utf8")); + assert.equal(fakeState.lastTurnStart.model, "gpt-5.6-luna"); + assert.equal(fakeState.lastTurnStart.effort, "max"); }); test("review rejects focus text because it is native-review only", () => { @@ -1796,6 +2120,8 @@ test("cancel sends turn interrupt to the shared app-server before killing a brok return null; }, { timeoutMs: 15000 }); + installFakeCodex(binDir, "interruptible-slow-task", "codex-cli 0.144.0"); + const cancelResult = run("node", [SCRIPT, "cancel", jobId, "--json"], { cwd: repo, env @@ -1817,6 +2143,7 @@ test("cancel sends turn interrupt to the shared app-server before killing a brok threadId: runningJob.threadId, turnId: runningJob.turnId }); + assert.equal(fakeState.appServerStarts, 1); const cleanup = run("node", [SESSION_HOOK, "SessionEnd"], { cwd: repo, @@ -2189,49 +2516,57 @@ test("commands lazily start and reuse one shared app-server after first use", as assert.equal(cleanup.status, 0, cleanup.stderr); }); -test("a differing --effort override respawns the shared app-server broker instead of reusing the stale one", async () => { +test("shared broker invalidates stale CLI, plugin, and legacy runtime state", () => { const repo = makeTempDir(); const binDir = makeTempDir(); const fakeStatePath = path.join(binDir, "fake-codex-state.json"); - - installFakeCodex(binDir); + installFakeCodex(binDir, "reject-gpt-5.6", "codex-cli 0.143.0"); initGitRepo(repo); fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); - run("git", ["add", "README.md"], { cwd: repo }); - run("git", ["commit", "-m", "init"], { cwd: repo }); - fs.writeFileSync(path.join(repo, "README.md"), "hello again\n"); - const env = buildEnv(binDir); - // 1) Warm the broker with no override. - const first = run("node", [SCRIPT, "review"], { cwd: repo, env }); + const first = run("node", [SCRIPT, "task", "first"], { cwd: repo, env }); assert.equal(first.status, 0, first.stderr); + assert.ok(loadBrokerSession(repo), "expected the first task to create a shared broker"); - const brokerSession = loadBrokerSession(repo); - if (!brokerSession) { - return; - } - - // 2) Second call with a different effort override must respawn, not reuse. - const second = run("node", [SCRIPT, "review", "--effort", "high"], { cwd: repo, env }); + installFakeCodex(binDir, "review-ok", "codex-cli 0.144.0"); + const second = run( + "node", + [SCRIPT, "task", "--model", "gpt-5.6-sol", "--effort", "high", "second"], + { cwd: repo, env } + ); assert.equal(second.status, 0, second.stderr); - const fakeState = JSON.parse(fs.readFileSync(fakeStatePath, "utf8")); - // The warm broker was started without the override, so it must be torn down - // and a fresh one spawned with -c model_reasoning_effort="high". - assert.equal(fakeState.appServerStarts, 2); - const argv = (fakeState.lastAppServerSpawnArgs || []).join(" "); - assert.match(argv, /model_reasoning_effort="high"/); + let state = JSON.parse(fs.readFileSync(fakeStatePath, "utf8")); + assert.equal(state.appServerStarts, 2); + assert.equal(loadBrokerSession(repo).runtime.codexVersion, "codex-cli 0.144.0"); - const cleanup = run("node", [SESSION_HOOK, "SessionEnd"], { + let broker = loadBrokerSession(repo); + fs.writeFileSync( + path.join(resolveStateDir(repo), "broker.json"), + `${JSON.stringify({ ...broker, runtime: { ...broker.runtime, pluginVersion: "1.0.6" } }, null, 2)}\n` + ); + const pluginUpgrade = run("node", [SCRIPT, "task", "after plugin upgrade"], { cwd: repo, env }); + assert.equal(pluginUpgrade.status, 0, pluginUpgrade.stderr); + state = JSON.parse(fs.readFileSync(fakeStatePath, "utf8")); + assert.equal(state.appServerStarts, 3); + + broker = loadBrokerSession(repo); + const { runtime: _runtime, ...legacyBroker } = broker; + fs.writeFileSync( + path.join(resolveStateDir(repo), "broker.json"), + `${JSON.stringify(legacyBroker, null, 2)}\n` + ); + const legacyUpgrade = run("node", [SCRIPT, "task", "after legacy upgrade"], { cwd: repo, env }); + assert.equal(legacyUpgrade.status, 0, legacyUpgrade.stderr); + state = JSON.parse(fs.readFileSync(fakeStatePath, "utf8")); + assert.equal(state.appServerStarts, 4); + + run("node", [SESSION_HOOK, "SessionEnd"], { cwd: repo, env, - input: JSON.stringify({ - hook_event_name: "SessionEnd", - cwd: repo - }) + input: JSON.stringify({ hook_event_name: "SessionEnd", cwd: repo }) }); - assert.equal(cleanup.status, 0, cleanup.stderr); }); test("setup reuses an existing shared app-server without starting another one", () => { @@ -2311,22 +2646,39 @@ test("status reports shared session runtime when a lazy broker is active", () => test("setup and status honor --cwd when reading shared session runtime", () => { const targetWorkspace = makeTempDir(); const invocationWorkspace = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir); + initGitRepo(targetWorkspace); - saveBrokerSession(targetWorkspace, { - endpoint: "unix:/tmp/fake-broker.sock" + const task = run("node", [SCRIPT, "task", "start shared runtime"], { + cwd: targetWorkspace, + env: buildEnv(binDir) }); + assert.equal(task.status, 0, task.stderr); + const broker = loadBrokerSession(targetWorkspace); + if (!broker) { + return; + } const status = run("node", [SCRIPT, "status", "--cwd", targetWorkspace], { - cwd: invocationWorkspace + cwd: invocationWorkspace, + env: buildEnv(binDir) }); assert.equal(status.status, 0, status.stderr); assert.match(status.stdout, /Session runtime: shared session/); const setup = run("node", [SCRIPT, "setup", "--cwd", targetWorkspace, "--json"], { - cwd: invocationWorkspace + cwd: invocationWorkspace, + env: buildEnv(binDir) }); assert.equal(setup.status, 0, setup.stderr); const payload = JSON.parse(setup.stdout); assert.equal(payload.sessionRuntime.mode, "shared"); - assert.equal(payload.sessionRuntime.endpoint, "unix:/tmp/fake-broker.sock"); + assert.equal(payload.sessionRuntime.endpoint, broker.endpoint); + + run("node", [SESSION_HOOK, "SessionEnd"], { + cwd: targetWorkspace, + env: buildEnv(binDir), + input: JSON.stringify({ hook_event_name: "SessionEnd", cwd: targetWorkspace }) + }); });