forked from Gitlink/gitlink-cli
feat(skills): 强化独立代码审查报告
This commit is contained in:
parent
d3bcbae82a
commit
5ce41cb36d
|
|
@ -1,321 +1,332 @@
|
||||||
---
|
---
|
||||||
name: gitlink-code-review
|
name: gitlink-code-review
|
||||||
version: 1.0.0
|
description: "GitLink 社区智能审查：审查一个或多个 PR 的贡献价值、变更范围、Review 修改履约、实现可行性、代码质量、逻辑、测试、维护性、性能、兼容性和安全性，并可执行仓库代码健康扫描与批量 Issue 分诊。生成关键结论前置、证据完整的只读 Markdown 报告和待人工审核的 Review 建议；默认不调用其他 Skill、不评论或修改远端。"
|
||||||
description: "智能代码审查：获取 PR 变更、分析代码质量、自动生成 Review 评论与摘要报告。当用户需要审查 Pull Request、检查代码质量或生成审查报告时触发。"
|
|
||||||
metadata:
|
|
||||||
requires:
|
|
||||||
bins: ["gitlink-cli"]
|
|
||||||
cliHelp: "gitlink-cli pr --help"
|
|
||||||
---
|
---
|
||||||
|
|
||||||
# gitlink-code-review（智能代码审查）
|
# GitLink 社区智能审查
|
||||||
|
|
||||||
**CRITICAL — 开始前必须先阅读 [`../gitlink-shared/SKILL.md`](../gitlink-shared/SKILL.md)，其中包含认证、权限处理和 API 注意事项。**
|
以 PR 审查为主线，保留仓库健康扫描和 Issue 分诊。优化信息顺序，不缩减原有分析能力。
|
||||||
**CRITICAL — 所有写入/删除操作前，务必先确认用户意图。**
|
|
||||||
**CRITICAL — GitLink 操作只能用 `gitlink-cli`。禁止用 `gh`（GitHub CLI）操作 GitLink 资源。`gh` 仅适用于 GitHub 平台。**
|
|
||||||
|
|
||||||
> **前置条件：** 先阅读 [`../gitlink-shared/SKILL.md`](../gitlink-shared/SKILL.md) 了解认证和全局参数。
|
## 默认调用契约
|
||||||
|
|
||||||
## 工作流概览
|
用户只需点名 `gitlink-code-review` 并提供仓库以及一个或多个 PR 编号。没有 PR 编号但明确要求仓库健康扫描或 Issue 分诊时，执行对应模式；同时要求多项能力时，生成一份综合报告。
|
||||||
|
|
||||||
本 Skill 提供一套完整的 AI 驱动代码审查工作流，覆盖从获取 PR 变更到生成审查报告的全过程。不需要额外的 CLI Shortcuts——现有 `gitlink-cli` 命令 + AI Agent 的分析能力即可完成。
|
默认遵守以下规则：
|
||||||
|
|
||||||
| 阶段 | 操作 | AI Agent 角色 |
|
- **完整审查**：不限制为固定五个维度；按改动实际风险选择价值、可行性、Review 履约、逻辑、质量、测试、维护性、性能、兼容性、安全、文档和协作等维度。
|
||||||
|------|------|--------------|
|
- **Review 闭环**：存在既有 Review 时，逐条判断作者是否修改、修改是否满足要求、证据是否充分以及是否引入回归。
|
||||||
| ① 获取上下文 | 拉取 PR 详情、变更文件、Diff | 执行 CLI 命令采集数据 |
|
- **只读远端**：可以生成 Review 结论、整体评论草稿和内联评论草稿，但不提交 Review、不评论、不 approve、不合并、不关闭、不分配、不改标签。
|
||||||
| ② 分析代码 | 检查每个文件的变更 | 逐文件审查，标记问题 |
|
- **独立运行**：不调用其他 Skill。遇到需要专项判断的内容，可以注明验证限制，但仍完成本 Skill 能够完成的分析。
|
||||||
| ③ 结构化反馈 | 按严重程度分级输出审查意见 | 生成分级 Review 评论 |
|
- **结论前置**：首屏先显示评审建议、阻断项、Review 履约结果和最多 5 项关键动作；blocking/high 使用颜色和粗体，并保留纯文本标签。
|
||||||
| ④ 提交评论 | 发表 Review 到 PR | 通过 API 提交 |
|
- **报告结论加依据**：Markdown 中的价值、实现、Review 履约、测试和安全等关键方面先醒目显示 `passed/failed/partial/not_run`，随后用 1 至 2 句说明实际功能、判定证据和影响；状态词不能脱离描述单独出现。
|
||||||
| ⑤ 生成报告 | 输出审查摘要 | 生成 Markdown 摘要 |
|
- **证据可追溯**：发现使用 `CR-`，Review 履约项使用 `RV-`，健康项使用 `RH-`，Issue 分诊项使用 `IT-` 稳定编号。
|
||||||
|
- **单文件落盘**：一次运行生成一份 UTF-8 Markdown，保存到 `reports/skill-runs/gitlink-code-review/<owner>-<repo>-<scope>-<yyyyMMdd-HHmmssZ>.md`。
|
||||||
|
|
||||||
---
|
最终回复的 PR 部分必须按 PR 分节，并分别显示 Review 建议、贡献价值、Review 履约、实现与逻辑、测试、安全和关键发现七张结论前置判断卡；不得压缩成一段“审查结论”。Issue 部分必须解释每个 P 级别含义、本批事项共同问题和下一步。最后声明 Review 草稿未提交并给出 Markdown 绝对路径。无法写入工作区时输出完整 Markdown 并标记“未落盘”。Markdown 不写 ANSI；凭据、cookie、token 和敏感值必须脱敏。
|
||||||
|
|
||||||
## 详细工作流
|
## 运行模式
|
||||||
|
|
||||||
### 工作流 1：PR 代码审查
|
### PR 审查模式
|
||||||
|
|
||||||
**场景**：收到 PR Review 请求后，进行完整代码审查。
|
给出 PR 编号时默认启用，包含 PR 变更、Review 履约、完整代码审查、运行验证和 Review 建议。
|
||||||
|
|
||||||
#### Step 1：获取 PR 上下文
|
### 仓库健康模式
|
||||||
|
|
||||||
```bash
|
用户要求仓库级检查时启用；综合报告中放在 PR 详细审查之后。检查文档、许可证、CI 配置、代码规范、测试结构、依赖管理、安全基线和 Issue 治理状态。
|
||||||
# 获取 PR 详情
|
|
||||||
gitlink-cli pr +view --id <pr_id> --format json
|
|
||||||
|
|
||||||
# 获取变更文件列表
|
### Issue 分诊模式
|
||||||
gitlink-cli pr +files --id <pr_id> --format json
|
|
||||||
|
|
||||||
# 获取 Diff 内容（含变更行号和代码上下文）
|
用户要求 Issue 扫描或综合社区审查时启用，支持三种明确范围：
|
||||||
gitlink-cli pr +diff --id <pr_id> --format json
|
|
||||||
```
|
|
||||||
|
|
||||||
#### Step 2：逐文件分析
|
- **前 N 条 open Issue**：例如“处理前 40 条 open Issue”；按最近更新时间降序取 N 条，并用真实状态二次过滤。
|
||||||
|
- **全部 open Issue**：自动翻页、按 Issue ID 去重并处理当前全部开启项；报告必须记录实际页数、条数和截断/失败情况。
|
||||||
|
- **指定 Issue**：例如“只处理 #12、#18、#31”；逐条读取并回显真实状态，closed 项只标记为历史项，不混入 open 待办。
|
||||||
|
|
||||||
对每个变更文件，根据文件类型执行针对性检查：
|
用户只说“处理 Issue”但没有范围时，默认取最近更新的前 40 条 open Issue，并在首屏明确该默认范围。只请求 PR 审查时不自动扫描 Issue，首屏省略 `Issue 待办`，报告末尾注明该模式未启用。Issue 首屏只列范围、各优先级数量和编号，详细分类统一放在报告最后。
|
||||||
|
|
||||||
**Python 文件检查项：**
|
## 聊天和报告首屏固定结构
|
||||||
- 语法与导入：未使用的 import、循环导入、wildcard import
|
|
||||||
- 代码规范：PEP 8 风格偏离、过长行（>88 chars）、命名规范
|
|
||||||
- 安全：硬编码密钥、SQL 注入风险、`eval()`/`exec()` 使用
|
|
||||||
- 性能：不必要的循环、缺少缓存、N+1 查询
|
|
||||||
- 错误处理：裸 `except`、吞异常、缺少 finally
|
|
||||||
|
|
||||||
**JavaScript/TypeScript 文件检查项：**
|
聊天可以比详细报告短，但必须逐 PR 保留全部专项方面；每一方面独立成行，最直接结论位于最前：
|
||||||
- 安全：`innerHTML` 直接赋值、`eval()` 使用
|
|
||||||
- 类型安全：`any` 滥用、缺失类型定义
|
|
||||||
- 性能：不必要的 re-render、大对象深拷贝
|
|
||||||
- 异步：未处理的 Promise、缺少 error boundary
|
|
||||||
- 依赖：已废弃 API 使用
|
|
||||||
|
|
||||||
**Go 文件检查项：**
|
|
||||||
- 错误处理：未检查的 error return、panic 滥用
|
|
||||||
- 并发：goroutine 泄漏、缺少 sync 保护
|
|
||||||
- 资源管理：未关闭的 file/conn、defer 使用
|
|
||||||
- 命名：导出标识符缺少注释、变量 shadowing
|
|
||||||
|
|
||||||
**通用检查项：**
|
|
||||||
- 硬编码的配置值、密钥、URL
|
|
||||||
- 缺少或错误的边界条件检查
|
|
||||||
- 过于复杂的函数（圈复杂度高）
|
|
||||||
- 魔法数字（未命名的常量）
|
|
||||||
- 重复代码（DRY 违反）
|
|
||||||
- 缺少或过时的注释
|
|
||||||
- 测试覆盖不足
|
|
||||||
|
|
||||||
#### Step 3：生成结构化审查结果
|
|
||||||
|
|
||||||
按以下 Severity 分级输出：
|
|
||||||
|
|
||||||
```markdown
|
```markdown
|
||||||
## PR #<id> 代码审查报告
|
## PR #<number>
|
||||||
|
**Review 建议：** <span style="color:#B42318"><strong>修改后再审</strong></span> **[action_required]**：存在 2 个影响真实使用的问题；依据：CR-<number>-001、CR-<number>-002；下一步：按发现逐项修复并复验。
|
||||||
|
**贡献价值：** <span style="color:#067647"><strong>价值成立</strong></span> **[passed]**：解决 <实际问题>；依据：默认分支差异、需求和受益范围；影响：<用户或维护收益>。
|
||||||
|
**Review 履约：** <span style="color:#175CD3"><strong>本轮无可核对 Review</strong></span> **[not_applicable]**：没有有效 Review 意见；依据：Review 列表与当前 head；影响：只评估当前完整 Diff。
|
||||||
|
**实现与逻辑：** <span style="color:#B42318"><strong>核心边界仍有错误</strong></span> **[failed]**：<触发条件与错误行为>；依据：`path/file.go:42` 与复现命令；下一步：<具体修改>。
|
||||||
|
**测试：** <span style="color:#B54708"><strong>关键失败路径缺失</strong></span> **[partial]**：正常测试通过但 <场景> 未覆盖；依据：测试文件与执行结果；下一步：补回归用例。
|
||||||
|
**安全：** <span style="color:#067647"><strong>未扩大安全边界</strong></span> **[passed]**：没有新增认证、执行或敏感输出路径；依据：Diff 与安全矩阵；影响：无安全阻断。
|
||||||
|
**关键发现：** <span style="color:#B42318"><strong>2 项必须修改</strong></span> **[high]**：CR-<number>-001、CR-<number>-002；依据：文件行号和复现证据；下一步：优先修复 high 项。
|
||||||
|
|
||||||
### 🔴 Critical（必须修改）
|
**Issue 分诊**
|
||||||
- <问题描述> — <文件>:<行号>
|
|
||||||
> <修改建议>
|
|
||||||
|
|
||||||
### 🟡 Warning（建议修改）
|
P0（立即处置）：0 条。没有发现安全事故、数据损坏或核心服务不可用事项。
|
||||||
- <问题描述> — <文件>:<行号>
|
P1（本轮优先处理）：#27、#26、#24。上述事项影响常用流程或阻塞维护工作，信息基本完整，应在当前维护周期确认负责人并推进。
|
||||||
> <修改建议>
|
P2（进入计划处理）：#25、#17。问题真实但不构成当前阻断，建议补充验收条件后排入迭代。
|
||||||
|
P3（可延后或先补信息）：#23、#22。影响较低或上下文不足，先请求复现信息、去重或确认需求。
|
||||||
### 🔵 Suggestion（可选优化）
|
|
||||||
- <问题描述> — <文件>:<行号>
|
|
||||||
> <修改建议>
|
|
||||||
|
|
||||||
### ✅ Positive（值得肯定）
|
|
||||||
- <做得好的地方>
|
|
||||||
```
|
```
|
||||||
|
|
||||||
#### Step 4：提交 Review 评论
|
问题编号、文件位置、问题数量和 Issue 概述必须来自本轮证据，不能复制示例。closed/merged 历史 PR 仍按同样七方面输出，以 `not_applicable` 说明无需当前门禁，并在 Review 建议中写清保持关闭或历史对照的依据。
|
||||||
|
|
||||||
```bash
|
## Markdown 报告首屏固定结构
|
||||||
# 方式 1：提交整体 Review
|
|
||||||
gitlink-cli pr +review --body '{
|
|
||||||
"body": "## 审查结果\n\n### 🔴 Critical\n...\n\n### 🟡 Warning\n...\n\n总体评价：...",
|
|
||||||
"event": "COMMENT"
|
|
||||||
}'
|
|
||||||
|
|
||||||
# 方式 2：在特定行添加内联评论（逐条提交）
|
首屏只保留直接改变维护者决策的信息：
|
||||||
gitlink-cli pr +review --body '{
|
|
||||||
"body": "这里存在安全风险：用户输入未经转义直接拼接到 SQL 查询中，存在注入风险。建议使用参数化查询。",
|
|
||||||
"event": "COMMENT",
|
|
||||||
"commit_id": "<commit_sha>",
|
|
||||||
"path": "src/query.py",
|
|
||||||
"position": 42
|
|
||||||
}'
|
|
||||||
```
|
|
||||||
|
|
||||||
> **注意：** `event` 参数支持 `COMMENT`（普通评论）和 `APPROVE`（批准）。对于需要修改的问题，使用 `COMMENT`。
|
|
||||||
|
|
||||||
#### Step 5：生成审查摘要
|
|
||||||
|
|
||||||
审查完成后，输出 Markdown 摘要供用户查阅：
|
|
||||||
|
|
||||||
```markdown
|
```markdown
|
||||||
## 📋 审查摘要 — PR #<id> <title>
|
# GitLink 社区审查摘要
|
||||||
|
|
||||||
| 指标 | 数据 |
|
## PR #123
|
||||||
|------|------|
|
**Review 建议：** <span style="color:#B42318"><strong>修改后再审</strong></span> **[action_required]**：正常流程可用，但 closed PR 会进入 open 队列；依据：`shortcuts/workflow/pr_fetch.go:403`、真实响应和缺失的回归 fixture；下一步：增加客户端二次过滤并补三类真实响应测试后复看。
|
||||||
| 审查文件数 | <n> |
|
|
||||||
| 变更行数 | +<add> / -<del> |
|
|
||||||
| Critical 问题 | <n> |
|
|
||||||
| Warning | <n> |
|
|
||||||
| Suggestion | <n> |
|
|
||||||
|
|
||||||
### 主要发现
|
**贡献价值：** <span style="color:#067647"><strong>价值成立</strong></span> **[passed]**：为维护者增加 SLA 与责任方识别；依据：默认分支没有等价输出、需求与受益范围；影响：减少人工排队。
|
||||||
1. **[Critical]** <最严重的问题>
|
**Review 履约：** <span style="color:#175CD3"><strong>没有待履约意见</strong></span> **[not_applicable]**：当前没有有效 Review；依据：Review 列表与 head SHA；影响：本轮只评价完整 Diff。
|
||||||
2. **[Warning]** <次要问题>
|
**实现与逻辑：** <span style="color:#B42318"><strong>核心队列结果不可靠</strong></span> **[failed]**：真实 open 查询会混入 closed PR；依据：真实响应与 `pull_request_status` 归一化路径；下一步：增加客户端二次过滤。
|
||||||
3. **[Suggestion]** <优化建议>
|
**测试：** <span style="color:#B54708"><strong>真实响应覆盖不完整</strong></span> **[partial]**：构建和理想响应测试通过；依据：测试命令和现有 fixture；下一步：补服务端忽略 state、数值状态和无责任字段场景。
|
||||||
|
**安全：** <span style="color:#067647"><strong>未扩大安全边界</strong></span> **[passed]**：改动为只读归一化；依据：Diff 未新增认证、权限、执行或敏感输出路径；影响：无安全阻断。
|
||||||
|
**关键发现：** <span style="color:#B42318"><strong>1 项 high 必须修复</strong></span> **[high]**：open 队列可能包含 closed PR；依据：CR-001 与真实响应；下一步：修复后复验。
|
||||||
|
|
||||||
### 总体评价
|
**代码发现：**
|
||||||
<整体评估：代码质量、审查通过建议>
|
- **blocking（阻止合并）：0 条。** 未发现已证实的漏洞、数据破坏或不可逆回归。
|
||||||
|
- <span style="color:#B54708"><strong>high（本轮必须修复）：1 条，CR-001。</strong></span> open 队列可能包含 closed PR，直接影响维护者判断。
|
||||||
|
- **medium（应补齐后复看）：2 条，CR-002、CR-003。** 缺少真实响应测试，更新时间回退来源也未显式说明。
|
||||||
|
- **low（可延后优化）：0 条。**
|
||||||
|
|
||||||
---
|
## 已关闭历史对照
|
||||||
*由 gitlink-code-review Skill 自动生成*
|
**#90：** 状态为 closed，只用于比较既有实现，不生成当前门禁、Review 建议或重新打开建议。
|
||||||
|
|
||||||
|
## Issue 分诊（最近更新的前 40 条 open，实际取得 15 条）
|
||||||
|
**P0（立即处置）：0 条。** 没有发现需要立刻止损的安全事故、数据损坏或核心服务不可用事项。
|
||||||
|
**P1（本轮优先处理）：#81、#82、#83。** 这些事项影响常用流程或阻塞维护工作，信息基本足够，应在当前维护周期确认负责人并推进。
|
||||||
|
**P2（进入计划处理）：#84、#85。** 问题真实但没有当前阻断证据，建议补充验收条件后进入迭代计划。
|
||||||
|
**P3（可延后或先补信息）：#86、#87。** 影响较低或上下文不足，先请求复现信息、去重或确认需求。
|
||||||
|
|
||||||
|
## 先处理这 3 项
|
||||||
|
|
||||||
|
1. <span style="color:#B42318"><strong>[RV-003][high] 补全 Review 要求</strong></span>：失败路径仍未返回可诊断错误。
|
||||||
|
2. <span style="color:#B54708"><strong>[CR-002][high] 增加回归测试</strong></span>：复杂分支名未覆盖 URL 编码。
|
||||||
|
3. <span style="color:#B54708"><strong>[CR-004][high] 收紧权限边界</strong></span>：写操作缺少资源归属校验。
|
||||||
```
|
```
|
||||||
|
|
||||||
---
|
示例中的编号、状态和发现仅用于定义格式，实际输出必须从本次 API、Diff、Review 和测试证据重新计算，禁止复制示例结论。
|
||||||
|
|
||||||
### 工作流 2：仓库代码健康度扫描
|
聊天和 Markdown 中每个 PR 都必须有独立的七方面判断卡，不能把多条 PR 或多个方面合并成一句“修改后再审”。每张卡必须以加粗、着色的结论开头，随后给出简短解释、明确的 `依据：` 和影响/下一步；任何结论都不能单独出现。closed/merged 历史项仍按七方面输出，以 `not_applicable` 说明无需当前门禁。Issue 首屏不逐条展开完整正文，但每个 P 级别必须说明级别含义、编号、本批事项的共同问题和下一步，不能只列计数。颜色只用于状态结论、总建议、blocking/high 和关键动作；始终保留 `[action_required]`、`[high]` 等文本回退。
|
||||||
|
|
||||||
**场景**：对仓库整体代码质量进行评估，不依赖 PR。
|
## 证据采集
|
||||||
|
|
||||||
|
优先获取统一上下文：
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
# 1. 获取仓库信息
|
gitlink-cli workflow +review-context --owner <owner> --repo <repo> --number <number> --include-commits=true --include-ci=true --format json
|
||||||
gitlink-cli repo +info --owner <owner> --repo <repo> --format json
|
|
||||||
|
|
||||||
# 2. 获取仓库文件列表（遍历关键目录）
|
|
||||||
gitlink-cli repo +files --query 'filepath=src&ref=master'
|
|
||||||
gitlink-cli repo +files --query 'filepath=tests&ref=master'
|
|
||||||
|
|
||||||
# 3. 获取关键文件内容
|
|
||||||
gitlink-cli repo +raw --ref=master/README.md
|
|
||||||
gitlink-cli repo +raw --ref=master/.gitignore
|
|
||||||
gitlink-cli repo +raw --ref=master/.eslintrc.js # 或类似配置
|
|
||||||
gitlink-cli repo +raw --ref=master/package.json # 或 go.mod, Cargo.toml
|
|
||||||
|
|
||||||
# 4. 获取语言统计和贡献者
|
|
||||||
gitlink-cli repo +languages
|
|
||||||
gitlink-cli repo +contributors
|
|
||||||
```
|
```
|
||||||
|
|
||||||
**健康度检查清单：**
|
接口不可用时分别获取：
|
||||||
|
|
||||||
| 检查项 | 标准 | 评分依据 |
|
```bash
|
||||||
|--------|------|----------|
|
gitlink-cli pr +view --owner <owner> --repo <repo> --id <pull_request_id> --format json
|
||||||
| 文档完整性 | 有 README、CONTRIBUTING、CHANGELOG | 文件是否存在、内容质量 |
|
gitlink-cli pr +files --owner <owner> --repo <repo> --id <pull_request_id> --format json
|
||||||
| 许可证 | 有 LICENSE 文件 | 是否存在、是否合规 |
|
gitlink-cli pr +diff --owner <owner> --repo <repo> --id <pull_request_id> --format json
|
||||||
| CI 配置 | 有 CI 配置（.github/workflows, Jenkinsfile 等） | 文件是否存在 |
|
gitlink-cli pr +reviews --owner <owner> --repo <repo> --id <pull_request_id> --format json
|
||||||
| 代码规范 | 有 linter 配置 | eslint/prettier/ruff/pylint 等 |
|
```
|
||||||
| 测试覆盖 | 有 test 目录或测试文件 | 测试文件比例 |
|
|
||||||
| 依赖管理 | 依赖文件完整且无已知漏洞 | package-lock/go.sum/poetry.lock |
|
|
||||||
| Issue 健康度 | Issue 有分类标签、响应及时 | 通过 Issue 列表分析 |
|
|
||||||
|
|
||||||
**输出格式：**
|
记录 owner、repo、用户可见 PR 编号、`pull_request_id`、base、head、head SHA、采集时间和数据来源。详情、Diff、Review、CI 和本地检出必须对应同一快照；不一致时标记 `stale`，不能写成通过。
|
||||||
|
|
||||||
|
多个 PR 必须建立独立上下文、独立结论和独立编号空间，不得混合 Diff 或证据。批量摘要只合并数量和优先级，不合并具体判断。
|
||||||
|
|
||||||
|
## PR 审查流程
|
||||||
|
|
||||||
|
### 1. 理解目标和贡献价值
|
||||||
|
|
||||||
|
对照标题、描述、关联 Issue、提交和实际 Diff，回答：
|
||||||
|
|
||||||
|
- 解决的问题是否真实、常用并适合仓库定位。
|
||||||
|
- 实际改动是否覆盖声明功能，是否存在未说明的范围扩张。
|
||||||
|
- 是否重复现有能力，或是否提供更完整、兼容、可维护的实现。
|
||||||
|
- 对用户、维护者、自动化脚本和后续扩展有什么实际影响。
|
||||||
|
|
||||||
|
作者声明只能作为验证目标，不能直接作为通过证据。
|
||||||
|
|
||||||
|
### 2. 分析 PR 变更
|
||||||
|
|
||||||
|
列出新增、删除、重构和行为变化，标明核心文件、测试、文档、依赖、权限、文件、网络、命令执行和敏感输出变化。区分：
|
||||||
|
|
||||||
|
- PR 初始实现包含的改动。
|
||||||
|
- Review 后新增的修复提交。
|
||||||
|
- 与 Review 无关的新范围。
|
||||||
|
- 修复过程中被删除或退化的既有能力。
|
||||||
|
|
||||||
|
只有当前完整 Diff 时，可以分析最终变更，但不得声称已经完成 Review 前后比较。
|
||||||
|
|
||||||
|
### 3. 验证 Review 修改履约
|
||||||
|
|
||||||
|
读取所有有效 Review、普通评论中的代码问题和后续提交。对每条可执行意见建立 `RV-` 项：
|
||||||
|
|
||||||
|
- `resolved`：当前实现满足要求，并有代码或测试证据。
|
||||||
|
- `partially_resolved`：只覆盖部分条件或缺少关键验证。
|
||||||
|
- `unresolved`：未修改，或修改与要求不一致。
|
||||||
|
- `regressed`：处理意见时引入新的行为、安全或兼容问题。
|
||||||
|
- `outdated`：目标代码已删除或结构变化使原意见不再适用。
|
||||||
|
- `not_verifiable`：缺少 Review 基线、提交映射或运行条件。
|
||||||
|
|
||||||
|
每项记录 reviewer、原意见摘要、原位置或时间、对应提交、当前位置、判断、证据和剩余动作。优先使用 Review 对应 commit SHA 与当前 head SHA 的增量 diff；无法建立基线时明确降低置信度。
|
||||||
|
|
||||||
|
不要把“代码发生变化”当作“已经满足 Review”，必须核对意见中的行为要求和边界条件。
|
||||||
|
|
||||||
|
### 4. 执行完整代码审查
|
||||||
|
|
||||||
|
按改动风险选择并覆盖相关维度：
|
||||||
|
|
||||||
|
- 逻辑正确性和边界条件。
|
||||||
|
- 错误处理、资源释放、并发和状态一致性。
|
||||||
|
- 测试的正常、失败、边界、兼容和回归路径。
|
||||||
|
- 架构一致性、职责划分、复杂度、重复实现和长期维护成本。
|
||||||
|
- 性能退化、批量复杂度、分页、缓存和资源耗尽风险。
|
||||||
|
- CLI/API/JSON/帮助/i18n/UTF-8/跨平台兼容性。
|
||||||
|
- 注入、路径遍历、越权、凭据泄露、危险外联、依赖和供应链风险。
|
||||||
|
- 文档、示例、错误提示和迁移说明是否与实现一致。
|
||||||
|
- 实现亮点、测试亮点和已经正确吸收的 Review 意见。
|
||||||
|
|
||||||
|
安全检查读取本 Skill 的 [`references/security-review-matrix.md`](references/security-review-matrix.md)。疑似密钥只报告类型和位置，不复制值。
|
||||||
|
|
||||||
|
### 5. 执行功能与回归验证
|
||||||
|
|
||||||
|
优先使用仓库 README、CI、Makefile 和现有测试定义的环境。验证 PR 描述中的关键功能、Review 涉及路径、正常路径、失败路径、兼容路径和安全边界。
|
||||||
|
|
||||||
|
记录命令、工作目录、检出 SHA、退出码、耗时和输出摘要。CI 只统计匹配当前 head SHA 的构建；分支匹配只能作为低置信度回退。未执行或证据过期时写 `not_run`、`partial` 或 `stale`，不能写成通过。
|
||||||
|
|
||||||
|
### 6. 生成 Review 建议
|
||||||
|
|
||||||
|
对每个 open PR 分别给出 `建议合并`、`修改后再审`、`暂缓合并` 或 `需要人工判断`，并在报告中生成一份可供维护者直接审核的 Review 草稿。不得只写“测试失败”“实现不完整”等泛化意见，草稿至少包含：
|
||||||
|
|
||||||
|
1. PR 实际解决的问题和已经做对的部分。
|
||||||
|
2. 每个必须修改的问题，包含 `CR-/RV-` 编号、文件与行号或复现命令、当前行为和用户影响。
|
||||||
|
3. 具体修改要求，说明应改哪段逻辑、补什么边界或保持什么兼容行为，而不是只说“请优化”。
|
||||||
|
4. 需要新增或重跑的验证，以及维护者再次 Review 时的通过条件。
|
||||||
|
5. 若不存在阻断项，明确说明建议通过的证据和仍需关注的非阻断风险。
|
||||||
|
|
||||||
|
无论使用预定义结论还是根据仓库语境生成其他结论，都必须给出与结论匹配的依据。常见结论至少遵循以下要求：
|
||||||
|
|
||||||
|
- **建议合并**：先说明 PR 解决的具体问题和实现亮点，再列出已核验的正常、失败、兼容或安全证据，明确没有必须修改的 `blocking/high` 问题；存在非阻断风险时说明为什么不影响当前合并。
|
||||||
|
- **修改后再审**：先肯定已经成立的功能，再逐项指出不合格的文件、逻辑、触发条件和影响，给出具体修改方式、需要补充的测试以及可核验的复审通过条件。
|
||||||
|
- **暂缓合并**：说明当前阻塞来自前置依赖、主线冲突、外部 API、发布窗口还是仓库决策，列出已有证据、继续合并的具体风险、解除阻塞的责任方和重新评估条件。
|
||||||
|
- **需要人工判断**：列出无法由代码事实单独决定的选项和权衡，说明已经确认与仍缺失的证据，并把维护者需要回答的问题写成可执行决策点。
|
||||||
|
- **保持关闭/拒绝**：说明能力是否已被主线或其他 PR 覆盖、问题是否不适合仓库定位，引用对照提交或重复实现证据，并说明为什么继续投入没有增量价值。
|
||||||
|
|
||||||
|
需要精确定位时生成内联评论草稿。多个 PR 的 Review 草稿必须分节，不能共享结论或问题编号。closed/merged 历史 PR 不生成新的 Review 草稿，除非用户明确要求复审历史实现。
|
||||||
|
|
||||||
|
所有 Review 草稿先使用统一结构，结论必须位于具体描述之前：
|
||||||
|
|
||||||
```markdown
|
```markdown
|
||||||
## 🏥 仓库健康度报告 — <owner>/<repo>
|
### PR #<number> Review 建议草稿（未提交）
|
||||||
|
**Review 建议：** <span style="color:<status-color>"><strong><结论></strong></span> **[<decision_status>]**：<用 1 至 3 句说明 PR 做了什么、关键证据以及为什么得到该结论。>
|
||||||
|
|
||||||
### 总体评分：<⭐x/5>
|
**依据与影响：** <引用 Diff、文件行号、Review、测试命令或主线对照，说明成立的功能、存在的问题和用户/维护者影响。>
|
||||||
|
|
||||||
| 维度 | 状态 | 评分 | 建议 |
|
**下一步：** <合并、具体修改与复验、解除依赖或需要维护者决定的问题；没有必改项时明确写出。>
|
||||||
|------|:----:|:----:|------|
|
|
||||||
| 📖 文档 | ✅/⚠️/❌ | ☆☆☆☆☆ | <建议> |
|
|
||||||
| 📜 许可证 | ✅/⚠️/❌ | ☆☆☆☆☆ | <建议> |
|
|
||||||
| 🔧 CI/CD | ✅/⚠️/❌ | ☆☆☆☆☆ | <建议> |
|
|
||||||
| 🎨 代码规范 | ✅/⚠️/❌ | ☆☆☆☆☆ | <建议> |
|
|
||||||
| 🧪 测试覆盖 | ✅/⚠️/❌ | ☆☆☆☆☆ | <建议> |
|
|
||||||
| 📦 依赖安全 | ✅/⚠️/❌ | ☆☆☆☆☆ | <建议> |
|
|
||||||
| 🐛 Issue 管理 | ✅/⚠️/❌ | ☆☆☆☆☆ | <建议> |
|
|
||||||
|
|
||||||
### 关键发现
|
|
||||||
1. <最需要改进的问题>
|
|
||||||
2. <次要问题>
|
|
||||||
3. <做得好的方面>
|
|
||||||
|
|
||||||
### 改进路线图
|
|
||||||
- **紧急（本周）：** ...
|
|
||||||
- **短期（本月）：** ...
|
|
||||||
- **长期（本季度）：** ...
|
|
||||||
```
|
```
|
||||||
|
|
||||||
---
|
例如，“修改后再审”不能只写状态，必须落到可执行问题：
|
||||||
|
|
||||||
### 工作流 3：批量 Issue Triage + 自动分配
|
```markdown
|
||||||
|
### PR #<number> Review 建议草稿（未提交）
|
||||||
|
**Review 建议：** <span style="color:#B42318"><strong>修改后再审</strong></span> **[action_required]**：这项改动解决了 <具体问题>，其中 <已验证的优点> 已有证据；但 `path/file.go:42` 在 <触发条件> 下仍会 <错误行为和影响>，当前不能合并。
|
||||||
|
|
||||||
**场景**：对新 Issue 进行自动分类、标签分配和责任人推荐。
|
**依据与影响：** [CR-001][high] <当前行为、证据和用户影响>；[RV-001][medium] <既有 Review 未满足部分及证据>。
|
||||||
|
|
||||||
|
**下一步：** 请 <具体修改要求>，新增 <正常/失败/兼容场景> 测试并运行 `<仓库命令>`；确认 <预期结果> 后重新 Review。
|
||||||
|
```
|
||||||
|
|
||||||
|
草稿中的路径、行号、命令和要求必须来自当前 PR 证据；无法定位时标为 `not_verifiable` 并说明缺什么，不得填入示例占位内容。
|
||||||
|
|
||||||
|
无论结论为何，都不得调用远端写接口。最终回复必须明确说明“Review 草稿尚未提交，需人工审核”。
|
||||||
|
|
||||||
|
### 7. 校验报告中的 Review 依据
|
||||||
|
|
||||||
|
PR 审查模式完成 Markdown 后必须运行：
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
# 1. 获取未标记的 Issue
|
python -X utf8 skills/gitlink-code-review/scripts/validate_review_report.py \
|
||||||
gitlink-cli issue +list --state open --format json
|
--report <absolute-report-path> \
|
||||||
|
--require-review
|
||||||
# 2. 逐个分析 Issue 内容
|
|
||||||
gitlink-cli issue +view --id <issue_id> --format json
|
|
||||||
|
|
||||||
# 3. 根据内容智能分类
|
|
||||||
# 分析标题和描述后，通过 Raw API 打标签
|
|
||||||
gitlink-cli issue +update --number '{
|
|
||||||
"issue_tag_ids": [<tag_id>],
|
|
||||||
"done_ratio": 0,
|
|
||||||
"subject": "<原始标题>",
|
|
||||||
"description": "<原始描述>"
|
|
||||||
}'
|
|
||||||
```
|
```
|
||||||
|
|
||||||
**分类规则参考：**
|
多个 PR 必须分别包含实质性的 Review 建议草稿。校验器通过后才能交付报告并复用首屏摘要作为聊天输出。
|
||||||
|
|
||||||
| Issue 关键词 | 推荐标签 | 优先级 |
|
校验器会拒绝以下输出：
|
||||||
|-------------|----------|:------:|
|
|
||||||
| bug, 错误, 失败, crash, 崩溃 | bug | 🔴 High |
|
|
||||||
| feature, 新增, 建议, 希望 | enhancement | 🔵 Low |
|
|
||||||
| 安全, 漏洞, 权限, 泄露 | security | 🔴 High |
|
|
||||||
| 性能, 慢, 卡顿, 优化 | performance | 🟡 Medium |
|
|
||||||
| 文档, README, 注释 | documentation | 🔵 Low |
|
|
||||||
| question, 如何, 怎么, 请问 | question | 🟡 Medium |
|
|
||||||
| 测试, test, 覆盖率 | testing | 🔵 Low |
|
|
||||||
|
|
||||||
---
|
- `Review 建议` 只有醒目结论和状态，没有在同一字段中紧跟依据。
|
||||||
|
- 依据过短，或没有事实、证据、影响、验证和可执行下一步中的任何一项。
|
||||||
|
- Review 草稿仍使用旧的 `**建议：** 修改后再审` 格式。
|
||||||
|
- 模板占位符没有替换，或 PR 审查报告完全缺少 Review 建议。
|
||||||
|
|
||||||
## Raw API 参考
|
校验失败时必须修改报告并重新执行，直到退出码为 0；不能把未通过校验的 Markdown 路径返回给用户。Issue-only 或仓库健康度-only 模式可以省略 `--require-review`。
|
||||||
|
|
||||||
代码审查相关的 GitLink API 端点：
|
## 仓库健康扫描
|
||||||
|
|
||||||
```bash
|
仓库模式至少检查：
|
||||||
# 获取 PR 详情
|
|
||||||
gitlink-cli pr +view --id --format json
|
|
||||||
|
|
||||||
# 获取 PR 变更文件列表
|
- README、CONTRIBUTING、CHANGELOG、LICENSE 和安全政策。
|
||||||
gitlink-cli pr +files --format json
|
- CI、格式化、lint、静态检查和跨平台配置。
|
||||||
|
- 测试目录、关键模块覆盖、fixture 质量和失败路径测试。
|
||||||
|
- 依赖锁文件、已知风险、更新策略和供应链边界。
|
||||||
|
- 代码组织、重复热点、复杂模块和维护者可理解性。
|
||||||
|
- Issue 分类、响应状态和长期未处理风险。
|
||||||
|
|
||||||
# 获取 PR Diff
|
健康项使用 `RH-` 编号，标明检查范围、事实证据、影响和建议。不能仅根据文件是否存在给出高分；无法读取内容或执行工具时明确限制。
|
||||||
gitlink-cli pr +diff --format json
|
|
||||||
|
|
||||||
# 提交 PR Review
|
## 批量 Issue 分诊
|
||||||
gitlink-cli pr +review --body '{"body":"...","event":"COMMENT"}'
|
|
||||||
|
|
||||||
# 获取仓库文件列表
|
先把用户输入规范化为 `first_n_open`、`all_open` 或 `issue_ids`，并在报告中记录排序、翻页、去重和最终纳入数量。open 范围必须按真实 Issue 状态二次过滤；接口失败时保留已取得页并明确 `partial`，不能用历史样例补足数量。
|
||||||
gitlink-cli repo +files --query 'filepath=<path>&ref=<branch>'
|
|
||||||
|
|
||||||
# 获取仓库语言统计
|
扫描纳入范围内未分类、近期新增或长期未处理的 Issue，按以下维度建立 `IT-` 项：
|
||||||
gitlink-cli repo +languages --format json
|
|
||||||
|
|
||||||
# 获取贡献者列表
|
- 类型：bug、feature、documentation、question、performance、security 或 maintenance。
|
||||||
gitlink-cli repo +contributors --format json
|
- 优先级：`P0` 立即处置、`P1` 本轮处理、`P2` 计划处理、`P3` 可延后。
|
||||||
|
- 所属模块和影响范围。
|
||||||
|
- 复现信息、环境、日志和预期行为是否完整。
|
||||||
|
- 是否疑似重复、依赖其他事项或需要关联 PR。
|
||||||
|
- 当前等待作者、维护者、负责人还是平台。
|
||||||
|
- 推荐标签、负责人、下一动作和回复草稿。
|
||||||
|
|
||||||
# 获取仓库动态
|
P0/P1 必须有证据，安全问题避免在报告中复制利用细节或敏感值。默认只生成建议；标签、分配、回复、关闭和其他写操作必须经过人工审核和新的明确授权。
|
||||||
gitlink-cli repo +activity --format json
|
|
||||||
```
|
|
||||||
|
|
||||||
## 代码审查最佳实践
|
聊天摘要和报告中的 Issue 分诊均按以下形式输出：`级别（处置含义）：编号；本批事项概述；建议下一步`。概述必须来自本批 Issue 的标题、正文、标签、响应状态和等待方，不能复制通用定义冒充分析。相同原因可以合并描述，特殊的 P0/P1 单独指出。
|
||||||
|
|
||||||
### 审查原则
|
## 发现与严重性
|
||||||
|
|
||||||
1. **先大局后细节**：先理解 PR 的目的和整体变更范围，再逐文件审查
|
每条 `CR-` 发现必须包含严重性、事实类型、文件与行号或复现命令、触发条件、影响、证据、最小修复建议和验证限制。
|
||||||
2. **关注行为，而非风格**：自动化工具（linter/formatter）能处理的风格问题优先交给工具
|
|
||||||
3. **提供可操作的建议**：不只是指出问题，要给出具体的修改方案
|
|
||||||
4. **肯定好的代码**：发现好的设计、清晰的命名、完善的测试时给予正面反馈
|
|
||||||
5. **控制评论量**：避免信息过载——最严重的 3-5 个问题比 20 个小问题更有价值
|
|
||||||
|
|
||||||
### 安全红线
|
- `blocking`：漏洞、数据损坏、核心行为错误、明显回归，或核心声明完全无法验证。
|
||||||
|
- `high`：高概率影响真实用户、关键失败路径、Review 要求或重要兼容行为。
|
||||||
|
- `medium`：存在边界、测试或维护缺口，但没有证据表明立即阻断。
|
||||||
|
- `low`：不影响当前正确性的可选改进，合并展示并放入详细部分。
|
||||||
|
|
||||||
以下问题必须标记为 **Critical**，不得忽略：
|
没有精确证据的内容只能标记 `candidate`，不能升级为 blocking。纯格式偏好和可自动修复的低价值问题不得进入首屏。
|
||||||
|
|
||||||
- 硬编码的密钥 / Token / 密码
|
首屏按严重性给出 `级别含义 + 数量/编号 + 本批问题摘要`。数量为 0 时简要说明未发现该级别的已证实问题；数量大于 0 时至少概括最影响决策的一类问题，不能只输出 `blocking 0 | high 1`。
|
||||||
- SQL / NoSQL 注入漏洞
|
|
||||||
- 命令注入（shell 命令拼接）
|
|
||||||
- 路径遍历（用户输入直接用于文件路径）
|
|
||||||
- 不安全的反序列化
|
|
||||||
- XSS（未转义的用户输入直接渲染）
|
|
||||||
|
|
||||||
### 输出规范
|
## 单一 Markdown 报告顺序
|
||||||
|
|
||||||
- 始终使用 `--format json` 获取结构化数据
|
1. 首屏按 PR 分开的实质性 Review 参考、带依据门禁、代码发现说明、Issue 分级说明和最多五项动作。
|
||||||
- 审查报告输出为 **Markdown 格式**，便于直接粘贴到 PR 评论
|
2. PR 目标、贡献价值和变更概览。
|
||||||
- 涉及文件/行号时使用精准引用，方便定位
|
3. `RV-` Review 修改履约明细。
|
||||||
- 批量操作前使用 `--dry-run` 预检
|
4. 每个 open PR 独立的完整 `CR-` 代码审查、正向证据和可供人工审核的 Review 草稿。
|
||||||
|
5. 构建、测试、功能验证和验证限制。
|
||||||
|
6. `RH-` 仓库代码健康度。
|
||||||
|
7. `IT-` Issue 分诊详细结果。
|
||||||
|
8. 证据账本和附录。
|
||||||
|
|
||||||
## 注意事项
|
没有启用的模式在报告中注明“本次未请求”，不虚构结果。Issue 具体说明始终位于 PR、验证和健康度内容之后。
|
||||||
|
|
||||||
- PR Review 提交后会通知所有关注该 PR 的参与者，评论内容请保持专业
|
## 完成前自检
|
||||||
- `pr +diff` 输出可能很大（大型 PR），Agent 应分段处理
|
|
||||||
- API 的 PR files 和 diff 接口有频率限制，避免短时间内重复请求
|
- 聊天回复和 Markdown 首屏是否都按 PR 分节，并完整保留七方面结论前置判断卡。
|
||||||
- 对于 draft PR（草稿），应提示用户先将其标记为 Ready for Review
|
- 每张卡是否在醒目结论后紧跟简短解释、明确 `依据：` 和影响/下一步；无论结论为何都没有只写状态。
|
||||||
|
- 是否完整分析实际相关维度，而不是机械限制为五项。
|
||||||
|
- 是否区分完整 PR Diff 与 Review 后增量 Diff。
|
||||||
|
- 每条有效 Review 是否有 `RV-` 状态、证据和剩余动作。
|
||||||
|
- Review 建议和草稿是否只写入报告，没有提交远端。
|
||||||
|
- PR 审查报告是否同时通过 `validate_review_report.py --require-review` 和 `validate_pr_cards.py` 七方面校验。
|
||||||
|
- 仓库健康扫描和 Issue 分诊是否在请求时保留，Issue 范围是否明确为前 N 条、全部 open 或指定编号，Issue 详情是否位于报告最后。
|
||||||
|
- blocking/high 是否有可复现证据，未执行测试是否标为 `not_run`。
|
||||||
|
- Issue 的聊天直接输出是否解释 P0/P1/P2/P3 的处置含义、对应编号、本批问题概述和下一步，而不是只列级别与编号。
|
||||||
|
- 是否生成一份 UTF-8 Markdown，并在最终回复中给出审查结论、Issue 分诊说明、Review 草稿状态和绝对路径。
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,4 @@
|
||||||
|
interface:
|
||||||
|
display_name: "社区智能审查"
|
||||||
|
short_description: "验证 PR、Review 修改、仓库健康和 Issue 分诊，生成决策优先报告。"
|
||||||
|
default_prompt: "使用 $gitlink-code-review 审查指定 GitLink PR；聊天和 Markdown 均按 PR 分节，将 Review 建议、贡献价值、Review 履约、实现与逻辑、测试、安全和关键发现分别做成结论前置判断卡，后接依据与影响；Issue 分级另行说明，不回写远端。"
|
||||||
|
|
@ -0,0 +1,41 @@
|
||||||
|
# 证据优先的代码审查示例
|
||||||
|
|
||||||
|
这个示例用于演示一次可复查的 PR 深审，不自动发布 Review。
|
||||||
|
|
||||||
|
## 采集
|
||||||
|
|
||||||
|
```powershell
|
||||||
|
$context = gitlink-cli workflow +review-context `
|
||||||
|
--owner Gitlink --repo gitlink-cli --number 123 `
|
||||||
|
--include-commits=true --include-ci=true --format json
|
||||||
|
$context | Set-Content .\pr-123-context.json -Encoding utf8
|
||||||
|
```
|
||||||
|
|
||||||
|
先记录 `run_id`、PR head SHA、`sections`、`notes` 和 `ci_summary`。如果 CI 没有按 SHA 或分支关联，或 `notes` 表示探针失败，报告中的 CI 门禁只能是 `partial`/`not_run`。
|
||||||
|
|
||||||
|
## 审查顺序
|
||||||
|
|
||||||
|
1. 从标题、正文和测试说明提取作者声明，不把标题当作事实。
|
||||||
|
2. 逐文件检查行为变化、错误处理、输入边界、资源释放、权限和敏感数据流。
|
||||||
|
3. 对每条发现记录 `CR-` 编号、直接证据、触发条件、影响和最小修复建议。
|
||||||
|
4. 区分 `observed`、`derived` 和 `unknown`；无法精确定位的问题只能标为 `candidate`。
|
||||||
|
5. 只在当前 head 的构建、测试和安全证据完整时给出较高置信度。
|
||||||
|
|
||||||
|
## 首屏输出
|
||||||
|
|
||||||
|
```markdown
|
||||||
|
## PR #123
|
||||||
|
**Review 建议：** <span style="color:#B54708"><strong>修改后再审</strong></span> **[action_required]**：失败路径和错误输出仍可能影响真实用户；依据：CR-001、CR-002 以及缺失的脱敏测试；下一步：修复并按当前 head 复验。
|
||||||
|
**贡献价值：** <span style="color:#067647"><strong>目标问题真实且增量明确</strong></span> **[passed]**：新增能力填补默认分支缺口；依据：Issue、baseline Diff 和受益范围；影响：减少维护者人工步骤。
|
||||||
|
**Review 履约：** <span style="color:#067647"><strong>既有意见已经满足</strong></span> **[passed]**：作者修复了错误映射并补正常路径测试；依据：Review 后提交、当前代码和 RV-001；影响：没有遗留 Review 阻断。
|
||||||
|
**实现与逻辑：** <span style="color:#067647"><strong>主流程行为正确</strong></span> **[passed]**：当前 head 的核心路径运行成功；依据：CI 按 SHA 匹配 `1/1` 和复现命令；影响：声明功能可用。
|
||||||
|
**测试：** <span style="color:#B54708"><strong>失败路径覆盖不完整</strong></span> **[partial]**：异常输入没有回归用例；依据：测试文件和测试清单；下一步：补失败与兼容测试。
|
||||||
|
**安全：** <span style="color:#B54708"><strong>脱敏行为尚未证明</strong></span> **[partial]**：Diff 可定位敏感输出风险；依据：错误路径和缺失的脱敏断言；下一步：验证日志不泄露敏感值。
|
||||||
|
**关键发现：** <span style="color:#B54708"><strong>2 项问题需要处理</strong></span> **[high]**：失败路径和敏感输出会影响合并判断；依据：CR-001、CR-002；下一步：修复后按同一 head 复验。
|
||||||
|
|
||||||
|
## 先做这 2 件事
|
||||||
|
1. **[CR-001][high] 补充** 失败路径测试（责任：作者；证据：`E-CR-001`）。
|
||||||
|
2. **[CR-002][medium] 复看** 错误输出中的敏感字段脱敏（责任：作者；证据：`diff:internal/client/client.go:42`）。
|
||||||
|
```
|
||||||
|
|
||||||
|
完整 Diff、命令输出、未匹配构建和正向反馈放入附录。本 Skill 只生成报告、整体 Review 草稿和内联评论草稿，不发布 `COMMENT`、`APPROVE` 或 `MERGE`；维护者人工审核后可在独立操作中决定是否发布。
|
||||||
|
|
@ -0,0 +1,41 @@
|
||||||
|
# 轻量 PR 审查示例
|
||||||
|
|
||||||
|
这个示例展示维护者默认看到的摘要，而不是完整审查记录。完整 diff 和命令输出放在附录。
|
||||||
|
|
||||||
|
```bash
|
||||||
|
gitlink-cli pr +view --owner Gitlink --repo gitlink-cli --id 123 --format json
|
||||||
|
gitlink-cli pr +files --owner Gitlink --repo gitlink-cli --id 123 --format json
|
||||||
|
gitlink-cli pr +diff --owner Gitlink --repo gitlink-cli --id 123 --format json
|
||||||
|
gitlink-cli pr +reviews --owner Gitlink --repo gitlink-cli --id 123 --format json
|
||||||
|
```
|
||||||
|
|
||||||
|
```markdown
|
||||||
|
## PR #123
|
||||||
|
**Review 建议：** <span style="color:#B54708"><strong>修改后再审</strong></span> **[action_required]**：批量操作入口和帮助文档已经形成完整主流程，但输入边界可能产生不可诊断错误或跨平台回归；依据：无权限请求、恶意路径和 Windows 中文错误输出尚未验证；下一步：补齐实现和测试后复看。
|
||||||
|
**贡献价值：** <span style="color:#067647"><strong>功能增量成立</strong></span> **[passed]**：PR 补齐高频批量操作，默认分支没有等价入口；依据：需求、Diff、帮助文本和受益范围；影响：减少重复人工操作。
|
||||||
|
**Review 履约：** <span style="color:#175CD3"><strong>本轮没有待核对意见</strong></span> **[not_applicable]**：未发现有效 Review；依据：Review 列表与当前 head SHA；影响：本轮只评价完整 Diff。
|
||||||
|
**实现与逻辑：** <span style="color:#B54708"><strong>正常路径可用但边界不完整</strong></span> **[partial]**：权限失败和恶意路径没有可靠处理证据；依据：实现分支与复现清单；下一步：补错误映射和输入校验。
|
||||||
|
**测试：** <span style="color:#B54708"><strong>关键失败路径缺测</strong></span> **[partial]**：现有测试只覆盖成功流程；依据：测试文件和执行结果；下一步：补权限、非法路径和 Windows UTF-8 回归。
|
||||||
|
**安全：** <span style="color:#B54708"><strong>输入边界未完整验证</strong></span> **[partial]**：改动触及路径和权限输入；依据：Diff 与安全矩阵仅覆盖部分场景；下一步：补恶意输入和无权限测试。
|
||||||
|
**关键发现：** <span style="color:#B54708"><strong>1 项 high 与 2 项 medium 待处理</strong></span> **[high]**：问题集中在权限失败和跨平台边界；依据：CR-001 至 CR-003；下一步：先修 high 再完成编码回归。
|
||||||
|
|
||||||
|
**代码发现：**
|
||||||
|
- **blocking（阻止合并）：0 条。** 未发现已证实的漏洞或数据破坏。
|
||||||
|
- **high（本轮必须修复）：1 条，CR-001。** 权限失败路径缺失，可能让无权限请求得到错误结果。
|
||||||
|
- **medium（应补齐后复看）：2 条，CR-002、CR-003。** Windows 中文错误和 API 回滚行为没有验证。
|
||||||
|
- **low（可延后优化）：0 条。**
|
||||||
|
|
||||||
|
## 先做这 3 件事
|
||||||
|
1. **[CR-001][high] 补充** 恶意路径和无权限请求测试（责任：作者）。
|
||||||
|
2. **[CR-002][medium] 验证** Windows PowerShell 下的中文错误输出（责任：作者）。
|
||||||
|
3. **[CR-003][medium] 复看** API 失败时的回滚行为（责任：维护者）。
|
||||||
|
|
||||||
|
### PR #123 Review 建议草稿（未提交）
|
||||||
|
**Review 建议：** <span style="color:#B54708"><strong>修改后再审</strong></span> **[action_required]**：批量操作入口和帮助文档已经形成完整主流程，但合并前需要补齐无权限请求、恶意路径和 Windows 中文错误输出，这些未验证边界会影响错误诊断和跨平台可用性。请在 `shortcuts/example/example.go:42` 保留当前正常路径，同时对无权限响应返回可诊断错误，并新增失败路径与 Windows UTF-8 回归测试。完成后运行 `go test ./shortcuts/example -count=1`，确认成功、无权限和非法路径三类场景均通过，再提交复看。
|
||||||
|
```
|
||||||
|
|
||||||
|
## 关键验证
|
||||||
|
|
||||||
|
- 正常路径、失败路径、兼容路径至少各一条。
|
||||||
|
- 触及 token、权限、命令、文件路径、外部 URL 或依赖时，执行共享安全矩阵对应检查。
|
||||||
|
- 报告落盘后确认 Markdown 为 UTF-8；JSON 可解析且没有 ANSI、HTML 或敏感值。
|
||||||
|
|
@ -60,15 +60,26 @@ gitlink-cli pr +files --id 42 --format json
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
gitlink-cli pr +diff --id 42 --format json
|
gitlink-cli pr +diff --id 42 --format json
|
||||||
|
gitlink-cli pr +reviews --id 42 --format json
|
||||||
```
|
```
|
||||||
|
|
||||||
### Step 4：逐文件审查
|
### Step 4：验证 Review 修改并逐文件审查
|
||||||
|
|
||||||
|
如果 PR 已有 Review，先把每条可执行意见映射到 Review 后的提交和当前代码，标记为 `resolved`、`partially_resolved`、`unresolved`、`regressed`、`outdated` 或 `not_verifiable`。代码发生变化本身不能证明意见已经解决。
|
||||||
|
|
||||||
对每个变更文件，分析代码质量。以下是审查结果示例：
|
对每个变更文件，分析代码质量。以下是审查结果示例：
|
||||||
|
|
||||||
```markdown
|
```markdown
|
||||||
## PR #42 代码审查报告
|
## PR #42 代码审查报告
|
||||||
|
|
||||||
|
**Review 建议：** <span style="color:#B42318"><strong>修复安全阻断后再审</strong></span> **[action_required]**：认证实现存在硬编码凭据和 SQL 注入风险；依据：CR-42-001、CR-42-002 与精确代码位置；下一步：完成参数化查询、密钥外置和安全回归后复看。
|
||||||
|
**贡献价值：** <span style="color:#067647"><strong>认证能力具有实际价值</strong></span> **[passed]**：PR 提供登录和 Token 流程；依据：需求、模块 Diff 和主要使用路径；影响：形成可用的认证入口。
|
||||||
|
**Review 履约：** <span style="color:#175CD3"><strong>没有既有意见可核对</strong></span> **[not_applicable]**：本轮没有有效 Review；依据：Review 列表和当前 head；影响：直接审查完整实现。
|
||||||
|
**实现与逻辑：** <span style="color:#B42318"><strong>认证边界不安全</strong></span> **[failed]**：查询直接拼接输入且密码处理不正确；依据：`src/auth/login.py:42`、`src/auth/login.py:88`；下一步：参数化查询并使用安全哈希。
|
||||||
|
**测试：** <span style="color:#B54708"><strong>安全与边界用例不足</strong></span> **[partial]**：已有测试覆盖主要成功路径；依据：`tests/test_auth.py` 和测试清单；下一步：补注入、空值、超长输入和过期 Token。
|
||||||
|
**安全：** <span style="color:#B42318"><strong>存在两个阻断级风险</strong></span> **[failed]**：硬编码密钥和 SQL 注入可被直接触发；依据：`src/config.py:15`、`src/auth/login.py:42`；下一步：移除凭据并使用参数化 API。
|
||||||
|
**关键发现：** <span style="color:#B42318"><strong>2 项 blocking 必须先修复</strong></span> **[blocking]**：CR-42-001、CR-42-002 会影响数据和凭据安全；依据：代码证据和攻击路径；下一步：阻断合并直到安全测试通过。
|
||||||
|
|
||||||
### 🔴 Critical
|
### 🔴 Critical
|
||||||
|
|
||||||
1. **JWT Secret 硬编码** — `src/config.py:15`
|
1. **JWT Secret 硬编码** — `src/config.py:15`
|
||||||
|
|
@ -113,15 +124,9 @@ gitlink-cli pr +diff --id 42 --format json
|
||||||
- 有类型注解，代码可读性好
|
- 有类型注解，代码可读性好
|
||||||
```
|
```
|
||||||
|
|
||||||
### Step 5：提交 Review
|
### Step 5：生成待人工审核的 Review 草稿
|
||||||
|
|
||||||
```bash
|
在 Markdown 报告中生成整体 Review 和必要的内联评论草稿，不调用 `pr +review` 或任何远端写接口。维护者审核、编辑并明确决定发布后，再在本次 Skill 之外执行提交。
|
||||||
# 提交整体 Review 评论
|
|
||||||
gitlink-cli pr +review --id 42 --owner Gitlink --repo forgeplus --body '{
|
|
||||||
"body": "## PR #42 代码审查报告\n\n### 🔴 Critical\n\n1. **JWT Secret 硬编码** — `src/config.py:15`\n JWT_SECRET 硬编码在源码中。建议使用 `os.getenv(\"JWT_SECRET\")`。\n\n2. **SQL 注入风险** — `src/auth/login.py:42`\n 直接拼接用户输入到 SQL 查询。建议使用参数化查询。\n\n### 🟡 Warning\n\n1. **密码明文存储** — 建议使用 bcrypt 哈希处理。\n\n### 总体评价\n\n代码整体结构清晰，测试覆盖良好。建议修复 Critical 问题后合并。",
|
|
||||||
"event": "COMMENT"
|
|
||||||
}'
|
|
||||||
```
|
|
||||||
|
|
||||||
### Step 6：输出审查摘要
|
### Step 6：输出审查摘要
|
||||||
|
|
||||||
|
|
@ -159,6 +164,8 @@ gitlink-cli pr +files --id <id> --format json
|
||||||
# 获取 Diff
|
# 获取 Diff
|
||||||
gitlink-cli pr +diff --id <id> --format json
|
gitlink-cli pr +diff --id <id> --format json
|
||||||
|
|
||||||
# 提交 Review
|
# 获取已有 Review，用于验证后续修改
|
||||||
gitlink-cli pr +review --body '{"body":"...","event":"COMMENT"}'
|
gitlink-cli pr +reviews --id <id> --format json
|
||||||
```
|
```
|
||||||
|
|
||||||
|
本工作流只生成报告和 Review 草稿，不提交远端。
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,15 @@
|
||||||
|
# PR 安全审查矩阵
|
||||||
|
|
||||||
|
代码审查必须先根据改动文件和数据流判断安全面是否命中；没有执行验证时使用 `not_run`，不能写成安全通过。
|
||||||
|
|
||||||
|
| 类别 | 重点信号 | 最低验证 | 默认级别 |
|
||||||
|
| --- | --- | --- | --- |
|
||||||
|
| 凭据泄露 | token、密码、私钥、`.env`、日志回显 | 扫描 diff、fixture 和日志，确认脱敏 | blocking |
|
||||||
|
| 命令注入 | shell 拼接、`exec`、用户输入进入命令 | 使用引号、空格和 shell 元字符输入 | blocking |
|
||||||
|
| 路径遍历 | 文件名、下载地址或压缩包来自外部 | 验证 `..`、绝对路径、符号链接和跨平台分隔符 | high |
|
||||||
|
| 注入与外连 | SQL、模板、Markdown、HTML、JSON、URL、webhook | 覆盖边界输入、转义、协议与内网地址 | high |
|
||||||
|
| 认证授权 | token 作用域、项目权限、管理员动作 | 覆盖未登录、无权、越权和过期 token | blocking |
|
||||||
|
| 依赖与资源 | 新增依赖、安装脚本、分页、并发和重试 | 锁定来源，覆盖最大数据、超时和取消 | high |
|
||||||
|
| 敏感输出 | 报告、错误、JSON、缓存或调试日志 | 确认不输出真实凭据或用户敏感信息 | high |
|
||||||
|
|
||||||
|
每个命中的安全项至少记录类别、状态、证据、验证命令和责任归属。关键词命中只能形成候选，不能单独证明漏洞；发现疑似真实密钥时只写类型、位置和轮换建议，不复制内容。
|
||||||
|
|
@ -0,0 +1,75 @@
|
||||||
|
#!/usr/bin/env python3
|
||||||
|
"""Validate that a code-review report gives evidence-backed Review advice."""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import argparse
|
||||||
|
import re
|
||||||
|
import sys
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
|
||||||
|
REVIEW_PREFIX = "**Review 建议：**"
|
||||||
|
LEGACY_PREFIX = "**建议：**"
|
||||||
|
REVIEW_PATTERN = re.compile(
|
||||||
|
r"^\*\*Review 建议：\*\*\s*"
|
||||||
|
r"<span\b[^>]*><strong>([^<]+)</strong></span>\s*"
|
||||||
|
r"\*\*\[([^\]]+)\]\*\*：\s*(\S.*)$"
|
||||||
|
)
|
||||||
|
REVIEW_DRAFT_HEADING = re.compile(r"^### PR #\d+ Review 建议草稿（未提交）\s*$")
|
||||||
|
EVIDENCE_MARKERS = ("依据", "测试", "Diff", "文件", "行号", "CR-", "RV-", "实现", "影响", "主线", "API", "命令", "已核验", "未发现", "通过", "失败", "缺少", "需要")
|
||||||
|
PLACEHOLDER_MARKERS = ("<结论>", "<具体", "<status-", "<decision_")
|
||||||
|
|
||||||
|
|
||||||
|
def validate_report(text: str, require_review: bool = False) -> list[str]:
|
||||||
|
errors: list[str] = []
|
||||||
|
review_lines: list[int] = []
|
||||||
|
lines = text.splitlines()
|
||||||
|
for line_number, line in enumerate(lines, start=1):
|
||||||
|
stripped = line.strip()
|
||||||
|
if stripped.startswith(LEGACY_PREFIX):
|
||||||
|
errors.append(f"line {line_number}: legacy conclusion-only field is not allowed")
|
||||||
|
if not stripped.startswith(REVIEW_PREFIX):
|
||||||
|
continue
|
||||||
|
review_lines.append(line_number)
|
||||||
|
match = REVIEW_PATTERN.match(stripped)
|
||||||
|
if not match:
|
||||||
|
errors.append(f"line {line_number}: Review decision requires a status tag and rationale")
|
||||||
|
continue
|
||||||
|
conclusion, status, rationale = match.groups()
|
||||||
|
if not conclusion.strip() or not status.strip() or len(rationale.strip()) < 30:
|
||||||
|
errors.append(f"line {line_number}: Review rationale is too short")
|
||||||
|
if not any(marker in rationale for marker in EVIDENCE_MARKERS):
|
||||||
|
errors.append(f"line {line_number}: Review rationale lacks evidence or an actionable next step")
|
||||||
|
if any(marker in rationale for marker in PLACEHOLDER_MARKERS):
|
||||||
|
errors.append(f"line {line_number}: unresolved template placeholder in rationale")
|
||||||
|
for index, line in enumerate(lines):
|
||||||
|
if REVIEW_DRAFT_HEADING.match(line.strip()) and not any(candidate.strip().startswith(REVIEW_PREFIX) for candidate in lines[index + 1:index + 7]):
|
||||||
|
errors.append(f"line {index + 1}: Review draft must start with a substantive Review suggestion")
|
||||||
|
if require_review and not review_lines:
|
||||||
|
errors.append("PR review mode requires at least one substantive Review decision")
|
||||||
|
return errors
|
||||||
|
|
||||||
|
|
||||||
|
def main() -> int:
|
||||||
|
parser = argparse.ArgumentParser(description="Validate substantive Review decisions.")
|
||||||
|
parser.add_argument("--report", required=True, type=Path)
|
||||||
|
parser.add_argument("--require-review", action="store_true")
|
||||||
|
args = parser.parse_args()
|
||||||
|
try:
|
||||||
|
text = args.report.read_text(encoding="utf-8", errors="strict")
|
||||||
|
except (OSError, UnicodeError) as exc:
|
||||||
|
print(f"review report validation failed: {exc}", file=sys.stderr)
|
||||||
|
return 2
|
||||||
|
errors = validate_report(text, args.require_review)
|
||||||
|
if errors:
|
||||||
|
print("review report validation failed:", file=sys.stderr)
|
||||||
|
for error in errors:
|
||||||
|
print(f"- {error}", file=sys.stderr)
|
||||||
|
return 1
|
||||||
|
print("review report validation passed")
|
||||||
|
return 0
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
raise SystemExit(main())
|
||||||
Loading…
Reference in New Issue