Merge pull request 'chore(release): 放行技能組稽核修正到預設分支' (#20) from develop into master

Reviewed-on: #20
Reviewed-by: 系統管理員 <1+admin@noreply.localhost>
This commit was merged in pull request #20.
This commit is contained in:
2026-08-31 03:54:52 +00:00
10 changed files with 298 additions and 37 deletions
+1 -1
View File
@@ -1,6 +1,6 @@
{
"name": "jsc-review",
"version": "0.0.9",
"version": "0.1.0",
"description": "程式碼審查:Refactoring 壞味道六組 + 註解規範 + 淺模組",
"skills": "./skills",
"author": {
+1 -1
View File
@@ -1,6 +1,6 @@
{
"name": "jsc-review",
"version": "0.0.9",
"version": "0.1.0",
"description": "程式碼審查:Refactoring 壞味道六組 + 註解規範 + 淺模組",
"skills": "./skills",
"jsc": {
+8 -4
View File
@@ -26,15 +26,17 @@ Marketplace 統一為 `jsc`(https://gitea.jsc.idv.tw/plugins/meta.git),安
### `code-review`
對 git diff 進行六組壞味道審查,每組一個 sub agent 平行執行;回報 `檔案:行號`、嚴重度、建議重構手法,修正與否由呼叫端決定。第 2 組同時擋「文件編號與審查流程痕跡夾帶」:註解只寫「為什麼這樣寫」,議題編號、wiki 頁編號、工作包編號、commit hash、`@` 提及、外部文件連結、審查輪次、發現編號與審查狀態一律不進註解,清單看 `references/comment-scope.md`。diff 是空的就直接回報「無發現」,不開任何 sub agent;六組全部回覆才進入彙整,沒東西可報的那組也要回「無發現」。安全性與 bug 審查交給 CLI 內建 review,不重複。
對 git diff 進行六組壞味道審查,每組一個 sub agent 平行執行;回報 `檔案:行號`、嚴重度、建議重構手法,修正與否由呼叫端決定。步驟 1 先把 diff 落成一份快照檔,六組讀同一份,git 只算一次,六組的結論也一致。第 2 組的註解範圍只管樣式判不出來的部分:專案代號、客戶名稱與情境相關的審查痕跡;禁止清單與允許清單的唯一來源是 `references/comment-scope.md`,這裡不再抄一份。樣式判得出來的項目由 `jsc-hooks/hooks/comment-scope.sh` 擋。diff 是空的就直接回報「無發現」,不開任何 sub agent;六組全部回覆才交給 `tools/merge-findings.sh` 彙整,沒東西可報的那組也要回「無發現」。安全性與 bug 審查交給 CLI 內建 review,不重複。
> 刻意移除的保護:第 2 組本來連樣式判得出來的項目也重掃一遍,所以 `JSC_COMMENT_SCOPE=off` 或該 CLI 沒接上 `comment-scope.sh` 時,第 2 組是最後一道網。現在那道網沒有了。hook 關掉或沒接線時,議題編號、wiki 頁編號、工作包編號、commit hash 留在註解裡不會有人擋,commit 前請改跑 `/jsc-review:comment-cleanup`。
### `api-doc`
稽核 API 專案的 Swagger 文件:每個可能回傳的 HTTP 狀態碼都要宣告回覆類型,每個輸入與輸出都要有說明與真實資料範例,並沿著巢狀資料模型逐層遞迴。控制器改完或實作完成時執行,例如由 `jsc-sdlc:implement` 呼叫,與 `jsc-review:code-review` 並列為兩關收尾稽核。先跑 `tools/swagger-detect.sh` 偵測,套件與掛接設定要雙重命中才算支援;只裝套件沒掛接就回報未啟用並停手,不開任何 sub agent。原始碼的註解契約歸 `jsc-review:code-review` 第 5 組,這支只管 Swagger 文件屬性與範例,兩支不重複回報。回報 `檔案:行號`、嚴重度與建議修法,本技能不改程式碼。
稽核 API 專案的 Swagger 文件:每個可能回傳的 HTTP 狀態碼都要宣告回覆類型,每個輸入與輸出都要有說明;範例只掛在純量成員上,類別型成員只留說明,範例責任往下推給它的屬性,一路遞迴到最內層的純量。集合看元素型別判斷:元素是純量就比照純量,附一份列出幾個元素的範例;元素是類別就比照類別,只留說明,遞迴改走進元素型別。控制器改完或實作完成時執行,例如由 `jsc-sdlc:implement` 呼叫,與 `jsc-review:code-review` 並列為兩關收尾稽核。先跑 `tools/swagger-detect.sh` 偵測,套件與掛接設定要雙重命中才算支援;只裝套件沒掛接就回報未啟用並停手,不開任何 sub agent。稽核分兩個面向,各一個 sub agent 平行執行:狀態碼一個,說明與範例連同巢狀資料模型的遞迴合成一個,同一批資料模型檔只讀一次,兩份檢核表分段列出。兩個面向都回覆才交給 `tools/merge-findings.sh` 彙整。原始碼的註解契約歸 `jsc-review:code-review` 第 5 組,這支只管 Swagger 文件屬性與範例,兩支不重複回報。回報 `檔案:行號`、嚴重度與建議修法,本技能不改程式碼。
### `comment-cleanup`
清理本次變更新增或修改的註解,把文件追蹤資訊與審查流程痕跡移除,只留下程式邏輯的實質理由。使用者要求「清一下註解」、「不要留 review 痕跡」,或 commit 前發現註解夾帶流程資訊時執行。判斷準則只看 `references/comment-scope.md`,本技能負責清理,`comment-scope.sh` 負責擋,`code-review` 第 2 組負責指出人工判讀案例。未明確要求時不動未變更的舊註解。
清理本次變更新增或修改的註解,把文件追蹤資訊與審查流程痕跡移除,只留下程式邏輯的實質理由。使用者要求「清一下註解」、「不要留 review 痕跡」,或 commit 前發現註解夾帶流程資訊時執行。判斷準則只看 `references/comment-scope.md`,本技能負責清理,`comment-scope.sh` 負責擋,`code-review` 第 2 組負責指出人工判讀案例。改寫每個檔案一個 sub agent 平行跑,建置與測試留到最後統一跑一次。未明確要求時不動未變更的舊註解。
<!-- JSC-SKILLS:END -->
@@ -49,7 +51,9 @@ Marketplace 統一為 `jsc`(https://gitea.jsc.idv.tw/plugins/meta.git),安
| 檔案 | 用途 |
| --- | --- |
| `tools/swagger-detect.sh` | 判斷專案有沒有真的啟用 Swagger。套件與設定雙重確認,缺一不算支援。輸出 `support=`、`stack=`、`package=`、`config=`;結束碼 0 支援、1 不支援、2 參數或路徑錯誤 |
| `tools/swagger-detect.sh` | 判斷專案有沒有真的啟用 Swagger。套件與設定雙重確認,缺一不算支援。輸出 `support=`、`stack=`、`package=`、`config=`;結束碼 0 支援、1 不支援、2 參數個數不對、路徑不存在或環境缺 grep |
| `tools/merge-findings.sh` | 合併 `code-review` 六組與 `api-doc` 兩個面向的發現。輸入是六欄 TSV(檔案、行號、嚴重度、組別、發現名稱、說明),同一個檔案與行號的發現併成一列,再依 高 → 中 → 低 排序。結束碼 0 有發現、1 無發現、2 參數或環境錯誤、3 輸入格式錯誤 |
| `tools/changed-comments.sh` | 列出本次變更新增或修改的註解行,供 `comment-cleanup` 決定清理範圍。不給參數比對工作區與 HEAD,給基準版本則比對 `{base}...HEAD`。輸出三欄 TSV(檔案、行號、內容);註解樣式與非程式碼副檔名清單沿用 `jsc-hooks/hooks/comment-scope.sh` 的同一份。結束碼 0 有結果、1 無結果、2 參數或環境錯誤 |
## 相關 domain
+1 -1
View File
@@ -1,6 +1,6 @@
{
"name": "jsc-review",
"version": "0.0.9",
"version": "0.1.0",
"description": "程式碼審查:Refactoring 壞味道六組 + 註解規範 + 淺模組",
"skills": "./skills/",
"jsc": {
+6 -5
View File
@@ -140,14 +140,15 @@
### 5.4 輸入與輸出範例
- **定義**:輸入與輸出參數都必須有範例;範例內容**優先嘗試從資料庫取得真實資料,失敗才透過邏輯推理**產生。
- **偵測訊號**:註解缺範例;範例與型別不符;範例顯然是佔位假資料而環境可取得真實資料。
- **建議重構手法**:以可用的連線查詢一筆代表性資料當範例(去識別化,不可含個資);無法連線才以邏輯推理造出合理範例並標明為推理值。
- **定義**:輸入與輸出參數都必須有說明,範例則只掛在**純量**成員上。純量參數與純量屬性(字串、數值、布林、日期、列舉)要有範例。類別型參數與中間層的類別屬性只要說明,自己不掛範例,範例責任往下推給該類別的屬性。集合看元素型別判斷:元素是純量就比照純量,附一份列出幾個元素的範例;元素是類別就比照類別,只留說明,往下走進元素型別。字典看值型別,判準相同。範例內容**優先嘗試從資料庫取得真實資料,失敗才透過邏輯推理**產生。
- **偵測訊號**:純量參數或純量屬性缺範例;範例與型別不符;範例顯然是佔位假資料而環境可取得真實資料;範例掛在類別型成員上,該類別的屬性卻一個範例都沒有。
- **建議重構手法**:以可用的連線查詢一筆代表性資料當範例(去識別化,不可含個資);無法連線才以邏輯推理造出合理範例並標明為推理值;範例掛錯層就往下搬到該類別的每個純量屬性上。
- **注意**:型別判準與 `jsc-review:api-doc` 的檢核表 A 完全相同,同一個成員在兩邊判出來的答案一樣。分工不變:本項只看原始碼註解,Swagger 文件屬性歸 `api-doc`,同一個標的不重複回報。
### 5.5 巢狀結構註解
- **定義**:如果參數有巢狀結構(例如 class 內還有 class),就必須完全補齊每一層的註解。
- **偵測訊號**:DTO/ViewModel 僅頂層有註解;內層類別、集合元素型別的欄位無說明或無範例。
- **定義**:如果參數有巢狀結構(例如 class 內還有 class),就必須完全補齊每一層的註解。每一層的純量屬性要有說明與範例,中間層的類別屬性只要說明;集合往元素型別走,遞迴走到「屬性全是純量」那一層為止。
- **偵測訊號**:DTO/ViewModel 僅頂層有註解;內層類別、集合元素型別的欄位無說明;任一層的純量屬性缺範例;遞迴半途停住,某一層的屬性完全沒被走到。
- **建議重構手法**:逐層補齊 5.1–5.4;巢狀過深(≥ 3 層)時同時評估 Extract Class 是否被濫用。
### 5.6 內含功能的導向連結
+33 -12
View File
@@ -1,6 +1,6 @@
---
name: api-doc
description: Audit an API project's Swagger/OpenAPI documentation: every returnable HTTP status code declares a response type, and every input and output carries a description plus a real data example, recursively down nested data models. Run after controller work or an implementation is complete, e.g. from jsc-sdlc implement, next to jsc-review:code-review. Detection runs first via tools/swagger-detect.sh; without both the Swagger package and its wiring, report unsupported and stop. The source comment contract belongs to jsc-review:code-review group 5; this skill covers Swagger document attributes only. Findings carry file:line, severity, and the fix; this skill never modifies code.
description: Audit an API project's Swagger/OpenAPI documentation: every returnable HTTP status code declares a response type, every input and output carries a description, and a real data example sits on scalar members only — a data-model member takes a description and hands the example duty down to its own properties, recursively to the innermost scalar. Run after controller work or an implementation is complete, e.g. from jsc-sdlc implement, next to jsc-review:code-review. Detection runs first via tools/swagger-detect.sh; without both the Swagger package and its wiring, report unsupported and stop. The source comment contract belongs to jsc-review:code-review group 5; this skill covers Swagger document attributes only. Findings carry file:line, severity, and the fix; this skill never modifies code.
---
# api-doc
@@ -21,22 +21,40 @@ Audit whether an API project's Swagger (OpenAPI) documentation is complete enoug
## Steps
1. Detect Swagger support: run `tools/swagger-detect.sh {project path}` (defaults to the current directory). The script confirms package **and** wiring, so an installed-but-never-enabled project comes back unsupported. Read its exit code: `0` supported, `1` unsupported, `2` bad argument or missing path. Completion condition: the exit code and the `support=`, `stack=`, `package=`, `config=` lines are captured.
2. On exit code `1`, report the literal 「本專案未啟用 Swagger 文件,略過 API 文件稽核」 plus the `stack=` and `package=` lines the script printed, and stop. Spawn no sub agent. On exit code `2`, report the script's error message verbatim and stop; the caller supplies a valid project path and re-runs. Completion condition: the run has ended with an honest reason, or exit code `0` moved it to step 3.
3. List the audit scope: every controller in the project, or only the controllers touched by `git diff` when the caller asked for a scoped run. Completion condition: the controller list is non-empty and reported; an empty list ends the run with the literal 「無發現」.
4. Audit in three aspects, and every aspect **MUST run as a sub agent**; the three may run in parallel:
1. Detect Swagger support: run `tools/swagger-detect.sh {project path}` (defaults to the current directory). The script confirms package **and** wiring, so an installed-but-never-enabled project comes back unsupported. Read its exit code: `0` supported; `1` unsupported; `2` one of three causes — more than one argument, a project path that does not exist, or no `grep` in the environment. Completion condition: the exit code and the `support=`, `stack=`, `package=`, `config=` lines are captured.
2. On exit code `1`, report the literal 「本專案未啟用 Swagger 文件,略過 API 文件稽核」 plus the `stack=` and `package=` lines the script printed, and stop. Spawn no sub agent. On exit code `2`, report the script's stderr message verbatim, then route by which of the three causes that message names: the usage line means the call passed more than one argument, so re-run with at most one path; 「找不到專案路徑」 means the caller supplies an existing project path and re-runs; 「環境缺 grep」 means the environment itself is broken, so report that `grep` must be installed and stop — re-running with another path never clears this one. Spawn no sub agent in any of the three. Completion condition: the run has ended with the cause named and the matching next action stated, or exit code `0` moved it to step 3.
3. List the audit scope: every controller in the project, or only the controllers touched by `git diff` when the caller asked for a scoped run. On a scoped run, read git's exit code: `0` the changed controllers are the scope; 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. Never fall back to the whole project there: the caller asked for one scope, and quietly auditing a much larger one buries the git failure under a far longer run. `jsc-review:code-review` step 1 stops on the same failure, so both skills answer a broken `git diff` the same way. Completion condition: the controller list is non-empty and reported; an empty list ends the run with the literal 「無發現」, and a git failure ends it with git's error.
4. Audit in two aspects, and every aspect **MUST run as a sub agent**; the two may run in parallel:
| Aspect | Scope |
| --- | --- |
| 1 Status codes | For every action, every HTTP status code it can actually return — success, validation failure, authorization failure, not found, server error — has a declared response type and body schema (`ProducesResponseType`, `@ApiResponse`, FastAPI `responses=`, drf-spectacular `@extend_schema`). A status code the code can produce but the document never declares is a finding; so is a declared status code the code can never produce |
| 2 Parameter description and example | Every input parameter and every output field has a description and an example. Examples come from real project data first — query the project's database, seed data, or fixtures. Only when real data is unreachable, derive an example by logic and mark it as a derived value in the document itself |
| 3 Recursive data model | When a parameter or return value is a data model, aspect 2 applies to every one of its properties. A model containing another model recurses to the innermost layer. Walk in from the controller and follow the input and output types down |
| 2 Description and example, down every data model | Two checklists over one pass of the controller's input and output types. Checklist A — description and example: every input parameter and every output field has a description, whatever its type, and the example requirement follows the type. A scalar member — string, number, boolean, date, enum — has an example as well. A data-model member has no example of its own; it keeps the description and hands the example duty down to its properties, where checklist B collects it. A collection is judged by its element type: scalar elements make the collection scalar, so it carries one example listing a couple of elements; model elements make the collection a data model, so it carries a description only and the walk enters the element type. A dictionary is judged by its value type on the same rule. Examples come from real project data first (the project's database, seed data, or fixtures), and only when real data is unreachable is an example derived by logic and marked as a derived value in the document itself. Checklist B — recursive walk: when a parameter or return value is a data model, or a collection of one, checklist A applies to every one of its properties, and a model containing another model recurses to the innermost layer. Every scalar property on every layer carries a description and an example; every intermediate model property carries a description only. Walk in from the controller, follow the input and output types down, and end each branch on the layer whose properties are all scalar. Run checklist A on each member as the walk in checklist B reaches it, so each data model file is opened once and both checklists are answered for it |
Instructions for each sub agent: read only, change nothing; report each finding as `file:line`, the aspect, severity, one sentence of evidence, and the concrete fix (which attribute to add, on which member). Findings are reported in Traditional Chinese.
Checklists A and B ran as two sub agents before, and they opened the same data model files twice. Aspect 2 is one sub agent now. The checklists stay listed apart so no check goes vague once they share a pass: a missing description or example is reported under A, a member the recursive walk never reached is reported under B, and field 4 of every row says which one.
Completion condition: all three aspects have returned — an aspect with nothing to report still returns 「無發現」 for itself.
5. Merge the three aspects' findings: deduplicate by location, then sort by severity. Start this step only once all three have returned. Completion condition: every finding appears exactly once, ordered 高 → 中 → 低.
6. Report the finding list. **This skill never modifies code**; the caller decides what to fix. Completion condition: the report is handed to the caller and the fix decision is left to them.
An example exists so the caller reads the literal payload straight off the document, and every literal value has exactly one owner. That is why a data-model member carries none: its payload is defined by its properties' own examples, and a second copy on the parent goes stale the day a property changes. A collection follows the same test on its element type — a list of scalars fits in one literal, so the example is complete where it stands, while a list of models only repeats what the element model already owns. Judge the element type, never the collection wrapper, so `List<string>` and `string` come out the same and `List<Address>` and `Address` come out the same.
Instructions for each sub agent: read only, change nothing. Return 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 scale below |
| 4 group | the aspect: `1` status codes, `2A` description and example, `2B` recursive walk |
| 5 item | the finding name, taken from the Severity table below |
| 6 detail | one sentence of evidence plus the concrete fix — which attribute to add, on which member |
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. An aspect with nothing to report returns the single line 「無發現」 instead of rows.
Completion condition: both aspects have returned, each with TSV rows or with 「無發現」.
5. Merge with `tools/merge-findings.sh`. Concatenate the two aspects' TSV rows in aspect order and pipe them into the script on stdin. Start this step only once both have returned. Read its exit code: `0` take its stdout as the merged list — deduplicated by location and sorted by severity, 高 first and 低 last; `1` no aspect 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` an aspect returned a malformed row, and the stderr names which row, so ask that one aspect'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.
6. 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.
Then release the review lock: run `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 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.
## Severity
@@ -46,13 +64,16 @@ The 高、中、低 scale is the one in `references/smells.md`. Map this skill's
| --- | --- |
| A returnable status code has no declared response type | 高 |
| A declared response type does not match the body the code returns | 高 |
| An input or output has no description | 中 |
| An input or output has no description, whatever its type | 中 |
| A scalar input, output, or property has no example | 中 |
| An example sits on a data-model member while its own properties carry none | 中 |
| A data model property is missing from the recursive walk entirely | 中 |
| A placeholder example while real data was reachable | 低 |
| A derived example that is not marked as derived | 低 |
## Notes
- An example on a data-model member is a finding only when that model's properties carry none of their own. That is the example hung on the wrong layer: the caller holds one blob no property definition backs, and the layers below look answered while every one of them is empty. A model whose properties are all covered keeps any extra whole-object example it already has; this skill asks for no such example and reports none.
- Examples taken from a database must be de-identified. Never put personal data into a Swagger document.
- 「推導值」 is the literal marker for a derived example; keep it in the document text so the next reader knows the value was never observed.
- When there are no findings, report the literal 「無發現」 explicitly; never leave the report empty.
+25 -7
View File
@@ -20,26 +20,44 @@ Review changed code against `references/smells.md` (from the book *Refactoring*)
## Steps
1. Get the review scope: `git diff` (uncommitted changes) or `git diff {base}...HEAD` (against the base branch when an implementation is complete); list the changed files. An empty diff → report the literal 「無發現」 and stop here; spawn no sub agent. Completion condition: the changed-file list is non-empty and reported, or the run already ended with 「無發現」.
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, including 2.5 comment scope leaks — comments carrying issue ids, wiki page ids, work package ids, commit hashes, @ mentions, external document links, review rounds, finding ids, reviewer aliases, or review status labels; the full banned and allowed lists live in `references/comment-scope.md` |
| 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; check every changed line and its enclosing function or class against the group's definitions and detection signals in smells.md; report each finding as `file:line`, smell name, severity (高、中、低 per the smells.md scale), one sentence of evidence, and the suggested refactoring. Findings are reported in Traditional Chinese.
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.
Completion condition: all six groups have returned — a group with nothing to report still returns 「無發現」 for its group.
3. Merge the six groups' findings: deduplicate (when one location hits several groups, merge and list every smell), then sort by severity. Start this step only once all six groups have returned; a group still running means the merge waits. Completion condition: every finding appears exactly once in the merged list, ordered 高 → 中 → 低.
4. Report the finding list. **This skill never modifies code**; the caller decides what to fix (inside the implementation flow, 高 and 中 are normally mandatory, 低 is judgment). Completion condition: the report is handed to the caller and the fix decision is left to them.
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.
## Notes
- `jsc-hooks`' `comment-scope.sh` already matches the pattern-detectable items after every file write. Group 2 here 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.
- `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.
+6 -6
View File
@@ -19,15 +19,15 @@ Use `references/comment-scope.md` for the banned list, allowed list, and rewrite
## Division of Labor
- `jsc-hooks/comment-scope.sh` blocks pattern-detectable violations after writes.
- `jsc-hooks/hooks/comment-scope.sh` blocks pattern-detectable violations after writes.
- `jsc-review:code-review` group 2 reports judgment-based cases that patterns cannot decide.
- This skill cleans the changed comments. Do not report the same finding again when a hook or code review already reported it; either clean it or explain why it is outside this skill's scope.
## Steps
1. Define the scope: inspect the current diff and list only added or modified comment lines. Include untouched legacy comments only when the user explicitly requested that. Completion condition: the cleanup scope is listed by file, or the run ends with the literal 「無發現」.
2. Compare each scoped comment with `references/comment-scope.md`. Remove process-only details and keep the real reason for the code. If a whole comment is only process detail and no real reason remains, delete the whole comment. Completion condition: every scoped comment is either unchanged with a reason, rewritten, or deleted.
3. Re-read the changed area after every rewrite. Confirm the sentence is complete, the logic still reads naturally, and no dangling fragment remains after deletion. Completion condition: every touched comment reads as a complete explanation or is gone.
4. Change comments and documentation strings only. Do not change executable behavior, identifiers, control flow, data shape, or tests except when a test fixture literally asserts the old comment text. Completion condition: `git diff` shows comment-only or documentation-string-only edits.
5. Run the smallest relevant build or test command for the changed project. If no project command is available, run syntax checks for touched scripts and report the gap. Completion condition: verification passed, or the exact missing command is reported.
1. Define the scope with `tools/changed-comments.sh`, run from the repository being cleaned: no argument for uncommitted work, `tools/changed-comments.sh {base}` when an implementation is complete and the base branch is the comparison point. It prints one row per changed comment line — file, line number, content — so this step never judges by eye which lines are comments; that judgment lives in the script, next to the one `jsc-hooks/hooks/comment-scope.sh` already uses. Read its exit code: `0` its stdout is the cleanup scope, and step 2 hands each file its own rows; `1` this change added or modified no comment line, so report the literal 「無發現」 and stop, spawn no sub agent; `2` a parameter or environment error, so report the script's stderr verbatim and stop, spawn no sub agent, and never substitute a hand-read diff — a scope that could not be computed is not an empty scope. Widen to untouched legacy comments only when the user explicitly asked for that, and say so in the report. Completion condition: the cleanup scope is listed by file, or the run ended with 「無發現」 or with the script's error.
2. Rewrite one file per sub agent, and every file **MUST run as a sub agent**. One file's comments never depend on another file's, so the sub agents run in parallel. Each sub agent is handed one file path and that file's scoped comment lines from step 1, and it compares each of them with `references/comment-scope.md`, removes process-only details, and keeps the real reason for the code. A comment that is only process detail with no real reason left is deleted whole. Each sub agent returns the file path, the categories it removed, and the line numbers it touched. Completion condition: every file's sub agent has returned, and every scoped comment is either unchanged with a stated reason, rewritten, or deleted.
3. Each sub agent re-reads its own changed area before it returns. It confirms every sentence is complete, the logic still reads naturally, and no dangling fragment remains after a deletion; a fragment it cannot resolve is reported instead of left behind. Completion condition: every sub agent has confirmed its file, and no returned report names an unresolved fragment.
4. Change comments and documentation strings only. Do not change executable behavior, identifiers, control flow, data shape, or tests except when a test fixture literally asserts the old comment text. `jsc-hooks/hooks/write-guard.sh` in `review` mode is the code-level backstop, but it covers this skill only as far as jsc-hooks can decide a comment line precisely: where that decision is not precise, the guard is limited to `jsc-review:code-review` and `jsc-review:api-doc`, which write nothing at all, and this skill runs unguarded. Only claude has a `PreToolUse` hook in the first place, so on codex, copilot, antigravity, and kiro this step's prose is the only constraint. Completion condition: `git diff` shows comment-only or documentation-string-only edits.
5. Run the smallest relevant build or test command once for the whole changed project, after every sub agent in step 2 has returned. Read the exit code. `0` — verification passed. Non-zero — the command ran and failed, so name the command, its exit code, and the failing output, then decide whether this run caused it: a failure that names a file this run touched is treated as caused here, so restore that file's comment syntax and re-run the command once; if it fails the same way again, revert this run's edits in that file and report the revert. A failure that names no file this run touched is reported as pre-existing, and the cleanup edits stay. If no project command is available, run syntax checks for the touched scripts and report the gap. Completion condition: the command exited `0`, or the report states the command, its exit code and whether the failure belongs to this run, or the exact missing command is reported.
6. Report the cleanup by category, not by full diff. Completion condition: the report names which categories were removed, which files were touched, and whether verification passed.
+107
View File
@@ -0,0 +1,107 @@
#!/usr/bin/env sh
# changed-comments.sh — 列出本次變更裡新增或修改的註解行,供 comment-cleanup 決定清理範圍。
#
# 用法:
# changed-comments.sh 比對工作區與 HEAD(git diff HEAD),即尚未提交的變更
# changed-comments.sh <base> 比對基準版本(git diff {base}...HEAD),實作完成時用
# 一律在要檢查的存取庫目錄下執行;本腳本不吃路徑參數,也不切換目錄。
#
# 輸出(TSV,一列一行註解,三欄,欄位之間一個 tab,只走 stdout):
# 1 檔案 路徑,相對存取庫根目錄
# 2 行號 該註解行在新版檔案裡的行號,正整數
# 3 內容 該行原文;行內的 tab 換成一個空白,欄位才不會錯位
#
# 結束碼:
# 0 有結果,至少印出一列
# 1 沒有結果,本次變更沒有新增或修改的註解行(呼叫端回報「無發現」)
# 2 參數或環境錯誤:參數超過一個、目前目錄不在 git 工作區、基準版本無效、缺 git 或 awk
#
# 判準的來源:註解行樣式與「不受本規則限制的非程式碼副檔名」兩份判準,都沿用 jsc-hooks 的
# hooks/comment-scope.sh(scan_file 裡的那兩份),一字不差地照抄。那支 hook 是規則實作的
# 正本,這裡只是把同一份判準搬到 git diff 上;哪天正本改了樣式,這裡要跟著改,不得各自演化。
# 跳過非程式碼檔不是可有可無的:markdown 的標題行開頭就是 #,不跳過就會把整份文件當成註解。
set -u
usage() {
echo '用法:changed-comments.sh [base](不給基準時比對工作區與 HEAD)' >&2
exit 2
}
[ "$#" -le 1 ] || usage
for cmd in git awk; do
command -v "$cmd" >/dev/null 2>&1 || {
echo "錯誤:環境缺 $cmd,無法列出變更的註解行。" >&2
exit 2
}
done
git rev-parse --is-inside-work-tree >/dev/null 2>&1 || {
echo '錯誤:目前目錄不在 git 工作區內,取不到變更範圍。' >&2
exit 2
}
# git diff 的失敗一律原樣往上帶:判不出範圍卻回「無發現」,等於把環境問題講成沒有東西要清。
if [ "$#" -eq 1 ]; then
base=$1
case "$base" in
-*) usage ;;
esac
git rev-parse --verify --quiet "$base^{commit}" >/dev/null 2>&1 || {
echo "錯誤:基準版本「$base」不是有效的 commit。" >&2
exit 2
}
DIFF=$(git diff --no-color -U0 "$base...HEAD" 2>&1) || {
printf '%s\n' "$DIFF" >&2
exit 2
}
else
DIFF=$(git diff --no-color -U0 HEAD 2>&1) || {
printf '%s\n' "$DIFF" >&2
exit 2
}
fi
# 樣式走環境變數交給 awk:走 -v 的話 awk 會先解釋字串裡的跳脫序列,樣式中的 \* 會被吃成
# 量詞 *,整條 ERE 的意思就變了。ENVIRON 不做這層處理,樣式進到 awk 時與正本一字不差。
COMMENT_RE='^[[:space:]]*(//|#|--|\*|/\*|<!--|;|%)|[[:space:]](//|#)[[:space:]]'
OUT=$(printf '%s\n' "$DIFF" | COMMENT_RE="$COMMENT_RE" awk '
BEGIN { OFS = "\t"; re = ENVIRON["COMMENT_RE"]; skip = 1 }
# 非程式碼檔沒有「程式碼註解」,整檔跳過(副檔名清單同 comment-scope.sh)。
function skipped(p) {
return (p ~ /\.(md|markdown|txt|rst|json|csv|tsv|svg|lock|log)$/ || p ~ /COMMIT_EDITMSG$/)
}
/^\+\+\+ / {
f = substr($0, 5) # 去掉開頭的「+++ 」
sub(/^b\//, "", f)
skip = (f == "" || f == "/dev/null" || skipped(f))
next
}
# 區塊標頭 @@ -a,b +c,d @@:第三欄的 +c 就是這一段在新版檔案裡的起始行號。
/^@@ / {
t = $3
sub(/^\+/, "", t)
sub(/,.*$/, "", t)
ln = t + 0
next
}
/^\+/ {
if (skip || ln < 1) next
line = substr($0, 2)
if (line ~ re) {
out = line
gsub(/\t/, " ", out)
print f, ln, out
}
ln++ # 只有新增行會佔掉新版檔案的行號,刪除行不會
}
')
[ -n "$OUT" ] || exit 1
printf '%s\n' "$OUT"
exit 0
+110
View File
@@ -0,0 +1,110 @@
#!/usr/bin/env sh
# merge-findings.sh — 合併多組審查發現,依位置去重,再依嚴重度排序。
# 用法:merge-findings.sh [發現檔…]
# 給檔名就讀那幾個檔,不給檔名就讀 stdin。code-review 六組與 api-doc 兩個面向
# 共用這一支,兩支技能都不再自己描述合併與排序規則。
#
# 輸入格式(TSV,一列一筆發現,六欄,欄位之間一個 tab):
# 1 file 檔案路徑,相對專案根目錄,不得留空
# 2 line 行號,正整數
# 3 severity 嚴重度,只收 高、中、低
# 4 group 組別或面向代號,例如 2 或 1 狀態碼
# 5 item 發現名稱,壞味道名稱或稽核項目名稱
# 6 detail 一句證據加上建議修法
# 空白列、開頭是 # 的列、第一欄為「無發現」的列一律略過。
#
# 輸出(TSV,同樣六欄,一個位置一列,印到 stdout):
# file 與 line 相同的發現合併成一列。
# severity 取該位置最高的一級。group、item、detail 依輸入順序以「|」相連。
# 排序:嚴重度 高 → 中 → 低;同一級再依 file 字典序、line 數值遞增。
#
# 結束碼:
# 0 合併成功,至少印出一列
# 1 沒有任何發現(輸入沒有可用的資料列),呼叫端回報「無發現」
# 2 參數或環境錯誤(找不到輸入檔,或缺 awk、sort、cut)
# 3 輸入格式錯誤(欄數不是 6、行號不是正整數,或嚴重度不在 高、中、低)
#
# 護欄:
# 格式錯誤一律 exit 3 並在 stderr 指出是第幾個檔的第幾列,不猜、不放行、不自行補欄。
# 錯誤訊息一律印繁中到 stderr,正常輸出只走 stdout。
set -u
for cmd in awk sort cut; do
command -v "$cmd" >/dev/null 2>&1 || {
echo "錯誤:環境缺 $cmd,無法合併發現。" >&2
exit 2
}
done
for f in "$@"; do
case "$f" in
-*) echo "用法:merge-findings.sh [發現檔…](不給檔名時讀 stdin)" >&2; exit 2 ;;
esac
[ -f "$f" ] || { echo "錯誤:找不到輸入檔 $f。請確認路徑後重試。" >&2; exit 2; }
done
TAB=$(printf '\t')
TMP="${TMPDIR:-/tmp}/jsc-merge-findings.$$"
trap 'rm -f "$TMP" "$TMP.sorted"' EXIT INT TERM
# 第一欄先放排序用的嚴重度序位,排完再切掉。
awk -F"$TAB" '
BEGIN { OFS = "\t"; n = 0; bad = 0 }
{
sub(/\r$/, "", $0)
if ($0 ~ /^[ \t]*$/) next
if ($0 ~ /^#/) next
if ($1 == "無發現") next
where = (FILENAME == "" || FILENAME == "-") ? "stdin" : FILENAME
if (NF != 6) {
printf("錯誤:%s 第 %d 列有 %d 欄,應為 6 欄。\n", where, FNR, NF) > "/dev/stderr"
bad = 1; next
}
if ($1 == "") {
printf("錯誤:%s 第 %d 列的檔案路徑是空的。\n", where, FNR) > "/dev/stderr"
bad = 1; next
}
if ($2 !~ /^[0-9]+$/ || $2 + 0 < 1) {
printf("錯誤:%s 第 %d 列的行號「%s」不是正整數。\n", where, FNR, $2) > "/dev/stderr"
bad = 1; next
}
if ($3 != "高" && $3 != "中" && $3 != "低") {
printf("錯誤:%s 第 %d 列的嚴重度「%s」不在 高、中、低。\n", where, FNR, $3) > "/dev/stderr"
bad = 1; next
}
rank = ($3 == "高") ? 1 : (($3 == "中") ? 2 : 3)
key = $1 SUBSEP $2
if (!(key in seen)) {
seen[key] = 1
order[++n] = key
kfile[key] = $1; kline[key] = $2; krank[key] = rank
kgroup[key] = $4; kitem[key] = $5; kdetail[key] = $6
} else {
if (rank < krank[key]) krank[key] = rank
kgroup[key] = kgroup[key] "|" $4
kitem[key] = kitem[key] "|" $5
kdetail[key] = kdetail[key] "|" $6
}
}
END {
if (bad) exit 3
if (n == 0) exit 1
for (i = 1; i <= n; i++) {
k = order[i]
sev = (krank[k] == 1) ? "高" : ((krank[k] == 2) ? "中" : "低")
print krank[k], kfile[k], kline[k], sev, kgroup[k], kitem[k], kdetail[k]
}
}
' "$@" > "$TMP"
rc=$?
[ "$rc" -eq 0 ] || exit "$rc"
LC_ALL=C sort -t"$TAB" -k1,1n -k2,2 -k3,3n "$TMP" > "$TMP.sorted" || {
echo "錯誤:排序失敗。" >&2
exit 2
}
cut -f2- "$TMP.sorted" || {
echo "錯誤:輸出失敗。" >&2
exit 2
}
exit 0