Skip to content

Commit 2ff42ff

Browse files
ddddddddwpbenym
ddddddddwp
and
benym
authored
feat: add on-demand comet review skill (#307)
* feat: add on-demand change review skill * docs(review): clarify manual review policy * fix(review): cover all untracked change files * fix(review): align untracked file scope * chore: trigger CI rerun * fix(review): align diff scope resolution --------- Co-authored-by: benym <benyuanming@gmail.com>
1 parent 0d35df5 commit 2ff42ff

12 files changed

Lines changed: 429 additions & 0 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ All notable changes to @rpamis/comet will be documented in this file.
88

99
- **Dashboard artifact previews**: Fullscreen previews now close with Escape, keep long tables horizontally scrollable, preserve readable table headers, and use a larger directory navigation scale.
1010
- **Hook allow-path documentation**: The website now explains how to configure project-relative `hook.allow_paths` directories for guarded workflow phases.
11+
- **On-demand change review**: The new `/comet-review` Skill reviews the current Native or Classic change against its implementation diff and existing evidence, reports prioritized correctness, security, edge-case, and coverage findings, and remains read-only without advancing or replacing Verify.
1112

1213
### Fixed
1314

‎README-zh.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,7 @@ comet eval ./my-skill --suite langsmith --html
480480
| `/comet` | 共享入口 — 根据 `.comet/config.yaml` 转发到项目配置的 Native 或 Classic |
481481
| `/comet-native` | Native 永久入口 — Shape、Build、Verify、Archive，自包含且可恢复 |
482482
| `/comet-classic` | Classic 永久入口 — OpenSpec + Superpowers 五阶段工作流 |
483+
| `/comet-review` | 手动只读审查当前 Native 或 Classic change，不推进阶段也不替代 Verify |
483484
| `/comet-open` | Classic 阶段 1：打开变更（提案、设计、任务分解） |
484485
| `/comet-design` | Classic 阶段 2：深度设计（头脑风暴、设计文档） |
485486
| `/comet-build` | Classic 阶段 3：规划与构建（实现计划、代码提交） |

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,7 @@ does not expand the backend command list; see the [Skill creation guide](docs/op
509509
| `/comet` | Shared entry — routes to the configured Native or Classic workflow from `.comet/config.yaml` |
510510
| `/comet-native` | Permanent Native entry — self-contained, recoverable Shape, Build, Verify, and Archive |
511511
| `/comet-classic` | Permanent Classic entry — the five-phase OpenSpec + Superpowers workflow |
512+
| `/comet-review` | Read-only manual review of the current Native or Classic change without advancing its phase |
512513
| `/comet-open` | Classic phase 1: Open a change (proposal, design, task breakdown) |
513514
| `/comet-design` | Classic phase 2: Deep design (brainstorming, Design Doc) |
514515
| `/comet-build` | Classic phase 3: Plan and build (implementation plan, code commits) |

‎assets/manifest.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
"comet-classic/reference/scripts.md",
1818
"comet-classic/reference/subagent-dispatch.md",
1919
"comet-classic/reference/workspace.md",
20+
"comet-review/SKILL.md",
21+
"comet-review/agents/openai.yaml",
2022
"comet-any/SKILL.md",
2123
"comet-any/agents/openai.yaml",
2224
"comet-any/reference/authoring-subagents.md",

‎assets/skills-zh/comet-review/SKILL.md‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
---
2+
name: comet-review
3+
description: "手动审查当前 Comet change 的实现差异，只报告正确性、安全和边界问题，不推进工作流。"
4+
disable-model-invocation: true
5+
---
6+
7+
# Comet 手动代码审查
8+
9+
对当前选中的 Comet change 执行一次按需、只读的代码审查。这个入口不属于任何阶段，也不替代 Build 或 Verify 的验证和审查。
10+
11+
本入口独立于 `review_mode`：`review_mode` 控制流程内的自动 review 策略，而 `/comet-review` 只代表用户手动触发的单次审查；调用本入口不得读取、修改或覆盖当前 change 的 `review_mode`。
12+
13+
## 不可变约束
14+
15+
本 Skill 的整个调用必须保持只读：
16+
17+
- 不修改、创建或删除文件；
18+
- 不暂存、提交、切换分支、创建分支或创建 worktree；
19+
- 不运行 `comet state select`、`comet native select`、`comet state set`、`comet state transition`、阶段守卫、`comet native next` 或归档命令；
20+
- 不修复发现的问题，不推进 phase，不更新 tasks、状态、验证报告或审查记录；
21+
- 不把本次结果称为 Verify 通过，也不把“没有发现”视为测试已经通过。
22+
23+
只允许执行读取文件、查询状态和查看 Git 差异所需的命令。任何可能运行项目代码、安装依赖或产生文件的检查都不属于本入口。
24+
25+
## 1. 定位项目与当前 change
26+
27+
1. 使用只读 Git 查询确定项目根目录；如果不是 Git 仓库，则使用当前 Comet 项目根目录。
28+
2. 在项目根目录运行：
29+
30+
```bash
31+
comet status . --json
32+
```
33+
34+
3. 读取 `.comet/current-change.json`，并按以下顺序确定审查对象：
35+
- 文件包含有效的 `comet.selection.v2` 时，使用其中的 `workflow` 和 `change`；
36+
- selection 缺失，且状态输出中只有一个未归档的 Comet change 时，只在本次审查内采用该 change，不写入 selection；
37+
- selection 缺失且存在多个 change 时，列出名称、workflow 和 phase，请用户指定一个后停止；
38+
- selection 指向缺失、已归档或状态无效的 change 时，报告 stale/invalid selection 后停止，不自行修复。
39+
40+
忽略不受 Comet 管理的普通 OpenSpec change。不得因为默认 workflow 与 selection 不同而改用默认 workflow。
41+
42+
## 2. 收集审查上下文
43+
44+
只读取当前 change 的必要上下文，并为每个事实保留来源路径或命令。
45+
46+
### Classic
47+
48+
1. 先读取并遵守 `comet-classic/reference/classic-layout.md`，解析当前项目的 Classic 逻辑根。
49+
2. 读取当前 change 的 `proposal.md`、`design.md`、`tasks.md` 和 `specs/*/spec.md`；存在关联 Design Doc 时一并读取。
50+
3. 使用以下只读状态查询获得 phase、基线和已有证据引用：
51+
52+
```bash
53+
comet state get <change-name> phase
54+
comet state get <change-name> base_ref
55+
comet state get <change-name> plan
56+
comet state get <change-name> verification_report
57+
```
58+
59+
4. 读取存在的 plan、验证报告，以及 `comet status . --json` 返回的 build/verify command checks。缺失证据应标为“未提供”，不能推断为失败或通过。
60+
61+
### Native
62+
63+
运行以下只读命令：
64+
65+
```bash
66+
comet native show <change-name> --json
67+
comet native status <change-name> --details --json
68+
```
69+
70+
读取返回的 brief、完整 proposed Specs、acceptance、Builder handoff、checks、verification、risks、blockers 和 verification report 引用。只使用当前 candidate/iteration 的证据；历史轮次仅用于解释残留风险，不得覆盖当前状态。
71+
72+
## 3. 确定实现差异
73+
74+
1. 先运行 `git status --short --untracked-files=all`，完整枚举已暂存、未暂存和未跟踪的工作树状态。
75+
2. 结合当前 change 的需求、工作区绑定、Git 历史和工作树状态，确定最可信且与当前 change 相关的审查范围。对于 Classic，优先使用有效的 plan `base-ref`；不存在或无效时回退到状态中的 `base_ref`，不要求两者一致。只有两者均无效时，才将 Classic 基线视为缺失。对于 Native，将状态中的工作区关系和当前 candidate 的实现范围证据作为判断依据。
76+
3. 查看从可信基线到当前工作树的完整差异，包括已提交、已暂存和未暂存修改。对属于当前 change 的所有未跟踪文件，包括源码、测试、文档、配置和元数据（例如 `SKILL.md` 与 `agents/openai.yaml`），直接读取内容并明确标注其未跟踪状态。
77+
4. 排除明确归属于其他 change 或用户无关工作的差异。只有歧义会实质影响审查结论时才询问用户；否则基于现有证据继续审查，并在结果中说明范围判断和假设。
78+
79+
如果结合上述证据仍无法确定可信且可验证的基线，继续审查当前可见的工作树差异，并在结果中显著标注“审查范围不完整”。
80+
81+
## 4. 执行审查
82+
83+
根据需求、任务和当前差异进行一次聚焦审查，只检查：
84+
85+
- 实现正确性和明显逻辑错误；
86+
- 安全风险、权限或路径边界问题；
87+
- 错误处理、兼容性和重要边界条件；
88+
- 任务遗漏、实现与当前 change 明确要求不一致；
89+
- 测试是否覆盖本次行为变化，以及已有测试证据能否支撑相应结论。
90+
91+
不要把风格偏好、无关重构或没有具体影响的猜测列为 finding。每条 finding 必须能指向具体文件和行号，并说明可触发的行为或风险；证据不足时降低严重度或放入“开放问题”。
92+
93+
严重度仅使用：
94+
95+
- `CRITICAL`：安全破坏、数据丢失或核心流程不可用；
96+
- `IMPORTANT`：明确的正确性错误、核心验收遗漏或高概率回归；
97+
- `WARNING`：真实但非阻塞的边界风险或测试缺口；
98+
- `SUGGESTION`：有明确收益但不影响当前正确性的改进。
99+
100+
## 5. 输出
101+
102+
先输出 findings，按严重度排序。每条使用以下格式：
103+
104+
```text
105+
[IMPORTANT] 简短标题 — path/to/file.ts:123
106+
影响：什么输入或场景会出现什么错误。
107+
依据：与 diff、任务、规格或证据的具体对应关系。
108+
```
109+
110+
随后输出：
111+
112+
- `审查范围`：workflow、change、phase、基线、纳入的差异和任何范围限制；
113+
- `证据状态`：已读取的测试/构建/验证证据及其新鲜度，不重新执行测试；
114+
- `开放问题`：只有确实阻碍判断的问题；
115+
- `结论`：finding 数量汇总，或明确写“未发现具体问题”。
116+
117+
即使没有 finding，也必须说明残余风险和未执行的检查。结尾固定提醒：
118+
119+
> 这是只读的手动审查，不会推进 Comet phase，也不能替代 `/comet-verify` 或 Native Verify。
120+
121+
如果用户随后要求修复 finding，把修复视为新的写入任务，退出本 Skill，并按仓库当前工作流规则重新进入开发流程。

‎assets/skills-zh/comet-review/agents/openai.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
interface:
2+
display_name: "Comet 手动代码审查"
3+
short_description: "只读审查当前 change，不推进工作流"
4+
policy:
5+
allow_implicit_invocation: false

‎assets/skills/comet-review/SKILL.md‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
---
2+
name: comet-review
3+
description: "Manually review the implementation diff for the current Comet change without advancing the workflow."
4+
disable-model-invocation: true
5+
---
6+
7+
# Comet Manual Code Review
8+
9+
Run an on-demand, read-only code review for the currently selected Comet change. This entry is phase-neutral and does not replace Build or Verify validation and review.
10+
11+
This entry is independent of `review_mode`: `review_mode` controls the in-workflow automatic review policy, while `/comet-review` represents a single review manually triggered by the user. Invoking this entry must not read, modify, or override the current change's `review_mode`.
12+
13+
## Invariants
14+
15+
The entire Skill invocation must remain read-only:
16+
17+
- Do not modify, create, or delete files.
18+
- Do not stage, commit, switch branches, create branches, or create worktrees.
19+
- Do not run `comet state select`, `comet native select`, `comet state set`, `comet state transition`, phase guards, `comet native next`, or archive commands.
20+
- Do not fix findings, advance the phase, or update tasks, state, verification reports, or review records.
21+
- Do not describe this review as a passing Verify result or treat “no findings” as proof that tests pass.
22+
23+
Only commands needed to read files, query status, and inspect Git diffs are allowed. Checks that may execute project code, install dependencies, or generate files are outside this entry's scope.
24+
25+
## 1. Resolve the project and current change
26+
27+
1. Use a read-only Git query to locate the project root. If the project is not a Git repository, use the current Comet project root.
28+
2. From the project root, run:
29+
30+
```bash
31+
comet status . --json
32+
```
33+
34+
3. Read `.comet/current-change.json` and resolve the review target in this order:
35+
- When it contains a valid `comet.selection.v2`, use its `workflow` and `change`.
36+
- When selection is missing and status reports exactly one unarchived Comet change, use that change for this review only without writing a selection.
37+
- When selection is missing and multiple changes exist, list their names, workflows, and phases, ask the user to choose one, and stop.
38+
- When selection points to a missing, archived, or invalid change, report the stale or invalid selection and stop without repairing it.
39+
40+
Ignore unmanaged plain OpenSpec changes. Do not replace the selection with the default workflow when they differ.
41+
42+
## 2. Collect review context
43+
44+
Read only the context needed for the current change, retaining a source path or command for every fact.
45+
46+
### Classic
47+
48+
1. Read and follow `comet-classic/reference/classic-layout.md` first to resolve the project's logical Classic roots.
49+
2. Read the current change's `proposal.md`, `design.md`, `tasks.md`, and `specs/*/spec.md`. Read the associated Design Doc when one exists.
50+
3. Use these read-only state queries to obtain the phase, baseline, and existing evidence references:
51+
52+
```bash
53+
comet state get <change-name> phase
54+
comet state get <change-name> base_ref
55+
comet state get <change-name> plan
56+
comet state get <change-name> verification_report
57+
```
58+
59+
4. Read the plan and verification report when present, plus build and verify command checks returned by `comet status . --json`. Label missing evidence as “not provided”; do not infer failure or success.
60+
61+
### Native
62+
63+
Run these read-only commands:
64+
65+
```bash
66+
comet native show <change-name> --json
67+
comet native status <change-name> --details --json
68+
```
69+
70+
Read the returned brief, complete proposed Specs, acceptance items, Builder handoff, checks, verification, risks, blockers, and verification report reference. Use evidence from the current candidate and iteration only. Historical iterations may explain residual risk but must not override current state.
71+
72+
## 3. Establish the implementation diff
73+
74+
1. Run `git status --short --untracked-files=all` first to fully enumerate staged, unstaged, and untracked worktree state.
75+
2. Use the current change's requirements, workspace binding, Git history, and worktree state to determine the most credible review scope related to the current change. For Classic, prefer a valid plan `base-ref`; when it is missing or invalid, fall back to the state `base_ref`. The two values do not need to match. Only when both values are invalid is the Classic baseline missing. For Native, treat the workspace relationship in state and the current candidate's implementation-scope evidence as inputs to this judgment.
76+
3. Inspect the complete diff from the trusted baseline to the current worktree, including committed, staged, and unstaged changes. Directly read all untracked files owned by the current change, including source, tests, documentation, configuration, and metadata (for example, `SKILL.md` and `agents/openai.yaml`), and label them as untracked.
77+
4. Exclude diffs clearly owned by another change or unrelated user work. Ask the user only when ambiguity would materially affect the review conclusions; otherwise continue from the available evidence and report the scope judgment and assumptions.
78+
79+
If the evidence above still does not yield a trusted, verifiable baseline, continue reviewing the visible worktree diff and prominently label the review scope as incomplete.
80+
81+
## 4. Perform the review
82+
83+
Review requirements, tasks, and the current diff, focusing only on:
84+
85+
- implementation correctness and concrete logic defects;
86+
- security, permission, and path-boundary risks;
87+
- error handling, compatibility, and important edge cases;
88+
- omitted tasks or implementation that contradicts explicit current-change requirements;
89+
- whether tests cover the behavior change and whether existing evidence supports the stated conclusion.
90+
91+
Do not report style preferences, unrelated refactors, or speculation without a concrete impact as findings. Every finding must identify a file and line and explain the triggering behavior or risk. When evidence is insufficient, lower the severity or place the item under open questions.
92+
93+
Use only these severities:
94+
95+
- `CRITICAL`: security compromise, data loss, or an unusable core workflow;
96+
- `IMPORTANT`: a concrete correctness defect, missing core acceptance behavior, or a high-probability regression;
97+
- `WARNING`: a real but non-blocking edge risk or test gap;
98+
- `SUGGESTION`: an improvement with a concrete benefit that does not affect current correctness.
99+
100+
## 5. Report
101+
102+
Lead with findings ordered by severity. Use this format for every finding:
103+
104+
```text
105+
[IMPORTANT] Short title — path/to/file.ts:123
106+
Impact: Which input or scenario produces which failure.
107+
Evidence: The concrete relationship to the diff, task, specification, or recorded evidence.
108+
```
109+
110+
Then report:
111+
112+
- `Review scope`: workflow, change, phase, baseline, included diffs, and any scope limitations;
113+
- `Evidence status`: test, build, and verification evidence read and its freshness, without rerunning tests;
114+
- `Open questions`: only questions that genuinely block a conclusion;
115+
- `Conclusion`: finding counts, or an explicit “No concrete findings.”
116+
117+
Even with no findings, state residual risks and checks that were not run. End with this fixed reminder:
118+
119+
> This is a read-only manual review. It does not advance the Comet phase and cannot replace `/comet-verify` or Native Verify.
120+
121+
If the user subsequently asks to fix a finding, treat that as a new write task, exit this Skill, and re-enter development through the repository's current workflow rules.

‎assets/skills/comet-review/agents/openai.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
interface:
2+
display_name: "Comet Manual Code Review"
3+
short_description: "Review the current change without advancing its workflow"
4+
policy:
5+
allow_implicit_invocation: false

‎domains/skill/platform-install.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ const LEGACY_HOOK_SCRIPTS = [
5656
const LEGACY_RULE_FILES = ['comet-phase-guard.md', 'comet-native-phase-guard.md'] as const;
5757
const NATIVE_SHARED_SKILL_PATHS = new Set([
5858
'comet/SKILL.md',
59+
'comet-review/SKILL.md',
60+
'comet-review/agents/openai.yaml',
5961
'comet/scripts/comet-entry-runtime.mjs',
6062
'comet/scripts/comet-hook-router.mjs',
6163
]);

‎test/app/init-e2e.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ async function readManifest() {
4949
function isNativeInstallSkillPath(skillPath: string): boolean {
5050
return (
5151
skillPath === 'comet/SKILL.md' ||
52+
skillPath.startsWith('comet-review/') ||
5253
skillPath === 'comet/scripts/comet-entry-runtime.mjs' ||
5354
skillPath === 'comet/scripts/comet-hook-router.mjs' ||
5455
skillPath.startsWith('comet-native/') ||

0 commit comments

Comments
 (0)