chore: trim inline comments to essential constraints

This commit is contained in:
Saadi Myftija
2026-08-21 11:37:38 +02:00
parent 03f3cf04d2
commit c1d03d6d5d
17 changed files with 36 additions and 123 deletions
@@ -12,11 +12,7 @@ const ParamsSchema = z.object({
deploymentId: z.string(),
});
// Returns the decrypted build-time env vars stored on a fromBundle deployment.
// Deliberately separate from the main GET deployment endpoint: this is secret
// material, and a dedicated route keeps access explicit and auditable. The vars
// are cleared when the deployment reaches a terminal status, so this only ever
// serves the active build window.
// Secret material, deliberately separate from the main GET deployment endpoint.
export async function loader({ request, params }: LoaderFunctionArgs) {
const parsedParams = ParamsSchema.safeParse(params);
@@ -25,8 +21,6 @@ export async function loader({ request, params }: LoaderFunctionArgs) {
}
try {
// Same auth as the sibling GET deployment route: env-key principals with
// read scope on deployments, no JWT.
const authResult = await authenticateApiKeyWithScope(request, {
action: "read",
resource: { type: "deployments" },
@@ -65,8 +59,7 @@ export async function loader({ request, params }: LoaderFunctionArgs) {
hasVars: deployment.buildEnvVars !== null,
});
// Terminal deployments have their vars cleared; even if a clear is still in
// flight, never serve secrets for a build that is no longer active.
// Never serve secrets for a build that is no longer active, even if a clear is still in flight
if (FINAL_DEPLOYMENT_STATUSES.includes(deployment.status)) {
return json({ variables: {} } satisfies GetDeploymentBuildEnvVarsResponseBody, {
status: 200,
@@ -79,10 +72,7 @@ export async function loader({ request, params }: LoaderFunctionArgs) {
});
}
// Vars exist but can't be read: fail LOUD. Returning an empty record here would
// be indistinguishable from "there were none" and let the build run without its
// build-time secrets (confusing failure at best, silently-wrong image at worst).
// Concrete trigger: ENCRYPTION_KEY rotation during the build window.
// Present-but-unreadable must fail loud: an empty record would let the build run without its secrets
const envelope = EncryptedSecretValueSchema.safeParse(deployment.buildEnvVars);
if (!envelope.success) {
@@ -60,7 +60,6 @@ export async function action({ request, params }: ActionFunctionArgs) {
.externalBuildData as InitializeDeploymentResponseBody["externalBuildData"],
eventStream: result.eventStream,
canceledDeployments: result.canceledDeployments,
// Only ack when we actually stored vars; older CLIs ignore this field.
...(result.buildEnvVarsStored ? { buildEnvVarsStored: true } : {}),
}
: { isPromoted: result.isPromoted }),
@@ -1236,11 +1236,8 @@ export function isCloud(): boolean {
return true;
}
// PR preview environments are cloud-style installs running against the
// cloud's staging services. Without this, anything gated on the billing
// client silently no-ops there (e.g. remote builds never get enqueued).
// Optional chaining: LOGIN_ORIGIN has a schema default, but test suites mock
// ~/env.server with partial objects and import this module transitively.
// Preview environments are cloud installs too; without this the billing client silently no-ops.
// Optional chaining because test suites mock the env module with partial objects.
if (env.LOGIN_ORIGIN?.endsWith(".triggerlabs.dev")) {
return true;
}
@@ -24,9 +24,7 @@ const objectStoreClient =
const artifactKeyPrefixByType = {
deployment_context: "deployments",
// Distinct prefix on purpose: the artifact key is the one signal that survives
// any schema skew, so the build server can recognize a bundle even if the
// fromBundle flag gets stripped somewhere along the enqueue chain.
// The key prefix is the one bundle signal that survives schema skew
deployment_bundle: "bundles",
} as const;
const artifactBytesSizeLimitByType = {
@@ -314,7 +314,6 @@ export class CreateDeploymentBackgroundWorkerServiceV4 extends BaseService {
name: error.name,
message: error.message,
},
// Build env vars only live for the active build window
buildEnvVars: Prisma.DbNull,
},
});
@@ -227,7 +227,6 @@ export class DeploymentService extends BaseService {
status: "CANCELED",
canceledAt: new Date(),
canceledReason: data?.canceledReason,
// Build env vars only live for the active build window
buildEnvVars: Prisma.DbNull,
},
}),
@@ -49,7 +49,6 @@ export class FailDeploymentService extends BaseService {
status: "FAILED",
failedAt: new Date(),
errorData: params.error,
// Build env vars only live for the active build window
buildEnvVars: Prisma.DbNull,
},
});
@@ -77,7 +77,6 @@ export class FinalizeDeploymentService extends BaseService {
deployedAt: new Date(),
// Only add the digest, if any
imageReference: imageDigest ? `${deployment.imageReference}@${imageDigest}` : undefined,
// Build env vars only live for the active build window
buildEnvVars: Prisma.DbNull,
},
});
@@ -30,8 +30,7 @@ import { errAsync } from "neverthrow";
const nanoid = customAlphabet("1234567890abcdefghijklmnopqrstuvwxyz", 8);
// Limits for fromBundle build env vars — they expand into --build-arg values, so
// keep them well under exec argv limits while staying generous for env vars.
// Build env vars expand into --build-arg values, so stay well under exec argv limits
const BUILD_ENV_VARS_MAX_BYTES = 128 * 1024;
const BUILD_ENV_VARS_MAX_KEYS = 200;
@@ -276,8 +275,6 @@ export class InitializeDeploymentService extends BaseService {
}
: undefined;
// Encrypt fromBundle build env vars for storage on the deployment row. Only
// meaningful for pre-bundled deploys; cleared on every terminal transition.
let encryptedBuildEnvVars: Awaited<ReturnType<typeof encryptSecret>> | undefined;
if (
@@ -46,7 +46,6 @@ export class TimeoutDeploymentService extends BaseService {
status: "TIMED_OUT",
failedAt: new Date(),
errorData: { message: errorMessage, name: "TimeoutError" },
// Build env vars only live for the active build window
buildEnvVars: Prisma.DbNull,
},
});
+1 -2
View File
@@ -75,8 +75,7 @@ export default defineConfig({
clientFiles: ["./app/entry.client.tsx", "./app/root.tsx", "./app/components/**/*.tsx"],
ssrFiles: ["./app/entry.server.tsx", "./app/root.tsx"],
},
// Local docker builds (and the dev build-server harness) reach the dev webapp as
// host.docker.internal — e.g. the in-build indexer fetching env vars.
// In-build calls from local docker (e.g. the indexer) reach the dev webapp via this host
allowedHosts: ["host.docker.internal"],
},
build: {
@@ -2271,9 +2271,8 @@ model WorkerDeployment {
externalBuildData Json?
buildServerMetadata Json?
/// Encrypted build-time env vars for pre-bundled (fromBundle) deploys an
/// EncryptedSecretValue envelope of a JSON record. Cleared when the deployment
/// reaches a terminal status; only ever exists for the active build window.
/// Encrypted build-time env vars for pre-bundled (fromBundle) deploys, as an
/// EncryptedSecretValue envelope. Cleared when the deployment reaches a terminal status.
buildEnvVars Json?
status WorkerDeploymentStatus @default(PENDING)
+1 -1
View File
@@ -690,7 +690,7 @@ export class CliApiClient {
);
}
// Best-effort cancel (204 on success, no body) — callers may ignore failures.
// 204 on success, no body
async cancelDeployment(deploymentId: string, reason?: string) {
if (!this.accessToken) {
throw new Error("cancelDeployment: No access token");
+16 -58
View File
@@ -105,10 +105,7 @@ type DeployCommandOptions = z.infer<typeof DeployCommandOptions>;
type Deployment = InitializeDeploymentResponseBody;
// Limits for the build-arg VALUES sent with --local-bundle deploys (they only exist in
// the in-memory build manifest — build.json is deliberately scrubbed because it gets
// COPY'd into the image). The server enforces the same limits authoritatively; this
// pre-check just fails fast with a friendly error before uploading anything.
// Pre-checks of the server-enforced limits, to fail before uploading anything
const BUILD_ENV_VARS_MAX_BYTES = 128 * 1024;
const BUILD_ENV_VARS_MAX_KEYS = 200;
@@ -363,9 +360,6 @@ async function _deployCommand(dir: string, options: DeployCommandOptions) {
}
if (options.fromBundle) {
// Builds the image from a pre-built bundle directory. The bundle carries no
// trigger.config.ts source, so this path skips config loading entirely and
// drives off the bundle's build.json + the deployment record.
await handleFromBundleDeploy({
bundleDir: options.fromBundle,
options,
@@ -660,8 +654,7 @@ async function _deployCommand(dir: string, options: DeployCommandOptions) {
});
}
// The shared "build the image and finalize the deployment" tail, used by the standard
// deploy path (after bundling) and by --from-bundle (building from a pre-built bundle).
// Shared tail of the standard deploy path (after bundling) and --from-bundle.
async function buildAndFinalizeDeployment({
apiClient,
projectId,
@@ -1300,18 +1293,12 @@ async function handleNativeBuildServerDeploy({
const archivePath = join(tmpDir, `deploy-${Date.now()}.tar.gz`);
// In --local-bundle mode, install + bundling happen locally (same as the classic
// non-native path) and only the resulting build context is uploaded; the build
// server then runs just the container build from it.
// --local-bundle: install + bundling happen locally; the server only runs the container build.
let bundleManifest: BuildManifest | undefined;
let bundleOutputPath: string | undefined;
// Build-arg values for --local-bundle, sent with the init request and stored
// encrypted on the deployment (they're scrubbed from build.json).
let bundleBuildEnvVars: Record<string, string> | undefined;
if (options.localBundle) {
// The container build runs on the build server with its own fixed settings —
// local build-tuning flags are not forwarded. Be honest about ignoring them.
const ignoredBuildFlags = [
options.compression !== "zstd" && "--compression",
options.cacheCompression !== "zstd" && "--cache-compression",
@@ -1370,11 +1357,7 @@ async function handleNativeBuildServerDeploy({
bundleManifest = buildManifest;
bundleOutputPath = destination.path;
// The build-arg values (scrubbed from build.json) travel via the deployment
// record (sent with the init request, stored encrypted server-side) — never
// as a file in the bundle. Despite the manifest type, extensions can set
// undefined values at runtime (e.g. env?.MISSING_VAR) — drop those, they'd
// be stripped by JSON serialization anyway. Pre-check the server's limits.
// Extensions can set undefined values at runtime despite the manifest type
bundleBuildEnvVars = Object.fromEntries(
Object.entries(buildManifest.build.env ?? {}).filter(
(entry): entry is [string, string] => typeof entry[1] === "string"
@@ -1401,10 +1384,7 @@ async function handleNativeBuildServerDeploy({
return;
}
// Sync env vars BEFORE initializing the deployment: initialization enqueues the
// remote build synchronously, so syncing afterwards would race a fast build —
// a run triggered right after promotion could execute without the synced vars.
// Syncing is environment-scoped and needs no deployment, so pre-init is safe.
// Sync BEFORE init: init enqueues the build synchronously, so a post-init sync races a fast build
if (!options.skipSyncEnvVars) {
const childVars = buildManifest.deploy.sync?.env ?? {};
const parentVars = buildManifest.deploy.sync?.parentEnv ?? {};
@@ -1467,11 +1447,8 @@ async function handleNativeBuildServerDeploy({
logger.debug("Artifact created", { artifactKey });
// Version-skew guard: an older server that does not know the deployment_bundle
// artifact type silently stores the upload as a plain source context, and the
// remote build would then try to install and bundle an already-bundled directory.
// The bundle-specific key prefix doubles as the ack that the server understood
// the type, independent of whether any build env vars are sent later.
// The bundle key prefix is the ack that the server understood the deployment_bundle
// type; an older server silently stores the upload as a plain source context.
if (options.localBundle && !artifactKey.startsWith("bundles/")) {
$deploymentSpinner.stop("Failed creating deployment artifact");
log.error(
@@ -1539,8 +1516,7 @@ async function handleNativeBuildServerDeploy({
userId,
gitMeta,
type: config.features.run_engine_v2 ? "MANAGED" : "V1",
// Deliberately config.runtime (not the resolved manifest runtime) so the persisted
// value is identical to classic native deploys.
// config.runtime (not the manifest runtime) to match classic native deploys
runtime: config.runtime,
isNativeBuild: true,
artifactKey,
@@ -1591,18 +1567,15 @@ async function handleNativeBuildServerDeploy({
return;
}
// Version-skew guard: an older server silently strips unknown fields, so if we sent
// build env vars and the server didn't ack storing them, the remote build would run
// without them and fail in a confusing way. Fail fast instead. Deliberately after
// the outcome=existing return: a reused deployment builds nothing, so no ack is due.
// No ack for sent build env vars means an older server stripped them; fail fast.
// After the outcome=existing return: a reused deployment builds nothing.
if (
options.localBundle &&
bundleBuildEnvVars &&
Object.keys(bundleBuildEnvVars).length > 0 &&
!deployment.buildEnvVarsStored
) {
// Courtesy cancel so the deployment doesn't linger as PENDING until the
// queue timeout reaps it. Best-effort: the hard error below is what matters.
// Best-effort cancel so the deployment does not linger until the queue timeout
const [cancelError] = await tryCatch(
apiClient.cancelDeployment(deployment.id, "Build environment variables were not stored")
);
@@ -1923,12 +1896,8 @@ export function verifyDirectory(dir: string, projectPath: string) {
}
}
// Builds and finalizes a deployment from a pre-built bundle directory (the output of
// the bundling step, as produced by --local-bundle / a dry-run build). Used primarily
// by the build server to run ONLY the container build for pre-bundled artifacts, but
// also works standalone for local testing. Skips config loading entirely — the bundle
// has no trigger.config.ts source; everything needed comes from the bundle's build.json
// and the deployment record (including the build-arg values, stored encrypted there).
// Runs only the container build from a pre-built bundle dir, skipping config loading
// entirely. Attach mode is the supported flow (build server); fresh-init is for testing.
async function handleFromBundleDeploy({
bundleDir,
options,
@@ -1975,8 +1944,7 @@ async function handleFromBundleDeploy({
const bundleManifest = manifestResult.data;
// Match the other deploy paths' promise: --dry-run never touches the server.
// Exit after the manifest is validated, before any branch/deployment calls.
// --dry-run must never touch the server
if (options.dryRun) {
logger.info(`Dry run complete. Validated bundle at ${bundlePath}`);
return;
@@ -1992,8 +1960,7 @@ async function handleFromBundleDeploy({
);
}
// In attach mode the branch env already exists (it was created by whatever
// initialized the deployment); a fresh-init preview deploy needs the upsert.
// In attach mode the branch env already exists
if (options.env === "preview" && branch && !existingDeploymentId) {
await upsertBranch({
accessToken: auth.accessToken,
@@ -2017,10 +1984,7 @@ async function handleFromBundleDeploy({
throw new Error("Failed to get project client");
}
// Recover the build-arg values scrubbed from build.json. In attach mode they were
// stored encrypted on the deployment by --local-bundle's init request; fetch them
// through the dedicated endpoint. An empty record is normal for builds that use no
// build-time env vars.
// In attach mode the build-arg values are stored encrypted on the deployment
let buildEnvVars: Record<string, string> | undefined;
if (existingDeploymentId) {
@@ -2035,16 +1999,10 @@ async function handleFromBundleDeploy({
buildEnvVars = buildEnvVarsResult.data.variables;
} else if (bundleManifest.build.env && Object.keys(bundleManifest.build.env).length > 0) {
// The scrubbed manifest can't carry values, but if a manifest somehow has them, use them.
buildEnvVars = bundleManifest.build.env;
}
if (!existingDeploymentId) {
// The supported flow is attach mode (the build server sets
// TRIGGER_EXISTING_DEPLOYMENT_ID). Fresh-init from a bundle is equivalent to a
// plain local build and mainly useful for local testing — warn so nobody relies
// on it against cloud by accident. There are no stored build env vars on this
// path; the build proceeds without them.
logger.warn(
"No existing deployment to attach to — initializing a fresh local-build deployment from the bundle. This path is intended for testing."
);
@@ -20,12 +20,10 @@ describe("createBundleArchive", () => {
});
it("archives bundle contents at the root, including dotfiles and nested dirs", async () => {
// Shape of a real buildWorker output dir
await writeFile(join(bundleDir, "build.json"), JSON.stringify({ contentHash: "abc" }));
await writeFile(join(bundleDir, "Containerfile"), "FROM scratch");
await writeFile(join(bundleDir, "package.json"), "{}");
await writeFile(join(bundleDir, "index.mjs"), "export {}");
// A build extension may produce a .dockerignore — it must survive archiving
await writeFile(join(bundleDir, ".dockerignore"), "*.log\n");
await mkdir(join(bundleDir, ".trigger", "skills", "my-skill"), { recursive: true });
await writeFile(join(bundleDir, ".trigger", "skills", "my-skill", "SKILL.md"), "# skill");
@@ -35,8 +33,6 @@ describe("createBundleArchive", () => {
const extractDir = join(outDir, "extracted");
await mkdir(extractDir);
// The build server extracts WITHOUT stripping path components — the contract
// is that bundle contents live at the archive root.
await tar.extract({ file: archivePath, cwd: extractDir });
const rootEntries = (await readdir(extractDir)).sort();
@@ -51,7 +47,6 @@ describe("createBundleArchive", () => {
].sort()
);
// Nested dot-dir contents survive
const skill = await readFile(
join(extractDir, ".trigger", "skills", "my-skill", "SKILL.md"),
"utf-8"
@@ -62,9 +57,7 @@ describe("createBundleArchive", () => {
it("excludes only .DS_Store — node_modules paths must survive", async () => {
await writeFile(join(bundleDir, "build.json"), "{}");
await writeFile(join(bundleDir, ".DS_Store"), "junk");
// The bundler emits controller entry points at paths mirroring the CLI's
// install location — under npx that contains a node_modules segment. Those
// files are load-bearing (the Containerfile's indexer stage runs them).
// Under npx the controller entry points live beneath a node_modules segment
const controllerDir = join(
bundleDir,
".npm",
@@ -76,7 +69,6 @@ describe("createBundleArchive", () => {
);
await mkdir(controllerDir, { recursive: true });
await writeFile(join(controllerDir, "managed-index-controller.mjs"), "x");
// dist-like names must NOT be excluded — the bundle IS build output
await mkdir(join(bundleDir, "dist"), { recursive: true });
await writeFile(join(bundleDir, "dist", "chunk.mjs"), "x");
+4 -11
View File
@@ -2,19 +2,12 @@ import { glob } from "tinyglobby";
import * as tar from "tar";
import { logger } from "../utilities/logger.js";
// The bundle dir is generated build output (bundled JS, synthesized package.json,
// build.json, Containerfile, .trigger/skills). Unlike the source-context archiver,
// it must NOT apply the usual build-output ignores (dist, build, .trigger) — those
// would strip the bundle itself. node_modules must NOT be excluded either: the
// bundler emits the controller entry points at paths mirroring the CLI's install
// location, which contains a node_modules segment when the CLI runs via npx.
// The bundle dir is generated build output, so the usual source ignores (dist,
// node_modules, ...) would strip load-bearing files: under npx the controller
// entry points live beneath a node_modules path segment.
const BUNDLE_IGNORES = ["**/.DS_Store"];
/**
* Archives a pre-built bundle directory (the buildWorker destination) so its
* contents land at the archive root the build server extracts without
* stripping path components.
*/
// Bundle contents land at the archive root; the build server extracts without stripping
export async function createBundleArchive(bundleDir: string, outputPath: string) {
logger.debug("Creating bundle archive", { bundleDir, outputPath });
+4 -8
View File
@@ -758,8 +758,7 @@ export const InitializeDeploymentResponseBody = z.object({
}),
})
.optional(),
// Ack that the server accepted and stored buildEnvVars from the request. The CLI
// treats its absence (older server) as a hard error when it sent non-empty vars.
// Ack that buildEnvVars were stored; absence on an older server is a client-side hard error
buildEnvVarsStored: z.boolean().optional(),
});
@@ -808,11 +807,9 @@ const InitializeDeploymentRequestBodyFull = InitializeDeploymentRequestBodyBase.
artifactKey: z.string().optional(),
configFilePath: z.string().optional(),
skipEnqueue: z.boolean().optional().default(false),
// The uploaded artifact is a pre-built bundle (local install + bundle already done);
// the build server should skip install/bundle and only run the container build.
// The artifact is a pre-built bundle; the build server only runs the container build
fromBundle: z.boolean().optional(),
// Build-time env var values for fromBundle deploys. Stored encrypted on the
// deployment and cleared once the deployment reaches a terminal status.
// Build-time env var values for fromBundle deploys, stored encrypted on the deployment
buildEnvVars: z.record(z.string()).optional(),
}).superRefine((data, ctx) => {
if (data.force && !data.externalId) {
@@ -943,8 +940,7 @@ export const GetDeploymentResponseBody = z.object({
export type GetDeploymentResponseBody = z.infer<typeof GetDeploymentResponseBody>;
// Response of the dedicated build-env-vars endpoint (secret material deliberately
// kept off GetDeploymentResponseBody). Empty record when none were stored.
// Secret material, deliberately kept off GetDeploymentResponseBody
export const GetDeploymentBuildEnvVarsResponseBody = z.object({
variables: z.record(z.string()),
});