appview: make the webhook cap agree between creation and delivery

The UI and the dispatcher asked the same question and got different
answers. getWebhookLimits short-circuited on a disabled billing manager and
returned unlimited without consulting it, while server.go hands
BillingManager.GetWebhookLimits straight to NewDispatcher, bypassing that
short-circuit entirely. With billing compiled out the stub answers
non-captains with (1, false).

So a non-captain saw "N / unlimited webhooks configured", could create as
many as they liked, and only the oldest was ever delivered. allTriggers was
false on that same path, so even the surviving one was restricted to
FreeTriggerMask; a webhook set to a scan trigger fired nothing at all, with
no message anywhere and only an INFO line server-side.

Route both paths through the manager so they cannot drift. The intended
non-billing policy is one webhook with TriggerFirst | TriggerPush |
TriggerQuota, which is what the stub already returned and what the shipped
config's Free tier specifies (max_webhooks: 1, webhook_all_triggers: false),
so enabling billing leaves free users exactly where they were and only
unlocks upward. That also removes a downgrade cliff: nobody can accumulate
webhooks under a phantom unlimited and lose them when billing turns on.

Drop the dead webhookLimits{Max: 1}, overwritten on the following line, and
give a nil manager the same policy rather than a third answer.

Latent, not live: production has zero webhooks configured today.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UAqi2hS2dhZoatqcWoYZQk

pkg/appview/handlers/webhook_limits_test.go

@@ -0,0 +1,83 @@
+//go:build !billing
+
+// These tests pin the policy a billing-compiled-out build runs with, which is
+// what production ships today (finding 1). They construct the stub manager, so
+// they cannot build under -tags billing where billing.New requires a real
+// config. A billing-enabled counterpart asserting the same agreement property
+// against the tier ladder is worth adding when billing is turned on.
+
+package handlers
+
+import (
+	"testing"
+
+	"atcr.io/pkg/appview/webhooks"
+	"atcr.io/pkg/billing"
+)
+
+// The creation gate and the delivery path must answer the same question the
+// same way. They are wired separately — the UI calls getWebhookLimits, while
+// the dispatcher gets BillingManager.GetWebhookLimits handed to it directly in
+// server.go — so nothing but a test stops them drifting apart again.
+//
+// Finding 22: getWebhookLimits used to short-circuit on a disabled manager and
+// answer "unlimited" without consulting it. The UI offered unlimited webhooks
+// while delivery capped non-captains at one and dropped every trigger outside
+// FreeTriggerMask, with no message anywhere.
+func TestGetWebhookLimits_AgreesWithDeliveryPath(t *testing.T) {
+	// The stub manager is what a billing-compiled-out build runs with.
+	mgr := billing.New(nil, nil, "", nil, "", nil)
+	h := &BaseUIHandler{BillingManager: mgr}
+
+	const userDID = "did:plc:notacaptain"
+
+	uiLimits := h.getWebhookLimits(userDID)
+
+	// This is exactly what NewDispatcher receives in server.go.
+	deliveryMax, deliveryAllTriggers := mgr.GetWebhookLimits(userDID)
+
+	if uiLimits.Max != deliveryMax {
+		t.Errorf("UI offers Max=%d but delivery allows %d", uiLimits.Max, deliveryMax)
+	}
+	if uiLimits.AllTriggers != deliveryAllTriggers {
+		t.Errorf("UI AllTriggers=%v but delivery uses %v", uiLimits.AllTriggers, deliveryAllTriggers)
+	}
+}
+
+// The intended non-billing policy: one webhook, restricted to the free
+// triggers. This mirrors the Free tier in the shipped config (max_webhooks: 1,
+// webhook_all_triggers: false), so turning billing on does not change what a
+// free user already had.
+func TestGetWebhookLimits_NonBillingDefault(t *testing.T) {
+	h := &BaseUIHandler{BillingManager: billing.New(nil, nil, "", nil, "", nil)}
+
+	limits := h.getWebhookLimits("did:plc:notacaptain")
+
+	if limits.Max != 1 {
+		t.Errorf("non-billing default should be 1 webhook, got %d", limits.Max)
+	}
+	if limits.AllTriggers {
+		t.Error("non-billing default must not grant triggers outside FreeTriggerMask")
+	}
+
+	// Name the three that survive, so a change to FreeTriggerMask has to come
+	// past this test rather than silently altering what a free user can pick.
+	want := webhooks.TriggerFirst | webhooks.TriggerPush | webhooks.TriggerQuota
+	if webhooks.FreeTriggerMask != want {
+		t.Errorf("FreeTriggerMask = %d, want TriggerFirst|TriggerPush|TriggerQuota (%d)",
+			webhooks.FreeTriggerMask, want)
+	}
+}
+
+// A nil manager must land on the same policy rather than inventing a third
+// answer, since the dispatcher would still be applying the real one.
+func TestGetWebhookLimits_NilManagerMatchesPolicy(t *testing.T) {
+	h := &BaseUIHandler{}
+
+	limits := h.getWebhookLimits("did:plc:whoever")
+
+	if limits.Max != 1 || limits.AllTriggers {
+		t.Errorf("nil manager gave Max=%d AllTriggers=%v, want 1/false",
+			limits.Max, limits.AllTriggers)
+	}
+}

pkg/appview/handlers/webhooks.go

@@ -243,16 +243,28 @@ func (h *TestWebhookHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 
 // ---- Shared helpers ----
 
-// getWebhookLimits returns the webhook limits for a user based on their billing tier.
-// When the billing manager is absent or disabled we treat the deployment as
-// "all features free": unlimited webhooks and all trigger types allowed.
-// Without this, self-hosted instances without billing config silently capped
-// users at 1 webhook with restricted triggers.
+// getWebhookLimits returns the webhook limits for a user based on their billing
+// tier.
+//
+// This must agree with the delivery path, which wires
+// BillingManager.GetWebhookLimits straight into the dispatcher (see
+// NewDispatcher in server.go). An earlier version short-circuited on a disabled
+// manager and answered "unlimited" without consulting it, so the UI offered
+// unlimited webhooks while delivery silently capped non-captains at one and
+// dropped every trigger outside FreeTriggerMask. Both paths now go through the
+// same method so they cannot drift again.
+//
+// With billing compiled out the manager's stub is the policy: hold captains get
+// unlimited, everyone else gets one webhook restricted to FreeTriggerMask
+// (TriggerFirst | TriggerPush | TriggerQuota).
 func (h *BaseUIHandler) getWebhookLimits(userDID string) webhookLimits {
-	if h.BillingManager == nil || !h.BillingManager.Enabled() {
-		return webhookLimits{Max: -1, AllTriggers: true}
+	if h.BillingManager == nil {
+		// No manager at all, which happens in tests and partially constructed
+		// handlers. Mirror the non-billing policy rather than inventing a third
+		// answer that neither path would agree with.
+		return webhookLimits{Max: 1}
 	}
-	limits := webhookLimits{Max: 1}
+	limits := webhookLimits{}
 	limits.Max, limits.AllTriggers = h.BillingManager.GetWebhookLimits(userDID)
 	limits.PaidTierName = h.BillingManager.GetFirstTierWithAllTriggers()
 	return limits