# RESUME — Approval Engine → All Documents + Inventory — ✅ ALL 6 WPs DONE

**Status:** ✅ **DONE.** All 6 WPs + **5 review-fix rounds** (the final cross-WP review returned BLOCK three times — every finding was a bug WE introduced). Deployed to `/app`. **Awaiting owner `/fullpush`** — no merge to main performed.
**Branch:** `hazemdev2` (both repos). **Dev DB:** `moonui2_dev_be` (one additive migration, forward-only).
**Ledger:** `LEDGER.md` — every WP, every finding, every deviation, every correction.
**Approved plan:** `../approval-all-documents-inventory-analysis.html`

## What shipped
The approval engine went from **8 → 13 document types**. The five newly wired ones are the ones that **move stock and post journal entries**:

| Document | What its commit does | Now gated |
|---|---|---|
| **Stock Adjustment** | moves stock — **no JE, no second signature** (the shrinkage/fraud vector) | ✅ |
| **Stock Issue** | decreases stock + posts a **COGS** journal entry | ✅ |
| **Inventory Receipt** | increases stock, drives the GR-IR accrual | ✅ |
| **Purchase GRN** | moves stock + posts the **GR-IR** journal entry | ✅ |
| **Sales Delivery Note** | deducts stock (when `stock_deduction_point = 'delivery'`) | ✅ |

**Design — "authorize, then post":** `submit-approval` holds the document; governance approve/reject go through the engine; the **existing commit** gained `assertApprovedForPost()` at its top. The commit was deliberately **not** routed through the engine — that would make the last approver's click the thing that moves the stock, destroying the keeper's role.

**The rename:** the inventory "اعتماد" button was never an approval — it **was the posting action**. It is now called what it is (**استلام / صرف / ترحيل**), and ✅ green is reserved exclusively for the governance approve. Without this the keeper saw two green ✓ buttons meaning opposite things.

**No document type is "configurable but inert" any more.** The config dropdown is driven by `GET /core/approval-workflows/document-types` and filters `wired === false` — the trap that made `delivery_note` and `grn` selectable-but-dead cannot recur.

## Holes found and closed that were NOT the assignment
The engine we were extending was **bypassable, and the inbox was broken**:
1. **Rejection was advisory.** `reject()` only flipped the logs; the document was untouched. A rejected sales order stayed `draft` → retry Confirm → the guard saw no *pending* log → **the rejected document committed.** (A rejected stock adjustment would have posted and destroyed stock on a retry.)
2. **The workflow was bypassable from the API.** Invoice/bill/purchase-return `canApprove()` allowed **Draft** → approve a draft directly → no engine logs → straight to Approved → post. **All levels skipped.**
3. **The inbox bricked documents.** Approving the last level from "My Approvals" cleared the engine but never advanced the document — an invoice whose approvals were all in **could never leave `PendingApproval`**. The inbox is the primary approver UI.
4. **🔴 The gate could be bought for a penny.** The adjustment threshold was computed from the **user's own `unit_cost`** (zero allowed). A 5,000-unit write-off typed at `0.01` → total 50 → under every threshold → **auto-approved, zero logs, posted in one click.** *The exact vector the feature exists to stop, reopened through the cost field.* Fixed: decrease lines are now priced at **system cost** (WAC/FIFO, read-only). Test pins it: the user types 50, **the engine sees 500,000**.
5. **Authorizing required the right to post.** Governance rode the commit permission — so *every authorizer was someone who could just post it himself*. The second signature was worth nothing. Fixed: governance = `core.approval-logs.act`, commit = its own permission. **Zero new permissions.**
6. **A phantom-stock bug our own change would have created.** `CancelDeliveryNote` read `status !== Draft` as "stock was deducted". The new `PendingApproval` matched it → cancelling a pending note would have **created stock from nothing**. Caught, fixed, and the whole codebase re-grepped for the pattern (**zero remaining**).
7. **The inbox was useless for 8 of 13 types** — the deep link dropped the approver on a bare list with no idea which row. All 13 fixed.
8. **Opening-balance approve/edit/delete were DEAD** — all three 404'd (routes never existed). Re-pointed at the receipts resource.

## The last two bugs (found by the final cross-WP review — both OURS)
- **A reachable DEADLOCK we introduced.** Our TOCTOU fix added a row lock on the GRN. Meanwhile a listener locked GRNs via an **unindexed** column → under REPEATABLE-READ that is a full table scan that **X-locks every row**. Two ordinary concurrent GRN approvals → **InnoDB deadlock, one transaction rolled back mid-post.** Fixed with a **UNIQUE index** + a **PK lock** in the listener + the invariant written into the code.
- **The reject race.** The reject endpoints evaluated their guard on an **unlocked** model, then blind-wrote Draft — so a post landing underneath was overwritten → **re-post → stock moved twice.** Our own CRITICAL-1 fix was closed against a *sequential* attacker and still open against a *concurrent* one. Fixed with lock + re-read in all five.
- **Honest limit:** the test driver is SQLite, which compiles `lockForUpdate()` to nothing — so no test *proves* InnoDB serialises the transactions. What is proven is that the mechanism changed (unique index; PK lock; no predicate lock left on that path) plus a test that the reject now **sees** a post committed underneath it (422, stays Approved).

## Tests
**Full suite (Inventory + Purchases + Sales + Core): 1623 passed / 26 failed — the 26 are *exactly* the pre-existing baseline, matched one by one. ZERO new failures.**
`ng build` green. ~90 new tests across the feature.

**The two proofs that matter**, asserted for every one of the five documents:
- **Pending ⇒ ZERO stock movements, ZERO journal entries** (table-wide counts, not scoped).
- **No workflow ⇒ `ApprovalLog::count() === 0`** and stock/GL byte-for-byte as before. *This is what protects everyone already using the system.*

## ⚠️ Operational note for the owner (do this before testing)
**Any role you name as an approver must hold `core.approval-logs.act`** ("Approval Logs → Act"). `owner`, `admin`, `manager` already do. **`accountant`, `hr_manager`, `lab_manager`, `Lab Director` do NOT** — despite holding several `.approve` permissions. Grant it from the new roles screen (one checkbox).
It was **deliberately not auto-granted**: auto-granting it to every `.approve` holder would have re-merged authorize and post and silently undone fix #5.

## 🔴 Open — surfaced, NOT done (do not assume otherwise)
1. **[LOUDEST] The adjustment VALUATION still trusts user cost.** The *threshold* is hardened; the *posting* still values the movement at the user's `unit_cost`. So an approved penny write-off posts 5,000 units out at **50 EGP in the books**. The control works — **the number in the ledger is wrong.** Pre-existing valuation bug, outside the approved scope. **Offered to the owner; awaiting his call.**
2. **Threshold hardening gap:** a product with no `StockBalance` *and* no FIFO layers resolves to system cost 0 → the user's penny governs again. Rare. Floor it at the product's default/last-purchase cost.
3. **Three flows create receipts that bypass the receipt workflow** (supplier bill under receipt-recognition, production finished-goods, opening balances). **Not a regression** — nothing gated receipts before — but the CHANGELOG promised more than the code delivers, so a caveat was added in both languages rather than leaving a false promise of protection.
4. **The older 8 documents still FUSE authorize + post** in one `approve()` on one permission. The 5 new ones are the correct shape. Natural next WP.
5. **The older 8 gate `submit-approval` on `.approve`** — only an approver can submit for approval, which is backwards (the preparer should submit).
6. **Inbox rot in 6 other documents** — `cancel()` doesn't close the cycle. The shared helper exists; each needs one line.
7. **Issue estimate/actual gap:** WAC/FIFO can move between submit and post, so a doc authorized at level 1 could post at level-2 value.
8. **A pending standalone receipt has no cancel/delete exit.**
9. **Salami-slicing** is inherent to per-document thresholds (five 1-unit delivery notes each fall under `min_amount`) — true of **every** document in the engine, invoices included. Worth knowing.

## Deferred by explicit decision (not oversights)
- **Inventory Count** — deliberately NOT wired. `finalize()` moves no stock; it produces a *Draft Adjustment*, which this feature already gates. Wiring it would approve the same event twice.
- **Warehouse Transfer** — the engine is entirely amount-threshold driven and a transfer has no value; a workflow would fire on everything or nothing. Needs a quantity-threshold mode in the engine.
- **Opening Balance** — creates and approves in one transaction; there is no draft state to hold. Needs an endpoint restructure.

## Next
Owner runs `/fullpush`. No merge to main was performed by this work.