fix(ci): salvage Codex review output on PTY-shutdown hang - #7042
Merged
Conversation
Codex CLI leaves a PTY descendant holding the action's inherited stdio after the turn completes; the action wrapper waits on a `close` event that never fires, so the job hangs until timeout-minutes kills it. The CLI writes the finished review to the output file before the hang, so the result is always present on disk — the job just never reads it. Add a salvage step that runs with `if: always()` after the Codex step: prefer the action's `final-message` output on a clean exit; fall back to the output file when the step timed out. Shape-validate the recovered JSON (non-empty object, has `overall_risk`); fail hard if neither source is present. Set `timeout-minutes: 20` and `continue-on-error: true` on the Codex step so a hang costs 20 minutes instead of 30 and the salvage step still runs. The output file is written to `runner.temp` (outside both checkouts) and the salvage step receives the action output as an env var, not from the PR-controlled tree, preserving the existing isolation posture. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
wpfleger96
had a problem deploying
to
codex-review
August 29, 2026 20:29 — with 
GitHub Actions
Error
GitHub Actions
Error
`runner` context is not allowed in `jobs.<job_id>.env`; the merged workflow would be rejected at load time, disabling the security review. Move `runner.temp` to the two step-level locations where it is valid: the Codex step's `output-file` input (inside `with`) and the salvage step's own `env` block. While here, fix a SC2129 shellcheck style warning in the salvage script by grouping the GITHUB_OUTPUT writes. Add `actionlint .github/workflows/codex-security-review.yml` to the `security-review-check` Justfile recipe so expression-validity errors are caught locally and in CI. Wire a pinned actionlint install (v1.7.12, SHA-256 verified) into the `changes` job in ci.yml immediately before the `just security-review-check` step. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
wpfleger96
had a problem deploying
to
codex-review
August 29, 2026 21:14 — with
GitHub Actions
Error
GitHub Actions
Error
The previous approach installed actionlint with a custom curl/checksum/tar step in ci.yml, which kept the binary outside the Hermit environment. This made `just security-review-check` fail with `command not found` on any fresh checkout using the documented Hermit-only PATH (`just ci` / `just bootstrap` paths), while CI stayed green only because the custom install step masked the gap. Track actionlint-1.7.12 in Hermit so both local and CI environments get the same pinned binary through the same provisioning path. Remove the six-line curl/checksum/tar install step from the ci.yml contracts job; cashapp/ activate-hermit already runs there and will lazily provision the binary. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
wpfleger96
had a problem deploying
to
codex-review
August 29, 2026 21:26 — with
GitHub Actions
Error
GitHub Actions
Error
The 20-minute step timeout was below the only bounded timing evidence for a real heavy review: output appeared at 28m46s after step start in run 33114428326. A legitimate review could be killed before the salvage file is written, discarding a valid result. Separate the step and job deadlines to preserve the 30-minute Codex execution budget while still leaving headroom for setup, step cancellation, and salvage to complete within the job ceiling. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
wpfleger96
had a problem deploying
to
codex-review
August 31, 2026 15:20 — with
GitHub Actions
Error
GitHub Actions
Error
jmecom
approved these changes
Aug 31, 2026
wpfleger96
pushed a commit
that referenced
this pull request
Aug 31, 2026
* origin/main: feat: render agent avatars as squircles (#7106) fix(ci): salvage Codex review output on PTY-shutdown hang (#7042) fix: retrieving cold memories; add regression task (#6950) Enforce NIP-OA authorization time bounds (#7004) feat(db): configurable writer session timeouts (lock, idle-txn, statement) (#6229) feat(desktop): use segmented controls for channel creation (#6845) feat(buzz-agent): surface stop reason and silent-turn WARN in telemetry (#7038) fix(desktop): surface channel history load failures (#7013) fix(composer): polish automatic mentions (#6956) Signed-off-by: Hayt <9e1c23a3fd83f61da34420e4e88ff1b16e45cafcc0cd9019eb07d4ecfa8ca9b0@buzz.block.builderlab.xyz>
jrobotham-square
added a commit
that referenced
this pull request
Aug 31, 2026
…age-rw * origin/main: feat: render agent avatars as squircles (#7106) fix(ci): salvage Codex review output on PTY-shutdown hang (#7042) fix: retrieving cold memories; add regression task (#6950) Enforce NIP-OA authorization time bounds (#7004) feat(db): configurable writer session timeouts (lock, idle-txn, statement) (#6229) feat(desktop): use segmented controls for channel creation (#6845) feat(buzz-agent): surface stop reason and silent-turn WARN in telemetry (#7038) fix(desktop): surface channel history load failures (#7013) fix(composer): polish automatic mentions (#6956) Signed-off-by: Joel Robotham <jrobotham@squareup.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
2 participants
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.
Codex CLI can leave a PTY descendant holding the action's inherited stdio after the turn completes. The
runCodexExec.tswrapper waits on acloseevent that never fires, so theReview pull requeststep hangs until the job timeout kills it — discarding the finished review the CLI already wrote to disk.The CLI writes the completed review to the
--output-last-messagefile (exposed asoutput-file) before the hang. This PR adds a salvage step that recovers it, and sets the step and job timeouts to preserve the full 30-minute Codex execution budget.Changes (
codex-security-review.yml):output-file: ${{ runner.temp }}/codex-review.jsonto theReview pull requeststep so the CLI writes the result before the hang. (runnercontext is valid insteps.with; not injobs.env.)timeout-minutes: 30andcontinue-on-error: trueto the Codex step — a hang now costs ≤30 minutes instead of 40, and the salvage step still runs.timeout-minutes: 40to give setup, step cancellation, and salvage sufficient headroom without colliding with the Codex execution budget. The original 30-minute job timeout was too narrow: evidence from run 33114428326 shows completed output appearing 28m46s after step start, meaning a 20-minute step timeout could kill a legitimate review before the salvage file exists.Salvage review outputstep withif: always(): preferssteps.run_codex.outputs.final-messageon a clean exit; falls back to the output file when the step timed out. The output file path is set in the step's ownenvblock (CODEX_OUTPUT_FILE: ${{ runner.temp }}/codex-review.json), whererunneris valid. Validates shape (non-empty JSON object, hasoverall_risk); fails the job hard if neither source is present.outputs.review_jsontosteps.salvage.outputs.review_json.Changes (
Justfile,ci.yml):actionlint .github/workflows/codex-security-review.ymltosecurity-review-checkso expression-validity errors are caught locally.actionlintvia Hermit (pinned v1.7.12) rather than a one-offInstall actionlintcurl step, so the same binary is used locally and in CI.Security posture is unchanged: the salvage step reads the action's own output and a file written to
runner.temp— neither is PR-controlled. Credential-stripping env block on the Codex step is untouched.Note this is a temporary workaround until openai/codex-action#169 is addressed