Skip to main content الرئيسية المنشئون onekeyhq app-modules 1k-code-review-pr
1k-code-review-pr 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.
الانتقال إلى التثبيت سوق المهارات اكتشف واستكشف مهارات الذكاء الاصطناعي التي بناها المجتمع.
التثبيت باستخدام Codex أو Claude انسخ هذا Prompt والصقه في Codex أو Claude أو مساعد آخر ليراجع صفحة Skill ويثبّتها لك.
عرض تفاصيل Prompt يتجاوز الأمر المباشر Prompt المخصّص للمراجعة. افحص المصدر قبل تشغيله.
npx skills add https://github.com/OneKeyHQ/app-modules --skill 1k-code-review-prيبقى الأمر في سطر واحد. مرّر أفقيًا لمراجعته كاملًا قبل النسخ.
تفضّل نسخة محلية؟ نزّل الملفات المتاحة حاليًا لدى SkillsMP.
تحميل Zip جاري التحميل...
المهن ذات الصلة SOC
استنادا إلى تصنيف SOC المهني
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-review1k-code-review-prFocus Security & supply-chain riskBuild reliability & runtime qualityBest 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:
git diff origin/x...HEAD --name-only | grep -E "^[^/]+$" | grep -v -E "\.(md|json|js|ts|yml|yaml)$"
git diff origin/x...HEAD --name-only | grep -E "(\.DS_Store|Thumbs\.db|\.env\.local|node_modules|\.log$)"
## 问题: 意外提交的文件
**文件** : `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.
Create placeholder files when skipping generation
Add file existence checks in CI
Pattern 2: Scripts with early exit without cleanup
if ! command -v tool &> /dev/null; then
echo "Tool not found"
exit 0
fi
if ! command -v tool &> /dev/null; then
echo "Tool not found"
mkdir -p "$OUTPUT_PATH "
touch "$OUTPUT_PATH /expected-file"
exit 0
fi
## 问题: 构建失败风险
**相关文件** :
- config-file.js
- script.sh
**问题描述** :
详细说明为什么构建可能失败
**修复方案** :
```bash
# 具体的代码修复
### 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
Claiming a tool is "only available on X" when it's available elsewhere
Incorrect version requirements
Misleading prerequisite descriptions
## 问题: 脚本注释不准确
**文件** : `path/to/script.sh:LINE`
**问题描述** :
注释说 X，但实际上 Y
**修复方案** :
将第 N 行从：
```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
File generation steps should have verification
Platform-specific steps should handle cross-platform CI
Critical paths should fail fast with clear errors
## 问题: 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
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 # PR #NUMBER 代码审查建议
## 问题 1: [问题标题]
**文件** : `path/to/file`
**问题描述** : 详细描述问题
**修复方案** :
```code
具体修复代码
问题 2: [问题标题]
修改清单总结 优先级 文件 修改类型 描述 高 file1 修改 描述 中 file2 删除 描述
测试验证
## 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
useEffect (() => {
doSomething (someValue);
}, []);
useEffect (() => {
doSomething (someValue);
}, [someValue]);
Pattern 2: Non-functional setState in useCallback
const handleClick = useCallback (() => {
setState ({ ...state, updated : true });
}, [state]);
const handleClick = useCallback (() => {
setState (prev =>updated : true }));
}, []);
Pattern 3: Missing cleanup in useEffect
useEffect (() => {
const timer = setInterval (() => { ... }, 1000 );
if (condition) return ;
return () => clearInterval (timer);
}, []);
useEffect (() => {
if (condition) return () => {};
const timer = setInterval (() => { ... }, 1000 );
return () => clearInterval (timer);
}, []);
Pattern 4: Async validation without abort
useEffect (() => {
validateAsync (value).then (setResult);
}, [value]);
useEffect (() => {
const controller = new AbortController ();
validateAsync (value, controller.signal ).then (setResult);
return () => controller.abort ();
}, [value]);
## 问题: 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
useEffect (() => {
const interval = setInterval (() => {
fetchData ();
}, 1000 );
return () => clearInterval (interval);
}, [fetchData]);
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
items.forEach (async item => {
const result = await api.validate (item);
});
const results = await api.validateBatch (items);
Loops with await inside - consider Promise.all with p-limit
Polling without request guards
Form validation that calls API per field
File processing without chunking
## 问题: 大量并发/串行请求
**文件** : `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
Package renamed (e.g., react-native-document-picker → @react-native-documents/picker)
Package no longer maintained
Security vulnerabilities in dependencies
## 问题: 依赖包已废弃
**文件** : `package.json`
**包名** : `deprecated-package`
**问题描述** :
该包已被废弃，官方推荐迁移到新包
**迁移建议** :
```bash
yarn remove deprecated-package
yarn add new-package
import { old } from 'deprecated-package' ;
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
try { riskyOperation (); } catch {}
try { riskyOperation (); } catch (e) { console .warn (e); }
Pattern 3: Silent early returns
const handleSubmit = (if (!data) return ;
};
const handleSubmit = (if (!data) {
Toast .warning ({ title : 'Please fill required fields' });
return ;
}
};
## 问题: 错误处理不完整
**文件** : `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
const item = items[index];
item.name ;
const item = items[index];
if (!item) return ;
item.name ;
Pattern 3: Callback without null check
onDomReady (() => {
webviewRef.current .reload ();
});
onDomReady (() => {
if (!webviewRef.current ) return ;
webviewRef.current .reload ();
});
Pattern 4: Index shifting after array mutation
for (let i = 0 ; i < items.length ; i++) {
if (shouldRemove (items[i])) {
items.splice (i, 1 );
}
}
const filtered = items.filter (item =>shouldRemove (item));
Common Crash Sources (from Sentry):
TypeError: Cannot read property 'X' of undefinednil NSString crash in TurboModuleEXC_BAD_ACCESS when URI is nilRetryableMountingLayerException: Unable to find viewState for tag ## 问题: 空值安全
**文件** : `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
const handleConfirm = (close ();
navigation.push ('NextPage' );
};
const handleConfirm = async (close ();
await new Promise (r =>setTimeout (r, 100 ));
navigation.push ('NextPage' );
};
Pattern 3: WebView ref access after cleanup
useEffect (() => {
return () => {
webviewRef.current ?.stopLoading ();
};
}, []);
useEffect (() => {
const webview = webviewRef.current ;
return () => {
webview?.stopLoading ();
};
}, []);
RetryableMountingLayerException - View already unmountedSurfaceControl crashes on Android - MIUI/HyperOS specificSIGSEGV during navigation cleanup - WebView race condition ## 问题: 竞态条件
**文件** : `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
if (!uri || uri.length === 0 ) return null ;
git diff origin/x...HEAD --name-only | grep -E "\.(android|ios)\.(ts|tsx)$"
git diff origin/x...HEAD | grep -E "NativeModules|TurboModule|requireNativeComponent"
## 问题: 平台特定问题
**平台** : 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
const balance = response.balance ;
const balance = String (response.balance );
Pattern 3: Division with potential zero
const rate = amount / total;
const rate = total > 0 ? amount / total : 0 ;
## 问题: 数值类型安全
**文件** : `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
useEffect (() => {
const savedCallback = onUpdate;
const interval = setInterval (() => {
savedCallback (getData ());
}, 1000 );
return () => clearInterval (interval);
}, []);
const onUpdateRef = useRef (onUpdate);
onUpdateRef.current = onUpdate;
useEffect (() => {
const interval = setInterval (() => {
onUpdateRef.current (getData ());
}, 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
const [value, setValue] = useState (props.initialValue );
const [value, setValue] = useState (props.initialValue );
useEffect (() => {
setValue (props.initialValue );
}, [props.initialValue ]);
## 问题: 陈旧数据
**文件** : `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<boolean>((resolve) => {
clearTimeout(timeoutRef.current);
timeoutRef.current = setTimeout(async () => {
const result = await validate(value);
resolve(result);
}, 300);
});
}, [validate]);
Pattern 2: Form validation hangs
rules={{
validate : (value ) => {
debouncedValidate (value);
return true ;
}
}}
rules={{
validate : (value ) => debouncedValidate (value)
}}
Pattern 3: Cleanup pending validations
useEffect (() => {
return () => {
};
}, []);
useEffect (() => {
return () => {
if (timeoutRef.current ) clearTimeout (timeoutRef.current );
if (pendingResolve.current ) pendingResolve.current (true );
};
}, []);
## 问题: 防抖验证 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
const CONTRACT = '0x123...' ;
import { getAddress } from 'ethers' ;
const CONTRACT = getAddress ('0x123...' );
Pattern 3: Missing input validation
const amounts = userInput.split (',' ).map (Number );
const amounts = userInput.split (',' ).map (v =>const num = Number (v.trim ());
if (isNaN (num) || num < 0 ) throw new Error ('Invalid amount' );
return num;
});
## 问题: 安全风险
**文件** : `path/to/file.tsx:LINE`
**风险类型** : 输入验证/资金安全/DoS风险
**问题描述** :
详细说明安全风险
**修复方案** :
```typescript
// 安全的实现
## Review Workflow
1. **Scope**: Run `git diff origin/x...HEAD --stat` to understand change scope
2. **Files**: Check for accidental commits and generated file references
3. **Scripts**: Review shell scripts for proper error handling and placeholders
4. **CI**: Verify CI workflows have appropriate verification steps
5. **Hooks**: Check React hooks for dependency issues, memory leaks, infinite loops
6. **Requests**: Verify concurrent/sequential request handling is optimized
7. **Dependencies**: Check for deprecated packages in new dependencies
8. **Errors**: Ensure proper error handling with user feedback
9. **Null Safety**: Check for missing null/undefined guards
10. **Race Conditions**: Look for Fabric race conditions, cleanup issues
11. **Platform**: Check Android/iOS specific crash patterns
12. **Type Safety**: Verify numeric types before BigNumber/decimal operations
13. **Stale Data**: Check for cache clearing on context switches
14. **Debounce**: Verify debounced async functions return proper promises
15. **Security**: Review security aspects for bulk/batch operations
16. **Docs**: Ensure PR description matches actual changes
17. **Report**: Generate structured report with priorities and fix suggestions
