code-review
Compare original and translation side by side
🇺🇸
Original
English🇨🇳
Translation
ChineseCode Review
代码审查
Purpose
目的
Most review output is noise: a list of things that look wrong, unranked, unrefuted, and impossible
to act on. This skill produces the opposite — a small number of findings, each with a required
behaviour, a feasible trigger, a concrete contradiction, an observable consequence, and the strongest
counterargument already checked.
Its central bet: the defects reviewers miss are rarely visible inside one file. They are
disagreements between two participants that each look reasonable alone — a caller and a callee, a
producer and a consumer, a writer and a later reader, two branches that should establish the same
state. A checklist applied file-by-file cannot see those, because the two halves are never in view at
the same time. So the unit of review here is the agreement, not the file.
大多数代码审查的输出都是噪音:一长串看起来有问题的条目,没有优先级、未经过反证,也无法落地处理。本Skill则相反——仅输出少量发现项,每一项都包含预期行为、可行触发条件、具体矛盾点、可观测后果,且已验证过最有力的反驳论点。
其核心假设是:审查者遗漏的缺陷很少能在单个文件中被发现。 这些缺陷源于两个参与方之间的约定不一致,而每一方单独看都合理——比如调用方与被调用方、生产者与消费者、写入者与后续读取者、本应达成相同状态的两个分支。逐文件应用的检查清单无法发现这些问题,因为约定的两方永远不会同时出现在视野中。因此,这里的审查单元是约定,而非文件。
Structure: one engine, three anchors
结构:一个引擎,三个锚点
Code review is the skill. Correctness is its core — the dimension generic tooling covers worst,
and the one described in full below. Performance and security are sub-cases: the same engine, the
same refutation discipline, the same report contract, with a different anchor and one or two extra
rules each.
| Sub-case | Anchor | Where its rules live |
|---|---|---|
| Correctness (core, default) | Agreements between participants across a boundary | This file + |
| Performance | Workload → resource demand → growth or contention → consequence | |
| Security | Source → trust boundary → sink, with an attacker controlling the source | |
Read a sub-case's file only when that sub-case is selected. Each is short on purpose: it states what
differs, and the rest of this file still applies.
Sub-cases are independently activated, not mutually exclusive. One root cause can carry correctness
and security impact — report it once, with both impacts.
代码审查是本Skill的核心功能。正确性是其核心维度——这是通用工具覆盖最差的维度,也是下文将详细阐述的维度。性能和安全是子场景:使用同一引擎、同一反证规则、同一报告规范,仅锚点不同,且每个子场景各有一两条额外规则。
| 子场景 | 锚点 | 规则位置 |
|---|---|---|
| 正确性 (核心,默认) | 跨边界参与方之间的约定 | 本文件 + |
| 性能 | 工作负载 → 资源需求 → 增长或争用 → 后果 | |
| 安全 | 数据源 → 信任边界 → 接收点,且攻击者可控制数据源 | |
仅当选中对应子场景时才需要阅读其文件。每个文件都刻意保持简短:仅说明差异点,本文件的其余内容仍然适用。
子场景是独立激活的,并非互斥。同一个根因可能同时产生正确性和安全影响——只需报告一次,并注明两种影响即可。
Invocation
调用方式
/pm-ai-shipping:code-review
/pm-ai-shipping:code-review dimensions=correctness scope=changes
/pm-ai-shipping:code-review dimensions=performance,security
/pm-ai-shipping:code-review dimensions=allClaude Code ships its own bundled . Use the plugin-qualified form above when you mean
this one.
/code-reviewThese are instruction arguments, not shell flags.
- Default: . Bare "review this" or "find bugs" means correctness only.
correctness - An explicit list selects exactly those sub-cases; selects three. Never silently reinterpret an unknown or empty selection — ask.
all - Scope: use what was asked. Otherwise review working changes if present, else the repository.
- State the selected dimensions, the scope and the comparison baseline before investigating.
- Reviewing changes means following dependencies beyond the changed lines, and distinguishing defects the change introduced from defects it merely revealed.
- Review and report. Apply fixes only when asked.
/pm-ai-shipping:code-review
/pm-ai-shipping:code-review dimensions=correctness scope=changes
/pm-ai-shipping:code-review dimensions=performance,security
/pm-ai-shipping:code-review dimensions=allClaude Code 自带内置的 命令。当你需要使用本Skill时,请使用上述带插件前缀的命令形式。
/code-review这些是指令参数,不是shell命令标志。
- 默认值:(正确性)。 仅说“审查这个”或“找bug”时,仅指正确性审查。
correctness - 显式指定列表时,仅选中对应的子场景;则选中全部三个。绝不要默默重新解释未知或空的选择——应当询问用户。
all - 范围: 按用户要求的范围执行。如果没有指定,若存在工作区变更则审查变更,否则审查整个仓库。
- 在开始调查前,需明确说明选中的维度、审查范围和对比基线。
- 审查变更意味着需要跟踪超出变更行的依赖关系,并区分变更引入的缺陷和变更仅暴露的缺陷。
- 仅进行审查并报告结果。仅在被要求时才应用修复。
Shared engine
共享引擎
Every sub-case uses one skeleton. Only the anchor and the refutation rules differ.
Map a flow → identify an obligation → inspect every participant → construct a violating execution
→ trace the consequence → attempt refutation → report.
Build one minimal map first: inputs, major execution flows, who owns which state, external
dependencies, observable effects. Each selected sub-case enriches it — do not build three maps, and
do not make a security-only run wait on correctness mapping.
所有子场景都使用同一个框架,仅锚点和反证规则不同。
梳理流程 → 识别约束条件 → 检查所有参与方 → 构造违规执行路径 → 追踪后果 → 尝试反证 → 报告。
首先构建一个最小化的流程图:输入、主要执行流程、各状态的所属方、外部依赖、可观测效应。每个选中的子场景都会对其进行补充——不要构建三个独立的流程图,也不要让仅安全的审查等待正确性的流程梳理。
Correctness: the agreement engine
正确性:约定引擎
A boundary is semantic, not a file split. It separates a caller and a callee, two callbacks, two
executions of the same function, a producer and a consumer, or a value written now and read later.
For each consequential agreement, hold these in working notes — not in the report:
Participants:
Value, entity or effect exchanged:
Authority (who decides the real answer):
Identity and lifetime/version:
Required relationship:
Evidence for that relationship:
Relevant transitions or orderings:
Observable consumer or consequence:Establish the obligation without inventing intent. Evidence comes from specifications,
documented contracts, language or protocol semantics, tests that encode an expectation, or a
necessary producer/consumer relationship. A consumer's implementation alone does not prove the
consumer is right. Where participants disagree, say why the disagreement produces a wrong outcome —
sometimes the contradiction is certain while which side should change is genuinely open. Missing
documentation is a limitation, not automatically a finding.
Start where agreements are most likely to break: values transformed or negotiated, identities
reassigned, work becoming asynchronous, state persisted and reloaded, several effects that must
agree. Then do a local pass over ordinary decisions, arithmetic, boundaries and error branches — the
anchor must not become a filter that discards plain bugs.
边界是语义上的,而非文件划分。它可以分隔调用方与被调用方、两个回调、同一函数的两次执行、生产者与消费者,或是当前写入与后续读取的值。
对于每个有重要影响的约定,将以下内容记录在工作笔记中——不要写在报告里:
Participants:
Value, entity or effect exchanged:
Authority (who decides the real answer):
Identity and lifetime/version:
Required relationship:
Evidence for that relationship:
Relevant transitions or orderings:
Observable consumer or consequence:确立约束条件时,不得臆测意图。 证据来源包括规格说明、已记录的契约、语言或协议语义、编码了预期的测试,或是必要的生产者/消费者关系。仅消费者的实现不能证明消费者是对的。当参与方存在分歧时,要说明为什么分歧会导致错误结果——有时候矛盾是确定的,但哪一方需要修改确实是待定的。缺失文档是一种局限性,不自动构成发现项。
从约定最可能破裂的地方开始: 被转换或协商的值、被重新分配的身份、变为异步的任务、被持久化后重新加载的状态、多个必须保持一致的效应。然后对普通的决策逻辑、算术运算、边界条件和错误分支进行局部检查——锚点不能成为过滤掉普通bug的筛子。
Force a violating execution
构造违规执行路径
A suspicion is not a finding until you construct the execution that breaks it. Where the
implementation permits:
- make a requested value differ from the accepted or effective one;
- keep two operations live at once and vary their completion order;
- change the relevant identity or generation between observation and use;
- compare distinct transitions that should end in equivalent state;
- inject failure between effects, and interruption before completion;
- exercise empty, exact-boundary and adjacent-boundary inputs.
Establish that each case is actually reachable. Do not assume it.
在构造出能触发问题的执行路径之前,怀疑都不算发现项。在实现允许的情况下:
- 让请求值与接受的或生效的值不同;
- 让两个操作同时处于活动状态,并改变它们的完成顺序;
- 在观察和使用之间,更改相关的身份或版本号;
- 对比本应达到等价状态的不同转换路径;
- 在多个效应之间注入故障,在完成前注入中断;
- 测试空输入、精确边界输入和临近边界输入。
要确认每种情况都是实际可达的,不要假设它可达。
Two lenses that need a forced probe, not a mention
两个需要主动探测而非仅提及的视角
Across a large evaluation of planted runtime defects in real codebases, two classes were almost never
even reported by strong agents — not missed at the fix, missed at the look. Naming them in a
checklist will not help; each needs an explicit probe:
- Authority reconciliation. Follow a proposed value through validation, normalisation, negotiation or commit, and find downstream state still derived from the proposal where the authority can return something different. A requested value is not an applied value. Probe: force them apart and ask what still reads the request.
- Identity and correlation. Trace how an operation's result finds its originating entity, then establish that the key is unique, stable and live for long enough — under overlap, reordering, removal and reuse. A label, a position or arrival order is suspicious exactly when those properties can fail. Probe: run two operations concurrently and complete them out of order.
The full set of thirteen diagnostic lenses, each with a detection tell, is in
. They are overlapping lenses, not a quota to fill.
references/correctness-taxonomy.md在对真实代码库中植入的运行时缺陷进行的大规模评估中,有两类缺陷几乎从未被强大的Agent报告过——不是修复时漏掉,而是根本没被发现。把它们列在检查清单里没用,每一类都需要主动探测:
- 权威值校验。 沿着提议值的流转路径,经过验证、规范化、协商或提交后,检查下游状态是否仍基于提议值推导——而权威方可能返回不同的值。请求值不等于生效值。 探测方法:强制让两者不同,检查还有哪些地方在读取请求值。
- 身份与关联。 追踪操作结果如何匹配到其原始实体,然后验证关联键在重叠、重排序、删除和复用的场景下是否足够唯一、稳定且有效。标签、位置或到达顺序在这些属性可能失效的场景下是可疑的。探测方法:并发运行两个操作,并让它们按乱序完成。
完整的13种诊断视角(每种都有对应的检测特征)详见 。它们是相互重叠的视角,而非需要填满的配额。
references/correctness-taxonomy.mdRefutation: the discipline that makes this worth running
反证:让审查有价值的准则
A candidate becomes a finding only with all five:
- A supported obligation — what must hold, and on what evidence.
- A feasible execution — inputs, state and ordering the real system permits.
- A concrete contradiction — where the obligation fails.
- An observable consequence — wrong output, state, effect, completion or progress.
- An examined counterargument — the strongest mechanism that would prevent or repair it.
Actively hunt for the refutation: an enclosing guarantee that makes the execution impossible;
synchronisation excluding the interleaving; reconciliation before any consequential read; an
intentional contract; a precondition excluding the input; a different owner responsible for it.
| Outcome | Rule |
|---|---|
| Keep | Evidence establishes the defect; the counterargument checked does not prevent it. |
| Drop | Cited evidence defeats the execution, the obligation or the consequence. |
| Unresolved | An essential contract or runtime fact is unknown. List it separately from findings. |
Do not import the security sub-case's attacker/victim test into correctness. A correctness defect
can harm only the person who triggered it and still be serious. Equally, "keep unless disproved" is
too permissive here — an ungrounded suspicion with no constructed execution is not a finding. When
both sub-cases are active, apply each test only to its own dimension.
Absorption is not prevention. The most expensive refutation mistake is finding something
downstream that happens to hide the defect - a cache that usually holds the value, a retry that
usually succeeds, a default that is usually right - and dropping the finding. That is not a
guarantee, it is a coincidence with good odds, and it fails the day the absorber is cold, evicted or
reconfigured. Drop only on a mechanism that makes the execution impossible, and say which mechanism
it was. For the same reason, "it works nearly always" describes a race, not a refutation - a
timing window that usually resolves correctly is a finding, and the fact that you had to reason about
which side usually wins is the evidence.
Passing tests, unfamiliar code, a suspicious name, a missing test and a sibling difference are
evidence to investigate — none of them is proof, and none is refutation. Deduplicate by violated
agreement and root cause, never by file. There is no findings quota; zero supported findings is a
valid result.
候选缺陷只有满足以下五个条件,才会成为正式发现项:
- 有依据的约束条件 —— 必须满足的要求,以及支撑的证据。
- 可行的执行路径 —— 真实系统允许的输入、状态和顺序。
- 具体的矛盾点 —— 约束条件失效的地方。
- 可观测的后果 —— 错误的输出、状态、效应、完成度或进度。
- 已验证的反驳论点 —— 最有力的预防或修复机制,且已验证其不成立。
主动寻找反证:比如能让该执行路径不可能存在的上层保证;排除该交错执行的同步机制;在任何有影响的读取之前的对齐操作;有意为之的契约;排除该输入的前置条件;应由其他负责方处理的情况。
| 结果 | 规则 |
|---|---|
| 保留 | 证据证实缺陷存在;已检查的反驳论点无法排除该缺陷。 |
| 丢弃 | 引用的证据推翻了执行路径、约束条件或后果。 |
| 未解决 | 关键契约或运行时事实未知。将其与发现项分开列出。 |
不要将安全子场景的攻击者/受害者测试套用到正确性审查中。正确性缺陷即使只会伤害触发它的人,也可能是严重的。 同样,“除非被证伪否则保留”在这里过于宽松——没有构造出执行路径的无端怀疑不算发现项。当两个子场景同时激活时,每个测试仅应用于其对应的维度。
掩盖不等于预防。 最昂贵的反证错误是:在下游找到某个刚好能掩盖缺陷的东西——比如通常能命中的缓存、通常会成功的重试、通常正确的默认值——然后就丢弃该发现项。这不是保证,只是概率很高的巧合,一旦掩盖机制失效(冷启动、被驱逐或重新配置)就会出问题。仅当存在能让执行路径完全不可能的机制时才能丢弃,并说明是哪种机制。同理,“它几乎总是能正常工作”描述的是竞态条件,而非反证——一个通常能正确解决的时间窗口问题本身就是发现项,而你需要推理哪一方通常会胜出这一事实就是证据。
测试通过、代码不熟悉、名称可疑、缺少测试、同类实现有差异,这些都是需要调查的线索——但都不是证据,也不是反证。按违反的约定和根因去重,绝不要按文件去重。没有发现项数量的配额;零个有依据的发现项也是有效的结果。
Parallelism
并行执行
Fan out over complete flows or connected groups of agreements — never over files, and never one
agent per taxonomy class. Partitioning by file is precisely the split that hides cross-boundary
defects, which are the ones worth finding.
- The coordinator builds the initial map and identifies shared state.
- Each worker gets a bounded flow, its participants, the selected sub-cases and open questions.
- Workers inspect both sides of their agreements and may follow dependencies outside their list.
- Workers return candidates, cited evidence, completed refutations and unresolved relationships.
- The coordinator reconciles assumptions and any relationship that crosses assignments.
- Strong candidates get a separate verification pass before they are reported.
Allow overlapping reads. Two workers reading the same authority is far cheaper than either one
holding half its contract. Keep integration capacity in reserve: an unresolved relationship spanning
two assignments stays unexamined until someone closes it. One level of fan-out is the target; if
delegation is unavailable or the scope is small, run the same procedure sequentially.
Run workers on the strongest model available, and match the current session's effort level. This
is recall-first work: a missed cross-boundary flow is the costly failure, and a worker that silently
drops to a cheaper model or a lower effort is the cheapest way to lose one. If any worker is rerouted
or downgraded, say which in the report — a reader who assumes one model saw everything will
misjudge the coverage.
One model. Name it on every worker. Fan-out here buys coverage, not a second opinion. Pass the
coordinator's own model explicitly on each spawn — "inherit" is not a routing decision, and a worker
that quietly lands on a cheaper model is the easiest way to lose a finding. Do not bring in a
different model, to review or to cross-check, unless you are explicitly asked: mixing models makes
the result unattributable, and when this skill is being measured or compared across models, one
foreign worker invalidates the number. The independent second-model pass is a separate,
explicitly-invoked step ( Step 6), never something this skill reaches for on its own.
/ship-checkTell workers they are reading, not editing. A review worker needs to read, search and navigate;
it must not modify the tree. State that in the worker's instructions — a worker that starts editing
drifts from reviewing into "helpfully" fixing and stops reporting what it silently repaired, and the
findings can no longer be checked against the code they describe. Say it in the prompt rather than
assuming the host will enforce it, and confirm the tree is unchanged when the run ends.
按完整流程或相互关联的约定组进行分发——绝不要按文件分发,也不要给每个分类分配一个Agent。按文件划分恰恰会掩盖跨边界缺陷,而这些正是值得发现的缺陷。
- 协调者构建初始流程图并识别共享状态。
- 每个工作节点分配到一个有边界的流程、其参与方、选中的子场景和待解决的问题。
- 工作节点检查其约定的双方,并可能跟踪列表之外的依赖。
- 工作节点返回候选缺陷、引用的证据、已完成的反证和未解决的关联关系。
- 协调者对齐假设,以及所有跨任务分配的关联关系。
- 高置信度的候选缺陷在报告前会经过单独的验证流程。
允许重叠读取。 两个工作节点读取同一个权威源,比各自只掌握一半契约的成本低得多。保留整合能力:跨越两个任务分配的未解决关联关系,直到有人处理它之前都保持未检查状态。目标是一级分发;如果无法委派任务或范围很小,则顺序执行相同的流程。
使用可用的最强模型运行工作节点,并匹配当前会话的投入级别。 这是召回优先的工作:遗漏跨边界流程是代价高昂的失败,而工作节点默默切换到更便宜的模型或降低投入级别是最容易导致遗漏的原因。如果任何工作节点被重新路由或降级,要在报告中说明——假设用同一个模型检查了所有内容的读者会错误判断覆盖范围。
统一模型。每个工作节点都要明确指定模型。 这里的分发是为了扩大覆盖范围,而非获取第二意见。每次生成工作节点时都要显式传递协调者使用的模型——“继承”不是一个路由决策,而工作节点默默使用更便宜的模型是最容易丢失发现项的原因。不要引入不同的模型来审查或交叉验证,除非被明确要求:混合模型会导致结果无法归因,当本Skill需要跨模型评估或比较时,一个外来的工作节点就会让数据失效。独立的第二模型验证是一个单独的、显式调用的步骤( 第6步),本Skill绝不会主动使用。
/ship-check告知工作节点它们仅可读,不可编辑。 审查工作节点需要读取、搜索和导航;绝不能修改代码树。要在工作节点的指令中明确说明——开始编辑的工作节点会从审查偏离到“好心”修复,不再报告它悄悄修复的问题,导致发现项无法再对照其描述的代码进行验证。要在提示词中说明,而不是假设宿主会强制执行,并且在运行结束时确认代码树未被修改。
Report
报告
Lead with supported findings, ordered by impact. Keep severity separate from evidential strength.
Review scope:
Comparison baseline:
Selected dimensions:
[Severity] [Dimension] Concrete consequence
Expectation: required behaviour, and the evidence for it
Trigger: feasible preconditions and execution
Defect: the violated relationship
Evidence: source locations for EVERY participant
Impact: observable consequence and affected scope
Refutation: strongest counterargument checked, and why it fails
Remedy: minimal correction to the violated relationship
Verification: what was executed, versus established from source
Coverage:
Unexamined areas and essential unknowns:Cite both participants for a cross-boundary defect, and do not group findings only by file — that
hides the relationship the review exists to find.
Coverage means work performed, not boxes ticked. For each selected sub-case report: examined with
supported findings · examined, none supported · not applicable, with reason · not examined, with
reason. Zero findings in a category does not mean "not covered", and a table of ticks is not
evidence of completeness. Say "no supported findings in the examined scope" — never that the code is
bug-free.
以有依据的发现项开头,按影响排序。将严重程度与证据强度分开。
Review scope:
Comparison baseline:
Selected dimensions:
[Severity] [Dimension] Concrete consequence
Expectation: required behaviour, and the evidence for it
Trigger: feasible preconditions and execution
Defect: the violated relationship
Evidence: source locations for EVERY participant
Impact: observable consequence and affected scope
Refutation: strongest counterargument checked, and why it fails
Remedy: minimal correction to the violated relationship
Verification: what was executed, versus established from source
Coverage:
Unexamined areas and essential unknowns:对于跨边界缺陷,要同时引用双方参与方的代码,不要仅按文件对发现项分组——这会掩盖审查本应发现的关联关系。
覆盖范围指的是实际执行的工作,而非打勾的方框。 对于每个选中的子场景,需报告:已检查且有支持的发现项 · 已检查但无支持的发现项 · 不适用(附原因) · 未检查(附原因)。某个类别零发现项不意味着“未覆盖”,一张打勾的表格也不是完整性的证据。要说“在审查范围内未发现有依据的问题”——绝不要说代码没有bug。
Notes
注意事项
- Say explicitly what is well built. A review that only accuses is easy to dismiss.
- The two sub-cases have mature commands behind them: (trust boundaries, sinks, OWASP backstop) and
/security-audit-static(over-fetching, indexes, caching). Run the command when the sub-case is the whole job; use the reference file when it is one dimension of a broader review. This skill does not restate either./performance-audit-static - For the doc-vs-code axis use the skill.
intended-vs-implemented - A static review produces code-review findings, not confirmed exploits or measured regressions.
- 要明确说明哪些部分设计得很好。只会指责的审查很容易被忽视。
- 两个子场景背后都有成熟的命令:(信任边界、接收点、OWASP兜底检查)和
/security-audit-static(过度获取、索引、缓存)。当子场景是全部任务时,直接运行命令;当它是更广泛审查的一个维度时,使用参考文件。本Skill不会重复阐述这些内容。/performance-audit-static - 对于文档与代码的一致性检查,请使用 Skill。
intended-vs-implemented - 静态审查产出的是代码审查发现项,而非已确认的漏洞利用或可测量的性能退化。