mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
fix(ci): lean Claude review — 5 turns, diff-only, no prompt embedding [skip-regression-check]
- Reduce --max-turns from 30 to 5 (enough for diff review) - Remove full source file reading instruction (diff-only review) - Don't embed diff in prompt — let Claude run git diff itself - Extract results from execution_file output instead of /tmp/ file - Reduces cost from ~$1.50 to ~$0.10-0.30 per review Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
This commit is contained in:
Henry Park
co-authored by
Claude Opus 4.6
Claude Opus 4.6
parent
454db90f84
commit
1ecc41e0ce
@@ -45,116 +45,17 @@ jobs:
|
||||
ref: ${{ inputs.ref }}
|
||||
fetch-depth: 0
|
||||
|
||||
- name: Generate diff context
|
||||
- name: Check diff availability
|
||||
id: diff
|
||||
run: |
|
||||
RANGE="${{ inputs.diff_range }}"
|
||||
if [ -z "$RANGE" ]; then
|
||||
echo "diff_available=false" >> "$GITHUB_OUTPUT"
|
||||
echo "No diff range provided"
|
||||
exit 0
|
||||
else
|
||||
echo "diff_available=true" >> "$GITHUB_OUTPUT"
|
||||
echo "Reviewing range: ${RANGE}"
|
||||
fi
|
||||
echo "diff_available=true" >> "$GITHUB_OUTPUT"
|
||||
|
||||
# File-level diffstat for overview
|
||||
git diff --stat $RANGE > /tmp/diffstat.txt
|
||||
# Commit log with full messages
|
||||
git log --format="- %h %s" $RANGE > /tmp/commits.txt
|
||||
# Full diff
|
||||
git diff $RANGE > /tmp/staging-diff.patch
|
||||
|
||||
# Truncate diff if too large (>200KB) to avoid token limits
|
||||
DIFF_SIZE=$(wc -c < /tmp/staging-diff.patch)
|
||||
if [ "$DIFF_SIZE" -gt 204800 ]; then
|
||||
echo "::warning::Diff is ${DIFF_SIZE} bytes, truncating to 200KB"
|
||||
head -c 204800 /tmp/staging-diff.patch > /tmp/staging-diff-truncated.patch
|
||||
mv /tmp/staging-diff-truncated.patch /tmp/staging-diff.patch
|
||||
fi
|
||||
|
||||
# Build the prompt with actual file contents (not heredoc references)
|
||||
{
|
||||
cat <<'STATIC_PART'
|
||||
You are reviewing a batch of commits that just landed on the staging branch
|
||||
of the IronClaw project (a Rust AI assistant). This is a thorough security
|
||||
and quality review.
|
||||
|
||||
## Your Review Process
|
||||
|
||||
1. **Read CLAUDE.md** for project coding standards and conventions
|
||||
2. **Read the full diff** below to understand what changed
|
||||
3. **Read the complete source files** for every changed file (use the Read tool)
|
||||
to understand the full context, not just the diff hunks
|
||||
4. **Trace data flows** through changed code paths to find bugs
|
||||
5. **Check for security issues** across the OWASP top 10 categories
|
||||
|
||||
## Severity Categories
|
||||
|
||||
### CRITICAL
|
||||
- Security vulnerabilities: injection, XSS, SSRF, path traversal, auth bypass
|
||||
- Data exfiltration risks: secrets in logs, unescaped user data reaching LLM
|
||||
- Memory safety: unsafe blocks, unbounded allocations, panics in prod code
|
||||
- .unwrap() or .expect() in production code (not tests)
|
||||
- Race conditions in concurrent code (Arc/RwLock misuse, TOCTOU)
|
||||
|
||||
### HIGH
|
||||
- Logic bugs that would cause incorrect behavior
|
||||
- Missing error handling that would cause silent failures
|
||||
- Breaking changes to public APIs or database schema
|
||||
- Regression in safety/sanitizer/leak detection layers
|
||||
|
||||
### MEDIUM
|
||||
- Missing tests for new functionality
|
||||
- Unnecessary complexity or poor abstractions
|
||||
- Performance concerns (N+1 queries, unbounded loops)
|
||||
|
||||
### LOW
|
||||
- Documentation gaps, naming suggestions
|
||||
|
||||
## Confidence Scoring
|
||||
|
||||
For EVERY finding, assign a confidence score from 0 to 100:
|
||||
- **90-100**: You are certain this is a real issue. You can point to the exact code and explain why it's wrong.
|
||||
- **70-89**: Very likely a real issue but you'd want a human to verify.
|
||||
- **50-69**: Possibly an issue but you're not fully sure of the context.
|
||||
- **0-49**: Speculative. Might be a false positive.
|
||||
|
||||
Be honest with your confidence. A CRITICAL finding you're unsure about should get a low confidence score.
|
||||
|
||||
## Output Format
|
||||
|
||||
Write your findings to a file called /tmp/review-results.json with this structure:
|
||||
{
|
||||
"summary": "One paragraph overall assessment",
|
||||
"critical": [{"title": "...", "description": "...", "file": "...", "line": 0, "confidence": 85}],
|
||||
"high": [{"title": "...", "description": "...", "file": "...", "line": 0, "confidence": 75}],
|
||||
"medium": [{"title": "...", "description": "...", "file": "...", "line": 0, "confidence": 60}],
|
||||
"low": [{"title": "...", "description": "...", "file": "...", "line": 0, "confidence": 50}]
|
||||
}
|
||||
|
||||
If you find no issues at a given severity, use an empty array.
|
||||
|
||||
## Context
|
||||
|
||||
Changed files:
|
||||
STATIC_PART
|
||||
cat /tmp/diffstat.txt
|
||||
echo ""
|
||||
echo "Commits:"
|
||||
cat /tmp/commits.txt
|
||||
echo ""
|
||||
echo "Diff:"
|
||||
cat /tmp/staging-diff.patch
|
||||
} > /tmp/review-prompt.txt
|
||||
|
||||
- name: Load prompt into env
|
||||
id: load-prompt
|
||||
if: steps.diff.outputs.diff_available == 'true'
|
||||
run: |
|
||||
# Use GITHUB_ENV with delimiter to handle multiline content
|
||||
DELIM=$(openssl rand -hex 16)
|
||||
echo "REVIEW_PROMPT<<${DELIM}" >> "$GITHUB_ENV"
|
||||
cat /tmp/review-prompt.txt >> "$GITHUB_ENV"
|
||||
echo "${DELIM}" >> "$GITHUB_ENV"
|
||||
|
||||
- name: Run Claude Code review
|
||||
id: claude-review
|
||||
@@ -162,22 +63,52 @@ jobs:
|
||||
uses: anthropics/claude-code-action@v1
|
||||
with:
|
||||
anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}
|
||||
prompt: ${{ env.REVIEW_PROMPT }}
|
||||
claude_args: "--max-turns 30"
|
||||
prompt: |
|
||||
Review the diff for range `${{ inputs.diff_range }}` in this repo.
|
||||
Run `git diff ${{ inputs.diff_range }}` to see what changed.
|
||||
|
||||
- name: Validate and process results
|
||||
Categorize each finding by severity (CRITICAL/HIGH/MEDIUM/LOW) and assign
|
||||
a confidence score (0-100). Only report issues you're confident about.
|
||||
|
||||
CRITICAL: security vulns, panics in prod (.unwrap/.expect), data exfiltration, race conditions
|
||||
HIGH: logic bugs, missing error handling, breaking API/schema changes
|
||||
MEDIUM: missing tests, unnecessary complexity, performance issues
|
||||
LOW: documentation gaps, naming suggestions
|
||||
|
||||
Confidence: 90-100 = certain, 70-89 = very likely, 50-69 = possible, 0-49 = speculative
|
||||
|
||||
Return structured JSON with: summary, critical[], high[], medium[], low[]
|
||||
Each finding: {title, description, file, line, confidence}
|
||||
claude_args: "--max-turns 5"
|
||||
|
||||
- name: Extract results from execution log
|
||||
id: process-results
|
||||
if: always() && steps.diff.outputs.diff_available == 'true'
|
||||
run: |
|
||||
# Default: no results
|
||||
if [ ! -f /tmp/review-results.json ]; then
|
||||
echo '{"summary":"Claude review did not produce structured output.","critical":[],"high":[],"medium":[],"low":[]}' > /tmp/review-results.json
|
||||
fi
|
||||
FALLBACK='{"summary":"Claude review did not produce structured output.","critical":[],"high":[],"medium":[],"low":[]}'
|
||||
EXEC_FILE="${{ steps.claude-review.outputs.execution_file }}"
|
||||
|
||||
# Validate JSON
|
||||
if ! jq empty /tmp/review-results.json 2>/dev/null; then
|
||||
echo "::warning::review-results.json is not valid JSON, using fallback"
|
||||
echo '{"summary":"Claude review produced invalid JSON.","critical":[],"high":[],"medium":[],"low":[]}' > /tmp/review-results.json
|
||||
if [ -n "$EXEC_FILE" ] && [ -f "$EXEC_FILE" ]; then
|
||||
# Extract the last assistant text message as the review result
|
||||
RESULT=$(jq -r '[.[] | select(.type == "assistant" and .message != null) | .message] | last // empty' "$EXEC_FILE" 2>/dev/null || true)
|
||||
|
||||
# Try to parse as JSON (Claude may wrap in markdown code block)
|
||||
if [ -n "$RESULT" ]; then
|
||||
# Strip markdown code fences if present
|
||||
CLEANED=$(echo "$RESULT" | sed 's/^```json//;s/^```//;s/```$//' | jq '.' 2>/dev/null || true)
|
||||
if [ -n "$CLEANED" ]; then
|
||||
echo "$CLEANED" > /tmp/review-results.json
|
||||
else
|
||||
echo "::warning::Could not parse Claude output as JSON, using fallback"
|
||||
echo "$FALLBACK" > /tmp/review-results.json
|
||||
fi
|
||||
else
|
||||
echo "::warning::No assistant messages in execution log, using fallback"
|
||||
echo "$FALLBACK" > /tmp/review-results.json
|
||||
fi
|
||||
else
|
||||
echo "::warning::No execution file found, using fallback"
|
||||
echo "$FALLBACK" > /tmp/review-results.json
|
||||
fi
|
||||
|
||||
# Ensure confidence field exists on all findings (default to 50 if missing)
|
||||
|
||||
Reference in New Issue
Block a user