Conversation
🦋 Changeset detectedLatest commit: bd1484d The changes in this PR will be included in the next version bump. This PR includes changesets to release 29 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 |
WalkthroughThis pull request introduces comprehensive private networking support through AWS PrivateLink across the Trigger.dev platform. The implementation spans multiple layers: backend dequeue system updates to propagate a Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections.new/route.tsx (2)
188-378: Consider extracting shared port entry logic to reduce duplication.
TerraformWizardandAIPromptWizardhave nearly identical state management and UI for hostname, ports, and region selection. Consider extracting a shared hook or component to reduce duplication.Example: Shared hook
function useResourceConfig() { const [hostname, setHostname] = useState(""); const [ports, setPorts] = useState<PortEntry[]>([{ port: "5432", protocol: "TCP" }]); const [region, setRegion] = useState("us-east-1"); const addPort = () => setPorts([...ports, { port: "", protocol: "TCP" }]); const removePort = (index: number) => setPorts(ports.filter((_, i) => i !== index)); const updatePort = (index: number, field: keyof PortEntry, value: string) => setPorts(ports.map((p, i) => (i === index ? { ...p, [field]: value } : p))); const validPorts = ports.filter((p) => p.port !== ""); return { hostname, setHostname, ports, addPort, removePort, updatePort, validPorts, region, setRegion }; }Also applies to: 380-523
🤖 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.private-connections.new/route.tsx around lines 188 - 378, TerraformWizard and AIPromptWizard duplicate hostname/ports/region state and UI; extract that shared logic into a hook (e.g., useResourceConfig) or a shared PortList component to centralize state (hostname, ports, region), handlers (addPort, removePort, updatePort), and derived values (validPorts), then replace the local state/handlers in both TerraformWizard and AIPromptWizard to use the hook/component so the Terraform-specific script generation remains in TerraformWizard while UI/state logic is reused.
260-269: Terraform script hardcodessupported_regionswhich may drift from actual availability.The Terraform configuration hardcodes
supported_regions = ["us-east-1", "eu-central-1"](line 264) while the form'savailableRegionsis fetched dynamically from the API. This could cause confusion if the regions diverge.Consider passing
availableRegionstoTerraformWizardand using it in the generated script, or adding a comment noting this is a simplification.🤖 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.private-connections.new/route.tsx around lines 260 - 269, The Terraform snippet hardcodes supported_regions which may diverge from the UI's dynamic availableRegions; update the flow so TerraformWizard receives the component's availableRegions and uses it when generating the aws_vpc_endpoint_service block (replace the literal supported_regions value with the passed availableRegions list), ensuring the resource aws_vpc_endpoint_service "trigger_privatelink" reflects the runtime availableRegions used by the form; alternatively, if you prefer a simpler change, add a clear comment in the Terraform output noting supported_regions is a simplified static list and may not match availableRegions from the UI.apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections._index/route.tsx (2)
152-152: Consider addingDELETINGtoTERMINAL_STATUSESfor polling purposes.Currently
TERMINAL_STATUSESonly includes["ACTIVE", "ERROR"], so connections inDELETINGstatus will trigger continuous polling. If no further state updates are expected forDELETINGconnections until they disappear, adding it to terminal statuses would reduce unnecessary revalidations.🤖 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.private-connections._index/route.tsx at line 152, TERMINAL_STATUSES currently lists only "ACTIVE" and "ERROR", causing connections in "DELETING" to keep polling; update the TERMINAL_STATUSES constant (type PrivateLinkConnectionStatus) to include "DELETING" so polling logic treats deletions as terminal and stops revalidations for connections in that state (update the array assigned to TERMINAL_STATUSES).
175-175:hasPrivateNetworkingis hardcoded totrue.This variable is set to
trueunconditionally but is used to conditionally render UI. Since the route already gates access viahasPrivateConnectionsin the loader, this appears intentional but could be confusing. Consider removing this variable and the related conditional, or adding a comment explaining the intent.🤖 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.private-connections._index/route.tsx at line 175, The constant hasPrivateNetworking is hardcoded to true and duplicates the loader gate hasPrivateConnections; remove the unused hasPrivateNetworking variable and any conditional branches that check it (or, if the intent is to make a permanent true toggle, add a clear comment above hasPrivateNetworking explaining why it's hardcoded and reference hasPrivateConnections in the loader). Locate usages of hasPrivateNetworking in this route (search for hasPrivateNetworking in the file) and either delete the variable and simplify the conditional rendering to rely on hasPrivateConnections, or keep the variable but replace the hardcoded assignment with a documented constant explaining its purpose.
🤖 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/features.server.ts`:
- Around line 17-22: Replace direct process.env access in hasPrivateConnections
with the exported env object from env.server.ts: import and use
env.PRIVATE_CONNECTIONS_ENABLED in hasPrivateConnections (the function name is
hasPrivateConnections) instead of process.env.PRIVATE_CONNECTIONS_ENABLED, and
ensure the PRIVATE_CONNECTIONS_ENABLED key is added to the env schema/export in
env.server.ts so the property exists and types are correct; keep the existing
isManagedCloud(host) call and return logic unchanged.
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.settings.private-connections._index/route.tsx:
- Around line 98-105: The call site uses tryCatch(deletePrivateLink(...))
expecting exceptions, but deletePrivateLink in platform.v3.server.ts currently
returns void and swallows errors; either change deletePrivateLink to throw on
failure (propagate the caught error) so tryCatch can catch it, or change its
signature to return a success/failure result (e.g., boolean or { ok, error })
and update this route to check that result instead of relying on tryCatch;
specifically update deletePrivateLink to rethrow or return a result, and adjust
the code around tryCatch/redirectWithErrorMessage/v3PrivateConnectionsPath in
this route to handle the new return or thrown error accordingly.
- Around line 59-62: Replace the console.error call in the tryCatch block that
calls getPrivateLinks with the project's structured logger: import the project's
logger (e.g., logger) and call logger.error with the error as structured data
and a clear message (for example logger.error({ error }, "Error loading private
link connections")). Update the top-of-file imports to include the logger and
remove the console.error usage so error reporting uses the project's logging
facility.
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.settings.private-connections.new/route.tsx:
- Line 72: Replace the direct process.env access for
PRIVATE_CONNECTIONS_AWS_ACCOUNT_IDS with the env export from app/env.server.ts:
read env.PRIVATE_CONNECTIONS_AWS_ACCOUNT_IDS, split by "," and filter(Boolean)
to populate awsAccountIds (the variable currently declared as awsAccountIds),
and remove any use of process.env.PRIVATE_CONNECTIONS_AWS_ACCOUNT_IDS; also add
PRIVATE_CONNECTIONS_AWS_ACCOUNT_IDS to the env schema in env.server.ts so the
type and runtime validation include this variable.
---
Nitpick comments:
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.settings.private-connections._index/route.tsx:
- Line 152: TERMINAL_STATUSES currently lists only "ACTIVE" and "ERROR", causing
connections in "DELETING" to keep polling; update the TERMINAL_STATUSES constant
(type PrivateLinkConnectionStatus) to include "DELETING" so polling logic treats
deletions as terminal and stops revalidations for connections in that state
(update the array assigned to TERMINAL_STATUSES).
- Line 175: The constant hasPrivateNetworking is hardcoded to true and
duplicates the loader gate hasPrivateConnections; remove the unused
hasPrivateNetworking variable and any conditional branches that check it (or, if
the intent is to make a permanent true toggle, add a clear comment above
hasPrivateNetworking explaining why it's hardcoded and reference
hasPrivateConnections in the loader). Locate usages of hasPrivateNetworking in
this route (search for hasPrivateNetworking in the file) and either delete the
variable and simplify the conditional rendering to rely on
hasPrivateConnections, or keep the variable but replace the hardcoded assignment
with a documented constant explaining its purpose.
In
`@apps/webapp/app/routes/_app.orgs`.$organizationSlug.settings.private-connections.new/route.tsx:
- Around line 188-378: TerraformWizard and AIPromptWizard duplicate
hostname/ports/region state and UI; extract that shared logic into a hook (e.g.,
useResourceConfig) or a shared PortList component to centralize state (hostname,
ports, region), handlers (addPort, removePort, updatePort), and derived values
(validPorts), then replace the local state/handlers in both TerraformWizard and
AIPromptWizard to use the hook/component so the Terraform-specific script
generation remains in TerraformWizard while UI/state logic is reused.
- Around line 260-269: The Terraform snippet hardcodes supported_regions which
may diverge from the UI's dynamic availableRegions; update the flow so
TerraformWizard receives the component's availableRegions and uses it when
generating the aws_vpc_endpoint_service block (replace the literal
supported_regions value with the passed availableRegions list), ensuring the
resource aws_vpc_endpoint_service "trigger_privatelink" reflects the runtime
availableRegions used by the form; alternatively, if you prefer a simpler
change, add a clear comment in the Terraform output noting supported_regions is
a simplified static list and may not match availableRegions from the UI.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d2e8fb2a-352f-4cd1-8550-7beb778499c9
📒 Files selected for processing (16)
.changeset/private-networking-dequeue.md.server-changes/private-networking.mdapps/supervisor/src/index.tsapps/supervisor/src/workloadManager/kubernetes.tsapps/supervisor/src/workloadManager/types.tsapps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/features.server.tsapps/webapp/app/hooks/useFeatures.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections._index/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections.new/route.tsxapps/webapp/app/services/platform.v3.server.tsapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/v3/runEngine.server.tsinternal-packages/run-engine/src/engine/billingCache.tsinternal-packages/run-engine/src/engine/systems/dequeueSystem.tspackages/core/src/v3/schemas/runEngine.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). (28)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 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 (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
🧰 Additional context used
📓 Path-based instructions (16)
**/*.{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}: For apps and internal packages (apps/*,internal-packages/*), usepnpm run typecheck --filter <package>for verification, never usebuildas it proves almost nothing about correctness
Use testcontainers helpers (redisTest,postgresTest,containerTestfrom@internal/testcontainers) for integration tests with Redis and PostgreSQL instead of mocking
When writing Trigger.dev tasks, always import from@trigger.dev/sdk- never use@trigger.dev/sdk/v3or deprecatedclient.defineJob
Files:
apps/supervisor/src/index.tsapps/supervisor/src/workloadManager/types.tspackages/core/src/v3/schemas/runEngine.tsapps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/v3/runEngine.server.tsapps/webapp/app/hooks/useFeatures.tsapps/webapp/app/utils/pathBuilder.tsapps/supervisor/src/workloadManager/kubernetes.tsapps/webapp/app/features.server.tsinternal-packages/run-engine/src/engine/billingCache.tsinternal-packages/run-engine/src/engine/systems/dequeueSystem.tsapps/webapp/app/services/platform.v3.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections.new/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections._index/route.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
**/*.{ts,tsx,js,jsx}: Use pnpm for package management in this monorepo (version 10.23.0) with Turborepo for orchestration - run commands from root withpnpm run
Add crumbs as you write code for debug tracing using//@Crumbscomments or `// `#region` `@crumbsblocks - they stay on the branch throughout development and are stripped viaagentcrumbs stripbefore merge
Files:
apps/supervisor/src/index.tsapps/supervisor/src/workloadManager/types.tspackages/core/src/v3/schemas/runEngine.tsapps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/v3/runEngine.server.tsapps/webapp/app/hooks/useFeatures.tsapps/webapp/app/utils/pathBuilder.tsapps/supervisor/src/workloadManager/kubernetes.tsapps/webapp/app/features.server.tsinternal-packages/run-engine/src/engine/billingCache.tsinternal-packages/run-engine/src/engine/systems/dequeueSystem.tsapps/webapp/app/services/platform.v3.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections.new/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections._index/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/supervisor/src/index.tsapps/supervisor/src/workloadManager/types.tspackages/core/src/v3/schemas/runEngine.tsapps/webapp/app/v3/runEngine.server.tsapps/webapp/app/hooks/useFeatures.tsapps/webapp/app/utils/pathBuilder.tsapps/supervisor/src/workloadManager/kubernetes.tsapps/webapp/app/features.server.tsinternal-packages/run-engine/src/engine/billingCache.tsinternal-packages/run-engine/src/engine/systems/dequeueSystem.tsapps/webapp/app/services/platform.v3.server.ts
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/supervisor/src/index.tsapps/supervisor/src/workloadManager/types.tspackages/core/src/v3/schemas/runEngine.tsapps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/v3/runEngine.server.tsapps/webapp/app/hooks/useFeatures.tsapps/webapp/app/utils/pathBuilder.tsapps/supervisor/src/workloadManager/kubernetes.tsapps/webapp/app/features.server.tsinternal-packages/run-engine/src/engine/billingCache.tsinternal-packages/run-engine/src/engine/systems/dequeueSystem.tsapps/webapp/app/services/platform.v3.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections.new/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections._index/route.tsx
apps/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
When modifying only server components (
apps/webapp/,apps/supervisor/, etc.) with no package changes, add a.server-changes/file instead of a changeset
Files:
apps/supervisor/src/index.tsapps/supervisor/src/workloadManager/types.tsapps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/v3/runEngine.server.tsapps/webapp/app/hooks/useFeatures.tsapps/webapp/app/utils/pathBuilder.tsapps/supervisor/src/workloadManager/kubernetes.tsapps/webapp/app/features.server.tsapps/webapp/app/services/platform.v3.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections.new/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections._index/route.tsx
apps/supervisor/src/workloadManager/**/*.{js,ts}
📄 CodeRabbit inference engine (apps/supervisor/CLAUDE.md)
Container orchestration abstraction (Docker or Kubernetes) should be implemented in
src/workloadManager/
Files:
apps/supervisor/src/workloadManager/types.tsapps/supervisor/src/workloadManager/kubernetes.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:
packages/core/src/v3/schemas/runEngine.tsapps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/v3/runEngine.server.tsapps/webapp/app/hooks/useFeatures.tsapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/features.server.tsapps/webapp/app/services/platform.v3.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections.new/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections._index/route.tsx
packages/core/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/core/CLAUDE.md)
Never import the root package (
@trigger.dev/core). Always use subpath imports such as@trigger.dev/core/v3,@trigger.dev/core/v3/utils,@trigger.dev/core/logger, or@trigger.dev/core/schemas
Files:
packages/core/src/v3/schemas/runEngine.ts
packages/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/**/*.{ts,tsx,js,jsx}: For public packages (packages/*), usepnpm run build --filter <package>for verification
Add a changeset viapnpm run changeset:addwhen modifying any public package (packages/*orintegrations/*) - default to patch for bug fixes and minor changes
Files:
packages/core/src/v3/schemas/runEngine.ts
packages/core/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
In the core package (
@trigger.dev/core), import subpaths only - never import from root
Files:
packages/core/src/v3/schemas/runEngine.ts
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/v3/runEngine.server.tsapps/webapp/app/hooks/useFeatures.tsapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/features.server.tsapps/webapp/app/services/platform.v3.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections.new/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections._index/route.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/v3/runEngine.server.tsapps/webapp/app/hooks/useFeatures.tsapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/features.server.tsapps/webapp/app/services/platform.v3.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections.new/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections._index/route.tsx
apps/webapp/app/**/*.{ts,tsx,server.ts}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Access environment variables via
envexport fromapp/env.server.ts. Never useprocess.envdirectly
Files:
apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/v3/runEngine.server.tsapps/webapp/app/hooks/useFeatures.tsapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/features.server.tsapps/webapp/app/services/platform.v3.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections.new/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections._index/route.tsx
apps/webapp/app/v3/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
In the webapp v3 directory, only modify V2 code paths when encountering V1/V2 branching in services - all new work uses Run Engine 2.0 (
@internal/run-engine) and redis-worker, not legacy V1 engine code
Files:
apps/webapp/app/v3/runEngine.server.ts
internal-packages/run-engine/src/engine/systems/**/*.ts
📄 CodeRabbit inference engine (internal-packages/run-engine/CLAUDE.md)
Integrate OpenTelemetry tracer and meter instrumentation in RunEngine systems for observability
Files:
internal-packages/run-engine/src/engine/systems/dequeueSystem.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/platform.v3.server.ts
🧠 Learnings (22)
📚 Learning: 2026-03-02T12:42:47.652Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: apps/supervisor/CLAUDE.md:0-0
Timestamp: 2026-03-02T12:42:47.652Z
Learning: Applies to apps/supervisor/src/workloadManager/**/*.{js,ts} : Container orchestration abstraction (Docker or Kubernetes) should be implemented in `src/workloadManager/`
Applied to files:
apps/supervisor/src/index.tsapps/supervisor/src/workloadManager/types.tsapps/supervisor/src/workloadManager/kubernetes.ts
📚 Learning: 2026-03-02T12:42:47.652Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: apps/supervisor/CLAUDE.md:0-0
Timestamp: 2026-03-02T12:42:47.652Z
Learning: Applies to apps/supervisor/src/workloadServer/**/*.{js,ts} : HTTP server for workload communication (heartbeats, snapshots) should be implemented in `src/workloadServer/`
Applied to files:
apps/supervisor/src/index.tsapps/supervisor/src/workloadManager/types.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/supervisor/src/index.tsapps/supervisor/src/workloadManager/types.tspackages/core/src/v3/schemas/runEngine.tsapps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/v3/runEngine.server.tsapps/webapp/app/hooks/useFeatures.tsapps/webapp/app/utils/pathBuilder.tsapps/supervisor/src/workloadManager/kubernetes.tsapps/webapp/app/features.server.tsinternal-packages/run-engine/src/engine/billingCache.tsinternal-packages/run-engine/src/engine/systems/dequeueSystem.tsapps/webapp/app/services/platform.v3.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections.new/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections._index/route.tsx
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.
Applied to files:
apps/supervisor/src/index.tsapps/supervisor/src/workloadManager/types.tspackages/core/src/v3/schemas/runEngine.tsapps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/v3/runEngine.server.tsapps/webapp/app/hooks/useFeatures.tsapps/webapp/app/utils/pathBuilder.tsapps/supervisor/src/workloadManager/kubernetes.tsapps/webapp/app/features.server.tsinternal-packages/run-engine/src/engine/billingCache.tsinternal-packages/run-engine/src/engine/systems/dequeueSystem.tsapps/webapp/app/services/platform.v3.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections.new/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections._index/route.tsx
📚 Learning: 2026-03-22T13:45:36.346Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/navigation/SideMenu.tsx:460-489
Timestamp: 2026-03-22T13:45:36.346Z
Learning: In triggerdotdev/trigger.dev, sidebar navigation items (SideMenu.tsx) are intentionally NOT gated behind feature-flag or permission checks at the nav level. Authorization is enforced at the route/loader level instead. Hiding nav items based on access checks is considered confusing UX. This applies to items like "AI Metrics" (v3BuiltInDashboardPath) and other dashboard links — they are always rendered in the sidebar regardless of hasQueryAccess or similar flags.
Applied to files:
apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsx
📚 Learning: 2026-02-03T18:27:49.039Z
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:49.039Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx, the menu buttons (like the Edit button with PencilSquareIcon) intentionally have no text labels - only icons are shown in the TableCellMenu. This is a deliberate UI design pattern for compact icon-only menu items.
Applied to files:
apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.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/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections.new/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections._index/route.tsx
📚 Learning: 2026-03-23T06:24:25.029Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: apps/webapp/CLAUDE.md:0-0
Timestamp: 2026-03-23T06:24:25.029Z
Learning: Applies to apps/webapp/app/v3/services/**/*.server.ts : Only modify V2 code paths when editing services that branch on `RunEngineVersion` to support both V1 and V2 (e.g., `cancelTaskRun.server.ts`, `batchTriggerV3.server.ts`)
Applied to files:
apps/webapp/app/v3/runEngine.server.tsapps/webapp/app/services/platform.v3.server.ts
📚 Learning: 2026-03-23T06:24:14.566Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-23T06:24:14.566Z
Learning: Applies to apps/webapp/app/v3/**/*.{ts,tsx} : In the webapp v3 directory, only modify V2 code paths when encountering V1/V2 branching in services - all new work uses Run Engine 2.0 (`internal/run-engine`) and redis-worker, not legacy V1 engine code
Applied to files:
apps/webapp/app/v3/runEngine.server.tsapps/webapp/app/utils/pathBuilder.tsapps/webapp/app/services/platform.v3.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/services/**/*.server.{ts,tsx} : Organize services in the webapp following the pattern `app/v3/services/*/*.server.ts`
Applied to files:
apps/webapp/app/utils/pathBuilder.tsapps/webapp/app/services/platform.v3.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/features.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections._index/route.tsx
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/**/*.{ts,tsx} : Access all environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env` in the Trigger.dev webapp
Applied to files:
apps/webapp/app/features.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 : Configure OpenTelemetry instrumentations and exporters in trigger.config.ts for enhanced logging
Applied to files:
apps/webapp/app/features.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 : Use build extensions in trigger.config.ts (additionalFiles, additionalPackages, aptGet, prismaExtension, etc.) to customize the build
Applied to files:
apps/webapp/app/features.server.ts
📚 Learning: 2026-03-02T12:43:43.173Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: packages/redis-worker/CLAUDE.md:0-0
Timestamp: 2026-03-02T12:43:43.173Z
Learning: Applies to packages/redis-worker/**/redis-worker/src/queue.ts : Job queue abstraction should be Redis-backed in src/queue.ts
Applied to files:
internal-packages/run-engine/src/engine/systems/dequeueSystem.ts
📚 Learning: 2026-03-03T13:08:03.862Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3166
File: packages/redis-worker/src/fair-queue/index.ts:1114-1121
Timestamp: 2026-03-03T13:08:03.862Z
Learning: In packages/redis-worker/src/fair-queue/index.ts, it's acceptable for the worker queue depth cap check to allow overshooting by up to batchClaimSize messages per iteration, as the next iteration will recheck and prevent sustained growth beyond the limit.
Applied to files:
internal-packages/run-engine/src/engine/systems/dequeueSystem.ts
📚 Learning: 2026-03-14T10:24:37.515Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3219
File: internal-packages/run-engine/src/run-queue/index.ts:774-777
Timestamp: 2026-03-14T10:24:37.515Z
Learning: In `internal-packages/run-engine/src/run-queue/index.ts`, checking `message.concurrencyKey` to branch CK vs non-CK logic is safe and intentional. The `concurrencyKey` field and the `:ck:` suffix in `message.queue` are always set atomically during the same enqueue call (`queueKey = this.keys.queueKey(env, message.queue, concurrencyKey)`). In `enqueueMessage`, `concurrencyKey` is the source of truth that creates the `:ck:` queue name (checking the queue name would be circular). In `#callAcknowledgeMessage`, `#callNackMessage`, and `#callMoveToDeadLetterQueue`, the `OutputPayload` is a single serialized JSON blob containing both fields — a message with `:ck:` in the queue name but a missing `concurrencyKey` cannot exist. Do not suggest replacing `message.concurrencyKey` checks with queue-name-derived predicates.
Applied to files:
internal-packages/run-engine/src/engine/systems/dequeueSystem.ts
📚 Learning: 2026-02-19T18:09:23.944Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3095
File: apps/webapp/app/components/navigation/DashboardDialogs.tsx:47-47
Timestamp: 2026-02-19T18:09:23.944Z
Learning: In the triggerdotdev/trigger.dev codebase, the pattern `isPaying === false` is used consistently to explicitly check for free plan users. This is a project convention that distinguishes between `isPaying === true` (paying), `isPaying === false` (free), and `isPaying === undefined` (no subscription data). Do not suggest changing this to negation pattern.
```
<!-- <review_comment_addressed>
Applied to files:
internal-packages/run-engine/src/engine/systems/dequeueSystem.ts
📚 Learning: 2026-02-04T16:34:48.876Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/routes/vercel.connect.tsx:13-27
Timestamp: 2026-02-04T16:34:48.876Z
Learning: In apps/webapp/app/routes/vercel.connect.tsx, configurationId may be absent for "dashboard" flows but must be present for "marketplace" flows. Enforce this with a Zod superRefine and pass installationId to repository methods only when configurationId is defined (omit the field otherwise).
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections.new/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections._index/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/_app.orgs.$organizationSlug.settings.private-connections.new/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/_app.orgs.$organizationSlug.settings.private-connections.new/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections._index/route.tsx
📚 Learning: 2026-03-23T06:24:25.029Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: apps/webapp/CLAUDE.md:0-0
Timestamp: 2026-03-23T06:24:25.029Z
Learning: Applies to apps/webapp/app/routes/**/*.ts : Use Remix flat-file route convention with dot-separated segments where `api.v1.tasks.$taskId.trigger.ts` maps to `/api/v1/tasks/:taskId/trigger`
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.private-connections._index/route.tsx
🔇 Additional comments (14)
internal-packages/run-engine/src/engine/billingCache.ts (1)
20-24: LGTM!The
BillingPlantype extension with optionalhasPrivateLinkis appropriate. The caching logic correctly propagates the field from the upstream billing callback, and the default free plan correctly omits the field (falsy by default).Also applies to: 67-81
apps/webapp/app/utils/pathBuilder.ts (1)
641-647: LGTM!The new path builder functions follow the established pattern of other organization settings paths in this file. Naming and implementation are consistent with existing conventions.
apps/webapp/app/hooks/useFeatures.ts (1)
5-8: LGTM!The default fallback now correctly includes both
TriggerFeaturesfields (isManagedCloudandhasPrivateConnections), ensuring the hook always returns a complete object matching the type definition. DefaultinghasPrivateConnectionstofalseis the appropriate safe default.apps/supervisor/src/index.ts (1)
270-270: LGTM!The
hasPrivateLinkflag is correctly extracted frommessage.organization.hasPrivateLinkand passed to the workload manager. The optional nature of the field in both the message schema andWorkloadManagerCreateOptionsensures type compatibility.apps/supervisor/src/workloadManager/types.ts (1)
38-39: LGTM!The optional
hasPrivateLinkfield is appropriately added to the workload creation options. The comment provides helpful context for the field's purpose.packages/core/src/v3/schemas/runEngine.ts (1)
267-270: LGTM!The
hasPrivateLinkfield is correctly added as an optional boolean to theorganizationobject in theDequeuedMessageschema. This is the appropriate place to define the field since it propagates through the dequeue pipeline to the supervisor. A changeset for this change already exists in the repository.apps/webapp/app/services/platform.v3.server.ts (1)
646-724: Implementation follows established patterns.The four new private link functions (
getPrivateLinks,createPrivateLink,deletePrivateLink,getPrivateLinkRegions) correctly follow the error handling pattern used throughout this file:
- Early return when
clientis unavailabletryCatchwrapper for async client calls- Logging on both thrown errors and
!result.successresponses- Silent failure (return
undefined/void) rather than throwingThe functions are already in use across the codebase (in private-connections route files) without issues, confirming that the BillingClient type from
@trigger.dev/platformincludes these methods.apps/supervisor/src/workloadManager/kubernetes.ts (1)
338-353: Implementation is correct for CiliumNetworkPolicy matching.The conditional
privatelinklabel addition withorgIdas the value enables network policy selectors to match pods by organization. The truthy check onopts.hasPrivateLinkcorrectly handles bothundefinedandfalsecases. Organization IDs use CUID format, which generates alphanumeric strings well under the 63-character Kubernetes label limit and fully comply with the K8s label value pattern..changeset/private-networking-dequeue.md (1)
1-5: LGTM!Changeset correctly documents a patch-level change for the optional
hasPrivateLinkfield addition.apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsx (1)
108-116: LGTM!The Private Connections menu item follows the existing pattern used for managed cloud features (e.g., Usage, Billing), with proper feature flag gating, icon styling, and data-action attribute.
.server-changes/private-networking.md (1)
1-6: LGTM!Server-changes documentation correctly describes the webapp feature addition with appropriate metadata.
internal-packages/run-engine/src/engine/systems/dequeueSystem.ts (2)
498-518: LGTM!The
hasPrivateLinkfield is correctly typed asboolean | undefinedand properly propagated from the billing cache result. When billing fails, it gracefully falls back toundefined, which downstream consumers can handle appropriately.
585-588: LGTM!The
hasPrivateLinkfield is correctly added to the organization object in the dequeued message, matching the schema extension inpackages/core/src/v3/schemas/runEngine.ts.apps/webapp/app/v3/runEngine.server.ts (1)
163-168: No changes needed. The code correctly accessesplan.v3Subscription.plan.limitsafter confirmingplan.v3Subscription.planexists at line 155. When the plan exists, thelimitsproperty is guaranteed to be present by design—the billing client uses fail-fast behavior to throw rather than return undefined limits. This matches the established pattern inplatform.v3.server.ts(lines 283–295).> Likely an incorrect or invalid review comment.
| function hasPrivateConnections(host: string): boolean { | ||
| if (process.env.PRIVATE_CONNECTIONS_ENABLED === "1") { | ||
| return isManagedCloud(host); | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Use env export from env.server.ts instead of process.env.
The coding guidelines require accessing environment variables through the env export of env.server.ts. Direct process.env access should be avoided.
Proposed fix
+import { env } from "~/env.server";
+
function hasPrivateConnections(host: string): boolean {
- if (process.env.PRIVATE_CONNECTIONS_ENABLED === "1") {
+ if (env.PRIVATE_CONNECTIONS_ENABLED === "1") {
return isManagedCloud(host);
}
return false;
}Note: You'll also need to add PRIVATE_CONNECTIONS_ENABLED to the env schema in env.server.ts.
As per coding guidelines: "Access environment variables via env export from app/env.server.ts. Never use process.env directly"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/webapp/app/features.server.ts` around lines 17 - 22, Replace direct
process.env access in hasPrivateConnections with the exported env object from
env.server.ts: import and use env.PRIVATE_CONNECTIONS_ENABLED in
hasPrivateConnections (the function name is hasPrivateConnections) instead of
process.env.PRIVATE_CONNECTIONS_ENABLED, and ensure the
PRIVATE_CONNECTIONS_ENABLED key is added to the env schema/export in
env.server.ts so the property exists and types are correct; keep the existing
isManagedCloud(host) call and return logic unchanged.
| const [error, connections] = await tryCatch(getPrivateLinks(organization.id)); | ||
| if (error) { | ||
| console.error("Error loading private link connections", error); | ||
| } |
There was a problem hiding this comment.
Use the project's logger instead of console.error.
The codebase uses a structured logger. Replace console.error with the appropriate logger import.
Proposed fix
+import { logger } from "~/services/logger.server";
const [error, connections] = await tryCatch(getPrivateLinks(organization.id));
if (error) {
- console.error("Error loading private link connections", error);
+ logger.error("Error loading private link connections", { error, organizationId: organization.id });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [error, connections] = await tryCatch(getPrivateLinks(organization.id)); | |
| if (error) { | |
| console.error("Error loading private link connections", error); | |
| } | |
| import { logger } from "~/services/logger.server"; | |
| // ... other code ... | |
| const [error, connections] = await tryCatch(getPrivateLinks(organization.id)); | |
| if (error) { | |
| logger.error("Error loading private link connections", { error, organizationId: organization.id }); | |
| } |
🤖 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.private-connections._index/route.tsx
around lines 59 - 62, Replace the console.error call in the tryCatch block that
calls getPrivateLinks with the project's structured logger: import the project's
logger (e.g., logger) and call logger.error with the error as structured data
and a clear message (for example logger.error({ error }, "Error loading private
link connections")). Update the top-of-file imports to include the logger and
remove the console.error usage so error reporting uses the project's logging
facility.
| const [error] = await tryCatch(deletePrivateLink(organization.id, connectionId)); | ||
| if (error) { | ||
| return redirectWithErrorMessage( | ||
| v3PrivateConnectionsPath({ slug: organizationSlug }), | ||
| request, | ||
| `Failed to delete connection: ${error.message}` | ||
| ); | ||
| } |
There was a problem hiding this comment.
deletePrivateLink returns void and doesn't throw - error handling won't work as expected.
Looking at deletePrivateLink in platform.v3.server.ts, the function returns void and logs errors internally without throwing. The tryCatch wrapper here will never catch an error since the function handles errors silently.
Consider having deletePrivateLink either throw on failure or return a success/failure result that can be checked.
Option A: Make deletePrivateLink throw on error
In platform.v3.server.ts:
export async function deletePrivateLink(
organizationId: string,
connectionId: string
): Promise<void> {
- if (!client) return;
+ if (!client) throw new Error("Platform client not configured");
const [error, result] = await tryCatch(client.deletePrivateLink(organizationId, connectionId));
if (error) {
logger.error("Error deleting private link", { organizationId, connectionId, error });
- return;
+ throw error;
}
if (!result.success) {
logger.error("Error deleting private link - no success", { organizationId, connectionId, error: result.error });
- return;
+ throw new Error(result.error ?? "Failed to delete private link");
}
}🤖 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.private-connections._index/route.tsx
around lines 98 - 105, The call site uses tryCatch(deletePrivateLink(...))
expecting exceptions, but deletePrivateLink in platform.v3.server.ts currently
returns void and swallows errors; either change deletePrivateLink to throw on
failure (propagate the caught error) so tryCatch can catch it, or change its
signature to return a success/failure result (e.g., boolean or { ok, error })
and update this route to check that result instead of relying on tryCatch;
specifically update deletePrivateLink to rethrow or return a result, and adjust
the code around tryCatch/redirectWithErrorMessage/v3PrivateConnectionsPath in
this route to handle the new return or thrown error accordingly.
|
|
||
| const [error, regions] = await tryCatch(getPrivateLinkRegions(organization.id)); | ||
|
|
||
| const awsAccountIds = process.env.PRIVATE_CONNECTIONS_AWS_ACCOUNT_IDS?.split(",").filter(Boolean) ?? []; |
There was a problem hiding this comment.
Use env export from env.server.ts instead of process.env.
Direct process.env access should be avoided per coding guidelines.
Proposed fix
+import { env } from "~/env.server";
+
- const awsAccountIds = process.env.PRIVATE_CONNECTIONS_AWS_ACCOUNT_IDS?.split(",").filter(Boolean) ?? [];
+ const awsAccountIds = env.PRIVATE_CONNECTIONS_AWS_ACCOUNT_IDS?.split(",").filter(Boolean) ?? [];Note: Add PRIVATE_CONNECTIONS_AWS_ACCOUNT_IDS to the env schema in env.server.ts.
As per coding guidelines: "Access environment variables via env export from app/env.server.ts. Never use process.env directly"
🤖 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.private-connections.new/route.tsx
at line 72, Replace the direct process.env access for
PRIVATE_CONNECTIONS_AWS_ACCOUNT_IDS with the env export from app/env.server.ts:
read env.PRIVATE_CONNECTIONS_AWS_ACCOUNT_IDS, split by "," and filter(Boolean)
to populate awsAccountIds (the variable currently declared as awsAccountIds),
and remove any use of process.env.PRIVATE_CONNECTIONS_AWS_ACCOUNT_IDS; also add
PRIVATE_CONNECTIONS_AWS_ACCOUNT_IDS to the env schema in env.server.ts so the
type and runtime validation include this variable.
1 participant
No description provided.