Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 18 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ ls ~/.claude/commands/
|-------|-------------|-------|
| `agent-creator` | Guide for creating custom Claude Code subagents with custom prompts and tools | Use when creating/updating agents |
| `aws-limits` | Reviews infrastructure code for AWS service quota violations | Use when reviewing Terraform/CloudFormation/CDK/Pulumi |
| `auto-review` | Runs the WalletConnect `claude/auto-review` CI review locally against branch changes before opening a PR | `auto-review` or "review my changes before PR" |
| `code-review` | Review code changes for bugs, security issues, and structural problems | `/code-review [guidance]` |
| `code-simplifier` | Simplify and refine code for clarity while preserving functionality | `/code-simplifier` |
| `command-creator` | Guide for creating custom slash commands with arguments and bash execution | Use when creating/updating commands |
Expand Down Expand Up @@ -92,6 +93,15 @@ Reviews infrastructure code for AWS service quota violations before they cause p
- Links to AWS documentation
- Suggests mitigations

#### auto-review
Reproduces the WalletConnect `claude/auto-review` GitHub Action locally, against the current branch's changes, so issues are caught before the PR is opened. Reads the local `git diff` instead of the GitHub PR API and prints findings to the terminal.

**Features:**
- Same review scope and five automated checks as CI (PR size, external-domain URLs, cache-control, GitHub Actions security, WalletConnect Pay architecture)
- Heuristic-gated subagents: breaking-changes, license-compliance, data-classification
- Read-only, issues-only, severity-ranked output (Critical > High > Medium > Low)
- `✅ No issues found` when clean

#### code-review
Reviews code changes using parallel subagents to analyze bugs/logic, security/auth, and patterns/structure. Automatically detects AWS infrastructure files and runs service quota checks.

Expand Down Expand Up @@ -622,6 +632,14 @@ skills/ # Repository root
│ ├── aws-limits/
│ │ ├── SKILL.md
│ │ └── REFERENCE.md
│ ├── auto-review/
│ │ ├── SKILL.md
│ │ ├── HEURISTICS.md
│ │ ├── REVIEW_PROMPT.md
│ │ └── agents/
│ │ ├── review-breaking-changes.md
│ │ ├── review-data-classification.md
│ │ └── review-license-compliance.md
│ ├── code-review/
│ │ └── SKILL.md
│ ├── code-simplifier/
Expand Down
62 changes: 62 additions & 0 deletions skills/auto-review/HEURISTICS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# Subagent Spawn Heuristics

Mirrors the `should-spawn-*.js` scripts in the action. Apply each to the changed-file list (paths + status) and patch text. Each produces `{ spawn, reason }`. Evaluate rules **top to bottom; first match wins**.

Inputs per file: `filename`, `status` (`added`/`modified`/`removed`), `patch` (the diff hunk text). Labels don't exist locally — treat the `skip-review` / `breaking` label rules as N/A unless the user explicitly says "skip review" or "this is a breaking change". The user can force any agent on ("force breaking changes", "force license", "force data classification").

## Shared skip patterns

- **Docs-only:** every changed file matches `/\.(md|txt|rst|adoc)$/i` → skip the breaking-changes and data-classification agents.
- **Test-only:** every changed file matches `/(\/__tests__\/|\.test\.|\.spec\.|test\/|tests\/|__mocks__\/)/i` → skip breaking-changes and data-classification.

(License compliance has its own narrow trigger and ignores the docs/test skips.)

## 1. Breaking changes (`brk-`)

1. User says skip → `spawn: false`.
2. Forced → `spawn: true` ("forced via input").
3. No files → `spawn: false`.
4. User flags it breaking → `spawn: true` ("breaking label").
5. All docs-only → `spawn: false`. All test-only → `spawn: false`.
6. Otherwise collect signals; if any present → `spawn: true` with the joined reasons:
- `action.yml` files: `/action\.ya?ml$/i`
- workflow files: `/\.github\/workflows\/.*\.ya?ml$/`
- package manifests: `/package\.json$|go\.mod$|setup\.py$|pyproject\.toml$|Cargo\.toml$/`
- type definitions: `/\.d\.ts$|types?\.(ts|js)$|interfaces?\.(ts|js)$/i`
- API routes: `/routes?\.[jt]sx?$|controllers?\.[jt]sx?$|handlers?\.[jt]sx?$|api\//i`
- schema/migration files: `/schema|migration|\.sql$/i`
- **removed files:** any file with `status === 'removed'`
- **breaking-change keywords in patch** (case-insensitive): `inputs:`, `outputs:`, `required:`, `default:`, `deprecated`, `export (default )?(function|class|const|interface|type|enum)`, `module.exports`, `"main"`, `"exports"`, `"bin"`, `"engines"`, `"peerDependencies"`
7. No signals → `spawn: false`.

## 2. License compliance (`lic-`)

1. Forced → `spawn: true`.
2. User says skip → `spawn: false`.
3. No files → `spawn: false`.
4. `spawn: true` if any changed file's **basename** is a dependency manifest or lockfile:
- npm: `package.json`, `pnpm-lock.yaml`, `yarn.lock`, `package-lock.json`
- Go: `go.mod`, `go.sum`
- Rust: `Cargo.toml`, `Cargo.lock`
- Python: `pyproject.toml`, `setup.py`, `setup.cfg`, and `requirements*.txt`
- Ruby: `Gemfile`, `Gemfile.lock`
- PHP: `composer.json`
- Java/Kotlin: `build.gradle`, `pom.xml`
- Reason: `Dependency files changed: <basename> (<ecosystem>), ...`
5. Else → `spawn: false`. (Docs/test skips do **not** apply here.)

## 3. Data classification (`dcl-`)

1. Forced → `spawn: true`.
2. User says skip → `spawn: false`.
3. No files → `spawn: false`.
4. All docs-only → `spawn: false`. All test-only → `spawn: false`.
5. Collect signals; if any present → `spawn: true`:
- Terraform/IaC files: `/\.tf$|\.tfvars$/`
- Kubernetes/Helm configs: `/\.(ya?ml)$/i` **AND** path matches `/k8s|kubernetes|helm|chart|deploy|manifests?/i` (a generic `.yml` like a CI workflow does **not** trigger)
- CloudFormation templates: `/cloudformation|cfn/i`
- environment/secret files: `/(^|\/)\.env|secret|credential/i`
- database/schema files: `/migration|schema|model/i`
- API route/handler files: `/routes?\.[jt]sx?$|controllers?\.[jt]sx?$|handlers?\.[jt]sx?$|middleware\.[jt]sx?$|api\//i`
- **sensitive keywords in patch** (case-insensitive): `password`, `secret`, `api_key`/`apiKey`, `private_key`/`privateKey`, `credential`, `token`, `encrypt`, `decrypt`, `kms`, `AES`, `TLS`, `email`, `phone`, `ssn`, `date_of_birth`/`dateOfBirth`, `personal`, `pii`, `gdpr`, `console.log`, `logger.`, `log.`, `logging`
6. No signals → `spawn: false`.
122 changes: 122 additions & 0 deletions skills/auto-review/REVIEW_PROMPT.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
# Main Review Prompt

This mirrors the core review prompt from the `claude/auto-review` action, adapted for a local pre-PR run. Apply it after gathering the diff (see SKILL.md step 1).

## Critical constraints

Be extremely concise; sacrifice grammar for concision. Read-only: no shell commands, builds, or execution. Issues-only: report problems, never praise. If no issues: `✅ No issues found`.

## Review scope

Focus on:

- Code quality / best practices for the project's technologies
- Bugs (especially critical paths, async ops)
- Performance (frontend / backend)
- Security (auth, APIs, data handling)
- Test coverage / quality
- Type safety, error handling, edge cases
- Maintainability / readability

**Diffs alone are not enough.** Read full file(s) to understand context. Code that looks wrong in isolation may be correct given surrounding logic.

## Automated checks

**Only report if violations are found. Skip a check if nothing is detected.**

### PR Size Assessment (check FIRST)

Flag if **>15 files OR >800 lines changed**:

> 🚨 **PR Too Large** **Files:** [N] **Lines:** [N] **Severity:** HIGH **Category:** maintainability
> This PR is doing too much. Suggest 2–4 focused PRs split by (1) logical concern (refactoring vs features vs infra vs deps) and (2) file/directory groupings. Format: "PR 1: [description] - [files]".

### External Domain URLs

Flag URLs to domains other than `reown.com`, `walletconnect.com`, `walletconnect.org`:

> 🔒 **External Domain URL** (Non-blocking) **URL:** [url] **File:** [path:line] — Verify intentional, review security implications.

### Static Resource Cache-Control

Flag static files (`.woff`, `.woff2`, `.ttf`, `.jpg`, `.png`, `.css`, `.js`, `.mp4`, etc.) with `max-age < 31536000` or missing explicit `Cache-Control`:

> ⚠️ **Cache-Control Issue** **Resource:** [url] **File:** [path:line] **Current:** [value] **Recommendation:** "Cache-Control: public, max-age=31536000, immutable"

### GitHub Actions Workflow Security

Scan `.github/workflows/*.y*ml` for:

- **CRITICAL:** `pull_request_target` + PR head checkout (`github.event.pull_request.head.*`) = arbitrary code execution
- **HIGH:** `pull_request_target` + script execution
- **MEDIUM:** Any `pull_request_target` usage (runs with secrets)

> 🚨 **GitHub Actions Security Risk** **Severity:** [level] **File:** [path:line] **Pattern:** [issue] **Recommendation:** [fix]

### WalletConnect Pay Architecture

Flag anti-patterns in payment / wallet / transaction code:

1. **CRITICAL:** Cross-service DB access (imports, queries, connections) → 🚨 Services must use APIs
2. **HIGH:** Missing idempotency keys in POST/PUT/PATCH/DELETE → ⚠️ Extract key, check store, return cached response
3. **HIGH:** External calls without timeout/retry → ⚠️ Add timeout, retry+backoff, circuit breaker
4. **HIGH:** Event consumers (SQS/SNS/Kafka) without message deduplication → ⚠️ Check message ID before mutations
5. **MEDIUM:** Multi-step workflows without saga compensation → ⚠️ Add rollback/compensating events
6. **MEDIUM:** State transitions without trace context → ⚠️ Add structured logging with traceId/correlationId

## Response format

Be concise — ONLY report issues that need fixing. If no issues: `✅ No issues found`. Wrap ALL issues in a collapsed `<details>` section. No praise, issues-only.

````markdown
<details>
<summary>Found N issue(s)</summary>

#### Issue 1: Brief description
**ID:** {file-slug}-{semantic-slug}-{hash}
**File:** path/to/file.ext:123
**Severity:** CRITICAL/HIGH/MEDIUM/LOW
**Category:** security/performance/code_quality/breaking_change

**Context:**
- **Pattern:** What the problematic code pattern is
- **Risk:** Why it's a problem technically
- **Impact:** Potential consequences (exploit, data loss, etc.)
- **Trigger:** Under what conditions this becomes exploitable

**Recommendation:** Fix with minimal code snippet (1–10 lines).
</details>
````

**ID Generation:** `{filename}-{2-4-key-terms}-{SHA256(path+desc).substr(0,4)}` — example: `login-sql-injection-f3a2`.

**Recommendation guidelines:** Include focused code snippets showing the exact fix. DO show specific changes needed. DON'T provide full implementations or boilerplate.

**Rules:** Use "Issue N:" not "#N". Include line numbers. Include code snippets in recommendations.

**Feedback style:** Constructive feedback with specific suggestions. Consider impact on system architecture and user experience. Focus exclusively on problems and their solutions.

### Worked example

````markdown
<details>
<summary>Found 1 issue(s)</summary>

#### Issue 1: SQL injection in user query
**ID:** users-sql-injection-f3a2
**File:** src/database/users.ts:45
**Severity:** HIGH
**Category:** security

**Context:**
- **Pattern:** Query at line 45 builds SQL via string concatenation with user-provided `userId`
- **Risk:** Allows arbitrary SQL injection via crafted input (e.g., `1' OR '1'='1`)
- **Impact:** Unauthorized data access, modification, or database destruction
- **Trigger:** Any endpoint accepting user input that reaches this query

**Recommendation:** Use parameterized queries:
```typescript
const result = await db.query('SELECT * FROM users WHERE id = $1', [userId]);
```
</details>
````
102 changes: 102 additions & 0 deletions skills/auto-review/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
---
name: auto-review
description: Runs a local Claude code review of branch changes before opening a PR, mirroring the WalletConnect/actions claude/auto-review GitHub Action. Same review scope, the same automated checks (PR size, external-domain URLs, cache-control, GitHub Actions security, WalletConnect Pay architecture), and the same conditional breaking-changes / license-compliance / data-classification subagents — but reads the local git diff instead of the GitHub PR API and prints findings to the terminal. Use when the user says "auto-review", "review my changes before PR", "run the auto review locally", "pre-PR review", or wants the CI review run before pushing.
---

# Auto Review (local)

## Goal

Reproduce the WalletConnect `claude/auto-review` GitHub Action **locally**, against the current branch's changes, so issues are caught before the PR is opened. Same review scope, same automated checks, same severity/ID output format, same three conditional subagents — just sourced from `git diff` instead of the GitHub PR API.

## When to use

- Before opening or pushing a PR, to preview what CI auto-review will flag.
- The user says "auto-review", "review my changes", "pre-PR review", "run the auto review locally".

## When not to use

- Reviewing an already-open PR on GitHub — use the CI action or `gh`-based review instead; this skill is for local, pre-push changes.
- General "explain this code" requests with no review intent.

## Critical constraints (match the action exactly)

- **Read-only.** No builds, no test runs, no shell mutations. Inspection only.
- **Issues-only.** Report problems, never praise. If nothing is found, say `✅ No issues found`.
- **Be extremely concise.** Sacrifice grammar for concision.
- **Diffs alone are not enough.** `Read` the full file around each change — code that looks wrong in isolation may be correct in context.
- This is a **full review** every run. The action's "incremental review" mode (reusing IDs from prior PR comments) does **not** apply locally — there are no prior comments.

## Workflow

### 1. Determine the diff scope

Establish the base and collect changed files + patches. Default base is the repo's main branch (`main`, else `master`). Let the user override (e.g. "review against develop").

```bash
# Base ref (prefer the tracked upstream's base; fall back to origin/main then main)
BASE="${BASE:-$(git rev-parse --abbrev-ref --symbolic-full-name @{u} 2>/dev/null | sed 's#/.*##' >/dev/null; echo origin/main)}"
git fetch origin --quiet 2>/dev/null || true

# Changed files (committed-on-branch + staged + unstaged) vs the merge-base
MB=$(git merge-base HEAD "$BASE" 2>/dev/null || git rev-parse HEAD)
git diff --name-status "$MB" -- ; echo "--- working tree ---" ; git status --porcelain
```

Use whichever combination reflects "what this PR will contain." If the branch has commits, diff `merge-base..HEAD`; if work is uncommitted, include the working tree. Get full patches with `git diff "$MB"` (and `git diff` / `git diff --staged` for uncommitted work).

If there are no changes, output `✅ No issues found` and stop.

### 2. Decide which subagents to spawn (heuristic gating)

Apply the heuristics in **[HEURISTICS.md](HEURISTICS.md)** to the changed-file list + patch text. Each yields a `{ spawn, reason }` decision:

- **Breaking changes** — `action.yml`/workflows, package manifests, type defs, API routes, schema/migration files, removed files, or breaking-change keywords in the patch.
- **License compliance** — any dependency manifest or lockfile changed.
- **Data classification** — IaC/Terraform, k8s/Helm, CloudFormation, env/secret files, db schema, API handlers, or sensitive-data keywords in the patch.

Skip-all if every changed file is docs-only or test-only (per the rules in HEURISTICS.md). The user can force any subagent on ("force breaking changes", etc.). State each decision briefly, e.g. `breaking-changes: spawn (package manifests, removed files)`.

### 3. Run the main review

Follow **[REVIEW_PROMPT.md](REVIEW_PROMPT.md)** — the review scope and the five automated checks (PR size, external-domain URLs, cache-control, GitHub Actions security, WalletConnect Pay architecture). Use `Read`, `Glob`, `Grep` to inspect full files; do not rely on the diff alone.

### 4. Spawn the gated subagents (in parallel)

For each subagent whose decision was `spawn: true`, launch a `general-purpose` agent via the Agent tool. Pass it:

1. "Read your spec file at `<this skill's directory>/agents/<spec>.md` — the `auto-review` skill folder containing this SKILL.md (e.g. `~/.claude/skills/auto-review/agents/<spec>.md` when installed there) — and follow its instructions."
2. The base ref and the list of changed files (with paths).
3. Instruction to review the **local working tree / branch diff**, not a GitHub PR.

Spec files: `agents/review-breaking-changes.md` (IDs `brk-`), `agents/review-license-compliance.md` (IDs `lic-`), `agents/review-data-classification.md` (IDs `dcl-`).

Launch all gated subagents in a single batch so they run concurrently.

### 5. Merge and output

Merge subagent findings into the main review's findings:

- Keep each subagent's prefixed IDs (`brk-`/`lic-`/`dcl-`) as-is.
- Deduplicate when the main review found the same issue independently — prefer the prefixed ID.
- Sort all findings by severity: **CRITICAL > HIGH > MEDIUM > LOW**.

Print the consolidated result to the terminal using the issue format in REVIEW_PROMPT.md (wrapped in a `<details>` block). If there are genuinely no issues across the main review and all subagents, output exactly `✅ No issues found`.

There is no inline-PR-comment step locally — the action's `findings.json` → GitHub comments flow is replaced by terminal output. If the user wants a file, write the findings to `auto-review-findings.md` in the repo root.

## Validation checklist

- [ ] Diff scope reflects what the PR will contain (branch commits and/or working tree), against the correct base.
- [ ] All three heuristics evaluated; spawn decisions + reasons stated.
- [ ] Full files read for flagged lines, not just the diff.
- [ ] All five automated checks considered (report only on violations).
- [ ] Findings carry ID, File:line, Severity, Category, Context, Recommendation.
- [ ] Subagent IDs keep their prefixes; duplicates merged; sorted by severity.
- [ ] No praise; `✅ No issues found` when clean.

## Notes on parity with CI

- The action defaults to model `claude-sonnet-4-6` and allows tools `Read, Glob, Grep, Task, WebFetch`. Locally, mirror the read-only tool set.
- External-domain allowlist is WalletConnect-specific: `reown.com`, `walletconnect.com`, `walletconnect.org`. Flag URLs to other domains as non-blocking.
- The WalletConnect Pay architecture check is intentionally project-specific (cross-service DB access, idempotency keys, timeouts/retries, message dedup, saga compensation, trace context).
Loading
Loading