Query enabled via feature flag #2968
Conversation
|
WalkthroughAdds a global environment flag Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (7)**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
{packages/core,apps/webapp}/**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
**/*.ts📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (1)📚 Learning: 2025-11-27T16:26:58.661ZApplied to files:
🧬 Code graph analysis (1)apps/webapp/app/v3/eventRepository/index.server.ts (1)
⏰ 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). (22)
🔇 Additional comments (3)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Review CompleteYour review story is ready! Comment !reviewfast on this PR to re-generate the story. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
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/v3/eventRepository/index.server.ts (1)
125-129: Critical: Variable shadowing causes runtime error.Line 125 declares a local variable
flagthat shadows the importedflagfunction. This creates a temporal dead zone (TDZ) error becauseflagis referenced before its declaration in the same statement.The static analysis tool correctly identified this: "This variable is used before its declaration."
🐛 Proposed fix: Rename the local variable
async function resolveTaskEventRepositoryFlag( featureFlags: Record<string, unknown> | undefined ): Promise<"clickhouse" | "clickhouse_v2" | "postgres"> { - const flag = await flag({ + const taskEventRepositoryFlag = await flag({ key: FEATURE_FLAG.taskEventRepository, defaultValue: env.EVENT_REPOSITORY_DEFAULT_STORE, overrides: featureFlags, }); - if (flag === "clickhouse_v2") { + if (taskEventRepositoryFlag === "clickhouse_v2") { return "clickhouse_v2"; } - if (flag === "clickhouse") { + if (taskEventRepositoryFlag === "clickhouse") { return "clickhouse"; } - return flag; + return taskEventRepositoryFlag; }
🧹 Nitpick comments (3)
apps/webapp/app/v3/featureFlags.server.ts (1)
85-88: Type reference ordering concern.
AllFlagsOptionsreferencesPartial<FeatureFlagCatalog>butFeatureFlagCatalogis defined later at line 141. While TypeScript's type hoisting handles this, consider reordering for clarity, or note that this works because types are hoisted.apps/webapp/app/v3/services/worker/workerGroupService.server.ts (1)
169-173: Inconsistent variable naming.The variable is named
flagsbut it's actuallymakeFlag(singular flag accessor). This is inconsistent with line 50 where the same pattern usesgetFlag. Consider renaming for clarity and consistency.♻️ Proposed fix for naming consistency
async getGlobalDefaultWorkerGroup() { - const flags = makeFlag(this._prisma); + const getFlag = makeFlag(this._prisma); - const defaultWorkerInstanceGroupId = await flags({ + const defaultWorkerInstanceGroupId = await getFlag({ key: FEATURE_FLAG.defaultWorkerInstanceGroupId, });apps/webapp/app/v3/canAccessQuery.server.ts (1)
35-40: Consider using the pre-exportedflagfunction instead of creating a new instance.The
featureFlags.server.tsfile already exports a pre-instantiatedflagfunction at line 134 (export const flag = makeFlag();). Using it directly would be more consistent with other usages in the codebase.♻️ Suggested refactor
-import { FEATURE_FLAG, makeFlag } from "~/v3/featureFlags.server"; +import { FEATURE_FLAG, flag } from "~/v3/featureFlags.server"; ... - const flag = makeFlag(); - const flagResult = await flag({ + const flagResult = await flag({ key: FEATURE_FLAG.hasQueryAccess, defaultValue: false, overrides: (org?.featureFlags as Record<string, unknown>) ?? {}, });
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/webapp/app/env.server.tsapps/webapp/app/presenters/OrganizationsPresenter.server.tsapps/webapp/app/presenters/v3/RegionsPresenter.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/v3/canAccessQuery.server.tsapps/webapp/app/v3/eventRepository/index.server.tsapps/webapp/app/v3/featureFlags.server.tsapps/webapp/app/v3/services/worker/workerGroupService.server.ts
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
apps/webapp/app/presenters/v3/RegionsPresenter.server.tsapps/webapp/app/v3/canAccessQuery.server.tsapps/webapp/app/presenters/OrganizationsPresenter.server.tsapps/webapp/app/v3/services/worker/workerGroupService.server.tsapps/webapp/app/v3/eventRepository/index.server.tsapps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/v3/featureFlags.server.tsapps/webapp/app/env.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/presenters/v3/RegionsPresenter.server.tsapps/webapp/app/v3/canAccessQuery.server.tsapps/webapp/app/presenters/OrganizationsPresenter.server.tsapps/webapp/app/v3/services/worker/workerGroupService.server.tsapps/webapp/app/v3/eventRepository/index.server.tsapps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/v3/featureFlags.server.tsapps/webapp/app/env.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
apps/webapp/app/presenters/v3/RegionsPresenter.server.tsapps/webapp/app/v3/canAccessQuery.server.tsapps/webapp/app/presenters/OrganizationsPresenter.server.tsapps/webapp/app/v3/services/worker/workerGroupService.server.tsapps/webapp/app/v3/eventRepository/index.server.tsapps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/v3/featureFlags.server.tsapps/webapp/app/env.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/presenters/v3/RegionsPresenter.server.tsapps/webapp/app/v3/canAccessQuery.server.tsapps/webapp/app/presenters/OrganizationsPresenter.server.tsapps/webapp/app/v3/services/worker/workerGroupService.server.tsapps/webapp/app/v3/eventRepository/index.server.tsapps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/v3/featureFlags.server.tsapps/webapp/app/env.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webappAccess environment variables via
envexport fromapps/webapp/app/env.server.ts, never useprocess.envdirectly
Files:
apps/webapp/app/presenters/v3/RegionsPresenter.server.tsapps/webapp/app/v3/canAccessQuery.server.tsapps/webapp/app/presenters/OrganizationsPresenter.server.tsapps/webapp/app/v3/services/worker/workerGroupService.server.tsapps/webapp/app/v3/eventRepository/index.server.tsapps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/v3/featureFlags.server.tsapps/webapp/app/env.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/app/presenters/v3/RegionsPresenter.server.tsapps/webapp/app/v3/canAccessQuery.server.tsapps/webapp/app/presenters/OrganizationsPresenter.server.tsapps/webapp/app/v3/services/worker/workerGroupService.server.tsapps/webapp/app/v3/eventRepository/index.server.tsapps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/v3/featureFlags.server.tsapps/webapp/app/env.server.ts
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/presenters/v3/RegionsPresenter.server.tsapps/webapp/app/v3/canAccessQuery.server.tsapps/webapp/app/presenters/OrganizationsPresenter.server.tsapps/webapp/app/v3/services/worker/workerGroupService.server.tsapps/webapp/app/v3/eventRepository/index.server.tsapps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/v3/featureFlags.server.tsapps/webapp/app/env.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
apps/webapp/app/v3/services/**/*.server.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Organize services in the webapp following the pattern
app/v3/services/*/*.server.ts
Files:
apps/webapp/app/v3/services/worker/workerGroupService.server.ts
apps/webapp/app/services/**/*.server.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Separate testable services from configuration files; follow the pattern of
realtimeClient.server.ts(testable service) andrealtimeClientGlobal.server.ts(configuration) in the webapp
Files:
apps/webapp/app/services/runsRepository/runsRepository.server.ts
🧠 Learnings (17)
📚 Learning: 2025-08-06T14:25:20.438Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2354
File: apps/webapp/app/presenters/v3/RegionsPresenter.server.ts:117-131
Timestamp: 2025-08-06T14:25:20.438Z
Learning: In RegionsPresenter.server.ts, the final sort by name that removes the "default first" ordering is intentional behavior as a way to prefer the default, as clarified by matt-aitken.
Applied to files:
apps/webapp/app/presenters/v3/RegionsPresenter.server.ts
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/v3/presenters/**/*.server.{ts,tsx} : Organize presenters in the webapp following the pattern `app/v3/presenters/*/*.server.ts` to move complex loader code into classes
Applied to files:
apps/webapp/app/presenters/v3/RegionsPresenter.server.tsapps/webapp/app/presenters/OrganizationsPresenter.server.ts
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Leverage the PostgreSQL database through the `trigger.dev/database` Prisma 6.14.0 client in the webapp for all data access patterns
Applied to files:
apps/webapp/app/presenters/OrganizationsPresenter.server.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 internal-packages/database/**/*.{ts,tsx} : Use Prisma for database interactions in internal-packages/database with PostgreSQL
Applied to files:
apps/webapp/app/presenters/OrganizationsPresenter.server.tsapps/webapp/app/services/runsRepository/runsRepository.server.tsapps/webapp/app/v3/featureFlags.server.ts
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/**/*.{ts,tsx} : Access all environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env` in the Trigger.dev webapp
Applied to files:
apps/webapp/app/presenters/OrganizationsPresenter.server.tsapps/webapp/app/env.server.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/trigger-sdk/**/*.{ts,tsx} : In the Trigger.dev SDK (packages/trigger-sdk), prefer isomorphic code like fetch and ReadableStream instead of Node.js-specific code
Applied to files:
apps/webapp/app/presenters/OrganizationsPresenter.server.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger.config.ts : Specify runtime environment (node or bun) in trigger.config.ts using the `runtime` property
Applied to files:
apps/webapp/app/presenters/OrganizationsPresenter.server.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: The SDK at packages/trigger-sdk is an isomorphic TypeScript SDK
Applied to files:
apps/webapp/app/presenters/OrganizationsPresenter.server.ts
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Applied to files:
apps/webapp/app/presenters/OrganizationsPresenter.server.tsapps/webapp/app/v3/eventRepository/index.server.tsapps/webapp/app/services/runsRepository/runsRepository.server.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `trigger.dev/sdk/v3` for all imports in Trigger.dev tasks
Applied to files:
apps/webapp/app/presenters/OrganizationsPresenter.server.ts
📚 Learning: 2025-08-14T10:09:02.528Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2390
File: internal-packages/run-engine/src/engine/index.ts:466-467
Timestamp: 2025-08-14T10:09:02.528Z
Learning: In the triggerdotdev/trigger.dev codebase, it's acceptable to pass `string | undefined` types directly to Prisma operations (both create and update). The codebase consistently uses this pattern and the team is comfortable with how Prisma handles undefined values.
Applied to files:
apps/webapp/app/presenters/OrganizationsPresenter.server.ts
📚 Learning: 2025-07-12T18:06:04.133Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2264
File: apps/webapp/app/services/runsRepository.server.ts:172-174
Timestamp: 2025-07-12T18:06:04.133Z
Learning: In apps/webapp/app/services/runsRepository.server.ts, the in-memory status filtering after fetching runs from Prisma is intentionally used as a workaround for ClickHouse data delays. This approach is acceptable because the result set is limited to a maximum of 100 runs due to pagination, making the performance impact negligible.
Applied to files:
apps/webapp/app/services/runsRepository/runsRepository.server.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/runsRepository/runsRepository.server.ts
📚 Learning: 2026-01-15T11:50:06.067Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T11:50:06.067Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : Access environment variables via `env` export from `apps/webapp/app/env.server.ts`, never use `process.env` directly
Applied to files:
apps/webapp/app/env.server.ts
📚 Learning: 2025-08-14T18:35:44.370Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2390
File: apps/webapp/app/env.server.ts:764-765
Timestamp: 2025-08-14T18:35:44.370Z
Learning: The BoolEnv helper in apps/webapp/app/utils/boolEnv.ts uses z.preprocess with inconsistent default value types across the codebase - some usages pass boolean defaults (correct) while others pass string defaults (incorrect), leading to type confusion. The helper should enforce boolean-only defaults or have clearer documentation.
Applied to files:
apps/webapp/app/env.server.ts
📚 Learning: 2025-12-08T15:19:56.823Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2760
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx:278-281
Timestamp: 2025-12-08T15:19:56.823Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx, the tableState search parameter uses intentional double-encoding: the parameter value contains a URL-encoded URLSearchParams string, so decodeURIComponent(value("tableState") ?? "") is required to fully decode it before parsing with new URLSearchParams(). This pattern allows bundling multiple filter/pagination params as a single search parameter.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
📚 Learning: 2025-09-03T14:35:52.384Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 2464
File: apps/webapp/app/utils/pathBuilder.ts:144-146
Timestamp: 2025-09-03T14:35:52.384Z
Learning: In the trigger.dev codebase, organization slugs are safe for URL query parameters and don't require URL encoding, as confirmed by the maintainer in apps/webapp/app/utils/pathBuilder.ts.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
🧬 Code graph analysis (7)
apps/webapp/app/presenters/v3/RegionsPresenter.server.ts (1)
apps/webapp/app/v3/featureFlags.server.ts (1)
makeFlag(28-64)
apps/webapp/app/v3/canAccessQuery.server.ts (2)
apps/webapp/app/env.server.ts (1)
env(1324-1324)apps/webapp/app/v3/featureFlags.server.ts (3)
flag(135-135)makeFlag(28-64)FEATURE_FLAG(4-10)
apps/webapp/app/presenters/OrganizationsPresenter.server.ts (2)
apps/webapp/app/v3/featureFlags.server.ts (2)
flags(136-136)validatePartialFeatureFlags(157-159)apps/webapp/app/components/primitives/Avatar.tsx (2)
parseAvatar(37-50)defaultAvatar(105-108)
apps/webapp/app/v3/services/worker/workerGroupService.server.ts (1)
apps/webapp/app/v3/featureFlags.server.ts (5)
makeFlag(28-64)FEATURE_FLAG(4-10)setFlag(137-137)makeSetFlag(66-83)flags(136-136)
apps/webapp/app/v3/eventRepository/index.server.ts (1)
apps/webapp/app/v3/featureFlags.server.ts (1)
flag(135-135)
apps/webapp/app/services/runsRepository/runsRepository.server.ts (1)
apps/webapp/app/v3/featureFlags.server.ts (1)
makeFlag(28-64)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx (1)
apps/webapp/app/v3/canAccessQuery.server.ts (1)
canAccessQuery(5-47)
🪛 Biome (2.3.13)
apps/webapp/app/v3/eventRepository/index.server.ts
[error] 125-125: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (11)
apps/webapp/app/v3/featureFlags.server.ts (1)
90-133: LGTM!The new
makeFlagsfunction correctly implements the precedence logic (overrides > database > defaults) for retrieving all feature flags. Theas anycasts are acceptable for dynamic key assignment in this context.apps/webapp/app/presenters/v3/RegionsPresenter.server.ts (1)
3-3: LGTM!The import and usage have been correctly updated to use
makeFlag(singular) for retrieving a single feature flag value.Also applies to: 51-54
apps/webapp/app/services/runsRepository/runsRepository.server.ts (1)
11-11: LGTM!The import and usage have been correctly updated to use
makeFlagfor retrieving therunsListRepositoryfeature flag.Also applies to: 166-170
apps/webapp/app/presenters/OrganizationsPresenter.server.ts (1)
156-166: LGTM!The implementation correctly fetches global flags once and merges them with organization-specific flags, giving org flags precedence via spread order
{ ...globalFlags, ...orgFlags }. This is an efficient and clean pattern for global defaults with per-org overrides.apps/webapp/app/v3/services/worker/workerGroupService.server.ts (1)
5-5: LGTM!The import and usage in
createWorkerGrouphave been correctly updated to use the singularmakeFlagandmakeSetFlagfunctions with appropriate variable naming (getFlag,setFlag).Also applies to: 50-61
apps/webapp/app/env.server.ts (1)
1193-1195: LGTM!The new
QUERY_FEATURE_ENABLEDenvironment variable follows the established pattern in this file for feature flags (string "1"/"0" convention). The placement near other query-related settings is logical.apps/webapp/app/v3/canAccessQuery.server.ts (1)
5-47: LGTM - Well-structured access control logic.The cascading access check (global flag → admin/impersonation → org feature flag → deny) is clear and follows the principle of least privilege. The implicit membership verification via the Prisma query is a good pattern that ensures non-members cannot access the feature.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx (4)
78-78: LGTM - Clean import of centralized access control.The import follows the established pattern and correctly references the new
canAccessQueryfunction from the v3 module.
100-112: LGTM - Access control properly integrated in loader.The
canAccessQueryfunction is called early in the loader before expensive database operations, with appropriate redirect behavior for unauthorized access.
165-190: LGTM - Consistent access control in action.The action appropriately returns a 403 JSON response for unauthorized access, which is correct for form submissions (as opposed to the loader's redirect behavior).
94-98: LGTM - Organization scope correctly added.The new "organization" scope option aligns with the
ActionSchemavalidation and theenforcedWhereClauselogic at lines 299-305, which correctly handles tenant isolation for all three scope levels.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- Renamed FeatureFlag functions to be singular where it makes sense. - Added function to handle multiple feature flags - canAccessQuery now checks the global feature flag and environment variable as well <!-- devin-review-badge-begin --> --- <a href="https://app.devin.ai/review/triggerdotdev/trigger.dev/pull/2968"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1"> <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open with Devin"> </picture> </a> <!-- devin-review-badge-end -->
Uh oh!
There was an error while loading. Please reload this page.