From 4ff2154aca9b517fb15e022ae3edc3140664dfe8 Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Wed, 5 Aug 2026 16:37:02 -0600 Subject: [PATCH 1/4] ci(deep-review): pin claude-code CLI to dodge bwrap sandbox regression claude-code-action >=2.1.216 has a bwrap sandbox regression: when subprocess isolation is on (auto-enabled by allowed_non_write_users), bwrap aborts every Bash call trying to mask a repo-root .mcp.json ("bwrap: Can't create file at /home/.mcp.json: Permission denied"). This kills git/gh and the reviewer fan-out, so the review posts an "environment failure" comment while the job still reports success. The action hardcodes its CLI version with no version input, so pin 2.1.215 (last pre-regression release) via path_to_claude_code_executable, asserting the installed binary reports the pinned version. Add a permanent smoke check that fails the job loud if the execution transcript carries the sandbox-failure signature, so a broken sandbox can never again post a zero-coverage review under a green checkmark. Unpin once upstream fixes the regression (tracked in HDX-4907). Upstream: https://github.com/anthropics/claude-code-action/issues/1547 --- .github/workflows/deep-review.yml | 101 ++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/.github/workflows/deep-review.yml b/.github/workflows/deep-review.yml index a3d3dda680..99b7d2d7e3 100644 --- a/.github/workflows/deep-review.yml +++ b/.github/workflows/deep-review.yml @@ -48,6 +48,12 @@ jobs: # Single-source the reviewer plugin ref: consumed by the plugin checkout, # the review gate, and the state marker. Bump in one place. PLUGIN_REF: compound-engineering-v3.6.1 + # TEMPORARY: pin the Claude Code CLI to the last version before the + # >=2.1.216 bwrap sandbox regression that breaks every Bash call. + # Single-sourced so the install arg and the post-install assertion + # cannot drift. UNPIN once upstream fixes it (tracked in HDX-4907). + # https://github.com/anthropics/claude-code-action/issues/1547 + CLAUDE_CLI_VERSION: 2.1.215 permissions: contents: read pull-requests: write @@ -249,6 +255,44 @@ jobs: ref: ${{ env.PLUGIN_REF }} path: ce-plugin + # --- CLI pin (TEMPORARY) ---------------------------------------------- + # claude-code-action hardcodes the Claude Code CLI version it installs + # (currently 2.1.220) and exposes no version input -- the only override + # is `path_to_claude_code_executable`. CLI >= 2.1.216 has a bwrap sandbox + # regression: whenever subprocess isolation is on (which + # `allowed_non_write_users: '*'` below auto-enables via + # CLAUDE_CODE_SUBPROCESS_ENV_SCRUB=1), bwrap aborts EVERY Bash call while + # trying to mask a repo-root `.mcp.json`, with: + # bwrap: Can't create file at /home/.mcp.json: Permission denied + # That kills git/gh and the reviewer fan-out, so the review posts an + # "environment failure" comment -- and the job still reports success. + # CLAUDE_CLI_VERSION is the last version before the regression. Install + # it into a fixed path and point the action at it below. + # Upstream issue: https://github.com/anthropics/claude-code-action/issues/1547 + # UNPIN once the upstream regression is fixed (tracked in HDX-4907). + - name: Pin Claude Code CLI to a pre-regression version + if: steps.gate.outputs.should_review == 'true' + run: | + set -euo pipefail + curl -fsSL https://claude.ai/install.sh | bash -s -- "$CLAUDE_CLI_VERSION" + CLAUDE_BIN="$HOME/.local/bin/claude" + if [ ! -x "$CLAUDE_BIN" ]; then + echo "::error::Pinned Claude Code CLI not found at $CLAUDE_BIN after install." + exit 1 + fi + # Hard-fail unless the installed binary actually reports the pinned + # version. Without this, an installer that ignores the version arg or + # silently drifts would run a regressed CLI under a green build, + # defeating the pin. `claude --version` prints e.g. "2.1.215 (Claude + # Code)"; assert the pinned version appears as a whole token. + INSTALLED_VERSION="$("$CLAUDE_BIN" --version 2>/dev/null || true)" + echo "Installed Claude Code CLI: ${INSTALLED_VERSION:-}" + if ! printf '%s' "$INSTALLED_VERSION" | grep -qE "(^|[^0-9.])${CLAUDE_CLI_VERSION//./\\.}([^0-9.]|$)"; then + echo "::error::Pinned CLI check failed: expected $CLAUDE_CLI_VERSION, got '${INSTALLED_VERSION:-}'. The installer may have ignored the version argument." + exit 1 + fi + echo "PINNED_CLAUDE=$CLAUDE_BIN" >> "$GITHUB_ENV" + - name: Run deep review id: review if: steps.gate.outputs.should_review == 'true' @@ -256,6 +300,11 @@ jobs: with: anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} github_token: ${{ secrets.GITHUB_TOKEN }} # bypasses OIDC auth (required for pull_request_target) + # Pin the CLI to dodge the >=2.1.216 bwrap sandbox regression that + # breaks every Bash call. Remove this input to revert to the action's + # bundled CLI once upstream fixes it. + # https://github.com/anthropics/claude-code-action/issues/1547 + path_to_claude_code_executable: ${{ env.PINNED_CLAUDE }} allowed_bots: dependabot,dependabot[bot],kodiakhq,kodiakhq[bot],github-actions,github-actions[bot],cursor,cursor[bot],claude,claude[bot] allowed_non_write_users: '*' # allow fork-PR contributors to trigger reviews @@ -419,6 +468,58 @@ jobs: --allowedTools "Bash(git:*),Bash(gh pr view:*),Bash(gh pr diff:*),Bash(gh pr list:*),Bash(gh issue view:*),Bash(gh issue list:*),Bash(gh search:*),Bash(gh api:*)" --json-schema '{"type":"object","properties":{"review":{"type":"string","description":"Complete markdown review starting with on the first line and ## Deep Review on the second line"}},"required":["review"]}' + # --- Sandbox smoke check ---------------------------------------------- + # PERMANENT guard -- KEEP this even after the CLI pin above is removed. + # It defends against the whole class of "green checkmark on a broken + # sandbox" failures, not just the specific #1547 regression. + # + # This bug's nastiest trait is that the job reports SUCCESS while every + # Bash call inside the action silently fails (the action's `conclusion` + # is derived only from the final result message, never from per-tool + # errors), so a zero-coverage "environment failure" review posts under a + # green checkmark. Detect the failure from the MACHINE-READABLE execution + # transcript rather than the model's free-text review body -- grepping + # the review would false-positive on any PR (like this one) that merely + # discusses the bwrap error. The execution_file is a JSON array of SDK + # messages; failed tool calls appear as `tool_result` blocks with + # `is_error: true` carrying the bwrap error text in `content`. + # https://github.com/anthropics/claude-code-action/issues/1547 + - name: Assert reviewer sandbox is healthy + if: steps.gate.outputs.should_review == 'true' + env: + EXECUTION_FILE: ${{ steps.review.outputs.execution_file }} + run: | + set -euo pipefail + # A missing transcript on a review run is itself suspicious (the SDK + # iterator can hang before writing the file), so fail rather than + # silently pass with no evidence. + if [ -z "${EXECUTION_FILE:-}" ] || [ ! -f "$EXECUTION_FILE" ]; then + echo "::error::No execution transcript from the reviewer (execution_file missing). Cannot confirm the sandbox was healthy." + exit 1 + fi + # Count tool_result blocks that errored with the bwrap sandbox + # signature. `content` may be a string or an array of text blocks, + # so normalize both. jq reads the file directly -- no pipe, so no + # pipefail/SIGPIPE fail-open. + BROKEN=$(jq ' + [ .[] + | select(.type == "user") + | .message.content[]? + | select(.type == "tool_result" and .is_error == true) + | ( .content + | if type == "array" then map(.text // "") | join("\n") + elif type == "string" then . + else tostring end ) + | select(test("bwrap: Can.t create file|Can.t create file at .*\\.mcp\\.json|bwrap:.*Permission denied")) + ] | length + ' "$EXECUTION_FILE") + echo "bwrap sandbox-failure tool_results: $BROKEN" + if [ "$BROKEN" -gt 0 ]; then + echo "::error::Deep review ran in a broken sandbox ($BROKEN bwrap Bash failures). The CLI pin is not taking effect or the regression changed shape. See https://github.com/anthropics/claude-code-action/issues/1547" + exit 1 + fi + echo "Sandbox smoke check passed -- no bwrap failures in the reviewer transcript." + # fromJSON() in `with:` has been observed to leave structured_output JSON # unparsed for the sibling claude-code-review workflow. Extract via jq. # From e624ea6867a64e87195b8bf9b8d73687f37c0a40 Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Wed, 5 Aug 2026 16:44:35 -0600 Subject: [PATCH 2/4] ci(deep-review): verify pinned CLI tarball integrity instead of trusting a remote installer The pin previously piped a mutable remote install script to bash and trusted the result via the binary's spoofable --version output -- compromise of the installer delivery chain could run arbitrary code on the runner and substitute the executable the credentialed review action later invokes. Fetch the immutable npm tarball for the pinned version instead, verify it against a SHA-512 hardcoded in the workflow (npm dist.integrity of @anthropic-ai/claude-code@2.1.215), and install from the verified local tarball with --ignore-scripts. The only lifecycle script executed is install.cjs from the hash-verified tarball, invoked explicitly to link the platform-native binary, which itself is an optionalDependency exact-pinned by the verified package.json -- anchoring the whole chain to the hash in this file. The version assertion remains as a sanity check only. --- .github/workflows/deep-review.yml | 50 +++++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 12 deletions(-) diff --git a/.github/workflows/deep-review.yml b/.github/workflows/deep-review.yml index 99b7d2d7e3..345429650a 100644 --- a/.github/workflows/deep-review.yml +++ b/.github/workflows/deep-review.yml @@ -50,10 +50,16 @@ jobs: PLUGIN_REF: compound-engineering-v3.6.1 # TEMPORARY: pin the Claude Code CLI to the last version before the # >=2.1.216 bwrap sandbox regression that breaks every Bash call. - # Single-sourced so the install arg and the post-install assertion - # cannot drift. UNPIN once upstream fixes it (tracked in HDX-4907). + # The SHA-512 is the npm dist.integrity of the + # @anthropic-ai/claude-code@ tarball; the pin step verifies the + # downloaded tarball against it before installing, so a compromised + # registry response cannot substitute the binary the credentialed review + # action later executes. Recompute it when bumping the version: + # npm view @anthropic-ai/claude-code@ dist.integrity + # UNPIN once upstream fixes it (tracked in HDX-4907). # https://github.com/anthropics/claude-code-action/issues/1547 CLAUDE_CLI_VERSION: 2.1.215 + CLAUDE_CLI_SHA512: sha512-lsWBvyMyBqg/rOZ06o/HEhlpOzHsH8IBf9RH4u5gzizQ1LWS/oXAh5nqWNUu3hn2d8QkyYg0aseNzMDlGD+3qg== permissions: contents: read pull-requests: write @@ -266,29 +272,49 @@ jobs: # bwrap: Can't create file at /home/.mcp.json: Permission denied # That kills git/gh and the reviewer fan-out, so the review posts an # "environment failure" comment -- and the job still reports success. - # CLAUDE_CLI_VERSION is the last version before the regression. Install - # it into a fixed path and point the action at it below. + # CLAUDE_CLI_VERSION is the last version before the regression. Rather + # than piping a mutable remote installer to bash, fetch the immutable + # npm tarball for that version, verify it against the SHA-512 hardcoded + # in this workflow (CLAUDE_CLI_SHA512), and install from the verified + # local tarball -- so trust rests on the hash in this file, not on the + # installer delivery chain or on the binary's spoofable --version output. # Upstream issue: https://github.com/anthropics/claude-code-action/issues/1547 # UNPIN once the upstream regression is fixed (tracked in HDX-4907). - name: Pin Claude Code CLI to a pre-regression version if: steps.gate.outputs.should_review == 'true' run: | set -euo pipefail - curl -fsSL https://claude.ai/install.sh | bash -s -- "$CLAUDE_CLI_VERSION" - CLAUDE_BIN="$HOME/.local/bin/claude" + TARBALL="$RUNNER_TEMP/claude-code-$CLAUDE_CLI_VERSION.tgz" + curl -fsSL -o "$TARBALL" \ + "https://registry.npmjs.org/@anthropic-ai/claude-code/-/claude-code-$CLAUDE_CLI_VERSION.tgz" + # Integrity gate: hard-fail unless the downloaded tarball matches the + # SHA-512 pinned in this workflow. Everything past this line runs + # only on verified content. + ACTUAL="sha512-$(openssl dgst -sha512 -binary "$TARBALL" | openssl base64 -A)" + if [ "$ACTUAL" != "$CLAUDE_CLI_SHA512" ]; then + echo "::error::Pinned CLI tarball integrity check failed: expected $CLAUDE_CLI_SHA512, got $ACTUAL. Refusing to install." + exit 1 + fi + INSTALL_DIR="$RUNNER_TEMP/claude-cli" + # --ignore-scripts so no dependency lifecycle script runs; the only + # script we execute is install.cjs from the hash-verified tarball, + # invoked explicitly below to link the platform-native binary. That + # binary comes from an optionalDependency exact-pinned (same version) + # by the verified package.json, so the whole chain is anchored to + # CLAUDE_CLI_SHA512. + npm install --prefix "$INSTALL_DIR" --no-audit --no-fund --ignore-scripts "$TARBALL" + node "$INSTALL_DIR/node_modules/@anthropic-ai/claude-code/install.cjs" + CLAUDE_BIN="$INSTALL_DIR/node_modules/.bin/claude" if [ ! -x "$CLAUDE_BIN" ]; then echo "::error::Pinned Claude Code CLI not found at $CLAUDE_BIN after install." exit 1 fi - # Hard-fail unless the installed binary actually reports the pinned - # version. Without this, an installer that ignores the version arg or - # silently drifts would run a regressed CLI under a green build, - # defeating the pin. `claude --version` prints e.g. "2.1.215 (Claude - # Code)"; assert the pinned version appears as a whole token. + # Sanity check (integrity is already guaranteed by the hash above): + # the binary should run and report the pinned version. INSTALLED_VERSION="$("$CLAUDE_BIN" --version 2>/dev/null || true)" echo "Installed Claude Code CLI: ${INSTALLED_VERSION:-}" if ! printf '%s' "$INSTALLED_VERSION" | grep -qE "(^|[^0-9.])${CLAUDE_CLI_VERSION//./\\.}([^0-9.]|$)"; then - echo "::error::Pinned CLI check failed: expected $CLAUDE_CLI_VERSION, got '${INSTALLED_VERSION:-}'. The installer may have ignored the version argument." + echo "::error::Pinned CLI check failed: expected $CLAUDE_CLI_VERSION, got '${INSTALLED_VERSION:-}'." exit 1 fi echo "PINNED_CLAUDE=$CLAUDE_BIN" >> "$GITHUB_ENV" From 0c4743af1bd5e68baccc75bc971d847ac520dc6e Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Wed, 5 Aug 2026 17:04:47 -0600 Subject: [PATCH 3/4] ci(deep-review): fix sandbox-guard false positives and stop discarding completed reviews Address the deep review's P0/P1 findings on the sandbox smoke check: - The bwrap signature test was unanchored and matched anywhere in any errored tool_result -- including transcripts that merely QUOTE the error text, which this workflow file now permanently contains. Require matches to (a) correlate to a Bash tool_use by id and (b) carry the bwrap signature at the start of a line, as real bwrap stderr does (quoted occurrences are diff-/comment-prefixed). - The guard failed the job before Extract/Post, whose bare if: is implicitly ANDed with success() -- so a guard failure discarded the already-completed review, posted nothing, and skipped the state marker, re-running the full fan-out on the next push. The guard now only detects and records; Extract/Post run under always() (still gated on the review step succeeding) so the review always posts, and a final step fails the job loud afterwards. Unhealthy runs omit the parseable state marker so the gate fail-opens and the next run re-reviews instead of trusting a zero-coverage review. A sandbox check that itself errors is treated as unhealthy, not fail-open. Verified against fixture transcripts: real bwrap failure (string and array content) -> broken; quoted signature in errored Bash output, non-Bash errored tool_result, healthy run -> pass; missing transcript -> broken; omitted marker does not parse under the gate's MARKER_RE. --- .github/workflows/deep-review.yml | 114 +++++++++++++++++++++++------- 1 file changed, 89 insertions(+), 25 deletions(-) diff --git a/.github/workflows/deep-review.yml b/.github/workflows/deep-review.yml index 345429650a..78b28b3dc0 100644 --- a/.github/workflows/deep-review.yml +++ b/.github/workflows/deep-review.yml @@ -509,41 +509,66 @@ jobs: # discusses the bwrap error. The execution_file is a JSON array of SDK # messages; failed tool calls appear as `tool_result` blocks with # `is_error: true` carrying the bwrap error text in `content`. + # + # To avoid false positives on transcripts that merely QUOTE the error + # text (e.g. an errored tool call whose output includes this workflow + # file), a match must (a) be a tool_result correlated to a Bash tool_use + # by id, and (b) carry the bwrap signature at the start of a line, as + # real bwrap stderr does (quoted occurrences are diff-/comment-prefixed). + # + # This step only DETECTS and records the verdict. The job is failed by + # "Fail if reviewer sandbox was unhealthy" below, AFTER the review + # comment is posted -- so a completed review is never discarded, and the + # state marker is omitted for unhealthy runs so the gate fail-opens and + # the next run re-reviews. # https://github.com/anthropics/claude-code-action/issues/1547 - - name: Assert reviewer sandbox is healthy + - name: Check reviewer sandbox health + id: sandbox if: steps.gate.outputs.should_review == 'true' env: EXECUTION_FILE: ${{ steps.review.outputs.execution_file }} run: | set -euo pipefail + broken() { + echo "::warning::$1" + { echo "broken=true"; echo "reason=$1"; } >> "$GITHUB_OUTPUT" + exit 0 + } # A missing transcript on a review run is itself suspicious (the SDK - # iterator can hang before writing the file), so fail rather than - # silently pass with no evidence. + # iterator can hang before writing the file), so treat it as + # unhealthy rather than silently passing with no evidence. if [ -z "${EXECUTION_FILE:-}" ] || [ ! -f "$EXECUTION_FILE" ]; then - echo "::error::No execution transcript from the reviewer (execution_file missing). Cannot confirm the sandbox was healthy." - exit 1 + broken "No execution transcript from the reviewer (execution_file missing). Cannot confirm the sandbox was healthy." fi - # Count tool_result blocks that errored with the bwrap sandbox - # signature. `content` may be a string or an array of text blocks, - # so normalize both. jq reads the file directly -- no pipe, so no + # Count errored tool_result blocks that (a) correlate to a Bash + # tool_use by id and (b) carry the bwrap signature at a line start. + # `content` may be a string or an array of text blocks, so normalize + # both. jq reads the file directly -- no pipe, so no # pipefail/SIGPIPE fail-open. - BROKEN=$(jq ' - [ .[] - | select(.type == "user") - | .message.content[]? - | select(.type == "tool_result" and .is_error == true) - | ( .content - | if type == "array" then map(.text // "") | join("\n") - elif type == "string" then . - else tostring end ) - | select(test("bwrap: Can.t create file|Can.t create file at .*\\.mcp\\.json|bwrap:.*Permission denied")) - ] | length + BROKEN_COUNT=$(jq ' + ([ .[] + | select(.type == "assistant") + | .message.content[]? + | select(.type == "tool_use" and .name == "Bash") + | .id + ]) as $bash_ids + | [ .[] + | select(.type == "user") + | .message.content[]? + | select(.type == "tool_result" and .is_error == true) + | select((.tool_use_id // "") as $id | $bash_ids | index($id)) + | ( .content + | if type == "array" then map(.text // "") | join("\n") + elif type == "string" then . + else tostring end ) + | select(test("(^|\\n)bwrap: .*(Can.t create file|Permission denied)")) + ] | length ' "$EXECUTION_FILE") - echo "bwrap sandbox-failure tool_results: $BROKEN" - if [ "$BROKEN" -gt 0 ]; then - echo "::error::Deep review ran in a broken sandbox ($BROKEN bwrap Bash failures). The CLI pin is not taking effect or the regression changed shape. See https://github.com/anthropics/claude-code-action/issues/1547" - exit 1 + echo "bwrap sandbox-failure Bash tool_results: $BROKEN_COUNT" + if [ "$BROKEN_COUNT" -gt 0 ]; then + broken "Deep review ran in a broken sandbox ($BROKEN_COUNT bwrap Bash failures). The CLI pin is not taking effect or the regression changed shape. See https://github.com/anthropics/claude-code-action/issues/1547" fi + echo "broken=false" >> "$GITHUB_OUTPUT" echo "Sandbox smoke check passed -- no bwrap failures in the reviewer transcript." # fromJSON() in `with:` has been observed to leave structured_output JSON @@ -554,12 +579,19 @@ jobs: # output a second time when the underlying skill returns a JSON-shaped # response. Detect that case (the inner string parses as an object with # a `review` key) and unwrap once more so we post markdown, not JSON. + # `always() && steps.review.outcome == 'success'` (rather than a bare + # `if:`, which is implicitly ANDed with success()) so that a failure in + # the sandbox health check above cannot discard an already-completed + # review: the review still posts, and the job is failed afterwards. - name: Extract review from structured output id: extract - if: steps.gate.outputs.should_review == 'true' + if: >- + always() && steps.gate.outputs.should_review == 'true' && + steps.review.outcome == 'success' env: STRUCTURED_OUTPUT: ${{ steps.review.outputs.structured_output }} DIFF_HASH: ${{ steps.gate.outputs.diff_hash }} + SANDBOX_BROKEN: ${{ steps.sandbox.outputs.broken }} run: | REVIEW="$(printf '%s' "$STRUCTURED_OUTPUT" | jq -r '.review')" if printf '%s' "$REVIEW" | jq -e 'type == "object" and has("review")' >/dev/null 2>&1; then @@ -567,7 +599,13 @@ jobs: fi # Prepend the hidden state marker consumed by the gate on the next # run. Keep this format in lockstep with the gate's parser (MARKER_RE). + # If the sandbox was unhealthy (or its check did not complete), omit + # the parseable marker so the gate fail-opens and the next run + # re-reviews instead of trusting a zero-coverage review. MARKER="" + if [ "${SANDBOX_BROKEN:-}" != "false" ]; then + MARKER="" + fi { echo 'review<> "$GITHUB_OUTPUT" - name: Post or update deep review - if: steps.gate.outputs.should_review == 'true' + if: >- + always() && steps.gate.outputs.should_review == 'true' && + steps.extract.outcome == 'success' uses: peter-evans/create-or-update-comment@v5 with: comment-id: ${{ steps.find-comment.outputs.comment-id }} issue-number: ${{ steps.pr.outputs.number }} body: ${{ steps.extract.outputs.review }} edit-mode: replace + + # Deferred failure for the sandbox health check: runs AFTER the review + # comment is posted so the evidence is preserved, then fails the job + # loud. Also treats a sandbox check that itself errored (e.g. jq choked + # on a reshaped transcript) as unhealthy rather than fail-open. + - name: Fail if reviewer sandbox was unhealthy + if: >- + always() && steps.gate.outputs.should_review == 'true' && + steps.review.outcome == 'success' + env: + SANDBOX_OUTCOME: ${{ steps.sandbox.outcome }} + SANDBOX_BROKEN: ${{ steps.sandbox.outputs.broken }} + SANDBOX_REASON: ${{ steps.sandbox.outputs.reason }} + run: | + set -euo pipefail + if [ "$SANDBOX_OUTCOME" != "success" ]; then + echo "::error::Sandbox health check did not complete (outcome: $SANDBOX_OUTCOME). Treating the reviewer sandbox as unhealthy." + exit 1 + fi + if [ "$SANDBOX_BROKEN" != "false" ]; then + echo "::error::${SANDBOX_REASON:-Reviewer sandbox unhealthy.}" + exit 1 + fi + echo "Reviewer sandbox healthy." From 3665e201979f03c8e4bb0f67575e29e8b9b17753 Mon Sep 17 00:00:00 2001 From: Brandon Pereira Date: Thu, 6 Aug 2026 09:19:34 -0600 Subject: [PATCH 4/4] ci(deep-review): treat unparseable or empty transcripts as unhealthy Address the deep review P0 on the sandbox health check: jq emits nothing (exit 0) on an existing-but-empty execution_file, leaving BROKEN_COUNT empty; [ "" -gt 0 ] then errors but is set-e-exempt as an if condition, so the check fell through to broken=false, stamped a valid state marker, and left the job green on a sandbox it never actually inspected -- with the gate suppressing re-review. Reject degraded transcripts up front (empty file, or anything that is not a non-empty JSON array) and case-validate that the derived count is numeric before comparing it. Verified against fixtures: empty file, null, [], and malformed JSON all now record broken=true; the five prior fixtures (real bwrap failures, quoted signature, non-Bash tool, healthy run) are unchanged. --- .github/workflows/deep-review.yml | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/.github/workflows/deep-review.yml b/.github/workflows/deep-review.yml index 78b28b3dc0..2185f3caa4 100644 --- a/.github/workflows/deep-review.yml +++ b/.github/workflows/deep-review.yml @@ -534,12 +534,21 @@ jobs: { echo "broken=true"; echo "reason=$1"; } >> "$GITHUB_OUTPUT" exit 0 } - # A missing transcript on a review run is itself suspicious (the SDK - # iterator can hang before writing the file), so treat it as - # unhealthy rather than silently passing with no evidence. + # A missing, empty, or reshaped transcript on a review run is itself + # suspicious (the SDK iterator can hang before writing the file, or + # crash after truncating it), so treat those as unhealthy rather + # than silently passing with no evidence: jq emits nothing (exit 0) + # on empty input, which would otherwise fall through to + # broken=false and stamp a valid state marker. if [ -z "${EXECUTION_FILE:-}" ] || [ ! -f "$EXECUTION_FILE" ]; then broken "No execution transcript from the reviewer (execution_file missing). Cannot confirm the sandbox was healthy." fi + if [ ! -s "$EXECUTION_FILE" ]; then + broken "Execution transcript exists but is empty. Cannot confirm the sandbox was healthy." + fi + if ! jq -e 'type == "array" and length > 0' "$EXECUTION_FILE" >/dev/null 2>&1; then + broken "Execution transcript is not a non-empty JSON array. Cannot confirm the sandbox was healthy." + fi # Count errored tool_result blocks that (a) correlate to a Bash # tool_use by id and (b) carry the bwrap signature at a line start. # `content` may be a string or an array of text blocks, so normalize @@ -564,7 +573,15 @@ jobs: | select(test("(^|\\n)bwrap: .*(Can.t create file|Permission denied)")) ] | length ' "$EXECUTION_FILE") - echo "bwrap sandbox-failure Bash tool_results: $BROKEN_COUNT" + echo "bwrap sandbox-failure Bash tool_results: ${BROKEN_COUNT:-}" + # Belt-and-suspenders: never let a non-numeric count reach the + # comparison below -- `[ "" -gt 0 ]` errors but is set-e-exempt as + # an `if` condition, so it would silently fall through to healthy. + case "$BROKEN_COUNT" in + '' | *[!0-9]*) + broken "Sandbox check could not derive a failure count from the execution transcript (got: '${BROKEN_COUNT:-}')." + ;; + esac if [ "$BROKEN_COUNT" -gt 0 ]; then broken "Deep review ran in a broken sandbox ($BROKEN_COUNT bwrap Bash failures). The CLI pin is not taking effect or the regression changed shape. See https://github.com/anthropics/claude-code-action/issues/1547" fi