SuperAPI AI Directory
Partner with us中文AI newsFavorites
Skillscs-code-review

cs-code-review

Testing & quality

View repository

Updated 10-16 05:05

About this skill

cs-code-review

Required reading at startup

Before making any judgment or taking any action, first run the CodeStable preflight: read .codestable/attention.md; if missing, first cs-onboard; do not substitute by reading an external AI entry point (see .codestable/reference/execution-conventions.md）。

This skill is across-cutting code review gate: after any process implementation is complete and before commit / QA / acceptance, perform an independent read-only review of the current changes. It only reads code and artifacts, and only writes {slug}-review.md, does not directly fix code, does not update the checklist, does not modify the spec, and does not replace QA or acceptance.

The goal of the review is not to pursue perfect code, but to confirm that this round's changes have not reduced the system's code health and do indeed move toward the goal of the corresponding spec (design / fix-note / refactor-design / user-confirmed scope). Do not manually block on issues that can be automatically formatted or linted; issues that affect correctness, maintainability, security, performance, testability, requirement satisfaction, or the credibility of subsequent acceptance must be pointed out.

For shared paths and naming conventions see .codestable/reference/shared-conventions.md Section 0. Report language: the body of the code review report is by default inChinese(see .codestable/attention.md the report language section); frontmatter / yaml fields are not translated.

Entry sources (cross-cutting)

Source Entry point spec artifact Destination after passing
cs-feat-impl(standard feature) impl complete, before QA design + checklist cs-feat-qa
cs-feat-ff(feature fast track) ff-note written to disk, before commit ff-note + user's original requirement final commit
cs-issue-fix fix-note written to disk, before commit report + analysis + fix-note final commit
cs-refactor apply-notes complete, before commit scan + refactor-design + checklist final commit
cs-refactor-ff self-verification passed, before commit user-confirmed refactor scope + verification command final commit
ad-hoc / pre-merge user request user-specified scope / git range give conclusion

Not cs-audit: audit actively scans a swath of code to find potential issues; code review only reviews the current change scope.

Input

Must read before entering review:

  • .codestable/attention.md
  • the source's spec artifacts (for feature see {slug}-design.md + {slug}-checklist.yaml; for issue see report+analysis+fix-note; for refactor see scan+refactor-design+checklist; for ff / ad-hoc see user-confirmed scope)
  • implementation completion report / most recent implementation record (if in the conversation, cite per conversation facts; if already written to a file, read the file)
  • git status --short
  • git diff(when there is a staged diff also read git diff --cached）
  • the human-written code files involved in the diff and adjacent key call sites
  • architecture / requirement / roadmap related documents pointed to by the spec (read-only, to judge whether the changes affect merging; for feature that is design Section 4)
  • evidence pack, gate results, DoD results in goal / gate mode; if missing, go back to the implementation gate to supplement evidence, do not guess on the spot
  • independent Task agent reviewer output (if started this round)

If the workspace has pre-existing dirty files outside this round's scope, first record them as baseline/unrelated changes; the review conclusion only targets changes attributable to this round. When attribution cannot be distinguished, write it as residual-risk, do not treat uncertainty as a pass.

Startup check

First confirm this round's source using the "Entry Source" table, then perform the corresponding prerequisite validation:

  1. The source's spec artifacts exist and are finalized — for feature, see {slug}-design.md（doc_type=feature-design、status=approved、feature matches the directory) + {slug}-checklist.yaml（steps all done); for issue, see report+analysis+fix-note; for refactor, see scan+refactor-design+checklist; for ff / ad-hoc, see the user-confirmed scope. If the finalized spec is missing, return to the corresponding implementation skill; do not review forcibly.
  2. In goal / gate mode, first read {slug}-evidence-pack.md、{slug}-gate-results.json and {slug}-dod-results.json; if missing or gate blocking is unexplained, return to implementation.before_review.
  3. The current diff shows this round's implementation changes; if there are no code or artifact changes at all, return to the source implementation skill.
  4. If there is already {slug}-review.md：
    • status: passed and the diff has not changed: prompt to proceed to "Post-Pass Destination" per the table.
    • status: changes-requested / blocked: read the old findings to confirm whether this is a re-review after review-fix.
    • The diff has changed: re-review, and record in the report which round this is.

Independent Task agent reviewer (two stages)

This stage is atwo-stage review, and the two stages complement each other:

Stage Purpose Required for gate
A Independent Task agent review Holistic review from the spec perspective, usingan independent contextto avoid the main agent's confirmation bias Yes (gate passage depends on it)
B OCR line-level scan Line-level bug / security / code pattern scan, covering A's blind spots No (run it if installed, degrade if missing)

Detection is done by the main agent self-checking its own tools at runtime, not by scripts guessing the environment — the main agent knows best which tools it has on hand.

Stage A: Independent Task agent review (required for gate)

The main agent starts an independent Task agent reviewer according to the Task agent selection rules (prefer Paseo subagent, otherwise the current host's native Codex/Claude Task/Agent; if neither is available, handle via local-only fallback):

  1. Has the mcp__paseo__create_agent tool → use Paseo subagent (preferred: can switch providers, achieving true heterogeneous review). First load / read paseo skill's current instructions and follow its rules: read ~/.paseo/orchestration-preferences.json, use providers.audit, do not hardcode the provider; if the file is missing, use reasonable defaults and explain in the report.
  2. Otherwise, has the current host's native Codex/Claude Task/Agent tool → start a native Task agent. An independent context is likewise achieved; if it is the same model, record the degradation and residual risk in the report.
  3. Neither is available → record local-only, and do not pretend to have started one. By default the gate does not pass; explicit user degradation is required (see the reviewer field).

Independent Task agent reviewer prompt (provide only raw materials, do not disclose any of the main agent's review conclusions):

你是 CodeStable 本次改动的独立代码审查 agent。只读，不修改文件，不更新 checklist/design。

请读取：
- .codestable/attention.md
- {design_path}
- {checklist_path}
- {evidence_pack_path}、{gate_results_path}、{dod_results_path}（存在时）
- 当前 git status / git diff / staged diff
- diff 涉及的人写代码和相邻关键调用点

按严重度输出：blocking / important / nit / suggestion / learning / praise / residual-risk。
每条 finding 必须有 file:line 或仓库事实证据、影响、建议修复边界。
额外输出 Test And QA Focus：QA 必须重点复核的场景、建议新增或加强的测试、review 无法确认的点。
不要写 {slug}-review.md；只把审查结果回传给主 agent。

Stage B: OCR line-level scan (run it if installed)

The main agent self-checks whether the ocr CLI is available:

which ocr && ocr llm test

If available, call it automatically (no explicit user request needed), passing --background to provide a spec summary and improve quality:

ocr review --audience agent --background "{feature slug / 目标 / 本次改动范围}"

OCR can only review the scope for the current round; it cannot discover the scope using bare workspace mode.ocr review By default it reviews staged + unstaged + untracked; when a Roadmap runs multiple features consecutively, historical dirty/untracked files will cause OCR to scan an ever-growing scope. Invocation rules:

  1. First establish current_scope_files: prefer taking the changed_filesfrom the scope-gate / evidence pack, otherwise take {feature-base}..HEAD or the current-round files from the implementation report.
  2. Committed diff → use ocr review --audience agent --from {feature-base} --to HEAD。
  3. Uncommitted diff → only if all non-ignored paths under git status --short belong to current_scope_filesmay you run ocr reviewbare; otherwise record skipped-scope-ambiguousand switch to a local line-level review current_scope_files。
  4. Before merging OCR findings,discardfindings that hit the following paths, regardless of whether OCR scanned them——
    • .codestable/(CodeStable's own spec / tool artifacts, which are never the object of line-level code review)
    • Any directory starting with . (.git/、.claude/、.venv/ , etc.)
    • .gitignore Files hit bygit check-ignore <path> (when .gitignore is true) Untracked, not ignored by current_scope_files new files must be covered by this round's review.

OCR's High / Medium / Low are mapped to cs-code-review severities and then merged:

OCR priority Mapping rule
High Backed by repository facts → blocking/important; insufficient evidence → residual-risk
Medium nit or suggestion
Low Discard (treated as noise); it does not enter the report

OCR does not make spec-fit judgments; a mapped finding must undergo local fact verification by the main agent before it can be escalated to blocking。ocr not installed / ocr llm test failure → record not-available, prompt the user to go through the open-code-review section of cs-onboard to install and configure it; do not block this round. Note that an ocr llm test timeout (context deadline exceeded) is most likely because the configuration uses the old llm.* block——ocr v1.x requires the provider/providers system; see the onboard section or the OCR checkup for codestable-doctor.py .

Merging principles

Links A and B can be started in parallel. Once a link has started, the main agent must not finalize {slug}-review.md, give passed , or proceed to post-pass routing before it returns。

  • A started reviewer returns → verify each item against local facts, deduplicate, merge into the report, and keep source annotations (paseo / native-agent / ocr / local）。
  • Reviewer fails / gets stuck / is blocked by permissions → report status: blocked, record pending|failed|blocked and the reason, and let the user decide: retry, wait, or explicitly degrade.
  • Do not poll indefinitely; wait for notification or for the user to bring back results.

External conclusions that have not been verified against local repository facts may only be written as residual-risk or ignored; they cannot be directly escalated to blocking。

reviewer field (gate anchor); write it according to the links actually completed this round:

Value Meaning gate
subagent+ocr Both link A (Task agent) + link B completed (ideal) Pass
subagent Link A completed, OCR unavailable / skipped Pass
ocr OCR only, missing Task agent review Override required
self Both links missing Override required

The gate by default requires subagent or subagent+ocr；ocr and self requires CODESTABLE_ALLOW_SELF_REVIEW_FALLBACK=1 before it can pass. OCR cannot replace stage A—different purposes (A is spec isolation review, B is line-level scanning).

Review process

1. Context and scope

  • Use sections 1/2/3 of the design to confirm the goal, explicitly out of scope, key decisions, and acceptance scenarios.
  • Use checklist steps to confirm the scope that the implementation claims to have completed.
  • Use git status / git diff to list the actual changed files, marking added, modified, deleted, untracked, and staged.
  • Assess diff size and risk: cross-module, cross-boundary, data migration, permissions/security, concurrency/async, user-visible UI, public API, test gaps.

2. Independent review merge

  • Main agent self-check tool: whether there is mcp__paseo__create_agent / native Codex/Claude Task/Agent tools,which ocr and whether it is available.
  • Stage A (independent Task agent, Paseo subagent preferred → native Task agent) and stage B (OCR) can be started in parallel without waiting for each other.
  • Locally you can first make an overall review draft, but the final verdict must wait until all started reviewers return before it is finalized.
  • After each stage returns: verify each item locally against facts, map OCR results to severity by High/Medium/Low, and only adopt what can be reproduced / has evidence; if evidence is insufficient, write only residual-risk.
  • If a stage fails / gets stuck: do not silently downgrade; report that stage's failed|blockedand let the user decide whether to retry or explicitly downgrade.
  • Keep sources in the report: mark which reviewer each finding comes from (paseo / native-agent / ocr / local）。

3. Overall review

Look at the whole first, then the line-level details:

  • design fit: whether the implementation satisfies the design without secretly expanding scope.
  • architecture fit: whether new code is placed at the correct layer, and whether it bypasses existing abstractions, introduces reverse dependencies, or excessive coupling.
  • complexity: whether it introduces over-generalization, patch branches, parameter bloat, or continued growth of large functions/classes for the current problem.
  • test strategy: whether existing tests and newly added tests can prove the key scenarios; whether tests would actually fail when the code breaks.
  • risk surface: error handling, data validation, security boundaries, permissions, concurrency, idempotency, performance, observability, rollback/uninstall.
  • documentation/merge impact: whether there are architecture / requirement / roadmap changes that acceptance must write back.

4. Line-level review

For human-written code, review file by file, covering at least:

  • logical correctness: boundary values, null values, exception paths, state transitions, timing issues, off-by-one.
  • error handling: whether error semantics are clear, whether errors are swallowed, and whether recovery logic and business logic are mixed together.
  • data and security: input validation, injection risk, sensitive information, permission checks, cross-tenant/cross-user isolation.
  • performance and resources: repeated IO, N+1, unbounded loops/caches, memory leaks, unreleased resources.
  • concurrency/async: races, deadlocks, cancellation, reentrancy, duplicate submission, idempotency.
  • maintainability: whether naming follows design terminology, whether existing helpers are reused, and whether duplicate logic is newly added.
  • cleanliness: debug output, temporary TODO/FIXME, commented-out code, unused imports, files outside the plan.

Generated code, lockfiles, and large data files may be sampled, but the report must state the sampling scope. Human-written business code must not be skipped.

5. Conclusion

Classify findings by severity and give a clear verdict:

  • passed: no blocking; important items have been fixed, there are no important items, or the user has explicitly accepted deferral.
  • changes-requested: there is blocking, or there are so many important items that they would affect acceptance credibility.
  • blocked: key inputs are missing, diff attribution cannot be determined, the design/implementation state does not meet the review preconditions, or an independent Task agent reviewer has been started for this round but the result is still pending / failed / blocked and the user has not confirmed a downgrade.

reviewer field (gate anchor)：{slug}-review.md frontmatter reviewer determines whether the downstream worktree / commit / finish gate is released; write it according to the actual completion status of the "independent Task agent reviewer (two-stage)" subagent+ocr / subagent / ocr / self(semantics as in the table in the previous section). If any started stage is still pending / failed / blocked, do not finalize passed, and do not write subagent。

Severity

  • blocking: must be fixed first. It will cause functionality to fail to meet the design, data/security/permission risks, obvious bugs, acceptance being impossible to execute credibly, serious architectural regression, or tests being completely unable to cover key risks.
  • important: should be fixed; if the user decides to defer, it must be explicitly recorded in the review report and acceptance residual risk.
  • nit: a small clarity or consistency suggestion, non-blocking.
  • suggestion: an alternative implementation idea, not required to be adopted this time.
  • learning: an informational explanation, no action required.
  • praise: records good practices worth preserving; a small number is enough.
  • residual-risk: uncertainty that review cannot fully eliminate and that requires focused QA / acceptance rechecking.

Do not escalate personal preferences into blocking. Blocking must be supported by repository facts, design contracts, reliable engineering principles, or reproducible examples.

Report template

The report goes in the spec directory of the source process, with filename {slug}-review.md; the feature source is .codestable/features/{feature}/{slug}-review.md, and issue/refactor etc. go in their respective process directories.

See the same package for the complete frontmatter and each section template references/report-template.md(resolve it based on the directory where the loaded SKILL.md is located; do not guess the path based on the business repository root). When there is no finding of a certain type, write none, do not delete the section; the next round of re-review must be able to compare.

review-fix handoff

The next destination is determined by the "source entry" table (the feature source is review-fix→cs-feat-impl, pass→cs-feat-qa; issue/refactor/ff each return to the corresponding implementation skill and commit wrap-up).

If there is blocking：

  1. report status: changes-requested。
  2. tell the user the next step is to trigger the review-fix mode of the source implementation skill.
  3. review-fix only fixes blocking findings; whether to fix important ones is up to the user or the implementer, but the scope must not be expanded opportunistically.
  4. After review-fix is complete, this review must be rerun; do not skip it and go directly to the source's post-pass destination.

If there is only important：

  • by default it is recommended to fix first; if the user explicitly accepts deferral, move it into residual risk in the report and allow proceeding to the post-pass destination.

If there is no blocking, and important items have been handled or explicitly accepted:

  • report status: passed。
  • tell the user the next step is the post-pass destination in the "source entry" table (feature→cs-feat-qa）。

Exit conditions

  • Read attention, the source spec artifact, implementation evidence, git status, git diff, and the relevant code.
  • Confirmed the source spec artifact is finalized (for feature, check that the checklist steps are all done); otherwise fall back to the source implementation skill.
  • The main agent has self-checked Task agent capabilities (Paseo subagent / native Codex/Claude Task/Agent) and ocr CLI, and recorded availability.
  • Phase A (independent isolated agent) and Phase B (OCR, when available) have both been started, or the reason for skipping is recorded.
  • All started phases have returned and been locally verified item by item for merge / rejection; otherwise report status: blocked, without entering QA.
  • Performed overall review and line-level review.
  • Clearly distinguished blocking / important / nit / suggestion / learning / praise / residual-risk.
  • Written to the source spec directory {slug}-review.md(feature, i.e. .codestable/features/{feature}/{slug}-review.md）。
  • status: passed when frontmatter reviewer has been written according to the actual completion of both phases as subagent+ocr / subagent(or it genuinely belongs to the no Task agent capability ocr / self fallback) — this is the release anchor for the downstream gate.
  • When there is blocking, did not enter downstream, but instead pointed to the review-fix of the source implementation skill.
  • When there is no blocking, clearly told the user the post-pass destination according to the "enter source" table (feature→cs-feat-qa）。

Common pitfalls

  • Fixing code while reviewing, turning a read-only gate into an implementation phase.
  • Only looking at the implementation report, not the real git diff。
  • Only looking at whether tests pass, without judging whether the tests are effective.
  • Escalating formatting, naming preferences, and personal style into blocking.
  • Discovering implementation outside the design but not going back to the design contract to judge it.
  • Copying external reviewer (Paseo subagent / native-agent / OCR) conclusions directly into blocking without local factual verification.
  • Mapping OCR High directly to blocking, skipping local verification.
  • Finalizing the local review as passed before a certain reviewer path has returned.
  • Silently downgrading to local-only when a certain reviewer path is stuck, without asking the user.
  • Skipping re-review after blocking is fixed and going straight to acceptance.
  • The review report is not persisted to disk, leaving acceptance without a traceable input.