d8cd111532
Session 53 closeout (HMW-mode ON, 'làm hết' full close). Code already shipped in44b9e54(Mig 47, Run #260) +dbf6648(C+D, Run #261), both prod-verified. - STATUS/HANDOFF: S53 entry (mig 46->47, test 200->203, menu +Off_AttendanceReport, bundle admin DfCfHUE9, database-agent verified-runtime). - Doc-drift E (H1 top-5): ef-core skill 43->47, agents/README roster 10->11 + plugin nac, CLAUDE.md root 45->47 mig + 186->203 test, docs/CLAUDE.md 56->57 gotcha + 91->92 ERD. - adap-report: database-agent executed-file -> verified-runtime (spawn-test caught Mig 46-unapplied-local). - session log 2026-06-08-S53 + 4 agent diaries (S53 work). - Memory: +project_database_agent_verified_local_drift (user-memory, outside repo). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
81 lines
19 KiB
Markdown
81 lines
19 KiB
Markdown
# Reviewer Agent — Persistent Memory
|
||
|
||
> **Persistent diary cross-session.** Auto-injected first ~200 lines at spawn (L1 HOT).
|
||
> Update BEFORE every stop. Tiered Memory v1: L1 HOT soft-cap ~30KB · L2 `archive/` on-demand · L3 RAG `search_memory` just-in-time. Keep entry ≤ 1.5K chars (gotcha #53).
|
||
> Full verbatim history pre-S40 → git `d2f52ba` + `archive/2026-05-q1..q2.md`.
|
||
|
||
---
|
||
|
||
## 🎯 Role baseline
|
||
|
||
Adversarial pre-commit reviewer SOLUTION_ERP. Read-only verify + live curl prod UAT (`*.solutions.com.vn`). Tools: Read, Grep, Glob, Bash (curl + git diff + sqlcmd read) + 5 RAG MCP. Skills: `dependency-audit-erp` + `contract-workflow` + `permission-matrix`. Output: PASS/FAIL + concrete issues file:line. NEVER write code.
|
||
|
||
---
|
||
|
||
## 🚨 Recurring bug patterns (catch priority)
|
||
|
||
- **#44 Silent 403 class-level Authorize quá strict** — Drafter dropdown empty silent (TanStack catch silent → UI empty). Grep `\[Authorize\(Policy=.*\)\]` class-level + curl non-admin expect 200. Fix: class-level `[Authorize]` only (any authenticated); POST/PUT/DELETE giữ `[Authorize(Policy="X.Create")]`.
|
||
- **#43 Step.Order ≠ index 0-based** — `Where(s=>s.Order==i)` wrong row. Fix: EF query → in-memory `OrderBy(Order).ToList()` → index.
|
||
- **#42 Dual schema V1/V2 — Service phải branch** — `if (entity.ApprovalWorkflowId is Guid awId) ApproveV2Async else V1Legacy`.
|
||
- **Wire BE claim** — grep diff `// Mock`/`alert(`/no POST-PUT-DELETE call + live curl expect 2XX. Severity CRITICAL block.
|
||
- **Cross-module security mirror (S29 Smart Friend)** — khi mirror entity/Command cross-module (PE→Contract→Budget V2), em main solo focus data shape MISS security guard. Pattern: `aw.ApplicableType == ExpectedType` validate ON Create BEFORE instantiation (mirror `PurchaseEvaluationFeatures.cs:62-77`). Attack: Drafter forge POST `/api/contracts` với `approvalWorkflowId` của PE/Budget → FK Restrict chỉ check Id existence NOT ApplicableType → wrong-scope pin. Also re-verify `IsActive`+`IsUserSelectable` server-side. Password ≥12 chars (Identity reject 11-char legacy). Severity MAJOR block push.
|
||
- **#17 EF migration 3-file** — `git diff --name-only | grep Migrations/` expect 3 (target + Designer + Snapshot).
|
||
- **#47 `.claude/agent-memory/**` NOT in paths-ignore** (PENDING bro decide) — MEMORY flush commit triggers CI ~3.5min waste. paths-ignore hiện `['docs/**','**/*.md','.claude/skills/**']` missing agent-memory. Severity minor (CI waste). ⚠️ S40 note: agent-memory commits đang trigger — recommend bro add.
|
||
|
||
---
|
||
|
||
## 📋 5-category checklist (EVERY review)
|
||
|
||
- **Cat 1 Wire BE/feature claim:** grep mock markers diff + `await api\.(post|put|delete|patch)\(` + live curl POST/PUT/DELETE if deploy claim + status matrix.
|
||
- **Cat 2 Schema integrity:** 3-file rule Mig + column types vs entity def. Reference `docs/gotchas.md` (55 active).
|
||
- **Cat 3 Security:** `[Authorize]` class-level ALL new controllers + per-action policy admin-scoped (gotcha #44) + FE PermissionGuard + menuKeys.ts mirror BE MenuKeys.cs + FluentValidation + EF parameterized.
|
||
- **Cat 4 Code quality:** `dotnet build SolutionErp.slnx` 0 err + `npm run build` × 2 app (TS6 strict) + tests baseline **130 PASS** (Phase 9 UAT exception OK) + no `--no-verify` + anti-fiddle (scope drift >20% LOC = FAIL) + mirror 2 FE app §3.9.
|
||
- **Cat 5 Test coverage:** new helper → xUnit · new endpoint → integration · bug → regression test-before-fix. Phase 9 UAT test-after default OK (`feedback_uat_skip_verify`). Baseline 130.
|
||
- **Cat 6 Authority boundary:** describe issue + acceptance criteria, NOT code edits. Escalate disagreement explicit.
|
||
|
||
---
|
||
|
||
## ⚠️ Anti-patterns + 🛡️ Smart Friend guard
|
||
|
||
1. ❌ Recommend code edits (only describe issue+criteria) · 2. ❌ Skip live curl if deploy claim · 3. ❌ Accept "wire" without grep proof · 4. ❌ Defer to em main authority (escalate explicit) · 5. ❌ Skip MEMORY · 6. ❌ **Lower bar match em main** (Smart Friend Cognition anti-pattern).
|
||
|
||
**Smart Friend (Cognition):** NEVER lower bar. Em main code fine → PASS. Em main issues → FAIL with specifics regardless social pressure. "Quality ceiling set by primary, not escalation." Value = raise quality through catch.
|
||
|
||
---
|
||
|
||
## 🧠 SOLUTION_ERP review essentials (S40 verified)
|
||
|
||
- **Tests baseline:** **130 PASS** (58 Domain + 72 Infra). Must increase khi feature added (§7); Phase 9 UAT exception (`feedback_uat_skip_verify`).
|
||
- **Gotchas:** **55 active** (`docs/gotchas.md`, format `### N.` highest #55). Latest #53 truncation · #54 529-fallback · #55 truncation-mid-exploration.
|
||
- **Migrations:** **40 latest `AddAttendances`** (path `src/Backend/SolutionErp.Infrastructure/Persistence/Migrations/`). Per-NV Allow* (Mig 29 F1/F3 5 flag + Mig 30 F4 on `ApprovalWorkflowLevels` per slot + F2 `Users.AllowDrafterSkipToFinal` per Drafter; Mig 31 SkipToFinal→ApproverLevel).
|
||
- **Endpoints:** ~211 · **84 SQL tables**.
|
||
- **Identity password ≥12 chars** (reject 11-char). Test creds: admin `admin@solutions.com.vn/Admin@123456` (full) · UAT `nv.test@solutions.com.vn/TestUser@123456` (Drafter CCM).
|
||
- **Prod:** api/admin/eoffice.solutions.com.vn. **Pin:** MediatR `12.4.1` (flag `Version="14`) · Swashbuckle `6.9.0` · Node CI `20.x`.
|
||
- **Conventions:** `docs/rules.md` (§3.9 mirror 2 FE, §5.2 commit, §6.5 docs narrative, §7 test timing, §2.8 pin).
|
||
|
||
---
|
||
|
||
## 📅 Recent activity (FIFO — older → archive/git)
|
||
|
||
- **2026-06-08 (S52-late Task C ItTicket admin reassign + Task D AttendanceReport menu-key pre-commit — PASS, 0 blocker):** Migration-FREE (menu = idempotent DbInitializer seed). 5 prod files: MenuKeys.cs (const+All[]), DbInitializer.cs (1 seed tuple), fe-admin {menuKeys.ts, Layout.tsx staticMap, ItTicketsPage.tsx}. **Independent re-verify GREEN:** `dotnet build SolutionErp.slnx` 0-warn/0-err · `npm run build` fe-admin tsc-b+vite OK (1945 modules, only pre-existing CSS @import + >500KB + ineffective-dynamic-import warns). **menuKeyMatchOk:** `"Off_AttendanceReport"` byte-identical 4 places (MenuKeys.cs:125 const == menuKeys.ts:68 == seed parent key == Layout staticMap:87); seed leaf parent=`MenuKeys.Off` order=8 icon=`FileBarChart` (verified valid lucide alias `FileChartColumn as FileBarChart` — getIcon resolves via Icons[name]); App.tsx route `/attendance/report → AttendanceReportPage` PRE-EXISTING committed S52 (6a66429, no diff, 170-LOC real page not stub) — Layout maps to REAL route. `types/menu.ts` correctly NOT mirrored (`key:string` not typed union). admin auto-perm via `SeedAdminPermissionsAsync` iterating `MenuKeys.All` (DbInit:1917, idempotent Contains:1919); new-leaf-on-existing-DB confirmed (upsert loop:1845-1862 TryGetValue-miss→Add). **reassignCorrect:** FE PUT `/it-tickets/{id}/assign` body `{assignedToUserId}` MATCHES BE record `AssignItTicketBody(Guid AssignedToUserId)` (ItTicketsController:42); endpoint `[Authorize(Roles="Admin")]` (:34) under class `[Authorize]` — admin-app FE calling = correct; user-list reuses EXISTING `/users` GET (`PagedResult<UserDto>` items{id,fullName,email}, no new BE endpoint, lazy `enabled:target!==null`); 204 NoContent handled (no body-parse); `invalidateQueries(['it-tickets'])` on success; handler sets BOTH AssignedToUserId+AssignedToFullName (WorkflowAppsFeatures:467-468) + validates assignee IsActive→NotFound, no try-catch (GlobalExceptionMiddleware). **feUserUnchanged:** `git diff -- fe-user/` EMPTY (Task C = fe-admin-only divergence, documented top-comment ItTicketsPage:3-5). **noScopeCreep:** git status prod = EXACTLY the 5 expected files, agent-memory noise ignored, no new migration, no BE beyond MenuKeys+DbInitializer, ItTicketsPage diff 98+/5- all Task-C-scoped (imports+state+mutation+Pencil-btn+Dialog), 0 mock/alert markers. **Learned:** menu-key wiring = verify byte-identity across the FULL mirror set (BE const + BE All[] + seed parent + FE menuKeys + FE staticMap) + confirm the target route actually EXISTS (grep App.tsx) — a staticMap entry pointing to a non-existent route silently drops the leaf (gotcha #50). **surprise:** lucide `FileBarChart` is a deprecated-alias (re-exported from FileChartColumn) but still valid — d.ts grep confirmed before flagging. Verdict PASS — safe to commit. Tag [s52-late, it-ticket-reassign, attendance-report-menukey, menukey-mirror-5way, gotcha44-disarmed, gotcha50-disarmed].
|
||
|
||
- **2026-06-08 (S53 gotcha #57 EXT Mig 47 — Master catalog filtered-unique pre-commit — PASS, 0 blocker, Smart Friend clean):** 4th/5th/6th cumulative gotcha #57 (after Holiday Mig 43 S45 + HRM ×3 Mig 45 S51). 3 Master configs (Department:18/Project:19/Supplier:24) Code unique index `.IsUnique()` → `+.HasFilter("[IsDeleted] = 0")` + Mig 47 (3-file) + 3 new tests. **Independent re-verify ALL GREEN:** build SolutionErp.slnx 0-warn/0-err · **full suite 203 PASS** (58 Dom + 145 Infra, Failed:0 Skipped:0, +3) · 3 new tests run isolated 3-passed-0-skipped. **Cat correctness:** filter string byte-identical to HolidayConfiguration:18 (xxd `5b 49 73 44 65 6c 65 74 65 64 5d 20 3d 20 30` — spaces around `=`, not guessed); index STAYS `unique:true` (2 active same-Code still violate — active uniqueness preserved); Supplier Type index (:25) UNTOUCHED non-unique unfiltered (snapshot:3590 bare). Mig Up=3×Drop+3×Create-filtered, Down=3×reverse-unfiltered (reversible). Snapshot+Designer both show filter on all 3 Master Code idx. **Test NOT tautology:** seeds IsDeleted=true row → real Create*CommandHandler (app-check `AnyAsync(Code==req.Code)` thru HasQueryFilter !IsDeleted PASSES) → asserts NotThrow + active-count==1 + IgnoreQueryFilters all==2; RED-before confirmed (3 failed SqliteException UNIQUE on unfiltered). Cmd signatures match test calls (Project 7-arg/Supplier 9-arg/Dept 4-arg). **noScopeCreep:** git status = exactly 3 configs + snapshot + Mig47 (2 untracked) + 1 test + 2 MEMORY; no FE, no extra mig, no stray. Mig 47 latest in seq (after Mig46 ItTicket SLA). **Learned:** cookie-cutter EXT of proven pattern → discriminator = byte-compare filter string (xxd) vs canonical sibling + verify index still unique (filter must NARROW scope not DROP uniqueness). app-level dup-check existence = the test premise; verify handler actually has `AnyAsync(Code)` else test premise false. **surprise:** implementer claimed "2 pre-existing DocxRenderer warnings" but clean incremental rebuild = 0 warn (unrelated, non-issue). Verdict PASS — safe commit. Tag [s53, gotcha57-ext, mig47, master-catalog, smart-friend-clean].
|
||
|
||
- **2026-06-08 (S52 P11-E AttendanceReport + P11-F MaTicket codegen pre-commit — PASS, 0 blocker):** Migration-free (no schema). Independent re-verify: build 0-err · **191 PASS** (58 Dom + 133 Infra, +5: 3 ItTicketCodeGen + 2 AttendanceReport) · fe-admin `tsc --noEmit` exit 0. **Cat3 gotcha #44 attack DISARMED:** `[Authorize(Roles="Admin")]` ×2 report endpoints — verified `AppRoles.Admin = "Admin"` literal (AppRoles.cs:5) == attribute string == FE `user?.roles.includes('Admin')`; "QTV" (DbInit:1454) = display-code NOT role-name; pattern proven (Catalogs/HrmConfigs identical). **Cat3 camelCase contract MATCH** field-for-field BE record PascalCase→FE interface (year/month/rows/grandTotal*/userId/fullName/ot*) — ASP.NET default camelCase, no Program.cs override. **BE handler correct:** `.Year/.Month` in IQueryable (EF-translatable DateTime), `.DayOfWeek`+holidaySet only AFTER `.ToListAsync()` (in-memory) — IQueryable-translation attack handled; holiday-check BEFORE weekend BEFORE weekday (test 2026-06-01 Mon-but-holiday proves override); `DateOnly.FromDateTime` correct (Holiday.Date=DateOnly); OtPolicy fallback 1.5/2.0/3.0; `IsDeleted` via AuditableEntity all 3 entities. Exporter mirrors ContractExcelExporter, ClosedXML 0.105.0, `RenderResult(Content,FileName,ContentType)` ctor order correct, DI registered. **MaTicket codegen:** `e` untracked at codegen time → inner SaveChanges persists ONLY sequence row, no double-insert; gen-on-Create (kanban no-workflow) vs Leave/OT gen-on-Submit — semantically correct; git-show confirms MaTicket was ALWAYS null pre-P11-F (closes gap). **1 MINOR (informational, defer):** sequence-gap-on-failure — codegen commits seq in own Serializable tx BEFORE `Add(e)+SaveChanges`; ticket-insert fail → burned IT/2026/NNN gap. NOT new defect = identical to existing Leave/OT pattern (project-wide accepted trade-off, cosmetic). MyAttendancePage MIRROR divergence intentional+documented (fe-user untouched, §3.9 OK). 0 mock markers. **Learned:** when spec NAMES an attack vector (gotcha #44 role-string), verify the LITERAL const value not just attribute presence — "QTV" display-code was the decoy; role-name match is the real check. **surprise:** Bash tool = bash not PowerShell (Select-String fails exit 127 → use grep). Verdict PASS — safe to commit. Tag [s52, p11ef, attendance-report, mat-codegen, gotcha44-disarmed].
|
||
|
||
- **2026-06-08 (S51 P11-C Vehicle+Driver + gotcha #57 pre-commit — PASS, 1 MAJOR caught) [em main proxy — reviewer return truncated gotcha #53]:** Reviewed Mig 44 (Vehicle/Driver catalog) + Mig 45 (filter 3 HRM unique) + FE KIND_CONFIG +2 + 5 tests (186 PASS). Independent build+test re-verify GREEN. **CAUGHT 1 MAJOR (Cat 3 cross-stack contract):** Driver FE↔BE required-field mismatch — FE render phoneNumber/licenseNumber/licenseClass OPTIONAL nhưng BE validator `NotEmpty()` + EF `.IsRequired()` NOT NULL → empty submit = 400/500. Root = inconsistent em-main brief (BE "mirror Vehicle"=required vs FE spec quên required). Fix: FE +`required:true` (align BE all-required như Vehicle). Cats khác clean (Mig diff clean, Authorize Roles=Admin writes, gotcha #57 grep-complete 3 HRM, DbInitializer idempotent + #51 infra-gated, SHA256 mirror, no copy-paste Driver↔Vehicle). **Learned:** parallel fan-out (BE∥FE file-disjoint) → bất kỳ inconsistency trong SHARED em-main contract chỉ lộ lúc integration; green tests ≠ correct contract (no test chạm empty-optional path). reviewer = the net. **surprise:** transient mid-deploy bundle hash (cicd lesson) + reviewer self-truncate trước khi ghi MEMORY → em main proxy. Verdict PASS post-fix. Tag [s51, p11-c, gotcha57, contract-mismatch-catch].
|
||
|
||
- **2026-06-07 (S49 Harness 1/2/3 adopt pre-commit — PASS all 3, no blocker):** Governance/infra adopt (no product code, no test impact). VERIFIED: H1/H2 = 2 sub scope-DISJOINT + tools `[Read,Grep,Glob,Bash+4RAG]` NO store_memory/Write (INFORM-only); genuinely **TAILORED not copy-paste** (SE 4-RAG vs AI_INFRA 2-RAG · dropped effort:max + agent-ops-monitor/sister · Fidelity→SE `reviewer`). H2 5-trục in harvest-curator.md + session-end §L.b(f). H2 wave-mode hmw.js mirror AI_INFRA + **B6 `git check-ignore` VERIFIED** (wave-*/+agent-teams/ ignored · hmw.js/README tracked). H3 self=`se` complete substitution · **SHA256 canonical formula byte-identical send==check** · 13 .gitkeep exact · adap-apply base-path `outbox\all\`. honest nấc executed-file/verified-runtime-PENDING. G-015 scan = 6 hits ALL negating ("KHÔNG enforced") = correct honesty. **1 MINOR (non-block):** README:11/18 "7-agent" ASCII diagram = **PRE-EXISTING** drift (git diff proved work này chỉ touch load-bearing title/decision-tree/tool-grant/matrix; diagram predates S47 frontend-designer) → tooling-auditor H1 designed-to-catch = self-validating adoption. **learned:** `git diff base..head` = discriminator introduced-defect vs pre-existing-drift (đừng đổ lỗi work mới cho drift cũ); name-collision tailor-verify = diff frontmatter AI_INFRA-canonical vs SE-instance. **surprise:** mojibake scan false-pos trên "ĐÃ" (U+00C3 = valid VN uppercase, KHÔNG double-encode → verify codepoint in-context trước flag); broadcast floor "12 .gitkeep" UNDERCOUNT (correct=13 incl `all/` adap-channel — em main đúng). Verdict PASS, safe commit + restart. Tag [s49, harness-adopt, governance, max-clean].
|
||
|
||
- **2026-05-30 (S43 P11-B LeaveBalance pre-commit — PASS, Max no-truncate):** 14 file (LeaveBalance entity+config+Mig42 + Features + Controller + deduction hook + Create/Update LeaveType guard + embed balance + FE×4 + tests). 154 PASS (130→154). **Deduction exactly-once VERIFIED** (terminal else only, guard Status!=DaGuiDuyet chặn re-approve; advance/reject/return no-deduct). **FK invariant fully closed** — grep 2 write site LeaveTypeId (Create + UpdateDraft) cả 2 guard AnyAsync→Conflict, bogus type không thể tới terminal FK insert. Embed balance = RequesterUserId (approver thấy đúng người tạo). admin `[Authorize(Roles=Admin)]`. **2 MINOR defer:** concurrency lost-update UsedDays (no RowVersion — human-sequential accept) · stale line-num comment. Verdict PASS. Tag `[s43, p11b-leavebalance, max-clean]`.
|
||
- **2026-05-28 (S35 G-H2 BE CRUD 16 endpoint pre-commit — PASS, Smart Friend 8× CLEAN):** 2 NEW file `HrmConfigFeatures.cs` 439 + Controller 137. build clean, 130/130 PASS. Cat1: 0 mock, 8 ConflictException (Holiday Update composite `(Year,Date)` BOTH fields). Cat3: class `[Authorize]` + 12 per-action `[Authorize(Roles="Admin")]`. Cat5: 8 Validator MaxLength MATCH EF source (Code=50 not spec 20). **2 MINOR defer:** ListHolidays no IsActive filter (inconsistent sibling) · OtPolicy "1 active unique" NOT enforced handler (G-P1 ambiguous nếu 2+ active). Verdict PASS. Tag `[s35, smart-friend-8x-clean]`.
|
||
- **2026-05-26 (S33 Plan B G-H1 Phase 2 pre-commit — PASS, Smart Friend 6× CLEAN):** 17 file (3 BE + 6 FE new + 6 mod + 2). SHA256 mirror 3 file IDENTICAL admin==user. 5 endpoint real mediator.Send 0 mock. Mig 34 `AddEmployeeProfiles` 7 table UNIQUE indexes + FK Cascade. SeedDemoEmployeeProfiles NOT gated DemoSeed (gotcha #51 ✓). gotcha #50 Layout staticMap mirror ✓. **3 MINOR defer:** EmployeeCode race SERIALIZABLE low-risk · Update 3 bool not nullable (partial reset) · Delete DateTime.UtcNow direct. Verdict PASS. Tag `[s33, hrm-mig34, smart-friend-6x]`.
|
||
- **Smart Friend cumulative 8× CLEAN:** (1) S22 #44 silent-403 · (2) S25 #48 SQLite tie-break · (3) S29 password ≥12 · (4) S29 ApplicableType cross-module · (5) S33 BW test · (6) S33 Plan B Phase 2 · (7) S35 FE forms · (8) S35 G-H2. Plus 9× G-O2 (S36, em không track ở đây). 2 MAJOR catches total (S29 password + S29 ApplicableType); rest clean với MINOR defer.
|
||
- **Archived S29-S33 detail + S32 startup → `archive/2026-05-q2.md` + git d2f52ba (S40 curate):** S33 Plan C B-Wrap 9/9 [Fact] verify · S33 startup drift audit (CLAUDE.md SEVERE → patched S40) · S32 wrap/startup standby · S29 wrap 2 MAJOR catch detail. KEY absorbed in bug patterns + Smart Friend cumulative above.
|
||
|
||
---
|
||
|
||
## 🔄 Curate trigger
|
||
- >~30KB → archive recent → L2 `archive/<period>.md`. Stale >3mo → remove.
|
||
- **Last curate: 2026-05-29 S40 em main proxy** (28.4→~18KB): archived S33 Plan C + S33 startup + S32×2 + S29 wrap detail → q2 + git d2f52ba; refreshed stale (81/111→130 test, 47→55 gotcha, 31→40 mig, ~146→211 endpoints). Foundation (bug patterns + 5-category + Smart Friend guard + cross-module security) preserved. Prev: S34 q2 · S22 q1.
|