1k-code-review-pr

仓库: onekeyhq/app-monorepo
安装量: 46
排名: #16237

安装

npx skills add https://github.com/onekeyhq/app-monorepo --skill 1k-code-review-pr
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:

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:

问题: 意外提交的文件
**
文件
**
:
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

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:

问题: 构建失败风险 ** 相关文件 ** : - config-file.js - script.sh ** 问题描述 ** : 详细说明为什么构建可能失败 ** 修复方案 ** : ```bash

具体的代码修复
主要修改点
:
修改点1
修改点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:

问题: 脚本注释不准确
**
文件
**
:
path/to/script.sh:LINE
**
问题描述
**
:
注释说 X，但实际上 Y
**
修复方案
**
:
将第 N 行从：
```bash
echo "原始内容"
改为：
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: File generation steps should have verification Platform-specific steps should handle cross-platform CI Critical paths should fail fast with clear errors Report Format:

问题: 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

PR #NUMBER 代码审查建议

问题 1: [问题标题]
**
文件
**
:
path/to/file
**
问题描述
**
详细描述问题
**
修复方案
**
:
```code
具体修复代码
优先级
高/中/低 问题 2: [问题标题] ... 修改清单总结 优先级 文件 修改类型 描述 高 file1 修改 描述 中 file2 删除 描述 测试验证 修复完成后，建议进行以下测试： 测试场景1 测试场景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
# 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]"
# Find array index access without bounds check
git diff origin/x...HEAD grep -E "[index] [i]
# Find potential race conditions (state updates in async)
git diff origin/x...HEAD grep -B5 "setState|set[A-Z]" grep -E "then(
# Find platform-specific files
git diff origin/x...HEAD --name-only grep -E ".(android ios
# Find BigNumber operations without type coercion
git diff origin/x...HEAD grep -E "BigNumber shiftedBy
# Find debounced functions that may not return promises
git diff origin/x...HEAD grep -E "debounce setTimeout.*validate" -A5
# Find setState without clearing stale data
git diff origin/x...HEAD grep -B3 -A3 "useEffect" grep -E "fetch
# 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
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
// 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
// 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
// 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
// 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:
##
问题: 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
// 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
// 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:
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
Report Format:
##
问题: 大量并发/串行请求
**
文件
**
:
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:
##
问题: 依赖包已废弃
**
文件
**
:
package.json
**
包名
**
:
deprecated-package
**
问题描述
**
:
该包已被废弃，官方推荐迁移到新包
**
迁移建议
**
:
```bash
yarn remove deprecated-package
yarn add new-package
API 变更
:
// 旧 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
// Bad: completely silent
try
{
riskyOperation
(
)
;
}
catch
{
}
// Good: at least log
try
{
riskyOperation
(
)
;
}
catch
(
e
)
{
console
.
warn
(
e
)
;
}
Pattern 3: Silent early returns
// 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:
##
问题: 错误处理不完整
**
文件
**
:
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
// 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
// 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
// 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:
##
问题: 空值安全
**
文件
**
:
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
// 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
// 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:
##
问题: 竞态条件
**
文件
**
:
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:
// 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:
# 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:
##
问题: 平台特定问题
**
平台
**
: 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
// 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
// Bad: division by zero or undefined
const
rate
=
amount
/
total
;
// Good: guard against zero
const
rate
=
total
>
0
?
amount
/
total
:
0
;
Report Format:
##
问题: 数值类型安全
**
文件
**
:
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
// 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
// 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:
##
问题: 陈旧数据
**
文件
**
:
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
// 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
// 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:
##
问题: 防抖验证 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
// 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
// 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:
##
问题: 安全风险
**
文件
**
:
path/to/file.tsx:LINE
**
风险类型
**
: 输入验证/资金安全/DoS风险
**
问题描述
**
:
详细说明安全风险
**
修复方案
**
:
```typescript
// 安全的实现
优先级
: 高
## Review Workflow
1. Checkout: Switch to the PR branch before reviewing: gh pr checkout <PR_NUMBER> (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
← 返回排行榜