From 362975882fbc0e593afb21a4ace228241dbbafe2 Mon Sep 17 00:00:00 2001 From: Soumya95 Date: Mon, 14 Sep 2026 13:01:24 +0530 Subject: [PATCH] fix: persist errorMessage on non-throwing turn failure and shorten summary (#757) Two gaps combined to make server-side turn failures illegible in status: 1. errorMessage was only written in the catch path, so a turn that failed with exitStatus != 0 but completed normally stored no reason anywhere reachable from 'status'. 2. The summary took the first line of the pretty-printed error body, which is a bare opening brace. Now: - tracked-jobs.mjs reads execution.errorMessage on the success branch and writes it to both the job file and the index when failed. - codex-companion.mjs task/review paths surface result.error.message as errorMessage and prefer it over rawOutput for the summary. - Summary and Codex error: progress line are passed through shorten() so multi-line JSON bodies cannot reduce to punctuation. - Regressions covered by two new state.test.mjs tests. --- plugins/codex/scripts/codex-companion.mjs | 14 +++- plugins/codex/scripts/lib/codex.mjs | 2 +- plugins/codex/scripts/lib/tracked-jobs.mjs | 4 + tests/state.test.mjs | 89 ++++++++++++++++++++++ 4 files changed, 105 insertions(+), 4 deletions(-) diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 83df468ad..408263dfb 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -399,7 +399,8 @@ async function executeReviewRun(request) { turnId: result.turnId, payload, rendered, - summary: firstMeaningfulLine(result.reviewText, `${reviewName} completed.`), + summary: shorten(firstMeaningfulLine(result.reviewText, `${reviewName} completed.`), 96), + errorMessage: result.error?.message ?? result.stderr ?? null, jobTitle: `Codex ${reviewName}`, jobClass: "review", targetLabel: target.label @@ -419,6 +420,7 @@ async function executeReviewRun(request) { status: result.status, failureMessage: result.error?.message ?? result.stderr }); + const failureMessage = result.error?.message ?? result.stderr ?? parsed.parseError ?? ""; const payload = { review: reviewName, target, @@ -450,7 +452,8 @@ async function executeReviewRun(request) { targetLabel: context.target.label, reasoningSummary: result.reasoningSummary }), - summary: parsed.parsed?.summary ?? parsed.parseError ?? firstMeaningfulLine(result.finalMessage, `${reviewName} finished.`), + summary: parsed.parsed?.summary ?? shorten(firstMeaningfulLine(failureMessage || result.finalMessage, `${reviewName} finished.`), 96), + errorMessage: failureMessage || null, jobTitle: `Codex ${reviewName}`, jobClass: "review", targetLabel: context.target.label @@ -515,6 +518,10 @@ async function executeTaskRun(request) { touchedFiles: result.touchedFiles, reasoningSummary: result.reasoningSummary }; + const summary = shorten( + firstMeaningfulLine(failureMessage || rawOutput, `${taskMetadata.title} finished.`), + 96 + ); return { exitStatus: result.status, @@ -522,7 +529,8 @@ async function executeTaskRun(request) { turnId: result.turnId, payload, rendered, - summary: firstMeaningfulLine(rawOutput, firstMeaningfulLine(failureMessage, `${taskMetadata.title} finished.`)), + summary, + errorMessage: failureMessage || null, jobTitle: taskMetadata.title, jobClass: "task", write: Boolean(request.write) diff --git a/plugins/codex/scripts/lib/codex.mjs b/plugins/codex/scripts/lib/codex.mjs index fead00cc4..d1c9e0b38 100644 --- a/plugins/codex/scripts/lib/codex.mjs +++ b/plugins/codex/scripts/lib/codex.mjs @@ -536,7 +536,7 @@ function applyTurnNotification(state, message) { break; case "error": state.error = message.params.error; - emitProgress(state.onProgress, `Codex error: ${message.params.error.message}`, "failed"); + emitProgress(state.onProgress, `Codex error: ${shorten(message.params.error.message, 96)}`, "failed"); break; case "turn/completed": if ((message.params.threadId ?? null) !== state.threadId) { diff --git a/plugins/codex/scripts/lib/tracked-jobs.mjs b/plugins/codex/scripts/lib/tracked-jobs.mjs index 902869012..48efb6770 100644 --- a/plugins/codex/scripts/lib/tracked-jobs.mjs +++ b/plugins/codex/scripts/lib/tracked-jobs.mjs @@ -154,6 +154,8 @@ export async function runTrackedJob(job, runner, options = {}) { try { const execution = await runner(); const completionStatus = execution.exitStatus === 0 ? "completed" : "failed"; + const errorMessage = + completionStatus === "failed" ? (execution.errorMessage ?? null) : null; const completedAt = nowIso(); writeJobFile(job.workspaceRoot, job.id, { ...runningRecord, @@ -163,6 +165,7 @@ export async function runTrackedJob(job, runner, options = {}) { pid: null, phase: completionStatus === "completed" ? "done" : "failed", completedAt, + errorMessage, result: execution.payload, rendered: execution.rendered }); @@ -174,6 +177,7 @@ export async function runTrackedJob(job, runner, options = {}) { summary: execution.summary, phase: completionStatus === "completed" ? "done" : "failed", pid: null, + errorMessage, completedAt }); appendLogBlock(options.logFile ?? job.logFile ?? null, "Final output", execution.rendered); diff --git a/tests/state.test.mjs b/tests/state.test.mjs index 0f8f57cea..567d6c1b1 100644 --- a/tests/state.test.mjs +++ b/tests/state.test.mjs @@ -6,6 +6,7 @@ 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 { runTrackedJob } from "../plugins/codex/scripts/lib/tracked-jobs.mjs"; test("resolveStateDir uses a temp-backed per-workspace directory", () => { const workspace = makeTempDir(); @@ -103,3 +104,91 @@ test("saveState prunes dropped job artifacts when indexed jobs exceed the cap", .sort() ); }); + +test("runTrackedJob persists errorMessage on a non-throwing failed execution", async () => { + const workspace = makeTempDir(); + const job = { + id: "task-fail-nonthrowing", + kind: "task", + kindLabel: "Task", + title: "Codex Task", + workspaceRoot: workspace, + jobClass: "task", + summary: "", + write: false, + createdAt: new Date().toISOString() + }; + + await runTrackedJob( + job, + async () => ({ + exitStatus: 400, + threadId: "thread-1", + turnId: "turn-1", + payload: { status: 400, rawOutput: "{\n \"type\": \"error\"\n}", touchedFiles: [] }, + rendered: "rendered output\n", + summary: "Unsupported value: 'x' is not supported.", + errorMessage: "Unsupported value: 'x' is not supported with the 'gpt-5.6-terra' model." + }), + {} + ); + + const storedJob = JSON.parse( + fs.readFileSync(resolveJobFile(workspace, job.id), "utf8") + ); + const state = JSON.parse(fs.readFileSync(resolveStateFile(workspace), "utf8")); + const indexed = state.jobs.find((entry) => entry.id === job.id); + + assert.equal(storedJob.status, "failed"); + assert.equal(storedJob.phase, "failed"); + assert.equal( + storedJob.errorMessage, + "Unsupported value: 'x' is not supported with the 'gpt-5.6-terra' model." + ); + assert.equal(indexed.status, "failed"); + assert.equal( + indexed.errorMessage, + "Unsupported value: 'x' is not supported with the 'gpt-5.6-terra' model." + ); + assert.equal(indexed.summary, "Unsupported value: 'x' is not supported."); +}); + +test("runTrackedJob stores no errorMessage for a completed execution", async () => { + const workspace = makeTempDir(); + const job = { + id: "task-ok", + kind: "task", + kindLabel: "Task", + title: "Codex Task", + workspaceRoot: workspace, + jobClass: "task", + summary: "", + write: false, + createdAt: new Date().toISOString() + }; + + await runTrackedJob( + job, + async () => ({ + exitStatus: 0, + threadId: "thread-2", + turnId: "turn-2", + payload: { status: 0, rawOutput: "OK", touchedFiles: [] }, + rendered: "OK\n", + summary: "OK", + errorMessage: null + }), + {} + ); + + const storedJob = JSON.parse( + fs.readFileSync(resolveJobFile(workspace, job.id), "utf8") + ); + const state = JSON.parse(fs.readFileSync(resolveStateFile(workspace), "utf8")); + const indexed = state.jobs.find((entry) => entry.id === job.id); + + assert.equal(storedJob.status, "completed"); + assert.equal(storedJob.errorMessage, null); + assert.equal(indexed.status, "completed"); + assert.equal(indexed.errorMessage, null); +});