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 MCP 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 MCP Integration

Check if mcp__codex__codex is in available tools.

If available:

• Send the full diff to Codex for an independent full review: 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.

• Retrieve response via mcp__codex__codex-reply

• 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):

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):

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)

• 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

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

🏛️ 架构合理性 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:

Inline comment on specific file:line

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]

Codex 交叉验证摘要 [REQUIRED if Codex was used, OMIT if not]

PR 评论分析 [REQUIRED if comments exist, OMIT if none]

评论误报分析 [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]

测试建议 [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.

• 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