Skip to content

Commit d0f2b38

Browse files
OneTwo3Dclaude
andauthored
Stock/concurrency invariants — explicit transfer guard + verification (audit-M-stock) (#206)
* fix(stock): make transfer reserved-stock guard explicit; document stock invariants (closes audit-M-stock) Stock/concurrency cluster (bkhk). Most findings were already resolved by prior work; this makes the remaining guard explicit + tested and documents the rest: 1. Transfer-of-allocated-stock: dispatch availability already nets the source warehouse's per-(product,warehouse) reservedQty (kept in sync with order allocations), so a transfer can't strand an order. Extracted that rule into a pure availableForTransfer/canDispatchTransferQty helper (used by dispatch) so it's explicit and regression-tested. 2. Manual receipt + WMS booked-in double-layer: VERIFIED resolved — reconcileBookedInQuantities nets localReceivedQty (read under the PO FOR UPDATE lock); already covered by mintsoft-phase6-booked-in tests. 3. Opening-stock race: VERIFIED resolved — applyOpeningStock takes a FOR UPDATE lock on the stock level before the existing-opening-layer check, serialising concurrent calls. 4. Transfer FIFO ordering at destination: documented + accepted (cosmetic; per layer cost basis preserved). 5. stock_levels CHECK >= 0: VERIFIED present (migration 20260512100000: quantity>=0, reservedQty>=0, reservedQty<=quantity). Documented all five in docs/workflows.md under Stock / concurrency invariants. Tests: 5 transfer-availability cases; type-check + lint clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(stock): preserve over-reservation diagnostic + precise docs (adversarial review) Codex adversarial pass on PR #206 (verified all three 'already-resolved' claims are substantively correct; no refuted claims): MEDIUM — the extracted helper clamped a negative (over-reserved) available to 0, so the dispatch error message showed '0 available' instead of the real negative delta, hiding a data-integrity signal. The error now reports the raw (unclamped) delta while the clamped value still gates dispatch. MEDIUM — docs overstated the reservedQty<=quantity constraint: it's NOT VALID (new-writes-only, never validated against historical rows), unlike the fully VALIDATEd quantity>=0 / reservedQty>=0. Docs corrected, and note the transfer guard (not just the constraint) is what protects allocations. LOW — docs now mention both the PO and stock-transfer FOR UPDATE locks for the WMS reconcile; JSDoc warns availableForTransfer's clamp must not be reused for data-integrity checks; added an over-reserved canDispatchTransferQty test. 6/6 helper tests; type-check + lint clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 0f976be commit d0f2b38

4 files changed

Lines changed: 103 additions & 3 deletions

File tree

‎app/actions/transfers.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
} from '@/lib/cost-layers'
1818
import { sliceTransferSnapshotForReceipt } from '@/lib/domain/wms/asn-reconciliation'
1919
import { toInventoryConstraintMessage } from '@/lib/domain/inventory/prisma-errors'
20+
import { canDispatchTransferQty } from '@/lib/domain/inventory/transfer-availability'
2021
import { addMoney, multiplyMoney, roundQuantity, toDecimal } from '@/lib/domain/math/decimal'
2122
import { serializeCostLayerSnapshot } from '@/lib/cost-layer-snapshots'
2223
import {
@@ -388,9 +389,13 @@ export async function dispatchTransfer(id: string): Promise<TransferResult> {
388389
where: { productId_warehouseId: { productId: line.productId, warehouseId: transfer.fromWarehouseId } },
389390
select: { quantity: true, reservedQty: true },
390391
})
391-
const available = level ? Number(level.quantity) - Number(level.reservedQty) : 0
392-
if (available < qty) {
393-
throw new Error(`Insufficient stock for ${line.sku}: ${available} available, ${qty} requested`)
392+
// audit-M-stock #1: net the source warehouse's reserved (allocated)
393+
// quantity so a transfer can't drain stock an order is holding there.
394+
if (!canDispatchTransferQty(level?.quantity, level?.reservedQty, qty)) {
395+
// Report the raw (unclamped) delta so an over-reservation (negative)
396+
// stays a visible diagnostic, not silently shown as 0.
397+
const rawAvailable = Number(level?.quantity ?? 0) - Number(level?.reservedQty ?? 0)
398+
throw new Error(`Insufficient stock for ${line.sku}: ${rawAvailable} available, ${qty} requested`)
394399
}
395400
}
396401

‎docs/workflows.md‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,3 +151,29 @@ refund payments.
151151

152152
Stock transfer status tracks inter-warehouse movement. `IN_TRANSIT` means stock
153153
has left the source warehouse and is not yet available at the destination.
154+
155+
**Stock / concurrency invariants (audit-M-stock).** A few cross-cutting guards
156+
worth knowing:
157+
158+
- **Transfers don't strand allocations.** Dispatch availability is the source
159+
warehouse's on-hand minus its `reservedQty` (`availableForTransfer`), and
160+
`StockLevel.reservedQty` is per-(product, warehouse) and kept in sync with
161+
order allocations — so a transfer can never drain stock an order is holding in
162+
that warehouse.
163+
- **Manual receipt + WMS booked-in don't double-count.** `reconcileBookedInQuantities`
164+
nets the locally-received quantity (read under a `FOR UPDATE` lock on the PO or
165+
the stock transfer) so a manual receipt already covering a line is not re-added
166+
when the Mintsoft booked-in webhook approves the same ASN.
167+
- **Opening stock can't duplicate.** `applyOpeningStock` takes a `FOR UPDATE`
168+
lock on the stock level before checking for an existing opening cost layer, so
169+
concurrent calls serialise and the second is rejected.
170+
- **Non-negative stock in the DB.** `quantity >= 0` and `reservedQty >= 0` are
171+
fully VALIDATEd CHECK constraints (every existing row checked). `reservedQty <=
172+
quantity` is a `NOT VALID` constraint — enforced on new writes but not yet
173+
validated against historical rows (that cleanup is deferred), so the transfer
174+
guard above (not just the constraint) is what protects allocations at dispatch.
175+
- **FIFO ordering at the destination** of a received transfer follows the
176+
dispatch-time cost-layer snapshot order. If transfers are received out of
177+
dispatch order the recreated layers can be marginally out of strict FIFO order
178+
at the destination — this is cosmetic for reporting and accepted (the cost
179+
basis per layer is preserved).
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// decimal-boundary-ok: server-action-boundary (numeric stock availability check)
2+
import { decimalToNumber, type DecimalLike } from '@/lib/decimal'
3+
4+
// ---------------------------------------------------------------------------
5+
// Transfer-dispatch availability (audit-M-stock #1)
6+
//
7+
// A transfer must not drain stock that an order has already reserved in the
8+
// SOURCE warehouse, or the order is stranded. StockLevel.reservedQty is
9+
// per-(product, warehouse) and kept in sync with order allocations, so the
10+
// available-to-transfer quantity is the warehouse's on-hand minus its reserved
11+
// — netting out exactly the allocations pointing at that warehouse. Centralised
12+
// here so the rule is explicit and regression-tested rather than an inline
13+
// subtraction.
14+
// ---------------------------------------------------------------------------
15+
16+
/**
17+
* On-hand minus reserved for a single (product, warehouse) stock level, clamped
18+
* to >= 0. Correct for dispatch gating; do NOT reuse for data-integrity checks —
19+
* the clamp hides an over-reservation (reservedQty > quantity), which raw
20+
* subtraction would surface as a negative.
21+
*/
22+
export function availableForTransfer(
23+
quantity: DecimalLike | null | undefined,
24+
reservedQty: DecimalLike | null | undefined,
25+
): number {
26+
const available = decimalToNumber(quantity ?? 0) - decimalToNumber(reservedQty ?? 0)
27+
return available > 0 ? available : 0
28+
}
29+
30+
/** Whether `requestedQty` can be dispatched without eating into reserved (allocated) stock. */
31+
export function canDispatchTransferQty(
32+
quantity: DecimalLike | null | undefined,
33+
reservedQty: DecimalLike | null | undefined,
34+
requestedQty: number,
35+
): boolean {
36+
return requestedQty <= availableForTransfer(quantity, reservedQty)
37+
}
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import assert from 'node:assert/strict'
2+
import test from 'node:test'
3+
4+
import { Prisma } from '@/app/generated/prisma/client'
5+
import { availableForTransfer, canDispatchTransferQty } from '@/lib/domain/inventory/transfer-availability'
6+
7+
test('availableForTransfer nets reserved (allocated) qty out of on-hand', () => {
8+
assert.equal(availableForTransfer(new Prisma.Decimal('100'), new Prisma.Decimal('30')), 70)
9+
})
10+
11+
test('availableForTransfer never goes negative', () => {
12+
assert.equal(availableForTransfer(new Prisma.Decimal('10'), new Prisma.Decimal('15')), 0)
13+
})
14+
15+
test('availableForTransfer treats missing level as zero', () => {
16+
assert.equal(availableForTransfer(null, null), 0)
17+
assert.equal(availableForTransfer(undefined, undefined), 0)
18+
})
19+
20+
test('canDispatchTransferQty: cannot dispatch into reserved stock', () => {
21+
// 100 on hand, 30 reserved for an order → only 70 transferable.
22+
assert.equal(canDispatchTransferQty(new Prisma.Decimal('100'), new Prisma.Decimal('30'), 70), true)
23+
assert.equal(canDispatchTransferQty(new Prisma.Decimal('100'), new Prisma.Decimal('30'), 71), false)
24+
})
25+
26+
test('canDispatchTransferQty: full unreserved stock is transferable', () => {
27+
assert.equal(canDispatchTransferQty(new Prisma.Decimal('50'), new Prisma.Decimal('0'), 50), true)
28+
})
29+
30+
test('canDispatchTransferQty: rejects when over-reserved (reservedQty > quantity)', () => {
31+
assert.equal(canDispatchTransferQty(new Prisma.Decimal('10'), new Prisma.Decimal('15'), 1), false)
32+
})

0 commit comments

Comments
 (0)