feat: recover a lost result from the Codex rollout transcript - #578
feat: recover a lost result from the Codex rollout transcript#578xoonjaeho wants to merge 5 commits into
Conversation
…nscript When a turn ends without writing a result back to the job record -- the companion returned first, the app-server turn was interrupted, the process died -- `result` has only a bare "Status: failed" block to show, and the work looks lost. It usually is not. Codex writes every turn to its own transcript at $CODEX_HOME/sessions/<YYYY>/<MM>/<DD>/rollout-<timestamp>-<threadId>.jsonl, and the job record already carries threadId. When nothing was stored, read the last assistant message from there and render it under an explicit PARTIAL heading with the source path, so it reads as an unverified lead rather than a finished verdict. Read-only: it never writes to the job record and never changes a job that did store output. Verified against 832 real transcripts on this host: every sampled thread recovered its real final message (547-1790 chars) in 1-6 ms despite the flat directory walk. Four mutations each fail their protecting test (last-vs-first message, malformed trailing line, role filter, render short-circuit). Suite: 98 tests / 86 pass / 12 fail against a 91 / 79 / 12 baseline. No new failures. The 12 are pre-existing Windows-only failures that CI does not see.
![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: 851c9fd7c1
ℹ️ 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".
| // only exist in codex's own rollout transcript. Recover the last one so a job | ||
| // that ended without writing back is not a total loss. | ||
| const threadId = storedJob?.threadId ?? job.threadId ?? null; | ||
| const recovered = storedJobHasOutput(storedJob) ? null : readLastAssistantMessage(threadId); |
There was a problem hiding this comment.
Persistent task resumes reuse the same Codex thread, but jobs are per turn and already store turnId; when an older failed job has no stored output and the user later resumes that thread, this recovers the last assistant message for the whole thread rather than the job's turn. In that scenario /codex:result <old-job> can present a later task's answer as the old job's partial result, so the transcript lookup needs to be bounded by the stored/job turnId or at least by the job's completion/update time.
Useful? React with 👍 / 👎.
`--resume-last` reuses a thread, so one rollout transcript can hold turns from several jobs. Recovering by thread alone returned the file's last assistant message, which for an older job is a newer job's answer -- worse than returning nothing. The assistant records carry no turn id; only `task_started`, `turn_context` and `task_complete` do, as `payload.turn_id`. So the scan tracks the turn those markers open and keeps only the messages inside the job's own turn. A job with no recorded turn id now recovers nothing rather than guessing.
…task_complete Three findings from the review of 851c9fd + b1b1697, all in the transcript recovery path: - `task_complete` set `openTurnId` instead of clearing it, so any record between one turn's end and the next turn's start was attributed to the finished turn. - The recovery always scanned the raw assistant records, which keep codex's internal citation markup -- present on 91 of 843 assistant messages sampled here. `task_complete.last_agent_message` is the same answer without it, and it is recorded for every completed turn (169/169 sampled). Prefer it, and fall back to the scan when the turn never completed or the message is empty, which is what an errored turn records. - Its presence also proves the turn finished, so a recovered answer no longer claims "the turn may not have finished" when it demonstrably did. Only the cut-short case keeps the PARTIAL label. Also: `storedJobHasOutput` counted a whitespace-only stored result as output, so a job that stored "\n" suppressed the recovery that had the real answer.
…essage `renderStoredJobResult`'s structured-review branch returned early on any truthy `rendered`, so a whitespace-only one printed just that whitespace and dropped the recovered transcript text that `storedJobHasOutput` had already decided to go looking for. Gate both on the same `hasText`. The blank-final-message test now uses whitespace rather than "": an empty string is falsy either way, so only whitespace catches a regression in the trim guard.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75974264ea
ℹ️ 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".
| if (isStructuredReviewStoredResult(storedJob) && hasText(storedJob?.rendered)) { | ||
| return true; | ||
| } | ||
| if (hasText(storedJob?.result?.rawOutput)) { |
There was a problem hiding this comment.
When a failed job persisted result.rawOutput as only whitespace (the case this helper now classifies as no stored output), handleResult will recover a transcript message, but renderStoredJobResult still returns early on the legacy truthy rawOutput check before it reaches the recovered block. In that scenario /codex:result prints the blank stored payload and discards the recovered answer, so the raw-output render branch needs the same trimmed hasText gate or must skip whitespace when recovered is present.
Useful? React with 👍 / 👎.
|
The P2 on Both premises checked out: jobs do store One correction to the obvious fix, in case it helps: the assistant records carry no turn id of their own. Only Details and verification in the PR description update above. |
…d check `7597426` fixed the structured-review branch and stopped there. The two branches below it -- `result.rawOutput` / `codex.stdout`, and the bare `rendered` fallback -- still returned early on untrimmed truthiness, so a job that stored `"\n"` printed that whitespace and discarded the recovered transcript message. `storedJobHasOutput` had already been changed to treat whitespace as no output, which is what makes the recovery run at all. When the two functions disagree about what "has output" means, one suppresses the recovery and the other throws it away; they now use one `hasText` between them. Reported on openai#578 for the rawOutput branch; the `rendered` fallback had it too.
|
Second finding on this PR ( You were right that All of them now go through one Test covers all four stored shapes, and was checked by restoring the untrimmed check and confirming it fails. |
1 participant
Problem
When a job ends without writing a result back to its record,
/codex:resulthas nothing to show:This happens whenever the turn and the record get out of sync — the companion returns before the turn completes, the app-server turn is interrupted, the launcher dies.
statusandresultare both dead ends at that point, so the leg reads as a total loss.It usually isn't. Codex has already written every assistant message to its own transcript at
$CODEX_HOME/sessions/<YYYY>/<MM>/<DD>/rollout-<timestamp>-<threadId>.jsonl, and the job record already carriesthreadId. Nothing new needs to be captured — the data is on disk and simply never read.Recovering it has repeatedly turned a "lost" leg into a usable answer for me, including one genuine high-severity finding that would otherwise have been thrown away.
Change
When
readStoredJobyields no output and the job has athreadId, look up the transcript and render its last assistant message under an explicit heading:storedJobHasOutputgates it.--jsonaspayload.recovered.resolveCodexHome()was already implemented inlib/codex.mjs; this only exports it.New file
lib/rollout.mjsdoes the filesystem work sorender.mjsstays pure.Verification
Against real data, not just fixtures. Run over the 832 real transcripts on this machine, sampling four threads:
019fba9c-063a-70a1…019fbab1-07d6-7d83…019fbace-b8a5-79c2…019fbad1-b569-7d93…Every one returned that thread's real final message. The directory walk is flat and uncached, and still costs single-digit milliseconds at 832 files.
Four mutations, each caught by its own test:
returns the last assistant message, not the firstsurvives the half-written final line a killed turn leavesrole === "assistant"filterreturns null when the transcript holds no assistant textsurfaces a recovered message instead of the empty-payload noticeThe malformed-line case is not hypothetical: a killed turn leaves a truncated final line, which is exactly the situation this feature exists for.
Suite: 98 tests / 86 pass / 12 fail, against a 91 / 79 / 12 baseline on the same host. No new failures. (Those 12 are pre-existing Windows-only failures — Unix-socket, temp-dir and real-CLI assertions. CI is ubuntu-only and does not see them.)
Notes
record.type === "response_item"withpayload.type === "message",payload.role === "assistant", andpayload.contenta list of{ type: "output_text", text }. Stringcontentis handled too.session_meta.id, so therollout-*-<threadId>.jsonlmatch is exact rather than heuristic.Update — turn scoping, and using the turn's own final message
Three commits since the original review, all in this path. The first fixes the P2 the automated review raised on this PR.
b1b1697— scope the recovery to the job's own turn.--resume-lastreuses a thread, so one transcript can hold turns from several jobs; 47 of 836 transcripts on my host do. Recovering by thread alone returned the file's last assistant message, which for an older job is a newer job's answer — worse than returning nothing. The job record already carriesturnId, and it holds the same value as the transcript'spayload.turn_id.The assistant records themselves carry no turn id — only
task_started,turn_contextandtask_completedo — so the scan tracks the turn those markers open and keeps only the messages inside the wanted one. A job with no recordedturnIdnow recovers nothing rather than guessing.fe3514f— prefertask_complete.last_agent_message, and stop mislabelling finished turns.task_completewas settingopenTurnIdinstead of clearing it, so records between one turn's end and the next turn's start were attributed to the finished turn.task_complete.last_agent_messageis the same answer without it, and it is recorded for every completed turn (169/169 sampled). Prefer it; fall back to the scan when the turn never completed, or when that message is empty, which is what an errored turn records.storedJobHasOutputcounted a whitespace-only stored result as output, so a job that stored"\n"suppressed the recovery that had the real answer.7597426—renderStoredJobResult's structured-review branch returned early on any truthyrendered, so a whitespace-only one printed just that whitespace and dropped the recovered text. Both now gate on the samehasText.Verification. 12 tests in
tests/rollout.test.mjs; each was checked by breaking the line it protects and confirming that test — and only that test — fails. Also exercised against two real (threadId, turnId) pairs from actual job records on disk: both return the right text with no citation markup, and a deliberately wrongturnIdreturns null.