mirror of
https://github.com/block/buzz.git
synced 2026-10-27 04:30:52 +00:00
fix(ci): salvage Codex review output on PTY-shutdown hang (#7042)
Codex CLI can leave a PTY descendant holding the action's inherited
stdio after the turn completes. The `runCodexExec.ts` wrapper waits on a
`close` event that never fires, so the `Review pull request` step 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-message` file
(exposed as `output-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`):**
- Add `output-file: ${{ runner.temp }}/codex-review.json` to the `Review
pull request` step so the CLI writes the result before the hang.
(`runner` context is valid in `steps.with`; not in `jobs.env`.)
- Add `timeout-minutes: 30` and `continue-on-error: true` to the Codex
step — a hang now costs ≤30 minutes instead of 40, and the salvage step
still runs.
- Set job `timeout-minutes: 40` to 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](https://github.com/block/buzz/actions/runs/33114428326/job/98665369165)
shows completed output appearing 28m46s after step start, meaning a
20-minute step timeout could kill a legitimate review before the salvage
file exists.
- Add a `Salvage review output` step with `if: always()`: prefers
`steps.run_codex.outputs.final-message` on a clean exit; falls back to
the output file when the step timed out. The output file path is set in
the step's own `env` block (`CODEX_OUTPUT_FILE: ${{ runner.temp
}}/codex-review.json`), where `runner` is valid. Validates shape
(non-empty JSON object, has `overall_risk`); fails the job hard if
neither source is present.
- Wire the job `outputs.review_json` to
`steps.salvage.outputs.review_json`.
**Changes (`Justfile`, `ci.yml`):**
- Add `actionlint .github/workflows/codex-security-review.yml` to
`security-review-check` so expression-validity errors are caught
locally.
- Provision `actionlint` via Hermit (pinned v1.7.12) rather than a
one-off `Install actionlint` curl 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
https://github.com/openai/codex-action/issues/169 is addressed
---------
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Co-authored-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
This commit is contained in:
57
.github/workflows/codex-security-review.yml
vendored
57
.github/workflows/codex-security-review.yml
vendored
@@ -214,7 +214,7 @@ jobs:
|
||||
if: needs.prepare-review.outputs.authorized == 'true'
|
||||
runs-on: ubuntu-latest
|
||||
environment: codex-review
|
||||
timeout-minutes: 30
|
||||
timeout-minutes: 40
|
||||
concurrency:
|
||||
group: codex-security-review-${{ needs.prepare-review.outputs.pr_number }}
|
||||
cancel-in-progress: true
|
||||
@@ -228,7 +228,7 @@ jobs:
|
||||
REVIEW_REPOSITORY: review-target
|
||||
REVIEW_DIFF_FILE: .git/codex-review.diff
|
||||
outputs:
|
||||
review_json: ${{ steps.run_codex.outputs.final-message }}
|
||||
review_json: ${{ steps.salvage.outputs.review_json }}
|
||||
steps:
|
||||
- name: Checkout exact pull request head
|
||||
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
|
||||
@@ -287,6 +287,12 @@ jobs:
|
||||
# action's local proxy rather than entering the Codex subprocess.
|
||||
- name: Review pull request
|
||||
id: run_codex
|
||||
# Codex CLI ≥0.149.x can leave a PTY descendant holding inherited stdio
|
||||
# after the turn completes, stalling the action indefinitely. The output
|
||||
# file is written before the hang, so a timeout here wastes at most 30
|
||||
# minutes instead of the full 40, and the salvage step recovers the result.
|
||||
timeout-minutes: 30
|
||||
continue-on-error: true
|
||||
uses: openai/codex-action@86365089eb2b84e0a8fb0717b304f8bdcb13b20e # v1.12
|
||||
env:
|
||||
# Checkout and fetch are complete. Remove runner credentials from the
|
||||
@@ -306,6 +312,8 @@ jobs:
|
||||
safety-strategy: drop-sudo
|
||||
permission-profile: ':read-only'
|
||||
working-directory: ${{ github.workspace }}/${{ env.REVIEW_CONTEXT }}
|
||||
# Written before the hang; salvaged below if the step times out.
|
||||
output-file: ${{ runner.temp }}/codex-review.json
|
||||
output-schema: |
|
||||
{
|
||||
"type": "object",
|
||||
@@ -442,6 +450,51 @@ jobs:
|
||||
assumptions. Review only the authorized PR range and ground every finding
|
||||
in a changed hunk and a plausible failure or abuse path.
|
||||
|
||||
# Salvage the finished review whether the Codex step completed cleanly or
|
||||
# timed out due to the PTY-shutdown hang. Prefer the action's final-message
|
||||
# output (set on a clean exit); fall back to the output file written by the
|
||||
# CLI before the hang. Fail the job only when neither source is available or
|
||||
# the recovered JSON is not a valid review shape.
|
||||
- name: Salvage review output
|
||||
id: salvage
|
||||
if: always()
|
||||
env:
|
||||
FINAL_MESSAGE: ${{ steps.run_codex.outputs.final-message }}
|
||||
CODEX_OUTPUT_FILE: ${{ runner.temp }}/codex-review.json
|
||||
run: |
|
||||
json=""
|
||||
|
||||
# Prefer the action output set on a clean exit.
|
||||
if [ -n "$FINAL_MESSAGE" ]; then
|
||||
json="$FINAL_MESSAGE"
|
||||
echo "source=action-output" >> "$GITHUB_STEP_SUMMARY"
|
||||
elif [ -s "$CODEX_OUTPUT_FILE" ]; then
|
||||
json="$(cat "$CODEX_OUTPUT_FILE")"
|
||||
echo "source=output-file" >> "$GITHUB_STEP_SUMMARY"
|
||||
else
|
||||
echo "No review output from action or output file." >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Minimal shape validation: non-empty JSON object with overall_risk.
|
||||
if ! echo "$json" | python3 -c "
|
||||
import sys, json
|
||||
d = json.load(sys.stdin)
|
||||
assert isinstance(d, dict), 'not an object'
|
||||
assert 'overall_risk' in d, 'missing overall_risk'
|
||||
"; then
|
||||
echo "Review JSON failed shape validation." >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Write as a multiline output (GitHub-safe delimiter).
|
||||
EOF=$(dd if=/dev/urandom bs=15 count=1 2>/dev/null | base64)
|
||||
{
|
||||
echo "review_json<<${EOF}"
|
||||
echo "$json"
|
||||
echo "${EOF}"
|
||||
} >> "$GITHUB_OUTPUT"
|
||||
|
||||
post-review:
|
||||
name: Post Codex Security Review
|
||||
needs: [prepare-review, security-review]
|
||||
|
||||
Reference in New Issue
Block a user