[Domain] refundOrder's idempotency replay ignores amount — key reuse with a different amount reports false success

[vedanshujain]

What happens

refundOrder's idempotent-replay check resolves the existing refund by idempotencyKey alone and, when its status is "recorded", returns success without ever comparing the stored refund's amount to cmd.amount:

packages/domain/src/orders/refund-order.ts:192-201

const existing = await deps.orderStore.getRefundByIdempotencyKey(cmd.idempotencyKey);
if (existing !== null && existing.status === "recorded") {
	return {
		ok: true,
		recorded: false,
		duplicate: true,
		fullyRefunded: order.state === "refunded",
		refund: existing,
		order,
	};
}

So: refund a $10 order for $5 under key K (recorded). Later, call refundOrder again with the same key K but amount: $8. The lookup finds the existing $5 recorded row, matches on key alone, and returns { ok: true, duplicate: true, refund: <the original $5 row> }. Nothing is applied for the extra $3 — but the caller sees ok: true, not an error.

Why it matters

A caller reasonably reads { ok: true } as "the refund I asked for was applied." Here it means "some earlier refund under this key exists" — silently discarding the fact that this request's content diverged from that earlier one. packages/plugin/src/admin/orders-page.ts (around line 1535) renders this as an "Already refunded" notice, which is accurate for a genuine retry but misleading for a genuinely different amount submitted under a reused key.

CLAUDE.md's non-negotiables call out idempotency explicitly: "Every command carries an idempotencyKey; the store enforces once-only. Test the replay case." The replay case that's tested is same-key-same-content; same-key-different-content is not, and currently succeeds silently instead of erroring.

What this is NOT

This is not an overdraw / cash-loss bug. reserveRefund (packages/domain/src/orders/refund-order.ts:243-262) arbitrates against the ceiling min(Σ captured, frozen total) on every new reservation, independent of this replay check — a request that actually reaches reservation can never push the ledger past what was captured. The defect here is confined to the pre-reservation replay short-circuit: it's a silent swallow plus a false-success report, not a path that can move more money than was captured. Please don't triage this as a P0 money-leak.

Existing test coverage

packages/domain/src/testing/refund-order-contract.ts, test "an idempotent replay records once and never calls the gateway twice" (~line 208) replays the exact same command object (same key and same amount) and asserts duplicate: true with no second gateway call. That's the only replay coverage in the shared contract suite (exercised against fake/postgres/sqlite adapters) — there is no case anywhere in packages/domain/test/, packages/store-postgres/test/, or the contract suite that reuses a key with a different amount. That case is unverified, and per the code above it currently returns a false ok: true.

Suggested fix

When getRefundByIdempotencyKey finds a "recorded" row, compare the stored row's content (amount at minimum; also currency) against cmd. If it matches, return the existing duplicate-success shape as today. If it diverges, reject with an explicit error rather than reporting success — mirroring Stripe's own idempotency_error ("Keys for idempotent requests can only be used with the same parameters"). This likely needs a new RefundOrderFailure variant.

The fix needs a contract test (in refund-order-contract.ts, run against every adapter) asserting:

• same key + same amount ⇒ duplicate: true (already covered)
• same key + different amount ⇒ an explicit error, not ok: true

Current exposure

An in-flight admin-UI overhaul is moving every admin write, including this one, to derive its idempotency key deterministically from command content — including the observed watermark (e.g. admin-refund:${orderId}:${amountCents}:${refundedSoFarCents}). Once that lands, a different amount necessarily mints a different key, so the admin UI can no longer reach this path. After that ships, this becomes defence-in-depth for other callers (the HTTP API directly, any future client) rather than a live admin-UI bug — worth fixing, but not urgent for the current UI.