diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index 61f07fe9f58..5e7dda00dca 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -219,6 +219,21 @@ jobs: echo "is_admin=false" >> "$GITHUB_OUTPUT" fi + - name: Load review procedure + id: review-procedure + if: steps.gate.outputs.proceed == 'true' + run: | + PROCEDURE="$GITHUB_WORKSPACE/.claude/prompts/pr-review.md" + if [[ ! -f "$PROCEDURE" ]]; then + echo "::error::Review procedure not found at $PROCEDURE" + exit 1 + fi + { + echo 'text<> "$GITHUB_OUTPUT" + - name: Run Claude Code Review if: steps.gate.outputs.proceed == 'true' timeout-minutes: 30 @@ -229,242 +244,34 @@ jobs: track_progress: true trigger_phrase: "/review" prompt: | - You are the lead reviewer for this PR in shellhub-io/shellhub (Community Edition). - - The cloud/ (enterprise) repo is checked out at $GITHUB_WORKSPACE/cloud/ for context. - Focus the review on the shellhub diff. Use cloud/ only to understand cross-repo impact. - - ## Step 0: Pre-flight checks - - 1. Check if the PR is closed or merged — if so, stop immediately. - 2. Check if the PR is a draft — if so, stop immediately. - 3. For automatic triggers only (not `/review`): check if Claude has already - commented on this PR — if so, stop to avoid duplicate reviews. - - ## Step 1: Check for existing reviews - - Fetch existing review comments to avoid duplicating feedback: - gh api repos/shellhub-io/shellhub/pulls/${{ github.event.pull_request.number || github.event.issue.number }}/comments - gh api repos/shellhub-io/shellhub/pulls/${{ github.event.pull_request.number || github.event.issue.number }}/reviews - - Update the tracking comment via mcp__github_comment__update_claude_comment: - - - ## Code Review - - - [x] Gathered PR context - - [ ] Reviewing with 5 specialized agents - - [ ] Posting feedback - - ## Step 2: Spawn 5 reviewer agents IN PARALLEL - - Launch exactly 5 Task agents in a SINGLE message (so they run in parallel). - Model assignments by agent role: - - Agent 1 (Code Quality): model: "sonnet" - - Agent 2 (Security): model: "opus" - - Agent 3 (Testing): model: "opus" - - Agent 4 (Go/TypeScript Patterns): model: "sonnet" - - Agent 5 (Architecture & Cross-repo): model: "opus" - - Validation subagents (Step 3.5): model: "sonnet" - Each agent gets: - - The PR diff (pass it in the prompt) - - The list of changed files - - Read access to the full codebase (current directory for shellhub, $GITHUB_WORKSPACE/cloud/ for cloud) - - Agent assignments: - 1. **Code Quality**: Project conventions (CLAUDE.md), dead code, single responsibility, error handling, commit hygiene - 2. **Security**: OWASP Top 10 (injection, XSS, CSRF, SSRF), hardcoded secrets, crypto/rand usage, access control, input validation - 3. **Testing**: Missing tests for new/changed behavior, edge cases, test determinism, untested error paths - 4. **Go/TypeScript Patterns**: Error wrapping (%w), goroutine leaks, context propagation, minimal interfaces, React patterns (no `any`, no unnecessary re-renders, Zustand) - 5. **Architecture & Cross-repo**: If PR changes pkg/, check $GITHUB_WORKSPACE/cloud/ for impact. API contract changes, interface compatibility, breaking changes. - - Each agent must return findings as a structured list: - - file_path: exact path relative to repo root - - line_start and line_end: exact line numbers in the NEW file - - severity: critical | high | medium | low - - description: what's wrong and why - - suggestion: corrected code (if applicable), or empty string - - Agent assumptions (applies to all agents): - - All tools are functional. Do not test tools or make exploratory calls. - - Only call a tool if it is required. Every tool call should have a clear purpose. - - Do not speculatively read files unless you have a specific reason to suspect an issue there. - - CRITICAL: Only flag HIGH SIGNAL issues. - - Flag issues where: - - Code will fail to compile or parse (syntax errors, type errors, missing imports) - - Code will definitely produce wrong results regardless of inputs (clear logic errors) - - Clear, unambiguous CLAUDE.md or project convention violations where you can quote the exact rule - - Code introduces an exploitable security vulnerability (not theoretical) - - Code will cause data loss, corruption, or undefined behavior - - Do NOT flag: - - Code style or quality concerns - - Potential issues that depend on specific inputs or state - - Subjective suggestions or improvements - - Performance concerns unless there is a clear algorithmic regression - - Suggestions to add comments or documentation - - If you are not certain an issue is real, do not flag it. - False positives erode trust and waste reviewer time. - - Additional rules: - - For each suggestion you make, verify it is correct: check that all - referenced variables exist, all event fields are available for every - trigger type, token permissions are sufficient, and your fix does not - introduce new issues. - - Read the actual source files (not just the diff) to understand surrounding - context before flagging an issue. - - Do NOT flag pre-existing issues that were not introduced in this branch. - - Agents must NOT post any GitHub comments. They only return findings. - - After all agents complete, update the tracking comment via mcp__github_comment__update_claude_comment: - - - ## Code Review - - - [x] Gathered PR context - - [x] Code review complete - - [ ] Posting feedback - - ## Step 3: Aggregate and deduplicate - - After all 5 agents complete, collect their findings. Then: - 1. Remove duplicate findings (same file + same line range + same issue) - 2. If the same pattern repeats across multiple locations, keep only the first occurrence and note "Same issue also at: file:line, file:line, ..." - 3. Compare against the existing review comments fetched in Step 1. Skip any finding already reported in a previous review thread on the same file and line. - - Post exactly ONE comment per unique issue. If the same logical issue - appears in multiple locations, post on the first occurrence and list - others in the body: "Same issue also at: `path/file.go:42`, `path/other.go:87`" - - ## Step 3.5: Validate each finding - - For each finding that survived deduplication, launch a validation subagent - (model: "sonnet"). Pass it the finding details and the PR diff context. - - The validation agent must: - 1. Read the source file at the reported location with surrounding context - 2. Confirm the issue is real — not a misreading, not pre-existing, not assumption-dependent - 3. Confirm the issue was introduced in this branch's diff - 4. If a suggestion is provided, verify it compiles and doesn't introduce new issues - 5. Return: confirmed (true/false), confidence (0-100), reason - - Discard any finding where confirmed is false OR confidence is below 80. - - Cap: if more than 10 findings survive deduplication, re-evaluate whether - the high-signal filter was applied correctly before spawning validation agents. - - Do NOT flag (remove even if an agent reports them): - - Pre-existing issues not introduced in this branch's diff - - Purely cosmetic formatting (whitespace, brace placement, import order) - - Issues a linter or formatter would catch (gofmt, eslint, prettier, golangci-lint) - - Subjective preferences with no concrete impact on correctness - - Issues silenced by lint-ignore comments (nolint, eslint-disable, etc.) - - General code quality concerns (e.g., lack of test coverage) unless explicitly required in CLAUDE.md - - Pedantic nitpicks that a senior engineer would not flag - - Suggestions to add error handling for errors already handled upstream - - Something that appears to be a bug but is actually correct - - ## Step 4: Post inline comments - - For each remaining finding, post an inline comment using - mcp__github_inline_comment__create_inline_comment on the specific file and line. - - When a fix is available, include a GitHub suggestion block: - ```suggestion - corrected code here - ``` - - When posting suggestions: - - Only use suggestion blocks for small, self-contained fixes (5 lines or fewer) - - For 6+ lines or structural changes, describe the fix without a suggestion block - - Never post a suggestion that requires changes in multiple locations to be correct - - Every suggestion must be directly committable — it must compile as-is - - Do NOT re-report issues that already exist in previous review threads. - - After posting all inline comments, update the tracking comment via mcp__github_comment__update_claude_comment: - - - ## Code Review - - - [x] Gathered PR context - - [x] Code review complete - - [x] Posted inline comments - - [ ] Finalizing summary - - ## Step 5: Update final summary - - This step is MANDATORY and must be your FINAL action before ending — even if - there are zero new findings, even if every finding was already reported in a - previous review thread. NEVER end your turn while the tracking comment still - contains unchecked `- [ ]` checkboxes. If you have nothing else to do, your - last action MUST still be a mcp__github_comment__update_claude_comment call - that replaces the checklist with the final summary below. - - This step is NOT contingent on Steps 2–4 succeeding. If an agent returned - nothing, a tool call was denied, or you hit any error, you MUST still fall - through to here and post the summary — treat the affected finding as absent - and continue. A denied or failed tool is never a reason to end your turn - early. When in doubt, use the "no new findings" summary below. - - After Step 4 is complete, update the tracking comment one final time via - mcp__github_comment__update_claude_comment to become the closing summary. - - When mentioning `/review` in any comment, always wrap it in backticks (`` `/review` ``). - - The PR author ${{ steps.author-check.outputs.is_admin == 'true' && 'IS' || 'is NOT' }} a member of the shellhub-io/admin team. - - ### If there are findings: - - Update the tracking comment to: - - - ## Code Review Complete - - Reviewed N files. **X inline issues** posted (breakdown by severity). - - ### Additional notes - (Only if there are non-inlinable findings — architectural concerns, - missing files, cross-cutting issues not tied to specific lines. - Omit this section entirely if all findings were posted inline.) - - --- - {For admin authors:} - To request another review round, comment `/review`. - {For non-admin authors:} - If you've addressed the feedback and want a new review, tag - `@shellhub-io/admin` and a team member can trigger it. - - ### If there are NO new findings: + You are the lead reviewer for a pull request in shellhub-io/shellhub. - This covers two cases: (a) no issues were found at all, and (b) a re-review - where every finding deduplicated against issues already reported in previous - review threads — i.e. nothing NEW to post. In either case you MUST still update - the tracking comment to: + ## This PR - - ## Code Review Complete + - Repo slug: shellhub-io/shellhub (Community Edition) + - PR number: ${{ github.event.pull_request.number || github.event.issue.number }} + - The PR author ${{ steps.author-check.outputs.is_admin == 'true' && 'IS' || 'is NOT' }} a member of the shellhub-io/admin team. - Reviewed N files across code quality, security, testing, language patterns, - and architecture — no new issues found. The code looks good as-is. + ## Repo specifics - {For non-admin authors only:} - If you push additional changes and want a new review, tag - `@shellhub-io/admin` and a team member can trigger it. + - This repo is checked out at the working directory. Its diff is the review scope. + - Sibling checkout for context: $GITHUB_WORKSPACE/cloud/ (Enterprise/Cloud). Use it to + understand cross-repo impact, never as review scope. + - Anonymous-route allowlist: `server/api/routes/anonymous.go` + - Conventions skills that apply: `go-conventions` for Go, `react-conventions` for `ui/` + - Extra correctness focus: none beyond the rules + - Cross-repo rule: if the PR changes `pkg/` or a `server/api/services` symbol, + check $GITHUB_WORKSPACE/cloud/ for impact — it consumes them through a `replace` + directive, so a rename here breaks its build. - Do NOT mention inline comments or `/review` follow-up when there are no new findings - (unless the author is non-admin, in which case include the tag guidance). + The procedure follows. Execute it exactly. - In both cases, always update the tracking comment. Never skip Step 5. + ${{ steps.review-procedure.outputs.text }} claude_args: | - --max-turns 50 - --model claude-opus-4-8 - --allowedTools "Task" "Read" "Grep" "Glob" "LS" "mcp__github_inline_comment__create_inline_comment" "mcp__github_comment__update_claude_comment" "Bash(gh api:*)" "Bash(gh pr diff:*)" + --max-turns 80 + --model claude-opus-5 + --allowedTools "Task" "Read" "Grep" "Glob" "LS" "mcp__github_inline_comment__create_inline_comment" "mcp__github_comment__update_claude_comment" "Bash(gh api:*)" "Bash(gh pr diff:*)" "Bash(gh pr view:*)" "Bash(gh issue view:*)" - name: Finalize tracking comment if: always() && steps.gate.outputs.proceed == 'true'