feat(webapp): admin editor for org concurrency quota#3515
feat(webapp): admin editor for org concurrency quota#3515isshaddad wants to merge 2 commits intofeat/admin-back-office-org-limitsfrom
Conversation
|
WalkthroughThis pull request implements an admin back-office feature for managing organization concurrency quota caps. It includes a new Zod-validated server action handler that accepts form submissions, calls a platform billing service method ( Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes DetailsThe review requires examination of validation schemas and error-handling paths across multiple layers (client validation, server-side form validation, platform API error mapping), state management in a controlled React form component, integration of the server action into the route's loader/action handlers, and correctness of the platform service method. The changes span six files with mixed frontend and backend concerns, introducing new logic patterns for quota management without extreme complexity in any single area. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.server.ts (1)
51-55: 💤 Low valueRedundant cast — TypeScript already narrows
resulthere.Inside
if (!result.success),resultis already narrowed by the discriminated union to{ success: false; error: string; code?: string }. The explicit cast serves no type-safety purpose.♻️ Simplify by removing the cast
if (!result.success) { - // The platform client's generic error path strips `code` to `error` only - // until the BillingClient.fetch passthrough fix lands; cast keeps the - // route forward-compatible so precise UI copy renders automatically once - // it does. - const err = result as { - success: false; - error: string; - code?: string; - }; return { ok: false, errors: {}, - formError: mapCodeToMessage(err.code, err.error), + formError: mapCodeToMessage(result.code, result.error), }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.server.ts` around lines 51 - 55, The cast is redundant because inside the if (!result.success) branch TypeScript already narrows result to the failure shape; remove the explicit cast and stop assigning result to a typed variable like const err = result as { success: false; error: string; code?: string }; instead use result (or const err = result) directly to read error/code within the ConcurrencyQuotaSection.server.ts logic, keeping the discriminated-union narrowing (references: variable result and the if (!result.success) branch).apps/webapp/app/routes/admin.back-office.orgs.$orgId.tsx (1)
191-194: 💤 Low valueType assertion on
actionDataforformErrorcan be made narrower.The
actionData as { formError?: string | null }cast widens all action return shapes. The"formError" in actionDataguard already ensures the property exists, so the cast is safe today, but it bypasses TypeScript's inference. A typed discriminated union or a typed helper would be more maintainable if more sections addformErrorin the future. Not a blocker given the existingerrorSectionguard at line 259.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/app/routes/admin.back-office.orgs`.$orgId.tsx around lines 191 - 194, The current broad cast (actionData as { formError?: string | null }) widens actionData; replace it with a narrow user-defined type guard or discriminated union so TypeScript can infer the shape without a blanket assertion. Implement a predicate like isActionDataWithFormError(actionData): actionData is { formError?: string | null } (or update the action return type to a union that includes { formError?: string | null }), then use that guard in the existing `"formError" in actionData` check to safely assign formError; reference the variables actionData, formError and the existing errorSection guard when adding the type guard.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.server.ts`:
- Around line 51-55: The cast is redundant because inside the if
(!result.success) branch TypeScript already narrows result to the failure shape;
remove the explicit cast and stop assigning result to a typed variable like
const err = result as { success: false; error: string; code?: string }; instead
use result (or const err = result) directly to read error/code within the
ConcurrencyQuotaSection.server.ts logic, keeping the discriminated-union
narrowing (references: variable result and the if (!result.success) branch).
In `@apps/webapp/app/routes/admin.back-office.orgs`.$orgId.tsx:
- Around line 191-194: The current broad cast (actionData as { formError?:
string | null }) widens actionData; replace it with a narrow user-defined type
guard or discriminated union so TypeScript can infer the shape without a blanket
assertion. Implement a predicate like isActionDataWithFormError(actionData):
actionData is { formError?: string | null } (or update the action return type to
a union that includes { formError?: string | null }), then use that guard in the
existing `"formError" in actionData` check to safely assign formError; reference
the variables actionData, formError and the existing errorSection guard when
adding the type guard.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a23156cc-55db-4eb0-8ca3-8f942f5186f1
📒 Files selected for processing (6)
.server-changes/admin-concurrency-quota.mdapps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.server.tsapps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.tsxapps/webapp/app/routes/admin.back-office.orgs.$orgId.tsxapps/webapp/app/routes/admin.tsxapps/webapp/app/services/platform.v3.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). (2)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{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
Files:
apps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.tsxapps/webapp/app/routes/admin.tsxapps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.server.tsapps/webapp/app/services/platform.v3.server.tsapps/webapp/app/routes/admin.back-office.orgs.$orgId.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/components/admin/backOffice/ConcurrencyQuotaSection.tsxapps/webapp/app/routes/admin.tsxapps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.server.tsapps/webapp/app/services/platform.v3.server.tsapps/webapp/app/routes/admin.back-office.orgs.$orgId.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.tsxapps/webapp/app/routes/admin.tsxapps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.server.tsapps/webapp/app/services/platform.v3.server.tsapps/webapp/app/routes/admin.back-office.orgs.$orgId.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: Access environment variables through theenvexport ofenv.server.tsinstead of directly accessingprocess.env
Use subpath exports from@trigger.dev/corepackage instead of importing from the root@trigger.dev/corepathUse named constants for sentinel/placeholder values (e.g.
const UNSET_VALUE = '__unset__') instead of raw string literals scattered across comparisons
Files:
apps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.tsxapps/webapp/app/routes/admin.tsxapps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.server.tsapps/webapp/app/services/platform.v3.server.tsapps/webapp/app/routes/admin.back-office.orgs.$orgId.tsx
apps/webapp/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Only use
useCallback/useMemofor context provider values, expensive derived data that is a dependency elsewhere, or stable refs required by a dependency array. Don't wrap ordinary event handlers or trivial computations
Files:
apps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.tsxapps/webapp/app/routes/admin.tsxapps/webapp/app/routes/admin.back-office.orgs.$orgId.tsx
{apps,internal-packages}/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
pnpm run typecheckto verify changes in apps and internal packages (apps/*,internal-packages/*) instead ofbuild, which proves almost nothing about correctness
Files:
apps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.tsxapps/webapp/app/routes/admin.tsxapps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.server.tsapps/webapp/app/services/platform.v3.server.tsapps/webapp/app/routes/admin.back-office.orgs.$orgId.tsx
{package.json,**/*.{ts,tsx,js}}
📄 CodeRabbit inference engine (CLAUDE.md)
Pin Zod to version 3.25.76 exactly across the entire monorepo - never use a different version or version range
Files:
apps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.tsxapps/webapp/app/routes/admin.tsxapps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.server.tsapps/webapp/app/services/platform.v3.server.tsapps/webapp/app/routes/admin.back-office.orgs.$orgId.tsx
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: Import from@trigger.dev/coreusing subpaths only, never the root export
Always import tasks from@trigger.dev/sdk, never from@trigger.dev/sdk/v3or deprecatedclient.defineJob
Add crumbs to code using//@Crumbscomments or `// `#region` `@crumbsblocks for debug tracing during development
Files:
apps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.tsxapps/webapp/app/routes/admin.tsxapps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.server.tsapps/webapp/app/services/platform.v3.server.tsapps/webapp/app/routes/admin.back-office.orgs.$orgId.tsx
**/*.{ts,tsx,js,jsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Code formatting is enforced using Prettier. Run
pnpm run formatbefore committing
Files:
apps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.tsxapps/webapp/app/routes/admin.tsxapps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.server.tsapps/webapp/app/services/platform.v3.server.tsapps/webapp/app/routes/admin.back-office.orgs.$orgId.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/components/admin/backOffice/ConcurrencyQuotaSection.server.tsapps/webapp/app/services/platform.v3.server.ts
apps/webapp/**/*.server.ts
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/**/*.server.ts: Never userequest.signalfor detecting client disconnects. UsegetRequestAbortSignal()fromapp/services/httpAsyncStorage.server.tsinstead, which is wired directly to Expressres.on('close')and fires reliably
Access environment variables viaenvexport fromapp/env.server.ts. Never useprocess.envdirectly
Always usefindFirstinstead offindUniquein Prisma queries.findUniquehas an implicit DataLoader that batches concurrent calls and has active bugs even in Prisma 6.x (uppercase UUIDs returning null, composite key SQL correctness issues, 5-10x worse performance).findFirstis never batched and avoids this entire class of issues
Files:
apps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.server.tsapps/webapp/app/services/platform.v3.server.ts
🧠 Learnings (6)
📚 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/components/admin/backOffice/ConcurrencyQuotaSection.tsxapps/webapp/app/routes/admin.tsxapps/webapp/app/routes/admin.back-office.orgs.$orgId.tsx
📚 Learning: 2026-03-22T13:26:12.060Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).
Applied to files:
apps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.tsxapps/webapp/app/routes/admin.tsxapps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.server.tsapps/webapp/app/services/platform.v3.server.tsapps/webapp/app/routes/admin.back-office.orgs.$orgId.tsx
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.
Applied to files:
apps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.tsxapps/webapp/app/routes/admin.tsxapps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.server.tsapps/webapp/app/services/platform.v3.server.tsapps/webapp/app/routes/admin.back-office.orgs.$orgId.tsx
📚 Learning: 2026-04-16T14:21:15.229Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3368
File: apps/webapp/app/components/logs/LogsTaskFilter.tsx:135-163
Timestamp: 2026-04-16T14:21:15.229Z
Learning: When rendering lists of task registry items in apps/webapp (e.g., <SelectItem /> rows) and using `key={item.slug}`, do not flag it as potentially non-unique. In trigger.dev’s `TaskIdentifier` table, the DB constraint `@unique([runtimeEnvironmentId, slug])` guarantees `slug` is unique within a given runtime environment, so `item.slug` is safe as the React key as long as the list is derived from that registry/constraint (and not from a legacy query that could produce duplicate slugs).
Applied to files:
apps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.tsx
📚 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/admin.tsxapps/webapp/app/routes/admin.back-office.orgs.$orgId.tsx
📚 Learning: 2026-03-26T09:02:07.973Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 3274
File: apps/webapp/app/services/runsReplicationService.server.ts:922-924
Timestamp: 2026-03-26T09:02:07.973Z
Learning: When parsing Trigger.dev task run annotations in server-side services, keep `TaskRun.annotations` strictly conforming to the `RunAnnotations` schema from `trigger.dev/core/v3`. If the code already uses `RunAnnotations.safeParse` (e.g., in a `#parseAnnotations` helper), treat that as intentional/necessary for atomic, schema-accurate annotation handling. Do not recommend relaxing the annotation payload schema or using a permissive “passthrough” parse path, since the annotations are expected to be written atomically in one operation and should not contain partial/legacy payloads that would require a looser parser.
Applied to files:
apps/webapp/app/services/platform.v3.server.ts
🔇 Additional comments (5)
apps/webapp/app/routes/admin.tsx (1)
19-19: LGTM — correct flexbox fix.Adding
flex flex-colto the admin wrapper ensures the<Outlet />can properly fill the remaining vertical space in nested routes..server-changes/admin-concurrency-quota.md (1)
1-11: LGTM — changelog entry accurately describes the feature.apps/webapp/app/services/platform.v3.server.ts (1)
456-479: LGTM — intentional return-on-failure design is correct.Unlike sibling add-on helpers (which return
undefinedon!result.success), returning the error result here is deliberate: it lets the caller surface thecode/errorfields to the admin UI. The downstream handler inConcurrencyQuotaSection.server.tscorrectly distinguishesundefined(client unavailable) from a non-success result (API error).apps/webapp/app/components/admin/backOffice/ConcurrencyQuotaSection.tsx (1)
37-51: LGTM — state initialization and edit-mode lifecycle are correct.
cancelEditcorrectly captures the currentcurrentQuotaprop at call time. The pair ofuseEffects cleanly handles reopening on error and closing on success without stale-closure issues.apps/webapp/app/routes/admin.back-office.orgs.$orgId.tsx (1)
83-88: This concern is unfounded.addOnsis not a new field being introduced in this PR—it is already accessed in multiple places across the codebase (TeamPresenter.server.ts, BranchesPresenter.server.ts, upsertBranch.server.ts, and ManageConcurrencyPresenter.server.ts) using the exact same pattern:v3Subscription?.addOns?.[field]?.purchasedandv3Subscription?.addOns?.[field]?.quota. The code at lines 84–87 follows this established convention, and ifaddOnswere untyped or missing, TypeScript would fail on all existing usages, not just this one.
Adds a Concurrency quota section to the per-org Back office page (
/admin/back-office/orgs/:orgId). Gives admins a UI to adjust the per-org cap on extra purchasable concurrency that previously had to be set out-of-band. Follows the same*Section.tsx + *Section.server.tspattern as the existing API rate limit / Batch rate limit / Maximum projects sections.Edit mode shows a live preview while typing: the cap delta, already-purchased amount, and resulting headroom after save. If the new cap would drop below already-purchased, an amber warning makes the state explicit.
How it works
BillingClient.setExtraConcurrencyQuotaon the platform client.Branch base
Stacked on top of
feat/admin-back-office-org-limits— this PR targets that branch (notmain) so the diff stays focused on the concurrency-quota work. It builds on the latest admin Back office UI: the*Section.tsx + *Section.server.tspattern and the per-org page intent-dispatch wiring introduced there.Dependencies
Depends on a release of
@trigger.dev/platformthat exportsBillingClient.setExtraConcurrencyQuota.Notes
admin.tsx— addedflex flex-colon the outer wrapper so content isn't clipped at the bottom of the viewport when sections overflow.setExtraConcurrencyQuotainplatform.v3.server.tscasts to a local response type until the platform package exports the method natively. Drops to a clean import in the follow-up version-bump commit.Test plan
codeerror banners render once the platform package's error-passthrough fix is released.