1k Code Review Pr

OneKey PR Code Review

输出语言: 中文

Review Scope

• Base branch: x
• Diff: git fetch origin && git diff origin/x...HEAD (triple-dot)

Workflow

1. Checkout — gh pr checkout <PR_NUMBER> (skip if already on branch)
2. Scope — git diff origin/x...HEAD --stat to see change scope
3. Triage — Determine which review modules apply (see triage table)
4. Primary Review — Read each changed file, apply relevant checks from references/
5. Codex Cross-Review — If Codex available, run full parallel review (see below)
6. PR Comment Analysis — Fetch all existing PR comments (bot + human), analyze with local codebase context (see below)
7. Merge Findings — Combine primary + Codex + PR comment findings, deduplicate, annotate confidence
8. Score — Rate the PR across 4 dimensions (see Scoring System). This step is MANDATORY — every report MUST include the scoring table.
9. Report — Generate structured report using the unified format. Follow the template exactly — every section is required.
10. GH Comment — Post qualifying findings as inline PR comments immediately (do not wait for 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:

1. 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}"
   )
   

2. Parse Agent result for structured findings
3. 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
4. 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):

# Top-level PR reviews (review bodies)
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}]'

# Inline review comments (file:line annotations)
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}]'

# General PR comments (issue-level)
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):

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:

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

# Quick hierarchy violation check on changed files
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

Scoring System

MANDATORY — every report must include this scoring table, no exceptions.

Rate the PR on 4 dimensions (1-10 each):

Total Score = weighted average, rounded to 1 decimal.

Scoring anchors — to keep scores consistent:

• Start at 8 for each dimension, deduct for issues found
• A single P0 security issue → Security capped at 3
• A single P0 crash/runtime 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:

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, post them immediately. Do not ask "是否确认？" and do not wait for a follow-up message. New conversations have no memory of a verbal "以后都自动发" — this section is the source of truth.

Comment threshold: P0 (any confidence) OR P1 with 🔵 High confidence. This means:

• All P0 findings (regardless of confidence)
• All P1 findings with 🔵 High confidence (cross-validated or confirmed from code)
• Excludes: P2 findings, and P1 with 🟠 Medium or ⚪ Low confidence

1. List the qualifying findings that warrant PR comments
2. Post them in the same turn as the report, using ManagePullRequest post_comment (path + line) when available, otherwise:

# Inline comment on specific file:line
gh api repos/{owner}/{repo}/pulls/{pr_number}/comments \
  --field body="**[P1] 问题标题**: 描述...

**建议修复:**
\`\`\`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:

• Always post qualifying findings in the same turn as the review report
• Only post findings meeting the comment threshold (see above)
• Include auto-fix in suggestion block when available
• Maximum 5 inline comments per PR
• GitHub can only attach review comments to lines in the PR diff; if the real site is unchanged, comment on the nearest changed line and name the unchanged site in the body
• If an existing inline thread already states the same issue, reply to that thread instead of opening a duplicate

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]

### [P0] [🔵 High] 问题标题 {Cross-validated ✅}
**文件**: `path/to/file.tsx:42`
**类型**: 安全 / 构建 / 运行时 / 性能 / 规范
**描述**: 问题是什么，为什么有风险
**Auto-fix:**
\```diff
- old code
+ new code
\```

---

### [P1] [🟠 Medium] 问题标题
**文件**: `path/to/file.tsx:18`
**类型**: 运行时
**描述**: ...
**修复建议**: ...

---

## 修改清单 [REQUIRED]

| 优先级 | 置信度 | 文件 | 类型 | 描述 | Auto-fix |
|--------|--------|------|------|------|----------|
| P0 | 🔵 High | file1.tsx:42 | 安全 | 描述 | ✅ |
| P1 | 🟠 Medium | file2.tsx:18 | 运行时 | 描述 | — |

## 测试建议 [REQUIRED]
1. 测试场景
2. 测试场景

## GH 评论操作 [REQUIRED if qualifying findings exist, OMIT if none]
以下问题（P0 任意置信度，或 P1 + 🔵 High 置信度）建议直接评论到 PR：
- [ ] 问题1 — `file.tsx:42`
- [ ] 问题2 — `file.tsx:88`

> 达标问题已在本回合直接发为 inline comments，无需再确认。

Priority Definitions

MANDATORY — every finding must carry exactly one P0–P2 tag as its priority. Do NOT use 高/中/低 or Critical/High/Medium/Low as finding priority labels.

Historical PR threads may still use the legacy 5-tier scale; when analyzing them, map old P1 → P0, old P2 → P1, old P3/P4 → P2.

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