ci(deep-review): pin claude-code CLI to dodge bwrap sandbox regression

.github/workflows/deep-review.yml

@@ -48,6 +48,18 @@ 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.
+      # The SHA-512 is the npm dist.integrity of the
+      # @anthropic-ai/claude-code@<version> 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@<version> 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
@@ -249,13 +261,76 @@ 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. 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
+          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
+          # 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:-<none>}"
+          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:-<none>}'."
+            exit 1
+          fi
+          echo "PINNED_CLAUDE=$CLAUDE_BIN" >> "$GITHUB_ENV"
+
       - name: Run deep review
         id: review
         if: steps.gate.outputs.should_review == 'true'
         uses: anthropics/claude-code-action@v1
         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 +494,100 @@ 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 <!-- deep-review --> 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`.
+      #
+      # 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: 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, 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
+          # both. jq reads the file directly -- no pipe, so no
+          # pipefail/SIGPIPE fail-open.
+          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 Bash tool_results: ${BROKEN_COUNT:-<empty>}"
+          # 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:-<empty>}')."
+              ;;
+          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
+          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
       # unparsed for the sibling claude-code-review workflow. Extract via jq.
       #
@@ -427,20 +596,33 @@ 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
             REVIEW="$(printf '%s' "$REVIEW" | jq -r '.review')"
           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="<!-- deep-review-state: diff=${DIFF_HASH}; plugin=${PLUGIN_REF} -->"
+          if [ "${SANDBOX_BROKEN:-}" != "false" ]; then
+            MARKER="<!-- deep-review-state: omitted (reviewer sandbox unhealthy) -->"
+          fi
           {
             echo 'review<<DEEP_REVIEW_EOF'
             printf '%s\n' "$MARKER"
@@ -450,10 +632,36 @@ jobs:
           } >> "$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."