Skip to content

Commit 89632aa

Browse files
committed
Improve code review quality and workflow
Integrates superpowers code-reviewer for more thorough reviews and switches to natural conversation flow. Reviews now post new comments instead of updating a single comment, with inline comments for code issues and summary comments for architectural concerns.
1 parent 98c3300 commit 89632aa

File tree

4 files changed

+93
-61
lines changed

4 files changed

+93
-61
lines changed

.claude/settings.json

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
{
2+
"extraKnownMarketplaces": {
3+
"superpowers-marketplace": {
4+
"source": {
5+
"source": "github",
6+
"repo": "obra/superpowers-marketplace"
7+
}
8+
}
9+
},
10+
"enabledPlugins": {
11+
"superpowers@superpowers-marketplace": true
12+
},
13+
"enabledMcpjsonServers": ["cloudflare-docs", "exa"],
14+
"permissions": {
15+
"allow": [
16+
"Bash(npm test:*)",
17+
"Bash(npm run typecheck:*)",
18+
"Bash(npm run check:*)"
19+
]
20+
}
21+
}

.github/workflows/claude-code-review.yml

Lines changed: 43 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ jobs:
3333
uses: anthropics/claude-code-action@v1
3434
with:
3535
anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}
36+
plugin_marketplaces: |
37+
obra/superpowers-marketplace
38+
plugins: |
39+
superpowers@superpowers-marketplace
3640
prompt: |
3741
You are Claude reviewing PRs for the Cloudflare Sandbox SDK.
3842
@@ -52,52 +56,51 @@ jobs:
5256
5357
**Then review the diff with that context.**
5458
55-
### 2. Check for Previous Review (if this is an update)
59+
### 2. Check for Previous Review (Internal Context Only)
5660
57-
**Detect if this is a PR update:**
58-
- Check for existing claude[bot] comment: `gh pr view ${{ github.event.pull_request.number }} --json comments --jq '.comments[] | select(.author.login == "claude[bot]" or .author.login == "github-actions[bot]")'`
61+
Check if you've reviewed this PR before:
62+
```bash
63+
gh pr view ${{ github.event.pull_request.number }} --json comments --jq '.comments[] | select(.author.login == "claude[bot]" or .author.login == "github-actions[bot]")'
64+
```
5965
60-
**If previous review exists:**
61-
- Identify what's new: `git log --oneline ${{ github.event.before }}..${{ github.event.after }}` (shows commits since last review)
62-
- Check if history was rewritten: `git cat-file -t ${{ github.event.before }} 2>/dev/null` (if fails, force push occurred)
63-
- Read the previous review to understand what was flagged
66+
If previous review exists:
67+
- Read it to understand what you said before
68+
- Note what commits have been added since: `git log --oneline ${{ github.event.before }}..${{ github.event.after }}`
69+
- Check if history was rewritten: `git cat-file -t ${{ github.event.before }} 2>/dev/null`
70+
- Use this context internally when writing your review
6471
65-
**Your review should cover BOTH:**
66-
1. **Incremental**: What changed since last review? Were previous issues addressed?
67-
2. **Holistic**: How does the FULL PR look now? Any issues when viewing the complete picture?
72+
**Don't announce "Update N" or link to previous reviews** - just write naturally.
6873
6974
### 3. Gather Context
7075
- Use `gh pr view ${{ github.event.pull_request.number }}` for PR description
7176
- Use `gh pr diff ${{ github.event.pull_request.number }}` for FULL diff (not just incremental)
7277
- Read CLAUDE.md for repo-specific conventions and architecture patterns
7378
- Use Read tool to examine key changed files
7479
75-
### 4. Internal Analysis
80+
### 4. Check Documentation Impact
81+
Use `mcp__cloudflare-docs__search_cloudflare_documentation` to check if this PR requires doc updates:
82+
- Does it change existing documented behavior?
83+
- Does it add new features needing documentation?
84+
- Does it have security implications needing best practices docs?
85+
- If yes, call out specific doc sections that need updates
86+
87+
### 5. Internal Analysis
7688
Before writing your review, analyze inside <analysis> tags (do NOT include in final comment):
7789
a. What is this PR accomplishing?
7890
b. Which packages/layers are affected?
7991
c. Are there architectural or security implications?
8092
d. What tests are needed?
8193
e. Does this need documentation updates?
8294
83-
### 5. Check Documentation Impact
84-
Use `mcp__cloudflare-docs__search_cloudflare_documentation` to check if this PR requires doc updates:
85-
- Does it change existing documented behavior?
86-
- Does it add new features needing documentation?
87-
- Does it have security implications needing best practices docs?
88-
- If yes, call out specific doc sections that need updates
95+
### 6. Execute Review
8996
90-
### 6. Review Focus
97+
Launch superpowers:code-reviewer subagent (Task tool) with the context gathered above:
98+
- Full PR diff from step 3
99+
- Relevant architecture patterns from CLAUDE.md
100+
- Documentation requirements from step 4
101+
- Previous review feedback (if incremental update from step 2)
91102
92-
**Prioritize** (things that slip through):
93-
- Missing/inadequate tests (especially E2E for container interactions)
94-
- Type safety violations (see CLAUDE.md TypeScript standards)
95-
- Architecture violations (see CLAUDE.md patterns: client pattern, DI, layer separation)
96-
- API design issues (see CLAUDE.md API Design: unclear naming, missing validation, poor error messages)
97-
98-
**Skip**:
99-
- Code style/formatting (Biome handles it)
100-
- Changeset reminders (bot handles it)
103+
The code-reviewer will analyze correctness, architecture, testing, and code quality.
101104
102105
### 7. Post Review
103106
@@ -108,38 +111,24 @@ jobs:
108111
- If issues found: State the issue, reference location, explain why it matters. Move on.
109112
110113
**Format**:
111-
- Reference code with file paths and line numbers (e.g., `packages/sandbox/src/file.ts:123`)
112-
- Use markdown headings (### for sections) only if you have multiple distinct categories
114+
- **Inline comments** (using `mcp__github_inline_comment__create_inline_comment`): Use for specific line-by-line code issues
115+
- **Main comment**: Summary only - overall assessment, architectural concerns, testing strategy, verdict. Do NOT duplicate inline comments here.
113116
114-
**For incremental reviews** (when updating existing comment):
115-
- Acknowledge fixed issues: "✓ [Previous issue] - addressed"
116-
- Flag new issues found in new changes
117-
- Flag any issues that emerged from holistic view
118-
- Keep unaddressed issues from previous review
117+
**Main comment - write naturally:**
118+
- Start with "## Claude Code Review"
119+
- If previous review exists, write conversationally: "The encoding issue is fixed. Tests look better too."
120+
- Mention what still needs work: "I still see the over-mocking in file-service.test.ts:45"
121+
- New concerns: "New issue: error handling in container.ts doesn't cover..."
122+
- Overall verdict: "Looks good" or "Needs fixes before merge"
119123
120-
**Posting your review**:
124+
**Post inline comments** for specific code issues using `mcp__github_inline_comment__create_inline_comment`
121125
122-
First, check for existing review comment:
126+
**Post your main comment:**
123127
```bash
124-
COMMENT_ID=$(gh api repos/${{ github.repository }}/issues/${{ github.event.pull_request.number }}/comments \
125-
--jq '.[] | select(.user.login == "claude[bot]") | select(.body | startswith("## Claude Code Review")) | .id' | head -1)
128+
gh pr comment ${{ github.event.pull_request.number }} --body "YOUR_REVIEW_HERE" --repo ${{ github.repository }}
126129
```
127130
128-
If COMMENT_ID exists (previous review comment found):
129-
- Update it: `gh api repos/${{ github.repository }}/issues/comments/$COMMENT_ID -X PATCH -f body="YOUR_REVIEW_HERE"`
130-
- Make sure your review starts with "## Claude Code Review" to maintain consistency
131-
132-
If no existing comment:
133-
- Create new: `gh pr comment ${{ github.event.pull_request.number }} --body "YOUR_REVIEW_HERE"`
134-
- Start your review with "## Claude Code Review" header
135-
136-
For inline code issues:
137-
- Use `mcp__github_inline_comment__create_inline_comment` to highlight specific code issues
138-
139-
Important: Only post GitHub comments - don't submit review text as messages.
140-
141-
Your goal: Catch real issues. Respect the developer's time - be concise, but don't omit important details.
142-
131+
Always post a NEW comment - never update previous ones. Natural conversation flow.
143132
# See https://github.com/anthropics/claude-code-action/blob/main/docs/usage.md
144133
# or https://docs.claude.com/en/docs/claude-code/cli-reference for available options
145-
claude_args: '--allowedTools "mcp__github_inline_comment__create_inline_comment,mcp__cloudflare-docs__search_cloudflare_documentation,mcp__exa__get_code_context_exa,mcp__exa__web_search_exa,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh api:*)"'
134+
claude_args: '--allowedTools "Task,Skill,mcp__github_inline_comment__create_inline_comment,mcp__cloudflare-docs__search_cloudflare_documentation,mcp__exa__get_code_context_exa,mcp__exa__web_search_exa,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh api:*),Bash(git *)"'

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,9 @@ tests/e2e/test-worker/wrangler.jsonc
137137

138138
.DS_Store
139139

140+
# Claude Code user-specific settings
141+
.claude/settings.local.json
142+
140143
# Turborepo cache
141144
.turbo
142145

CLAUDE.md

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -286,18 +286,37 @@ Turbo handles task orchestration (`turbo.json`) with dependency-aware builds.
286286

287287
**Be concise, not verbose.** Every word should add value. Avoid unnecessary details about implementation mechanics - focus on what changed and why it matters.
288288

289-
Example:
289+
**Subject line should stand alone** - don't require reading the body to understand the change. Body is optional and only needed for non-obvious context.
290+
291+
Good examples:
290292

291293
```
292294
Add session isolation for concurrent executions
295+
```
296+
297+
```
298+
Fix encoding parameter handling in file operations
299+
300+
The encoding parameter wasn't properly passed through the validation
301+
layer, causing base64 content to be treated as UTF-8.
302+
```
293303

294-
Previously, multiple concurrent exec() calls would interfere with each
295-
other's working directories and environment variables. This adds proper
296-
session management to isolate execution contexts.
304+
Bad examples:
305+
306+
```
307+
Update files
308+
309+
Changes some things related to sessions and also fixes a bug.
310+
```
311+
312+
```
313+
Add file operations support
297314
298-
The SessionManager tracks active sessions and ensures cleanup when
299-
processes complete. This is critical for multi-tenant scenarios where
300-
different users share the same sandbox instance.
315+
Implements FileClient with read/write methods and adds FileService
316+
in the container with a validation layer. Includes comprehensive test
317+
coverage for edge cases and supports both UTF-8 text and base64 binary
318+
encodings. Uses proper error handling with custom error types from the
319+
shared package for consistency across the SDK.
301320
```
302321

303322
## Important Patterns

0 commit comments

Comments
 (0)