fix(payments): a console refund that its own notification beat is not a repeat
CI / changes (pull_request) Successful in 2s
CI / unit (pull_request) Successful in 11s
CI / integration (pull_request) Successful in 22s
CI / ui (pull_request) Successful in 1m16s
CI / conformance (pull_request) Successful in 10s
CI / gate (pull_request) Successful in 1s
CI / deploy (pull_request) Successful in 2m28s

Refunding from /_gm reported "Already refunded" for a refund that had just
succeeded, which reads as "you clicked twice".

The two recording paths race. YooKassa fires refund.succeeded the moment
POST /v3/refunds returns, so the notification handler often writes the reversal
before the console's own write lands. Both name the same refund id, so the
ledger's idempotency index rejects the second — which is the mechanism working
exactly as intended: on the contour the money moved once, one refund row was
written, the balance is right and no abuse flag was raised. Only the message
was wrong about why.

The console now reports success on that path, and the notification's log line
no longer claims the refund was "issued outside the console" when it may well
have come from it.

Covered by an integration test that lands the notification first and then
refunds from the console, asserting the operator is told it succeeded and that
exactly one refund row exists.
This commit is contained in:
Ilia Denisov
2026-07-28 12:15:59 +02:00
parent 4cac09c9f3
commit e3961fe4ca
6 changed files with 71 additions and 1 deletions
+5
View File
@@ -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
@@ -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)
}
}
@@ -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
}
+4 -1
View File
@@ -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))
}
+5
View File
@@ -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
+5
View File
@@ -326,6 +326,11 @@ at-least-once + идемпотентный приём (дедуп по `telegram
записывается вовсе и громко логируется для оператора: непроизвольного способа решить, скольких Фишек
стоит часть возврата, нет.
Два пути записи при этом безобидно гоняются: ЮKassa шлёт `refund.succeeded` сразу после создания
возврата, поэтому уведомление часто записывает реверс раньше, чем это успевает сделать сама консоль.
Оба называют один и тот же refund-id, поэтому вторая запись отсекается индексом идемпотентности и
ничего не отзывается дважды; консоль при этом сообщает об успешном возврате, а не о повторном нажатии.
Возвраты VK по-прежнему через поддержку, TG Stars — вызовом `refundStarPayment`, оба фиксируются
руками после факта. Все сходятся на одном движке — метод `Refund`
(`internal/payments`): матчит оплаченный заказ, пишет **refund**-строку журнала (идемпотентно по