fix(cloud): account deletion asks the billing sidecar by user, so every Stripe customer the user owns is cancelled (BUG-3560) (#2036)

A user can own a Stripe customer stamped with their id that is not the
linked one (TASK-3549 keeps one link and unions the rest), and before
this only the linked customer was cancelled; with nothing linked the
sidecar was never asked. The other customer kept charging after the
account was gone.

CancelCustomer now carries the user id, and in cloud mode deletion asks
the sidecar even with no linked customer. A sidecar that predates
user_id refuses an empty customer_id with 400; for a user with no
linked customer that is exactly what not asking did before, so that one
answer lets the deletion proceed, and either deploy order is safe. Any
other failure, and any failure with a linked customer, still aborts.

Claude-Session: https://claude.ai/code/session_01FAVXA5mJ5BNhKmv95jxQkQ

internal/billing/cloud_client.go

@@ -186,31 +186,37 @@ func (c *CloudClient) GetBillingMetrics() (*BillingMetricsResponse, error) {
 // success), so retries after a partial failure complete cleanly.
 //
 // Request: POST {baseURL}/billing/cancel-customer
-// Body:    {"customer_id": "cus_xxx", "cloud_secret": "..."}
+// Body:    {"customer_id": "cus_xxx", "user_id": "...", "cloud_secret": "..."}
+//
+// userID (BUG-3560) lets the sidecar also cancel every other customer the
+// user owns (stamped pad_user_id), and makes an empty customerID valid: a
+// user can own a stamped customer with nothing linked. A sidecar that
+// predates it ignores user_id and refuses an empty customer_id with 400.
 // 200 OK:  {"ok": true, "subscriptions_cancelled": N}
 //
 // Returns nil on 200. On any non-200, returns a *SidecarError so the caller
 // can branch on Status. On transport failure (DNS, connect, timeout) returns
 // a bare error — treated by callers as "retryable, abort the delete".
-func (c *CloudClient) CancelCustomer(customerID string) error {
+func (c *CloudClient) CancelCustomer(customerID, userID string) error {
 	if c == nil {
 		return errors.New("billing: CancelCustomer called on nil CloudClient")
 	}
-	if customerID == "" {
-		// Defensive — handleDeleteAccount is expected to skip the call when
-		// StripeCustomerID is empty, but if somebody wires it differently
-		// we refuse to POST an empty cus_ that would burn a sidecar call
-		// and log-spam the 400 it would return.
-		return errors.New("billing: customerID is empty")
+	if customerID == "" && userID == "" {
+		// Nothing to name: the sidecar would refuse it with 400.
+		return errors.New("billing: customerID and userID are both empty")
 	}
 	if c.baseURL == "" || c.cloudSecret == "" {
 		return errors.New("billing: CloudClient is not configured (missing baseURL or cloudSecret)")
 	}
 
-	payload, err := json.Marshal(map[string]string{
+	body := map[string]string{
 		"customer_id":  customerID,
 		"cloud_secret": c.cloudSecret,
-	})
+	}
+	if userID != "" {
+		body["user_id"] = userID
+	}
+	payload, err := json.Marshal(body)
 	if err != nil {
 		return fmt.Errorf("billing: marshal cancel-customer request: %w", err)
 	}

internal/billing/cloud_client_test.go

@@ -186,7 +186,7 @@ func TestCancelCustomer_HappyPath_SendsCorrectRequest(t *testing.T) {
 		_, _ = w.Write([]byte(`{"ok":true,"subscriptions_cancelled":2}`))
 	})
 
-	if err := client.CancelCustomer("cus_abc"); err != nil {
+	if err := client.CancelCustomer("cus_abc", ""); err != nil {
 		t.Fatalf("expected nil error on 200, got %v", err)
 	}
 
@@ -235,7 +235,7 @@ func TestCancelCustomer_NonOK_ReturnsSidecarError(t *testing.T) {
 				_, _ = w.Write([]byte(tc.body))
 			})
 
-			err := client.CancelCustomer("cus_abc")
+			err := client.CancelCustomer("cus_abc", "")
 			if err == nil {
 				t.Fatalf("expected error on %d, got nil", tc.status)
 			}
@@ -260,7 +260,7 @@ func TestCancelCustomer_TransportFailure_NotSidecarError(t *testing.T) {
 	client := NewCloudClient("http://127.0.0.1:1", "test-secret")
 	client.http.Timeout = 500 * time.Millisecond
 
-	err := client.CancelCustomer("cus_abc")
+	err := client.CancelCustomer("cus_abc", "")
 	if err == nil {
 		t.Fatal("expected transport error, got nil")
 	}
@@ -280,7 +280,7 @@ func TestCancelCustomer_EmptyCustomerID_ReturnsError(t *testing.T) {
 		w.WriteHeader(http.StatusOK)
 	})
 
-	err := client.CancelCustomer("")
+	err := client.CancelCustomer("", "")
 	if err == nil {
 		t.Fatal("expected error for empty customer_id, got nil")
 	}
@@ -291,14 +291,14 @@ func TestCancelCustomer_EmptyCustomerID_ReturnsError(t *testing.T) {
 
 func TestCancelCustomer_NilClient_ReturnsError(t *testing.T) {
 	var c *CloudClient
-	if err := c.CancelCustomer("cus_abc"); err == nil {
+	if err := c.CancelCustomer("cus_abc", ""); err == nil {
 		t.Fatal("expected error from nil receiver, got nil")
 	}
 }
 
 func TestCancelCustomer_UnconfiguredClient_ReturnsError(t *testing.T) {
 	c := NewCloudClient("", "")
-	err := c.CancelCustomer("cus_abc")
+	err := c.CancelCustomer("cus_abc", "")
 	if err == nil {
 		t.Fatal("expected error for unconfigured client, got nil")
 	}
@@ -398,7 +398,7 @@ func TestCancelCustomer_LargeResponseBody_DoesNotReadPastCap(t *testing.T) {
 		_, _ = w.Write([]byte(big))
 	})
 
-	err := client.CancelCustomer("cus_abc")
+	err := client.CancelCustomer("cus_abc", "")
 	var se *SidecarError
 	if !errors.As(err, &se) {
 		t.Fatalf("expected SidecarError, got %T", err)
@@ -407,3 +407,21 @@ func TestCancelCustomer_LargeResponseBody_DoesNotReadPastCap(t *testing.T) {
 		t.Errorf("body was %d bytes, expected <= %d (cap)", len(se.Body), maxResponseBody)
 	}
 }
+
+// BUG-3560: the user id travels, and with it an empty customer id is a valid
+// call (a stamped customer can exist with nothing linked).
+func TestCancelCustomer_SendsUserIDAndAllowsEmptyCustomer(t *testing.T) {
+	var gotBody map[string]string
+	client, _ := newStub(t, func(w http.ResponseWriter, r *http.Request) {
+		b, _ := io.ReadAll(r.Body)
+		_ = json.Unmarshal(b, &gotBody)
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write([]byte(`{"ok":true}`))
+	})
+	if err := client.CancelCustomer("", "user-1"); err != nil {
+		t.Fatalf("expected nil error, got %v", err)
+	}
+	if gotBody["user_id"] != "user-1" || gotBody["customer_id"] != "" {
+		t.Fatalf("body = %v, want user_id=user-1 and an empty customer_id", gotBody)
+	}
+}

internal/server/handlers_account.go

@@ -9,6 +9,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/PerpetualSoftware/pad/internal/billing"
 	"github.com/PerpetualSoftware/pad/internal/models"
 	"github.com/PerpetualSoftware/pad/internal/store"
 	"github.com/PerpetualSoftware/pad/internal/watchevents"
@@ -198,9 +199,8 @@ func (s *Server) handleDeleteAccount(w http.ResponseWriter, r *http.Request) {
 	// would keep getting charged with no way for us to find and cancel
 	// it. TASK-690 / PLAN-645.
 	//
-	// Skipped when:
-	//   - the user has no Stripe customer (free plan, OAuth-only, never paid)
-	//   - the sidecar isn't configured (self-hosted deploys with no billing)
+	// Skipped when the sidecar isn't configured (self-hosted deploys with no
+	// billing).
 	//
 	// Failure strategy: any non-nil error aborts the delete with a 500.
 	// pad-cloud normalizes Stripe 404/resource_missing to a 200 success on
@@ -211,9 +211,27 @@ func (s *Server) handleDeleteAccount(w http.ResponseWriter, r *http.Request) {
 	// continue": doing so would wipe the user's StripeCustomerID while
 	// leaving the Stripe subscription billing, which is the exact
 	// regression this task exists to prevent.
+	//
+	// BUG-3560: the sidecar is asked even when no customer is linked, with
+	// the user id, because a user can own a Stripe customer stamped with
+	// their id and not linked here (TASK-3549 keeps one link and unions the
+	// rest); it cancels every customer the user owns. A sidecar that predates
+	// user_id refuses an empty customer_id with 400: it could not have
+	// cancelled anything, which is what not asking did before, so that one
+	// answer lets the deletion proceed.
 	stripeWasCancelled := false
-	if fullUser.StripeCustomerID != "" && s.cloudSidecar != nil {
-		if err := s.cloudSidecar.CancelCustomer(fullUser.StripeCustomerID); err != nil {
+	if s.cloudSidecar != nil {
+		err := s.cloudSidecar.CancelCustomer(fullUser.StripeCustomerID, fullUser.ID)
+		var sidecarErr *billing.SidecarError
+		switch {
+		case err == nil:
+			// Possibly only a stamped, unlinked customer: still billing that
+			// was cancelled, which the partial-delete alert below must know.
+			stripeWasCancelled = true
+		case fullUser.StripeCustomerID == "" && errors.As(err, &sidecarErr) && sidecarErr.Status == http.StatusBadRequest:
+			slog.Warn("delete account: sidecar predates user_id; no linked customer to cancel, continuing",
+				"user_id", user.ID)
+		default:
 			slog.Error("delete account: sidecar cancel-customer failed, aborting delete",
 				"user_id", user.ID,
 				"customer_id", fullUser.StripeCustomerID,
@@ -222,7 +240,6 @@ func (s *Server) handleDeleteAccount(w http.ResponseWriter, r *http.Request) {
 				"We couldn't cancel your subscription. Your account was NOT deleted and you have NOT been charged anything new. Please try again in a few minutes or contact support.")
 			return
 		}
-		stripeWasCancelled = true
 	}
 
 	// TASK-3272: the deletion soft-deletes every live workspace this user

internal/server/handlers_account_test.go

@@ -25,14 +25,17 @@ import (
 type fakeSidecar struct {
 	calls          int32 // atomic counter of CancelCustomer invocations
 	lastCustomerID atomic.Pointer[string]
+	lastUserID     atomic.Pointer[string]
 	err            error
 	hook           func(customerID string) // runs under CancelCustomer; optional
 }
 
-func (f *fakeSidecar) CancelCustomer(customerID string) error {
+func (f *fakeSidecar) CancelCustomer(customerID, userID string) error {
 	atomic.AddInt32(&f.calls, 1)
 	id := customerID // copy so the atomic pointer doesn't alias the caller's stack
 	f.lastCustomerID.Store(&id)
+	uid := userID
+	f.lastUserID.Store(&uid)
 	if f.hook != nil {
 		f.hook(customerID)
 	}
@@ -151,11 +154,12 @@ func TestHandleDeleteAccount_CancelsStripeBeforeDelete(t *testing.T) {
 	}
 }
 
-// TestHandleDeleteAccount_SkipsWhenNoStripeCustomer verifies that users
-// without a Stripe customer ID (free tier, OAuth-only, never paid) don't
-// trigger a sidecar call. Otherwise we'd burn a sidecar RPC on every
-// free-user delete and log-spam 400 "customer_id must start with 'cus_'".
-func TestHandleDeleteAccount_SkipsWhenNoStripeCustomer(t *testing.T) {
+// BUG-3560: a user with no LINKED Stripe customer can still own one stamped
+// with their id (TASK-3549 keeps one link and unions the rest), so the
+// sidecar is asked with the user id even then, and cancels whatever the user
+// owns. (Before, such a user was never asked about, and that customer kept
+// charging after the account was gone.)
+func TestHandleDeleteAccount_AsksSidecarByUserWhenNoLinkedCustomer(t *testing.T) {
 	srv := testServer(t)
 	fake := &fakeSidecar{}
 	srv.SetCloudSidecar(fake)
@@ -166,26 +170,62 @@ func TestHandleDeleteAccount_SkipsWhenNoStripeCustomer(t *testing.T) {
 	if rr.Code != http.StatusOK {
 		t.Fatalf("delete-account: expected 200, got %d: %s", rr.Code, rr.Body.String())
 	}
-
-	if fake.callCount() != 0 {
-		t.Errorf("CancelCustomer: expected 0 calls for non-paying user, got %d", fake.callCount())
+	if fake.callCount() != 1 {
+		t.Fatalf("CancelCustomer: expected 1 call, got %d", fake.callCount())
+	}
+	if got := *fake.lastCustomerID.Load(); got != "" {
+		t.Errorf("customer id = %q, want empty", got)
+	}
+	if got := *fake.lastUserID.Load(); got != userID {
+		t.Errorf("user id = %q, want %q", got, userID)
 	}
-
 	u, err := srv.store.GetUser(userID)
 	if err != nil {
 		t.Fatalf("get user after delete: %v", err)
 	}
 	if u != nil {
-		t.Error("expected user to be deleted even without a Stripe customer")
+		t.Error("expected user to be deleted")
+	}
+}
+
+// A sidecar that predates user_id refuses an empty customer_id with 400. It
+// could not have cancelled anything, which is exactly what not asking did
+// before, so this one answer lets the deletion of a user with no linked
+// customer proceed: either deploy order is safe.
+func TestHandleDeleteAccount_OlderSidecar400WithNoLinkedCustomerProceeds(t *testing.T) {
+	srv := testServer(t)
+	fake := &fakeSidecar{err: &billing.SidecarError{Status: http.StatusBadRequest, Body: `{"error":"customer_id must start with 'cus_'"}`}}
+	srv.SetCloudSidecar(fake)
+
+	userID, token := bootstrapAccountDeleteUser(t, srv, "")
+
+	rr := deleteAccountReq(srv, map[string]interface{}{"password": "correct-horse-battery-staple"}, token)
+	if rr.Code != http.StatusOK {
+		t.Fatalf("expected 200, got %d: %s", rr.Code, rr.Body.String())
+	}
+	if u, _ := srv.store.GetUser(userID); u != nil {
+		t.Error("expected user to be deleted")
+	}
+}
+
+// Any other failure with no linked customer still aborts: a stamped customer
+// may exist, and deleting the account would leave it charging.
+func TestHandleDeleteAccount_NoLinkedCustomerSidecar5xxAborts(t *testing.T) {
+	srv := testServer(t)
+	fake := &fakeSidecar{err: &billing.SidecarError{Status: http.StatusInternalServerError, Body: `{"error":"boom"}`}}
+	srv.SetCloudSidecar(fake)
+
+	userID, token := bootstrapAccountDeleteUser(t, srv, "")
+
+	rr := deleteAccountReq(srv, map[string]interface{}{"password": "correct-horse-battery-staple"}, token)
+	if rr.Code != http.StatusInternalServerError {
+		t.Fatalf("expected 500, got %d: %s", rr.Code, rr.Body.String())
+	}
+	if u, _ := srv.store.GetUser(userID); u == nil {
+		t.Error("user must still exist after an aborted delete")
 	}
 }
 
-// TestHandleDeleteAccount_SkipsWhenNoSidecarConfigured verifies that a
-// self-hosted deploy (no CloudSidecar wired) lets deletes complete for
-// paying users without blowing up on nil dereference. In practice this
-// arrangement shouldn't exist (if the user has a Stripe customer ID,
-// cloud mode was configured at some point) but it's the graceful-fallback
-// contract: missing sidecar ≠ broken deletes.
 func TestHandleDeleteAccount_SkipsWhenNoSidecarConfigured(t *testing.T) {
 	srv := testServer(t)
 	// No SetCloudSidecar — the reverse hook is nil.

internal/server/handlers_admin_billing_test.go

@@ -23,7 +23,7 @@ type fakeBillingSidecar struct {
 	err      error
 }
 
-func (f *fakeBillingSidecar) CancelCustomer(string) error {
+func (f *fakeBillingSidecar) CancelCustomer(string, string) error {
 	return nil // unused by these tests
 }
 

internal/server/server.go

@@ -891,7 +891,7 @@ type CloudSidecar interface {
 	// misconfig), or 5xx (upstream breakage). Continuing after an error
 	// would wipe the user's StripeCustomerID while leaving the subscription
 	// billing, which is exactly the regression TASK-690 exists to prevent.
-	CancelCustomer(customerID string) error
+	CancelCustomer(customerID, userID string) error
 
 	// GetBillingMetrics fetches an aggregated Stripe-derived snapshot from
 	// pad-cloud's /admin/metrics/billing endpoint (active subs, MRR, ARR,