|
| 1 | +name: Code Quality Review |
| 2 | +description: Reviews code for DRY, YAGNI, modularity, and other quality principles |
| 3 | + |
| 4 | +inputs: |
| 5 | + github_token: |
| 6 | + description: 'GitHub token for API access' |
| 7 | + required: true |
| 8 | + claude_code_oauth_token: |
| 9 | + description: 'Claude Code OAuth token' |
| 10 | + required: true |
| 11 | + |
| 12 | +outputs: |
| 13 | + passed: |
| 14 | + description: 'Whether the review passed' |
| 15 | + value: ${{ steps.result.outputs.passed }} |
| 16 | + result_json: |
| 17 | + description: 'Full JSON result' |
| 18 | + value: ${{ steps.result.outputs.result_json }} |
| 19 | + |
| 20 | +runs: |
| 21 | + using: composite |
| 22 | + steps: |
| 23 | + - name: Get changed files and imports |
| 24 | + id: context |
| 25 | + shell: bash |
| 26 | + env: |
| 27 | + GITHUB_TOKEN: ${{ inputs.github_token }} |
| 28 | + run: | |
| 29 | + # Get changed files |
| 30 | + if [ "${{ github.event_name }}" = "pull_request" ]; then |
| 31 | + gh pr view ${{ github.event.pull_request.number }} --json files --jq '.files[].path' > /tmp/changed_files.txt |
| 32 | + else |
| 33 | + git diff --name-only HEAD~1 HEAD > /tmp/changed_files.txt 2>/dev/null || echo "" > /tmp/changed_files.txt |
| 34 | + fi |
| 35 | +
|
| 36 | + # Filter to code files only |
| 37 | + grep -E '\.(ts|tsx|js|jsx|py|go|rs|java|rb)$' /tmp/changed_files.txt > /tmp/changed_code_files.txt 2>/dev/null || echo "" > /tmp/changed_code_files.txt |
| 38 | +
|
| 39 | + CHANGED_COUNT=$(wc -l < /tmp/changed_code_files.txt | tr -d ' ') |
| 40 | + echo "changed_count=$CHANGED_COUNT" >> $GITHUB_OUTPUT |
| 41 | +
|
| 42 | + if [ "$CHANGED_COUNT" -eq 0 ]; then |
| 43 | + echo "No code files changed" |
| 44 | + exit 0 |
| 45 | + fi |
| 46 | +
|
| 47 | + # Find imports/dependencies of changed files |
| 48 | + mkdir -p /tmp/related_files |
| 49 | + while IFS= read -r file; do |
| 50 | + if [ -f "$file" ]; then |
| 51 | + # Extract imports from TypeScript/JavaScript files |
| 52 | + if [[ "$file" =~ \.(ts|tsx|js|jsx)$ ]]; then |
| 53 | + grep -E "^import .* from ['\"]" "$file" 2>/dev/null | \ |
| 54 | + sed "s/.*from ['\"]\\([^'\"]*\\)['\"].*/\\1/" | \ |
| 55 | + while read -r import_path; do |
| 56 | + # Handle relative imports |
| 57 | + if [[ "$import_path" == .* ]]; then |
| 58 | + dir=$(dirname "$file") |
| 59 | + # Try common extensions |
| 60 | + for ext in "" ".ts" ".tsx" ".js" ".jsx" "/index.ts" "/index.tsx" "/index.js"; do |
| 61 | + resolved="$dir/$import_path$ext" |
| 62 | + if [ -f "$resolved" ]; then |
| 63 | + echo "$resolved" >> /tmp/related_files/imports.txt |
| 64 | + break |
| 65 | + fi |
| 66 | + done |
| 67 | + fi |
| 68 | + done |
| 69 | + fi |
| 70 | + fi |
| 71 | + done < /tmp/changed_code_files.txt |
| 72 | +
|
| 73 | + # Deduplicate related files |
| 74 | + if [ -f /tmp/related_files/imports.txt ]; then |
| 75 | + sort -u /tmp/related_files/imports.txt > /tmp/related_imports.txt |
| 76 | + RELATED_COUNT=$(wc -l < /tmp/related_imports.txt | tr -d ' ') |
| 77 | + else |
| 78 | + echo "" > /tmp/related_imports.txt |
| 79 | + RELATED_COUNT=0 |
| 80 | + fi |
| 81 | + echo "related_count=$RELATED_COUNT" >> $GITHUB_OUTPUT |
| 82 | +
|
| 83 | + # Combine for analysis scope |
| 84 | + cat /tmp/changed_code_files.txt /tmp/related_imports.txt 2>/dev/null | sort -u > /tmp/analysis_scope.txt |
| 85 | +
|
| 86 | + - name: Run Code Quality Review |
| 87 | + if: steps.context.outputs.changed_count != '0' |
| 88 | + id: review |
| 89 | + uses: anthropics/claude-code-base-action@beta |
| 90 | + with: |
| 91 | + claude_code_oauth_token: ${{ inputs.claude_code_oauth_token }} |
| 92 | + allowed_tools: "View,GlobTool,Grep,Bash(git diff:*),Bash(cat:*),Bash(find:*)" |
| 93 | + max_turns: 20 |
| 94 | + prompt: | |
| 95 | + # Code Quality Review Agent |
| 96 | +
|
| 97 | + You are a code quality review specialist. Your job is to evaluate code changes against software engineering best practices and principles. |
| 98 | +
|
| 99 | + ## Context |
| 100 | + - Changed Files: ${{ steps.context.outputs.changed_count }} |
| 101 | + - Related Files (imports): ${{ steps.context.outputs.related_count }} |
| 102 | +
|
| 103 | + ## Files to Analyze |
| 104 | + Read /tmp/analysis_scope.txt for the full list of files to review. |
| 105 | + Read /tmp/changed_code_files.txt for just the files that were directly changed. |
| 106 | +
|
| 107 | + ## Your Task |
| 108 | +
|
| 109 | + 1. **Read the changed files**: |
| 110 | + - Use `git diff HEAD~1 HEAD -- <file>` to see what changed |
| 111 | + - Understand the nature of the changes |
| 112 | +
|
| 113 | + 2. **Read related files**: |
| 114 | + - Check /tmp/related_imports.txt for files imported by changed files |
| 115 | + - Understand how changes interact with existing code |
| 116 | +
|
| 117 | + 3. **Evaluate against quality principles**: |
| 118 | +
|
| 119 | + **DRY (Don't Repeat Yourself)**: |
| 120 | + - Is there duplicated code that should be extracted? |
| 121 | + - Are there patterns that appear multiple times? |
| 122 | + - Could shared utilities reduce repetition? |
| 123 | +
|
| 124 | + **YAGNI (You Ain't Gonna Need It)**: |
| 125 | + - Is there over-engineering or premature abstraction? |
| 126 | + - Are there unused parameters, options, or flexibility? |
| 127 | + - Is the solution more complex than needed? |
| 128 | +
|
| 129 | + **Modularity**: |
| 130 | + - Are files/functions focused on a single responsibility? |
| 131 | + - Are dependencies well-organized? |
| 132 | + - Could large files be split? |
| 133 | +
|
| 134 | + **Complexity**: |
| 135 | + - Are there deeply nested conditionals? |
| 136 | + - Are functions too long (>50 lines)? |
| 137 | + - Is the logic easy to follow? |
| 138 | +
|
| 139 | + **Coupling**: |
| 140 | + - Are modules tightly coupled? |
| 141 | + - Would changes here require changes elsewhere? |
| 142 | + - Are interfaces clean and minimal? |
| 143 | +
|
| 144 | + **Cohesion**: |
| 145 | + - Do related things stay together? |
| 146 | + - Are unrelated concerns separated? |
| 147 | + - Is the code organization logical? |
| 148 | +
|
| 149 | + 4. **Identify issues by severity**: |
| 150 | + - **Critical**: Major violations that will cause maintenance problems |
| 151 | + - **Major**: Significant issues that should be addressed |
| 152 | + - **Minor**: Style/preference issues, nice-to-haves |
| 153 | +
|
| 154 | + 5. **Make your decision**: |
| 155 | + - PASS: No critical issues, code follows reasonable practices |
| 156 | + - FAIL: Critical quality issues that should be fixed before merge |
| 157 | +
|
| 158 | + ## IMPORTANT RULES |
| 159 | +
|
| 160 | + - **DO NOT edit any files** - you are a reviewer only |
| 161 | + - Focus on the CHANGES, not pre-existing code quality issues |
| 162 | + - Be pragmatic - perfect is the enemy of good |
| 163 | + - Consider the context and scope of the changes |
| 164 | + - Avoid nitpicking on style if it's consistent |
| 165 | +
|
| 166 | + ## Output Format |
| 167 | +
|
| 168 | + Your response MUST end with a JSON code block in this exact format: |
| 169 | +
|
| 170 | + ```json |
| 171 | + { |
| 172 | + "passed": true/false, |
| 173 | + "confidence": 0.0-1.0, |
| 174 | + "file_analyses": [ |
| 175 | + { |
| 176 | + "file_path": "path/to/file.ts", |
| 177 | + "imports_analyzed": ["list of related files checked"], |
| 178 | + "issues": [ |
| 179 | + { |
| 180 | + "type": "DRY|YAGNI|MODULARITY|COMPLEXITY|COUPLING|COHESION", |
| 181 | + "severity": "critical|major|minor", |
| 182 | + "line_range": "10-25", |
| 183 | + "description": "what the issue is", |
| 184 | + "suggestion": "how to fix it" |
| 185 | + } |
| 186 | + ], |
| 187 | + "positive_aspects": ["good things about this code"] |
| 188 | + } |
| 189 | + ], |
| 190 | + "principle_scores": { |
| 191 | + "dry": 1-10, |
| 192 | + "yagni": 1-10, |
| 193 | + "modularity": 1-10, |
| 194 | + "complexity": 1-10, |
| 195 | + "coupling": 1-10, |
| 196 | + "cohesion": 1-10 |
| 197 | + }, |
| 198 | + "blocking_issues": ["critical issues that block merge"], |
| 199 | + "refactoring_suggestions": [ |
| 200 | + { |
| 201 | + "priority": "high|medium|low", |
| 202 | + "description": "what to refactor", |
| 203 | + "affected_files": ["files involved"] |
| 204 | + } |
| 205 | + ], |
| 206 | + "summary": "Brief summary of code quality assessment" |
| 207 | + } |
| 208 | + ``` |
| 209 | +
|
| 210 | + - name: Extract result |
| 211 | + id: result |
| 212 | + shell: bash |
| 213 | + run: | |
| 214 | + # Check if review was skipped (no code files) |
| 215 | + if [ "${{ steps.context.outputs.changed_count }}" = "0" ]; then |
| 216 | + echo '{"passed":true,"status":"skipped","summary":"No code files changed"}' > /tmp/review_output.json |
| 217 | + echo "passed=true" >> $GITHUB_OUTPUT |
| 218 | + echo "result_json=$(cat /tmp/review_output.json | jq -c '.')" >> $GITHUB_OUTPUT |
| 219 | + exit 0 |
| 220 | + fi |
| 221 | +
|
| 222 | + EXEC_FILE="${{ steps.review.outputs.execution_file }}" |
| 223 | + CONCLUSION="${{ steps.review.outputs.conclusion }}" |
| 224 | +
|
| 225 | + if [ "$CONCLUSION" = "success" ]; then |
| 226 | + DEFAULT_OUTPUT='{"passed":true,"confidence":0.8,"summary":"Code quality review completed","status":"passed"}' |
| 227 | + else |
| 228 | + DEFAULT_OUTPUT='{"passed":false,"confidence":0.5,"summary":"Code quality review found issues","status":"failed"}' |
| 229 | + fi |
| 230 | +
|
| 231 | + if [ -f "$EXEC_FILE" ]; then |
| 232 | + LAST_TEXT=$(jq -r '[.[] | select(.type == "assistant") | .message.content[]? | select(.type == "text") | .text] | last // ""' "$EXEC_FILE" 2>/dev/null) |
| 233 | + OUTPUT=$(echo "$LAST_TEXT" | sed -n '/```json/,/```/{/```json/d;/```/d;p;}' | tr -d '\r') |
| 234 | +
|
| 235 | + if echo "$OUTPUT" | jq . >/dev/null 2>&1 && [ -n "$OUTPUT" ]; then |
| 236 | + PASSED=$(echo "$OUTPUT" | jq -r '.passed // false') |
| 237 | + if [ "$PASSED" = "true" ]; then |
| 238 | + OUTPUT=$(echo "$OUTPUT" | jq '. + {status: "passed"}') |
| 239 | + else |
| 240 | + OUTPUT=$(echo "$OUTPUT" | jq '. + {status: "failed"}') |
| 241 | + fi |
| 242 | + echo "$OUTPUT" > /tmp/review_output.json |
| 243 | + else |
| 244 | + echo "$DEFAULT_OUTPUT" > /tmp/review_output.json |
| 245 | + fi |
| 246 | + else |
| 247 | + echo "$DEFAULT_OUTPUT" > /tmp/review_output.json |
| 248 | + fi |
| 249 | +
|
| 250 | + PASSED=$(jq -r '.passed // false' /tmp/review_output.json) |
| 251 | + echo "passed=$PASSED" >> $GITHUB_OUTPUT |
| 252 | +
|
| 253 | + RESULT_JSON=$(cat /tmp/review_output.json | jq -c '.') |
| 254 | + echo "result_json=$RESULT_JSON" >> $GITHUB_OUTPUT |
0 commit comments