improve plan update

[thibaultleouay]

No description provided.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
openstatus-dashboard	Ready	Preview, Comment	Mar 4, 2026 8:41am
openstatus-status-page	Ready	Preview, Comment	Mar 4, 2026 8:41am
openstatus-web	Ready	Preview, Comment	Mar 4, 2026 8:41am

[thibaultleouay]

@claude review this

[claude]

Claude finished @thibaultleouay's task in 1m 28s —— View job

---

Code Review

• Gather context and read changed files
• Analyze the diff
• Provide review feedback

---

Summary

This PR improves the Stripe webhook handling for plan updates and downgrades. The main changes are:

1. customerSubscriptionUpdated: Replaces commented-out add-on logic with proper plan detection, workspace update, duplicate subscription cancellation, and upgrade/downgrade analytics.
2. customerSubscriptionDeleted: Adds active-subscription guard and enforces free plan limits on downgrade (deactivates monitors, deletes extra status pages, cleans up notifications).
3. Cleanup: Removes a debug console.log(event).

---

Issues

Bug: endsAt not reset on subscription deletion (packages/api/src/router/stripe/webhook.ts:251-260)

In customerSubscriptionDeleted, the workspace update resets subscriptionId, plan, and paidUntil — but endsAt is left at the old value. If endsAt is used for display or grace period logic, it will show stale data after a cancellation.

.set({
  subscriptionId: null,
  plan: "free",
  paidUntil: null,
  limits: JSON.stringify(getLimits("free")),
  // endsAt: null  ← missing
})

Fix this →

---

Concern: No database transaction for multi-step downgrade (packages/api/src/router/stripe/webhook.ts:251-330)

The customerSubscriptionDeleted handler runs multiple DB operations sequentially (update workspace → deactivate monitors → delete pages → reset page → delete notifications) without a transaction. If the process crashes or throws midway, the workspace will be in a partially downgraded state — e.g., the workspace is marked free but extra monitors are still active.

Consider wrapping in a transaction if the DB client supports it, or at minimum ordering the operations from least-destructive to most-destructive.

---

Concern: N+1 queries in downgrade cleanup (packages/api/src/router/stripe/webhook.ts:283-330)

Individual UPDATE/DELETE queries are issued per monitor and per notification in a loop. These could be batched using inArray(monitor.id, idsToDeactivate) for better performance (especially for workspaces with many resources).

// Instead of:
for (const m of activeMonitors.slice(1)) {
  await opts.ctx.db.update(monitor).set({ active: false }).where(eq(monitor.id, m.id)).run();
}

// Consider:
const idsToDeactivate = activeMonitors.slice(1).map((m) => m.id);
if (idsToDeactivate.length > 0) {
  await opts.ctx.db.update(monitor).set({ active: false }).where(inArray(monitor.id, idsToDeactivate)).run();
}

---

Positives

• Good guard (L43-45): Early return for non-active subscriptions in customerSubscriptionUpdated prevents processing irrelevant events (e.g., past_due, canceled).
• Smart deletion guard (L242-249): Checking for remaining active subscriptions before downgrading in customerSubscriptionDeleted correctly handles the plan-upgrade case where the old subscription is deleted but a new one already exists.
• Free plan enforcement: The downgrade cleanup (keep oldest monitor, keep oldest page, prefer email notification) is a sensible and user-friendly approach.
• Analytics parity: Upgrade/downgrade events are now properly fired from customerSubscriptionUpdated, matching the behavior in sessionCompleted.
• Duplicate subscription cancellation (L91-103): Cancelling other active subscriptions after an update is a solid safeguard, and the try/catch prevents a failed cancel from blocking the whole handler.
• limits reset (L257): customerSubscriptionDeleted now correctly resets limits to free-tier values.

---

[cloudflare-workers-and-pages]

Deploying openstatus-docs with    Cloudflare Pages

Latest commit:	a6f090f
Status:	✅  Deploy successful!
Preview URL:	https://18af3497.openstatus-1sy.pages.dev
Branch Preview URL:	https://improve-plan-update.openstatus-1sy.pages.dev

View logs

[thibaultleouay]

@claude review this

[claude]

Claude finished @thibaultleouay's task in 1m 53s —— View job

---

Code Review

• Gather context
• Read changed files and diff
• Analyze and provide review feedback

---

Summary

This PR improves Stripe webhook handling for plan updates and cancellations. The changes cover customerSubscriptionUpdated (replaces dead commented-out code with working plan detection and analytics) and customerSubscriptionDeleted (adds active-subscription guard, wraps the downgrade in a DB transaction, and enforces free-tier limits).

---

Fixes Since Last Review

The two major issues from the previous review are now resolved:

• endsAt: null is correctly included in the workspace reset on deletion (L260). ✅
• Database transaction now wraps the entire multi-step downgrade in customerSubscriptionDeleted (L253). ✅

---

Remaining Issues

N+1 queries in downgrade loops (webhook.ts:287-293, 327-331)

Still outstanding from the last review. Each monitor and each notification is updated/deleted in a separate query inside the transaction. For workspaces with many resources this is inefficient and increases transaction lock time.

// Current (N+1):
for (const m of activeMonitors.slice(1)) {
  await tx.update(monitor).set({ active: false }).where(eq(monitor.id, m.id)).run();
}

// Better:
const idsToDeactivate = activeMonitors.slice(1).map((m) => m.id);
if (idsToDeactivate.length > 0) {
  await tx.update(monitor).set({ active: false }).where(inArray(monitor.id, idsToDeactivate)).run();
}

Same pattern applies to the notification deletion loop. Fix this →

---

customerSubscriptionUpdated fires workspace update even for non-plan events (webhook.ts:81-91)

When detectedPlan is undefined (e.g., a subscription is updated for a billing or payment reason, not a plan change), the handler still updates subscriptionId, endsAt, paidUntil, and limits (with existing limits). This is mostly harmless since it keeps fields in sync, but it could silently overwrite limits with stale parsed values if the schema changes. Consider adding a guard to skip the update entirely when nothing meaningful changed.

---

No event deduplication (webhook.ts:42, 237)

Stripe may deliver the same webhook event more than once. Neither handler deduplicates by event.id, so a retry could trigger duplicate analytics events (UpgradeWorkspace, DowngradeWorkspace) or redundant DB writes. This is a pre-existing issue but worth tracking, especially now that more business logic runs in these handlers.

---

_workspace[0] access is technically unsafe (webhook.ts:353)

After the transaction, _workspace[0].id is accessed without a null check. TypeScript can't narrow this because the throw is inside tx. It's safe at runtime (the transaction throws if empty), but adding if (!_workspace[0]) throw ... after the transaction makes the intent explicit and avoids TypeScript complaints.

---

Positives

• Status check guard (L45-47): Early return for non-active subscriptions is a clean way to avoid processing past_due, canceled, or trialing updates.
• Active subscription guard in deletion (L249-251): Correctly handles plan upgrades where the old subscription is deleted but a new one already exists — prevents a false downgrade.
• Transaction wrapping: All the multi-step downgrade logic now runs atomically.
• endsAt: null reset: Properly clears the period end date on cancellation.
• Cancellation safeguard with try/catch (L100-104): A failed cancel of a duplicate subscription won't block the whole handler.
• Analytics parity: Upgrade/downgrade events are now fired from customerSubscriptionUpdated, matching sessionCompleted behavior.
• Free-tier cleanup: Keep-oldest strategy for monitors, pages, and preferring email notification is a sensible UX choice.
• Member cleanup (L333-348): Removing non-owner members and pending invitations on downgrade is thorough and correct.

---