110 lines
16 KiB
Markdown
110 lines
16 KiB
Markdown
REVIEWER-GATE-K2: IN-PROGRESS — chưa chốt
|
||
|
||
# sub-reviewer-gate-k2 — GATE K2 (adversarial, trước commit-push) — S164/S165
|
||
|
||
> Luật ghi-đĩa-trong-lúc-làm (#53): mỗi mục đo xong ghi NGAY. Dòng-1 = verdict, cập nhật cuối cùng.
|
||
> Vai: reviewer READ-only. KHÔNG sửa prod-code. Được chạy dotnet/npm build.
|
||
|
||
## §0 Tiền đề đo được (git, trước mọi phán)
|
||
|
||
- Branch `main`. `git status --porcelain`: 31 file M + 5 file untracked (2 mig K2 + 1 test file K2 + 2 auto-memory).
|
||
- Diffstat vùng code: 31 files changed, +1005/-126 (gồm cả .claude/docs).
|
||
- File K2 untracked: `20260731085624_AddKhkkApprovalGroupCatalogAndFinalizeRuntime.cs` + `.Designer.cs`
|
||
+ `tests/.../Application/ContractSigningPlanGroupAndCatalogTests.cs`.
|
||
|
||
## §1 Trọng tài BUILD/TEST — reviewer tự chạy tươi (nguyên văn)
|
||
|
||
- `dotnet build SolutionErp.slnx` → `Build succeeded. 0 Warning(s) 0 Error(s)` · `Time Elapsed 00:00:18.24` ⇒ **KHỚP claim 0W/0E**.
|
||
- `dotnet test SolutionErp.slnx`:
|
||
- Domain: `Passed! - Failed: 0, Passed: 45, Skipped: 0, Total: 45, Duration: 69 ms`
|
||
- Infrastructure: `Passed! - Failed: 0, Passed: 562, Skipped: 0, Total: 562, Duration: 1 m 11 s`
|
||
- Tổng **607/0 = 45D + 562I** ⇒ **KHỚP claim `sub-test-k2.md:1` từng con số.**
|
||
|
||
## §2 Ranh giới (a)-(d) — đo trên diff
|
||
|
||
- (a) `ContractSigningPlanWorkflowService.cs`: diff CHỈ 1 hunk `@@ -164,6 +164,23 @@` (+17/-0) nằm trọn trong `SubmitAsync`.
|
||
`ReturnOrRejectAsync` (`:359 plan.Phase = targetPhase`) + `ApplyApprovedValuesOnFinalize` + `:295 = DaDuyet` **0 dòng đổi** ⇒ ranh B3-B4 GIỮ theo NỘI DUNG.
|
||
- (b) K1 `ContractCatalog*`: `git status --porcelain` trên 3 path K1 = **0 dòng** (đã commit `50e6d8c`) ⇒ không bị đụng.
|
||
- (c) route/menu: `App.tsx` · `lib/menuKeys.ts` · `components/Layout.tsx` **KHÔNG có trong diff** ⇒ 0 route/menu mới (K4a turf giữ).
|
||
(`MenuKeys.cs` BE cũng không trong diff của wave K2 — chỉ `.claude/skills/permission-matrix/SKILL.md` là doc.)
|
||
- (d) 3 test lead vá: `ContractSigningPlanSchemaTests.cs` (+56/-…) + `ContractSigningPlanApprovalTests.cs` (+19) vẫn còn trong diff, file mới `ContractSigningPlanGroupAndCatalogTests.cs` tách riêng ⇒ không bị lane đè.
|
||
|
||
## §3 `npm run build` ×2 — reviewer tự chạy tươi
|
||
|
||
| App | Kết quả | Bundle (nguyên văn) | So claim lane |
|
||
|---|---|---|---|
|
||
| fe-admin | ✅ PASS, `✓ 1961 modules transformed` / `✓ built in 647ms` | `dist/assets/index-DORfkFnc.js 1,722.60 kB │ gzip: 428.08 kB` | KHỚP TỪNG KÝ TỰ |
|
||
| fe-user | ✅ PASS, `✓ built in 575ms` | `dist/assets/index-qYI2yszp.js 1,638.80 kB │ gzip: 413.92 kB` | KHỚP TỪNG KÝ TỰ |
|
||
|
||
Warning cả 2 app = pre-existing (chunk > 500 kB · `INEFFECTIVE_DYNAMIC_IMPORT` `realtime.ts`), 0 TS error.
|
||
`noUnusedLocals: true` ⇒ build XANH = phép thử TS6133 có răng (import `KHKK_APPROVAL_GROUP_LABELS` ở ListPage đã có người dùng thật).
|
||
|
||
## §4 Code-vs-spec K2
|
||
|
||
**Mig 71 — 6 item ĐỦ, 3-file ĐỦ.** `20260731085624_…FinalizeRuntime.cs` + `.Designer.cs` (2 file `??` mới) + `ApplicationDbContextModelSnapshot.cs` (M) — snapshot có đủ `ApprovalGroup` · `EndedByLevelFinalize` · `CatalogEntryId` · `TenHangMuc(500)` · `HasIndex("ApprovalGroup","Phase")` · `HasIndex("CatalogEntryId")`. Backfill `:61` `UPDATE [ContractSigningPlans] SET [ApprovalGroup] = 1;` **vô điều kiện** đúng chữ vá. Unique `:93-98` 3 cột + `filter: "[IsDeleted] = 0"` đúng #57.
|
||
**Đo DB (sqlcmd, `SolutionErp_Dev`) — reviewer tự chạy:** `tables=97` · top mig `20260731085624_AddKhkkApprovalGroupCatalogAndFinalizeRuntime` · 2 cột header + 2 cột line **có mặt** · `sys.indexes is_unique=1` chỉ còn `PK_…` và `IX_…_ContractSigningPlanId_SupplierId_CatalogEntryId` filter `([IsDeleted]=(0))` ⇒ **unique cũ 2-cột ĐÃ CHẾT THẬT** (index swap, không phải thêm chồng). `plans=0 · lines_cnt=0` ⇒ Σ tiền local là **control RỖNG** — lane khai đúng độ mạnh, KHÔNG đòi thêm phép đo (xem §6).
|
||
|
||
**Submit-guard 2 vế — PHÁN vụ "vá-5 THẮNG nền `:126`": lead ĐỌC ĐÚNG ưu tiên.** Chứng bằng chữ:
|
||
- `spec-cum1-chi-tiet:4` nguyên văn: *"**12 VÁ dưới đây THẮNG khi xung đột**"* ⇒ luật ưu tiên do CHÍNH spec đặt, không phải lead tự phong.
|
||
- `spec-cum1-chi-tiet:12` (vá-5): *"auto-gen line lúc tạo phiếu được mồi null NHƯNG **submit-guard CHẶN null**"*.
|
||
- Nền `sub-invest-fable-b2-cum1.md:126`: *"Line `CatalogEntryId == null` (legacy nhóm-1 backfill) → CHO QUA"*.
|
||
⇒ Xung đột THẬT, vá thắng, code chọn CHẶN (`WorkflowService.cs:174-176`). Đúng. Kèm điểm cộng: cái GIÁ của lựa chọn (phiếu cũ bị Trả-lại sẽ **kẹt trình** tới khi K4b land) được khai THẲNG trong comment tại chỗ, không giấu.
|
||
Vế (ii) khác-nhóm 409 `:178-182` theo đúng code mẫu nền K2.d.
|
||
|
||
**List `?group=` end-to-end — 5/5 mắt xích có thật:** controller `ContractSigningPlansController.cs:42` `[FromQuery(Name="group")] int? group` → truyền vị trí thứ 5 `:49` → record `ListContractSigningPlansQuery:705 int? ApprovalGroup` → filter `:735-736` (đặt SAU vế IDOR, chỉ thu hẹp) → DTO `ListItemDto:63 ApprovalGroup` → FE `types/khkk.ts:158 approvalGroup: number`. FE **cố ý chưa đẻ ô lọc** (K4b/K5) — khai rõ ở `sub-implfe-k2.md §2` + comment `types/khkk.ts:28-32`. *(bẫy tên tham số → F-6.)*
|
||
|
||
**Σ×3 + display-gate:** `Features.cs:793-804` — PeReference · Proposed · **+Approved** (`.Sum(l => l.ApprovedAmount) ?? 0m`). Detail `:689` `BudgetFrozen = plan.Phase == DaDuyet`. FE: `KhkkListPage` +cột "Tổng duyệt" (8 `<th>` == 8 `<td>`, `colSpan` 7→**8** ở CẢ 2 chỗ ×2 app — đếm tay khớp) · `KhkkDetailPage:439-446` badge 🔒 qua prop `actions` (`SectionCard` có sẵn prop này, không bịa).
|
||
|
||
**Phase-guard + T-f:** `EnsureDraftEditable` (`Features.cs:229-237`) có **3 call-site thật** `:518 :928 :982`; K2 KHÔNG dựng guard mồ côi. T-f (`…GroupAndCatalogTests.cs:570-641`) **nối đường ghi THẬT**: dựng `UpdateContractSigningPlanDraftCommandHandler` thật, `ThrowAsync<ConflictException>`, rồi đọc lại DB khẳng định `GhiChu == null` **và** `Changelogs == 0` ⇒ không phải reflection suông, có cả vế "ném rồi thì không ghi gì".
|
||
|
||
**FE vá-4 + OG-7 — mở file thật, đối chiếu 4 điểm (≥3 yêu cầu):**
|
||
1. select nhóm default 1 — `KhkkCreatePage.tsx:62` `useState<number>(KHKK_APPROVAL_GROUP_DEFAULT)`, const `= 1` (`types/khkk.ts:121`). ✅
|
||
2. POST tường minh — `:99` `approvalGroup,` trong body (không dựa default ngầm BE). ✅
|
||
3. nút "Tạo thêm cho nhóm khác" — `:240-243`, đặt NGOÀI `<button>` PE (bọc `<div key>`, tránh button-lồng-button). ✅
|
||
4. hết 8 nhóm = **chữ** — `:236` `free === null ? <span className="italic …">Đã dùng hết 8 nhóm duyệt</span> : <Button…>` ⇒ không mời bấm thứ chắc chắn 409. ✅
|
||
Kèm: option nhóm đã dùng `disabled` + hậu tố "(đã có kế hoạch)" `:283-287`; nút Lưu `disabled … || groupTaken` `:360`.
|
||
|
||
**SHA-pair 5/5 — reviewer tự tính lại (`sha256sum`, 12 ký tự đầu):**
|
||
`types/khkk.ts 72a71451b98f` · `KhkkCreatePage 677f4ab803b0` · `KhkkDetailPage 1863153b4f99` · `KhkkListPage e892310434f2` · `KhkkWorkflowPanel 2b64144ef6e8` — **admin == user cả 5**, và **panel == baseline `2b64144ef6e8`** ⇒ control DƯƠNG đứng: phép đo có phân biệt được "đụng" và "không đụng". 5/5 khớp con số lane tự khai.
|
||
|
||
## §5 Nghi-vấn lead-solo (adversarial)
|
||
|
||
**(i) Có đường trình nào né submit-guard không? — KHÔNG.** `grep -rn "\.Phase = \|Phase = ContractSigningPlanPhase" src/Backend` cho đúng 4 site ghi: `Features.cs:428` (`= DangSoanThao`, create) · `WorkflowService.cs:191` (`= ChoDuyet`) · `:295` (`= DaDuyet`, finalize) · `:359` (`= targetPhase`, chỉ nhận TraLai/TuChoi từ switch `:127-136`). **`= ChoDuyet` duy nhất 1 site**, nằm TRONG `SubmitAsync` SAU guard; `SubmitAsync` private, caller duy nhất là switch `case "submit"`. Guard cũng KHÔNG có nhánh `isAdmin` bỏ qua. ⇒ bác nghi-vấn.
|
||
|
||
**(ii) Backfill=1 vô-điều-kiện có đè nhóm ≠1 không? — KHÔNG, an toàn theo CẤU TRÚC.** `AddColumn` `:47-52` chạy NGAY TRƯỚC `Sql UPDATE` `:61` trong cùng transaction ⇒ tại thời điểm UPDATE **mọi row đều = 0**, không tồn tại row mang nhóm ≠1 để đè. Kịch bản rollback+re-apply: `Down()` `:124-127` **DropColumn** trước ⇒ dữ liệu nhóm mất **do Down**, không do backfill; re-apply set 1 là hành vi đúng của cột vừa tái sinh. ⇒ bác nghi-vấn. *(Dư lượng thật, xem F-9: `defaultValue: 0` để lại DEFAULT constraint 0 trên cột.)*
|
||
|
||
**(iii) DTO append-CUỐI có vỡ FE cũ không? — KHÔNG, và lane khai ĐỦ.** 4 record đổi (`ListItemDto` +2 · `LineDto` +2 · `DetailDto` +2 · `ApprovedPeAwaitingPlanDto` +1), tất cả append cuối; JSON bind theo TÊN. Construction site positional duy nhất của mỗi record nằm cùng file và đã sửa — `dotnet build 0 Error` chính là phép chứng (thiếu 1 site sẽ là CS7036). FE khai đúng cả chỗ **lệch**: `KhkkDetailDto extends KhkkListItemDto` nhưng BE `DetailDto` KHÔNG có `TotalApprovedAmount` ⇒ FE để `number | null` + ghi lý do tại chỗ (`types/khkk.ts:158-164`).
|
||
|
||
## §6 Σ tiền — xác nhận độ mạnh khai báo
|
||
|
||
Lane khai: (a) tầng CẤU TRÚC `grep PeReferenceAmount|ProposedAmount|ApprovedAmount` trong file mig = **0 hit** — reviewer đọc lại file mig, xác nhận 0 hit; (b) tầng PHÉP ĐO local = **control RỖNG** (`plans=0 · lines_cnt=0`, reviewer đo lại bằng sqlcmd, khớp). Lane **tự dán nhãn "control rỗng, không phải bằng chứng có răng"** thay vì dán "PASS". ⇒ **Khai đúng độ mạnh; reviewer KHÔNG đòi phép đo không tồn tại.** Răng thật nằm ở phép đo prod post-deploy (cicd K2).
|
||
|
||
## §7 FINDING (10) — 0 blocker kỹ-thuật, 1 gate quy-trình
|
||
|
||
**F-1 [MEDIUM · data-integrity · sửa ở K4b]** `ContractSigningPlanWorkflowService.cs:178-182` — vế "khác nhóm" hỏi SAI câu: `AnyAsync(c => catalogIds.Contains(c.Id) && c.ApprovalGroup != plan.ApprovalGroup)` = "có tồn tại entry nhóm khác không", trong khi điều cần bảo đảm là "MỌI id đều resolve tới entry ĐÚNG nhóm". `ContractCatalogEntryConfiguration.cs:29` có `HasQueryFilter(x => !x.IsDeleted)` và `CatalogEntryId` là **loose-Guid 0 FK** ⇒ dòng trỏ tới hạng mục **đã xoá mềm** (hoặc Guid rác) **biến mất khỏi tập truy vấn** → `Any` = false → **trình QUA**. Vắng-mặt đọc thành sạch. Tiêu chí đóng: đếm `entries.Count(id ∈ ids && ApprovalGroup == plan.ApprovalGroup) == ids.Distinct().Count()`, lệch thì 409 nêu rõ id nào không resolve. *(Chưa khai thác được ở K2 vì chưa có đường gán hạng mục — nhưng K4b mở đường đó, phải đóng cùng lúc.)*
|
||
|
||
**F-2 [MEDIUM · claim mạnh hơn việc đã làm]** 2 site khai lớp bảo vệ (a)/(1) — *"validator đường gán hạng mục: `CatalogEntryId` BẮT BUỘC — dòng mới qua API không bao giờ mang NULL"* — ở **THÌ HIỆN TẠI**, nhưng lớp đó **CHƯA TỒN TẠI** (K2 không có command ghi Line; validator thuộc K4b). Site: `20260731085624_…FinalizeRuntime.cs:36-40` và `ContractSigningPlanLineConfiguration.cs:26-31`. Hiện thật **1 lớp** (submit-guard), không phải 2. Đối chứng cùng diff: comment trong `SubmitAsync:180` viết ĐÚNG thì tương lai (*"validator đường-gán K4b là fail-fast tầng trước"*) ⇒ 2 giọng lệch nhau trong cùng một wave. Tiêu chí đóng: đổi 2 site sang thì tương lai + đánh dấu nợ K4b.
|
||
|
||
**F-3 [MAJOR · gate quy-trình · đóng TRƯỚC/CÙNG commit]** `docs/STATUS.md:6` đang ghi `Mig **70** … **598 test PASS** (45D+553I)`; thực đo sau K2: **Mig 71** · **607 test (45D+562I)** · bảng vẫn **97** (Mig 71 = 0 CreateTable) · MenuKeys/Policies KHÔNG đổi. `docs/STATUS.md` **không nằm trong working-tree diff** ⇒ commit K2 nguyên trạng = drift tức thì. Cùng LỚP với finding đã nêu ở gate K1 (`STATUS.md:6` chưa khai DELTA) — **tái phát, chưa đóng**.
|
||
|
||
**F-4 [LOW-MEDIUM · doc sẽ đi kèm commit]** `.claude/skills/ef-core-migration/SKILL.md` (uncommitted) ghi `Total: **96 bảng**` và bảng migration **dừng ở Mig 69** — đã bị chính K1 (`50e6d8c`, Mig 70 `+ContractCatalogEntries`) làm sai TRƯỚC khi kịp land; đo thật `sys.tables = 97`. Thiếu row Mig 70 **và** Mig 71. Ai commit K2 sẽ mang con số sai này vào lịch sử.
|
||
|
||
**F-5 [LOW · neo-số trong chú thích]** `KhkkListPage.tsx:176-177` (×2 app) còn ghi *"bảng **7 cột** cuộn ngang"* trong khi CHÍNH diff này nâng lên **8 cột** (đã sửa đúng `colSpan` 7→8 hai chỗ). Sửa mã mà không sửa số trong chú thích cạnh nó.
|
||
|
||
**F-6 [LOW · acceptance tự lão hoá + không có test phủ]** Nền `sub-invest-fable-b2-cum1.md` K2.h ghi acceptance `curl list ?approvalGroup=2`, nhưng controller bind `[FromQuery(Name="group")]` ⇒ gõ `?approvalGroup=2` **không lọc gì cả**, trả **toàn bộ** nhóm với HTTP 200 — sai IM LẶNG chứ không 400. Thêm: 9 test mới đo ở tầng **query handler**, **0 test** chạm tên tham số ở controller ⇒ đổi/gõ nhầm tên sẽ không ai bắt. Tiêu chí đóng: sửa chữ acceptance thành `?group=2` (và/hoặc 1 test controller-binding).
|
||
|
||
**F-7 [MEDIUM · lý-do-thiết-kế nêu SAI]** Rào (PeId, ApprovalGroup) là **check-then-insert không khoá** (`Features.cs:388-395` `AnyAsync` rồi `Add`) ⇒ 2 request đồng thời cùng (PE, nhóm) đều qua rào → **2 phiếu sống cùng nhóm**, làm hỏng đúng bất-biến `COUNT == Σ 8 folder`. Lý do bỏ unique DB được nêu **2 lần** (`ContractSigningPlanConfiguration.cs:33-36` + `Features.cs:390-393`): *"unique index không diễn tả được vế `Phase != TuChoi`"* — **sai kỹ thuật**: SQL Server cho phép filtered index với so sánh hằng, `UNIQUE (PurchaseEvaluationId, ApprovalGroup) WHERE [Phase] <> 3 AND [IsDeleted] = 0` là hợp lệ và tự nhả chỗ khi phiếu chuyển TuChoi. Không đòi phải dựng index ngay — đòi **sửa lý do**, vì lý do sai sẽ được chép tiếp sang K4b/K5.
|
||
|
||
**F-8 [LOW · rollback]** `Down()` `:132-137` tạo lại UNIQUE (Plan, Supplier) SAU khi đã DropColumn `CatalogEntryId` ⇒ một khi K4b sinh **nhiều dòng/NCC khác hạng mục** (đúng năng lực K2 mở ra), rollback Mig 71 sẽ **FAIL giữa chừng**. Khai trước rẻ hơn phát hiện lúc cần rollback.
|
||
|
||
**F-9 [INFO · dư lượng của (ii)]** `AddColumn<int>(… defaultValue: 0)` `:47-52` để lại **DEFAULT constraint = 0** trên cột `ApprovalGroup` ở DB. Mọi đường EF an toàn (entity default `= 1`, handler luôn set tường minh), nhưng INSERT bằng **SQL thô** (seeder tương lai) sẽ ra nhóm **0** — ngoài dải 1..8, rơi NGOÀI cây 8 folder mà không có gì kêu.
|
||
|
||
**F-10 [INFO · positive validation]** Những chỗ CHỊU được soi: (a) `existingGroups` dùng chung `db.ContractSigningPlans` nên **thừa hưởng global filter `!IsDeleted`** (`Configuration:53`) ⇒ nghi-vấn mở của FE lane §5.3 ("phiếu xoá mềm có rơi khỏi tập không") = **CÓ RƠI**, picker nhất quán với rào Create; (b) consumer của endpoint picker chỉ có `KhkkCreatePage` ×2 app (grep toàn `src/Backend` + `fe-*/src`) ⇒ đổi hình dạng không làm phồng badge/KPI nào khác; (c) auto-gen line không sinh 2 dòng cùng NCC nên unique 3-cột toàn-NULL **không đẻ lỗi 2601 mới** so với unique 2-cột cũ (tập hành vi tương đương ở nhánh NULL).
|
||
|
||
## §8 Ghi chú phạm vi commit (không tính là finding)
|
||
|
||
`git status` có **10 file governance/doc** (`CLAUDE.md`, `.claude/skills/*`, `.claude/commands/*`, `.claude/auto-memory/*`, `docs/governance/*`, `.session-counter.json`) **không thuộc K2** đang cùng nằm trong working tree. Người commit cần stage CÓ CHỦ ĐÍCH (hoặc khai chúng trong commit message), đừng để `git add -A` gộp im lặng.
|
||
|
||
END sub-reviewer-gate-k2 — VERDICT=PASS-WITH-FLAGS · 10 finding (F-3 = gate quy-trình phải đóng trước/cùng commit; F-1/F-2/F-7 nợ K4b) · build 0W/0E · test 607/0 (45D+562I) · npm ×2 PASS · SHA-pair 5/5 + control-dương đứng.
|
||
|