Skip to content

fix: let Claude Code review passes read the run's patch files in CI - #84

Merged
brentvatne merged 1 commit into
mainfrom
@brent/claude-code-patch-read-scope
Oct 20, 2026
Merged

brentvatne merged 1 commit into
mainfrom
@brent/claude-code-patch-read-scope

Conversation

@brentvatne

Copy link
Copy Markdown
Member

In CI, review passes on the Claude Code engine (the default for adopters) could not read the patch files their prompts point to. Every read was denied.

Why

buildClaudeArgs scopes each granted read tool to the working directory, which is the PR-head tree that ecr ci materializes in a temp directory. The patch workspace is written to <runsDir>/<runId>/patches, and in CI runsDir is the base checkout's .expo-code-review/.runs, outside that tree. With --permission-mode dontAsk, reads there are denied.

What this broke:

  • a reviewer's "other files this PR changed — read their patch files on demand" context (buildReviewerTask);
  • the cross-file pass's files past its 6,000-line inline budget, which it gets as patch paths only (buildCrossCuttingTask).

Local runs were unaffected, because .runs/ sits inside the repo.

Change

  • buildClaudeArgs takes extraReadRoots. Each granted read tool gets one allow rule per root. A pass with no tools still gets no rules.
  • The Claude Code handle carries the run's patchDir. runReview passes patchDirFor(runDir), a new helper that writePatchWorkspace uses too.
  • Only patches/ is granted. The rest of .runs/, including reviews.jsonl, stays unreadable.
  • LLP 0003 and LLP 0004 are updated, along with the writePatchWorkspace comment, which wrongly said the files were written inside the repo.

Verification

Live, against the pinned @anthropic-ai/claude-code@2.1.280, with the arguments buildClaudeArgs generates and a CI-shaped layout (a PR-head tree in one temp directory, .runs/ under a separate workspace):

Read target Before After
File inside the PR-head tree allowed allowed
.runs/<id>/patches/*.patch denied allowed
.runs/reviews.jsonl denied denied

Unit tests cover the new allow rules, the absence of a wider .runs/ rule, the no-tools case, and the handle's patch directory reaching the CLI's arguments. bun test, typecheck, fmt:check and ./ref-check pass.

Not covered: the OpenCode engine sets no read permissions of its own, so whether it can read outside the tree is still unchecked.

🤖 Generated with Claude Code

The Claude Code engine scopes every read tool to the PR-head tree, but in CI the
patch workspace is written under the base checkout's `.runs/`, outside that tree.
Every patch read the prompts ask for was denied: a reviewer's context-only files
and the cross-file pass's files past its inline budget. The run's `patches/`
directory is now a second read scope; the rest of `.runs/` stays unreadable.
@github-actions

Copy link
Copy Markdown

🤖 AI code review

Decision: Ready for human review

Overall PR risk: Low. This change modifies buildClaudeArgs to grant Claude Code read access to the patch directory in CI environments. The change is scoped precisely: only patches/ receives a new allow rule, while .runs/reviews.jsonl and other sensitive files remain unreadable. Patch files contain only ecr-written content with sanitized filenames. Passes with no read tools receive no additional rules. The blast radius is limited to Claude Code review passes that use read tools; revert is a single parameter change.

🔗 Review setup:

  • This PR changes code the reviewer prompts cite (src, src/tests). Check that the guidance quoting it is still correct.

No findings.


This review is advisory — it never blocks a merge and never auto-approves.

@brentvatne
brentvatne merged commit 13caa59 into main Oct 20, 2026
3 checks passed
@brentvatne
brentvatne deleted the @brent/claude-code-patch-read-scope branch October 20, 2026 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant