diff --git a/.claude/skills/code-review/SKILL.md b/.claude/skills/code-review/SKILL.md new file mode 100644 index 0000000..0c97a2c --- /dev/null +++ b/.claude/skills/code-review/SKILL.md @@ -0,0 +1,398 @@ +--- +name: code-review +description: Use when reviewing AI-generated backend code, or when user says "review", "审查", "验收", "audit this code", "检查代码". +--- + +# code-review + +> All file paths in checklist references are relative to this skill's directory (`code-review/references/`). + +## Overview + +Three-layer progressive review that catches AI-generated code defects AI commonly misses — transaction consistency, idempotency, distributed locks, parameter validation, SQL performance, concurrency safety, sensitive data masking, permission control, and error fallback. Outputs a scored text report with auto-fix and manual review gates, plus an exported standalone HTML report (`code-review-report.html`). + +## Invocation + +Two paths into this skill: + +- **Auto-trigger:** After AI generates or modifies any backend code (controllers, services, data access, utilities, middleware, config), automatically enter review. +- **Manual trigger:** User says "审查", "review", "验收", "audit this code", "检查代码", "code review". + +## Skip & Risk Alert + +User may skip review with "跳过审查" or "不用审查". Before skipping, check if the change touches high-risk areas: authentication, payment, permissions, database schema changes, or data migration. If yes, issue a brief risk alert: + +> ⚠️ 检测到高风险模块变更（{area}），建议执行审查后再合并。 + +If user still chooses to skip, respect it. + +## When NOT to Use + +- **Trivial changes** — single-line fixes, comment changes, formatting-only diffs +- **Frontend-only code** — this skill is scoped for backend code review +- **User explicitly declines** — user says "跳过审查", "no review needed", "skip review" +- **Throwaway prototypes** — experimental code that won't reach CI/CD or production + +## Review Tier Configuration + +Check for `.code-review.yaml` in the project root to determine the review tier. If absent, default to **standard**. + +```yaml +# .code-review.yaml +tier: standard # fast | standard | strict +``` + +| Tier | Scope | Behavior | +|------|-------|----------| +| **fast** | Prototypes, non-critical modules | Layer 1 scan + 🔴 blocker check only. Skip 🟡 and 🔵. | +| **standard** (default) | Regular business code | All three layers. 🔴 + 🟡 issues must be fixed. | +| **strict** | Core: payment, permissions, data migration | Full review + mandatory manual review + 🔴🟡 fixes require per-item user confirmation. | + +Single-invocation override: "快速审查" → fast, "严格审查" → strict. + +## Review Flow + +### Step 0: Determine Scope + +Analyze the changes to determine review granularity: + +- **Change-level** (default): Only review AI-generated/modified code blocks and affected adjacent logic. +- **File-level**: When change involves file reorganization. +- **Service-level**: When cross-service call changes exist, associate upstream/downstream. +- **Chain-level**: When the full call chain is affected (DB → DAO → Service → Controller). + +Declare scope at the top of the review report. + +--- + +### Layer 1: Chain Decomposition & Inspection + +Decompose code into seven categories. For each, identify high-risk gaps that AI commonly misses: + +1. **Interface Layer** — Parameter validation, response conventions, HTTP status codes, rate limiting +2. **Business Layer** — Business logic alignment with requirements, state machine correctness, idempotency design, distributed locks +3. **Data Layer** — SQL injection, query performance, index usage, transaction boundaries and consistency +4. **Utility Layer** — Input validity, no side effects, error return values +5. **Error Handling** — Exception classification, fallback logic, error message sanitization, retry strategies +6. **Security** — AuthN/AuthZ, sensitive data masking, permission control, CSRF/XSS prevention +7. **Performance** — N+1 queries, caching strategy, connection pooling, batch operations + +Also verify: does not deviate from requirements; does not violate architecture conventions. + +For each category, mark: ✅ Clean / ⚠️ Issues Found / — Not Applicable. + +--- + +### Layer 2: Quantitative Quality Analysis + +Evaluate against measurable metrics. No subjective assessment. + +| Metric | How to Measure | +|--------|---------------| +| **Requirement Coverage** | Map requirement items → code implementation. Flag uncovered items. | +| **Logic Alignment** | Compare against requirement docs (or baseline if no docs). Accuracy of business rules. | +| **Exception Branch Coverage** | Count normal paths vs. exception paths. Flag gaps. | +| **SQL Performance Risk** | Detect slow queries, missing indexes, full table scans. | +| **Code Redundancy Rate** | Identify extractable common logic, copy-paste code. | +| **Vulnerability Risk Rate** | Check against OWASP / security standards. | +| **High-Risk Scenario Coverage** | Concurrency, transactions, data consistency, race conditions. | + +**No-Requirements Baseline:** When no requirement document exists, use: +1. Industry standards (OWASP, RESTful conventions, SQL best practices) +2. Similar business best practices (by project type: ecommerce/social/SaaS) +3. Previously accumulated prompt knowledge +4. Existing architecture conventions in the project + +Mark the report with "No requirements doc — reviewed against generic baseline" and reduce weights of requirement coverage and logic alignment. + +**Output Classification:** + +- **Ready** — Passes review, ready for use +- **Needs Fix** — Has issues but fixable, with fix plan attached +- **Unusable** — Cannot be used, needs rewrite, with reason stated + +--- + +### Layer 3: Optimization & Acceptance Standards + +#### Standards Enforcement + +- **Code standards** — naming, structure, comments +- **Security standards** — input validation, output encoding, permission checks +- **Performance standards** — query optimization, caching, async processing + +#### Tool Scanning (if applicable to the language/ecosystem) + +- Code scanning (linter, static analysis) — run and report +- SQL check (EXPLAIN, slow queries, index suggestions) — run and report +- Vulnerability detection (dependency vulnerabilities, injection points) — run and report + +#### Manual Review Checklist + +High-risk modules are force-flagged for manual review. Load the specialized checklist from `references/manual-review/{module}.md` when relevant: + +| Module | File | +|--------|------| +| Payment | `references/manual-review/payment.md` | +| Order | `references/manual-review/order.md` | +| Inventory | `references/manual-review/inventory.md` | +| Permission | `references/manual-review/permission.md` | +| Distributed Lock | `references/manual-review/distributed-lock.md` | +| Data Migration | `references/manual-review/data-migration.md` | + +#### Knowledge Accumulation + +After each review, identify patterns for prompt accumulation: +- **Business Rules** (same module repeating same logic errors) → suggest accumulation +- **Historical Pitfalls** (same error pattern ≥2 occurrences) → suggest accumulation +- **Architecture Constraints** (violations of project architecture conventions) → suggest accumulation + +Propose accumulation items at the end of the report. User confirms before adding to prompt library. + +**Accumulation workflow:** +```text +Review findings → auto-extract high-frequency/high-risk issues + → ask user to confirm (avoid noise) + → record to prompt library (categorized: business rules / pitfalls / architecture constraints) + → next AI generation auto-loads matched prompts + → compare issue rates over time to verify effectiveness +``` + +Unconfirmed accumulations do not auto-activate. + +#### Acceptance Gate + +```text +AI draft → Skill review (auto) → Manual review & fix → Code review → Deploy + ↑ + AI raw code must NOT go directly to production +``` + +--- + +### Step 4: Export HTML Report + +After the review is complete and all fixes are applied, generate a standalone HTML report. Follow the instructions in the [HTML Report Export](#html-report-export) section below. + +--- + +## Severity Levels + +| Level | Meaning | Examples | +|-------|---------|----------| +| 🔴 Blocker | Must fix immediately. Cannot merge/deploy. | SQL injection, hardcoded secrets, missing transaction causing data inconsistency, auth bypass | +| 🟡 Major | Should fix before merge. | Missing input validation, missing error fallback, N+1 query, unmasked sensitive data | +| 🔵 Minor | Can optimize later. | Non-standard naming, duplicate code, missing comments, optimizable loop | + +**Rule:** If any 🔴 blocker exists → verdict is ❌ FAIL regardless of other scores. + +--- + +## Auto-Fix Strategy + +Fix order: 🔴 Blocker → 🟡 Major → 🔵 Minor + +### Fix Boundary Rules + +| Issue Type | Auto-Fix | Manual Only | +|-----------|----------|-------------| +| Missing parameter validation | ✅ Add validation | — | +| SQL injection | ✅ Parameterized queries | — | +| N+1 queries | ✅ Batch queries | — | +| Sensitive data masking | ✅ Apply masking | — | +| Transaction boundaries | ⚠️ Single-table/single-DB transactions | Cross-DB/nested/distributed | +| Idempotency design | — | ❌ Business confirmation needed | +| Distributed locks | — | ❌ Architecture confirmation needed | +| Permission model | — | ❌ Product confirmation needed | + +### Post-Fix Regression + +After each fix: +1. **Regression scope:** Re-run Layer 1 scan on all modified files +2. **Re-check metrics:** Re-run affected Layer 2 metrics +3. **Rollback condition:** If fix introduces new 🔴 or 🟡 issue → auto-rollback, mark as "Fix Failed", list under manual review +4. **Fix report:** Each fix marked "Fix Success / Fix Failed / Rolled Back" +5. **Re-export HTML:** Regenerate `code-review-report.html` after all fixes in the batch are applied + +### Before Fixing + +Always present the fix plan to the user and wait for confirmation before executing. + +--- + +## HTML Report Export + +After all review layers and fixes are complete, generate a standalone HTML report file. + +### HTML Generation + +1. Load the HTML template from `references/report-template.html` +2. Populate the template placeholders (`{{PLACEHOLDER}}`) with actual review data +3. Write the output as `code-review-report.html` in the project root directory + +### Template Placeholders + +| Placeholder | Description | +|-------------|-------------| +| `{{SCOPE}}` | Review scope level | +| `{{TIER}}` | Review tier | +| `{{TIMESTAMP}}` | Current timestamp | +| `{{FILES}}` | Comma-separated file list | +| `{{BASELINE}}` | Review baseline (requirements doc / generic) | +| `{{VERDICT}}` | `✅ PASS` or `❌ FAIL` | +| `{{VERDICT_CLASS}}` | CSS class: `pass` or `fail` | +| `{{L1_*}}` | Layer 1 status: `✅ Clean` / `⚠️ Issues Found` / `— N/A` | +| `{{L1_*_CLASS}}` | CSS class: `status-clean` / `status-issues` / `status-na` | +| `{{REQ_COVERAGE}}` | Requirement coverage percentage | +| `{{REQ_COV_CLASS}}` | CSS class: `metric-good` / `metric-warn` | +| `{{LOGIC_ALIGNMENT}}` | Logic alignment percentage | +| `{{EXC_COVERAGE}}` | Exception coverage percentage | +| `{{EXC_COV_CLASS}}` | CSS class | +| `{{SQL_RISK}}` | SQL risk level | +| `{{SQL_RISK_CLASS}}` | CSS class | +| `{{REDUNDANCY_RATE}}` | Redundancy rate | +| `{{VULN_RISK}}` | Vulnerability rate | +| `{{VULN_RISK_CLASS}}` | CSS class | +| `{{HRISK_COVERAGE}}` | High-risk scenario coverage | +| `{{HRISK_CLASS}}` | CSS class | +| `{{READY_COUNT}}` / `{{NEEDS_FIX_COUNT}}` / `{{UNUSABLE_COUNT}}` | Classification counts | +| `{{BLOCKER_COUNT}}` / `{{MAJOR_COUNT}}` / `{{MINOR_COUNT}}` | Severity counts | +| `{{ISSUES_LIST}}` | HTML markup for all issues, each as a `.issue` div | +| `{{MANUAL_REVIEW_SECTION}}` | HTML markup for manual review items, or empty string | +| `{{ACCUMULATION_SECTION}}` | HTML markup for knowledge accumulation suggestions, or empty string | + +### Issue HTML Format + +Each issue in `{{ISSUES_LIST}}` should follow this structure: + +```html +
+ 🔴 + [Data] + Missing transaction +
→ order-service.ts:156
+
Fix: Added transaction wrap
+
+``` + +Use `issue-blocker` / `issue-major` / `issue-minor` and `tag-blocker` / `tag-major` / `tag-minor` accordingly. + +### Manual Review HTML Format + +```html +
+

Manual Review Required

+
Payment callback idempotency
+
Distributed lock timeout
+
+``` + +### Knowledge Accumulation HTML Format + +```html +
+

Knowledge Accumulation Suggestions

+
Business Rules — order status state machine errors
+
Historical Pitfalls — missing transaction wrap in batch operations
+
+``` + +### File Output + +The generated HTML file is saved as `code-review-report.html` in the project root directory. It is a fully self-contained file with all styles inlined — no external dependencies. + +--- + +## Report Format (Text) + +```text +╔══════════════════════════════════╗ +║ Backend Code Review Report ║ +╠══════════════════════════════════╣ +║ Scope: {change-level/file-level/service-level/chain-level} ║ +║ Files: {file list} ║ +║ Time: {timestamp} ║ +║ Tier: {fast/standard/strict} ║ +║ Verdict: ✅ PASS / ❌ FAIL ║ +╠══════════════════════════════════╣ +║ Layer 1 · Chain Decomposition ║ +║ Interface: ✅ Business: ⚠️ ║ +║ Data: ✅ Utility: ✅ ║ +║ Error: ⚠️ Security: ✅ ║ +║ Performance: ✅ ║ +╠══════════════════════════════════╣ +║ Layer 2 · Quantitative Metrics ║ +║ Requirement Coverage: 85% (3/5 uncovered) ║ +║ Logic Alignment: 90% ║ +║ Exception Coverage: 60% ⚠️ ║ +║ SQL Risk: Low ║ +║ Redundancy Rate: 12% ║ +║ Vulnerability Rate: 0% ║ +║ High-Risk Coverage: Missing concurrency ║ +╠══════════════════════════════════╣ +║ Classification ║ +║ Ready: 3 Needs Fix: 2 Unusable: 0 ║ +╠══════════════════════════════════╣ +║ 🔴 Blocker: 1 🟡 Major: 3 🔵 Minor: 4 ║ +╠══════════════════════════════════╣ +║ Issues: ║ +║ 🔴 [Data] Missing transaction ║ +║ → order-service.ts:156 ║ +║ → Fix: Added transaction wrap ║ +║ 🟡 [Security] Unvalidated input ║ +║ → auth.ts:42 ║ +║ → Fix: Added validation ║ +║ ... ║ +║ Manual Review Required: ║ +║ → Payment callback idempotency ║ +║ → Distributed lock timeout ║ +╚══════════════════════════════════╝ +``` + +### Report Interaction + +- **Issue navigation:** Issue line numbers link to code location +- **Fix preview:** Fix plan shows diff preview +- **Batch operations:** Confirm/deny fixes by severity level; batch-skip 🔵 items +- **Per-item confirmation:** 🔴 and high-risk 🟡 fixes confirmed one at a time + +--- + +## Language Adaptation + +Detect project language/framework and load the corresponding checklist from `references/`: + +| Language/Framework | Checklist File | +|-------------------|---------------| +| Java + Spring Boot | `references/java-spring.md` | +| Python + Django | `references/python-django.md` | +| Python + FastAPI | `references/python-fastapi.md` | +| Node.js + Express | `references/node-express.md` | +| Go + Gin | `references/go-gin.md` | +| No match | Use `references/review-checklist.md` (generic) | + +--- + +## Constraints + +- **No silent modifications:** Present fix plan before executing. Get user confirmation. +- **No fixes on FAIL:** When 🔴 blockers cause ❌ FAIL, report first and wait for user decision. +- **Context-aware:** With requirement docs → full review. Without → use "no-requirements baseline", reduce requirement/logic metric weights. +- **Historical learning:** High-frequency/high-risk findings auto-suggested for prompt accumulation. User confirms before activation. +- **Language-adaptive:** Auto-detect project language/framework, load matching checklist. Fall back to generic. +- **Config-driven:** `.code-review.yaml` controls review tier. No hardcoded rules. + +--- + +## Common Mistakes + +| Mistake | Fix | +|---------|-----| +| Rushing review without checking each layer | Complete all three layers in order. Layer 1 catches structural issues, Layer 2 catches metrics gaps, Layer 3 catches process gaps. | +| Skipping no-requirements baseline and guessing requirements | When no req docs exist, explicitly use the 4-part baseline (industry standards, business practices, accumulated prompts, architecture conventions). Mark the report accordingly. | +| Fixing 🔵 minor issues before 🔴 blockers | Fix order is strict: 🔴 → 🟡 → 🔵. A 🔴 blocker means ❌ FAIL — fix it first or don't fix at all. | +| Applying fixes without user confirmation | Always present the fix plan and wait for confirmation before executing. | +| Forgetting to mark high-risk modules for manual review | Payment, order, inventory, permissions, distributed locks, and data migration always require manual review. Load the corresponding checklist. | +| Loading the wrong language checklist | Auto-detect language/framework from project config (package.json, pom.xml, go.mod, etc.). Fall back to generic checklist if detection fails. | +| Omitting the "No requirements doc" annotation | Always state the review baseline in the report header so readers know what standards were applied. | +| Forgetting to export the HTML report after fixes | After all fixes are applied, always generate the HTML report (`code-review-report.html`) so the final output reflects the latest state. | diff --git a/.claude/skills/code-review/references/go-gin.md b/.claude/skills/code-review/references/go-gin.md new file mode 100644 index 0000000..d4e302c --- /dev/null +++ b/.claude/skills/code-review/references/go-gin.md @@ -0,0 +1,48 @@ +# Go Gin Review Checklist + +Extends the generic checklist with Go/Gin-specific items. + +## Interface Layer (Gin Handlers) + +- [ ] `ShouldBindJSON` / `ShouldBindQuery` with error handling +- [ ] Binding structs have `binding:"required"` tags +- [ ] Custom validators registered with `binding.Validator` +- [ ] Response helpers used consistently (not raw `c.JSON` everywhere) +- [ ] Middleware applied at appropriate scope (global vs group vs handler) + +## Business Layer + +- [ ] Business logic in service structs with interfaces +- [ ] Context propagation (`context.Context`) through all layers +- [ ] Dependency injection via constructor, not global variables + +## Data Layer (GORM / sqlx / database/sql) + +- [ ] GORM: `Preload()` instead of lazy loading in loops +- [ ] GORM: `Where("field = ?", value)` — parameterized queries +- [ ] database/sql: prepared statements with placeholders +- [ ] Connection pool: `SetMaxOpenConns`, `SetMaxIdleConns`, `SetConnMaxLifetime` +- [ ] `rows.Close()` always called (or use `defer`) + +## Error Handling + +- [ ] Errors wrapped with `fmt.Errorf("context: %w", err)` for traceability +- [ ] `errors.Is()` and `errors.As()` for error type checking +- [ ] No `panic()` in request handlers (use recovery middleware) +- [ ] Gin recovery middleware configured + +## Security + +- [ ] `gin-contrib/cors` with specific origins +- [ ] Rate limiting middleware (e.g., `gin-contrib/limiter`) +- [ ] JWT or session middleware for auth +- [ ] Secrets from environment, never committed +- [ ] `gin.SetMode(gin.ReleaseMode)` in production + +## Performance + +- [ ] Goroutine pools for concurrent operations (avoid unbounded goroutines) +- [ ] `sync.Pool` for frequently allocated objects +- [ ] Database query limits on all SELECTs +- [ ] `context.WithTimeout` for all external calls +- [ ] JSON serialization with `json:"-"` on sensitive fields diff --git a/.claude/skills/code-review/references/java-spring.md b/.claude/skills/code-review/references/java-spring.md new file mode 100644 index 0000000..9ade4b6 --- /dev/null +++ b/.claude/skills/code-review/references/java-spring.md @@ -0,0 +1,48 @@ +# Java Spring Boot Review Checklist + +Extends the generic checklist with Java/Spring-specific items. + +## Interface Layer (Spring MVC) + +- [ ] `@RestController` methods have `@Valid` on request bodies +- [ ] Custom validators implement `ConstraintValidator` correctly +- [ ] `@ExceptionHandler` or `@ControllerAdvice` for global error handling +- [ ] `@ResponseStatus` used appropriately on custom exceptions +- [ ] DTOs use records or Lombok `@Data` — not exposing entities directly +- [ ] `@RequestMapping` produces/consumes specified + +## Business Layer (Spring Service) + +- [ ] `@Transactional` on service methods that modify multiple tables +- [ ] `@Transactional(rollbackFor = Exception.class)` — not just RuntimeException +- [ ] Transaction propagation set correctly (REQUIRED vs REQUIRES_NEW) +- [ ] No `@Transactional` on private methods (proxy limitation) + +## Data Layer (Spring Data JPA / MyBatis) + +- [ ] JPA: `@Entity` classes have proper `equals()` and `hashCode()` +- [ ] JPA: No eager fetching on `@ManyToOne` without explicit need +- [ ] JPA: `@Query` with nativeQuery=false by default (prevent SQL injection) +- [ ] MyBatis: All SQL uses `#{}` not `${}` for user input +- [ ] Connection pool settings reviewed (HikariCP defaults usually fine) + +## Error Handling + +- [ ] Checked exceptions either handled or declared +- [ ] `try-with-resources` for AutoCloseable resources +- [ ] No `catch (Exception e) { e.printStackTrace(); }` — use logger instead + +## Security (Spring Security) + +- [ ] `SecurityFilterChain` configured correctly +- [ ] CSRF protection enabled for state-changing endpoints +- [ ] `@PreAuthorize` or `@Secured` on protected methods +- [ ] Password encoding uses `BCryptPasswordEncoder` or better +- [ ] No secrets in `application.properties` — use env vars or vault + +## Performance + +- [ ] `@Async` used for non-blocking operations with proper thread pool config +- [ ] `@Cacheable` with TTL and eviction strategy +- [ ] JPA: `@BatchSize` or batch insert for bulk operations +- [ ] RestTemplate/WebClient timeouts configured diff --git a/.claude/skills/code-review/references/manual-review/data-migration.md b/.claude/skills/code-review/references/manual-review/data-migration.md new file mode 100644 index 0000000..9b115c0 --- /dev/null +++ b/.claude/skills/code-review/references/manual-review/data-migration.md @@ -0,0 +1,25 @@ +# Data Migration — Manual Review Checklist + +## Rollback Plan +- [ ] Rollback script tested and ready before migration +- [ ] Rollback handles partial migration state +- [ ] Rollback is idempotent +- [ ] Database backup taken before migration + +## Consistency Verification +- [ ] Migration verification query defined (row count, checksum, sample compare) +- [ ] Old and new data validated before old table dropped +- [ ] Foreign key constraints preserved +- [ ] Indexes migrated or recreated + +## Large Table Strategy +- [ ] Batch processing with configurable batch size +- [ ] Migration doesn't lock table for extended period +- [ ] Progress tracking (how many rows processed) +- [ ] Rate limiting to avoid DB overload +- [ ] Dry-run mode tested before actual migration + +## Idempotency +- [ ] Migration script is idempotent (can be re-run safely) +- [ ] Uses INSERT ON CONFLICT / INSERT IGNORE / MERGE as appropriate +- [ ] Migration tracks what has been processed (checkpoint table) diff --git a/.claude/skills/code-review/references/manual-review/distributed-lock.md b/.claude/skills/code-review/references/manual-review/distributed-lock.md new file mode 100644 index 0000000..3fd58bb --- /dev/null +++ b/.claude/skills/code-review/references/manual-review/distributed-lock.md @@ -0,0 +1,23 @@ +# Distributed Lock — Manual Review Checklist + +## Timeout Strategy +- [ ] Lock TTL defined and justified (not arbitrary) +- [ ] TTL exceeds maximum expected operation time +- [ ] Watchdog/refresh mechanism if operation may exceed TTL +- [ ] Lock auto-releases on process crash (not permanent deadlock) + +## Release Mechanism +- [ ] Lock released by the same instance that acquired it (ownership check) +- [ ] Release is atomic (check-then-release is not atomic — use Lua script or compare-and-delete) +- [ ] Lock release in finally/defer block guaranteed to execute + +## Retry Strategy +- [ ] Retry with exponential backoff, not infinite retry +- [ ] Maximum retry count or timeout defined +- [ ] Failed lock acquisition returns clear error (not silent fallback) + +## Granularity & Deadlock +- [ ] Lock key granularity appropriate (per-resource, not global) +- [ ] Multiple lock acquisition order consistent (prevents deadlock) +- [ ] No nested locks on same resource +- [ ] Lock contention monitored/metrics available diff --git a/.claude/skills/code-review/references/manual-review/inventory.md b/.claude/skills/code-review/references/manual-review/inventory.md new file mode 100644 index 0000000..a989f9a --- /dev/null +++ b/.claude/skills/code-review/references/manual-review/inventory.md @@ -0,0 +1,21 @@ +# Inventory Module — Manual Review Checklist + +## Concurrency Safety +- [ ] Stock deduction uses atomic operations (optimistic lock, Redis decr, DB atomic update) +- [ ] No SELECT + UPDATE gap for stock operations +- [ ] Lock granularity: per-SKU, not global lock on inventory table + +## Pre-sale / Reservation +- [ ] Pre-sale inventory separated from actual inventory +- [ ] Reservation timeout and auto-release mechanism +- [ ] Reservation-to-actual-deduction transition is atomic + +## Audit Trail +- [ ] Every stock change logged with: SKU, quantity, operation type, order ID, timestamp +- [ ] Stock reconciliation queryable from audit log +- [ ] No direct DB updates bypassing the service layer + +## Stock Recovery +- [ ] Order cancellation triggers stock recovery +- [ ] Recovery is idempotent (double-cancel doesn't double-recover) +- [ ] Recovery handles edge case: stock recovered after SKU deleted diff --git a/.claude/skills/code-review/references/manual-review/order.md b/.claude/skills/code-review/references/manual-review/order.md new file mode 100644 index 0000000..eb51807 --- /dev/null +++ b/.claude/skills/code-review/references/manual-review/order.md @@ -0,0 +1,23 @@ +# Order Module — Manual Review Checklist + +## State Machine +- [ ] All order states defined and documented +- [ ] Valid transitions enforced (no impossible state changes) +- [ ] Terminal states properly handled (no further mutations) +- [ ] State change audit log exists + +## Concurrent Deduction +- [ ] Inventory deduction uses optimistic lock (version number) or pessimistic lock (SELECT FOR UPDATE) +- [ ] No race condition between check and deduction (not SELECT → check → UPDATE) +- [ ] Oversell prevention verified + +## Timeout Cancellation +- [ ] Unpaid order timeout mechanism (scheduled job, delayed queue, or TTL) +- [ ] Cancellation releases held inventory +- [ ] Cancellation releases held coupons/promotions +- [ ] Timeout is configurable, not hardcoded + +## Distributed Transaction +- [ ] Cross-service operations (order + inventory + payment) have compensation logic +- [ ] Saga pattern or eventual consistency defined +- [ ] Failure recovery documented (what happens if order succeeds but payment fails) diff --git a/.claude/skills/code-review/references/manual-review/payment.md b/.claude/skills/code-review/references/manual-review/payment.md new file mode 100644 index 0000000..1ffed7b --- /dev/null +++ b/.claude/skills/code-review/references/manual-review/payment.md @@ -0,0 +1,26 @@ +# Payment Module — Manual Review Checklist + +## Callback Idempotency +- [ ] Is payment callback idempotent? (duplicate notification won't double-charge) +- [ ] Idempotency key sourced from payment provider's transaction ID +- [ ] Idempotency check happens before any state change + +## Amount Precision +- [ ] All monetary amounts use integer cents or decimal with fixed precision +- [ ] No floating-point arithmetic in payment calculations +- [ ] Rounding strategy defined and consistent (round half up vs floor) + +## Reconciliation +- [ ] Reconciliation logic matches payment provider's settlement model +- [ ] Discrepancy thresholds defined (when to auto-adjust vs flag for manual review) +- [ ] Reconciliation runs are idempotent + +## Refund State Machine +- [ ] All refund states defined (pending, processing, completed, failed) +- [ ] Transition rules enforced (can't refund a refunded payment) +- [ ] Partial refund logic correct (remaining refundable amount tracked) + +## Third-Party Timeout +- [ ] Payment provider timeout handled (request timed out ≠ payment failed) +- [ ] Retry strategy for querying payment status +- [ ] Circuit breaker or backoff for provider outages diff --git a/.claude/skills/code-review/references/manual-review/permission.md b/.claude/skills/code-review/references/manual-review/permission.md new file mode 100644 index 0000000..272c8d7 --- /dev/null +++ b/.claude/skills/code-review/references/manual-review/permission.md @@ -0,0 +1,28 @@ +# Permission Module — Manual Review Checklist + +## RBAC Model +- [ ] Roles and permissions clearly defined and documented +- [ ] No permission inheritance cycles +- [ ] Super admin role cannot be accidentally assigned to regular users +- [ ] Role changes are logged + +## Authorization Enforcement +- [ ] Every protected endpoint checks permissions (not just login state) +- [ ] Resource-level permissions: user can only access own data +- [ ] Admin endpoints segregated from user endpoints (different middleware chain) + +## Horizontal Escalation +- [ ] User A cannot access User B's resources by changing ID in request +- [ ] Resource ownership verified on every request (not just at list level) +- [ ] Bulk operations check permissions per-item, not just the batch endpoint + +## Vertical Escalation +- [ ] Regular users cannot perform admin actions +- [ ] Role checks happen server-side (not trusting client-claimed role) +- [ ] Privileged operations require re-authentication + +## Token Strategy +- [ ] Access token: short-lived (15-60 min) +- [ ] Refresh token: long-lived, stored securely (httpOnly cookie) +- [ ] Token rotation on refresh (old refresh token invalidated) +- [ ] Token revocation mechanism (logout invalidates all user tokens) diff --git a/.claude/skills/code-review/references/node-express.md b/.claude/skills/code-review/references/node-express.md new file mode 100644 index 0000000..ccf34ba --- /dev/null +++ b/.claude/skills/code-review/references/node-express.md @@ -0,0 +1,45 @@ +# Node.js Express Review Checklist + +Extends the generic checklist with Node.js/Express-specific items. + +## Interface Layer (Express Routes) + +- [ ] Request validation middleware (Joi, Zod, express-validator) +- [ ] Response format consistent across all routes +- [ ] `express.json()` with size limit configured +- [ ] Route handlers are async with try/catch or wrapped with async handler + +## Business Layer + +- [ ] Business logic in service modules, not in route handlers +- [ ] Dependency injection or factory pattern for testability +- [ ] Config loaded from environment, not hardcoded + +## Data Layer (Sequelize / Prisma / Knex) + +- [ ] Sequelize: eager loading uses `include` with proper scoping +- [ ] Prisma: `select` or `include` to avoid over-fetching +- [ ] Raw queries always parameterized (`$1`, `?` placeholders) +- [ ] Connection pool configured (`max`, `min`, `idleTimeoutMillis`) + +## Error Handling + +- [ ] Global error handler middleware `(err, req, res, next)` +- [ ] Async route handlers wrapped (express-async-errors or manual wrapper) +- [ ] Error responses never expose stack traces in production +- [ ] `uncaughtException` and `unhandledRejection` handlers + +## Security + +- [ ] `helmet` middleware configured +- [ ] `cors` with specific origin allowlist +- [ ] `express-rate-limit` on auth and sensitive endpoints +- [ ] `httpOnly`, `secure`, `sameSite` flags on cookies +- [ ] No `eval()` or `Function()` with user input + +## Performance + +- [ ] Compression middleware (`compression`) +- [ ] Database queries have limits and pagination +- [ ] Heavy operations offloaded to worker threads or queue +- [ ] Static assets served via CDN or reverse proxy, not Express diff --git a/.claude/skills/code-review/references/python-django.md b/.claude/skills/code-review/references/python-django.md new file mode 100644 index 0000000..31c8ea8 --- /dev/null +++ b/.claude/skills/code-review/references/python-django.md @@ -0,0 +1,51 @@ +# Python Django Review Checklist + +Extends the generic checklist with Django-specific items. + +## Interface Layer (Django Views / DRF) + +- [ ] DRF: Serializers validate all input fields explicitly +- [ ] DRF: `serializer.is_valid(raise_exception=True)` used consistently +- [ ] DRF: Permission classes set on every ViewSet +- [ ] Function-based views have `@require_http_methods` decorator +- [ ] Django forms have `clean_*` methods for custom validation + +## Business Layer + +- [ ] Business logic in services/managers, not in views +- [ ] `transaction.atomic()` wraps multi-table writes +- [ ] `select_for_update()` used for pessimistic locking where needed +- [ ] `@transaction.atomic` decorator on methods with multiple DB writes + +## Data Layer (Django ORM) + +- [ ] `select_related()` for foreign key access in loops +- [ ] `prefetch_related()` for many-to-many access in loops +- [ ] `QuerySet.exists()` instead of `len(queryset)` for existence checks +- [ ] `QuerySet.count()` instead of `len(queryset)` for counts +- [ ] `QuerySet.only()` or `defer()` for large tables with unused columns +- [ ] Migrations are generated and reviewed (`makemigrations` + `check --deploy`) + +## Error Handling + +- [ ] Custom exception middleware or DRF exception handler +- [ ] `DEBUG = False` in production (prevents stack trace leaks) +- [ ] `LOGGING` configured with appropriate levels per module +- [ ] Sentry or similar error tracking integrated + +## Security (Django) + +- [ ] `SECRET_KEY` not hardcoded — loaded from environment +- [ ] `ALLOWED_HOSTS` properly restricted +- [ ] `SECURE_SSL_REDIRECT`, `SESSION_COOKIE_SECURE`, `CSRF_COOKIE_SECURE` enabled +- [ ] `X_FRAME_OPTIONS`, `SECURE_HSTS_SECONDS` set +- [ ] CSRF middleware enabled (default) +- [ ] File upload: `FILE_UPLOAD_MAX_MEMORY_SIZE` and validation + +## Performance + +- [ ] Celery tasks for async operations with proper retry config +- [ ] Django cache framework used with appropriate backend (Redis/Memcached) +- [ ] Database indexes on filtered/sorted fields +- [ ] `iterator()` for large querysets to avoid memory issues +- [ ] `bulk_create()` / `bulk_update()` for batch operations diff --git a/.claude/skills/code-review/references/python-fastapi.md b/.claude/skills/code-review/references/python-fastapi.md new file mode 100644 index 0000000..9f80777 --- /dev/null +++ b/.claude/skills/code-review/references/python-fastapi.md @@ -0,0 +1,47 @@ +# Python FastAPI Review Checklist + +Extends the generic checklist with FastAPI-specific items. + +## Interface Layer (FastAPI Routes) + +- [ ] Pydantic models used for request/response schemas +- [ ] Pydantic validators (`@validator`, `@field_validator`) for custom logic +- [ ] `response_model` specified on all endpoints +- [ ] Query/Path parameters have `title`, `description`, `examples` +- [ ] `status_code` set explicitly on non-200 responses +- [ ] Dependency injection used for shared logic (auth, DB session) + +## Business Layer + +- [ ] Business logic separated from route handlers +- [ ] `Depends(get_db)` pattern for database session management +- [ ] Background tasks (`BackgroundTasks`) used for non-blocking operations + +## Data Layer (SQLAlchemy / asyncpg) + +- [ ] SQLAlchemy: session management via dependency injection +- [ ] SQLAlchemy: `selectinload()` / `joinedload()` for eager loading +- [ ] SQLAlchemy async: proper async session usage (`AsyncSession`) +- [ ] Raw SQL: always parameterized, never f-string interpolation + +## Error Handling + +- [ ] Custom exception handlers registered (`@app.exception_handler`) +- [ ] HTTPException with appropriate status codes +- [ ] Validation errors return structured response (Pydantic error format) +- [ ] Unhandled exceptions caught by global handler + +## Security + +- [ ] `CORSMiddleware` with specific origins, not `allow_origins=["*"]` +- [ ] OAuth2 / JWT integration via FastAPI security utilities +- [ ] `Security()` or `Depends()` for auth checks (not manual header parsing) +- [ ] Rate limiting middleware (e.g., slowapi) +- [ ] Secrets loaded from environment or secret manager + +## Performance + +- [ ] Async endpoints (`async def`) where I/O-bound +- [ ] `httpx.AsyncClient` with connection pooling for external API calls +- [ ] Response compression middleware (`GZipMiddleware`) +- [ ] Database connection pool size tuned diff --git a/.claude/skills/code-review/references/report-template.html b/.claude/skills/code-review/references/report-template.html new file mode 100644 index 0000000..925dd98 --- /dev/null +++ b/.claude/skills/code-review/references/report-template.html @@ -0,0 +1,211 @@ + + + + + +Code Review Report + + + +
+ + +
+

🔍 Backend Code Review Report

+
+
Scope{{SCOPE}}
+
Tier{{TIER}}
+
Time{{TIMESTAMP}}
+
Files{{FILES}}
+
Baseline{{BASELINE}}
+
+ Verdict + {{VERDICT}} +
+
+
+ + +
+

Layer 1 · Chain Decomposition

+
+
Interface{{L1_INTERFACE}}
+
Business{{L1_BUSINESS}}
+
Data{{L1_DATA}}
+
Utility{{L1_UTILITY}}
+
Error Handling{{L1_ERROR}}
+
Security{{L1_SECURITY}}
+
Performance{{L1_PERFORMANCE}}
+
+
+ + +
+

Layer 2 · Quantitative Metrics

+ + + + + + + + + + + +
MetricValue
Requirement Coverage{{REQ_COVERAGE}}
Logic Alignment{{LOGIC_ALIGNMENT}}
Exception Branch Coverage{{EXC_COVERAGE}}
SQL Performance Risk{{SQL_RISK}}
Code Redundancy Rate{{REDUNDANCY_RATE}}
Vulnerability Risk Rate{{VULN_RISK}}
High-Risk Scenario Coverage{{HRISK_COVERAGE}}
+
+ + +
+

Classification

+
+
{{READY_COUNT}}
Ready
+
{{NEEDS_FIX_COUNT}}
Needs Fix
+
{{UNUSABLE_COUNT}}
Unusable
+
+
+ + +
+

Severity Summary

+
+ 🔴 Blocker: {{BLOCKER_COUNT}} + 🟡 Major: {{MAJOR_COUNT}} + 🔵 Minor: {{MINOR_COUNT}} +
+ + {{ISSUES_LIST}} +
+ + + {{MANUAL_REVIEW_SECTION}} + + + {{ACCUMULATION_SECTION}} + + +
+ Generated by code-review skill · {{TIMESTAMP}} +
+ +
+ + diff --git a/.claude/skills/code-review/references/review-checklist.md b/.claude/skills/code-review/references/review-checklist.md new file mode 100644 index 0000000..cddf0a0 --- /dev/null +++ b/.claude/skills/code-review/references/review-checklist.md @@ -0,0 +1,95 @@ +# Generic Code Review Checklist + +Language-agnostic checklist covering the seven code categories across eight quality metrics. Used when no language-specific checklist matches. + +--- + +## 1. Interface Layer + +- [ ] All public endpoints have explicit parameter validation +- [ ] Response format is consistent across endpoints +- [ ] HTTP status codes are semantically correct (2xx success, 4xx client error, 5xx server error) +- [ ] Rate limiting considered for public-facing endpoints +- [ ] Request/response DTOs are complete (no partial exposure of internal models) +- [ ] Content-Type headers are set correctly +- [ ] Pagination implemented for list endpoints + +## 2. Business Layer + +- [ ] Business logic matches requirements (check each rule) +- [ ] State machines have all transitions defined (no impossible states) +- [ ] Idempotency designed for non-idempotent operations (create, update with side effects) +- [ ] Distributed lock usage reviewed for correctness (lock key, timeout, release) +- [ ] Business exceptions are domain-specific, not generic +- [ ] No business logic leaks into interface or data layer + +## 3. Data Layer + +- [ ] All queries use parameterized statements (no string concatenation) +- [ ] Every query has appropriate indexes (check EXPLAIN output) +- [ ] Transaction boundaries are correct (ACID where needed, rollback on error) +- [ ] No full table scans on large tables +- [ ] Batch operations used for bulk inserts/updates +- [ ] Connection pooling configured +- [ ] N+1 queries eliminated (use JOIN or batch fetch) +- [ ] Sensitive data encrypted at rest if required + +## 4. Utility Layer + +- [ ] All utility functions validate inputs +- [ ] Utility functions are pure (no side effects) unless explicitly documented +- [ ] Error states return explicit values, not null/undefined without documentation +- [ ] Date/time handling uses consistent timezone approach +- [ ] String operations handle Unicode and edge cases (empty, very long) + +## 5. Error Handling + +- [ ] Exceptions are categorized (not all caught as generic Exception/Error) +- [ ] Every external call (DB, API, file I/O) has a fallback or error boundary +- [ ] Error messages returned to clients do not leak internal details +- [ ] Retry strategy exists for transient failures (network, deadlock) +- [ ] Logging includes enough context for debugging (request ID, user, operation) +- [ ] Circuit breaker considered for critical external dependencies + +## 6. Security + +- [ ] Authentication enforced on all protected endpoints +- [ ] Authorization checked per operation, not just per endpoint +- [ ] User input validated and sanitized (XSS, injection prevention) +- [ ] Sensitive data (passwords, tokens, PII) never logged or exposed in responses +- [ ] CSRF protection for state-changing operations (if cookie-based auth) +- [ ] CORS configured restrictively, not wildcard +- [ ] Secrets (API keys, DB credentials) loaded from environment, never hardcoded +- [ ] File upload has size limits and type validation + +## 7. Performance + +- [ ] No N+1 query patterns +- [ ] Appropriate caching strategy (with invalidation logic) +- [ ] Heavy operations are async where possible +- [ ] Database queries have reasonable LIMIT clauses +- [ ] Large responses support pagination or streaming +- [ ] Connection pooling and timeouts configured +- [ ] Memory usage considered for large datasets + +--- + +## Quantitative Metrics Reference + +| Metric | Check | +|--------|-------| +| Requirement Coverage | Every requirement item mapped to ≥1 code location | +| Logic Alignment | Business rules implemented exactly as specified | +| Exception Branch Coverage | ≥1 error path for every 2 happy paths (baseline) | +| SQL Performance Risk | No full scans on tables > 10k rows; queries have indexes | +| Code Redundancy Rate | < 10% of code is copy-pasted (same logic in ≥3 places) | +| Vulnerability Risk Rate | 0 known vulnerability patterns (OWASP Top 10) | +| High-Risk Scenario Coverage | Concurrency, transactions, idempotency addressed for stateful ops | + +--- + +## Classification Rules + +- **Ready:** All 🔴 and 🟡 items passed. 🔵 items optional. +- **Needs Fix:** 🟡 items found but fixable. No 🔴 items. +- **Unusable:** 🔴 items found or logic fundamentally incorrect. diff --git a/.code-review.yaml b/.code-review.yaml new file mode 100644 index 0000000..dad5711 --- /dev/null +++ b/.code-review.yaml @@ -0,0 +1,19 @@ +# Code Review 配置 +# 生效范围: cobol-java-v3 项目所有后端代码 +# 当 AI 生成或修改 Python/COBOL 代码后自动执行 review + +tier: standard # fast | standard | strict + +# strict 模式额外配置 +strict: + # 高风险模块 — 修改这些目录必须手动确认 review + high_risk_dirs: + - cobol_testgen/core.py + - cobol_testgen/coverage.py + - cobol_testgen/cond.py + # 自动触发 review 的文件扩展名 + watch_extensions: + - .py + - .cbl + - .cpy + - .lark diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..5b21c8f --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,38 @@ +# COBOL Test Data Generator — 项目指令 + +## 代码审查（自动触发） + +本项目集成了 `code-review` skill（位于 `.claude/skills/code-review/`）。 + +### 触发规则 + +- **自动触发**：AI 生成或修改 `.py` / `.cbl` / `.cpy` / `.lark` 文件后，自动执行 code-review +- **手动触发**：说 "审查代码"、"review"、"验收"、"检查代码" + +### review 流程 + +1. Layer 1：链分解（Interface / Business / Data / Utility / Error / Security / Performance） +2. Layer 2：量化指标（需求覆盖、异常分支、SQL风险等） +3. Layer 3：优化标准 + 输出 HTML 报告 + +### 严重级别 + +| 级别 | 含义 | 处理 | +|:----:|------|------| +| 🔴 Blocker | 必须立即修复 | ❌ FAIL，不可合并 | +| 🟡 Major | 合并前应修复 | ⚠️ 建议修复 | +| 🔵 Minor | 可后续优化 | — | + +### 配置 + +参见 `.code-review.yaml` 调整 review 严苛程度（fast / standard / strict）。 + +## 项目结构 + +``` +cobol_testgen/ # 核心代码 +test-data/ # 测试套件 +benchmark-programs/ # 测试基准（58 电信程序） +.claude/skills/ # 项目级 skills +.code-review.yaml # code-review 配置 +```