From ae68189eec893c4e92facc6d96eda4a07e907222 Mon Sep 17 00:00:00 2001 From: Pablo Garcia Pizano Date: Fri, 3 Apr 2026 00:06:09 -0500 Subject: [PATCH 1/3] [night-shift] test: add unit tests for args parsing and prompt interpolation Add tests/args.test.mjs covering parseArgs (boolean flags, value options, inline values, short aliases, passthrough positionals, missing value error) and splitRawArgumentString (space splitting, single/double quotes, backslash escaping, trailing backslash). Add tests/prompts.test.mjs covering interpolateTemplate (single key, multiple keys, unknown key, no placeholders, duplicate key). Co-Authored-By: Claude Opus 4.6 --- tests/args.test.mjs | 70 ++++++++++++++++++++++++++++++++++++++++++ tests/prompts.test.mjs | 25 +++++++++++++++ 2 files changed, 95 insertions(+) create mode 100644 tests/args.test.mjs create mode 100644 tests/prompts.test.mjs 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/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"); +}); From 83e81389e1a1d7001688a88847d24aec5259bacf Mon Sep 17 00:00:00 2001 From: Pablo Garcia Pizano Date: Sat, 11 Apr 2026 00:07:40 -0500 Subject: [PATCH 2/3] [night-shift] test: add edge case tests for codex rescue/delegation flow Cover graceful output when codex returns empty stdout, correct error message formatting on non-zero exit codes and signals, timeout/stall handling with slow-task and interruptible-slow-task behaviors, cancel interruption of running turns, parseStructuredOutput edge cases, and binaryAvailable for missing binaries. Add "empty-stdout" behavior to the fake codex fixture. Co-Authored-By: Claude Opus 4.6 --- tests/fake-codex-fixture.mjs | 4 + tests/rescue-edge-cases.test.mjs | 246 +++++++++++++++++++++++++++++++ 2 files changed, 250 insertions(+) create mode 100644 tests/rescue-edge-cases.test.mjs 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/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/); +}); From b1913aef7d896f0ce61146a07a2b17212c383cad Mon Sep 17 00:00:00 2001 From: Pablo Garcia Pizano Date: Sat, 11 Apr 2026 00:08:07 -0500 Subject: [PATCH 3/3] [night-shift] docs: list internal skills shipped with the plugin in README The README documented all 7 commands and the rescue subagent but omitted the three internal skills (codex-cli-runtime, codex-result-handling, gpt-5-4-prompting) that ship with the plugin. Add them to the "After install" section with a note that they are not user-invocable. Co-Authored-By: Claude Opus 4.6 --- README.md | 1 + 1 file changed, 1 insertion(+) 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: