diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index da090b2..7de110e 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -252,6 +252,11 @@ The native Android app is a Capacitor 8 wrapper of the `ui` SPA, scaffolded unde IP bans being prod-only. It bought nothing anyway: the order is resolved from the notification metadata **before** any provider call, so a forged id costs one indexed read, and guessing a live order id means guessing a uuid. + - **The console refund and the `refund.succeeded` notification race, benignly.** YooKassa fires the + event the moment `POST /v3/refunds` returns, so the notification often records the reversal before + the console's own write lands. Both name the same refund id, so the ledger's idempotency index + catches the second and nothing is revoked twice — but the console must not then report "already + refunded", which reads as "you clicked twice" for a refund that just succeeded. - **Never key a re-check interval off the order TTL.** The order lifetime answers "how long may a customer take to pay"; the re-check answers "how soon do we notice a lost callback". Tying them together made a failed notification cost the customer the full 30-minute TTL before the chips diff --git a/backend/internal/inttest/payments_yookassa_test.go b/backend/internal/inttest/payments_yookassa_test.go index 9eae809..0c098a0 100644 --- a/backend/internal/inttest/payments_yookassa_test.go +++ b/backend/internal/inttest/payments_yookassa_test.go @@ -821,3 +821,46 @@ func TestYooKassaPendingRefundRecordsNothing(t *testing.T) { t.Errorf("balance = %d, want 0", got) } } + +// TestYooKassaConsoleRefundAfterItsOwnNotification covers the race the two recording paths really +// run: the console asks the provider to refund, the provider fires refund.succeeded at once, and the +// notification records the reversal before the console gets to. Both name the same refund id, so the +// idempotency index catches the second write and nothing is revoked twice — but the operator must be +// told the refund succeeded, not that they clicked twice. +func TestYooKassaConsoleRefundAfterItsOwnNotification(t *testing.T) { + f, shop := newFakeYooKassa(t) + srv, pay := yookassaServer(t, shop) + acc := provisionAccount(t) + orderID, paymentID := fundedYooKassaOrder(t, f, srv, pay, acc) + + // The provider's notification lands first, for the refund the console is about to create. + refundID := "refund-" + paymentID + f.setRefund(refundID, yookassa.Refund{ + Status: yookassa.StatusSucceeded, PaymentID: paymentID, + Amount: yookassa.Amount{Value: "149.00", Currency: "RUB"}, + }) + if code := postYooKassaRefundNotify(t, srv, refundID, paymentID); code != http.StatusOK { + t.Fatalf("refund notify = %d, want 200", code) + } + if got := readBalance(t, acc, "direct"); got != 0 { + t.Fatalf("balance after the notification = %d, want 0", got) + } + + base := "http://admin.test/_gm/users/" + acc.String() + code, body := consoleDo(srv.Handler(), http.MethodPost, base+"/refund", "order_id="+orderID.String(), "http://admin.test") + if code != http.StatusOK { + t.Fatalf("console refund = %d, want 200", code) + } + if strings.Contains(body, "Already refunded") { + t.Errorf("the console reported a repeat click for a refund it had just made successfully: %q", body) + } + if !strings.Contains(body, "Refunded") { + t.Errorf("console message = %q, want it to report the refund succeeded", body) + } + if ledgerRows(t, acc, "refund") != 1 { + t.Errorf("refund ledger rows = %d, want 1", ledgerRows(t, acc, "refund")) + } + if abuse, loss := readRisk(t, acc); abuse || loss != 0 { + t.Errorf("risk = (abuse %v, loss %d), want none", abuse, loss) + } +} diff --git a/backend/internal/server/handlers_admin_refund.go b/backend/internal/server/handlers_admin_refund.go index fb43156..c674028 100644 --- a/backend/internal/server/handlers_admin_refund.go +++ b/backend/internal/server/handlers_admin_refund.go @@ -49,6 +49,15 @@ func (s *Server) consoleRefund(c *gin.Context) { return } if out.AlreadyRefunded { + if ref.Provider == providerYooKassa { + // The provider refunded the money on this very click, so this is a success, not a repeat. + // The reversal was already in the ledger because the provider's own refund notification + // reached us first — it names the same refund id, which is exactly why the idempotency + // index caught it and nothing was revoked twice. Saying "already refunded" here would read + // as "you clicked twice". + s.renderConsoleMessage(c, "Refunded", "the money is back with the customer; the reversal was already recorded, so nothing changed just now", back) + return + } s.renderConsoleMessage(c, "Already refunded", "this order was already refunded", back) return } diff --git a/backend/internal/server/handlers_yookassa.go b/backend/internal/server/handlers_yookassa.go index 07d27b4..969f5dd 100644 --- a/backend/internal/server/handlers_yookassa.go +++ b/backend/internal/server/handlers_yookassa.go @@ -282,7 +282,10 @@ func (s *Server) handleYooKassaRefundNotification(c *gin.Context, note yookassa. return } if !out.AlreadyRefunded { - s.log.Info("yookassa refund notify: reversed a refund issued outside the console", + // This is the first record of the refund. It is usually one an operator made in the merchant + // cabinet, but it can equally be a console refund whose notification outran the console's own + // write — both name the same refund id, so either way it is recorded exactly once. + s.log.Info("yookassa refund notify: reversed a refund", zap.String("order", ref.OrderID.String()), zap.String("refund", refund.ID), zap.Int("revoked", out.Revoked), zap.Int("loss", out.Loss)) } diff --git a/docs/PAYMENTS.md b/docs/PAYMENTS.md index 25e5067..2963093 100644 --- a/docs/PAYMENTS.md +++ b/docs/PAYMENTS.md @@ -333,6 +333,11 @@ nothing twice. The engine is **full-refund-only** by design (it revokes exactly and rejects any other amount), so a **partial** refund is recorded as nothing at all and logged loudly for an operator: there is no non-arbitrary way to decide how many chips a part-refund costs. +The two recording paths race, benignly: YooKassa fires `refund.succeeded` the moment the refund is +created, so the notification often writes the reversal before the console's own write lands. Both +name the same refund id, so the second is caught by the idempotency index and nothing is revoked +twice; the console reports the refund as successful rather than as a repeated click. + VK refunds are still handled by support and Telegram Stars refunds issued with `refundStarPayment`, both recorded by hand afterwards. All of them converge on one engine — the `Refund` method (`internal/payments`): it matches the paid order, appends a **refund** ledger diff --git a/docs/PAYMENTS_ru.md b/docs/PAYMENTS_ru.md index 628e222..e53f512 100644 --- a/docs/PAYMENTS_ru.md +++ b/docs/PAYMENTS_ru.md @@ -326,6 +326,11 @@ at-least-once + идемпотентный приём (дедуп по `telegram записывается вовсе и громко логируется для оператора: непроизвольного способа решить, скольких Фишек стоит часть возврата, нет. +Два пути записи при этом безобидно гоняются: ЮKassa шлёт `refund.succeeded` сразу после создания +возврата, поэтому уведомление часто записывает реверс раньше, чем это успевает сделать сама консоль. +Оба называют один и тот же refund-id, поэтому вторая запись отсекается индексом идемпотентности и +ничего не отзывается дважды; консоль при этом сообщает об успешном возврате, а не о повторном нажатии. + Возвраты VK по-прежнему через поддержку, TG Stars — вызовом `refundStarPayment`, оба фиксируются руками после факта. Все сходятся на одном движке — метод `Refund` (`internal/payments`): матчит оплаченный заказ, пишет **refund**-строку журнала (идемпотентно по