---
name: POS overhaul
description: The 2026-08 comprehensive POS (point-of-sale) diagnosis and 12-package fix — what was broken and why, the six root-cause families, the locked design decisions, what the seam review caught after all packages were "done", and the deliberate leftovers. Read before touching Modules/POS or src/app/features/pos.
updated: 2026-08-01
refs:
  - knowledge-base/plans/pos-comprehensive-investigation.html   # the diagnosis
  - knowledge-base/plans/pos-fix/LEDGER.md                      # per-package execution record
  - moon-erp-be/Modules/POS
  - moon-erp/src/app/features/pos
related:
  - default-accounts/pos-accounts
---

# POS overhaul (2026-08-01)

**Status:** ✅ all 12 packages + 4 seam fixes implemented on `hazemdev2`, deployed to moonui2 `/app`, **uncommitted** — `/fullpush` is the owner's step. Never shipped to a client from here.

## Why it happened

The owner reported "problems in almost every area of POS, e.g. payment". Four parallel investigation tracks (BE · FE · tests/regressions · design) found ~34 defects that collapse into **six families**:

| Family | Root cause |
|---|---|
| **أ** FE↔BE field-name mismatch (8 instances) | `FormRequest::validated()` **silently drops** any undeclared key. The invoice posted "successfully" while tax/discount/variant were missing. |
| **ب** POS bypasses the Sales guard layer (4 guards) | POS was built as a shortcut *around* the Sales screen: it calls the **Actions** directly, so it inherited the accounting and **not** the FormRequest guards. |
| **ج** The till is not a ledger | No change/over-short/rounding account, **no refund path at all**, returns unlinked to the shift, reports never netted refunds. |
| **د** The cashier loop | F-keys dead in the resting state, change-due vanished after the sale, no denominations, focus never returned to the scanner, silent bad scans. |
| **هـ** The tests guarded an empty corridor | Two independent mechanisms (service product + unseeded settings) meant the stock/COGS body of `PostSalesInvoice` **had never executed in a POS test**. |
| **و** Isolated | Cross-company held-order leak, coupon dead on its own start date, `receipt_number` not unique, two-sessions race, no offline idempotency. |

**The one true regression was a setting, not code:** the company was flipped to `sales.cogs_recognition_point = delivery` (built for partial delivery in Sales). Correct for a delivery business, wrong for a counter — the customer walks out with the goods while stock and COGS wait for a warehouse approval. Nothing in POS changed; the world around it did.

## Locked decisions (owner-approved — do not re-litigate)

1. **Tax** — POS selects a real `tax_rate_id`, like the Sales screen. Never a raw percentage.
2. **Payment amount** — the server records the **amount due**, never the tendered amount. Change is a display concept. (WP1 clamps; **WP2 rejects above a 0.01 tolerance** — the clamp is only the net.)
3. **Delivery mode** — a POS sale **always** deducts stock and posts COGS at sale time, via `RecognitionIntent::AtSale` passed at the POS call site. **No GDN and no delivery note are created** — both are approvable documents that would deduct a second time.
4. **Cash variance** — stays **data-only**, no journal entry. The fix was the *expected* figure, not the accounting.
5. **Unknown payload keys** — rejected with a 422 naming the key.

## Load-bearing facts for whoever comes next

- **`RejectsUnknownKeys` makes the server strict.** Any new FE field must be declared in the POS request class *first*, or every sale 422s. This is why `local_id` could not be sent until WP8-BE landed.
- **`ValidatesMinimumSalePrice`** lives in **Sales**, not Core, on purpose: its dependencies are a `sales.*` setting and a `sales::` message. `ValidatesLineVariantOwnership` is in Core because Purchases/Inventory consume it.
- **Idempotency:** `sales_invoices.pos_idempotency_key` + unique `(company_id, pos_idempotency_key)`. The pre-check answers the sequential retry; **the unique index arbitrates the race**, and the invoice INSERT is the first write so the loser rolls back before any stock/payment/JE. Replay → **200** + `meta.idempotent_replay`. The FE mints `local_id` **before the first attempt** (minting it at enqueue time left the online attempt — the one that can succeed without answering — anonymous).
- **Refund credit notes inherit the invoice's three compound header-discount percentages** rather than an apportioned amount: compound rates are scale-invariant, so a part return carries exactly its share and a full return reproduces the invoice total. Fixed in the **shared** layer — the back-office `/sales/returns` path had the identical defect.
- **The till refund cap:** `min(note total, amount_paid − Σ prior till refunds)`. Cancelled credit notes **still consume the cap**, because `CancelSalesReturn` does not put the banknotes back.
- **Pest loads every test file into ONE process.** A duplicate top-level `function` name is a fatal redeclare → **exit 255 with zero output**. It killed the whole POS suite mid-batch (`posSalePayload` defined by two parallel agents) and has now bitten this project **three times** (Clinic, Inventory, POS). Prefix helpers with their file's subject.

## What the seam review caught after every package reported "done"

Twelve packages, eight agents, all green individually. A whole-tree review then found **four blockers** none of them could see:

1. **A refund paid more than the customer paid** on every coupon sale — the header discount lives only on the invoice header, and the credit note re-priced from the untouched item rows (92 collected → 115 refunded).
2. **`POST /pos/refunds` accepted any posted invoice of the company** — unpaid, or not a POS sale at all. A cashier could refund a 60-day wholesale invoice and hand over cash the company never received, and the shift's expected-cash then subtracted it so the close reconciled cleanly over the loss.
3. **The new `pos.refunds.create` permission had no bridge migration** — the updater runs `migrate`, not the seeders, so the headline feature would have been dead on 100% of the existing fleet.
4. **A shift could be closed over queued offline sales**, which then 422 forever against a closed session and park — cash in the drawer, no invoice, unpostable.

**Lesson worth keeping:** per-package green is not evidence about the seams. The defects that mattered most were *created by* the integration, and two of them were only reachable because two correct packages met.

## Deliberate leftovers (not defects of omission — decided)

- `CancelSalesReturn` does not reverse the POS cash-out JE; the cap was made conservative against it (cancelled refunds still consume the cap) so it cannot cause a double payout, but the ledger asymmetry stands.
- `CancelSalesInvoice` still allows cancelling an unpaid invoice that has live credit notes (the POS double-restore route is closed; a back-office one is theoretical).
- **Line-level** `discount_percent` can still undercut the min-price floor — pre-existing and **identical on `/sales/invoices` and `/sales/orders`**, so closing it in POS alone would give the till and the Sales screen two different answers. One follow-up across all three requests.
- `PostSalesPayment` sets `partner_id` on a JE **line**, but `journal_entry_lines` has no such column and the header never gets it ⇒ **no partner attribution in the AR sub-ledger for any payment, POS or Sales**. Found in WP0, still open, **wider than POS**.
- Sub-millieme drift when a money coupon is stored as a 3-decimal percent; the cash cap absorbs it.
- POS reports keep `total` (goods returned) while the shift uses `pos_refund_amount` (cash out) — divergence documented in both.

## Numbers

| | Before | After |
|---|---|---|
| `Modules/POS/tests/` | **10 passed / 34 assertions** | **181 passed / 781 assertions**, 0 failed |
| `Modules/Sales/tests/` | 467 / 11 pre-existing failures | **471 / 11** — same three files, no new failure |

Six migrations, all forward-only and verified applied on `moonui2_dev_be`.