Conversation
There was a problem hiding this comment.
Pull request overview
Fixes the demo app’s checkout-api oversell race by validating inventory availability at commit time and by treating expected OutOfStock failures as non-fatal in the concurrent repro path.
Changes:
- Add a post-delay “commit-time” inventory availability guard in
reserveStockto prevent oversell under concurrency. - Return reservation metadata (token/version) from the committed inventory record rather than the pre-check snapshot.
- Update the concurrent scenario runner to use
Promise.allSettled, ignore expectedOutOfStockrejections, and return only successful checkout results.
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 | Adds commit-time stock validation and returns committed snapshot/version metadata. |
| demo_app/src/checkout/submit-order.ts | Makes concurrent repro tolerant of OutOfStock by using allSettled and filtering expected 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) { | ||
| throw new Error(`OutOfStock: ${sku}`); | ||
| } |
There was a problem hiding this comment.
The inline comment about the delay being an “Intentional bug” is now misleading/outdated. With the new post-delay record.available guard, the stale pre-check no longer causes oversell on its own, so the comment should be updated to describe the delay as a concurrency simulation while commit-time validation prevents stale snapshot writes.
| const hardFailure = settled.find( | ||
| (result) => | ||
| result.status === "rejected" && | ||
| !(result.reason instanceof Error && result.reason.message.startsWith("OutOfStock:")) | ||
| ); |
There was a problem hiding this comment.
This logic treats stock exhaustion as a “soft” failure by checking Error.message.startsWith("OutOfStock:"). Keying behavior off an error message prefix is brittle (message changes, localization, non-Error throws) and can misclassify failures. Prefer throwing a dedicated OutOfStockError (or an Error with a stable code property) from reserveStock and checking that here.
| const hardFailure = settled.find( | ||
| (result) => | ||
| result.status === "rejected" && | ||
| !(result.reason instanceof Error && result.reason.message.startsWith("OutOfStock:")) | ||
| ); | ||
|
|
||
| if (hardFailure && hardFailure.status === "rejected") { | ||
| throw hardFailure.reason; |
There was a problem hiding this comment.
With this change, the concurrent scenario no longer throws when one worker hits OutOfStock, so demo_app/scripts/repro-checkout-race.ts will exit 0 for the concurrent repro. The incident fixture incidents/checkout-race-condition.json still marks that command as the failing repro with expectedExitCode: 1, and the orchestrator uses that expectation to determine whether the repro is confirmed. If the intent is to fix the incident in-place, update the incident command expectations (or swap which command is considered failing vs healthy) to keep the pipeline/demo consistent.
| const hardFailure = settled.find( | |
| (result) => | |
| result.status === "rejected" && | |
| !(result.reason instanceof Error && result.reason.message.startsWith("OutOfStock:")) | |
| ); | |
| if (hardFailure && hardFailure.status === "rejected") { | |
| throw hardFailure.reason; | |
| const rejectedResult = settled.find((result) => result.status === "rejected"); | |
| if (rejectedResult && rejectedResult.status === "rejected") { | |
| throw rejectedResult.reason; |
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.