-
-
Notifications
You must be signed in to change notification settings - Fork 725
Feat: unified deploys for self-hosted and cloud users incl. multi-platform support #2138
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 71de319 The changes in this PR will be included in the next version bump. 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 update implements a major refactor and simplification of the deployment, build, and registry handling logic across the CLI, web application, and backend services. The registry proxy feature and its supporting code are removed, including environment variables and middleware. Deployment and build flows are unified to support a streamlined local or remote build process, with explicit platform and builder handling. Environment schemas and database models are updated to include and require image platform information. The CLI commands for deployment and worker builds are simplified, removing self-hosted and registry-specific flags in favor of a unified local build mode. Docker build logic is refactored to manage buildx builders, support multi-platform builds, and extract image digests from metadata. Error handling and logging are enhanced throughout, and API schemas are updated to reflect the new build and deployment contract. 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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (6)
packages/cli-v3/src/entryPoints/managed-index-controller.ts (1)
95-96
: Consider adding validation for platform environment variables.The platform environment variables are read directly from
process.env
without validation. Consider validating that they contain expected platform values if they're provided.- const buildPlatform = process.env.BUILDPLATFORM; - const targetPlatform = process.env.TARGETPLATFORM; + const buildPlatform = process.env.BUILDPLATFORM; + const targetPlatform = process.env.TARGETPLATFORM; + + // Validate platform formats if provided + if (buildPlatform && !/^[a-z0-9]+\/[a-z0-9]+$/.test(buildPlatform)) { + throw new Error(`Invalid BUILDPLATFORM format: ${buildPlatform}`); + } + if (targetPlatform && !/^[a-z0-9]+\/[a-z0-9]+$/.test(targetPlatform)) { + throw new Error(`Invalid TARGETPLATFORM format: ${targetPlatform}`); + }apps/webapp/app/v3/services/createDeploymentBackgroundWorkerV4.server.ts (1)
166-173
: Good timeout management improvement!Scheduling a timeout when entering the "DEPLOYING" state prevents deployments from hanging indefinitely. The configurable timeout via
DEPLOY_TIMEOUT_MS
provides flexibility.Consider adding a comment explaining the timeout behavior for future maintainers.
+ // Schedule a timeout to prevent deployments from hanging indefinitely + // The timeout will transition the deployment to "TIMED_OUT" status if not completed await TimeoutDeploymentService.enqueue( deployment.id, "DEPLOYING", "Indexing timed out", new Date(Date.now() + env.DEPLOY_TIMEOUT_MS), this._prisma );apps/webapp/app/v3/services/finalizeDeployment.server.ts (1)
125-136
: Consider throwing an error for invalid digests instead of silently returning undefined.While the validation logic is correct, returning
undefined
for an invalid digest might make issues harder to debug. Since this is a finalization step, it might be better to fail fast with a clear error.Consider this alternative approach:
function validatedImageDigest(imageDigest?: string): string | undefined { if (!imageDigest) { return; } if (!/^sha256:[a-f0-9]{64}$/.test(imageDigest.trim())) { - logger.error("Invalid image digest", { imageDigest }); - return; + throw new ServiceValidationError(`Invalid image digest format: ${imageDigest}`); } return imageDigest.trim(); }packages/core/src/v3/schemas/api.ts (1)
331-331
: Consider adding migration guidance for the deprecated field.The addition of
imagePlatform
and deprecation ofselfHosted
are appropriate changes. However, the deprecation comment could be more helpful.Consider expanding the deprecation comment to guide users:
- /** @deprecated This is now determined by the webapp. This is only used to warn users with old CLI versions. */ + /** + * @deprecated This field is now determined by the webapp based on environment configuration. + * It's only used for backward compatibility to warn users with CLI versions < 4.0.0. + * This field will be removed in a future version. + */ selfHosted: z.boolean().optional(),Also applies to: 340-341
packages/cli-v3/src/deploy/buildImage.ts (2)
913-917
: Simplify boolean expression.The ternary operator with boolean literals is unnecessary here.
Apply this diff to simplify:
- return imageTag.startsWith("localhost") || - imageTag.startsWith("127.0.0.1") || - imageTag.startsWith("0.0.0.0") - ? false - : true; + return !(imageTag.startsWith("localhost") || + imageTag.startsWith("127.0.0.1") || + imageTag.startsWith("0.0.0.0"));🧰 Tools
🪛 Biome (1.9.4)
[error] 913-917: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
935-935
: Simplify boolean expression.The ternary operator returning boolean literals is unnecessary.
Apply this diff to simplify:
- return push ? false : true; + return !push;🧰 Tools
🪛 Biome (1.9.4)
[error] 935-935: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
references/hello-world/trigger.config.ts
is excluded by!references/**
📒 Files selected for processing (38)
.env.example
(1 hunks)apps/supervisor/src/env.ts
(1 hunks)apps/supervisor/src/util.ts
(1 hunks)apps/supervisor/src/workloadManager/docker.ts
(4 hunks)apps/webapp/app/components/GitMetadata.tsx
(1 hunks)apps/webapp/app/entry.server.tsx
(1 hunks)apps/webapp/app/env.server.ts
(1 hunks)apps/webapp/app/presenters/v3/DeploymentPresenter.server.ts
(2 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments.$deploymentParam/route.tsx
(3 hunks)apps/webapp/app/routes/api.v1.deployments.$deploymentId.background-workers.ts
(3 hunks)apps/webapp/app/routes/api.v1.deployments.ts
(2 hunks)apps/webapp/app/services/runsReplicationInstance.server.ts
(1 hunks)apps/webapp/app/v3/handleSocketIo.server.ts
(2 hunks)apps/webapp/app/v3/registryProxy.server.ts
(0 hunks)apps/webapp/app/v3/remoteImageBuilder.server.ts
(2 hunks)apps/webapp/app/v3/services/createDeploymentBackgroundWorkerV3.server.ts
(1 hunks)apps/webapp/app/v3/services/createDeploymentBackgroundWorkerV4.server.ts
(4 hunks)apps/webapp/app/v3/services/failDeployment.server.ts
(1 hunks)apps/webapp/app/v3/services/finalizeDeployment.server.ts
(5 hunks)apps/webapp/app/v3/services/finalizeDeploymentV2.server.ts
(9 hunks)apps/webapp/app/v3/services/initializeDeployment.server.ts
(5 hunks)apps/webapp/app/v3/services/timeoutDeployment.server.ts
(1 hunks)apps/webapp/package.json
(0 hunks)apps/webapp/server.ts
(0 hunks)internal-packages/database/prisma/migrations/20250530131049_add_worker_deployment_image_platform/migration.sql
(1 hunks)internal-packages/database/prisma/schema.prisma
(1 hunks)internal-packages/run-engine/src/engine/systems/dequeueSystem.ts
(1 hunks)packages/cli-v3/src/apiClient.ts
(3 hunks)packages/cli-v3/src/commands/deploy.ts
(7 hunks)packages/cli-v3/src/commands/switch.ts
(5 hunks)packages/cli-v3/src/commands/workers/build.ts
(3 hunks)packages/cli-v3/src/deploy/buildImage.ts
(17 hunks)packages/cli-v3/src/entryPoints/managed-index-controller.ts
(2 hunks)packages/cli-v3/src/utilities/fileSystem.ts
(1 hunks)packages/cli-v3/src/utilities/tempDirectories.ts
(1 hunks)packages/core/src/v3/apiClient/core.ts
(2 hunks)packages/core/src/v3/schemas/api.ts
(3 hunks)packages/core/src/v3/schemas/runEngine.ts
(1 hunks)
💤 Files with no reviewable changes (3)
- apps/webapp/package.json
- apps/webapp/server.ts
- apps/webapp/app/v3/registryProxy.server.ts
🧰 Additional context used
🧬 Code Graph Analysis (12)
apps/webapp/app/v3/handleSocketIo.server.ts (1)
apps/webapp/app/v3/services/createDeploymentBackgroundWorkerV3.server.ts (1)
CreateDeploymentBackgroundWorkerServiceV3
(21-171)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments.$deploymentParam/route.tsx (1)
apps/webapp/app/v3/services/createDeploymentBackgroundWorkerV4.server.ts (1)
deployment
(178-196)
apps/webapp/app/entry.server.tsx (2)
apps/webapp/app/env.server.ts (1)
env
(784-784)apps/webapp/app/v3/remoteImageBuilder.server.ts (1)
remoteBuildsEnabled
(61-63)
apps/webapp/app/v3/services/timeoutDeployment.server.ts (1)
apps/webapp/app/db.server.ts (1)
PrismaClientOrTransaction
(21-21)
apps/webapp/app/v3/remoteImageBuilder.server.ts (1)
apps/webapp/app/env.server.ts (1)
env
(784-784)
apps/webapp/app/v3/services/createDeploymentBackgroundWorkerV4.server.ts (3)
internal-packages/run-engine/src/engine/errors.ts (1)
ServiceValidationError
(59-67)apps/webapp/app/v3/services/timeoutDeployment.server.ts (1)
TimeoutDeploymentService
(7-71)apps/webapp/app/env.server.ts (1)
env
(784-784)
packages/cli-v3/src/apiClient.ts (2)
packages/core/src/v3/apiClient/core.ts (1)
ApiResult
(706-711)packages/core/src/v3/schemas/api.ts (2)
FailDeploymentResponseBody
(363-365)FailDeploymentResponseBody
(367-367)
apps/webapp/app/v3/services/initializeDeployment.server.ts (3)
apps/webapp/app/v3/remoteImageBuilder.server.ts (2)
remoteBuildsEnabled
(61-63)createRemoteImageBuild
(6-27)internal-packages/run-engine/src/engine/errors.ts (1)
ServiceValidationError
(59-67)apps/webapp/app/env.server.ts (1)
env
(784-784)
apps/supervisor/src/env.ts (1)
apps/supervisor/src/envUtil.ts (1)
BoolEnv
(6-12)
apps/webapp/app/v3/services/finalizeDeployment.server.ts (3)
apps/webapp/app/v3/services/failDeployment.server.ts (1)
FailDeploymentService
(15-56)apps/webapp/app/v3/services/createDeploymentBackgroundWorkerV4.server.ts (1)
deployment
(178-196)apps/webapp/app/v3/services/timeoutDeployment.server.ts (1)
TimeoutDeploymentService
(7-71)
apps/supervisor/src/workloadManager/docker.ts (3)
apps/supervisor/src/workloadManager/types.ts (1)
WorkloadManagerOptions
(3-14)apps/supervisor/src/env.ts (1)
env
(101-101)packages/core/src/utils.ts (1)
tryCatch
(5-18)
packages/cli-v3/src/commands/switch.ts (1)
packages/cli-v3/src/cli/common.ts (2)
handleTelemetry
(39-49)wrapCommandAction
(53-103)
🪛 Biome (1.9.4)
packages/cli-v3/src/deploy/buildImage.ts
[error] 913-917: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
[error] 935-935: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
⏰ Context from checks skipped due to timeout of 90000ms (25)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (10, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (9, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (64)
apps/webapp/app/services/runsReplicationInstance.server.ts (2)
18-21
: Trivial formatting change
An extra space was added after the “🗃️” emoji in the “Runs replication service not enabled” log. This is purely cosmetic and does not affect functionality.
22-24
: Trivial formatting change
An extra space was added after the “🗃️” emoji in the “Runs replication service enabled” log. This is purely cosmetic and does not affect functionality.packages/core/src/v3/apiClient/core.ts (2)
20-20
: Good type safety improvement!Adding the explicit import of
ErrorEvent
type improves type specificity for error handling.
668-668
: Excellent type alignment with EventSource API!Changing the parameter type from
Event
toErrorEvent
correctly aligns with the EventSource specification, whereonerror
handlers specifically receiveErrorEvent
objects. This improves type safety and provides better IntelliSense support.packages/cli-v3/src/utilities/fileSystem.ts (1)
59-59
: Typo correction - good catch!Fixed the typographical error in the function name from
safeFeadJSONFile
tosafeReadJSONFile
.apps/webapp/app/presenters/v3/DeploymentPresenter.server.ts (1)
77-77
: Consistent implementation of imagePlatform support!The addition of
imagePlatform
field is implemented correctly:
- Added to the database query selection (line 77)
- Included in the returned deployment object (line 161)
This follows the established pattern and supports the multi-platform deployment feature mentioned in the PR objectives.
Also applies to: 161-161
internal-packages/database/prisma/schema.prisma (1)
861-861
: LGTM! Well-designed schema addition for multi-platform support.The
imagePlatform
field addition is well-implemented:
- Non-nullable with sensible default value prevents breaking changes
- Default "linux/amd64" covers the most common deployment scenario
- Supports the unified deployment process for multi-platform builds mentioned in the PR objectives
apps/webapp/app/components/GitMetadata.tsx (1)
12-12
: LGTM! Improved UI ordering.Moving the commit metadata to render after branch metadata provides a more logical display order, showing branch information before commit details.
internal-packages/database/prisma/migrations/20250530131049_add_worker_deployment_image_platform/migration.sql (1)
1-2
: LGTM! Migration correctly implements the schema change.The migration safely adds the
imagePlatform
column with appropriate constraints:
- NOT NULL with DEFAULT ensures no data integrity issues
- Default value matches the schema definition
- Existing WorkerDeployment records will get the default platform value
apps/webapp/app/v3/services/failDeployment.server.ts (1)
21-26
: LGTM! More defensive database query approach.The change from
findUnique
tofindFirst
is a sensible defensive programming practice:
- Functionally equivalent when data is unique (as expected with
friendlyId
)- More resilient to potential edge cases or data inconsistencies
- Aligns with similar changes mentioned in other services
- The
environmentId
filter provides additional security validationpackages/cli-v3/src/utilities/tempDirectories.ts (1)
40-40
: LGTM! Good addition for debugging support.The addition of
process.env.KEEP_TMP_DIRS
check provides external control over temporary directory cleanup, which is valuable for debugging build issues or implementing build caching scenarios. The logic correctly maintains backward compatibility.apps/webapp/app/entry.server.tsx (2)
224-224
: Import aligns with registry proxy removal.Good replacement of the removed
registryProxy
export withremoteBuildsEnabled
function import, supporting the unified deployment approach.
230-234
: Useful logging for build mode visibility.The conditional logging provides clear visibility into whether remote or local builds are enabled, which will help with debugging deployment issues.
internal-packages/run-engine/src/engine/systems/dequeueSystem.ts (1)
487-487
: Platform metadata addition looks good.The addition of
imagePlatform
to the deployment object in theDequeuedMessage
correctly uses optional chaining and aligns with the schema changes for multi-platform support. This provides necessary platform information to workers during task execution.packages/core/src/v3/schemas/runEngine.ts (1)
242-242
: Schema extension properly implemented.The addition of optional
imagePlatform
field to the deployment schema is correctly implemented using Zod'soptional()
method, ensuring backward compatibility while supporting the new multi-platform deployment functionality..env.example (1)
83-83
: LGTM: Configuration update aligns with registry proxy removal.The replacement of
ENABLE_REGISTRY_PROXY
withDEPOT_ORG_ID
is consistent with the PR's objective to remove registry proxy functionality and unify the deployment process using Depot.apps/supervisor/src/util.ts (1)
1-5
: Excellent refactoring: Improved platform detection using std-env.The change from manual
process.platform
checks to usingisMacOS
andisWindows
constants fromstd-env
improves code readability and centralizes platform detection logic while maintaining the same functional behavior.apps/webapp/app/routes/api.v1.deployments.$deploymentId.background-workers.ts (2)
8-8
: LGTM: Migration to V4 service for multi-platform support.The upgrade to
CreateDeploymentBackgroundWorkerServiceV4
aligns with the PR's objective to add multi-platform deployment support and represents the latest version of the background worker creation service.Also applies to: 45-45
66-66
: Good improvement: Respects error status codes.The change from hardcoded
{ status: 400 }
to{ status: e.status ?? 400 }
properly respects the error's status code when available, providing more accurate HTTP responses.apps/webapp/app/v3/remoteImageBuilder.server.ts (2)
7-7
: LGTM: Good refactoring to use centralized function.The change from direct environment variable checking to using the
remoteBuildsEnabled()
function improves code maintainability and readability.
61-63
: LGTM: Well-designed utility function.The
remoteBuildsEnabled()
function centralizes the logic for determining remote build capability. The function name is descriptive and the implementation correctly checks all required environment variables.apps/webapp/app/v3/services/createDeploymentBackgroundWorkerV3.server.ts (1)
15-21
: LGTM: Proper deprecation approach.The deprecation is well-documented with clear JSDoc comments explaining the reason and replacement. The class renaming with the V3 suffix provides good version clarity.
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments.$deploymentParam/route.tsx (2)
94-97
: LGTM: Useful addition to debug information.Adding the platform information to the admin debug tooltip enhances troubleshooting capabilities by surfacing the
imagePlatform
field that's now part of the deployment data structure.
237-269
: LGTM: Improved conditional rendering logic.The refactoring from mutually exclusive conditional rendering to independent conditions is a good improvement. This allows both deployment errors and task information to be displayed simultaneously when both exist, providing better visibility into deployment state.
apps/webapp/app/v3/services/timeoutDeployment.server.ts (1)
49-50
: LGTM: Good API improvements with transaction support.The changes enhance the service by:
- Adding optional transaction support via the
tx
parameter for better database consistency- Correcting the parameter name from
id
todeploymentId
for better clarity- Properly passing the transaction context through to the underlying queue operation
These changes improve the API design and maintain transaction integrity.
Also applies to: 55-55, 63-63
packages/cli-v3/src/entryPoints/managed-index-controller.ts (1)
114-140
: Excellent error handling improvement with appropriate logging.The change from direct await to capturing the response and adding structured error handling is a significant improvement. The decision to not fail the deployment for multi-platform scenarios is well-reasoned and documented.
apps/webapp/app/routes/api.v1.deployments.ts (2)
37-37
: LGTM: Consistent with unified image reference handling.The change from
imageTag
toimageRef
aligns with the broader refactoring to use unified image references throughout the system.
46-47
: Good addition of platform awareness to deployment response.Adding
imagePlatform
to the response provides essential platform information for deployments, supporting the multi-platform deployment feature.apps/supervisor/src/env.ts (1)
44-51
: Well-structured Docker configuration additions.The new Docker environment variables provide comprehensive configuration support:
DOCKER_API_VERSION
with a reasonable default- Optional platform override capability
- Registry authentication support
- Configurable image digest handling
The use of
BoolEnv
forDOCKER_STRIP_IMAGE_DIGEST
is consistent with the existing pattern in the codebase.apps/webapp/app/v3/services/initializeDeployment.server.ts (5)
49-54
: Appropriate validation for deployment configuration conflict.The validation correctly prevents self-hosted deployments when remote builds are enabled, maintaining configuration consistency. The error message is clear and informative.
56-56
: Simplified external build data creation logic.Removing the conditional logic and always calling
createRemoteImageBuild
simplifies the code flow. The function itself handles the case when remote builds are not enabled by returningundefined
.
71-75
: Simplified and unified image reference construction.The unified
imageRef
construction eliminates separate managed/unmanaged logic, making the code cleaner and more maintainable. The approach aligns with the unified deployment strategy.
98-99
: Good integration of platform awareness.Adding
imagePlatform
from environment configuration ensures consistent platform specification across the deployment lifecycle. This supports the multi-platform deployment feature effectively.
59-59
:✅ Verification successful
Verify consistency of user lookup method change.
The change from
findUnique
tofindFirst
maintains the same filtering criteria but changes the query semantics. Ensure this change is intentional and consistent with similar lookups throughout the codebase.
🏁 Script executed:
#!/bin/bash # Description: Check for other user lookup patterns in the codebase to ensure consistency # Expected: Find other instances of user lookup by ID with org membership filtering rg -A 10 -B 2 "user\.find(Unique|First)" --type ts | grep -A 15 -B 5 "orgMemberships"Length of output: 1672
findFirst is correct for nested orgMemberships filter
Prisma’s
findUnique
only accepts unique fields, so switching tofindFirst
is intentional and required when filtering onorgMemberships
. Other lookups (e.g. inapps/webapp/app/services/apiAuth.server.ts
) usefindUnique
solely byid
, so there’s no inconsistency.packages/cli-v3/src/apiClient.ts (2)
293-293
: Good improvement to promise handling!Removing the
reject
parameter and always resolving with anApiResult
type is a cleaner pattern that avoids unhandled promise rejections and provides more predictable error handling for consumers.
312-321
: Excellent error handling enhancement!Prefixing error messages with HTTP status codes provides valuable debugging context. The implementation correctly handles cases where status might be undefined.
Also applies to: 332-336
apps/webapp/app/env.server.ts (1)
269-269
: Verify breaking change is properly communicated.Making
DEPLOY_REGISTRY_HOST
required is a breaking change that will cause the application to fail at startup if this environment variable is not set. Please ensure:
- This breaking change is documented in the PR description ✓ (confirmed)
- Migration guide is provided for existing deployments
- The error message when this variable is missing is clear and actionable
The addition of
DEPLOY_IMAGE_PLATFORM
with a sensible default is a good approach for backward compatibility.Also applies to: 273-273
apps/webapp/app/v3/services/createDeploymentBackgroundWorkerV4.server.ts (2)
12-12
: LGTM!Good practice using versioning (V4 suffix) for the service class to maintain backward compatibility.
Also applies to: 14-14
23-31
: Good telemetry practice!Recording platform information in span attributes will be valuable for debugging multi-platform deployments.
apps/webapp/app/v3/services/finalizeDeployment.server.ts (3)
10-11
: LGTM!The new imports align with the removal of the registry proxy and the addition of proper failure and timeout handling services.
63-63
: LGTM!The image digest validation and conditional appending to the image reference is implemented correctly. The code properly handles cases where the digest might be missing or invalid.
Also applies to: 73-74
78-78
: LGTM!Correctly dequeues any pending timeout jobs after successful deployment, preventing unnecessary timeout notifications.
apps/webapp/app/v3/services/finalizeDeploymentV2.server.ts (4)
20-24
: LGTM!Good backward compatibility approach - delegating to v1 service when remote builds are not enabled ensures existing deployments continue to work.
91-94
: LGTM!Appropriate validation ensuring all deployments have an image reference, which aligns with the new unified deployment approach.
115-115
: LGTM!The simplified image reference handling is cleaner and aligns with the requirement that all deployments must have an image reference.
Also applies to: 155-155, 181-181
133-135
:⚠️ Potential issueFix undefined variable error.
The
finalizedDeployment
variable is used on line 135 but never defined since line 133 doesn't assign the result offinalizeService.call()
.Apply this fix:
const finalizeService = new FinalizeDeploymentService(); -await finalizeService.call(authenticatedEnv, id, body); +const finalizedDeployment = await finalizeService.call(authenticatedEnv, id, body); return finalizedDeployment;Likely an incorrect or invalid review comment.
packages/core/src/v3/schemas/api.ts (2)
57-58
: LGTM!The addition of optional
buildPlatform
andtargetPlatform
fields supports the new multi-platform build capabilities mentioned in the PR objectives.
296-299
: LGTM!The schema simplification correctly reflects the new deployment finalization process where only the image digest is needed, as the image reference is now set during deployment initialization.
packages/cli-v3/src/commands/workers/build.ts (2)
322-345
: LGTM! Improved build parameter structure aligns with unified deployment process.The refactoring from
selfHosted
/buildPlatform
toisLocalBuild
/imagePlatform
provides better clarity. The platform is now sourced from the deployment response, ensuring consistency across the system.
440-440
: Using deployment.imageTag in output message is the correct approach.This ensures consistency by using the deployment's authoritative image tag rather than the build result.
apps/supervisor/src/workloadManager/docker.ts (3)
21-54
: Excellent implementation of Docker configuration with proper security practices.The constructor properly:
- Configures Docker API version for compatibility
- Logs platform override information for debugging
- Securely handles registry credentials (logging username/URL but not password)
- Provides clear warnings when authentication is not configured
120-217
: Well-architected platform-aware Docker image handling with robust error recovery.The implementation excellently handles:
- Optional image digest stripping for flexibility
- Smart platform compatibility checking that inspects local images and pulls when architecture mismatches
- Comprehensive error handling using the
tryCatch
utility pattern- Detailed logging for debugging multi-platform deployment issues
266-272
: Clean implementation of stream reading helper.The
readAllChunks
function provides a simple way to consume the Docker image pull response stream.packages/cli-v3/src/commands/switch.ts (2)
27-84
: Excellent enhancement to support direct profile switching with robust error handling.The implementation provides:
- Convenient direct profile switching via positional argument
- Appropriate behavior for CI/TTY environments
- Clear, actionable error messages directing users to the
login
command- Efficient early returns when already on the target profile
54-55
: Improved error handling with consistent behavior across all code paths.The changes ensure:
- Proper error propagation by throwing instead of silent returns
- Consistent outro messages before errors
- Duplicate current profile checks in both direct and interactive modes
Also applies to: 103-106
packages/cli-v3/src/commands/deploy.ts (6)
48-60
: LGTM! Clean refactoring of deployment options.The replacement of explicit self-hosted flags with local build options (
load
,push
,network
,builder
) simplifies the interface and aligns well with the unified deployment approach.
316-316
: Good simplification of local build detection.Deriving
isLocalBuild
from the absence ofexternalBuildData
is a cleaner approach than using explicit flags.
319-328
: Excellent fail-fast verification for local builds.Checking for Docker buildx availability before attempting local builds prevents cryptic errors later in the process.
397-433
: LGTM! Build options properly aligned with new structure.The buildImage call correctly uses the new options including
isLocalBuild
,imagePlatform
, and local build options.
514-514
: Correct usage of imageDigest for finalization.Using
imageDigest
instead ofimageReference
properly aligns with the removal of registry proxy dependencies.
635-646
: Good UX improvement for error diagnostics.Displaying the last few lines of logs (filtered to remove comment lines) provides immediate context without overwhelming the user.
packages/cli-v3/src/deploy/buildImage.ts (3)
254-268
: Excellent metadata parsing implementation.Using a Zod schema for BuildKit metadata parsing is a robust approach that provides type safety and validation. The error handling with logging is also well implemented.
Also applies to: 489-505
307-404
: Comprehensive Docker builder management.The implementation properly handles:
- Builder existence checks
- Network configuration validation
- Builder recreation when network changes
- Clear error messages at each step
This is a significant improvement in local build reliability.
860-887
: Good multi-platform build support.The QEMU registration check and setup for multi-platform builds on Linux is well implemented. This prevents build failures when targeting multiple platforms.
apps/webapp/app/v3/services/createDeploymentBackgroundWorkerV4.server.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
packages/cli-v3/src/deploy/buildImage.ts (2)
100-104
:⚠️ Potential issueUpdate the error message to reflect the new deployment process.
The error message references the
--self-hosted
flag which has been removed according to the PR objectives. Users now deploy usingnpx trigger.dev@v4-beta deploy
for both cloud and self-hosted deployments.Apply this diff to fix the error message:
- "Failed to initialize deployment. The deployment does not have any external build data. To deploy this project, you must use the --self-hosted flag to build and push the image yourself." + "Failed to initialize deployment. The deployment does not have any external build data. To deploy this project, you must use the local build mode."
307-528
: 🛠️ Refactor suggestionConsider breaking down this function for better maintainability.
The
localBuildImage
function is over 200 lines and handles multiple responsibilities. Consider extracting the following into separate functions:
- Builder management logic (lines 310-404)
- Docker build arguments preparation (lines 406-458)
- Build execution and error handling (lines 465-482)
- Metadata parsing and digest extraction (lines 484-500)
This would improve readability, testability, and make the code easier to maintain.
🧹 Nitpick comments (7)
packages/cli-v3/src/deploy/buildImage.ts (7)
808-812
: Make URL normalization more robust.The current implementation could replace "localhost" in unexpected places (e.g., path segments). Consider parsing the URL properly to only replace the hostname.
Apply this more robust implementation:
-function normalizeApiUrlForBuild(apiUrl: string): string { - return apiUrl.replace("localhost", "host.docker.internal"); -} +function normalizeApiUrlForBuild(apiUrl: string): string { + try { + const url = new URL(apiUrl); + if (url.hostname === "localhost") { + url.hostname = "host.docker.internal"; + } + return url.toString(); + } catch { + // Fallback for invalid URLs + return apiUrl; + } +}
898-918
: Simplify the boolean logic in shouldPush function.The ternary operators are unnecessary when the condition already evaluates to a boolean.
Apply this simplification:
-function shouldPush(imageTag: string, push?: boolean) { - switch (push) { - case true: { - return true; - } - case false: { - return false; - } - case undefined: { - return imageTag.startsWith("localhost") || - imageTag.startsWith("127.0.0.1") || - imageTag.startsWith("0.0.0.0") - ? false - : true; - } - default: { - assertExhaustive(push); - } - } -} +function shouldPush(imageTag: string, push?: boolean) { + switch (push) { + case true: { + return true; + } + case false: { + return false; + } + case undefined: { + return !(imageTag.startsWith("localhost") || + imageTag.startsWith("127.0.0.1") || + imageTag.startsWith("0.0.0.0")); + } + default: { + assertExhaustive(push); + } + } +}🧰 Tools
🪛 Biome (1.9.4)
[error] 908-912: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
920-936
: Simplify the boolean logic in shouldLoad function.The ternary operator is unnecessary when the condition already evaluates to a boolean.
Apply this simplification:
-function shouldLoad(load?: boolean, push?: boolean) { - switch (load) { - case true: { - return true; - } - case false: { - return false; - } - case undefined: { - return push ? false : true; - } - default: { - assertExhaustive(load); - } - } -} +function shouldLoad(load?: boolean, push?: boolean) { + switch (load) { + case true: { + return true; + } + case false: { + return false; + } + case undefined: { + return !push; + } + default: { + assertExhaustive(load); + } + } +}🧰 Tools
🪛 Biome (1.9.4)
[error] 930-930: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
814-831
: Add logging when falling back to default IP.When no suitable network interface is found, the function silently falls back to "127.0.0.1". This could cause connectivity issues in certain Docker configurations.
Consider adding debug logging:
function getHostIP() { const interfaces = networkInterfaces(); for (const [name, iface] of Object.entries(interfaces)) { if (!iface) { continue; } for (const net of iface) { // Skip internal/loopback and non-IPv4 addresses if (!net.internal && net.family === "IPv4") { return net.address; } } } + logger.debug("No suitable network interface found, falling back to 127.0.0.1"); return "127.0.0.1"; }
841-849
: Improve QEMU registration check robustness.The current implementation only checks for
qemu-aarch64
which may not be sufficient for all multi-platform build scenarios. Also, errors are silently ignored.Consider checking multiple architectures and adding debug logging:
function isQemuRegistered() { try { - // Check a single QEMU handler - const binfmt = readFileSync("/proc/sys/fs/binfmt_misc/qemu-aarch64", "utf8"); - return binfmt.includes("enabled"); + // Check common QEMU handlers for cross-platform builds + const handlers = ["qemu-aarch64", "qemu-arm", "qemu-x86_64"]; + for (const handler of handlers) { + try { + const binfmt = readFileSync(`/proc/sys/fs/binfmt_misc/${handler}`, "utf8"); + if (binfmt.includes("enabled")) { + return true; + } + } catch { + // Handler doesn't exist, continue checking + } + } + return false; } catch (e) { + logger.debug("Failed to check QEMU registration", { error: e }); return false; } }
254-268
: Extract common metadata parsing logic.Both
remoteBuildImage
andlocalBuildImage
have identical metadata parsing logic. Consider extracting this into a shared helper function to reduce duplication.Add a helper function:
async function parseImageMetadata(metadataPath: string, onLog?: (log: string) => void): Promise<string | undefined> { const rawMetadata = await safeReadJSONFile(metadataPath); const meta = BuildKitMetadata.safeParse(rawMetadata); if (!meta.success) { logger.error("Failed to parse metadata.json", { errors: meta.error.message, path: metadataPath, }); return undefined; } logger.debug("Parsed metadata.json", { metadata: meta.data, path: metadataPath }); return meta.data["containerimage.digest"]; }Then use it in both functions:
- const metadataPath = join(options.cwd, "metadata.json"); - const rawMetadata = await safeReadJSONFile(metadataPath); - - const meta = BuildKitMetadata.safeParse(rawMetadata); - - let digest: string | undefined; - if (!meta.success) { - logger.error("Failed to parse metadata.json", { - errors: meta.error.message, - path: metadataPath, - }); - } else { - logger.debug("Parsed metadata.json", { metadata: meta.data, path: metadataPath }); - digest = meta.data["containerimage.digest"]; - } + const metadataPath = join(options.cwd, "metadata.json"); + const digest = await parseImageMetadata(metadataPath, options.onLog);Also applies to: 484-500
332-373
: Consider adding comments to explain the builder network logic.The builder network configuration logic is complex and would benefit from additional comments explaining:
- Why network configuration matters for the builder
- When and why a builder needs to be recreated
- The expected format of the docker buildx inspect output
This would help future maintainers understand the reasoning behind this implementation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/webapp/app/v3/services/failDeployment.server.ts
(1 hunks)apps/webapp/app/v3/services/finalizeDeployment.server.ts
(5 hunks)packages/cli-v3/src/commands/deploy.ts
(7 hunks)packages/cli-v3/src/deploy/buildImage.ts
(17 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/webapp/app/v3/services/failDeployment.server.ts
- apps/webapp/app/v3/services/finalizeDeployment.server.ts
- packages/cli-v3/src/commands/deploy.ts
🧰 Additional context used
🧠 Learnings (1)
packages/cli-v3/src/deploy/buildImage.ts (1)
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2138
File: packages/cli-v3/src/deploy/buildImage.ts:342-342
Timestamp: 2025-06-02T14:41:17.319Z
Learning: The `docker buildx inspect` command does not support the `--format` flag, unlike the regular `docker inspect` command. When parsing output from `docker buildx inspect`, regex-based parsing is the appropriate approach since structured JSON output is not available.
🪛 Biome (1.9.4)
packages/cli-v3/src/deploy/buildImage.ts
[error] 908-912: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
[error] 930-930: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
⏰ Context from checks skipped due to timeout of 90000ms (24)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (10, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (9, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 10)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
packages/cli-v3/src/deploy/buildImage.ts (1)
657-660
: Multi-platform build support looks good!The addition of
TARGETPLATFORM
andBUILDPLATFORM
ARGs and environment variables follows Docker best practices for multi-platform builds. This enables proper cross-compilation support.Also applies to: 769-772
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/supervisor/src/workloadManager/docker.ts (3)
41-54
: Consider security implications of logging registry username.The Docker registry authentication setup is well-implemented with proper conditional logic. However, consider whether logging the registry username in plain text could pose security concerns in your environment, especially if logs are stored or transmitted insecurely.
If you prefer to avoid logging sensitive information, consider this adjustment:
this.logger.info("🐋 Using Docker registry credentials", { - username: env.DOCKER_REGISTRY_USERNAME, + username: "[REDACTED]", url: env.DOCKER_REGISTRY_URL, });
121-125
: Consider more robust image digest stripping.The current image digest stripping logic works for standard cases but makes assumptions about image reference format. Consider adding validation or documentation about the expected format.
For a more robust approach, consider:
let imageRef = opts.image; if (env.DOCKER_STRIP_IMAGE_DIGEST) { - imageRef = opts.image.split("@")[0]!; + // Strip digest (everything after the last "@" if it contains "sha256:") + const atIndex = opts.image.lastIndexOf("@sha256:"); + if (atIndex !== -1) { + imageRef = opts.image.substring(0, atIndex); + } }
267-273
: LGTM: Clean helper function with potential for improvement.The
readAllChunks
helper function is well-implemented for reading Docker API responses. Consider adding error handling and making it more generic if it might be reused elsewhere.For enhanced robustness, consider:
-async function readAllChunks(reader: NodeJS.ReadableStream) { +async function readAllChunks(reader: NodeJS.ReadableStream): Promise<string[]> { const chunks = []; - for await (const chunk of reader) { - chunks.push(chunk.toString()); + try { + for await (const chunk of reader) { + chunks.push(chunk.toString()); + } + } catch (error) { + throw new Error(`Failed to read stream chunks: ${error instanceof Error ? error.message : String(error)}`); } return chunks; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.env.example
(1 hunks).github/workflows/unit-tests-webapp.yml
(2 hunks)apps/supervisor/src/env.ts
(2 hunks)apps/supervisor/src/workloadManager/docker.ts
(5 hunks)packages/cli-v3/src/entryPoints/managed/logger.ts
(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/cli-v3/src/entryPoints/managed/logger.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- .env.example
- .github/workflows/unit-tests-webapp.yml
- apps/supervisor/src/env.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/supervisor/src/workloadManager/docker.ts (3)
apps/supervisor/src/workloadManager/types.ts (1)
WorkloadManagerOptions
(3-14)apps/supervisor/src/env.ts (1)
env
(102-102)packages/core/src/utils.ts (1)
tryCatch
(5-18)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 10)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: typecheck / typecheck
🔇 Additional comments (5)
apps/supervisor/src/workloadManager/docker.ts (5)
17-18
: LGTM: Well-structured field additions.The new private readonly fields for authentication and platform override are properly typed and follow good encapsulation practices.
33-39
: LGTM: Excellent platform override implementation.The platform override setup includes informative logging that will be valuable for debugging multi-platform builds. The inclusion of both target and host platform information is particularly helpful.
75-75
: LGTM: Consistent environment variable addition.The
PRETTY_LOGS
environment variable addition follows the established pattern and will improve logging output formatting in containers.
137-139
: LGTM: Proper platform override implementation.The conditional platform override in container creation options correctly follows Docker API conventions and supports the multi-platform build objectives.
21-23
:✅ Verification successful
Verify environment variable validation for Docker API version.
The Docker API version is now configurable via environment variables, which supports the PR objective of locking the API version for compatibility. Ensure that
env.DOCKER_API_VERSION
has proper validation in the environment schema to prevent runtime issues with invalid versions.
🏁 Script executed:
#!/bin/bash # Description: Verify that DOCKER_API_VERSION has proper validation in the environment schema. # Search for DOCKER_API_VERSION validation in environment configuration rg -A 5 -B 5 "DOCKER_API_VERSION" apps/supervisor/src/env.tsLength of output: 524
Environment variable validation confirmed for DOCKER_API_VERSION
The
apps/supervisor/src/env.ts
schema defines:
DOCKER_API_VERSION: z.string().default("v1.41")
This ensures:
- Any provided value is validated as a string.
- A safe default (
"v1.41"
) is used when the environment variable is missing.No further validation or fallback handling is needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/ninety-games-grow.md (1)
6-11
: Nit: use “deployment” instead of “deploy”
In the fifth bullet (line 10), consider replacing “The deploy--self-hosted
flag” with “The deployment--self-hosted
flag” for grammatical accuracy.🧰 Tools
🪛 LanguageTool
[grammar] ~10-~10: The word ‘deploy’ is a verb. Did you mean the noun “deployment” (= release, placement)?
Context: ...s now fully managed by the webapp - The deploy--self-hosted
flag is no longer requi...(PREPOSITION_VERB)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
.changeset/ninety-games-grow.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
.changeset/ninety-games-grow.md
[grammar] ~10-~10: The word ‘deploy’ is a verb. Did you mean the noun “deployment” (= release, placement)?
Context: ...s now fully managed by the webapp - The deploy --self-hosted
flag is no longer requi...
(PREPOSITION_VERB)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: typecheck / typecheck
🔇 Additional comments (1)
.changeset/ninety-games-grow.md (1)
1-4
: Frontmatter format is correct
The YAML header properly declares the package bumps fortrigger.dev
and@trigger.dev/core
and uses the expected changeset syntax.
Self-hosters can now deploy just like cloud users: 'npx trigger.dev@v4-beta deploy`
Local vs remote build detection is now fully driven by the webapp. This means registry details must be set via webapp env vars in all cases.
DOCKER_*
env vars, incl authDEPLOY_REGISTRY_HOST
now required