feat(webapp): self serve preview branches and team members#3201
feat(webapp): self serve preview branches and team members#3201
Conversation
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a generic SearchInput component (renamed from LogsSearchInput) with an optional 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🧪 Generate unit tests (beta)
📝 Coding Plan for PR comments
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…ss-route error handling, and fix open redirect vulnerability.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsx (1)
342-349:⚠️ Potential issue | 🟡 MinorGuard against division by zero in progress calculations.
If
limits.limitis ever 0 (e.g., edge case during data loading or misconfigured plan), these calculations would produceInfinityorNaN, causing incorrect SVG rendering.🛡️ Proposed fix
<circle className={`fill-none ${requiresUpgrade ? "stroke-error" : "stroke-success"}`} strokeWidth="4" r="10" cx="12" cy="12" - strokeDasharray={`${(limits.used / limits.limit) * 62.8} 62.8`} + strokeDasharray={`${limits.limit > 0 ? (limits.used / limits.limit) * 62.8 : 0} 62.8`} strokeDashoffset="0" strokeLinecap="round" /> </svg> </div> } - content={`${Math.round((limits.used / limits.limit) * 100)}%`} + content={`${limits.limit > 0 ? Math.round((limits.used / limits.limit) * 100) : 0}%`} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/routes/_app.orgs`.$organizationSlug.settings.team/route.tsx around lines 342 - 349, The progress calculations use limits.used / limits.limit directly which can produce Infinity/NaN when limits.limit is 0; update the expressions in the SVG strokeDasharray and the content percent (references: strokeDasharray={`${(limits.used / limits.limit) * 62.8} 62.8`} and content={`${Math.round((limits.used / limits.limit) * 100)}%`}) to guard against division-by-zero by using a safe denominator (e.g., denom = limits.limit > 0 ? limits.limit : 1), compute the ratio = Math.min(Math.max(limits.used / denom, 0), 1) and then use ratio * 62.8 for strokeDasharray and Math.round(ratio * 100) + '%' for content so the SVG and displayed percent remain valid.
🧹 Nitpick comments (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.tsx (1)
655-655: Remove unused variable.The
organizationvariable is declared but never used in this component.🧹 Proposed fix
function PurchaseBranchesModal({ ... }) { const lastSubmission = useActionData(); - const organization = useOrganization(); const [form, { amount }] = useForm({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.tsx at line 655, The variable "organization" is declared via useOrganization() but never used; remove the unused declaration to clean up the component—delete the line containing "const organization = useOrganization();" (or if you intended to use it, reference "organization" where needed); ensure no other code depends on useOrganization() in this file (search for useOrganization and organization) before removing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/webapp/app/routes/_app.orgs`.$organizationSlug.settings.team/route.tsx:
- Around line 342-349: The progress calculations use limits.used / limits.limit
directly which can produce Infinity/NaN when limits.limit is 0; update the
expressions in the SVG strokeDasharray and the content percent (references:
strokeDasharray={`${(limits.used / limits.limit) * 62.8} 62.8`} and
content={`${Math.round((limits.used / limits.limit) * 100)}%`}) to guard against
division-by-zero by using a safe denominator (e.g., denom = limits.limit > 0 ?
limits.limit : 1), compute the ratio = Math.min(Math.max(limits.used / denom,
0), 1) and then use ratio * 62.8 for strokeDasharray and Math.round(ratio * 100)
+ '%' for content so the SVG and displayed percent remain valid.
---
Nitpick comments:
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.tsx:
- Line 655: The variable "organization" is declared via useOrganization() but
never used; remove the unused declaration to clean up the component—delete the
line containing "const organization = useOrganization();" (or if you intended to
use it, reference "organization" where needed); ensure no other code depends on
useOrganization() in this file (search for useOrganization and organization)
before removing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 53ed5d07-80d8-4b21-bd60-31c124dd1422
📒 Files selected for processing (3)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsxapps/webapp/app/v3/services/setSeatsAddOn.server.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/webapp/app/v3/services/setSeatsAddOn.server.ts
📜 Review details
⏰ 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). (25)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 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 (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: typecheck / typecheck
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{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}: In TypeScript SDK usage, always import from@trigger.dev/sdk, never from@trigger.dev/sdk/v3or use deprecated client.defineJob
Import from@trigger.dev/coresubpaths only, never from the root
Use the Run Engine 2.0 (@internal/run-engine) and redis-worker for all new work, not legacy V1 MarQS queue or deprecated V1 functions
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.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/routes/_app.orgs.$organizationSlug.settings.team/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.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 webapp
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.tsx
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/routes/_app.orgs.$organizationSlug.settings.team/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.tsx
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.tsx
apps/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
When modifying only server components (apps/webapp/, apps/supervisor/, etc.) with no package changes, add a .server-changes/ file instead of a changeset
Files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.tsx
🧠 Learnings (24)
📓 Common learnings
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3200
File: docs/config/config-file.mdx:353-368
Timestamp: 2026-03-10T12:44:19.869Z
Learning: In the triggerdotdev/trigger.dev repository, docs PRs are often written as companions to implementation PRs (e.g., PR `#3200` documents features being added in PR `#3196`). When reviewing docs PRs, the documented features may exist in a companion/companion PR branch rather than main. Always check companion PRs referenced in the PR description before flagging missing implementations.
📚 Learning: 2026-02-03T18:27:40.429Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx:553-555
Timestamp: 2026-02-03T18:27:40.429Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx, the menu buttons (e.g., Edit with PencilSquareIcon) in the TableCellMenu are intentionally icon-only with no text labels as a compact UI pattern. This is a deliberate design choice for this route; preserve the icon-only behavior for consistency in this file.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.tsx
📚 Learning: 2026-02-04T16:34:48.876Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/routes/vercel.connect.tsx:13-27
Timestamp: 2026-02-04T16:34:48.876Z
Learning: In apps/webapp/app/routes/vercel.connect.tsx, configurationId may be absent for "dashboard" flows but must be present for "marketplace" flows. Enforce this with a Zod superRefine and pass installationId to repository methods only when configurationId is defined (omit the field otherwise).
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/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.settings.team/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.tsx
📚 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.settings.team/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.tsx
📚 Learning: 2026-03-10T17:56:26.581Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3201
File: apps/webapp/app/v3/services/setSeatsAddOn.server.ts:25-29
Timestamp: 2026-03-10T17:56:26.581Z
Learning: In the `triggerdotdev/trigger.dev` webapp, service classes such as `SetSeatsAddOnService` and `SetBranchesAddOnService` do NOT need to perform their own userId-to-organizationId authorization checks. Auth is enforced at the route layer: `requireUserId(request)` authenticates the user, and the `_app.orgs.$organizationSlug` layout route enforces that the authenticated user is a member of the org. Any `userId` and `organizationId` reaching these services from org-scoped routes are already validated. This is the consistent pattern used across all org-scoped services in the codebase.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsx
📚 Learning: 2026-02-11T16:50:14.167Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.$dashboardId.widgets.tsx:126-131
Timestamp: 2026-02-11T16:50:14.167Z
Learning: In apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.$dashboardId.widgets.tsx, MetricsDashboard entities are intentionally scoped to the organization level, not the project level. The dashboard lookup should filter by organizationId only (not projectId), allowing dashboards to be accessed across projects within the same organization. The optional projectId field on MetricsDashboard serves other purposes and should not be used as an authorization constraint.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/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.settings.team/route.tsx
📚 Learning: 2026-03-02T12:43:37.906Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: packages/core/CLAUDE.md:0-0
Timestamp: 2026-03-02T12:43:37.906Z
Learning: Applies to packages/core/**/*.{ts,tsx,js,jsx} : Never import the root package (trigger.dev/core). Always use subpath imports such as trigger.dev/core/v3, trigger.dev/core/v3/utils, trigger.dev/core/logger, or trigger.dev/core/schemas
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsx
📚 Learning: 2026-03-02T12:42:41.110Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-02T12:42:41.110Z
Learning: Applies to **/*.{ts,tsx} : Import from trigger.dev/core subpaths only, never from the root
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/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.settings.team/route.tsx
📚 Learning: 2026-03-02T12:43:48.124Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: packages/trigger-sdk/CLAUDE.md:0-0
Timestamp: 2026-03-02T12:43:48.124Z
Learning: Applies to packages/trigger-sdk/**/*.{ts,tsx,js,jsx} : Always import from `trigger.dev/sdk`. Never use `trigger.dev/sdk/v3` (deprecated path alias)
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsx
📚 Learning: 2026-03-02T12:42:41.110Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-02T12:42:41.110Z
Learning: Applies to apps/webapp/app/v3/**/*.{ts,tsx} : In apps/webapp/app/v3/ directory, only modify V2 code paths - V1/V2 branching should only have V2 modified, as V1 code is legacy
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/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.settings.team/route.tsx
📚 Learning: 2026-03-02T12:42:41.110Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-02T12:42:41.110Z
Learning: Applies to **/*.{ts,tsx} : In TypeScript SDK usage, always import from trigger.dev/sdk, never from trigger.dev/sdk/v3 or use deprecated client.defineJob
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/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/core,apps/webapp}/**/*.{ts,tsx} : Use zod for validation in packages/core and apps/webapp
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsx
📚 Learning: 2026-01-28T16:57:47.620Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 2964
File: apps/webapp/app/components/AskAI.tsx:121-141
Timestamp: 2026-01-28T16:57:47.620Z
Learning: In the trigger.dev webapp codebase, the Button component (apps/webapp/app/components/primitives/Buttons) does not spread unknown props to the underlying <button> element—it only passes specific props (type, disabled, onClick, name, value, ref, form, autoFocus). When using Radix UI's TooltipTrigger with asChild, a span wrapper around the Button is necessary to receive Radix props (aria-describedby, onPointerEnter, onPointerLeave, data-state) while the Button handles its own behavior. Directly making the Button the child of TooltipTrigger with asChild will break accessibility and tooltip functionality.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/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 `schemaTask()` from `trigger.dev/sdk/v3` with Zod schema for payload validation
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsx
📚 Learning: 2026-02-19T18:09:23.944Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3095
File: apps/webapp/app/components/navigation/DashboardDialogs.tsx:47-47
Timestamp: 2026-02-19T18:09:23.944Z
Learning: In the triggerdotdev/trigger.dev codebase, the pattern `isPaying === false` is used consistently to explicitly check for free plan users. This is a project convention that distinguishes between `isPaying === true` (paying), `isPaying === false` (free), and `isPaying === undefined` (no subscription data). Do not suggest changing this to negation pattern.
```
<!-- <review_comment_addressed>
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsx
📚 Learning: 2026-03-02T12:43:02.539Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: docs/CLAUDE.md:0-0
Timestamp: 2026-03-02T12:43:02.539Z
Learning: Applies to docs/**/*.mdx : Always import from `trigger.dev/sdk` in code examples (never from `trigger.dev/sdk/v3`)
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/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.settings.team/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 logger methods (debug, log, info, warn, error) from `trigger.dev/sdk/v3` for structured logging in tasks
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsx
📚 Learning: 2026-02-11T16:37:32.429Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/components/primitives/charts/Card.tsx:26-30
Timestamp: 2026-02-11T16:37:32.429Z
Learning: In projects using react-grid-layout, avoid relying on drag-handle class to imply draggability. Ensure drag-handle elements only affect dragging when the parent grid item is configured draggable in the layout; conditionally apply cursor styles based on the draggable prop. This improves correctness and accessibility.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.tsx
📚 Learning: 2025-07-12T18:00:06.163Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2264
File: apps/webapp/app/utils/searchParams.ts:16-18
Timestamp: 2025-07-12T18:00:06.163Z
Learning: The `objectToSearchParams` function in `apps/webapp/app/utils/searchParams.ts` is used to generate URL parameters from objects and is separate from code that parses incoming search parameters. Changes to this function only affect places where it's used to create URLs, not places that parse search parameters from external sources.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.tsx
🔇 Additional comments (15)
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsx (6)
1-63: LGTM!Imports are correctly structured. The
@trigger.dev/core/utilssubpath import fortryCatchaligns with project conventions.
129-166: Authorization is already enforced at the layout layer.The past review flagged a missing auth check here, but per the codebase pattern, the
_app.orgs.$organizationSluglayout route already enforces that the authenticated user (fromrequireUserId) is a member of the organization. This is the consistent pattern used across all org-scoped services. Based on learnings from this repository, service classes such asSetSeatsAddOnServicedo NOT need to perform their own userId-to-organizationId authorization checks.
486-544: LGTM!The cooldown implementation is well-structured with proper cleanup. The initial cooldown calculation from
invite.updatedAtprevents spam after page refresh, and the interval-based countdown with cleanup is correctly implemented.
546-569: LGTM!The accessibility concern from past reviews has been addressed with
aria-label="Revoke invite"on line 560.
571-588: LGTM!The
triggerButtonprop type has been correctly updated toReact.ReactElementto satisfy Radix'sDialogTriggerwithasChild, addressing the previous review concern.
811-834: LGTM!The state logic correctly handles all seat change scenarios with proper boundary checks. The string union return type aligns with the project's preference for string unions over enums.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.branches/route.tsx (9)
1-80: LGTM!Imports are well-organized and compliant with coding guidelines. The
@trigger.dev/core/v3subpath import is correctly used.
128-137: LGTM!The discriminated union schema cleanly separates the two action types with appropriate validation constraints.
139-191: LGTM!The purchase-branches action handler properly validates input, handles errors from the service layer, and provides appropriate success/error feedback. The use of
tryCatchand discriminated error handling is clean.
226-529: LGTM!The page component cleanly integrates the new purchase flow with appropriate conditional rendering. The guard
canPurchaseBranches && branchPricingensures the modal only renders when pricing data is available.
531-558: LGTM!The refactored
BranchFilterscomponent correctly usesSearchInputwithresetParamsto handle pagination reset. TheuseCallbackwith an empty dependency array is fine sincesetSearchParamsis stable.
560-634: LGTM!The
UpgradePanelcomponent cleanly handles the branching logic between purchase modal and upgrade dialog based on plan capabilities.
636-866: Overall implementation is solid.The modal correctly handles multiple purchase states (increase, decrease, above_quota, need_to_archive) with appropriate UI feedback and pricing calculations. The summary breakdown is clear and user-friendly.
868-891: LGTM!The state calculation logic correctly handles all edge cases. The ordering of checks ensures
need_to_archiveis checked before returningdecrease, andabove_quotaonly applies to increases.
893-977: LGTM!The
NewBranchPanelcomponent remains unchanged and correctly handles the branch creation flow.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| const PurchaseSchema = z.discriminatedUnion("action", [ | ||
| z.object({ | ||
| action: z.literal("purchase"), | ||
| amount: z.coerce.number().min(0, "Amount must be 0 or more"), | ||
| }), | ||
| z.object({ | ||
| action: z.literal("quota-increase"), | ||
| amount: z.coerce.number().min(1, "Amount must be greater than 0"), | ||
| }), | ||
| ]); |
There was a problem hiding this comment.
🔴 Missing .int() validation on branch purchase amount allows fractional values
The PurchaseSchema in the branches route omits .int() on the amount field, allowing non-integer values (e.g., 1.5) to pass validation. The equivalent PurchaseSchema in the team settings route at apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsx:110 correctly uses .int("Must be a whole number"). A fractional amount would pass client-side validation and be sent to the billing API, which likely doesn't expect fractional branch counts.
| const PurchaseSchema = z.discriminatedUnion("action", [ | |
| z.object({ | |
| action: z.literal("purchase"), | |
| amount: z.coerce.number().min(0, "Amount must be 0 or more"), | |
| }), | |
| z.object({ | |
| action: z.literal("quota-increase"), | |
| amount: z.coerce.number().min(1, "Amount must be greater than 0"), | |
| }), | |
| ]); | |
| const PurchaseSchema = z.discriminatedUnion("action", [ | |
| z.object({ | |
| action: z.literal("purchase"), | |
| amount: z.coerce.number().int("Must be a whole number").min(0, "Amount must be 0 or more"), | |
| }), | |
| z.object({ | |
| action: z.literal("quota-increase"), | |
| amount: z.coerce.number().int("Must be a whole number").min(1, "Amount must be greater than 0"), | |
| }), | |
| ]); |
Was this helpful? React with 👍 or 👎 to provide feedback.
| @@ -0,0 +1,100 @@ | |||
| import { BaseService } from "./baseService.server"; | |||
| import { tryCatch } from "@trigger.dev/core"; | |||
There was a problem hiding this comment.
🟡 Import from @trigger.dev/core root path violates repository convention
The file imports tryCatch from @trigger.dev/core (root path). The CLAUDE.md and .cursor/rules/webapp.mdc both explicitly state: "When importing from @trigger.dev/core in the webapp, we never import the root @trigger.dev/core path, instead we favor one of the subpath exports." The sibling file setSeatsAddOn.server.ts:2 correctly imports from @trigger.dev/core/utils.
| import { tryCatch } from "@trigger.dev/core"; | |
| import { tryCatch } from "@trigger.dev/core/utils"; |
Was this helpful? React with 👍 or 👎 to provide feedback.
| const lastSubmission = useActionData(); | ||
| const [form, { amount }] = useForm({ | ||
| id: "purchase-branches", | ||
| lastSubmission: lastSubmission as any, | ||
| onValidate({ formData }) { | ||
| return parse(formData, { schema: PurchaseSchema }); | ||
| }, | ||
| shouldRevalidate: "onSubmit", | ||
| }); | ||
|
|
||
| const [amountValue, setAmountValue] = useState(extraBranches); | ||
| const navigation = useNavigation(); | ||
| const isLoading = navigation.state !== "idle" && navigation.formMethod === "POST"; |
There was a problem hiding this comment.
🚩 PurchaseBranchesModal uses useActionData while PurchaseSeatsModal uses useFetcher — different patterns for same feature
The PurchaseBranchesModal uses useActionData() and a regular <Form> (line 654, 705), while the PurchaseSeatsModal in _app.orgs.$organizationSlug.settings.team/route.tsx:590 uses useFetcher() and <fetcher.Form>. The fetcher approach is superior because: (1) it isolates form state from the page's navigation cycle, (2) it avoids cross-contamination with NewBranchPanel which also calls useActionData() on the same route, and (3) it doesn't require a full-page redirect on success. The branches modal works around this by using a ?purchaseSuccess=true URL param to signal success, while the seats modal cleanly checks fetcher.data.ok. Consider aligning to the fetcher pattern for consistency and robustness.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Adds 2 self serve features
1. self serve preview branches
2. self serve team members