Skip to content

ci(review): run the code-review skill's axes on a pull request #7074

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
otavio merged 1 commit into from
Sep 10, 2026
Merged

ci(review): run the code-review skill's axes on a pull request #7074

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
263 changes: 35 additions & 228 deletions .github/workflows/claude-code-review.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<<PR_REVIEW_PROCEDURE_EOF'
cat "$PROCEDURE"
echo 'PR_REVIEW_PROCEDURE_EOF'
} >> "$GITHUB_OUTPUT"

- name: Run Claude Code Review
if: steps.gate.outputs.proceed == 'true'
timeout-minutes: 30
Expand All @@ -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:

<!-- claude-code-review -->
## 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:

<!-- claude-code-review -->
## 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:

<!-- claude-code-review -->
## 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:

<!-- claude-code-review -->
## 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

<!-- claude-code-review -->
## 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'
Expand Down
Loading