feat: consolidate 4 AI review workflows into 1 - #114
Merged
Conversation
Merge ai-review.yml, claude-review-phase1/2/3.yml into a single ai-code-review.yml with 3 jobs: - preflight: gate (bot-actor, draft, size, label checks) - review: comment-only review + conditional PAL MCP consensus - autofix: opt-in via ai-patch label, creates draft PR Key changes: - Default behavior: comment-only review (was 4 concurrent reviews) - Consensus: opt-in via ai-consensus label or high-stakes files - Autofix: opt-in via ai-patch label, blocked for forks/protected files - All actions SHA-pinned, tool syntax normalized to colon form - Cost monitor updated to reference consolidated workflow - Recursive trigger safety: GITHUB_TOKEN only, never PAT Saves $4.50-13.50 per PR by eliminating duplicate reviews. Addresses: DreamServer PR #683 review items #1, #2, #3 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reviewer's GuideConsolidates the existing multi-phase Claude AI review workflows into a single SHA-pinned Sequence diagram for AI review, consensus, and autofix on a pull requestsequenceDiagram
actor Developer
participant GitHub
participant Preflight as Job_preflight
participant Review as Job_review
participant Autofix as Job_autofix
participant Bedrock as AWS_Bedrock
participant Anthropic as Anthropic_API
participant PAL as PAL_MCP
Developer->>GitHub: Open or update PR
GitHub->>Preflight: Trigger ai-code-review.yml
Preflight->>Preflight: Inspect actor, draft status, labels
Preflight->>Preflight: Compute diff size and changed files
Preflight->>GitHub: Set outputs should_review / should_consensus / should_autofix
alt should_review == true
GitHub->>Review: Start review job
Review->>Review: Validate providers (Bedrock / Anthropic)
alt Bedrock configured
Review->>Bedrock: Run claude-code-action review
Bedrock-->>Review: review-results.md
Review->>GitHub: Post PR comment from review-results.md
else Bedrock failed or not configured
alt Anthropic configured
Review->>Anthropic: Run claude-code-action review
Anthropic-->>Review: review-results.md
Review->>GitHub: Post PR comment from review-results.md
else No providers configured
Review->>Review: Skip review
end
end
alt should_consensus == true
Review->>PAL: POST consensus request (diff + metadata)
PAL-->>Review: Synthesized recommendation
Review->>GitHub: Post consensus PR comment
else should_consensus == false
Review-->>GitHub: No consensus requested
end
else should_review == false
Preflight-->>GitHub: Review job not started
end
alt should_autofix == true
GitHub->>Autofix: Start autofix job
Autofix->>Autofix: Check fork status and security gates
alt safe == true
alt Bedrock configured
Autofix->>Bedrock: Run claude-code-action edits
Bedrock-->>Autofix: Code changes
else Bedrock failed or not configured
alt Anthropic configured
Autofix->>Anthropic: Run claude-code-action edits
Anthropic-->>Autofix: Code changes
else No providers configured
Autofix->>Autofix: Skip autofix
end
end
Autofix->>Autofix: Validate diff size and protected paths
alt has_changes and passes validation
Autofix->>GitHub: Create draft PR via create-pull-request
Autofix->>GitHub: Comment on original PR with link to draft
else No valid changes
Autofix-->>GitHub: No draft PR created
end
else safe == false (fork PR)
Autofix-->>GitHub: Autofix blocked for forks
end
else should_autofix == false
Preflight-->>GitHub: Autofix job not started
end
GitHub-->>Developer: Show review comments and optional draft PR
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughConsolidates three phase-specific AI review workflows into a single Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub (PR Event)
participant PF as Preflight Job
participant RV as Review Job
participant CS as Consensus Job
participant AF as Autofix Job
participant API as Claude Provider (Bedrock/Anthropic)
participant PM as PAL MCP (Optional)
GH->>PF: Trigger on PR or issue comment / labels
PF->>PF: Evaluate gates (actor, skip/force labels, size, protected paths, enable consensus/autofix)
alt Preflight Fails
PF-->>GH: Skip workflow or post notice
else Preflight Passes
PF->>RV: Start review
RV->>API: Request review (Bedrock primary)
alt Bedrock succeeds
API-->>RV: Review results
else
RV->>API: Fallback to Anthropic
API-->>RV: Review results
end
RV->>GH: Post review comment & summary
alt Consensus enabled
PF->>CS: Run consensus
CS->>PM: Call PAL MCP (if configured)
alt PAL MCP responds
PM-->>CS: Recommendation
CS->>GH: Post consensus comment
else
CS->>GH: Post "PAL MCP not configured" notice
end
end
alt Autofix enabled
PF->>AF: Allow autofix (block forks/protected paths)
AF->>API: Request patch (Bedrock primary, fallback Anthropic)
API-->>AF: Patch
AF->>AF: Validate patch (non-empty, <500 lines, no protected paths, no secrets)
alt Validation passes
AF->>GH: Create draft PR (ai-fix/*) and comment link
else
AF->>GH: Post rejection reason
end
AF->>GH: Append summary
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The diff calculations in the review and consensus jobs assume
origin/${BASE_REF}exists (e.g.git diff --stat "origin/${BASE_REF}...HEAD"), which may fail on fork PRs or non-maindefault branches—consider usinggithub.event.pull_request.base.sha/head.shaor explicitly fetching the base ref instead. - The protected-file patterns for autofix use
.envbut the docs and README mention.env*; to avoid gaps in protection, consider aligning the patterns (e.g..envvs.env*) between the workflow logic and the documented guarantees.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The diff calculations in the review and consensus jobs assume `origin/${BASE_REF}` exists (e.g. `git diff --stat "origin/${BASE_REF}...HEAD"`), which may fail on fork PRs or non-`main` default branches—consider using `github.event.pull_request.base.sha`/`head.sha` or explicitly fetching the base ref instead.
- The protected-file patterns for autofix use `.env` but the docs and README mention `.env*`; to avoid gaps in protection, consider aligning the patterns (e.g. `.env` vs `.env*`) between the workflow logic and the documented guarantees.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
AI Code Review — PR #114OverviewThis PR consolidates 4 separate AI review workflows ( Critical Issues1. PAL MCP Default Endpoint is a Placeholder (Line 464) PAL_MCP_ENDPOINT: ${{ secrets.PAL_MCP_ENDPOINT || 'https://pal-mcp.example.com/api/tools/consensus' }}
2. Unsafe jq Response Parsing (Lines 492-495) RECOMMENDATION=$(echo "$RESPONSE" | jq -r '.synthesized_recommendation // "Error: No recommendation"')
if ! echo "$RESPONSE" | jq -e '.synthesized_recommendation' >/dev/null 2>&1; then
echo "::error::Invalid PAL MCP response"
exit 1
fiHigh Priority3. Shell Injection Risk in Git Diff Commands (Lines 98, 228) LINES_CHANGED=$(git diff --stat "origin/${BASE_REF}...HEAD" 2>/dev/null | tail -1 | awk '{print $4+$6}')
4. Weak Secret Detection Pattern (Line 691) if git diff | grep -iE "(api[_-]?key|password|secret|token)\s*[:=]" > /dev/null 2>&1; then
5. Missing Error Handling for Git Commands
if ! git diff --name-only "origin/${BASE_REF}...HEAD" >/dev/null 2>&1; then
echo "::error::Failed to compute diff"
exit 1
fi6. Consensus Model Names May Need Verification (Lines 478-480) {model: "gpt-5-mini", stance: "for"},
{model: "claude-haiku-4.5", stance: "against"},
{model: "gemini-2-flash", stance: "neutral"}
Medium Priority7. Inconsistent Continue-on-Error Strategy
8. Protected File Pattern Duplication
9. Diff Size Limit (Line 668) if [[ "${DIFF_LINES:-0}" -gt 500 ]]; then
10. Cost Estimates in Documentation May Be Outdated
Positive ObservationsExcellent Consolidation Design
Strong Security Posture
Clean Architecture
Developer Experience
Cost Efficiency
Review Summary
Recommendations
AI Code Review (AWS Bedrock). Advisory only — use human judgment. |
Contributor
GitNexus Impact Analysis⚪ NONE Overall Risk Level
Per-File Impact
Affected ProcessesAffected ModulesDetailed Impact by FileGenerated by GitNexus impact analysis |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
.github/workflows/CLAUDE_REVIEW_SETUP.md (3)
71-85:⚠️ Potential issue | 🟡 MinorStale secret reference:
CLAUDE_CODE_OAUTH_TOKENis not used by the new workflow.Lines 72-74 reference
CLAUDE_CODE_OAUTH_TOKEN, but the newai-code-review.ymlworkflow usesANTHROPIC_API_KEYorAWS_BEARER_TOKEN_BEDROCK. The secrets guidance at lines 130-142 is correct, but this section creates confusion.Update Phase 1 requirements to match the new workflow:
#### Phase 1 (Required): -``` -CLAUDE_CODE_OAUTH_TOKEN=<your-oauth-token-from-claude-console> -``` +At least one Claude API provider (see Step 2 below).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/CLAUDE_REVIEW_SETUP.md around lines 71 - 85, Update Phase 1 in the doc to remove the stale CLAUDE_CODE_OAUTH_TOKEN reference and instead instruct users to provide at least one Claude API provider; specifically, replace the CLAUDE_CODE_OAUTH_TOKEN lines with a single line like "At least one Claude API provider (see Step 2 below)." Also ensure the surrounding text aligns with the current ai-code-review.yml expectations (which use ANTHROPIC_API_KEY or AWS_BEARER_TOKEN_BEDROCK) so readers aren't confused about which secrets to supply.
370-378:⚠️ Potential issue | 🟡 MinorTroubleshooting section references obsolete token.
The troubleshooting section still mentions
CLAUDE_CODE_OAUTH_TOKENwhich is not used by the consolidated workflow. This should be updated to referenceANTHROPIC_API_KEYorAWS_BEARER_TOKEN_BEDROCK.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/CLAUDE_REVIEW_SETUP.md around lines 370 - 378, Update the troubleshooting section that currently references CLAUDE_CODE_OAUTH_TOKEN to reflect the consolidated workflow by replacing that token name and its instructions with the correct secrets: ANTHROPIC_API_KEY (for Anthropic) or AWS_BEARER_TOKEN_BEDROCK (for Bedrock). Locate any mentions of CLAUDE_CODE_OAUTH_TOKEN in the doc and change the copy, example gh secret command, and any token prefix notes to the appropriate key name(s) (ANTRHOPIC_API_KEY / AWS_BEARER_TOKEN_BEDROCK) and adjust validation details accordingly so the guidance matches the active workflow.
1-4:⚠️ Potential issue | 🟡 MinorDocumentation inconsistency: Still references "three-phase AI code review system."
Line 3 mentions "three-phase AI code review system," but the PR consolidates everything into a single workflow. The phase terminology is now misleading since users interact with labels (
ai-consensus,ai-patch) rather than selecting phases.Consider updating the intro to reflect the consolidated architecture:
-Complete setup instructions for the three-phase AI code review system. +Complete setup instructions for the consolidated AI code review system.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/CLAUDE_REVIEW_SETUP.md around lines 1 - 4, Update the intro line that currently reads "three-phase AI code review system" to reflect the consolidated single workflow architecture; replace that phrase with wording that explains reviews are driven by repository labels (ai-consensus, ai-patch) rather than separate phases, and ensure the intro briefly states users interact via labels and automation rather than selecting phases so the documentation matches the PR changes..github/workflows/setup-claude-review.sh (2)
115-136:⚠️ Potential issue | 🟠 Major
configure_secrets()sets unusedCLAUDE_CODE_OAUTH_TOKEN.The script prompts for and sets
CLAUDE_CODE_OAUTH_TOKEN, but the newai-code-review.ymlworkflow usesANTHROPIC_API_KEYorAWS_BEARER_TOKEN_BEDROCK. Users following this script will configure the wrong secret.Update to match the new workflow's requirements:
-# Claude OAuth Token (required for all phases) -if ! echo "$SECRET_LIST" | grep -q "CLAUDE_CODE_OAUTH_TOKEN"; then - warning "CLAUDE_CODE_OAUTH_TOKEN not found" +# Claude API provider (required - at least one) +if ! echo "$SECRET_LIST" | grep -qE "(ANTHROPIC_API_KEY|AWS_BEARER_TOKEN_BEDROCK)"; then + warning "No Claude API provider configured" echo "" - echo "📋 Steps to get Claude OAuth token:" - echo " 1. Visit https://claude.com/console" - ... + echo "📋 Configure at least one provider:" + echo " Option A: ANTHROPIC_API_KEY (from Anthropic Console)" + echo " Option B: AWS_BEARER_TOKEN_BEDROCK + AWS_REGION"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/setup-claude-review.sh around lines 115 - 136, The script currently prompts for and sets CLAUDE_CODE_OAUTH_TOKEN (seen in the CLAUDE_CODE_OAUTH_TOKEN check and gh secret set call), but the new ai-code-review.yml workflow expects ANTHROPIC_API_KEY or AWS_BEARER_TOKEN_BEDROCK; update configure_secrets to stop creating the unused CLAUDE secret and instead prompt for and store the correct secret(s): ask for ANTHROPIC_API_KEY (and/or AWS_BEARER_TOKEN_BEDROCK if applicable), validate non-empty input, and call gh secret set ANTHROPIC_API_KEY (or gh secret set AWS_BEARER_TOKEN_BEDROCK) with the provided values; ensure messages and success/warning strings reference the new secret names so users configure the secrets the workflow actually uses.
71-105:⚠️ Potential issue | 🟡 Minor
select_phase()is now misleading; phases no longer exist.The function asks users to select phases 1-4, but the consolidated workflow doesn't use phase-based files. Instead, features are controlled by labels (
ai-consensus,ai-patch). This creates confusion.Consider either:
- Removing phase selection entirely, or
- Repurposing it to ask which features to enable (labels)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/setup-claude-review.sh around lines 71 - 105, The select_phase() function is misleading because phases no longer exist; replace it with a feature-selection prompt that asks which labels/features to enable (e.g., "Enable ai-consensus? [y/N]" and "Enable ai-patch? [y/N]") instead of choices 1–4, and set explicit variables (e.g., ENABLE_CONSENSUS, ENABLE_PATCH or LABELS array) used by the rest of the script; update references to PHASE and PHASE_NAME to use these new flags, or if you prefer removing interactivity, delete select_phase() and any usage of PHASE/PHASE_NAME and drive behavior purely from labels (ai-consensus, ai-patch) or env vars so the workflow no longer asks for non-existent phases..github/workflows/ci.yml (1)
68-78:⚠️ Potential issue | 🟠 Major
claude-reviewjob still enabled; will duplicate AI reviews withai-code-review.yml.The comment on line 72 acknowledges that
ai-code-review.ymlhandles "full PAL MCP review," but this job still runs and posts its own review comments. This creates duplicate reviews on every PR.Consider either:
- Removing this job entirely (since
ai-code-review.ymlis the consolidated workflow), or- Adding a label check like
!contains(github.event.pull_request.labels.*.name, 'ai-reviewed')to skip when the consolidated workflow will handle it🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 68 - 78, The claude-review job is still enabled and duplicates AI reviews from the consolidated ai-code-review.yml workflow; either remove the claude-review job definition entirely or update its if condition (the existing if: check on the claude-review job) to skip when the consolidated workflow will run by adding a label guard such as checking that the pull request does NOT contain an 'ai-reviewed' label (e.g., augment the contains(...) condition to include !contains(github.event.pull_request.labels.*.name, 'ai-reviewed')) so only one AI review runs per PR..github/workflows/DEPLOYMENT_STATUS.md (1)
214-223:⚠️ Potential issue | 🟡 MinorTroubleshooting references obsolete workflow filename.
Line 214 references
claude-review.ymlwhich no longer exists. Should beai-code-review.yml:**Check**: -1. Workflow file exists: `ls -la .github/workflows/claude-review.yml` +1. Workflow file exists: `ls -la .github/workflows/ai-code-review.yml` 2. YAML syntax: `yamllint .github/workflows/*.yml` ... **Debug**: ```bash -gh run list --workflow=claude-review.yml --limit 5 +gh run list --workflow=ai-code-review.yml --limit 5🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/DEPLOYMENT_STATUS.md around lines 214 - 223, Update the obsolete workflow name referenced in DEPLOYMENT_STATUS.md: replace occurrences of "claude-review.yml" with the current workflow filename "ai-code-review.yml" (e.g., in the gh command examples like the `gh run list --workflow=claude-review.yml --limit 5` and any related `gh run view` debug snippets). Ensure all examples and debug commands that reference the workflow (the string "claude-review.yml" and the `gh run list` invocation) are updated to use "ai-code-review.yml" so the documented troubleshooting commands match the actual workflow name.
🧹 Nitpick comments (4)
.github/workflows/README_CLAUDE_REVIEW.md (1)
41-50: Add language specifier to code block for better rendering.Per markdown best practices (and the static analysis hint), the code block should specify a language:
-``` +```text .github/workflows/ ├── ai-code-review.yml # Consolidated review workflow🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/README_CLAUDE_REVIEW.md around lines 41 - 50, Update the fenced code block in README_CLAUDE_REVIEW.md that shows the .github/workflows tree by adding a language specifier (use "text") to the opening fence (i.e., change ``` to ```text) so the snippet renders correctly; locate the tree snippet in the file and replace the opening backticks accordingly..github/workflows/ai-review-cost-monitor.yml (1)
53-78: Shell variable quoting could be improved, but low risk here.Static analysis flags unquoted variables in arithmetic operations (e.g.,
$AI_REVIEW_RUNS * $AI_REVIEW_COST). While technically correct per shellcheck, these variables contain numbers from controlled sources (ghoutput or hardcoded values), so the risk is minimal.For robustness, consider quoting:
-DAILY_AI_REVIEW=$(echo "$AI_REVIEW_RUNS * $AI_REVIEW_COST" | bc) +DAILY_AI_REVIEW=$(echo "${AI_REVIEW_RUNS:-0} * $AI_REVIEW_COST" | bc)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ai-review-cost-monitor.yml around lines 53 - 78, The arithmetic expressions use unquoted shell variables (e.g., AI_REVIEW_RUNS, AI_REVIEW_COST, NIGHTLY_RUNS, etc.) when passed to bc; update each calculation for DAILY_AI_REVIEW, DAILY_NIGHTLY, DAILY_CI, DAILY_TRIAGE, DAILY_DOCS, DAILY_RELEASE and DAILY_TOTAL to quote the variables (e.g., use "$AI_REVIEW_RUNS" and "$AI_REVIEW_COST") so the values from the GH outputs/hardcoded costs are safely expanded; keep the same bc invocations but ensure every variable reference in those expressions is wrapped in double quotes to avoid word-splitting or empty-value issues..github/workflows/ai-code-review.yml (2)
116-133: Pattern matching uses substring match, which may over-trigger.Using
grep -q "$pattern"performs substring matching. For example, pattern.envwill match.environment,test.env.backup, etc. Patternscripts/will matchmyscripts/orother/scripts/.For security-related gates (high-stakes detection), over-matching is generally safer. However, if precise matching is desired:
-if echo "$CHANGED_FILES" | grep -q "$pattern"; then +if echo "$CHANGED_FILES" | grep -qE "(^|/)${pattern}"; thenThis is a minor concern since over-matching errs on the side of caution.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ai-code-review.yml around lines 116 - 133, The HIGH_STAKES_PATTERNS substring matching can over-trigger; update the loop that inspects HIGH_STAKES_PATTERNS and CHANGED_FILES so directory patterns (those ending with "/") are matched only as path prefixes (use grep -qE "^$pattern") and file patterns are matched as exact filenames (use grep -q -xF "$pattern"), preserving the existing SHOULD_CONSENSUS assignment and notice echo; modify the for-loop logic that iterates HIGH_STAKES_PATTERNS and the conditional that currently uses grep -q "$pattern" to branch on whether the pattern ends with "/" and apply the appropriate grep invocation instead.
96-104:awkparsing of git diff stat output is fragile.The command
git diff --stat | tail -1 | awk '{print $4+$6}'assumes a specific format where insertions are in column 4 and deletions in column 6. This format varies across git versions and locales. If the format differs (e.g., "1 file changed, 5 insertions(+)"), the calculation may yield incorrect values or 0.Consider using
git diff --shortstatwhich has a more predictable format, or use--numstatand sum the values:-LINES_CHANGED=$(git diff --stat "origin/${BASE}...HEAD" 2>/dev/null | tail -1 | awk '{print $4+$6}') +LINES_CHANGED=$(git diff --numstat "origin/${BASE}...HEAD" 2>/dev/null | awk '{s+=$1+$2} END {print s+0}')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ai-code-review.yml around lines 96 - 104, The LINES_CHANGED calculation using git diff --stat and awk is fragile; update the block that sets LINES_CHANGED (the shell code referencing BASE, SHOULD_REVIEW and HAS_FORCE_LABEL) to use a more robust command such as git diff --shortstat or git diff --numstat and reliably sum insertions and deletions instead of parsing fixed columns; ensure you parse/sum the numeric fields (or fallback to 0) and preserve the existing logic that disables review when LINES_CHANGED > 1000 unless HAS_FORCE_LABEL == "true".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ai-code-review.yml:
- Around line 487-495: The curl call that populates RESPONSE needs timeouts and
HTTP-error handling: call curl with connection and total timeouts (e.g.,
--connect-timeout and --max-time) and use --fail or capture the HTTP status (via
-w) so you can detect non-2xx responses; if curl fails or the status is not 2xx,
write a clear error into RECOMMENDATION (and optionally fail the job) instead of
proceeding to parse a missing body. Update the block that sets RESPONSE (using
PAL_MCP_ENDPOINT, PAL_MCP_API_KEY, and /tmp/consensus-request.json) and the
subsequent RECOMMENDATION extraction to check for curl errors/HTTP status and
handle them gracefully.
- Around line 505-521: The recommendation string is interpolated directly into a
template literal (variable recommendation) and can contain backticks or ${}
which may break the script or allow injection; before building body, sanitize or
escape recommendation to neutralize backticks and template placeholders (e.g.,
replace "`" and "${" sequences or otherwise escape/JSON-encode the value) and
then use the sanitized value when constructing body passed to
github.rest.issues.createComment (still using context.repo and env.PR_NUMBER).
Ensure the escaping is done where recommendation is assigned so all downstream
uses (body and any logging) use the safe string.
- Around line 8-16: The workflows currently both post reviews because
ai-code-review.yml's review job doesn't label the original PR, so ci.yml's
claude-review job still runs; either add a step in the ai-code-review.yml review
job to apply the skip-ci-review label to the original PR (use the GitHub REST
API or actions/github-script targeting the pull request number from
github.event.pull_request.number) before posting comments, or remove the
claude-review job from ci.yml entirely to prevent duplicate comments; locate the
"review" job and the "autofix" job in ai-code-review.yml and the "claude-review"
job in ci.yml to implement one of these two fixes.
---
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 68-78: The claude-review job is still enabled and duplicates AI
reviews from the consolidated ai-code-review.yml workflow; either remove the
claude-review job definition entirely or update its if condition (the existing
if: check on the claude-review job) to skip when the consolidated workflow will
run by adding a label guard such as checking that the pull request does NOT
contain an 'ai-reviewed' label (e.g., augment the contains(...) condition to
include !contains(github.event.pull_request.labels.*.name, 'ai-reviewed')) so
only one AI review runs per PR.
In @.github/workflows/CLAUDE_REVIEW_SETUP.md:
- Around line 71-85: Update Phase 1 in the doc to remove the stale
CLAUDE_CODE_OAUTH_TOKEN reference and instead instruct users to provide at least
one Claude API provider; specifically, replace the CLAUDE_CODE_OAUTH_TOKEN lines
with a single line like "At least one Claude API provider (see Step 2 below)."
Also ensure the surrounding text aligns with the current ai-code-review.yml
expectations (which use ANTHROPIC_API_KEY or AWS_BEARER_TOKEN_BEDROCK) so
readers aren't confused about which secrets to supply.
- Around line 370-378: Update the troubleshooting section that currently
references CLAUDE_CODE_OAUTH_TOKEN to reflect the consolidated workflow by
replacing that token name and its instructions with the correct secrets:
ANTHROPIC_API_KEY (for Anthropic) or AWS_BEARER_TOKEN_BEDROCK (for Bedrock).
Locate any mentions of CLAUDE_CODE_OAUTH_TOKEN in the doc and change the copy,
example gh secret command, and any token prefix notes to the appropriate key
name(s) (ANTRHOPIC_API_KEY / AWS_BEARER_TOKEN_BEDROCK) and adjust validation
details accordingly so the guidance matches the active workflow.
- Around line 1-4: Update the intro line that currently reads "three-phase AI
code review system" to reflect the consolidated single workflow architecture;
replace that phrase with wording that explains reviews are driven by repository
labels (ai-consensus, ai-patch) rather than separate phases, and ensure the
intro briefly states users interact via labels and automation rather than
selecting phases so the documentation matches the PR changes.
In @.github/workflows/DEPLOYMENT_STATUS.md:
- Around line 214-223: Update the obsolete workflow name referenced in
DEPLOYMENT_STATUS.md: replace occurrences of "claude-review.yml" with the
current workflow filename "ai-code-review.yml" (e.g., in the gh command examples
like the `gh run list --workflow=claude-review.yml --limit 5` and any related
`gh run view` debug snippets). Ensure all examples and debug commands that
reference the workflow (the string "claude-review.yml" and the `gh run list`
invocation) are updated to use "ai-code-review.yml" so the documented
troubleshooting commands match the actual workflow name.
In @.github/workflows/setup-claude-review.sh:
- Around line 115-136: The script currently prompts for and sets
CLAUDE_CODE_OAUTH_TOKEN (seen in the CLAUDE_CODE_OAUTH_TOKEN check and gh secret
set call), but the new ai-code-review.yml workflow expects ANTHROPIC_API_KEY or
AWS_BEARER_TOKEN_BEDROCK; update configure_secrets to stop creating the unused
CLAUDE secret and instead prompt for and store the correct secret(s): ask for
ANTHROPIC_API_KEY (and/or AWS_BEARER_TOKEN_BEDROCK if applicable), validate
non-empty input, and call gh secret set ANTHROPIC_API_KEY (or gh secret set
AWS_BEARER_TOKEN_BEDROCK) with the provided values; ensure messages and
success/warning strings reference the new secret names so users configure the
secrets the workflow actually uses.
- Around line 71-105: The select_phase() function is misleading because phases
no longer exist; replace it with a feature-selection prompt that asks which
labels/features to enable (e.g., "Enable ai-consensus? [y/N]" and "Enable
ai-patch? [y/N]") instead of choices 1–4, and set explicit variables (e.g.,
ENABLE_CONSENSUS, ENABLE_PATCH or LABELS array) used by the rest of the script;
update references to PHASE and PHASE_NAME to use these new flags, or if you
prefer removing interactivity, delete select_phase() and any usage of
PHASE/PHASE_NAME and drive behavior purely from labels (ai-consensus, ai-patch)
or env vars so the workflow no longer asks for non-existent phases.
---
Nitpick comments:
In @.github/workflows/ai-code-review.yml:
- Around line 116-133: The HIGH_STAKES_PATTERNS substring matching can
over-trigger; update the loop that inspects HIGH_STAKES_PATTERNS and
CHANGED_FILES so directory patterns (those ending with "/") are matched only as
path prefixes (use grep -qE "^$pattern") and file patterns are matched as exact
filenames (use grep -q -xF "$pattern"), preserving the existing SHOULD_CONSENSUS
assignment and notice echo; modify the for-loop logic that iterates
HIGH_STAKES_PATTERNS and the conditional that currently uses grep -q "$pattern"
to branch on whether the pattern ends with "/" and apply the appropriate grep
invocation instead.
- Around line 96-104: The LINES_CHANGED calculation using git diff --stat and
awk is fragile; update the block that sets LINES_CHANGED (the shell code
referencing BASE, SHOULD_REVIEW and HAS_FORCE_LABEL) to use a more robust
command such as git diff --shortstat or git diff --numstat and reliably sum
insertions and deletions instead of parsing fixed columns; ensure you parse/sum
the numeric fields (or fallback to 0) and preserve the existing logic that
disables review when LINES_CHANGED > 1000 unless HAS_FORCE_LABEL == "true".
In @.github/workflows/ai-review-cost-monitor.yml:
- Around line 53-78: The arithmetic expressions use unquoted shell variables
(e.g., AI_REVIEW_RUNS, AI_REVIEW_COST, NIGHTLY_RUNS, etc.) when passed to bc;
update each calculation for DAILY_AI_REVIEW, DAILY_NIGHTLY, DAILY_CI,
DAILY_TRIAGE, DAILY_DOCS, DAILY_RELEASE and DAILY_TOTAL to quote the variables
(e.g., use "$AI_REVIEW_RUNS" and "$AI_REVIEW_COST") so the values from the GH
outputs/hardcoded costs are safely expanded; keep the same bc invocations but
ensure every variable reference in those expressions is wrapped in double quotes
to avoid word-splitting or empty-value issues.
In @.github/workflows/README_CLAUDE_REVIEW.md:
- Around line 41-50: Update the fenced code block in README_CLAUDE_REVIEW.md
that shows the .github/workflows tree by adding a language specifier (use
"text") to the opening fence (i.e., change ``` to ```text) so the snippet
renders correctly; locate the tree snippet in the file and replace the opening
backticks accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c84c65e2-2174-4869-8e55-8c37d4ae0566
📒 Files selected for processing (11)
.github/workflows/CLAUDE_REVIEW_SETUP.md.github/workflows/DEPLOYMENT_STATUS.md.github/workflows/README_CLAUDE_REVIEW.md.github/workflows/ai-code-review.yml.github/workflows/ai-review-cost-monitor.yml.github/workflows/ai-review.yml.github/workflows/ci.yml.github/workflows/claude-review-phase1.yml.github/workflows/claude-review-phase2.yml.github/workflows/claude-review-phase3.yml.github/workflows/setup-claude-review.sh
💤 Files with no reviewable changes (4)
- .github/workflows/ai-review.yml
- .github/workflows/claude-review-phase1.yml
- .github/workflows/claude-review-phase3.yml
- .github/workflows/claude-review-phase2.yml
Comment on lines
+8
to
+16
| on: | ||
| pull_request: | ||
| types: [opened, synchronize, reopened, ready_for_review, labeled] | ||
| issue_comment: | ||
| types: [created] | ||
|
|
||
| concurrency: | ||
| group: ai-code-review-${{ github.event.pull_request.number || github.event.issue.number }} | ||
| cancel-in-progress: true |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify both workflows trigger on pull_request and could produce duplicate reviews
echo "=== ai-code-review.yml triggers ==="
rg -n "^on:" -A 10 .github/workflows/ai-code-review.yml | head -15
echo ""
echo "=== ci.yml claude-review job condition ==="
rg -n "claude-review:" -A 15 .github/workflows/ci.yml | head -20Repository: Tony363/SuperClaude
Length of output: 1054
🏁 Script executed:
#!/bin/bash
# Check if ai-code-review.yml preflight job sets the skip-ci-review label
echo "=== Search for skip-ci-review label setting in ai-code-review.yml ==="
rg -n "skip-ci-review\|add.*label\|github.api" .github/workflows/ai-code-review.yml | head -30
echo ""
echo "=== Preflight job section (lines 21-173) ==="
sed -n '21,173p' .github/workflows/ai-code-review.yml | head -80Repository: Tony363/SuperClaude
Length of output: 3544
🏁 Script executed:
#!/bin/bash
# Check if ai-code-review.yml sets any labels (particularly skip-ci-review)
echo "=== Permissions for label modification ==="
rg -n "permissions:" -A 10 .github/workflows/ai-code-review.yml | grep -E "permissions:|pull-requests"
echo ""
echo "=== Search for label setting actions or gh api calls ==="
rg -n "label|gh.*label\|gh.*api.*label" .github/workflows/ai-code-review.yml
echo ""
echo "=== Complete review job (check for label operations) ==="
sed -n '177,250p' .github/workflows/ai-code-review.ymlRepository: Tony363/SuperClaude
Length of output: 4173
🏁 Script executed:
#!/bin/bash
# Check what the labels setting is around line 732
echo "=== Context around line 732 (labels setting) ==="
sed -n '720,750p' .github/workflows/ai-code-review.yml
echo ""
echo "=== Check autofix job for label setting ==="
sed -n '558,650p' .github/workflows/ai-code-review.yml | head -100
echo ""
echo "=== Search for all label assignments in review or autofix jobs ==="
rg -n "add.*label\|gh.*api.*labels\|\/labels" .github/workflows/ai-code-review.yml -A 2 -B 1Repository: Tony363/SuperClaude
Length of output: 5392
Duplicate review runs confirmed: Both ai-code-review.yml and ci.yml will post comments on the same PR.
ai-code-review.yml triggers on pull_request (lines 9–10) and runs the review job (line 177), which posts a comment. ci.yml's claude-review job also triggers on pull_request (line 74) and posts a comment unless the skip-ci-review label is present on the original PR (line 76).
However, ai-code-review.yml only sets skip-ci-review on the draft PR created by the autofix job (line 732–733), not on the original PR. This means the safeguard in ci.yml is never triggered, and both workflows will post duplicate review comments on the original PR.
Consider either:
- Adding a step in the review job to set
skip-ci-reviewon the original PR, or - Removing the
claude-reviewjob fromci.ymlentirely sinceai-code-review.ymlnow provides comprehensive review coverage.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ai-code-review.yml around lines 8 - 16, The workflows
currently both post reviews because ai-code-review.yml's review job doesn't
label the original PR, so ci.yml's claude-review job still runs; either add a
step in the ai-code-review.yml review job to apply the skip-ci-review label to
the original PR (use the GitHub REST API or actions/github-script targeting the
pull request number from github.event.pull_request.number) before posting
comments, or remove the claude-review job from ci.yml entirely to prevent
duplicate comments; locate the "review" job and the "autofix" job in
ai-code-review.yml and the "claude-review" job in ci.yml to implement one of
these two fixes.
Comment on lines
+487
to
+495
| RESPONSE=$(curl -s -X POST "$PAL_MCP_ENDPOINT" \ | ||
| -H "Authorization: Bearer $PAL_MCP_API_KEY" \ | ||
| -H "Content-Type: application/json" \ | ||
| -d @/tmp/consensus-request.json) | ||
|
|
||
| RECOMMENDATION=$(echo "$RESPONSE" | jq -r '.synthesized_recommendation // "Error: No recommendation"') | ||
| echo "recommendation<<EOF" >> "$GITHUB_OUTPUT" | ||
| echo "$RECOMMENDATION" >> "$GITHUB_OUTPUT" | ||
| echo "EOF" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
PAL MCP consensus curl call lacks timeout and error handling.
The curl call to the PAL MCP endpoint has no timeout (--max-time), no connection timeout (--connect-timeout), and doesn't check HTTP status codes. If the endpoint is slow or returns an error, the workflow could hang or silently fail with an unhelpful message.
🛡️ Proposed fix to add timeout and error handling
-RESPONSE=$(curl -s -X POST "$PAL_MCP_ENDPOINT" \
+RESPONSE=$(curl -s --fail --max-time 60 --connect-timeout 10 -X POST "$PAL_MCP_ENDPOINT" \
-H "Authorization: Bearer $PAL_MCP_API_KEY" \
-H "Content-Type: application/json" \
- -d `@/tmp/consensus-request.json`)
+ -d `@/tmp/consensus-request.json`) || {
+ echo "::warning::PAL MCP request failed"
+ RESPONSE='{}'
+}
RECOMMENDATION=$(echo "$RESPONSE" | jq -r '.synthesized_recommendation // "Error: No recommendation"')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ai-code-review.yml around lines 487 - 495, The curl call
that populates RESPONSE needs timeouts and HTTP-error handling: call curl with
connection and total timeouts (e.g., --connect-timeout and --max-time) and use
--fail or capture the HTTP status (via -w) so you can detect non-2xx responses;
if curl fails or the status is not 2xx, write a clear error into RECOMMENDATION
(and optionally fail the job) instead of proceeding to parse a missing body.
Update the block that sets RESPONSE (using PAL_MCP_ENDPOINT, PAL_MCP_API_KEY,
and /tmp/consensus-request.json) and the subsequent RECOMMENDATION extraction to
check for curl errors/HTTP status and handle them gracefully.
Comment on lines
+505
to
+521
| script: | | ||
| const recommendation = `${{ steps.consensus.outputs.recommendation }}`; | ||
| const body = `## Multi-Model Security Consensus | ||
|
|
||
| ### Consensus Recommendation | ||
|
|
||
| ${recommendation} | ||
|
|
||
| --- | ||
| **Models Consulted**: GPT-5-mini (for), Claude Haiku 4.5 (against), Gemini-2-Flash (neutral) | ||
| **Human review required** — do not auto-merge security-sensitive changes.`; | ||
|
|
||
| await github.rest.issues.createComment({ | ||
| ...context.repo, | ||
| issue_number: ${{ env.PR_NUMBER }}, | ||
| body: body | ||
| }); |
There was a problem hiding this comment.
Potential script injection via consensus recommendation output.
The recommendation output is directly interpolated into a JavaScript template literal (line 506). If the PAL MCP API returns content containing backticks or ${}, it could break the script or potentially enable code injection.
🛡️ Proposed fix using environment variable
- name: Post consensus comment
if: >-
needs.preflight.outputs.should_consensus == 'true' &&
steps.check_pal.outputs.configured == 'true'
continue-on-error: true
uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8
+ env:
+ RECOMMENDATION: ${{ steps.consensus.outputs.recommendation }}
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
script: |
- const recommendation = `${{ steps.consensus.outputs.recommendation }}`;
+ const recommendation = process.env.RECOMMENDATION;
const body = `## Multi-Model Security Consensus🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ai-code-review.yml around lines 505 - 521, The
recommendation string is interpolated directly into a template literal (variable
recommendation) and can contain backticks or ${} which may break the script or
allow injection; before building body, sanitize or escape recommendation to
neutralize backticks and template placeholders (e.g., replace "`" and "${"
sequences or otherwise escape/JSON-encode the value) and then use the sanitized
value when constructing body passed to github.rest.issues.createComment (still
using context.repo and env.PR_NUMBER). Ensure the escaping is done where
recommendation is assigned so all downstream uses (body and any logging) use the
safe string.
Tests were checking for deleted phase2/phase3 workflow files. Updated to test the new ai-code-review.yml consolidated workflow: - Renamed fixtures: phase2_config/phase3_config → ai_code_review_config - Replaced TestPhase2Workflow and TestPhase3Workflow with TestAICodeReviewWorkflow validating the 3-job consolidated structure - Tests now validate: preflight gate, review+consensus, autofix, protected file checks, GITHUB_TOKEN usage, draft PR creation All 12 tests ported to new structure + assertions preserved. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Claude Code Review (via AWS Bedrock)OverviewThis PR consolidates 4 separate AI review workflows into a single unified pipeline with a 3-job architecture (preflight → review → autofix). The changes significantly reduce code duplication, eliminate redundant concurrent reviews saving $4.50-13.50 per PR, and implement robust security controls. All GitHub Actions are SHA-pinned, and comprehensive integration tests validate the new structure. Critical Issues1. Non-existent AI Model References
{model: "gpt-4o-mini", stance: "for", stance_prompt: "..."}2. Duplicated Protected File Patterns
env:
PROTECTED_PATTERNS: ".github/workflows/ secrets/ .env CLAUDE.md .claude/skills/ agents/core/ agents/traits/"High Priority3. Git Origin Dependency Assumption
BASE="${BASE_REF:-main}"
git remote get-url origin >/dev/null 2>&1 || git remote add origin "${{ github.repositoryUrl }}"4. Cost Estimate Accuracy
5. Secret Scanning Regex Limitations
Medium Priority6. Workflow Test Clarity
for key in ("name", True, "jobs"): # 'on' is parsed as True by YAML7. Error Message for Large PRs
echo "::warning::PR too large for AI review (${LINES_CHANGED} lines >1000). Add 'force-review' label to override, or split into smaller PRs."8. Arithmetic Safety in awk
LINES_CHANGED=$(echo "$DIFF_STATS" | awk '{print ($4+0) + ($6+0)}')9. Consensus Model Selection
10. Test Coverage Gap
Positive Observations✅ Excellent Security Architecture
✅ Robust Fallback Strategy
✅ Comprehensive Testing
✅ Cost Optimization
✅ Clear Separation of Concerns
✅ Good Error Handling
✅ Documentation Quality
Review Summary
Security: Strong multi-layered controls with proper permissions scoping. Deducted one point for duplicated protected patterns and basic secret regex. Code Quality: Clean, well-structured workflow with good error handling. Minor issues with hardcoded values and git origin assumptions. Architecture: Excellent design with clear job separation, proper dependencies, and effective fallback strategy. Cost-optimized and maintainable. Testing: Comprehensive integration tests covering critical paths. Could benefit from fallback mechanism testing and edge case coverage. RecommendationAPPROVE with minor changes: This is a high-quality consolidation that significantly improves maintainability and reduces costs. The critical issue with model names must be fixed before merge, but the overall architecture is sound. The duplicated protected patterns should be centralized to prevent future security issues. AI Code Review (AWS Bedrock). Advisory only — use human judgment. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/integration/test_workflow_configs.py (1)
4-4:⚠️ Potential issue | 🟡 MinorStale PR reference in docstring.
The docstring references PR
#91, but this test file is being updated as part of PR#114.📝 Suggested fix
-and bash script patterns in workflow files changed by PR `#91`. +and bash script patterns in workflow files changed by PR `#114`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/test_workflow_configs.py` at line 4, Update the module-level docstring in test_workflow_configs.py to replace the stale PR reference "#91" with the current PR number "#114" (i.e., update the string that mentions PR `#91`), ensuring the docstring accurately reflects that these workflow file/bash script pattern changes are part of PR `#114`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/integration/test_workflow_configs.py`:
- Around line 156-167: The test test_preflight_checks_high_stakes_files fails
because the step lookup expects a step name/id containing "high-stakes" or
"high_stakes" but the actual preflight job steps in ai_code_review_config are
"Checkout code" and "Evaluate gates" (id "gate"); update the step-finding logic
(searching the steps list in
ai_code_review_config["jobs"]["preflight"]["steps"]) to look for the actual step
(e.g., match name "Evaluate gates" or id "gate") and then assert that that
step's "run" script contains ".github/workflows/"; ensure you update references
to variables check_step and run_script accordingly so the test locates the
correct step instead of the non-existent high-stakes step.
---
Outside diff comments:
In `@tests/integration/test_workflow_configs.py`:
- Line 4: Update the module-level docstring in test_workflow_configs.py to
replace the stale PR reference "#91" with the current PR number "#114" (i.e.,
update the string that mentions PR `#91`), ensuring the docstring accurately
reflects that these workflow file/bash script pattern changes are part of PR
`#114`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2b9ff1e7-a8ef-4ce8-a9c8-69649360a741
📒 Files selected for processing (1)
tests/integration/test_workflow_configs.py
Changed test to look for 'gate' step ID (actual name in workflow) instead of 'high-stakes' string. The high-stakes file detection is part of the 'Evaluate gates' step with id='gate'. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/integration/test_workflow_configs.py`:
- Around line 126-130: The test test_review_depends_on_preflight currently does
substring matching because it checks needs == "preflight" or "preflight" in
needs; instead normalize needs to a list and assert exact membership: if needs
is a string wrap it into a single-element list, then assert "preflight" in
needs_list; update the assertion in test_review_depends_on_preflight to use the
normalized needs_list (variable names: job, needs) so values like
"preflight-review" no longer pass.
- Around line 111-112: The test class TestAICodeReviewWorkflow in this
integration tests suite is not marked for marker-based selection; add the
integration marker by either decorating the class with `@pytest.mark.integration`
(ensure pytest is imported) or setting pytestmark = pytest.mark.integration at
module scope so the entire file is treated as integration tests and
picked/skipped correctly.
- Around line 171-188: The test_autofix_blocks_protected_files test currently
only requires workflow files plus either "secrets/" or ".env"; update the
assertions so the validation_step's run_script checks for all documented
protected paths instead of an either/or: ensure run_script contains
".github/workflows/", "secrets/" (or ".env" if you prefer keeping env
alternative), and also includes "CLAUDE.md" and "agents/core/"; modify the
assert logic around run_script to require these strings (referencing variables
job, steps, validation_step, run_script) so the test fails if any protected path
is missing.
- Around line 203-217: The test test_uses_github_token_not_pat uses loose
substring checks on pr_create_step -> with -> token; replace that with an
explicit allowlist of the accepted built-in GitHub token expressions (e.g. "${{
secrets.GITHUB_TOKEN }}", "${{ github.token }}", "GITHUB_TOKEN", and any other
project-standard canonical forms) and assert that token exactly matches one of
those allowed values and does not equal or reference any PAT secret (e.g.
contains "PAT" or "secrets.PAT"). Update the assertions in
test_uses_github_token_not_pat to compare token against this allowlist rather
than using substring containment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 17b94268-8470-4494-ad3b-15e76943ad9a
📒 Files selected for processing (1)
tests/integration/test_workflow_configs.py
Comment on lines
+111
to
+112
| class TestAICodeReviewWorkflow: | ||
| """Tests for consolidated ai-code-review.yml workflow (replaces phase1/2/3).""" |
There was a problem hiding this comment.
Mark this suite as integration.
This new suite lives under tests/integration/, but it is not marked, so marker-based selection/skip rules will miss it.
🏷️ Suggested change
+@pytest.mark.integration
class TestAICodeReviewWorkflow:As per coding guidelines, "Mark slower test journeys with @pytest.mark.slow or integration per pyproject.toml configuration".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/integration/test_workflow_configs.py` around lines 111 - 112, The test
class TestAICodeReviewWorkflow in this integration tests suite is not marked for
marker-based selection; add the integration marker by either decorating the
class with `@pytest.mark.integration` (ensure pytest is imported) or setting
pytestmark = pytest.mark.integration at module scope so the entire file is
treated as integration tests and picked/skipped correctly.
Comment on lines
+126
to
+130
| def test_review_depends_on_preflight(self, ai_code_review_config): | ||
| """Review job needs preflight to complete first.""" | ||
| job = ai_code_review_config["jobs"]["review"] | ||
| needs = job.get("needs") | ||
| assert needs == "preflight" or "preflight" in needs |
There was a problem hiding this comment.
Use exact needs membership here.
Line 130 falls back to substring matching when needs is a scalar string, so a value like "preflight-review" would still pass. Normalize first and assert exact membership.
🔧 Tighten the assertion
job = ai_code_review_config["jobs"]["review"]
- needs = job.get("needs")
- assert needs == "preflight" or "preflight" in needs
+ needs = job.get("needs", [])
+ if isinstance(needs, str):
+ needs = [needs]
+ assert "preflight" in needs📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def test_review_depends_on_preflight(self, ai_code_review_config): | |
| """Review job needs preflight to complete first.""" | |
| job = ai_code_review_config["jobs"]["review"] | |
| needs = job.get("needs") | |
| assert needs == "preflight" or "preflight" in needs | |
| def test_review_depends_on_preflight(self, ai_code_review_config): | |
| """Review job needs preflight to complete first.""" | |
| job = ai_code_review_config["jobs"]["review"] | |
| needs = job.get("needs", []) | |
| if isinstance(needs, str): | |
| needs = [needs] | |
| assert "preflight" in needs |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/integration/test_workflow_configs.py` around lines 126 - 130, The test
test_review_depends_on_preflight currently does substring matching because it
checks needs == "preflight" or "preflight" in needs; instead normalize needs to
a list and assert exact membership: if needs is a string wrap it into a
single-element list, then assert "preflight" in needs_list; update the assertion
in test_review_depends_on_preflight to use the normalized needs_list (variable
names: job, needs) so values like "preflight-review" no longer pass.
Comment on lines
+171
to
+188
| def test_autofix_blocks_protected_files(self, ai_code_review_config): | ||
| """Autofix job prevents modifications to protected files.""" | ||
| job = ai_code_review_config["jobs"]["autofix"] | ||
| steps = job["steps"] | ||
| validation_step = None | ||
| for step in steps: | ||
| if "security" in step.get("name", "").lower() or step.get("id") == "check": | ||
| if ( | ||
| "protect" in step.get("name", "").lower() | ||
| or "validate" in step.get("name", "").lower() | ||
| ): | ||
| validation_step = step | ||
| break | ||
| assert validation_step is not None | ||
| assert ".github/workflows/" in validation_step["run"] | ||
|
|
||
| def test_has_concurrency_control(self, phase3_config): | ||
| """Phase 3 has workflow-level concurrency to prevent parallel runs.""" | ||
| assert "concurrency" in phase3_config | ||
| assert phase3_config["concurrency"].get("cancel-in-progress") is True | ||
|
|
||
| def test_never_auto_merges(self, phase3_config): | ||
| """Phase 3 creates draft PRs, never auto-merges.""" | ||
| # The name or comment should indicate draft PR creation | ||
| assert "Draft PR" in phase3_config["name"] or "draft" in str(phase3_config).lower() | ||
| assert validation_step is not None, "Protected file validation not found" | ||
| run_script = validation_step["run"] | ||
| # Should block workflow files, secrets, env files | ||
| assert ".github/workflows/" in run_script | ||
| assert "secrets/" in run_script or ".env" in run_script | ||
|
|
There was a problem hiding this comment.
Cover all documented protected autofix paths.
This test currently passes if the validation script mentions workflow files plus either secrets/ or .env. The PR contract and workflow guardrails also protect CLAUDE.md and agents/core/, so a regression there would stay green.
🔒 Expand the safety assertions
run_script = validation_step["run"]
- # Should block workflow files, secrets, env files
- assert ".github/workflows/" in run_script
- assert "secrets/" in run_script or ".env" in run_script
+ # Should block all documented protected paths
+ for protected_pattern in (
+ ".github/workflows/",
+ "secrets/",
+ ".env",
+ "CLAUDE.md",
+ "agents/core/",
+ ):
+ assert protected_pattern in run_script🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/integration/test_workflow_configs.py` around lines 171 - 188, The
test_autofix_blocks_protected_files test currently only requires workflow files
plus either "secrets/" or ".env"; update the assertions so the validation_step's
run_script checks for all documented protected paths instead of an either/or:
ensure run_script contains ".github/workflows/", "secrets/" (or ".env" if you
prefer keeping env alternative), and also includes "CLAUDE.md" and
"agents/core/"; modify the assert logic around run_script to require these
strings (referencing variables job, steps, validation_step, run_script) so the
test fails if any protected path is missing.
Comment on lines
+203
to
+217
| def test_uses_github_token_not_pat(self, ai_code_review_config): | ||
| """Autofix uses GITHUB_TOKEN to prevent recursive triggers.""" | ||
| job = ai_code_review_config["jobs"]["autofix"] | ||
| steps = job["steps"] | ||
| pr_create_step = None | ||
| for step in steps: | ||
| if "create-pull-request" in step.get("uses", ""): | ||
| pr_create_step = step | ||
| break | ||
| assert pr_create_step is not None, "create-pull-request step not found" | ||
| # Should use GITHUB_TOKEN, not PAT | ||
| with_block = pr_create_step.get("with", {}) | ||
| token = with_block.get("token", "") | ||
| assert "GITHUB_TOKEN" in token, "Must use GITHUB_TOKEN not PAT for recursive safety" | ||
| assert "PAT" not in token, "Must not use PAT_TOKEN" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python3 - <<'PY'
from pathlib import Path
lines = Path(".github/workflows/ai-code-review.yml").read_text().splitlines()
for i, line in enumerate(lines):
if "create-pull-request@" in line:
for candidate in lines[i:i + 25]:
stripped = candidate.strip()
if stripped.startswith("token:"):
print(stripped)
raise SystemExit(0)
raise SystemExit("token line for create-pull-request step not found")
PYRepository: Tony363/SuperClaude
Length of output: 97
🏁 Script executed:
head -n 220 tests/integration/test_workflow_configs.py | tail -n 30Repository: Tony363/SuperClaude
Length of output: 1651
Replace substring matching with an allowlist of built-in GitHub tokens.
Lines 216-217 use loose substring matching that would incorrectly pass for values like ${{ secrets.MY_GITHUB_TOKEN }} or ${{ vars.GITHUB_TOKEN }}, defeating the recursive-safety enforcement this test is meant to guarantee.
🔐 Tighten the token assertion
with_block = pr_create_step.get("with", {})
token = with_block.get("token", "")
- assert "GITHUB_TOKEN" in token, "Must use GITHUB_TOKEN not PAT for recursive safety"
- assert "PAT" not in token, "Must not use PAT_TOKEN"
+ assert token in {
+ "${{ secrets.GITHUB_TOKEN }}",
+ "${{ github.token }}",
+ }, "Must use a built-in GitHub token, not a PAT or custom secret"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/integration/test_workflow_configs.py` around lines 203 - 217, The test
test_uses_github_token_not_pat uses loose substring checks on pr_create_step ->
with -> token; replace that with an explicit allowlist of the accepted built-in
GitHub token expressions (e.g. "${{ secrets.GITHUB_TOKEN }}", "${{ github.token
}}", "GITHUB_TOKEN", and any other project-standard canonical forms) and assert
that token exactly matches one of those allowed values and does not equal or
reference any PAT secret (e.g. contains "PAT" or "secrets.PAT"). Update the
assertions in test_uses_github_token_not_pat to compare token against this
allowlist rather than using substring containment.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ai-review.yml+claude-review-phase1/2/3.ymlinto singleai-code-review.ymlArchitecture
preflightreviewreviewconsensusai-consensuslabel OR high-stakes filesautofixai-patchlabel onlySecurity
GITHUB_TOKENonly (never PAT) — prevents recursive trigger loops.github/workflows/*,secrets/*,.env*,CLAUDE.md,agents/core/*Files
ai-code-review.yml(consolidated)ai-review.yml,claude-review-phase1.yml,claude-review-phase2.yml,claude-review-phase3.ymlai-review-cost-monitor.yml,ci.yml, docs (README_CLAUDE_REVIEW.md,CLAUDE_REVIEW_SETUP.md,DEPLOYMENT_STATUS.md,setup-claude-review.sh)Test plan
ai-consensuslabel triggers consensus stepsai-patchlabel triggers autofix job🤖 Generated with Claude Code
Summary by Sourcery
Consolidate the AI code review automation into a single workflow with preflight, review, consensus, and autofix stages while tightening cost tracking, security controls, and action pinning.
Enhancements:
ai-code-review.ymlworkflow that orchestrates preflight gating, advisory code review, optional multi-model consensus, and optional autofix draft PR creation.Build:
ai-code-review.ymlpipeline and update the cost monitor to track it instead of phase-specific workflows.CI:
Documentation:
Summary by CodeRabbit
New Features
Documentation
Chores
Tests