diff --git a/README.md b/README.md index ddeeff43..76ecd3e4 100644 --- a/README.md +++ b/README.md @@ -63,6 +63,7 @@ After install, you should see: - the slash commands listed below - the `codex:codex-rescue` subagent in `/agents` +- three internal skills used by the rescue subagent: `codex-cli-runtime`, `codex-result-handling`, and `gpt-5-4-prompting` (these are not user-invocable) One simple first run is: diff --git a/plugins/codex/commands/rescue.md b/plugins/codex/commands/rescue.md index c92a2896..6b23384e 100644 --- a/plugins/codex/commands/rescue.md +++ b/plugins/codex/commands/rescue.md @@ -1,6 +1,6 @@ --- description: Delegate investigation, an explicit fix request, or follow-up rescue work to the Codex rescue subagent -argument-hint: "[--background|--wait] [--resume|--fresh] [--model ] [--effort ] [what Codex should investigate, solve, or continue]" +argument-hint: "[--background|--wait] [--resume|--fresh] [--model ] [--effort ] [--context ] [what Codex should investigate, solve, or continue]" context: fork allowed-tools: Bash(node:*), AskUserQuestion --- @@ -17,7 +17,7 @@ Execution mode: - 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. +- `--model`, `--effort`, and `--context` 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: diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 201d1c7a..844a0c26 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -77,7 +77,7 @@ function printUsage() { " 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 task [--background] [--write] [--resume-last|--resume|--fresh] [--model ] [--effort ] [--context ] [prompt]", " 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]" @@ -451,9 +451,12 @@ async function executeTaskRun(request) { throw new Error("Provide a prompt, a prompt file, piped stdin, or use --resume-last."); } + const contextSuffix = request.context ? `\n\n---\n\nAdditional context:\n${request.context}` : ""; + const fullPrompt = request.prompt ? `${request.prompt}${contextSuffix}` : ""; + const result = await runAppServerTurn(workspaceRoot, { resumeThreadId, - prompt: request.prompt, + prompt: fullPrompt, defaultPrompt: resumeThreadId ? DEFAULT_CONTINUE_PROMPT : "", model: request.model, effort: request.effort, @@ -570,7 +573,7 @@ function buildTaskJob(workspaceRoot, taskMetadata, write) { }); } -function buildTaskRequest({ cwd, model, effort, prompt, write, resumeLast, jobId }) { +function buildTaskRequest({ cwd, model, effort, prompt, write, resumeLast, jobId, context }) { return { cwd, model, @@ -578,7 +581,8 @@ function buildTaskRequest({ cwd, model, effort, prompt, write, resumeLast, jobId prompt, write, resumeLast, - jobId + jobId, + context }; } @@ -703,10 +707,11 @@ async function handleReview(argv) { async function handleTask(argv) { const { options, positionals } = parseCommandInput(argv, { - valueOptions: ["model", "effort", "cwd", "prompt-file"], + valueOptions: ["model", "effort", "cwd", "prompt-file", "context"], booleanOptions: ["json", "write", "resume-last", "resume", "fresh", "background"], aliasMap: { - m: "model" + m: "model", + c: "context" } }); @@ -727,6 +732,8 @@ async function handleTask(argv) { resumeLast }); + const context = options.context ?? null; + if (options.background) { ensureCodexReady(cwd); requireTaskRequest(prompt, resumeLast); @@ -739,7 +746,8 @@ async function handleTask(argv) { prompt, write, resumeLast, - jobId: job.id + jobId: job.id, + context }); const { payload } = enqueueBackgroundTask(cwd, job, request); outputCommandResult(payload, renderQueuedTaskLaunch(payload), options.json); @@ -758,6 +766,7 @@ async function handleTask(argv) { write, resumeLast, jobId: job.id, + context, onProgress: progress }), { json: options.json } diff --git a/plugins/codex/scripts/lib/broker-lifecycle.mjs b/plugins/codex/scripts/lib/broker-lifecycle.mjs index ef763819..b42d113b 100644 --- a/plugins/codex/scripts/lib/broker-lifecycle.mjs +++ b/plugins/codex/scripts/lib/broker-lifecycle.mjs @@ -40,19 +40,31 @@ export async function waitForBrokerEndpoint(endpoint, timeoutMs = 2000) { return false; } -export async function sendBrokerShutdown(endpoint) { +export async function sendBrokerShutdown(endpoint, timeoutMs = 5000) { await new Promise((resolve) => { const socket = connectToEndpoint(endpoint); socket.setEncoding("utf8"); + + const timer = setTimeout(() => { + socket.destroy(); + resolve(); + }, timeoutMs); + timer.unref?.(); + + const cleanup = () => { + clearTimeout(timer); + resolve(); + }; + socket.on("connect", () => { socket.write(`${JSON.stringify({ id: 1, method: "broker/shutdown", params: {} })}\n`); }); socket.on("data", () => { socket.end(); - resolve(); + cleanup(); }); - socket.on("error", resolve); - socket.on("close", resolve); + socket.on("error", cleanup); + socket.on("close", cleanup); }); } diff --git a/plugins/codex/skills/codex-cli-runtime/SKILL.md b/plugins/codex/skills/codex-cli-runtime/SKILL.md index 0e91bfb5..f3c4bf86 100644 --- a/plugins/codex/skills/codex-cli-runtime/SKILL.md +++ b/plugins/codex/skills/codex-cli-runtime/SKILL.md @@ -33,6 +33,7 @@ Command selection: - `--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`. +- `--context ""`: pass additional context to Codex that will be appended to the prompt. Use this to provide extra background information, constraints, or specifications. - `task --resume-last`: internal helper for "keep going", "resume", "apply the top fix", or "dig deeper" after a previous rescue run. Safety rules: diff --git a/tests/args.test.mjs b/tests/args.test.mjs new file mode 100644 index 00000000..68c67ef6 --- /dev/null +++ b/tests/args.test.mjs @@ -0,0 +1,70 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { parseArgs, splitRawArgumentString } from "../plugins/codex/scripts/lib/args.mjs"; + +// --- parseArgs --- + +test("parseArgs: boolean flag --flag=true sets true, --flag=false sets false", () => { + const configTrue = parseArgs(["--verbose=true"], { booleanOptions: ["verbose"] }); + assert.equal(configTrue.options.verbose, true); + + const configFalse = parseArgs(["--verbose=false"], { booleanOptions: ["verbose"] }); + assert.equal(configFalse.options.verbose, false); +}); + +test("parseArgs: value option --output consumes next token", () => { + const { options } = parseArgs(["--output", "/tmp/out.txt"], { valueOptions: ["output"] }); + assert.equal(options.output, "/tmp/out.txt"); +}); + +test("parseArgs: inline value --output=path uses inline value", () => { + const { options } = parseArgs(["--output=/tmp/out.txt"], { valueOptions: ["output"] }); + assert.equal(options.output, "/tmp/out.txt"); +}); + +test("parseArgs: short alias -o resolved via aliasMap", () => { + const { options } = parseArgs(["-o", "/tmp/out.txt"], { + valueOptions: ["output"], + aliasMap: { o: "output" }, + }); + assert.equal(options.output, "/tmp/out.txt"); +}); + +test("parseArgs: positionals after -- land in positionals array", () => { + const { options, positionals } = parseArgs( + ["--verbose", "--", "--not-a-flag", "file.txt"], + { booleanOptions: ["verbose"] } + ); + assert.equal(options.verbose, true); + assert.deepEqual(positionals, ["--not-a-flag", "file.txt"]); +}); + +test("parseArgs: missing value for value option throws Error", () => { + assert.throws( + () => parseArgs(["--output"], { valueOptions: ["output"] }), + { message: "Missing value for --output" } + ); +}); + +// --- splitRawArgumentString --- + +test("splitRawArgumentString: space-separated tokens", () => { + assert.deepEqual(splitRawArgumentString("foo bar baz"), ["foo", "bar", "baz"]); +}); + +test("splitRawArgumentString: single-quoted string with spaces becomes one token", () => { + assert.deepEqual(splitRawArgumentString("hello 'foo bar' world"), ["hello", "foo bar", "world"]); +}); + +test("splitRawArgumentString: double-quoted string with spaces becomes one token", () => { + assert.deepEqual(splitRawArgumentString('hello "foo bar" world'), ["hello", "foo bar", "world"]); +}); + +test("splitRawArgumentString: backslash escape preserves next char", () => { + assert.deepEqual(splitRawArgumentString("foo\\ bar baz"), ["foo bar", "baz"]); +}); + +test("splitRawArgumentString: trailing backslash appended literally", () => { + assert.deepEqual(splitRawArgumentString("foo\\"), ["foo\\"]); +}); diff --git a/tests/commands.test.mjs b/tests/commands.test.mjs index ef5adb09..ee95df61 100644 --- a/tests/commands.test.mjs +++ b/tests/commands.test.mjs @@ -102,7 +102,7 @@ test("rescue command absorbs continue semantics", () => { 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, /`--model`, `--effort`, and `--context` 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); diff --git a/tests/fake-codex-fixture.mjs b/tests/fake-codex-fixture.mjs index 1e6f13dd..f8c9ea57 100644 --- a/tests/fake-codex-fixture.mjs +++ b/tests/fake-codex-fixture.mjs @@ -169,6 +169,10 @@ function structuredReviewPayload(prompt) { } function taskPayload(prompt, resume) { + if (BEHAVIOR === "empty-stdout") { + return ""; + } + if (prompt.includes("") && prompt.includes("Only review the work from the previous Claude turn.")) { if (BEHAVIOR === "adversarial-clean") { return "ALLOW: No blocking issues found in the previous turn."; diff --git a/tests/prompts.test.mjs b/tests/prompts.test.mjs new file mode 100644 index 00000000..3fb7fba6 --- /dev/null +++ b/tests/prompts.test.mjs @@ -0,0 +1,25 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { interpolateTemplate } from "../plugins/codex/scripts/lib/prompts.mjs"; + +test("interpolateTemplate: replaces {{KEY}} with provided variable", () => { + assert.equal(interpolateTemplate("Hello {{NAME}}", { NAME: "World" }), "Hello World"); +}); + +test("interpolateTemplate: replaces multiple different keys in one pass", () => { + const result = interpolateTemplate("{{GREETING}}, {{NAME}}!", { GREETING: "Hi", NAME: "Alice" }); + assert.equal(result, "Hi, Alice!"); +}); + +test("interpolateTemplate: unknown key is replaced with empty string", () => { + assert.equal(interpolateTemplate("Hello {{MISSING}}", {}), "Hello "); +}); + +test("interpolateTemplate: template with no placeholders is returned unchanged", () => { + assert.equal(interpolateTemplate("no placeholders here", { KEY: "val" }), "no placeholders here"); +}); + +test("interpolateTemplate: key appearing twice is replaced both times", () => { + assert.equal(interpolateTemplate("{{X}} and {{X}}", { X: "ok" }), "ok and ok"); +}); diff --git a/tests/rescue-edge-cases.test.mjs b/tests/rescue-edge-cases.test.mjs new file mode 100644 index 00000000..ede0a6d5 --- /dev/null +++ b/tests/rescue-edge-cases.test.mjs @@ -0,0 +1,246 @@ +import fs from "node:fs"; +import path from "node:path"; +import test from "node:test"; +import assert from "node:assert/strict"; +import { fileURLToPath } from "node:url"; + +import { buildEnv, installFakeCodex } from "./fake-codex-fixture.mjs"; +import { initGitRepo, makeTempDir, run } from "./helpers.mjs"; + +const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); +const PLUGIN_ROOT = path.join(ROOT, "plugins", "codex"); +const SCRIPT = path.join(PLUGIN_ROOT, "scripts", "codex-companion.mjs"); + +// --------------------------------------------------------------------------- +// 1. Graceful output when codex returns empty stdout +// --------------------------------------------------------------------------- + +test("task renders a fallback message when codex returns empty stdout", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir, "empty-stdout"); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "hello.txt"), "hello\n"); + run("git", ["add", "hello.txt"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + + const result = run("node", [SCRIPT, "task", "--write", "do something"], { + cwd: repo, + env: buildEnv(binDir) + }); + + // The script should still exit successfully (turn completed) but show a + // meaningful fallback instead of blank output. + const output = result.stdout.trim(); + assert.ok(output.length > 0, "stdout must not be empty when codex returns empty output"); + assert.ok( + output.includes("did not return") || output.includes("Codex"), + `Expected a fallback message, got: ${output}` + ); +}); + +// --------------------------------------------------------------------------- +// 2. Correct error message surfaced when exit code is non-zero +// --------------------------------------------------------------------------- + +test("formatCommandFailure includes exit code and stderr", async () => { + const { formatCommandFailure } = await import( + "../plugins/codex/scripts/lib/process.mjs" + ); + + const result = { + command: "codex", + args: ["app-server"], + status: 1, + signal: null, + stdout: "", + stderr: "authentication failed", + error: null + }; + + const message = formatCommandFailure(result); + assert.match(message, /exit=1/, "should include exit code"); + assert.match(message, /authentication failed/, "should include stderr text"); +}); + +test("formatCommandFailure prefers stderr over stdout", async () => { + const { formatCommandFailure } = await import( + "../plugins/codex/scripts/lib/process.mjs" + ); + + const result = { + command: "codex", + args: ["task"], + status: 2, + signal: null, + stdout: "some output", + stderr: "real error here", + error: null + }; + + const message = formatCommandFailure(result); + assert.match(message, /real error here/, "should prefer stderr"); + assert.ok(!message.includes("some output"), "should not include stdout when stderr is present"); +}); + +test("formatCommandFailure includes signal when process is killed", async () => { + const { formatCommandFailure } = await import( + "../plugins/codex/scripts/lib/process.mjs" + ); + + const result = { + command: "codex", + args: ["app-server"], + status: null, + signal: "SIGKILL", + stdout: "", + stderr: "", + error: null + }; + + const message = formatCommandFailure(result); + assert.match(message, /signal=SIGKILL/, "should include signal name"); +}); + +test("runCommandChecked throws on non-zero exit code with formatted message", async () => { + const { runCommandChecked } = await import( + "../plugins/codex/scripts/lib/process.mjs" + ); + + assert.throws( + () => runCommandChecked("node", ["-e", "process.exit(42)"]), + (err) => { + assert.ok(err instanceof Error); + assert.match(err.message, /exit=42/, "error message should include exit code"); + return true; + } + ); +}); + +// --------------------------------------------------------------------------- +// 3. Timeout / stall handling — codex stalls and the companion surfaces the +// partial state gracefully +// --------------------------------------------------------------------------- + +test("task with slow-task behavior still completes when codex eventually responds", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir, "slow-task"); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "hello.txt"), "hello\n"); + run("git", ["add", "hello.txt"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + + const result = run("node", [SCRIPT, "task", "--write", "do something slow"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, `Expected exit 0, stderr: ${result.stderr}`); + assert.ok(result.stdout.includes("Task prompt accepted"), "should contain task output despite delay"); +}); + +test("task cancel interrupts a running codex turn", () => { + // Use the interruptible-slow-task behavior which sets up a 5s delay but + // responds to turn/interrupt. + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir, "interruptible-slow-task"); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "hello.txt"), "hello\n"); + run("git", ["add", "hello.txt"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + + // Start a task in the background, then cancel it immediately. + const startResult = run( + "node", + [SCRIPT, "task", "--background", "--write", "slow task"], + { + cwd: repo, + env: buildEnv(binDir) + } + ); + + assert.equal(startResult.status, 0, `background start failed: ${startResult.stderr}`); + + // Extract job id from the output. + const jobIdMatch = startResult.stdout.match(/task-\w+/); + assert.ok(jobIdMatch, `Could not find job id in output: ${startResult.stdout}`); + const jobId = jobIdMatch[0]; + + // Give the background worker a moment to start, then cancel. + const cancelResult = run("node", [SCRIPT, "cancel", jobId], { + cwd: repo, + env: buildEnv(binDir) + }); + + // Cancel should succeed — it terminates the process tree and marks the job. + assert.equal(cancelResult.status, 0, `cancel failed: ${cancelResult.stderr}`); + assert.ok( + cancelResult.stdout.includes("cancelled") || cancelResult.stdout.includes("Cancelled"), + `cancel output should confirm cancellation: ${cancelResult.stdout}` + ); +}); + +// --------------------------------------------------------------------------- +// 4. Edge case: parseStructuredOutput handles empty / invalid input +// --------------------------------------------------------------------------- + +test("parseStructuredOutput returns fallback for empty string", async () => { + const { parseStructuredOutput } = await import( + "../plugins/codex/scripts/lib/codex.mjs" + ); + + const result = parseStructuredOutput("", { failureMessage: "custom fallback" }); + assert.equal(result.parsed, null); + assert.equal(result.parseError, "custom fallback"); + assert.equal(result.rawOutput, ""); +}); + +test("parseStructuredOutput returns fallback for null input", async () => { + const { parseStructuredOutput } = await import( + "../plugins/codex/scripts/lib/codex.mjs" + ); + + const result = parseStructuredOutput(null); + assert.equal(result.parsed, null); + assert.ok(result.parseError.length > 0, "should have a non-empty error message"); + assert.equal(result.rawOutput, ""); +}); + +test("parseStructuredOutput returns parse error for invalid JSON", async () => { + const { parseStructuredOutput } = await import( + "../plugins/codex/scripts/lib/codex.mjs" + ); + + const result = parseStructuredOutput("not valid json {{{"); + assert.equal(result.parsed, null); + assert.ok(result.parseError.length > 0, "should report the JSON parse error"); + assert.equal(result.rawOutput, "not valid json {{{"); +}); + +test("parseStructuredOutput parses valid JSON", async () => { + const { parseStructuredOutput } = await import( + "../plugins/codex/scripts/lib/codex.mjs" + ); + + const input = JSON.stringify({ verdict: "approve", summary: "ok" }); + const result = parseStructuredOutput(input); + assert.deepEqual(result.parsed, { verdict: "approve", summary: "ok" }); + assert.equal(result.parseError, null); + assert.equal(result.rawOutput, input); +}); + +// --------------------------------------------------------------------------- +// 5. Edge case: binaryAvailable reports correct status for missing binaries +// --------------------------------------------------------------------------- + +test("binaryAvailable returns not-found for missing binary", async () => { + const { binaryAvailable } = await import( + "../plugins/codex/scripts/lib/process.mjs" + ); + + const result = binaryAvailable("__nonexistent_binary_12345__"); + assert.equal(result.available, false); + assert.match(result.detail, /not found/); +});