feat: optional v4 deploy registry overrides #2370
Conversation
WalkthroughThis change introduces a new versioned set of deployment registry environment variables ("v4") into the environment schema, with fallback logic to existing "v3" variables. A new Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/webapp/app/env.server.ts(1 hunks)apps/webapp/app/v3/getDeploymentImageRef.server.ts(3 hunks)apps/webapp/app/v3/registryConfig.server.ts(1 hunks)apps/webapp/app/v3/services/finalizeDeploymentV2.server.ts(8 hunks)apps/webapp/app/v3/services/initializeDeployment.server.ts(2 hunks)apps/webapp/test/getDeploymentImageRef.test.ts(5 hunks)apps/webapp/test/registryConfig.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
apps/webapp/app/v3/services/initializeDeployment.server.tsapps/webapp/app/v3/registryConfig.server.tsapps/webapp/test/registryConfig.test.tsapps/webapp/test/getDeploymentImageRef.test.tsapps/webapp/app/env.server.tsapps/webapp/app/v3/getDeploymentImageRef.server.tsapps/webapp/app/v3/services/finalizeDeploymentV2.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
apps/webapp/app/v3/services/initializeDeployment.server.tsapps/webapp/app/v3/registryConfig.server.tsapps/webapp/test/registryConfig.test.tsapps/webapp/test/getDeploymentImageRef.test.tsapps/webapp/app/env.server.tsapps/webapp/app/v3/getDeploymentImageRef.server.tsapps/webapp/app/v3/services/finalizeDeploymentV2.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: In the webapp, all environment variables must be accessed through theenvexport ofenv.server.ts, instead of directly accessingprocess.env.
When importing from@trigger.dev/corein the webapp, never import from the root@trigger.dev/corepath; always use one of the subpath exports as defined in the package's package.json.
Files:
apps/webapp/app/v3/services/initializeDeployment.server.tsapps/webapp/app/v3/registryConfig.server.tsapps/webapp/test/registryConfig.test.tsapps/webapp/test/getDeploymentImageRef.test.tsapps/webapp/app/env.server.tsapps/webapp/app/v3/getDeploymentImageRef.server.tsapps/webapp/app/v3/services/finalizeDeploymentV2.server.ts
**/*.test.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Our tests are all vitest
Files:
apps/webapp/test/registryConfig.test.tsapps/webapp/test/getDeploymentImageRef.test.ts
🧠 Learnings (20)
📚 Learning: applies to trigger.config.ts : build extensions such as `additionalfiles`, `additionalpackages`, `ap...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to trigger.config.ts : Build extensions such as `additionalFiles`, `additionalPackages`, `aptGet`, `emitDecoratorMetadata`, `prismaExtension`, `syncEnvVars`, `puppeteer`, `ffmpeg`, and `esbuildPlugin` must be configured in `trigger.config.ts` as shown.
Applied to files:
apps/webapp/app/v3/services/initializeDeployment.server.tsapps/webapp/app/v3/registryConfig.server.tsapps/webapp/app/env.server.tsapps/webapp/app/v3/services/finalizeDeploymentV2.server.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when using retry, queue, machine, or maxduration option...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When using retry, queue, machine, or maxDuration options, configure them as shown in the examples for Trigger.dev tasks.
Applied to files:
apps/webapp/app/v3/services/initializeDeployment.server.ts
📚 Learning: applies to apps/webapp/app/services/**/*.server.ts : for testable services, separate service logic a...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-07-18T17:49:47.180Z
Learning: Applies to apps/webapp/app/services/**/*.server.ts : For testable services, separate service logic and configuration, as exemplified by `realtimeClient.server.ts` (service) and `realtimeClientGlobal.server.ts` (configuration).
Applied to files:
apps/webapp/app/v3/services/initializeDeployment.server.tsapps/webapp/app/v3/registryConfig.server.tsapps/webapp/test/registryConfig.test.tsapps/webapp/app/v3/services/finalizeDeploymentV2.server.ts
📚 Learning: applies to trigger.config.ts : global lifecycle hooks, telemetry, runtime, machine settings, log lev...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to trigger.config.ts : Global lifecycle hooks, telemetry, runtime, machine settings, log level, max duration, and build configuration must be set in `trigger.config.ts` as shown.
Applied to files:
apps/webapp/app/v3/services/initializeDeployment.server.ts
📚 Learning: applies to trigger.config.ts : the `trigger.config.ts` file must use `defineconfig` from `@trigger.d...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to trigger.config.ts : The `trigger.config.ts` file must use `defineConfig` from `trigger.dev/sdk/v3` and follow the configuration structure shown.
Applied to files:
apps/webapp/app/v3/registryConfig.server.tsapps/webapp/app/v3/services/finalizeDeploymentV2.server.ts
📚 Learning: applies to apps/webapp/app/**/*.test.{ts,tsx} : tests in the webapp should only import classes and f...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-07-18T17:49:47.180Z
Learning: Applies to apps/webapp/app/**/*.test.{ts,tsx} : Tests in the webapp should only import classes and functions from files matching `app/**/*.ts`, and those files should not use environment variables directly; all configuration should be passed as options.
Applied to files:
apps/webapp/test/registryConfig.test.tsapps/webapp/test/getDeploymentImageRef.test.tsapps/webapp/app/env.server.tsapps/webapp/app/v3/services/finalizeDeploymentV2.server.ts
📚 Learning: applies to apps/webapp/app/**/*.test.{ts,tsx} : test files in the webapp should not import `env.serv...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-07-18T17:49:47.180Z
Learning: Applies to apps/webapp/app/**/*.test.{ts,tsx} : Test files in the webapp should not import `env.server.ts`, either directly or indirectly. Tests should only import classes and functions from files matching `app/**/*.ts` of the webapp, and those files should not use environment variables directly; everything should be passed through as options instead.
Applied to files:
apps/webapp/test/registryConfig.test.tsapps/webapp/app/env.server.tsapps/webapp/app/v3/services/finalizeDeploymentV2.server.ts
📚 Learning: applies to **/*.test.{ts,tsx} : our tests are all vitest...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-18T17:49:24.468Z
Learning: Applies to **/*.test.{ts,tsx} : Our tests are all vitest
Applied to files:
apps/webapp/test/registryConfig.test.ts
📚 Learning: applies to apps/webapp/**/*.{ts,tsx} : in the webapp, all environment variables must be accessed thr...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-07-18T17:49:47.180Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : In the webapp, all environment variables must be accessed through the `env` export of `env.server.ts`, instead of directly accessing `process.env`.
Applied to files:
apps/webapp/test/registryConfig.test.tsapps/webapp/app/env.server.tsapps/webapp/app/v3/services/finalizeDeploymentV2.server.ts
📚 Learning: in the trigger.dev codebase, the supervisor component uses docker_enforce_machine_presets while the ...
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2150
File: apps/supervisor/src/workloadManager/docker.ts:115-115
Timestamp: 2025-06-04T16:02:22.957Z
Learning: In the Trigger.dev codebase, the supervisor component uses DOCKER_ENFORCE_MACHINE_PRESETS while the docker provider component uses ENFORCE_MACHINE_PRESETS. These are separate components with separate environment variable configurations for the same logical concept of enforcing machine presets.
Applied to files:
apps/webapp/app/env.server.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when implementing schema tasks, use `schematask` from `...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When implementing schema tasks, use `schemaTask` from `trigger.dev/sdk/v3` and validate payloads as shown.
Applied to files:
apps/webapp/app/env.server.tsapps/webapp/app/v3/services/finalizeDeploymentV2.server.ts
📚 Learning: in the docs.json configuration for the trigger.dev documentation (mintlify system), both "tags": ["v...
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2155
File: docs/docs.json:179-183
Timestamp: 2025-06-06T16:54:23.316Z
Learning: In the docs.json configuration for the Trigger.dev documentation (Mintlify system), both "tags": ["v4"] and "tag": "v4" properties can be used together and work correctly, even though this behavior is undocumented and may not work in local development environments.
Applied to files:
apps/webapp/app/env.server.ts
📚 Learning: applies to apps/webapp/app/v3/presenters/**/*.server.ts : favor the use of 'presenters' to move comp...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-07-18T17:49:47.180Z
Learning: Applies to apps/webapp/app/v3/presenters/**/*.server.ts : Favor the use of 'presenters' to move complex loader code into a class, as seen in `app/v3/presenters/**/*.server.ts`.
Applied to files:
apps/webapp/app/v3/services/finalizeDeploymentV2.server.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : you must use `@trigger.dev/sdk/v3` when writing trigger...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : You MUST use `trigger.dev/sdk/v3` when writing Trigger.dev tasks.
Applied to files:
apps/webapp/app/v3/services/finalizeDeploymentV2.server.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : always generate trigger.dev tasks using the `task` func...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : ALWAYS generate Trigger.dev tasks using the `task` function from `trigger.dev/sdk/v3` and export them as shown in the correct pattern.
Applied to files:
apps/webapp/app/v3/services/finalizeDeploymentV2.server.ts
📚 Learning: before generating any code for trigger.dev tasks, verify: (1) are you importing from `@trigger.dev/s...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Before generating any code for Trigger.dev tasks, verify: (1) Are you importing from `trigger.dev/sdk/v3`? (2) Have you exported every task? (3) Have you generated any deprecated code patterns?
Applied to files:
apps/webapp/app/v3/services/finalizeDeploymentV2.server.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : you must `export` every task, including subtasks, in tr...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : You MUST `export` every task, including subtasks, in Trigger.dev task files.
Applied to files:
apps/webapp/app/v3/services/finalizeDeploymentV2.server.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when implementing scheduled (cron) tasks, use `schedule...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When implementing scheduled (cron) tasks, use `schedules.task` from `trigger.dev/sdk/v3` and follow the shown patterns.
Applied to files:
apps/webapp/app/v3/services/finalizeDeploymentV2.server.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when triggering a task from backend code, use `tasks.tr...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When triggering a task from backend code, use `tasks.trigger`, `tasks.batchTrigger`, or `tasks.triggerAndPoll` as shown in the examples.
Applied to files:
apps/webapp/app/v3/services/finalizeDeploymentV2.server.ts
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : tasks must be exported, even subtasks in the same file....
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Tasks must be exported, even subtasks in the same file.
Applied to files:
apps/webapp/app/v3/services/finalizeDeploymentV2.server.ts
🧬 Code Graph Analysis (3)
apps/webapp/app/v3/registryConfig.server.ts (1)
apps/webapp/app/env.server.ts (1)
env(1063-1063)
apps/webapp/app/v3/getDeploymentImageRef.server.ts (1)
apps/webapp/app/v3/registryConfig.server.ts (1)
RegistryConfig(3-11)
apps/webapp/app/v3/services/finalizeDeploymentV2.server.ts (4)
apps/webapp/app/v3/registryConfig.server.ts (2)
getRegistryConfig(13-35)RegistryConfig(3-11)apps/coordinator/src/exec.ts (1)
tmpDir(216-218)packages/cli-v3/src/utilities/fileSystem.ts (2)
createTempDir(92-100)writeJSONFile(73-75)apps/webapp/app/v3/getDeploymentImageRef.server.ts (2)
isEcrRegistry(154-161)getEcrAuthToken(345-379)
⏰ 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 / webapp / 🧪 Unit Tests: Webapp (3, 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 (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- 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: Analyze (javascript-typescript)
🔇 Additional comments (9)
apps/webapp/app/v3/services/initializeDeployment.server.ts (3)
13-13: LGTM!The import of
getRegistryConfigis correctly added and follows the project's import conventions.
76-83: LGTM!The refactoring to use a consolidated
registryobject instead of individual registry parameters improves code organization and maintainability. The change aligns well with the centralized registry configuration approach.
73-74: Verify v4 registry override conditionsThe code currently treats every
payload.type === "MANAGED"as a v4 deployment and unconditionally calls:const isV4Deployment = payload.type === "MANAGED";
const registryConfig = getRegistryConfig(isV4Deployment);This may unintentionally switch existing “MANAGED” deployments to v4 registry settings, whereas the PR description refers to “optional v4 deploy registry overrides.”
Please confirm whether all managed deployments should now use v4 settings or if this needs to be gated by an explicit flag/version check. For example, you might:
- Introduce a feature flag or environment variable (e.g.
USE_V4_DEPLOY_REGISTRY)- Add a field to the request payload (e.g.
useV4Registry: boolean)- Inspect the project’s current engine/version before defaulting to v4
Files to review:
- apps/webapp/app/v3/services/initializeDeployment.server.ts (lines ~73–74)
- apps/webapp/app/v3/registryConfig.server.ts
apps/webapp/test/getDeploymentImageRef.test.ts (1)
59-67: LGTM!The test updates correctly reflect the new API structure using a consolidated
registryobject. All test cases consistently use the properRegistryConfigstructure with appropriate properties, maintaining comprehensive test coverage for various scenarios.Also applies to: 81-89, 102-110, 126-134, 149-157
apps/webapp/app/v3/registryConfig.server.ts (2)
3-11: LGTM!The
RegistryConfigtype definition is well-structured with appropriate required and optional fields that cover all necessary registry configuration parameters including ECR-specific settings.
13-35: LGTM!The
getRegistryConfigfunction implementation correctly separates v4 and v3 registry configurations and follows the coding guideline of using theenvexport fromenv.server.ts. The logic is clear and maintains the separation between deployment types.The function properly delegates to the environment schema for handling v4-to-v3 fallback logic, which is the correct architectural approach.
apps/webapp/app/env.server.ts (1)
233-233: LGTM!The v4 environment variable additions are excellently implemented using zod's transformation capabilities. The layered configuration approach with fallbacks to v3 variables is well-designed:
- Proper use of
.transform()for fallback logic- Correct use of
.pipe()to maintain required string types for HOST and NAMESPACE- Maintains backward compatibility while enabling new v4 functionality
- Follows zod best practices throughout
This provides a clean foundation for the optional v4 deploy registry overrides feature.
Also applies to: 241-272
apps/webapp/app/v3/getDeploymentImageRef.server.ts (1)
13-13: Clean refactoring to use RegistryConfig.The refactoring to use the
RegistryConfigobject instead of individual parameters is well-executed. The encapsulation improves maintainability and makes the function signature cleaner.Also applies to: 100-152
apps/webapp/app/v3/services/finalizeDeploymentV2.server.ts (1)
252-289: Well-structured authentication logic.The refactored
ensureLoggedIntoDockerRegistryfunction properly handles both ECR and non-ECR registries with appropriate credential retrieval and validation.
|
Optionally use different registry config for v4, with tests.