From 2e469c57eb33ab03446cbd8b761c1e3836c37456 Mon Sep 17 00:00:00 2001 From: Scott Compel Date: Tue, 4 Aug 2026 17:00:01 -0600 Subject: [PATCH] feat(codex): add --json structured output to task and review subcommands task --wait --json, review --json, and adversarial-review --json now print exactly one structured JSON object ({ kind, status, cwd, model, effort, jobId, threadId, finalMessage, findings, touchedFiles, error }) derived from the same execution result as the rendered markdown view. Default (no --json) output is byte-for-byte unchanged. - buildStructuredRunResult/extractReviewFindings live in lib/render.mjs next to the markdown renderers; adversarial findings pass through the review-output schema shape untouched - errors under --json emit the same failed-shape JSON on stdout while preserving the nonzero exit and stderr message - task --background --json now reuses buildSingleJobSnapshot so job references match the status/result --json shape - task accepts --wait as the explicit foreground flag (conflicts with --background) - stop-review-gate-hook reads finalMessage (with rawOutput fallback) - runtime tests cover task/review/adversarial --json happy and failure paths; existing background --json tests updated to the snapshot shape Co-Authored-By: Claude Fable 5 --- plugins/codex/scripts/codex-companion.mjs | 229 ++++++++++++------ plugins/codex/scripts/lib/render.mjs | 35 +++ .../codex/scripts/stop-review-gate-hook.mjs | 2 +- tests/runtime.test.mjs | 159 +++++++++++- 4 files changed, 338 insertions(+), 87 deletions(-) diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 83df468ad..e9617d30d 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -54,6 +54,8 @@ import { } from "./lib/tracked-jobs.mjs"; import { resolveWorkspaceRoot } from "./lib/workspace.mjs"; import { + buildStructuredRunResult, + extractReviewFindings, renderNativeReviewResult, renderReviewResult, renderStoredJobResult, @@ -77,9 +79,9 @@ function printUsage() { [ "Usage:", " node scripts/codex-companion.mjs setup [--enable-review-gate|--disable-review-gate] [--json]", - " node scripts/codex-companion.mjs review [--wait|--background] [--base ] [--scope ]", - " node scripts/codex-companion.mjs adversarial-review [--wait|--background] [--base ] [--scope ] [focus text]", - " node scripts/codex-companion.mjs task [--background] [--write] [--resume-last|--resume|--fresh] [--model ] [--effort ] [prompt]", + " node scripts/codex-companion.mjs review [--wait|--background] [--base ] [--scope ] [--json]", + " node scripts/codex-companion.mjs adversarial-review [--wait|--background] [--base ] [--scope ] [--json] [focus text]", + " node scripts/codex-companion.mjs task [--wait|--background] [--write] [--resume-last|--resume|--fresh] [--model ] [--effort ] [--json] [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]", @@ -392,6 +394,20 @@ async function executeReviewRun(request) { }, { reviewLabel: reviewName, targetLabel: target.label, reasoningSummary: result.reasoningSummary } ); + const structured = buildStructuredRunResult( + "review", + { + exitStatus: result.status, + threadId: result.threadId, + finalMessage: result.reviewText, + failureMessage: result.error?.message ?? result.stderr + }, + { + cwd: request.cwd, + model: request.model ?? null, + jobId: request.jobId ?? null + } + ); return { exitStatus: result.status, @@ -399,6 +415,7 @@ async function executeReviewRun(request) { turnId: result.turnId, payload, rendered, + structured, summary: firstMeaningfulLine(result.reviewText, `${reviewName} completed.`), jobTitle: `Codex ${reviewName}`, jobClass: "review", @@ -450,6 +467,21 @@ async function executeReviewRun(request) { targetLabel: context.target.label, reasoningSummary: result.reasoningSummary }), + structured: buildStructuredRunResult( + "review", + { + exitStatus: result.status, + threadId: result.threadId, + finalMessage: parsed.rawOutput, + findings: extractReviewFindings(parsed.parsed), + failureMessage: result.error?.message ?? result.stderr + }, + { + cwd: request.cwd, + model: request.model ?? null, + jobId: request.jobId ?? null + } + ), summary: parsed.parsed?.summary ?? parsed.parseError ?? firstMeaningfulLine(result.finalMessage, `${reviewName} finished.`), jobTitle: `Codex ${reviewName}`, jobClass: "review", @@ -522,6 +554,22 @@ async function executeTaskRun(request) { turnId: result.turnId, payload, rendered, + structured: buildStructuredRunResult( + "task", + { + exitStatus: result.status, + threadId: result.threadId, + finalMessage: rawOutput, + touchedFiles: result.touchedFiles, + failureMessage + }, + { + cwd: request.cwd, + model: request.model ?? null, + effort: request.effort ?? null, + jobId: request.jobId ?? null + } + ), summary: firstMeaningfulLine(rawOutput, firstMeaningfulLine(failureMessage, `${taskMetadata.title} finished.`)), jobTitle: taskMetadata.title, jobClass: "task", @@ -661,7 +709,7 @@ async function runForegroundCommand(job, runner, options = {}) { stderr: !options.json }); const execution = await runTrackedJob(job, () => runner(progress), { logFile }); - outputResult(options.json ? execution.payload : execution.rendered, options.json); + outputResult(options.json ? execution.structured ?? execution.payload : execution.rendered, options.json); if (execution.exitStatus !== 0) { process.exitCode = execution.exitStatus; } @@ -709,6 +757,11 @@ function enqueueBackgroundTask(cwd, job, request) { }; } +function emitStructuredCommandFailure(kind, cwd, error) { + const message = error instanceof Error ? error.message : String(error); + outputResult(buildStructuredRunResult(kind, { exitStatus: 1, failureMessage: message }, { cwd }), true); +} + async function handleReviewCommand(argv, config) { const { options, positionals } = parseCommandInput(argv, { valueOptions: ["base", "scope", "model", "cwd"], @@ -719,37 +772,45 @@ async function handleReviewCommand(argv, config) { }); const cwd = resolveCommandCwd(options); - const workspaceRoot = resolveCommandWorkspace(options); - const focusText = positionals.join(" ").trim(); - const target = resolveReviewTarget(cwd, { - base: options.base, - scope: options.scope - }); + try { + const workspaceRoot = resolveCommandWorkspace(options); + const focusText = positionals.join(" ").trim(); + const target = resolveReviewTarget(cwd, { + base: options.base, + scope: options.scope + }); - config.validateRequest?.(target, focusText); - const metadata = buildReviewJobMetadata(config.reviewName, target); - const job = createCompanionJob({ - prefix: "review", - kind: metadata.kind, - title: metadata.title, - workspaceRoot, - jobClass: "review", - summary: metadata.summary - }); - await runForegroundCommand( - job, - (progress) => - executeReviewRun({ - cwd, - base: options.base, - scope: options.scope, - model: options.model, - focusText, - reviewName: config.reviewName, - onProgress: progress - }), - { json: options.json } - ); + config.validateRequest?.(target, focusText); + const metadata = buildReviewJobMetadata(config.reviewName, target); + const job = createCompanionJob({ + prefix: "review", + kind: metadata.kind, + title: metadata.title, + workspaceRoot, + jobClass: "review", + summary: metadata.summary + }); + await runForegroundCommand( + job, + (progress) => + executeReviewRun({ + cwd, + base: options.base, + scope: options.scope, + model: options.model, + focusText, + reviewName: config.reviewName, + jobId: job.id, + onProgress: progress + }), + { json: options.json } + ); + } catch (error) { + if (options.json) { + emitStructuredCommandFailure("review", cwd, error); + } + throw error; + } } async function handleReview(argv) { @@ -762,64 +823,78 @@ async function handleReview(argv) { async function handleTask(argv) { const { options, positionals } = parseCommandInput(argv, { valueOptions: ["model", "effort", "cwd", "prompt-file"], - booleanOptions: ["json", "write", "resume-last", "resume", "fresh", "background"], + booleanOptions: ["json", "wait", "write", "resume-last", "resume", "fresh", "background"], aliasMap: { m: "model" } }); const cwd = resolveCommandCwd(options); - const workspaceRoot = resolveCommandWorkspace(options); - const model = normalizeRequestedModel(options.model); - const effort = normalizeReasoningEffort(options.effort); - const prompt = readTaskPrompt(cwd, options, positionals); - - const resumeLast = Boolean(options["resume-last"] || options.resume); - const fresh = Boolean(options.fresh); - if (resumeLast && fresh) { - throw new Error("Choose either --resume/--resume-last or --fresh."); - } - const write = Boolean(options.write); - const taskMetadata = buildTaskRunMetadata({ - prompt, - resumeLast - }); - - if (options.background) { - ensureCodexAvailable(cwd); - requireTaskRequest(prompt, resumeLast); - - const job = buildTaskJob(workspaceRoot, taskMetadata, write); - const request = buildTaskRequest({ - cwd, - model, - effort, + try { + if (options.wait && options.background) { + throw new Error("Choose either --wait or --background."); + } + const workspaceRoot = resolveCommandWorkspace(options); + const model = normalizeRequestedModel(options.model); + const effort = normalizeReasoningEffort(options.effort); + const prompt = readTaskPrompt(cwd, options, positionals); + + const resumeLast = Boolean(options["resume-last"] || options.resume); + const fresh = Boolean(options.fresh); + if (resumeLast && fresh) { + throw new Error("Choose either --resume/--resume-last or --fresh."); + } + const write = Boolean(options.write); + const taskMetadata = buildTaskRunMetadata({ prompt, - write, - resumeLast, - jobId: job.id + resumeLast }); - const { payload } = enqueueBackgroundTask(cwd, job, request); - outputCommandResult(payload, renderQueuedTaskLaunch(payload), options.json); - return; - } - const job = buildTaskJob(workspaceRoot, taskMetadata, write); - await runForegroundCommand( - job, - (progress) => - executeTaskRun({ + if (options.background) { + ensureCodexAvailable(cwd); + requireTaskRequest(prompt, resumeLast); + + const job = buildTaskJob(workspaceRoot, taskMetadata, write); + const request = buildTaskRequest({ cwd, model, effort, prompt, write, resumeLast, - jobId: job.id, - onProgress: progress - }), - { json: options.json } - ); + jobId: job.id + }); + const { payload } = enqueueBackgroundTask(cwd, job, request); + if (options.json) { + outputResult(buildSingleJobSnapshot(cwd, job.id), true); + return; + } + outputResult(renderQueuedTaskLaunch(payload), false); + return; + } + + const job = buildTaskJob(workspaceRoot, taskMetadata, write); + await runForegroundCommand( + job, + (progress) => + executeTaskRun({ + cwd, + model, + effort, + prompt, + write, + resumeLast, + jobId: job.id, + onProgress: progress + }), + { json: options.json } + ); + } catch (error) { + if (options.json) { + emitStructuredCommandFailure("task", cwd, error); + } + throw error; + } } async function handleTransfer(argv) { diff --git a/plugins/codex/scripts/lib/render.mjs b/plugins/codex/scripts/lib/render.mjs index 2ec185236..3c1a52656 100644 --- a/plugins/codex/scripts/lib/render.mjs +++ b/plugins/codex/scripts/lib/render.mjs @@ -70,6 +70,41 @@ function normalizeReviewResultData(data) { }; } +export function extractReviewFindings(parsed) { + if (!parsed || validateReviewResultShape(parsed)) { + return null; + } + return parsed.findings; +} + +export function buildStructuredRunResult(kind, result, meta = {}) { + const failed = (result.exitStatus ?? 1) !== 0; + const structured = { + kind, + status: failed ? "failed" : "completed", + cwd: meta.cwd ?? null, + model: meta.model ?? null, + effort: meta.effort ?? null, + jobId: meta.jobId ?? null, + threadId: result.threadId ?? null, + finalMessage: typeof result.finalMessage === "string" ? result.finalMessage : "" + }; + + if (Array.isArray(result.findings)) { + structured.findings = result.findings; + } + if (Array.isArray(result.touchedFiles)) { + structured.touchedFiles = result.touchedFiles; + } + if (failed) { + structured.error = { + message: String(result.failureMessage ?? "").trim() || `Codex ${kind} failed.` + }; + } + + return structured; +} + function isStructuredReviewStoredResult(storedJob) { const result = storedJob?.result; if (!result || typeof result !== "object" || Array.isArray(result)) { diff --git a/plugins/codex/scripts/stop-review-gate-hook.mjs b/plugins/codex/scripts/stop-review-gate-hook.mjs index 2346bdcf4..32fdb467e 100644 --- a/plugins/codex/scripts/stop-review-gate-hook.mjs +++ b/plugins/codex/scripts/stop-review-gate-hook.mjs @@ -129,7 +129,7 @@ function runStopReview(cwd, input = {}) { try { const payload = JSON.parse(result.stdout); - return parseStopReviewOutput(payload?.rawOutput); + return parseStopReviewOutput(payload?.finalMessage ?? payload?.rawOutput); } catch { return { ok: false, diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 8f276835b..e43ca0138 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -345,6 +345,145 @@ test("task reports the actual Codex auth error when the run is rejected", () => assert.match(result.stderr, /authentication expired; run codex login/); }); +test("task --wait --json emits a single structured task result", () => { + 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", "--wait", "--json", "--model", "spark", "--effort", "low", "summarize the repo"], + { + cwd: repo, + env: buildEnv(binDir) + } + ); + + assert.equal(result.status, 0, result.stderr); + const payload = JSON.parse(result.stdout); + assert.equal(payload.kind, "task"); + assert.equal(payload.status, "completed"); + assert.equal(fs.realpathSync(payload.cwd), fs.realpathSync(repo)); + assert.equal(payload.model, "gpt-5.3-codex-spark"); + assert.equal(payload.effort, "low"); + assert.match(payload.jobId, /^task-/); + assert.match(payload.threadId, /^thr_/); + assert.equal(payload.finalMessage, "Handled the requested task.\nTask prompt accepted."); + assert.deepEqual(payload.touchedFiles, []); + assert.equal("error" in payload, false); +}); + +test("task --json emits a structured error when the Codex run fails", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir, "auth-run-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", "--json", "check failed auth"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.notEqual(result.status, 0); + const payload = JSON.parse(result.stdout); + assert.equal(payload.kind, "task"); + assert.equal(payload.status, "failed"); + assert.match(payload.error.message, /authentication expired; run codex login/); + assert.match(result.stderr, /authentication expired; run codex login/); +}); + +test("review --json emits a single structured review result", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + 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", "--json"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + const payload = JSON.parse(result.stdout); + assert.equal(payload.kind, "review"); + assert.equal(payload.status, "completed"); + assert.equal(fs.realpathSync(payload.cwd), fs.realpathSync(repo)); + assert.match(payload.jobId, /^review-/); + assert.match(payload.threadId, /^thr_/); + assert.match(payload.finalMessage, /Reviewed uncommitted changes/); + assert.match(payload.finalMessage, /No material issues found/); + assert.equal("findings" in payload, false); + assert.equal("error" in payload, false); +}); + +test("review --json emits a structured error when the Codex run fails", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir, "auth-run-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 }); + fs.writeFileSync(path.join(repo, "README.md"), "hello again\n"); + + const result = run("node", [SCRIPT, "review", "--json"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.notEqual(result.status, 0); + const payload = JSON.parse(result.stdout); + assert.equal(payload.kind, "review"); + assert.equal(payload.status, "failed"); + assert.match(payload.error.message, /authentication expired; run codex login/); +}); + +test("adversarial-review --json emits schema-conforming findings", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir); + initGitRepo(repo); + fs.mkdirSync(path.join(repo, "src")); + fs.writeFileSync(path.join(repo, "src", "app.js"), "export const value = items[0];\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 = items[0].id;\n"); + + const result = run("node", [SCRIPT, "adversarial-review", "--json"], { + cwd: repo, + env: buildEnv(binDir) + }); + + assert.equal(result.status, 0, result.stderr); + const payload = JSON.parse(result.stdout); + assert.equal(payload.kind, "review"); + assert.equal(payload.status, "completed"); + assert.match(payload.jobId, /^review-/); + assert.match(payload.threadId, /^thr_/); + assert.match(payload.finalMessage, /needs-attention/); + assert.equal(payload.findings.length, 1); + const finding = payload.findings[0]; + assert.equal(finding.severity, "high"); + assert.equal(finding.title, "Missing empty-state guard"); + assert.equal(finding.file, "src/app.js"); + assert.equal(finding.line_start, 4); + assert.equal(finding.line_end, 6); + assert.equal(finding.confidence, 0.87); + assert.equal(finding.recommendation, "Handle empty collections before indexing."); +}); + test("review accepts the quoted raw argument style for built-in base-branch review", () => { const repo = makeTempDir(); const binDir = makeTempDir(); @@ -936,12 +1075,13 @@ test("task --background enqueues a detached worker and exposes per-job status", assert.equal(launched.status, 0, launched.stderr); const launchPayload = JSON.parse(launched.stdout); - assert.equal(launchPayload.status, "queued"); - assert.match(launchPayload.jobId, /^task-/); + assert.equal(launchPayload.job.status, "queued"); + assert.match(launchPayload.job.id, /^task-/); + const launchedJobId = launchPayload.job.id; const waitedStatus = run( "node", - [SCRIPT, "status", launchPayload.jobId, "--wait", "--timeout-ms", "15000", "--json"], + [SCRIPT, "status", launchedJobId, "--wait", "--timeout-ms", "15000", "--json"], { cwd: repo, env: buildEnv(binDir) @@ -950,11 +1090,11 @@ test("task --background enqueues a detached worker and exposes per-job status", assert.equal(waitedStatus.status, 0, waitedStatus.stderr); const waitedPayload = JSON.parse(waitedStatus.stdout); - assert.equal(waitedPayload.job.id, launchPayload.jobId); + assert.equal(waitedPayload.job.id, launchedJobId); assert.equal(waitedPayload.job.status, "completed"); const resultPayload = await waitFor(() => { - const result = run("node", [SCRIPT, "result", launchPayload.jobId, "--json"], { + const result = run("node", [SCRIPT, "result", launchedJobId, "--json"], { cwd: repo, env: buildEnv(binDir) }); @@ -964,7 +1104,7 @@ test("task --background enqueues a detached worker and exposes per-job status", return JSON.parse(result.stdout); }); - assert.equal(resultPayload.job.id, launchPayload.jobId); + assert.equal(resultPayload.job.id, launchedJobId); assert.equal(resultPayload.job.status, "completed"); assert.match(resultPayload.storedJob.rendered, /Handled the requested task/); }); @@ -1048,8 +1188,9 @@ test("review accepts --background while still running as a tracked review job", assert.equal(launched.status, 0, launched.stderr); const launchPayload = JSON.parse(launched.stdout); - assert.equal(launchPayload.review, "Review"); - assert.match(launchPayload.codex.stdout, /No material issues found/); + assert.equal(launchPayload.kind, "review"); + assert.equal(launchPayload.status, "completed"); + assert.match(launchPayload.finalMessage, /No material issues found/); const status = run("node", [SCRIPT, "status"], { cwd: repo, @@ -1755,7 +1896,7 @@ test("cancel sends turn interrupt to the shared app-server before killing a brok assert.equal(launched.status, 0, launched.stderr); const launchPayload = JSON.parse(launched.stdout); - const jobId = launchPayload.jobId; + const jobId = launchPayload.job.id; assert.ok(jobId); const stateDir = resolveStateDir(repo);