-
-
Notifications
You must be signed in to change notification settings - Fork 913
feat(cli): deterministic image builds for deployments #2778
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: bc0b18e The changes in this PR will be included in the next version bump. This PR includes changesets to release 26 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 PR threads deployment identifiers through the run and workload stack and makes build/manifest output more deterministic. Supervisor now passes Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 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 |
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli-v3/src/deploy/buildImage.ts (1)
196-214: Remove TRIGGER_DEPLOYMENT_ID from build args to achieve deterministic, deployment-agnostic imagesWhile
SOURCE_DATE_EPOCH=0andrewrite-timestamp=truehandle timestamp reproducibility, passingTRIGGER_DEPLOYMENT_IDas a build arg (line 200, 523) means the resulting image config will still vary per deployment, causing different digests even for identical code.TRIGGER_DEPLOYMENT_VERSION is declared in the Dockerfile templates but is not passed as a build arg, so it doesn't affect the digest. However, TRIGGER_DEPLOYMENT_ID does and should be removed from the build args and set at runtime instead (e.g., via supervisor/kube env) to achieve the determinism goal.
Also applies to: 519-539, 731-752, 839-885
🧹 Nitpick comments (4)
internal-packages/run-engine/src/engine/systems/dequeueSystem.ts (1)
543-567: Empty-string deployment fields become the sentinel for “no deployment”The new construction:
- Always supplies string values for
deployment.id,deployment.friendlyId, anddeployment.version(using?? ""), and- Adds
deployment.versionto match the core schema.This is type-safe and compatible with the updated
DequeuedMessageschema, but it also means “no deployment” (notably in development environments) is now represented as empty strings instead ofundefined. If any downstream logic previously treatedundefinedspecially or assumed non-empty values imply a valid deployment, it’s worth double-checking those call sites to ensure this sentinel change is acceptable.apps/supervisor/src/index.ts (1)
247-264: ConfirmdeploymentIdis meant to carry the friendly id, not the DB idHere
deploymentIdis populated withmessage.deployment.friendlyIdrather thanmessage.deployment.id, while the type name suggests it might be the underlying DB id. That’s totally fine if the intent is “human‑readable deployment identifier” for logs/telemetry, but it’s easy for future readers to assume it’s the primary key.If the friendly id is indeed what runners should see, consider either:
- Leaving behavior as-is but clarifying in a comment, or
- Renaming the option/env to something like
deploymentFriendlyIdto avoid ambiguity.Otherwise, if the intent is to expose the DB id, this line should use
message.deployment.idinstead.packages/cli-v3/src/build/buildWorker.ts (1)
15-16: Deterministic package.json generation looks good; verify dev/peer/scripts behaviorRefactoring
writeDeployFilesto synthesizepackage.jsonfrombuildManifest.externalsand write it viawriteJSONFile(..., true)gives you a deterministic, minimal runtime manifest. SortingtrustedDependenciesis also a nice touch for reproducibility.Two things to double‑check:
- Runtime coverage: With
dependenciesnow driven purely bybuildManifest.externals, any package that’s only in the originaldevDependenciesbut still needed at runtime will no longer be installed in the image. Please verify your externals generation covers all true runtime dependencies.- Scripts/dev/peer deps: You’re explicitly zeroing
devDependencies,peerDependencies, andscripts. That’s good for slimming and determinism, but confirm you don’t rely onnpm run-style scripts or peer metadata inside the container before merging.Also applies to: 177-212
packages/cli-v3/src/deploy/buildImage.ts (1)
423-543: Local docker buildx: deterministic output flags are correct; consider push/load + inspect behaviorUsing
SOURCE_DATE_EPOCH=0and--output type=image,name=${imageTag},rewrite-timestamp=true,...for the localdocker buildx buildpath is consistent with the Depot path and should normalize timestamps in both the image config and layers.One subtle point to confirm: when
pushis true andloadends up false (default fromshouldLoad),type=image,...,push=truewill typically not leave a local image in the engine. Your subsequentdocker image inspect ${imageTag}to computeimageSizeBytesmay then return no data, silently leavingimageSizeBytes = 0. If you rely on accurate image size metrics for pushed-only builds, you may want to:
- either force
load=truewhenpush=true && options.load === undefined, or- skip the size inspection when you know the image is not loaded locally.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
apps/supervisor/src/index.ts(1 hunks)apps/supervisor/src/workloadManager/docker.ts(1 hunks)apps/supervisor/src/workloadManager/kubernetes.ts(1 hunks)apps/supervisor/src/workloadManager/types.ts(1 hunks)internal-packages/run-engine/src/engine/systems/dequeueSystem.ts(1 hunks)packages/cli-v3/package.json(2 hunks)packages/cli-v3/src/build/buildWorker.ts(2 hunks)packages/cli-v3/src/build/bundle.ts(1 hunks)packages/cli-v3/src/deploy/buildImage.ts(4 hunks)packages/cli-v3/src/entryPoints/managed-index-controller.ts(2 hunks)packages/cli-v3/src/entryPoints/managed/env.ts(1 hunks)packages/cli-v3/src/utilities/buildManifest.ts(1 hunks)packages/cli-v3/src/utilities/fileSystem.ts(2 hunks)packages/core/src/v3/schemas/runEngine.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/supervisor/src/workloadManager/docker.tspackages/core/src/v3/schemas/runEngine.tspackages/cli-v3/src/utilities/fileSystem.tspackages/cli-v3/src/build/bundle.tsapps/supervisor/src/index.tsapps/supervisor/src/workloadManager/kubernetes.tspackages/cli-v3/src/build/buildWorker.tspackages/cli-v3/src/entryPoints/managed-index-controller.tspackages/cli-v3/src/deploy/buildImage.tsinternal-packages/run-engine/src/engine/systems/dequeueSystem.tsapps/supervisor/src/workloadManager/types.tspackages/cli-v3/src/entryPoints/managed/env.tspackages/cli-v3/src/utilities/buildManifest.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/supervisor/src/workloadManager/docker.tspackages/core/src/v3/schemas/runEngine.tspackages/cli-v3/src/utilities/fileSystem.tspackages/cli-v3/src/build/bundle.tsapps/supervisor/src/index.tsapps/supervisor/src/workloadManager/kubernetes.tspackages/cli-v3/src/build/buildWorker.tspackages/cli-v3/src/entryPoints/managed-index-controller.tspackages/cli-v3/src/deploy/buildImage.tsinternal-packages/run-engine/src/engine/systems/dequeueSystem.tsapps/supervisor/src/workloadManager/types.tspackages/cli-v3/src/entryPoints/managed/env.tspackages/cli-v3/src/utilities/buildManifest.ts
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/supervisor/src/workloadManager/docker.tspackages/core/src/v3/schemas/runEngine.tspackages/cli-v3/src/utilities/fileSystem.tspackages/cli-v3/src/build/bundle.tsapps/supervisor/src/index.tsapps/supervisor/src/workloadManager/kubernetes.tspackages/cli-v3/src/build/buildWorker.tspackages/cli-v3/src/entryPoints/managed-index-controller.tspackages/cli-v3/src/deploy/buildImage.tsinternal-packages/run-engine/src/engine/systems/dequeueSystem.tsapps/supervisor/src/workloadManager/types.tspackages/cli-v3/src/entryPoints/managed/env.tspackages/cli-v3/src/utilities/buildManifest.tspackages/cli-v3/package.json
{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.ts
🧠 Learnings (15)
📚 Learning: 2025-06-04T16:02:22.957Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 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/supervisor/src/workloadManager/docker.ts
📚 Learning: 2025-11-10T09:09:07.399Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 2663
File: apps/webapp/app/env.server.ts:1205-1206
Timestamp: 2025-11-10T09:09:07.399Z
Learning: In the trigger.dev webapp, S2_ACCESS_TOKEN and S2_DEPLOYMENT_LOGS_BASIN_NAME environment variables must remain optional until an OSS version of S2 is available, to avoid breaking environments that don't have S2 provisioned.
Applied to files:
apps/supervisor/src/workloadManager/docker.tsapps/supervisor/src/workloadManager/kubernetes.tspackages/cli-v3/src/deploy/buildImage.tspackages/cli-v3/src/entryPoints/managed/env.ts
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/**/*.{ts,tsx} : Access all environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env` in the Trigger.dev webapp
Applied to files:
apps/supervisor/src/workloadManager/docker.tsapps/supervisor/src/workloadManager/kubernetes.tspackages/cli-v3/src/entryPoints/managed/env.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/supervisor/src/workloadManager/docker.tspackages/cli-v3/src/build/bundle.tsapps/supervisor/src/workloadManager/kubernetes.tspackages/cli-v3/src/build/buildWorker.tspackages/cli-v3/src/deploy/buildImage.tspackages/cli-v3/src/entryPoints/managed/env.tspackages/cli-v3/src/utilities/buildManifest.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `idempotencyKeyTTL` option to define a time window during which duplicate triggers return the original run
Applied to files:
apps/supervisor/src/workloadManager/docker.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Attach metadata to task runs using the metadata option when triggering, and access/update it inside runs using metadata functions
Applied to files:
apps/supervisor/src/workloadManager/docker.tspackages/cli-v3/src/deploy/buildImage.tspackages/cli-v3/src/entryPoints/managed/env.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Scope idempotency keys globally or to current run using the scope parameter
Applied to files:
apps/supervisor/src/workloadManager/docker.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 build process in trigger.config.ts using `build` object with external packages, extensions, and JSX settings
Applied to files:
packages/cli-v3/src/build/bundle.tspackages/cli-v3/src/build/buildWorker.tspackages/cli-v3/src/entryPoints/managed-index-controller.tspackages/cli-v3/src/deploy/buildImage.tspackages/cli-v3/src/entryPoints/managed/env.tspackages/cli-v3/src/utilities/buildManifest.ts
📚 Learning: 2025-08-20T07:41:13.973Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2424
File: apps/supervisor/src/workloadManager/docker.ts:175-178
Timestamp: 2025-08-20T07:41:13.973Z
Learning: dockerode createImage() supports multiple signatures including createImage(auth, options) -> Promise<ReadableStream> where auth is the first parameter (AuthConfig object) and options is the second parameter. Both createImage(auth, options) and createImage(options, {authconfig: auth}) are valid approaches.
Applied to files:
packages/cli-v3/src/deploy/buildImage.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger.config.ts : Specify runtime environment (node or bun) in trigger.config.ts using the `runtime` property
Applied to files:
packages/cli-v3/src/entryPoints/managed/env.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:
packages/cli-v3/src/entryPoints/managed/env.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `trigger.dev/sdk/v3` for all imports in Trigger.dev tasks
Applied to files:
packages/cli-v3/src/entryPoints/managed/env.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `schemaTask()` from `trigger.dev/sdk/v3` with Zod schema for payload validation
Applied to files:
packages/cli-v3/src/entryPoints/managed/env.ts
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: Applies to packages/trigger-sdk/**/*.{ts,tsx} : In the Trigger.dev SDK (packages/trigger-sdk), prefer isomorphic code like fetch and ReadableStream instead of Node.js-specific code
Applied to files:
packages/cli-v3/package.json
📚 Learning: 2025-11-27T16:27:48.109Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T16:27:48.109Z
Learning: pnpm version `10.23.0` and Node.js version `20.11.1` are required for development
Applied to files:
packages/cli-v3/package.json
🧬 Code graph analysis (3)
packages/cli-v3/src/utilities/fileSystem.ts (1)
packages/core/src/v3/apps/http.ts (1)
json(65-75)
packages/cli-v3/src/build/buildWorker.ts (1)
packages/cli-v3/src/utilities/fileSystem.ts (1)
writeJSONFile(162-164)
packages/cli-v3/src/entryPoints/managed-index-controller.ts (1)
packages/cli-v3/src/utilities/fileSystem.ts (1)
writeJSONFile(162-164)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (11)
packages/cli-v3/src/build/bundle.ts (1)
417-419: Approve conditional outputHashes for deterministic deploy builds.The change correctly omits
outputHashesfor deploy builds by providing an empty object while preserving it for dev builds. This ensures deterministic and reproducible deployments. The comment clearly explains the rationale—dev builds require the hash map for deduplication during rebuilds, while deploy builds omit it to achieve determinism.Downstream consumers in
copyDirWithStoreare already designed to handle emptyoutputHashesvia optional chaining and a fallback tocomputeFileHash, so the empty object will not cause issues.packages/cli-v3/src/utilities/buildManifest.ts (1)
4-4: Destructuring looks good.The explicit extraction of
externalsis appropriate for deterministic handling downstream.packages/core/src/v3/schemas/runEngine.ts (1)
248-253: DequeuedMessage deployment fields now correctly reflect non-optional runtime contractMaking
deployment.id,deployment.friendlyId, anddeployment.versionrequired strings in the schema matches howDequeueSystemnow always supplies string values (with safe fallbacks) and avoidsundefinedleaking across the boundary. This is consistent and should help downstream consumers rely on deployment metadata being present.apps/supervisor/src/workloadManager/docker.ts (1)
70-87: Deployment metadata correctly propagated into Docker container envAdding
TRIGGER_DEPLOYMENT_ID=${opts.deploymentId}andTRIGGER_DEPLOYMENT_VERSION=${opts.deploymentVersion}toenvVarscleanly exposes deployment context to the runner while keeping it purely runtime (no impact on image determinism) and still avoiding env leakage in logs.apps/supervisor/src/workloadManager/kubernetes.ts (1)
110-133: Kubernetes runner now has the same deployment context as DockerThe new
TRIGGER_DEPLOYMENT_IDandTRIGGER_DEPLOYMENT_VERSIONenv vars for therun-controllercontainer keep Kubernetes in sync with the Docker workload manager, ensuring runners in both orchestrators see consistent deployment metadata.packages/cli-v3/src/entryPoints/managed/env.ts (1)
21-24: Deployment env vars correctly reclassified as runtime-onlyTreating
TRIGGER_DEPLOYMENT_IDandTRIGGER_DEPLOYMENT_VERSIONas required runtime env vars (instead of build-time) matches the new supervisor behavior and removes deployment identity from image contents, which is important for deterministic builds.apps/supervisor/src/workloadManager/types.ts (1)
20-38: WorkloadManagerCreateOptions correctly extended with deployment identityAdding
deploymentIdanddeploymentVersionas required fields onWorkloadManagerCreateOptionsformalizes the need for deployment metadata at workload creation time and matches how Docker/Kubernetes managers now construct container env. This is a good, explicit contract extension—just ensure any alternative WorkloadManager implementations are updated accordingly.packages/cli-v3/package.json (1)
95-120: Dependency changes for deterministic JSON outputAdding
json-stable-stringifyaligns with the newwriteJSONFileimplementation and is appropriate for deterministic JSON serialization. I don't see any immediate issues with this dependency wiring from the CLI side.Please confirm that
json-stable-stringify@^1.3.0behaves as expected with your current Node/toolchain versions (especially around large objects and non-plain JSON values) and that no other packages still rely on@s2-dev/streamstorebefore finalizing this PR.packages/cli-v3/src/utilities/fileSystem.ts (1)
2-3: Switch to json-stable-stringify in writeJSONFile looks correctUsing
json-stable-stringifyhere gives deterministic key ordering across all JSON artifacts, withprettycontrolling indentation only. The call pattern (stringify(json, pretty ? { space: 2 } : undefined)) matches the library API and is safe given the existingsafeWriteFilebehavior.Please double‑check that all callers of
writeJSONFileonly pass JSON‑serializable data (no circular refs, functions, etc.), since any serialization error will now come fromjson-stable-stringifyinstead of the built‑inJSON.stringify.Also applies to: 162-164
packages/cli-v3/src/entryPoints/managed-index-controller.ts (1)
13-14: Persisting a timings‑free, stably formatted index.json is a good fit for reproducibilityDropping
timingsfrom the persistedindex.jsonand routing the write throughwriteJSONFile(..., true)nicely removes a source of non‑determinism while standardizing JSON formatting.Please confirm that no downstream consumer reads
timingsfromindex.json(as opposed to usingworkerManifestin‑memory) so this omission doesn’t silently change behavior in production.Also applies to: 90-94
packages/cli-v3/src/deploy/buildImage.ts (1)
178-219: Depot remote build: timestamp normalization arguments look correct; confirm CLI supportAdding
--build-arg SOURCE_DATE_EPOCH=0and--output type=image,rewrite-timestamp=trueshould normalize file and image timestamps for Depot builds while keeping the rest of the arguments intact.Please verify against the pinned
@depot/cliversion that:
--output type=image,rewrite-timestamp=trueis supported in combination with--save, and- the behavior of
--load(when set) with this--outputconfiguration still matches your expectations for where the image ends up and how metadata.json is produced.
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli-v3/src/deploy/buildImage.ts (1)
781-782: Remove unusedTRIGGER_DEPLOYMENT_VERSIONARG declarations from the Dockerfile, or clarify if they're intentionally left as placeholders.The Dockerfile declares
ARG TRIGGER_DEPLOYMENT_VERSION(lines 782, 890) and sets it as anENVvariable in the indexer stage (lines 792, 900), but the build commands do not pass this value via--build-arg. The variable is required at runtime—it's used incontroller.tsandtaskRunProcessProvider.ts—and the supervisor correctly provides it via the-eflag during container execution (apps/supervisor/src/workloadManager/docker.ts:76andkubernetes.ts:131).Since
TRIGGER_DEPLOYMENT_VERSIONis always supplied at runtime by the supervisor (not baked into the image), either remove these unused ARG/ENV declarations for clarity, or add a comment explaining they're kept as placeholders for consistency with other deployment variables.Applies also to: 791-792, 889-890, 899-900
🧹 Nitpick comments (1)
packages/cli-v3/src/deploy/buildImage.ts (1)
16-56: Consider using type instead of interface.As per coding guidelines, prefer types over interfaces for TypeScript. This applies to
BuildImageOptions,DepotBuildImageOptions, andSelfHostedBuildImageOptionsdefined in this file.Apply this pattern:
-export interface BuildImageOptions { +export type BuildImageOptions = { // ... properties -} +};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/five-pens-fly.md(1 hunks)internal-packages/run-engine/src/engine/systems/dequeueSystem.ts(1 hunks)packages/cli-v3/src/deploy/buildImage.ts(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- internal-packages/run-engine/src/engine/systems/dequeueSystem.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
packages/cli-v3/src/deploy/buildImage.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
packages/cli-v3/src/deploy/buildImage.ts
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
packages/cli-v3/src/deploy/buildImage.ts
🧠 Learnings (8)
📚 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:
packages/cli-v3/src/deploy/buildImage.ts.changeset/five-pens-fly.md
📚 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 build process in trigger.config.ts using `build` object with external packages, extensions, and JSX settings
Applied to files:
packages/cli-v3/src/deploy/buildImage.ts
📚 Learning: 2025-08-20T07:41:13.973Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2424
File: apps/supervisor/src/workloadManager/docker.ts:175-178
Timestamp: 2025-08-20T07:41:13.973Z
Learning: dockerode createImage() supports multiple signatures including createImage(auth, options) -> Promise<ReadableStream> where auth is the first parameter (AuthConfig object) and options is the second parameter. Both createImage(auth, options) and createImage(options, {authconfig: auth}) are valid approaches.
Applied to files:
packages/cli-v3/src/deploy/buildImage.ts
📚 Learning: 2025-11-10T09:09:07.399Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 2663
File: apps/webapp/app/env.server.ts:1205-1206
Timestamp: 2025-11-10T09:09:07.399Z
Learning: In the trigger.dev webapp, S2_ACCESS_TOKEN and S2_DEPLOYMENT_LOGS_BASIN_NAME environment variables must remain optional until an OSS version of S2 is available, to avoid breaking environments that don't have S2 provisioned.
Applied to files:
packages/cli-v3/src/deploy/buildImage.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Export tasks with unique IDs within the project to enable proper task discovery and execution
Applied to files:
.changeset/five-pens-fly.md
📚 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: Run `npx trigger.devlatest init` to initialize a Trigger.dev project
Applied to files:
.changeset/five-pens-fly.md
📚 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: Run `npx trigger.devlatest dev` to start the Trigger.dev development server
Applied to files:
.changeset/five-pens-fly.md
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `trigger.dev/sdk/v3` for all imports in Trigger.dev tasks
Applied to files:
.changeset/five-pens-fly.md
⏰ 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 (3, 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 (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- 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 (1, 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 - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
.changeset/five-pens-fly.md (1)
1-6: LGTM!The changeset correctly documents the feature addition with appropriate minor version bumps for the affected packages.
packages/cli-v3/src/deploy/buildImage.ts (3)
222-223: LGTM! Deterministic build timestamp.Adding
SOURCE_DATE_EPOCH=0correctly normalizes file timestamps for reproducible builds.Also applies to: 572-573
201-208: LGTM! Load parameter properly propagated.The
loadoption is correctly threaded through both remote and local build flows intogetOutputOptions.Also applies to: 534-541
1119-1163: LGTM! Output options correctly enhanced for deterministic builds.The changes properly add:
rewrite-timestamp=trueto normalize timestamps in all output- Conditional
load=truewhen requested- Proper parameter typing
These align with the PR's reproducible build objectives.
This PR makes our image builds deterministic and reproducible by ensuring that identical source code always produces the same image layers and image digest. This means that deployments where nothing has changed will no longer invalidate the image cache in our worker cluster nodes, thus avoid making the cold starts for runs worse.
Context
New deployments currently increase the cold start times for runs, as they generate a new image which needs to be pulled in the worker cluster where runs are executed. It happens also when the source code for the deployment has not changed due to non-deterministic steps in our build system. This addresses the latter issue by making builds reproducible.
Main changes
TRIGGER_DEPLOYMENT_IDandTRIGGER_DEPLOYMENT_VERSIONin the image, we now pass these via the supervisor instead.json-stable-stringifyfor consistent key ordering in the files we generate for the build, e.g.,package.json,build.json,index.json.metafile.jsonfrom the image contents as it is not actually used in the container. This is only relevant for theanalyzecommand.SOURCE_DATE_EPOCH=0andrewrite-timestamp=trueto Docker builds to normalize file timestamps.timingsandoutputHashesfrom build outputs and manifests.The builds are now reproducible for both native build server and Depot paths. This should also lead to better image layer cache reuse in general.