# Retail System Audit

**Branch:** `retail-audit` (from `origin/main` @ `22572e4`)
**Date:** 2026-05-11
**Scope:** Retail-only — POS, products/catalog, variants, categories, pricing, bundles, barcodes/labels, sales, returns, voids, shifts/drawer, inventory movements, stock levels, receiving, transfers, stocktakes, wastage, branch scoping, accounting posting, receipts, frontend wiring, offline/outbox.

**Mode:** READ-ONLY. No code changes, no migrations, no refactors. Findings only.

---

## 1. Executive summary

The Retail surface is **functionally complete** but has a small number of **operationally serious gaps** clustered around three themes:

1. **Money + inventory integrity** — POS sales accept a client-supplied `unit_price` with no catalogue check, payment-sum-vs-total is never validated, and `InventoryService::move()` allows on-hand to go negative. Together these are the highest-impact items.
2. **Permission granularity** — there is NO `permission:pos.*` or `permission:retail.*` middleware on any retail route. `PosController` has zero `authorizeP()` calls. Inventory controllers gate writes on the broad `inventory.view` permission only. Step-up is wired on void + drawer-moves but not on returns/refunds.
3. **Dead / partial surfaces** — `ProductVariantController` (BE, 92 LOC, 4 routes) has **no frontend consumer**; bundles model exists but its POS-sale decomposition isn't covered; transfer "in-transit" is a status string, not a real ledger state.

The accounting posters (`PosSalePoster`, `PosVoidPoster`, `PosReturnPoster`, `InventoryMovementPoster`) ARE keyed on `(company, source, source_ref)` so journal-side idempotency is solid. The offline outbox (`app/src/core/offline/outbox.js`) exists and is wired into `useCreateSale`.

386 retail-related tests pass; **4 fail — all are AP/AR step-up token issues (HTTP 419), not Retail-caused** (failures live in `AccountingTest` / `ArApVoucherPostingTest`).

---

## 2. Baseline

| Check | Result |
|---|---|
| Branch | `retail-audit` @ `22572e4` (latest main) |
| Working tree | clean (one untracked orphan platform doc, unrelated) |
| `security:audit-tenant-permissions` | **clean** (0 violations) |
| `php artisan test --filter='Retail\|Pos\|Inventory\|Catalog\|Pricing\|Return\|Shift\|Journal\|Accounting'` | **386 pass, 4 fail** (the 4 are pre-existing AP/AR step-up failures returning 419 — unrelated to Retail) |
| `npx vitest run` | 191/191 pass |
| Retail-domain routes | 368 (includes accounting & adjacent) |
| Failures attributable to Retail | **0** |

---

## 3. Backend audit

### 3.1 Surface inventory

| Layer | Where | Size |
|---|---|---|
| POS controller | `app/Http/Controllers/Pos/PosController.php` | 730 LOC |
| POS service | `app/Services/Pos/PosService.php` | 274 LOC |
| Catalog controllers (10) | `Catalog/{Product,Category,PriceList,PricingRule,ProductBarcode,ProductBatch,ProductSupplier,ProductVariant,Unit,CatalogScan}Controller` | 1335 LOC total |
| Inventory controllers (7) | `Inventory/{Adjustment,Inventory,InventoryReport,Receiving,Stocktake,Transfer,Wastage}Controller` | 2127 LOC total |
| Retail services | `Pos/PosService`, `Catalog/{PriceList,CatalogScan}Service`, `Pricing/PricingResolver`, `Inventory/{Inventory,Stocktake,ExpiryLookup}Service`, `Inventory/ReorderAlertNotifier` | 1221 LOC total |
| Accounting posters | `PosSalePoster` (168), `PosVoidPoster` (74), `PosReturnPoster` (164), `InventoryMovementPoster` (235) | 641 LOC total |
| Models | PosSale, PosSaleLine, PosSalePayment, PosShift, PosDrawerMovement, PosReturn, PosReturnLine, Product, ProductVariant, ProductBarcode, ProductBatch, BundleComponent, Category, PricingRule, InventoryAdjustment, InventoryLevel, InventoryTransfer, Receiving, StockLocation, StockMovement, Stocktake, StocktakeCountLine, WastageEvent | — |

### 3.2 POS sale lifecycle

**Flow** (`PosController::createSale` → `PosService::createSale`):
1. Validate request (`PosController:375-389`)
2. Verify shift exists + belongs to caller's tenant (`PosController:391-394`)
3. Inside `DB::transaction` (`PosService:46`): create `PosSale`, resolve price via `PricingResolver`, decrement stock via `InventoryService::move`, charge each payment, create `PosSalePayment` rows, auto-post GL via `PosSalePoster`.

**What's right**:
- Single transaction wrapping sale + stock + payments + journal.
- GL posting via `PosSalePoster::post` is idempotent on `(company, source='pos_sale', source_ref=sale.id)` (`PosSalePoster.php:162`).
- `guardSameTenant()` on every existing-entity load (show, void, return).
- ZATCA QR + signature placeholder fields are present (`PosService:218-221`).

**What's broken / risky**:

| # | Finding | Evidence |
|---|---|---|
| 1 | **Client-supplied `unit_price` overrides catalogue.** The request validates `lines.*.unit_price` as `nullable, numeric, min:0`. A client can send any price and it's used directly — bypassing `PricingResolver`. | `PosController.php:383` |
| 2 | **Payment sum is never compared to total.** Loop sums `$paid += $p['amount']` then computes `change = max(0, $paid - $total)`. A sale with `payments[]` summing to LESS than `total` is accepted; the sale posts with the full total and `change_due=0`, undercharging the customer. | `PosService.php:208-210` |
| 3 | **`shift_id` validated to exist + match tenant, but no branch-membership check.** A cashier with knowledge of another branch's shift id can ring up on it. | `PosController.php:391-394` (only checks `company_id`) |

### 3.3 Void / return

**`voidSale`** (`PosController:407-442`):
- Status guard at line 410 (`Only completed sales can be voided`) prevents replay at the controller level — a second call sees `status='voided'` and 422s. Operational double-void is well-protected.
- Stock reversed inside DB::transaction.
- Journal reversal idempotent on `(company, 'pos_sale_void', sale.id)` (`PosVoidPoster`).
- Route IS step-up gated: `routes/api.php:696` has `stepup:pos.sales.create`.

**`returnSale`** (`PosController:444-499`):
- NOT step-up gated (`routes/api.php:697`). A 50k SAR refund needs only the sanctum token + tenant access.
- `refund_payments.*.amount` validated `min:0`, with no upper bound tied to the original sale's payment amounts.
- `lines.*.unit_price` validated `min:0` with no upper bound tied to the original line's unit_price → **price-bump-on-refund** is possible.

**Risk classification** for void/return:
- Void: **WORKING** (idempotent, step-up gated, reversal posters mirror posting).
- Return: **PARTIAL** — works, but missing the refund-vs-original guards.

### 3.4 Shift / drawer

**`openShift`** (`PosController` open path): checks for existing open shift per cashier. The check IS scoped to the cashier; not clear from a quick read whether it's also scoped to the branch they're trying to open.

**`closeShift`**: accepts `counted_cash`, computes variance, persists.

**`drawerMove`** (`PosController:driver` path):
- Route IS step-up gated (`routes/api.php:700`: `stepup:pos.sales.create`).
- No idempotency token on the movement row — but the operational pattern (manager step-up + fresh ticket per movement) provides a practical safeguard.

**Risk**: variance recording on `closeShift` exists but there's no automatic flag / approval requirement on large variances. A 10k SAR shortfall closes silently.

### 3.5 Inventory movements

`InventoryService::move()` is the **single entry point** for all stock writes. Every other controller (`Receiving`, `Stocktake`, `Transfer`, `Adjustment`, `Wastage`, POS sale / void / return) calls it.

**What's right**:
- One canonical path. Single DB::transaction. Locks the level row first (`InventoryService:34`). Auto-posts via `InventoryMovementPoster::post` (`InventoryService:78-79`).
- `branch_id` is required on every movement (`InventoryService:54`).
- `stock_movements` has indexes on `(company_id, product_id, variant_id, location_id)`, `(ref_type, ref_id)`, and `moved_at` (`2026_05_05_060001_create_inventory_phase3_tables.php`).
- `inventory_levels.on_hand` is updated transactionally with each movement (line 66-68).

**What's broken / risky**:

| # | Finding | Evidence |
|---|---|---|
| 4 | **No negative-stock guard.** Line 47 computes `$newOnHand = on_hand + qty` without clamping at zero. A POS sale or wastage on a product with 0 on-hand goes to negative. No warning, no opt-in. | `InventoryService.php:47` |
| 5 | **`branch_id` is not indexed on `stock_movements`.** Per-branch reports walk the `(company_id, product_id, variant_id, location_id)` index and filter — fine at small scale, slow at large. | migration 2026_05_05_060001 lines for `stock_movements` |
| 6 | **`on_hand` is denormalised.** The append-only `stock_movements` is the source of truth (`balance_after` is recorded per row), but `inventory_levels.on_hand` is updated in the same transaction. A direct insert into `stock_movements` from a future code path would drift `inventory_levels`. The mitigation is the `InventoryService::move()` chokepoint convention — not enforced at the DB level. | `InventoryService:66-68` |

### 3.6 Receiving / transfers / stocktakes / wastage / adjustments

All five flows have draft → post semantics (wastage posts atomically).

**What's right**:
- All write paths go through `InventoryService::move()` for stock.
- Every poster is idempotent on `(company, source, source_ref)`.
- Stocktake supports scan-based count lines (`stocktakes/{id}/scan`).

**What's risky**:

| # | Finding | Evidence |
|---|---|---|
| 7 | **Adjustments / Receivings / Wastage only require the `inventory.view` permission.** `authorizeP('inventory.view')` gates BOTH reads AND writes. A user who only needs the inventory dashboard can post adjustments. No granular permissions exist (`inventory.adjustments.create`, `inventory.wastage.write`, etc.). | `AdjustmentController.php:54`; `ReceivingController.php:25,55,71`; `WastageController.php` |
| 8 | **Transfer "in-transit" is a status, not a ledger state.** Stock leaves source on `ship()` (deducts source location), but in-transit goods don't appear on any location's on-hand until `receive()`. A "total stock by tenant" report misses goods between ship+receive. | `TransferController::ship/receive` |
| 9 | **Wastage posts atomically with no draft.** `WastageController::store` creates and posts in one transaction — no chance to review before the GL journal is written. Acceptable design choice, but worth flagging because there's no reversal endpoint either. | `WastageController.php` |

### 3.7 Permissions + step-up matrix (Retail routes)

| Route | `permission:` middleware | `stepup:` middleware | Controller `authorizeP()` |
|---|---|---|---|
| `POST /pos/sales` | — | — | — |
| `POST /pos/sales/{sale}/void` | — | `stepup:pos.sales.create` | — |
| `POST /pos/sales/{sale}/return` | — | — | — |
| `POST /pos/drawer-movements` | — | `stepup:pos.sales.create` | — |
| `POST /pos/shifts/open` | — | — | — |
| `POST /pos/shifts/{shift}/close` | — | — | — |
| `POST /catalog/products` | — | — | not seen — `ProductController` does authorizeP on most reads (`catalog.products.view`) but writes use `Gate::authorize('catalog.products.write', …)` patterns |
| `POST /adjustments` | — | — | `authorizeP('inventory.view')` only |
| `POST /receivings/{id}/post` | — | — | `authorizeP('inventory.view')` |
| `POST /wastage` | — | — | `authorizeP('inventory.view')` |
| `POST /transfers/{id}/ship` | — | — | `authorizeP('inventory.view')` |
| `POST /stocktakes/{id}/post` | — | — | `authorizeP('inventory.view')` |

**Bottom line**: zero declarative `permission:` middleware on Retail routes; `PosController` has zero `authorizeP()` calls. Tenant isolation is honoured everywhere (every controller uses `guardSameTenant()` or company-scoped queries). **Role-based access control is effectively binary: have a tenant token → can do everything Retail.**

### 3.8 Pricing / variants / bundles

- **`PricingResolver`** (`app/Services/Pricing/PricingResolver.php`, 136 LOC): resolves best price for `(product, qty, customer, branch, date)`. Walks pricing rules + price lists.
- **Price lists are company-scoped, NOT branch-scoped** — confirmed by `PriceList` model lacking a `branch_id`. A multi-branch tenant cannot run different prices per branch.
- **Variants**: backend `ProductVariantController` (92 LOC, 4 routes — list/store/update/delete). **No frontend consumer** for `/variants` or any `useVariant*` hook. The feature is dead from a tenant's POV.
- **Bundles**: `BundleComponent` model exists; CRUD lives in `GrowthController::bundles*`. POS sale time decomposition (whether a bundle expands to its components or is sold as a unit on the GL) is **not visible** in `PosService::createSale` — needs deeper investigation in a follow-up phase.

### 3.9 Backend top issues (verified)

| # | Severity | File:line | Issue |
|---|---|---|---|
| 1 | **Critical** | `PosService.php:208-210` | Payment sum vs total not validated — undercharge possible |
| 2 | **Critical** | `PosController.php:383` | Client-supplied `unit_price` bypasses `PricingResolver` |
| 3 | **Critical** | `InventoryService.php:47` | No negative-stock guard anywhere |
| 4 | **High** | `routes/api.php:697` | `returnSale` not step-up gated, no upper bound on refund line price |
| 5 | **High** | `AdjustmentController.php:54` + peers | Inventory writes gated by `inventory.view` (read permission). No granular adj/wastage/transfer write perms. |
| 6 | **High** | `PosController.php` (entire class) | Zero `authorizeP()` calls — any tenant user can ring up sales / open shifts / etc. |
| 7 | **Medium** | `PosController.php:391-394` | `shift_id` accepted without branch-membership check on the actor |
| 8 | **Medium** | migration 2026_05_05_060001 | `stock_movements` has no `branch_id` index |
| 9 | **Medium** | `TransferController::ship/receive` | In-transit is a status, not on-hand at any location → tenant reports under-count |
| 10 | **Low** | `ProductVariantController` (whole) | Backend exists, no FE consumer — dead surface |

---

## 4. Frontend audit

### 4.1 Surface inventory

| Module | Path | Notable files |
|---|---|---|
| Retail shell | `app/src/modules/retail/` | `RetailPage.jsx`, `retail.hooks.js`, `retail.api.js`, `retail.nav.js` |
| POS pages | `app/src/modules/retail/pos/` | `PosPage.jsx` (669 LOC), `CheckoutModal.jsx` (422), `ReturnsWizardPage.jsx` (442), `PosFullscreen.jsx` (264), `PosLauncherPage.jsx` (238), `ReceiptsPage.jsx` (178), `ReturnsPage.jsx` (125), `ReceiptComplete.jsx` (121), `CfdPage.jsx` (122), `PosFlowPage.jsx` (234 — storyboard, demo-only), `pos.utils.js`, `cfd-bus.js`, `ZatcaQR.jsx` |
| Merchandise (catalog) | `app/src/modules/merchandise/` | `ProductsPage.jsx`, `AddProductPage.jsx`, `ProductEditModal.jsx`, `CategoriesPage.jsx`, `PriceListsPage.jsx`, `PricingPage.jsx`, `PricingResolverPage.jsx`, `BarcodesPage.jsx`, `BatchesPage.jsx`, `UnitsPage.jsx`, `ProductKindsPage.jsx`, `LabelPrintPage.jsx`, `ImportCsvModal.jsx`, `CatalogShell.jsx`, `catalog.api.js`, `catalog.hooks.js` |
| Inventory | `app/src/modules/inventory/` | `OverviewPage.jsx`, `AlertsPage.jsx`, `LowStockPage.jsx`, `ReorderPage.jsx`, `ReorderRulesPage.jsx`, `ExpiryPage.jsx`, `ProductHistoryPage.jsx`, `ReceiveWizardPage.jsx`, `ReceivingsPage.jsx`, `TransferComposerPage.jsx`, `TransfersPage.jsx`, `StocktakeWizardPage.jsx`, `StocktakesPage.jsx`, `WastagePage.jsx`, `WastageLogDialog.jsx`, `AdjustmentsPage.jsx`, `VanLoadOutPage.jsx`, `VanInventoryPage.jsx`, `VanCloseOutPage.jsx`, `InventoryShell.jsx` |
| Offline outbox | `app/src/core/offline/` | `outbox.js`, `outboxRunner.js` (+ tests) |

### 4.2 POS — sale flow

**Wiring**:
- `useCreateSale` → `POST /pos/sales` ✓ (`retail.api.js`)
- `useCreateReturn` → `POST /pos/sales/{id}/return` ✓
- `useActiveShift` → `GET /pos/shifts/active` ✓
- Manager override modal wired via `ManagerOverride` component (`CheckoutModal.jsx`)

**Offline behaviour**:
- `useCreateSale` checks `navigator.onLine`. If offline, returns a synthetic sale stub with `_offline: true` and enqueues to `outbox` (`retail.hooks.js:128-175`).
- `outboxRunner.js` drains on `online` event + 30 s safety-net polling.
- Idempotency keys are stable per queue entry (no double-post on retry).

**FE risks**:

| # | Severity | File / area | Issue |
|---|---|---|---|
| 1 | **High** | `pos.utils.js:26-57` + `CheckoutModal.jsx:120-138` | Cart totals (subtotal, VAT, total) are computed client-side and sent to `POST /pos/sales`. BE re-validates the line-item unit_price as `min:0` only (see Backend #2). **The BE must re-compute and reject — it doesn't.** |
| 2 | **Medium** | `outboxRunner.js:~54` | Outbox replay loop breaks on first failed call. Flaky network = stuck queue until next 30s poll. |
| 3 | **Medium** | `CheckoutModal.jsx:166` | Failed sale POST surfaces error but no "Retry" button — cashier must close + reopen cart. |
| 4 | **Medium** | offline sale stub `retail.hooks.js:128-143` | The stub echoes client-computed `total` using a frontend `VAT_RATE` constant. If BE VAT changes, offline sales drift until reconciled. |
| 5 | **Low** | `PosPage.jsx:165` | `clearCart()` has no confirmation dialog. |

### 4.3 Merchandise — products / variants / categories / pricing

**Wiring**:
- `GET /catalog/products`, `POST /catalog/products`, `POST /catalog/products/import-csv` all wired.
- `ImportCsvModal.jsx` polls import status until done/failed (~30 polls / 60 s cap then aborts).
- Pricing rules CRUD wired to `/catalog/pricing/rules*`.

**Gaps**:

| # | Severity | File / area | Issue |
|---|---|---|---|
| 6 | **High** | (whole) | **No variant editor UI**. `ProductVariantController` exists on BE (4 routes), zero FE consumer. Variants created via API are invisible / unmanageable in the UI. |
| 7 | **Medium** | `CategoriesPage.jsx` | Categories rendered as a flat list, not a tree. BE supports parent_id; FE doesn't surface hierarchy. |
| 8 | **Medium** | `ImportCsvModal.jsx:68` | Bulk CSV import job polls for 60 s then aborts. Large imports (10k+ SKUs) appear to fail silently. No "view job later" affordance. |
| 9 | **Low** | `PricingPage.jsx` | Branch-aware pricing not surfaced (BE doesn't support branch-scoped price lists, see Backend §3.8). |

### 4.4 Inventory FE

| Page | Wired to | Status |
|---|---|---|
| `OverviewPage` | `/inventory/overview` | ✓ |
| `AlertsPage` | `/inventory/alerts` | ✓ |
| `ExpiryPage` | `/inventory/expiry` | ✓ |
| `ReorderPage` / `ReorderRulesPage` | `/inventory/reorder` + `/inventory/reorder-levels` | ✓ |
| `ReceiveWizardPage` / `ReceivingsPage` | `/receivings*` | ✓ two-stage create→post reflected |
| `TransferComposerPage` / `TransfersPage` | `/transfers*` | ✓ ship + receive split correctly |
| `StocktakeWizardPage` / `StocktakesPage` | `/stocktakes*` | ✓ two-stage count→post reflected |
| `WastagePage` / `WastageLogDialog` | `/wastage` | ✓ |
| `AdjustmentsPage` | `/adjustments` | ✓ |
| `ProductHistoryPage` | `/inventory/products/{id}/history` | ✓ |
| `VanLoadOutPage` / `VanInventoryPage` / `VanCloseOutPage` | `/inventory/van/{location}` | ✓ |

No obvious gaps in the inventory FE wiring. The two-stage flows are reflected. Step-up tickets are attached to mutations (per `inventory.hooks.js` patterns).

---

## 5. FE ↔ BE wiring matrix

| Area | FE | BE | Connected | Tests | Status | Notes |
|---|---|---|---|---|---|---|
| POS sale | ✓ | ✓ | ✓ | strong | **Partial** | unit_price + payment-sum gaps (Backend #1, #2) |
| POS returns | ✓ | ✓ | ✓ | smoke | **Partial** | refund-vs-original guards missing; no step-up |
| POS voids | ✓ | ✓ | ✓ | strong | **Working** | step-up gated, idempotent journal |
| POS shifts | ✓ | ✓ | ✓ | smoke | **Partial** | variance not gated; branch-scope check loose |
| POS drawer | ✓ | ✓ | ✓ | smoke | **Working** | step-up gated |
| Products list/CRUD | ✓ | ✓ | ✓ | smoke | **Working** | — |
| Variants | ✗ | ✓ | ✗ | none | **Broken (FE dead)** | no consumer; controller orphan |
| Categories | ✓ (flat) | ✓ | ✓ | smoke | **Partial** | no tree UI |
| Price lists | ✓ | ✓ | ✓ | smoke | **Partial** | company-only, no branch dimension |
| Pricing rules | ✓ | ✓ | ✓ | smoke | **Working** | — |
| Barcodes | ✓ | ✓ | ✓ | smoke | **Partial** | no regex format check |
| Labels | partial | n/a | partial | none | **Partial** | `LabelPrintPage.jsx` exists; print pipeline depth unclear |
| Stock levels | ✓ | ✓ | ✓ | strong | **Working** | — |
| Stock movements log | ✓ | ✓ | ✓ | smoke | **Working** | — |
| Receiving | ✓ | ✓ | ✓ | strong | **Partial** | gating perm too broad |
| Transfers | ✓ | ✓ | ✓ | strong | **Partial** | in-transit invisible to reports |
| Stocktakes | ✓ | ✓ | ✓ | strong | **Working** | — |
| Wastage | ✓ | ✓ | ✓ | strong | **Partial** | no draft / reversal |
| Adjustments | ✓ | ✓ | ✓ | smoke | **Partial** | gating perm too broad |
| Low stock / reorder | ✓ | ✓ | ✓ | smoke | **Working** | — |
| Bundles | ✗ (Growth) | ✓ (Growth) | ✓ | smoke | **Unclear** | POS-time decomposition needs follow-up |
| Suppliers / PO / RFQ | ✓ | ✓ | ✓ | smoke | **Out of Retail scope** | own module |

---

## 6. Accounting & inventory risk review (classified)

### 6.1 Sale / void / return GL posting

| Event | Poster | Idempotent | Branch on lines | Tax/VAT | Tender split | Status |
|---|---|---|---|---|---|---|
| Sale | `PosSalePoster` | ✓ on `(company, 'pos_sale', sale.id)` | ✓ via `PosSale.branch_id` | computed BE | DR per-tender / CR revenue + VAT | **WORKING** |
| Void | `PosVoidPoster` | ✓ on `(company, 'pos_sale_void', sale.id)` | ✓ | reversed | reversed | **WORKING** |
| Return | `PosReturnPoster` | ✓ on `(company, 'pos_return', return.id)` | ✓ | computed BE | refund per-tender | **WORKING** |

Caveat: the **upstream** sale request still trusts client `unit_price` (Backend #2). A poisoned price flows into the journal as legitimate revenue. The posting layer itself is sound; the data feeding it isn't always.

### 6.2 Inventory journal posting

| Event | Poster | Idempotent | Branch | Status |
|---|---|---|---|---|
| Adjustment | `InventoryMovementPoster` | per movement source_ref | ✓ | **WORKING** for adjustment/wastage/stocktake; the poster skips POS-side movement journals because those are owned by sale posters |
| Wastage | same | ✓ | ✓ | **WORKING** |
| Stocktake variance | same | ✓ | ✓ | **WORKING** |
| Transfer | same | per movement | ✓ | **PARTIAL** — see in-transit risk |
| Receiving | same | per movement | ✓ | **PARTIAL** — qty_received race on concurrent GRNs against the same PO (advisory; mitigated by `lockForUpdate()` per call but not across simultaneous web requests) |

### 6.3 Inventory invariants

| Invariant | Enforced? | How / where it can break |
|---|---|---|
| `stock_movements` is append-only source of truth | ✓ by convention | Direct insert bypassing `InventoryService::move()` would drift `inventory_levels` |
| `inventory_levels.on_hand` = `SUM(qty)` for movements per (product, variant, location) | ✓ inside `InventoryService::move()` transaction | A future code path not using the service is the only break |
| `branch_id` set on every movement | ✓ at the service layer | Required arg; can't be elided |
| `on_hand >= 0` | **✗ NOT ENFORCED** | Sales / wastage / transfers can drive on-hand negative — Backend #3 |
| One open shift per cashier per branch | partial | Existing-shift check by user; branch-scoping unclear in code |
| Per-tenant scope on every row | ✓ | `guardSameTenant()` everywhere, global `BelongsToTenant` scope |

---

## 7. Critical issues (rank-ordered)

| # | Title | Severity | File:line | Impact |
|---|---|---|---|---|
| **1** | **Negative inventory allowed** | Critical | `app/Services/Inventory/InventoryService.php:47` | A sale / wastage / transfer can drive `on_hand` below zero with no warning. Cost-of-goods, reorder, and tax all break downstream. |
| **2** | **Client-supplied unit_price bypasses catalogue** | Critical | `app/Http/Controllers/Pos/PosController.php:383` | A modified FE / a curl request can post a sale with any unit_price. Revenue + VAT both compromised. The `PricingResolver` is invoked only when `unit_price` is null. |
| **3** | **Payment sum vs total never validated** | Critical | `app/Services/Pos/PosService.php:208-210` | A sale completes with `payments[]` summing to less than `total`. Change is computed but the cashier can finalise an undercharged sale. |

## 8. High-priority issues

| # | Title | Severity | File:line | Impact |
|---|---|---|---|---|
| **4** | **Returns not step-up gated; no refund vs original guard** | High | `routes/api.php:697`; `PosController.php:444-499` | A 50k SAR refund needs only a tenant token. `refund_payments.*.amount` and `lines.*.unit_price` have no upper bound vs the original sale. |
| **5** | **Inventory writes gated by `inventory.view` permission** | High | `AdjustmentController.php:54` + peers | Any user with the inventory dashboard permission can create adjustments / receivings / wastage / post stocktakes. No granular write perms exist. |
| **6** | **PosController has zero `authorizeP()` calls** | High | `app/Http/Controllers/Pos/PosController.php` | Any authenticated tenant user can ring up sales, open shifts, request returns. The only gate on dangerous ops is step-up on void/drawer. |
| **7** | **No variant editor on FE** | High | (entire) | Backend exposes 4 variant routes; FE has zero consumer. Variants created via API or import can't be edited via UI. |

## 9. Medium-priority issues

| # | Title | Severity | Evidence |
|---|---|---|---|
| **8** | **Shift `shift_id` not branch-membership-checked on caller** | Medium | `PosController.php:391-394` |
| **9** | **`stock_movements.branch_id` not indexed** | Medium | migration `2026_05_05_060001` |
| **10** | **Transfer in-transit invisible on inventory reports** | Medium | `TransferController::ship/receive` |
| **11** | **Outbox runner breaks on first replay failure** | Medium | `outboxRunner.js:~54` |
| **12** | **CSV importer caps polling at 60s** | Medium | `ImportCsvModal.jsx:68` |
| **13** | **No "Retry" button on failed sale POST** | Medium | `CheckoutModal.jsx:166` |
| **14** | **Cart totals computed client-side** | Medium | `pos.utils.js:26-57` (mitigated only because BE *should* re-compute — but currently doesn't strictly) |
| **15** | **Offline sale stub uses FE `VAT_RATE` constant** | Medium | `retail.hooks.js:128-143` |
| **16** | **Categories rendered flat, not as a tree** | Medium | `CategoriesPage.jsx` |
| **17** | **Price lists company-scoped only — no branch dimension** | Medium | `PriceList` model |
| **18** | **Shift variance not gated for approval on large amounts** | Medium | `PosController::closeShift` |

## 10. Low-priority cleanup

| # | Title | Evidence |
|---|---|---|
| **19** | `clearCart()` has no confirmation | `PosPage.jsx:165` |
| **20** | Barcode `code` has no regex validation | `ProductBarcodeController` |
| **21** | Wastage / adjustment qty has no max bound | `WastageController`, `AdjustmentController` |
| **22** | `unit_cost` on movement is nullable + can be zero (skews valuation) | `InventoryService.php:61` |
| **23** | `PosFlowPage.jsx` is a demo storyboard (234 LOC) — leave behind a comment or move to `/docs/` | `PosFlowPage.jsx` |
| **24** | Bundle decomposition at POS sale time not visible in `PosService::createSale` — needs follow-up | `PosService.php` |
| **25** | `LabelPrintPage.jsx` print pipeline depth unclear — likely partial | `LabelPrintPage.jsx` |

---

## 11. Recommended fix order

Recommended fix-and-PR order, smallest-blast-first within each block. Each item is independently shippable.

**Block A — Money / inventory integrity (CRITICAL, ship in one PR)**

1. **Negative-stock guard.** Add `if (newOnHand < 0 && !opts['allow_negative']) throw …` in `InventoryService::move()`. Wastage / writeoff / inventory-correction adjustment can pass the flag; POS sale / wastage / transfer cannot.
2. **Authoritative server-side unit_price.** Change `PosService::createSale()` to always re-resolve via `PricingResolver`, even when the client sent a `unit_price`. Keep the client value as a `client_unit_price_hint` in `meta` for audit. Reject sales where the resolved price differs by more than a configurable tolerance.
3. **Payment sum validation.** After the payment loop in `PosService::createSale`, throw `PAYMENT_INSUFFICIENT` if `$paid < $total - 0.01`. Keep `change = max(0, paid - total)`.

**Block B — Permissions & step-up (HIGH, one PR)**

4. **Add `permission:retail.*` middleware** to POS routes:
   - `POST /pos/sales` → `permission:pos.sales.create`
   - `POST /pos/sales/{id}/return` → `permission:pos.refunds.create` + `stepup:pos.refunds.create`
   - `POST /pos/shifts/open` / `close` → `permission:pos.shifts.manage`
5. **Granular inventory write permissions.** Split `inventory.view` into `inventory.view`, `inventory.adjustments.create`, `inventory.wastage.create`, `inventory.transfers.write`, `inventory.receivings.post`. Migrate the existing roles in a single seeder migration.

**Block C — Returns hardening (HIGH, one PR)**

6. **Refund-vs-original guards.** Reject `refund_payments.*.amount` exceeding what was originally paid on that tender; reject `lines.*.unit_price` above the original sale line's unit_price.
7. **Branch-membership check** when creating sales / opening shifts (reject if `shift.branch_id` not in user's branch memberships).

**Block D — FE gaps (HIGH, one PR each)**

8. **Variant editor.** Either build a minimal variant editor inside `ProductEditModal`, or deprecate `ProductVariantController` and remove the routes. Pick a direction.
9. **Outbox replay continuity.** Change `outboxRunner.js` to keep trying subsequent items after a failure (per-item failure isolation), with exponential backoff per item.
10. **Cart totals — sanity-check on BE.** Even after Block A's `unit_price` fix, have BE compute `subtotal/vat/total` from the (now authoritative) unit_prices and compare to client-supplied values; reject mismatches > tolerance.

**Block E — Operational debt (MEDIUM, batch in one PR)**

11. Add `branch_id` index on `stock_movements`.
12. Treat transfer in-transit as a real ledger state (an "in-transit" virtual location with on-hand reflecting goods on the road).
13. Wastage / adjustment max-amount validation + step-up for high-value events.
14. Tree UI for categories.
15. CSV importer: stop polling after 60s but surface the job id with a "Check status later" affordance.

**Block F — Cleanup (LOW, separate PR or tag at the end)**

16. `clearCart()` confirmation, barcode regex, unit_cost nullability, `PosFlowPage` storyboard cleanup, bundle decomposition investigation, label print pipeline investigation.

---

## 12. Out of scope / deferred decisions

These are flagged but explicitly NOT covered by this audit and need product / architecture input before any code:

- **Bundles at POS time**: are bundle line items expanded to component movements (preferred for inventory accuracy) or kept as the bundle (preferred for receipt clarity)? `PosService::createSale` doesn't make this visible.
- **Branch-scoped pricing**: should price lists carry a `branch_id`? Multi-branch tenants need this if they have different cost structures per location.
- **In-transit ledger**: should we add an "in-transit" virtual `stock_location` per branch, or carry in-transit as a separate `inventory_levels`-like table?
- **Label printing**: ESC/POS, ZPL, browser print dialog? Current `LabelPrintPage` doesn't have an explicit driver visible.
- **Allow-negative reasons**: which inventory operations should the negative-stock guard exempt? Today nothing exempts it; a future write-off path may need to.

---

## 13. Sign-off

This branch (`retail-audit`) contains only this document. No code changes. No migrations. No deploy.

**Branch:** `retail-audit`
**HEAD:** to-be-set on commit
**Test summary:** 386 retail-related backend tests pass; 4 fail (all pre-existing AP/AR step-up issues, not Retail). 191/191 vitest pass. Security audit clean.

**Top 10 Retail issues** (cross-referenced to sections 7–8):

1. (§7 #1) Negative inventory allowed — `InventoryService:47`
2. (§7 #2) Client-supplied `unit_price` bypasses catalogue — `PosController:383`
3. (§7 #3) Payment sum vs total never validated — `PosService:208-210`
4. (§8 #4) Returns not step-up gated; no refund-vs-original guards — `routes/api.php:697`
5. (§8 #5) Inventory writes gated by `inventory.view` — `AdjustmentController:54`
6. (§8 #6) `PosController` has zero `authorizeP()` calls
7. (§8 #7) No variant editor on FE — `ProductVariantController` orphan
8. (§9 #8) Shift `shift_id` not branch-membership-checked — `PosController:391-394`
9. (§9 #10) Transfer in-transit invisible to on-hand reports
10. (§9 #11) Outbox runner breaks on first replay failure

**Recommended first fixes** (in order):
1. Negative-stock guard (Block A #1) — one-line change in `InventoryService::move()`.
2. Authoritative server-side unit_price (Block A #2) — small change in `PosService::createSale`.
3. Payment sum validation (Block A #3) — one assertion after the payment loop in `PosService::createSale`.

These three together close the largest revenue / inventory risk surface in retail. They're all server-side, additive (no migration), and ~50 lines total.

---

## 14. Addenda

Two follow-up read-only audits cover surfaces touched only briefly in §7–§9:

- **`ZF_AUDIT.md`** — Zero-Friction pipeline (OCR / shelf scan / reconciliation / ZATCA).
  Headline findings: three of four job kinds are silent stubs; `jobsRun` swallows
  exceptions into HTTP 200; `reconciliationPost` accepts attacker-controlled
  `actual` value that drives a posted GL journal; no `module:zf` gate; no
  `authorizeP()` calls anywhere in `ZfController`.
- **`MERCHANDISING_AUDIT.md`** — catalog / products / variants / pricing / labels.
  Headline findings: variant builder UI in `AddProductPage` is non-functional
  (data silently dropped on submit); catalog writes gated by `catalog.products.view`;
  `cost_price` exposed unconditionally to every viewer.

**Corrigendum to §13 top-10, item #7.**
The line *"No variant editor on FE — `ProductVariantController` orphan"* was
imprecise. There **is** a variant editor in `AddProductPage.jsx:578-705`. It
just doesn't persist anything — `onSubmit` drops `form.variants` from the
payload, and the `/catalog/products/{product}/variants` endpoints stay
orphaned (no FE consumer in `catalog.api.js`). See `MERCHANDISING_AUDIT.md`
finding MERCH-1 for the full breakdown. The risk ranking stays the same;
the failure mode is "silent data loss" rather than "missing feature."

**End of audit. No code changes proposed in this branch.**