Report aborted turns honestly instead of surfacing the model's preamble - #89
Merged
Conversation
… report The idle stall watchdog interrupts a turn, but executeTaskRun still hands back result.finalMessage as rawOutput and drops the failure reason from the JSON payload entirely, so a controller sees exit 1 plus a plausible-looking status report and no explanation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
When the idle stall watchdog interrupts a turn, `finalMessage` is the last agent message that existed at abort time -- typically the model's opening narration, not its report. `executeTaskRun` handed that straight back as `rawOutput`, and dropped the stall reason from the JSON payload entirely, so an SDD controller saw exit 1 plus a plausible-looking status report and no explanation, and would re-dispatch on top of already-applied edits. Gate `rawOutput` on turn completion, move the partial text to `partialOutput`, carry `failureMessage` in the payload, and render the failure reason plus the files this run already touched. Apply the same completion gate to the structured review path. Closes #88 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The recovery note lived under Dispatch and Follow-Through, which governs
every dispatch, so its blanket "resume ${IMPLEMENTER_THREAD_ID}" would
have pointed a failed reviewer at the write-capable implementer thread --
abandoning the review and risking unintended edits. Split it: implementers
resume their own thread after a touched-files check, reviewers re-dispatch
fresh because they changed nothing on disk.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #88
Root cause
Not a phantom interrupt. The plugin's own idle stall watchdog fired. Confirmed from the three failing jobs' log files:
DEFAULT_TURN_STALL_TIMEOUT_MSis 15 min; the rollouts show 15m01s, 21m00s and 21m06s of complete notification silence before the abort.handleStallcallsturn/interrupt, and Codex records every interrupt asturn_aborted / reason: "interrupted"regardless of who issued it — which is why it read as a user cancellation. Two of the three went silent within six seconds of each other on independent turns, which points at an upstream stall rather than the model thinking. The watchdog did its job and is unchanged here.The
duration_msin theturn_abortedevent (247070) is also not the wall clock —completed_at - started_atis 1434s for that same turn — so the coincidence with the 240s status-wait timeout noted in the issue was a red herring.The actual defect
Everything wrong here is in how the aborted turn is reported:
executeTaskRunsetrawOutput = result.finalMessageunconditionally.finalMessageisturnState.lastAgentMessage, so on an aborted turn it is the model's opening narration — well-formed prose that/codex:implementparses as the report body.renderTaskResultreturnsrawOutputwhenever it is non-empty, so it shadowed the failure message too.Fix
rawOutputis gated on turn completion (result.status === 0). An incomplete turn's last agent message moves to a newpartialOutputfield, so every downstream consumer that keys offrawOutputcorrectly sees "no report".partialOutputandfailureMessage.renderTaskResultleads with the failure reason, lists the files the run already modified, and reproduces the partial text under a heading that says plainly it is not the final report. Completed turns render byte-identically to before.payload.codex.stdout.implement.mdnow warns the controller that a failed dispatch may have already applied edits: checktouchedFiles/git statusand resume with--resume-idinstead of re-dispatching fresh.Verification
The first commit is a failing repro, committed before any product change:
tests/fake-codex-fixture.mjsgains anidle-hung-turn-after-preamblebehavior — emits a non-finalagentMessageplus afileChange, then goes silent until the idle watchdog fires.tests/runtime.test.mjsdrives a real end-to-endtask --writerun against it. Before the fix it failed withrawOutputequal to the preamble; after,rawOutputis empty,partialOutputholds the preamble,failureMessageholds the stall reason, andtouchedFilesstill reports the edit that landed.tests/render.test.mjspins both branches ofrenderTaskResult, including exact-string equality on the completed-turn path.Full suite: 303 passed, 0 failed (
node --test --test-concurrency=1 tests/*.test.mjs).Codex SDD reviewers: spec
SPEC_COMPLIANT, code qualityAPPROVED.🤖 Generated with Claude Code