⚠️ No Changeset found

Latest commit: a6d6c52

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.

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

Walkthrough

Adds a Limits feature end-to-end: a new SideMenu entry linking to a Limits page; a server-side LimitsPresenter that aggregates rate limits, quotas, features, and plan data (including fetching remaining rate-limit tokens via Redis); a Remix route and UI rendering sections for Current Plan, Concurrency, Rate Limits, Quotas, and Features; a shared createLimiterFromConfig helper used where limiters are constructed; a new limitsPath helper; Tailwind color tokens for concurrency/limits/regions; and an icon dependency bump.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2760
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx:278-281
Timestamp: 2025-12-08T15:19:56.823Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx, the tableState search parameter uses intentional double-encoding: the parameter value contains a URL-encoded URLSearchParams string, so decodeURIComponent(value("tableState") ?? "") is required to fully decode it before parsing with new URLSearchParams(). This pattern allows bundling multiple filter/pagination params as a single search parameter.

Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp

Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : When importing from `trigger.dev/core` in the webapp, use subpath exports from the package.json instead of importing from the root path

Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T11:01:34.792Z
Learning: Applies to **/*.{ts,tsx,js} : Import from trigger.dev/core using subpaths only; never import from root of trigger.dev/core

Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `trigger.dev/sdk/v3` for all imports in Trigger.dev tasks

Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: Applies to packages/trigger-sdk/**/*.{ts,tsx} : In the Trigger.dev SDK (packages/trigger-sdk), prefer isomorphic code like fetch and ReadableStream instead of Node.js-specific code

Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T11:01:34.792Z
Learning: Applies to **/*.{ts,tsx,js} : Always import task definitions from trigger.dev/sdk, never from trigger.dev/sdk/v3 or deprecated client.defineJob pattern

Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T11:01:34.792Z
Learning: Applies to **/*.{ts,tsx} : Every Trigger.dev task must be exported; use task() function with unique id and run async function

Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `tasks.trigger()` with type-only imports to trigger tasks from backend code without importing the task implementation

Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/**/*.{ts,tsx} : Access all environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env` in the Trigger.dev webapp

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.

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

PR Review: New Limits Page

Overview

This PR adds a new "Limits" page to the webapp that displays rate limits, quotas, and plan features for an organization. The implementation follows existing patterns in the codebase (similar to the concurrency page).

Code Quality and Best Practices

Strengths:

• Well-structured presenter pattern following existing conventions (LimitsPresenter extends BasePresenter)
• Good separation of concerns between presenter (data fetching) and route (UI rendering)
• Consistent use of existing UI components (Table, Badge, InfoIconTooltip, etc.)
• Type safety with proper TypeScript types exported from the presenter
• Auto-revalidation using the established useAutoRevalidate hook pattern

Minor Improvements:

1. Unused userId parameter (line 85-86 in LimitsPresenter.server.ts): The userId is passed to the presenter but never used within it. While authorization is properly handled at the route level via findProjectBySlug, consider either using it for tracing/logging or removing it from the parameter list to avoid confusion.

2. Unused projectId parameter: Similarly, projectId is passed but never used in the presenter. The org-level queries don't need it.

Potential Issues

1. Singleton Redis client initialization (lines 14-24 in LimitsPresenter.server.ts): The Redis client is created as a singleton at module load time. If the rate limit Redis environment variables aren't set, this could throw an error during module initialization rather than gracefully handling missing config. Consider lazy initialization or adding null checks.

2. Rate limit token calculation assumptions: The getRateLimitRemainingTokens function makes assumptions about the internal storage format of @upstash/ratelimit ("tokens:timestamp" format). If the library's internal format changes, this would silently return incorrect values. The comment documents this well, but it's worth noting this is a fragile integration.

Security Considerations

• Authorization is properly enforced at the route level via findProjectBySlug and findEnvironmentBySlug, which verify the user is a member of the organization
• API key handling: The environment API key is passed to query Redis for rate limit tokens. The key is hashed with SHA-256 before being used as a Redis key (same as the rate limiter itself), which is appropriate

Performance Considerations

1. Multiple sequential database queries: The presenter makes 4 separate count() queries (schedules, alerts, branches, org). These could potentially be parallelized with Promise.all() for better performance, though the impact may be minimal given they're hitting the replica.

2. Auto-reload interval: 5 second polling interval is reasonable for this type of dashboard. Consider whether this is needed at all since limit data doesn't change frequently.

Test Coverage

No tests are included with this PR. Given the CLAUDE.md guidance about using testcontainers for tests, consider adding:

• Unit tests for durationToMs function
• Integration tests for LimitsPresenter using postgresTest and redisTest
• Tests for edge cases like missing plan data or null limits

Minor Suggestions

1. Line 121: isOnTopPlan is hardcoded to check for "v3_pro_1". Consider making this configurable or documenting why this specific plan code is the "top" plan.

2. Lines 272, 282, 292, 298 in presenter: Comments like "Would need to query Redis/realtime service for this" indicate incomplete functionality. Consider either implementing these or removing the currentUsage field for these quotas to avoid showing misleading zeros.

3. Tailwind config: The new limits color is defined as colors.purple[500] which is the same as queues. Consider whether a distinct color would help differentiate the menu items.

Overall Assessment

This is a well-structured feature that follows established patterns in the codebase. The main areas for improvement are adding test coverage and potentially parallelizing the database queries. The security model is sound and the UI implementation is clean and consistent with existing pages.

PR Review: New Limits Page

This PR adds a comprehensive "Limits" page to the webapp that displays rate limits, quotas, and plan features to users. Overall, this is a well-structured feature with good separation between the presenter and the UI components.

Strengths

1. Good architecture: The LimitsPresenter follows the existing presenter pattern and properly separates data fetching from rendering
2. Reuses existing infrastructure: Correctly uses createLimiterFromConfig from the auth middleware and follows established patterns for Redis client singletons
3. Thoughtful UX: Auto-revalidation with useAutoRevalidate, environment selector for rate limits, clear upgrade paths based on plan tier
4. Consistent styling: Uses existing component primitives and extends tailwind config appropriately

Issues and Suggestions

1. Potential Performance Issue - Sequential Database Queries

File: apps/webapp/app/presenters/v3/LimitsPresenter.server.ts:136-169

The presenter makes 4 sequential database queries (organization, schedules, alerts, branches). These could be parallelized with Promise.all:

const [organization, scheduleCount, alertChannelCount, activeBranchCount] = await Promise.all([
  this._replica.organization.findUniqueOrThrow({...}),
  this._replica.taskSchedule.count({...}),
  this._replica.projectAlertChannel.count({...}),
  this._replica.runtimeEnvironment.count({...}),
]);

This would reduce latency, especially important since this page auto-refreshes every 5 seconds.

2. Unused Parameter

File: apps/webapp/app/presenters/v3/LimitsPresenter.server.ts:86

The userId and projectId parameters are passed to call() but never used. Either remove them or document why they might be needed for future authorization checks.

3. Hardcoded Plan Code

File: apps/webapp/app/presenters/v3/LimitsPresenter.server.ts:122

const isOnTopPlan = currentPlan?.v3Subscription?.plan?.code === "v3_pro_1";

Consider extracting this plan code to a constant or using a property from the plan itself to determine if it's the top tier, to avoid breaking if plan codes change.

4. Duplicate Rate Limit Config Resolution Logic

File: apps/webapp/app/presenters/v3/LimitsPresenter.server.ts:333-371

The resolveBatchRateLimitConfig function is nearly identical to the one in batchLimits.server.ts. Consider consolidating to avoid drift between the two implementations.

5. Missing Error Handling for Redis Client

File: apps/webapp/app/presenters/v3/LimitsPresenter.server.ts:16-25

The singleton Redis client creation doesn't handle the case where Redis environment variables might be undefined. If RATE_LIMIT_REDIS_HOST is not set, this could fail silently or throw an unclear error. The getRateLimitRemainingTokens function does handle errors gracefully (line 424-429), which is good.

6. Consider Adding Test Coverage

Per the CLAUDE.md guidelines, tests should go next to source files. Consider adding:

• LimitsPresenter.server.test.ts using postgresTest and redisTest from testcontainers
• Tests for the config resolution functions

7. Minor UI Considerations

• Line 129: The docs link points to /limits which may not exist yet - ensure the docs page is created
• Line 569-571: The percentage calculation could divide by zero if quota.limit is 0 (though the > 0 check prevents this)

Security

• No security concerns identified
• The API key hashing (createHash("sha256")) matches the middleware implementation
• No sensitive data is exposed in the UI that shouldn't be

Overall

This is a solid implementation that follows project conventions well. The main areas for improvement are performance (parallel queries) and code duplication (rate limit config resolution). The feature provides valuable visibility into system limits for users.

Recommendation: Approve with minor changes to parallelize the database queries and consolidate the rate limit config resolution logic.

PR Review: feat(webapp): New limits page (2)

This PR adds a new Limits page to the webapp that displays rate limits, quotas, and plan features for organizations. Overall, this is a well-structured addition with clean separation between the presenter and route components.

Strengths

1. Clean Architecture

• Good separation of concerns with LimitsPresenter handling data fetching and the route handling UI
• Proper use of the existing BasePresenter pattern
• Uses the replica database for read operations, which is the correct pattern

2. Thoughtful UX

• Auto-revalidation every 5 seconds with useAutoRevalidate
• Environment selector for viewing rate limits per environment
• Clear visual indicators with color-coded usage percentages
• Helpful tooltips explaining each limit type

3. Security Considerations

• API keys are hashed using SHA-256 before querying Redis (matching the middleware pattern)
• No sensitive data exposed in the UI

Potential Issues & Suggestions

1. Redis Client Error Handling (LimitsPresenter.server.ts:16-25)

The singleton Redis client is created immediately at module load time. If Redis is unavailable during startup, this could cause issues.

const rateLimitRedisClient = singleton("rateLimitQueryRedisClient", () =>
  createRedisRateLimitClient({...})
);

Consider wrapping this in a lazy initialization pattern or adding error handling around the Redis client creation.

2. Unused userId Parameter (LimitsPresenter.server.ts:86)

The call method accepts a userId parameter but never uses it. Either remove it or consider if authorization checks should use it:

public async call({
  userId,  // <-- Never used
  projectId,
  organizationId,
  environmentApiKey,
}: {...})

3. Hardcoded Plan Detection (LimitsPresenter.server.ts:122)

The "top plan" check is hardcoded to a specific plan code:

const isOnTopPlan = currentPlan?.v3Subscription?.plan?.code === "v3_pro_1";

This could become a maintenance burden if plan codes change. Consider adding a property like isTopTier to the plan definition itself.

4. Unused projectId Parameter (LimitsPresenter.server.ts:88)

Similar to userId, the projectId parameter is accepted but unused in the presenter.

5. Missing Error Boundary (route.tsx)

The route catches errors via tryCatch and throws a 400 response, but there is no error boundary component for better UX when things fail. Consider adding an ErrorBoundary export.

6. Reference Project Import Path (rateLimitStress.ts:1)

The stress test file imports from @trigger.dev/sdk/v3 instead of @trigger.dev/sdk:

import { logger, task, tasks, RateLimitError } from "@trigger.dev/sdk/v3";

Per CLAUDE.md, the correct import is @trigger.dev/sdk (the /v3 suffix is deprecated).

7. Rate Limit Query Creates New Ratelimit Instance (LimitsPresenter.server.ts:410-418)

Each call to getRateLimitRemainingTokens creates a new Ratelimit instance:

const ratelimit = new Ratelimit({
  redis: rateLimitRedisClient,
  limiter,
  ephemeralCache: new Map(),  // <-- New cache each time
  analytics: false,
  prefix: `ratelimit:${keyPrefix}`,
});

Since this page auto-revalidates every 5 seconds, creating new ephemeral caches each time is wasteful. Consider caching the Ratelimit instances.

Performance Considerations

1. Multiple Sequential Database Queries (LimitsPresenter.server.ts:136-169)

The presenter makes four separate count queries:

• taskSchedule.count
• projectAlertChannel.count
• runtimeEnvironment.count
• Organization with _count

Consider using Promise.all to run these in parallel:

const [scheduleCount, alertChannelCount, activeBranchCount] = await Promise.all([
  this._replica.taskSchedule.count({...}),
  this._replica.projectAlertChannel.count({...}),
  this._replica.runtimeEnvironment.count({...}),
]);

2. Rate Limit Redis Queries (LimitsPresenter.server.ts:171-181)

Similarly, the two getRateLimitRemainingTokens calls could be parallelized:

const [apiRateLimitTokens, batchRateLimitTokens] = await Promise.all([
  getRateLimitRemainingTokens("api", environmentApiKey, apiRateLimitConfig),
  getRateLimitRemainingTokens("batch", environmentApiKey, batchRateLimitConfig),
]);

Test Coverage

No tests were added for LimitsPresenter. Given the complexity of the presenter (rate limit queries, plan resolution, config parsing), adding tests would be valuable. The presenter could be tested using the existing testcontainers pattern mentioned in CLAUDE.md.

Minor Nits

1. tailwind.config.js - The new limits and regions color variables are defined but line 351-352 in SideMenu.tsx updates concurrency and regions icons to use them while still using text-orange-500 directly for ConcurrencyIcon in route.tsx:225

2. Duplicate resolution functions - resolveBatchRateLimitConfig exists in both LimitsPresenter.server.ts and batchLimits.server.ts. Consider extracting to a shared utility.

Summary

This is a solid PR that adds valuable visibility into organization limits. The main areas for improvement are:

1. Parallelize database and Redis queries for better performance
2. Add tests for the presenter
3. Fix the SDK import path in the reference project
4. Consider removing unused parameters or adding error boundaries

The UI looks clean and the component structure is well-organized. Good work!

PR Review: feat(webapp): New limits page (2)

This is a well-structured addition of a new Limits page that provides users visibility into their rate limits, quotas, and plan features. The implementation follows existing patterns in the codebase and includes good refactoring of shared code.

Overall Assessment: ✅ Looks Good

The PR is well-organized and ready for merge with a few minor suggestions.

Strengths

1. Good code reuse: The createLimiterFromConfig function was extracted and shared between authorizationRateLimitMiddleware.server.ts and batchLimits.server.ts, eliminating duplicate code.

2. Consistent patterns: The presenter/route pattern follows existing conventions in the codebase (LimitsPresenter extending BasePresenter).

3. User-friendly UI: The page includes helpful tooltips, color-coded usage indicators, and clear upgrade paths (View plans vs Contact us).

4. Auto-refresh: Using useAutoRevalidate with a 5-second interval ensures users see current rate limit token counts.

5. Environment-aware: Rate limits are tracked per API key, and the environment selector allows users to view limits for different environments.

Suggestions

1. Unused userId parameter in presenter

LimitsPresenter.server.ts:85-88

The userId parameter is passed but never used in the presenter. Consider removing it if not needed:

public async call({
  userId,  // <-- Not used anywhere in the method
  projectId,
  organizationId,
  environmentApiKey,
}: {...})

2. Import path in reference test file

references/hello-world/src/trigger/rateLimitStress.ts:1

The import uses @trigger.dev/sdk/v3 which is flagged as deprecated in CLAUDE.md:

import { logger, task, tasks, RateLimitError } from "@trigger.dev/sdk/v3";

Should this be @trigger.dev/sdk instead?

3. Consider parallel database queries

LimitsPresenter.server.ts:136-169

The three count queries (scheduleCount, alertChannelCount, activeBranchCount) are executed sequentially. Since they're independent, they could be parallelized with Promise.all for better performance:

const [scheduleCount, alertChannelCount, activeBranchCount] = await Promise.all([
  this._replica.taskSchedule.count({...}),
  this._replica.projectAlertChannel.count({...}),
  this._replica.runtimeEnvironment.count({...}),
]);

4. Hardcoded plan code for "top plan" detection

LimitsPresenter.server.ts:122

const isOnTopPlan = currentPlan?.v3Subscription?.plan?.code === "v3_pro_1";

This hardcoded plan code may become stale if plan codes change. Consider whether this should be derived from plan metadata (e.g., a tier field) or kept centralized in a constants file.

5. Minor: Type assertion in resolveBatchConcurrencyConfig

LimitsPresenter.server.ts:384-387

The type assertion could be replaced with proper Zod validation for consistency with the other resolve functions:

const config = batchConcurrencyConfig as Record<string, unknown>;

Questions

1. Rate limit stress test file: Is references/hello-world/src/trigger/rateLimitStress.ts intended to be included in this PR, or should it be in a separate testing/tooling PR? It's useful for testing but adds 257 lines to a feature PR.

2. Missing test coverage: There don't appear to be any unit tests for LimitsPresenter. Given the complexity of resolving configs and querying rate limit tokens, consider adding tests using testcontainers.

Security

No security concerns identified. The API key is hashed before being used as a rate limit key, consistent with existing patterns. Sensitive data (API keys) are not exposed in the UI.

Summary

This is a clean implementation that provides valuable visibility into rate limits and quotas. The minor suggestions above are non-blocking. Nice work on the UI polish with color-coded usage indicators and comprehensive tooltips!

PR Review: feat(webapp): New limits page

Overall Assessment

This is a well-structured PR that adds a comprehensive limits dashboard showing rate limits, quotas, and plan features. The code follows the existing patterns in the codebase and provides a useful feature for users to understand their limits. Here is detailed feedback:

Code Quality and Best Practices

Strengths:

• Clean separation of concerns: presenter handles data fetching, route handles presentation
• Good use of existing components (Table, Badge, InfoIconTooltip, etc.)
• Proper extraction of createLimiterFromConfig to avoid code duplication (previously duplicated in batchLimits.server.ts and authorizationRateLimitMiddleware.server.ts)
• Well-typed with proper TypeScript types exported from the presenter
• Good use of useAutoRevalidate for real-time updates

Suggestions:

1. Import consistency in reference file (line 1): The rateLimitStress.ts reference file imports from @trigger.dev/sdk/v3 but per CLAUDE.md, you should "Always import from @trigger.dev/sdk. Never use @trigger.dev/sdk/v3"

2. Minor duplication: The resolveApiRateLimitConfig and resolveBatchRateLimitConfig functions in LimitsPresenter.server.ts are nearly identical. Consider a generic helper, though this may be intentional for clarity.

Potential Bugs or Issues

1. Rate limit prefix inconsistency (LimitsPresenter.server.ts:407): The presenter uses "api" and "batch" as key prefixes when querying remaining tokens. However, the actual middleware uses different prefixes set in the server configuration. You should verify that these prefixes match exactly what the rate limit middleware uses, otherwise the getRemaining call will query the wrong keys and return incorrect (or null) values.

2. Missing limitsPath import in SideMenu.tsx: The import is added but I would verify this does not break when the route does not exist (e.g., during navigation). Looks fine based on how other paths are handled.

3. Hardcoded "top plan" check (LimitsPresenter.server.ts:118): The hardcoded plan code check (v3_pro_1) could become stale if plan codes change. Consider adding a comment explaining this or using a constant.

Performance Considerations

1. Multiple sequential database queries (LimitsPresenter.server.ts:132-158): The presenter makes several sequential count() queries for taskSchedule, projectAlertChannel, and runtimeEnvironment. These could potentially be parallelized with Promise.all() for better performance.

2. Rate limit token queries: Similarly, getRateLimitRemainingTokens is called twice sequentially. These could also be parallelized.

3. 5-second auto-refresh interval: The page polls every 5 seconds which is reasonable, but consider if this frequency is necessary for data that does not change rapidly (quotas do not change often).

Security Concerns

1. API key passed to presenter: The presenter receives environmentApiKey to query rate limits. This is handled appropriately - it is hashed before being used as a Redis key identifier, matching the existing pattern in the rate limit middleware. No security issues identified.

2. No sensitive data exposure: The page only shows limits/quotas, not actual API keys or secrets.

Test Coverage

Missing tests: There do not appear to be any tests added for:

• LimitsPresenter.server.ts
• The route loader

Per the codebase conventions, tests should be added using testcontainers (not mocks). Consider adding unit tests for the presenter and integration tests for the loader.

The reference rateLimitStress.ts file is appropriately a manual testing tool and does not need automated tests.

Minor Issues

1. Unused import warning potential: The Limiter type is now exported from rateLimiter.server.ts but verify the import in authorizationRateLimitMiddleware.server.ts includes it.

2. Tabler icons upgrade: The PR upgrades @tabler/icons-react from 2.39.0 to 3.36.1 which is a major version bump. Verify this does not cause any visual regressions or API breaking changes in other parts of the codebase using these icons.

Summary

This is a solid PR that adds valuable functionality. The main items to address:

• Required: Fix the SDK import in rateLimitStress.ts (@trigger.dev/sdk not @trigger.dev/sdk/v3)
• Recommended: Verify rate limit key prefixes match actual middleware configuration
• Recommended: Parallelize database queries for better performance
• Nice to have: Add test coverage for the presenter

Overall, good work on the implementation!