review/skills/code-review/SKILL.md

name, description

name

description

code-review

Review changed code against the Refactoring smell catalog in six groups (bloaters, obscurity, couplers, dispensables, comment contract, shallow modules). Run when a file change is complete or an implementation is complete, e.g. from jsc-sdlc implement. Each group runs as a sub agent over the git diff; findings are reported with file:line, severity, and refactoring, and the caller decides whether to fix. Not a replacement for the CLI's built-in security or bug review.

code-review

Review changed code against references/smells.md (from the book Refactoring). The reference is written in Traditional Chinese; read it as-is.

When to run

1. When a file change is complete: one file or one related group of files is done.
2. When an implementation is complete: all todos of a work package are done. This is the call site in jsc-sdlc:implement — the end of a work package, after its last todo and before it is committed and turned into a PR.

Division of labor

• This skill covers the Refactoring smells, the comment contract (group 5), and shallow modules (group 6).
• Security, logic bugs, and test coverage belong to the CLI's built-in review (e.g. claude's /security-review); do not duplicate them.
• Swagger document auditing belongs to jsc-review:api-doc: response type declarations per HTTP status code, Swagger parameter descriptions, and Swagger examples down the nested data models. Group 5 here covers the source comment contract only; the two never report the same gap twice.

Steps

1. Snapshot the review scope once, to a path that belongs to this run. In one command, expand ${TMPDIR:-/tmp}/jsc-code-review.$$.diff into a variable, redirect git diff (uncommitted changes) or git diff {base}...HEAD (against the base branch when an implementation is complete) into it, and print the expanded path. The $$ is what keeps two runs on one machine apart; a fixed file name lets a second run overwrite the first one's snapshot mid-review, and the six groups in step 2 would then judge bytes that were never their own diff. Read git's exit code: 0 the snapshot is written; any non-zero code means git refused (not a repository, unknown base revision, unreadable object) — report git's stderr verbatim and stop, spawn no sub agent. An empty snapshot file → delete it, report the literal 「無發現」 and stop; spawn no sub agent. The six groups in step 2 all read this one file, so git computes the diff once and every group judges the same bytes. Completion condition: the expanded snapshot path is reported as a literal path — never as the unexpanded $$ pattern — together with a non-empty changed-file list, or the run already ended with 「無發現」 or with git's error.

2. Review in six groups, and every group MUST run as a sub agent; the six groups may run in parallel:

   Group	Scope
   1 Bloaters	smells.md group 1
   2 Obscurity	smells.md group 2. For 2.5 comment scope leaks, report only what a pattern cannot decide: project code names, customer names, and context-dependent review traces. references/comment-scope.md is the single source of the banned and allowed lists — read it, never restate it here. Pattern-detectable items belong to jsc-hooks/hooks/comment-scope.sh and stay outside this group (see Notes for what that leaves uncovered)
   3 Couplers	smells.md group 3
   4 Dispensables & Others	smells.md group 4
   5 Comment contract	smells.md group 5, 5.1 to 5.8 — including 5.6 navigation links to the functions a method calls, 5.7 XML comment tag layout (opening and closing tag each on its own line), and 5.8 code-element markup by language convention (<paramref>, <see cref>, <c> in XML; backticks in JSDoc and docstrings)
   6 Shallow Module	smells.md group 6

   Instructions for each sub agent: read only, change nothing; read the diff snapshot at the exact path step 1 reported, that one file only, and never run git diff again; check every changed line and its enclosing function or class against the group's definitions and detection signals in smells.md.

   Each sub agent returns its findings as TSV rows — one finding per row, six fields separated by a single tab, in this order:

   Field	Content
   1 file	path relative to the repository root
   2 line	line number, a positive integer
   3 severity	高, 中, or 低 on the smells.md scale
   4 group	this group's number, 1 to 6
   5 item	the smell name
   6 detail	one sentence of evidence plus the suggested refactoring

   Fields 5 and 6 are written in Traditional Chinese. Free-form prose is not accepted: tools/merge-findings.sh deduplicates on fields 1 and 2, so any row off this format is rejected and its finding never reaches the report. A group with nothing to report returns the single line 「無發現」 instead of rows.

   Completion condition: all six groups have returned, each with TSV rows or with 「無發現」.

3. Merge with tools/merge-findings.sh. Concatenate the six groups' TSV rows in group order and pipe them into the script on stdin. Start this step only once all six groups have returned; a group still running means the merge waits. Read its exit code: 0 take its stdout as the merged list — deduplicated by location and sorted by severity, 高 first and 低 last; 1 no group returned a row, so report the literal 「無發現」 and stop; 2 environment or input error, so report the script's stderr verbatim and stop, and never hand-merge as a substitute; 3 a group returned a malformed row, and the stderr names which row, so ask that one group's sub agent to re-emit in the format above and run the merge again — after a second 3, report the malformed rows verbatim and stop. Completion condition: the merged list is on hand with every location appearing exactly once, or the run ended with 「無發現」 or with the reported error.

4. Report the finding list in Traditional Chinese, one line per merged row. This skill never modifies code. jsc-hooks/hooks/write-guard.sh in review mode enforces that in code: while this skill runs, it blocks write tools. Which tools those are is declared in that script's own header; never restate the list here, because a stale copy of it reads as coverage the guard does not have. Only claude has a PreToolUse hook, so codex, copilot, antigravity, and kiro never reach that guard — on those four this rule and the sub agent instructions are the only constraint. The caller decides what to fix (inside the implementation flow, 高 and 中 are normally mandatory, 低 is judgment).

   Then close the run down in two moves. Delete the step 1 snapshot file; nothing else ever reads it, and left behind it accumulates one stale diff per review. Release the review lock by running jsc-hooks/hooks/write-guard.sh release, which clears $JSC_HOME/sessions/{sid}.lastskill. That file is how the guard recognizes the running skill, and no event tells the guard a skill ended: leave it in place and the caller's first fix — the fix this very report asked for — is blocked by the audit that just finished. State the fallback in the report either way, because an older jsc-hooks treats release as an unknown mode and exits 0 without clearing anything: the lock then lifts by itself once the file is older than JSC_WRITE_GUARD_TTL (900 seconds by default), and JSC_WRITE_GUARD=off opens it immediately.

   Completion condition: the report is handed to the caller, the snapshot is deleted, the release command has been run and the wait plus the JSC_WRITE_GUARD=off escape hatch are stated, and the fix decision is left to them.

5. Record how this run ended, as the very last thing this skill does — after the snapshot deletion and the release, so a cleanup that did not complete is still visible to it:

   jsc-hooks/tools/report-status.sh skill-end jsc-review:code-review {status} {exit} "{detail}"

   Resolve that path the way step 4 already resolves jsc-hooks/hooks/write-guard.sh — the sibling plugin directory, no separate lookup rule for this one call. A missing script is not a failure here: skip this step in silence and let the run end as it stands. The script swallows its own write errors and exits 0 even then, so nothing branches on its code either. This skill writes no code; it must not start failing over a line it could not write about itself.

   status	This skill's case
   ok	All six groups returned and the merged list reached the caller, the snapshot is deleted and the lock is clear. A merge-findings.sh exit 1 is ok too: six groups read the diff and none had anything to report, which is a real answer
   blocked	The review scope never existed, so no sub agent ran: step 1's git diff refused — not a repository, an unknown base revision, or an unreadable object. Nothing was reviewed and nothing could be
   degraded	The report is handed over but the close-out is short: the step 1 snapshot is still on disk, or write-guard.sh release did not clear $JSC_HOME/sessions/{sid}.lastskill and the lock lingers until JSC_WRITE_GUARD_TTL
   failed	The six groups ran and their findings never reached a report: merge-findings.sh exit 2, or a second exit 3 with malformed rows still on the table
   aborted	There was nothing to review — the step 1 snapshot came back empty, so the diff holds no changed line, the file was deleted and no sub agent was spawned. Keep this apart from the ok above: both end in 「無發現」, and only the event tells a clean review from a review that never had a subject

   {exit} is the exit code of whatever decided the status, 0 for ok. {detail} is one short line well under 200 characters: changed-file and finding counts plus exit codes, never file paths, code excerpts, branch names, or personal data.

   Completion condition: the command has run, or the script was absent and this step was skipped.

Notes

• jsc-hooks/hooks/comment-scope.sh already matches the pattern-detectable comment scope items after every file write. Group 2 covers what patterns cannot decide — project code names, customer names, and context-dependent review traces — plus the overall judgment; the two never report the same finding twice.
• Removed protection, on purpose. Group 2 used to re-scan the pattern-detectable items as well, which made it the last net whenever the hook was not running: JSC_COMMENT_SCOPE=off turns comment-scope.sh off entirely, and a CLI with no comment-scope.sh wiring never runs it at all. That net is gone. On such a run, an issue id, a wiki page id, a work package id, or a commit hash left in a comment now reaches the commit with nothing catching it. Run jsc-review:comment-cleanup before commit whenever the hook is off or unwired.
• If group 5 examples are fetched from a database, they must be de-identified; never include personal data.
• When there are no findings, report the literal 「無發現」 explicitly; never leave the report empty.