# 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`