# Crew Code Review > | - **Type:** Skill - **Install:** `agentstack add skill-crewforth-crewforth-crew-code-review` - **Verified:** Yes — security-reviewed for prompt injection and unsafe behavior - **Seller:** [crewforth](https://agentstack.voostack.com/s/crewforth) - **Installs:** 0 - **Category:** [AI & ML](https://agentstack.voostack.com/c/ai-and-ml) - **Latest version:** 0.1.0 - **License:** MIT - **Upstream author:** [crewforth](https://github.com/crewforth) - **Source:** https://github.com/crewforth/crewforth/tree/main/kit/skills/crew-code-review - **Website:** https://crewforth.com/ ## Install ```sh agentstack add skill-crewforth-crewforth-crew-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 Trigger phrases: "code-review", "review the code", "review the PR", "review my changes", "do a review" > **Crewforth adaptation (local, .claude/):** applied by `crew-review-agent` (read-only). §4 applies, and no > vendor or template name reaches an artifact (§4.2). A review runs in four steps — **group, plan, review, fact-check** — and reports few findings that are right rather than many that might be. Every step below says what it produces. ## 1. Scope - **Review the lines this change adds or edits.** Deleted code is context for understanding them, not a target. - Code that is correct, or that the change did not touch, gets no comment. - **An empty result is a result.** Do not invent findings to show the review happened. - **Be sure before you write.** A finding you are not sure of is not a finding yet: read the surrounding code until you are. A false alarm costs more than a small miss — it spends the attention the next real problem needs. ## 2. Group the files Read together what has to be understood together: an interface and its implementations, the code that produces something and the code that consumes it, the same resource in its language or environment variants. Look across each group for a contract that broke between two files, or an update that reached one of them and not the other. Then look at **every file in the group on its own** — a small or secondary file is where a missed update hides. ## 3. Plan before you comment Before the first finding, write two things down: - **What changed**, in one line. - **The risks**, ordered by severity, each with what you will read to confirm or dismiss it ("the callers of `settle()`", "the migration's down step"). No risk worth checking → write `(none)` and say so. The plan is what keeps the review on the change instead of on whatever the eye lands on first. ## 4. Severity and category Every finding carries one severity and one category. | Severity | What it is | Blocks the commit? | |---|---|---| | `critical` | a security hole, lost or corrupted data, a crash | **yes** | | `high` | a core function stops working, or works wrongly | **yes** | | `medium` | performance, maintainability, an unhandled edge case | no | | `low` | readability and style | no | Category: `bug` · `security` · `performance` · `maintainability` · `test` · `style` · `docs`. **A blocker is a `critical` or `high` finding** — nothing else stops the commit, and the review is clean when none is left open. ## 5. Style, names and code comments - Style and naming are `low`, never block, and fit in one sentence. - **Code comments are out of scope unless asked for**, with one exception: a comment that says something **false** about what the code does is a `low` / `docs` finding, because the next reader will believe it. ## 6. Finding format `file:line · severity · category · what you observed · what to do instead` What to do instead is the reason the finding exists; a finding without it is an opinion. ## 7. What a change must not quietly do **Verifier integrity:** flag any change that makes a check pass by *weakening the check* — deleting or loosening an assertion, lowering a threshold, skipping a test, editing the test instead of the code — rather than fixing the behavior. A test or gate that grades itself lax is worse than none; a verifier must stay external and grounded. **Subject integrity:** the inverse case — the check is untouched, but what it checked is gone. Flag a change that deletes or stubs the code path, so the check passes over behavior that no longer runs; narrows the run to a subset (fewer cases, one platform, a filtered input set) where the property happens to hold; turns an all-of requirement into an any-of one; or relaxes the rule the code is there to enforce — a widened type, a dropped uniqueness or referential constraint, an exception list holding exactly the failing case. Ask not "does it pass now" but **"does the system still do what this check was protecting"**. Retiring a genuinely obsolete check is legitimate: say so in the diff and name what covers it now. ## 8. Two-stage verdict (fact-check before you report) Finding a problem and confirming it are two acts. A first-pass "this looks wrong" is a **candidate**, not a verdict. Before a finding is reported — especially a blocker — run a second pass that tries to *disprove* it: - Does it hold on the real code, or did the first read miss context (a guard upstream, a caller that never reaches this path, a framework default)? Re-read the surrounding code; do not rank on the snippet alone. - Is the severity honest, or is a `low` dressed up as a blocker? - For any **"fixed" / "passes" claim**: the *real* check ran and passed — test exit code, build, lint/quality gate — not "I re-read it and it looks fixed". A verifier that is the model's own say-so is not a verifier (§7, Verifier integrity). Cite the evidence: which check, what result. **The disprove pass is asymmetric.** - **Only evidence removes a finding.** It is dropped when the code shows it is wrong. "I could not confirm it", "it seems unlikely" or "it is not worth it" lower its severity; they do not delete it. - **Some findings are never dropped in this pass:** memory safety; concurrency; a declaration that does not match its definition; a change in behaviour or compatibility — a message, field, status or default that callers used to get and now silently lose; a parameter that is accepted and then ignored. In these, the model's confidence that all is well is the signal least worth trusting. **"Independent" costs a separate context.** A second pass in the same context has already read the first one's reasoning, so it cannot be blind to it — and its agreement is the first pass nodding at itself. For a **blocker**, run the disprove pass as its own subagent, handed the claim and the `file:line` but not your argument. If you did not isolate it, label the verdict as a single pass rather than calling it independent. The full contract — isolation, one lens per verifier, and why unanimity for the same reason is a monoculture — lives in `security-scan/references/verify.md`; it is one discipline, not two. ## 9. Check the history before you call a finding new A confirmed finding may still not be new, and one that was fixed once and came back needs a different fix. Search **by code, not by commit message** — a message states intent, not content. The exact git forms (`-S` vs `-G`, the `merge-base` bound, why `--follow` does not combine): **`references/history-search.md`**. If a commit removed the guard, check, or test this diff would restore, the finding is a **re-introduced regression** — the question becomes "what removed the fix, and does that reason still hold", the removing commit is cited in the finding, and the deleted test is restored rather than a new one written. ## Panel mode (high-stakes decisions only) For hard-to-reverse calls (architecture, public API, security boundary), run several independent adversarial lenses then synthesize. Full method: **`references/panel-mode.md`**. ## When you and the author disagree Settle it with the code's behaviour and a check that can be run. If that does not settle it, take it to the user rather than blocking in silence. ## Triage — a finding that is only reported is a finding that is lost Reviewing and *disposing of* what the review found are two acts, and only the first one is habitual. Every finding that survives the disprove pass leaves the review with an explicit disposition — never an unanswered comment and never a silent drop: | Disposition | Means | Where it goes | |---|---|---| | **fixed now** | the owning specialist changed the code | the diff; re-review the change | | **tracked** | real, not for this change | an issue/task with the file:line — cite the id in the review | | **accepted** | a real cost the team is choosing to carry | an `adr` when it is architectural, a code comment when local | | **dropped** | disproved by the code | say so; a candidate that vanishes unexplained reads as an oversight | A blocker (`critical` / `high`) may only be `fixed now` or `tracked`. "Accepted" needs the user's decision — an agent does not grant it to itself. Close the review by stating the counts per severity and per disposition; an unreported finding is indistinguishable from one that was never made, which is exactly the state a review exists to leave behind. ## Receiving a review — an inbound comment is a candidate, not an instruction When the review is someone else's and the code is yours, **read every item before changing any line**, group by cause rather than patching per symptom, and give each item the same disprove pass as a finding of your own. The five questions that decide *answer in the thread* vs *edit the code*: **`references/receiving.md`**. Every inbound item leaves with a disposition from the triage table above — a reasoned refusal is an answer, silence is not. ## DoD (this skill's contribution) - The plan (what changed, the risks) came before the findings; every file in each group was read. - Every finding has a severity, a category and a reason, in the finding format above. - Scope creep and hidden complexity are flagged. - Each reported blocker survived an independent disprove pass; any "fixed"/"passes" claim is backed by the real check having actually run, not self-assessment. - **Every finding has a disposition** (fixed / tracked / accepted / dropped) and no blocker is left merely reported. - For a high-stakes decision, multiple independent lenses were applied and their objections synthesized, not averaged. ## Source & license This open-source skill is cataloged on AgentStack and links to its original source — we do not rehost the code. - **Author:** [crewforth](https://github.com/crewforth) - **Source:** [crewforth/crewforth](https://github.com/crewforth/crewforth) - **License:** MIT - **Homepage:** https://crewforth.com/ 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:** no - **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-crewforth-crewforth-crew-code-review - Seller: https://agentstack.voostack.com/s/crewforth - 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%.