Skip to content

Billing limits#3996

Draft
kathiekiwi wants to merge 8 commits into
mainfrom
feature/billing-limits
Draft

Billing limits#3996
kathiekiwi wants to merge 8 commits into
mainfrom
feature/billing-limits

Conversation

@kathiekiwi

Copy link
Copy Markdown
Collaborator

Closes #

✅ Checklist

  • I have followed every step in the contributing guide
  • The PR title follows the convention.
  • I ran and tested the code works

Testing

[Describe the steps you took to test this change]


Changelog

[Short description of what has changed]


Screenshots

[Screenshots]

💯

kathiekiwi and others added 8 commits June 19, 2026 14:40
Add the EnvironmentPauseSource enum and migration, plus the billing-limit platform client wrappers and schemas.
Configure a spend limit, manage billing alerts, and surface org-wide banners.
Converge billable environments to paused via webhook and a reconciliation worker; block manual resume.
Reject triggers with a 422 once entitlement reports no access, and bust the entitlement cache on state changes.
Recovery UI and durable resolve: cancel queued runs before unpausing, with reconciliation as a safety net.
Optionally cancel in-progress runs on limit hit via a deduplicated bulk-cancel job.
… tests

Add the usage-bar marker, documentation, and test coverage.
Replace the invalid @trigger.dev/platform/v3 import so billing limit typecheck passes in CI.

Co-authored-by: Cursor <cursoragent@cursor.com>
@changeset-bot

changeset-bot Bot commented Jun 19, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 5c1a4bf

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

This PR implements a billing limits feature that lets organizations configure a monthly compute spend cap. When the cap is reached, a billing platform webhook triggers a grace period during which billable environments are paused; new task triggers are rejected once the grace period ends. A recovery flow lets users increase or remove the limit and choose to resume queued runs or accept cancellation. A Redis-backed worker periodically reconciles environment pause state against billing platform data. The feature adds a new /settings/billing-limits settings page that consolidates limit configuration, alert thresholds, and the recovery panel, replacing the old /settings/billing-alerts route (which now redirects). The unified OrgBanner component replaces the prior UpgradePrompt and EnvironmentBanner components with a selector-driven switch over all banner states.

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is entirely a template with all sections unfilled (unchecked checklist, placeholder testing/changelog/screenshots sections), providing no actual information about the changes, rationale, or testing performed. Complete the PR description by filling in all sections: link the relevant issue(s), confirm checklist items, document testing steps, provide a changelog summary, and include any relevant screenshots.
Docstring Coverage ⚠️ Warning Docstring coverage is 20.31% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Billing limits' is vague and generic; it does not describe the specific feature or main change in a way that would help developers scanning history understand the scope or intent. Use a more descriptive title that captures the main feature, such as 'Add billing limits enforcement with environment pausing and recovery flows' or similar.
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/billing-limits

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 17

🧹 Nitpick comments (4)
apps/webapp/app/env.server.ts (1)

1474-1474: ⚡ Quick win

Default to explicit opt-in ("0") for the new worker flag.

Per codebase conventions, new background worker feature flags should hard-default to "0" rather than inheriting from a parent flag like WORKER_ENABLED. The current default could cause the billing limit worker to auto-start unexpectedly on upgrade for deployments that already have WORKER_ENABLED set, introducing unplanned background load.

Suggested fix
-    BILLING_LIMIT_WORKER_ENABLED: z.string().default(process.env.WORKER_ENABLED ?? "true"),
+    BILLING_LIMIT_WORKER_ENABLED: z.string().default("0"),

Based on learnings: "In apps/*/app/env.server.ts, any new background/periodic worker feature flag should hard-default to '0' (explicit opt-in) rather than inheriting from a parent flag."

Source: Learnings

apps/webapp/app/v3/services/billingLimit/billingLimitReconciliation.server.ts (1)

67-75: ⚡ Quick win

Sequential billing-limit lookups will slow reconciliation as org count grows.

At Line 73 and Line 83, getBillingLimit() is awaited serially in loops. Consider bounded parallelism (Promise.all + limiter) to keep reconcile ticks timely.

Also applies to: 77-85

apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.billing-limits/route.tsx (1)

95-97: ⚡ Quick win

Add an inline note explaining why primary prisma is required here.

These lookups correctly use prisma.organization.findFirst(...); adding a one-line comment will reduce future regressions back to $replica in auth-scoped slug resolution paths.

Based on learnings: slug→organization resolution for authorization scope should stay on primary Prisma because replica lag can lead to unscoped RBAC checks.

Also applies to: 189-191

Source: Learnings

apps/webapp/app/components/billing/selectOrgBanner.ts (1)

3-10: ⚡ Quick win

Replace runtime enum with a string-union constant type.

Using export enum OrgBannerKind here conflicts with the repo TypeScript guideline and adds avoidable runtime enum output.

♻️ Suggested change
-export enum OrgBannerKind {
-  LimitRejected = "limit-rejected",
-  LimitGrace = "limit-grace",
-  NoLimitConfigured = "no-limit",
-  Upgrade = "upgrade",
-  EnvironmentWarning = "env-warning",
-  None = "none",
-}
+export const ORG_BANNER_KIND = {
+  LimitRejected: "limit-rejected",
+  LimitGrace: "limit-grace",
+  NoLimitConfigured: "no-limit",
+  Upgrade: "upgrade",
+  EnvironmentWarning: "env-warning",
+  None: "none",
+} as const;
+
+export type OrgBannerKind = (typeof ORG_BANNER_KIND)[keyof typeof ORG_BANNER_KIND];

As per coding guidelines, **/*.{ts,tsx}: “Use types over interfaces for TypeScript” and “Avoid using enums; prefer string unions or const objects instead.”

Source: Coding guidelines


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: c7c9a3c5-ee1c-4334-b40a-d0b9aca60dd8

📥 Commits

Reviewing files that changed from the base of the PR and between 315baf2 and 5c1a4bf.

📒 Files selected for processing (81)
  • .server-changes/billing-limits.md
  • apps/webapp/app/components/billing/AnimatedOrgBannerBar.tsx
  • apps/webapp/app/components/billing/BillingAlertsSection.tsx
  • apps/webapp/app/components/billing/BillingLimitConfigSection.tsx
  • apps/webapp/app/components/billing/BillingLimitRecoveryPanel.tsx
  • apps/webapp/app/components/billing/BillingLimitResolveProgress.tsx
  • apps/webapp/app/components/billing/OrgBanner.tsx
  • apps/webapp/app/components/billing/UpgradePrompt.tsx
  • apps/webapp/app/components/billing/billingAlertsFormat.ts
  • apps/webapp/app/components/billing/billingLimitFormat.ts
  • apps/webapp/app/components/billing/selectOrgBanner.ts
  • apps/webapp/app/components/layout/AppLayout.tsx
  • apps/webapp/app/components/navigation/EnvironmentBanner.tsx
  • apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsx
  • apps/webapp/app/components/primitives/AnimatedCallout.tsx
  • apps/webapp/app/components/primitives/PageHeader.tsx
  • apps/webapp/app/entry.server.tsx
  • apps/webapp/app/env.server.ts
  • apps/webapp/app/hooks/useOrganizations.ts
  • apps/webapp/app/hooks/useScrollContainerToTop.ts
  • apps/webapp/app/models/organization.server.ts
  • apps/webapp/app/presenters/OrganizationsPresenter.server.ts
  • apps/webapp/app/presenters/v3/EnvironmentVariablesPresenter.server.ts
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.billing-alerts/route.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.billing-limits/billingLimitsRevalidation.ts
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.billing-limits/billingLimitsRoute.server.ts
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.billing-limits/route.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.usage/route.tsx
  • apps/webapp/app/routes/_app.orgs.$organizationSlug/route.tsx
  • apps/webapp/app/routes/admin.api.v1.orgs.$organizationId.billing-limit.hit.ts
  • apps/webapp/app/routes/admin.api.v1.orgs.$organizationId.billing-limit.reject.ts
  • apps/webapp/app/routes/admin.api.v1.orgs.$organizationId.billing-limit.resolve.ts
  • apps/webapp/app/routes/storybook.callout/route.tsx
  • apps/webapp/app/runEngine/validators/triggerTaskValidator.ts
  • apps/webapp/app/runEngine/validators/validateProductionEntitlement.server.ts
  • apps/webapp/app/services/billingLimit.schemas.ts
  • apps/webapp/app/services/platform.v3.server.ts
  • apps/webapp/app/services/upsertBranch.server.ts
  • apps/webapp/app/utils/pathBuilder.ts
  • apps/webapp/app/v3/billingLimitWorker.server.ts
  • apps/webapp/app/v3/outOfEntitlementError.server.ts
  • apps/webapp/app/v3/services/billingLimit/BillingLimitBulkCancelService.server.ts
  • apps/webapp/app/v3/services/billingLimit/billingLimitCancelInProgressRuns.server.ts
  • apps/webapp/app/v3/services/billingLimit/billingLimitConstants.ts
  • apps/webapp/app/v3/services/billingLimit/billingLimitConvergeEnvironments.server.ts
  • apps/webapp/app/v3/services/billingLimit/billingLimitConvergeEnvironmentsService.server.ts
  • apps/webapp/app/v3/services/billingLimit/billingLimitConvergeResolve.server.ts
  • apps/webapp/app/v3/services/billingLimit/billingLimitHit.server.ts
  • apps/webapp/app/v3/services/billingLimit/billingLimitPendingResolve.types.ts
  • apps/webapp/app/v3/services/billingLimit/billingLimitPendingResolveCoordinator.server.ts
  • apps/webapp/app/v3/services/billingLimit/billingLimitPendingResolveFailure.server.ts
  • apps/webapp/app/v3/services/billingLimit/billingLimitQueuedRuns.server.ts
  • apps/webapp/app/v3/services/billingLimit/billingLimitReconcileQueue.server.ts
  • apps/webapp/app/v3/services/billingLimit/billingLimitReconcileTarget.server.ts
  • apps/webapp/app/v3/services/billingLimit/billingLimitReconciliation.server.ts
  • apps/webapp/app/v3/services/billingLimit/getBillingLimitQueuedRunCount.server.ts
  • apps/webapp/app/v3/services/billingLimit/getInitialEnvPauseStateForBillingLimit.server.ts
  • apps/webapp/app/v3/services/billingLimit/manualPauseEnvironmentGuard.server.ts
  • apps/webapp/app/v3/services/billingLimit/runBillingLimitReconcileTick.server.ts
  • apps/webapp/app/v3/services/pauseEnvironment.server.ts
  • apps/webapp/app/v3/services/triggerTask.server.ts
  • apps/webapp/server.ts
  • apps/webapp/test/billingAlertsFormat.test.ts
  • apps/webapp/test/billingLimit.schemas.test.ts
  • apps/webapp/test/billingLimitBulkCancelInProgress.test.ts
  • apps/webapp/test/billingLimitConvergeEnvironments.test.ts
  • apps/webapp/test/billingLimitConvergeEnvironmentsService.test.ts
  • apps/webapp/test/billingLimitConvergeResolve.test.ts
  • apps/webapp/test/billingLimitEnvCreatePause.test.ts
  • apps/webapp/test/billingLimitHit.test.ts
  • apps/webapp/test/billingLimitPauseEnvironment.test.ts
  • apps/webapp/test/billingLimitQueuedRuns.test.ts
  • apps/webapp/test/billingLimitReconcileTick.test.ts
  • apps/webapp/test/billingLimitReconciliation.test.ts
  • apps/webapp/test/billingLimitTriggerEntitlement.test.ts
  • apps/webapp/test/billingLimitsRoute.test.ts
  • apps/webapp/test/orgBanner.test.ts
  • docs/how-to-reduce-your-spend.mdx
  • internal-packages/database/prisma/migrations/20260614120000_add_environment_pause_source_billing_limit/migration.sql
  • internal-packages/database/prisma/schema.prisma
💤 Files with no reviewable changes (2)
  • apps/webapp/app/components/billing/UpgradePrompt.tsx
  • apps/webapp/app/components/navigation/EnvironmentBanner.tsx

Comment on lines +52 to +58
alertLevels: z.preprocess((i) => {
const values = typeof i === "string" ? [i] : Array.isArray(i) ? i : [];
return values
.filter((v) => v !== "")
.map((v) => Number(v))
.filter((n) => Number.isFinite(n));
}, z.number().array().refine(thresholdValuesAreUnique, "Each alert must be unique")),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Reject malformed threshold values instead of silently dropping them.

Lines 55-57 currently remove non-finite parsed values, so a non-empty invalid input can be silently ignored and still persist a partial alert set. This should fail validation explicitly.

💡 Suggested fix
   alertLevels: z.preprocess((i) => {
     const values = typeof i === "string" ? [i] : Array.isArray(i) ? i : [];
-    return values
-      .filter((v) => v !== "")
-      .map((v) => Number(v))
-      .filter((n) => Number.isFinite(n));
+    return values
+      .filter((v) => v !== "")
+      .map((v) => Number(v));
   }, z.number().array().refine(thresholdValuesAreUnique, "Each alert must be unique")),

Comment on lines +18 to +22
function getUpgradeResetDate(): Date {
const nextMonth = new Date();
nextMonth.setUTCMonth(nextMonth.getMonth() + 1);
nextMonth.setUTCDate(1);
nextMonth.setUTCHours(0, 0, 0, 0);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify mixed local/UTC month API usage in this function
rg -n 'setUTCMonth\(.+getMonth\(' apps/webapp/app/components/billing/OrgBanner.tsx
sed -n '18,24p' apps/webapp/app/components/billing/OrgBanner.tsx

Repository: triggerdotdev/trigger.dev

Length of output: 331


Use UTC month math consistently in getUpgradeResetDate().

Line 20 mixes getMonth() (local time) with setUTCMonth() (UTC), which can compute the wrong reset month around timezone boundaries. Use getUTCMonth() instead.

Proposed fix
-  nextMonth.setUTCMonth(nextMonth.getMonth() + 1);
+  nextMonth.setUTCMonth(nextMonth.getUTCMonth() + 1);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function getUpgradeResetDate(): Date {
const nextMonth = new Date();
nextMonth.setUTCMonth(nextMonth.getMonth() + 1);
nextMonth.setUTCDate(1);
nextMonth.setUTCHours(0, 0, 0, 0);
function getUpgradeResetDate(): Date {
const nextMonth = new Date();
nextMonth.setUTCMonth(nextMonth.getUTCMonth() + 1);
nextMonth.setUTCDate(1);
nextMonth.setUTCHours(0, 0, 0, 0);

Comment on lines +34 to +40
} else if (billingLimit && !billingLimit.isConfigured && showSelfServe) {
return OrgBannerKind.NoLimitConfigured;
}

if (hasExceededFreeTier) {
return OrgBannerKind.Upgrade;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Upgrade banner is masked by no-limit banner in overlapping states.

Line 34 returns NoLimitConfigured before the free-tier check on Line 38. If both are true, users won’t see the upgrade CTA even when over free-tier limits.

🐛 Suggested ordering fix
-  } else if (billingLimit && !billingLimit.isConfigured && showSelfServe) {
-    return OrgBannerKind.NoLimitConfigured;
-  }
-
   if (hasExceededFreeTier) {
     return OrgBannerKind.Upgrade;
   }
+
+  if (billingLimit && !billingLimit.isConfigured && showSelfServe) {
+    return OrgBannerKind.NoLimitConfigured;
+  }

Comment on lines 185 to 203
switch (action) {
case "environment-pause":
const pauseService = new PauseEnvironmentService();
await pauseService.call(environment, "paused");
{
const result = await pauseService.call(environment, "paused");
if (!result.success) {
return redirectWithErrorMessage(redirectPath, request, result.error);
}
}
return redirectWithSuccessMessage(redirectPath, request, "Environment paused");
case "environment-resume":
const resumeService = new PauseEnvironmentService();
await resumeService.call(environment, "resumed");
{
const result = await resumeService.call(environment, "resumed");
if (!result.success) {
return redirectWithErrorMessage(redirectPath, request, result.error);
}
}
return redirectWithSuccessMessage(redirectPath, request, "Environment resumed");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="apps/webapp/app/routes/_app.orgs.\$organizationSlug.projects.\$projectParam.env.\$envParam.queues/route.tsx"

# Show the affected switch segment with line numbers
nl -ba "$FILE" | sed -n '180,210p'

# Verify each case label is block-scoped
python - <<'PY'
from pathlib import Path
p = Path("apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx")
s = p.read_text()
print('case "environment-pause": {', 'case "environment-pause": {' in s)
print('case "environment-resume": {', 'case "environment-resume": {' in s)
PY

Repository: triggerdotdev/trigger.dev

Length of output: 112


🏁 Script executed:

#!/bin/bash
# Read the specific file and show lines around 185-203
FILE="apps/webapp/app/routes/_app.orgs.\$organizationSlug.projects.\$projectParam.env.\$envParam.queues/route.tsx"

# First, check if file exists
if [ -f "$FILE" ]; then
  echo "=== Lines 180-210 of file ==="
  sed -n '180,210p' "$FILE" | cat -n
else
  # Try to find the correct file path
  echo "File not found at exact path. Searching for similar files..."
  find . -type f -name "route.tsx" -path "*queues*" 2>/dev/null | head -5
fi

Repository: triggerdotdev/trigger.dev

Length of output: 1456


Wrap environment-pause / environment-resume case bodies in braces.

Line 185 (and 194) violate noSwitchDeclarations; the const pauseService and const resumeService declarations are unscoped in case clauses, risking scope leakage to other cases. Compare with the queue-pause/queue-resume cases below, which correctly wrap their bodies in braces. The existing nested braces should be removed and replaced with a single outer block brace on the case label.

🧰 Tools
🪛 Biome (2.5.0)

[error] 187-187: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

(lint/correctness/noSwitchDeclarations)


[error] 196-196: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

(lint/correctness/noSwitchDeclarations)

Source: Linters/SAST tools

Comment on lines +129 to +136
const firstDayOfMonth = new Date();
firstDayOfMonth.setUTCDate(1);
firstDayOfMonth.setUTCHours(0, 0, 0, 0);

const firstDayOfNextMonth = new Date();
firstDayOfNextMonth.setUTCMonth(firstDayOfNextMonth.getUTCMonth() + 1);
firstDayOfNextMonth.setUTCDate(1);
firstDayOfNextMonth.setUTCHours(0, 0, 0, 0);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fix next-month boundary calculation order.

Line 134 mutates month before normalizing day; on month-end dates this can overflow into the wrong month and skew the usage window. Normalize day first, then advance month.

💡 Suggested fix
-  const firstDayOfNextMonth = new Date();
-  firstDayOfNextMonth.setUTCMonth(firstDayOfNextMonth.getUTCMonth() + 1);
-  firstDayOfNextMonth.setUTCDate(1);
-  firstDayOfNextMonth.setUTCHours(0, 0, 0, 0);
+  const firstDayOfNextMonth = new Date();
+  firstDayOfNextMonth.setUTCDate(1);
+  firstDayOfNextMonth.setUTCHours(0, 0, 0, 0);
+  firstDayOfNextMonth.setUTCMonth(firstDayOfNextMonth.getUTCMonth() + 1);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const firstDayOfMonth = new Date();
firstDayOfMonth.setUTCDate(1);
firstDayOfMonth.setUTCHours(0, 0, 0, 0);
const firstDayOfNextMonth = new Date();
firstDayOfNextMonth.setUTCMonth(firstDayOfNextMonth.getUTCMonth() + 1);
firstDayOfNextMonth.setUTCDate(1);
firstDayOfNextMonth.setUTCHours(0, 0, 0, 0);
const firstDayOfMonth = new Date();
firstDayOfMonth.setUTCDate(1);
firstDayOfMonth.setUTCHours(0, 0, 0, 0);
const firstDayOfNextMonth = new Date();
firstDayOfNextMonth.setUTCDate(1);
firstDayOfNextMonth.setUTCHours(0, 0, 0, 0);
firstDayOfNextMonth.setUTCMonth(firstDayOfNextMonth.getUTCMonth() + 1);

Comment on lines +39 to +42
const completion = await complete(pending.organizationId);
if (!completion) {
throw new Error("Billing platform client unavailable");
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

complete() result is not validated, so unresolved orgs can be treated as resolved.

At Line 40, only undefined is treated as failure. { completed: false } currently passes and won’t be retried.

Proposed fix
-      const completion = await complete(pending.organizationId);
-      if (!completion) {
+      const completion = await complete(pending.organizationId);
+      if (!completion || completion.completed !== true) {
         throw new Error("Billing platform client unavailable");
       }

Comment on lines +26 to +29
const billingLimit = deps.getBillingLimit
? await deps.getBillingLimit(organizationId)
: await (await import("~/services/platform.v3.server")).getBillingLimit(organizationId);
const targetState = resolveConvergeTargetFromBillingLimit(billingLimit);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add a resilient fallback for billing-limit lookup failures.

Line 26-29 makes env/branch creation availability dependent on a platform lookup. A transient platform failure currently hard-fails these paths instead of degrading gracefully.

Suggested fix
+import { logger } from "~/services/logger.server";
@@
-  const billingLimit = deps.getBillingLimit
-    ? await deps.getBillingLimit(organizationId)
-    : await (await import("~/services/platform.v3.server")).getBillingLimit(organizationId);
+  let billingLimit: BillingLimitResult | undefined;
+  try {
+    billingLimit = deps.getBillingLimit
+      ? await deps.getBillingLimit(organizationId)
+      : await (await import("~/services/platform.v3.server")).getBillingLimit(organizationId);
+  } catch (error) {
+    logger.error("Failed to fetch billing limit for initial env pause state", {
+      organizationId,
+      error,
+    });
+    return { paused: false, pauseSource: null };
+  }

return;
}

await updateEnvConcurrencyLimits(environment, 0);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make post-create billing pause application non-fatal.

Line 48 throws directly from a post-create side-effect. Callers can end up with a created environment but a failed request response, which is a partial-success failure mode.

Suggested fix
+import { logger } from "~/services/logger.server";
@@
-  await updateEnvConcurrencyLimits(environment, 0);
+  try {
+    await updateEnvConcurrencyLimits(environment, 0);
+  } catch (error) {
+    logger.error("Failed to apply billing-limit pause after env create", {
+      environmentId: environment.id,
+      organizationId: environment.organizationId,
+      error,
+    });
+  }

Comment on lines +61 to +71
for (const target of targets) {
await reconcileTarget(target, {
bustCaches,
enqueueConverge,
});
}

await clearProcessedQueue(
queuedOrgIds,
targets.map((target) => target.organizationId)
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

One reconcile failure aborts the entire tick and leaves queue cleanup skipped.

At Line 62, a thrown error from one org stops processing remaining targets and clearProcessedQueue() never runs. Isolate failures per target and only clear successfully processed org IDs.

Suggested pattern
-  for (const target of targets) {
-    await reconcileTarget(target, {
-      bustCaches,
-      enqueueConverge,
-    });
-  }
-
-  await clearProcessedQueue(
-    queuedOrgIds,
-    targets.map((target) => target.organizationId)
-  );
+  const processedOrgIds: string[] = [];
+  for (const target of targets) {
+    try {
+      await reconcileTarget(target, { bustCaches, enqueueConverge });
+      processedOrgIds.push(target.organizationId);
+    } catch (error) {
+      // log and continue
+    }
+  }
+
+  await clearProcessedQueue(queuedOrgIds, processedOrgIds);

Comment on lines +44 to +49
const runtimeEnvironment = await this._prisma.runtimeEnvironment.findFirst({
where: { id: environment.id },
select: {
pauseSource: true,
},
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make pause-source enforcement atomic to avoid a race.

The check at Line 44 and the write at Line 71 are separate operations. A concurrent billing-limit pause can occur between them, allowing a manual resume to clear pauseSource incorrectly.

Suggested patch (atomic resume write)
-      await this._prisma.runtimeEnvironment.update({
-        where: {
-          id: environment.id,
-        },
-        data: {
-          paused: action === "paused",
-          pauseSource: action === "resumed" ? null : undefined,
-        },
-      });
+      if (action === "resumed") {
+        const resumed = await this._prisma.runtimeEnvironment.updateMany({
+          where: {
+            id: environment.id,
+            NOT: { pauseSource: "BILLING_LIMIT" as any },
+          },
+          data: {
+            paused: false,
+            pauseSource: null,
+          },
+        });
+
+        if (resumed.count === 0) {
+          throw new Error(
+            "This environment is paused because your organization reached its billing limit. Resolve the limit on the billing limits settings page to resume."
+          );
+        }
+      } else {
+        await this._prisma.runtimeEnvironment.update({
+          where: { id: environment.id },
+          data: { paused: true },
+        });
+      }

Also applies to: 71-78

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant