Stock/concurrency invariants — explicit transfer guard + verification (audit-M-stock)#206
Conversation
…ck 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>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…rsarial 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>
Codex adversarial review — findings + fixes (commit 0ae2085)Codex was at its usage cap; the adversarial pass ran by hand with equal rigour. It independently verified all three "already-resolved" claims are substantively correct (reconcile nets Fixed:
Validation6/6 helper tests; type-check + lint clean. |
Wave 4 of the workflow-audit remediation (epic onetwo3d-ims-r3xh). Stock/concurrency cluster (bkhk).
Most findings were already closed by prior work; this PR makes the remaining guard explicit + regression-tested and documents the cluster.
reservedQty(kept in sync with order allocations), so a transfer can't strand an order. Extracted that rule into a pureavailableForTransfer/canDispatchTransferQtyhelper (now used by dispatch) so it's explicit and regression-tested.reconcileBookedInQuantitiesnetslocalReceivedQty(read under the POFOR UPDATElock); covered bymintsoft-phase6-booked-intests.applyOpeningStocktakes aFOR UPDATElock on the stock level before the existing-opening-layer check, serialising concurrent calls.stock_levelsCHECK >= 0 — VERIFIED present (migration20260512100000:quantity>=0,reservedQty>=0,reservedQty<=quantity).All five documented in
docs/workflows.mdunder "Stock / concurrency invariants".Tests
5 transfer-availability cases (nets reserved, never negative, missing level, can't-dispatch-into-reserved, full unreserved); type-check + lint clean.
Closes onetwo3d-ims-bkhk.