--- name: 1k-code-review-pr description: Structured PR code review checklist for OneKey monorepo. Use when reviewing PRs to identify build issues, script problems, CI workflow gaps, and documentation inconsistencies. Focuses on actionable feedback with priority levels and specific fix suggestions. 代码审查. Code Review PR. disable-model-invocation: true allowed-tools: Read, Grep, Glob, Bash, WebFetch --- # OneKey PR Code Review **输出语言**: 使用中文输出所有审查报告内容。 This skill provides a structured approach to reviewing PRs in the OneKey monorepo, focusing on practical issues that can break builds, cause runtime errors, or lead to maintenance problems. ## Review Scope - Base branch: `x` (main branch) - Use triple-dot diff: `git fetch origin && git diff origin/x...HEAD` ## Relationship with `pr-review` Skill This skill complements the existing `pr-review` skill: | Aspect | `pr-review` | `1k-code-review-pr` | |--------|-------------|---------------------| | Focus | **Security** & supply-chain risk | **Build reliability** & runtime quality | | Best for | Dependency updates, auth changes, sensitive data | Business logic, UI components, scripts | | Depth | Deep security analysis | Practical code patterns | **Recommendation**: For complex PRs, use both skills for comprehensive coverage. ## Review Checklist ### 1. Accidental File Commits (HIGH PRIORITY) Check for files that shouldn't be in the repository: ```bash # Check for files in root directory that look suspicious git diff origin/x...HEAD --name-only | grep -E "^[^/]+$" | grep -v -E "\.(md|json|js|ts|yml|yaml)$" # Check for common accidental commits git diff origin/x...HEAD --name-only | grep -E "(\.DS_Store|Thumbs\.db|\.env\.local|node_modules|\.log$)" ``` **Report Format:** ```markdown ## 问题: 意外提交的文件 **文件**: `filename` **问题描述**: 描述为什么这个文件不应该被提交 **修复方案**: ```bash git rm filename ``` **优先级**: 高 ``` ### 2. Build Failure Risks (HIGH PRIORITY) Check for configurations that reference files which may not exist: **Pattern 1: extraResources referencing generated files** ```javascript // electron-builder configs 'extraResources': [ { 'from': 'path/to/generated/file', 'to': '...' } ] ``` If the file is generated by a script that may skip execution (e.g., platform-specific tools), the build will fail. **Fix Pattern:** - Create placeholder files when skipping generation - Add file existence checks in CI **Pattern 2: Scripts with early exit without cleanup** ```bash # Bad: exits without creating expected output if ! command -v tool &> /dev/null; then echo "Tool not found" exit 0 # Build will fail if expecting output file fi # Good: create placeholder before exit if ! command -v tool &> /dev/null; then echo "Tool not found" mkdir -p "$OUTPUT_PATH" touch "$OUTPUT_PATH/expected-file" # Placeholder exit 0 fi ``` **Report Format:** ```markdown ## 问题: 构建失败风险 **相关文件**: - config-file.js - script.sh **问题描述**: 详细说明为什么构建可能失败 **修复方案**: ```bash # 具体的代码修复 ``` **主要修改点**: 1. 修改点1 2. 修改点2 **优先级**: 高 ``` ### 3. Script Accuracy (MEDIUM PRIORITY) Check for inaccurate comments or misleading error messages: ```bash # Check error messages and comments in shell scripts grep -n "echo.*Warning\|echo.*Error\|#.*requires\|#.*only available" apps/*/scripts/*.sh ``` **Common Issues:** - Claiming a tool is "only available on X" when it's available elsewhere - Incorrect version requirements - Misleading prerequisite descriptions **Report Format:** ```markdown ## 问题: 脚本注释不准确 **文件**: `path/to/script.sh:LINE` **问题描述**: 注释说 X,但实际上 Y **修复方案**: 将第 N 行从: ```bash echo "原始内容" ``` 改为: ```bash echo "修正后内容" ``` **优先级**: 中 ``` ### 4. CI Workflow Verification (LOW-MEDIUM PRIORITY) Check for missing verification steps in CI: **Pattern: Operations without verification** ```yaml # Bad: no verification after generation - name: Generate Assets run: bash scripts/generate.sh # Good: verify generation succeeded - name: Generate Assets run: bash scripts/generate.sh - name: Verify Assets run: bash scripts/verify.sh ``` **Check Points:** 1. File generation steps should have verification 2. Platform-specific steps should handle cross-platform CI 3. Critical paths should fail fast with clear errors **Report Format:** ```markdown ## 问题: CI 工作流缺少验证 **相关文件**: - .github/workflows/workflow.yml **问题描述**: CI 在执行某操作后没有验证是否成功 **修复方案**: ```yaml - name: Verify Step run: | # verification logic ``` **优先级**: 低 ``` ### 5. Documentation Consistency (LOW PRIORITY) Check for PR description matching actual changes: ```bash # List all changed files git diff origin/x...HEAD --name-status # Compare with PR description gh pr view PRNUM --json body ``` **Check Points:** - All significant file changes mentioned in PR - iOS/Android changes called out for mobile icon updates - Breaking changes clearly documented - Migration steps provided if needed ## Output Format ### Summary Report Structure ```markdown # PR #NUMBER 代码审查建议 ## 问题 1: [问题标题] **文件**: `path/to/file` **问题描述**: 详细描述问题 **修复方案**: ```code 具体修复代码 ``` **优先级**: 高/中/低 ## 问题 2: [问题标题] ... ## 修改清单总结 | 优先级 | 文件 | 修改类型 | 描述 | |--------|------|----------|------| | 高 | file1 | 修改 | 描述 | | 中 | file2 | 删除 | 描述 | ## 测试验证 修复完成后,建议进行以下测试: 1. 测试场景1 2. 测试场景2 ``` ## Priority Definitions | Priority | Description | Action | |----------|-------------|--------| | **高 (High)** | Build failures, security issues, data loss risks | Must fix before merge | | **中 (Medium)** | Incorrect behavior, misleading docs, maintainability | Should fix before merge | | **低 (Low)** | Nice-to-have improvements, minor inconsistencies | Can fix in follow-up PR | ## Quick Commands ```bash # Get PR diff summary git diff origin/x...HEAD --stat # Check for generated file references in configs grep -r "extraResources\|extraFiles" apps/*/electron-builder*.js # Find shell scripts with early exits grep -n "exit 0\|exit 1" apps/*/scripts/*.sh # Check CI workflow steps yq '.jobs.*.steps[].name' .github/workflows/*.yml 2>/dev/null || \ grep -A2 "name:" .github/workflows/*.yml # Find useEffect with eslint-disable (potential dependency issues) git diff origin/x...HEAD | grep -A5 "useEffect" | grep "eslint-disable" # Find loops with await inside (performance issue) git diff origin/x...HEAD | grep -E "for.*\{|forEach|\.map\(" -A10 | grep "await" # Check for deprecated packages in new dependencies git diff origin/x...HEAD -- '**/package.json' | grep '^\+' | \ grep -oE '"[^"]+": "[^"]+"' | cut -d'"' -f2 | \ xargs -I{} sh -c 'npm view {} deprecated 2>/dev/null && echo "^^^ {}"' # Find silent error handling (catch without user feedback) git diff origin/x...HEAD | grep -A3 "catch" | grep -v "Toast\|throw\|error" # Check for missing file size validation in uploads git diff origin/x...HEAD | grep -B5 -A10 "file\." | grep -v "size" # Find potential null/undefined issues (missing optional chaining) git diff origin/x...HEAD | grep -E "\.current\.[a-zA-Z]|ref\.[a-zA-Z]" | grep -v "?." # Find array index access without bounds check git diff origin/x...HEAD | grep -E "\[index\]|\[i\]|\[0\]" -A2 | grep -v "if.*length\|if.*!" # Find potential race conditions (state updates in async) git diff origin/x...HEAD | grep -B5 "setState\|set[A-Z]" | grep -E "then\(|await" # Find platform-specific files git diff origin/x...HEAD --name-only | grep -E "\.(android|ios|native|desktop)\.(ts|tsx)$" # Find BigNumber operations without type coercion git diff origin/x...HEAD | grep -E "BigNumber|shiftedBy|dividedBy" | grep -v "Number(" # Find debounced functions that may not return promises git diff origin/x...HEAD | grep -E "debounce|setTimeout.*validate" -A5 | grep -v "Promise\|resolve" # Find setState without clearing stale data git diff origin/x...HEAD | grep -B3 -A3 "useEffect" | grep -E "fetch|load" | grep -v "set.*\[\]|set.*null" # Find captured refs in useEffect cleanup (potential stale ref) git diff origin/x...HEAD | grep -B10 "return.*=>" | grep "const.*=.*Ref.current" # Check for division operations without zero guards git diff origin/x...HEAD | grep -E "/ [a-zA-Z]" | grep -v "if.*===.*0\|if.*>.*0" # Find map/forEach with index that mutates array git diff origin/x...HEAD | grep -E "\.map\(|\.forEach\(" -A5 | grep -E "splice|shift|pop" ``` ### 6. React Hooks Safety (HIGH PRIORITY) Check for hooks-related issues that can cause infinite loops, memory leaks, or race conditions: **Pattern 1: useEffect with missing dependencies** ```typescript // Bad: eslint-disable hides dependency issues useEffect(() => { doSomething(someValue); // someValue not in deps // eslint-disable-next-line react-hooks/exhaustive-deps }, []); // Good: include all dependencies or use refs useEffect(() => { doSomething(someValue); }, [someValue]); ``` **Pattern 2: Non-functional setState in useCallback** ```typescript // Bad: uses stale state const handleClick = useCallback(() => { setState({ ...state, updated: true }); // state may be stale }, [state]); // Good: functional update const handleClick = useCallback(() => { setState(prev => ({ ...prev, updated: true })); }, []); ``` **Pattern 3: Missing cleanup in useEffect** ```typescript // Bad: timer not cleaned up useEffect(() => { const timer = setInterval(() => { ... }, 1000); if (condition) return; // Early return without cleanup! return () => clearInterval(timer); }, []); // Good: always return cleanup useEffect(() => { if (condition) return () => {}; const timer = setInterval(() => { ... }, 1000); return () => clearInterval(timer); }, []); ``` **Pattern 4: Async validation without abort** ```typescript // Bad: no cancellation on unmount useEffect(() => { validateAsync(value).then(setResult); }, [value]); // Good: with AbortController useEffect(() => { const controller = new AbortController(); validateAsync(value, controller.signal).then(setResult); return () => controller.abort(); }, [value]); ``` **Report Format:** ```markdown ## 问题: Hooks 安全风险 **文件**: `path/to/component.tsx:LINE` **问题描述**: - 风险类型:死循环/内存泄漏/竞态条件/过时闭包 - 具体说明 **修复方案**: ```typescript // 修复后的代码 ``` **优先级**: 高 ``` ### 7. Concurrent Request Control (HIGH PRIORITY) Check for code that may overwhelm backend with too many requests: **Pattern 1: Sequential await in loop (performance issue)** ```typescript // Bad: 500 addresses = 500 sequential API calls = 50+ seconds for (const address of addresses) { await validateAddress(address); // O(n) API calls } // Good: concurrent with rate limiting import pLimit from 'p-limit'; const limit = pLimit(10); // max 10 concurrent await Promise.all(addresses.map(addr => limit(() => validateAddress(addr)) )); ``` **Pattern 2: Missing request deduplication** ```typescript // Bad: polling may stack up requests useEffect(() => { const interval = setInterval(() => { fetchData(); // No guard against overlapping requests }, 1000); return () => clearInterval(interval); }, [fetchData]); // fetchData changes = new interval // Good: with request guard const isLoading = useRef(false); useEffect(() => { const interval = setInterval(async () => { if (isLoading.current) return; isLoading.current = true; try { await fetchData(); } finally { isLoading.current = false; } }, 1000); return () => clearInterval(interval); }, []); ``` **Pattern 3: Unbatched validation** ```typescript // Bad: validate each item individually items.forEach(async item => { const result = await api.validate(item); }); // Good: batch API if available const results = await api.validateBatch(items); ``` **Check Points:** 1. Loops with `await` inside - consider `Promise.all` with `p-limit` 2. Polling without request guards 3. Form validation that calls API per field 4. File processing without chunking **Report Format:** ```markdown ## 问题: 大量并发/串行请求 **文件**: `path/to/file.tsx:LINE` **问题描述**: | 场景 | 请求数 | 预估耗时 | |------|--------|----------| | 100 项 | 100 次 | 10 秒 | | 500 项 | 500 次 | 50 秒 | **修复方案**: ```typescript // 使用并发控制 import pLimit from 'p-limit'; const limit = pLimit(10); await Promise.all(items.map(i => limit(() => process(i)))); ``` **优先级**: 高 ``` ### 8. Deprecated Dependencies (MEDIUM PRIORITY) Check for deprecated or renamed packages: ```bash # Check for deprecated packages in package.json changes git diff origin/x...HEAD -- '**/package.json' | grep -E '^\+.*"[^"]+": "[^"]+"' # Verify package status npm view PACKAGE_NAME deprecated 2>/dev/null ``` **Common Patterns:** - Package renamed (e.g., `react-native-document-picker` → `@react-native-documents/picker`) - Package no longer maintained - Security vulnerabilities in dependencies **Report Format:** ```markdown ## 问题: 依赖包已废弃 **文件**: `package.json` **包名**: `deprecated-package` **问题描述**: 该包已被废弃,官方推荐迁移到新包 **迁移建议**: ```bash yarn remove deprecated-package yarn add new-package ``` **API 变更**: ```typescript // 旧 API import { old } from 'deprecated-package'; // 新 API import { new } from 'new-package'; ``` **优先级**: 中 ``` ### 9. Error Handling Patterns (MEDIUM PRIORITY) Check for proper error handling: **Pattern 1: Silent failures** ```typescript // Bad: error swallowed, user gets no feedback try { await submitForm(); } catch (error) { console.error(error); // Only logged, not shown } // Good: show error to user try { await submitForm(); } catch (error) { Toast.error({ title: error.message }); } ``` **Pattern 2: Empty catch blocks** ```typescript // Bad: completely silent try { riskyOperation(); } catch {} // Good: at least log try { riskyOperation(); } catch (e) { console.warn(e); } ``` **Pattern 3: Silent early returns** ```typescript // Bad: function exits without feedback const handleSubmit = () => { if (!data) return; // User clicks button, nothing happens // ... }; // Good: provide feedback const handleSubmit = () => { if (!data) { Toast.warning({ title: 'Please fill required fields' }); return; } // ... }; ``` **Report Format:** ```markdown ## 问题: 错误处理不完整 **文件**: `path/to/file.tsx:LINE` **问题描述**: 错误被静默处理,用户无法得知失败原因 **修复方案**: ```typescript // 添加用户反馈 Toast.error({ title: error.message }); ``` **优先级**: 中 ``` ### 10. Null/Undefined Safety (HIGH PRIORITY) Check for missing null/undefined guards that can cause crashes: **Pattern 1: Optional chaining missing** ```typescript // Bad: crashes if ref is undefined const value = ref.current.getValue(); // Good: optional chaining const value = ref.current?.getValue(); ``` **Pattern 2: Array access without bounds check** ```typescript // Bad: may access undefined const item = items[index]; item.name; // TypeError if undefined // Good: with guard const item = items[index]; if (!item) return; item.name; ``` **Pattern 3: Callback without null check** ```typescript // Bad: callback may fire after unmount onDomReady(() => { webviewRef.current.reload(); // ref may be null }); // Good: with null check onDomReady(() => { if (!webviewRef.current) return; webviewRef.current.reload(); }); ``` **Pattern 4: Index shifting after array mutation** ```typescript // Bad: index becomes invalid after splice for (let i = 0; i < items.length; i++) { if (shouldRemove(items[i])) { items.splice(i, 1); // i now points to wrong item! } } // Good: iterate backwards or use filter const filtered = items.filter(item => !shouldRemove(item)); ``` **Common Crash Sources (from Sentry):** - `TypeError: Cannot read property 'X' of undefined` - `nil NSString crash in TurboModule` - `EXC_BAD_ACCESS when URI is nil` - `RetryableMountingLayerException: Unable to find viewState for tag` **Report Format:** ```markdown ## 问题: 空值安全 **文件**: `path/to/file.tsx:LINE` **问题描述**: 缺少空值检查,可能导致 TypeError 或崩溃 **修复方案**: ```typescript // 添加可选链或空值检查 if (!value) return; ``` **优先级**: 高 ``` ### 11. Race Conditions & Cleanup (HIGH PRIORITY) Check for race conditions in React components, especially with React Native Fabric: **Pattern 1: State update after unmount** ```typescript // Bad: may update unmounted component useEffect(() => { fetchData().then(data => { setState(data); // Component may be unmounted }); }, []); // Good: with mount check useEffect(() => { let isMounted = true; fetchData().then(data => { if (isMounted) setState(data); }); return () => { isMounted = false; }; }, []); ``` **Pattern 2: Dialog close + navigation race** ```typescript // Bad: Fabric crash during rapid navigation const handleConfirm = () => { dialog.close(); navigation.push('NextPage'); // Race with dialog unmount }; // Good: delay navigation const handleConfirm = async () => { dialog.close(); await new Promise(r => setTimeout(r, 100)); // Allow cleanup navigation.push('NextPage'); }; ``` **Pattern 3: WebView ref access after cleanup** ```typescript // Bad: ref may be null during cleanup useEffect(() => { return () => { webviewRef.current?.stopLoading(); // May crash }; }, []); // Good: capture ref before cleanup useEffect(() => { const webview = webviewRef.current; return () => { webview?.stopLoading(); // Uses captured reference }; }, []); ``` **Common Fabric Crashes:** - `RetryableMountingLayerException` - View already unmounted - `SurfaceControl crashes on Android` - MIUI/HyperOS specific - `SIGSEGV during navigation cleanup` - WebView race condition **Report Format:** ```markdown ## 问题: 竞态条件 **文件**: `path/to/file.tsx:LINE` **问题描述**: 组件卸载时状态更新/Dialog 关闭与导航竞争 **修复方案**: ```typescript // 添加 isMounted 检查或延迟 ``` **优先级**: 高 ``` ### 12. Platform-specific Issues (MEDIUM PRIORITY) Check for platform-specific code that may cause issues: **Android Common Issues:** ```typescript // MIUI/HyperOS splash screen crash // - SurfaceControl error needs defensive handling // - Add try-catch for splash operations // Layout re-measurement // - tabBarHidden changes need layout update // - Use LayoutAnimation or forceUpdate // OOM with images // - Check bitmap memory limits // - Use resizeMode and memory-efficient loading ``` **iOS Common Issues:** ```typescript // EXC_BAD_ACCESS with nil strings // - Always check for nil before NSString operations // - Use optional binding in native code // expo-image crashes // - Validate URI before passing to Image // - Handle empty/nil URI gracefully: if (!uri || uri.length === 0) return null; // Stack overflow in recursive calls // - Debounce recursive operations // - Add depth limits ``` **Quick Check Commands:** ```bash # Find platform-specific files in diff git diff origin/x...HEAD --name-only | grep -E "\.(android|ios)\.(ts|tsx)$" # Check for native module interactions git diff origin/x...HEAD | grep -E "NativeModules|TurboModule|requireNativeComponent" ``` **Report Format:** ```markdown ## 问题: 平台特定问题 **平台**: Android/iOS **文件**: `path/to/file.tsx:LINE` **问题描述**: [平台] 特有的崩溃或异常行为 **修复方案**: ```typescript // 平台特定的防御性代码 ``` **优先级**: 中 ``` ### 13. Type Safety for Numeric Operations (MEDIUM PRIORITY) Check for type coercion issues with BigNumber/decimal operations: **Pattern 1: Ensure number type before BigNumber** ```typescript // Bad: decimals might be string from JSON const amount = new BigNumber(value).shiftedBy(-decimals); // Good: ensure number type const amount = new BigNumber(value).shiftedBy(-Number(decimals)); ``` **Pattern 2: String vs Number in API responses** ```typescript // Bad: API might return string or number const balance = response.balance; // Could be "100" or 100 // Good: normalize type const balance = String(response.balance); // Or Number() if needed ``` **Pattern 3: Division with potential zero** ```typescript // Bad: division by zero or undefined const rate = amount / total; // Good: guard against zero const rate = total > 0 ? amount / total : 0; ``` **Report Format:** ```markdown ## 问题: 数值类型安全 **文件**: `path/to/file.tsx:LINE` **问题描述**: 类型强制转换可能导致 NaN 或计算错误 **修复方案**: ```typescript // 确保类型正确 const value = Number(input); if (isNaN(value)) throw new Error('Invalid number'); ``` **优先级**: 中 ``` ### 14. Stale Data Management (MEDIUM PRIORITY) Check for stale data issues when context changes: **Pattern 1: Clear cache on context switch** ```typescript // Bad: stale provider list shown when switching const [providers, setProviders] = useState([]); useEffect(() => { fetchProviders(type).then(setProviders); }, [type]); // Good: clear before fetching useEffect(() => { setProviders([]); // Clear stale data immediately fetchProviders(type).then(setProviders); }, [type]); ``` **Pattern 2: State/callback captured at wrong time** ```typescript // Bad: callback captured at setup time, becomes stale useEffect(() => { const savedCallback = onUpdate; // Captured at setup const interval = setInterval(() => { savedCallback(getData()); // Uses stale callback! }, 1000); return () => clearInterval(interval); }, []); // Missing onUpdate dependency // Good: use ref for latest callback value const onUpdateRef = useRef(onUpdate); onUpdateRef.current = onUpdate; // Always fresh useEffect(() => { const interval = setInterval(() => { onUpdateRef.current(getData()); // Always uses latest }, 1000); return () => clearInterval(interval); }, []); ``` **Note on Refs in Cleanup:** For DOM/component refs (like `webviewRef`), you SHOULD capture the ref before cleanup to ensure you're cleaning up the correct resource. See Section 11, Pattern 3 for the correct ref cleanup pattern. **Pattern 3: State derived from props not updating** ```typescript // Bad: initial state never updates const [value, setValue] = useState(props.initialValue); // Good: sync with prop changes const [value, setValue] = useState(props.initialValue); useEffect(() => { setValue(props.initialValue); }, [props.initialValue]); ``` **Report Format:** ```markdown ## 问题: 陈旧数据 **文件**: `path/to/file.tsx:LINE` **问题描述**: 上下文切换时显示旧数据,造成用户困惑 **修复方案**: ```typescript // 切换时立即清除旧数据 setData(null); fetchNewData().then(setData); ``` **优先级**: 中 ``` ### 15. Debounced Async Validation (MEDIUM PRIORITY) Check for issues with debounced validation that returns promises: **Pattern 1: Promise never resolves** ```typescript // Bad: debounced function may not resolve promise const debouncedValidate = useMemo(() => { let timeoutId: NodeJS.Timeout; return (value: string) => { clearTimeout(timeoutId); timeoutId = setTimeout(async () => { await validate(value); // Promise not returned! }, 300); }; }, []); // Good: track and resolve promises const debouncedValidate = useCallback((value: string) => { return new Promise((resolve) => { clearTimeout(timeoutRef.current); timeoutRef.current = setTimeout(async () => { const result = await validate(value); resolve(result); }, 300); }); }, [validate]); ``` **Pattern 2: Form validation hangs** ```typescript // Bad: react-hook-form waits forever rules={{ validate: (value) => { debouncedValidate(value); // Returns undefined, not promise! return true; // Always passes initially } }} // Good: return the promise rules={{ validate: (value) => debouncedValidate(value) }} ``` **Pattern 3: Cleanup pending validations** ```typescript // Bad: validation continues after unmount useEffect(() => { return () => { // No cleanup of pending validation }; }, []); // Good: clear pending validation useEffect(() => { return () => { if (timeoutRef.current) clearTimeout(timeoutRef.current); if (pendingResolve.current) pendingResolve.current(true); }; }, []); ``` **Report Format:** ```markdown ## 问题: 防抖验证 Promise 处理 **文件**: `path/to/file.tsx:LINE` **问题描述**: 防抖验证函数未正确返回 Promise,导致表单验证挂起 **修复方案**: ```typescript // 确保返回 Promise 并在卸载时清理 ``` **优先级**: 中 ``` ### 16. Security Considerations for Bulk Operations (HIGH PRIORITY) Check security aspects of features handling multiple items: **Pattern 1: File upload without size limits** ```typescript // Bad: no file size check const handleFile = async (file: File) => { const content = await file.text(); // Could be gigabytes }; // Good: check size first const MAX_SIZE = 5 * 1024 * 1024; // 5MB const handleFile = async (file: File) => { if (file.size > MAX_SIZE) { throw new Error('File too large'); } const content = await file.text(); }; ``` **Pattern 2: Hardcoded contract addresses** ```typescript // Risk: if address is wrong, funds could be lost const CONTRACT = '0x123...'; // Should verify checksum // Better: validate address format import { getAddress } from 'ethers'; const CONTRACT = getAddress('0x123...'); // Throws if invalid ``` **Pattern 3: Missing input validation** ```typescript // Bad: trusting user input const amounts = userInput.split(',').map(Number); // Good: validate each value const amounts = userInput.split(',').map(v => { const num = Number(v.trim()); if (isNaN(num) || num < 0) throw new Error('Invalid amount'); return num; }); ``` **Report Format:** ```markdown ## 问题: 安全风险 **文件**: `path/to/file.tsx:LINE` **风险类型**: 输入验证/资金安全/DoS风险 **问题描述**: 详细说明安全风险 **修复方案**: ```typescript // 安全的实现 ``` **优先级**: 高 ``` ## Review Workflow 1. **Checkout**: Switch to the PR branch before reviewing: `gh pr checkout ` (skip if already on the target branch) 2. **Scope**: Run `git diff origin/x...HEAD --stat` to understand change scope 3. **Files**: Check for accidental commits and generated file references 4. **Scripts**: Review shell scripts for proper error handling and placeholders 5. **CI**: Verify CI workflows have appropriate verification steps 6. **Hooks**: Check React hooks for dependency issues, memory leaks, infinite loops 7. **Requests**: Verify concurrent/sequential request handling is optimized 8. **Dependencies**: Check for deprecated packages in new dependencies 9. **Errors**: Ensure proper error handling with user feedback 10. **Null Safety**: Check for missing null/undefined guards 11. **Race Conditions**: Look for Fabric race conditions, cleanup issues 12. **Platform**: Check Android/iOS specific crash patterns 13. **Type Safety**: Verify numeric types before BigNumber/decimal operations 14. **Stale Data**: Check for cache clearing on context switches 15. **Debounce**: Verify debounced async functions return proper promises 16. **Security**: Review security aspects for bulk/batch operations 17. **Docs**: Ensure PR description matches actual changes 18. **Report**: Generate structured report with priorities and fix suggestions