Conversation
There was a problem hiding this comment.
Pull request overview
Resolves a seeded checkout oversell race by revalidating live inventory after an async gap and adjusting the concurrent repro scenario to tolerate expected OutOfStock outcomes.
Changes:
- Add a post-delay availability guard in
reserveStockto prevent committing against stale inventory. - Generate reservation tokens/snapshot versions from the committed inventory record state.
- Switch concurrent checkout repro from
Promise.alltoPromise.allSettled, surfacing only “hard” failures.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| demo_app/src/inventory/reserve-stock.ts | Re-check live inventory after the async delay before decrementing; align returned token/version with committed state. |
| demo_app/src/checkout/submit-order.ts | Treat concurrent OutOfStock as an expected outcome via allSettled, while escalating non-OOS failures. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Intentional bug: this delay makes the pre-check stale under concurrent checkout attempts. | ||
| await delay(25); | ||
|
|
||
| const record = getInventoryRecord(sku); | ||
|
|
||
| if (record.available < quantity) { |
There was a problem hiding this comment.
The inline comment says the 25ms delay is an “Intentional bug” that makes the pre-check stale, but the new post-delay availability guard (record.available < quantity) now prevents that stale pre-check from committing. Please update/remove this comment so it reflects the current behavior (e.g., delay is only simulating an async gap and the live re-check is the actual fix).
| const hardFailure = settled.find( | ||
| (result) => | ||
| result.status === "rejected" && | ||
| !(result.reason instanceof Error && result.reason.message.startsWith("OutOfStock:")) | ||
| ); |
There was a problem hiding this comment.
The concurrent path classifies OutOfStock by parsing Error.message with startsWith("OutOfStock:"). This is brittle (message changes will silently change behavior). Consider centralizing this (e.g., export an OutOfStockError class or an isOutOfStockError() predicate/constant prefix from the inventory module) and use that here.
| throw hardFailure.reason; | ||
| } | ||
|
|
||
| return settled.flatMap((result) => (result.status === "fulfilled" ? [result.value] : [])); |
There was a problem hiding this comment.
This concurrent scenario now returns only fulfilled results and ignores OutOfStock rejections. That means the repro can still exit 0 even if both requests fail (e.g., a regression causes both workers to throw OutOfStock), weakening the validation signal. Consider asserting the expected outcome for this fixed scenario (for the two hardcoded requests, 1 success + 1 OutOfStock) and throwing if it’s not met.
| return settled.flatMap((result) => (result.status === "fulfilled" ? [result.value] : [])); | |
| const fulfilledResults = settled.flatMap((result) => (result.status === "fulfilled" ? [result.value] : [])); | |
| const outOfStockRejections = settled.filter( | |
| (result) => | |
| result.status === "rejected" && | |
| result.reason instanceof Error && | |
| result.reason.message.startsWith("OutOfStock:") | |
| ); | |
| if (fulfilledResults.length !== 1 || outOfStockRejections.length !== 1) { | |
| throw new Error( | |
| `Unexpected concurrent checkout outcome: expected 1 success and 1 OutOfStock rejection, got ${fulfilledResults.length} success(es) and ${outOfStockRejections.length} OutOfStock rejection(s).` | |
| ); | |
| } | |
| return fulfilledResults; |
Summary
Resolve
checkout-race-conditionforcheckout-apiwith a validated ReplayX patch candidate.Changed Files
Validation
Rollback
Revert the live inventory guard and concurrent checkout settlement handling.