1k-code-review-pr skill

---
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<boolean>((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 <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