| name | 1k-code-review-pr |
| description | Comprehensive PR code review for OneKey monorepo. Use when reviewing PRs, code changes, or diffs — covers security (secrets/PII leakage, supply-chain, AuthN/AuthZ), code quality (hooks, race conditions, null safety, concurrent requests), and OneKey-specific patterns (Fabric crashes, MIUI, BigNumber). Triggers on "review PR", "review this PR", "code review", "check this diff", "审查 PR", "代码审查", "review |
| allowed-tools | Read, Grep, Glob, Bash, WebFetch |
OneKey PR Code Review
输出语言: 中文
Review Scope
- Base branch:
x
- Diff:
git fetch origin && git diff origin/x...HEAD (triple-dot)
Workflow
- Checkout —
gh pr checkout <PR_NUMBER> (skip if already on branch)
- Scope —
git diff origin/x...HEAD --stat to see change scope
- Triage — Determine which review modules apply (see triage table)
- Primary Review — Read each changed file, apply relevant checks from
references/
- Codex Cross-Review — If Codex available, run full parallel review (see below)
- PR Comment Analysis — Fetch all existing PR comments (bot + human), analyze with local codebase context (see below)
- Merge Findings — Combine primary + Codex + PR comment findings, deduplicate, annotate confidence
- Score — Rate the PR across 4 dimensions (see Scoring System). This step is MANDATORY — every report MUST include the scoring table.
- Report — Generate structured report using the unified format. Follow the template exactly — every section is required.
- GH Comment — For Blocker issues, offer to post inline PR comments (with confirmation)
Codex Cross-Review Integration
Check if Codex is available by confirming the codex:codex-rescue subagent type can be dispatched. If uncertain, invoke /codex:setup to check readiness.
If available:
- Dispatch a full independent review to Codex via
Agent(subagent_type="codex:codex-rescue"):
Agent(
subagent_type = "codex:codex-rescue",
prompt = "Review this PR diff for the OneKey crypto wallet monorepo. Focus on:
- Security vulnerabilities (secret leakage, auth bypass, supply-chain risks)
- Runtime bugs (race conditions, null safety, memory leaks)
- Architecture violations (import hierarchy, cross-platform issues)
- Code quality (hooks safety, error handling, performance)
Report each finding with: file:line, severity (Critical/High/Medium/Low), description, fix suggestion.
Diff:
${FULL_DIFF}"
)
- Parse Agent result for structured findings
- Merge into primary review:
- Both found same issue → Mark
{Cross-validated ✅}, auto-promote to 🔵 High confidence
- Codex-only finding → Include with tag
[Codex], review manually to assign confidence
- Primary-only finding → Include normally
- Add a Codex 交叉验证摘要 table in the report (see report template)
If unavailable: Skip silently. Set "Codex 交叉验证: ⏭️ 未启用" in the report header. Do NOT mention Codex anywhere else.
PR Comment Analysis
Collect ALL existing comments on the PR — bot and human — then analyze each with your local codebase context. You have full source access, type system, and dependency graph; most commenters only saw the diff. Use this asymmetry.
Fetching All Comments
Use gh api to get full user metadata (including type field for bot detection):
gh api repos/{owner}/{repo}/pulls/{pr_number}/reviews \
--jq '[.[] | select(.body != "") | {author: .user.login, is_bot: (.user.type == "Bot"), body: .body, state: .state, association: .author_association}]'
gh api repos/{owner}/{repo}/pulls/{pr_number}/comments \
--jq '[.[] | {author: .user.login, is_bot: (.user.type == "Bot"), path: .path, line: .line, body: .body, association: .author_association}]'
gh api repos/{owner}/{repo}/issues/{pr_number}/comments \
--jq '[.[] | {author: .user.login, is_bot: (.user.type == "Bot"), body: .body, association: .author_association}]'
Bot detection — use the user.type == "Bot" field from GitHub API, not hardcoded username lists. This automatically covers any bot (current and future) without maintenance.
If no comments exist, set "PR 评论分析: ⏭️ 无评论" in the report header and skip this section.
Analysis Framework
For each substantive comment (skip empty approvals, CI status badges, pure formatting):
| Verdict | Meaning | Action |
|---|
| ✅ Confirmed | Comment identifies a real issue | Include in findings, tag source [<author>] |
| 🔍 Enriched | Real issue, but analysis is shallow or fix is wrong | Include with deeper fix guidance from your codebase knowledge |
| ❌ Noise | Not an issue given full codebase context | Note in "评论误报分析" with brief explanation of why |
| 📋 Already Covered | Your primary review caught it | Cross-validate, boost confidence |
Your local advantages — use them aggressively:
- Full source — trace data flow across files, not just the diff
- Type system — run
tsc, verify types end-to-end
- Architecture — you know OneKey's import hierarchy and platform patterns
- Dependencies —
yarn info, changelogs, actual vulnerability reachability
- Runtime reasoning — state flows, async lifecycles, race conditions
When someone flags something vague, dig into the source to confirm or refute. When a comment misses context (e.g., a function is safely guarded upstream), explain why. When a comment is right, amplify with richer context.
Cross-Validation Rules
- Comment + primary review agree → Mark
{Cross-validated ✅}, promote to 🔵 High
- Comment-only finding you confirm → Include at appropriate confidence with
[<author>] tag
- Comment-only finding you can't confirm or refute → Include as ⚪ Low with note
- Comment you refute with evidence → Add to "评论误报分析" section
Security Comment Special Handling
For security-related comments (from bots like Snyk/Dependabot or from human reviewers):
- Vulnerability reports — check if the vulnerable code path is actually reachable in OneKey's usage
- License issues — verify against OneKey's license policy
- Dependency alerts — check if the flagged version is actually used (not just in lockfile)
Triage: Which Checks to Run
Run git diff origin/x...HEAD --name-only and match:
| Changed Files Match | Load |
|---|
package.json, lockfiles, node_modules patches, patches/*.patch | [security-and-supply-chain.md] — full supply-chain review |
**/auth/**, **/vault/**, **/signing/**, **/crypto/**, manifest.json, **/manifest/*.js | [security-and-supply-chain.md] — full security review |
Any .ts/.tsx with business logic | [code-quality-patterns.md] — hooks, race conditions, null safety |
.android.ts(x), .ios.ts(x), .native.ts(x), .desktop.ts(x), .ext.ts(x), .web.ts(x), native modules, BigNumber usage | [onekey-platform-patterns.md] — platform crashes & numeric safety |
Shell scripts (.sh), CI workflows (.yml) | [onekey-platform-patterns.md] — build & CI section |
Always check regardless of file type:
- Accidental file commits (
.DS_Store, .env, node_modules)
- Import hierarchy violations (see below)
- Avoidable reimplementation when stable existing helpers, hooks, services, or components already cover the same behavior (see below)
- PR description matches actual changes
- Run relevant commands from [quick-commands.md]
Import Hierarchy (ALWAYS verify)
@onekeyhq/shared <- FORBIDDEN to import from other OneKey packages
↓
@onekeyhq/components <- ONLY imports shared
↓
@onekeyhq/core <- ONLY imports shared
↓
@onekeyhq/kit-bg <- imports shared, core (NEVER components or kit)
↓
@onekeyhq/kit <- imports shared, components, kit-bg
↓
apps/* <- imports all
git diff origin/x...HEAD --name-only | grep -E '\.tsx?$' | \
while IFS= read -r f; do [ -f "$f" ] && grep -l "from.*@onekeyhq" "$f" 2>/dev/null; done | \
while IFS= read -r f; do echo "=== $f ==="; grep "from.*@onekeyhq" "$f"; done
Existing Implementation Reuse (ALWAYS verify)
For newly added helpers, hooks, services, components, constants, formatters, validators, or business logic:
- Search the codebase for existing implementations with the same intent before accepting the new code.
- Prefer established OneKey APIs, components, hooks, utilities, and service methods over local reimplementation.
- Flag reimplementation only when the existing abstraction is semantically equivalent, stable, and does not introduce worse coupling.
- In each reuse finding, include the existing file/function to reuse and explain why it covers the new behavior.
File Risk Classification
| Risk | Patterns | Action |
|---|
| Critical | **/vault/**, **/signing/**, **/crypto/**, **/core/src/**, hardware wallet SDK | Line-by-line review |
| High | **/auth/**, API endpoints, state management, package.json, manifest.json | Deep review |
| Medium | UI components, platform-specific code, background services | Standard review |
| Low | Comments, type-only, formatting, tests, docs | Scan for anomalies |
Scoring System
MANDATORY — every report must include this scoring table, no exceptions.
Rate the PR on 4 dimensions (1-10 each):
| Dimension | Weight | What to evaluate |
|---|
| 🔒 安全性 | 35% | Secret leakage, auth bypass, supply-chain risk, input validation |
| 💎 代码质量 | 30% | Hooks safety, error handling, race conditions, null safety, DRY, reuse of existing implementations |
| 🏛️ 架构合理性 | 20% | Import hierarchy, separation of concerns, cross-platform consistency |
| ✅ 完整性 | 15% | Edge cases handled, test coverage, migration paths, docs |
Total Score = weighted average, rounded to 1 decimal.
| Score | Verdict | Action |
|---|
| 8.0 - 10.0 | ✅ 可直接合入 | No blockers, minor suggestions only |
| 5.0 - 7.9 | ⚠️ 需修改后复审 | Has issues that should be fixed before merge |
| < 5.0 | ❌ 建议打回重做 | Fundamental issues in security or architecture |
Scoring anchors — to keep scores consistent:
- Start at 8 for each dimension, deduct for issues found
- A single Critical security issue → Security capped at 3
- A single High bug → Code Quality capped at 5
- Import hierarchy violation → Architecture capped at 4
Confidence Levels
MANDATORY — every finding must use exactly one of these three emoji tags. Do NOT use percentages, do NOT use plain text like "高/中/低" without the emoji. Always use this exact format:
| Tag | Meaning | When to use |
|---|
| 🔵 High | Confirmed, verifiable from code | Clear bug, obvious violation, reproducible |
| 🟠 Medium | Likely issue, needs context | Pattern suggests problem, might be intentional |
| ⚪ Low | Possible issue, needs human check | Heuristic match, depends on business logic |
Cross-validated findings (primary + Codex agree, or primary + PR comment agree) → automatically 🔵 High.
Auto-Fix Patches
MANDATORY for these categories — if a finding matches one of these, you MUST include a diff patch:
console.error/warn/log → project logger (defaultLogger)- Missing optional chaining on nullable refs
- Import hierarchy violations
- Missing cleanup in useEffect
- BigNumber type coercion (
Number(decimals))
- Missing type in union type definitions
Format — always use this exact structure:
**Auto-fix:**
\```diff
- old code
+ new code
\```
For other findings where the fix is unambiguous and doesn't require business context, also include auto-fix. When in doubt, include it — it's more useful to have a suggested fix than not.
Do NOT generate auto-fix for:
- Logic changes requiring business context understanding
- Security fixes needing architectural decisions
- Performance optimizations with tradeoffs
GH CLI Inline Comments
After generating the report, if there are findings that meet the comment threshold:
Comment threshold: 🔴 高 priority (any confidence) OR 🟡 中 priority with 🔵 High confidence. This means:
- All 🔴 高 findings (regardless of confidence)
- All 🟡 中 findings with 🔵 High confidence (cross-validated or confirmed from code)
- Excludes: 🟢 低 findings, and 🟡 中 with 🟠 Medium or ⚪ Low confidence
- List the qualifying findings that warrant PR comments
- Ask the reviewer: "以下问题建议直接评论到 PR 上，是否确认？"
- Only after explicit yes, post via:
gh api repos/{owner}/{repo}/pulls/{pr_number}/comments \
--field body="🟡 **问题标题**: 描述...
**建议修复:**
\`\`\`suggestion
修复代码
\`\`\`
_— Auto-review by Claude_" \
--field path="path/to/file.tsx" \
--field line=42 \
--field side="RIGHT" \
--field commit_id="$(git rev-parse HEAD)"
Rules:
- Never post without explicit reviewer confirmation
- Only post findings meeting the comment threshold (see above)
- Include auto-fix in
suggestion block when available
- Maximum 5 inline comments per PR
Unified Report Format
CRITICAL: Follow this template exactly. Every section marked [REQUIRED] must appear in every report. Do not skip or reorder sections.
# PR #NUMBER 代码审查报告
## 审查概要 [REQUIRED]
- **变更范围**: X 个文件, +Y / -Z 行
- **风险等级**: Critical / High / Medium / Low
- **涉及平台**: Extension / Mobile / Desktop / Web
- **Codex 交叉验证**: ✅ 已启用 / ⏭️ 未启用
- **PR 评论分析**: ✅ 已分析 (N 条评论, 其中 M 条来自 Bot) / ⏭️ 无评论
## 评分 [REQUIRED — NEVER SKIP THIS SECTION]
| 维度 | 得分 | 说明 |
|------|------|------|
| 🔒 安全性 | X/10 | 简要说明 |
| 💎 代码质量 | X/10 | 简要说明 |
| 🏛️ 架构合理性 | X/10 | 简要说明 |
| ✅ 完整性 | X/10 | 简要说明 |
| **总分** | **X.X/10** | **✅ 可直接合入 / ⚠️ 需修改后复审 / ❌ 建议打回** |
## Codex 交叉验证摘要 [REQUIRED if Codex was used, OMIT if not]
| 发现 | Primary | Codex | 状态 |
|------|---------|-------|------|
| 问题描述 | Yes/No | Yes/No | 交叉验证 / 仅 Primary / 仅 Codex |
## PR 评论分析 [REQUIRED if comments exist, OMIT if none]
| 来源 | 类型 | 发现 | 判定 | 说明 |
|------|------|------|------|------|
| Snyk | 🤖 Bot | 依赖漏洞 CVE-XXXX | ✅ Confirmed | 漏洞路径在 OneKey 中可达 |
| @reviewer | 👤 Human | 缺少 null check | 🔍 Enriched | 实际需要在上游 hook 中处理 |
| Devin | 🤖 Bot | 变量命名建议 | ❌ Noise | 命名符合项目规范 |
### 评论误报分析 [OMIT if no noise findings]
- **[来源] 误报**: 具体说明为什么这不是问题（引用源码上下文）
## 发现的问题 [REQUIRED]
### [🔴 高] [🔵 High] 问题标题 {Cross-validated ✅}
**文件**: `path/to/file.tsx:42`
**类型**: 安全 / 构建 / 运行时 / 性能 / 规范
**描述**: 问题是什么，为什么有风险
**Auto-fix:**
\```diff
- old code
+ new code
\```
---
### [🟡 中] [🟠 Medium] 问题标题
**文件**: `path/to/file.tsx:18`
**类型**: 运行时
**描述**: ...
**修复建议**: ...
---
## 修改清单 [REQUIRED]
| 优先级 | 置信度 | 文件 | 类型 | 描述 | Auto-fix |
|--------|--------|------|------|------|----------|
| 🔴 高 | 🔵 High | file1.tsx:42 | 安全 | 描述 | ✅ |
| 🟡 中 | 🟠 Medium | file2.tsx:18 | 运行时 | 描述 | — |
## 测试建议 [REQUIRED]
1. 测试场景
2. 测试场景
## GH 评论操作 [REQUIRED if qualifying findings exist, OMIT if none]
以下问题（🔵 High 置信度 + 🟡 中及以上）建议直接评论到 PR：
- [ ] 问题1 — `file.tsx:42`
- [ ] 问题2 — `file.tsx:88`
> 确认后将通过 `gh` CLI 发送 inline comments。
Priority Definitions
| Priority | Criteria | Action |
|---|
| 🔴 高 | Build failure, security vulnerability, data loss, crash | Must fix before merge |
| 🟡 中 | Runtime bug, incorrect behavior, maintainability | Should fix before merge |
| 🟢 低 | Nice-to-have, minor inconsistency | Can fix in follow-up |
Review Discipline
- Read the code — don't just grep. Read each changed file to understand intent.
- No false positives — only report issues you're confident about. Uncertain? Lower the confidence.
- No style nitpicks — focus on security, correctness, architecture, performance.
- Context matters — understand why the code was written this way before suggesting changes.
- Reuse first — before accepting new abstractions or duplicated business logic, search for existing local implementations and flag unnecessary reimplementation with concrete reuse targets.
- Prioritize — 3 high-quality findings beats 20 marginal complaints.
- Score honestly — the score reflects reality, not diplomacy.
- Auto-fix aggressively — when the fix is clear, always include a diff patch. Reviewers prefer actionable suggestions.
Reference Files
- references/security-and-supply-chain.md — PII leakage, AuthN/AuthZ, supply-chain, manifest permissions
- references/code-quality-patterns.md — Hooks, race conditions, null safety, concurrent requests, error handling
- references/onekey-platform-patterns.md — Android/iOS crashes, Fabric, BigNumber, build/CI
- references/quick-commands.md — Bash one-liners for automated checking
