[CLAUDE] Docs: Chunk C — chốt Session 21 turn 3 fix gotcha #45 PE button "Trả lại" mismatch

Add gotcha #45 narrative đầy đủ ~120 dòng KEEP rule §6.5:
- Triệu chứng UAT screenshot + user mô tả "Trả về nhưng hệ thống vẫn duyệt"
- Root cause 3 chỗ inconsistency table + BE service path
- Severity CRITICAL data integrity
- Fix Chunk A BE code + 3 test list
- Fix Chunk B FE code diff × 2 app
- Pattern reusable (boundary guard semantic invariant) + phòng tránh tương lai
- References 2 commit + Session 17 spec

+ gotchas.md checklist debug entry 22 quick lookup.

Update STATUS.md Last updated header + count 81→84 test + 44→45 gotcha.
Insert HANDOFF.md TL;DR S21 t3 đầy đủ Chunk A/B/C + state cumulative.
New session log docs/changelog/sessions/2026-05-12-2100-s21-turn3-fix-tra-lai-bug45.md.

Verify:
- 84 test PASS (dotnet test SolutionErp.slnx — Chunk A persisted)
- npm run build × 2 app pass (Chunk B persisted)
- KHÔNG paraphrase / KHÔNG cắt narrative cũ S21 t1/t2/S20 (rule §6.5 KEEP)

Pending: bro confirm push remote `0a3b747..HEAD` 3 commits ahead.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
pqhuy1987
2026-05-13 09:46:52 +07:00
parent 4b29d00716
commit 6d30ba42d1
4 changed files with 362 additions and 3 deletions

View File

@ -0,0 +1,182 @@
# Session 21 turn 3 — 2026-05-12 21:00 — Bug fix CRITICAL "Trả về nhưng hệ thống vẫn duyệt" PE workflow (gotcha #45)
**Dev:** Claude Opus 4.7 1M Max (em main solo, no SOLUTION_ERP sub-agent spawn)
**Duration:** ~1.5h diagnose + fix + test + docs
**Base commit:** `0a3b747` (S21 t2 RAG planning chốt)
**Commits này turn:** `de00887` (BE Chunk A) → `4b29d00` (FE Chunk B) → this (Chunk C Docs)
## Trigger
User UAT 2026-05-12 21:00 screenshot button labeled `← Trả lại` trong PE Workflow Panel (menu "Duyệt") với caption thắc mắc "Trả về nhưng hệ thống vẫn duyệt" + yêu cầu "check lại nhé chỗ Duyệt NCC".
Đây là bug CRITICAL data integrity — NV nhấn "Trả lại" vô tình "duyệt" phiếu sang Cấp tiếp theo + UPSERT opinion vĩnh viễn vào `PurchaseEvaluationLevelOpinions` (Mig 26). Khó rollback vì BE đã `SaveChangesAsync`.
## Diagnose
Em main solo (Implementer REFUSE per multi-agent rule — reasoning chain cross BE/FE+test tightly coupled). Grep `Trả lại|isReject|TraLai` trong fe-user/fe-admin → tìm 3 chỗ inconsistency trong `PeWorkflowPanel.tsx`:
| # | Location | Logic | Bug? |
|---|---|---|---|
| 1 | L205-207 button `isSendBack` | include cả `DangSoanThao` lẫn `TraLai` → label `← Trả lại` đúng | ✅ no bug |
| 2 | L64-66 payload `isReject` | CHỈ check `DangSoanThao`, thiếu `TraLai` → gửi `decision: 1` (Approve) thay vì `2` | 🔴 BUG ROOT |
| 3 | L247-248 dialog `isSendBack` | CHỈ check `DangSoanThao`, thiếu `TraLai` → title fallback `'✓ Duyệt → Trả lại'` (sai semantic) + no amber warning | 🔴 BUG phụ |
**BE side audit** `PurchaseEvaluationWorkflowService.TransitionAsync`:
- L51 `if (decision == Reject)` branch → handle BOTH TuChoi (set Phase=TuChoi) + TraLai (set Phase=TraLai, clear pointer). Correct.
- L97 `APPROVE STEP` branch khi `decision=Approve && fromPhase=ChoDuyet``ApproveV2Async` UPSERT opinion + advance Cấp pointer.
- → Khi FE gửi `decision=1` do bug `isReject` thiếu nhánh TraLai, BE entry → APPROVE branch thay vì REJECT branch → phiếu approve mặc dù user định trả lại.
## Chunk A — BE defense-in-depth + 3 regression test (`de00887`)
### Test-before §7 BẮT BUỘC strict flow
**Step 1:** Write test file `tests/SolutionErp.Infrastructure.Tests/Services/PurchaseEvaluationWorkflowServiceGuardTests.cs` 3 test KHÔNG có BE guard → run → expect FAIL.
**Step 2:** Confirm 2 test FAIL (reproduce bug — BE đi sâu vào ApproveV2Async throw "Phiếu chưa pin workflow definition hoặc workflow không có step") + 1 test PASS (happy path Reject branch — đã pass vì BE đã đúng cho decision=Reject từ Session 17).
**Step 3:** Add BE guard sau L48, trước L51:
```csharp
// ===== GUARD: targetPhase TraLai/TuChoi BẮT BUỘC decision=Reject =====
// Defense-in-depth chặn FE inconsistency (gotcha #45 — Session 21 turn 3).
// Bug: button "← Trả lại" gửi decision=Approve khi target=TraLai → BE skip
// Reject branch → enter APPROVE STEP → ApproveV2Async UPSERT opinion.
if ((targetPhase == PurchaseEvaluationPhase.TraLai
|| targetPhase == PurchaseEvaluationPhase.TuChoi)
&& decision != ApprovalDecision.Reject)
{
throw new ConflictException(
$"Transition tới {targetPhase} BẮT BUỘC decision=Reject (nhận {decision}). " +
"Báo lỗi caller — payload mismatch giữa target phase và decision " +
"(xem gotcha #45 + docs/workflow-contract.md).");
}
```
**Step 4:** Re-run test → 3/3 PASS. Run full suite → 84/84 PASS (58 Domain + 26 Infra = +3 from 81 baseline).
### 3 test cases
1. **`TransitionAsync_TargetTraLai_WithApproveDecision_Throws_AndDoesNotMutateState`** — Bug reproduce. Setup PE ở Phase=ChoDuyet, CurrentApprovalLevelOrder=1. Act: gửi target=TraLai + decision=Approve. Assert: throw `ConflictException` "*TraLai*Reject*" + Phase KHÔNG đổi + CurrentApprovalLevelOrder=1 (no advance).
2. **`TransitionAsync_TargetTuChoi_WithApproveDecision_Throws_AndDoesNotMutateState`** — Consistency cover. Cùng pattern với TuChoi.
3. **`TransitionAsync_TargetTraLai_WithRejectDecision_SetsPhaseTraLai`** — Happy path control. decision=Reject + target=TraLai → BE đi vào Reject branch, set Phase=TraLai, clear pointer (CurrentApprovalLevelOrder=null + CurrentWorkflowStepIndex=null + SlaDeadline=null).
+ `NoOpNotificationService internal sealed` stub trong cùng file (Tests scope) — reusable cho future PE service tests, avoid `INotificationService` real DI complexity.
### Pattern reusable
- **Boundary guard semantic invariant.** Bất kỳ BE service nào nhận payload từ FE → audit invariant `(domain state X) ⇔ (input parameter Y)` → throw early nếu mismatch. Defense-in-depth thay vì trust FE đúng.
- **Test-before flow strict:** Write test → confirm FAIL với exception KHÁC expected (proves bug reproduce) → add fix → confirm PASS với exception ĐÚNG expected. KHÔNG bỏ qua bước "confirm FAIL" — đảm bảo test actually catches bug.
## Chunk B — FE fix mirror 2 app (`4b29d00`)
3 chỗ × 2 app = 6 edits.
### fe-user/src/components/pe/PeWorkflowPanel.tsx
**Edit #1 (L64-66 `isReject`):**
```typescript
const isReject = target === PurchaseEvaluationPhase.TuChoi
|| (target === PurchaseEvaluationPhase.DangSoanThao
&& evaluation.phase !== PurchaseEvaluationPhase.DangSoanThao)
|| (target === PurchaseEvaluationPhase.TraLai // ← THÊM nhánh TraLai
&& evaluation.phase !== PurchaseEvaluationPhase.TraLai)
```
**Edit #2 (L247-248 dialog `isSendBack`):**
```typescript
const isSendBack = (target === PurchaseEvaluationPhase.DangSoanThao
|| target === PurchaseEvaluationPhase.TraLai) // ← THÊM TraLai
&& evaluation.phase !== PurchaseEvaluationPhase.DangSoanThao
&& evaluation.phase !== PurchaseEvaluationPhase.TraLai // ← THÊM guard
```
**Comment update:** Thêm context bug + cross-ref BE guard Chunk A trong comment.
### fe-admin/src/components/pe/PeWorkflowPanel.tsx
Mirror y hệt (rule §3.9 mirror 2 app — duplicate có chủ đích).
### Verify
```bash
# fe-user
cd fe-user && npm run build
✓ built in 17.91s
# fe-admin
cd fe-admin && npm run build
✓ built in 6.71s
```
0 TS6 err, 0 new warnings. Warning chunk size pre-existing (KHÔNG introduced).
## Chunk C — Docs (this commit)
### `docs/gotchas.md` +#45
~120 dòng narrative đầy đủ KEEP rule §6.5:
- **Triệu chứng** — UAT screenshot user mô tả hành vi
- **Root cause** — 3 chỗ inconsistency table + BE service path narrative
- **Severity** CRITICAL — data integrity issue khó rollback
- **Fix Chunk A** BE code block + 3 test list
- **Fix Chunk B** FE code block diff
- **Pattern reusable** — boundary guard semantic invariant + button label ↔ payload sync
- **Phòng tránh tương lai** — grep audit khi spec mới thêm phase + test-before §7 strict flow
- **References** — 2 commit + Session 17 spec
`docs/gotchas.md` checklist debug +entry 22 quick lookup.
### `docs/STATUS.md`
Edit Last updated header thêm S21 t3 + count 81→84 test + 44→45 gotcha. Giữ nguyên S21 t2/t1/S20 narrative cũ (rule §6.5 KEEP).
### `docs/HANDOFF.md`
Insert TL;DR S21 t3 section trên cùng (trước S21 t2). Header Last updated mới + narrative đầy đủ Chunk A/B/C + state table cumulative. Giữ S21 t2/t1/S20 narrative cũ.
### Session log
File này — `docs/changelog/sessions/2026-05-12-2100-s21-turn3-fix-tra-lai-bug45.md`.
## Stats cumulative
| Metric | Trước (S21 t2) | Sau (S21 t3) | Δ |
|---|---|---|---|
| DB tables | 59 | 59 | 0 |
| Migrations | 27 | 27 | 0 |
| Endpoints | ~142 | ~142 | 0 |
| FE pages | 34 | 34 | 0 |
| **Unit tests** | 81 | **84** | **+3** (PE guard) |
| **Gotchas** | 44 | **45** | **+1** (#45) |
| Memory entries | 17 | 17 | 0 |
| Skills | 6 | 6 | 0 |
| Sub-agents | 4 seeds-only | 4 seeds-only | 0 |
| Commits S21 t3 | — | **3** | (`de00887` + `4b29d00` + this) |
## Lessons learned
1. **Mảng inconsistency 3 chỗ cùng pattern** — khi spec mới thêm value vào set check (vd Session 17 thêm `TraLai` làm Phase RIÊNG thay vì DangSoanThao revert), DỄ QUÊN grep TOÀN BỘ logic check `=== OldValue` để thêm `|| === NewValue`. Tốt nhất extract helper function `isReject(target, currentPhase): boolean` share 1 nơi thay vì duplicate 3 chỗ.
2. **BE guard defense-in-depth có giá trị thực** — trong S21 t3, nếu BE chỉ trust FE đúng → bug ship prod, user UAT report, mất data integrity. BE guard early catch payload mismatch + ConflictException rõ ràng → fix nhanh + safe.
3. **Test-before flow strict không chỉ là "viết test" — còn confirm FAIL** — em main đầu tiên định viết test + fix cùng commit (cho gọn). Nhưng test-before §7 BẮT BUỘC confirm test FAIL trước fix. Bước này quan trọng — confirm test actually reproduce bug (assert đúng exception type/message), không chỉ là "test xanh sau fix".
4. **Multi-agent decision tree áp đúng** — Bug fix tightly coupled BE+FE+test reasoning chain → Implementer REFUSE per rule (Cognition "writes single-threaded"). Em main solo correct decision, KHÔNG cố split → tránh agent thrash.
## Handoff
- ✅ Chunk A `de00887` committed local — chưa push
- ✅ Chunk B `4b29d00` committed local — chưa push
- ✅ Chunk C (this) — committed sau khi save session log
-**PENDING bro confirm push remote**`git push origin main` 3 commit ahead `0a3b747..HEAD`
- ⏭ Sau push: CI sẽ trigger (NOT docs-only — có file `.cs` + `.tsx`) → 🟩 CICD Monitor spawn smoke verify (per plan G Trial Week 1)
User next action expected: "fix đi rồi tao giao thêm task" → sau Chunk C wrap → bro chốt task tiếp theo (có thể là Plan B Contract V2 wire hoặc fix khác phát sinh UAT).
## References
- Gotcha #45: `docs/gotchas.md`
- BE service: `src/Backend/SolutionErp.Infrastructure/Services/PurchaseEvaluationWorkflowService.cs`
- FE component (× 2 app): `{fe-admin,fe-user}/src/components/pe/PeWorkflowPanel.tsx`
- Test: `tests/SolutionErp.Infrastructure.Tests/Services/PurchaseEvaluationWorkflowServiceGuardTests.cs`
- Workflow spec Session 17: `PurchaseEvaluationPhase.cs` enum doc + Service comment L15-19
- Rule §7 test-before: `docs/rules.md`
- Rule §3.9 mirror 2 FE: `docs/rules.md`