Files
solution-erp/.claude/agent-memory/reviewer/MEMORY.md
pqhuy1987 cc8a7d34b3 [CLAUDE] Docs: S22 chốt cuối — gotcha #47 + 4 agent MEMORY flush + session log cumulative
Session 22 chốt cuối — bro confirm sub-agent solution OK.

Highlights cumulative S21 chốt → S22 chốt:
- 11 commits S22 pushed remote `3d725c4..b04a11a`
- Plan G S22 evidence: 4 sub-agents (3 seeds-only + 1 CICD Monitor Run #188 PASS)
- Plan C + D + E done · Plan F ABORTED pre-flight blocker
- 5 turn S22+ feedback iteration (disable 3 button + seed 20 user + rename role-based + attachment view + Mig 30 per-NV opt-in)

Docs updates:
- STATUS Last updated S22 chốt + S22 prev row preserved (§6.5 KEEP narrative)
- HANDOFF Last updated S22 chốt + S22 prev row preserved
- Session log mới `2026-05-13-2200-s22-chot-cuoi.md` (~12KB narrative + 11 commit table + 7 lessons learned + handoff S23)
- Gotcha #47 mới `.claude/agent-memory/** thiếu paths-ignore filter` (CICD waste 3.5min per MEMORY flush) — PENDING bro fix `.gitea/workflows/deploy.yml`

4 agent MEMORY.md flushed S22:
- Investigator: 30 mig + 104 test + S22 context essentials + Mig 30 entry + cross-ref `feedback_per_nv_permission_scope` 2× reinforced
- Implementer: +6 patterns (7-12 per-NV opt-in / tách endpoint narrow scope / defense-in-depth FE+BE / reflection regression / cookie-cutter test infra / InternalsVisibleTo) + S22 activity (REFUSED 100% cross-stack)
- Reviewer: +Gotcha #47 + Mig 30 + 104 test baseline + S22 self-review narrative + Identity password ≥12 chars note
- CICD Monitor: refresh test 84 → 104 + Mig 29 → 30 (Run #188 PASS preserved)

User memory reinforcement:
- `feedback_per_nv_permission_scope.md` +Section "Reinforcement S22+5" — pattern proven 2× với Mig 30 F4. Anti-pattern default scope expansion. Decision tree thêm scope khi feedback ambiguous → admin opt-in flag per slot
- `MEMORY.md` index entry updated cross-ref S22+5 reinforcement

Stats final:
- 30 migrations (+1 Mig 30)
- 104 tests PASS (+20 S22)
- 47 gotchas (+1 #47 pending fix)
- ~146 endpoints (+3)
- 33 active prod users (rename role-based)
- 6 skills · 4 sub-agents unchanged

KHÔNG cắt narrative cũ — Edit specific lines + Append new entries per §6.5.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-13 23:25:37 +07:00

11 KiB
Raw Blame History

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)

Gotcha #47 — .claude/agent-memory/** NOT in paths-ignore filter (S22 discovery, PENDING bro decide)

  • Symptom: MEMORY.md drift patch commit (end-of-session flush) triggers full CI deploy ~3.5min waste
  • Verify: check .gitea/workflows/deploy.yml paths-ignore — currently ['docs/**', '**/*.md', '.claude/skills/**'] MISSING .claude/agent-memory/**
  • Discovery: S21 CICD Monitor Run #188 verify chốt initial — confirmed via path filter audit
  • Fix recommended: add .claude/agent-memory/** vào paths-ignore (em main KHÔNG tự edit — flag bro decide; pending add to docs/gotchas.md 47th entry)
  • Severity: minor (CI waste only, no functional impact)

📋 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: 104/104 PASS (58 Domain + 46 Infra — +20 từ 84 S21: 5 regression gotcha #44 Authorize class-level + 7 ReturnMode + 7 Guard EnsureCanRejectV2Async + 1 V2 actor scope reject). 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). +#47 paths-ignore agent-memory gap (S22 PENDING bro confirm add to docs)
  • Migrations: 30 latest AddAllowApproverEditBudgetToLevels (S22+5 — per-slot F4 flag admin opt-in cho Approver scope edit budget ChoDuyet branch). Mig 29 prev RefactorAdvancedOptionsToPerLevelAndDrafterUser (S21 t5 per-NV split)
  • Per-NV Allow scope split* (Mig 29 + Mig 30) — F1+F3 5 flag + F4 AllowApproverEditBudget (S22) on ApprovalWorkflowLevels (per Approver slot), F2 1 flag on Users.AllowDrafterSkipToFinal (per Drafter). DTO: currentLevelOptions + drafterAllowSkipToFinal thay vì workflowOptions
  • Endpoints: ~146 (+3 S22: allow-skip-final / budget-adjust / attachments/view)
  • Identity password policy ≥12 chars (S22+2 enforced by ASP.NET Identity stack — reject User@123456 11 chars). Existing HANDOFF mention "User@123456" pattern S4 outdated, current test creds Admin@123456 + TestUser@123456 OK
  • 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)
  • Users active: 33 (rename role-based pattern act.nv / act.pp / act.tp etc.)
  • 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 <PackageReference Include="MediatR" Version="14... hoặc tương tự.


📅 Recent activity (last 10 FIFO)

  • 2026-05-13 (S22 18:00→21:00, no spawn): Em main solo self-review S22 — Reviewer KHÔNG spawn per UAT mode feedback_uat_skip_verify. Em main verify build clean + test pass + npm build × 2 app mỗi chunk (11 commits cumulative). Key validations: (1) Plan E phân quyền strict V2 — actor.UserId scope in List + Detail, Inbox đã strict từ S17, loose UAT clause || ApprovalWorkflowId != null removed. (2) S22+1 V2 actor scope guard BE helper EnsureCanRejectV2Async chặn request forge non-approver PATCH /transitions decision=Reject (mirror FE button disable) — defense-in-depth FE+BE pattern. (3) S22+4 AdjustBudgetCommand handler 3-tier scope (Drafter Nháp/Trả lại + Approver ChoDuyet + Admin) — S22+5 refactor ChoDuyet branch dùng level.AllowApproverEditBudget flag (admin opt-in per slot) thay default Approver scope (security scope correction per bro feedback). (4) S22+2 Identity password policy enforced ≥12 chars (reject User@123456 11 chars outdated). Anti-patterns observed: (a) Default scope expansion mistake S22+4 → S22+5 fix — KHÔNG default expand permission scope without admin tick (per-NV opt-in pattern Mig 29 lesson reinforced); (b) History display field assumption BudgetAdjustSection initial — em assume PeDetailBundle.changelogs exists, FAIL TS2339 — pattern verify type fields trước render; (c) PS 5.1 Vietnamese diacritics gotcha #30 reinforced — script seed-test-users-prod.ps1 first attempt FAIL parser error, ASCII-only discipline. S22+4 attachment view endpoint + S22+4 budget-adjust endpoint chưa qua live curl verify (defer post-deploy — bro UAT direct). S22+5 Mig 30 applied LocalDB Dev + Design via dotnet ef database update. New gotcha discovered: #47 paths-ignore agent-memory gap (recommend add to docs/gotchas.md — pending bro decide). Cumulative state: 104 test (+20), 30 mig (+1 Mig 30), 46 gotcha unchanged (gotcha #47 pending), ~146 endpoints (+3), 33 active users. Smart Friend guard still active for future spawn.
  • 2026-05-13 (S21 t3-t5, no spawn): Em main solo verify via dotnet build + npm build × 2 app + dotnet test suite mỗi chunk. Reviewer KHÔNG spawn — em main self-review per UAT mode feedback_uat_skip_verify (skip dotnet test mỗi chunk, vẫn build verify). Gotcha #45 fix self-test 3 regression test (test-before §7). S21 t3-t5 push cumulative 12 commits — CICD Monitor verify post-deploy thay vai Reviewer (deploy ship + bundle hash + schema verify). Cumulative state: 84 test, 29 mig, 45 gotcha, 19 memory entries. Pattern saved cho future review focus: per-NV permission audit (Level table vs User table flag), EF migration backfill SQL injection between ADD-DROP order. Smart Friend guard still active for future spawn.
  • 2026-05-11 (setup): Reviewer agent initialized. Baseline knowledge load complete (44 gotchas + 5-category checklist + 6 skills cumulative). No reviews performed yet. Awaiting first SendMessage from em main. Smart Friend guard active.

🔄 Curate trigger

  • Memory size > 25KB → archive recent entries to archive/<period>.md
  • Duplicate entries detected → merge
  • Stale > 3 months → remove

Last curate: 2026-05-11 (initial seed)