Code review
Skill sumitake/agent-collab/plugins/agent-collab/skills/code-review
Unified dynamic-host agent collaboration policy and signed-runtime client
npx -y skills add sumitake/agent-collab --skill code-reviewAssembled from the repository path, not quoted from the project. Check it against their README if it does not work.
3 things to look at
- 24 days oldThe repository was created 24 days ago. New is not bad, but a brand new repository carrying a familiar-sounding name is the shape a typosquat arrives in, and there has been no time for anyone else to find a problem with it.
- no licenseNo license file was found in the repository. Code published without one is not open source by default, so using it at work is a question for whoever answers licensing questions where you are.
- 0 stars0 stars. Stars are a popularity signal and not a quality one, but at this level it is likely that nobody has read this closely except its author, and you would be relying on your own review.
What its author says it does
Copied from the file, not written here
Send a code diff, pull request, file, or directory to the reviewer for an independent cross-family code review focused on security vulnerabilities, edge cases, concurrency hazards, performance bottlenecks, and architectural smells. Use when the user says "code review with the reviewer," "have the reviewer review this code," "have the reviewer review this PR," "have the reviewer review this diff," "check this for security flaws," "security audit," "concurrency audit," or "performance review." Also offer this proactively when the active primary is about to commit a change that touches authentication, authorization, cryptography, financial calculations, payment flows, concurrency primitives, schema migrations, or any module where a class of bug — not just an instance — could have user-visible consequences.
SKILL.md
16.1 KB, as published. Nobody here has run it
Unified runtime invocation
Resolve the plugin root from this loaded file: SKILL.md is at <plugin-root>/skills/<skill-name>/SKILL.md. Invoke only python3 "<plugin-root>/coordinator.py" and send one bounded JSON request on stdin. Before constructing it, read the Coordinator request schema in <plugin-root>/README.md; never invent fields or route/action pairs. The public coordinator re-observes the active host/model, captures artifact provenance, excludes same-family routes, and verifies the co-packaged native manifest. It runs standalone from the installed plugin. Never discover a provider executable or reconstruct a raw command. Frontmatter tier is a routing recommendation, never a coordinator request field. For a review, cross-check, tiebreaker, or fallback over an authored artifact, capture its exact UTF-8 content and observed author model in the optional artifact object even when governance is false; never paste it into the prompt as a provenance substitute.
Code review — independent cross-family deep-read on a code artifact
A code review is a structured, lens-driven critique of a code artifact (diff, pull request, file, directory) by a model from the other family. The point is to surface defects the active primary would not have caught — security flaws, race conditions, missing rollback paths, edge cases the author normalized — not to confirm the code "looks fine." A review that returns no findings on a non-trivial change is usually a failed review; either the prompt was too soft or the code is genuinely trivial.
The cross-family setup is load-bearing. the active primary (resolved family) authored the code (the common case) and is therefore not the right reviewer for its own work. the reviewer (independent family) brings different training corpora, different default failure-mode emphases, and a clean read on the artifact unconstrained by the implementation choices that led to the current state.
When to use
Use this skill when one or more of the following are true:
- The user explicitly asks for it — "code review with the reviewer," "have the reviewer review this code," "have the reviewer review this PR," "have the reviewer review this diff," "check this for security flaws," "security audit," "concurrency audit," "performance review."
- A change is large or complex. Multi-file PRs, refactors crossing module boundaries, or any change touching > ~500 lines.
- A change touches a security-sensitive surface. Authentication, authorization, cryptography, password / token / key handling, input validation, sanitization, SSRF / SSRF-adjacent paths, deserialization, file uploads, anything user-controlled hitting a privileged operation.
- A change touches concurrency, state-machine, or migration logic. Locks, queues, transactions, schema migrations, distributed-state code, anything where the wrong sequence of events produces silent corruption.
- A change touches money or measurement. Pricing, billing, currency conversion, tax calculation, clinical-trial dosing, scientific-instrument calibration — anywhere a numerical bug has real-world consequence.
- A change touches concurrency primitives that the surrounding codebase has historically gotten wrong. Past defects in this area justify a second-family read on the new attempt.
When to skip
Skip this skill when:
- The change is a typo fix, comment update, or formatting-only edit. No semantic change → no defect class to surface. Wasting
pro-tier latency. - The change is auto-generated by a deterministic tool (formatter output, code-mod, lock-file regeneration). Review the tool's correctness once; do not review every emission.
- The user wants generic style feedback. That is what linters are for. This skill is for defect-class surfacing, not bike-shedding.
- The user has just received a code review from the reviewer on the same artifact and is asking for a second pass without new changes. The bottleneck is decision-making (which findings to act on), not under-review.
Verifier independence (functional contract)
A review is independent only when its observed author family differs from both
the immutable primary snapshot and artifact-author snapshot. The shared policy
recognizes Anthropic, Google, OpenAI, xAI, Zhipu, and genuinely unknown lineage;
OpenCode itself is a transport, not a family. Resolve through coordinator.py
immediately before every call. Governance fails closed when either snapshot is
unknown or no distinct-family advisory route is eligible. Non-governance work
may proceed only with an independence warning. Claude is async inbox-only.
Procedure
1. Identify the artifact and the reviewer scope
Pin down what is under review: a single diff (preferred — bounded scope), a pull-request range, a full file, or a directory of related files. Larger scope produces less-precise reviews; if the artifact is huge, consider splitting into focused passes (e.g., security-sensitive files first, then performance-sensitive files).
If the artifact came in as a PR URL or branch name, materialize the actual diff before sending; the verifier needs the literal code, not a reference. To keep the review focused, exclude auto-generated or routine noise (such as lockfiles or vendor manifests) using git pathspec exclusions (for example: git diff <base>..<head> -- ':!*.lock' ':!package-lock.json' ':!*.min.js' or other repository-specific patterns like ':!.claude-plugin/*.json').
2. Frame the review lens deliberately
A generic "review this code" produces generic linting feedback. Specify the failure classes you want surfaced. The standard lens, in order of priority for most changes:
- Security vulnerabilities — injection (SQL, command, template, XPath, LDAP), XSS, SSRF, deserialization, path traversal, broken access control, insecure crypto, weak randomness, secrets in code/logs, insecure defaults.
- Unhandled edge cases and missing error handling — null/none/undefined paths, empty collections, off-by-one, integer overflow / underflow, timezone / locale / unicode hazards, error paths that swallow exceptions, partial-failure states.
- Race conditions, concurrency, and ordering hazards — TOCTOU bugs, missing locks, lock-ordering inversions, unsafe shared mutable state, double-close / use-after-free, optimistic-vs-pessimistic locking assumptions.
- Resource hazards — memory leaks, file-descriptor leaks, connection-pool exhaustion, unbounded queues, missing back-pressure.
- Performance bottlenecks — N+1 queries, O(n²) on a hot path, missing indexes, allocations in tight loops, blocking I/O on async paths, missing batching.
- Architectural anti-patterns — leaky abstractions, circular dependencies, god classes, hidden coupling, broken layering, business logic in serializers.
Modify the lens for domain-specific reviews (e.g., add HIPAA-PHI handling for clinical software; add float-precision rules for financial software; add data-residency rules for cross-border systems).
3. Call the verifier
Submit the sealed code-review role through python3 "<plugin-root>/coordinator.py" with
effort='high' in every eligible advisory row and no tier request field. Central policy chooses an eligible independent
reviewer. Use this prompt template — the JSONL output schema is a functional
contract that downstream tooling consumes:
Review the attached code as a senior security and performance engineer for the resolved-family-authored change below. Focus areas in priority order:
1. Security vulnerabilities (injection, XSS, SSRF, deserialization, path traversal, broken access control, insecure crypto, weak randomness, secrets exposure)
2. Unhandled edge cases or missing error handling (null paths, empty collections, integer overflow, timezone / locale / unicode hazards, swallowed exceptions, partial-failure states)
3. Race conditions, concurrency, and ordering hazards (TOCTOU, missing locks, lock-ordering inversions, unsafe shared state)
4. Resource hazards (memory / FD / connection leaks, unbounded queues, missing back-pressure)
5. Performance bottlenecks (N+1 queries, O(n²) on hot paths, blocking I/O on async paths, missing batching)
6. Architectural anti-patterns (leaky abstractions, circular deps, hidden coupling, broken layering)
Ignore style and formatting. Verify each finding against the actual code — do not flag plausible-sounding issues that are not present.
Output ONLY JSONL (one JSON object per line, no preamble, no closing, no code fence) per this schema:
{"severity":"Critical|High|Medium|Low","file":"<path>","line":<number>,"issue":"<short description>","fix":"<concrete fix recommendation>"}
If no real issues surface, emit exactly one line:
{"severity":"None","summary":"<what you checked and why the code is safe>"}
--- ARTIFACT ---
[paste the diff or file contents, with file paths as section headers if multi-file]
Retry-on-malformed. If the response is not valid JSONL — wrapped in a markdown code fence (json ... ), or with conversational preamble, or with malformed JSON on any line — retry exactly once with:
Previous response was not valid JSONL. Re-emit RAW JSONL with NO code fence (do not wrap in ```json blocks), one JSON object per line, nothing else. Preserve all findings; only the format needs correcting.
If the second attempt is also malformed, surface that explicitly to the user — do not silently fabricate JSONL around the prose. A malformed code-review response is itself a signal: either the verifier hit a content-policy guardrail, the prompt confused it, or the artifact triggered a refusal. Report the failure, do not hide it.
Code-fence wrapping has been an empirical pattern on the Gemini side; less common but still possible from Claude-family verifiers. The retry pattern is load-bearing regardless of which family is acting as verifier.
4. Verify findings, then synthesize
Do not relay the verifier's JSONL directly. For each finding:
- Verify it against the actual code. Open the file at the flagged line. Confirm the issue is real, not a hallucination or a pattern-match on similar-looking code that does not actually have the flaw.
- Score the actionable findings. Critical + High should be addressed before merge / deployment. Medium + Low go to a follow-up issue list if not addressed inline.
- Group findings by file / module. A single file with five findings is more concerning than five files with one finding each — the former signals systemic issues, the latter looks like a scatter.
- Quote the flagged lines in the user-facing summary so the user can see the exact code without context-switching.
End with a synthesis paragraph: which findings are load-bearing (must be addressed), which are noise (can be set aside), and your recommendation on whether the change is ready to merge / deploy as-is, or needs revision first. A code review that ends with the raw JSONL pushed back to the user has not finished its job.
Examples across domains
Code review applies broadly. A representative sample of where independent cross-family review pays off, beyond traditional web-app code:
| Domain | Code artifact under review | What this skill typically surfaces |
|---|---|---|
| Product engineering | Feature-flag rollout logic for a new pricing tier | Edge cases around partial-rollout state, stale-cache flag bypass, missing audit-log on flag flip |
| Backend / web | Auth-token refresh flow for a multi-tenant SaaS | TOCTOU between token validation and use, missing rate limit, insecure cookie attributes |
| Financial software | New tax-calculation module for international invoicing | Float-precision in cumulative subtotals, locale-specific rounding, currency-conversion timing assumptions |
| Clinical software | Dosing-calculation function for a chemotherapy protocol | Off-by-one in age bracket, missing range check, ambiguous unit handling (mg vs mg/kg) |
| Scientific computing | Sample-statistics aggregator for a particle-physics pipeline | Numerical-stability hazards in cumulative variance, NaN-propagation in weighted sums, floating-point order-dependence in parallel reductions |
| Embedded / IoT | Firmware OTA-update verifier on a smart-thermostat | Insufficient signature verification, rollback-protection gap, missing power-fail-mid-update recovery |
| Database engineering | Schema migration adding a NOT NULL column to a 50M-row table | Lock-table-for-duration hazard, missing backfill batching, missing rollback path, downstream-consumer impact |
| Distributed systems | New consensus-protocol module for a coordination service | Split-brain hazards, leader-election race, log-replication tail-bug, quorum-arithmetic off-by-one |
| Security tooling | Custom WAF rule for a newly-discovered attack pattern | False-positive cliff at the rule boundary, ReDoS in the matching regex, bypass via case / encoding variant |
| ML infrastructure | Online feature-store write path for a fraud-detection model | TOCTOU on feature-version stamp, silent type-coercion, training-serving skew via aggregation difference |
The review lens shifts with the domain (clinical software emphasizes dosing safety; financial software emphasizes precision; security tooling emphasizes false-positive vs false-negative trade-off), but the JSONL schema and retry-on-malformed contract stay constant.
Anti-patterns
- Generic "review this" prompts. They produce generic linting output. Specify the lens (security, concurrency, performance) or the domain-specific failure classes.
- Reviewing trivial or auto-generated changes. Wastes
pro-tier latency, dilutes the audit log, trains the user to ignore code-review output. - Treating every finding as actionable. Verify each one against the actual code. Hallucinations and pattern-matches on similar-looking code are common; relaying them as-is wastes the user's time and erodes trust in the skill.
- Using
flashtier. Security and performance reasoning benefit from depth;flashproduces a checklist-level read that misses the subtle bugs this skill exists to catch. - Relaying the raw JSONL to the user. The JSONL is machine-parseable input to the synthesis step, not the user-facing deliverable. Group, prioritize, quote, recommend.
- Reviewing the wrong artifact. A PR URL is not the diff; materialize
git diff <base>..<head>before sending, using pathspec exclusions to filter out routine files (like lockfiles or generated assets; see step 1). A file is not the change; isolate the changed hunks when the change is small. - Skipping the verifier-independence check when the code under review was authored by a independent-family agent. That review is correlated with its author; the audit log will record a review that did not, in substance, occur.
- Skipping the retry-on-malformed step. Code-fence wrapping is common (especially Gemini-family); the retry is non-optional. If the second attempt is also malformed, surface the failure rather than fabricating structure around prose.
- Reviewing for style. Linters do that. This skill is for defect-class surfacing.
- Asking the verifier to "fix" the code rather than review it. This skill is review-only; remediation is a separate step (the user decides which findings to act on; another tool — or the active primary directly — implements the fix).