code-reviewer

Code Reviewer

A mechanical, checklist-driven scan: walk every category below in order, record every match with a severity, then emit the report. This is a scanner, not a workflow — it does not cover change sizing, review etiquette, or process (see code-review-and-quality for that). It does not walk a security-priority checklist end to end (see security-review for that). Its distinguishing category is concurrency, which the other review skills in this catalog do not cover.

Category checklist

Work through every category for every changed file. Don't stop at the first finding — a diff with one CRITICAL often has MEDIUM issues too.

Correctness

• Logic bugs, off-by-one errors, wrong boundary conditions
• Missing nil/zero-value checks and unhandled edge cases
• Incorrect assumptions about a library's behavior — check the actual doc/signature, not intuition (e.g., does strings.TrimSuffix no-op silently if the suffix doesn't match? yes — a common mis-assumption)

Security

• SQL/command/template injection via unsanitized input
• Hardcoded secrets, credentials, or API keys
• Insecure cryptographic choices (MD5/SHA1 for anything security-sensitive, ECB mode, math/rand for tokens)
• Missing authentication or per-resource authorization on new endpoints
• Path traversal, open redirects, SSRF-shaped calls (server fetches a user-influenced URL)

Error handling

• Errors silently swallowed (_ = err) or logged but not propagated
• Missing cleanup on error paths — defer f.Close() placed before the error check, not after
• Panic recovery missing in goroutines (an unrecovered panic in a goroutine crashes the whole process, not just that goroutine)
• Errors wrapped with %w losing sentinel-comparability, or not wrapped at all, losing context

Concurrency (Go)

• Data races: map or slice read/write from multiple goroutines without a mutex or sync.Map. Don't just read the code — run go test -race and check the diff's package is covered.
• Goroutine leaks: a goroutine started with no way for it to ever return — no context.Context, no done channel, no bound on the channel it's blocked sending to.
• Deadlocks: locks acquired in inconsistent order across two code paths; a channel send/receive with no counterpart and no select with a ctx.Done() case.
• mu.Lock() not immediately followed by defer mu.Unlock() on the next line — anything between them is a window where an early return skips the unlock.
• A WaitGroup.Add call issued inside the goroutine it's supposed to be counting, instead of before go func(){...}() — a benign-looking reorder that reintroduces the race go vet's loopclosure/-race won't always catch on the first run.