feat(webapp): New limits page #2885
Conversation
samejr
commented
Jan 14, 2026
samejr
commented
|
WalkthroughAdds a Limits feature end-to-end: UI (SideMenu entry and new route) and path helper; a LimitsPresenter that aggregates rate limits, quotas, features, plan and usage (including remaining tokens via Redis); centralized limiter creation via createLimiterFromConfig and corresponding middleware updates; Tailwind theme color tokens for concurrency/limits/regions; package dependency bump; Prisma schema change and two SQL migrations to make TaskScheduleInstance.projectId non-nullable and add an index; new tests for required filters and several smaller refactors. Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (7)**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
{packages/core,apps/webapp}/**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
**/*.ts📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}📄 CodeRabbit inference engine (AGENTS.md)
Files:
⏰ 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)
🔇 Additional comments (1)
✏️ Tip: You can disable this entire section by setting 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 |
PR Review: New Limits PageOverviewThis 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 PracticesStrengths:
Minor Improvements:
Potential Issues
Security Considerations
Performance Considerations
Test CoverageNo tests are included with this PR. Given the CLAUDE.md guidance about using testcontainers for tests, consider adding:
Minor Suggestions
Overall AssessmentThis 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. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
PR Review: New Limits PageThis 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
Issues and Suggestions1. Potential Performance Issue - Sequential Database QueriesFile: The presenter makes 4 sequential database queries (organization, schedules, alerts, branches). These could be parallelized with 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 ParameterFile: The 3. Hardcoded Plan CodeFile: 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 LogicFile: The 5. Missing Error Handling for Redis ClientFile: The singleton Redis client creation doesn't handle the case where Redis environment variables might be undefined. If 6. Consider Adding Test CoveragePer the CLAUDE.md guidelines, tests should go next to source files. Consider adding:
7. Minor UI Considerations
Security
OverallThis 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. Strengths1. Clean Architecture
2. Thoughtful UX
3. Security Considerations
Potential Issues & Suggestions1. 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 The 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 4. Unused Similar to 5. Missing Error Boundary (route.tsx) The route catches errors via 6. Reference Project Import Path (rateLimitStress.ts:1) The stress test file imports from import { logger, task, tasks, RateLimitError } from "@trigger.dev/sdk/v3";Per CLAUDE.md, the correct import is 7. Rate Limit Query Creates New Ratelimit Instance (LimitsPresenter.server.ts:410-418) Each call to 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 Performance Considerations1. Multiple Sequential Database Queries (LimitsPresenter.server.ts:136-169) The presenter makes four separate count queries:
Consider using 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 const [apiRateLimitTokens, batchRateLimitTokens] = await Promise.all([
getRateLimitRemainingTokens("api", environmentApiKey, apiRateLimitConfig),
getRateLimitRemainingTokens("batch", environmentApiKey, batchRateLimitConfig),
]);Test CoverageNo tests were added for Minor Nits
SummaryThis is a solid PR that adds valuable visibility into organization limits. The main areas for improvement are:
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 GoodThe PR is well-organized and ready for merge with a few minor suggestions. Strengths
Suggestions1. Unused
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsx:
- Line 5: The import of tryCatch currently comes from "@trigger.dev/core";
update the import to use the utils subpath export by importing tryCatch from
"@trigger.dev/core/utils" instead so it follows the webapp subpath export
guideline; modify the import statement referencing tryCatch in the file where
it's currently imported to point to the utils subpath.
🧹 Nitpick comments (5)
apps/webapp/app/services/authorizationRateLimitMiddleware.server.ts (1)
133-143: Good addition of shared limiter factory.The helper function centralizes limiter creation logic. The nested ternary handles all three discriminated union cases correctly.
🔧 Optional: Consider a switch statement for readability
A switch statement could be slightly more readable, especially if more limiter types are added in the future:
export function createLimiterFromConfig(config: RateLimiterConfig): Limiter { - return config.type === "fixedWindow" - ? Ratelimit.fixedWindow(config.tokens, config.window) - : config.type === "tokenBucket" - ? Ratelimit.tokenBucket(config.refillRate, config.interval, config.maxTokens) - : Ratelimit.slidingWindow(config.tokens, config.window); + switch (config.type) { + case "fixedWindow": + return Ratelimit.fixedWindow(config.tokens, config.window); + case "tokenBucket": + return Ratelimit.tokenBucket(config.refillRate, config.interval, config.maxTokens); + case "slidingWindow": + return Ratelimit.slidingWindow(config.tokens, config.window); + } }This is a minor style preference - the current implementation is correct.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsx (1)
392-411: Consider using a discriminator field instead of string comparison.Using
info.name === "Batch rate limit"for conditional logic is fragile. If the display name changes inLimitsPresenter, this logic will silently break.Consider adding an
idortypefield toRateLimitInfofor reliable discrimination:Example approach
// In LimitsPresenter.server.ts, add an id field: export type RateLimitInfo = { + id: "api" | "batch"; name: string; // ... }; // Then in the route: - {info.name === "Batch rate limit" ? ( + {info.id === "batch" ? (apps/webapp/app/presenters/v3/LimitsPresenter.server.ts (3)
277-302: Several quotas displaycurrentUsage: 0as placeholders.The quotas for
realtimeConnections,devQueueSize, anddeployedQueueSizeshowcurrentUsage: 0because the actual values require querying external services. While this is documented in comments, users may find these zero values misleading.Consider either:
- Hiding the "Current" column for these quotas in the UI
- Using
nullto indicate unavailable data (and showing "–" in the UI)
119-123: Consider extracting the hardcoded plan code to a constant or adding anisTopPlanflag in the plan definition.The comparison
plan?.code === "v3_pro_1"on line 122 is isolated to this one location, but hardcoding the plan code makes it fragile if plan codes change in the platform service. Since the plan code is sourced from an external service (getCurrentPlan), any coordinated changes to the plan naming could silently break this check without compiler warnings.Extracting this to a shared constant or adding an
isTopPlanproperty directly to the plan definition would improve maintainability and make future plan changes less error-prone.
333-351: defaultConfig bypasses RateLimiterConfig validation; consider adding duration format validation.The
as Durationcast on line 337 (and similarly line 357 for batch config) creates a default configuration that's returned without validation against theRateLimiterConfigschema. Whileenv.API_RATE_LIMIT_REFILL_INTERVALis validated as a string with a sensible default, the schema doesn't validate the duration format itself—only that the value is a string.To improve consistency and catch misconfigurations earlier, consider adding a duration format validator to the environment schema (e.g., validating patterns like "10s", "1m", etc.) so invalid values are rejected at startup rather than failing at runtime.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlreferences/hello-world/src/trigger/rateLimitStress.tsis excluded by!references/**
📒 Files selected for processing (8)
apps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/presenters/v3/LimitsPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsxapps/webapp/app/runEngine/concerns/batchLimits.server.tsapps/webapp/app/services/authorizationRateLimitMiddleware.server.tsapps/webapp/app/utils/pathBuilder.tsapps/webapp/package.jsonapps/webapp/tailwind.config.js
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/package.jsonapps/webapp/tailwind.config.jsapps/webapp/app/services/authorizationRateLimitMiddleware.server.tsapps/webapp/app/runEngine/concerns/batchLimits.server.tsapps/webapp/app/presenters/v3/LimitsPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsxapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/components/navigation/SideMenu.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/tailwind.config.jsapps/webapp/app/services/authorizationRateLimitMiddleware.server.tsapps/webapp/app/runEngine/concerns/batchLimits.server.tsapps/webapp/app/presenters/v3/LimitsPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsxapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/components/navigation/SideMenu.tsx
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: Import from@trigger.dev/coreusing subpaths only; never import from root of@trigger.dev/core
Always import task definitions from@trigger.dev/sdk, never from@trigger.dev/sdk/v3or deprecated client.defineJob pattern
Files:
apps/webapp/tailwind.config.jsapps/webapp/app/services/authorizationRateLimitMiddleware.server.tsapps/webapp/app/runEngine/concerns/batchLimits.server.tsapps/webapp/app/presenters/v3/LimitsPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsxapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/components/navigation/SideMenu.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects insteadEvery Trigger.dev task must be exported; use task() function with unique id and run async function
Files:
apps/webapp/app/services/authorizationRateLimitMiddleware.server.tsapps/webapp/app/runEngine/concerns/batchLimits.server.tsapps/webapp/app/presenters/v3/LimitsPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsxapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/components/navigation/SideMenu.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/services/authorizationRateLimitMiddleware.server.tsapps/webapp/app/runEngine/concerns/batchLimits.server.tsapps/webapp/app/presenters/v3/LimitsPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsxapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/components/navigation/SideMenu.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webappIn webapp development, access environment variables via env export from apps/webapp/app/env.server.ts; never use process.env directly
Files:
apps/webapp/app/services/authorizationRateLimitMiddleware.server.tsapps/webapp/app/runEngine/concerns/batchLimits.server.tsapps/webapp/app/presenters/v3/LimitsPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsxapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/components/navigation/SideMenu.tsx
apps/webapp/app/services/**/*.server.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Separate testable services from configuration files; follow the pattern of
realtimeClient.server.ts(testable service) andrealtimeClientGlobal.server.ts(configuration) in the webapp
Files:
apps/webapp/app/services/authorizationRateLimitMiddleware.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/services/authorizationRateLimitMiddleware.server.tsapps/webapp/app/runEngine/concerns/batchLimits.server.tsapps/webapp/app/presenters/v3/LimitsPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsxapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/components/navigation/SideMenu.tsx
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/app/services/authorizationRateLimitMiddleware.server.tsapps/webapp/app/runEngine/concerns/batchLimits.server.tsapps/webapp/app/presenters/v3/LimitsPresenter.server.tsapps/webapp/app/utils/pathBuilder.ts
🧠 Learnings (8)
📚 Learning: 2026-01-12T17:18:09.451Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2870
File: apps/webapp/app/services/redisConcurrencyLimiter.server.ts:56-66
Timestamp: 2026-01-12T17:18:09.451Z
Learning: In `apps/webapp/app/services/redisConcurrencyLimiter.server.ts`, the query concurrency limiter will not be deployed with Redis Cluster mode, so multi-key operations (keyKey and globalKey in different hash slots) are acceptable and will function correctly in standalone Redis mode.
Applied to files:
apps/webapp/app/services/authorizationRateLimitMiddleware.server.tsapps/webapp/app/runEngine/concerns/batchLimits.server.ts
📚 Learning: 2026-01-12T11:01:34.792Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T11:01:34.792Z
Learning: Applies to apps/webapp/app/**/*.test.{ts,tsx} : In testable code, never import env.server.ts in test files; pass configuration as options instead
Applied to files:
apps/webapp/app/services/authorizationRateLimitMiddleware.server.ts
📚 Learning: 2025-11-27T16:26:58.661Z
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/v3/presenters/**/*.server.{ts,tsx} : Organize presenters in the webapp following the pattern `app/v3/presenters/*/*.server.ts` to move complex loader code into classes
Applied to files:
apps/webapp/app/presenters/v3/LimitsPresenter.server.ts
📚 Learning: 2025-06-06T23:55:01.933Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2155
File: hosting/docker/.env.example:4-7
Timestamp: 2025-06-06T23:55:01.933Z
Learning: In the trigger.dev project, .env.example files should contain actual example secret values rather than placeholders, as these help users understand the expected format. The files include clear warnings about not using these defaults in production and instructions for generating proper secrets.
Applied to files:
apps/webapp/app/presenters/v3/LimitsPresenter.server.ts
📚 Learning: 2025-06-25T14:14:11.965Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2195
File: hosting/k8s/helm/values-production-example.yaml:95-102
Timestamp: 2025-06-25T14:14:11.965Z
Learning: In the Trigger.dev Helm chart production examples, the maintainer prefers to include initial/bootstrap credentials with clear warnings that they should be changed after first login, rather than requiring external secret references that could complicate initial setup. This follows their pattern of providing working examples with explicit security guidance.
Applied to files:
apps/webapp/app/presenters/v3/LimitsPresenter.server.ts
📚 Learning: 2025-11-14T16:03:06.917Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2681
File: apps/webapp/app/services/platform.v3.server.ts:258-302
Timestamp: 2025-11-14T16:03:06.917Z
Learning: In `apps/webapp/app/services/platform.v3.server.ts`, the `getDefaultEnvironmentConcurrencyLimit` function intentionally throws an error (rather than falling back to org.maximumConcurrencyLimit) when the billing client returns undefined plan limits. This fail-fast behavior prevents users from receiving more concurrency than their plan entitles them to. The org.maximumConcurrencyLimit fallback is only for self-hosted deployments where no billing client exists.
Applied to files:
apps/webapp/app/presenters/v3/LimitsPresenter.server.tsapps/webapp/app/utils/pathBuilder.ts
📚 Learning: 2025-12-08T15:19:56.823Z
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.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsx
📚 Learning: 2025-11-27T16:26:58.661Z
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
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsx
🧬 Code graph analysis (4)
apps/webapp/app/services/authorizationRateLimitMiddleware.server.ts (1)
apps/webapp/app/services/rateLimiter.server.ts (1)
Limiter(16-16)
apps/webapp/app/runEngine/concerns/batchLimits.server.ts (1)
apps/webapp/app/services/authorizationRateLimitMiddleware.server.ts (1)
createLimiterFromConfig(137-143)
apps/webapp/app/presenters/v3/LimitsPresenter.server.ts (2)
apps/webapp/app/services/rateLimiter.server.ts (2)
createRedisRateLimitClient(74-105)Duration(17-17)apps/webapp/app/services/authorizationRateLimitMiddleware.server.ts (5)
RateLimiterConfig(46-50)RateLimiterConfig(52-52)RateLimitTokenBucketConfig(37-42)RateLimitTokenBucketConfig(44-44)createLimiterFromConfig(137-143)
apps/webapp/app/components/navigation/SideMenu.tsx (2)
apps/webapp/app/utils/pathBuilder.ts (3)
concurrencyPath(502-508)regionsPath(518-524)limitsPath(510-516)apps/webapp/app/components/navigation/SideMenuItem.tsx (1)
SideMenuItem(7-53)
🔇 Additional comments (19)
apps/webapp/package.json (1)
114-114: No action required. The codebase uses@tabler/icons-reacticons in a straightforward manner—imported as named exports and rendered as JSX components—which is fully compatible with v3. There are no usages ofTablerIconPropsor explicit type annotations that would be affected by v3's breaking changes to internal types and signatures.Likely an incorrect or invalid review comment.
apps/webapp/tailwind.config.js (2)
163-165: LGTM!The new color tokens follow the established pattern for icon colors. The naming is consistent with other semantic tokens in the file.
241-244: LGTM!The new color tokens are properly exposed in the theme's color mapping.
apps/webapp/app/utils/pathBuilder.ts (1)
510-516: LGTM!The new
limitsPathfunction follows the established pattern used byconcurrencyPathandregionsPath, maintaining consistency in the codebase.apps/webapp/app/components/navigation/SideMenu.tsx (3)
354-357: Good refactor to semantic color tokens.Using
text-concurrencyinstead of hardcodedtext-amber-500improves maintainability by centralizing color definitions in the Tailwind config.
362-365: LGTM!Consistent with the semantic color token refactor.
367-373: No action needed—Limits is intentionally available to all users. Unlike Concurrency, which is a managed-cloud-only feature, the Limits route has no managed cloud guards and the LimitsPresenter provides data for all plans. Leaving the menu item unconditional is correct.apps/webapp/app/runEngine/concerns/batchLimits.server.ts (2)
4-7: Good refactor to centralize limiter creation.Importing
createLimiterFromConfigfrom the middleware module eliminates code duplication and ensures consistent limiter construction across the codebase.
35-46: LGTM!The refactored
createOrganizationRateLimiteris cleaner and delegates limiter creation to the shared helper function.apps/webapp/app/services/authorizationRateLimitMiddleware.server.ts (2)
10-10: LGTM!Adding the
Limitertype import enables proper typing for the new helper function.
264-264: LGTM!The middleware now uses the shared helper function, maintaining consistency with the rest of the codebase.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsx (5)
59-102: LGTM!The loader follows proper Remix conventions with authentication, parameter validation using Zod, appropriate 404 responses, and structured error handling with
tryCatch.
104-181: LGTM!Clean component structure with proper auto-revalidation, context hooks usage, and well-organized section rendering.
560-682: LGTM with minor note.The component logic is sound with proper handling for retention quotas and upgrade rendering. The string comparisons for quota types (Lines 570, 612) share the same fragility concern noted for rate limits, but the implementation is otherwise clean and well-organized.
732-799: LGTM!Clean implementation with proper handling of different feature value types and upgrade options.
807-854: LGTM!Well-documented utility function with clear mode handling, and clean variant-based badge rendering.
apps/webapp/app/presenters/v3/LimitsPresenter.server.ts (3)
1-25: LGTM!Proper imports with environment variables accessed through
env.server.ts, and appropriate singleton pattern for Redis client reuse.
27-82: LGTM!Well-structured type definitions using
typeinstead ofinterfaceas per coding guidelines.
398-431: LGTM!The SHA256 hashing here is for rate limit key identification (matching the middleware's approach), not for password storage—the security scanner alerts are a false positive for this context. Error handling with graceful degradation to
nullis appropriate.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
...app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsx
Show resolved
Hide resolved
samejr
changed the title
PR Review: feat(webapp): New limits pageOverall AssessmentThis 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 PracticesStrengths:
Suggestions:
Potential Bugs or Issues
Performance Considerations
Security Concerns
Test CoverageMissing tests: There do not appear to be any tests added for:
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
SummaryThis is a solid PR that adds valuable functionality. The main items to address:
Overall, good work on the implementation! |
matt-aitken
force-pushed
the
limits-page-2
branch
from
4e62f4c to
1af4aa8
Compare
Review CompleteYour review story is ready! Comment !reviewfast on this PR to re-generate the story. |
# Conflicts: # internal-packages/tsql/src/query/printer.test.ts # internal-packages/tsql/src/query/printer.ts
matt-aitken
force-pushed
the
limits-page-2
branch
from
1af4aa8 to
4c9a4d9
Compare
The backfill is for self-hosters, it will be a no-op for already filled rows (i.e. cloud)
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/webapp/app/presenters/v3/LimitsPresenter.server.ts`:
- Around line 152-160: The activeBranchCount query currently uses projectId but
the limit is described as "Maximum number of active preview branches" for the
organization; update the count to use organizationId instead of projectId so it
counts across the org. Locate the activeBranchCount calculation
(this._replica.runtimeEnvironment.count) and change the where clause to filter
by organizationId and the same branchName not null / archivedAt null conditions
(RuntimeEnvironment has an index on organizationId), leaving project-scoped
logic untouched elsewhere.
- Around line 145-149: The alert channel count query in
LimitsPresenter.server.ts (the alertChannelCount calculation using
this._replica.projectAlertChannel.count) is scoped to projectId but the
QuotaInfo.description claims "across all projects"; fix by either making the
query org-scoped or correcting the description: to make it org-wide, change the
count filter to use the project relation (e.g., count where
project.organizationId equals the current organizationId) so it aggregates
across all projects in the org, or alternatively update the
QuotaInfo.description text to explicitly state it is project-scoped to match the
existing query.
♻️ Duplicate comments (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsx (1)
5-5: Use the@trigger.dev/core/utilssubpath export fortryCatch.Per coding guidelines, imports from
@trigger.dev/coremust use subpath exports.-import { tryCatch } from "@trigger.dev/core"; +import { tryCatch } from "@trigger.dev/core/utils";
🧹 Nitpick comments (8)
apps/webapp/app/presenters/v3/LimitsPresenter.server.ts (3)
120-120: Hardcoded plan code is fragile.The plan code
"v3_pro_1"is hardcoded to determineisOnTopPlan. Consider extracting this to a constant or configuration, or querying the billing service for "top plan" status directly to avoid maintenance issues when plans change.
344-362: Consider extracting shared config resolution logic.
resolveBatchRateLimitConfigduplicates the logic frombatchLimits.server.ts. While acceptable for a read-only presenter, consider extracting to a shared module if this pattern expands.
364-383: Consider using zod for batch concurrency config validation.The manual type checking with
as Record<string, unknown>could be replaced with zod validation for consistency with other config resolution functions in the codebase. As per coding guidelines, zod is preferred for validation in the webapp.Suggested zod-based implementation
+import { z } from "zod"; + +const BatchConcurrencyConfigSchema = z.object({ + processingConcurrency: z.number(), +}); + function resolveBatchConcurrencyConfig(batchConcurrencyConfig?: unknown): { processingConcurrency: number; } { const defaultConfig = { processingConcurrency: env.BATCH_CONCURRENCY_LIMIT_DEFAULT, }; if (!batchConcurrencyConfig) { return defaultConfig; } - if (typeof batchConcurrencyConfig === "object" && batchConcurrencyConfig !== null) { - const config = batchConcurrencyConfig as Record<string, unknown>; - if (typeof config.processingConcurrency === "number") { - return { processingConcurrency: config.processingConcurrency }; - } - } + const parsed = BatchConcurrencyConfigSchema.safeParse(batchConcurrencyConfig); + if (parsed.success) { + return { processingConcurrency: parsed.data.processingConcurrency }; + } return defaultConfig; }apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsx (5)
88-92: Consider sanitizing error messages before exposing to clients.Throwing
error.messagedirectly in the response body could expose internal implementation details or sensitive information. Consider using a generic error message or sanitizing the error.if (error) { - throw new Response(error.message, { + throw new Response("Failed to load limits data", { status: 400, }); }
393-409: Fragile string comparison for conditional logic.The check
info.name === "Batch rate limit"is fragile—if the name changes in the presenter, this logic silently breaks. Consider adding a discriminator property (e.g.,info.keyorinfo.id) toRateLimitInfofor reliable identification.
510-518: Clarify intent ofcurrentUsage: 0for batch concurrency quota.The hardcoded
currentUsage: 0may confuse future maintainers. If batch concurrency doesn't have trackable usage, consider adding a comment explaining why, or use a flag likeshowUsage: falsein the type definition.quotaRows.push({ name: "Batch processing concurrency", description: "Controls how many batch items can be processed simultaneously.", limit: batchConcurrency.limit, - currentUsage: 0, + currentUsage: 0, // Batch concurrency is a limit config, not a countable resource source: batchConcurrency.source, canExceed: true, // Allow contact us on top plan, view plans otherwise isUpgradable: true, });
569-574: Fragile string-based type detection for quota display logic.Both
isRetentionQuotaand the "Projects" check (line 611) rely on string comparisons. Consider adding aquotaTypeorkeydiscriminator toQuotaInfofor more robust identification.
741-748: Add comment clarifying cents-to-dollars conversion.The division by 100 assumes the value is in cents. A brief comment would help maintainability:
if (feature.name === "Included compute" && typeof feature.value === "number") { if (!feature.enabled || feature.value === 0) { return <span className="text-text-dimmed">None</span>; } + // value is stored in cents return ( <span className="font-medium text-text-bright">${formatNumber(feature.value / 100)}</span> ); }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlreferences/hello-world/src/trigger/rateLimitStress.tsis excluded by!references/**
📒 Files selected for processing (10)
apps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/presenters/v3/LimitsPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsxapps/webapp/app/runEngine/concerns/batchLimits.server.tsapps/webapp/app/services/authorizationRateLimitMiddleware.server.tsapps/webapp/app/utils/pathBuilder.tsapps/webapp/package.jsonapps/webapp/tailwind.config.jsinternal-packages/tsql/src/query/printer.test.tsinternal-packages/tsql/src/query/printer.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/webapp/tailwind.config.js
- internal-packages/tsql/src/query/printer.test.ts
- internal-packages/tsql/src/query/printer.ts
- apps/webapp/app/components/navigation/SideMenu.tsx
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/package.jsonapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/services/authorizationRateLimitMiddleware.server.tsapps/webapp/app/runEngine/concerns/batchLimits.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsxapps/webapp/app/presenters/v3/LimitsPresenter.server.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
apps/webapp/app/utils/pathBuilder.tsapps/webapp/app/services/authorizationRateLimitMiddleware.server.tsapps/webapp/app/runEngine/concerns/batchLimits.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsxapps/webapp/app/presenters/v3/LimitsPresenter.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/utils/pathBuilder.tsapps/webapp/app/services/authorizationRateLimitMiddleware.server.tsapps/webapp/app/runEngine/concerns/batchLimits.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsxapps/webapp/app/presenters/v3/LimitsPresenter.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
apps/webapp/app/utils/pathBuilder.tsapps/webapp/app/services/authorizationRateLimitMiddleware.server.tsapps/webapp/app/runEngine/concerns/batchLimits.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsxapps/webapp/app/presenters/v3/LimitsPresenter.server.ts
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/utils/pathBuilder.tsapps/webapp/app/services/authorizationRateLimitMiddleware.server.tsapps/webapp/app/runEngine/concerns/batchLimits.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsxapps/webapp/app/presenters/v3/LimitsPresenter.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webappAccess environment variables via
envexport fromapps/webapp/app/env.server.ts, never useprocess.envdirectly
Files:
apps/webapp/app/utils/pathBuilder.tsapps/webapp/app/services/authorizationRateLimitMiddleware.server.tsapps/webapp/app/runEngine/concerns/batchLimits.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsxapps/webapp/app/presenters/v3/LimitsPresenter.server.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/app/utils/pathBuilder.tsapps/webapp/app/services/authorizationRateLimitMiddleware.server.tsapps/webapp/app/runEngine/concerns/batchLimits.server.tsapps/webapp/app/presenters/v3/LimitsPresenter.server.ts
apps/webapp/app/services/**/*.server.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Separate testable services from configuration files; follow the pattern of
realtimeClient.server.ts(testable service) andrealtimeClientGlobal.server.ts(configuration) in the webapp
Files:
apps/webapp/app/services/authorizationRateLimitMiddleware.server.ts
🧠 Learnings (17)
📚 Learning: 2025-11-14T16:03:06.917Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2681
File: apps/webapp/app/services/platform.v3.server.ts:258-302
Timestamp: 2025-11-14T16:03:06.917Z
Learning: In `apps/webapp/app/services/platform.v3.server.ts`, the `getDefaultEnvironmentConcurrencyLimit` function intentionally throws an error (rather than falling back to org.maximumConcurrencyLimit) when the billing client returns undefined plan limits. This fail-fast behavior prevents users from receiving more concurrency than their plan entitles them to. The org.maximumConcurrencyLimit fallback is only for self-hosted deployments where no billing client exists.
Applied to files:
apps/webapp/app/utils/pathBuilder.tsapps/webapp/app/presenters/v3/LimitsPresenter.server.ts
📚 Learning: 2026-01-12T17:18:09.451Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2870
File: apps/webapp/app/services/redisConcurrencyLimiter.server.ts:56-66
Timestamp: 2026-01-12T17:18:09.451Z
Learning: In `apps/webapp/app/services/redisConcurrencyLimiter.server.ts`, the query concurrency limiter will not be deployed with Redis Cluster mode, so multi-key operations (keyKey and globalKey in different hash slots) are acceptable and will function correctly in standalone Redis mode.
Applied to files:
apps/webapp/app/services/authorizationRateLimitMiddleware.server.tsapps/webapp/app/runEngine/concerns/batchLimits.server.ts
📚 Learning: 2025-12-08T15:19:56.823Z
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.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsx
📚 Learning: 2025-11-27T16:26:58.661Z
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
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsx
📚 Learning: 2025-11-27T16:26:58.661Z
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
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsx
📚 Learning: 2026-01-15T11:50:06.044Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T11:50:06.044Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Import from `trigger.dev/core` using subpaths only, never import from root
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsx
📚 Learning: 2025-11-27T16:27:35.304Z
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
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsx
📚 Learning: 2025-11-27T16:26:37.432Z
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
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsx
📚 Learning: 2026-01-15T11:50:06.044Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T11:50:06.044Z
Learning: Applies to **/*.{ts,tsx} : Always import tasks from `trigger.dev/sdk`, never use `trigger.dev/sdk/v3` or deprecated `client.defineJob` pattern
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsx
📚 Learning: 2025-11-27T16:27:35.304Z
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
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsx
📚 Learning: 2025-11-27T16:26:58.661Z
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
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsx
📚 Learning: 2026-01-15T11:50:06.044Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T11:50:06.044Z
Learning: Applies to **/*.{ts,tsx} : Every Trigger.dev task must be exported and have a unique `id` property with no timeouts in the run function
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsx
📚 Learning: 2025-11-27T16:26:58.661Z
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/v3/presenters/**/*.server.{ts,tsx} : Organize presenters in the webapp following the pattern `app/v3/presenters/*/*.server.ts` to move complex loader code into classes
Applied to files:
apps/webapp/app/presenters/v3/LimitsPresenter.server.ts
📚 Learning: 2025-06-06T23:55:01.933Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2155
File: hosting/docker/.env.example:4-7
Timestamp: 2025-06-06T23:55:01.933Z
Learning: In the trigger.dev project, .env.example files should contain actual example secret values rather than placeholders, as these help users understand the expected format. The files include clear warnings about not using these defaults in production and instructions for generating proper secrets.
Applied to files:
apps/webapp/app/presenters/v3/LimitsPresenter.server.ts
📚 Learning: 2025-06-25T14:14:11.965Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2195
File: hosting/k8s/helm/values-production-example.yaml:95-102
Timestamp: 2025-06-25T14:14:11.965Z
Learning: In the Trigger.dev Helm chart production examples, the maintainer prefers to include initial/bootstrap credentials with clear warnings that they should be changed after first login, rather than requiring external secret references that could complicate initial setup. This follows their pattern of providing working examples with explicit security guidance.
Applied to files:
apps/webapp/app/presenters/v3/LimitsPresenter.server.ts
📚 Learning: 2025-05-27T16:23:50.632Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2086
File: apps/webapp/app/routes/api.v1.projects.$projectRef.branches.archive.ts:47-65
Timestamp: 2025-05-27T16:23:50.632Z
Learning: In the RuntimeEnvironment model, multiple branches can have the same branchName, but only one active (non-archived) branch per name is allowed at any given time. This constraint is enforced in application code rather than database constraints.
Applied to files:
apps/webapp/app/presenters/v3/LimitsPresenter.server.ts
📚 Learning: 2025-07-12T18:06:04.133Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2264
File: apps/webapp/app/services/runsRepository.server.ts:172-174
Timestamp: 2025-07-12T18:06:04.133Z
Learning: In apps/webapp/app/services/runsRepository.server.ts, the in-memory status filtering after fetching runs from Prisma is intentionally used as a workaround for ClickHouse data delays. This approach is acceptable because the result set is limited to a maximum of 100 runs due to pagination, making the performance impact negligible.
Applied to files:
apps/webapp/app/presenters/v3/LimitsPresenter.server.ts
🧬 Code graph analysis (2)
apps/webapp/app/services/authorizationRateLimitMiddleware.server.ts (1)
apps/webapp/app/services/rateLimiter.server.ts (1)
Limiter(16-16)
apps/webapp/app/runEngine/concerns/batchLimits.server.ts (1)
apps/webapp/app/services/authorizationRateLimitMiddleware.server.ts (1)
createLimiterFromConfig(137-143)
⏰ 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 (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 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 (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 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 (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 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 (8, 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 - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (17)
apps/webapp/app/utils/pathBuilder.ts (1)
510-516: LGTM!The
limitsPathfunction follows the established pattern used byconcurrencyPathandregionsPath, maintaining consistency with the existing path builder conventions.apps/webapp/app/services/authorizationRateLimitMiddleware.server.ts (2)
133-143: LGTM! Good centralization of limiter creation logic.The new helper function properly handles all three limiter types from the
RateLimiterConfigdiscriminated union. TypeScript's exhaustive type checking ensures this remains correct if new types are added.
264-264: Clean refactor to use the centralized helper.apps/webapp/app/runEngine/concerns/batchLimits.server.ts (2)
4-7: LGTM! Good alignment with centralized limiter creation.The import change properly adopts the shared
createLimiterFromConfighelper, eliminating duplicate limiter construction logic.
38-38: Clean refactor using the shared helper.apps/webapp/app/presenters/v3/LimitsPresenter.server.ts (4)
15-25: LGTM! Proper singleton pattern for Redis client.The singleton setup follows the established pattern and correctly uses the
envexport for configuration as per coding guidelines.
27-82: Well-structured types following coding guidelines.The types use
typeinstead ofinterfaceand string unions instead of enums, consistent with the coding guidelines.
395-399: SHA-256 hash usage is appropriate here.The static analysis warning about "insufficient computational effort" is a false positive in this context. SHA-256 is used here for creating a deterministic lookup key for rate limiting, not for password storage. This matches the established pattern in
authorizationRateLimitMiddleware.server.ts.
401-421: LGTM! Proper rate limit token lookup with error handling.The function correctly creates a Ratelimit instance with matching configuration and gracefully handles failures by returning
nullwith appropriate logging.apps/webapp/package.json (1)
114-114: No action required — icon usage is compatible with v3.The imported icons (IconCardsFilled, IconDiamondFilled, IconTallymark4, IconRotateClockwise2, IconToggleLeft) already follow
@tabler/icons-reactv3 naming conventions. All instances use className for styling rather than deprecated props like strokeWidth, ensuring compatibility. No code changes are needed.apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.limits/route.tsx (7)
103-180: Well-structured page component with proper data flow.The page component follows Remix conventions correctly with typed loader data, auto-revalidation, and clean separation into section components.
182-218: LGTM!Clean component with appropriate conditional rendering for plan upgrade actions.
220-245: LGTM!Simple and focused component that correctly links to the concurrency management page.
247-341: LGTM!Good table structure with informative tooltips explaining rate limit types. The environment selector integration is a nice touch for per-environment visibility.
416-446: LGTM!Flexible badge component that handles both direct type prop and config object extraction cleanly.
448-482: LGTM!Clean display logic for different rate limit configuration types with appropriate formatting.
800-853: LGTM!Well-documented utility function with clear threshold logic.
SourceBadgecorrectly uses a const object pattern instead of enums per coding guidelines.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
