Skip to content

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

Merged
kodiakhq[bot] merged 7 commits into
mainfrom
brandon/fix-deep-code-review
Aug 7, 2026
Merged

ci(deep-review): pin claude-code CLI to dodge bwrap sandbox regression#2823
kodiakhq[bot] merged 7 commits into
mainfrom
brandon/fix-deep-code-review

Conversation

@brandon-pereira

@brandon-pereirabrandon-pereira commented Aug 5, 2026

Copy link
Copy Markdown
Member

Why

The Deep Review action can post "environment failure" comments (zero-coverage reviews) while the job reports success.

Root cause is an upstream regression in the Claude Code CLI that anthropics/claude-code-action@v1 bundles and auto-bumps (~daily): anthropics/claude-code-action#1547. CLI >= 2.1.216 breaks the bwrap sandbox — whenever subprocess isolation is enabled (which our allowed_non_write_users: '*' auto-enables via CLAUDE_CODE_SUBPROCESS_ENV_SCRUB=1), bwrap aborts every Bash call while trying to mask a repo-root .mcp.json:

bwrap: Can't create file at /home/.mcp.json: Permission denied

That kills git/gh and the reviewer sub-agent fan-out, so no diff is ever obtained. 2.1.215 is the last known-good version; the regression is not fixed forward through the current 2.1.220.

What changed

deep-review.yml:

  1. Pin the CLI to 2.1.215 (temporary). The action hardcodes its CLI version with no version input, so the only supported override is path_to_claude_code_executable. A new step fetches the immutable npm tarball for the pinned version, verifies it against a SHA-512 hardcoded in the workflow (the npm dist.integrity of @anthropic-ai/claude-code@2.1.215), and installs from the verified local tarball with --ignore-scripts — the only lifecycle script executed is install.cjs from the hash-verified tarball, run explicitly to link the platform-native binary (itself an optionalDependency exact-pinned by the verified package.json). Trust rests on the hash in the workflow, not on a mutable remote installer or the binary's spoofable --version output (kept as a sanity check only). Version + hash are single-sourced in env vars so they cannot drift. Unpin once upstream fixes the regression (tracked in HDX-4907).
  2. Add a sandbox smoke check (permanent). A new step fails the job loud if the machine-readable execution transcript contains bwrap sandbox-failure tool results, so a broken sandbox can never again post a zero-coverage review under a green checkmark. Detection reads the execution_file transcript (tool_result blocks with is_error: true carrying the bwrap signature) rather than the free-text review body, which would false-positive on any PR that merely discusses the error.

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: anthropics/claude-code-action#1547
@vercel

vercelBot commented Aug 5, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
ProjectDeploymentActionsUpdated (UTC)
hyperdx-ossIgnoredIgnoredPreviewAug 7, 2026 6:04pm
hyperdx-storybookIgnoredIgnoredPreviewAug 7, 2026 6:04pm

Request Review

@changeset-bot

changeset-botBot commented Aug 5, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 865e497

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions

Copy link
Copy Markdown
Contributor

🟢 Tier 1 — Trivial

Docs, images, lock files, a dependency bump, or an automated release. No functional code changes detected.

Why this tier:

  • All files are docs / images / lock files

Review process: Auto-merge once CI passes. No human review required.
SLA: Resolves automatically.

Stats
  • Production files changed: 0
  • Production lines changed: 0
  • Branch: brandon/fix-deep-code-review
  • Author: brandon-pereira

To override this classification, remove the review/tier-1 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@github-actionsgithub-actionsBot added the review/tier-1 Trivial — auto-merge candidate once CI passes label Aug 5, 2026
@greptile-apps

greptile-appsBot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR pins Deep Review to Claude Code CLI 2.1.215 and adds transcript-based sandbox health detection.

  • Downloads and verifies the pinned top-level npm tarball before installation.
  • Routes the pinned executable into the review action.
  • Detects bwrap failures, omits healthy-state markers for unhealthy reviews, and fails the workflow after preserving the review comment.

Confidence Score: 4/5

The PR should not merge until the platform-native package used by the pinned CLI is independently authenticated.

The mutable installer issue is fixed by verifying the top-level tarball, but npm still resolves the platform-native optional dependency without a lockfile or pinned integrity value and the resulting executable receives the workflow's Anthropic and GitHub credentials.

Files Needing Attention: .github/workflows/deep-review.yml

Important Files Changed

FilenameOverview
.github/workflows/deep-review.ymlPins the Claude CLI, verifies the top-level package, and adds fail-closed sandbox transcript checks while preserving completed review comments.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Download pinned CLI tarball] --> B[Verify SHA-512]
B --> C[npm install dependencies]
C --> D[Run install.cjs]
D --> E[Run credentialed deep review]
E --> F[Inspect execution transcript]
F -->|Healthy| G[Post review with state marker]
F -->|Unhealthy| H[Post review without state marker]
H --> I[Fail workflow]
Loading

Reviews (7): Last reviewed commit: "Merge branch 'main' into brandon/fix-dee..." | Re-trigger Greptile

Comment thread.github/workflows/deep-review.yml Outdated
…ing 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-actions

github-actionsBot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

E2E Test Results

All tests passed • 277 passed • 1 skipped • 1125s

StatusCount
✅ Passed277
❌ Failed0
⚠️ Flaky0
⏭️ Skipped1

Tests ran across 4 shards in parallel.

View full report →

@github-actions

github-actionsBot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Deep Review

✅ No critical issues found. The CLI pin + sandbox guard are net improvements over the base workflow, and the happy path (pin to known-good 2.1.215 → healthy sandbox → state marker stamped → review posted → green job) is correct. The items below harden new defensive code and correct an overstated claim.

🟡 P2 -- recommended

  • .github/workflows/deep-review.yml:573 -- The sandbox-health jq matches only the two literal strings Can't create file/Permission denied, so a reshaped bwrap abort such as a userns Operation not permitted yields a zero count, reports the sandbox healthy, stamps a valid state marker, and lets the next run gate-skip — re-manifesting the green-checkmark-on-broken-sandbox failure the guard exists to prevent.
    • Fix: Match any errored Bash tool_result whose content has a line beginning with bwrap:, and treat zero bwrap-correlated Bash results on a review run as unhealthy.
    • correctness, adversarial, reliability
  • .github/workflows/deep-review.yml:288 -- The pinned-CLI curl fetch and the following npm install have no retry or timeout, so a transient npm-registry blip hard-fails the pin step and the review posts no comment at all.
    • Fix: Add --retry, --connect-timeout, and --max-time to the curl and wrap the npm install in a bounded retry.
  • .github/workflows/deep-review.yml:557 -- The intricate bwrap-detection jq has no fixture coverage; its only exercise is a live PR, so SDK-transcript schema drift or a logic bug fails open undetected.
    • Fix: Commit sample execution_file transcripts and assert the jq output in CI (real bwrap failure → count > 0; clean run and a run that merely quotes the error → count 0).
    • testing, correctness, adversarial
🔵 P3 nitpicks (4)
  • .github/workflows/deep-review.yml:305 -- CLAUDE_CLI_SHA512 verifies only the wrapper npm package; the binary the credentialed action actually executes is a platform optionalDependency fetched separately and verified only by npm registry integrity, so the comment's "trust rests on the hash in this file" overstates the guarantee (posture is still no worse than baseline).
    • Fix: Scope the comment's claim to the wrapper package, or anchor the native dependency's integrity via a committed lockfile with npm ci.
    • security, adversarial, reliability
  • .github/workflows/deep-review.yml:529 -- If execution_file is not the action's actual output name, or is unpopulated in some modes, EXECUTION_FILE is always empty and every healthy review fails closed — a red job plus a forced re-review on every run.
    • Fix: Confirm the output name against anthropics/claude-code-action@v1's action.yml.
  • .github/workflows/deep-review.yml:284 -- The steps.gate.outputs.should_review == 'true' guard is now repeated across seven steps (up from two); a future gating change must touch every copy or silently regress one step.
    • Fix: Gate once at the job level or via a single derived condition to shrink the duplication surface.
  • .github/workflows/deep-review.yml:264 -- The temporary-pin rationale and unpin instructions are duplicated across three comment blocks (env, step header, and the path_to_claude_code_executable inline note), so an unpin PR can easily leave a stale block behind.
    • Fix: Consolidate the unpin instructions to one location and reference it from the others.

Reviewers (7): correctness, security, adversarial, reliability, testing, maintainability, project-standards.

Testing gaps:

  • The unhealthy-sandbox control-flow branches (marker omission, deferred job failure, always() gating) are exercised only by a live PR hitting the real regression.
  • No pre-merge check that CLAUDE_CLI_SHA512 matches the pinned CLAUDE_CLI_VERSION, and no test that a tampered tarball aborts the pin step.

…g 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.
jordan-simonovski
jordan-simonovski previously approved these changes Aug 5, 2026
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.
Comment thread.github/workflows/deep-review.yml
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automergereview/tier-1Trivial — auto-merge candidate once CI passes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@brandon-pereira@wrn14897@jordan-simonovski