reviewing-code
Imported from alexei-led/cc-thingz/dist/codex/dev-flow/skills/reviewing-code.
Install
npx skills add https://github.com/alexei-led/cc-thingz/tree/master/dist/codex/dev-flow/skills/reviewing-code
claude plugin marketplace add https://llmmart.ai/marketplace.json && claude plugin install alexei-led-cc-thingz@llmmart
git clone https://github.com/alexei-led/cc-thingz.git
The skills CLI installs just this skill, for any of its supported agents. Claude Code installs the whole alexei-led/cc-thingz collection as a plugin from our marketplace. Git is the plain clone.
Skill manifest
Code Review
Produce findings, not edits, for the requested diff, PR, changed files, or file
list. Read references/severity-rubric.md before scoring or reporting: it owns
the dimensions, severity, confidence, and score rules.
Load a language reference only for languages in scope:
- C#:
references/csharp.md - Go:
references/go.md - Java/Kotlin:
references/java-kotlin.md - Python:
references/python.md - Rust:
references/rust.md - TypeScript:
references/typescript.md - Web, HTML, CSS, JS, HTMX:
references/web.md
Other languages: use the rubric alone and report reduced coverage.
Scope
Use the scope the user named. Otherwise ask one question offering:
- uncommitted changes
- branch compared to the default branch
- specific files or a PR diff
Use one git or PR command for the whole review. Without command access, work
from the supplied diff and files, and ask for them if absent. No changes in
scope: report Nothing to review.
Modes
Default is standard.
- Quick: small, low-risk diffs. Security and correctness; changed lines plus direct context.
- Standard: security, correctness, and tests; follow callers or callees when a finding needs them.
- Deep: on request for a thorough, risk, or merge-safety review. All rubric dimensions, including boundary inputs and affected tests.
- Team: on request for team, parallel, or multi-pass review, or offered for a large diff. Split by dimension or file group, then merge into one report: dedupe by
file:lineplus claim, keep the higher severity only when evidence supports it, and put disagreements under Needs review. - External: only when the user explicitly asks for an external, second-model, or second-opinion review. Keep private code local unless the bridge runs locally or the user approved sharing. Rate external output with the same rubric; claims you cannot verify go to Needs review. Report whether it completed, was unavailable, or was skipped and why.
- Simplify: on request for an over-engineering or "what can we delete" review. Simplicity dimension only. Instead of the Output template, one line per finding:
file:line — <tag> <what>. <replacement>., with tagsdelete,stdlib/native(name the replacement),yagni,shrink. End withnet: -N lines possible.orLean already. Ship.
Evidence
- Read enough surrounding code to prove each claim. Run the project's checks when you can; a failing check in scope is evidence.
- If a code-graph tool (GitNexus, codegraph) is installed, use it for caller, impact, and test-coverage questions on broad or public-API changes; a stale index is a coverage gap, not evidence.
- Web research is for public facts only (CVEs, official docs). Never send private code or diffs to web tools.
- Skip style that project tooling already enforces. List suspicious code outside scope as out of scope instead of widening the review.
- Review generated or vendored files only when source generation is in scope.
Output
## Code Review Summary
Scope: <description>
Depth: quick | standard | deep | team | external
Languages: <list>
Coverage: complete | partial — <reason>
Graph evidence: none | <tool> — <freshness/gaps>
External review: not requested | completed | unavailable | skipped — <reason>
Score: <N/10 if requested> — confidence <high|medium|low>
### Critical
- `file:line` — <category>, confidence <level>. <issue> Scenario: <how it fails>. Fix: <concrete fix>.
### Warnings
- `file:line` — <category>, confidence <level>. <issue> Scenario: <how it fails>. Fix: <concrete fix>.
### Suggestions
- `file:line` — <category>, confidence <level>. <improvement>. Fix: <concrete fix>.
### Needs review
- `file:line or tool/context gap` — <missing context and why it matters>.
### Summary
<2-3 sentences: merge risk and next actions, or "No confirmed findings".>
Omit empty sections, except Needs review when it explains partial coverage.
Platform additions
No target-specific additions.
Files (cc-thingz)
-
references
-
csharp.md 772 B
# C# /.NET Review Focus Use writing-csharp for toolchain commands. - Nullable warnings hidden with `!`, suppressions, or broad defaults instead of fixing the source. - Sync-over-async (`.Result`, `.Wait()`), missing `await`, `async void` outside event handlers. - LINQ deferred execution: repeated enumeration, wrong cardinality (`First` vs `Single`), client-side evaluation. - `CancellationToken` not propagated to external I/O. - Scoped services captured by singletons; shared mutable state across requests. - Auth or ownership missing at controllers, minimal APIs, gRPC handlers, or worker entry points. - Background services without bounded retries, shutdown handling, or idempotency. - Build, test, or analyzer failure in scope is Critical when output confirms it. -
go.md 820 B
# Go Review Focus Use writing-go for toolchain commands. Check `go.mod` and CI before any version-gated claim. - Nil interfaces holding typed nil pointers; nil map writes; sends on nil or closed channels. - Errors ignored, wrapped without `%w`, or cancellation swallowed; `context` not passed to external calls. - Goroutines without an exit path, unbounded fan-out, `WaitGroup` misuse, shared maps written without sync. - Missing `Close` on response bodies, rows, statements, and files, especially on error branches. - `defer` inside hot loops; serial external calls over collections. - Concurrency changes need `-race` evidence or race-sensitive tests. - Go 1.22+ fixed per-iteration loop variables; flag capture bugs only in older modules. - Build, vet, or race failure in scope is Critical when output confirms it. -
java-kotlin.md 1 KB
# Java and Kotlin Review Focus Use writing-java-kotlin for toolchain commands. Check `build.gradle*`, `pom.xml`, and toolchain settings before any version-gated claim. - Kotlin platform types (`T!`) and `!!` at Java interop boundaries; missing nullability annotations there. - Coroutines: `GlobalScope`, lost cancellation, blocking calls inside `suspend` functions. - `InterruptedException` swallowed without restoring the interrupt flag. - JPA/Hibernate: lazy collections touched outside a transaction, N+1 in loops, entities leaking across API boundaries instead of DTOs. - Jackson `@JsonTypeInfo` or default typing enabling polymorphic deserialization; visibility changes exposing fields. - SpEL built from user input; JPQL/HQL concatenation; XML parsers without external-entity processing disabled (XXE). - Transaction boundaries missing or too broad; unclosed streams, connections, or executors. - Full `@SpringBootTest` for logic a unit or slice test covers. - `javax.*` and `jakarta.*` mixed in one module; Java 21 or Kotlin 2 features used on an older toolchain. -
python.md 784 B
# Python Review Focus Use writing-python for toolchain commands. Check `pyproject.toml`, `.python-version`, and CI before any version-gated claim. - Mutable defaults and module-level mutable state; broad `except` that hides failures or drops the cause. - Type hints that do not match runtime shape at boundaries. - `eval`, `exec`, `pickle`, `yaml.load`, dynamic import, or `shell=True` on untrusted input. - Blocking I/O inside `async def`; network or subprocess calls without timeout. - Naive vs aware `datetime` mixing; pagination off-by-one. - Unbounded caches or background tasks without shutdown. - Free-threading or subinterpreter concerns apply only when the project opts in. - No validation library is not a finding; flag only unvalidated input reaching sensitive behavior. -
rust.md 1 KB
# Rust Review Focus Use writing-rust for toolchain commands. Check the `Cargo.toml` edition, `rust-toolchain.toml`, MSRV, and enabled features before any version-gated claim. - `unsafe` without a documented soundness invariant is Critical; FFI that assumes C ownership or alignment without proof. - `unwrap`/`expect` on paths that can fail; `?` where the error should be handled locally. - Integer overflow wraps silently in release; `as` casts that truncate sizes, offsets, or indexes. - Blocking calls or `std::sync::Mutex` guards held across `.await`; missing cancellation or timeouts on tasks. - Secrets exposed through derived `Debug`; non-CSPRNG randomness for tokens. - Clones in hot paths where a borrow or `Cow` works; unbounded channels. - Edition 2024 changes `unsafe` rules; do not judge one edition by the other's rules. - `cfg` and feature flags can hide live code; check enabled features before calling code dead. - Concurrency or `unsafe` changes need Miri or loom evidence when the project configures them. -
severity-rubric.md 4.5 KB
# Review Severity Rubric Every finding needs evidence, impact, and a concrete fix. If one is missing, downgrade it or move it to Needs review. ## Finding fields - Severity: Critical, Warning, Suggestion, or Needs review. - Category: one of the dimensions below. - Confidence: high, medium, or low. - Evidence: `file:line` or quoted tool output. - Scenario: how the bug, exploit, missed behavior, or future failure happens. - Fix: the smallest concrete change that resolves it. ## Dimensions - **Security**: auth, authorization, injection, unsafe deserialization, secrets, crypto, SSRF, XSS, CSRF, path traversal, data exposure. - **Correctness**: logic errors, edge cases, null or empty handling, contract mismatches, broken callers, unchecked errors, migrations, API compatibility. - **Tests**: missing regression tests, uncovered changed behavior, weak assertions, over-mocking, missing error or boundary cases. - **Reliability**: resource leaks, retries, timeouts, cancellation, races, idempotency, cleanup, failure observability. - **Performance**: realistic hot paths, unbounded work, N+1 queries, blocking I/O, memory growth, avoidable cost. - **Maintainability**: dead code, confusing indirection, shallow wrappers, mixed responsibilities, brittle coupling, unclear invariants. - **Simplicity**: reinvented stdlib or native behavior, single-implementation abstractions, speculative flexibility, dependencies a few lines would replace, dead flags or config. - **Docs**: public API docs, migration notes, user-facing behavior, accessibility text, operator docs affected by the change. ## Severity Critical: - Exploitable security issue; data loss, corruption, or privacy leak. - Crash, deadlock, race, or resource leak in a normal or high-risk path. - Broken core behavior, public API contract, migration, auth, billing, or permission check. - Build, typecheck, lint, or test failure in the reviewed scope. Warning: - Likely correctness bug in an edge or realistic path. - Missing validation, authorization, timeout, error handling, or cleanup at a concrete boundary. - Meaningful test gap for changed business behavior, a bug fix, or a risky branch. - Performance problem at realistic input size, hot path, or cost. - Maintainability issue likely to cause defects, not just preference. Suggestion: - Non-blocking improvement to readability, structure, docs, tests, or performance with a concrete payoff. Needs review: - Plausible risk where required context (tooling, runtime, config, server-side or generated code, deployment behavior) is unavailable. - Not a confirmed finding; it preserves uncertainty without inflating defects. ## Confidence - **High**: evidence directly proves the issue; the fix is clear and local. - **Medium**: evidence is strong, but one assumption depends on nearby code, config, or runtime behavior. - **Low**: missing context prevents confirmation. Prefer Needs review unless the risk is severe and the gap is explicit. ## Decision rules - No evidence, no finding. No concrete scenario, at most Suggestion. - Missing context goes to Needs review, not hedged language. - Style, naming, or formatting is a finding only when it materially harms correctness, maintainability, docs, or tests. - Map tool warnings by impact; not every warning is Critical. - Security findings need an attack path or sensitive asset. - Test findings name the missing behavior or branch, not "coverage is low". ## Score Score only when asked. Start at 10, apply caps, then deductions. State score confidence and, for partial coverage, the cap reason. Use one decimal only when arithmetic needs it. Caps: - Any confirmed Critical: max 5. Two or more: max 4. - Exploitable security, data loss, corruption, or privacy leak: max 3. - Build, typecheck, lint, or test failure in scope: max 6. - Missing tests for risky changed behavior: max 7. - Mostly low-confidence findings: max 7. - Partial review (scope, diff, tools, or context missing): max 8. Deductions: Warning −1 (max −3); Suggestion −0.25 (max −1); Needs review 0 unless it blocks completeness, then use the partial-review cap. Anchors: - 10: complete review, no confirmed findings, relevant tests present. - 8: only minor maintainability, docs, or test-clarity suggestions. - 6: one real Warning, or missing tests for meaningful changed logic. - 4: one Critical in correctness, security, reliability, or build/test health. - 2: high-confidence exploitable security issue, data loss, or broken core path. Between two plausible scores, take the lower only when evidence matches a cap; otherwise take the midpoint with medium confidence. -
typescript.md 1007 B
# TypeScript Review Focus Use writing-typescript for toolchain commands and the project's package manager. - Runtime data trusted by type alone. Flag concrete untrusted input (bodies, params, headers, env, storage, webhooks, third-party responses) reaching sensitive behavior without a guard; do not flag every boundary for lacking a schema library. - Floating promises, missing `await`, lost rejections; non-exhaustive discriminated unions. - Prototype pollution from merging untrusted objects; raw HTML or markdown sinks; secrets in client bundles. - Auth or ownership missing at routes, resolvers, and server actions; CORS or header changes exposing credentials. - Sequential awaits that should batch; sync I/O in request or render paths; unbounded caches or concurrency. - React: effect dependencies, stale closures, missing effect cleanup. Next/server actions: server/client boundary leaks, cache invalidation. - Audit output: rate by reachability in shipped dependencies, not raw advisory severity. -
web.md 782 B
# Web Review Focus Use writing-web for validators and project scripts. - Escaping: for server-rendered templates, read enough context to tell trusted from user data. Raw sinks, unsafe markdown, and inline handlers are XSS paths. - CSRF on state-changing forms and HTMX requests when server context shows the need; if the middleware or template engine is unseen, use Needs review. - HTMX `hx-target`, `hx-swap`, `hx-trigger`, or header mismatches that break the flow. - Duplicate or mismatched IDs, labels, and form names. - Accessibility: accessible names, keyboard reachability, focus outline removed without replacement, ARIA fighting native semantics. Unmeasurable contrast goes to Needs review. - Blocking scripts, unsized or unlazy images, heavy DOM work on repeated events.
-
-
SKILL.md 4.7 KB
--- {"description":"Use when reviewing changed code, PRs, diffs, or specific files. Finds evidence-backed defects in security, correctness, tests, reliability, performance, maintainability, and docs. Supports quick, standard, deep, team, and external-review modes, plus a simplify mode for over-engineering and \"what can we delete\" reviews. NOT for repo-wide architecture review, general codebase exploration, fixing issues (use fixing-code), or improving tests without a code review (use improving-tests).","name":"reviewing-code"} --- <!-- Codex platform guidance --> <!-- Use this platform's installed tool names exactly for shell, file reads, and search. If a referenced helper or optional tool is unavailable, say so and continue with built-in tools. --> # Code Review Produce findings, not edits, for the requested diff, PR, changed files, or file list. Read `references/severity-rubric.md` before scoring or reporting: it owns the dimensions, severity, confidence, and score rules. Load a language reference only for languages in scope: - C#: `references/csharp.md` - Go: `references/go.md` - Java/Kotlin: `references/java-kotlin.md` - Python: `references/python.md` - Rust: `references/rust.md` - TypeScript: `references/typescript.md` - Web, HTML, CSS, JS, HTMX: `references/web.md` Other languages: use the rubric alone and report reduced coverage. ## Scope Use the scope the user named. Otherwise ask one question offering: - uncommitted changes - branch compared to the default branch - specific files or a PR diff Use one git or PR command for the whole review. Without command access, work from the supplied diff and files, and ask for them if absent. No changes in scope: report `Nothing to review.` ## Modes Default is standard. - **Quick**: small, low-risk diffs. Security and correctness; changed lines plus direct context. - **Standard**: security, correctness, and tests; follow callers or callees when a finding needs them. - **Deep**: on request for a thorough, risk, or merge-safety review. All rubric dimensions, including boundary inputs and affected tests. - **Team**: on request for team, parallel, or multi-pass review, or offered for a large diff. Split by dimension or file group, then merge into one report: dedupe by `file:line` plus claim, keep the higher severity only when evidence supports it, and put disagreements under Needs review. - **External**: only when the user explicitly asks for an external, second-model, or second-opinion review. Keep private code local unless the bridge runs locally or the user approved sharing. Rate external output with the same rubric; claims you cannot verify go to Needs review. Report whether it completed, was unavailable, or was skipped and why. - **Simplify**: on request for an over-engineering or "what can we delete" review. Simplicity dimension only. Instead of the Output template, one line per finding: `file:line — <tag> <what>. <replacement>.`, with tags `delete`, `stdlib`/`native` (name the replacement), `yagni`, `shrink`. End with `net: -N lines possible.` or `Lean already. Ship.` ## Evidence - Read enough surrounding code to prove each claim. Run the project's checks when you can; a failing check in scope is evidence. - If a code-graph tool (GitNexus, codegraph) is installed, use it for caller, impact, and test-coverage questions on broad or public-API changes; a stale index is a coverage gap, not evidence. - Web research is for public facts only (CVEs, official docs). Never send private code or diffs to web tools. - Skip style that project tooling already enforces. List suspicious code outside scope as out of scope instead of widening the review. - Review generated or vendored files only when source generation is in scope. ## Output ```markdown ## Code Review Summary Scope: <description> Depth: quick | standard | deep | team | external Languages: <list> Coverage: complete | partial — <reason> Graph evidence: none | <tool> — <freshness/gaps> External review: not requested | completed | unavailable | skipped — <reason> Score: <N/10 if requested> — confidence <high|medium|low> ### Critical - `file:line` — <category>, confidence <level>. <issue> Scenario: <how it fails>. Fix: <concrete fix>. ### Warnings - `file:line` — <category>, confidence <level>. <issue> Scenario: <how it fails>. Fix: <concrete fix>. ### Suggestions - `file:line` — <category>, confidence <level>. <improvement>. Fix: <concrete fix>. ### Needs review - `file:line or tool/context gap` — <missing context and why it matters>. ### Summary <2-3 sentences: merge risk and next actions, or "No confirmed findings".> ``` Omit empty sections, except Needs review when it explains partial coverage. ## Platform additions No target-specific additions.
Comments (0)
Sign in to join the conversation.
Reviews (0)
No reviews yet.
No comments yet.