fix(payg): run the entitlement guard before the charge interceptor (#6622)

## Problem

When the `EntitlementGuard` refuses a request with **402** (team is over
its free allowance / spending cap, or has no subscription to bill), the
handler never runs — so it must not charge. But it did: the guard (order
**1100**) ran *after* the charge interceptor (**1000**), so
`openProcess` had already written the charge before the 402, and
`afterCompletion` then billed it as "customer paid for the attempt" — a
ledger debit, and a **Stripe meter for a subscribed-over-cap team**.

## Fix

**Run the guard first** (order **900**, before the charge interceptor at
1000). Spring runs interceptors in ascending order on the way in and
**skips a later interceptor's `preHandle` (and `afterCompletion`)
entirely once an earlier one returns `false`** — so a refused request
short-circuits with its 402 *before* the charge interceptor runs at all.
A blocked request never opens a process, materialises inputs, or writes
a charge.

This replaces the earlier attribute-flag + `afterCompletion`-refund
approach with a simpler reorder (per review): the no-charge-on-block
guarantee is now **structural**, and it also avoids the wasted
open-then-refund churn (no temp-file write, no debit/refund pair) for
refused requests.

### Why the reorder is safe
- `EntitlementGuard` reads no `PaygChargeInterceptor` state and has **no
`afterCompletion`** (only `preHandle`), so reverse-order teardown is a
non-issue.
- The legacy `UnifiedCreditInterceptor` (default order **0**, and only
registered under the `legacy-credits` profile) still runs first, so any
legacy rejection wins.
- For *admitted* requests both interceptors still run (guard then
charge) — behaviour is unchanged; only refused requests now
short-circuit before the charge.

## Tests

`PaygWebMvcConfigTest` locks the `ENTITLEMENT_GUARD_ORDER <
INTERCEPTOR_ORDER` invariant (if it's ever reversed, refused requests
would bill again — this fails first). Existing `EntitlementGuardTest`
already proves the guard returns 402 on a degraded/billable request.
`:saas:test` + spotless green.

## Related (separate, in progress)

The **fail-cleanly + fire-the-modal** half (suppress the error toast,
trigger the subscribe/raise-cap modal via the existing
`subscribed`/`category` signal — catching the 402 centrally so direct
API usage is handled, and propagating the entitlement reason through the
async policy-run status) lands **with Ethan's modal** so we don't remove
the toast before there's a popup to replace it.
This commit is contained in:
ConnorYoh
2026-06-11 20:42:40 +01:00
committed by GitHub
parent b1a960a240
commit ee9fdeed6b
2 changed files with 37 additions and 11 deletions
@@ -42,22 +42,26 @@ public class PaygWebMvcConfig implements WebMvcConfigurer {
}
/**
* Interceptor ordering: the legacy {@code UnifiedCreditInterceptor} (registered with default
* order = 0 in {@code CreditInterceptorConfig}) must run BEFORE this one so credit rejections
* short-circuit before we hash inputs. Explicit positive order guarantees this regardless of
* {@code WebMvcConfigurer} bean discovery order.
* The {@code PaygChargeInterceptor} runs after the {@link #ENTITLEMENT_GUARD_ORDER guard} (and
* after the legacy {@code UnifiedCreditInterceptor}, default order 0), so {@code openProcess}
* only fires for requests the guard has admitted. See {@link #ENTITLEMENT_GUARD_ORDER} for the
* full ordering rationale.
*/
public static final int INTERCEPTOR_ORDER = 1000;
/**
* The entitlement guard runs AFTER the charge interceptor so cap-rejected requests still leave
* the charge interceptor's preHandle state in the consistent open-or-bypassed shape (and so the
* guard's 402 reaches the client without the charge interceptor needing to know about it).
* Spring runs interceptors in registration order on the way in, reverse order on the way out;
* the guard's preHandle short-circuit ({@code return false}) prevents the handler from running
* but Spring still invokes the charge interceptor's afterCompletion to clean up temp files.
* The {@code EntitlementGuard} runs BEFORE the charge interceptor. Spring runs interceptors in
* ascending order on the way in and skips a later interceptor's {@code preHandle} (and its
* {@code afterCompletion}) entirely once an earlier one returns {@code false} — so a request the
* guard refuses (over its free allowance / spending cap, or with no subscription to bill)
* short-circuits with its 402 before the charge interceptor ever runs. A blocked request
* therefore never opens a process, materialises inputs, or writes a charge: a refused operation
* must not bill, and running the guard first guarantees that structurally rather than by
* compensating after the fact. Stays above the legacy {@code UnifiedCreditInterceptor} (default
* order 0, only registered under the {@code legacy-credits} profile) so a legacy rejection still
* wins.
*/
public static final int ENTITLEMENT_GUARD_ORDER = 1100;
public static final int ENTITLEMENT_GUARD_ORDER = 900;
@Override
public void addInterceptors(InterceptorRegistry registry) {
@@ -0,0 +1,22 @@
package stirling.software.saas.payg.filter;
import static org.assertj.core.api.Assertions.assertThat;
import org.junit.jupiter.api.Test;
/**
* Locks the interceptor ordering that makes "a blocked request is never charged" hold structurally:
* the {@link stirling.software.saas.payg.entitlement.EntitlementGuard} must run BEFORE the {@link
* PaygChargeInterceptor}. Spring skips a later interceptor's {@code preHandle} once an earlier one
* returns {@code false}, so guard-first means a refused (402) request never reaches {@code
* openProcess}. If these constants are ever reordered the wrong way, refused requests would start
* billing again — this test fails first.
*/
class PaygWebMvcConfigTest {
@Test
void entitlementGuardRunsBeforeChargeInterceptor() {
assertThat(PaygWebMvcConfig.ENTITLEMENT_GUARD_ORDER)
.isLessThan(PaygWebMvcConfig.INTERCEPTOR_ORDER);
}
}