Stop simultaneous transfers from overdrawing a branch

A transfer read the sending branch's stock and then wrote its movements
as two separate steps. Requests arriving together all saw the same
balance and all passed the check: five simultaneous transfers of 4 from
a stock of 10 all succeeded, sending 20.

The check and the write now run under withStockLock: a short lease on
one product's stock at one branch, taken with a single atomic upsert
(it only succeeds when no unexpired lease exists) and released when the
request finishes. A second request waits briefly, then sees the updated
balance; after 5 seconds it gets 409 "try again". A lease expires after
10 seconds, so a crashed request cannot block the stock for good.

Physical counts take the same lock, so a count is never computed from a
balance a transfer is changing. A double-tapped transfer that races
itself still moves stock once. Sales do not take the lock: they are
recorded even when stock runs out, by design.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-12 10:57:17 +03:00
parent 1fe2421cc4
commit ee5bb5beb8
6 changed files with 277 additions and 54 deletions
+2 -4
View File
@@ -62,7 +62,7 @@ the decisions taken along the way, and what is still open.
| Decision | Why |
|---|---|
| Offline sales are always accepted, even if they take stock below zero. | The sale already happened. Refusing it would lose real cash. Negative stock is shown on the Stock page and fixed with a physical count. |
| Transfers are refused when the sending branch lacks the stock. | Transfers are made online by a manager, so they can be checked. |
| Transfers are refused when the sending branch lacks the stock. | Transfers are made online by a manager, so they can be checked. The check and the move run under a short per-branch, per-product lock (`withStockLock` in `src/lib/server/stock.ts`), so simultaneous transfers cannot together overdraw a branch; physical counts take the same lock. |
| The server price is authoritative; a different price shown on the device is kept (`clientUnitPrice`) and flagged (`priceMismatch`). | Trusting the device's price would let a cashier under-charge. Overwriting it silently would hide a cash discrepancy. |
| A queued sale stays with the branch where it was recorded, even if the cashier has since moved. | That is where the stock left and the cash was taken. |
| A product deactivated while a phone was offline is still recorded at its price. | Same reason as the first row: the sale happened. |
@@ -76,7 +76,7 @@ the decisions taken along the way, and what is still open.
sales from the old version are sent on the next sync.
## Testing
- `npm test` runs 56 API tests against an in-memory MongoDB, covering role
- `npm test` runs 62 API tests against an in-memory MongoDB, covering role
enforcement, tenant isolation for sales, stock and transfers, suspension,
the sync protocol (including a 120-sale backlog replayed twice), and the
business day.
@@ -137,8 +137,6 @@ prices read-only.
- **Isolation by convention.** Tenant filters are still written per route.
The tests cover the sales, stock, transfer and access paths, not every
route.
- **Concurrent transfers.** Two transfers submitted at the same moment
could together send more than the branch holds.
- **Cost of goods is an average.** It uses each product's average buying
price across the business, not first-in-first-out per branch. Units sold
with no purchase on record are counted and flagged, but have no cost.