Skip to content

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

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

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

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:10
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

This PR addresses the checkout-race-condition incident in the checkout-api demo app by preventing overselling during concurrent checkouts and by making the concurrent repro scenario handle expected out-of-stock outcomes without failing the whole run.

Changes:

  • Add a post-delay (commit-time) inventory availability guard in reserveStock and ensure reservation metadata reflects the committed snapshot version.
  • Update concurrent checkout scenario execution to use Promise.allSettled, treating OutOfStock as an expected rejection while still surfacing unexpected 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 Adds a commit-time availability check to prevent negative inventory and aligns returned reservation version/token with the committed record.
demo_app/src/checkout/submit-order.ts Switches concurrent execution to allSettled, filters expected OutOfStock failures, and rethrows unexpected errors.

💡 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 inline comment says the delay is an "Intentional bug" that makes the pre-check stale, but the new post-delay record.available guard means the function is no longer intentionally buggy in that way. Please update/remove the comment so it reflects the current behavior (e.g., explain why the delay exists in the repro without implying the implementation is still incorrect).

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.

OutOfStock handling is currently based on parsing Error.message.startsWith("OutOfStock:"), which is brittle and can easily break if the message format changes (or gets localized). Prefer a dedicated error type (e.g., class OutOfStockError extends Error { sku }) or a structured discriminator (like error.code === "OUT_OF_STOCK") and check that instead.

Copilot uses AI. Check for mistakes.
Comment on lines +17 to +29
const settled = await Promise.allSettled(requests.map((request) => processCheckoutWorker(request)));

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;
}

return settled.flatMap((result) => (result.status === "fulfilled" ? [result.value] : []));
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 from Promise.all to Promise.allSettled and swallowing OutOfStock rejections changes the concurrent repro from exit code 1 to exit code 0 (via demo_app/scripts/repro-checkout-race.ts). If any automation (e.g., the incident spec's "failing" command expectation) relies on the concurrent repro failing to confirm the incident, it will now report a mismatch—please make sure the corresponding repro expectations/metadata are updated to match the new behavior.

Suggested change
const settled = await Promise.allSettled(requests.map((request) => processCheckoutWorker(request)));
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;
}
return settled.flatMap((result) => (result.status === "fulfilled" ? [result.value] : []));
return Promise.all(requests.map((request) => processCheckoutWorker(request)));

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