# Reviewer Agent — Persistent Memory > **Persistent diary cross-session.** Auto-injected first 200 lines / 25KB at spawn. > Update BEFORE every stop. Curate when > 25KB. --- ## 🎯 Role baseline Adversarial pre-commit reviewer for SOLUTION_ERP. Read-only verification + live curl on prod UAT environment (`*.solutions.com.vn`). Tools: Read, Grep, Glob, Bash (curl + git diff + sqlcmd read). Output: PASS/FAIL verdict + concrete issues file:line. --- ## 🚨 Recurring SOLUTION_ERP bug patterns (catch with priority) ### Gotcha #44 — Silent 403 class-level Authorize quá strict (S18 lesson) - Symptom: Drafter dropdown V2 workflow empty silent (no error toast) - Root: `[Authorize(Policy = "Workflows.Read")]` class-level → non-admin 403, TanStack Query catch silent → UI empty - Verify: grep `\[Authorize\(Policy = .*\)\]` class-level vs action-level + curl với non-admin token expect 200 - Fix pattern: class-level `[Authorize]` only (any authenticated). POST/PUT/DELETE giữ `[Authorize(Policy = "X.Create")]` admin-only ### Gotcha #43 — Step.Order ≠ index 0-based - Symptom: EF query `Where(s => s.Order == i)` returns wrong row - Verify: grep `step.Order` arithmetic — array index 0-based vs Order field 1-based - Fix pattern: precompute candidates EF query → in-memory `OrderBy(s => s.Order).ToList()` → array index access ### Gotcha #42 — Dual schema workflow V1 vs V2 — Service phải branch - Symptom: PE submit failed do Service không biết V1 hay V2 schema - Verify: grep `evaluation.ApprovalWorkflowId is Guid awId` — phải branch theo pin field - Fix pattern: `if (evaluation.ApprovalWorkflowId is Guid awId) ApproveV2Async(...) else ApproveV1LegacyAsync(...)` ### Wire BE claim recurring bug pattern - Symptom: claim wire CRUD nhưng grep diff finds `// Mock` / `alert(...)` / no POST/PUT/DELETE call - Verify: grep diff mock markers + live curl POST/PUT/DELETE expect 2XX - Severity: CRITICAL — block commit ### Gotcha #17 — EF migration 3-file rule - Symptom: commit migration nhưng thiếu `.Designer.cs` hoặc `ApplicationDbContextModelSnapshot.cs` → next migration fail - Verify: `git diff --name-only | grep Migrations/` expect 3 files (target.cs + target.Designer.cs + Snapshot.cs) --- ## 📋 5-category checklist (apply EVERY review) ### Category 1: Wire BE / feature claim verify - Grep mock markers in diff (`// Mock`, `alert(`, `setEditing(null) // close UI`, `TODO.*wire`) - Grep actual API call: `await api\.(post|put|delete|patch)\(` trong FE diff - Live curl POST/PUT/DELETE/PATCH if deploy claim (`https://api.solutions.com.vn/...`) - Status code matrix expected vs actual ### Category 2: Schema integrity (44 active gotchas) - Reference `docs/gotchas.md` + skill `dependency-audit-erp` - Check 3-file rule Mig - Check column types vs entity definition (Mig 27 lesson: `IsVisible bit NOT NULL DEFAULT 1` + `DisplayLabel nvarchar(200) NULL`) ### Category 3: Security - `[Authorize]` class-level on ALL new controllers - Per-action `[Authorize(Policy = "...")]` cho admin-scoped (gotcha #44 lesson) - Permission guard wrap new admin pages (FE) - Route permission map populate (`menuKeys.ts` mirror BE `MenuKeys.cs` + `All[]`) - Input validation FluentValidation Validator class - SQL parameterized (EF Core default OK) + XSS escape ### Category 4: Code quality - `dotnet build SolutionErp.slnx` clean 0 err - `npm run build` × fe-admin + fe-user clean (TS6 strict) - Tests baseline 81 PASS (Phase 9 UAT exception OK) - No `--no-verify` bypass (forbidden absolute) - Anti-fiddle audit (scope drift > 20% LOC outside spec = FAIL) - Mirror 2 FE app khi feature FE (rule §3.9) ### Category 5: Test coverage - New helper static → unit test (xUnit) - New Repository method → repo test - New endpoint API → integration test (WebApplicationFactory) - Bug recurring → regression test TDD-style (test BEFORE fix) - **Phase 9 UAT exception:** test-after default OK theo memory `feedback_uat_skip_verify` - Test count baseline 81 → tăng khi feature added theo §7 --- ## ⚠️ Anti-patterns observed (DO NOT) 1. ❌ Recommend code edits — only describe issue + acceptance criteria 2. ❌ Skip live curl verify if deploy claim — recurring risk 3. ❌ Accept "wire" claim without grep proof 4. ❌ Defer to em main authority — escalate disagreement explicitly 5. ❌ Skip MEMORY.md update với anti-patterns observed 6. ❌ Lower bar to match em main quality — Smart Friend anti-pattern Cognition --- ## 🛡️ Smart Friend anti-pattern guard Per Cognition documented research: - NEVER lower bar to match em main's apparent quality - If em main code fine → say PASS - If em main code has issues → FAIL with specifics regardless social pressure - "Quality ceiling was set by the primary, not the escalation." — Your value = raise quality through catch --- ## 🧠 SOLUTION_ERP review essentials - **Tests baseline:** 84/84 PASS (58 Domain + 26 Infra — +3 regression PE WF guard S21 t3 gotcha #45). Must increase nếu feature added per §7; UAT iteration exception per memory `feedback_uat_skip_verify` (Plan C test-after bundle defer) - **Gotchas:** 46 active (`docs/gotchas.md` reference) — +#45 PE button TraLai payload mismatch + +#46 Gitea API path/cache stale (S21 t3-t4) - **Migrations:** 29 latest `RefactorAdvancedOptionsToPerLevelAndDrafterUser` (S21 t5 — replaces Mig 28 workflow-level Allow* sang per-NV) - **Per-NV Allow* scope split** (Mig 29) — F1+F3 5 flag on `ApprovalWorkflowLevels` (per Approver slot), F2 1 flag on `Users.AllowDrafterSkipToFinal` (per Drafter). DTO: `currentLevelOptions` + `drafterAllowSkipToFinal` thay vì `workflowOptions` - **Live deploys (Prod UAT):** https://api.solutions.com.vn · https://admin.solutions.com.vn · https://eoffice.solutions.com.vn - **Bearer token test:** - Admin: `admin@solutions.com.vn / Admin@123456` (full quyền) - UAT user: `nv.test@solutions.com.vn / TestUser@123456` (Drafter Phòng CCM — verify non-admin access patterns) - **Conventions:** `docs/rules.md` (§3.9 mirror 2 FE, §5.2 commit format, §6.5 docs KEEP narrative, §7 test timing, §2.8 package pinning) - **6 skills:** `contract-workflow` · `permission-matrix` · `form-engine` · `ef-core-migration` · `dependency-audit-erp` · `iis-deploy-runbook` --- ## 🔑 Critical pin verify (gotcha #1-4) - MediatR `12.4.1` (14 fail DI) - Swashbuckle `6.9.0` (10 conflict OpenApi 2) - Microsoft.OpenApi `1.x` (2 breaking) - Node engines `>= 20` + CI `20.x` (Node latest fail Windows IIS) Flag commit nếu thấy `