diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 870246242..19595054b 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -1,17 +1,18 @@ { - "name": "openai-codex", + "name": "cbepx", "owner": { - "name": "OpenAI" + "name": "CBEPX", + "url": "https://github.com/CBEPX" }, "metadata": { - "description": "Codex plugins to use in Claude Code for delegation and code review.", - "version": "1.0.6" + "description": "CBEPX fork of the OpenAI Codex plugin for Claude Code: max/ultra effort, per-thread config overrides, gpt-5.6 aliases, rescue agent fixes.", + "version": "1.1.0" }, "plugins": [ { "name": "codex", "description": "Use Codex from Claude Code to review code or delegate tasks.", - "version": "1.0.6", + "version": "1.1.0", "author": { "name": "OpenAI" }, diff --git a/.github/workflows/pull-request-ci.yml b/.github/workflows/pull-request-ci.yml index ebcff0b65..9f54ddfd6 100644 --- a/.github/workflows/pull-request-ci.yml +++ b/.github/workflows/pull-request-ci.yml @@ -2,6 +2,8 @@ name: Pull Request CI on: pull_request: + push: + branches: [main, "release/**"] permissions: contents: read diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 000000000..32d8c1490 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,34 @@ +# Changelog + +## 1.1.0 — 2026-08-27 + +Fork of [openai/codex-plugin-cc](https://github.com/openai/codex-plugin-cc) 1.0.6 (`db52e28`). Marketplace `cbepx`, plugin name unchanged (`codex`). + +### Merged from upstream pull requests +- #616 accept `max` and `ultra` reasoning efforts +- #688 resolve model aliases on `review` / `adversarial-review` +- #426 `on-request` approval policy for `--write` task runs +- #501 answer MCP elicitation requests instead of rejecting them +- #608 rescue agent awaits the delegated result instead of returning a placeholder +- #690 explicit Bash blocks in `status`/`result`/`cancel`/`transfer` commands (pass permission classifiers) +- #547 unknown flags are CLI errors, never part of the prompt; `--help` prints usage and exits 0 +- #645 / #644 job records store resolved model/effort/sandbox; reasoning start is logged +- #672 SessionStart hook timeout raised; #668 idempotent `CLAUDE_ENV_FILE` exports; #682 stop gate fails closed on malformed input; #396 `CODEX_REVIEW_GATE_MAX_ROUNDS` + +### Fork changes +- Slash-command arguments reach the companion through a quoted heredoc on stdin (`--args-stdin`) instead of a shell string: Claude Code substitutes `$ARGUMENTS` before bash runs, so `$(...)`/backticks in a prompt used to execute on the host shell, outside Codex's sandbox. Rescue job ids are validated before use. `/codex:rescue` keeps the two channels separate — the request prose goes to `--prompt-file` through its own quoted heredoc so quotes, backslashes and newlines survive byte-exact, while `--args-stdin` carries only runtime flags — and randomizes both heredoc delimiters per call; the other seven command bodies keep the fixed `CODEX_ARGS` delimiter because their payload is only flags and job ids. +- Approval requests (`execCommandApproval`, `applyPatchApproval`, `item/commandExecution/requestApproval`, `item/fileChange/requestApproval`, `item/permissions/requestApproval`) are answered with each type's refusal variant instead of a `-32601` protocol error that made `--write` turns fail or hang. +- Background task records are written before the worker is spawned (a fast worker used to find no record and exit while the launch reported `queued`), and the worker reads the full request — including `--config` values — from a private one-shot `jobs/.request.json` (mode 0600); the job record `status`/`result` echo keeps secret-looking config values redacted. +- Model and reasoning effort are sent per thread via `thread/start.config` (`model`, `review_model` for native review, `model_reasoning_effort`); generic `--config` pairs are applied first, dedicated flags override them; `--effort` now works on `review` and `adversarial-review`. +- Repeatable `--config key=value` on `task`, `review`, `adversarial-review` forwards any `config.toml` override to the thread (values are JSON-parsed; quote a literal string as `'"true"'`). Prompt-taking commands stop option parsing at the first positional, so prompt text like `ls -R` is never mis-parsed. +- `--resume-last` opens a fresh app-server session (cold resume) so `--config`, sandbox and approval policy take effect, and never sends `model` on `thread/resume` (it would drop the persisted model); the resumed turn's model/effort ride on `turn/start`. +- All MCP elicitations are declined (no operator is present); URL/form flows must be completed in an interactive Codex session. +- `/codex:rescue` is synchronous by default without the `Agent` tool: `task --background` → `status --wait` in ≤9-minute slices (job id carried literally between Bash calls) → `result`; launch failures stop immediately with visible stderr; `Agent` only for `--background`; `--write` is never added unless the user explicitly asked Codex to modify files. +- `/codex:rescue` asks before continuing an existing Codex thread (`Continue current Codex thread` / `Start a new Codex thread`) instead of resuming silently; its `allowed-tools` is now `Bash, AskUserQuestion, Agent` because the body is multi-command shell. +- A resume refuses to start a second turn on a thread that a queued or running job is still using, including a job from another Claude session. +- Model aliases: `sol`, `luna`, `terra`, `mini` (plus `spark`); rescue agent has no pinned `model:`; runtime skill mentions `$agent-compat:skill-router` for uncommon domains. +- Stop-gate script timeout (13 min) is below the hook timeout (15 min); `spawnSync` uses `SIGKILL` and a 16 MiB buffer. +- Hermetic test environment (`tests/test-env.mjs`); CI on push; `npm run build` type-checks the JSDoc. + +## 1.0.6 and earlier +See upstream releases: https://github.com/openai/codex-plugin-cc/releases diff --git a/README.md b/README.md index 937a3037b..8abdc7296 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,7 @@ # Codex plugin for Claude Code +> **CBEPX fork.** Install with `claude plugin marketplace add CBEPX/codex-plugin-cc` then `claude plugin install codex@cbepx`. Differences from upstream are listed in [CHANGELOG.md](CHANGELOG.md). Upstream: openai/codex-plugin-cc. + Use Codex from inside Claude Code for code reviews or to delegate tasks to Codex. This plugin is for Claude Code users who want an easy way to start using Codex from the workflow @@ -24,13 +26,13 @@ they already have. Add the marketplace in Claude Code: ```bash -/plugin marketplace add openai/codex-plugin-cc +/plugin marketplace add CBEPX/codex-plugin-cc ``` Install the plugin: ```bash -/plugin install codex@openai-codex +/plugin install codex@cbepx ``` Reload plugins: @@ -159,7 +161,9 @@ Ask Codex to redesign the database connection to be more resilient. **Notes:** - 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` +- `--effort` accepts `none`, `minimal`, `low`, `medium`, `high`, `xhigh`, `max`, and `ultra`. Which of those a given model actually supports is decided by Codex, not by the plugin — run `codex debug models` to see the reasoning levels each model advertises. +- model aliases: `spark` -> `gpt-5.3-codex-spark`, `sol` -> `gpt-5.6-sol`, `luna` -> `gpt-5.6-luna`, `terra` -> `gpt-5.6-terra`, `mini` -> `gpt-5.4-mini` +- `--config key=value` (repeatable, also on `/codex:review` and `/codex:adversarial-review`) forwards a `config.toml` override to the Codex thread, e.g. `--config model_provider=ollama`. On `--resume-last` the plugin opens a fresh app-server session (cold resume) so `--config` overrides, sandbox and approval policy take effect; model and effort for the resumed turn are sent on the turn, never on the resume request. - follow-up rescue requests can continue the latest Codex task in the repo ### `/codex:transfer` @@ -236,6 +240,17 @@ When the review gate is enabled, the plugin uses a `Stop` hook to run a targeted > [!WARNING] > The review gate can create a long-running Claude/Codex loop and may drain usage limits quickly. Only enable it when you plan to actively monitor the session. +#### Bounding the review gate + +By default the gate keeps blocking the stop until Codex is satisfied, which is what can create the loop above. Set `CODEX_REVIEW_GATE_MAX_ROUNDS` to cap how many consecutive gate rounds run in a single session before the stop is allowed through: + +```bash +# allow at most 5 stop-gate review rounds per session, then let the stop proceed +export CODEX_REVIEW_GATE_MAX_ROUNDS=5 +``` + +When unset or `0`, the gate is unbounded (the previous behavior). The count is per session, increments on each blocked round (tracked via `stop_hook_active`), and resets once a stop is allowed or a fresh user turn begins. + ## Typical Flows ### Review Before Shipping diff --git a/docs/superpowers/plans/2026-08-27-codex-plugin-cc-v1.1.0.md b/docs/superpowers/plans/2026-08-27-codex-plugin-cc-v1.1.0.md new file mode 100644 index 000000000..30f42e2a9 --- /dev/null +++ b/docs/superpowers/plans/2026-08-27-codex-plugin-cc-v1.1.0.md @@ -0,0 +1,788 @@ +# codex-plugin-cc fork v1.1.0 Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Ship `codex@cbepx` v1.1.0 — upstream 1.0.6 plus the drift fixes Claude Code actually hit in Aug 2026 (effort `max`, model/effort really reaching Codex, per-run config overrides, approval for `--write`, MCP elicitation, rescue agent returning a result, commands that pass the auto-mode classifier, sane hook timeouts) — installable from `CBEPX/codex-plugin-cc`. + +**Architecture:** Fork keeps upstream layout (`.claude-plugin/marketplace.json` + `plugins/codex/`) so `git merge upstream/main` stays cheap. Community PRs that are MERGEABLE and small are merged as branches (`git fetch upstream pull/N/head`), preserving authorship. Model/effort/config overrides are sent per thread via `thread/start.config` / `thread/resume.config` (protocol-confirmed: `ThreadStartParams.config?: {[key]: JsonValue}`; `ReviewStartParams` has no model/effort — the review thread's config is the only route). No broker changes. + +**Tech Stack:** Node ≥18.18 (dev on 24.14), ESM `.mjs`, `node --test`, fake Codex fixture (`tests/fake-codex-fixture.mjs`), `gh` CLI, Serena MCP for symbol edits. + +**Spec:** memory `codex-plugin-cc-fork-backlog` (ranked backlog) + `~/.claude/plans/claude-code-structured-crystal.md` §Step 7. Usage evidence: 21 Aug-2026 sessions; 11/12 rescue-agent calls returned a placeholder; `max` rejected → `xhigh` forced; `/codex:status` inline `!` body blocked by classifier twice. + +## Global Constraints + +- Repo: `/Users/g.mehrenin/project/personal/codex-plugin-cc`, remotes `origin=CBEPX/codex-plugin-cc`, `upstream=openai/codex-plugin-cc`. Work on branch `release/v1.1.0` from `main` (= upstream `db52e28`, 1.0.6). +- Plugin name stays `codex` (so `/codex:*`, `codex:codex-rescue`, `Skill(codex:rescue)` keep working); marketplace name becomes `cbepx`. +- `npm test` must be green after every task. Baseline: 91 tests. Tests must pass **inside a Claude Code session** (Task 0 isolates leaked `CLAUDE_PLUGIN_DATA`/`CODEX_COMPANION_*` env). +- Node `>=18.18.0` in `package.json` engines — no Node-22-only APIs. +- Commit messages: conventional (`feat:`, `fix:`, `chore:`, `merge:`), trailer `Co-Authored-By: Claude Fable 5 `. +- Merge mechanics for upstream PRs (same every time): `git fetch upstream pull/N/head:pr/N && git merge --no-ff --no-edit pr/N`; on conflict, keep BOTH sides for test-file insertions (they add independent `test(...)` blocks at the same anchor), then `npm test`. +- Test gate command (never mask the exit status behind a pipe): `npm test > /tmp/npm-test.log 2>&1; status=$?; rg -e 'ℹ (tests|pass|fail)' -e '^not ok' /tmp/npm-test.log; test "$status" -eq 0` — the `test` at the end is the gate. Same for smoke commands: capture `status=$?` before formatting output. +- Tooling rule (user): never use `grep`/`egrep`/`fgrep` — use ripgrep `rg` (or an equivalent) in every command, script, test and brief; e.g. `rg -n 'pattern' file`, `... | rg -e 'ℹ (tests|pass|fail)' -e '^not ok'`. +- Amended 2026-08-27 14:30 after a Codex adversarial review of this plan (13 findings; rulings in the SDD ledger). Tasks 5–8 carry the amendments. +- Not in scope (backlog v1.2+): lifecycle/broker leak (#540/#543/#425/#376/#457), structured `--json` (#593), sandbox from config.toml (#646), `--profile` flag (covered by `--config`), Windows. + +--- + +### Task 0: Branch + hermetic test env + CI on push + +**Files:** +- Create: `tests/test-env.mjs` +- Modify: `package.json` (scripts.test) +- Modify: `.github/workflows/pull-request-ci.yml` (trigger) + +**Interfaces:** +- Produces: `npm test` == `node --import ./tests/test-env.mjs --test tests/*.test.mjs`; env vars `CLAUDE_PLUGIN_DATA`, `CODEX_COMPANION_SESSION_ID`, `CODEX_COMPANION_TRANSCRIPT_PATH`, `CODEX_COMPANION_APP_SERVER_ENDPOINT`, `CLAUDE_ENV_FILE`, `CODEX_PLUGIN_CC_ARGS` are always unset when tests start. + +- [ ] **Step 1: Branch** + +```bash +cd /Users/g.mehrenin/project/personal/codex-plugin-cc && git checkout -b release/v1.1.0 main +``` + +- [ ] **Step 2: Reproduce the failure (inside Claude Code the env leaks)** + +Run: `CLAUDE_PLUGIN_DATA=/tmp/leak npm test > /tmp/npm-test.log 2>&1; status=$?; rg -e 'ℹ (pass|fail)' /tmp/npm-test.log; echo status=$status` +Expected: `ℹ fail 4` (state.test.mjs `resolveStateDir uses a temp-backed per-workspace directory` and 3 siblings). + +- [ ] **Step 3: Write `tests/test-env.mjs`** + +```js +// Hermetic test environment: strip host-session variables that Claude Code / +// the plugin's own SessionStart hook export, so tests see a clean machine. +for (const name of [ + "CLAUDE_PLUGIN_DATA", + "CLAUDE_ENV_FILE", + "CODEX_COMPANION_SESSION_ID", + "CODEX_COMPANION_TRANSCRIPT_PATH", + "CODEX_COMPANION_APP_SERVER_ENDPOINT", + "CODEX_COMPANION_APP_SERVER_PID_FILE", + "CODEX_COMPANION_APP_SERVER_LOG_FILE", + "CODEX_PLUGIN_CC_ARGS" +]) { + delete process.env[name]; +} +``` + +- [ ] **Step 4: Wire it into `package.json`** + +Replace `"test": "node --test tests/*.test.mjs"` with `"test": "node --import ./tests/test-env.mjs --test tests/*.test.mjs"`. + +- [ ] **Step 5: Verify** + +Run: `CLAUDE_PLUGIN_DATA=/tmp/leak npm test > /tmp/npm-test.log 2>&1; status=$?; rg -e 'ℹ (tests|pass|fail)' /tmp/npm-test.log; echo status=$status` +Expected: `ℹ tests 91`, `ℹ pass 91`, `ℹ fail 0`. + +- [ ] **Step 6: CI also on push to main/release branches** + +In `.github/workflows/pull-request-ci.yml` replace +```yaml +on: + pull_request: +``` +with +```yaml +on: + pull_request: + push: + branches: [main, "release/**"] +``` + +- [ ] **Step 7: Commit** + +```bash +git add tests/test-env.mjs package.json .github/workflows/pull-request-ci.yml +git commit -m "chore(test): hermetic test env; run CI on push" -m "Co-Authored-By: Claude Fable 5 " +``` + +--- + +### Task 1: Merge upstream PR #616 — accept `max`/`ultra` reasoning efforts + +**Files:** (via merge) `plugins/codex/scripts/codex-companion.mjs` (`VALID_REASONING_EFFORTS`, usage text, error text), `plugins/codex/commands/rescue.md`, `plugins/codex/skills/codex-cli-runtime/SKILL.md`, `README.md`, `tests/commands.test.mjs`, `tests/runtime.test.mjs`. + +**Interfaces:** +- Produces: `normalizeReasoningEffort("max") === "max"`; error text `Unsupported reasoning effort "X". Use one of: none, minimal, low, medium, high, xhigh, max, ultra.` + +- [ ] **Step 1: Merge** + +```bash +git fetch upstream pull/616/head:pr/616 && git merge --no-ff --no-edit pr/616 +``` +Expected: clean merge (PR is MERGEABLE against main). + +- [ ] **Step 2: Test** + +Run: `npm test > /tmp/npm-test.log 2>&1; status=$?; rg -e 'ℹ (tests|pass|fail)' -e '^not ok' /tmp/npm-test.log; test "$status" -eq 0` +Expected: `ℹ fail 0`, tests ≥ 94 (adds `task forwards max/ultra reasoning effort…` ×2 and `task rejects an unknown reasoning effort`). + +- [ ] **Step 3: Smoke the real error path** + +Run: `node plugins/codex/scripts/codex-companion.mjs task --effort supreme x 2>&1 | head -2` +Expected: `Unsupported reasoning effort "supreme". Use one of: none, minimal, low, medium, high, xhigh, max, ultra.` + +(No extra commit — the merge commit is the commit.) + +--- + +### Task 2: Merge #688 — normalize `--model` aliases on review/adversarial-review + +**Files:** (via merge) `codex-companion.mjs` `handleReviewCommand` (calls `normalizeRequestedModel(options.model)`), `tests/fake-codex-fixture.mjs` (+`lastThreadStart`), `tests/runtime.test.mjs`. + +- [ ] **Step 1: Merge** + +```bash +git fetch upstream pull/688/head:pr/688 && git merge --no-ff --no-edit pr/688 +``` +Likely conflict: `tests/runtime.test.mjs` around the anchor `test("task forwards model selection and reasoning effort to app-server turn/start"` — both #616 and #688 append tests after it. Resolution: keep both blocks in either order, remove markers. + +- [ ] **Step 2: Test** — gate command from Global Constraints → `fail 0`. + +- [ ] **Step 3: Commit if you resolved a conflict** + +```bash +git add tests/runtime.test.mjs && git commit --no-edit +``` + +--- + +### Task 3: Merge #426 (approval `on-request` for `--write`) and #501 (accept MCP elicitations) + +**Files:** (via merge) `codex-companion.mjs` `executeTaskRun` (+`approvalPolicy`), `lib/codex.mjs` `runAppServerTurn` (passes `approvalPolicy` into `startThread`/`resumeThread`), `lib/app-server.mjs` (`item/tool/requestUserInput`/elicitation requests answered instead of `-32601`), `tests/fake-codex-fixture.mjs`, `tests/runtime.test.mjs`, new `tests/app-server.test.mjs`. + +**Interfaces:** +- Produces: `runAppServerTurn(cwd, { approvalPolicy: "on-request" | "never", … })`; `buildThreadParams` already honours `options.approvalPolicy`. + +- [ ] **Step 1: Merge #426** + +```bash +git fetch upstream pull/426/head:pr/426 && git merge --no-ff --no-edit pr/426 +``` +Likely conflict: `tests/fake-codex-fixture.mjs` near line 313 (`rl.on("line"…` handler — #688 added `lastThreadStart`, #426 adds approval bookkeeping). Keep both. + +- [ ] **Step 2: Test, commit resolution if any.** + +- [ ] **Step 3: Merge #501** + +```bash +git fetch upstream pull/501/head:pr/501 && git merge --no-ff --no-edit pr/501 +``` + +- [ ] **Step 4: Test** + +Run: gate command from Global Constraints → `fail 0`; `tests/app-server.test.mjs` present and passing. + +--- + +### Task 4: Merge #608 (rescue agent awaits the result) and #690 (explicit Bash blocks in commands) + +**Files:** (via merge) `plugins/codex/agents/codex-rescue.md`, `plugins/codex/skills/codex-cli-runtime/SKILL.md`, `plugins/codex/commands/{cancel,result,status,transfer}.md`, `tests/commands.test.mjs`. + +- [ ] **Step 1: Merge both** + +```bash +git fetch upstream pull/608/head:pr/608 && git merge --no-ff --no-edit pr/608 +git fetch upstream pull/690/head:pr/690 && git merge --no-ff --no-edit pr/690 +``` +Possible conflict in `tests/commands.test.mjs` (both #616 and #608 edit the `rescue command absorbs continue semantics` assertions) — keep the #608 assertion lines and the #616 `max|ultra` regex. + +- [ ] **Step 2: Verify the command bodies no longer use inline `` !` `` ** + +Run: `rg -l '^!`' plugins/codex/commands/` +Expected: no output. + +- [ ] **Step 3: Test** → `fail 0`. + +--- + +### Task 5: Merge #547 (`--help`/unknown flags are errors), #645 + #644 (job records store resolved model/effort/sandbox; log reasoning start) + +**Files:** (via merge) `codex-companion.mjs` (`normalizeArgv`, every `handleX` gains an unknown-flag guard), `lib/args.mjs` (`parseArgs` strict mode), new `tests/args.test.mjs`; `lib/codex.mjs` (`runAppServerReview`/`runAppServerTurn` return `resolved: {model, effort, sandbox}`), `lib/tracked-jobs.mjs`, fixture, `tests/runtime.test.mjs`. + +- [ ] **Step 1: Merge in this order** (645 is the largest, last): + +```bash +for n in 547 644 645; do git fetch upstream pull/$n/head:pr/$n && git merge --no-ff --no-edit pr/$n || break; done +``` +Expected conflicts (all keep-both): `codex-companion.mjs` `handleReviewCommand` (547 adds a guard at the top, 688 changed the model line — keep both), `tests/runtime.test.mjs` test insertions, `tests/fake-codex-fixture.mjs` line ~313/347 (approval + lastThreadStart + 645's resolved-settings echo). + +**Semantic conflict checklist for #645 (Git may auto-merge these silently — verify by reading, not by trusting a clean merge):** +- `plugins/codex/scripts/lib/codex.mjs` `runAppServerTurn`: #645 rewrites the `const response = await startThread(...)` / `resumeThread(...)` hunks from a base that has no `approvalPolicy`; #426 added `approvalPolicy: options.approvalPolicy` to BOTH calls. After the merge both calls must still pass `approvalPolicy` (`rg -n approvalPolicy plugins/codex/scripts/lib/codex.mjs` must show it inside `runAppServerTurn` for start AND resume). +- `tests/fake-codex-fixture.mjs`: the `thread/start` / `thread/resume` handlers must simultaneously keep `lastThreadStart`/`lastThreadResume` (#688), the approval bookkeeping (#426), and #645's resolved-settings response. +- Run the PR-specific tests by name after the merge, not only the suite total: `node --import ./tests/test-env.mjs --test --test-name-pattern 'approval|on-request|alias|resolved|model selection' tests/runtime.test.mjs` → all pass. + +- [ ] **Step 2: Test** → `fail 0` (gate command from Global Constraints) and the name-pattern run above. + +- [ ] **Step 3: Smoke** + +Run: `node plugins/codex/scripts/codex-companion.mjs task --help 2>&1 | head -3; echo "exit=$?"` +Expected: usage text on stderr, non-zero exit, **no** Codex thread started. + +--- + +### Task 6: Merge hook hardening — #672, #668, #682, #396 — and fix the stop-gate timeout collision + +**Files:** (via merge) `plugins/codex/hooks/hooks.json` (SessionStart timeout 5→**60** — #672's test asserts exactly 60; do not "resolve" it to another value), `plugins/codex/scripts/session-lifecycle-hook.mjs` (`appendEnvVar` idempotent), `plugins/codex/scripts/stop-review-gate-hook.mjs` (fail closed on malformed stdin; `CODEX_REVIEW_GATE_MAX_ROUNDS`), `README.md`, tests. +- Modify (hand): `plugins/codex/scripts/stop-review-gate-hook.mjs` — timeout constants, `spawnSync` options, timeout message. + +**Interfaces:** +- Produces: `STOP_REVIEW_TIMEOUT_MINUTES = 13`, `STOP_REVIEW_TIMEOUT_MS = STOP_REVIEW_TIMEOUT_MINUTES * 60 * 1000` (< hooks.json `Stop.timeout: 900`); the `spawnSync` call uses `timeout: STOP_REVIEW_TIMEOUT_MS, killSignal: "SIGKILL", maxBuffer: 16 * 1024 * 1024`; the user-facing timeout message says `${STOP_REVIEW_TIMEOUT_MINUTES} minutes` (no literal "15 minutes" anywhere in the file). + +- [ ] **Step 1: Merge** + +```bash +for n in 672 668 682 396; do git fetch upstream pull/$n/head:pr/$n && git merge --no-ff --no-edit pr/$n || break; done +``` +#682 (`runStopReview` input validation) and #396 (round cap in `main`) both touch the stop hook's `main` flow — likely a clean textual merge, but verify the combined behaviour by reading: malformed stdin must be rejected BEFORE any round-state mutation, and the round counter must increase only on a real `block`. If #396's counter increments before #682's validation runs, reorder so validation comes first. + +- [ ] **Step 2: Failing test for the timeout ordering and message** + +Append to `tests/commands.test.mjs`: + +```js +test("stop gate script timeout is shorter than the Stop hook timeout and its message matches", () => { + const hooks = JSON.parse(fs.readFileSync(path.join(PLUGIN_ROOT, "hooks", "hooks.json"), "utf8")); + const stopTimeoutSeconds = hooks.hooks.Stop[0].hooks[0].timeout; + const source = fs.readFileSync(path.join(PLUGIN_ROOT, "scripts", "stop-review-gate-hook.mjs"), "utf8"); + const minutes = source.match(/const STOP_REVIEW_TIMEOUT_MINUTES = (\d+);/); + assert.ok(minutes, "STOP_REVIEW_TIMEOUT_MINUTES must be a named constant"); + assert.match(source, /const STOP_REVIEW_TIMEOUT_MS = STOP_REVIEW_TIMEOUT_MINUTES \* 60 \* 1000;/); + assert.ok(Number(minutes[1]) * 60 < stopTimeoutSeconds, "script timeout must be below the hook timeout"); + assert.doesNotMatch(source, /15 minutes/); + assert.match(source, /\$\{STOP_REVIEW_TIMEOUT_MINUTES\} minutes/); + assert.match(source, /killSignal: "SIGKILL"/); + assert.match(source, /maxBuffer: 16 \* 1024 \* 1024/); +}); +``` +(`fs`, `path`, `PLUGIN_ROOT` are already imported/defined at the top of `tests/commands.test.mjs`.) + +- [ ] **Step 3: Run it — expect FAIL** (no `STOP_REVIEW_TIMEOUT_MINUTES`, literal "15 minutes" present at ~line 116). + +Run: `node --import ./tests/test-env.mjs --test --test-name-pattern 'stop gate script timeout' tests/commands.test.mjs` + +- [ ] **Step 4: Fix** + +In `plugins/codex/scripts/stop-review-gate-hook.mjs`: replace `const STOP_REVIEW_TIMEOUT_MS = 15 * 60 * 1000;` with +```js +const STOP_REVIEW_TIMEOUT_MINUTES = 13; +const STOP_REVIEW_TIMEOUT_MS = STOP_REVIEW_TIMEOUT_MINUTES * 60 * 1000; +``` +In the `spawnSync(process.execPath, [...], { ... timeout: STOP_REVIEW_TIMEOUT_MS ... })` call add `killSignal: "SIGKILL", maxBuffer: 16 * 1024 * 1024` (the default 1 MiB `maxBuffer` kills a chatty child; `SIGTERM` can be ignored, `SIGKILL` cannot). Replace the literal `15 minutes` in the timeout message (~line 116) with `${STOP_REVIEW_TIMEOUT_MINUTES} minutes` (make that string a template literal if it isn't). + +- [ ] **Step 5: Test** → `fail 0`. + +- [ ] **Step 6: Commit** + +```bash +git add plugins/codex/scripts/stop-review-gate-hook.mjs tests/commands.test.mjs +git commit -m "fix(stop-gate): keep script timeout below the Stop hook timeout" -m "Co-Authored-By: Claude Fable 5 " +``` + +--- + +### Task 7: Per-thread `config` overrides — model, effort, and repeatable `--config key=value` + +Why: `ReviewStartParams` has no model/effort (#476/#651), `thread/start.model` is not reliably honoured (#408), but `ThreadStartParams.config` / `ThreadResumeParams.config` are. One place (`buildThreadParams`) fixes review + adversarial + task, and `--config` replaces the 555-line `CODEX_PLUGIN_CC_ARGS` PR (#419) for the per-run case. + +Codex-review rulings baked in (ledger 2026-08-27): (a) **resume never mirrors `--effort` into `thread/resume.config`** — on a cold resume `config.model_reasoning_effort` counts as a model override and cancels the persisted `model`/`model_provider` (app-server `has_model_resume_override`), so `task --resume-last --effort max` would silently switch models; effort on resume goes only through the existing `turn/start.effort`. Explicit `--config` pairs DO pass on resume (user asked for them). (b) **precedence**: generic `--config` first, dedicated `--model`/`--effort` override it, so `review` and `task` behave identically. (c) **native review sets `review_model` too**: Codex's `/review` honours a separate `review_model` override, so `--model` on `review` must set both `config.model` and `config.review_model`. (d) **parsing happens once, after `normalizeArgv`**, inside `parseArgs` (slash commands deliver `"$ARGUMENTS"` as ONE argv element; #547 makes unknown options errors) — a pre-pass collector cannot see `--config` and would then be rejected. (e) **prompt-taking commands stop option parsing at the first positional** (`task`, `adversarial-review` focus text): `task --effort max investigate ls -R usage` must keep `-R` in the prompt (#547 regression). + +**Files:** +- Modify: `plugins/codex/scripts/lib/args.mjs` — `parseArgs` gains `repeatableOptions` and `stopAtFirstPositional`. +- Modify: `plugins/codex/scripts/lib/codex.mjs` — `buildThreadConfig` (new, exported), `buildThreadParams`, `buildResumeParams`, `runAppServerReview`, `runAppServerTurn`. +- Modify: `plugins/codex/scripts/codex-companion.mjs` — `MODEL_ALIASES`; `parseConfigOverrides` (new); `handleReviewCommand`; `executeReviewRun`; `handleTask`; `buildTaskRequest`; `executeTaskRun`; `printUsage`. +- Modify: `tests/fake-codex-fixture.mjs` — store full `thread/start`/`thread/resume` params; compute the response's `model`/`reasoningEffort` from `params.model ?? params.config?.model` and `params.config?.model_reasoning_effort`. +- Test: `tests/args.test.mjs` (exists after #547), `tests/thread-config.test.mjs` (new), `tests/runtime.test.mjs`. + +**Interfaces:** +- Consumes: `normalizeRequestedModel(model)`, `normalizeReasoningEffort(effort)` (codex-companion.mjs ~103/~115); `normalizeArgv(argv)` (codex-companion.mjs ~140, splits a single `"$ARGUMENTS"` string); `parseArgs(argv, { valueOptions, booleanOptions?, … })` in `lib/args.mjs` as left by #547 — read it first. +- Produces: + - `parseArgs(argv, { …, repeatableOptions: ["config"], stopAtFirstPositional: true })` → `options.config` is `string[]` (each `key=value`), `--config=key=value` accepted, `--` ends option parsing, and with `stopAtFirstPositional` every token from the first positional on is a positional (no option parsing inside the prompt). + - `parseConfigOverrides(list: string[])` → `Record`; throws `--config expects key=value, got "".` when `=` is missing or the key is empty; later duplicates win. + - `buildThreadConfig({ model, effort, config, reviewModel })` → `{ ...config(parsed), model?, review_model?, model_reasoning_effort? } | null` (exported from `lib/codex.mjs`); dedicated keys are written AFTER the generic map. + - `buildThreadParams(cwd, options)` → adds `config: buildThreadConfig(options)`; `buildResumeParams(threadId, cwd, options)` → adds `config: buildThreadConfig({ config: options.config })` (no model, no effort). + - `runAppServerReview(cwd, { model, effort, config, … })` starts its thread with `{ model, effort, config, reviewModel: model }`; `runAppServerTurn(cwd, { model, effort, config, … })` passes `{ model, effort, config }` to `startThread` and `{ config }` to `resumeThread` (plus `effort` on `turn/start` as today). + - CLI: `review|adversarial-review [--effort ] [--config key=value]...`, `task [--config key=value]...`. + - `MODEL_ALIASES`: `spark→gpt-5.3-codex-spark`, `sol→gpt-5.6-sol`, `luna→gpt-5.6-luna`, `terra→gpt-5.6-terra`, `mini→gpt-5.4-mini`. + +- [ ] **Step 1: Failing unit tests for `buildThreadConfig`** + +Create `tests/thread-config.test.mjs`: + +```js +import test from "node:test"; +import assert from "node:assert/strict"; +import { buildThreadConfig } from "../plugins/codex/scripts/lib/codex.mjs"; + +test("buildThreadConfig returns null when nothing is set", () => { + assert.equal(buildThreadConfig({}), null); + assert.equal(buildThreadConfig({ config: {} }), null); +}); + +test("buildThreadConfig maps model, review model and effort to Codex config keys", () => { + assert.deepEqual(buildThreadConfig({ model: "gpt-5.6-sol", effort: "max", reviewModel: "gpt-5.6-sol" }), { + model: "gpt-5.6-sol", + review_model: "gpt-5.6-sol", + model_reasoning_effort: "max" + }); +}); + +test("buildThreadConfig lets dedicated flags win over generic overrides and parses JSON-ish values", () => { + assert.deepEqual( + buildThreadConfig({ + effort: "max", + config: { model_reasoning_effort: "low", "sandbox_workspace_write.network_access": "true", model_provider: "ollama", n: "3" } + }), + { "sandbox_workspace_write.network_access": true, model_provider: "ollama", n: 3, model_reasoning_effort: "max" } + ); +}); +``` + +- [ ] **Step 2: Run — expect FAIL** (`buildThreadConfig` not exported). + +Run: `node --import ./tests/test-env.mjs --test tests/thread-config.test.mjs` + +- [ ] **Step 3: Implement in `lib/codex.mjs`** (Serena: `insert_before_symbol` `buildThreadParams` for the helpers, `replace_symbol_body` on `buildThreadParams` and `buildResumeParams`) + +```js +function parseConfigValue(value) { + if (typeof value !== "string") { + return value; + } + try { + return JSON.parse(value); + } catch { + return value; + } +} + +export function buildThreadConfig({ model, effort, config, reviewModel } = {}) { + const merged = {}; + for (const [key, value] of Object.entries(config ?? {})) { + merged[key] = parseConfigValue(value); + } + if (model) { + merged.model = model; + } + if (reviewModel) { + merged.review_model = reviewModel; + } + if (effort) { + merged.model_reasoning_effort = effort; + } + return Object.keys(merged).length > 0 ? merged : null; +} + +function buildThreadParams(cwd, options = {}) { + return { + cwd, + model: options.model ?? null, + approvalPolicy: options.approvalPolicy ?? "never", + sandbox: options.sandbox ?? "read-only", + config: buildThreadConfig(options), + serviceName: SERVICE_NAME, + ephemeral: options.ephemeral ?? true + }; +} +``` +In `buildResumeParams` add `config: buildThreadConfig({ config: options.config }),` next to `sandbox` — **only** the generic overrides; never `model`/`effort` (resume ruling above). + +- [ ] **Step 4: Thread the options through `lib/codex.mjs`** — `runAppServerReview`: the `startThread(client, cwd, { model: options.model, sandbox: "read-only", … })` call gets `effort: options.effort, config: options.config, reviewModel: options.model`. `runAppServerTurn`: the `startThread(...)` call gets `effort: options.effort, config: options.config`; the `resumeThread(...)` call gets `config: options.config` only. Keep `effort: options.effort ?? null` on `turn/start` as today. + +- [ ] **Step 5: Unit test passes** + +Run: `node --import ./tests/test-env.mjs --test tests/thread-config.test.mjs` → 3 pass. + +- [ ] **Step 6: Failing parser tests** — append to `tests/args.test.mjs` (created by #547; it imports `parseArgs` from `../plugins/codex/scripts/lib/args.mjs`): + +```js +test("parseArgs collects repeatable options and honours -- and --opt=value", () => { + const { options, positionals } = parseArgs( + ["--config", "a=1", "--config=b=x=y", "--model", "sol", "--", "--not-an-option", "tail"], + { valueOptions: ["model"], repeatableOptions: ["config"] } + ); + assert.deepEqual(options.config, ["a=1", "b=x=y"]); + assert.equal(options.model, "sol"); + assert.deepEqual(positionals, ["--not-an-option", "tail"]); +}); + +test("parseArgs with stopAtFirstPositional keeps option-looking prompt words", () => { + const { options, positionals } = parseArgs( + ["--effort", "max", "investigate", "ls", "-R", "usage", "--model", "x"], + { valueOptions: ["effort", "model"], stopAtFirstPositional: true } + ); + assert.equal(options.effort, "max"); + assert.equal(options.model, undefined); + assert.deepEqual(positionals, ["investigate", "ls", "-R", "usage", "--model", "x"]); +}); + +test("parseArgs rejects a repeatable option without a value", () => { + assert.throws(() => parseArgs(["--config"], { repeatableOptions: ["config"] }), /--config/); +}); +``` + +- [ ] **Step 7: Run — expect FAIL.** `node --import ./tests/test-env.mjs --test tests/args.test.mjs` + +- [ ] **Step 8: Extend `parseArgs` in `lib/args.mjs`** — read the post-#547 implementation first and add, following its existing style: `config.repeatableOptions` (array of names; each occurrence pushes onto `options[name]`, `--name=value` form included, missing value throws the same error shape #547 uses for missing values), a `--` sentinel (everything after it is positional), and `config.stopAtFirstPositional` (once a token is not an option, all remaining tokens are positionals). Do not change behaviour for callers that pass neither new key. + +- [ ] **Step 9: Parser tests pass.** + +- [ ] **Step 10: Failing runtime tests** — append to `tests/runtime.test.mjs` (helpers `makeTempDir`, `installFakeCodex`, `initGitRepo`, `run`, `buildEnv`, `SCRIPT` exist at the top of the file): + +```js +function seededRepo() { + const repo = makeTempDir(); + 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 }); + return repo; +} + +test("review forwards model, review_model, effort and config overrides into thread/start config", () => { + const repo = seededRepo(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + fs.writeFileSync(path.join(repo, "README.md"), "hello world\n"); + + const result = run( + "node", + [SCRIPT, "review", "--wait", "--model", "sol", "--effort", "max", "--config", "model_provider=ollama", "--config", "foo.bar=3"], + { cwd: repo, env: buildEnv(binDir) } + ); + + assert.equal(result.status, 0, result.stderr); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.deepEqual(fakeState.lastThreadStart.config, { + model_provider: "ollama", + "foo.bar": 3, + model: "gpt-5.6-sol", + review_model: "gpt-5.6-sol", + model_reasoning_effort: "max" + }); +}); + +test("review accepts slash-command style single-string arguments", () => { + const repo = seededRepo(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + fs.writeFileSync(path.join(repo, "README.md"), "hello world\n"); + + const result = run("node", [SCRIPT, "review", "--wait --effort xhigh --config model_provider=ollama"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.deepEqual(fakeState.lastThreadStart.config, { model_provider: "ollama", model_reasoning_effort: "xhigh" }); +}); + +test("task forwards config overrides and keeps option-looking prompt words", () => { + const repo = seededRepo(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + + const result = run("node", [SCRIPT, "task", "--effort", "max", "--config", "model_provider=ollama", "investigate", "ls", "-R", "usage"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.deepEqual(fakeState.lastThreadStart.config, { model_provider: "ollama", model_reasoning_effort: "max" }); + assert.match(JSON.stringify(fakeState.lastTurnStart.input), /investigate ls -R usage/); +}); + +test("task --resume-last never puts model or effort into thread/resume config", () => { + const repo = seededRepo(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + + const first = run("node", [SCRIPT, "task", "--model", "sol", "--effort", "high", "first"], { cwd: repo, env: buildEnv(binDir) }); + assert.equal(first.status, 0, first.stderr); + const second = run("node", [SCRIPT, "task", "--resume-last", "--effort", "max", "--config", "model_provider=ollama", "again"], { + cwd: repo, + env: buildEnv(binDir) + }); + assert.equal(second.status, 0, second.stderr); + + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.deepEqual(fakeState.lastThreadResume.config, { model_provider: "ollama" }); + assert.equal(fakeState.lastTurnStart.effort, "max"); +}); + +test("task --background stores config overrides in the job request", () => { + const repo = seededRepo(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + + const result = run("node", [SCRIPT, "task", "--background", "--json", "--config", "model_provider=ollama", "bg"], { + cwd: repo, + env: buildEnv(binDir) + }); + assert.equal(result.status, 0, result.stderr); + const jobId = JSON.parse(result.stdout).jobId; + const done = run("node", [SCRIPT, "status", jobId, "--wait", "--timeout-ms", "20000", "--json"], { cwd: repo, env: buildEnv(binDir) }); + assert.equal(done.status, 0, done.stderr); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.deepEqual(fakeState.lastThreadStart.config, { model_provider: "ollama" }); +}); +``` +(If the fixture's existing `thread/resume` handler does not record `lastThreadResume`, or `status --wait --json` has a different output shape, adapt the test to what `tests/runtime.test.mjs` already does for `--resume-last` and `--background` — copy the surrounding test's mechanics, keep these assertions.) + +- [ ] **Step 11: Run — expect FAIL.** + +- [ ] **Step 12: Implement the CLI side in `codex-companion.mjs` — exact call chain (do every item):** + +1. `MODEL_ALIASES`: +```js +const MODEL_ALIASES = new Map([ + ["spark", "gpt-5.3-codex-spark"], + ["sol", "gpt-5.6-sol"], + ["luna", "gpt-5.6-luna"], + ["terra", "gpt-5.6-terra"], + ["mini", "gpt-5.4-mini"] +]); +``` +2. Insert after `normalizeReasoningEffort`: +```js +function parseConfigOverrides(list = []) { + const config = {}; + for (const pair of list) { + const eq = pair.indexOf("="); + if (eq <= 0) { + throw new Error(`--config expects key=value, got "${pair}".`); + } + config[pair.slice(0, eq)] = pair.slice(eq + 1); + } + return config; +} +``` +3. `handleReviewCommand(argv, config)` (~line 712): its `parseArgs(normalizeArgv(argv), { valueOptions: [...] })` call gets `"effort"` added to `valueOptions`, `repeatableOptions: ["config"]`, and — for the adversarial variant, which takes focus text — `stopAtFirstPositional: true` (native `review` takes no positionals; keep strict there). Next to `const model = normalizeRequestedModel(options.model);` add `const effort = normalizeReasoningEffort(options.effort);` and `const configOverrides = parseConfigOverrides(options.config);`. Put `effort` and `config: configOverrides` into the request object built at ~line 742 (the one that already carries `model`). +4. `executeReviewRun(request)` (~line 358): pass `effort: request.effort, config: request.config` into BOTH the native call `runAppServerReview(cwd, { model: request.model, … })` (~line 370) and the adversarial call `runAppServerTurn(cwd, { model: request.model, … })` (~line 411). +5. `handleTask(argv)` (~line 767): `parseArgs` call gets `repeatableOptions: ["config"]` and `stopAtFirstPositional: true`; compute `const configOverrides = parseConfigOverrides(options.config);`; pass `config: configOverrides` into BOTH the background request (~line 793) and the foreground request (~line 811). +6. `buildTaskRequest(...)` (~lines 604–613): add `config` to its parameters and to the returned object (otherwise the stored background job loses it; `handleTaskWorker` ~line 875 already spreads the stored request). +7. `executeTaskRun(request)` (~line 485): pass `config: request.config` into `runAppServerTurn`. +8. `printUsage()`: add `[--effort ] [--config key=value]...` to the review/adversarial-review lines, `[--config key=value]...` to the task line, and `sol|luna|terra|mini` next to `spark` in every `--model ` hint. + +- [ ] **Step 12b: Decline form-mode MCP elicitations (Codex merge-review finding)** — #501 auto-accepts every `mcpServer/elicitation/request` with `content: null`; for `params.mode === "form"` or `"openai/form"` the app-server contract requires structured content, so the tool call fails or proceeds without the requested values. First append to `tests/app-server.test.mjs` (it already exercises `handleServerRequest` for the URL case — copy its mechanics): + +```js +test("form-mode elicitation requests are declined instead of accepted with empty content", () => { + // Arrange exactly like the existing accept test, but with params.mode = "form". + // Assert the reply is { action: "decline" } (no content), and that mode "url" still gets { action: "accept", content: null, _meta: null }. +}); +``` +Fill the body by mirroring the existing test's setup for the request/response capture. Then in `plugins/codex/scripts/lib/app-server.mjs` `handleServerRequest`: when `message.params?.mode === "form" || message.params?.mode === "openai/form"` respond with `{ action: "decline" }`; keep the accept path for every other mode. Run: `node --import ./tests/test-env.mjs --test tests/app-server.test.mjs` → all pass. + +- [ ] **Step 13: Fixture** — in `tests/fake-codex-fixture.mjs`: `thread/start` stores the full params as `state.lastThreadStart = params` (keep whatever #688/#426/#645 already record alongside), `thread/resume` stores `state.lastThreadResume = params`; the `ThreadStartResponse`/`ThreadResumeResponse` it fabricates derive `model` from `params.model ?? params.config?.model ?? ` and `reasoningEffort` from `params.config?.model_reasoning_effort ?? ` so #645's `resolved` reflects config precedence. + +- [ ] **Step 14: Test** → `fail 0` (gate command). + +- [ ] **Step 15: Docs** — `plugins/codex/commands/{review,adversarial-review}.md` `argument-hint`: add `[--effort ] [--config key=value]`; `rescue.md` + `task` line: `[--model ]`, `[--config key=value]`; `skills/codex-cli-runtime/SKILL.md`: one line "`--config key=value` (repeatable) forwards a `config.toml` override to the Codex thread (`thread/start.config`; on `--resume-last` only these overrides are sent, model/effort are not re-applied), e.g. `--config model_provider=ollama`". README "Notes" bullet for aliases + `--config` + the resume rule. Update `tests/commands.test.mjs` regexes that assert the old `--model ` text if they now fail. + +- [ ] **Step 16: Commit** + +```bash +git add plugins/codex tests README.md +git commit -m "feat: per-thread config overrides (--config), effort on reviews, gpt-5.6 model aliases" -m "Model and reasoning effort are sent via thread/start.config (ReviewStartParams has no such fields; thread/start.model is unreliable). Closes upstream #476 #651 #468 #408 for this fork." -m "Co-Authored-By: Claude Fable 5 " +``` + +--- + +### Task 8: `/codex:rescue` returns a result — synchronous path without `Agent`, visible failures, model aliases, agent-compat hint + +Why (Codex-review ruling): since Claude Code 2.1.232 every `Agent` subagent runs in the background and the caller gets "Async agent launched…" — the 11/12 placeholder results in Aug 2026 were host behaviour, not the plugin's `task --background`. #608 (merged in Task 4) only makes the agent's inner Bash foreground; it cannot make the outer `Agent` synchronous. So the default (`--wait`) rescue path must not go through `Agent` at all: the slash command runs the companion inline via Bash — `task --background` (returns a job id immediately, keeps the 10-min Bash cap out of the way), then `status --wait` in ≤9-minute slices until the job finishes, then `result `. `Agent` is used only when the user asks for `--background`. Separately, the agent's "if Bash fails return nothing" rule turned auth/timeout failures into silent losses — failures must be visible. + +**Files:** +- Modify: `plugins/codex/commands/rescue.md` (body: inline synchronous flow; `Agent` only for `--background`) +- Modify: `plugins/codex/agents/codex-rescue.md` (remove `model:` line; failure reporting) +- Modify: `plugins/codex/skills/codex-cli-runtime/SKILL.md` (aliases; failure rule; agent-compat hint) +- Test: `tests/commands.test.mjs` + +**Interfaces:** +- Consumes: companion CLI `task --background --json` → stdout JSON with `jobId`; `status --wait --timeout-ms --json` (exits 0 when the job reached a terminal state, non-zero on wait timeout — check `handleStatus` in `codex-companion.mjs` for the exact exit code and reuse it); `result `. +- Produces: `commands/rescue.md` body below; `agents/codex-rescue.md` without a `model:` key (omission = inherit, and the docs say `CLAUDE_CODE_SUBAGENT_MODEL`/per-call `model` can still override — do not claim otherwise); SKILL.md sentences the test asserts. + +- [ ] **Step 1: Failing test** — first open `tests/commands.test.mjs` and find the assertions #608/#616 left for the rescue command, agent and skill (`rescue command absorbs continue semantics`); update any assertion that contradicts the new contract (e.g. regexes requiring the `Agent` tool for the default path, or `return nothing`) rather than deleting the test. Then append: + +```js +test("rescue runs synchronously through the companion and uses Agent only for --background", () => { + const rescue = fs.readFileSync(path.join(PLUGIN_ROOT, "commands", "rescue.md"), "utf8"); + const agent = fs.readFileSync(path.join(PLUGIN_ROOT, "agents", "codex-rescue.md"), "utf8"); + const runtimeSkill = fs.readFileSync(path.join(PLUGIN_ROOT, "skills", "codex-cli-runtime", "SKILL.md"), "utf8"); + assert.match(rescue, /task --background --json/); + assert.match(rescue, /status "\$JOB" --wait --timeout-ms 540000/); + assert.match(rescue, /result "\$JOB"/); + assert.match(rescue, /Only when the request contains `--background`.*Agent/s); + assert.doesNotMatch(agent, /^model:/m); + assert.doesNotMatch(agent, /return nothing/i); + assert.match(agent, /exit status and stderr/i); + assert.doesNotMatch(runtimeSkill, /return nothing/i); + assert.match(runtimeSkill, /Map `sol` to `--model gpt-5\.6-sol`/i); + assert.match(runtimeSkill, /\$agent-compat:skill-router/); +}); +``` + +- [ ] **Step 2: Run — expect FAIL.** `node --import ./tests/test-env.mjs --test --test-name-pattern 'rescue' tests/commands.test.mjs` + +- [ ] **Step 3: Rewrite `plugins/codex/commands/rescue.md`** — keep the frontmatter's `description` and `allowed-tools: Bash(node:*), AskUserQuestion, Agent`; set `argument-hint: "[--background|--wait] [--resume|--fresh] [--model ] [--effort ] [--config key=value]... [what Codex should investigate, solve, or continue]"`. Body: + +````markdown +Delegate the request to Codex through the shared companion runtime. Default is synchronous: the user gets Codex's answer in this turn. + +1. Strip `--wait` if present (it is the default). If the request contains `--resume`, use `task --resume-last`; if `--fresh`, use a fresh `task`; otherwise run `node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" task-resume-candidate --json` and follow its recommendation. Pass `--model`, `--effort` and every `--config key=value` through unchanged. Never add `--write` unless the user explicitly asked Codex to modify files. + +2. Start the job and wait for it, in ≤9-minute slices so the Bash tool's 10-minute cap never kills a long run: + +```bash +JOB=$(node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" task --background --json "" | node -e 'let d="";process.stdin.on("data",c=>d+=c).on("end",()=>process.stdout.write(JSON.parse(d).jobId))') +until node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" status "$JOB" --wait --timeout-ms 540000 --json >/dev/null; do :; done +node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" result "$JOB" +``` +Run the `until` loop as one Bash call with `timeout: 600000`; if it returns because Bash timed out, run the same `until … done` line again — the job keeps running in the background. Show the `result` output to the user verbatim, then add your own assessment. + +3. Only when the request contains `--background`: invoke the `codex:codex-rescue` subagent via the `Agent` tool (`subagent_type: "codex:codex-rescue"`, prompt = the raw request minus `--background`) and tell the user the job id will arrive as a completion notification; they can also run `/codex:status` and `/codex:result `. + +Do not call `Skill(codex:rescue)` from here (it re-enters this command). If any Bash step exits non-zero, show its stderr to the user — never report "no result". +```` + +- [ ] **Step 4: Edit `agents/codex-rescue.md`** — delete the `model: sonnet` line entirely. Replace the sentence that says to return nothing when the Bash call fails with: "If the Bash call fails or Codex cannot be invoked, return the command's exit status and stderr verbatim so the failure is visible; never return an empty result." Also (Codex merge-review P1): the agent's single foreground `task` Bash call dies at the Bash tool's 10-minute cap even when the agent itself runs in the background, losing long results and leaving stale jobs. Replace the "exactly one foreground Bash call" instruction with the same detached pattern the slash command uses — `task --background --json` to get `jobId`, then `status "$JOB" --wait --timeout-ms 540000 --json` in a loop until it exits 0, then `result "$JOB"` — and delete the rule that forbids the agent from polling `status`/`result` (it may poll its own job only). Update the corresponding `tests/commands.test.mjs` assertions (the #608 regexes about the foreground inner Bash call) to the new contract. Add to the Step 1 test: `assert.match(agent, /task --background --json/); assert.match(agent, /status "\$JOB" --wait --timeout-ms 540000/); assert.doesNotMatch(agent, /Do not .*poll status/i);` + +- [ ] **Step 5: Edit `skills/codex-cli-runtime/SKILL.md`** — (a) after the line `Map \`spark\` to \`--model gpt-5.3-codex-spark\`…` add `- Map \`sol\` to \`--model gpt-5.6-sol\`, \`luna\` to \`--model gpt-5.6-luna\`, \`terra\` to \`--model gpt-5.6-terra\`, \`mini\` to \`--model gpt-5.4-mini\`.`; (b) replace the "return nothing" failure rule with the same visible-failure sentence as the agent; (c) add under "Command selection": `- If the request names an uncommon domain (private infra runbooks, vendor-specific tooling), prepend to the task text: "If a matching skill is not already loaded, run \`$agent-compat:skill-router\` to find a reviewed playbook before starting." (Codex has the agent-compat plugin installed; it routes to reviewed route-only skills offline.)` + +- [ ] **Step 6: Test** → `fail 0` (gate command). Also `rg -n 'return nothing' plugins/codex` → no output. + +- [ ] **Step 7: Commit** + +```bash +git add plugins/codex/commands/rescue.md plugins/codex/agents/codex-rescue.md plugins/codex/skills/codex-cli-runtime/SKILL.md tests/commands.test.mjs +git commit -m "feat(rescue): synchronous inline path (no Agent), visible failures, gpt-5.6 aliases, agent-compat hint" -m "Claude Code >=2.1.232 runs every Agent subagent in the background, so the default rescue path now drives the companion directly: task --background, status --wait slices, result." -m "Co-Authored-By: Claude Fable 5 " +``` + +--- + +### Task 9: Release v1.1.0 — marketplace `cbepx`, CHANGELOG, version bump, tag, install + +**Files:** +- Modify: `.claude-plugin/marketplace.json` (`name: "cbepx"`, `owner: {name: "CBEPX", url: "https://github.com/CBEPX"}`, version) +- Modify: `plugins/codex/.claude-plugin/plugin.json` (version via `npm run bump-version`; keep `name: "codex"`) +- Modify: `package.json` (`name: "@cbepx/codex-plugin-cc"`, version) +- Modify: `README.md` (fork notice at top) +- Create: `CHANGELOG.md` (repo root; upstream has none) +- Test: `tests/bump-version.test.mjs` (existing — must stay green), `tests/commands.test.mjs` + +- [ ] **Step 1: Failing test — marketplace identity** + +Append to `tests/commands.test.mjs`: + +```js +test("marketplace is published under cbepx while the plugin keeps the codex name", () => { + const marketplace = JSON.parse(fs.readFileSync(path.join(ROOT, ".claude-plugin", "marketplace.json"), "utf8")); + const plugin = JSON.parse(fs.readFileSync(path.join(PLUGIN_ROOT, ".claude-plugin", "plugin.json"), "utf8")); + assert.equal(marketplace.name, "cbepx"); + assert.equal(marketplace.owner.name, "CBEPX"); + assert.equal(plugin.name, "codex"); + assert.equal(marketplace.plugins[0].name, "codex"); + assert.equal(marketplace.plugins[0].version, plugin.version); +}); +``` +(`ROOT` is defined at the top of `tests/commands.test.mjs` as the repo root; if it is named differently there, use that name.) + +- [ ] **Step 2: Run — expect FAIL** (`openai-codex`). + +- [ ] **Step 3: Bump + rename** + +```bash +npm run bump-version -- 1.1.0 && npm run check-version +``` +Then edit `.claude-plugin/marketplace.json`: `"name": "cbepx"`, `"owner": { "name": "CBEPX", "url": "https://github.com/CBEPX" }`, `metadata.description`: `"CBEPX fork of the OpenAI Codex plugin for Claude Code: max/ultra effort, per-thread config overrides, gpt-5.6 aliases, rescue agent fixes."`. `package.json` `"name": "@cbepx/codex-plugin-cc"`. + +- [ ] **Step 4: `CHANGELOG.md`** + +```markdown +# Changelog + +## 1.1.0 — 2026-08-27 + +Fork of [openai/codex-plugin-cc](https://github.com/openai/codex-plugin-cc) 1.0.6 (`db52e28`). Marketplace `cbepx`, plugin name unchanged (`codex`). + +### Merged from upstream pull requests +- #616 accept `max` and `ultra` reasoning efforts +- #688 resolve model aliases on `review` / `adversarial-review` +- #426 `on-request` approval policy for `--write` task runs +- #501 answer MCP elicitation requests instead of rejecting them +- #608 rescue agent awaits the delegated result instead of returning a placeholder +- #690 explicit Bash blocks in `status`/`result`/`cancel`/`transfer` commands (pass permission classifiers) +- #547 `task --help` and unknown flags are CLI errors, never a prompt +- #645 / #644 job records store resolved model/effort/sandbox; reasoning start is logged +- #672 SessionStart hook timeout raised; #668 idempotent `CLAUDE_ENV_FILE` exports; #682 stop gate fails closed on malformed input; #396 `CODEX_REVIEW_GATE_MAX_ROUNDS` + +### Fork changes +- Model and reasoning effort are sent per thread via `thread/start.config` (`model`, `review_model` for native review, `model_reasoning_effort`); generic `--config` pairs are applied first, dedicated flags override them; `--effort` now works on `review` and `adversarial-review`. +- Repeatable `--config key=value` on `task`, `review`, `adversarial-review` forwards any `config.toml` override to the thread (values are JSON-parsed; quote a literal string as `'"true"'`). Prompt-taking commands stop option parsing at the first positional, so prompt text like `ls -R` is never mis-parsed. +- `--resume-last` opens a fresh app-server session (cold resume) so `--config`, sandbox and approval policy take effect, and never sends `model` on `thread/resume` (it would drop the persisted model); the resumed turn's model/effort ride on `turn/start`. +- Form-mode MCP elicitations (`mode: form` / `openai/form`) are declined instead of being accepted with empty content; URL-mode elicitations are still accepted. +- `/codex:rescue` is synchronous by default without the `Agent` tool: `task --background` → `status --wait` in ≤9-minute slices (job id carried literally between Bash calls) → `result`; launch failures stop immediately with visible stderr; `Agent` only for `--background`; `--write` is never added unless the user explicitly asked Codex to modify files. +- Model aliases: `sol`, `luna`, `terra`, `mini` (plus `spark`); rescue agent has no pinned `model:`; runtime skill mentions `$agent-compat:skill-router` for uncommon domains. +- Stop-gate script timeout (13 min) is below the hook timeout (15 min); `spawnSync` uses `SIGKILL` and a 16 MiB buffer. +- Hermetic test environment (`tests/test-env.mjs`); CI on push; `npm run build` type-checks the JSDoc. + +## 1.0.6 and earlier +See upstream releases: https://github.com/openai/codex-plugin-cc/releases +``` + +- [ ] **Step 5: README fork notice** — insert after the first heading: + +```markdown +> **CBEPX fork.** Install with `claude plugin marketplace add CBEPX/codex-plugin-cc` then `claude plugin install codex@cbepx`. Differences from upstream are listed in [CHANGELOG.md](CHANGELOG.md). Upstream: openai/codex-plugin-cc. +``` + +- [ ] **Step 6: Test** → `fail 0` (including `bump-version.test.mjs`). + +- [ ] **Step 7: Validate the plugin manifest** + +Run: `claude plugin validate . --strict 2>&1 | tail -3` +Expected: no errors. + +- [ ] **Step 8: Commit, tag, push** + +```bash +git add -A && git commit -m "chore(release): v1.1.0 — cbepx marketplace, changelog" -m "Co-Authored-By: Claude Fable 5 " +git tag -a v1.1.0 -m "v1.1.0" +git push -u origin release/v1.1.0 --tags +``` + +- [ ] **Step 9: Merge to main via PR on the fork** (keeps CI history) + +```bash +gh pr create -R CBEPX/codex-plugin-cc --base main --head release/v1.1.0 --title "release: v1.1.0" --body "$(sed -n '3,40p' CHANGELOG.md)" +``` +Wait for CI (`gh pr checks --watch`), then `gh pr merge --merge` and `git checkout main && git pull`. + +- [ ] **Step 10: Switch the local Claude Code install** + +```bash +claude plugin marketplace add CBEPX/codex-plugin-cc +claude plugin install codex@cbepx -y +claude plugin disable codex@openai-codex +``` +Then in `~/.claude/settings.json` confirm `enabledPlugins["codex@cbepx"] === true` and `codex@openai-codex === false` (uninstall the upstream copy only after one successful `/codex:status` from the new install). + +--- + +## Verification (end-to-end, in a fresh Claude Code session inside `~/project/infra` or any git repo) + +1. `/codex:status` — renders (command body is an explicit Bash block; no classifier prompt). +2. `/codex:rescue --model sol --effort max Strictly read-only: summarize the last commit` — the answer arrives **in the same turn** (inline companion path, no `Agent`); `/codex:status --all` shows the job with `model: gpt-5.6-sol`, `effort: max` (resolved fields from #645). +3. `node ~/.claude/plugins/cache/cbepx/codex/1.1.0/scripts/codex-companion.mjs review --wait --model sol --effort xhigh --json` → the job record's resolved model is `gpt-5.6-sol` and effort `xhigh` even with a conflicting `review_model` set in `~/.codex/config.toml` for the test (set it temporarily, then remove). +4. Cold resume: `task --model sol --effort high "first"` then `task --resume-last --effort max "again"` → `status --json` of the second job still reports model `gpt-5.6-sol` (not the config default) and effort `max`. +5. `node … task --effort supreme x` → `Unsupported reasoning effort "supreme". Use one of: … max, ultra.`; `task --effort max investigate ls -R usage` → prompt reaches Codex intact (no `Unknown option: -R`). +6. `/codex:setup --enable-review-gate` in a scratch repo, make an edit, stop → gate runs and finishes < 13 min or reports its own "13 minutes" timeout message. +7. `npm test` inside the Claude session → `fail 0` (exit-status-checked, never via a pipe). diff --git a/package-lock.json b/package-lock.json index 0c919c3db..224af69a7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { - "name": "@openai/codex-plugin-cc", - "version": "1.0.6", + "name": "@cbepx/codex-plugin-cc", + "version": "1.1.0", "lockfileVersion": 3, "requires": true, "packages": { "": { - "name": "@openai/codex-plugin-cc", - "version": "1.0.6", + "name": "@cbepx/codex-plugin-cc", + "version": "1.1.0", "license": "Apache-2.0", "devDependencies": { "@types/node": "^25.5.0", diff --git a/package.json b/package.json index b1d984d1a..9422ccaac 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { - "name": "@openai/codex-plugin-cc", - "version": "1.0.6", + "name": "@cbepx/codex-plugin-cc", + "version": "1.1.0", "private": true, "type": "module", "description": "Use Codex from Claude Code to review code or delegate tasks.", @@ -13,7 +13,7 @@ "check-version": "node scripts/bump-version.mjs --check", "prebuild": "mkdir -p plugins/codex/.generated/app-server-types && codex app-server generate-ts --out plugins/codex/.generated/app-server-types", "build": "tsc -p tsconfig.app-server.json", - "test": "node --test tests/*.test.mjs" + "test": "node --import ./tests/test-env.mjs --test tests/*.test.mjs" }, "devDependencies": { "@types/node": "^25.5.0", diff --git a/plugins/codex/.claude-plugin/plugin.json b/plugins/codex/.claude-plugin/plugin.json index e91e5238c..4838522f0 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", + "version": "1.1.0", "description": "Use Codex from Claude Code to review code or delegate tasks.", "author": { "name": "OpenAI" diff --git a/plugins/codex/agents/codex-rescue.md b/plugins/codex/agents/codex-rescue.md index 7009ec86a..08fdbbc9b 100644 --- a/plugins/codex/agents/codex-rescue.md +++ b/plugins/codex/agents/codex-rescue.md @@ -1,7 +1,6 @@ --- name: codex-rescue description: Proactively use when Claude Code is stuck, wants a second implementation or diagnosis pass, needs a deeper root-cause investigation, or should hand a substantial coding task to Codex through the shared runtime -model: sonnet tools: Bash skills: - codex-cli-runtime @@ -19,27 +18,58 @@ Selection guidance: Forwarding rules: -- Use exactly one `Bash` call to invoke `node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" task ...`. -- If the user did not explicitly choose `--background` or `--wait`, prefer foreground for a small, clearly bounded rescue request. -- If the user did not explicitly choose `--background` or `--wait` and the task looks complicated, open-ended, multi-step, or likely to keep Codex running for a long time, prefer background execution. +- Launch the job, then wait for it — two separate Bash calls, in ≤9-minute wait slices so the Bash tool's 10-minute cap never kills a long run. Bash calls share no variables — always set `JOB=` literally at the top of every later call; never rely on a `$JOB` left over from a previous call. + + The request prose and the runtime flags travel in two separate channels of the same Bash call: the prose is written byte-exact to `$PROMPT` by its own quoted heredoc and passed as `--prompt-file`, while the `--args-stdin` heredoc carries only the runtime flags (`--model`, `--effort`, `--config key=value`, `--resume-last`, `--write` as applicable). Never put the request text in the flags heredoc: it is tokenized, so quotes, backslashes and newlines in a stack trace, a regex or a code block would be mangled. Give both heredoc delimiters a fresh random suffix on every call — `CODEX_PROMPT_` / `CODEX_ARGS_`, e.g. 8 hex characters — and never reuse a suffix that appears in the request text: a payload line equal to the delimiter would end the heredoc early and run the rest on the host shell. + + Launch (one Bash call): + +```bash +trap 'rm -f "$ERR" "$OUT" "$PROMPT"' EXIT +ERR=$(mktemp); PROMPT=$(mktemp) +cat > "$PROMPT" <<'CODEX_PROMPT_' + +CODEX_PROMPT_ +JOB=$(node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" task --background --json --prompt-file "$PROMPT" --args-stdin <<'CODEX_ARGS_' 2>"$ERR" | node -e 'let d="";process.stdin.on("data",c=>d+=c).on("end",()=>{try{process.stdout.write(JSON.parse(d).jobId||"")}catch(e){}})' + +CODEX_ARGS_ +) +[ -n "$JOB" ] || { cat "$ERR"; exit 1; } +[[ "$JOB" =~ ^[A-Za-z0-9_-]+$ ]] || { echo "invalid job id"; exit 1; } +echo "JOB=$JOB" +``` + If this call exits non-zero, its output is the launch failure — return it verbatim and stop; never return an empty result. Otherwise its last line is `JOB=`; read `` from it. + + Wait and fetch the result (one Bash call, tool `timeout: 600000`). Set `JOB=` literally as the first line, using the id you just read: + +```bash +trap 'rm -f "$ERR" "$OUT" "$PROMPT"' EXIT +JOB= +[[ "$JOB" =~ ^[A-Za-z0-9_-]+$ ]] || { echo "invalid job id"; exit 1; } +OUT=$(mktemp); ERR=$(mktemp) +while node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" status "$JOB" --wait --timeout-ms 540000 --json >"$OUT" 2>"$ERR"; node -e 'let d="";process.stdin.on("data",c=>d+=c).on("end",()=>{try{const s=(JSON.parse(d).job||{}).status;process.exit(s==="queued"||s==="running"?3:(s?0:2))}catch(e){process.exit(2)}})' < "$OUT"; rc=$?; [ "$rc" -eq 3 ]; do sleep 1; done +[ "$rc" -eq 0 ] || { cat "$OUT" "$ERR"; exit "$rc"; } +node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" result "$JOB" +``` + Exits 3 while the job is `queued`/`running` (loop, with `sleep 1` so it can't spin hot), 0 on a terminal status, 2 if the status output is empty or unparseable — either non-3 outcome ends the loop. If this call is cut off by the tool's own timeout, the job keeps running server-side — run it again with the same literal `JOB=` line. If it exits non-zero, return its output verbatim and stop; never return an empty result. Otherwise return the `result` stdout as-is. +- You may check this job's own `status` and fetch its `result` to carry out the launch/wait above; do not inspect the repository, read files, grep, cancel jobs, summarize output, or do any other follow-up work of your own. - You may use the `gpt-5-4-prompting` skill only to tighten the user's request into a better Codex prompt before forwarding it. - Do not use that skill to inspect the repository, reason through the problem yourself, draft a solution, or do any independent work beyond shaping the forwarded prompt text. -- 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. -- Do not call `review`, `adversarial-review`, `status`, `result`, or `cancel`. This subagent only forwards to `task`. +- Do not call `review`, `adversarial-review`, or `cancel`. This subagent only forwards to `task` and checks its own job's `status`/`result`. - Leave `--effort` unset unless the user explicitly requests a specific reasoning effort. - Leave model unset by default. Only add `--model` when the user explicitly asks for a specific model. - If the user asks for `spark`, map that to `--model gpt-5.3-codex-spark`. - If the user asks for a concrete model name such as `gpt-5.4-mini`, pass it through with `--model`. -- Treat `--effort ` and `--model ` as runtime controls and do not include them in the task text you pass through. -- Default to a write-capable Codex run by adding `--write` unless the user explicitly asks for read-only behavior or only wants review, diagnosis, or research without edits. +- Treat `--effort `, `--model `, and `--config key=value` as runtime controls and do not include them in the task text you pass through. +- Never add `--write` unless the user explicitly asked Codex to modify files. - Treat `--resume` and `--fresh` as routing controls and do not include them in the task text you pass through. - `--resume` means add `--resume-last`. - `--fresh` means do not add `--resume-last`. - If the user is clearly asking to continue prior Codex work in this repository, such as "continue", "keep going", "resume", "apply the top fix", or "dig deeper", add `--resume-last` unless `--fresh` is present. - Otherwise forward the task as a fresh `task` run. - Preserve the user's task text as-is apart from stripping routing flags. -- Return the stdout of the `codex-companion` command exactly as-is. -- If the Bash call fails or Codex cannot be invoked, return nothing. +- Return the `result` stdout exactly as-is. +- If the Bash call fails or Codex cannot be invoked, return the command's exit status and stderr verbatim so the failure is visible; never return an empty result. Response style: diff --git a/plugins/codex/commands/adversarial-review.md b/plugins/codex/commands/adversarial-review.md index da440ab4d..71c8d7493 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] [focus ...]' +argument-hint: '[--wait|--background] [--base ] [--scope auto|working-tree|branch] [--model ] [--effort ] [--config key=value] [focus ...]' disable-model-invocation: true allowed-tools: Read, Glob, Grep, Bash(node:*), Bash(git:*), AskUserQuestion --- @@ -47,7 +47,9 @@ Argument handling: Foreground flow: - Run: ```bash -node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" adversarial-review "$ARGUMENTS" +node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" adversarial-review --args-stdin <<'CODEX_ARGS' +$ARGUMENTS +CODEX_ARGS ``` - Return the command stdout verbatim, exactly as-is. - Do not paraphrase, summarize, or add commentary before or after it. @@ -57,7 +59,9 @@ Background flow: - Launch the review with `Bash` in the background: ```typescript Bash({ - command: `node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" adversarial-review "$ARGUMENTS"`, + command: `node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" adversarial-review --args-stdin <<'CODEX_ARGS' +$ARGUMENTS +CODEX_ARGS`, description: "Codex adversarial review", run_in_background: true }) diff --git a/plugins/codex/commands/cancel.md b/plugins/codex/commands/cancel.md index a1472b836..a9dd00972 100644 --- a/plugins/codex/commands/cancel.md +++ b/plugins/codex/commands/cancel.md @@ -5,4 +5,10 @@ disable-model-invocation: true allowed-tools: Bash(node:*) --- -!`node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" cancel "$ARGUMENTS"` +Cancel the requested background Codex job by running the Bash command below. + +```bash +node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" cancel --args-stdin <<'CODEX_ARGS' +$ARGUMENTS +CODEX_ARGS +``` diff --git a/plugins/codex/commands/rescue.md b/plugins/codex/commands/rescue.md index 56de9555d..d88ea6767 100644 --- a/plugins/codex/commands/rescue.md +++ b/plugins/codex/commands/rescue.md @@ -1,49 +1,53 @@ --- 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]" -allowed-tools: Bash(node:*), AskUserQuestion, Agent +argument-hint: "[--background|--wait] [--resume|--fresh] [--model ] [--effort ] [--config key=value]... [what Codex should investigate, solve, or continue]" +allowed-tools: Bash, AskUserQuestion, Agent --- -Invoke the `codex:codex-rescue` subagent via the `Agent` tool (`subagent_type: "codex:codex-rescue"`), forwarding the raw user request as the prompt. -`codex:codex-rescue` is a subagent, not a skill — do not call `Skill(codex:codex-rescue)` (no such skill) or `Skill(codex:rescue)` (that re-enters this command and hangs the session). The command runs inline so the `Agent` tool stays in scope; forked general-purpose subagents do not expose it. -The final user-visible response must be Codex's output verbatim. +Delegate the request to Codex through the shared companion runtime. Default is synchronous: the user gets Codex's answer in this turn. -Raw user request: -$ARGUMENTS +Raw slash-command arguments: +`$ARGUMENTS` -Execution mode: +If the request contains `--background`, skip directly to step 3 — steps 1 and 2 are the default synchronous path and do not run for a `--background` request. -- If the request includes `--background`, run the `codex:codex-rescue` subagent in the background. -- If the request includes `--wait`, run the `codex:codex-rescue` subagent in the foreground. -- If neither flag is present, default to foreground. -- `--background` and `--wait` are execution flags for Claude Code. Do not forward them to `task`, and do not treat them as part of the natural-language task text. -- `--model` and `--effort` are runtime-selection flags. Preserve them for the forwarded `task` call, but do not treat them as part of the natural-language task text. -- If the request includes `--resume`, do not ask whether to continue. The user already chose. -- If the request includes `--fresh`, do not ask whether to continue. The user already chose. -- Otherwise, before starting Codex, check for a resumable rescue thread from this Claude session by running: +1. Strip `--wait` if present (it is the default). If the request contains `--resume`, use `task --resume-last`; if `--fresh`, use a fresh `task`. Otherwise run `node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" task-resume-candidate --json`: when it reports no resumable thread, start a fresh `task`; when it reports one, ask with `AskUserQuestion` exactly once before choosing — options `Continue current Codex thread` (use `task --resume-last`) and `Start a new Codex thread` (use a fresh `task`). Resuming silently would append this request to an unrelated earlier thread, so never pick `--resume-last` on your own. Pass `--model`, `--effort` and every `--config key=value` through unchanged. Never add `--write` unless the user explicitly asked Codex to modify files. + +2. Launch the job, then wait for it — two separate Bash calls, in ≤9-minute wait slices so the Bash tool's 10-minute cap never kills a long run. Bash calls share no variables — always set `JOB=` literally at the top of every later call; never rely on a `$JOB` left over from a previous call. + +The request prose and the runtime flags travel in two separate channels of the same Bash call: the prose is written byte-exact to `$PROMPT` by its own quoted heredoc and passed as `--prompt-file`, while the `--args-stdin` heredoc carries only the runtime flags (`--model`, `--effort`, `--config key=value`, `--resume-last`, `--write` as applicable). Never put the request text in the flags heredoc: it is tokenized, so quotes, backslashes and newlines in a stack trace, a regex or a code block would be mangled. Give both heredoc delimiters a fresh random suffix on every call — `CODEX_PROMPT_` / `CODEX_ARGS_`, e.g. 8 hex characters — and never reuse a suffix that appears in the request text: a payload line equal to the delimiter would end the heredoc early and run the rest on the host shell. + +2a. Launch (one Bash call): + +```bash +trap 'rm -f "$ERR" "$OUT" "$PROMPT"' EXIT +ERR=$(mktemp); PROMPT=$(mktemp) +cat > "$PROMPT" <<'CODEX_PROMPT_' + +CODEX_PROMPT_ +JOB=$(node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" task --background --json --prompt-file "$PROMPT" --args-stdin <<'CODEX_ARGS_' 2>"$ERR" | node -e 'let d="";process.stdin.on("data",c=>d+=c).on("end",()=>{try{process.stdout.write(JSON.parse(d).jobId||"")}catch(e){}})' + +CODEX_ARGS_ +) +[ -n "$JOB" ] || { cat "$ERR"; exit 1; } +[[ "$JOB" =~ ^[A-Za-z0-9_-]+$ ]] || { echo "invalid job id"; exit 1; } +echo "JOB=$JOB" +``` +If this call exits non-zero, its output is the launch failure (Codex missing, unauthenticated, a bad flag, etc.) — show it to the user verbatim and stop; never report "no result". Otherwise its last line is `JOB=`; read `` from it. + +2b. Wait and fetch the result (one Bash call, tool `timeout: 600000`). Set `JOB=` literally as the first line, using the id you just read from 2a: ```bash -node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" task-resume-candidate --json +trap 'rm -f "$ERR" "$OUT" "$PROMPT"' EXIT +JOB= +[[ "$JOB" =~ ^[A-Za-z0-9_-]+$ ]] || { echo "invalid job id"; exit 1; } +OUT=$(mktemp); ERR=$(mktemp) +while node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" status "$JOB" --wait --timeout-ms 540000 --json >"$OUT" 2>"$ERR"; node -e 'let d="";process.stdin.on("data",c=>d+=c).on("end",()=>{try{const s=(JSON.parse(d).job||{}).status;process.exit(s==="queued"||s==="running"?3:(s?0:2))}catch(e){process.exit(2)}})' < "$OUT"; rc=$?; [ "$rc" -eq 3 ]; do sleep 1; done +[ "$rc" -eq 0 ] || { cat "$OUT" "$ERR"; exit "$rc"; } +node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" result "$JOB" ``` +The status check exits 3 while the job is still `queued`/`running` (loop — `sleep 1` keeps a fast exit-3 from spinning hot), 0 once the job reaches a terminal status, or 2 if the status output was empty or unparseable; either non-3 outcome ends the loop. If this Bash call is itself cut off by the tool's own timeout before the loop finishes, the job keeps running server-side — run 2b again with the same literal `JOB=` line. If it exits non-zero, show its output verbatim and stop; never report "no result". Otherwise show the `result` output to the user verbatim, then add your own assessment. + +3. Only when the request contains `--background`: invoke the `codex:codex-rescue` subagent via the `Agent` tool (`subagent_type: "codex:codex-rescue"`, prompt = the raw request minus `--background`) and tell the user the job id will arrive as a completion notification; they can also run `/codex:status` and `/codex:result `. -- If that helper reports `available: true`, use `AskUserQuestion` exactly once to ask whether to continue the current Codex thread or start a new one. -- The two choices must be: - - `Continue current Codex thread` - - `Start a new Codex thread` -- If the user is clearly giving a follow-up instruction such as "continue", "keep going", "resume", "apply the top fix", or "dig deeper", put `Continue current Codex thread (Recommended)` first. -- Otherwise put `Start a new Codex thread (Recommended)` first. -- If the user chooses continue, add `--resume` before routing to the subagent. -- If the user chooses a new thread, add `--fresh` before routing to the subagent. -- If the helper reports `available: false`, do not ask. Route normally. - -Operating rules: - -- The subagent is a thin forwarder only. It should use one `Bash` call to invoke `node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" task ...` and return that command's stdout as-is. -- Return the Codex companion stdout verbatim to the user. -- Do not paraphrase, summarize, rewrite, or add commentary before or after it. -- Do not ask the subagent to inspect files, monitor progress, poll `/codex:status`, fetch `/codex:result`, call `/codex:cancel`, summarize output, or do follow-up work of its own. -- Leave `--effort` unset unless the user explicitly asks for a specific reasoning effort. -- Leave the model unset unless the user explicitly asks for one. If they ask for `spark`, map it to `gpt-5.3-codex-spark`. -- Leave `--resume` and `--fresh` in the forwarded request. The subagent handles that routing when it builds the `task` command. -- If the helper reports that Codex is missing or unauthenticated, stop and tell the user to run `/codex:setup`. -- If the user did not supply a request, ask what Codex should investigate or fix. +Do not call `Skill(codex:rescue)` from here (it re-enters this command). If any Bash step exits non-zero, show its stderr to the user — never report "no result". diff --git a/plugins/codex/commands/result.md b/plugins/codex/commands/result.md index 3abc2d931..56d3858c3 100644 --- a/plugins/codex/commands/result.md +++ b/plugins/codex/commands/result.md @@ -5,7 +5,13 @@ disable-model-invocation: true allowed-tools: Bash(node:*) --- -!`node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" result "$ARGUMENTS"` +Show the stored final output for a finished Codex job by running the Bash command below, then present the full output. + +```bash +node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" result --args-stdin <<'CODEX_ARGS' +$ARGUMENTS +CODEX_ARGS +``` Present the full command output to the user. Do not summarize or condense it. Preserve all details including: - Job ID and status diff --git a/plugins/codex/commands/review.md b/plugins/codex/commands/review.md index fb70a4876..b3ddddaf2 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]' +argument-hint: '[--wait|--background] [--base ] [--scope auto|working-tree|branch] [--model ] [--effort ] [--config key=value]' disable-model-invocation: true allowed-tools: Read, Glob, Grep, Bash(node:*), Bash(git:*), AskUserQuestion --- @@ -42,7 +42,9 @@ Argument handling: Foreground flow: - Run: ```bash -node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" review "$ARGUMENTS" +node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" review --args-stdin <<'CODEX_ARGS' +$ARGUMENTS +CODEX_ARGS ``` - Return the command stdout verbatim, exactly as-is. - Do not paraphrase, summarize, or add commentary before or after it. @@ -52,7 +54,9 @@ Background flow: - Launch the review with `Bash` in the background: ```typescript Bash({ - command: `node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" review "$ARGUMENTS"`, + command: `node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" review --args-stdin <<'CODEX_ARGS' +$ARGUMENTS +CODEX_ARGS`, description: "Codex review", run_in_background: true }) diff --git a/plugins/codex/commands/setup.md b/plugins/codex/commands/setup.md index fb33a150a..2ebbb0cb6 100644 --- a/plugins/codex/commands/setup.md +++ b/plugins/codex/commands/setup.md @@ -7,7 +7,9 @@ allowed-tools: Bash(node:*), Bash(npm:*), AskUserQuestion Run: ```bash -node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" setup --json $ARGUMENTS +node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" setup --json --args-stdin <<'CODEX_ARGS' +$ARGUMENTS +CODEX_ARGS ``` If the result says Codex is unavailable and npm is available: @@ -25,7 +27,9 @@ npm install -g @openai/codex - Then rerun: ```bash -node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" setup --json $ARGUMENTS +node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" setup --json --args-stdin <<'CODEX_ARGS' +$ARGUMENTS +CODEX_ARGS ``` If Codex is already installed or npm is unavailable: diff --git a/plugins/codex/commands/status.md b/plugins/codex/commands/status.md index 8f70663d1..f53c2b265 100644 --- a/plugins/codex/commands/status.md +++ b/plugins/codex/commands/status.md @@ -5,7 +5,13 @@ disable-model-invocation: true allowed-tools: Bash(node:*) --- -!`node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" status "$ARGUMENTS"` +Run the Codex status command with the Bash tool (the `allowed-tools` frontmatter above permits it), then format the output as described below: + +```bash +node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" status --args-stdin <<'CODEX_ARGS' +$ARGUMENTS +CODEX_ARGS +``` If the user did not pass a job ID: - Render the command output as a single Markdown table for the current and past runs in this session. diff --git a/plugins/codex/commands/transfer.md b/plugins/codex/commands/transfer.md index 42170e51d..ebba14fbe 100644 --- a/plugins/codex/commands/transfer.md +++ b/plugins/codex/commands/transfer.md @@ -5,6 +5,12 @@ disable-model-invocation: true allowed-tools: Bash(node:*) --- -!`node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" transfer "$ARGUMENTS"` +Transfer the current Claude Code session into a resumable Codex thread by running the Bash command below, then present the output to the user. + +```bash +node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" transfer --args-stdin <<'CODEX_ARGS' +$ARGUMENTS +CODEX_ARGS +``` Present the command output to the user exactly as returned. Preserve the Codex session ID and the `codex resume ` command. diff --git a/plugins/codex/hooks/hooks.json b/plugins/codex/hooks/hooks.json index 19e33b818..bd54ad05a 100644 --- a/plugins/codex/hooks/hooks.json +++ b/plugins/codex/hooks/hooks.json @@ -7,7 +7,7 @@ { "type": "command", "command": "node \"${CLAUDE_PLUGIN_ROOT}/scripts/session-lifecycle-hook.mjs\" SessionStart", - "timeout": 5 + "timeout": 60 } ] } diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 83df468ad..2c00f5637 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -27,12 +27,15 @@ import { collectReviewContext, ensureGitRepository, resolveReviewTarget } from " import { binaryAvailable, terminateProcessTree } from "./lib/process.mjs"; import { loadPromptTemplate, interpolateTemplate } from "./lib/prompts.mjs"; import { + consumeJobRequestFile, generateJobId, getConfig, listJobs, + removeJobRequestFile, setConfig, upsertJob, - writeJobFile + writeJobFile, + writeJobRequestFile } from "./lib/state.mjs"; import { buildSingleJobSnapshot, @@ -68,8 +71,23 @@ 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 MODEL_ALIASES = new Map([["spark", "gpt-5.3-codex-spark"]]); +const VALID_REASONING_EFFORTS = new Set([ + "none", + "minimal", + "low", + "medium", + "high", + "xhigh", + "max", + "ultra" +]); +const MODEL_ALIASES = new Map([ + ["spark", "gpt-5.3-codex-spark"], + ["sol", "gpt-5.6-sol"], + ["luna", "gpt-5.6-luna"], + ["terra", "gpt-5.6-terra"], + ["mini", "gpt-5.4-mini"] +]); const STOP_REVIEW_TASK_MARKER = "Run a stop-gate review of the previous Claude turn."; function printUsage() { @@ -77,13 +95,16 @@ 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 ] [--config key=value]...", + " node scripts/codex-companion.mjs adversarial-review [--wait|--background] [--base ] [--scope ] [--model ] [--effort ] [--config key=value]... [focus text]", + " node scripts/codex-companion.mjs task [--background] [--write] [--resume-last|--resume|--fresh] [--model ] [--effort ] [--config key=value]... [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]", - " node scripts/codex-companion.mjs cancel [job-id] [--json]" + " node scripts/codex-companion.mjs cancel [job-id] [--json]", + "", + "Any subcommand also accepts --args-stdin: the whole argument string is read", + "from stdin and tokenized here, so no shell ever sees the caller's text." ].join("\n") ); } @@ -121,14 +142,48 @@ 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: none, minimal, low, medium, high, xhigh, max, ultra.` ); } return normalized; } +function parseConfigOverrides(list = []) { + const config = {}; + for (const pair of list) { + const eq = pair.indexOf("="); + if (eq <= 0) { + throw new Error(`--config expects key=value, got "${pair}".`); + } + config[pair.slice(0, eq)] = pair.slice(eq + 1); + } + return config; +} + +// Claude Code substitutes `$ARGUMENTS` (and a rescue request's text) into the +// command body *before* bash runs it, so any `$(...)`/backtick the user typed +// would execute on the host shell, outside Codex's sandbox. Command bodies feed +// the raw argument string in through a quoted heredoc on stdin instead, and +// `--args-stdin` tokenizes it here with the same shell-like splitter +// `normalizeArgv` already uses — never through a shell. +const ARGS_STDIN_FLAG = "--args-stdin"; +let argvTokenizedFromStdin = false; + +function applyArgsStdin(argv) { + const flagIndex = argv.indexOf(ARGS_STDIN_FLAG); + if (flagIndex === -1) { + return argv; + } + argvTokenizedFromStdin = true; + return [ + ...argv.slice(0, flagIndex), + ...splitRawArgumentString(readStdinIfPiped()), + ...argv.slice(flagIndex + 1) + ]; +} + function normalizeArgv(argv) { - if (argv.length === 1) { + if (!argvTokenizedFromStdin && argv.length === 1) { const [raw] = argv; if (!raw || !raw.trim()) { return []; @@ -140,14 +195,25 @@ function normalizeArgv(argv) { function parseCommandInput(argv, config = {}) { return parseArgs(normalizeArgv(argv), { + rejectUnknownOptions: true, ...config, + booleanOptions: ["help", ...(config.booleanOptions ?? [])], aliasMap: { C: "cwd", + h: "help", ...(config.aliasMap ?? {}) } }); } +function maybePrintCommandHelp(options) { + if (!options.help) { + return false; + } + printUsage(); + return true; +} + function resolveCommandCwd(options = {}) { return options.cwd ? path.resolve(process.cwd(), options.cwd) : process.cwd(); } @@ -217,6 +283,9 @@ async function handleSetup(argv) { valueOptions: ["cwd"], booleanOptions: ["json", "enable-review-gate", "disable-review-gate"] }); + if (maybePrintCommandHelp(options)) { + return; + } if (options["enable-review-gate"] && options["disable-review-gate"]) { throw new Error("Choose either --enable-review-gate or --disable-review-gate."); @@ -370,6 +439,8 @@ async function executeReviewRun(request) { const result = await runAppServerReview(request.cwd, { target: reviewTarget, model: request.model, + effort: request.effort, + config: request.config, onProgress: request.onProgress }); const payload = { @@ -397,6 +468,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.`), @@ -411,6 +483,8 @@ async function executeReviewRun(request) { const result = await runAppServerTurn(context.repoRoot, { prompt, model: request.model, + effort: request.effort, + config: request.config, sandbox: "read-only", outputSchema: readOutputSchema(REVIEW_SCHEMA), onProgress: request.onProgress @@ -444,6 +518,7 @@ async function executeReviewRun(request) { exitStatus: result.status, threadId: result.threadId, turnId: result.turnId, + resolved: result.resolved, payload, rendered: renderReviewResult(parsed, { reviewLabel: reviewName, @@ -484,10 +559,13 @@ async function executeTaskRun(request) { const result = await runAppServerTurn(workspaceRoot, { resumeThreadId, + excludeJobId: request.jobId, prompt: request.prompt, defaultPrompt: resumeThreadId ? DEFAULT_CONTINUE_PROMPT : "", model: request.model, effort: request.effort, + config: request.config, + approvalPolicy: request.write ? "on-request" : "never", sandbox: request.write ? "workspace-write" : "read-only", onProgress: request.onProgress, persistThread: true, @@ -520,6 +598,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.`)), @@ -601,11 +680,12 @@ function buildTaskJob(workspaceRoot, taskMetadata, write) { }); } -function buildTaskRequest({ cwd, model, effort, prompt, write, resumeLast, jobId }) { +function buildTaskRequest({ cwd, model, effort, config, prompt, write, resumeLast, jobId }) { return { cwd, model, effort, + config, prompt, write, resumeLast, @@ -681,22 +761,62 @@ function spawnDetachedTaskWorker(cwd, jobId) { return child; } +const PRIVATE_CONFIG_KEY_PATTERN = /key|token|secret|auth|password/i; + +// `status --json` / `result --json` echo the stored job record back to the user +// and to Claude, so a `--config model_providers.x.http_headers.Authorization=...` +// would end up in the transcript. The worker reads the real values from the +// private one-shot payload file; the record keeps only a redacted copy. +function redactPrivateConfigValues(config) { + if (!config || typeof config !== "object") { + return config; + } + return Object.fromEntries( + Object.entries(config).map(([key, value]) => [key, PRIVATE_CONFIG_KEY_PATTERN.test(key) ? "[redacted]" : value]) + ); +} + function enqueueBackgroundTask(cwd, job, request) { const { logFile } = createTrackedProgress(job); appendLogLine(logFile, "Queued for background execution."); - const child = spawnDetachedTaskWorker(cwd, job.id); + // Persist before spawning: a worker that starts instantly must find its + // record, otherwise it exits while the parent reports `queued`. + const requestFile = writeJobRequestFile(job.workspaceRoot, job.id, request); const queuedRecord = { ...job, status: "queued", phase: "queued", - pid: child.pid ?? null, + pid: null, logFile, - request + requestFile, + request: { ...request, config: redactPrivateConfigValues(request.config) } }; writeJobFile(job.workspaceRoot, job.id, queuedRecord); upsertJob(job.workspaceRoot, queuedRecord); + let child; + try { + child = spawnDetachedTaskWorker(cwd, job.id); + if (child.pid === undefined) { + throw new Error("Could not spawn the background Codex worker."); + } + } catch (error) { + // No worker will ever read the payload, so do not leave it (0600, possibly + // holding `--config` secrets) on disk until the job is pruned. + removeJobRequestFile(job.workspaceRoot, job.id); + const errorMessage = error instanceof Error ? error.message : String(error); + const failedRecord = { ...queuedRecord, status: "failed", phase: "failed", errorMessage, requestFile: null }; + writeJobFile(job.workspaceRoot, job.id, failedRecord); + upsertJob(job.workspaceRoot, failedRecord); + throw error; + } + + // Nothing writes this record from here on: the worker owns it from the moment + // it starts, and `runTrackedJob` stores its own pid (the same `child.pid`) as + // its first act. A post-spawn patch from the parent would race the worker's + // own `upsertJob` and could rewind `running` back to `queued`. + return { payload: { jobId: job.id, @@ -711,15 +831,25 @@ function enqueueBackgroundTask(cwd, job, request) { async function handleReviewCommand(argv, config) { const { options, positionals } = parseCommandInput(argv, { - valueOptions: ["base", "scope", "model", "cwd"], + valueOptions: ["base", "scope", "model", "effort", "cwd"], booleanOptions: ["json", "background", "wait"], + repeatableOptions: ["config"], + // Only the adversarial variant takes free-form focus text; stop option + // parsing there so option-looking prompt words survive (#547). + stopAtFirstPositional: Boolean(config.acceptsFocusText), aliasMap: { m: "model" } }); + if (maybePrintCommandHelp(options)) { + return; + } const cwd = resolveCommandCwd(options); const workspaceRoot = resolveCommandWorkspace(options); + const model = normalizeRequestedModel(options.model); + const effort = normalizeReasoningEffort(options.effort); + const configOverrides = parseConfigOverrides(options.config); const focusText = positionals.join(" ").trim(); const target = resolveReviewTarget(cwd, { base: options.base, @@ -743,7 +873,9 @@ async function handleReviewCommand(argv, config) { cwd, base: options.base, scope: options.scope, - model: options.model, + model, + effort, + config: configOverrides, focusText, reviewName: config.reviewName, onProgress: progress @@ -763,15 +895,21 @@ async function handleTask(argv) { const { options, positionals } = parseCommandInput(argv, { valueOptions: ["model", "effort", "cwd", "prompt-file"], booleanOptions: ["json", "write", "resume-last", "resume", "fresh", "background"], + repeatableOptions: ["config"], + stopAtFirstPositional: true, aliasMap: { m: "model" } }); + if (maybePrintCommandHelp(options)) { + return; + } const cwd = resolveCommandCwd(options); const workspaceRoot = resolveCommandWorkspace(options); const model = normalizeRequestedModel(options.model); const effort = normalizeReasoningEffort(options.effort); + const configOverrides = parseConfigOverrides(options.config); const prompt = readTaskPrompt(cwd, options, positionals); const resumeLast = Boolean(options["resume-last"] || options.resume); @@ -794,6 +932,7 @@ async function handleTask(argv) { cwd, model, effort, + config: configOverrides, prompt, write, resumeLast, @@ -812,6 +951,7 @@ async function handleTask(argv) { cwd, model, effort, + config: configOverrides, prompt, write, resumeLast, @@ -827,6 +967,9 @@ async function handleTransfer(argv) { valueOptions: ["cwd", "source"], booleanOptions: ["json"] }); + if (maybePrintCommandHelp(options)) { + return; + } const cwd = resolveCommandCwd(options); const { payload, rendered } = await executeTransfer(cwd, { @@ -851,7 +994,9 @@ async function handleTaskWorker(argv) { throw new Error(`No stored job found for ${options["job-id"]}.`); } - const request = storedJob.request; + // The private payload carries the unredacted request; fall back to the record + // for jobs queued before that file existed. + const request = consumeJobRequestFile(workspaceRoot, options["job-id"]) ?? storedJob.request; if (!request || typeof request !== "object") { throw new Error(`Stored job ${options["job-id"]} is missing its task request payload.`); } @@ -885,6 +1030,9 @@ async function handleStatus(argv) { valueOptions: ["cwd", "timeout-ms", "poll-interval-ms"], booleanOptions: ["json", "all", "wait"] }); + if (maybePrintCommandHelp(options)) { + return; + } const cwd = resolveCommandCwd(options); const reference = positionals[0] ?? ""; @@ -912,6 +1060,9 @@ function handleResult(argv) { valueOptions: ["cwd"], booleanOptions: ["json"] }); + if (maybePrintCommandHelp(options)) { + return; + } const cwd = resolveCommandCwd(options); const reference = positionals[0] ?? ""; @@ -930,6 +1081,9 @@ function handleTaskResumeCandidate(argv) { valueOptions: ["cwd"], booleanOptions: ["json"] }); + if (maybePrintCommandHelp(options)) { + return; + } const cwd = resolveCommandCwd(options); const workspaceRoot = resolveCommandWorkspace(options); @@ -965,6 +1119,9 @@ async function handleCancel(argv) { valueOptions: ["cwd"], booleanOptions: ["json"] }); + if (maybePrintCommandHelp(options)) { + return; + } const cwd = resolveCommandCwd(options); const reference = positionals[0] ?? ""; @@ -1022,12 +1179,14 @@ async function handleCancel(argv) { } async function main() { - const [subcommand, ...argv] = process.argv.slice(2); + const [subcommand, ...rawArgv] = process.argv.slice(2); if (!subcommand || subcommand === "help" || subcommand === "--help") { printUsage(); return; } + const argv = applyArgsStdin(rawArgv); + switch (subcommand) { case "setup": await handleSetup(argv); @@ -1037,7 +1196,8 @@ async function main() { break; case "adversarial-review": await handleReviewCommand(argv, { - reviewName: "Adversarial Review" + reviewName: "Adversarial Review", + acceptsFocusText: true }); break; case "task": diff --git a/plugins/codex/scripts/lib/app-server.mjs b/plugins/codex/scripts/lib/app-server.mjs index 72b30a764..c5c65de46 100644 --- a/plugins/codex/scripts/lib/app-server.mjs +++ b/plugins/codex/scripts/lib/app-server.mjs @@ -22,6 +22,8 @@ const PLUGIN_MANIFEST = JSON.parse(fs.readFileSync(PLUGIN_MANIFEST_URL, "utf8")) export const BROKER_ENDPOINT_ENV = "CODEX_COMPANION_APP_SERVER_ENDPOINT"; export const BROKER_BUSY_RPC_CODE = -32001; +const NON_INTERACTIVE_DENIAL_REASON = "Non-interactive Codex runner: no operator to approve."; + /** @type {ClientInfo} */ const DEFAULT_CLIENT_INFO = { title: "Codex Plugin", @@ -54,7 +56,7 @@ function createProtocolError(message, data) { return error; } -class AppServerClientBase { +export class AppServerClientBase { constructor(cwd, options = {}) { this.cwd = cwd; this.options = options; @@ -153,11 +155,51 @@ class AppServerClientBase { } } + // `--write` runs use `approvalPolicy: "on-request"`, so app-server sends + // server->client approval requests. Rejecting them with -32601 makes the turn + // error out or hang. This client runs Codex non-interactively — there is no + // operator to ask — so every approval is answered with that request type's own + // refusal variant. The generated types disagree on shape: the v1 approvals take + // a `ReviewDecision` (refusal `{ denied: { rejection } }`), the v2 item + // approvals take a plain `"decline"` enum with no room for a reason, and + // `PermissionsRequestApprovalResponse` has no refusal variant at all, so + // granting nothing for the turn is its fail-closed answer. handleServerRequest(message) { - this.sendMessage({ - id: message.id, - error: buildJsonRpcError(-32601, `Unsupported server request: ${message.method}`) - }); + switch (message.method) { + case "execCommandApproval": + case "applyPatchApproval": + this.sendMessage({ + id: message.id, + result: { decision: { denied: { rejection: NON_INTERACTIVE_DENIAL_REASON } } } + }); + return; + + case "item/commandExecution/requestApproval": + case "item/fileChange/requestApproval": + this.sendMessage({ id: message.id, result: { decision: "decline" } }); + return; + + case "item/permissions/requestApproval": + this.sendMessage({ id: message.id, result: { permissions: {}, scope: "turn" } }); + return; + + // MCP servers (e.g. ChatGPT connectors surfaced as `codex_apps`) ask for + // the operator's consent via an elicitation. Accepting one fabricates that + // consent: a url-mode accept tells the MCP server an out-of-band + // authorization succeeded when nobody completed it, and a form-mode accept + // with `content: null` lets the tool call proceed without the values it + // asked for. Decline every mode — url/form flows must be completed in an + // interactive Codex session. + case "mcpServer/elicitation/request": + this.sendMessage({ id: message.id, result: { action: "decline" } }); + return; + + default: + this.sendMessage({ + id: message.id, + error: buildJsonRpcError(-32601, `Unsupported server request: ${message.method}`) + }); + } } handleExit(error) { diff --git a/plugins/codex/scripts/lib/args.mjs b/plugins/codex/scripts/lib/args.mjs index 6b1518502..944fe84f2 100644 --- a/plugins/codex/scripts/lib/args.mjs +++ b/plugins/codex/scripts/lib/args.mjs @@ -1,7 +1,10 @@ export function parseArgs(argv, config = {}) { const valueOptions = new Set(config.valueOptions ?? []); const booleanOptions = new Set(config.booleanOptions ?? []); + const repeatableOptions = new Set(config.repeatableOptions ?? []); const aliasMap = config.aliasMap ?? {}; + const rejectUnknownOptions = Boolean(config.rejectUnknownOptions); + const stopAtFirstPositional = Boolean(config.stopAtFirstPositional); const options = {}; const positionals = []; let passthrough = false; @@ -21,11 +24,16 @@ export function parseArgs(argv, config = {}) { if (!token.startsWith("-") || token === "-") { positionals.push(token); + if (stopAtFirstPositional) { + passthrough = true; + } continue; } if (token.startsWith("--")) { - const [rawKey, inlineValue] = token.slice(2).split("=", 2); + const separator = token.indexOf("="); + const rawKey = separator === -1 ? token.slice(2) : token.slice(2, separator); + const inlineValue = separator === -1 ? undefined : token.slice(separator + 1); const key = aliasMap[rawKey] ?? rawKey; if (booleanOptions.has(key)) { @@ -33,19 +41,30 @@ export function parseArgs(argv, config = {}) { continue; } - if (valueOptions.has(key)) { + if (valueOptions.has(key) || repeatableOptions.has(key)) { const nextValue = inlineValue ?? argv[index + 1]; if (nextValue === undefined) { throw new Error(`Missing value for --${rawKey}`); } - options[key] = nextValue; + if (repeatableOptions.has(key)) { + (options[key] ??= []).push(nextValue); + } else { + options[key] = nextValue; + } if (inlineValue === undefined) { index += 1; } continue; } + if (rejectUnknownOptions) { + throw new Error(`Unknown option: --${rawKey}`); + } + positionals.push(token); + if (stopAtFirstPositional) { + passthrough = true; + } continue; } @@ -57,17 +76,28 @@ export function parseArgs(argv, config = {}) { continue; } - if (valueOptions.has(key)) { + if (valueOptions.has(key) || repeatableOptions.has(key)) { const nextValue = argv[index + 1]; if (nextValue === undefined) { throw new Error(`Missing value for -${shortKey}`); } - options[key] = nextValue; + if (repeatableOptions.has(key)) { + (options[key] ??= []).push(nextValue); + } else { + options[key] = nextValue; + } index += 1; continue; } + if (rejectUnknownOptions) { + throw new Error(`Unknown option: -${shortKey}`); + } + positionals.push(token); + if (stopAtFirstPositional) { + passthrough = true; + } } return { options, positionals }; diff --git a/plugins/codex/scripts/lib/codex.mjs b/plugins/codex/scripts/lib/codex.mjs index fead00cc4..17750584b 100644 --- a/plugins/codex/scripts/lib/codex.mjs +++ b/plugins/codex/scripts/lib/codex.mjs @@ -4,6 +4,7 @@ * @typedef {import("./app-server-protocol").ThreadItem} ThreadItem * @typedef {import("./app-server-protocol").ThreadResumeParams} ThreadResumeParams * @typedef {import("./app-server-protocol").ThreadStartParams} ThreadStartParams + * @typedef {NonNullable} ThreadConfig * @typedef {import("./app-server-protocol").Turn} Turn * @typedef {import("./app-server-protocol").UserInput} UserInput * @typedef {((update: string | { message: string, phase: string | null, threadId?: string | null, turnId?: string | null, stderrMessage?: string | null, logTitle?: string | null, logBody?: string | null }) => void)} ProgressReporter @@ -43,6 +44,7 @@ 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 { listJobs } from "./state.mjs"; const SERVICE_NAME = "claude_code_codex_plugin"; const TASK_THREAD_PREFIX = "Codex Companion Task"; @@ -59,6 +61,47 @@ function cleanCodexStderr(stderr) { .join("\n"); } +/** + * @param {unknown} value + * @returns {any} + */ +function parseConfigValue(value) { + if (typeof value !== "string") { + return value; + } + try { + return JSON.parse(value); + } catch { + return value; + } +} + +/** + * Codex honours per-thread `config.toml` overrides on `thread/start`/`thread/resume`, + * which is the only reliable way to pin a model or reasoning effort for a review + * (`ReviewStartParams` carries neither) — see upstream #476/#651/#408. + * @param {{ model?: string | null, effort?: string | null, config?: Record | null, reviewModel?: string | null }} [options] + * @returns {ThreadConfig | null} + */ +export function buildThreadConfig(options = {}) { + const { model, effort, config, reviewModel } = options; + /** @type {ThreadConfig} */ + const merged = {}; + for (const [key, value] of Object.entries(config ?? {})) { + merged[key] = parseConfigValue(value); + } + if (model) { + merged.model = model; + } + if (reviewModel) { + merged.review_model = reviewModel; + } + if (effort) { + merged.model_reasoning_effort = effort; + } + return Object.keys(merged).length > 0 ? merged : null; +} + /** @returns {ThreadStartParams} */ function buildThreadParams(cwd, options = {}) { return { @@ -66,19 +109,27 @@ function buildThreadParams(cwd, options = {}) { model: options.model ?? null, approvalPolicy: options.approvalPolicy ?? "never", sandbox: options.sandbox ?? "read-only", + config: buildThreadConfig(options), serviceName: SERVICE_NAME, ephemeral: options.ephemeral ?? true }; } -/** @returns {ThreadResumeParams} */ +/** + * Resume deliberately forwards only the explicit `--config` overrides: a + * `config.model_reasoning_effort` — or a top-level `model` — on resume counts as + * a model override in app-server (`has_model_resume_override`) and would cancel + * the thread's persisted model/provider. Model and effort for the resumed turn + * ride on `turn/start` instead. + * @returns {ThreadResumeParams} + */ function buildResumeParams(threadId, cwd, options = {}) { return { threadId, cwd, - model: options.model ?? null, approvalPolicy: options.approvalPolicy ?? "never", - sandbox: options.sandbox ?? "read-only" + sandbox: options.sandbox ?? "read-only", + config: buildThreadConfig({ config: options.config }) }; } @@ -263,6 +314,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; } @@ -610,10 +663,10 @@ async function captureTurn(client, threadId, startRequest, options = {}) { } } -async function withAppServer(cwd, fn) { +async function withAppServer(cwd, fn, clientOptions = {}) { let client = null; try { - client = await CodexAppServerClient.connect(cwd); + client = await CodexAppServerClient.connect(cwd, clientOptions); const result = await fn(client); await client.close(); return result; @@ -632,7 +685,7 @@ async function withAppServer(cwd, fn) { throw error; } - const directClient = await CodexAppServerClient.connect(cwd, { disableBroker: true }); + const directClient = await CodexAppServerClient.connect(cwd, { ...clientOptions, disableBroker: true }); try { return await fn(directClient); } finally { @@ -1007,15 +1060,25 @@ export async function runAppServerReview(cwd, options = {}) { return withAppServer(cwd, async (client) => { 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, + config: options.config, + reviewModel: options.model, 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 + }; emitProgress(options.onProgress, `Thread ready (${sourceThreadId}).`, "starting", { - threadId: sourceThreadId + threadId: sourceThreadId, + resolved }); const delivery = options.delivery ?? "inline"; @@ -1046,6 +1109,7 @@ export async function runAppServerReview(cwd, options = {}) { threadId: turnState.threadId, sourceThreadId, turnId: turnState.turnId, + resolved, reviewText: turnState.reviewText, reasoningSummary: turnState.reasoningSummary, turn: turnState.finalTurn, @@ -1092,36 +1156,65 @@ export async function importExternalAgentSession(cwd, options = {}) { }); } +// Two concurrent turns on one thread interleave their history. The +// session-scoped resume-candidate lookup cannot see jobs from other Claude +// sessions, so the thread itself is checked here, where every resume passes. +function assertThreadIsFree(cwd, threadId, excludeJobId = null) { + const busy = listJobs(cwd).find( + (job) => + job.id !== excludeJobId && + job.threadId === threadId && + (job.status === "queued" || job.status === "running") + ); + if (busy) { + throw new Error(`Thread ${threadId} is busy in job ${busy.id}; wait for it or run cancel ${busy.id} first.`); + } +} + export async function runAppServerTurn(cwd, options = {}) { const availability = getCodexAvailability(cwd); if (!availability.available) { throw new Error("Codex CLI is not installed or is missing required runtime support. Install it with `npm install -g @openai/codex`, then rerun `/codex:setup`."); } + // A resume must run on its own app-server: a broker-backed server keeps the + // thread loaded, so `thread/resume` becomes a hot rejoin and its config, + // approvalPolicy and sandbox overrides are ignored. return withAppServer(cwd, async (client) => { - let threadId; + let response; if (options.resumeThreadId) { + assertThreadIsFree(cwd, options.resumeThreadId, options.excludeJobId); emitProgress(options.onProgress, `Resuming thread ${options.resumeThreadId}.`, "starting"); - const response = await resumeThread(client, options.resumeThreadId, cwd, { - model: options.model, + response = await resumeThread(client, options.resumeThreadId, cwd, { + config: options.config, + approvalPolicy: options.approvalPolicy, sandbox: options.sandbox, ephemeral: false }); - threadId = response.thread.id; } else { emitProgress(options.onProgress, "Starting Codex task thread.", "starting"); - const response = await startThread(client, cwd, { + response = await startThread(client, cwd, { model: options.model, + effort: options.effort, + config: options.config, + approvalPolicy: options.approvalPolicy, sandbox: options.sandbox, ephemeral: options.persistThread ? false : true, threadName: options.persistThread ? options.threadName : options.threadName ?? null }); - threadId = response.thread.id; } + const threadId = response.thread.id; + let resolved = { + model: response.model, + modelProvider: response.modelProvider, + reasoningEffort: response.reasoningEffort, + sandbox: response.sandbox + }; emitProgress(options.onProgress, `Thread ready (${threadId}).`, "starting", { - threadId + threadId, + resolved }); const prompt = options.prompt?.trim() || options.defaultPrompt || ""; @@ -1140,13 +1233,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, @@ -1156,7 +1259,7 @@ export async function runAppServerTurn(cwd, options = {}) { touchedFiles: collectTouchedFiles(turnState.fileChanges), commandExecutions: turnState.commandExecutions }; - }); + }, { disableBroker: Boolean(options.resumeThreadId) }); } export async function findLatestTaskThread(cwd) { diff --git a/plugins/codex/scripts/lib/state.mjs b/plugins/codex/scripts/lib/state.mjs index 2da23498f..a9a54246b 100644 --- a/plugins/codex/scripts/lib/state.mjs +++ b/plugins/codex/scripts/lib/state.mjs @@ -108,6 +108,7 @@ export function saveState(cwd, state) { continue; } removeJobFile(resolveJobFile(cwd, job.id)); + removeFileIfExists(resolveJobRequestFile(cwd, job.id)); removeFileIfExists(job.logFile); } @@ -189,3 +190,31 @@ export function resolveJobFile(cwd, jobId) { ensureStateDir(cwd); return path.join(resolveJobsDir(cwd), `${jobId}.json`); } + +// The full task request can carry secrets (`--config` values such as auth +// headers), so background workers read it from a private one-shot file instead +// of the job record that `status`/`result` echo back to the user. +export function resolveJobRequestFile(cwd, jobId) { + ensureStateDir(cwd); + return path.join(resolveJobsDir(cwd), `${jobId}.request.json`); +} + +export function writeJobRequestFile(cwd, jobId, payload) { + const requestFile = resolveJobRequestFile(cwd, jobId); + fs.writeFileSync(requestFile, `${JSON.stringify(payload, null, 2)}\n`, { encoding: "utf8", mode: 0o600 }); + return requestFile; +} + +export function removeJobRequestFile(cwd, jobId) { + removeFileIfExists(resolveJobRequestFile(cwd, jobId)); +} + +export function consumeJobRequestFile(cwd, jobId) { + const requestFile = resolveJobRequestFile(cwd, jobId); + if (!fs.existsSync(requestFile)) { + return null; + } + const payload = JSON.parse(fs.readFileSync(requestFile, "utf8")); + fs.unlinkSync(requestFile); + return payload; +} diff --git a/plugins/codex/scripts/lib/tracked-jobs.mjs b/plugins/codex/scripts/lib/tracked-jobs.mjs index 902869012..0c7d56d34 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/scripts/session-lifecycle-hook.mjs b/plugins/codex/scripts/session-lifecycle-hook.mjs index 778571e6c..34a22646b 100644 --- a/plugins/codex/scripts/session-lifecycle-hook.mjs +++ b/plugins/codex/scripts/session-lifecycle-hook.mjs @@ -36,7 +36,28 @@ function appendEnvVar(name, value) { if (!process.env.CLAUDE_ENV_FILE || value == null || value === "") { return; } - fs.appendFileSync(process.env.CLAUDE_ENV_FILE, `export ${name}=${shellEscape(value)}\n`, "utf8"); + + const line = `export ${name}=${shellEscape(value)}`; + + let lines = []; + + try { + lines = fs + .readFileSync(process.env.CLAUDE_ENV_FILE, "utf8") + .split("\n") + .filter(Boolean) + .filter((l) => !l.startsWith(`export ${name}=`)); + } catch { + // File doesn't exist yet. + } + + lines.push(line); + + fs.writeFileSync( + process.env.CLAUDE_ENV_FILE, + lines.join("\n") + "\n", + "utf8" + ); } function cleanupSessionJobs(cwd, sessionId) { diff --git a/plugins/codex/scripts/stop-review-gate-hook.mjs b/plugins/codex/scripts/stop-review-gate-hook.mjs index 2346bdcf4..4d35a2aa5 100644 --- a/plugins/codex/scripts/stop-review-gate-hook.mjs +++ b/plugins/codex/scripts/stop-review-gate-hook.mjs @@ -8,15 +8,17 @@ import { fileURLToPath } from "node:url"; import { getCodexAvailability } from "./lib/codex.mjs"; import { loadPromptTemplate, interpolateTemplate } from "./lib/prompts.mjs"; -import { getConfig, listJobs } from "./lib/state.mjs"; +import { getConfig, setConfig, listJobs } from "./lib/state.mjs"; import { sortJobsNewestFirst } from "./lib/job-control.mjs"; import { SESSION_ID_ENV } from "./lib/tracked-jobs.mjs"; import { resolveWorkspaceRoot } from "./lib/workspace.mjs"; -const STOP_REVIEW_TIMEOUT_MS = 15 * 60 * 1000; +const STOP_REVIEW_TIMEOUT_MINUTES = 13; +const STOP_REVIEW_TIMEOUT_MS = STOP_REVIEW_TIMEOUT_MINUTES * 60 * 1000; const SCRIPT_DIR = path.dirname(fileURLToPath(import.meta.url)); const ROOT_DIR = path.resolve(SCRIPT_DIR, ".."); const STOP_REVIEW_TASK_MARKER = "Run a stop-gate review of the previous Claude turn."; +const GATE_ROUNDS_CONFIG_KEY = "stopReviewGateRoundsBySession"; function readHookInput() { const raw = fs.readFileSync(0, "utf8").trim(); @@ -37,6 +39,40 @@ function logNote(message) { process.stderr.write(`${message}\n`); } +// Optional cap on how many consecutive stop-gate rounds run in one session. +// Unset or 0 keeps the previous unbounded behavior. +function getMaxRounds() { + const raw = process.env.CODEX_REVIEW_GATE_MAX_ROUNDS; + if (raw == null || raw === "") { + return 0; + } + const parsed = Number.parseInt(raw, 10); + return Number.isFinite(parsed) && parsed > 0 ? parsed : 0; +} + +function gateSessionId(input) { + return input.session_id || process.env[SESSION_ID_ENV] || "default"; +} + +function readGateRounds(workspaceRoot, sessionId) { + const rounds = getConfig(workspaceRoot)[GATE_ROUNDS_CONFIG_KEY]; + if (!rounds || typeof rounds !== "object") { + return 0; + } + return Number(rounds[sessionId]) || 0; +} + +function writeGateRounds(workspaceRoot, sessionId, count) { + const current = getConfig(workspaceRoot)[GATE_ROUNDS_CONFIG_KEY]; + const next = current && typeof current === "object" ? { ...current } : {}; + if (count > 0) { + next[sessionId] = count; + } else { + delete next[sessionId]; + } + setConfig(workspaceRoot, GATE_ROUNDS_CONFIG_KEY, next); +} + function filterJobsForCurrentSession(jobs, input = {}) { const sessionId = input.session_id || process.env[SESSION_ID_ENV] || null; if (!sessionId) { @@ -106,14 +142,16 @@ function runStopReview(cwd, input = {}) { cwd, env: childEnv, encoding: "utf8", - timeout: STOP_REVIEW_TIMEOUT_MS + timeout: STOP_REVIEW_TIMEOUT_MS, + killSignal: "SIGKILL", + maxBuffer: 16 * 1024 * 1024 }); if (result.error?.code === "ETIMEDOUT") { return { ok: false, reason: - "The stop-time Codex review task timed out after 15 minutes. Run /codex:review --wait manually or bypass the gate." + `The stop-time Codex review task timed out after ${STOP_REVIEW_TIMEOUT_MINUTES} minutes. Run /codex:review --wait manually or bypass the gate.` }; } @@ -140,7 +178,16 @@ function runStopReview(cwd, input = {}) { } function main() { - const input = readHookInput(); + let input; + try { + input = readHookInput(); + } catch { + emitDecision({ + decision: "block", + reason: "The stop review gate could not read or parse hook input; refusing to fail open." + }); + return; + } const cwd = input.cwd || process.env.CLAUDE_PROJECT_DIR || process.cwd(); const workspaceRoot = resolveWorkspaceRoot(cwd); const config = getConfig(workspaceRoot); @@ -163,8 +210,24 @@ function main() { return; } + const sessionId = gateSessionId(input); + const maxRounds = getMaxRounds(); + // A fresh user turn (not a gate-induced continuation) starts a new count. + const priorRounds = input.stop_hook_active ? readGateRounds(workspaceRoot, sessionId) : 0; + + if (maxRounds > 0 && priorRounds >= maxRounds) { + writeGateRounds(workspaceRoot, sessionId, 0); + logNote( + `Codex stop-time review gate reached its limit of ${maxRounds} round(s) for this session; allowing the stop. ` + + "Set CODEX_REVIEW_GATE_MAX_ROUNDS to adjust, or run /codex:review --wait manually for another pass." + ); + logNote(runningTaskNote); + return; + } + const review = runStopReview(cwd, input); if (!review.ok) { + writeGateRounds(workspaceRoot, sessionId, priorRounds + 1); emitDecision({ decision: "block", reason: runningTaskNote ? `${runningTaskNote} ${review.reason}` : review.reason @@ -172,6 +235,7 @@ function main() { return; } + writeGateRounds(workspaceRoot, sessionId, 0); logNote(runningTaskNote); } diff --git a/plugins/codex/skills/codex-cli-runtime/SKILL.md b/plugins/codex/skills/codex-cli-runtime/SKILL.md index 0e91bfb50..15a8151ae 100644 --- a/plugins/codex/skills/codex-cli-runtime/SKILL.md +++ b/plugins/codex/skills/codex-cli-runtime/SKILL.md @@ -12,32 +12,37 @@ Primary helper: - `node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" task ""` Execution rules: -- The rescue subagent is a forwarder, not an orchestrator. Its only job is to invoke `task` once and return that stdout unchanged. +- The rescue subagent is a forwarder, not an orchestrator. It launches once with `task --background --json`, polls only that job's own `status`, then returns the `result` stdout unchanged. - Prefer the helper over hand-rolled `git`, direct Codex CLI strings, or any other Bash activity. -- Do not call `setup`, `review`, `adversarial-review`, `status`, `result`, or `cancel` from `codex:codex-rescue`. +- Do not call `setup`, `review`, `adversarial-review`, or `cancel` from `codex:codex-rescue`. `status` and `result` are allowed, but only for the job you just launched — never another job. - Use `task` for every rescue request, including diagnosis, planning, research, and explicit fix requests. - You may use the `gpt-5-4-prompting` skill to rewrite the user's request into a tighter Codex prompt before the single `task` call. - That prompt drafting is the only Claude-side work allowed. Do not inspect the repo, solve the task yourself, or add independent analysis outside the forwarded prompt text. - Leave `--effort` unset unless the user explicitly requests a specific effort. - Leave model unset by default. Add `--model` only when the user explicitly asks for one. - Map `spark` to `--model gpt-5.3-codex-spark`. -- Default to a write-capable Codex run by adding `--write` unless the user explicitly asks for read-only behavior or only wants review, diagnosis, or research without edits. +- Map `sol` to `--model gpt-5.6-sol`, `luna` to `--model gpt-5.6-luna`, `terra` to `--model gpt-5.6-terra`, `mini` to `--model gpt-5.4-mini`. +- Never add `--write` unless the user explicitly asked Codex to modify files. Command selection: -- Use exactly one `task` invocation per rescue handoff. +- If the request names an uncommon domain (private infra runbooks, vendor-specific tooling), prepend to the task text: "If a matching skill is not already loaded, run `$agent-compat:skill-router` to find a reviewed playbook before starting." (Codex has the agent-compat plugin installed; it routes to reviewed route-only skills offline.) +- Launch exactly one job per rescue handoff with `task --background --json`, then poll only that job with `status --wait --timeout-ms 540000 --json` until it reaches a terminal status, then fetch it with `result `. +- Bash calls share no shell state — carry the job id as literal text between calls, never as a leftover `$JOB` shell variable. If a wait call is cut off by the Bash tool's own 10-minute timeout, re-issue it with the same literal id; the job keeps running server-side. - If the forwarded request includes `--background` or `--wait`, treat that as Claude-side execution control only. Strip it before calling `task`, and do not treat it as part of the natural-language task text. - If the forwarded request includes `--model`, normalize `spark` to `gpt-5.3-codex-spark` and pass it through to `task`. - If the forwarded request includes `--effort`, pass it through to `task`. +- If the forwarded request includes `--config key=value`, pass every occurrence through to `task` unchanged. - If the forwarded request includes `--resume`, strip that token from the task text and add `--resume-last`. - 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`. +- `--config key=value` (repeatable) forwards a `config.toml` override to the Codex thread (`thread/start.config`), e.g. `--config model_provider=ollama`. On `--resume-last` the plugin opens a fresh app-server session (cold resume) so `--config` overrides, sandbox and approval policy take effect; model and effort for the resumed turn are sent on the turn, never on the resume request. +- `--effort`: accepted values are `none`, `minimal`, `low`, `medium`, `high`, `xhigh`, `max`, `ultra`. Not every model supports every value; Codex validates the value against the reasoning levels the selected model advertises. - `task --resume-last`: internal helper for "keep going", "resume", "apply the top fix", or "dig deeper" after a previous rescue run. Safety rules: -- Default to write-capable Codex work in `codex:codex-rescue` unless the user explicitly asks for read-only behavior. +- Never add `--write` unless the user explicitly asked Codex to modify files. - Preserve the user's task text as-is apart from stripping routing flags. -- 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. +- Do not inspect the repository, read files, grep, cancel jobs, summarize output, or do any other follow-up work of your own beyond launching and polling your own job. - Return the stdout of the `task` command exactly as-is. -- If the Bash call fails or Codex cannot be invoked, return nothing. +- If the Bash call fails or Codex cannot be invoked, return the command's exit status and stderr verbatim so the failure is visible; never return an empty result. diff --git a/scripts/bump-version.mjs b/scripts/bump-version.mjs index 19b9888f8..c56ad4d47 100644 --- a/scripts/bump-version.mjs +++ b/scripts/bump-version.mjs @@ -156,6 +156,26 @@ function readPackageVersion(root) { return packageJson.version; } +// `npm install` rewrites package-lock.json from package.json, but a lockfile +// carried over from a fork's upstream keeps the upstream package name until +// someone regenerates it. Check it here so the drift cannot ship. +function checkLockfileIdentity(root) { + const expectedName = readJson(root, "package.json").name; + const lock = readJson(root, "package-lock.json"); + const mismatches = []; + + for (const [label, actual] of [ + ["name", lock.name], + ['packages[""].name', lock.packages?.[""]?.name] + ]) { + if (actual !== expectedName) { + mismatches.push(`package-lock.json ${label}: expected ${expectedName}, found ${actual ?? ""}`); + } + } + + return mismatches; +} + function checkVersions(root, expectedVersion) { const mismatches = []; @@ -169,7 +189,7 @@ function checkVersions(root, expectedVersion) { } } - return mismatches; + return [...mismatches, ...checkLockfileIdentity(root)]; } function bumpVersion(root, version) { @@ -208,7 +228,7 @@ function main() { if (options.check) { const mismatches = checkVersions(options.root, version); if (mismatches.length > 0) { - throw new Error(`Version metadata is out of sync:\n${mismatches.join("\n")}`); + throw new Error(`Release metadata is out of sync:\n${mismatches.join("\n")}`); } console.log(`All version metadata matches ${version}.`); return; diff --git a/tests/app-server.test.mjs b/tests/app-server.test.mjs new file mode 100644 index 000000000..6f70230fb --- /dev/null +++ b/tests/app-server.test.mjs @@ -0,0 +1,91 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { AppServerClientBase } from "../plugins/codex/scripts/lib/app-server.mjs"; + +/** Minimal client that records the JSON-RPC messages it would send. */ +class CapturingClient extends AppServerClientBase { + constructor() { + super(process.cwd()); + this.sent = []; + } + sendMessage(message) { + this.sent.push(message); + } +} + +test("handleServerRequest answers MCP elicitation requests instead of rejecting them", () => { + const client = new CapturingClient(); + client.handleServerRequest({ + id: 7, + method: "mcpServer/elicitation/request", + params: { threadId: "t1" } + }); + assert.deepEqual(client.sent, [{ id: 7, result: { action: "decline" } }]); +}); + +test("handleServerRequest still rejects unknown server requests with -32601", () => { + const client = new CapturingClient(); + client.handleServerRequest({ id: 8, method: "some/unknown/request", params: {} }); + assert.equal(client.sent.length, 1); + assert.equal(client.sent[0].id, 8); + assert.equal(client.sent[0].result, undefined); + assert.equal(client.sent[0].error.code, -32601); +}); + +test("every elicitation mode is declined, so consent is never fabricated", () => { + // Accepting a url-mode elicitation tells the MCP server that an out-of-band + // authorization succeeded when nobody completed it. There is no operator here, + // so no mode may be accepted. + for (const mode of ["form", "openai/form", "url", undefined, "something-new"]) { + const client = new CapturingClient(); + client.handleServerRequest({ + id: 9, + method: "mcpServer/elicitation/request", + params: { threadId: "t1", ...(mode === undefined ? {} : { mode }) } + }); + assert.deepEqual(client.sent, [{ id: 9, result: { action: "decline" } }], `mode ${mode} must be declined`); + } +}); + +// The non-interactive runner has no operator, so every approval request must be +// answered with that request type's own refusal variant. The generated types +// disagree on shape: the v1 approvals take a `ReviewDecision` whose refusal is +// `{ denied: { rejection } }`, the v2 item approvals take a plain `"decline"` +// enum with no room for a reason, and `PermissionsRequestApprovalResponse` has +// no refusal variant at all — granting nothing is its fail-closed answer. +const DENIAL_REASON = "Non-interactive Codex runner: no operator to approve."; + +test("v1 approval requests are answered with the ReviewDecision denied variant", () => { + for (const [id, method] of [ + [21, "execCommandApproval"], + [22, "applyPatchApproval"] + ]) { + const client = new CapturingClient(); + client.handleServerRequest({ id, method, params: { threadId: "t1" } }); + assert.deepEqual(client.sent, [ + { id, result: { decision: { denied: { rejection: DENIAL_REASON } } } } + ]); + } +}); + +test("v2 item approval requests are answered with the decline decision", () => { + for (const [id, method] of [ + [23, "item/commandExecution/requestApproval"], + [24, "item/fileChange/requestApproval"] + ]) { + const client = new CapturingClient(); + client.handleServerRequest({ id, method, params: { threadId: "t1" } }); + assert.deepEqual(client.sent, [{ id, result: { decision: "decline" } }]); + } +}); + +test("permission approval requests grant nothing for the turn", () => { + const client = new CapturingClient(); + client.handleServerRequest({ + id: 25, + method: "item/permissions/requestApproval", + params: { threadId: "t1", permissions: { network: { enabled: true } } } + }); + assert.deepEqual(client.sent, [{ id: 25, result: { permissions: {}, scope: "turn" } }]); +}); diff --git a/tests/args.test.mjs b/tests/args.test.mjs new file mode 100644 index 000000000..02c4e2de2 --- /dev/null +++ b/tests/args.test.mjs @@ -0,0 +1,126 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { fileURLToPath } from "node:url"; +import path from "node:path"; + +import { parseArgs, splitRawArgumentString } from "../plugins/codex/scripts/lib/args.mjs"; +import { makeTempDir, run, initGitRepo } from "./helpers.mjs"; +import { buildEnv, installFakeCodex } from "./fake-codex-fixture.mjs"; +import fs from "node:fs"; + +const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); +const SCRIPT = path.join(ROOT, "plugins", "codex", "scripts", "codex-companion.mjs"); + +test("parseArgs rejects unknown long options when configured", () => { + const helped = parseArgs(["--help", "--cwd", "/tmp"], { + booleanOptions: ["help"], + valueOptions: ["cwd"], + rejectUnknownOptions: true + }); + assert.equal(helped.options.help, true); + assert.equal(helped.options.cwd, "/tmp"); + assert.deepEqual(helped.positionals, []); + + assert.throws( + () => + parseArgs(["--not-a-flag"], { + booleanOptions: ["json"], + rejectUnknownOptions: true + }), + /Unknown option: --not-a-flag/ + ); +}); + +test("parseArgs keeps unknown options as positionals by default", () => { + const { options, positionals } = parseArgs(["--not-a-flag", "hello"], { + booleanOptions: ["json"] + }); + assert.deepEqual(options, {}); + assert.deepEqual(positionals, ["--not-a-flag", "hello"]); +}); + +test("task --help prints usage and does not dispatch a Codex thread", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); + + const result = run("node", [SCRIPT, "task", "--help", "--cwd", repo, "--json"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + assert.match(result.stdout, /Usage:/); + assert.match(result.stdout, /codex-companion\.mjs task/); + assert.equal(result.stderr.trim(), ""); + + const fakeStatePath = path.join(binDir, "fake-codex-state.json"); + if (fs.existsSync(fakeStatePath)) { + const state = JSON.parse(fs.readFileSync(fakeStatePath, "utf8")); + assert.equal((state.threads ?? []).length, 0); + } +}); + +test("task unknown --flag errors without dispatching a Codex thread", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); + + const result = run("node", [SCRIPT, "task", "--not-a-real-flag", "--cwd", repo], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.notEqual(result.status, 0); + assert.match(result.stderr, /Unknown option: --not-a-real-flag/); + + const fakeStatePath = path.join(binDir, "fake-codex-state.json"); + if (fs.existsSync(fakeStatePath)) { + const state = JSON.parse(fs.readFileSync(fakeStatePath, "utf8")); + assert.equal((state.threads ?? []).length, 0); + } +}); + +test("parseArgs collects repeatable options and honours -- and --opt=value", () => { + const { options, positionals } = parseArgs( + ["--config", "a=1", "--config=b=x=y", "--model", "sol", "--", "--not-an-option", "tail"], + { valueOptions: ["model"], repeatableOptions: ["config"] } + ); + assert.deepEqual(options.config, ["a=1", "b=x=y"]); + assert.equal(options.model, "sol"); + assert.deepEqual(positionals, ["--not-an-option", "tail"]); +}); + +test("parseArgs with stopAtFirstPositional keeps option-looking prompt words", () => { + const { options, positionals } = parseArgs( + ["--effort", "max", "investigate", "ls", "-R", "usage", "--model", "x"], + { valueOptions: ["effort", "model"], stopAtFirstPositional: true } + ); + assert.equal(options.effort, "max"); + assert.equal(options.model, undefined); + assert.deepEqual(positionals, ["investigate", "ls", "-R", "usage", "--model", "x"]); +}); + +test("parseArgs rejects a repeatable option without a value", () => { + assert.throws(() => parseArgs(["--config"], { repeatableOptions: ["config"] }), /--config/); +}); + +test("splitRawArgumentString keeps shell metacharacters as literal token content", () => { + assert.deepEqual(splitRawArgumentString("investigate $(id) `whoami` ${HOME} a|b;c&d"), [ + "investigate", + "$(id)", + "`whoami`", + "${HOME}", + "a|b;c&d" + ]); +}); + +test("splitRawArgumentString groups quoted runs and keeps quoted newlines inside one token", () => { + assert.deepEqual(splitRawArgumentString("--config 'a b=c d' \"e f\""), ["--config", "a b=c d", "e f"]); + assert.deepEqual(splitRawArgumentString("'line one\nline two'"), ["line one\nline two"]); + assert.deepEqual(splitRawArgumentString("--all\n--json"), ["--all", "--json"]); +}); diff --git a/tests/bump-version.test.mjs b/tests/bump-version.test.mjs index 205b0e9fe..58f130793 100644 --- a/tests/bump-version.test.mjs +++ b/tests/bump-version.test.mjs @@ -86,3 +86,26 @@ test("bump-version check mode reports stale metadata", () => { assert.match(result.stderr, /plugins\/codex\/\.claude-plugin\/plugin\.json version/); assert.match(result.stderr, /\.claude-plugin\/marketplace\.json metadata\.version/); }); + +test("bump-version check mode reports a lockfile whose name drifted from package.json", () => { + const root = makeVersionFixture(); + const lockPath = path.join(root, "package-lock.json"); + const lock = readJson(lockPath); + lock.name = "@upstream/codex-plugin-cc"; + lock.packages[""].name = "@upstream/codex-plugin-cc"; + writeJson(lockPath, lock); + + const result = run("node", [SCRIPT, "--root", root, "--check"], { cwd: ROOT }); + + assert.notEqual(result.status, 0); + assert.match(result.stderr, /package-lock\.json name: expected @openai\/codex-plugin-cc, found @upstream\/codex-plugin-cc/); + assert.match(result.stderr, /package-lock\.json packages\[""\]\.name/); +}); + +test("bump-version check mode passes when the lockfile identity matches", () => { + const root = makeVersionFixture(); + + const result = run("node", [SCRIPT, "--root", root, "--check", "1.0.2"], { cwd: ROOT }); + + assert.equal(result.status, 0, result.stderr); +}); diff --git a/tests/commands.test.mjs b/tests/commands.test.mjs index c34b06059..6053dee3a 100644 --- a/tests/commands.test.mjs +++ b/tests/commands.test.mjs @@ -20,10 +20,10 @@ test("review command uses AskUserQuestion and background Bash while staying revi assert.match(source, /return Codex's output verbatim to the user/i); assert.match(source, /```bash/); assert.match(source, /```typescript/); - assert.match(source, /review "\$ARGUMENTS"/); + assert.match(source, /review --args-stdin <<'CODEX_ARGS'/); assert.match(source, /\[--scope auto\|working-tree\|branch\]/); 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, /command:\s*`node "\$\{CLAUDE_PLUGIN_ROOT\}\/scripts\/codex-companion\.mjs" review --args-stdin <<'CODEX_ARGS'\n\$ARGUMENTS\nCODEX_ARGS`/); assert.match(source, /description:\s*"Codex review"/); assert.match(source, /Do not call `BashOutput`/); assert.match(source, /Return the command stdout verbatim, exactly as-is/i); @@ -48,10 +48,10 @@ test("adversarial review command uses AskUserQuestion and background Bash while assert.match(source, /return Codex's output verbatim to the user/i); assert.match(source, /```bash/); assert.match(source, /```typescript/); - assert.match(source, /adversarial-review "\$ARGUMENTS"/); - assert.match(source, /\[--scope auto\|working-tree\|branch\] \[focus \.\.\.\]/); + assert.match(source, /adversarial-review --args-stdin <<'CODEX_ARGS'/); + assert.match(source, /\[--scope auto\|working-tree\|branch\].*\[focus \.\.\.\]/); 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, /command:\s*`node "\$\{CLAUDE_PLUGIN_ROOT\}\/scripts\/codex-companion\.mjs" adversarial-review --args-stdin <<'CODEX_ARGS'\n\$ARGUMENTS\nCODEX_ARGS`/); assert.match(source, /description:\s*"Codex adversarial review"/); assert.match(source, /Do not call `BashOutput`/); assert.match(source, /Return the command stdout verbatim, exactly as-is/i); @@ -90,8 +90,8 @@ test("rescue command absorbs continue semantics", () => { const readme = fs.readFileSync(path.join(ROOT, "README.md"), "utf8"); const runtimeSkill = read("skills/codex-cli-runtime/SKILL.md"); - assert.match(rescue, /The final user-visible response must be Codex's output verbatim/i); - assert.match(rescue, /allowed-tools:\s*Bash\(node:\*\),\s*AskUserQuestion,\s*Agent/); + assert.match(rescue, /Show the `result` output to the user verbatim/i); + assert.match(rescue, /allowed-tools:\s*Bash,\s*AskUserQuestion,\s*Agent/); // Regression for #234: `Skill(codex:rescue)` from the main agent recursed // because rescue.md named the routing with ambiguous prose ("Route this // request to the `codex:codex-rescue` subagent") while running under @@ -99,50 +99,40 @@ test("rescue command absorbs continue semantics", () => { // `Agent` tool, so the fork fell back to `Skill` and re-entered this // command. Pin the explicit transport and the inline (no-fork) execution. assert.match(rescue, /subagent_type: "codex:codex-rescue"/); - assert.match(rescue, /do not call `Skill\(codex:codex-rescue\)`/i); + assert.match(rescue, /do not call `Skill\(codex:rescue\)`/i); assert.doesNotMatch(rescue, /^context:\s*fork\b/m); assert.match(rescue, /--background\|--wait/); assert.match(rescue, /--resume\|--fresh/); - assert.match(rescue, /--model /); - assert.match(rescue, /--effort /); + assert.match(rescue, /--model /); + assert.match(rescue, /--effort /); assert.match(rescue, /task-resume-candidate --json/); - assert.match(rescue, /AskUserQuestion/); - assert.match(rescue, /Continue current Codex thread/); + assert.match(rescue, /AskUserQuestion.*Continue current Codex thread/s); assert.match(rescue, /Start a new Codex thread/); - assert.match(rescue, /run the `codex:codex-rescue` subagent in the background/i); - assert.match(rescue, /default to foreground/i); - assert.match(rescue, /Do not forward them to `task`/i); - assert.match(rescue, /`--model` and `--effort` are runtime-selection flags/i); - assert.match(rescue, /Leave `--effort` unset unless the user explicitly asks for a specific reasoning effort/i); - assert.match(rescue, /If they ask for `spark`, map it to `gpt-5\.3-codex-spark`/i); - assert.match(rescue, /If the request includes `--resume`, do not ask whether to continue/i); - assert.match(rescue, /If the request includes `--fresh`, do not ask whether to continue/i); - assert.match(rescue, /If the user chooses continue, add `--resume`/i); - assert.match(rescue, /If the user chooses a new thread, add `--fresh`/i); - assert.match(rescue, /thin forwarder only/i); - assert.match(rescue, /Return the Codex companion stdout verbatim to the user/i); - assert.match(rescue, /Do not paraphrase, summarize, rewrite, or add commentary before or after it/i); - assert.match(rescue, /return that command's stdout as-is/i); - assert.match(rescue, /Leave `--resume` and `--fresh` in the forwarded request/i); + assert.match(rescue, /Default is synchronous/i); + assert.match(rescue, /Strip `--wait` if present/i); + assert.match(rescue, /Pass `--model`, `--effort` and every `--config key=value` through unchanged/i); + assert.match(rescue, /Delegate the request to Codex through the shared companion runtime/i); assert.match(agent, /--resume/); assert.match(agent, /--fresh/); assert.match(agent, /thin forwarding wrapper/i); - assert.match(agent, /prefer foreground for a small, clearly bounded rescue request/i); - assert.match(agent, /If the user did not explicitly choose `--background` or `--wait` and the task looks complicated, open-ended, multi-step, or likely to keep Codex running for a long time, prefer background execution/i); - assert.match(agent, /Use exactly one `Bash` call/i); - assert.match(agent, /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(agent, /Do not call `review`, `adversarial-review`, `status`, `result`, or `cancel`/i); + assert.match(agent, /result "\$JOB"/); + assert.doesNotMatch(agent, /prefer background execution/i); + assert.match(runtimeSkill, /Launch exactly one job per rescue handoff with `task --background --json`/i); + assert.match(agent, /Bash tool's 10-minute cap/i); + assert.match(agent, /do not inspect the repository, read files, grep, cancel jobs, summarize output, or do any other follow-up work of your own/i); + assert.match(agent, /Do not call `review`, `adversarial-review`, or `cancel`/i); assert.match(agent, /Leave `--effort` unset unless the user explicitly requests a specific reasoning effort/i); assert.match(agent, /Leave model unset by default/i); assert.match(agent, /If the user asks for `spark`, map that to `--model gpt-5\.3-codex-spark`/i); assert.match(agent, /If the user asks for a concrete model name such as `gpt-5\.4-mini`, pass it through with `--model`/i); - assert.match(agent, /Return the stdout of the `codex-companion` command exactly as-is/i); - assert.match(agent, /If the Bash call fails or Codex cannot be invoked, return nothing/i); + assert.match(agent, /Return the `result` stdout exactly as-is/i); + assert.match(agent, /If the Bash call fails or Codex cannot be invoked, return the command's exit status and stderr verbatim/i); assert.match(agent, /gpt-5-4-prompting/); assert.match(agent, /only to tighten the user's request into a better Codex prompt/i); assert.match(agent, /Do not use that skill to inspect the repository, reason through the problem yourself, draft a solution, or do any independent work/i); - assert.match(runtimeSkill, /only job is to invoke `task` once and return that stdout unchanged/i); - assert.match(runtimeSkill, /Do not call `setup`, `review`, `adversarial-review`, `status`, `result`, or `cancel`/i); + assert.match(runtimeSkill, /launches once with `task --background --json`, polls only that job's own `status`, then returns the `result` stdout unchanged/i); + assert.match(runtimeSkill, /Do not call `setup`, `review`, `adversarial-review`, or `cancel` from `codex:codex-rescue`/i); + assert.match(runtimeSkill, /`status` and `result` are allowed, but only for the job you just launched/i); assert.match(runtimeSkill, /use the `gpt-5-4-prompting` skill to rewrite the user's request into a tighter Codex prompt/i); assert.match(runtimeSkill, /That prompt drafting is the only Claude-side work allowed/i); assert.match(runtimeSkill, /Leave `--effort` unset unless the user explicitly requests a specific effort/i); @@ -150,13 +140,13 @@ 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, /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(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, cancel jobs, summarize output, or do any other follow-up work of your own beyond launching and polling your own job/i); + assert.match(runtimeSkill, /If the Bash call fails or Codex cannot be invoked, return the command's exit status and stderr verbatim/i); assert.match(readme, /`codex:codex-rescue` subagent/i); assert.match(readme, /if you do not pass `--model` or `--effort`, Codex chooses its own defaults/i); assert.match(readme, /--model gpt-5\.4-mini --effort medium/i); - assert.match(readme, /`spark`, the plugin maps that to `gpt-5\.3-codex-spark`/i); + assert.match(readme, /`spark` -> `gpt-5\.3-codex-spark`/i); assert.match(readme, /continue a previous Codex task/i); assert.match(readme, /### `\/codex:setup`/); assert.match(readme, /### `\/codex:review`/); @@ -170,6 +160,29 @@ test("rescue command absorbs continue semantics", () => { assert.match(readme, /### `\/codex:cancel`/); }); +test("rescue runs synchronously through the companion and uses Agent only for --background", () => { + const rescue = fs.readFileSync(path.join(PLUGIN_ROOT, "commands", "rescue.md"), "utf8"); + const agent = fs.readFileSync(path.join(PLUGIN_ROOT, "agents", "codex-rescue.md"), "utf8"); + const runtimeSkill = fs.readFileSync(path.join(PLUGIN_ROOT, "skills", "codex-cli-runtime", "SKILL.md"), "utf8"); + assert.match(rescue, /task --background --json/); + assert.match(rescue, /status "\$JOB" --wait --timeout-ms 540000/); + assert.match(rescue, /result "\$JOB"/); + assert.match(rescue, /Only when the request contains `--background`.*Agent/s); + assert.match(rescue, /--config/); + assert.doesNotMatch(agent, /^model:/m); + assert.doesNotMatch(agent, /return nothing/i); + assert.match(agent, /exit status and stderr/i); + assert.match(agent, /task --background --json/); + assert.match(agent, /status "\$JOB" --wait --timeout-ms 540000/); + assert.doesNotMatch(agent, /Do not .*poll status/i); + assert.match(agent, /--config/); + assert.doesNotMatch(runtimeSkill, /return nothing/i); + assert.match(runtimeSkill, /Map `sol` to `--model gpt-5\.6-sol`/i); + assert.match(runtimeSkill, /\$agent-compat:skill-router/); + assert.doesNotMatch(agent, /adding `--write` unless/i); + assert.doesNotMatch(runtimeSkill, /adding `--write` unless/i); +}); + test("transfer, result, and cancel commands are exposed as deterministic runtime entrypoints", () => { const transfer = read("commands/transfer.md"); const result = read("commands/result.md"); @@ -177,12 +190,12 @@ test("transfer, result, and cancel commands are exposed as deterministic runtime const resultHandling = read("skills/codex-result-handling/SKILL.md"); assert.match(transfer, /disable-model-invocation:\s*true/); - assert.match(transfer, /codex-companion\.mjs" transfer "\$ARGUMENTS"/); + assert.match(transfer, /codex-companion\.mjs" transfer --args-stdin <<'CODEX_ARGS'/); assert.match(transfer, /codex resume /); assert.match(result, /disable-model-invocation:\s*true/); - assert.match(result, /codex-companion\.mjs" result "\$ARGUMENTS"/); + assert.match(result, /codex-companion\.mjs" result --args-stdin <<'CODEX_ARGS'/); assert.match(cancel, /disable-model-invocation:\s*true/); - assert.match(cancel, /codex-companion\.mjs" cancel "\$ARGUMENTS"/); + assert.match(cancel, /codex-companion\.mjs" cancel --args-stdin <<'CODEX_ARGS'/); assert.match(resultHandling, /do not turn a failed or incomplete Codex run into a Claude-side implementation attempt/i); assert.match(resultHandling, /if Codex was never successfully invoked, do not generate a substitute answer at all/i); }); @@ -210,6 +223,13 @@ test("hooks keep session-end cleanup and stop gating enabled", () => { assert.match(source, /session-lifecycle-hook\.mjs/); }); +test("session start hook allows enough time to restore session state", () => { + const hooks = JSON.parse(read("hooks/hooks.json")); + const sessionStartHook = hooks.hooks.SessionStart[0].hooks[0]; + + assert.equal(sessionStartHook.timeout, 60); +}); + test("setup command can offer Codex install and still points users to codex login", () => { const setup = read("commands/setup.md"); const readme = fs.readFileSync(path.join(ROOT, "README.md"), "utf8"); @@ -217,9 +237,133 @@ test("setup command can offer Codex install and still points users to codex logi assert.match(setup, /argument-hint:\s*'\[--enable-review-gate\|--disable-review-gate\]'/); assert.match(setup, /AskUserQuestion/); assert.match(setup, /npm install -g @openai\/codex/); - assert.match(setup, /codex-companion\.mjs" setup --json \$ARGUMENTS/); + assert.match(setup, /codex-companion\.mjs" setup --json --args-stdin <<'CODEX_ARGS'/); assert.match(readme, /!codex login/); assert.match(readme, /offer to install Codex for you/i); assert.match(readme, /\/codex:setup --enable-review-gate/); assert.match(readme, /\/codex:setup --disable-review-gate/); }); + +test("stop gate script timeout is shorter than the Stop hook timeout and its message matches", () => { + const hooks = JSON.parse(fs.readFileSync(path.join(PLUGIN_ROOT, "hooks", "hooks.json"), "utf8")); + const stopTimeoutSeconds = hooks.hooks.Stop[0].hooks[0].timeout; + const source = fs.readFileSync(path.join(PLUGIN_ROOT, "scripts", "stop-review-gate-hook.mjs"), "utf8"); + const minutes = source.match(/const STOP_REVIEW_TIMEOUT_MINUTES = (\d+);/); + assert.ok(minutes, "STOP_REVIEW_TIMEOUT_MINUTES must be a named constant"); + assert.match(source, /const STOP_REVIEW_TIMEOUT_MS = STOP_REVIEW_TIMEOUT_MINUTES \* 60 \* 1000;/); + assert.ok(Number(minutes[1]) * 60 < stopTimeoutSeconds, "script timeout must be below the hook timeout"); + assert.doesNotMatch(source, /15 minutes/); + assert.match(source, /\$\{STOP_REVIEW_TIMEOUT_MINUTES\} minutes/); + assert.match(source, /killSignal: "SIGKILL"/); + assert.match(source, /maxBuffer: 16 \* 1024 \* 1024/); +}); + +test("marketplace is published under cbepx while the plugin keeps the codex name", () => { + const marketplace = JSON.parse(fs.readFileSync(path.join(ROOT, ".claude-plugin", "marketplace.json"), "utf8")); + const plugin = JSON.parse(fs.readFileSync(path.join(PLUGIN_ROOT, ".claude-plugin", "plugin.json"), "utf8")); + assert.equal(marketplace.name, "cbepx"); + assert.equal(marketplace.owner.name, "CBEPX"); + assert.equal(plugin.name, "codex"); + assert.equal(marketplace.plugins[0].name, "codex"); + assert.equal(marketplace.plugins[0].version, plugin.version); +}); + +function assertArgumentsNeverReachTheShell(label, body) { + body.split("\n").forEach((line, index) => { + if (!line.includes("$ARGUMENTS")) { + return; + } + const trimmed = line.trim(); + assert.ok( + trimmed === "$ARGUMENTS" || trimmed === "`$ARGUMENTS`", + `${label}:${index + 1} exposes $ARGUMENTS to the shell: ${line}` + ); + }); +} + +test("command bodies hand arguments to the companion via a quoted heredoc, never inside a shell string", () => { + const commandFiles = fs.readdirSync(path.join(PLUGIN_ROOT, "commands")).sort(); + for (const file of commandFiles) { + const body = read(path.join("commands", file)); + assert.doesNotMatch(body, /"\$ARGUMENTS"/, `${file} still interpolates $ARGUMENTS inside a shell string`); + // $ARGUMENTS may only appear as inline-code prose (`$ARGUMENTS`) or as the + // whole body line of a quoted heredoc. Anywhere else the shell expands what + // Claude Code substituted before bash ever ran. + assertArgumentsNeverReachTheShell(file, body); + // rescue.md randomizes its delimiter suffix per call; the flag-only bodies keep the fixed one. + const expectedDelimiter = file === "rescue.md" ? /--args-stdin <<'CODEX_ARGS_/ : /--args-stdin <<'CODEX_ARGS'/; + assert.match(body, expectedDelimiter, `${file} must pass arguments through a quoted heredoc`); + } + + const rescue = read("commands/rescue.md"); + const agent = read("agents/codex-rescue.md"); + for (const [label, body] of [["rescue.md", rescue], ["codex-rescue.md", agent]]) { + assert.doesNotMatch(body, /""/, `${label} still interpolates the request text inside a shell string`); + assert.match( + body, + /task --background --json --prompt-file "\$PROMPT" --args-stdin <<'CODEX_ARGS/, + `${label} must launch through a quoted heredoc` + ); + assert.match( + body, + /\[\[ "\$JOB" =~ \^\[A-Za-z0-9_-\]\+\$ \]\] \|\| \{ echo "invalid job id"; exit 1; \}/, + `${label} must validate the job id before using it` + ); + } +}); + +// The request prose and the runtime flags travel in separate channels: the prose +// via --prompt-file (byte-exact) and only the flags through the tokenizer. +function readArgsHeredocBody(body) { + const lines = body.split("\n"); + const start = lines.findIndex((line) => line.includes("--args-stdin <<'CODEX_ARGS")); + assert.notEqual(start, -1, "no --args-stdin heredoc found"); + const end = lines.findIndex((line, index) => index > start && line.trim().startsWith("CODEX_ARGS")); + assert.notEqual(end, -1, "unterminated --args-stdin heredoc"); + return lines.slice(start + 1, end).join("\n"); +} + +test("rescue sends the request prose through --prompt-file, never through the argument tokenizer", () => { + for (const [label, body] of [ + ["rescue.md", read("commands/rescue.md")], + ["codex-rescue.md", read("agents/codex-rescue.md")] + ]) { + assert.match(body, /cat > "\$PROMPT" <<'CODEX_PROMPT_/, `${label} must write the request prose with its own quoted heredoc`); + assert.match(body, /--prompt-file "\$PROMPT"/, `${label} must pass the prose file to the companion`); + assert.doesNotMatch( + readArgsHeredocBody(body), + //, + `${label} still routes the request text through the argument tokenizer` + ); + assert.match(body, /rm -f "\$ERR" "\$OUT" "\$PROMPT"/, `${label} must clean up the prose file`); + + // A payload line equal to a fixed delimiter would close the heredoc early and + // run the rest on the host shell. + assert.match(body, /fresh random suffix on every call/, `${label} must require per-call heredoc delimiters`); + assert.match(body, /`CODEX_PROMPT_` \/ `CODEX_ARGS_`/, `${label} must name both randomized delimiters`); + } +}); + +test("README documents the fork's own install commands", () => { + const readme = fs.readFileSync(path.join(ROOT, "README.md"), "utf8"); + + assert.match(readme, /plugin marketplace add CBEPX\/codex-plugin-cc/); + assert.match(readme, /plugin install codex@cbepx/); + + // No install line may point at the upstream marketplace or plugin id. The one + // line allowed to name upstream is the "Upstream:" attribution. + readme.split("\n").forEach((line, index) => { + if (!/plugin (marketplace add|install)/.test(line) || !line.includes("openai")) { + return; + } + assert.ok(line.includes("Upstream:"), `README.md:${index + 1} still documents an upstream install: ${line}`); + }); + assert.doesNotMatch(readme, /openai-codex/); +}); + +test("bump-version --check pins the lockfile identity to package.json", () => { + const lock = JSON.parse(fs.readFileSync(path.join(ROOT, "package-lock.json"), "utf8")); + const pkg = JSON.parse(fs.readFileSync(path.join(ROOT, "package.json"), "utf8")); + assert.equal(lock.name, pkg.name); + assert.equal(lock.packages[""].name, pkg.name); +}); diff --git a/tests/fake-codex-fixture.mjs b/tests/fake-codex-fixture.mjs index f83c96a0d..fb058e0b6 100644 --- a/tests/fake-codex-fixture.mjs +++ b/tests/fake-codex-fixture.mjs @@ -116,6 +116,14 @@ function send(message) { process.stdout.write(JSON.stringify(message) + "\\n"); } +function resolvedModel(params) { + return params.model || params.config?.model || "gpt-5.4"; +} + +function resolvedEffort(params) { + return params.config?.model_reasoning_effort ?? (BEHAVIOR === "resolved-effort" ? "medium" : null); +} + function nextThread(state, cwd, ephemeral) { const thread = { id: "thr_" + state.nextThreadId++, @@ -313,7 +321,9 @@ rl.on("line", (line) => { 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 } }); + state.lastThreadStart = { ...message.params, threadId: thread.id }; + saveState(state); + send({ id: message.id, result: { thread: buildThread(thread), model: resolvedModel(message.params), modelProvider: "openai", serviceTier: null, cwd: thread.cwd, approvalPolicy: message.params.approvalPolicy || "never", sandbox: { type: "readOnly", access: { type: "fullAccess" }, networkAccess: false }, reasoningEffort: resolvedEffort(message.params) } }); send({ method: "thread/started", params: { thread: { id: thread.id } } }); break; } @@ -346,8 +356,9 @@ rl.on("line", (line) => { } const thread = ensureThread(state, message.params.threadId); thread.updatedAt = now(); + state.lastThreadResume = { ...message.params }; 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 } }); + send({ id: message.id, result: { thread: buildThread(thread), model: resolvedModel(message.params), modelProvider: "openai", serviceTier: null, cwd: thread.cwd, approvalPolicy: message.params.approvalPolicy || "never", sandbox: { type: "readOnly", access: { type: "fullAccess" }, networkAccess: false }, reasoningEffort: resolvedEffort(message.params) } }); break; } @@ -437,6 +448,9 @@ rl.on("line", (line) => { } case "turn/start": { + if (BEHAVIOR === "turn-start-fails") { + throw new Error("turn/start failed after thread resolution"); + } const thread = ensureThread(state, message.params.threadId); const prompt = (message.params.input || []) .filter((item) => item.type === "text") diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 8f276835b..ce2162dcd 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -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,6 +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/); + assert.deepEqual(readPersistedJob(repo).resolved, FAKE_RESOLVED_SETTINGS); }); test("task runs when the active provider does not require OpenAI login", () => { @@ -384,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", () => { @@ -501,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", () => { @@ -701,6 +721,7 @@ test("session start hook exports the Claude session id, transcript path, and plu test("write task output focuses on the Codex result without generic follow-up hints", () => { 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"); @@ -714,6 +735,58 @@ test("write task output focuses on the Codex result without generic follow-up hi assert.equal(result.status, 0, result.stderr); assert.equal(result.stdout, "Handled the requested task.\nTask prompt accepted.\n"); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastThreadStart.approvalPolicy, "on-request"); + assert.equal(fakeState.lastThreadStart.sandbox, "workspace-write"); +}); + +test("read-only task keeps never approval policy on app-server thread/start", () => { + 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"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + + const result = run("node", [SCRIPT, "task", "inspect the failing test"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastThreadStart.approvalPolicy, "never"); + assert.equal(fakeState.lastThreadStart.sandbox, "read-only"); +}); + +test("task --resume-last --write forwards write approval policy to app-server thread/resume", () => { + 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"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: 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", "--resume-last", "--write", "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.lastThreadResume.threadId, "thr_1"); + assert.equal(fakeState.lastThreadResume.approvalPolicy, "on-request"); + assert.equal(fakeState.lastThreadResume.sandbox, "workspace-write"); }); test("task --resume acts like --resume-last without leaking the flag into the prompt", () => { @@ -767,7 +840,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 }); @@ -782,6 +855,121 @@ 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"); + // `--effort` is applied via thread/start.config, so the resolved settings echo it back. + const resolvedWithEffort = { ...FAKE_RESOLVED_SETTINGS, reasoningEffort: "xhigh" }; + assert.deepEqual(storedJob.resolved, resolvedWithEffort); + const stateDir = resolveStateDir(repo); + const state = JSON.parse(fs.readFileSync(path.join(stateDir, "state.json"), "utf8")); + assert.deepEqual(state.jobs[0].resolved, resolvedWithEffort); +}); + +for (const effort of ["max", "ultra"]) { + test(`task forwards ${effort} reasoning effort to app-server turn/start`, () => { + 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"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + + const result = run("node", [SCRIPT, "task", "--effort", effort, "diagnose the failing test"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastTurnStart.effort, effort); + }); +} + +test("task rejects an unknown reasoning effort", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir); + 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", "supreme", "diagnose the failing test"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.notEqual(result.status, 0); + assert.match(result.stderr, /Unsupported reasoning effort "supreme"/); +}); + +test("review resolves model aliases the same way task does", () => { + 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"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastThreadStart.model, "gpt-5.3-codex-spark"); +}); + +test("adversarial review resolves model aliases the same way task does", () => { + 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, "adversarial-review", "--model", "spark"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastThreadStart.model, "gpt-5.3-codex-spark"); }); test("task logs reasoning summaries and assistant messages to the job log", () => { @@ -939,6 +1127,18 @@ test("task --background enqueues a detached worker and exposes per-job status", 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"], @@ -966,6 +1166,8 @@ 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/); }); @@ -1979,6 +2181,19 @@ test("stop hook runs a stop-time review task and blocks on findings when the rev assert.match(status.stdout, /Codex Stop Gate Review/); }); +test("stop hook blocks when hook input is malformed JSON", () => { + const blocked = run(process.execPath, [STOP_HOOK], { + cwd: ROOT, + input: "{not-json" + }); + + assert.equal(blocked.status, 0, blocked.stderr); + assert.deepEqual(JSON.parse(blocked.stdout), { + decision: "block", + reason: "The stop review gate could not read or parse hook input; refusing to fail open." + }); +}); + test("stop hook logs running tasks to stderr without blocking when the review gate is disabled", () => { const repo = makeTempDir(); initGitRepo(repo); @@ -2257,3 +2472,337 @@ test("setup and status honor --cwd when reading shared session runtime", () => { assert.equal(payload.sessionRuntime.mode, "shared"); assert.equal(payload.sessionRuntime.endpoint, "unix:/tmp/fake-broker.sock"); }); + +function seededRepo() { + const repo = makeTempDir(); + 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 }); + return repo; +} + +test("review forwards model, review_model, effort and config overrides into thread/start config", () => { + const repo = seededRepo(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + fs.writeFileSync(path.join(repo, "README.md"), "hello world\n"); + + const result = run( + "node", + [SCRIPT, "review", "--wait", "--model", "sol", "--effort", "max", "--config", "model_provider=ollama", "--config", "foo.bar=3"], + { cwd: repo, env: buildEnv(binDir) } + ); + + assert.equal(result.status, 0, result.stderr); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.deepEqual(fakeState.lastThreadStart.config, { + model_provider: "ollama", + "foo.bar": 3, + model: "gpt-5.6-sol", + review_model: "gpt-5.6-sol", + model_reasoning_effort: "max" + }); +}); + +test("review accepts slash-command style single-string arguments", () => { + const repo = seededRepo(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + fs.writeFileSync(path.join(repo, "README.md"), "hello world\n"); + + const result = run("node", [SCRIPT, "review", "--wait --effort xhigh --config model_provider=ollama"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.deepEqual(fakeState.lastThreadStart.config, { model_provider: "ollama", model_reasoning_effort: "xhigh" }); +}); + +test("task forwards config overrides and keeps option-looking prompt words", () => { + const repo = seededRepo(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + + const result = run("node", [SCRIPT, "task", "--effort", "max", "--config", "model_provider=ollama", "investigate", "ls", "-R", "usage"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.deepEqual(fakeState.lastThreadStart.config, { model_provider: "ollama", model_reasoning_effort: "max" }); + assert.match(fakeState.lastTurnStart.prompt, /investigate ls -R usage/); +}); + +test("task --resume-last never puts model or effort into thread/resume config", () => { + const repo = seededRepo(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + + const first = run("node", [SCRIPT, "task", "--model", "sol", "--effort", "high", "first"], { cwd: repo, env: buildEnv(binDir) }); + assert.equal(first.status, 0, first.stderr); + const startsAfterFirst = JSON.parse(fs.readFileSync(statePath, "utf8")).appServerStarts; + const second = run("node", [SCRIPT, "task", "--resume-last", "--effort", "max", "--config", "model_provider=ollama", "again"], { + cwd: repo, + env: buildEnv(binDir) + }); + assert.equal(second.status, 0, second.stderr); + + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.deepEqual(fakeState.lastThreadResume.config, { model_provider: "ollama" }); + assert.equal(fakeState.lastTurnStart.effort, "max"); + // A hot broker rejoin would ignore the config/sandbox overrides, so the resume + // must have run on a freshly spawned app-server process. + assert.equal(fakeState.appServerStarts, startsAfterFirst + 1); +}); + +test("task --resume-last cold-resumes without a thread/resume model override", () => { + const repo = seededRepo(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + + const first = run("node", [SCRIPT, "task", "--model", "sol", "--effort", "high", "first"], { cwd: repo, env: buildEnv(binDir) }); + assert.equal(first.status, 0, first.stderr); + const startsAfterFirst = JSON.parse(fs.readFileSync(statePath, "utf8")).appServerStarts; + + const second = run("node", [SCRIPT, "task", "--resume-last", "--model", "sol", "--effort", "max", "again"], { + cwd: repo, + env: buildEnv(binDir) + }); + assert.equal(second.status, 0, second.stderr); + + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + // A top-level `model` on thread/resume sets has_model_resume_override and stops + // Codex restoring the thread's persisted model/provider/effort. + assert.equal(fakeState.lastThreadResume.model, undefined); + assert.equal(fakeState.lastThreadResume.config, null); + assert.equal(fakeState.appServerStarts, startsAfterFirst + 1); + assert.equal(fakeState.lastTurnStart.model, "gpt-5.6-sol"); + assert.equal(fakeState.lastTurnStart.effort, "max"); +}); + +test("task --background stores config overrides in the job request", () => { + const repo = seededRepo(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + + const result = run("node", [SCRIPT, "task", "--background", "--json", "--config", "model_provider=ollama", "bg"], { + cwd: repo, + env: buildEnv(binDir) + }); + assert.equal(result.status, 0, result.stderr); + const jobId = JSON.parse(result.stdout).jobId; + const done = run("node", [SCRIPT, "status", jobId, "--wait", "--timeout-ms", "20000", "--json"], { cwd: repo, env: buildEnv(binDir) }); + assert.equal(done.status, 0, done.stderr); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.deepEqual(fakeState.lastThreadStart.config, { model_provider: "ollama" }); +}); + +test("status --args-stdin tokenizes the raw argument string from stdin", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir); + initGitRepo(repo); + + const viaStdin = run("node", [SCRIPT, "status", "--args-stdin"], { + cwd: repo, + env: buildEnv(binDir), + input: "--all --json\n" + }); + const viaArgv = run("node", [SCRIPT, "status", "--all", "--json"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(viaStdin.status, 0, viaStdin.stderr); + assert.equal(viaArgv.status, 0, viaArgv.stderr); + assert.deepEqual(JSON.parse(viaStdin.stdout), JSON.parse(viaArgv.stdout)); +}); + +test("task --args-stdin keeps shell metacharacters inside the prompt instead of executing them", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + const sentinel = path.join(makeTempDir(), "pwned"); + installFakeCodex(binDir); + 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 rawArguments = `--effort max investigate $(touch ${sentinel}) \`id\``; + const result = run("node", [SCRIPT, "task", "--args-stdin"], { + cwd: repo, + env: buildEnv(binDir), + input: `${rawArguments}\n` + }); + + assert.equal(result.status, 0, result.stderr); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastTurnStart.effort, "max"); + assert.equal(fakeState.lastTurnStart.prompt, `investigate $(touch ${sentinel}) \`id\``); + assert.equal(fs.existsSync(sentinel), false); +}); + +test("task --background persists the job record before spawning the worker", () => { + const repo = seededRepo(); + const binDir = makeTempDir(); + installFakeCodex(binDir, "slow-task"); + + const launched = run("node", [SCRIPT, "task", "--background", "--json", "investigate the ordering"], { + cwd: repo, + env: buildEnv(binDir) + }); + assert.equal(launched.status, 0, launched.stderr); + const { jobId } = JSON.parse(launched.stdout); + + // The launch command has returned, so the record must already be readable by + // the worker no matter how fast it started. + const stateDir = resolveStateDir(repo); + const indexed = JSON.parse(fs.readFileSync(path.join(stateDir, "state.json"), "utf8")).jobs.find((job) => job.id === jobId); + assert.ok(indexed, "job must be in the state index as soon as the launch returns"); + assert.ok(["queued", "running"].includes(indexed.status), `unexpected status ${indexed.status}`); + assert.ok(fs.existsSync(path.join(stateDir, "jobs", `${jobId}.json`)), "job file must exist as soon as the launch returns"); + + const waited = run("node", [SCRIPT, "status", jobId, "--wait", "--timeout-ms", "20000", "--json"], { + cwd: repo, + env: buildEnv(binDir) + }); + assert.equal(waited.status, 0, waited.stderr); + assert.equal(JSON.parse(waited.stdout).job.status, "completed"); +}); + +test("task --background keeps secret --config values out of every job record", () => { + const repo = seededRepo(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + + const launched = run( + "node", + [ + SCRIPT, + "task", + "--background", + "--json", + "--config", + "model_providers.x.http_headers.Authorization=SECRET_SENTINEL_42", + "--config", + "model_provider=ollama", + "x" + ], + { cwd: repo, env: buildEnv(binDir) } + ); + assert.equal(launched.status, 0, launched.stderr); + const { jobId } = JSON.parse(launched.stdout); + + const waited = run("node", [SCRIPT, "status", jobId, "--wait", "--timeout-ms", "20000", "--json"], { + cwd: repo, + env: buildEnv(binDir) + }); + assert.equal(waited.status, 0, waited.stderr); + assert.equal(JSON.parse(waited.stdout).job.status, "completed"); + const resultRun = run("node", [SCRIPT, "result", jobId, "--json"], { cwd: repo, env: buildEnv(binDir) }); + assert.equal(resultRun.status, 0, resultRun.stderr); + + const stateDir = resolveStateDir(repo); + const exposures = { + "state index": fs.readFileSync(path.join(stateDir, "state.json"), "utf8"), + "job file": fs.readFileSync(path.join(stateDir, "jobs", `${jobId}.json`), "utf8"), + "status --json stdout": waited.stdout, + "result --json stdout": resultRun.stdout + }; + for (const [label, text] of Object.entries(exposures)) { + assert.equal(text.includes("SECRET_SENTINEL_42"), false, `${label} leaked the secret --config value`); + assert.equal(text.includes("[redacted]"), true, `${label} should keep the redacted placeholder`); + assert.equal(text.includes("ollama"), true, `${label} should keep non-secret config values readable`); + } + + // The one-shot payload file is deleted by the worker once it has read it. + assert.equal(fs.existsSync(path.join(stateDir, "jobs", `${jobId}.request.json`)), false); + + // The worker still forwarded the real value to Codex. + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastThreadStart.config["model_providers.x.http_headers.Authorization"], "SECRET_SENTINEL_42"); + assert.equal(fakeState.lastThreadStart.config.model_provider, "ollama"); +}); + +test("a resume refuses to start a second turn on a thread another job is still using", () => { + const repo = seededRepo(); + const binDir = makeTempDir(); + installFakeCodex(binDir); + const env = { ...buildEnv(binDir), CODEX_COMPANION_SESSION_ID: "sess-current" }; + + const first = run("node", [SCRIPT, "task", "first"], { cwd: repo, env }); + assert.equal(first.status, 0, first.stderr); + + // Another Claude session is mid-turn on the very thread this session would + // resume. Its job is invisible to this session's resume-candidate lookup, so + // only the thread-level guard can catch it. + const statePath = path.join(resolveStateDir(repo), "state.json"); + const state = JSON.parse(fs.readFileSync(statePath, "utf8")); + const busyJob = { + id: "task-other-running", + status: "running", + phase: "running", + title: "Codex Task", + jobClass: "task", + sessionId: "sess-other", + threadId: "thr_1", + summary: "Other session active task", + updatedAt: "2026-03-24T20:05:00.000Z" + }; + state.jobs.push(busyJob); + fs.writeFileSync(statePath, `${JSON.stringify(state, null, 2)}\n`, "utf8"); + + const blocked = run("node", [SCRIPT, "task", "--resume-last", "follow up"], { cwd: repo, env }); + assert.notEqual(blocked.status, 0); + assert.match( + blocked.stderr, + /Thread thr_1 is busy in job task-other-running; wait for it or run cancel task-other-running first\./ + ); + + // Once that job finishes, the same resume goes through. + busyJob.status = "completed"; + busyJob.phase = "done"; + fs.writeFileSync(statePath, `${JSON.stringify(state, null, 2)}\n`, "utf8"); + + const resumed = run("node", [SCRIPT, "task", "--resume-last", "follow up"], { cwd: repo, env }); + assert.equal(resumed.status, 0, resumed.stderr); + const fakeState = JSON.parse(fs.readFileSync(path.join(binDir, "fake-codex-state.json"), "utf8")); + assert.equal(fakeState.lastTurnStart.threadId, "thr_1"); + assert.equal(fakeState.lastTurnStart.prompt, "follow up"); +}); + +test("task --prompt-file wins over --args-stdin and keeps the prompt byte-exact", () => { + const repo = seededRepo(); + const binDir = makeTempDir(); + const statePath = path.join(binDir, "fake-codex-state.json"); + installFakeCodex(binDir); + + // Everything splitRawArgumentString would eat: quotes as grouping, backslashes + // as escapes, newlines as separators. + const promptText = `line one \\d+ "quoted" 'single' C:\\Users\\x\nsecond line with $(id) and \`backticks\``; + const promptFile = path.join(makeTempDir(), "request.txt"); + fs.writeFileSync(promptFile, promptText, "utf8"); + + const result = run("node", [SCRIPT, "task", "--prompt-file", promptFile, "--args-stdin"], { + cwd: repo, + env: buildEnv(binDir), + input: "--effort max\n" + }); + + assert.equal(result.status, 0, result.stderr); + const fakeState = JSON.parse(fs.readFileSync(statePath, "utf8")); + assert.equal(fakeState.lastTurnStart.prompt, promptText); + assert.equal(fakeState.lastTurnStart.effort, "max"); +}); diff --git a/tests/state.test.mjs b/tests/state.test.mjs index 0f8f57cea..b1148f06e 100644 --- a/tests/state.test.mjs +++ b/tests/state.test.mjs @@ -5,7 +5,16 @@ import test from "node:test"; import assert from "node:assert/strict"; import { makeTempDir } from "./helpers.mjs"; -import { resolveJobFile, resolveJobLogFile, resolveStateDir, resolveStateFile, saveState } from "../plugins/codex/scripts/lib/state.mjs"; +import { + consumeJobRequestFile, + resolveJobFile, + resolveJobLogFile, + resolveJobRequestFile, + resolveStateDir, + resolveStateFile, + saveState, + writeJobRequestFile +} from "../plugins/codex/scripts/lib/state.mjs"; test("resolveStateDir uses a temp-backed per-workspace directory", () => { const workspace = makeTempDir(); @@ -103,3 +112,27 @@ test("saveState prunes dropped job artifacts when indexed jobs exceed the cap", .sort() ); }); + +test("job request payloads are written owner-only and consumed exactly once", () => { + const workspace = makeTempDir(); + const payload = { prompt: "go", config: { "http_headers.Authorization": "SECRET" } }; + + const requestFile = writeJobRequestFile(workspace, "task-1", payload); + assert.equal(requestFile, resolveJobRequestFile(workspace, "task-1")); + assert.equal(fs.statSync(requestFile).mode & 0o777, 0o600); + + assert.deepEqual(consumeJobRequestFile(workspace, "task-1"), payload); + assert.equal(fs.existsSync(requestFile), false); + assert.equal(consumeJobRequestFile(workspace, "task-1"), null); +}); + +test("saveState drops the private request payload of pruned jobs", () => { + const workspace = makeTempDir(); + const requestFile = writeJobRequestFile(workspace, "task-dropped", { prompt: "go" }); + + saveState(workspace, { jobs: [{ id: "task-dropped", updatedAt: "2026-01-01T00:00:00.000Z" }] }); + assert.equal(fs.existsSync(requestFile), true); + + saveState(workspace, { jobs: [] }); + assert.equal(fs.existsSync(requestFile), false); +}); diff --git a/tests/test-env.mjs b/tests/test-env.mjs new file mode 100644 index 000000000..47106acb8 --- /dev/null +++ b/tests/test-env.mjs @@ -0,0 +1,14 @@ +// Hermetic test environment: strip host-session variables that Claude Code / +// the plugin's own SessionStart hook export, so tests see a clean machine. +for (const name of [ + "CLAUDE_PLUGIN_DATA", + "CLAUDE_ENV_FILE", + "CODEX_COMPANION_SESSION_ID", + "CODEX_COMPANION_TRANSCRIPT_PATH", + "CODEX_COMPANION_APP_SERVER_ENDPOINT", + "CODEX_COMPANION_APP_SERVER_PID_FILE", + "CODEX_COMPANION_APP_SERVER_LOG_FILE", + "CODEX_PLUGIN_CC_ARGS" +]) { + delete process.env[name]; +} diff --git a/tests/thread-config.test.mjs b/tests/thread-config.test.mjs new file mode 100644 index 000000000..adc9d8eb6 --- /dev/null +++ b/tests/thread-config.test.mjs @@ -0,0 +1,26 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { buildThreadConfig } from "../plugins/codex/scripts/lib/codex.mjs"; + +test("buildThreadConfig returns null when nothing is set", () => { + assert.equal(buildThreadConfig({}), null); + assert.equal(buildThreadConfig({ config: {} }), null); +}); + +test("buildThreadConfig maps model, review model and effort to Codex config keys", () => { + assert.deepEqual(buildThreadConfig({ model: "gpt-5.6-sol", effort: "max", reviewModel: "gpt-5.6-sol" }), { + model: "gpt-5.6-sol", + review_model: "gpt-5.6-sol", + model_reasoning_effort: "max" + }); +}); + +test("buildThreadConfig lets dedicated flags win over generic overrides and parses JSON-ish values", () => { + assert.deepEqual( + buildThreadConfig({ + effort: "max", + config: { model_reasoning_effort: "low", "sandbox_workspace_write.network_access": "true", model_provider: "ollama", n: "3" } + }), + { "sandbox_workspace_write.network_access": true, model_provider: "ollama", n: 3, model_reasoning_effort: "max" } + ); +});