From 6c692458d0313e235ed9ca2576f4450e48df5f3b Mon Sep 17 00:00:00 2001 From: Jan Frehar Date: Sun, 23 Aug 2026 20:07:26 +0200 Subject: [PATCH] fix: resolve codex rescue plugin root --- plugins/codex/agents/codex-rescue.md | 4 +- plugins/codex/commands/rescue.md | 33 ++++++- .../codex/skills/codex-cli-runtime/SKILL.md | 40 +++++++- tests/commands.test.mjs | 96 ++++++++++++++++++- 4 files changed, 167 insertions(+), 6 deletions(-) diff --git a/plugins/codex/agents/codex-rescue.md b/plugins/codex/agents/codex-rescue.md index 7009ec86a..ab5e6675c 100644 --- a/plugins/codex/agents/codex-rescue.md +++ b/plugins/codex/agents/codex-rescue.md @@ -19,7 +19,9 @@ Selection guidance: Forwarding rules: -- Use exactly one `Bash` call to invoke `node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" task ...`. +- Use exactly one `Bash` call. Execute the `Primary helper` from the preloaded `codex-cli-runtime` skill unchanged, replacing its `...` placeholder only with the routed `task` arguments. +- Do not copy, shorten, or reimplement that helper: it owns plugin-root resolution, argv forwarding, inherited stdio, and exit propagation. +- Invoke `task` exactly once and return the helper stdout exactly as-is. - 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. - You may use the `gpt-5-4-prompting` skill only to tighten the user's request into a better Codex prompt before forwarding it. diff --git a/plugins/codex/commands/rescue.md b/plugins/codex/commands/rescue.md index 56de9555d..b25154226 100644 --- a/plugins/codex/commands/rescue.md +++ b/plugins/codex/commands/rescue.md @@ -23,9 +23,38 @@ Execution mode: - Otherwise, before starting Codex, check for a resumable rescue thread from this Claude session by running: ```bash -node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" task-resume-candidate --json +node -e ' + const fs = require("node:fs"); + const path = require("node:path"); + const { spawnSync } = require("node:child_process"); + const scriptFor = root => path.join(root, "scripts", "codex-companion.mjs"); + const validRoot = root => { + if (!root) return false; + const script = scriptFor(root); + try { + if (!fs.statSync(script).isFile()) return false; + fs.accessSync(script, fs.constants.R_OK); + return true; + } catch { return false; } + }; + let roots = validRoot(process.env.CLAUDE_PLUGIN_ROOT) ? [process.env.CLAUDE_PLUGIN_ROOT] : []; + if (roots.length === 0) { + const configDir = process.env.CLAUDE_CONFIG_DIR || (process.env.HOME && path.join(process.env.HOME, ".claude")); + if (!configDir) process.exit(1); + try { + const registry = JSON.parse(fs.readFileSync(path.join(configDir, "plugins", "installed_plugins.json"), "utf8")); + const records = registry && registry.version === 2 && registry.plugins && registry.plugins["codex@openai-codex"]; + roots = Array.isArray(records) ? records.map(record => record && record.installPath).filter(validRoot) : []; + } catch { process.exit(1); } + } + if (roots.length !== 1) process.exit(1); + const result = spawnSync(process.execPath, [scriptFor(roots[0]), ...process.argv.slice(1)], { stdio: "inherit" }); + process.exit(result.status === null ? 1 : result.status); +' task-resume-candidate --json ``` +Use that resolver exactly: a valid, non-empty `CLAUDE_PLUGIN_ROOT` is the fast path; otherwise read only `${CLAUDE_CONFIG_DIR:-$HOME/.claude}/plugins/installed_plugins.json` and require exactly one valid record under the exact `codex@openai-codex` key. Each valid record must have an `installPath` containing a readable regular `scripts/codex-companion.mjs`. An unreadable or malformed registry, a missing key, or zero or multiple valid records must fail before companion execution. Never glob plugin caches or versions, and never use `eval`. Keep routed arguments after the inline script, inherit child stdio, and propagate its exact exit status. + - 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` @@ -38,7 +67,7 @@ node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" task-resume-candidate - 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. +- The subagent is a thin forwarder only. It should use one `Bash` call, run the same fail-closed Node bootstrap used by the resume preflight with routed `task` arguments after the inline script, invoke the companion task exactly once, 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. diff --git a/plugins/codex/skills/codex-cli-runtime/SKILL.md b/plugins/codex/skills/codex-cli-runtime/SKILL.md index 0e91bfb50..987ca29f1 100644 --- a/plugins/codex/skills/codex-cli-runtime/SKILL.md +++ b/plugins/codex/skills/codex-cli-runtime/SKILL.md @@ -8,8 +8,44 @@ user-invocable: false Use this skill only inside the `codex:codex-rescue` subagent. -Primary helper: -- `node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" task ""` +Primary helper (put the routed arguments after the inline script): + +```bash +node -e ' + const fs = require("node:fs"); + const path = require("node:path"); + const { spawnSync } = require("node:child_process"); + const scriptFor = root => path.join(root, "scripts", "codex-companion.mjs"); + const validRoot = root => { + if (!root) return false; + const script = scriptFor(root); + try { + if (!fs.statSync(script).isFile()) return false; + fs.accessSync(script, fs.constants.R_OK); + return true; + } catch { return false; } + }; + let roots = validRoot(process.env.CLAUDE_PLUGIN_ROOT) ? [process.env.CLAUDE_PLUGIN_ROOT] : []; + if (roots.length === 0) { + const configDir = process.env.CLAUDE_CONFIG_DIR || (process.env.HOME && path.join(process.env.HOME, ".claude")); + if (!configDir) process.exit(1); + try { + const registry = JSON.parse(fs.readFileSync(path.join(configDir, "plugins", "installed_plugins.json"), "utf8")); + const records = registry && registry.version === 2 && registry.plugins && registry.plugins["codex@openai-codex"]; + roots = Array.isArray(records) ? records.map(record => record && record.installPath).filter(validRoot) : []; + } catch { process.exit(1); } + } + if (roots.length !== 1) process.exit(1); + const result = spawnSync(process.execPath, [scriptFor(roots[0]), ...process.argv.slice(1)], { stdio: "inherit" }); + process.exit(result.status === null ? 1 : result.status); +' task ... +``` + +Resolver rules: +- Keep the resolver and companion execution in the one allowed Bash call, with exactly one `task` invocation. +- A valid, non-empty `CLAUDE_PLUGIN_ROOT` is the fast path. Otherwise read only `${CLAUDE_CONFIG_DIR:-$HOME/.claude}/plugins/installed_plugins.json`, select the exact `codex@openai-codex` key, and require exactly one record whose `installPath` contains a readable regular `scripts/codex-companion.mjs`. +- An unreadable or malformed registry, a missing key, or zero or multiple valid records must fail before companion execution. Never glob plugin caches or versions, and never use `eval`. +- Keep routed arguments after the inline script. Forward them with `process.argv.slice(1)`, inherit child stdio, and propagate its exact exit status. Execution rules: - The rescue subagent is a forwarder, not an orchestrator. Its only job is to invoke `task` once and return that stdout unchanged. diff --git a/tests/commands.test.mjs b/tests/commands.test.mjs index 3724ffa47..37c526eab 100644 --- a/tests/commands.test.mjs +++ b/tests/commands.test.mjs @@ -3,6 +3,7 @@ import path from "node:path"; import test from "node:test"; import assert from "node:assert/strict"; import { fileURLToPath } from "node:url"; +import { makeTempDir, run } from "./helpers.mjs"; const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); const PLUGIN_ROOT = path.join(ROOT, "plugins", "codex"); @@ -11,6 +12,30 @@ function read(relativePath) { return fs.readFileSync(path.join(PLUGIN_ROOT, relativePath), "utf8"); } +function assertRegistryResolver(source) { + assert.match(source, /validRoot\(process\.env\.CLAUDE_PLUGIN_ROOT\)/); + assert.match(source, /CLAUDE_CONFIG_DIR \|\| \(process\.env\.HOME && path\.join\(process\.env\.HOME, "\.claude"\)\)/); + assert.match(source, /path\.join\(configDir, "plugins", "installed_plugins\.json"\)/); + assert.match(source, /registry\.version === 2/); + assert.match(source, /registry\.plugins\["codex@openai-codex"\]/); + assert.match(source, /record && record\.installPath/); + assert.match(source, /fs\.statSync\(script\)\.isFile\(\)/); + assert.match(source, /fs\.accessSync\(script, fs\.constants\.R_OK\)/); + assert.match(source, /roots\.length !== 1/); + assert.match(source, /spawnSync\(process\.execPath/); + assert.match(source, /process\.argv\.slice\(1\)/); + assert.match(source, /stdio: "inherit"/); + assert.match(source, /result\.status === null \? 1 : result\.status/); + assert.doesNotMatch(source, /\.claude\/plugins\/cache|plugins\/cache\/|openai-codex\/\d+\.\d+/); + assert.doesNotMatch(source, /\beval\s/); +} + +function firstBashBlock(source) { + const match = source.match(/```bash\n([\s\S]*?)\n```/); + assert.ok(match, "expected a bash code block"); + return match[1]; +} + test("review command uses AskUserQuestion and background Bash while staying review-only", () => { const source = read("commands/review.md"); assert.match(source, /AskUserQuestion/); @@ -88,6 +113,15 @@ test("rescue command absorbs continue semantics", () => { const agent = read("agents/codex-rescue.md"); const readme = fs.readFileSync(path.join(ROOT, "README.md"), "utf8"); const runtimeSkill = read("skills/codex-cli-runtime/SKILL.md"); + assertRegistryResolver(rescue); + assertRegistryResolver(runtimeSkill); + assert.equal((runtimeSkill.match(/^\s*' task \.\.\.$/gm) || []).length, 1); + assert.equal((rescue.match(/^' task-resume-candidate --json$/gm) || []).length, 1); + assert.match(rescue, /same fail-closed Node bootstrap used by the resume preflight/i); + assert.match(agent, /- codex-cli-runtime/); + assert.match(agent, /Execute the `Primary helper` from the preloaded `codex-cli-runtime` skill unchanged/i); + assert.match(agent, /Invoke `task` exactly once/i); + assert.match(runtimeSkill, /exactly one `task` invocation/i); 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/); @@ -167,6 +201,66 @@ test("rescue command absorbs continue semantics", () => { assert.match(readme, /### `\/codex:result`/); assert.match(readme, /### `\/codex:cancel`/); }); +test("rescue bootstrap resolves unset plugin root and fails closed on ambiguity", () => { + const temp = makeTempDir("codex-rescue-root-"); + const configDir = path.join(temp, "config"); + const pluginRoot = path.join(temp, "plugin root"); + const pluginsDir = path.join(configDir, "plugins"); + const scriptsDir = path.join(pluginRoot, "scripts"); + const registryPath = path.join(pluginsDir, "installed_plugins.json"); + const companionPath = path.join(scriptsDir, "codex-companion.mjs"); + fs.mkdirSync(pluginsDir, { recursive: true }); + fs.mkdirSync(scriptsDir, { recursive: true }); + fs.writeFileSync(companionPath, "console.log(JSON.stringify(process.argv.slice(2)));\n"); + const resumeBootstrap = firstBashBlock(read("commands/rescue.md")); + const taskBootstrap = firstBashBlock(read("skills/codex-cli-runtime/SKILL.md")) + .replace(/^' task \.\.\.$/m, "' task --resume-last \"prompt with spaces\""); + const env = { ...process.env, CLAUDE_CONFIG_DIR: configDir, CLAUDE_PLUGIN_ROOT: "" }; + + try { + fs.writeFileSync(registryPath, JSON.stringify({ + version: 2, + plugins: { "codex@openai-codex": [{ installPath: pluginRoot }] } + })); + const fallback = run("/bin/bash", ["-c", resumeBootstrap], { env }); + assert.equal(fallback.status, 0, fallback.stderr); + assert.equal(fallback.stdout, "[\"task-resume-candidate\",\"--json\"]\n"); + + const taskFallback = run("/bin/bash", ["-c", taskBootstrap], { env }); + assert.equal(taskFallback.status, 0, taskFallback.stderr); + assert.equal(taskFallback.stdout, "[\"task\",\"--resume-last\",\"prompt with spaces\"]\n"); + + fs.writeFileSync(registryPath, JSON.stringify({ + version: 2, + plugins: { "codex@openai-codex": [{ installPath: pluginRoot }, { installPath: pluginRoot }] } + })); + const ambiguous = run("/bin/bash", ["-c", resumeBootstrap], { env }); + assert.notEqual(ambiguous.status, 0); + assert.equal(ambiguous.stdout, ""); + + fs.writeFileSync(registryPath, JSON.stringify({ + version: 1, + plugins: { "codex@openai-codex": [{ installPath: pluginRoot }] } + })); + const wrongVersion = run("/bin/bash", ["-c", resumeBootstrap], { env }); + assert.notEqual(wrongVersion.status, 0); + assert.equal(wrongVersion.stdout, ""); + + fs.writeFileSync(registryPath, "not json"); + const malformed = run("/bin/bash", ["-c", resumeBootstrap], { env }); + assert.notEqual(malformed.status, 0); + assert.equal(malformed.stdout, ""); + + fs.rmSync(registryPath); + const fastPath = run("/bin/bash", ["-c", resumeBootstrap], { + env: { ...env, CLAUDE_PLUGIN_ROOT: pluginRoot } + }); + assert.equal(fastPath.status, 0, fastPath.stderr); + assert.equal(fastPath.stdout, "[\"task-resume-candidate\",\"--json\"]\n"); + } finally { + fs.rmSync(temp, { recursive: true, force: true }); + } +}); test("result and cancel commands are exposed as deterministic runtime entrypoints", () => { const result = read("commands/result.md"); @@ -186,7 +280,7 @@ test("internal docs use task terminology for rescue runs", () => { const promptingSkill = read("skills/gpt-5-4-prompting/SKILL.md"); const promptRecipes = read("skills/gpt-5-4-prompting/references/codex-prompt-recipes.md"); - assert.match(runtimeSkill, /codex-companion\.mjs" task ""/); + assert.match(runtimeSkill, /^' task \.\.\.$/m); assert.match(runtimeSkill, /Use `task` for every rescue request/i); assert.match(runtimeSkill, /task --resume-last/i); assert.match(promptingSkill, /Use `task` when the task is diagnosis/i);