Stop simultaneous transfers from overdrawing a branch #4

2026-10-12T07:58:55Z

denis_ireri commented

2026-10-12 07:58:55 +00:00

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. Reproduced with a test before fixing: five simultaneous transfers of 4 from a stock of 10 all succeeded, sending 20 units.

Fix

The balance check and the movements now run under withStockLock (src/lib/server/stock.ts): a short lease on one product's stock at one branch, stored in a new StockLock collection.
- Acquired with a single atomic upsert that only succeeds when no unexpired lease exists; released when the request finishes.
- A second request for the same branch and product waits briefly, then sees the updated balance. After 5 seconds it gets 409 "This stock is being updated by someone else. Please try again."
- A lease expires after 10 seconds, so a crashed request cannot block that stock for good.
- Different products or branches never wait on each other.
Physical counts and adjustments 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 (it re-checks for the finished transfer once it holds the lock).
Sales do not take the lock: they are recorded even when stock runs out, by design (see docs/PILOT_READINESS.md).

No MongoDB transactions or replica-set requirement; works on Atlas and a standalone local database alike.

Testing

npm test: 62 tests pass, 6 new in tests/concurrency.test.ts:
- 5 simultaneous transfers of 4 from 10: exactly 2 succeed, 3 get 409; final stock 2 and 8 (this test failed before the fix).
- The same race repeated five times.
- A double-tapped transfer racing itself: applied once.
- A physical count and a transfer submitted together: the result reflects both, in either order.
- A stale lock left by a crashed request is taken over once expired.
- Transfers of different products do not block each other.
Production build passes.

No migration or deploy step needed.

🤖 Generated with Claude Code

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. Reproduced with a test before fixing: five simultaneous transfers of 4 from a stock of 10 all succeeded, sending 20 units. ## Fix - The balance check and the movements now run under `withStockLock` (`src/lib/server/stock.ts`): a short lease on one product's stock at one branch, stored in a new `StockLock` collection. - Acquired with a single atomic upsert that only succeeds when no unexpired lease exists; released when the request finishes. - A second request for the same branch and product waits briefly, then sees the updated balance. After 5 seconds it gets 409 "This stock is being updated by someone else. Please try again." - A lease expires after 10 seconds, so a crashed request cannot block that stock for good. - Different products or branches never wait on each other. - Physical counts and adjustments 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 (it re-checks for the finished transfer once it holds the lock). - Sales do not take the lock: they are recorded even when stock runs out, by design (see `docs/PILOT_READINESS.md`). No MongoDB transactions or replica-set requirement; works on Atlas and a standalone local database alike. ## Testing - `npm test`: 62 tests pass, 6 new in `tests/concurrency.test.ts`: - 5 simultaneous transfers of 4 from 10: exactly 2 succeed, 3 get 409; final stock 2 and 8 (this test failed before the fix). - The same race repeated five times. - A double-tapped transfer racing itself: applied once. - A physical count and a transfer submitted together: the result reflects both, in either order. - A stale lock left by a crashed request is taken over once expired. - Transfers of different products do not block each other. - Production build passes. No migration or deploy step needed. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

denis_ireri added 1 commit 2026-10-12 07:58:56 +00:00

Stop simultaneous transfers from overdrawing a branch ee5bb5beb8

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>

denis_ireri referenced this pull request

2026-10-12 08:38:21 +00:00

Make email and phone unique per business, not platform-wide #5

denis_ireri merged commit 5e0e44c647 into main

2026-10-12 08:40:43 +00:00

denis_ireri deleted branch fix/transfer-race

2026-10-12 08:59:59 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Ezekiel_Nyangweso/ERP#4