Conversation
…mmary (openai#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.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 362975882f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| status: result.status, | ||
| failureMessage: result.error?.message ?? result.stderr | ||
| }); | ||
| const failureMessage = result.error?.message ?? result.stderr ?? parsed.parseError ?? ""; |
There was a problem hiding this comment.
When an adversarial review returns malformed structured output without an app-server error, result.stderr is normally the empty string, which is non-nullish and therefore prevents parsed.parseError from being selected here. The subsequent summary falls back to the malformed response's first line (often just {), regressing the previous behavior that surfaced the JSON parse error and recreating the unhelpful status summary this change is intended to fix. Treat an empty stderr value as absent before falling back to parsed.parseError.
Useful? React with 👍 / 👎.
| payload, | ||
| rendered, | ||
| summary: firstMeaningfulLine(result.reviewText, `${reviewName} completed.`), | ||
| summary: shorten(firstMeaningfulLine(result.reviewText, `${reviewName} completed.`), 96), |
There was a problem hiding this comment.
When a native review/start turn finishes with a nonzero status and an error but produces no review text, this still records the summary as Review completed.; if the review text is a formatted error body, it can still record only {. Because /codex:status renders this indexed summary rather than errorMessage, failed native reviews continue to show a misleading or meaningless summary despite the useful error now being persisted. For failed results, derive the summary from result.error?.message or stderr before considering reviewText.
Useful? React with 👍 / 👎.
Upstream PR openai#763 (Soumya95), two conflicts resolved. A turn that failed without throwing (a rejected model, an unsupported parameter) stored only a truncated summary, so /codex:result showed that the job failed but not why. The failure text is now persisted as errorMessage on both the job file and the state index, and summaries are shortened to 96 characters instead of carrying a whole error into an index entry. Conflicts: both in tests/state.test.mjs and both unions. The import block keeps the fork's list and adds runTrackedJob. The test block keeps the fork's tests and appends the PR's two; the conflict cut through the fork's last test (its closing `});` was the line both sides shared after the marker), so that brace is restored explicitly in the resolution. Verified: node --check on the three touched scripts; tests/state.test.mjs, tests/tracked-jobs.test.mjs and tests/runtime.test.mjs 139/139. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UXfvnjSC72HsM6EEPVt2Tg
Adds openai#731, openai#737, openai#747 and openai#763 to "Differences From Upstream", and notes in Requirements that Node no longer has to be on the system PATH now that the hooks go through scripts/run-node.sh (including CODEX_COMPANION_NODE for pinning one). Verified: full npm test 254/254. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UXfvnjSC72HsM6EEPVt2Tg
1 participant
Summary
Fixes #757: a server-side turn failure that completes normally stored no
errorMessageanywhere, sostatusshowed onlySummary: {— the first line of the pretty-printed JSON error body.Two independent gaps caused this:
errorMessagewas only written on the throwing path.tracked-jobs.mjsseterrorMessageinside thecatchblock. A turn that failed withexitStatus != 0but completed normally ran through the success branch — noerrorMessagewas ever stored, andrenderJobStatusReportfell through to "No captured result payload was stored."The summary took the first line of raw output.
firstMeaningfulLine(rawOutput, ...)picked{whenrawOutputwas a pretty-printed error body. TheCodex error:progress line (codex.mjs:539) similarly output the error body without truncation.Changes
plugins/codex/scripts/lib/tracked-jobs.mjsexecution.errorMessageand persist it to both the job file and the index row whencompletionStatus === "failed".plugins/codex/scripts/codex-companion.mjsexecuteTaskRun: surfaceresult.error.messageaserrorMessage; preferfailureMessageoverrawOutputfor the summary; pass throughshorten().errorMessageand pass summary throughshorten(), so multi-line JSON bodies can never reduce to punctuation.plugins/codex/scripts/lib/codex.mjsCodex error:progress line viashorten(message.params.error.message, 96).tests/state.test.mjserrorMessagein job file and index.errorMessage.Verification
All 93 tests pass:
node --test tests/*.test.mjs— 0 failures.