mirror of
https://github.com/jnMetaCode/superpowers-zh.git
synced 2026-09-02 22:54:06 +08:00
chore(skills): 同步上游 P0 漂移修复(hooks-cursor + brainstorm + code-reviewer 引用) (#30)
* chore(skills): 同步上游 P0 漂移修复(hooks-cursor + brainstorm + code-reviewer 引用)
来自质量审计的 3 个 P0 缺陷,全部是上游漂移类("主站有的我们没跟"),逐项 cherry-pick / 翻译同步:
1. hooks/hooks-cursor.json:command 从直接调 unix shell 脚本改回上游的
polyglot wrapper "./hooks/run-hook.cmd session-start"。修复 Windows
Cursor 用户 hook 完全不触发的回归。
2. skills/brainstorming/scripts/{server.cjs,start-server.sh,stop-server.sh}:
完整 cherry-pick 上游 v5.0.6(commit 9e3ed21)的 CONTENT_DIR/STATE_DIR
peer-dir 拆分。修复 visual brainstorming 用户因 SKILL.md 指向新路径但
server.cjs 还用旧 SCREEN_DIR 结构导致卡死。
3. skills/requesting-code-review/SKILL.md:
- 3 处 "superpowers:code-reviewer" 命名子代理引用 → 上游 v5.1.0 改用
"general-purpose" Task + 模板路径的形式
- 占位符从 5 个(含 WHAT_WAS_IMPLEMENTED + DESCRIPTION 冗余)精简到 4 个
(只保留 DESCRIPTION 作为 "Brief summary of what you built"),与上游对齐
- 示例段落里的占位符顺序和命名同步上游
- "执行计划" 集成段:从 "每批(3 个任务)后审查" 改为 "每个任务完成后或
在自然 checkpoint 审查",对齐上游 v5.1.0 的 subagent 节奏调整
验证:3 文件 diff vs upstream 全部 0 行漂移;残留
"superpowers:code-reviewer" 字符串 0 处。
不在本 PR 范围(issue #19 其他批次留独立 PR):
- using-git-worktrees + finishing-a-development-branch v5.1.0 安全修复 → PR #28
- brainstorming + writing-plans 的 review loop 简化 → 后续 PR
- subagent-driven-development 的节奏调整其他部分 → 后续 PR
Refs: 本次全量质量审计(102/106 测试 96.2% 通过率)
* chore(skills): 同步 code-reviewer.md 到上游 v5.1.0 self-contained 版
补漏:本次质量审计第二轮(支持文件结构对比)发现 code-reviewer.md
有真结构漂移——上游 6 个 header,我们 16 个,原因是上游 v5.1.0
(PR #1299)把 reviewer persona + checklist + dispatch 模板整合到
单一 code-reviewer.md 里,self-contained。我们仍是拆分式老版。
这是 issue #19 列出的 "v5.1.0 Code Review 整合" 待办项。同性质上游
同步(主站有的我们没跟),合并到本 PR 的 P0 漂移清单里。
改动:重写 skills/requesting-code-review/code-reviewer.md:
- 上游结构:H1 标题 + 散文 intro + 单一 Task tool 代码块(含完整
prompt 模板)+ Placeholders 说明 + Reviewer returns 说明 + Example
Output 段
- 翻译策略:散文逐句翻译;占位符 {DESCRIPTION}/{PLAN_OR_REQUIREMENTS}/
{BASE_SHA}/{HEAD_SHA} 保留英文;Critical/Important/Minor 等术语保留
- 验证:上游 6 header == 我方 6 header(结构 1:1 对齐)
不在范围:reviewer-prompt.md 其他文件(spec/plan-document-reviewer-prompt.md、
subagent-driven-development/*-prompt.md)的翻译漂移——已确认全是
translation drift(H 数对齐 1:1),无结构漂移,本 PR 不动。
This commit is contained in:
@@ -3,7 +3,7 @@
|
||||
"hooks": {
|
||||
"sessionStart": [
|
||||
{
|
||||
"command": "./hooks/session-start"
|
||||
"command": "./hooks/run-hook.cmd session-start"
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
@@ -76,8 +76,10 @@ function decodeFrame(buffer) {
|
||||
const PORT = process.env.BRAINSTORM_PORT || (49152 + Math.floor(Math.random() * 16383));
|
||||
const HOST = process.env.BRAINSTORM_HOST || '127.0.0.1';
|
||||
const URL_HOST = process.env.BRAINSTORM_URL_HOST || (HOST === '127.0.0.1' ? 'localhost' : HOST);
|
||||
const SCREEN_DIR = process.env.BRAINSTORM_DIR || '/tmp/brainstorm';
|
||||
const OWNER_PID = process.env.BRAINSTORM_OWNER_PID ? Number(process.env.BRAINSTORM_OWNER_PID) : null;
|
||||
const SESSION_DIR = process.env.BRAINSTORM_DIR || '/tmp/brainstorm';
|
||||
const CONTENT_DIR = path.join(SESSION_DIR, 'content');
|
||||
const STATE_DIR = path.join(SESSION_DIR, 'state');
|
||||
let ownerPid = process.env.BRAINSTORM_OWNER_PID ? Number(process.env.BRAINSTORM_OWNER_PID) : null;
|
||||
|
||||
const MIME_TYPES = {
|
||||
'.html': 'text/html', '.css': 'text/css', '.js': 'application/javascript',
|
||||
@@ -112,10 +114,10 @@ function wrapInFrame(content) {
|
||||
}
|
||||
|
||||
function getNewestScreen() {
|
||||
const files = fs.readdirSync(SCREEN_DIR)
|
||||
const files = fs.readdirSync(CONTENT_DIR)
|
||||
.filter(f => f.endsWith('.html'))
|
||||
.map(f => {
|
||||
const fp = path.join(SCREEN_DIR, f);
|
||||
const fp = path.join(CONTENT_DIR, f);
|
||||
return { path: fp, mtime: fs.statSync(fp).mtime.getTime() };
|
||||
})
|
||||
.sort((a, b) => b.mtime - a.mtime);
|
||||
@@ -142,7 +144,7 @@ function handleRequest(req, res) {
|
||||
res.end(html);
|
||||
} else if (req.method === 'GET' && req.url.startsWith('/files/')) {
|
||||
const fileName = req.url.slice(7);
|
||||
const filePath = path.join(SCREEN_DIR, path.basename(fileName));
|
||||
const filePath = path.join(CONTENT_DIR, path.basename(fileName));
|
||||
if (!fs.existsSync(filePath)) {
|
||||
res.writeHead(404);
|
||||
res.end('Not found');
|
||||
@@ -230,7 +232,7 @@ function handleMessage(text) {
|
||||
touchActivity();
|
||||
console.log(JSON.stringify({ source: 'user-event', ...event }));
|
||||
if (event.choice) {
|
||||
const eventsFile = path.join(SCREEN_DIR, '.events');
|
||||
const eventsFile = path.join(STATE_DIR, 'events');
|
||||
fs.appendFileSync(eventsFile, JSON.stringify(event) + '\n');
|
||||
}
|
||||
}
|
||||
@@ -258,32 +260,33 @@ const debounceTimers = new Map();
|
||||
// ========== Server Startup ==========
|
||||
|
||||
function startServer() {
|
||||
if (!fs.existsSync(SCREEN_DIR)) fs.mkdirSync(SCREEN_DIR, { recursive: true });
|
||||
if (!fs.existsSync(CONTENT_DIR)) fs.mkdirSync(CONTENT_DIR, { recursive: true });
|
||||
if (!fs.existsSync(STATE_DIR)) fs.mkdirSync(STATE_DIR, { recursive: true });
|
||||
|
||||
// Track known files to distinguish new screens from updates.
|
||||
// macOS fs.watch reports 'rename' for both new files and overwrites,
|
||||
// so we can't rely on eventType alone.
|
||||
const knownFiles = new Set(
|
||||
fs.readdirSync(SCREEN_DIR).filter(f => f.endsWith('.html'))
|
||||
fs.readdirSync(CONTENT_DIR).filter(f => f.endsWith('.html'))
|
||||
);
|
||||
|
||||
const server = http.createServer(handleRequest);
|
||||
server.on('upgrade', handleUpgrade);
|
||||
|
||||
const watcher = fs.watch(SCREEN_DIR, (eventType, filename) => {
|
||||
const watcher = fs.watch(CONTENT_DIR, (eventType, filename) => {
|
||||
if (!filename || !filename.endsWith('.html')) return;
|
||||
|
||||
if (debounceTimers.has(filename)) clearTimeout(debounceTimers.get(filename));
|
||||
debounceTimers.set(filename, setTimeout(() => {
|
||||
debounceTimers.delete(filename);
|
||||
const filePath = path.join(SCREEN_DIR, filename);
|
||||
const filePath = path.join(CONTENT_DIR, filename);
|
||||
|
||||
if (!fs.existsSync(filePath)) return; // file was deleted
|
||||
touchActivity();
|
||||
|
||||
if (!knownFiles.has(filename)) {
|
||||
knownFiles.add(filename);
|
||||
const eventsFile = path.join(SCREEN_DIR, '.events');
|
||||
const eventsFile = path.join(STATE_DIR, 'events');
|
||||
if (fs.existsSync(eventsFile)) fs.unlinkSync(eventsFile);
|
||||
console.log(JSON.stringify({ type: 'screen-added', file: filePath }));
|
||||
} else {
|
||||
@@ -297,10 +300,10 @@ function startServer() {
|
||||
|
||||
function shutdown(reason) {
|
||||
console.log(JSON.stringify({ type: 'server-stopped', reason }));
|
||||
const infoFile = path.join(SCREEN_DIR, '.server-info');
|
||||
const infoFile = path.join(STATE_DIR, 'server-info');
|
||||
if (fs.existsSync(infoFile)) fs.unlinkSync(infoFile);
|
||||
fs.writeFileSync(
|
||||
path.join(SCREEN_DIR, '.server-stopped'),
|
||||
path.join(STATE_DIR, 'server-stopped'),
|
||||
JSON.stringify({ reason, timestamp: Date.now() }) + '\n'
|
||||
);
|
||||
watcher.close();
|
||||
@@ -309,8 +312,8 @@ function startServer() {
|
||||
}
|
||||
|
||||
function ownerAlive() {
|
||||
if (!OWNER_PID) return true;
|
||||
try { process.kill(OWNER_PID, 0); return true; } catch (e) { return false; }
|
||||
if (!ownerPid) return true;
|
||||
try { process.kill(ownerPid, 0); return true; } catch (e) { return e.code === 'EPERM'; }
|
||||
}
|
||||
|
||||
// Check every 60s: exit if owner process died or idle for 30 minutes
|
||||
@@ -320,14 +323,27 @@ function startServer() {
|
||||
}, 60 * 1000);
|
||||
lifecycleCheck.unref();
|
||||
|
||||
// Validate owner PID at startup. If it's already dead, the PID resolution
|
||||
// was wrong (common on WSL, Tailscale SSH, and cross-user scenarios).
|
||||
// Disable monitoring and rely on the idle timeout instead.
|
||||
if (ownerPid) {
|
||||
try { process.kill(ownerPid, 0); }
|
||||
catch (e) {
|
||||
if (e.code !== 'EPERM') {
|
||||
console.log(JSON.stringify({ type: 'owner-pid-invalid', pid: ownerPid, reason: 'dead at startup' }));
|
||||
ownerPid = null;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
server.listen(PORT, HOST, () => {
|
||||
const info = JSON.stringify({
|
||||
type: 'server-started', port: Number(PORT), host: HOST,
|
||||
url_host: URL_HOST, url: 'http://' + URL_HOST + ':' + PORT,
|
||||
screen_dir: SCREEN_DIR
|
||||
screen_dir: CONTENT_DIR, state_dir: STATE_DIR
|
||||
});
|
||||
console.log(info);
|
||||
fs.writeFileSync(path.join(SCREEN_DIR, '.server-info'), info + '\n');
|
||||
fs.writeFileSync(path.join(STATE_DIR, 'server-info'), info + '\n');
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -78,16 +78,17 @@ fi
|
||||
SESSION_ID="$$-$(date +%s)"
|
||||
|
||||
if [[ -n "$PROJECT_DIR" ]]; then
|
||||
SCREEN_DIR="${PROJECT_DIR}/.superpowers/brainstorm/${SESSION_ID}"
|
||||
SESSION_DIR="${PROJECT_DIR}/.superpowers/brainstorm/${SESSION_ID}"
|
||||
else
|
||||
SCREEN_DIR="/tmp/brainstorm-${SESSION_ID}"
|
||||
SESSION_DIR="/tmp/brainstorm-${SESSION_ID}"
|
||||
fi
|
||||
|
||||
PID_FILE="${SCREEN_DIR}/.server.pid"
|
||||
LOG_FILE="${SCREEN_DIR}/.server.log"
|
||||
STATE_DIR="${SESSION_DIR}/state"
|
||||
PID_FILE="${STATE_DIR}/server.pid"
|
||||
LOG_FILE="${STATE_DIR}/server.log"
|
||||
|
||||
# Create fresh session directory
|
||||
mkdir -p "$SCREEN_DIR"
|
||||
# Create fresh session directory with content and state peers
|
||||
mkdir -p "${SESSION_DIR}/content" "$STATE_DIR"
|
||||
|
||||
# Kill any existing server
|
||||
if [[ -f "$PID_FILE" ]]; then
|
||||
@@ -106,22 +107,16 @@ if [[ -z "$OWNER_PID" || "$OWNER_PID" == "1" ]]; then
|
||||
OWNER_PID="$PPID"
|
||||
fi
|
||||
|
||||
# On Windows/MSYS2, the MSYS2 PID namespace is invisible to Node.js.
|
||||
# Skip owner-PID monitoring — the 30-minute idle timeout prevents orphans.
|
||||
case "${OSTYPE:-}" in
|
||||
msys*|cygwin*|mingw*) OWNER_PID="" ;;
|
||||
esac
|
||||
|
||||
# Foreground mode for environments that reap detached/background processes.
|
||||
if [[ "$FOREGROUND" == "true" ]]; then
|
||||
echo "$$" > "$PID_FILE"
|
||||
env BRAINSTORM_DIR="$SCREEN_DIR" BRAINSTORM_HOST="$BIND_HOST" BRAINSTORM_URL_HOST="$URL_HOST" BRAINSTORM_OWNER_PID="$OWNER_PID" node server.cjs
|
||||
env BRAINSTORM_DIR="$SESSION_DIR" BRAINSTORM_HOST="$BIND_HOST" BRAINSTORM_URL_HOST="$URL_HOST" BRAINSTORM_OWNER_PID="$OWNER_PID" node server.cjs
|
||||
exit $?
|
||||
fi
|
||||
|
||||
# Start server, capturing output to log file
|
||||
# Use nohup to survive shell exit; disown to remove from job table
|
||||
nohup env BRAINSTORM_DIR="$SCREEN_DIR" BRAINSTORM_HOST="$BIND_HOST" BRAINSTORM_URL_HOST="$URL_HOST" BRAINSTORM_OWNER_PID="$OWNER_PID" node server.cjs > "$LOG_FILE" 2>&1 &
|
||||
nohup env BRAINSTORM_DIR="$SESSION_DIR" BRAINSTORM_HOST="$BIND_HOST" BRAINSTORM_URL_HOST="$URL_HOST" BRAINSTORM_OWNER_PID="$OWNER_PID" node server.cjs > "$LOG_FILE" 2>&1 &
|
||||
SERVER_PID=$!
|
||||
disown "$SERVER_PID" 2>/dev/null
|
||||
echo "$SERVER_PID" > "$PID_FILE"
|
||||
|
||||
@@ -1,19 +1,20 @@
|
||||
#!/usr/bin/env bash
|
||||
# Stop the brainstorm server and clean up
|
||||
# Usage: stop-server.sh <screen_dir>
|
||||
# Usage: stop-server.sh <session_dir>
|
||||
#
|
||||
# Kills the server process. Only deletes session directory if it's
|
||||
# under /tmp (ephemeral). Persistent directories (.superpowers/) are
|
||||
# kept so mockups can be reviewed later.
|
||||
|
||||
SCREEN_DIR="$1"
|
||||
SESSION_DIR="$1"
|
||||
|
||||
if [[ -z "$SCREEN_DIR" ]]; then
|
||||
echo '{"error": "Usage: stop-server.sh <screen_dir>"}'
|
||||
if [[ -z "$SESSION_DIR" ]]; then
|
||||
echo '{"error": "Usage: stop-server.sh <session_dir>"}'
|
||||
exit 1
|
||||
fi
|
||||
|
||||
PID_FILE="${SCREEN_DIR}/.server.pid"
|
||||
STATE_DIR="${SESSION_DIR}/state"
|
||||
PID_FILE="${STATE_DIR}/server.pid"
|
||||
|
||||
if [[ -f "$PID_FILE" ]]; then
|
||||
pid=$(cat "$PID_FILE")
|
||||
@@ -42,11 +43,11 @@ if [[ -f "$PID_FILE" ]]; then
|
||||
exit 1
|
||||
fi
|
||||
|
||||
rm -f "$PID_FILE" "${SCREEN_DIR}/.server.log"
|
||||
rm -f "$PID_FILE" "${STATE_DIR}/server.log"
|
||||
|
||||
# Only delete ephemeral /tmp directories
|
||||
if [[ "$SCREEN_DIR" == /tmp/* ]]; then
|
||||
rm -rf "$SCREEN_DIR"
|
||||
if [[ "$SESSION_DIR" == /tmp/* ]]; then
|
||||
rm -rf "$SESSION_DIR"
|
||||
fi
|
||||
|
||||
echo '{"status": "stopped"}'
|
||||
|
||||
@@ -5,7 +5,7 @@ description: 完成任务、实现重要功能或合并前使用,用于验证
|
||||
|
||||
# 请求代码审查
|
||||
|
||||
派遣 superpowers:code-reviewer 子代理来在问题扩散之前发现它们。审查者获得的是精心组织的评估上下文——绝不是你的会话历史。这样可以让审查者专注于工作成果而非你的思考过程,同时保留你自己的上下文以便继续工作。
|
||||
派遣代码审查子代理,在问题扩散之前发现它们。审查者获得的是精心组织的评估上下文——绝不是你的会话历史。这样可以让审查者专注于工作成果而非你的思考过程,同时保留你自己的上下文以便继续工作。
|
||||
|
||||
**核心原则:** 早审查,勤审查。
|
||||
|
||||
@@ -29,16 +29,15 @@ BASE_SHA=$(git rev-parse HEAD~1) # 或 origin/main
|
||||
HEAD_SHA=$(git rev-parse HEAD)
|
||||
```
|
||||
|
||||
**2. 派遣 code-reviewer 子代理:**
|
||||
**2. 派遣代码审查子代理:**
|
||||
|
||||
使用 Task 工具,指定 superpowers:code-reviewer 类型,填写 `code-reviewer.md` 中的模板
|
||||
使用 Task 工具,指定 `general-purpose` 类型,填写 `code-reviewer.md` 中的模板
|
||||
|
||||
**占位符说明:**
|
||||
- `{WHAT_WAS_IMPLEMENTED}` - 你刚完成的内容
|
||||
- `{DESCRIPTION}` - 你刚完成的内容简要说明
|
||||
- `{PLAN_OR_REQUIREMENTS}` - 预期功能
|
||||
- `{BASE_SHA}` - 起始提交
|
||||
- `{HEAD_SHA}` - 结束提交
|
||||
- `{DESCRIPTION}` - 简要说明
|
||||
|
||||
**3. 处理反馈:**
|
||||
- Critical 问题立即修复
|
||||
@@ -56,12 +55,11 @@ HEAD_SHA=$(git rev-parse HEAD)
|
||||
BASE_SHA=$(git log --oneline | grep "Task 1" | head -1 | awk '{print $1}')
|
||||
HEAD_SHA=$(git rev-parse HEAD)
|
||||
|
||||
[派遣 superpowers:code-reviewer 子代理]
|
||||
WHAT_WAS_IMPLEMENTED: 会话索引的验证和修复功能
|
||||
[派遣代码审查子代理]
|
||||
DESCRIPTION: 添加了 verifyIndex() 和 repairIndex(),支持 4 种问题类型
|
||||
PLAN_OR_REQUIREMENTS: docs/superpowers/plans/deployment-plan.md 中的任务 2
|
||||
BASE_SHA: a7981ec
|
||||
HEAD_SHA: 3df7661
|
||||
DESCRIPTION: 添加了 verifyIndex() 和 repairIndex(),支持 4 种问题类型
|
||||
|
||||
[子代理返回]:
|
||||
优点:架构清晰,测试真实
|
||||
@@ -82,8 +80,8 @@ HEAD_SHA=$(git rev-parse HEAD)
|
||||
- 修复后再进入下一个任务
|
||||
|
||||
**执行计划:**
|
||||
- 每批(3 个任务)后审查
|
||||
- 获取反馈,修复,继续
|
||||
- 每个任务完成后或在自然 checkpoint 审查
|
||||
- 获取反馈,应用,继续
|
||||
|
||||
**临时开发:**
|
||||
- 合并前审查
|
||||
|
||||
@@ -1,146 +1,166 @@
|
||||
# 代码审查代理
|
||||
# 代码审查员提示模板
|
||||
|
||||
你正在审查代码变更的生产就绪程度。
|
||||
派遣代码审查员子代理时使用此模板。
|
||||
|
||||
**你的任务:**
|
||||
1. 审查 {WHAT_WAS_IMPLEMENTED}
|
||||
2. 对照 {PLAN_OR_REQUIREMENTS} 进行比较
|
||||
3. 检查代码质量、架构、测试
|
||||
4. 按严重程度分类问题
|
||||
5. 评估生产就绪程度
|
||||
**用途:** 在工作成果扩散到更多工作之前,对照需求和代码质量标准做一次审查。
|
||||
|
||||
## 实现内容
|
||||
```
|
||||
Task tool(general-purpose):
|
||||
description: "审查代码改动"
|
||||
prompt: |
|
||||
你是一名资深代码审查员,精通软件架构、设计模式与最佳实践。
|
||||
你的工作是对照计划或需求审查已完成的工作,在问题扩散之前发现它们。
|
||||
|
||||
{DESCRIPTION}
|
||||
## 实现内容
|
||||
|
||||
## 需求/计划
|
||||
{DESCRIPTION}
|
||||
|
||||
{PLAN_REFERENCE}
|
||||
## 需求 / 计划
|
||||
|
||||
## 待审查的 Git 范围
|
||||
{PLAN_OR_REQUIREMENTS}
|
||||
|
||||
**Base:** {BASE_SHA}
|
||||
**Head:** {HEAD_SHA}
|
||||
## 待审查的 Git 范围
|
||||
|
||||
```bash
|
||||
git diff --stat {BASE_SHA}..{HEAD_SHA}
|
||||
git diff {BASE_SHA}..{HEAD_SHA}
|
||||
**Base:** {BASE_SHA}
|
||||
**Head:** {HEAD_SHA}
|
||||
|
||||
```bash
|
||||
git diff --stat {BASE_SHA}..{HEAD_SHA}
|
||||
git diff {BASE_SHA}..{HEAD_SHA}
|
||||
```
|
||||
|
||||
## 检查内容
|
||||
|
||||
**计划对齐:**
|
||||
- 实现是否匹配计划 / 需求?
|
||||
- 偏差是有道理的改进,还是有问题的偏离?
|
||||
- 计划中的所有功能都到位了吗?
|
||||
|
||||
**代码质量:**
|
||||
- 关注点分离清晰吗?
|
||||
- 错误处理到位吗?
|
||||
- 该有类型安全的地方有吗?
|
||||
- DRY 但没有过早抽象?
|
||||
- 边界情况处理了吗?
|
||||
|
||||
**架构:**
|
||||
- 设计决策合理吗?
|
||||
- 可扩展性和性能合理吗?
|
||||
- 有没有安全隐患?
|
||||
- 与周围代码集成是否干净?
|
||||
|
||||
**测试:**
|
||||
- 测试验证的是真实行为,不是 mock?
|
||||
- 边界情况覆盖了吗?
|
||||
- 该有集成测试的地方有吗?
|
||||
- 所有测试都通过吗?
|
||||
|
||||
**生产就绪:**
|
||||
- 如果改了 schema,有迁移策略吗?
|
||||
- 考虑了向后兼容吗?
|
||||
- 文档完整吗?
|
||||
- 没有明显 bug?
|
||||
|
||||
## 校准标准
|
||||
|
||||
按实际严重程度分类。不是所有问题都是 Critical。
|
||||
在列出问题之前先认可做得好的地方——准确的肯定能让实现者
|
||||
更愿意接受后续的反馈。
|
||||
|
||||
如果发现与计划有重大偏差,明确标出,让实现者确认这个偏差
|
||||
是不是有意为之。如果问题出在计划本身而不是实现,也要说清楚。
|
||||
|
||||
## 输出格式
|
||||
|
||||
### 优点
|
||||
[哪些地方做得好?具体一点。]
|
||||
|
||||
### 问题
|
||||
|
||||
#### Critical(必须修复)
|
||||
[bug、安全问题、数据丢失风险、功能损坏]
|
||||
|
||||
#### Important(应该修复)
|
||||
[架构问题、缺失功能、错误处理不到位、测试漏洞]
|
||||
|
||||
#### Minor(锦上添花)
|
||||
[代码风格、优化机会、文档润色]
|
||||
|
||||
每个问题包含:
|
||||
- File:line 引用
|
||||
- 哪里有问题
|
||||
- 为什么重要
|
||||
- 怎么修(如果不明显)
|
||||
|
||||
### 建议
|
||||
[关于代码质量、架构或流程的改进建议]
|
||||
|
||||
### 评估
|
||||
|
||||
**可以合并吗?** [是 | 否 | 修完再合]
|
||||
|
||||
**理由:** [1-2 句技术评估]
|
||||
|
||||
## 关键规则
|
||||
|
||||
**要做:**
|
||||
- 按实际严重程度分类
|
||||
- 具体(file:line,别含糊)
|
||||
- 解释为什么这个问题重要
|
||||
- 认可优点
|
||||
- 给出明确判断
|
||||
|
||||
**不要:**
|
||||
- 没检查就说"看起来 OK"
|
||||
- 把小事标成 Critical
|
||||
- 对没真看过的代码给反馈
|
||||
- 含糊其辞("改进错误处理")
|
||||
- 回避给出明确判断
|
||||
```
|
||||
|
||||
## 审查清单
|
||||
**占位符说明:**
|
||||
- `{DESCRIPTION}` —— 已构建内容的简要说明
|
||||
- `{PLAN_OR_REQUIREMENTS}` —— 预期功能(计划文件路径、任务文本或需求)
|
||||
- `{BASE_SHA}` —— 起始 commit
|
||||
- `{HEAD_SHA}` —— 结束 commit
|
||||
|
||||
**代码质量:**
|
||||
- 关注点分离是否清晰?
|
||||
- 错误处理是否恰当?
|
||||
- 类型安全(如适用)?
|
||||
- 是否遵循 DRY 原则?
|
||||
- 边界情况是否已处理?
|
||||
|
||||
**架构:**
|
||||
- 设计决策是否合理?
|
||||
- 是否考虑了可扩展性?
|
||||
- 性能影响如何?
|
||||
- 安全方面有无隐患?
|
||||
|
||||
**测试:**
|
||||
- 测试是否真正测试了逻辑(而不只是 mock)?
|
||||
- 边界情况是否覆盖?
|
||||
- 需要的地方是否有集成测试?
|
||||
- 所有测试是否通过?
|
||||
|
||||
**需求:**
|
||||
- 是否满足了计划中的所有需求?
|
||||
- 实现是否与规格一致?
|
||||
- 有无范围蔓延?
|
||||
- 破坏性变更是否已记录?
|
||||
|
||||
**生产就绪:**
|
||||
- 迁移策略(如有 schema 变更)?
|
||||
- 是否考虑了向后兼容性?
|
||||
- 文档是否完备?
|
||||
- 有无明显 bug?
|
||||
|
||||
## 输出格式
|
||||
|
||||
### 优点
|
||||
[做得好的地方?要具体。]
|
||||
|
||||
### 问题
|
||||
|
||||
#### Critical(必须修复)
|
||||
[Bug、安全问题、数据丢失风险、功能异常]
|
||||
|
||||
#### Important(应该修复)
|
||||
[架构问题、缺失功能、错误处理不足、测试缺口]
|
||||
|
||||
#### Minor(可以改进)
|
||||
[代码风格、优化机会、文档改进]
|
||||
|
||||
**每个问题需包含:**
|
||||
- 文件:行号引用
|
||||
- 什么有问题
|
||||
- 为什么重要
|
||||
- 如何修复(如果不明显的话)
|
||||
|
||||
### 建议
|
||||
[对代码质量、架构或流程的改进建议]
|
||||
|
||||
### 评估
|
||||
|
||||
**可以合并吗?** [是/否/修复后可以]
|
||||
|
||||
**理由:** [1-2 句话的技术评估]
|
||||
|
||||
## 关键规则
|
||||
|
||||
**应该做的:**
|
||||
- 按实际严重程度分类(不是所有问题都是 Critical)
|
||||
- 要具体(文件:行号,不要含糊)
|
||||
- 解释问题为什么重要
|
||||
- 肯定做得好的地方
|
||||
- 给出明确结论
|
||||
|
||||
**不该做的:**
|
||||
- 没仔细看就说"看起来不错"
|
||||
- 把小问题标为 Critical
|
||||
- 对没有审查的代码给反馈
|
||||
- 含糊其辞("改善错误处理")
|
||||
- 回避给出明确结论
|
||||
**审查员返回:** 优点、问题(Critical / Important / Minor)、建议、评估
|
||||
|
||||
## 输出示例
|
||||
|
||||
```
|
||||
### 优点
|
||||
- 数据库 schema 清晰,迁移规范(db.ts:15-42)
|
||||
- 测试覆盖全面(18 个测试,覆盖所有边界情况)
|
||||
- 错误处理良好,有降级方案(summarizer.ts:85-92)
|
||||
- 数据库 schema 干净,迁移规范(db.ts:15-42)
|
||||
- 测试覆盖全面(18 个测试,所有边界情况都覆盖)
|
||||
- 错误处理有 fallback,做得很好(summarizer.ts:85-92)
|
||||
|
||||
### 问题
|
||||
|
||||
#### Important
|
||||
1. **CLI 包装器缺少帮助文本**
|
||||
- 文件:index-conversations:1-31
|
||||
- 问题:没有 --help 选项,用户无法发现 --concurrency
|
||||
- 修复:添加 --help 分支,附带使用示例
|
||||
1. **CLI wrapper 缺少帮助文本**
|
||||
- File: index-conversations:1-31
|
||||
- 问题:没有 --help flag,用户不会发现 --concurrency
|
||||
- 修复:加 --help case 含使用示例
|
||||
|
||||
2. **日期校验缺失**
|
||||
- 文件:search.ts:25-27
|
||||
- 问题:无效日期静默返回空结果
|
||||
- 修复:校验 ISO 格式,抛出带示例的错误
|
||||
2. **缺少日期校验**
|
||||
- File: search.ts:25-27
|
||||
- 问题:无效日期会静默返回空结果
|
||||
- 修复:校验 ISO 格式,抛错并附示例
|
||||
|
||||
#### Minor
|
||||
1. **进度指示器**
|
||||
- 文件:indexer.ts:130
|
||||
- 问题:长时间操作没有"X / Y"计数器
|
||||
- 影响:用户不知道还要等多久
|
||||
1. **进度指示**
|
||||
- File: indexer.ts:130
|
||||
- 问题:长操作没有 "X of Y" 计数
|
||||
- 影响:用户不知道要等多久
|
||||
|
||||
### 建议
|
||||
- 添加进度报告以改善用户体验
|
||||
- 考虑用配置文件管理排除的项目(提高可移植性)
|
||||
- 加进度上报改善用户体验
|
||||
- 考虑用配置文件管理排除项目(提升可移植性)
|
||||
|
||||
### 评估
|
||||
|
||||
**可以合并:修复后可以**
|
||||
**可以合并吗:修完再合**
|
||||
|
||||
**理由:** 核心实现扎实,架构合理,测试充分。Important 问题(帮助文本、日期校验)容易修复,不影响核心功能。
|
||||
**理由:** 核心实现扎实,架构和测试都很好。Important 问题(帮助文本、
|
||||
日期校验)很容易修,且不影响核心功能。
|
||||
```
|
||||
|
||||
Reference in New Issue
Block a user