[FanouZeng-TT]

Category (Required)

• Core Protocol: Changes to the base communication layer, global context, or breaking refactors. (Requires Technical Council approval)
• Governance/Contributing: Updates to GOVERNANCE.md, CONTRIBUTING.md, or CODEOWNERS. (Requires Governance Council approval)
• Capability: New schemas (Discovery, Cart, etc.) or extensions. (Requires Maintainer approval)
• Documentation: Updates to README, or documentations regarding schema or capabilities. (Requires Maintainer approval)
• Infrastructure: CI/CD, Linters, or build scripts. (Requires DevOps Maintainer approval)
• Maintenance: Version bumps, lockfile updates, or minor bug fixes. (Requires DevOps Maintainer approval)
• SDK: Language-specific SDK updates and releases. (Requires DevOps Maintainer approval)
• Samples / Conformance: Maintaining samples and the conformance suite. (Requires Maintainer approval)
• UCP Schema: Changes to the ucp-schema tool (resolver, linter, validator). (Requires Maintainer approval)
• Community Health (.github): Updates to templates, workflows, or org-level configs. (Requires DevOps Maintainer approval)

Description

Complete Checkout is commented as an "Atomic Inventory Reservation and Completion", but the reservation, the order insert and the checkout update were three independent writes. A technical failure in the middle answered 500 without rolling anything back:

• an order write failure left stock decremented for a checkout that never completed;
• a checkout write failure additionally left an order row the checkout does not reference, so the client could retry and consume stock again.

This change runs the reservation, saveOrder and saveCheckout inside one better-sqlite3 transaction, so any failure rolls the three tables back together and the client can retry safely. An out-of-stock line short-circuits the transaction instead of the previous hand-rolled release of already-reserved items. Webhook delivery stays outside the transaction because external I/O must not hold the SQLite write lock.

Scope

Only the Node reference completion path and its data helper change. The HTTP contract, error envelopes, payment handling, webhook signing/delivery semantics, Python sample and schemas are untouched. The Python idempotency-race fix (#245) is a different failure mode (concurrent requests, one winner) and a different language/runtime; the Node referential-integrity and concurrency candidates are tracked separately and do not overlap these three files.

Tests

Fail-before (with the source changes reverted, fault injected by a SQLite trigger):

• an order write failure rolls back the inventory reservation — expected stock unchanged, got 99 instead of 100.
• a checkout write failure rolls back the order and the reservation — expected stock unchanged, got 98 instead of 99.

Pass-after:

• npm test --prefix rest/nodejs — 173 passed, 0 failed
• npm run build --prefix rest/nodejs
• npx prettier@4.0.0-alpha.8 --check on the three changed files
• git diff --check

[FanouZeng-TT]

Friendly bump — this one is green on all checks and has been open for a few days. Happy to adjust scope, add more context, or fill in any missing test coverage if that would make review easier.

[damaz91]

Thanks for putting this together, @FanouZeng-TT! Wrapping completion in a single better-sqlite3 transaction and testing partial-write faults with SQLite BEFORE INSERT / BEFORE UPDATE triggers is a very clean approach.

There is one rollback issue in the out-of-stock path for multi-item checkouts (plus a small suggestion around saveIdempotencyRecord placement) to address before we merge:

1. Multi-item out-of-stock early return commits prior reservations (rest/nodejs/src/api/checkout.ts): better-sqlite3's db.transaction(fn)() commits whenever fn returns normally and only rolls back when fn throws. In a multi-item checkout where line 1 succeeds in reserveStock(...) and line 2 fails, returning { outOfStockItem: line.item.id } exits normally—committing line 1's stock decrement while answering 409 OUT_OF_STOCK (whereas previously the reservedItems / releaseStock loop compensated earlier lines). Please throw OutOfStockError inside runInTransaction, forward UcpError instances in completeCheckout's catch block (if (e instanceof UcpError) return ucpErrorResponse(c, e);), and add a test in rest/nodejs/test/lifecycle.test.ts covering a 2-item checkout where the second item is out of stock at completion time.
2. Include saveIdempotencyRecord in the transaction (recommended): Since notifyWebhook does not mutate checkout, moving if (idempotencyKey) { saveIdempotencyRecord(...); } inside runInTransaction (right after saveCheckout, before await this.notifyWebhook(...)) commits idempotency_keys atomically with inventory, orders, and checkouts and avoids an async window where a retry arriving while webhook delivery is in flight sees completed status and gets 409 CHECKOUT_NOT_MODIFIABLE.

[damaz91]

db.transaction(fn)() in better-sqlite3 executes COMMIT whenever fn returns without throwing. If checkout.line_items has multiple items and item 1 reserves stock while item 2 returns false, returning { outOfStockItem: line.item.id } commits item 1's inventory deduction even though completion fails with 409.

Throwing OutOfStockError inside runInTransaction lets better-sqlite3 roll back all prior reservations in the transaction:

      runInTransaction(() => {
        for (const line of checkout.line_items) {
          const product = getProduct(line.item.id);
          if (product && !reserveStock(line.item.id, line.quantity)) {
            throw new OutOfStockError(
              `Item ${line.item.id} is out of stock`,
              409
            );
          }
        }

        checkout.status = CheckoutResponseStatusSchema.enum.completed;
        checkout.order = {
          id: orderId,
          permalink_url: order.permalink_url,
        };

        saveOrder(order.id, order);
        saveCheckout(id, checkout.status, checkout);
        if (idempotencyKey) {
          saveIdempotencyRecord(
            idempotencyKey,
            requestHash,
            200,
            JSON.stringify(checkout)
          );
        }
      });

Then in completeCheckout's catch (e) block below (lines 1217–1220), guard UcpError before the 500 fallback (matching createCheckout and updateCheckout):

    } catch (e) {
      if (e instanceof UcpError) {
        return ucpErrorResponse(c, e);
      }
      console.error("Error completing checkout", e);
      return c.json({ detail: "Internal server error" }, 500);
    }

[FanouZeng-TT]

Both points are addressed in cdf0bf7 — thanks, the first one was a real defect.

1. Multi-item rollback: the out-of-stock path now throws OutOfStockError inside runInTransaction instead of returning a sentinel, so a partially reserved checkout rolls back entirely. completeCheckout's catch block forwards UcpError instances rather than collapsing them into a 500, so the 409 still reaches the client. New test: a two-line checkout whose second line sells out at completion returns 409 with the first line's stock untouched — fail-before showed 99 !== 100, i.e. the old code committed that reservation.
2. Idempotency record: saveIdempotencyRecord now runs inside the same transaction, right after saveCheckout and before the webhook, so a retry during webhook delivery replays the stored response instead of seeing completed and getting 409. Covered by a replay test asserting stock is reserved once and one order is written.

Full suite: 175 tests pass, npm run build and Prettier clean.

[damaz91]

Thanks for the quick turnaround, @FanouZeng-TT! Throwing OutOfStockError inside runInTransaction, persisting the idempotency record in the same transaction before webhook delivery, and adding the multi-line rollback and replay tests look great. LGTM!

[karangoel16]

Thanks for fixing this properly. I went through the diff and the new tests, and the atomicity story holds up.

The throw instead of return on out-of-stock is the detail that makes this work. Returning there would have committed the earlier lines' reservations, so throwing to roll everything back is the right move.

Moving the idempotency record inside the transaction, before the webhook, is also the right call. A retry while delivery is in flight now replays instead of hitting completed and getting a 409. And keeping the webhook itself outside the transaction is correct, external I/O should not hold the write lock.

I verified the 409 path still works since OutOfStockError extends UcpError, and there are no leftover uses of releaseStock. Approving.