From 8fe29fdaddaf015aa8a41e25bcc7f3c9ae060f65 Mon Sep 17 00:00:00 2001 From: Marcelo Gobetti Date: Wed, 1 Jul 2026 21:21:28 -0300 Subject: [PATCH] Stop wasting review turns retrying tools the sandbox denies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude Code exposes its full standard toolset to the model regardless of --allowedTools scoping; an allow-list only gates execution at call time, it doesn't hide the tool from the model's context (that requires a bare deny rule, which we don't use here). So the model has no way to know in advance which of its visible tools will actually work in this sandboxed review — it only finds out by attempting the call and eating a permission denial, one turn at a time. That was cheap and mostly invisible on Sonnet 4.6. Now that the org default is Sonnet 5, reviews retry disallowed tools noticeably more (build/lint/test attempts, LS, a WebFetch for the org CLAUDE.md link), and on larger diffs the wasted turns tip the review past max_turns entirely — ht-android hit three straight max-turns failures on one PR today, while a same-day, same-config sibling PR with a slightly smaller diff passed with turns to spare. Fix, without touching max_turns: - Tell the prompt directly which tools are actually available, so the model stops discovering the boundary by trial and error. - Add LS to the allow-list — already granted one step later, in the sticky-comment update. - Fold the Mobile and Infrastructure rubric sections — previously reachable only via a WebFetch to the org CLAUDE.md — directly into the prompt, since the rest of that file was already duplicated here. Removes the WebFetch attempt instead of just tolerating its denial. --- .github/workflows/claude-review.yml | 34 ++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index df3d237..f0174eb 100644 --- a/.github/workflows/claude-review.yml +++ b/.github/workflows/claude-review.yml @@ -59,12 +59,19 @@ jobs: prompt: | /review - IMPORTANT: Also reference and apply the organization-wide code review guidelines available at: - https://github.com/isapp/.github/blob/main/CLAUDE.md + ## Environment constraints - These org-level guidelines cover security (OWASP, SQL injection, auth), compliance (HIPAA, SOC2, FedRAMP), - data handling, infrastructure, performance, testing, APIs, and dependencies. - Apply these standards IN ADDITION to any repository-specific CLAUDE.md guidelines. + This review runs with a restricted toolset: only Read, Glob, Grep, LS, and + `git log`/`git diff`/`git show` are available. There is no ability to run + builds, linters, or test suites, install dependencies, or reach the network. + Do not attempt repo setup, linting, or test commands, and do not retry a + denied tool call — go straight to a static review of the diff with the + allowed tools. If something genuinely requires a disallowed tool, note that + once in the final comment instead of retrying. + + The review rubric below already incorporates the organization-wide code + review guidelines. Apply it IN ADDITION to any repository-specific + CLAUDE.md guidelines. # Review rubric (Claude) @@ -92,6 +99,21 @@ jobs: - Data validation: constraints at database level, not just application level - PHI/PII: ensure proper encryption, audit trails for sensitive data access + ### Mobile (iOS/Android) + - UI thread safety: no network/database calls on main thread + - Background work: proper WorkManager (Android) or BackgroundTask (iOS) usage + - Permissions: clear rationale strings, runtime permission checks + - Network: timeouts, retries with exponential backoff, offline handling + - Security: certificate pinning, secure storage (Keychain/KeyStore), code obfuscation + - Accessibility: VoiceOver/TalkBack support, proper content descriptions + + ### Infrastructure (Terraform/IaC/Docker) + - IaC: Terraform state encryption, no hardcoded credentials, least-privilege IAM policies + - Container security: non-root users, minimal base images, scan for vulnerabilities + - Secrets: use AWS Secrets Manager/SSM Parameter Store, never commit secrets + - Network: proper security groups, VPC configuration, no public exposure of sensitive resources + - Monitoring: CloudWatch logs and alarms for critical resources + ## Should-fix ### Performance @@ -151,5 +173,5 @@ jobs: 5. Provide detailed feedback on code review ${{ inputs.extra_prompt }} - claude_args: "--model ${{ inputs.model }} --max-turns ${{ inputs.max_turns }} --allowedTools 'Read,Glob,Grep,Bash(git log:*),Bash(git diff:*),Bash(git show:*)'" + claude_args: "--model ${{ inputs.model }} --max-turns ${{ inputs.max_turns }} --allowedTools 'Read,Glob,Grep,LS,Bash(git log:*),Bash(git diff:*),Bash(git show:*)'" track_progress: true