From 835afbe5e061883653bf1b23ad3a369eaa1c1129 Mon Sep 17 00:00:00 2001
From: Chengdong Zhang <chengdzhang@ebay.com>
Date: Fri, 18 Sep 2026 17:41:06 +0800
Subject: [PATCH] fix(api): decrement SKU stock only when remaining quantity is
 sufficient

Prevent oversell by conditioning stock updates and locking SKUs in primary-key order, and record the same concurrent-counter rule in the API spec and agent guide.

Co-authored-by: Cursor <cursoragent@cursor.com>
---
 AGENTS.md                          |  2 +-
 apps/api/src/modules/order/repo.rs | 16 +++++++++++-----
 apps/api/tests/order_service.rs    | 19 +++++++++++++++++++
 docs/tech-specs/rust-api.md        |  1 +
 4 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/AGENTS.md b/AGENTS.md
index 2af421f..a5f7006 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -15,7 +15,7 @@
 - **金额**：`i64`/`number` 最小单位 + 币种码；换算走后端 `/api/currencies/convert` 或共享 `formatMoney(minor, code, exponent, locale)`。**禁止浮点金额运算**，禁止在前端硬编码 exponent=2（JPY 是 0）——用币种表里的 exponent。
 - **i18n 内容**：凡是面向用户的内容字段都是 `{"en","zh"}` JSONB；UI 文案一律 `$t()`，缺键加到本应用的 `locales-extra.ts`，**不要**从应用代理改 `packages/shared` 语言包以外的文件（共享包改动属契约变更，需谨慎并跑齐三个构建）。
 - **TS 纪律**：禁止 `any`/`as any`/`@ts-ignore`；外部数据用类型守卫或 schema 校验；客户端方法类型在 `@vmall/shared` 命名导出。
-- **Rust 纪律**：错误统一 `ApiError`（`{"error":{"code","message"}}`）；SQL 用 `sqlx::query*` + 显式 bind；状态迁移必须校验前置状态（参考 orders/shipments 的 `UPDATE ... WHERE status = ...` 模式）。
+- **Rust 纪律**：错误统一 `ApiError`（`{"error":{"code","message"}}`）；SQL 用 `sqlx::query*` + 显式 bind；状态迁移必须校验前置状态（参考 orders/shipments 的 `UPDATE ... WHERE status = ...` 模式）。**并发计数扣减**（目前只有 `skus.stock`）：多行 `FOR UPDATE` 必须先 `ORDER BY` 主键；扣减必须 `SET col = col - $qty WHERE id = $1 AND col >= $qty`，`rows_affected = 0` → `Conflict`。禁止无条件 `SET col = col - n`。回补用 `col = col + n`，不要用读出的绝对值写回。商家覆盖赋值（`SET stock = $n`）不是扣减，不走此模式。
 - **RBAC**：受保护路由声明角色（`auth.require(&[...])`）；店铺资源必须过 `auth.own_shop()` 作用域，跨店返回 404。
 
 ## 开发流程（OpenSpec）
diff --git a/apps/api/src/modules/order/repo.rs b/apps/api/src/modules/order/repo.rs
index abb2b81..034f6fe 100644
--- a/apps/api/src/modules/order/repo.rs
+++ b/apps/api/src/modules/order/repo.rs
@@ -184,11 +184,16 @@ pub async fn insert_item(
 }
 
 pub async fn decrement_stock(tx: &mut PgConnection, sku_id: Uuid, qty: i32) -> ApiResult<()> {
-    sqlx::query("UPDATE skus SET stock = stock - $2 WHERE id = $1")
-        .bind(sku_id)
-        .bind(qty)
-        .execute(&mut *tx)
-        .await?;
+    let result = sqlx::query(
+        "UPDATE skus SET stock = stock - $2 WHERE id = $1 AND stock >= $2",
+    )
+    .bind(sku_id)
+    .bind(qty)
+    .execute(&mut *tx)
+    .await?;
+    if result.rows_affected() == 0 {
+        return Err(ApiError::Conflict("insufficient stock".into()));
+    }
     Ok(())
 }
 
@@ -329,6 +334,7 @@ pub async fn lock_purchasable_skus(
          JOIN shops sh ON sh.id = p.shop_id
          WHERE s.id = ANY($1) AND s.active = TRUE
            AND p.status = 'published' AND sh.status = 'active'
+         ORDER BY s.id
          FOR UPDATE OF s",
     )
     .bind(sku_ids)
diff --git a/apps/api/tests/order_service.rs b/apps/api/tests/order_service.rs
index dda85c4..9040740 100644
--- a/apps/api/tests/order_service.rs
+++ b/apps/api/tests/order_service.rs
@@ -79,6 +79,25 @@ async fn checkout_rejects_empty_cart() {
     assert!(matches!(err, ApiError::BadRequest(m) if m.contains("empty")));
 }
 
+#[tokio::test]
+#[serial]
+async fn decrement_stock_rejects_when_guard_fails() {
+    let state = common::spawn_state().await;
+    let sku_id = sellable_sku(&state, "guard", 100, 1).await;
+    let mut tx = state.db.begin().await.unwrap();
+    let err = order::repo::decrement_stock(&mut tx, sku_id, 2)
+        .await
+        .unwrap_err();
+    tx.rollback().await.unwrap();
+    assert!(matches!(err, ApiError::Conflict(m) if m.contains("insufficient stock")));
+    let left: i32 = sqlx::query_scalar("SELECT stock FROM skus WHERE id = $1")
+        .bind(sku_id)
+        .fetch_one(&state.db)
+        .await
+        .unwrap();
+    assert_eq!(left, 1);
+}
+
 #[tokio::test]
 #[serial]
 async fn checkout_rejects_insufficient_stock() {
diff --git a/docs/tech-specs/rust-api.md b/docs/tech-specs/rust-api.md
index bc8f6f7..4e00d8a 100644
--- a/docs/tech-specs/rust-api.md
+++ b/docs/tech-specs/rust-api.md
@@ -28,6 +28,7 @@ Implementation considerations
 - **Money:** `i64` minor units + ISO code; `money::convert_minor` is a pure function, not a repository.
 - **SQL:** `sqlx::query*` / `query_as` with explicit binds. Prefer column lists over `SELECT *` on `users` (use `USER_COLUMNS` + `User` row vs `UserPublic` JSON).
 - **State machines:** `UPDATE … WHERE status = …`; zero rows → `ApiError::Conflict`, never a silent no-op success.
+- **Concurrent counters:** any column that concurrent requests decrement (today: `skus.stock`) follows the same “predicate or it did not happen” rule as status. `SELECT … FOR UPDATE` of multiple rows MUST `ORDER BY` the primary key so lock order is global. Decrement MUST be `UPDATE … SET col = col - $qty WHERE id = $1 AND col >= $qty`; `rows_affected = 0` → `Conflict` (do not rely on `CHECK (col >= 0)` as the only signal). Restore with `col = col + n`, never `SET col = $absolute` from a stale read. Merchant overwrite (`SET stock = $n` on SKU upsert) is assignment, not decrement. Remaining shipment qty is not a stored counter: `create` serializes on `orders … FOR UPDATE` and checks remainder in that transaction.
 - **RBAC:** `AuthUser::require`, `require_customer`, `require_admin`, `require_shop` / `own_shop`. Cross-shop resource access is 404, not 403.
 
 High-level request flow