mirror of
https://github.com/tiennm99/ccs.git
synced 2026-07-16 02:11:28 +00:00
fix(ai-review): address all review findings — restore allowedTools, add fallbacks
Fixes from ai-review bot feedback on PR #838: - [HIGH] Restore --allowedTools with Agent added (was incorrectly removed, not redundant with bypassPermissions — it's a tool whitelist) - [MED] Add inline fallback prompts when subagent files missing from base branch - [MED] Add graceful degradation in orchestrator for Agent tool failures and empty subagent prompts - [MED] Replace hardcoded PROMPT_EOF delimiters with random openssl-generated delimiters to prevent early heredoc termination - [LOW] Add per-subagent turn budget guidance (8-10 turns each, 50 total) - [LOW] Add "Do NOT fetch diff separately" instruction to all 4 subagent prompts
This commit is contained in:
Tam Nhu Tran
@@ -2,7 +2,10 @@
|
||||
|
||||
You are a review orchestrator. Your ONLY job is to dispatch subagent reviewers and merge their findings.
|
||||
|
||||
**MANDATORY RULE: You MUST use the Agent tool to spawn subagent reviewers for all standard and deep PRs. You are FORBIDDEN from reviewing code yourself — delegate ALL review work to subagents. The ONLY exception is trivial PRs (<=2 files, <=30 lines, no sensitive paths).**
|
||||
**MANDATORY RULE: You MUST use the Agent tool to spawn subagent reviewers for all standard and deep PRs. Delegate ALL review work to subagents. The ONLY exceptions where you may review directly:**
|
||||
- **Trivial PRs** (<=2 files, <=30 lines, no sensitive paths)
|
||||
- **Agent tool unavailable** — if Agent tool calls fail or error, fall back to reviewing the diff yourself using the same checklist areas. Add a note at the top of your review: "⚠️ Subagent dispatch failed — falling back to single-agent review."
|
||||
- **Empty subagent prompts** — if all `<*-review-prompt>` XML tags are empty, review directly and note: "⚠️ Subagent prompts not yet available on base branch — using single-agent review."
|
||||
|
||||
Your workflow:
|
||||
1. Triage the PR scope
|
||||
@@ -35,6 +38,8 @@ Read the diff ONCE with `gh pr diff`, then spawn all 3 agents in a SINGLE respon
|
||||
|
||||
Do NOT make each agent read the diff separately — pass it in their prompt.
|
||||
|
||||
**Turn budget:** Each subagent should complete within 8-10 turns. The total budget is 50 turns shared across all agents + orchestration. If a subagent hasn't completed after 10 turns, proceed with whatever output it has produced.
|
||||
|
||||
## Step 3: Adversarial Review (Sequential)
|
||||
|
||||
After ALL 3 parallel reviewers complete, spawn ONE more subagent:
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
# Adversarial Red-Team Review Prompt
|
||||
|
||||
You are an adversarial code reviewer. Your ONLY job is to find what 3 prior reviewers (security, quality, CCS compliance) MISSED. DO NOT repeat findings already reported by prior reviewers -- those are provided as context. Focus on ADDED/MODIFIED lines (+ prefix). DO NOT praise the code. ONLY report problems.
|
||||
The full PR diff is provided at the end of this prompt. Do NOT fetch the diff separately — use what is provided.
|
||||
|
||||
## Context
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
# CCS Project Compliance Review Prompt
|
||||
|
||||
You are a CCS project compliance reviewer. Verify adherence to CCS-specific rules and conventions. These are project-specific constraints -- violations are automatic findings. Focus on ADDED/MODIFIED lines (+ prefix).
|
||||
The full PR diff is provided at the end of this prompt. Do NOT fetch the diff separately — use what is provided.
|
||||
|
||||
## CCS Rules (ALL 12 must be checked)
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
# Code Quality & Correctness Review Prompt
|
||||
|
||||
You are a code quality reviewer. Focus on correctness, robustness, and performance in the provided diff. Focus on ADDED/MODIFIED lines (+ prefix).
|
||||
The full PR diff is provided at the end of this prompt. Do NOT fetch the diff separately — use what is provided.
|
||||
|
||||
## Checklist Areas
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
# Security & Injection Review Prompt
|
||||
|
||||
You are a security-focused code reviewer. Analyze ONLY security concerns in the provided diff. Focus on ADDED/MODIFIED lines (+ prefix). Pre-existing code is out of scope unless the change makes it newly exploitable.
|
||||
The full PR diff is provided at the end of this prompt. Do NOT fetch the diff separately — use what is provided.
|
||||
|
||||
## Checklist Areas
|
||||
|
||||
|
||||
@@ -219,25 +219,48 @@ jobs:
|
||||
|
||||
# Load subagent prompts (all from base branch for security)
|
||||
SECURITY_PROMPT=$(git show "origin/${BASE_REF}:.github/review-prompts/security.md" 2>/dev/null || echo "")
|
||||
if [ -z "$SECURITY_PROMPT" ]; then
|
||||
echo "::warning::security.md not found on base branch — using inline fallback"
|
||||
SECURITY_PROMPT="You are a security reviewer. Check the diff for injection vulnerabilities, auth bypasses, race conditions, secrets exposure, and supply chain risks. Report findings as: #### [HIGH|MEDIUM|LOW] [SECURITY] file:line"
|
||||
fi
|
||||
|
||||
QUALITY_PROMPT=$(git show "origin/${BASE_REF}:.github/review-prompts/quality.md" 2>/dev/null || echo "")
|
||||
if [ -z "$QUALITY_PROMPT" ]; then
|
||||
echo "::warning::quality.md not found on base branch — using inline fallback"
|
||||
QUALITY_PROMPT="You are a code quality reviewer. Check for error handling gaps, false assumptions, performance issues, dead code, and test gaps. Report findings as: #### [HIGH|MEDIUM|LOW] [QUALITY] file:line"
|
||||
fi
|
||||
|
||||
CCS_PROMPT=$(git show "origin/${BASE_REF}:.github/review-prompts/ccs-compliance.md" 2>/dev/null || echo "")
|
||||
if [ -z "$CCS_PROMPT" ]; then
|
||||
echo "::warning::ccs-compliance.md not found on base branch — using inline fallback"
|
||||
CCS_PROMPT="You are a CCS compliance reviewer. Check for: no emojis in CLI output, getCcsDir() usage, cross-platform parity, --help updates, string-only settings, conventional commits. Report findings as: #### [HIGH|MEDIUM|LOW] [CCS] file:line"
|
||||
fi
|
||||
|
||||
ADVERSARIAL_PROMPT=$(git show "origin/${BASE_REF}:.github/review-prompts/adversarial.md" 2>/dev/null || echo "")
|
||||
if [ -z "$ADVERSARIAL_PROMPT" ]; then
|
||||
echo "::warning::adversarial.md not found on base branch — using inline fallback"
|
||||
ADVERSARIAL_PROMPT="You are an adversarial reviewer. Find what prior reviewers missed: interaction bugs, implicit coupling, missing rollback, boundary violations. Do NOT repeat prior findings. Report as: #### [HIGH|MEDIUM|LOW] [ADVERSARIAL] file:line"
|
||||
fi
|
||||
|
||||
echo "security_prompt<<PROMPT_EOF" >> "$GITHUB_OUTPUT"
|
||||
SECURITY_DELIM="SECURITY_$(openssl rand -hex 16)"
|
||||
echo "security_prompt<<${SECURITY_DELIM}" >> "$GITHUB_OUTPUT"
|
||||
echo "$SECURITY_PROMPT" >> "$GITHUB_OUTPUT"
|
||||
echo "PROMPT_EOF" >> "$GITHUB_OUTPUT"
|
||||
echo "${SECURITY_DELIM}" >> "$GITHUB_OUTPUT"
|
||||
|
||||
echo "quality_prompt<<PROMPT_EOF" >> "$GITHUB_OUTPUT"
|
||||
QUALITY_DELIM="QUALITY_$(openssl rand -hex 16)"
|
||||
echo "quality_prompt<<${QUALITY_DELIM}" >> "$GITHUB_OUTPUT"
|
||||
echo "$QUALITY_PROMPT" >> "$GITHUB_OUTPUT"
|
||||
echo "PROMPT_EOF" >> "$GITHUB_OUTPUT"
|
||||
echo "${QUALITY_DELIM}" >> "$GITHUB_OUTPUT"
|
||||
|
||||
echo "ccs_prompt<<PROMPT_EOF" >> "$GITHUB_OUTPUT"
|
||||
CCS_DELIM="CCS_$(openssl rand -hex 16)"
|
||||
echo "ccs_prompt<<${CCS_DELIM}" >> "$GITHUB_OUTPUT"
|
||||
echo "$CCS_PROMPT" >> "$GITHUB_OUTPUT"
|
||||
echo "PROMPT_EOF" >> "$GITHUB_OUTPUT"
|
||||
echo "${CCS_DELIM}" >> "$GITHUB_OUTPUT"
|
||||
|
||||
echo "adversarial_prompt<<PROMPT_EOF" >> "$GITHUB_OUTPUT"
|
||||
ADVERSARIAL_DELIM="ADVERSARIAL_$(openssl rand -hex 16)"
|
||||
echo "adversarial_prompt<<${ADVERSARIAL_DELIM}" >> "$GITHUB_OUTPUT"
|
||||
echo "$ADVERSARIAL_PROMPT" >> "$GITHUB_OUTPUT"
|
||||
echo "PROMPT_EOF" >> "$GITHUB_OUTPUT"
|
||||
echo "${ADVERSARIAL_DELIM}" >> "$GITHUB_OUTPUT"
|
||||
|
||||
- name: Run Claude Code Review
|
||||
id: claude-review
|
||||
@@ -300,6 +323,7 @@ jobs:
|
||||
--model ${{ env.REVIEW_MODEL }}
|
||||
--permission-mode bypassPermissions
|
||||
--max-turns 50
|
||||
--allowedTools "Agent,Glob,Grep,Read,Write,Bash(gh pr diff *),Bash(gh pr view *),Bash(git diff *),Bash(git log *),Bash(git show *),Bash(cat *),Bash(ls *),Bash(wc *),Bash(head *),Bash(tail *),Bash(find *)"
|
||||
|
||||
# Fallback: if Claude didn't write the review file, extract from execution output
|
||||
- name: Extract review from execution output (fallback)
|
||||
|
||||
Reference in New Issue
Block a user