feat(webapp): consolidate auth path + add comprehensive auth tests#3499
Draft
matt-aitken wants to merge 61 commits intomainfrom
Draft
feat(webapp): consolidate auth path + add comprehensive auth tests#3499matt-aitken wants to merge 61 commits intomainfrom
matt-aitken wants to merge 61 commits intomainfrom
Conversation
…Client from public interface - Replace buildBearerAbility/buildSessionAbility with authenticateBearer/authenticateSession - Add RbacEnvironment, RbacUser, BearerAuthResult, SessionAuthResult types - Remove PrismaClient from @trigger.dev/plugins interface (no Prisma crossing repo boundary) - Remove @trigger.dev/database dependency and api-extractor from plugins package - Switch plugins build to tsup --dts, delete api-extractor.json and tsconfig.dts.json - OSS fallback imports PrismaClient from @trigger.dev/database directly - OSS loader passes helpers-only to enterprise plugin, (prisma, helpers) to fallback - Add rbac.server.ts singleton to webapp - PoC: migrate admin.concurrency route to rbac.authenticateSession + canSuper()
…wJWT option, buildJwtAbility
Adds a `forceFallback` option to the RBAC loader so tests (and any other consumer that sets RBAC_FORCE_FALLBACK=1) pin authentication to the OSS fallback regardless of whether the enterprise plugin is installed. - internal-packages/rbac: LazyController and RoleBaseAccess.create now accept RbacCreateOptions.forceFallback. When true, load() skips the dynamic import of @triggerdotdev/plugins/rbac and constructs RoleBaseAccessFallback directly. - apps/webapp env: new RBAC_FORCE_FALLBACK BoolEnv, threaded into app/services/rbac.server.ts. - testcontainers webapp: set RBAC_FORCE_FALLBACK=1 so e2e tests exercise the fallback deterministically. - api-auth.e2e.test.ts: covers the fallback wiring end-to-end via the existing /admin/concurrency route, which already uses rbac.authenticateSession + ability.canSuper().
Close the coverage gap identified in the TRI-8716 audit before TRI-8719 swaps apiBuilder.server.ts to rbac.authenticateBearer. All new tests run against the legacy authenticateApiRequestWithFailure / authenticateApiRequestWithPersonalAccessToken path and must stay green after the migration. - Action requests (createActionApiRoute) via POST /api/v1/idempotencyKeys/:key/reset: * Valid private API key → passes auth (400 on zod body validation, not 401/403). * Missing Authorization → 401. * Invalid API key → 401. - JWT on the same action route (allowJWT: true, superScopes write:runs, admin): * JWT with matching scope → passes auth. * JWT with read-only scope → 403. - Personal access tokens (createLoaderPATApiRoute) via GET /api/v1/projects/:ref/runs: * Missing Authorization → 401. * API key (tr_dev_*) on PAT-only route → 401. * Wrong-prefix or malformed PAT → 401. * Well-formed but unknown PAT → 401. * Revoked PAT → 401. * Valid PAT on unknown project → 404 (auth passes). - Edge case: valid API key whose project.deletedAt is set → 401. Also fix the TRI-8715 redirect assertion: the webapp sends clients to /login?redirectTo=... so compare by pathname rather than exact string. New helper test/helpers/seedTestPAT.ts seeds a User + PersonalAccessToken row using the same hashing/encryption scheme the webapp uses (shared test ENCRYPTION_KEY), so the webapp subprocess can authenticate against the seeded token. otu and realtime.skipColumns propagation are deferred: they're only observable via real trigger / realtime-stream flows, which need workers/streams enabled and are out of scope for a coverage PR. The migration preserves those fields via BearerAuthResult.jwt; dedicated coverage can ride with TRI-8719 if needed.
Close the resource-scoped JWT coverage gap before TRI-8719 swaps
apiBuilder to rbac.authenticateBearer. Target:
POST /api/v1/waitpoints/tokens/:waitpointFriendlyId/complete — allowJWT,
resource: { waitpoints: params.waitpointFriendlyId }, superScopes:
[write:waitpoints, admin].
New helper test/helpers/seedTestWaitpoint.ts seeds a Waitpoint in
COMPLETED status so the handler short-circuits once auth passes, keeping
the 200 assertion independent of run-engine workers.
7 new tests exercise the legacy checkAuthorization scope algebra that
the migration must preserve:
- scope matches exact resource id → 200
- scope targets a different id of the same type → 403
- type-level scope (no id) grants all resources of that type → 200
- read-only scope on a write route → 403
- scope targets a different resource type → 403
- admin super-scope → 200 (legacy super-scope listing)
- unrelated type scope with no super-scope match → 403
Without these, the only JWT coverage was coarse type-level allow/deny
against routes whose resource callbacks returned () => 1 or () => ({}),
leaving resource-id matching entirely untested end-to-end.
Lock in the legacy checkAuthorization behaviours that TRI-8719 must
preserve once it swaps in rbac.authenticateBearer + ability.can.
Three tests in a new describe block 'JWT bearer auth — behaviours to
preserve through TRI-8719':
- Custom action route: type-level write:tasks JWT on
POST /api/v1/tasks/:taskId/trigger (action: trigger) → auth passes
today via exact superScope match. Must keep passing after TRI-8719
via the ACTION_ALIASES map (trigger ← write).
- Multi-key resource: read:tags:<tag> JWT on /api/v1/runs/:runId/trace
where the seeded run has that tag → auth passes today because
legacy checks each resource key. Must keep passing after TRI-8719
via ability.can's array-resource form.
- Multi-key resource: read:batch:<friendlyId> JWT on
/api/v1/runs/:runId/trace where the seeded run is in that batch →
same rationale as the tags case.
Dropped the planned empty-resource test: researching it surfaced that
legacy checkAuthorization denies empty-resource requests BEFORE the
super-scope check runs, so api.v1.batches.ts and idempotencyKeys reset
currently reject all JWTs despite allowJWT: true. TRI-8719's plan
(adding explicit { type: 'runs' }) is an intentional improvement, not
a preservation — documented in the TRI-8719 description comment.
New helper test/helpers/seedTestRun.ts seeds a minimal TaskRun (and,
optionally, an associated BatchTaskRun) that ApiRetrieveRunPresenter's
findRun can resolve for multi-key resource tests. The tests only
assert 'auth passes' (!== 401, !== 403) — the handler's downstream
behaviour (which may fail in a worker-less test env) isn't relevant
to the auth-layer contract.
Foundational changes before swapping apiBuilder to rbac.authenticateBearer. No behaviour change yet — apiBuilder is still on the legacy path. Array resources: - @trigger.dev/plugins RbacAbility.can now accepts RbacResource | RbacResource[]. Array form means 'grant access if any element passes', preserving the legacy checkAuthorization multi-key semantic once TRI-8719 completes. - internal-packages/rbac ability.ts: permissive/super/deny pass through unchanged; buildJwtAbility iterates the array and short-circuits on first match. Action alias wrapper (internal-packages/rbac/src/index.ts): - ACTION_ALIASES map + withActionAliases function. Wraps an underlying RbacAbility so that can(action, resource) retries with alias actions when the direct check fails. Currently: trigger, batchTrigger, update are all satisfied by a scope whose action is write — matching legacy superScope behaviour for route.action values that don't align with scope prefixes. - LazyController wraps the ability it gets from authenticateBearer / authenticateSession. authenticateAuthorize* stop delegating to the underlying's own Authorize methods (that would bypass the wrapper) and instead do the inline ability.can check against the wrapped ability. The enterprise plugin (TRI-8720) does not need to know about aliases — the wrapper applies uniformly regardless of which ability came back. Tests: - ability.test.ts: +4 tests for array resource form (31 total in file). - loader.test.ts: +11 tests for withActionAliases (direct match, alias retry for trigger/batchTrigger/update, id-scoped retry, admin passes, array form retry, canSuper delegation). - Unit suite: 31 tests, all passing. - Webapp typecheck: clean.
…I-8719 Phase B)
Swap all three apiBuilder call sites (loader, action, multi-method) from
authenticateApiRequestWithFailure + checkAuthorization to a single RBAC
plugin bridge. 30 route files migrated in lockstep — drop the
authorization.superScopes option, convert resource callbacks to return
RbacResource or RbacResource[] in the new shape.
Infrastructure:
- apiBuilder: new authenticateRequestForApiBuilder helper funnels all
three builders through rbac.authenticateBearer and follow-up
findEnvironmentById to rebuild the legacy ApiAuthenticationResultSuccess
shape handlers still expect (no handler-facing changes).
- @internal/rbac: re-export RbacAbility, RbacResource from
@trigger.dev/plugins so webapp only depends on @trigger.dev/rbac.
Route-file changes (Risk mitigations from the ticket):
- Custom actions (trigger, batchTrigger, update) unchanged at the route
level — the ACTION_ALIASES wrapper from Phase A handles them
transparently.
- Multi-key runs routes (api.v3.runs.$runId, realtime.v1.runs.$runId,
realtime.v1.streams.$runId.$streamId, api.v1.runs.$runId.events /
.spans.$spanId / .trace, realtime.v1.streams.$runId.input.$streamId
second block, plus the batch-array routes) now return
RbacResource[] — any resource match grants access. Undefined batch
ids are filtered out to avoid accidentally matching a type-level
read:batch scope with no id.
- Empty-resource routes (api.v1.batches, api.v1.idempotencyKeys.$key.reset)
now return { type: 'runs' } — JWTs with read:runs / write:runs start
working where they were previously denied by the legacy
empty-resource short-circuit. Intentional improvement, strict
superset of today's accept set.
- Search-params routes (realtime.v1.runs, api.v1.runs) return an array
with a collection-level { type: 'runs' } plus any filtered tag/task
entries so JWTs that work today continue to work.
Verification:
- pnpm run typecheck --filter webapp: clean.
- pnpm run test --filter @internal/rbac: 31 unit tests pass (wrapper +
array-resource semantics).
- E2E suite (test/api-auth.e2e.test.ts): all 31 tests pass on the new
code path — the three pre-migration 'behaviours to preserve' tests
(type-level write:tasks triggers a task, read:tags:<tag> reaches a
run with that tag, read:batch:<id> reaches a run in that batch) are
still green post-swap.
Packaging:
- .changeset/rbac-plugin-array-resources.md: minor bump for
@trigger.dev/plugins (array-resource overload on RbacAbility.can).
- .server-changes/rbac-apibuilder-migration.md: webapp-only note.
Add a session-auth route builder analogous to apiBuilder.server.ts that
routes dashboard auth through rbac.authenticateSession and runs the
ability check (canSuper or can) before the handler runs. Routes that
only need a logged-in user (no authorisation) keep using requireUser /
requireUserId — the builder is opt-in for routes with explicit auth.
Builder shape:
dashboardLoader({ authorization: { requireSuper: true } }, async ({ user, ability }) => ...)
dashboardLoader({ authorization: { action, resource } }, ...)
dashboardAction(...)
Auth failure throws a redirect Response so the success-path return type
stays narrow (useTypedLoaderData<typeof loader>() picks up the handler's
TypedJsonResponse). Optional context callback feeds organizationId /
projectId to authenticateSession when needed (enterprise-only — fallback
ignores context today).
Migrated 14 platform admin routes from
`requireUser` + `if (!user.admin)` to dashboardLoader / dashboardAction
with requireSuper: true:
admin.tsx
admin._index.tsx
admin.concurrency.tsx
admin.feature-flags.tsx
admin.notifications.tsx
admin.orgs.tsx
admin.data-stores.tsx
admin.back-office.tsx
admin.back-office._index.tsx
admin.back-office.orgs.$orgId.tsx
admin.llm-models._index.tsx
admin.llm-models.$modelId.tsx
admin.llm-models.new.tsx
admin.llm-models.missing._index.tsx
admin.llm-models.missing.$model.tsx
Routes that have admin-only sub-features (e.g. show-extra-fields-if-admin
on otherwise public routes) stay on requireUser. Migration of those is a
separate concern — they don't gate access on admin, they just branch
display.
Behavioural change: action handlers that previously threw
`new Response('Unauthorized', { status: 403 })` on non-admins now redirect
to / along with the loader. Uniform behaviour, but XHR fetchers that
expected a 403 status would now follow the redirect instead. The admin
pages migrated here don't appear to have XHR fetchers that depend on the
403, but worth flagging.
Verification:
- pnpm run typecheck --filter webapp: clean.
- pnpm run test --filter @internal/rbac: 31 unit tests pass.
- E2E suite: all 31 tests pass — including the
/admin/concurrency redirect test (now exercising the new builder).
Widen check.resource on the convenience methods to RbacResource | RbacResource[] so they match RbacAbility.can. Previously the interface declared only RbacResource on these methods, which left an inconsistency — anyone wanting to pass an array of resources had to call authenticateBearer + ability.can manually instead of using the convenience method. Surfaced when reviewing the cloud enterprise controller (TRI-8720), which had unilaterally widened its implementation to RbacResource[] and would have failed type-check if any caller routed an array through the typed interface. Updated: - packages/plugins/src/rbac.ts — RoleBaseAccessController interface. - internal-packages/rbac/src/fallback.ts — RoleBaseAccessFallback matches. - LazyController already uses Parameters<...> and tracks the interface, so it picks up the change automatically. @trigger.dev/plugins gets a minor bump (changeset added). Verification: - pnpm run typecheck across @trigger.dev/plugins, @trigger.dev/rbac, webapp — clean. - pnpm run test --filter @internal/rbac — 31 unit tests pass. - e2e suite unaffected (no signature change at runtime — pure type widening).
…suite (TRI-8732)
Foundation for TRI-8731. The smoke api-auth.e2e.test.ts spins up its own
webapp + Postgres container per test file (~30s startup each). The
comprehensive matrix would have 12+ files, so per-file startup would
dominate runtime. Instead this harness boots one container for the whole
suite and rapid-fires tests across multiple files.
Layout:
- vitest.e2e.full.config.ts — globalSetup + pool: forks. Picks up
test/**/*.e2e.full.test.ts.
- test/setup/global-e2e-full-setup.ts — calls startTestServer() once,
provides baseUrl + databaseUrl to test workers via vitest's
provide()/inject() API. Tears down on suite end.
- test/helpers/sharedTestServer.ts — getTestServer() pulls the provided
values, constructs a per-worker PrismaClient, exposes
{ webapp, prisma } matching the existing TestServer shape.
- test/helpers/seedTestSession.ts — produces a Cookie header value
compatible with the webapp's createCookieSessionStorage config so
dashboard tests (TRI-8742) can authenticate as a seeded user.
- test/auth-api.e2e.full.test.ts, test/auth-dashboard.e2e.full.test.ts,
test/auth-cross-cutting.e2e.full.test.ts — three file shells with
top-level describe blocks. Family subtasks (TRI-8733+) add nested
describes inside.
- .github/workflows/e2e-webapp-auth-full.yml — workflow_dispatch +
nightly schedule + pull_request paths-filtered (only triggers on PRs
touching auth-relevant files).
- test/README.md — documents the unit / smoke-e2e / full-e2e split.
Touching @internal/testcontainers:
- TestServer interface gains databaseUrl so per-worker PrismaClient
reconstruction has the connection string without going through the
serialised prisma instance (which can't cross worker boundaries).
- utils.ts — assertNonNullable's vitest import was previously eager at
module load. globalSetup runs outside any vitest worker, so that
eager init crashed (createExpect needs worker state). Switched to a
lazy require('vitest') inside the function body. The function still
runs in test workers where worker state exists.
- logs.ts — TaskContext changed to type-only import for the same
module-load-time concern (transitively imported by webapp.ts).
Verification:
- pnpm run typecheck across @internal/testcontainers + webapp — clean.
- pnpm exec vitest run --config vitest.e2e.full.config.ts —
3/3 tests pass in 19.37s with one observed container startup.
Subsequent family subtasks add describes with no per-file container
cost.
The placeholder it() in each file (just hits /healthcheck or counts
users) gets removed by the family subtasks as they add real coverage.
Mutation methods on RoleBaseAccessController now return discriminated
Result unions instead of throwing on expected error paths:
- RoleMutationResult — { ok: true; role: Role } | { ok: false; error }
for createRole, updateRole.
- RoleAssignmentResult — { ok: true } | { ok: false; error: string }
for deleteRole, setUserRole, removeUserRole, setTokenRole,
removeTokenRole.
The cloud webapp surfaces the 'error' strings directly to users
(system role edits, plan-tier gating, validation conflicts), so a
thrown exception now signals only an unexpected failure (DB outage,
bug). Read methods (getUserRole, getTokenRole, allRoles,
allPermissions) are unchanged.
OSS fallback returns { ok: false, error: 'RBAC plugin not installed' }
for every mutation — matches the prior behaviour (createRole/updateRole
already threw with this message; the others were silent no-ops, which
made misuse hard to detect). The LazyController in @internal/rbac
forwards the new return types verbatim. Changeset: patch.
Customer-facing surface: only public type widening of mutation method
return types — no runtime behaviour change for OSS callers (they get
a Result error instead of a thrown error or silent no-op; both indicate
'do not call these without the enterprise plugin').
The dev build was crashing with 'dashboardLoader is not a function'
on first navigation to any /admin route, then the browser would
hard-reload back to the previous page. Symptom: clicking 'Admin
dashboard' (or anywhere /@ → /admin chain) flashed admin then bounced
back, with no obvious cause server-side (every loader returned 200).
Root cause: routes export their loader at module top-level via the
wrapper:
export const loader = dashboardLoader(...);
The factory call evaluates at module load. dashboardBuilder lived in
a .server.ts file, which Remix strips from the client bundle. In the
prod build the loader export + its RHS are both tree-shaken, so the
import is unreferenced and removed — fine. In the dev build the call
is preserved (HMR/source-map friendliness) and resolves
dashboardLoader to undefined on the client, throwing on module load.
Remix's recovery is to reload the page, which lands on the previous
URL because that's the last known-good navigation entry.
Fix: split the wrapper so the import target exists on both server
and client.
- dashboardBuilder.ts (no .server) — exports types + dashboardLoader /
dashboardAction wrappers. Wrappers return closures whose bodies
dynamic-import the server impl. The closure body never runs on the
client, so the dynamic import only resolves at server runtime.
Client just sees a function that returns another function — the
top-level call now works there.
- dashboardBuilder.server.ts — slimmed down to authenticateAndAuthorize
+ the redirect/authorization helpers. Imported via dynamic import
from the wrapper. Stays out of the client bundle.
Routes drop the .server suffix on the import path. No change to the
route's loader/action surface. Verified end-to-end via Chrome
DevTools: /@ → /admin chain renders the admin dashboard cleanly,
no console errors, no extra document fetch back to the org URL.
…ting (TRI-8748) Wire RBAC into the existing org Teams page (settings/team). OSS plugin - Adds RoleBaseAccessController.getAssignableRoleIds(orgId) — the subset of allRoles(orgId) that can be assigned right now. Returns [] in the OSS fallback (consistent with allRoles also returning [] there). Pure UI affordance: server-side enforcement remains setUserRole's lookupAssignableRole. Public package change with patch-level changeset. Enterprise plugin - Implements getAssignableRoleIds against PlansClient: system roles pass through isSystemRoleAssignable (Owner/Admin always; Member / Viewer require Pro+); custom roles require canCreateCustomRoles (Enterprise tier). Mirrors the gates in setUserRole so UI and server agree. Webapp - TeamPresenter now also returns rbac.allRoles(orgId), getAssignableRoleIds(orgId), and per-member role assignments. Per-member is N+1 today (low-traffic settings page); a batched lookup is filed as a future optimisation. - Route migrated from requireUserId to dashboardLoader / dashboardAction via the split builder (commit a2cdbfb). Loader gates on read:members; action stays permissive at the wrapper level so the existing remove/leave + purchase-seats flows keep working with their per-intent checks. New set-role intent gates on manage:members and calls rbac.setUserRole — surfaces the Result error inline next to the dropdown when the server rejects (system role rename, plan-gated assignment, foreign-org role). - UI: native select next to each member, defaults to that member's current role. Options not in assignableRoleIds render disabled with an (upgrade) suffix. Auto-submits on change via fetcher. Invite + Remove buttons hide/disable when canManageMembers is false (server-side ability check pre-computed in the loader). Self-leave is always allowed regardless of manage:members. Verification - Typecheck clean across @internal/rbac, webapp, enterprise/plugins, enterprise/db, packages/plans. - Browser smoke test deferred until webapp dev server is running.
Pairs with the enterprise/db backfill migration (cloud side) so every
new (user, org) pair gets a UserRole row from day one without anyone
falling through to PERMISSIVE_ABILITY on the Teams page.
Mapping mirrors the backfill (legacy ADMIN had full access; the new
Admin role excludes billing + member management, so legacy ADMIN
belongs in the new Owner slot, not the new Admin slot):
legacy ADMIN -> Owner (sys_role_owner)
legacy MEMBER -> Member (sys_role_member)
Changes:
- services/rbac.server.ts: export SYSTEM_ROLE_IDS constant. The IDs
are seeded by the enterprise/db migration and never change; both
org creation and invite acceptance import from here so the role
reference is in one place.
- models/organization.server.ts: createOrganization calls
rbac.setUserRole({ roleId: owner }) after the org row is created.
Outside any transaction (rbac uses a separate Drizzle/postgres-js
connection). On OSS the fallback returns ok=false; we log + continue
since the legacy OrgMember.role write is the source of truth there.
- models/member.server.ts: acceptInvite assigns Owner if the invite
was ADMIN (defensive — the current UI only invites with MEMBER) or
Member otherwise. setUserRole runs after the prisma transaction
commits for the same reason as above. Returns the same shape as
before so callers don't change.
Verification: typecheck clean. Migration step (TRI-8854 part 1) is on
the cloud side; together they ensure both existing and new (user, org)
pairs land on a sensible RBAC role.
Lets users pick a system role at PAT-create time and persists it via enterprise.TokenRole so PAT-authenticated requests will run with that role's permissions once the auth-side wiring lands. V1 scope decisions (worth flagging for review): 1. System roles only. PATs are user-scoped (not org-scoped) and custom roles are per-org — the role-to-org mapping for a multi-org user's PAT is a non-trivial design question that doesn't need to be answered for v1. Show the four seeded system roles (Owner/Admin/Member/Viewer); a follow-up can add custom roles once we've decided what "this PAT uses an org X custom role" means semantically. 2. Default to caller's own role. Loader queries rbac.getUserRole against the user's first org membership (createdAt ASC) and uses that as the dropdown default — a PAT can't be more privileged than the person creating it without an explicit upgrade. Falls back to Member for users with no role assignment yet (OSS or new user pre-backfill). 3. No plan gating. Plan tiers are per-org; PAT roles are global. Plan gating only made sense in the org-scoped Teams page UI (TRI-8748). 4. No privilege-escalation check. Today's PATs run through the legacy auth path with full superScopes — even a "Owner" PAT here is strictly less permissive than the status quo. Locking down "the PAT can't exceed the creator's role" is a hardening for a later ticket once the read-side actually keys off TokenRole. Changes: - services/personalAccessToken.server.ts: createPersonalAccessToken takes an optional roleId. When provided, calls rbac.setTokenRole after the Prisma PAT row is created. On a real failure the PAT is compensating-deleted (the two writes live on different ORMs sharing one connection — co-transactions are awkward, compensating delete is simpler). The OSS fallback's "RBAC plugin not installed" return is treated as success-with-no-role: the PAT row stays, just no TokenRole gets written, matching pre-RBAC behaviour. - routes/account.tokens/route.tsx: loader fetches system roles + caller's current role; create form shows a role <Select> with the caller's role as default; OSS path (allRoles returns []) hides the dropdown entirely. Action passes roleId through to the service. Out of scope here (covered elsewhere): - The PAT auth-side path that will JOIN TokenRole and build an ability from the role's permissions. Lives in the enterprise plugin's authenticatePat path; tracked under the TRI-8741 test surface and the broader auth-consolidation work in TRI-8744. - CLI auth-code PAT (createPersonalAccessTokenFromAuthorizationCode) unchanged. CLI PATs continue to be created without an explicit role — they go through the legacy permissive path and existing user expectations of "trigger dev just works" are preserved. Verification: typecheck clean on webapp. Browser smoke test deferred to your local run.
Extends the smoke matrix (test/api-auth.e2e.test.ts, TRI-8716) which
already covers basic 401 cases (missing auth, wrong-prefix, unknown
PAT, revoked PAT, valid-PAT-on-nonexistent-project) by adding the
cases that need the full user → org → project → environment graph
seeded.
New helper: seedTestUserProject(prisma, opts?) returns a user + org
+ project + dev environment + a non-revoked PAT in one call. The
existing seedTestEnvironment doesn't create the OrgMember link that
findProjectByRef's `members: { some: { userId } }` scope check needs,
so PAT-comprehensive tests need this composite fixture.
Cases added under "PAT-authenticated routes — comprehensive" against
GET /api/v1/projects/:projectRef/runs:
- JWT on PAT route: 401 (PAT route doesn't accept JWTs).
- valid PAT, same-org project: 2xx (auth + scoping pass).
- valid PAT, cross-org project: 404 (not 403 — findProjectByRef
returns null and the route maps null to 404, locked in).
- valid PAT, soft-deleted project: 200 (findProjectByRef doesn't
filter on deletedAt — observed behaviour, called out so a future
change is conscious; the ticket described this as 404 but the
route's actual contract is 200).
- admin user accessing another org's project: still 404 (the global
user.admin flag doesn't grant cross-org visibility — the route is
per-user).
- admin user accessing their own org's project: 2xx (companion check
to confirm admin=true isn't accidentally subtracting permission).
Verification: typecheck clean. Test execution deferred to your normal
e2e run (the .full suite is slow due to container startup; CI will
catch any false expectations).
Validates the dashboardLoader({ authorization: { requireSuper: true } })
gate that the 14 admin pages were migrated to in TRI-8717 (and the
dashboardBuilder split fix landed earlier on this branch).
Coverage:
- "Admin pages — requireSuper gate": three representative routes
(/admin, /admin/concurrency, /admin/back-office) crossed with the
three auth states:
- No session → 302 to /login?redirectTo=<original-path>.
- Non-admin session → 302 to / (no path leakage in redirectTo —
a non-admin re-auth shouldn't bounce them back to /admin).
- Admin session → handler runs (status < 300).
All 14 admin routes share the same dashboardLoader config, so
testing every file would just confirm the wrapper works (the
harness already proves that). If config drifts per-route in the
future, add targeted tests for the divergent ones.
- "Admin action — requireSuper gate (admin.feature-flags POST)":
locks in the behaviour change from the TRI-8717 migration. The
legacy admin actions returned 403 Unauthorized; dashboardAction's
unauthorizedRedirect is "/", so non-admins now get a 302 to "/"
instead. Any XHR client branching on 403 needs updating — the
test makes a silent regression loud.
Reuses seedTestSession + seedTestUser from helpers/seedTestSession.ts
(the helper that ships with the shared-container harness from
TRI-8732). No new helpers needed.
Verification: typecheck clean. Test execution deferred to your
normal e2e run; CI will catch any false expectations.
Cross-cutting behaviours that aren't tied to a route family. Strategy: one representative API-key route + one representative JWT route exercise the edge cases — the auth layer is shared across every API route via apiBuilder.server.ts, so coverage here generalises. Smoke matrix already covers trivial cases (missing/ invalid key, basic JWT pass, soft-deleted project); this fills the gaps that need explicit fixture setup. Cases added: Revoked API key grace window (against /api/v1/runs/.../result): - revoked key with expiresAt > now: auth passes (rotation grace). - revoked key with expiresAt < now: 401. JWT edge cases (against /api/v1/waitpoints/tokens/.../complete): - expirationTime in the past (epoch 1) → 401. generateJWT only accepts string expirationTimes; constructed with jose's SignJWT directly to set an absolute past timestamp. - pub: false → 401 (token not meant for client-side use). - no sub claim → 401 (auth can't resolve env without it). - signed with another env's apiKey (sub-vs-signature mismatch) → 401. - malformed (3 parts but invalid base64 in payload) → 401 (must surface as 401, not 500 — guards against panic-on-malformed). Cross-environment isolation: - env A's JWT used to fetch env B's resource → not 200. Verifies the auth layer resolves env from the JWT's sub claim, NOT from the URL — env A's view scopes its lookup to env A and doesn't see env B's data. Critical security property: would let any customer read another's runs if it ever broke. Out of scope here: - Plugin force-fallback variant (running the suite under RBAC_FORCE_FALLBACK=1 and the unset default) — would need a second harness invocation. Filed mentally for follow-up. - Revoked PAT decryption-mismatch case (hash collision is effectively impossible to construct on demand). Verification: typecheck clean. Test execution deferred to your normal e2e run.
Two routes share the same resource-id JWT pattern: - POST /api/v1/waitpoints/tokens/:friendlyId/complete - POST /realtime/v1/streams/:runId/input/:streamId The smoke matrix already exercises the full waitpoint scope matrix (exact-id, type-level, action mismatch, wrong type, admin super-scope). This adds: Waitpoints — gap-fill (3 cases): - private API key (tr_dev_*) → 200 - JWT with write:all → 200 - cross-env: env A's JWT cannot complete env B's waitpoint → not 200 Input streams — full matrix (9 cases, no smoke coverage): - missing auth → 401 - private API key → auth passes - JWT exact-id scope → auth passes - JWT type-level scope → auth passes - JWT wrong resource id → 403 - JWT read action on write route → 403 - JWT write:all → auth passes - JWT admin → auth passes - cross-env JWT → not 200 (security property) Pass-cases on input streams use "not 401, not 403" rather than asserting a specific 2xx — the realtime streams path returns various codes depending on stream state, but the auth layer is the only thing this test cares about. Reuses seedTestWaitpoint and seedTestRun from the existing helpers. No new fixtures. Verification: typecheck clean. Test execution deferred to your normal e2e run.
Prompts route family — read + update actions, both single-id
({type:"prompts", id: params.slug}) and collection-level
({type:"prompts", id: "all"}) resource shapes.
Auth resolves before any DB lookup, so tests use non-existent
slugs throughout; handler 404s but auth-passed assertion
("not 401, not 403") is what the matrix verifies.
Coverage:
Prompts list — GET /api/v1/prompts (5 cases):
- missing auth → 401
- private API key → auth passes
- JWT read:prompts → passes
- JWT read:runs → 403 (type mismatch)
- JWT admin → passes
Prompts retrieve — GET /api/v1/prompts/:slug (7 cases, full matrix):
- missing auth → 401
- private API key → passes
- JWT read:prompts → passes
- JWT read:prompts:<exact slug> → passes
- JWT read:prompts:<other> → 403
- JWT read:runs → 403 (type mismatch)
- JWT admin → passes
Prompts override — POST /api/v1/prompts/:slug/override (6 cases):
Tests the ACTION_ALIASES write→update behaviour:
- missing auth → 401
- JWT write:prompts:<slug> matching → passes
- JWT write:prompts (type-level) → passes
- JWT read:prompts → 403 (action mismatch — read NOT aliased)
- JWT write:prompts:<other> → 403
- JWT admin → passes
Promote/reactivate sanity (2 cases):
- promote: JWT write:prompts → passes
- reactivate: JWT read:prompts → 403
Multi-method override (POST/PUT/PATCH/DELETE) is not exhaustively
tested per-method — they share the same authorization config so
covering POST suffices. If a method ever overrides authorization,
add a targeted test.
Verification: typecheck clean. Test execution still blocked by the
e2e.full webapp-boot issue noted on TRI-8731.
Read-only family with distinct resource types per route:
- GET /api/v1/deployments { type: "deployments", id: "list" }
- GET /api/v1/query/schema { type: "query", id: "schema" }
- GET /api/v1/query/dashboards { type: "query", id: "dashboards" }
- POST /api/v1/query body-derived via detectTables(query)
→ tables.length > 0
? tables.map(id => ({type:"query", id}))
: { type: "query", id: "all" }
Coverage:
Deployments list (7 cases):
- missing auth → 401
- private API key → passes
- JWT read:deployments → passes
- JWT read:all → passes
- JWT admin → passes
- JWT read:runs → 403 (type mismatch)
- JWT write:deployments → 403 (action mismatch)
Query schema sanity (3 cases):
- missing auth → 401
- JWT read:query → passes
- JWT read:deployments → 403 (type mismatch)
Query dashboards sanity (2 cases):
- missing auth → 401
- JWT read:query → passes
Query ad-hoc body-derived (6 cases):
- missing auth → 401
- body "SELECT * FROM runs" + JWT read:query:runs → passes
(any-match against the body-derived array)
- body "SELECT 1" (no detectable tables) + JWT read:query → passes
(defaults to id="all"; type-level scope matches)
- body with 'runs' + JWT read:query:other_table → 403
- JWT admin → passes regardless of body
- JWT write:query → 403 (action mismatch)
Verification: typecheck clean. Test execution still blocked by the
e2e.full webapp-boot issue noted on TRI-8731.
This closes the TRI-8731 test family (8733, 8734, 8735, 8736, 8737,
8738, 8739, 8740, 8741, 8742, 8743 — all done across today's
commits).
The e2e.full harness was failing to boot the webapp testcontainer
with `TypeError: Cannot convert undefined or null to object at
allMachines (build/index.js:71583)`. Root cause: the testcontainer
was setting NODE_ENV=test, which surfaces a circular-init order
regression in the production bundle that NODE_ENV=production
dodges (modules init in a different order under prod-mode and the
relevant singleton resolves before the cycle re-enters). Production
builds work fine — the harness just needs to match prod-mode boot.
Single one-line change in testcontainers: NODE_ENV is now
"production" instead of "test". Tests don't depend on test-mode
semantics — they just need an isolated webapp + DB.
After unblocking, fixed 11 tests whose strict 2xx assertions were
correct against a request-time-resolved handler but wrong against
the test container (where ClickHouse and external services are
dummy URLs):
- 8 tests on api.v1.runs and api.v1.projects/<ref>/runs (PAT route):
the run-list presenter hits ClickHouse which 500s in tests. Auth
passes; assertion changed from strict 200 to "not 401/403".
- 2 tests on api.v1.prompts/:slug (retrieve): the apiBuilder runs
findResource BEFORE authorization. With no Prompt fixture seeded
the route 404s before the auth check, so a non-matching scope
appears as 404 rather than 403. Both states mean "user can't see"
— assertion changed to "not 200" with a comment explaining the
ordering.
- 1 test on prompts /override/reactivate: route's BodySchema requires
`{ version: positive int }`. My empty body 400'd at validation
before auth. Sending `{ version: 1 }` lets validation pass and
the auth check fires; gets the expected 403.
Plus 1 fixture fix on auth-cross-cutting.e2e.full.test.ts: the
TaskRun.create call needed `queue: "task/test-task"` (matches the
seedTestRun helper).
Verification:
pnpm exec vitest run --config vitest.e2e.full.config.ts
→ 3 files, 162 tests, all pass. ~14 seconds.
…859) Replace the hardcoded `RBAC_FORCE_FALLBACK: "1"` env var with an optional `forceRbacFallback` parameter on `startWebapp` and `startTestServer`. Default `true` preserves OSS suite behaviour (every existing call site keeps fallback-pinned semantics). Cloud's enterprise variant of the e2e suite passes `false` so the spawned webapp loads the real `@triggerdotdev/plugins/rbac` instead of the OSS fallback. Same harness, different RBAC implementation under test. Verification: OSS e2e.full suite still 162/162 passes.
…(TRI-8731)
After rebasing rbac-packages on origin/main, the e2e.full harness
regressed with the same `TypeError: Cannot convert undefined or null
to object at allMachines (build/index.js:71862)` that TRI-8731
worked around by switching the testcontainer to NODE_ENV=production.
Recent main commits changed the bundled init order enough that the
production-mode dodge no longer applies — the cycle now triggers in
both NODE_ENV=test and NODE_ENV=production.
Root cause is structural:
app/services/platform.v3.server → imports createEnvironment
from app/models/organization.server
app/models/organization.server → imports getDefaultEnvironmentConcurrencyLimit
from app/services/platform.v3.server
Inside an esbuild __esm bundle, this manifests as:
init_platform_v3_server() runs init_organization_server() in the
middle of its body. organization.server's body re-enters
init_platform_v3_server(), which short-circuits because the outer
call already cleared its `fn` — so `({ defaultMachine, machines } =
singleton("machinePresets", ...))` never completes its destructure
and both vars stay undefined. Object.entries(undefined) crashes
when `allMachines()` runs inside `createRunEngine()`.
Fix: move the only function in platform.v3.server.ts that imports
from organization.server (`projectCreated`, the sole caller of
`createEnvironment`) into its own file. platform.v3.server.ts no
longer imports from organization.server, so the cycle is gone. Two
trivial supporting changes:
- export `isCloud` from platform.v3.server (projectCreated needs it)
- drop the now-unused `Organization` and `Project` type imports
No dynamic imports, no application-code workarounds — just a
structural file split.
Verified:
- 162/162 e2e.full pass (auth-api, auth-cross-cutting, auth-dashboard)
- 31/31 api-auth.e2e pass
- 31/31 @trigger.dev/rbac unit tests pass
- 7/7 cloud enterprise e2e.full pass against the same webapp build
Adds a Settings → Roles page that lists every role visible to the org,
with each role's permissions rendered in a Table grouped by category
(Runs, Tasks, Waitpoints, Realtime, Deployments, Prompts, Query,
Tokens, Organisation, Wildcards). Each permission shows its name and
description.
Page surfaces:
- Role name + description + System/Custom badge
- "Not on this plan" badge for roles outside the current plan tier
(system roles gated by PlansClient.isSystemRoleAssignable)
- "Create role" button:
- Free / Hobby / Pro: opens an "Upgrade to Enterprise" dialog
with a Contact us CTA (deep-links to trigger.dev/contact)
- Enterprise: hidden — the create-role UI is a follow-up after
TRI-8747's controller-level CRUD already in place
Plumbing:
- apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.roles
- organizationRolesPath helper in pathBuilder
- Roles SideMenuItem next to Team in OrganizationSettingsSideMenu
- "View all role permissions →" link on the Teams page next to the
Active team members section so an Owner about to assign a role can
audit the choice
The enterprise plugin's getUserRole now derives a user's role from the
legacy public.OrgMember.role column whenever no explicit UserRole row
exists (separate cloud commit). That makes the upfront UserRole writes
in createOrganization and acceptInvite redundant — the role display
and ability checks both work from day one based on OrgMember alone.
Removed:
- The rbac.setUserRole call + SYSTEM_ROLE_IDS import from
apps/webapp/app/models/organization.server.ts (createOrganization)
- The rbac.setUserRole call + SYSTEM_ROLE_IDS import from
apps/webapp/app/models/member.server.ts (acceptInvite)
A UserRole row is now only ever inserted when an Owner explicitly
changes someone's role on the Teams page. Everyone else's role is
derived live from OrgMember.role.
Code comments throughout the OSS-facing RBAC surface mentioned the
enterprise plugin, CASL, the cloud webapp, the cloud-side test suite,
and specific cloud file paths. Two reasons not to keep that:
- Reputation: comments framing the OSS code as "the OSS path" vs
"the enterprise path" pollute the public repo with implementation
framing that shouldn't be there.
- Implementation leakage: enterprise/cloud comments give away
structural details about the closed-source plugin (where its data
lives, what library it uses, which Linear tickets track it).
Rewrites use neutral language — "the loaded RBAC plugin (if any)",
"the default fallback", "an installed plugin" — and drop references
to specific cloud-side files / TRI-IDs / CASL.
Plan-tier names ("Enterprise" as a public product tier in the Roles
page upsell, `planCode === "enterprise"` checks, `<TierEnterprise />`
in pre-existing files) are intentionally left as-is — they're the
public marketing name for a paid tier, not implementation detail.
Removed `.server-changes/rbac-userrole-default-assignment.md` —
documented a feature that was reverted in d2bf617 (upfront UserRole
inserts on create-org / acceptInvite).
Verified: 162/162 OSS e2e.full pass, 31/31 OSS rbac unit pass.
Pairs with the cloud-side change that removes `admin`, `read:all`, and `write:all` from PERMISSION_CATALOGUE. With no catalogue entry sitting in the Wildcards group, the corresponding entries in the client-side PERMISSION_GROUP_BY_NAME map are dead and the group is removed from GROUP_ORDER.
…I-8893)
Pairs with the cloud-side CASL refactor that switches role storage to
packed CASL rules + introduces conditional rules (e.g. Member's prod
env-var restrictions). Two interface changes here:
- Permission gains optional `inverted` and `conditions` fields. The
Roles page renders `inverted: true` rules as ✗ and `conditions`
(e.g. `{ envType: "PRODUCTION" }`) as a tier badge.
- RbacResource gains an open-ended `[key: string]: unknown` index so
routes can pass condition-relevant fields alongside `type` / `id`
(e.g. `{ type: "envvars", envType: env.type }`). The plugin's
CASL-backed matcher reads these off the resource object.
Roles page UI: TableHeader gains an "Allowed" column rendering ✓/✗
per rule, and conditional rules show a `(production only)` /
`(non-production only)` Badge next to the permission name. Group order
gains a leading "All" for Owner/Admin's wildcard rules and an
"Environment" group for the new envvars/apiKeys catalogue pairs.
…8904)
Replaces the per-role tables with a single comparison grid: rows are
catalogue permissions grouped by category (Runs, Tasks, Environment,
…), columns are Owner, Admin, Developer, Member, then any
custom roles, then Description. Each cell shows whether that role
grants the permission.
Cell rendering driven by `effectivePermissions(role.rules)` (TRI-8893):
- No matching rules → ✗ in muted colour
- Allow rule(s), no inverted → ✓ in success green
- Allow rule(s) plus a conditional `cannot` → ✓ green + a tier badge
rendered beneath ("non-prod only" for envType=PRODUCTION etc.)
- Only inverted unconditional rule → ✗ in error colour
Plan tier hint in column headers — Developer / Member columns get a
small "Pro" Badge on Free/Hobby; custom roles get "Enterprise". Cells
still render the comparison data so users see what they'd unlock.
Loader extended to call `rbac.allPermissions(orgId)` so the catalogue
drives the row enumeration. Owner column ends up with ✓ on every row
(one rendered Permission per catalogue entry, expanded from the
`manage:all` packed rule via CASL's rulesFor walk).
Also: `SYSTEM_ROLE_IDS` updated from `{owner, admin, member, viewer}`
to `{owner, admin, developer, member}` — Viewer was dropped in TRI-8893
when the role ladder finalised; this catches up the OSS-side helper.
account.tokens uses `SYSTEM_ROLE_IDS.member` as the PAT default; the
new (more restricted) Member is the right default for that flow.
Adds a Role `<Select>` to the invite form. Dropdown options are
filtered by:
1. The inviter's own role — strictly below their level (Owner can
pick any of the 4; Admin can pick Developer or Member; Developer/
Member don't see the picker because they can't invite anyway).
2. The org's plan tier — `rbac.getAssignableRoleIds(orgId)` already
reflects this (Free/Hobby = Owner+Admin only, Pro+ unlocks
Developer+Member).
The picker is hidden entirely when `rbac.allRoles(orgId)` returns []
(OSS deployments with no plugin installed) — legacy invite path is
unchanged for self-hosters.
Schema: nullable `OrgMemberInvite.rbacRoleId text` column. On accept,
if it's set, `acceptInvite` calls the plugin's `setUserRole` after
the OrgMember insert (outside the Prisma transaction since the plugin
uses a separate Drizzle / postgres-js connection — same compensating
pattern as PAT-role assignment). If it's null, the runtime fallback
derives a role from the legacy `OrgMember.role` write at first
auth — no behaviour change.
Server-side validation in the action layer rejects:
- rbacRoleId not in `getAssignableRoleIds(orgId)` (plan-tier check).
- rbacRoleId at or above the inviter's own level (the
canInviteAtRole ladder).
Legacy `OrgMemberInvite.role` enum (ADMIN/MEMBER) is still written
based on the chosen RBAC role — Owner/Admin → "ADMIN", Developer/
Member → "MEMBER" — so OSS auth keeps working.
Verified:
- typecheck clean
- 162/162 OSS e2e.full
- 7/7 cloud enterprise e2e.full
Adds a new `systemRoleIds(): Promise<SystemRoleIds | null>` method on
the `RoleBaseAccessController` interface. Returns
`{ owner, admin, developer, member }` from any installed plugin and
`null` from the default fallback (matches the `allRoles → []`
semantics — there are no seeded roles to refer to in OSS).
Drops the `SYSTEM_ROLE_IDS` constant from `~/services/rbac.server` so
consumers can't reach for hardcoded role-id strings. Updates the four
sites that used it:
- `models/member.server.ts` (invite flow's legacy-role mapping)
- `routes/account.tokens` (PAT default)
- `routes/_app.orgs.$organizationSlug.settings.roles` (Roles page
comparison grid column ordering + plan-tier badges)
- `routes/_app.orgs.$organizationSlug.invite` (role picker)
The Roles page and invite route both pass the IDs through their
loaders rather than referencing them at module top level — which was
the root cause of the "Invite a team member button hard-refreshes the
dashboard" bug: importing a `.server.ts` symbol from client-rendered
code left a dangling client-bundle reference.
Verified: typecheck clean, 162/162 OSS e2e.full, 7/7 cloud
enterprise e2e.full.
SelectLinkItem passes render={<Link>} so the row navigates on click. In
non-Combobox Selects (most use cases — the role picker on the Teams
page, etc.) SelectItem was overriding render to undefined, silently
dropping the Link wrapper. Pass props.render through verbatim when
there's no Combobox; wrap in ComboboxItem only when one's present.
The OSS no longer needs to know individual role names. systemRoles(orgId) returns a plugin-owned, ordered SystemRole[] (id, name, description, available) — the cloud plugin owns the canonical order, the descriptions, and the per-org plan-tier 'available' flag. Hidden roles (Member in v1) are filtered out entirely. OSS callers iterate the array and use array index for the level ladder; no role-name strings except for the legacy OrgMember.role enum mapping shim, which is now isolated to one filter in member.server.ts.
🦋 Changeset detectedLatest commit: 398ca55 The changes in this PR will be included in the next version bump. This PR includes changesets to release 31 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
WalkthroughAdds a new internal RBAC package and plugin contracts, a fallback RBAC implementation, and a lazy loader. Integrates RBAC into the webapp: role UIs (organization Roles page, invite role picker, PAT role selection, team role assignment), route builders ( Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsx (1)
362-380:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGate pending-invite actions behind
canManageMemberstoo.After widening the page itself to
read:members, the resend/revoke controls are still rendered for users who cannot manage members. That leaves dead UI at best, and it risks an auth gap if those sibling routes were previously relying on page-level gating.Suggested change
- <div className="flex grow items-center justify-end gap-x-2"> - <ResendButton invite={invite} /> - <RevokeButton invite={invite} /> - </div> + <div className="flex grow items-center justify-end gap-x-2"> + {canManageMembers ? ( + <> + <ResendButton invite={invite} /> + <RevokeButton invite={invite} /> + </> + ) : null} + </div>🤖 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 362 - 380, The pending-invite action buttons (ResendButton and RevokeButton) are still rendered even when the viewer only has read:members; wrap or conditionally render those controls behind the canManageMembers flag so only users with permission see them—e.g., guard the action container (the div containing ResendButton and RevokeButton) or each button with {canManageMembers && ...} in the invites.map rendering to prevent dead UI and potential auth gaps.
🧹 Nitpick comments (3)
apps/webapp/test/helpers/seedTestRun.ts (1)
6-10: ⚡ Quick winUse a type alias here instead of an interface.
This file is TypeScript, and the exported shape should follow the repo rule.
Suggested fix
-export interface SeededRun { +export type SeededRun = { run: TaskRun; runFriendlyId: string; // `run_...` batchFriendlyId?: string; // `batch_...` when { withBatch: true } -} +};As per coding guidelines, Use types over interfaces for TypeScript.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/test/helpers/seedTestRun.ts` around lines 6 - 10, Replace the exported interface SeededRun with an exported type alias: define "export type SeededRun = { run: TaskRun; runFriendlyId: string; batchFriendlyId?: string }" so the shape remains identical but uses a type instead of an interface; update any imports/uses referencing SeededRun if needed to ensure the symbol name remains the same.apps/webapp/test/helpers/seedTestSession.ts (1)
15-29: ⚡ Quick winCentralize the test session contract.
SESSION_SECRETand the cookie shape are duplicated here and ininternal-packages/testcontainers/src/webapp.ts, so a one-sided change will turn every dashboard auth test into a hard-to-diagnose auth failure. Pulling the shared test-session config into one exported constant would remove that drift point.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/test/helpers/seedTestSession.ts` around lines 15 - 29, The SESSION_SECRET and cookie session shape are duplicated; centralize them by extracting the shared config into a single exported constant (e.g., TEST_SESSION_SECRET and TEST_SESSION_STORAGE_CONFIG or a single TEST_SESSION object) and import it here instead of redefining SESSION_SECRET and calling createCookieSessionStorage locally; update uses of SESSION_SECRET and sessionStorage in this file to consume the shared export and remove the local duplicate definitions (referencing the symbols SESSION_SECRET, sessionStorage, and createCookieSessionStorage to locate the current code to replace).packages/plugins/src/rbac.ts (1)
81-88: ⚡ Quick winPrefer exported type aliases over interfaces in this RBAC contract.
These new public contracts are all declared as
interface, which goes against the repo’s TS convention. Switching them totypekeeps the surface effectively the same here while aligning the package with the rest of the codebase.As per coding guidelines,
**/*.{ts,tsx}: Use types over interfaces for TypeScript.Also applies to: 104-216
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugins/src/rbac.ts` around lines 81 - 88, Replace the exported interface declarations with exported type aliases to follow the repo TypeScript convention; specifically change RbacAbility (and the other exported interfaces in this file between lines ~104-216) from "export interface X { ... }" to "export type X = { ... }" while preserving the exact members and names (e.g., can, canSuper on RbacAbility and any RbacResource, RbacPolicy, etc.), keeping the public contract identical except using type aliases instead of interfaces.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/e2e-webapp-auth-full.yml:
- Around line 19-35: The workflow's pull_request path filters omit shared auth
fixtures (e.g., apps/webapp/test/helpers/seedTestEnvironment.ts) so PRs that
change those helpers won't trigger the e2e-auth-full run; update the path filter
in .github/workflows/e2e-webapp-auth-full.yml to include that file and the
helpers directory (for example add
apps/webapp/test/helpers/seedTestEnvironment.ts and/or
apps/webapp/test/helpers/**) so changes to seedTestEnvironment and other shared
auth helpers (seedTestSession, sharedTestServer, etc.) will trigger the
workflow.
In `@apps/webapp/app/routes/_app.orgs`.$organizationSlug.invite/route.tsx:
- Around line 202-211: The error message is inconsistent with the authorization
check using isStrictlyBelow(inviterRole?.id ?? null, submittedRbacRoleId) which
rejects equal roles; update the JSON error string returned in that branch to
clearly state the stricter rule (e.g., "You can only invite members strictly
below your own role" or similar) so the text matches the behavior in the
conditional that uses isStrictlyBelow.
In `@apps/webapp/app/routes/_app.orgs`.$organizationSlug.settings.roles/route.tsx:
- Around line 170-173: The "Create role" CTA is hidden for enterprise accounts
because CreateRoleUpsell is only rendered when !isEnterprise; update the NavBar
blocks (where PageTitle and CreateRoleUpsell are used) to ensure enterprise
users see a create-role control by either rendering the actual create button for
enterprise (e.g., use the existing CreateRoleButton or RoleCreateModal trigger)
when isEnterprise is true, or always render CreateRoleUpsell but switch it to
the real create-action for enterprise. Locate and change the two occurrences
(the NavBar block with PageTitle "Roles" and the similar block around the later
occurrence) to conditionally render the appropriate component based on
isEnterprise so enterprise plans have a visible create-role CTA.
In `@apps/webapp/app/routes/account.tokens/route.tsx`:
- Around line 68-90: The dropdown is built from allRoles.filter(r => r.isSystem)
but the default and server trust come from rbac.systemRoles(...).filter(r =>
r.available), causing a mismatch; update loadSystemRolesForUser to call
rbac.systemRoles(orgId) and filter by .available (e.g., systemRoles =
rbac.systemRoles(orgMember.organizationId).filter(r => r.available)) so the UI
only shows assignable system roles, and in the token-creation action (the POST
handler that reads roleId) revalidate that the submitted roleId exists in that
same available systemRoles set before creating the PAT (reject or error if not).
Apply the same change to the other similar blocks referenced (lines ~97-111 and
~141-165) so both the picker and server-side checks use
rbac.systemRoles(...).filter(r => r.available).
In `@apps/webapp/app/routes/admin.llm-models.missing._index.tsx`:
- Around line 33-35: Replace the ad-hoc parseInt logic for lookbackHours with
the existing SearchParams zod schema: use SearchParams.parse (or .safeParse) on
url.searchParams (or an object containing lookbackHours) and read the
validated/coerced lookbackHours from the result instead of parseInt; update the
loader code where lookbackHours is set (the const lookbackHours = ... line) to
use the parsed value so NaN cannot occur and downstream logic receives a
validated number.
In `@apps/webapp/app/routes/admin.llm-models.missing`.$model.tsx:
- Around line 23-25: The code reads lookbackHours via parseInt and may produce
NaN; replace that with a Zod coercion and bounds check: create a schema like
z.coerce.number().int().min(1).max(168).default(24) (or your desired max) and
call schema.parse on url.searchParams.get("lookbackHours") (or the raw value) to
produce a safe lookbackHours number; then use the validated lookbackHours
variable in the existing request handling and service call (refer to the
existing lookbackHours const and the URL = new URL(request.url) usage) so the
payload always receives a valid defaulted/bounded integer.
In `@apps/webapp/app/routes/admin.orgs.tsx`:
- Around line 36-38: Replace the thrown generic Error for invalid query params
with an explicit 400 client response: in the block that checks
searchParams.success (the if (!searchParams.success) branch), return a Response
(or Remix json/badRequest helper) with the searchParams.error (or a default
message) and status set to 400 instead of throwing; this ensures the handler in
routes/admin.orgs.tsx treats malformed/invalid query parameters as a client
error rather than a server exception.
In `@apps/webapp/app/routes/api.v2.tasks.batch.ts`:
- Around line 33-39: The batch authorization currently builds resources in the
authorization block for action "batchTrigger" using the resource closure and
relies on anyResource() which uses resource.some() (OR semantics); change the
check so the authorization requires AND semantics across the batch by ensuring
every task resource is authorized: update the resource-building/authorization
call referenced in the route (the authorization object for action "batchTrigger"
that maps body.items to resources) to either use the all-match helper (e.g., an
allResource/allResources variant) or adjust the logic to perform
resource.every(...) instead of resource.some(), so that all provided tasks must
be authorized before allowing the batch trigger; refer to
internal-packages/rbac/src/ability.ts (replace anyResource()/some with an
all-match implementation) and the authorization block in
routes/api.v2.tasks.batch.ts for the change.
In `@apps/webapp/app/routes/realtime.v1.streams`.$runId.$streamId.ts:
- Around line 100-110: The resource builder now reads run.taskIdentifier,
run.runTags and downstream code uses run.realtimeStreamsVersion but findResource
currently only returns id, friendlyId and batch; update the data retrieval in
findResource so the returned run object includes taskIdentifier, runTags (or
run_tags as stored) and realtimeStreamsVersion (or its DB field) in addition to
id/friendlyId/batch so the RBAC check and realtime stream setup receive those
fields; locate the findResource implementation and extend its SELECT/returned
fields to include these properties referenced in the resource function and later
handlers.
In `@apps/webapp/app/services/projectCreated.server.ts`:
- Around line 24-33: The check for staging entitlement accesses nested
subscription fields that may be undefined; update the guard around
getCurrentPlan()'s result so you only check hasStagingEnvironment when
v3Subscription, v3Subscription.plan and plan.limits exist (e.g., use a safe
optional chain or explicit null checks on plan.v3Subscription and
plan.v3Subscription.plan before reading
plan.v3Subscription.plan.limits.hasStagingEnvironment) and then call
createEnvironment(...) for STAGING and PREVIEW only when that guarded condition
is true; references: getCurrentPlan, plan, v3Subscription, createEnvironment.
In `@apps/webapp/test/auth-cross-cutting.e2e.full.test.ts`:
- Around line 206-213: The test currently allows either 401 or 404 for the
cross-env lookup (lines using server.webapp.fetch with friendlyId and jwt),
which masks a regression where auth could fail entirely; change the assertion to
require a 404 to assert env isolation (replace the
expect(res.status).not.toBe(200) / expect([401, 404]).toContain(res.status)
checks with an explicit expect(res.status).toBe(404)), or alternatively add a
control fetch using the same jwt against a known env-A resource to assert the
JWT is valid before asserting the cross-env lookup returns 404; update
assertions around server.webapp.fetch and res.status accordingly.
In `@internal-packages/rbac/src/fallback.ts`:
- Around line 34-107: authenticateBearer currently only checks public JWTs and
runtimeEnvironment.apiKey, so Personal Access Tokens (PATs) are never
recognized; add a PAT lookup branch after the runtimeEnvironment.findFirst call
fails: query the personalAccessToken (or equivalent) record using the rawToken,
validate it (not revoked/expired), load its associated
runtimeEnvironment/project/organization/orgMember as needed, set the returned
subject to type "personalAccessToken" with the token's
userId/organizationId/projectId, and return the appropriate environment
(toRbacEnvironment) and ability for PATs; ensure existing branches and error
returns remain unchanged and reuse symbols authenticateBearer,
runtimeEnvironment, toRbacEnvironment, and subject.type "personalAccessToken" so
callers get a valid PAT-authenticated response.
In `@internal-packages/rbac/src/index.ts`:
- Around line 80-93: The current catch treats all ERR_MODULE_NOT_FOUND the same;
update the ERR_MODULE_NOT_FOUND branch to inspect the error message
(err.message) for the actual module specifier that failed to load (the same
module specifier used in the dynamic import) and distinguish two cases: if the
missing specifier equals the plugin's import specifier then treat it as "no
plugin installed" and only log the lightweight fallback message when
process.env.RBAC_LOG_FALLBACK === "1"; otherwise treat it as a broken/transitive
dependency and log the full error loudly (use console.error with err) so it
surfaces in CI/production. Keep the existing variables isModuleNotFound and the
RBAC_LOG_FALLBACK check but add the err.message check to decide which message to
emit.
In `@packages/plugins/tsup.config.ts`:
- Line 3: The file currently uses a default export of defineConfig; replace this
with a named exported function per repo rules. Change the default export to a
function (e.g., export function tsupConfig() or export function
createTsupConfig()) that returns the defineConfig({...}) result and export it as
a named export so callers import the config by name rather than via a default
export; update any local references/imports to use the new named export (look
for defineConfig and the current default export usage).
---
Outside diff comments:
In `@apps/webapp/app/routes/_app.orgs`.$organizationSlug.settings.team/route.tsx:
- Around line 362-380: The pending-invite action buttons (ResendButton and
RevokeButton) are still rendered even when the viewer only has read:members;
wrap or conditionally render those controls behind the canManageMembers flag so
only users with permission see them—e.g., guard the action container (the div
containing ResendButton and RevokeButton) or each button with {canManageMembers
&& ...} in the invites.map rendering to prevent dead UI and potential auth gaps.
---
Nitpick comments:
In `@apps/webapp/test/helpers/seedTestRun.ts`:
- Around line 6-10: Replace the exported interface SeededRun with an exported
type alias: define "export type SeededRun = { run: TaskRun; runFriendlyId:
string; batchFriendlyId?: string }" so the shape remains identical but uses a
type instead of an interface; update any imports/uses referencing SeededRun if
needed to ensure the symbol name remains the same.
In `@apps/webapp/test/helpers/seedTestSession.ts`:
- Around line 15-29: The SESSION_SECRET and cookie session shape are duplicated;
centralize them by extracting the shared config into a single exported constant
(e.g., TEST_SESSION_SECRET and TEST_SESSION_STORAGE_CONFIG or a single
TEST_SESSION object) and import it here instead of redefining SESSION_SECRET and
calling createCookieSessionStorage locally; update uses of SESSION_SECRET and
sessionStorage in this file to consume the shared export and remove the local
duplicate definitions (referencing the symbols SESSION_SECRET, sessionStorage,
and createCookieSessionStorage to locate the current code to replace).
In `@packages/plugins/src/rbac.ts`:
- Around line 81-88: Replace the exported interface declarations with exported
type aliases to follow the repo TypeScript convention; specifically change
RbacAbility (and the other exported interfaces in this file between lines
~104-216) from "export interface X { ... }" to "export type X = { ... }" while
preserving the exact members and names (e.g., can, canSuper on RbacAbility and
any RbacResource, RbacPolicy, etc.), keeping the public contract identical
except using type aliases instead of interfaces.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 222ec396-3f09-4be3-ae04-d454d2f49ded
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (105)
.changeset/rbac-assignable-role-ids.md.changeset/rbac-authenticate-authorize-arrays.md.changeset/rbac-mutation-result-types.md.changeset/rbac-plugin-array-resources.md.changeset/rbac-system-role-ids-method.md.changeset/rbac-system-roles.md.github/workflows/e2e-webapp-auth-full.yml.server-changes/rbac-apibuilder-migration.md.server-changes/rbac-dashboard-builder.md.server-changes/rbac-force-fallback.md.server-changes/rbac-invite-role-picker.md.server-changes/rbac-pat-role-selection.mdapps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/components/primitives/Select.tsxapps/webapp/app/env.server.tsapps/webapp/app/models/member.server.tsapps/webapp/app/models/organization.server.tsapps/webapp/app/models/project.server.tsapps/webapp/app/presenters/TeamPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.invite/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.roles/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsxapps/webapp/app/routes/account.tokens/route.tsxapps/webapp/app/routes/admin._index.tsxapps/webapp/app/routes/admin.back-office._index.tsxapps/webapp/app/routes/admin.back-office.orgs.$orgId.tsxapps/webapp/app/routes/admin.back-office.tsxapps/webapp/app/routes/admin.concurrency.tsxapps/webapp/app/routes/admin.feature-flags.tsxapps/webapp/app/routes/admin.llm-models.$modelId.tsxapps/webapp/app/routes/admin.llm-models._index.tsxapps/webapp/app/routes/admin.llm-models.missing.$model.tsxapps/webapp/app/routes/admin.llm-models.missing._index.tsxapps/webapp/app/routes/admin.llm-models.new.tsxapps/webapp/app/routes/admin.notifications.tsxapps/webapp/app/routes/admin.orgs.tsxapps/webapp/app/routes/admin.tsxapps/webapp/app/routes/api.v1.batches.$batchId.tsapps/webapp/app/routes/api.v1.deployments.tsapps/webapp/app/routes/api.v1.idempotencyKeys.$key.reset.tsapps/webapp/app/routes/api.v1.prompts.$slug.override.reactivate.tsapps/webapp/app/routes/api.v1.prompts.$slug.override.tsapps/webapp/app/routes/api.v1.prompts.$slug.promote.tsapps/webapp/app/routes/api.v1.prompts.$slug.tsapps/webapp/app/routes/api.v1.prompts.$slug.versions.tsapps/webapp/app/routes/api.v1.prompts._index.tsapps/webapp/app/routes/api.v1.query.dashboards._index.tsapps/webapp/app/routes/api.v1.query.schema.tsapps/webapp/app/routes/api.v1.query.tsapps/webapp/app/routes/api.v1.runs.$runId.events.tsapps/webapp/app/routes/api.v1.runs.$runId.spans.$spanId.tsapps/webapp/app/routes/api.v1.runs.$runId.trace.tsapps/webapp/app/routes/api.v1.runs.tsapps/webapp/app/routes/api.v1.tasks.$taskId.trigger.tsapps/webapp/app/routes/api.v1.tasks.batch.tsapps/webapp/app/routes/api.v1.waitpoints.tokens.$waitpointFriendlyId.complete.tsapps/webapp/app/routes/api.v2.batches.$batchId.tsapps/webapp/app/routes/api.v2.runs.$runParam.cancel.tsapps/webapp/app/routes/api.v2.tasks.batch.tsapps/webapp/app/routes/api.v3.batches.tsapps/webapp/app/routes/api.v3.runs.$runId.tsapps/webapp/app/routes/realtime.v1.batches.$batchId.tsapps/webapp/app/routes/realtime.v1.runs.$runId.tsapps/webapp/app/routes/realtime.v1.runs.tsapps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.tsapps/webapp/app/routes/realtime.v1.streams.$runId.input.$streamId.tsapps/webapp/app/services/personalAccessToken.server.tsapps/webapp/app/services/platform.v3.server.tsapps/webapp/app/services/projectCreated.server.tsapps/webapp/app/services/rbac.server.tsapps/webapp/app/services/routeBuilders/apiBuilder.server.tsapps/webapp/app/services/routeBuilders/dashboardBuilder.server.tsapps/webapp/app/services/routeBuilders/dashboardBuilder.tsapps/webapp/app/utils/pathBuilder.tsapps/webapp/package.jsonapps/webapp/test/README.mdapps/webapp/test/api-auth.e2e.test.tsapps/webapp/test/auth-api.e2e.full.test.tsapps/webapp/test/auth-cross-cutting.e2e.full.test.tsapps/webapp/test/auth-dashboard.e2e.full.test.tsapps/webapp/test/helpers/seedTestPAT.tsapps/webapp/test/helpers/seedTestRun.tsapps/webapp/test/helpers/seedTestSession.tsapps/webapp/test/helpers/seedTestUserProject.tsapps/webapp/test/helpers/seedTestWaitpoint.tsapps/webapp/test/helpers/sharedTestServer.tsapps/webapp/test/setup/global-e2e-full-setup.tsapps/webapp/vitest.e2e.full.config.tsinternal-packages/database/prisma/migrations/20260430140000_add_rbac_role_id_to_org_member_invite/migration.sqlinternal-packages/database/prisma/schema.prismainternal-packages/rbac/package.jsoninternal-packages/rbac/src/ability.test.tsinternal-packages/rbac/src/ability.tsinternal-packages/rbac/src/fallback.tsinternal-packages/rbac/src/index.tsinternal-packages/rbac/src/loader.test.tsinternal-packages/rbac/tsconfig.jsoninternal-packages/rbac/vitest.config.tsinternal-packages/testcontainers/src/utils.tsinternal-packages/testcontainers/src/webapp.tspackages/plugins/package.jsonpackages/plugins/src/index.tspackages/plugins/src/rbac.tspackages/plugins/tsconfig.jsonpackages/plugins/tsup.config.ts
…ker tightening #1 Batch trigger AND semantics (security): `api.v[12].tasks.batch` now uses `everyResource(...)` so a JWT scoped to taskA can no longer submit a batch that also includes taskB / taskC. Added an `everyResource` helper to `apiBuilder` (Symbol-marked wrapper that flips `ability.can` to `every`). Multi-key OR semantics still apply for single-resource arrays (a run carries multiple identifiers). Updated the e2e test to assert AND behaviour. #3 Realtime stream resource (correctness): `findResource` for `realtime.v1.streams.$runId.$streamId` now selects `taskIdentifier`, `runTags`, and `realtimeStreamsVersion` — fields the auth resource builder + handler read but findResource was returning undefined for. #4 projectCreated optional chaining (crash bug): added the missing `?.` between v3Subscription and plan so a missing subscription no longer throws and aborts project creation. #5 RBAC plugin loader logging: distinguish "plugin itself missing" from "plugin found but a transitive dep failed to resolve" by inspecting the ERR_MODULE_NOT_FOUND error message for the plugin's own module specifier. The transitive-dep case now logs at error level (matches the comment's stated behaviour). Removed the orphan log line that contradicted it. #6 account.tokens picker source mismatch: the picker now sources roles from the same plan-tier-filtered list (`systemRoles().filter(available)`) as the default-role calculation. Added server-side roleId revalidation in the create action so a hand-crafted POST can't bind a PAT to an unavailable role.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
internal-packages/rbac/src/index.ts (1)
85-100:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
message.includes(moduleName)still can't tell “plugin missing” from “plugin broken”.A transitive
ERR_MODULE_NOT_FOUNDusually includes the parent plugin path in the message, so this branch can still classify a broken installed plugin as “not installed” and quietly fall back.Suggested fix
- const isPluginItselfMissing = - isModuleNotFound && message.includes(moduleName); + const missingSpecifier = + message.match(/Cannot find (?:package|module) ['"]([^'"]+)['"]/)?.[1]; + const isPluginItselfMissing = + isModuleNotFound && missingSpecifier === moduleName;In Node.js ESM, when dynamic `import("@triggerdotdev/plugins/rbac")` fails because a transitive dependency is missing, can the thrown `ERR_MODULE_NOT_FOUND` / `MODULE_NOT_FOUND` message still include the parent plugin path, making `err.message.includes("@triggerdotdev/plugins/rbac")` true even though the missing specifier is different?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal-packages/rbac/src/index.ts` around lines 85 - 100, The current check using message.includes(moduleName) can misclassify a plugin with a missing transitive dependency as "not installed"; instead parse the missing-specifier from the thrown error message and compare it exactly to moduleName. Update the logic around isModuleNotFound / isPluginItselfMissing: when err.code is "ERR_MODULE_NOT_FOUND" or "MODULE_NOT_FOUND", extract the missing specifier from err.message with a regex (e.g. capture the quoted module path from messages like "Cannot find module 'X' imported from Y" or similar) and set isPluginItselfMissing only when that extracted specifier === moduleName (or equals the package name form you expect); fall back to the existing console.error branch when you cannot reliably extract or it doesn’t match. Ensure you reference the same variables (moduleName, isModuleNotFound, isPluginItselfMissing) and use the parsedSpecifier in the comparison.
🧹 Nitpick comments (1)
apps/webapp/test/auth-api.e2e.full.test.ts (1)
725-760: ⚡ Quick winAdd the mixed-task denial case for v2 as well.
This block only proves the happy path. It doesn’t lock in the actual security fix for
api.v2.tasks.batch:batchTrigger:tasks:taskAmust still be rejected for[taskA, taskB]. Since v1 and v2 duplicate the authorization closure in separate route files, v1’s negative test won’t catch a future drift in v2.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/webapp/test/auth-api.e2e.full.test.ts` around lines 725 - 760, Add a negative test to the "Trigger task — batch v2 (api.v2.tasks.batch) sanity" suite that mirrors the v1 mixed-task denial: use getTestServer(), seedTestEnvironment(), and generateJWT() to create a token whose scopes only allow batchTrigger:tasks:taskA, then POST to "/api/v2/tasks/batch" with items containing both taskA and taskB and assert the response is rejected (expect 401 or 403). Place the test alongside the existing "JWT with write:tasks: auth passes" case and ensure it checks for the denial status (not accepted) to lock in v2's authorization behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/webapp/app/routes/account.tokens/route.tsx`:
- Around line 114-123: The defaultRoleId can point to an unassignable role;
change its calculation so it is clamped to the actual assignable roles shown in
the picker: when computing defaultRoleId, check if userRoleId exists in the
rendered roles list (the same source used to render the select) and use it only
if found, otherwise fall back to lowestAvailable (from rbac.systemRoles) or "";
update the assignment of defaultRoleId (currently using userRoleId ??
lowestAvailable) to perform this membership check against roles (and keep the
existing fallback behavior).
---
Duplicate comments:
In `@internal-packages/rbac/src/index.ts`:
- Around line 85-100: The current check using message.includes(moduleName) can
misclassify a plugin with a missing transitive dependency as "not installed";
instead parse the missing-specifier from the thrown error message and compare it
exactly to moduleName. Update the logic around isModuleNotFound /
isPluginItselfMissing: when err.code is "ERR_MODULE_NOT_FOUND" or
"MODULE_NOT_FOUND", extract the missing specifier from err.message with a regex
(e.g. capture the quoted module path from messages like "Cannot find module 'X'
imported from Y" or similar) and set isPluginItselfMissing only when that
extracted specifier === moduleName (or equals the package name form you expect);
fall back to the existing console.error branch when you cannot reliably extract
or it doesn’t match. Ensure you reference the same variables (moduleName,
isModuleNotFound, isPluginItselfMissing) and use the parsedSpecifier in the
comparison.
---
Nitpick comments:
In `@apps/webapp/test/auth-api.e2e.full.test.ts`:
- Around line 725-760: Add a negative test to the "Trigger task — batch v2
(api.v2.tasks.batch) sanity" suite that mirrors the v1 mixed-task denial: use
getTestServer(), seedTestEnvironment(), and generateJWT() to create a token
whose scopes only allow batchTrigger:tasks:taskA, then POST to
"/api/v2/tasks/batch" with items containing both taskA and taskB and assert the
response is rejected (expect 401 or 403). Place the test alongside the existing
"JWT with write:tasks: auth passes" case and ensure it checks for the denial
status (not accepted) to lock in v2's authorization behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: cafa6923-4f13-4957-8535-c3b55198bc38
📒 Files selected for processing (8)
apps/webapp/app/routes/account.tokens/route.tsxapps/webapp/app/routes/api.v1.tasks.batch.tsapps/webapp/app/routes/api.v2.tasks.batch.tsapps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.tsapps/webapp/app/services/projectCreated.server.tsapps/webapp/app/services/routeBuilders/apiBuilder.server.tsapps/webapp/test/auth-api.e2e.full.test.tsinternal-packages/rbac/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/webapp/app/services/projectCreated.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). (1)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (15)
**/*.{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/routes/api.v2.tasks.batch.tsapps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.tsinternal-packages/rbac/src/index.tsapps/webapp/test/auth-api.e2e.full.test.tsapps/webapp/app/routes/account.tokens/route.tsxapps/webapp/app/routes/api.v1.tasks.batch.tsapps/webapp/app/services/routeBuilders/apiBuilder.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/routes/api.v2.tasks.batch.tsapps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.tsapps/webapp/test/auth-api.e2e.full.test.tsapps/webapp/app/routes/account.tokens/route.tsxapps/webapp/app/routes/api.v1.tasks.batch.tsapps/webapp/app/services/routeBuilders/apiBuilder.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/routes/api.v2.tasks.batch.tsapps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.tsinternal-packages/rbac/src/index.tsapps/webapp/test/auth-api.e2e.full.test.tsapps/webapp/app/routes/account.tokens/route.tsxapps/webapp/app/routes/api.v1.tasks.batch.tsapps/webapp/app/services/routeBuilders/apiBuilder.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/routes/api.v2.tasks.batch.tsapps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.tsinternal-packages/rbac/src/index.tsapps/webapp/test/auth-api.e2e.full.test.tsapps/webapp/app/routes/api.v1.tasks.batch.tsapps/webapp/app/services/routeBuilders/apiBuilder.server.ts
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/routes/api.v2.tasks.batch.tsapps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.tsapps/webapp/test/auth-api.e2e.full.test.tsapps/webapp/app/routes/account.tokens/route.tsxapps/webapp/app/routes/api.v1.tasks.batch.tsapps/webapp/app/services/routeBuilders/apiBuilder.server.ts
{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/routes/api.v2.tasks.batch.tsapps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.tsinternal-packages/rbac/src/index.tsapps/webapp/test/auth-api.e2e.full.test.tsapps/webapp/app/routes/account.tokens/route.tsxapps/webapp/app/routes/api.v1.tasks.batch.tsapps/webapp/app/services/routeBuilders/apiBuilder.server.ts
{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/routes/api.v2.tasks.batch.tsapps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.tsinternal-packages/rbac/src/index.tsapps/webapp/test/auth-api.e2e.full.test.tsapps/webapp/app/routes/account.tokens/route.tsxapps/webapp/app/routes/api.v1.tasks.batch.tsapps/webapp/app/services/routeBuilders/apiBuilder.server.ts
**/*.{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/routes/api.v2.tasks.batch.tsapps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.tsinternal-packages/rbac/src/index.tsapps/webapp/test/auth-api.e2e.full.test.tsapps/webapp/app/routes/account.tokens/route.tsxapps/webapp/app/routes/api.v1.tasks.batch.tsapps/webapp/app/services/routeBuilders/apiBuilder.server.ts
**/*.{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/routes/api.v2.tasks.batch.tsapps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.tsinternal-packages/rbac/src/index.tsapps/webapp/test/auth-api.e2e.full.test.tsapps/webapp/app/routes/account.tokens/route.tsxapps/webapp/app/routes/api.v1.tasks.batch.tsapps/webapp/app/services/routeBuilders/apiBuilder.server.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use vitest for all tests in the Trigger.dev repository
Files:
apps/webapp/test/auth-api.e2e.full.test.ts
apps/webapp/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Do not import
env.server.tsdirectly or indirectly into test files; instead pass environment-dependent values through options/parameters to make code testableFor testable code, never import
env.server.tsin test files. Pass configuration as options instead (e.g.,realtimeClient.server.tstakes config as constructor arg,realtimeClientGlobal.server.tscreates singleton with env config)
Files:
apps/webapp/test/auth-api.e2e.full.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx,js}: Use vitest exclusively for testing and never mock anything - use testcontainers instead
Place test files next to source files using the patternMyService.ts->MyService.test.ts
**/*.test.{ts,tsx,js}: Use vitest for unit testing and run tests withpnpm run test
Test files should live beside the files under test with descriptivedescribeanditblocks
Tests should avoid mocks or stubs and use helpers from@internal/testcontainerswhen Redis or Postgres are needed
Files:
apps/webapp/test/auth-api.e2e.full.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use testcontainers with
redisTest,postgresTest, orcontainerTestfrom@internal/testcontainersfor testing with Redis/PostgreSQL dependencies
Files:
apps/webapp/test/auth-api.e2e.full.test.ts
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/routes/account.tokens/route.tsx
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/services/routeBuilders/apiBuilder.server.ts
🧠 Learnings (46)
📚 Learning: 2026-03-25T15:29:25.889Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2026-03-25T15:29:25.889Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `batch.triggerByTask()` to batch trigger multiple tasks by passing task instances
Applied to files:
apps/webapp/app/routes/api.v2.tasks.batch.tsapps/webapp/test/auth-api.e2e.full.test.tsapps/webapp/app/routes/api.v1.tasks.batch.ts
📚 Learning: 2026-03-25T15:29:25.889Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2026-03-25T15:29:25.889Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `task.batchTrigger()` to trigger multiple runs of a task from inside another task
Applied to files:
apps/webapp/app/routes/api.v2.tasks.batch.tsapps/webapp/test/auth-api.e2e.full.test.tsapps/webapp/app/routes/api.v1.tasks.batch.ts
📚 Learning: 2026-03-25T15:29:25.889Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2026-03-25T15:29:25.889Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `batch.triggerByTaskAndWait()` to batch trigger multiple tasks by instance and wait for results
Applied to files:
apps/webapp/app/routes/api.v2.tasks.batch.tsapps/webapp/test/auth-api.e2e.full.test.tsapps/webapp/app/routes/api.v1.tasks.batch.ts
📚 Learning: 2026-04-16T14:19:16.330Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: apps/webapp/CLAUDE.md:0-0
Timestamp: 2026-04-16T14:19:16.330Z
Learning: Applies to apps/webapp/{app/v3/services/triggerTask.server.ts,app/v3/services/batchTriggerV3.server.ts} : In `triggerTask.server.ts` and `batchTriggerV3.server.ts`, do NOT add database queries. Task defaults (TTL, etc.) are resolved via `backgroundWorkerTask.findFirst()` in the queue concern (`queues.server.ts`). Piggyback on the existing query instead of adding new ones
Applied to files:
apps/webapp/app/routes/api.v2.tasks.batch.tsapps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.tsapps/webapp/test/auth-api.e2e.full.test.tsapps/webapp/app/routes/api.v1.tasks.batch.ts
📚 Learning: 2026-03-25T15:29:25.889Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2026-03-25T15:29:25.889Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `batch.triggerAndWait()` to trigger multiple different tasks and wait for all results
Applied to files:
apps/webapp/app/routes/api.v2.tasks.batch.tsapps/webapp/test/auth-api.e2e.full.test.tsapps/webapp/app/routes/api.v1.tasks.batch.ts
📚 Learning: 2026-03-25T15:29:25.889Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2026-03-25T15:29:25.889Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `task.batchTriggerAndWait()` to batch trigger a task and wait for all results from inside another task
Applied to files:
apps/webapp/app/routes/api.v2.tasks.batch.tsapps/webapp/test/auth-api.e2e.full.test.tsapps/webapp/app/routes/api.v1.tasks.batch.ts
📚 Learning: 2026-03-31T21:37:31.732Z
Learnt from: isshaddad
Repo: triggerdotdev/trigger.dev PR: 3283
File: docs/migration-n8n.mdx:19-21
Timestamp: 2026-03-31T21:37:31.732Z
Learning: In the trigger.dev SDK (`packages/trigger-sdk/src/v3`), `tasks.triggerAndWait()` and `tasks.batchTriggerAndWait()` are real, valid exported APIs defined in `shared.ts` and re-exported via the `tasks` object in `tasks.ts`. They accept a task ID string as their first argument (not a task instance). These are distinct from the instance methods `yourTask.triggerAndWait()` and `yourTask.batchTriggerAndWait()`. Do not flag `tasks.triggerAndWait()` or `tasks.batchTriggerAndWait()` as non-existent APIs.
Applied to files:
apps/webapp/app/routes/api.v2.tasks.batch.tsapps/webapp/test/auth-api.e2e.full.test.ts
📚 Learning: 2026-04-16T14:19:16.330Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: apps/webapp/CLAUDE.md:0-0
Timestamp: 2026-04-16T14:19:16.330Z
Learning: Applies to apps/webapp/app/v3/services/{cancelTaskRun,batchTriggerV3}.server.ts : When editing services that branch on `RunEngineVersion` to support both V1 and V2 (e.g., `cancelTaskRun.server.ts`, `batchTriggerV3.server.ts`), only modify V2 code paths
Applied to files:
apps/webapp/app/routes/api.v2.tasks.batch.tsapps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.tsapps/webapp/test/auth-api.e2e.full.test.tsapps/webapp/app/routes/api.v1.tasks.batch.ts
📚 Learning: 2026-03-25T15:29:25.889Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2026-03-25T15:29:25.889Z
Learning: Use `tasks.batchTrigger()` to trigger multiple runs of a single task from backend code with different payloads
Applied to files:
apps/webapp/app/routes/api.v2.tasks.batch.tsapps/webapp/app/routes/api.v1.tasks.batch.ts
📚 Learning: 2026-03-25T15:29:25.889Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2026-03-25T15:29:25.889Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `idempotencyKeys.create()` to create idempotency keys for task triggering to ensure idempotent operations
Applied to files:
apps/webapp/app/routes/api.v2.tasks.batch.ts
📚 Learning: 2026-04-13T21:44:00.032Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3368
File: apps/webapp/app/services/taskIdentifierRegistry.server.ts:24-67
Timestamp: 2026-04-13T21:44:00.032Z
Learning: In `apps/webapp/app/services/taskIdentifierRegistry.server.ts`, the sequential upsert/updateMany/findMany writes in `syncTaskIdentifiers` are intentionally NOT wrapped in a Prisma transaction. This function runs only during deployment-change events (low-concurrency path), and any partial `isInLatestDeployment` state is acceptable because it self-corrects on the next deployment. Do not flag this as a missing-transaction/atomicity issue in future reviews.
Applied to files:
apps/webapp/app/routes/api.v2.tasks.batch.tsapps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.tsapps/webapp/test/auth-api.e2e.full.test.ts
📚 Learning: 2026-04-07T14:12:59.018Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3331
File: apps/webapp/app/runEngine/concerns/batchPayloads.server.ts:112-136
Timestamp: 2026-04-07T14:12:59.018Z
Learning: In `apps/webapp/app/runEngine/concerns/batchPayloads.server.ts`, the `pRetry` call wrapping `uploadPacketToObjectStore` intentionally retries **all** error types (no `shouldRetry` filter / `AbortError` guards). The maintainer explicitly prefers over-retrying to under-retrying because multiple heterogeneous object store backends are supported and it is impractical to enumerate all permanent error signatures. Do not flag this as an issue in future reviews.
Applied to files:
apps/webapp/app/routes/api.v2.tasks.batch.ts
📚 Learning: 2026-05-01T15:45:05.096Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3499
File: internal-packages/rbac/src/fallback.ts:34-107
Timestamp: 2026-05-01T15:45:05.096Z
Learning: In triggerdotdev/trigger.dev, `authenticateBearer` (in both the OSS RBAC fallback `internal-packages/rbac/src/fallback.ts` and the cloud RBAC plugin) is intentionally scoped to runtime environment API keys and Public JWTs only. Personal Access Token (PAT) authentication is handled by a separate route builder `createLoaderPATApiRoute` which calls `authenticateApiRequestWithPersonalAccessToken` directly. Do not flag the absence of PAT handling inside `authenticateBearer` as a bug — the two auth paths are architecturally distinct and this is consistent on both OSS and cloud sides.
Applied to files:
apps/webapp/app/routes/api.v2.tasks.batch.tsapps/webapp/test/auth-api.e2e.full.test.tsapps/webapp/app/routes/account.tokens/route.tsx
📚 Learning: 2026-03-25T15:29:25.889Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2026-03-25T15:29:25.889Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `task()` from `trigger.dev/sdk` for basic task definitions with `id` and `run` properties
Applied to files:
apps/webapp/app/routes/api.v2.tasks.batch.tsapps/webapp/test/auth-api.e2e.full.test.ts
📚 Learning: 2026-03-25T15:29:25.889Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2026-03-25T15:29:25.889Z
Learning: Use `batch.trigger()` to trigger multiple different tasks at once from backend code
Applied to files:
apps/webapp/app/routes/api.v2.tasks.batch.tsapps/webapp/app/routes/api.v1.tasks.batch.ts
📚 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/routes/api.v2.tasks.batch.tsapps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.tsinternal-packages/rbac/src/index.tsapps/webapp/test/auth-api.e2e.full.test.tsapps/webapp/app/routes/account.tokens/route.tsxapps/webapp/app/routes/api.v1.tasks.batch.tsapps/webapp/app/services/routeBuilders/apiBuilder.server.ts
📚 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/routes/api.v2.tasks.batch.tsapps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.tsinternal-packages/rbac/src/index.tsapps/webapp/test/auth-api.e2e.full.test.tsapps/webapp/app/routes/account.tokens/route.tsxapps/webapp/app/routes/api.v1.tasks.batch.tsapps/webapp/app/services/routeBuilders/apiBuilder.server.ts
📚 Learning: 2026-04-20T15:06:19.815Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3417
File: apps/webapp/app/routes/realtime.v1.sessions.$session.$io.ts:37-51
Timestamp: 2026-04-20T15:06:19.815Z
Learning: In `apps/webapp/app/routes/realtime.v1.sessions.$session.$io.ts` (and all session realtime read paths), `$replica` is intentionally used for the `resolveSessionByIdOrExternalId` call — including the `closedAt` guard in the PUT/initialize path. The project convention is to use `$replica` consistently across all session realtime routes. The race window (replica lag allowing a ghost-initialize after close) is accepted as not realistic in practice (clients follow the close API response; they do not race it). If replica lag ever causes issues, the mitigation is to revisit all realtime routes together, not to swap individual routes to `prisma`. Do not flag `$replica` usage in session realtime routes as a stale-read issue.
Applied to files:
apps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.ts
📚 Learning: 2026-04-20T15:06:11.054Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3417
File: apps/webapp/app/routes/realtime.v1.streams.$runId.$target.$streamId.append.ts:16-26
Timestamp: 2026-04-20T15:06:11.054Z
Learning: In `apps/webapp/app/routes/realtime.v1.streams.$runId.$target.$streamId.append.ts` and `apps/webapp/app/routes/realtime.v1.sessions.$session.$io.append.ts`, the `MAX_APPEND_BODY_BYTES` cap of 512 KiB (1024 * 512) is intentional even though `appendPart` wraps the body in JSON (which could expand quote-heavy payloads beyond S2's 1 MiB per-record limit). The maintainer considers worst-case quote-heavy payloads pathological and not realistic. If S2 rejections occur in practice, an encoded-size guard will be added inside `appendPart` rather than lowering the raw body cap on every caller. Do not flag this as an issue in future reviews.
Applied to files:
apps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.ts
📚 Learning: 2026-04-16T13:24:09.546Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3399
File: apps/webapp/app/services/realtime/redisRealtimeStreams.server.ts:26-42
Timestamp: 2026-04-16T13:24:09.546Z
Learning: In `apps/webapp/app/services/realtime/redisRealtimeStreams.server.ts`, `RedisRealtimeStreams` is only ever instantiated once as a process-wide singleton via `singleton("realtimeStreams", initializeRedisRealtimeStreams)` in `apps/webapp/app/services/realtime/v1StreamsGlobal.server.ts` (line 30). Therefore, the instance-level `_sharedRedis` field and `sharedRedis` getter are effectively process-scoped. Do not flag them as a per-request connection leak. The v2 streaming path uses a completely separate class (`S2RealtimeStreams`).
Applied to files:
apps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.ts
📚 Learning: 2026-03-25T15:29:25.889Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2026-03-25T15:29:25.889Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `metadata.stream()` to stream data in realtime from inside tasks
Applied to files:
apps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.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/routes/realtime.v1.streams.$runId.$streamId.ts
📚 Learning: 2026-04-16T14:19:16.330Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: apps/webapp/CLAUDE.md:0-0
Timestamp: 2026-04-16T14:19:16.330Z
Learning: Applies to apps/webapp/app/v3/services/queues.server.ts : If adding a new task-level default, add it to the existing `select` clause in the `backgroundWorkerTask.findFirst()` query in `queues.server.ts` — do NOT add a second query. If the default doesn't need to be known at trigger time, resolve it at dequeue time instead
Applied to files:
apps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.ts
📚 Learning: 2026-03-24T10:42:43.111Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3255
File: apps/webapp/app/routes/api.v1.runs.$runId.spans.$spanId.ts:100-100
Timestamp: 2026-03-24T10:42:43.111Z
Learning: In `apps/webapp/app/routes/api.v1.runs.$runId.spans.$spanId.ts` (and related span-handling code in trigger.dev), `span.entity` is a required (non-optional) field on the `SpanDetail` type and is always present. Do not flag `span.entity.type` as a potential null pointer / suggest optional chaining (`span.entity?.type`) in this context.
Applied to files:
apps/webapp/app/routes/realtime.v1.streams.$runId.$streamId.ts
📚 Learning: 2026-04-27T16:39:43.098Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3453
File: internal-packages/run-engine/src/engine/systems/debounceSystem.ts:517-547
Timestamp: 2026-04-27T16:39:43.098Z
Learning: In `internal-packages/run-engine/src/engine/systems/debounceSystem.ts`, the `try/catch` around `runLock.lock(...)` in `handleExistingRun` routes errors matching `#isLockContentionError` (`LockAcquisitionTimeoutError`, `name === "ExecutionError"`, `name === "ResourceLockedError"`) to a fallback. This is intentionally NOT guarded by a `lockAcquired` flag because the only code executed inside the lock callback (`#handleExistingRunLocked`) calls Prisma and ioredis, neither of which emits errors with those names — those names are redlock-specific. There are no nested `runLock.lock` calls in this path so callback-thrown errors cannot be misclassified. A `lockAcquired` guard should be revisited only if a nested lock call is ever introduced inside `#handleExistingRunLocked`.
Applied to files:
internal-packages/rbac/src/index.ts
📚 Learning: 2026-05-01T15:45:05.096Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3499
File: internal-packages/rbac/src/fallback.ts:34-107
Timestamp: 2026-05-01T15:45:05.096Z
Learning: When reviewing triggerdotdev/trigger.dev RBAC auth code, do not treat missing Personal Access Token (PAT) handling inside `authenticateBearer` as a bug. `authenticateBearer` is intentionally scoped to runtime environment API keys and Public JWTs only; PAT auth is handled via the separate PAT route builder (e.g., `createLoaderPATApiRoute`) which calls `authenticateApiRequestWithPersonalAccessToken` directly. Ensure that reviewers compare auth behavior against these distinct architectural paths (OSS fallback and cloud plugin) before flagging an issue.
Applied to files:
internal-packages/rbac/src/index.ts
📚 Learning: 2026-05-01T15:45:05.518Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3499
File: apps/webapp/test/auth-cross-cutting.e2e.full.test.ts:206-213
Timestamp: 2026-05-01T15:45:05.518Z
Learning: In `apps/webapp/test/auth-cross-cutting.e2e.full.test.ts`, the cross-environment JWT isolation test intentionally asserts `expect([401, 404]).toContain(res.status)` rather than a strict `expect(res.status).toBe(404)`. The dual-status assertion is deliberate: both 401 (auth rejected) and 404 (resource not found in the resolved env) prove the negative — that the JWT cannot access a resource scoped to a different environment. The loose assertion is kept so a planned change to the auth response code (e.g. returning 404 instead of 401 for cross-env mismatch) does not immediately break this test. The control case that proves the JWT itself is valid is covered by other tests in the same describe block.
Applied to files:
apps/webapp/test/auth-api.e2e.full.test.ts
📚 Learning: 2026-04-07T14:12:18.946Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3331
File: apps/webapp/test/engine/batchPayloads.test.ts:5-24
Timestamp: 2026-04-07T14:12:18.946Z
Learning: In `apps/webapp/test/engine/batchPayloads.test.ts`, using `vi.mock` for `~/v3/objectStore.server` (stubbing `hasObjectStoreClient` and `uploadPacketToObjectStore`), `~/env.server` (overriding offload thresholds), and `~/v3/tracer.server` (stubbing `startActiveSpan`) is intentional and acceptable. Simulating controlled transient upload failures (e.g., fail N times then succeed) to verify `p-retry` behavior cannot be reproduced with real services or testcontainers. This file is an explicit exception to the repo's general no-mocks policy.
Applied to files:
apps/webapp/test/auth-api.e2e.full.test.ts
📚 Learning: 2026-04-16T13:45:22.317Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3368
File: apps/webapp/test/engine/taskIdentifierRegistry.test.ts:3-19
Timestamp: 2026-04-16T13:45:22.317Z
Learning: In `apps/webapp/test/engine/taskIdentifierRegistry.test.ts`, the `vi.mock` calls for `~/services/taskIdentifierCache.server` (stubbing `getTaskIdentifiersFromCache` and `populateTaskIdentifierCache`), `~/models/task.server` (stubbing `getAllTaskIdentifiers`), and `~/db.server` (stubbing `prisma` and `$replica`) are intentional. The suite uses real Postgres via testcontainers for all `TaskIdentifier` DB operations, but isolates the Redis cache layer and legacy query fallback as separate concerns not exercised in this test file. Do not flag these mocks as violations of the no-mocks policy in future reviews.
Applied to files:
apps/webapp/test/auth-api.e2e.full.test.ts
📚 Learning: 2026-04-15T15:39:31.575Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2026-04-15T15:39:31.575Z
Learning: Applies to apps/webapp/**/*.test.{ts,tsx} : Do not import `env.server.ts` directly or indirectly into test files; instead pass environment-dependent values through options/parameters to make code testable
Applied to files:
apps/webapp/test/auth-api.e2e.full.test.tsapps/webapp/app/routes/api.v1.tasks.batch.ts
📚 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 **/*.{test,spec}.{ts,tsx} : Use vitest for all tests in the Trigger.dev repository
Applied to files:
apps/webapp/test/auth-api.e2e.full.test.ts
📚 Learning: 2026-03-02T12:43:25.254Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: internal-packages/run-engine/CLAUDE.md:0-0
Timestamp: 2026-03-02T12:43:25.254Z
Learning: Applies to internal-packages/run-engine/src/engine/tests/**/*.test.ts : Implement tests for RunEngine in `src/engine/tests/` using testcontainers for Redis and PostgreSQL containerization
Applied to files:
apps/webapp/test/auth-api.e2e.full.test.ts
📚 Learning: 2026-05-01T15:24:56.404Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-05-01T15:24:56.404Z
Learning: Applies to **/*.test.{ts,tsx,js} : Use vitest exclusively for testing and never mock anything - use testcontainers instead
Applied to files:
apps/webapp/test/auth-api.e2e.full.test.ts
📚 Learning: 2026-04-16T14:19:16.330Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: apps/webapp/CLAUDE.md:0-0
Timestamp: 2026-04-16T14:19:16.330Z
Learning: Applies to apps/webapp/**/*.test.{ts,tsx} : For testable code, never import `env.server.ts` in test files. Pass configuration as options instead (e.g., `realtimeClient.server.ts` takes config as constructor arg, `realtimeClientGlobal.server.ts` creates singleton with env config)
Applied to files:
apps/webapp/test/auth-api.e2e.full.test.tsapps/webapp/app/routes/api.v1.tasks.batch.ts
📚 Learning: 2026-03-03T13:07:33.177Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3166
File: internal-packages/run-engine/src/batch-queue/tests/index.test.ts:711-713
Timestamp: 2026-03-03T13:07:33.177Z
Learning: In `internal-packages/run-engine/src/batch-queue/tests/index.test.ts`, test assertions for rate limiter stubs can use `toBeGreaterThanOrEqual` rather than exact equality (`toBe`) because the consumer loop may call the rate limiter during empty pops in addition to actual item processing, and this over-calling is acceptable in integration tests.
Applied to files:
apps/webapp/test/auth-api.e2e.full.test.ts
📚 Learning: 2026-05-01T15:44:47.539Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3499
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.roles/route.tsx:170-173
Timestamp: 2026-05-01T15:44:47.539Z
Learning: In `apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.roles/route.tsx`, the `CreateRoleUpsell` component is intentionally only rendered for non-enterprise plans (`!isEnterprise`). Enterprise plans intentionally have no "Create role" CTA because the create-role flow is not yet built. A real Create button will be added in a separate ticket alongside the create-role action wiring. Do not flag the missing enterprise create-role entry point as a bug until that ticket lands.
Applied to files:
apps/webapp/app/routes/account.tokens/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/account.tokens/route.tsx
📚 Learning: 2026-04-29T21:49:48.296Z
Learnt from: isshaddad
Repo: triggerdotdev/trigger.dev PR: 3475
File: apps/webapp/app/components/admin/backOffice/RateLimitSection.tsx:61-70
Timestamp: 2026-04-29T21:49:48.296Z
Learning: In `apps/webapp/app/components/admin/backOffice/RateLimitSection.tsx`, the local form state (`refillRate`, `intervalStr`, `maxTokens`) is intentionally seeded only once via `useState` initializers from `current` (the effective token-bucket config). This is safe in the Remix model because: (1) a successful save redirects, causing a remount with fresh loader data; (2) a failed 400 returns no redirect, so `current` stays the same and React preserves the user's typed input; (3) navigating to a different org causes remount and re-seeds state; (4) Cancel explicitly re-seeds via `cancelEdit()`. Do NOT add a `useEffect` that re-seeds from `current` on config changes — it would clobber mid-edit valid input during background revalidation, which is a regression.
Applied to files:
apps/webapp/app/routes/account.tokens/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 internal-packages/database/**/*.{ts,tsx} : Use Prisma for database interactions in internal-packages/database with PostgreSQL
Applied to files:
apps/webapp/app/routes/account.tokens/route.tsx
📚 Learning: 2026-03-13T13:42:25.092Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3213
File: apps/webapp/app/routes/admin.llm-models.new.tsx:65-91
Timestamp: 2026-03-13T13:42:25.092Z
Learning: In `apps/webapp/app/routes/admin.llm-models.new.tsx`, sequential Prisma writes for model/tier creation are intentionally not wrapped in a transaction. The form is admin-only with low concurrency risk, and the blast radius is considered minimal for admin tooling.
Applied to files:
apps/webapp/app/routes/account.tokens/route.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/account.tokens/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/account.tokens/route.tsx
📚 Learning: 2026-04-02T19:18:26.255Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3319
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.bulk-actions/route.tsx:179-189
Timestamp: 2026-04-02T19:18:26.255Z
Learning: In this repo’s route components that render the Inspector `ResizablePanelGroup` panels, it’s acceptable to pass `collapsed={!isShowingInspector}` together with a no-op `onCollapseChange={() => {}}` when panel visibility is intentionally controlled only by route parameters (e.g., `*Param` search/route params) rather than user drag/collapse interactions. Do not flag an empty/no-op `onCollapseChange` as “missing wiring” in these cases; only flag it when collapse state is expected to change based on user interaction.
Applied to files:
apps/webapp/app/routes/account.tokens/route.tsx
📚 Learning: 2026-04-16T14:19:16.330Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: apps/webapp/CLAUDE.md:0-0
Timestamp: 2026-04-16T14:19:16.330Z
Learning: Applies to apps/webapp/**/*.server.ts : Access environment variables via `env` export from `app/env.server.ts`. Never use `process.env` directly
Applied to files:
apps/webapp/app/routes/api.v1.tasks.batch.ts
📚 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/services/routeBuilders/apiBuilder.server.ts
📚 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/routeBuilders/apiBuilder.server.ts
Comment on lines
+114
to
+123
| // Default the role picker to the user's own role in their primary | ||
| // org so a freshly-created PAT isn't more privileged than the | ||
| // person creating it. Falls back to the most-restrictive role | ||
| // available on the org's plan if they don't have one. When the | ||
| // user isn't a member of any org or no RBAC plugin is installed, | ||
| // the picker is hidden anyway, so defaultRoleId is just a | ||
| // placeholder. | ||
| const sys = orgId ? await rbac.systemRoles(orgId) : null; | ||
| const lowestAvailable = (sys ?? []).filter((r) => r.available).at(-1)?.id ?? ""; | ||
| const defaultRoleId = userRoleId ?? lowestAvailable; |
Contributor
There was a problem hiding this comment.
Clamp defaultRoleId to the assignable system-role list.
defaultRoleId = userRoleId ?? lowestAvailable can point at a custom role or a plan-blocked system role, while the picker only renders roles. In that case the hidden roleId posts an invalid default and PAT creation bounces with the new 400 validation until the user manually changes the select.
Suggested fix
- const sys = orgId ? await rbac.systemRoles(orgId) : null;
- const lowestAvailable = (sys ?? []).filter((r) => r.available).at(-1)?.id ?? "";
- const defaultRoleId = userRoleId ?? lowestAvailable;
+ const availableRoleIds = new Set(roles.map((role) => role.id));
+ const lowestAvailable = roles.at(-1)?.id ?? "";
+ const defaultRoleId =
+ userRoleId && availableRoleIds.has(userRoleId) ? userRoleId : lowestAvailable;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/webapp/app/routes/account.tokens/route.tsx` around lines 114 - 123, The
defaultRoleId can point to an unassignable role; change its calculation so it is
clamped to the actual assignable roles shown in the picker: when computing
defaultRoleId, check if userRoleId exists in the rendered roles list (the same
source used to render the select) and use it only if found, otherwise fall back
to lowestAvailable (from rbac.systemRoles) or ""; update the assignment of
defaultRoleId (currently using userRoleId ?? lowestAvailable) to perform this
membership check against roles (and keep the existing fallback behavior).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Consolidates the webapp's authentication and authorization into a small set of route helpers, replacing the ad-hoc
requireUser/requireUserId/authenticatedEnvironmentForAuthenticationcalls scattered across routes. Same security model, but the per-request flow (authenticate → authorize → load) now lives in one place per route family.Adds a comprehensive end-to-end auth test suite that didn't exist before — 162 tests covering API key, PAT and JWT auth across the public API surface, plus dashboard session auth for admin pages.
Changes
Dashboard auth (started, partial rollout)
Admin and settings pages migrated to a unified loader/action helper that authenticates the session, runs an authorization check, and exposes the result to the route. Other dashboard routes still on the old pattern; remaining migration tracked separately.
Migrated routes:
admin.*(14 admin / back-office / feature-flags / LLM-models / notifications / orgs / concurrency pages)_app.orgs.$organizationSlug.settings.team_app.orgs.$organizationSlug.settings.rolesAPI / realtime / engine auth (complete for the migrated families)
71 routes migrated to a unified
apiBuilderthat centralizes Bearer / PAT / Public-JWT authentication and applies the per-route authorization check before the handler runs. Includes:api.v1.*andapi.v2.*andapi.v3.*— tasks, runs, batches, queues, prompts, deployments, query, sessions, waitpoints, packets, workers, idempotency keysrealtime.v1.*— runs, batches, sessions, streamsengine.v1.*— dev / worker-action protocolsSide effect: action aliases preserved historic JWT scope semantics where the new model is stricter (e.g. a
write:tasksJWT now also satisfiestrigger/batchTrigger/updateactions on the same resource — matched at the auth boundary, not in the route handler).Auth test suite (new —
*.e2e.full.test.ts)162 e2e tests run against a real spawned webapp + Postgres (no mocks). Coverage matrix:
Test plan
pnpm run typecheck --filter webappcleanpnpm exec vitest run --config apps/webapp/vitest.e2e.full.config.ts— 162/162 pass