# Code Review > Lifecycle code review Go/Rust/TS/Python. Auto-detects toolchain, runs static analysis, emits severity-ranked findings + file:line evidence, diffs, risk tags (SAFE/REVIEW/BREAKING). Covers OWASP Top 10, perf anti-patterns, test quality. Read-only — every finding cites file:line. Triggers: 'review code', 'revisar código', 'auditar código', '/code-review'. - **Type:** Skill - **Install:** `agentstack add skill-bruno-cunha-souza-valarmindskills-code-review` - **Verified:** Yes — security-reviewed for prompt injection and unsafe behavior - **Seller:** [Bruno-Cunha-Souza](https://agentstack.voostack.com/s/bruno-cunha-souza) - **Installs:** 0 - **Category:** [Agent Skills](https://agentstack.voostack.com/c/agent-skills) - **Latest version:** 0.1.0 - **License:** MIT - **Upstream author:** [Bruno-Cunha-Souza](https://github.com/Bruno-Cunha-Souza) - **Source:** https://github.com/Bruno-Cunha-Souza/ValarMindSkills/tree/main/skills/code-review ## Install ```sh agentstack add skill-bruno-cunha-souza-valarmindskills-code-review ``` Requires the [AgentStack CLI](https://agentstack.voostack.com/docs/cli). Works with Claude Code, Cursor, and any MCP-compatible agent. ## About # Code Review Lifecycle > "Code is read far more often than it is written. A review is the first reading after the first write." — adapted from the Go proverbs. This skill conducts a structured, evidence-first review of a code change set. It is **read-only by default** — it never edits code, it produces a report. It is **language-aware** for Go, Rust, TypeScript (Node and Bun), and Python (CPython 3.13 / 3.14 on FastAPI / Django / Flask); other languages are best-effort using the generic principles. It is **lifecycle-driven**: detect → sweep → read → assess → report → cross-link. The skill exists because LLM reviewers tend to hallucinate findings: invented function names, wrong file paths, fabricated CVEs, and severity inflation. Every guardrail in the Constraints section is there to push back on those failure modes. ## When to Use - A pull request is open and the user wants a thorough review before approving or merging. - A specific commit, branch, or directory needs an audit (security, performance, maintainability, or all three). - Legacy code is about to be refactored and the user wants a baseline assessment. - A pre-release gate confirms the diff matches the quality level the team thinks they are running at. - An incident post-mortem revealed a class of bug and the user wants the surrounding code swept for it. - The user explicitly asks: `'review code'`, `'code review'`, `'revisar código'`, `'PR review'`, `'auditar código'`, or invokes `/valarmindskills:code-review`. ## Do not use when - The user wants to **fix** code — this skill never edits. Hand off to the user or to `@code-debugger` for runtime issues. - The user wants a **commit message** or **release notes** — use `@github-commit` or `@github-release-note`. - The change is a single typo, comment edit, or trivial rename — review overhead exceeds the value; tell the user and stop. - The change is in a language the skill cannot detect (not Go, Rust, TypeScript, Python, or covered by an explicit `@` skill). Surface the gap and ask whether the user wants a generic pass. - The user's primary ask is to **run tests**, reproduce a failure, or debug runtime behavior — use `@code-debugger`. This skill may run optional verification commands only when they are explicitly requested or needed to validate a review finding. - The diff is on infrastructure (Terraform, Kubernetes manifests) — use `@code-security-review` (Next branch — `references/nextjs/`; Go branch — `references/golang/`; Python branch — `references/python/`) or `@ci-cd-generator` for those domains. ## Prerequisites Install before starting a review. Each tool's absence is logged and the related Phase is degraded but never silently skipped. The default mode is **static review**: read diffs, run linters, type checks, SAST, and dependency audit. Tests and runtime commands are **optional verification**, not part of the default review. | Tool | Purpose | | --- | --- | | `git` | Diff and blame inspection | | `gh` (GitHub CLI) | Pull request metadata, review comments | | `rg` (ripgrep) | Pattern sweep across the diff | | `fd` | Fast file finder | | `jscpd` | Multi-language clone detection | | `semgrep` | Polyglot SAST with rules per language | | `golangci-lint` | Go meta-linter (50+ linters) | | `staticcheck` | Go advanced static analysis | | `govulncheck` | Go CVE scan | | `cargo clippy` | Rust idiomatic linter | | `cargo audit` | Rust CVE scan | | `cargo deny` | Rust dependency policy | | `tsc` | TypeScript compiler (`--noEmit`) | | `eslint` / `biome` | TypeScript linter | | `knip` | Find unused TS exports/files/deps | | `npm audit` / `bun audit` | Node/Bun CVE scan | | `ruff` | Python lint + format (replaces flake8/black/isort/most of pylint) | | `mypy` / `pyright` | Python strict type check | | `bandit` | Python SAST (CWE-mapped) | | `pip-audit` / `safety` | Python CVE scan | | `pytest` | Python test runner (optional verification only) | | Test runners (`go test`, `cargo test`, `bun test`, `vitest`, `pytest`) | Optional verification only | Required access: - [ ] Read access to the repository and the diff (locally or via `gh pr diff`) - [ ] Permission to invoke linters, type checks, and dependency scanners on the host - [ ] Explicit permission or user request before running tests or other runtime verification - [ ] If the review targets a private dependency: read access to that module The skill does **not** require write access. It never commits, never pushes, never edits source files. ## Phase 0 — Project & Scope Detection Detect language, package manager, review scope, and diff range before sweeping anything. Run the steps in order; stop at the first conclusive match per axis. ```bash # Step 1 — language at the repo root test -f go.mod && echo "language: go" test -f Cargo.toml && echo "language: rust" test -f package.json && echo "language: typescript" test -f tsconfig.json && echo " ts-config: present" test -f pyproject.toml && echo "language: python" test -f requirements.txt && echo "language: python (legacy manifest)" # Step 2 — TypeScript runtime (only if language=typescript) test -f bun.lockb && echo "runtime: bun, pm: bun" test -f pnpm-lock.yaml && echo "runtime: node, pm: pnpm" test -f yarn.lock && echo "runtime: node, pm: yarn" test -f package-lock.json && echo "runtime: node, pm: npm" # Step 2b — Python package manager (only if language=python) test -f uv.lock && echo " pm: uv" test -f poetry.lock && echo " pm: poetry" test -f Pipfile.lock && echo " pm: pipenv" # Step 3 — review scope and base branch git rev-parse --abbrev-ref HEAD gh pr view --json number,title,baseRefName,headRefName 2>/dev/null BASE_REF="$(gh pr view --json baseRefName --jq .baseRefName 2>/dev/null || git symbolic-ref --short refs/remotes/origin/HEAD 2>/dev/null | sed 's@^origin/@@' || echo main)" BASE_REMOTE="origin/$BASE_REF" BASE_SHA="$(git merge-base "$BASE_REMOTE" HEAD 2>/dev/null || git merge-base "$BASE_REF" HEAD)" DIFF_RANGE="$BASE_SHA...HEAD" git diff --name-only "$DIFF_RANGE" | wc -l git diff --shortstat "$DIFF_RANGE" # Step 4 — polyglot or monorepo fd -t f -d 5 '^(go.mod|Cargo.toml|package.json|pyproject.toml)$' . # multiple roots → monorepo ``` Persist as `$LANG ∈ {go, rust, typescript, python, polyglot, other}`, `$BASE_REF`, `$BASE_SHA`, and `$DIFF_RANGE`. | `$LANG` | Reference to load | Primary linter | | --- | --- | --- | | `go` | [references/GOLANG.md](references/GOLANG.md) | `golangci-lint` | | `rust` | [references/RUST.md](references/RUST.md) | `cargo clippy` | | `typescript` | [references/TYPESCRIPT.md](references/TYPESCRIPT.md) (+ [references/NEXTJS.md](references/NEXTJS.md) if Next.js 16+ App Router detected via `package.json` and `app/`) | `tsc --noEmit` + `eslint`/`biome` | | `python` | [references/PYTHON.md](references/PYTHON.md) | `ruff` + `mypy`/`pyright` + `bandit` | | `polyglot` | Run Phase 1–5 per language detected | per-language | | `other` | Skip Phase 1.2 sweeps; run Phase 2 + generic Phase 3–5 | semgrep generic ruleset | If the diff exceeds **50 files** or **1500 lines**, ask the user to split the review or to scope it to a subset before proceeding. Large reviews dilute attention and amplify hallucination risk. ### 0.1 Diff Scope Contract Default to the diff. A finding is in scope only when the changed line is in `$DIFF_RANGE` or the changed code makes an existing line newly reachable, newly exposed, or newly unsafe. Issues outside the diff are reported as `Out-of-scope observation` unless the user explicitly requested a baseline audit. Use null-delimited file lists when passing changed files between tools. The loop is portable across empty diffs and file names with spaces: ```bash git diff --name-only -z "$DIFF_RANGE" -- '*.go' | while IFS= read -r -d '' file; do rg -n '' "$file"; done git diff --name-only -z "$DIFF_RANGE" -- '*.rs' | while IFS= read -r -d '' file; do rg -n '' "$file"; done git diff --name-only -z "$DIFF_RANGE" -- '*.ts' '*.tsx' | while IFS= read -r -d '' file; do rg -n '' "$file"; done git diff --name-only -z "$DIFF_RANGE" -- '*.py' | while IFS= read -r -d '' file; do rg -n '' "$file"; done ``` ### 0.2 Monorepo / Workspace Handling When multiple `go.mod`, `Cargo.toml`, or `package.json` files are present, map each changed file to the nearest owning root before running toolchains. Run Phase 1–5 per touched root, not from the repository root unless the project convention requires it. | Root type | Ownership rule | Static command root | | --- | --- | --- | | Go module | nearest ancestor `go.mod` | run Go tools from that module | | Cargo workspace | workspace root if `workspace` exists; otherwise crate root | run Cargo tools with package filters when available | | npm/pnpm/yarn/bun workspace | nearest package or workspace root from lockfile config | run package scripts in touched package first | | Python project | nearest ancestor `pyproject.toml` (else `requirements.txt`) | run `ruff` / `mypy` / `bandit` from that root | ## Phase 1 — Static Analysis Sweep Run the static toolchain first; treat results as **leads**, never as conclusions. Calibration: every linter has known false-positive classes — start each automated finding at **Medium** severity and only promote to **High** with manual confirmation in Phase 2. ### 1.1 Automated Toolchain ```bash # Polyglot SAST (always run if available) semgrep --config=auto --error --severity=ERROR --severity=WARNING # Go golangci-lint run ./... staticcheck ./... go vet -all ./... govulncheck ./... # Rust cargo clippy --all-targets --all-features -- -D warnings cargo audit cargo deny check # TypeScript / Node / Bun bunx tsc --noEmit # or: npx tsc --noEmit bunx biome check . # or: bunx eslint . bunx knip bun audit # or: npm audit --omit=dev # Python ruff check . ruff format --check . mypy --strict . # or: pyright bandit -r . -q # run from package root (pyproject.toml dir); use src/ if project uses src-layout pip-audit safety check # Duplication (any language) npx jscpd --min-lines 5 --min-tokens 50 ./ ``` For each tool, capture the raw output and keep the version (` --version`) in the findings report. A finding without a tool version is not reproducible. ### 1.1.1 Optional Verification Commands Run tests only when the user asks, CI output is unavailable, or a finding needs confirmation. Label them separately from static tools in the report. ```bash go test ./... # add -race only for concurrency findings or explicit request cargo test --all-features bun test # or: npx vitest run pytest -q # Python ``` Never claim pass/fail unless the command and relevant output are shown in the report. ### 1.2 Pattern Sweep — language-agnostic For each category below, run the grep across changed files only using `$DIFF_RANGE` and null-delimited file lists, then read the matching files for context. | # | Category | Detection | | --- | --- | --- | | 1 | **Hardcoded secrets** | `rg -i '(password\|secret\|api[_-]?key\|token\|bearer)\s*[:=]\s*["\x27][A-Za-z0-9/+=_-]{8,}["\x27]'` | | 2 | **TODO/FIXME/XXX** | `rg -n '\b(TODO\|FIXME\|XXX\|HACK)\b'` (finding only when newly introduced, shipping to main, and not linked to an issue) | | 3 | **Stack trace exposure** | `rg -n '(stack\|stacktrace\|traceback\|panic)' --type-add 'web:*.{go,ts,tsx,rs,py}' --type web` | | 4 | **Unbounded loops / collections** | `rg -n 'for\s*\(\s*;;\s*\)\|while\s*\(true\)\|loop\s*\{' ` | | 5 | **Disabled error handling** | `rg -n '_ =\|catch\s*\(\s*_\s*\)\|\.unwrap\(\)\|\.expect\(\|\.ok\(\)\.unwrap\(\|except\s*:\|except\s+Exception\s*:'` | | 6 | **Insecure crypto** | `rg -n '(md5\|sha1\|des\|InsecureSkipVerify\|crypto/rand vs math/rand)'` | | 7 | **Logging of sensitive data** | `rg -n 'log\.(Info\|Debug\|Print).*\b(password\|token\|secret\|cookie\|authorization)\b'` | | 8 | **Wide-open CORS** | `rg -n 'Access-Control-Allow-Origin.*\*\|AllowAllOrigins\|origin: ["\x27]\*'` | | 9 | **Disabled lints / suppressions** | `rg -n '(// nolint\|//nolint\|#\[allow\(\|@ts-ignore\|@ts-nocheck\|eslint-disable\|# noqa\|# ruff: noqa\|# type: ignore\|# pyright: ignore)'` | | 10 | **Test code in production paths** | `rg -n '(println\!\|console\.log\|fmt\.Println\|^\s*print\()' --glob '!**/*test*'` | Per-language sweeps live in [references/GOLANG.md](references/GOLANG.md), [references/RUST.md](references/RUST.md), [references/TYPESCRIPT.md](references/TYPESCRIPT.md), and [references/PYTHON.md](references/PYTHON.md). ## Phase 2 — Manual Read-Through Read the diff with the same posture a teammate would: from base branch to head, file by file, **top to bottom**. The automated tools cannot judge intent — this phase is where intent meets implementation. ### 2.1 Read order 1. **Tests first** if any new tests exist — they encode the author's intent. 2. **Public API surface** — exported functions, types, routes, schemas, CLI flags. 3. **Internal logic** — handlers, services, business rules. 4. **Plumbing** — DI wiring, config, build. 5. **Infrastructure** — Dockerfile, workflow, IaC (only if it touches the diff). ### 2.2 Read-through questions For every changed function or block, answer in your head: - Is the **name** intention-revealing? Could a reader infer purpose without reading the body? - Does the function **do one thing**? If not, why is the merge OK? - What happens with **nil / empty / negative / huge / concurrent** inputs? - What **invariants** must hold before and after this code runs? Are they checked or assumed? - Where does **untrusted input** enter? Where does it leave the boundary trusted? - What **resource** is acquired? Where is it released? Under failure? - What **time** does this code take in the worst case? Memory? Allocations? - Could this **race** with another goroutine / task / Promise? - Are **errors propagated with context** or swallowed? - If this **panics / crashes / throws**, what is the blast radius? A finding is born only when the answer is unsatisfactory **and** the evidence is in the diff. Hallucinations are findings born without one of those two preconditions. ### 2.3 Diff hygiene | Smell | Detection | Action | | --- | --- | --- | | Unrelated changes mixed in | `git diff --name-only` against the PR description | Ask the author to split | | Whitespace-only churn | `git diff --ignore-all-space` shows a smaller diff | Note as Low severity, not blocking | | Large generated files committed | `*.lock`, `*.min.js`, `dist/` in the diff | Verify intentional; check `.gitignore` | | Re-formatted file alongside a tiny logic change | Many lines changed, few semantic | Ask for a separate format-only commit | ## Phase 3 — Security Review Run after Phase 2 because security findings depend on understanding intent. The OWASP API Top 10 (2023) and OWASP Web Top 10 (2025) are the reference frames. ### 3.1 Security categories (sweep + read) | OWASP | Category | What to read | | --- | --- | --- | | API1 | Broken Object Level Authorization (BOLA / IDOR) | Every handler that takes an ID — does it check ownership? | | API2 | Broken Authentication | Token issue/refresh/revoke paths; password handling | | API3 | Broken Object Property Level Authorization | Mass assignment; allow-listed fields on update | | API4 | Unrestricted Resource Consumption | Rate limits, body size limits, pagination caps | | API5 | Broken Function Level Authorization | Admin / non-admin route separation, RBAC checks | | API6 | Unrestricted Access to Sensitive Business Flows | Captcha / quotas on signup, order, transfer | | API7 | Server-Side Request Forgery | Outbound HTTP calls with user-supplied URLs | | API8 | Security Misconfiguration | Headers (CSP, HSTS), TLS, debug flags, default creds | | API9 | Improper Inventory Management | Public endpoints not in OpenAPI / spec | | API10 | Unsafe Consumption of APIs | Deserialization of upstream JSON without schema | | Web1–10 | Web equivalents … ## Source & license This open-source skill is cataloged on AgentStack and links to its original source — we do not rehost the code. - **Author:** [Bruno-Cunha-Souza](https://github.com/Bruno-Cunha-Souza) - **Source:** [Bruno-Cunha-Souza/ValarMindSkills](https://github.com/Bruno-Cunha-Souza/ValarMindSkills) - **License:** MIT Install and usage instructions live in the source repository linked above. ## Pricing - **Free** — Free ## Security capabilities Automated source analysis of v0.1.0 — what this tool can access: - **Network access:** no - **Filesystem access:** yes - **Shell / process execution:** no - **Environment & secrets:** no - **Dynamic code execution:** no *"Yes" means the capability is present in the source — more access means more to trust, not that it is unsafe.* ## Versions - **0.1.0** — security scan: passed — Imported from the upstream source. ## Links - Listing page: https://agentstack.voostack.com/l/skill-bruno-cunha-souza-valarmindskills-code-review - Seller: https://agentstack.voostack.com/s/bruno-cunha-souza - Browse the marketplace: https://agentstack.voostack.com/browse --- Listed on AgentStack — the marketplace for AI agent skills and MCP servers. Every listing is security-reviewed. Creators keep 70%.