From 73baf60317f63dfc528adb568e718d5ab39f3727 Mon Sep 17 00:00:00 2001
From: James Brunton <james@stirlingpdf.com>
Date: Wed, 2 Sep 2026 14:20:51 +0100
Subject: [PATCH] Stop charging for failed runs

---
 .../payg/filter/PaygChargeInterceptor.java    | 13 ++++++---
 .../filter/PaygChargeInterceptorTest.java     | 29 ++++++++++++++++---
 2 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/app/saas/src/main/java/stirling/software/saas/payg/filter/PaygChargeInterceptor.java b/app/saas/src/main/java/stirling/software/saas/payg/filter/PaygChargeInterceptor.java
index 99300d6fcf..3375d91a88 100644
--- a/app/saas/src/main/java/stirling/software/saas/payg/filter/PaygChargeInterceptor.java
+++ b/app/saas/src/main/java/stirling/software/saas/payg/filter/PaygChargeInterceptor.java
@@ -65,7 +65,7 @@ import stirling.software.saas.util.AuthenticationUtils;
  * service.
  *
  * <p>{@code afterCompletion}: branches on HTTP status — 2xx hashes the response body for OUTPUT
- * lineage; 4xx records a step append for audit; 5xx triggers refund-and-close (OPENED) or
+ * lineage; any error (4xx or 5xx) is never charged and triggers refund-and-close (OPENED) or
  * step-quota return (JOINED). Closes all input temp files and the response wrapper at the end.
  *
  * <p>Fail-open everywhere: any unexpected {@link RuntimeException} is swallowed, logged at WARN,
@@ -392,9 +392,14 @@ public class PaygChargeInterceptor implements AsyncHandlerInterceptor {
             return;
         }
         if (status >= 400) {
-            // 4xx: customer paid for the attempt. No OUTPUT recording, no refund.
-            // Still a successful-from-billing-standpoint OPENED process — meter it below.
-            meterIfOpened(jobId, disposition);
+            // Errored work is never charged: refund the request that opened the process, or drop
+            // the added step for a joined follow-up.
+            if (disposition == ChargeOutcome.Disposition.OPENED) {
+                chargeService.markFirstStepFailed(jobId, "first-step-4xx:" + status);
+                refundsCounter.increment();
+            } else {
+                chargeService.decrementStepCount(jobId);
+            }
             return;
         }
 
diff --git a/app/saas/src/test/java/stirling/software/saas/payg/filter/PaygChargeInterceptorTest.java b/app/saas/src/test/java/stirling/software/saas/payg/filter/PaygChargeInterceptorTest.java
index 8dab9f741b..acf845fc44 100644
--- a/app/saas/src/test/java/stirling/software/saas/payg/filter/PaygChargeInterceptorTest.java
+++ b/app/saas/src/test/java/stirling/software/saas/payg/filter/PaygChargeInterceptorTest.java
@@ -274,7 +274,7 @@ class PaygChargeInterceptorTest {
     }
 
     @Test
-    void afterCompletion_4xx_appendsFailedStepNoRefundNoOutputs() throws Exception {
+    void afterCompletion_4xx_opened_refundsAndNeverMeters() throws Exception {
         authenticateWithApiKey(makeUser(7L, 42L));
         UUID jobId = UUID.randomUUID();
         when(chargeService.openProcess(any(), anyList()))
@@ -288,13 +288,34 @@ class PaygChargeInterceptorTest {
         interceptor.preHandle(req, res, handlerMethodForFakeController());
         interceptor.afterCompletion(req, res, handlerMethodForFakeController(), null);
 
-        verify(chargeService, never()).markFirstStepFailed(any(), any());
+        // Errored work is never charged: the opener is refunded, and it never meters.
+        verify(chargeService).markFirstStepFailed(eq(jobId), eq("first-step-4xx:422"));
         verify(chargeService, never()).decrementStepCount(any());
         verify(jobService, never()).recordOutput(any(), any());
         verify(jobService)
                 .appendStep(eq(jobId), any(), eq(JobStepStatus.FAILED), any(), any(), eq("422"));
-        // 4xx is a full charge (customer paid for the attempt), so it still meters.
-        verify(chargeService).meterJobUsage(jobId);
+        assertThat(meterRegistry.counter("payg.filter.refunds").count()).isEqualTo(1.0);
+        verify(chargeService, never()).meterJobUsage(any());
+    }
+
+    @Test
+    void afterCompletion_4xx_joined_callsDecrementStepCount() throws Exception {
+        authenticateWithApiKey(makeUser(7L, 42L));
+        UUID jobId = UUID.randomUUID();
+        when(chargeService.openProcess(any(), anyList()))
+                .thenReturn(new ChargeOutcome(jobId, 0, ChargeOutcome.Disposition.JOINED));
+
+        MockMultipartHttpServletRequest req = newMultipart();
+        req.addFile(new MockMultipartFile("file", "x.pdf", "application/pdf", "abc".getBytes()));
+        MockHttpServletResponse res = new MockHttpServletResponse();
+        res.setStatus(422);
+
+        interceptor.preHandle(req, res, handlerMethodForFakeController());
+        interceptor.afterCompletion(req, res, handlerMethodForFakeController(), null);
+
+        verify(chargeService).decrementStepCount(jobId);
+        verify(chargeService, never()).markFirstStepFailed(any(), any());
+        verify(chargeService, never()).meterJobUsage(any());
     }
 
     @Test