/amber-review

name: amber-review
description: >
  Perform a comprehensive code review using repository-specific standards. Use
  when reviewing code changes, checking PR quality, auditing convention compliance,
  or validating changes before merge. Triggers on: "review code", "check changes",
  "code review", "amber review", "review PR", "audit conventions", "quality check".

Amber Review

Stringent, standards-driven code review against this repository's documented patterns, security requirements, and architectural conventions.

User Input

$ARGUMENTS

Consider the user input before proceeding (if not empty). The input may specify files, a PR number, a branch, or a focus area.

Execution Steps

1. Load Review Context

Read all of the following files to build your review context. Do not skip any.

1. `CLAUDE.md` (master project instructions) 2. `specs/standards/backend/conventions.spec.md` (Go backend, Gin, K8s integration) 3. `specs/standards/frontend/conventions.spec.md` (NextJS, Shadcn UI, React Query) 4. `specs/standards/security/security.spec.md` (auth, RBAC, token handling, container security) 5. `specs/standards/backend/k8s-client.spec.md` (user token vs service account) 6. `specs/standards/backend/error-handling.spec.md` (consistent error patterns) 7. `specs/standards/frontend/react-query.spec.md` (data fetching patterns) 8. `specs/standards/control-plane/conventions.spec.md` (K8s operator, reconciliation, OwnerReferences)

2. Identify Changes to Review

Determine the scope based on user input:

• **If a PR number is provided**: Use `gh pr diff <number>` to get the diff
• **If files/paths are provided**: Review those specific files
• **If a branch is provided**: Diff against `main`
• **If no input**: Review all uncommitted changes (`git diff` + `git diff --cached`)

3. Perform Review

Evaluate every changed file against the loaded standards. Apply ALL relevant checks.

Review Axes

1. **Code Quality** — Does it follow CLAUDE.md patterns? Naming conventions? 2. **Security** — User token auth (`GetK8sClientsForRequest`), RBAC checks, token redaction, input validation, SecurityContext on Job pods, no secrets in code 3. **Performance** — Unnecessary re-renders, missing query key parameters, N+1 queries, unbounded list operations 4. **Testing** — Adequate coverage for new functionality? Tests follow existing patterns? 5. **Architecture** — Follows project structure? Correct layer separation? 6. **Error Handling** — No `panic()`, no silent failures, wrapped errors with context, generic user messages with detailed server logs

Backend-Specific Checks (Go)

• [ ] All user operations use `GetK8sClientsForRequest`, never service account fallback
• [ ] No tokens in logs (use `len(token)`)
• [ ] Type-safe unstructured access (`unstructured.NestedMap`, not direct assertions)
• [ ] No `panic()` in production code
• [ ] Errors wrapped with `fmt.Errorf("context: %w", err)`
• [ ] `errors.IsNotFound` handled for 404 scenarios
• [ ] OwnerReferences set on child resources (Jobs, Secrets, PVCs)

Frontend-Specific Checks (TypeScript/React)

• [ ] Zero `any` types (use proper types or `unknown`)
• [ ] Shadcn UI components only (no custom buttons, inputs, dialogs)
• [ ] React Query for all data operations (no manual `fetch()` in components)
• [ ] `type` preferred over `interface`
• [ ] Single-use components colocated with their page
• [ ] Loading and error states handled
• [ ] Query keys include all relevant parameters

Security Checks (All Components)

• [ ] RBAC check performed before resource access
• [ ] No tokens or secrets in logs or error messages
• [ ] Input validated (K8s DNS labels, URL parsing)
• [ ] Log injection prevented (no raw newlines in logged user input)
• [ ] Generic error messages to users, detailed logs server-side
• [ ] Container SecurityContext: `AllowPrivilegeEscalation: false`, `Drop: ALL`

4. Classify Findings by Severity

• **Blocker** — Must fix before merge. Security vulnerabilities, data loss risk, service account misuse, token leaks
• **Critical** — Should fix before merge. RBAC bypasses, missing error handling, `any` types, `panic()` in handlers
• **Major** — Important to address. Architecture violations, missing tests, performance concerns
• **Minor** — Nice-to-have. Style improvements, documentation gaps

5. Produce Review Report

# Claude Code Review

## Summary
[1-3 sentence overview]

## Findings

### Blocker
[Must fix — or "None"]

### Critical
[Should fix — or "None"]

### Major
[Important — or "None"]

### Minor
[Nice-to-have — or "None"]

## Positive Highlights
[Things done well — always include at least one]

## Recommendations
[Prioritized action items]

For each issue, include: file path and line number, what the problem is, which standard it violates, suggested fix.

When to Use This vs Individual Agents

• **`/amber-review`**: Comprehensive single-session review across all components. Best for pre-merge quality gates.
• **Individual agents** (backend-review, frontend-review, operator-review, runner-review, security-review): Specialized checks for a single component. Best for focused work on one area or ongoing automated checks.
• **`/align`**: Codebase-wide convention scoring. Best for periodic health checks.

Operating Principles

• **Be stringent**: This is a quality gate, not a rubber stamp.
• **Be specific**: Reference exact file:line, exact standard, exact fix.
• **Be fair**: Always acknowledge what was done well.
• **No false positives**: Only flag issues backed by loaded standards.
• **Existing code is not in scope**: Only review changed/added lines.