harness-tools/skills/develop/code-review/SKILL.md

name, description

name	description
code-review	Review changes since a fixed commit, branch, tag, or merge-base along separate repository-standards and specification axes. Use for branch, PR, work-in-progress, or workflow-record review after implementation and verification.

Code review

Run two independent, read-only reviews over one pinned diff:

• Standards: repository instructions and documented conventions.
• Specification: the originating issue, spec, or approved workflow record.

Keep the axes separate so conformity cannot hide incorrect scope and correct scope cannot hide standards violations.

1. Pin the review bytes

Require a fixed point; ask when none was supplied. Resolve and record:

• review base commit;
• immutable product HEAD commit, never mutable HEAD in the reviewer brief;
• clean status and product tree hash;
• git log <base>..<product-head> --oneline;
• exact command git diff --binary --full-index --find-renames <base>...<product-head> --.

Run that command once in the parent, hash its exact bytes with SHA-256, and record the digest. Stop on a bad ref, dirty product state, or empty diff unless an empty comparison was explicitly expected.

2. Resolve specification and standards

Accept the first available specification source:

1. a caller-supplied approved workflow record or spec path;
2. issue references in commits, using docs/agents/issue-tracker.md when configured;
3. a matching file under docs/, specs/, or .scratch/;
4. an explicit user statement that no specification exists.

A workflow record is a complete specification source and never requires issue-tracker configuration. Ask only when no source or explicit no-spec decision exists. Skip the specification reviewer and report “no specification available” only after that explicit decision.

Collect applicable AGENTS.md, CLAUDE.md, contributing, coding-standard, and ADR sources. Add this smell baseline where repository standards do not override it; treat each as a judgment call and skip tooling-enforced issues:

• Mysterious Name: a name hides purpose.
• Duplicated Code: the same logic shape recurs.
• Feature Envy: behavior reaches into another object's data more than its own.
• Data Clumps: fields or parameters repeatedly travel together.
• Primitive Obsession: primitives stand in for a domain concept.
• Repeated Switches: the same type cascade recurs.
• Shotgun Surgery: one change requires scattered edits.
• Divergent Change: one module changes for unrelated reasons.
• Speculative Generality: abstraction exists without a current requirement.
• Message Chains: callers navigate long object chains.
• Middle Man: code delegates without adding value.
• Refused Bequest: an implementation ignores most inherited behavior.

3. Enforce reviewer independence

List every display name, profile, child/session ID, and ambiguous run that authored or mutated the diff. Select two different reviewer identities outside that set. Require repository read access plus Git-capable bash, with no declared write, edit, or nested-agent capability.

Capture branch, HEAD, status, tree hash, and changed paths before review. Launch the standards and specification reviewers as separate direct subagent calls rather than a wrapped parallel call. Give both the pinned commits, exact diff command, expected digest, commit list, mutator exclusions, and identity-reporting requirement.

Require each reviewer to:

1. regenerate the exact diff and confirm its SHA-256 before analysis;
2. remain read-only and perform no remediation;
3. bind every finding to file/hunk evidence and severity;
4. report display/profile/child/session identity when exposed;
5. stay under 400 words.

For standards, include every standards source and the full baseline above. Ask for documented violations separately from baseline judgments. For specification, include the approved source and ask for missing/partial requirements, scope creep, and apparently implemented but incorrect behavior with quoted requirements.

Treat missing delivery, digest mismatch, reviewer/mutator identity overlap, or before/after repository mutation as no verdict. Any reviewer mutation also makes that identity a mutator.

4. Aggregate without reranking

Present the reports under ## Standards and ## Specification. Lightly clean formatting only; keep findings and axis severity separate. End with finding counts and the worst issue within each axis, not one combined winner.

Bind approval to the base, product HEAD, clean status, tree hash, and diff digest.

5. Remediate through non-reviewers

Validate findings against repository evidence. Assign accepted fixes to an original implementer or another non-reviewer. Rerun affected checks and the full required suite, create a new pinned fingerprint, and launch two fresh independent reviewers.

Escalate after two unsuccessful remediation cycles or when a finding materially changes scope, acceptance criteria, ownership, verification, or architecture. Return material changes to plan approval before production mutation.

Complete when: both axes approve the exact verified fingerprint and before/after evidence proves reviewers stayed read-only.