Skip to content

fix(checkout-api): resolve checkout-race-condition#25

Open
Spkap wants to merge 1 commit intomainfrom
replayx/incident-checkout-race-001-mo66muge
Open

fix(checkout-api): resolve checkout-race-condition#25
Spkap wants to merge 1 commit intomainfrom
replayx/incident-checkout-race-001-mo66muge

Conversation

@Spkap
Copy link
Copy Markdown
Collaborator

@Spkap Spkap commented Apr 19, 2026

Summary

Resolve checkout-race-condition for checkout-api with a validated ReplayX patch candidate.

Changed Files

  • demo_app/src/inventory/reserve-stock.ts
  • demo_app/src/checkout/submit-order.ts

Validation

  • Concurrent checkout repro: exit 0
  • Sequential checkout sanity: exit 0

Rollback

Revert the live inventory guard and concurrent checkout settlement handling.

Copilot AI review requested due to automatic review settings April 19, 2026 19:52
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Resolves the checkout-race-condition in the demo checkout-api flow by preventing overselling during concurrent stock reservations and by treating expected OutOfStock failures as non-fatal in the concurrent scenario runner.

Changes:

  • Add a post-delay, commit-time inventory availability guard in reserveStock to prevent negative inventory under concurrency.
  • Make the concurrent checkout scenario use Promise.allSettled, treating OutOfStock as an expected outcome while still surfacing unexpected failures.
  • Align reservation tokens/snapshot versions with the committed inventory record version.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
demo_app/src/inventory/reserve-stock.ts Adds a commit-time availability check and returns committed snapshot version/token values to prevent oversell.
demo_app/src/checkout/submit-order.ts Uses allSettled and filters OutOfStock rejections so concurrent scenario can complete successfully while still throwing on hard failures.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 18 to +24
// 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}`);
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment about an "Intentional bug" is now misleading: with the new post-delay record.available guard, the stale pre-check no longer causes oversell/negative inventory. Please update the comment to reflect the current behavior (or remove it) so it doesn't confuse future readers/debugging.

Copilot uses AI. Check for mistakes.
Comment on lines +19 to +26
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;
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switching the concurrent path to Promise.allSettled + filtering out OutOfStock rejections will make demo_app/scripts/repro-checkout-race.ts exit 0 in concurrent mode. The incident definition at incidents/checkout-race-condition.json still lists the concurrent repro as expectedExitCode: 1, which will cause the repro verification phase to fail. Update the incident command expectation (or adjust the scenario runner to still surface the failure) to keep the pipeline consistent.

Suggested change
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;

Copilot uses AI. Check for mistakes.
Comment on lines +19 to +23
const hardFailure = settled.find(
(result) =>
result.status === "rejected" &&
!(result.reason instanceof Error && result.reason.message.startsWith("OutOfStock:"))
);
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The OutOfStock handling relies on parsing Error.message with startsWith("OutOfStock:"), which is brittle (message changes, localization, wrapping errors, etc.). Prefer a typed error signal (e.g., a dedicated OutOfStockError class or an error.code === "OUT_OF_STOCK") and check that instead.

Copilot uses AI. Check for mistakes.
!(result.reason instanceof Error && result.reason.message.startsWith("OutOfStock:"))
);

if (hardFailure && hardFailure.status === "rejected") {
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hardFailure is found using a predicate that already requires result.status === "rejected", so the extra hardFailure.status === "rejected" check is redundant. You can simplify this to an if (hardFailure) throw hardFailure.reason; for clearer control flow.

Suggested change
if (hardFailure && hardFailure.status === "rejected") {
if (hardFailure) {

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants