Skip to content

feat(k8s): add placement tags for flexible node selection #2390

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 19 commits into from
Aug 15, 2025

Conversation

nicktrn
Copy link
Collaborator

@nicktrn nicktrn commented Aug 13, 2025

Enables runs to be scheduled on specific nodes based on configurable placement tags. The system uses k/v pairs, with billing information automatically converted to placement tags during dequeue.

Environment Variables

Supervisor

  • PLACEMENT_TAGS_ENABLED=false - Enable placement tags scheduling (default: false)
  • PLACEMENT_TAGS_PREFIX=node.cluster.x-k8s.io - Kubernetes label prefix for placement tags (default shown)
  • BATCH_TRIGGER_CACHED_RUNS_CHECK_ENABLED - Whether to perform potentially expensive checks (default: false)

Backwards Compatibility

All placement tag scheduling is disabled by default and schema changes are optional.

Copy link

changeset-bot bot commented Aug 13, 2025

🦋 Changeset detected

Latest commit: 3af19ab

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

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

Copy link
Contributor

coderabbitai bot commented Aug 13, 2025

Walkthrough

Adds placement tag support and surfaces placementTags through the dequeue→supervisor→Kubernetes workload path, including a PlacementTag schema and placementTags on DequeuedMessage. Introduces billing/plan awareness: a new BillingCache (memory + Redis), billing options on RunEngine, invalidateBillingCache API, and planType threaded into triggers and TaskRun records. DequeueSystem reads billing via BillingCache and emits a paid placement tag. Adds TaskRun.planType column and migration. Webapp and validator layers gain plan-aware entitlement, queue-limit checks, and an invalidateBillingCache propagation point. Several env and BoolEnv typings adjusted.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~90 minutes


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between d5a1467 and 3af19ab.

📒 Files selected for processing (1)
  • packages/core/src/v3/schemas/runEngine.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/core/src/v3/schemas/runEngine.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
  • GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
  • GitHub Check: typecheck / typecheck
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
  • GitHub Check: Analyze (javascript-typescript)
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/tier-scheduling

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (8)
apps/webapp/app/v3/runEngine.server.ts (1)

108-122: Simplify billing mapping; single expression keeps behavior identical

You can collapse the multi-branch into a single expression while preserving semantics.

     billing: {
-      getCurrentPlan: async (orgId: string) => {
-        const plan = await getCurrentPlan(orgId);
-
-        if (!plan) {
-          return { isPaying: false };
-        }
-
-        if (!plan.v3Subscription) {
-          return { isPaying: false };
-        }
-
-        return { isPaying: plan.v3Subscription.isPaying };
-      },
+      getCurrentPlan: async (orgId: string) => {
+        const plan = await getCurrentPlan(orgId);
+        return { isPaying: !!plan?.v3Subscription?.isPaying };
+      },
     },
apps/supervisor/src/workloadManager/kubernetes.ts (2)

49-66: Guard against misconfiguration and improve debuggability in tier scheduling

If tier scheduling is enabled but labelKey is empty/misconfigured, this will set an empty nodeSelector key. Also, adding a log helps with troubleshooting which tier value was applied.

Consider:

   private addTierScheduling(
     podSpec: Omit<k8s.V1PodSpec, "containers">,
     isPaidTier: boolean
   ): Omit<k8s.V1PodSpec, "containers"> {
     if (!this.tierConfig.enabled) {
       return podSpec;
     }
 
+    if (!this.tierConfig.labelKey) {
+      this.logger.warn("[KubernetesWorkloadManager] Tier scheduling enabled but TIER_LABEL_KEY is empty; skipping tier nodeSelector");
+      return podSpec;
+    }
+
     const labelValue = isPaidTier ? this.tierConfig.paidValue : this.tierConfig.freeValue;
 
+    this.logger.debug("[KubernetesWorkloadManager] Applying tier nodeSelector", {
+      labelKey: this.tierConfig.labelKey,
+      labelValue,
+      isPaidTier,
+    });
+
     return {
       ...podSpec,
       nodeSelector: {
         ...podSpec.nodeSelector,
         [this.tierConfig.labelKey]: labelValue,
       },
     };
   }

29-33: Initialize and validate tier config once; emit a single startup log

On construction, log whether tier scheduling is enabled and its config (sanitized). This helps ops quickly validate environment configuration at startup.

   constructor(private opts: WorkloadManagerOptions) {
     this.k8s = createK8sApi();
     this.tierConfig = this.tierSchedulingConfig;
+
+    if (this.tierConfig.enabled) {
+      this.logger.log("[KubernetesWorkloadManager] Tier scheduling enabled", {
+        labelKey: this.tierConfig.labelKey,
+        freeValue: this.tierConfig.freeValue,
+        paidValue: this.tierConfig.paidValue,
+      });
+      if (!this.tierConfig.labelKey) {
+        this.logger.warn("[KubernetesWorkloadManager] TIER_LABEL_KEY is empty; tier scheduling will be skipped");
+      }
+    } else {
+      this.logger.log("[KubernetesWorkloadManager] Tier scheduling disabled");
+    }
apps/webapp/app/services/platform.v3.server.ts (1)

312-314: Make cache invalidation non-blocking and resilient to failures

These invalidations shouldn’t block the user-facing redirect. Wrap them to avoid throwing and to log failures without impacting UX.

-          // Invalidate billing cache since plan changed
-          opts?.invalidateBillingCache?.(organization.id);
+          // Invalidate billing cache since plan changed (non-blocking)
+          try {
+            opts?.invalidateBillingCache?.(organization.id);
+          } catch (e) {
+            logger.warn("Billing cache invalidation failed after plan change", {
+              organizationId: organization.id,
+              error: e instanceof Error ? e.message : String(e),
+            });
+          }
-        // Invalidate billing cache since subscription changed
-        opts?.invalidateBillingCache?.(organization.id);
+        // Invalidate billing cache since subscription changed (non-blocking)
+        try {
+          opts?.invalidateBillingCache?.(organization.id);
+        } catch (e) {
+          logger.warn("Billing cache invalidation failed after subscription update", {
+            organizationId: organization.id,
+            error: e instanceof Error ? e.message : String(e),
+          });
+        }
-        // Invalidate billing cache since subscription was canceled
-        opts?.invalidateBillingCache?.(organization.id);
+        // Invalidate billing cache since subscription was canceled (non-blocking)
+        try {
+          opts?.invalidateBillingCache?.(organization.id);
+        } catch (e) {
+          logger.warn("Billing cache invalidation failed after subscription cancelation", {
+            organizationId: organization.id,
+            error: e instanceof Error ? e.message : String(e),
+          });
+        }

Also applies to: 327-329, 336-338

internal-packages/run-engine/src/engine/index.ts (1)

1353-1359: Expose both fire-and-forget and awaitable invalidation APIs

The background invalidation is fine for most flows. Consider also exposing an awaitable variant (e.g., invalidateBillingCacheAsync returning a Promise) for admin/ops paths that need to guarantee freshness before proceeding.

Apply this diff to introduce an awaitable option without changing existing callers:

   /**
    * Invalidates the billing cache for an organization when their plan changes
    * Runs in background and handles all errors internally
    */
   invalidateBillingCache(orgId: string): void {
     this.dequeueSystem.invalidateBillingCache(orgId);
   }
+
+  /**
+   * Awaitable variant of invalidateBillingCache for flows that must ensure freshness
+   */
+  async invalidateBillingCacheAsync(orgId: string): Promise<void> {
+    return this.dequeueSystem.invalidateBillingCacheAsync?.(orgId) ?? Promise.resolve();
+  }
internal-packages/run-engine/src/engine/systems/dequeueSystem.ts (3)

34-37: Make TTLs configurable; consider shorter stale TTL or rely on invalidation

5/10-minute fresh/stale TTLs are reasonable, but since you now have explicit invalidation, consider:

  • Exposing these TTLs via RunEngineOptions.billing?.cacheTtl to tune in ops.
  • Shortening stale TTL (e.g., equal to fresh) to reduce maximum drift when invalidation isn’t triggered.

Apply this diff to accept optional overrides without breaking defaults:

-const BILLING_FRESH_TTL = 60000 * 5; // 5 minutes
-const BILLING_STALE_TTL = 60000 * 10; // 10 minutes
+const DEFAULT_BILLING_FRESH_TTL = 60000 * 5; // 5 minutes
+const DEFAULT_BILLING_STALE_TTL = 60000 * 10; // 10 minutes

And inside the constructor (see lines 63-69), compute:

-    this.billingCache = createCache({
-      billing: new Namespace<{ isPaying: boolean }>(ctx, {
-        stores: [memory, redisCacheStore],
-        fresh: BILLING_FRESH_TTL,
-        stale: BILLING_STALE_TTL,
-      }),
-    });
+    const freshTtl =
+      (this.options.billing as any)?.cacheTtl?.freshMs ?? DEFAULT_BILLING_FRESH_TTL;
+    const staleTtl =
+      (this.options.billing as any)?.cacheTtl?.staleMs ?? DEFAULT_BILLING_STALE_TTL;
+    this.billingCache = createCache({
+      billing: new Namespace<{ isPaying: boolean }>(ctx, {
+        stores: [memory, redisCacheStore],
+        fresh: freshTtl,
+        stale: staleTtl,
+      }),
+    });

72-83: Good: fire-and-forget invalidation with error swallowing

This matches the “background” contract. For flows that need to wait for invalidation, consider adding an awaitable variant:

   invalidateBillingCache(orgId: string): void {
     this.billingCache.billing.remove(orgId).catch((error) => {
       this.$.logger.warn("Failed to invalidate billing cache", {
         orgId,
         error: error.message,
       });
     });
   }
+
+  // Optional: awaitable variant
+  async invalidateBillingCacheAsync(orgId: string): Promise<void> {
+    try {
+      await this.billingCache.billing.remove(orgId);
+    } catch (error) {
+      this.$.logger.warn("Failed to invalidate billing cache", {
+        orgId,
+        error: error instanceof Error ? error.message : String(error),
+      });
+    }
+  }

1-704: Add minimal tests for billing cache behavior and fallbacks

Recommend unit/integration tests for:

  • Default behavior when billing config is absent (should be isPaying=false).
  • SWR cache hit path (no extra billing calls).
  • Invalidation path (remove -> next call fetches).
  • Error path (billing throws/returns empty -> falls back to false, no throw).

I can scaffold Jest tests for DequeueSystem with a mock billing.getCurrentPlan, plus a fake Redis store to validate remove/SWR behavior. Want me to push a test file for this?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6f29b7d and 021591f.

📒 Files selected for processing (12)
  • .changeset/proud-nails-grin.md (1 hunks)
  • apps/supervisor/src/env.ts (1 hunks)
  • apps/supervisor/src/index.ts (1 hunks)
  • apps/supervisor/src/workloadManager/kubernetes.ts (3 hunks)
  • apps/supervisor/src/workloadManager/types.ts (1 hunks)
  • apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (2 hunks)
  • apps/webapp/app/services/platform.v3.server.ts (3 hunks)
  • apps/webapp/app/v3/runEngine.server.ts (2 hunks)
  • internal-packages/run-engine/src/engine/index.ts (2 hunks)
  • internal-packages/run-engine/src/engine/systems/dequeueSystem.ts (5 hunks)
  • internal-packages/run-engine/src/engine/types.ts (1 hunks)
  • packages/core/src/v3/schemas/runEngine.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations

Files:

  • apps/webapp/app/services/platform.v3.server.ts
  • apps/supervisor/src/workloadManager/types.ts
  • apps/supervisor/src/env.ts
  • internal-packages/run-engine/src/engine/types.ts
  • packages/core/src/v3/schemas/runEngine.ts
  • apps/supervisor/src/workloadManager/kubernetes.ts
  • apps/supervisor/src/index.ts
  • apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx
  • internal-packages/run-engine/src/engine/index.ts
  • apps/webapp/app/v3/runEngine.server.ts
  • internal-packages/run-engine/src/engine/systems/dequeueSystem.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)

We use zod a lot in packages/core and in the webapp

Files:

  • apps/webapp/app/services/platform.v3.server.ts
  • packages/core/src/v3/schemas/runEngine.ts
  • apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx
  • apps/webapp/app/v3/runEngine.server.ts
apps/webapp/**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/webapp.mdc)

apps/webapp/**/*.{ts,tsx}: In the webapp, all environment variables must be accessed through the env export of env.server.ts, instead of directly accessing process.env.
When importing from @trigger.dev/core in the webapp, never import from the root @trigger.dev/core path; always use one of the subpath exports as defined in the package's package.json.

Files:

  • apps/webapp/app/services/platform.v3.server.ts
  • apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx
  • apps/webapp/app/v3/runEngine.server.ts
apps/webapp/app/services/**/*.server.ts

📄 CodeRabbit Inference Engine (.cursor/rules/webapp.mdc)

For testable services, separate service logic and configuration, as exemplified by realtimeClient.server.ts (service) and realtimeClientGlobal.server.ts (configuration).

Files:

  • apps/webapp/app/services/platform.v3.server.ts
🧬 Code Graph Analysis (7)
apps/webapp/app/services/platform.v3.server.ts (2)
internal-packages/run-engine/src/engine/systems/dequeueSystem.ts (2)
  • orgId (572-670)
  • orgId (672-703)
apps/webapp/app/models/message.server.ts (1)
  • redirectWithSuccessMessage (162-179)
apps/supervisor/src/env.ts (1)
apps/supervisor/src/envUtil.ts (1)
  • BoolEnv (6-12)
internal-packages/run-engine/src/engine/types.ts (1)
internal-packages/run-engine/src/engine/systems/dequeueSystem.ts (2)
  • orgId (572-670)
  • orgId (672-703)
apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (2)
apps/webapp/app/services/platform.v3.server.ts (1)
  • setPlan (284-349)
apps/webapp/app/v3/runEngine.server.ts (1)
  • engine (9-9)
internal-packages/run-engine/src/engine/index.ts (1)
internal-packages/run-engine/src/engine/systems/dequeueSystem.ts (2)
  • orgId (572-670)
  • orgId (672-703)
apps/webapp/app/v3/runEngine.server.ts (2)
internal-packages/run-engine/src/engine/systems/dequeueSystem.ts (2)
  • orgId (572-670)
  • orgId (672-703)
apps/webapp/app/services/platform.v3.server.ts (1)
  • getCurrentPlan (184-219)
internal-packages/run-engine/src/engine/systems/dequeueSystem.ts (5)
internal-packages/run-engine/src/engine/systems/systems.ts (1)
  • SystemResources (10-20)
internal-packages/run-engine/src/engine/types.ts (1)
  • RunEngineOptions (18-76)
internal-packages/redis/src/index.ts (1)
  • RedisOptions (4-4)
internal-packages/cache/src/index.ts (3)
  • DefaultStatefulContext (3-3)
  • MemoryStore (7-7)
  • Namespace (4-4)
internal-packages/run-engine/src/engine/errors.ts (1)
  • ServiceValidationError (59-68)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
  • GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
  • GitHub Check: typecheck / typecheck
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (16)
apps/supervisor/src/env.ts (1)

80-84: LGTM! Well-structured tier scheduling configuration.

The tier scheduling environment variables are properly defined with sensible defaults and appropriate types. The placement after Kubernetes storage settings is logical, and the default values align with the PR objectives.

.changeset/proud-nails-grin.md (1)

1-5: LGTM! Changeset correctly documents the feature.

The changeset appropriately categorizes this as a patch version change for @trigger.dev/core and clearly describes the feature being added.

apps/supervisor/src/index.ts (1)

250-250: LGTM! Proper billing tier extraction with safe fallback.

The implementation correctly extracts the billing tier information from the dequeued message and provides a safe fallback to false when billing information is unavailable. This aligns with the backwards compatibility requirement mentioned in the PR objectives.

apps/supervisor/src/workloadManager/types.ts (1)

35-36: LGTM! Clean interface extension for tier scheduling.

The isPaidTier field is appropriately optional and well-commented. The placement at the end of the interface maintains good organization.

internal-packages/run-engine/src/engine/types.ts (2)

33-35: LGTM! Well-designed billing API surface.

The billing configuration is appropriately optional and follows a clean functional interface pattern. The getCurrentPlan method signature is straightforward and focused on the essential isPaying boolean, which aligns perfectly with the tier scheduling requirements.


33-35: Billing cache invalidation is correctly wired

We’ve confirmed that cache invalidation is implemented and invoked on every plan change:

  • internal-packages/run-engine/src/engine/systems/dequeueSystem.ts
    • Defines invalidateBillingCache(orgId) which calls this.billingCache.billing.remove(orgId).
  • internal-packages/run-engine/src/engine/index.ts
    engine.invalidateBillingCache delegates to DequeueSystem.invalidateBillingCache.
  • apps/webapp/app/services/platform.v3.server.ts
    setPlan calls opts.invalidateBillingCache(organization.id) on new plan, update, and cancellation.
  • apps/webapp/app/routes/.../select-plan.tsx
    • Binds engine.invalidateBillingCache into setPlan so every user-driven plan change clears the cache.

No further changes needed.

packages/core/src/v3/schemas/runEngine.ts (1)

264-270: Schema extension LGTM; optional billing payload is safe and backward-compatible

Adding billing as an optional field on DequeuedMessage with currentPlan.isPaying is consistent with the rest of the zod schema and preserves version compatibility.

apps/supervisor/src/workloadManager/kubernetes.ts (1)

88-89: Correct integration point for tier scheduling

Using ...this.addTierScheduling(this.#defaultPodSpec, opts.isPaidTier ?? false) cleanly composes with the existing #defaultPodSpec (and merges with any prior nodeSelector). This is the right place to apply the node selector.

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

288-290: No changes needed: invalidateBillingCache is fire-and-forget and returns void
Confirmed that in both dequeueSystem.ts and engine/index.ts, invalidateBillingCache(orgId: string): void handles its internal Promise via .catch() and returns void. The existing type opts?: { invalidateBillingCache?: (orgId: string) => void } is correct.

internal-packages/run-engine/src/engine/index.ts (1)

300-302: Wiring billing config and cache Redis options into DequeueSystem looks correct

Passing billing and selecting Redis options via cache.redis fallback to runLock.redis is consistent with how other subsystems are initialized. Good separation via DequeueSystem.

internal-packages/run-engine/src/engine/systems/dequeueSystem.ts (6)

1-8: Cache primitives import and usage are appropriate

Using createCache with MemoryStore + RedisCacheStore is a good fit for cross-instance SWR semantics.


23-23: ServiceValidationError import aligns with existing error handling patterns

Consistent with other systems; good choice if you keep throwing on validation failures.


30-32: Constructor options extension is sound

Adding billing and redisOptions here keeps this system self-contained and testable.


42-45: Cache namespace typing is clear and minimal

Storing only isPaying keeps the cache payload small and purpose-built. LGTM.


51-70: Cache initialization is robust and production-ready

  • Memory + Redis stores with a namespaced prefix prevent key collisions.
  • useModernCacheKeyBuilder ensures stable keys across versions.

506-507: Embedding billing in DequeuedMessage is correct

The structure matches the updated schema and is wrapped with a satisfies DequeuedMessage check. Good safety net.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

🧹 Nitpick comments (2)
internal-packages/database/prisma/migrations/20250814092224_add_task_run_plan_type/migration.sql (1)

1-2: Consider using a Postgres enum for planType for data integrity

TEXT allows arbitrary values. If you adopt a Prisma enum, mirror it in SQL for runtime guarantees.

Here’s an enum-based migration (for a new migration or an amended one before merge):

-- CreateEnum
DO $$ BEGIN
  CREATE TYPE "PlanType" AS ENUM ('free', 'paid', 'enterprise');
EXCEPTION
  WHEN duplicate_object THEN null;
END $$;

-- AlterTable
ALTER TABLE "TaskRun" ADD COLUMN "planType" "PlanType";

If this migration has already shipped, create a new migration to convert TEXT -> enum safely.

internal-packages/run-engine/src/engine/billingCache.ts (1)

16-19: Make TTLs configurable via options for flexibility across environments.

Defaults are reasonable, but exposing overrides helps tune cache freshness for different deployments.

Apply:

 export type BillingCacheOptions = {
   billingOptions?: RunEngineOptions["billing"];
   redisOptions: RedisOptions;
   logger: Logger;
+  freshTtlMs?: number;
+  staleTtlMs?: number;
 };

And use the overrides:

-      currentPlan: new Namespace<BillingPlan>(ctx, {
+      currentPlan: new Namespace<BillingPlan>(ctx, {
         stores: [memory, redisCacheStore],
-        fresh: BILLING_FRESH_TTL,
-        stale: BILLING_STALE_TTL,
+        fresh: options.freshTtlMs ?? BILLING_FRESH_TTL,
+        stale: options.staleTtlMs ?? BILLING_STALE_TTL,
       }),
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these settings in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 021591f and 5d75706.

📒 Files selected for processing (8)
  • apps/webapp/app/v3/runEngine.server.ts (2 hunks)
  • internal-packages/cache/src/index.ts (1 hunks)
  • internal-packages/database/prisma/migrations/20250814092224_add_task_run_plan_type/migration.sql (1 hunks)
  • internal-packages/database/prisma/schema.prisma (1 hunks)
  • internal-packages/run-engine/src/engine/billingCache.ts (1 hunks)
  • internal-packages/run-engine/src/engine/index.ts (6 hunks)
  • internal-packages/run-engine/src/engine/systems/dequeueSystem.ts (4 hunks)
  • internal-packages/run-engine/src/engine/types.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • internal-packages/run-engine/src/engine/systems/dequeueSystem.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations

Files:

  • internal-packages/cache/src/index.ts
  • internal-packages/run-engine/src/engine/billingCache.ts
  • apps/webapp/app/v3/runEngine.server.ts
  • internal-packages/run-engine/src/engine/types.ts
  • internal-packages/run-engine/src/engine/index.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)

We use zod a lot in packages/core and in the webapp

Files:

  • apps/webapp/app/v3/runEngine.server.ts
apps/webapp/**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/webapp.mdc)

apps/webapp/**/*.{ts,tsx}: In the webapp, all environment variables must be accessed through the env export of env.server.ts, instead of directly accessing process.env.
When importing from @trigger.dev/core in the webapp, never import from the root @trigger.dev/core path; always use one of the subpath exports as defined in the package's package.json.

Files:

  • apps/webapp/app/v3/runEngine.server.ts
🧠 Learnings (6)
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : The `run` function contains your task logic in Trigger.dev tasks.

Applied to files:

  • internal-packages/run-engine/src/engine/index.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When triggering a task from backend code, use `tasks.trigger`, `tasks.batchTrigger`, or `tasks.triggerAndPoll` as shown in the examples.

Applied to files:

  • internal-packages/run-engine/src/engine/index.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When implementing scheduled (cron) tasks, use `schedules.task` from `trigger.dev/sdk/v3` and follow the shown patterns.

Applied to files:

  • internal-packages/run-engine/src/engine/index.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When triggering a task from inside another task, use `yourTask.trigger`, `yourTask.batchTrigger`, `yourTask.triggerAndWait`, `yourTask.batchTriggerAndWait`, `batch.triggerAndWait`, `batch.triggerByTask`, or `batch.triggerByTaskAndWait` as shown.

Applied to files:

  • internal-packages/run-engine/src/engine/index.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When using Realtime features, use the `runs.subscribeToRun`, `runs.subscribeToRunsWithTag`, and `runs.subscribeToBatch` APIs as shown.

Applied to files:

  • internal-packages/run-engine/src/engine/index.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When using metadata in tasks, use the `metadata` API as shown, and only inside run functions or task lifecycle hooks.

Applied to files:

  • internal-packages/run-engine/src/engine/index.ts
🧬 Code Graph Analysis (4)
internal-packages/run-engine/src/engine/billingCache.ts (3)
internal-packages/run-engine/src/engine/types.ts (1)
  • RunEngineOptions (18-76)
internal-packages/redis/src/index.ts (1)
  • RedisOptions (4-4)
internal-packages/cache/src/index.ts (6)
  • DefaultStatefulContext (3-3)
  • createCache (2-2)
  • Namespace (4-4)
  • Result (8-8)
  • CacheError (6-6)
  • Ok (8-8)
apps/webapp/app/v3/runEngine.server.ts (2)
internal-packages/run-engine/src/engine/billingCache.ts (1)
  • getCurrentPlan (67-78)
apps/webapp/app/services/platform.v3.server.ts (1)
  • getCurrentPlan (184-219)
internal-packages/run-engine/src/engine/types.ts (1)
internal-packages/run-engine/src/engine/billingCache.ts (1)
  • BillingPlan (20-23)
internal-packages/run-engine/src/engine/index.ts (2)
internal-packages/run-engine/src/engine/billingCache.ts (1)
  • BillingCache (31-92)
packages/core/src/v3/isomorphic/friendlyId.ts (1)
  • RunId (93-93)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
  • GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
  • GitHub Check: typecheck / typecheck
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (11)
internal-packages/cache/src/index.ts (2)

6-7: Re-exporting CacheError looks good

Type-only re-export avoids bundling runtime code and prevents circular deps.


8-8: No action required — @unkey/error is already declared in internal-packages/cache/package.json

Verified: internal-packages/cache/package.json contains "@unkey/error": "^0.2.0" (around line 10).

  • File to note: internal-packages/cache/package.json
internal-packages/run-engine/src/engine/types.ts (2)

16-16: Type-only import for BillingPlan is correct

Prevents runtime import and avoids potential cycles. Good choice.


33-35: Billing cache invalidation is already exposed — no change needed

RunEngine already exposes invalidateBillingCache(orgId) and the web app binds/uses it, so billing changes are being invalidated as intended.

  • internal-packages/run-engine/src/engine/index.ts:1391-1393 — invalidateBillingCache(orgId) { this.billingCache.invalidate(orgId); }
  • apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx:157 — invalidateBillingCache: engine.invalidateBillingCache.bind(engine)
  • apps/webapp/app/services/platform.v3.server.ts:289 (opts type includes invalidateBillingCache?) and calls at 313, 328, 337: opts?.invalidateBillingCache?.(organization.id)
apps/webapp/app/v3/runEngine.server.ts (1)

4-4: Import looks good

Using webapp service layer is consistent with our env and platform access patterns.

internal-packages/run-engine/src/engine/index.ts (4)

1-1: Importing BillingCache is the right abstraction boundary.

Clean separation of billing concerns from the engine logic.


82-83: Private billing cache field scoped to the engine — good encapsulation.

Keeps the billing cache internal to RunEngine.


298-303: BillingCache initialization uses shared Redis options — sensible default.

Good choice to reuse cache?.redis falling back to runLock.redis.


1387-1394: invalidateBillingCache is a useful hook for plan changes.

Good to expose this publicly for UI/actions to call post-plan updates.

internal-packages/run-engine/src/engine/billingCache.ts (2)

67-78: Graceful fallback to free plan and SWR fetch are solid.

  • Returning Ok(free) when no provider is configured matches the rollout plan.
  • Using memory + Redis stores with SWR is a good balance between latency and accuracy.

84-91: Non-throwing invalidate with warning logging is appropriate.

This avoids surfacing transient Redis errors to callers during plan changes.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these settings in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 5d75706 and 2435f39.

📒 Files selected for processing (2)
  • internal-packages/run-engine/src/engine/index.ts (6 hunks)
  • internal-packages/run-engine/src/engine/systems/dequeueSystem.ts (4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations

Files:

  • internal-packages/run-engine/src/engine/systems/dequeueSystem.ts
  • internal-packages/run-engine/src/engine/index.ts
🧠 Learnings (5)
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : The `run` function contains your task logic in Trigger.dev tasks.

Applied to files:

  • internal-packages/run-engine/src/engine/index.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When triggering a task from backend code, use `tasks.trigger`, `tasks.batchTrigger`, or `tasks.triggerAndPoll` as shown in the examples.

Applied to files:

  • internal-packages/run-engine/src/engine/index.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When implementing scheduled (cron) tasks, use `schedules.task` from `trigger.dev/sdk/v3` and follow the shown patterns.

Applied to files:

  • internal-packages/run-engine/src/engine/index.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When triggering a task from inside another task, use `yourTask.trigger`, `yourTask.batchTrigger`, `yourTask.triggerAndWait`, `yourTask.batchTriggerAndWait`, `batch.triggerAndWait`, `batch.triggerByTask`, or `batch.triggerByTaskAndWait` as shown.

Applied to files:

  • internal-packages/run-engine/src/engine/index.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When using Realtime features, use the `runs.subscribeToRun`, `runs.subscribeToRunsWithTag`, and `runs.subscribeToBatch` APIs as shown.

Applied to files:

  • internal-packages/run-engine/src/engine/index.ts
🧬 Code Graph Analysis (1)
internal-packages/run-engine/src/engine/index.ts (3)
internal-packages/run-engine/src/engine/billingCache.ts (1)
  • BillingCache (31-92)
internal-packages/run-engine/src/engine/systems/dequeueSystem.ts (1)
  • orgId (547-645)
packages/core/src/v3/isomorphic/friendlyId.ts (1)
  • RunId (93-93)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
  • GitHub Check: typecheck / typecheck
  • GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
internal-packages/run-engine/src/engine/systems/dequeueSystem.ts (2)

21-21: Wiring BillingCache into DequeueSystemOptions looks correct

Type-only import and option threading are clean and avoid value import in this module. No concerns.


477-481: DequeuedMessage.billing included — confirmed

Confirmed: packages/core/src/v3/schemas/runEngine.ts defines billing.currentPlan.isPaying (z.boolean()) inside DequeuedMessage (lines ~264–269). No schema changes required.

internal-packages/run-engine/src/engine/index.ts (4)

298-303: BillingCache instantiation is sound and single-sourced

Good choice to centralize billing caching here with fallback to cache.redis or runLock.redis. Logger injection ensures internal errors are observable without throwing.


309-310: Passing BillingCache into DequeueSystem is correct

This cleanly scopes billing lookups to the dequeue path without leaking concerns into unrelated systems.


377-401: Trigger path: non-fatal billing lookup with observability — LGTM

The trigger flow logs and proceeds when billing lookup fails, preserving backward compatibility. This aligns with the PR objective to avoid blocking on billing failures.


1386-1393: Public cache invalidation hook — good addition

Simple, non-throwing invalidation aligns with the cache’s internal error handling and enables UI hooks to refresh billing state.

@nicktrn nicktrn changed the title feat(k8s): add billing-aware run scheduling with cached lookups feat(k8s): add placement tags for flexible node selection Aug 14, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

♻️ Duplicate comments (1)
internal-packages/run-engine/src/engine/systems/dequeueSystem.ts (1)

385-407: Non-fatal billing lookup with safe fallback looks good

Graceful handling of billing cache errors/misses with a fallback to TaskRun.planType keeps dequeue robust. The corrected boolean (lockedTaskRun.planType ?? "free") !== "free" also fixes the prior undefined/null misclassification.

🧹 Nitpick comments (2)
internal-packages/run-engine/src/engine/systems/dequeueSystem.ts (1)

388-407: Add tracing attribute for billing decision (observability nit)

Consider recording the resolved paying state on the span to help trace scheduling decisions downstream.

-              } else {
-                isPaying = billingResult.val.isPaying;
-              }
+              } else {
+                isPaying = billingResult.val.isPaying;
+              }
+              // Record the resolved billing state in tracing
+              try {
+                span.setAttribute("billing.isPaying", isPaying);
+              } catch {}
apps/webapp/app/runEngine/services/batchTrigger.server.ts (1)

805-806: Narrow idempotencyKey to non-null for Map keys (type-safety)

Although the query filters to non-null keys, Prisma types the field as string | null. Narrow before constructing the map to avoid null keys in types.

-    const cachedRunsMap = new Map(cachedRuns.map((run) => [run.idempotencyKey, run]));
+    const cachedRunsMap = new Map(
+      cachedRuns
+        .filter((r): r is typeof r & { idempotencyKey: string } => Boolean(r.idempotencyKey))
+        .map((run) => [run.idempotencyKey, run] as const)
+    );
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these settings in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between cb71249 and ca65e2b.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (18)
  • .changeset/proud-nails-grin.md (1 hunks)
  • apps/supervisor/src/env.ts (1 hunks)
  • apps/supervisor/src/index.ts (1 hunks)
  • apps/supervisor/src/workloadManager/kubernetes.ts (4 hunks)
  • apps/supervisor/src/workloadManager/types.ts (2 hunks)
  • apps/webapp/app/env.server.ts (1 hunks)
  • apps/webapp/app/runEngine/concerns/queues.server.ts (1 hunks)
  • apps/webapp/app/runEngine/services/batchTrigger.server.ts (23 hunks)
  • apps/webapp/app/runEngine/services/triggerTask.server.ts (2 hunks)
  • apps/webapp/app/runEngine/types.ts (3 hunks)
  • apps/webapp/app/runEngine/validators/triggerTaskValidator.ts (3 hunks)
  • apps/webapp/app/services/platform.v3.server.ts (5 hunks)
  • apps/webapp/app/v3/services/triggerTask.server.ts (1 hunks)
  • apps/webapp/package.json (1 hunks)
  • internal-packages/run-engine/src/engine/index.ts (6 hunks)
  • internal-packages/run-engine/src/engine/systems/dequeueSystem.ts (4 hunks)
  • internal-packages/run-engine/src/engine/types.ts (3 hunks)
  • packages/core/src/v3/schemas/runEngine.ts (2 hunks)
✅ Files skipped from review due to trivial changes (1)
  • apps/webapp/package.json
🚧 Files skipped from review as they are similar to previous changes (7)
  • apps/supervisor/src/workloadManager/kubernetes.ts
  • apps/supervisor/src/env.ts
  • packages/core/src/v3/schemas/runEngine.ts
  • apps/supervisor/src/workloadManager/types.ts
  • internal-packages/run-engine/src/engine/types.ts
  • .changeset/proud-nails-grin.md
  • apps/supervisor/src/index.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations

Files:

  • apps/webapp/app/v3/services/triggerTask.server.ts
  • apps/webapp/app/runEngine/concerns/queues.server.ts
  • apps/webapp/app/env.server.ts
  • apps/webapp/app/runEngine/types.ts
  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/webapp/app/runEngine/validators/triggerTaskValidator.ts
  • internal-packages/run-engine/src/engine/systems/dequeueSystem.ts
  • apps/webapp/app/services/platform.v3.server.ts
  • apps/webapp/app/runEngine/services/batchTrigger.server.ts
  • internal-packages/run-engine/src/engine/index.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)

We use zod a lot in packages/core and in the webapp

Files:

  • apps/webapp/app/v3/services/triggerTask.server.ts
  • apps/webapp/app/runEngine/concerns/queues.server.ts
  • apps/webapp/app/env.server.ts
  • apps/webapp/app/runEngine/types.ts
  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/webapp/app/runEngine/validators/triggerTaskValidator.ts
  • apps/webapp/app/services/platform.v3.server.ts
  • apps/webapp/app/runEngine/services/batchTrigger.server.ts
apps/webapp/**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/webapp.mdc)

apps/webapp/**/*.{ts,tsx}: In the webapp, all environment variables must be accessed through the env export of env.server.ts, instead of directly accessing process.env.
When importing from @trigger.dev/core in the webapp, never import from the root @trigger.dev/core path; always use one of the subpath exports as defined in the package's package.json.

Files:

  • apps/webapp/app/v3/services/triggerTask.server.ts
  • apps/webapp/app/runEngine/concerns/queues.server.ts
  • apps/webapp/app/env.server.ts
  • apps/webapp/app/runEngine/types.ts
  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/webapp/app/runEngine/validators/triggerTaskValidator.ts
  • apps/webapp/app/services/platform.v3.server.ts
  • apps/webapp/app/runEngine/services/batchTrigger.server.ts
apps/webapp/app/services/**/*.server.ts

📄 CodeRabbit Inference Engine (.cursor/rules/webapp.mdc)

For testable services, separate service logic and configuration, as exemplified by realtimeClient.server.ts (service) and realtimeClientGlobal.server.ts (configuration).

Files:

  • apps/webapp/app/services/platform.v3.server.ts
🧠 Learnings (16)
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : ALWAYS generate Trigger.dev tasks using the `task` function from `trigger.dev/sdk/v3` and export them as shown in the correct pattern.

Applied to files:

  • apps/webapp/app/v3/services/triggerTask.server.ts
  • apps/webapp/app/runEngine/types.ts
  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/webapp/app/runEngine/validators/triggerTaskValidator.ts
  • apps/webapp/app/runEngine/services/batchTrigger.server.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When implementing scheduled (cron) tasks, use `schedules.task` from `trigger.dev/sdk/v3` and follow the shown patterns.

Applied to files:

  • apps/webapp/app/v3/services/triggerTask.server.ts
  • apps/webapp/app/runEngine/types.ts
  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/webapp/app/runEngine/services/batchTrigger.server.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When triggering a task from backend code, use `tasks.trigger`, `tasks.batchTrigger`, or `tasks.triggerAndPoll` as shown in the examples.

Applied to files:

  • apps/webapp/app/v3/services/triggerTask.server.ts
  • apps/webapp/app/runEngine/types.ts
  • apps/webapp/app/runEngine/services/batchTrigger.server.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When implementing schema tasks, use `schemaTask` from `trigger.dev/sdk/v3` and validate payloads as shown.

Applied to files:

  • apps/webapp/app/v3/services/triggerTask.server.ts
  • apps/webapp/app/runEngine/types.ts
  • apps/webapp/app/runEngine/services/triggerTask.server.ts
  • apps/webapp/app/runEngine/validators/triggerTaskValidator.ts
  • apps/webapp/app/runEngine/services/batchTrigger.server.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : You MUST use `trigger.dev/sdk/v3` when writing Trigger.dev tasks.

Applied to files:

  • apps/webapp/app/v3/services/triggerTask.server.ts
  • apps/webapp/app/runEngine/types.ts
  • apps/webapp/app/runEngine/services/batchTrigger.server.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Tasks must be exported, even subtasks in the same file.

Applied to files:

  • apps/webapp/app/v3/services/triggerTask.server.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : The `run` function contains your task logic in Trigger.dev tasks.

Applied to files:

  • apps/webapp/app/runEngine/types.ts
  • apps/webapp/app/runEngine/services/batchTrigger.server.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When using retry, queue, machine, or maxDuration options, configure them as shown in the examples for Trigger.dev tasks.

Applied to files:

  • apps/webapp/app/runEngine/types.ts
  • apps/webapp/app/runEngine/validators/triggerTaskValidator.ts
  • apps/webapp/app/runEngine/services/batchTrigger.server.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When triggering a task from inside another task, use `yourTask.trigger`, `yourTask.batchTrigger`, `yourTask.triggerAndWait`, `yourTask.batchTriggerAndWait`, `batch.triggerAndWait`, `batch.triggerByTask`, or `batch.triggerByTaskAndWait` as shown.

Applied to files:

  • apps/webapp/app/runEngine/types.ts
  • apps/webapp/app/runEngine/services/batchTrigger.server.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : You MUST `export` every task, including subtasks, in Trigger.dev task files.

Applied to files:

  • apps/webapp/app/runEngine/types.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When using metadata in tasks, use the `metadata` API as shown, and only inside run functions or task lifecycle hooks.

Applied to files:

  • apps/webapp/app/runEngine/types.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Each task needs a unique ID within your project.

Applied to files:

  • apps/webapp/app/runEngine/types.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : If you are able to generate an example payload for a task, do so.

Applied to files:

  • apps/webapp/app/runEngine/types.ts
📚 Learning: 2025-08-14T10:09:02.501Z
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2390
File: internal-packages/run-engine/src/engine/index.ts:466-467
Timestamp: 2025-08-14T10:09:02.501Z
Learning: In the triggerdotdev/trigger.dev codebase, it's acceptable to pass `string | undefined` types directly to Prisma operations (both create and update). The codebase consistently uses this pattern and the team is comfortable with how Prisma handles undefined values.

Applied to files:

  • internal-packages/run-engine/src/engine/systems/dequeueSystem.ts
  • internal-packages/run-engine/src/engine/index.ts
📚 Learning: 2025-05-12T17:23:05.574Z
Learnt from: matt-aitken
PR: triggerdotdev/trigger.dev#2035
File: apps/webapp/app/v3/services/finalizeTaskRun.server.ts:99-103
Timestamp: 2025-05-12T17:23:05.574Z
Learning: Prisma ignores undefined values in update operations. When a field has the value `undefined` in a Prisma update operation's data object, Prisma will automatically exclude that field from the update query rather than trying to set it to null. This means there's no need to manually build update objects to filter out undefined values.

Applied to files:

  • internal-packages/run-engine/src/engine/index.ts
📚 Learning: 2025-05-12T17:23:05.574Z
Learnt from: matt-aitken
PR: triggerdotdev/trigger.dev#2035
File: apps/webapp/app/v3/services/finalizeTaskRun.server.ts:99-103
Timestamp: 2025-05-12T17:23:05.574Z
Learning: Prisma ignores undefined values in update operations. When a field is set to undefined in a Prisma update operation's data object, Prisma will omit that field from the update rather than trying to set it to null or another value. This means there's no need to conditionally build update objects to exclude undefined values.

Applied to files:

  • internal-packages/run-engine/src/engine/index.ts
🧬 Code Graph Analysis (6)
apps/webapp/app/runEngine/concerns/queues.server.ts (4)
internal-packages/run-engine/src/engine/tests/setup.ts (1)
  • AuthenticatedEnvironment (17-19)
internal-packages/run-engine/src/index.ts (1)
  • AuthenticatedEnvironment (4-4)
apps/webapp/app/runEngine/types.ts (1)
  • QueueValidationResult (38-46)
apps/webapp/app/v3/queueSizeLimits.server.ts (1)
  • guardQueueSizeLimitsForEnv (11-34)
apps/webapp/app/env.server.ts (1)
apps/webapp/app/utils/boolEnv.ts (1)
  • BoolEnv (3-9)
apps/webapp/app/runEngine/validators/triggerTaskValidator.ts (1)
apps/webapp/app/runEngine/types.ts (2)
  • EntitlementValidationParams (87-89)
  • EntitlementValidationResult (111-119)
internal-packages/run-engine/src/engine/systems/dequeueSystem.ts (2)
internal-packages/run-engine/src/engine/billingCache.ts (1)
  • BillingCache (31-92)
packages/core/src/v3/schemas/runEngine.ts (1)
  • placementTag (234-236)
apps/webapp/app/runEngine/services/batchTrigger.server.ts (5)
apps/webapp/app/runEngine/concerns/queues.server.ts (1)
  • DefaultQueueManager (19-233)
apps/webapp/app/runEngine/validators/triggerTaskValidator.ts (1)
  • DefaultTriggerTaskValidator (17-106)
apps/webapp/app/v3/services/batchTriggerV3.server.ts (6)
  • options (894-900)
  • BatchTriggerTaskServiceOptions (52-59)
  • environment (318-413)
  • batch (544-578)
  • batch (751-819)
  • batch (821-892)
internal-packages/run-engine/src/index.ts (1)
  • AuthenticatedEnvironment (4-4)
apps/webapp/app/env.server.ts (1)
  • env (1065-1065)
internal-packages/run-engine/src/engine/index.ts (2)
internal-packages/run-engine/src/engine/billingCache.ts (1)
  • BillingCache (31-92)
internal-packages/run-engine/src/engine/systems/dequeueSystem.ts (2)
  • DequeueSystem (24-642)
  • orgId (543-641)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
  • GitHub Check: typecheck / typecheck
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
  • GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (20)
apps/webapp/app/v3/services/triggerTask.server.ts (1)

36-37: LGTM: options gains optional planType.

The addition is consistent with the PR’s planType propagation and will be forwarded to the RunEngineTriggerTaskService as part of options.

apps/webapp/app/runEngine/validators/triggerTaskValidator.ts (3)

7-15: LGTM: type-only imports and EntitlementValidationResult adoption.

Type import cleanup and referencing EntitlementValidationResult align with the updated contract.


41-44: LGTM: validateEntitlement now returns the richer result type.

Returning Promise matches the updated interface and broadens what consumers can do with the result.


59-60: Propagate plan details on success.

Returning { ok: true, plan: result?.plan } makes the plan available to downstream logic. Confirm that getEntitlement reliably returns a plan when access is granted; otherwise, plan may be undefined and consumers should handle that.

Would you like me to add a narrow helper type guard or a small utility to coerce a missing plan into a default (e.g., a “free” plan) to simplify downstream logic?

apps/webapp/app/runEngine/types.ts (3)

1-4: LGTM: convert to type-only imports and add ReportUsagePlan.

This reduces runtime import overhead and clarifies type boundaries.


20-21: LGTM: add planType to TriggerTaskServiceOptions.

Matches the broader planType propagation in the PR.


111-120: LGTM: introduce EntitlementValidationResult union.

This makes entitlement checks more expressive and enables carrying plan metadata. Ensure consumers narrow on ok to access error/plan correctly.

internal-packages/run-engine/src/engine/systems/dequeueSystem.ts (1)

477-478: Placement tag emission aligned with PR intent

Emitting a normalized "paid" placement tag directly on the dequeued message is consistent with the PR’s scheduling design and remains backward compatible (downstream decides whether to act on it).

apps/webapp/app/runEngine/services/triggerTask.server.ts (2)

126-155: Entitlement-aware planType propagation with skipChecks override is sound

The dual path (validate entitlement vs. skipChecks with explicit planType) keeps the trigger flow flexible and non-blocking. The warning when skipChecks is true without planType is useful.


338-339: Confirmed — no change needed: TriggerTaskServiceOptions already has skipChecks and planType

Both fields are present in the type definitions:

  • apps/webapp/app/v3/services/triggerTask.server.ts — skipChecks?: boolean (line 29), planType?: string (line 36)
  • apps/webapp/app/runEngine/types.ts — skipChecks?: boolean (line 17), planType?: string (line 20)
internal-packages/run-engine/src/engine/index.ts (4)

298-303: BillingCache wiring is correct and uses existing Redis options

Instantiating BillingCache with the cache/runLock redis options and logger is clean. This matches how other subsystems consume Redis configuration.


367-369: TriggerParams: planType addition is well-integrated

Threading planType into trigger params aligns with the new billing/placement model.


443-444: Prisma create: passing planType as string | undefined is acceptable in this codebase

Per team convention, undefined is fine to pass to Prisma here.


1363-1369: Nice: explicit billing cache invalidation API

Providing invalidateBillingCache makes it easy for the webapp to react to plan changes. Good encapsulation; the underlying cache handles errors internally.

apps/webapp/app/services/platform.v3.server.ts (2)

286-292: Hooked billing cache invalidation on plan updates — good call

Optional invalidation callback keeps the platform service decoupled while ensuring engine cache coherence when plan changes.


437-457: Entitlement API now resilient and permissive on failure

Returning hasAccess: true on errors avoids blocking UX when billing is down, consistent with the PR’s backward-compat goal.

apps/webapp/app/runEngine/services/batchTrigger.server.ts (4)

90-101: Entitlement validation and planType extraction upfront is solid

Validating once at batch entry and propagating planType downstream minimizes repeated checks and aligns with the skipChecks flow later.


548-575: Queue size guard based on newRunCount is a good safety net

Counting only new runs before validating queue limits avoids penalizing idempotent/cached items. Clear logs aid observability.


704-711: Passing skipChecks + planType into TriggerTaskService is consistent with the single validation model

This preserves the plan context and avoids redundant entitlement/queue checks at the per-item level.


773-775: No action needed — flag is parsed to a boolean

BoolEnv pre-processes string values ("true" / "1", case-insensitive, trimmed) into booleans, and the schema sets:

  • apps/webapp/app/utils/boolEnv.ts — BoolEnv (preprocess -> z.boolean())
  • apps/webapp/app/env.server.ts — BATCH_TRIGGER_CACHED_RUNS_CHECK_ENABLED: BoolEnv.default("false") (line ~764)
  • apps/supervisor/src/envUtil.test.ts — tests confirming "true"/"1" => true, "false"/invalid => false

So env.BATCH_TRIGGER_CACHED_RUNS_CHECK_ENABLED is a boolean and the existing check if (!env.BATCH_TRIGGER_CACHED_RUNS_CHECK_ENABLED) is correct.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (1)
apps/webapp/app/runEngine/types.ts (1)

124-127: Validator contract correctly updated to return the richer result.

This aligns with the entitlement flow changes and prior review feedback.

Confirm all implementations now return EntitlementValidationResult and that downstream code handles the ok + plan case:

#!/bin/bash
# Find validateEntitlement implementations and check returns
rg -n --pretty --glob '!**/node_modules/**' 'validateEntitlement\s*\(' -A 8 -B 4

# Find usages that inspect the result shape
rg -n --pretty --glob '!**/node_modules/**' 'validateEntitlement\(' -A 3 -B 1
🧹 Nitpick comments (1)
apps/supervisor/src/envUtil.ts (1)

14-17: Typed BoolEnv.default is good; consider consolidation and minor extensions

The type-level restriction to boolean defaults is spot on and addresses prior inconsistencies. Two optional improvements:

  • Mirror this for .catch(...) to keep the same boolean-only ergonomics.
  • The same parser lives in apps/webapp/app/utils/boolEnv.ts; consider hoisting a shared helper to avoid drift.
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these settings in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between ca65e2b and fb68afb.

📒 Files selected for processing (5)
  • apps/supervisor/src/env.ts (3 hunks)
  • apps/supervisor/src/envUtil.ts (1 hunks)
  • apps/webapp/app/env.server.ts (1 hunks)
  • apps/webapp/app/runEngine/types.ts (4 hunks)
  • apps/webapp/app/utils/boolEnv.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/webapp/app/env.server.ts
  • apps/supervisor/src/env.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations

Files:

  • apps/webapp/app/utils/boolEnv.ts
  • apps/webapp/app/runEngine/types.ts
  • apps/supervisor/src/envUtil.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)

We use zod a lot in packages/core and in the webapp

Files:

  • apps/webapp/app/utils/boolEnv.ts
  • apps/webapp/app/runEngine/types.ts
apps/webapp/**/*.{ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/webapp.mdc)

apps/webapp/**/*.{ts,tsx}: In the webapp, all environment variables must be accessed through the env export of env.server.ts, instead of directly accessing process.env.
When importing from @trigger.dev/core in the webapp, never import from the root @trigger.dev/core path; always use one of the subpath exports as defined in the package's package.json.

Files:

  • apps/webapp/app/utils/boolEnv.ts
  • apps/webapp/app/runEngine/types.ts
🧠 Learnings (14)
📚 Learning: 2025-08-14T18:35:44.306Z
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2390
File: apps/webapp/app/env.server.ts:764-765
Timestamp: 2025-08-14T18:35:44.306Z
Learning: The BoolEnv helper in apps/webapp/app/utils/boolEnv.ts uses z.preprocess with inconsistent default value types across the codebase - some usages pass boolean defaults (correct) while others pass string defaults (incorrect), leading to type confusion. The helper should enforce boolean-only defaults or have clearer documentation.

Applied to files:

  • apps/webapp/app/utils/boolEnv.ts
  • apps/supervisor/src/envUtil.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When implementing schema tasks, use `schemaTask` from `trigger.dev/sdk/v3` and validate payloads as shown.

Applied to files:

  • apps/webapp/app/runEngine/types.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When using retry, queue, machine, or maxDuration options, configure them as shown in the examples for Trigger.dev tasks.

Applied to files:

  • apps/webapp/app/runEngine/types.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When triggering a task from backend code, use `tasks.trigger`, `tasks.batchTrigger`, or `tasks.triggerAndPoll` as shown in the examples.

Applied to files:

  • apps/webapp/app/runEngine/types.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : The `run` function contains your task logic in Trigger.dev tasks.

Applied to files:

  • apps/webapp/app/runEngine/types.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When implementing scheduled (cron) tasks, use `schedules.task` from `trigger.dev/sdk/v3` and follow the shown patterns.

Applied to files:

  • apps/webapp/app/runEngine/types.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : ALWAYS generate Trigger.dev tasks using the `task` function from `trigger.dev/sdk/v3` and export them as shown in the correct pattern.

Applied to files:

  • apps/webapp/app/runEngine/types.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : You MUST use `trigger.dev/sdk/v3` when writing Trigger.dev tasks.

Applied to files:

  • apps/webapp/app/runEngine/types.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When triggering a task from inside another task, use `yourTask.trigger`, `yourTask.batchTrigger`, `yourTask.triggerAndWait`, `yourTask.batchTriggerAndWait`, `batch.triggerAndWait`, `batch.triggerByTask`, or `batch.triggerByTaskAndWait` as shown.

Applied to files:

  • apps/webapp/app/runEngine/types.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : You MUST `export` every task, including subtasks, in Trigger.dev task files.

Applied to files:

  • apps/webapp/app/runEngine/types.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When using metadata in tasks, use the `metadata` API as shown, and only inside run functions or task lifecycle hooks.

Applied to files:

  • apps/webapp/app/runEngine/types.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Each task needs a unique ID within your project.

Applied to files:

  • apps/webapp/app/runEngine/types.ts
📚 Learning: 2025-07-18T17:50:25.014Z
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : If you are able to generate an example payload for a task, do so.

Applied to files:

  • apps/webapp/app/runEngine/types.ts
📚 Learning: 2024-10-16T12:24:37.279Z
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#1407
File: apps/kubernetes-provider/src/index.ts:37-39
Timestamp: 2024-10-16T12:24:37.279Z
Learning: In Node.js, when assigning default values to environment variables using the `||` operator (e.g., `const VAR = process.env.VAR || 'default'`), empty strings are treated as falsy values, so the default value will be used without additional checks.

Applied to files:

  • apps/supervisor/src/envUtil.ts
🧬 Code Graph Analysis (3)
apps/webapp/app/utils/boolEnv.ts (1)
apps/supervisor/src/envUtil.ts (1)
  • BoolEnv (15-17)
apps/webapp/app/runEngine/types.ts (2)
internal-packages/run-engine/src/engine/tests/setup.ts (1)
  • AuthenticatedEnvironment (17-19)
internal-packages/run-engine/src/shared/index.ts (1)
  • AuthenticatedEnvironment (4-6)
apps/supervisor/src/envUtil.ts (1)
apps/webapp/app/utils/boolEnv.ts (1)
  • BoolEnv (12-14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
  • GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
  • GitHub Check: typecheck / typecheck
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
apps/webapp/app/runEngine/types.ts (3)

1-4: Good switch to type-only imports.

Reduces runtime footprint and avoids accidental circular deps. Looks consistent with our guidelines.


114-123: EntitlementValidationResult shape looks good.

Clear discriminated union and optional plan on success enables data flow without widening error paths. No issues.


65-68: validateQueueLimits signature verified — no action required

I confirmed the change is implemented consistently and defaults to 1 when omitted.

  • apps/webapp/app/runEngine/types.ts — QueueManager.validateQueueLimits(env, itemsToAdd?: number)
  • apps/webapp/app/runEngine/concerns/queues.server.ts — DefaultQueueManager.validateQueueLimits(... itemsToAdd?: number) → calls guardQueueSizeLimitsForEnv(this.engine, environment, itemsToAdd); guardQueueSizeLimitsForEnv(itemsToAdd: number = 1) implements the default = 1 behavior.
  • apps/webapp/app/runEngine/services/triggerTask.server.ts — calls validateQueueLimits(environment) (relies on default = 1).
  • apps/webapp/app/runEngine/services/batchTrigger.server.ts — calls validateQueueLimits(environment, newRunCount) (passes explicit count).

No fixes required.

apps/webapp/app/utils/boolEnv.ts (1)

11-14: No string defaults found — all BoolEnv.default call sites use boolean literals

I searched the repo for BoolEnv.default usages and string-literal defaults; none were found. Call sites discovered:

  • apps/webapp/app/env.server.ts:399: MARQS_DISABLE_REBALANCING: BoolEnv.default(false),
  • apps/webapp/app/env.server.ts:473: RUN_ENGINE_DEBUG_WORKER_NOTIFICATIONS: BoolEnv.default(false),
  • apps/webapp/app/env.server.ts:764: BATCH_TRIGGER_CACHED_RUNS_CHECK_ENABLED: BoolEnv.default(false),
  • apps/supervisor/src/env.ts:18: TRIGGER_WORKLOAD_API_ENABLED: BoolEnv.default(true),
  • apps/supervisor/src/env.ts:32: RUNNER_PRETTY_LOGS: BoolEnv.default(false),
  • apps/supervisor/src/env.ts:35: TRIGGER_DEQUEUE_ENABLED: BoolEnv.default(true),
  • apps/supervisor/src/env.ts:47: RESOURCE_MONITOR_ENABLED: BoolEnv.default(false),
  • apps/supervisor/src/env.ts:54: DOCKER_STRIP_IMAGE_DIGEST: BoolEnv.default(true),
  • apps/supervisor/src/env.ts:58: DOCKER_ENFORCE_MACHINE_PRESETS: BoolEnv.default(true),
  • apps/supervisor/src/env.ts:59: DOCKER_AUTOREMOVE_EXITED_CONTAINERS: BoolEnv.default(true),
  • apps/supervisor/src/env.ts:73: KUBERNETES_FORCE_ENABLED: BoolEnv.default(false),
  • apps/supervisor/src/env.ts:81: PLACEMENT_TAGS_ENABLED: BoolEnv.default(false),
  • apps/supervisor/src/env.ts:85: METRICS_ENABLED: BoolEnv.default(true),
  • apps/supervisor/src/env.ts:86: METRICS_COLLECT_DEFAULTS: BoolEnv.default(true),
  • apps/supervisor/src/env.ts:91: POD_CLEANER_ENABLED: BoolEnv.default(true),
  • apps/supervisor/src/env.ts:96: FAILED_POD_HANDLER_ENABLED: BoolEnv.default(true),
  • apps/supervisor/src/env.ts:100: DEBUG: BoolEnv.default(false),
  • apps/supervisor/src/env.ts:101: SEND_RUN_DEBUG_LOGS: BoolEnv.default(false),

No further action required — the typed BoolEnv.default will enforce boolean-only defaults at compile time.

@nicktrn nicktrn merged commit fa7f4b1 into main Aug 15, 2025
31 checks passed
@nicktrn nicktrn deleted the feat/tier-scheduling branch August 15, 2025 08:52
Copy link

sentry-io bot commented Aug 18, 2025

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

Did you find this useful? React with a 👍 or 👎

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.

3 participants