Merge branch 'main' into posthog-proxy-app-analytics

Author: D-K-P

.server-changes/README.md

@@ -42,10 +42,16 @@ The body text (below the frontmatter) is a one-line description of the change. K
 
 These entries are public-facing - they ship verbatim in user-visible release notes. A few rules to keep them clean:
 
+- **Write for the user, not the reviewer.** Lead with what the user notices or has to do. If a reader who doesn't know the codebase can't tell what changed for them, rewrite it.
 - **One sentence is usually enough.** The body is the bullet in the changelog. If you need a paragraph, you're probably describing the implementation rather than the change.
 - **Describe behavior, not implementation.** Skip internal scopes, middleware names, library specifics, framework internals. Users care about what's different for them, not how it's wired.
 - **Never name internal tools or infra.** Observability stacks, internal services, infra components, monitoring backends, CI surfaces, AWS specifics - none of these belong in user-facing notes.
 
+Before / after:
+
+- ❌ _"The image verification step now parses the manifest's layer media types and returns a new result the finalizer rejects."_ (describes the wiring; a user can't act on it)
+- ✅ _"Deploying with an outdated CLI could produce an image that fails to start on every run. These deploys are now stopped before going live, with a message asking you to upgrade the CLI and re-deploy."_ (what the user sees and does)
+
 ## Lifecycle
 
 1. Engineer adds a `.server-changes/` file in their PR

.server-changes/reject-unpullable-deploy-image.md

@@ -0,0 +1,6 @@
+---
+area: webapp
+type: fix
+---
+
+Deploying with an outdated CLI could produce an image that fails to start on every run. These deploys are now stopped before going live, with a message asking you to upgrade the CLI and re-deploy.

AGENTS.md

@@ -104,6 +104,8 @@ pnpm run changeset:add
 
 When modifying only server components (`apps/webapp/`, `apps/supervisor/`, etc.) with no package changes, add a `.server-changes/` file instead. See `.server-changes/README.md` for format and documentation.
 
+**Write the description for users, not maintainers.** Both changesets and `.server-changes/` notes ship verbatim in user-visible release notes. Lead with what changed *for the user* - one plain sentence describing behavior, not implementation, and never naming internal tools or infra. The full writing guidance in `.server-changes/README.md` applies to changesets too.
+
 ## Dependency Pinning
 
 Zod is pinned to a single version across the entire monorepo (currently `3.25.76`). When adding zod to a new or existing package, use the **exact same version** as the rest of the repo - never a different version or a range. Mismatched zod versions cause runtime type incompatibilities (e.g., schemas from one package can't be used as body validators in another).

apps/webapp/app/v3/services/finalizeDeploymentV2.server.ts

@@ -186,6 +186,12 @@ export class FinalizeDeploymentV2Service extends BaseService {
       );
     }
 
+    if (result === "nonconformant") {
+      throw new ServiceValidationError(
+        "Deployment image is not runnable: it contains zstd-compressed layers inside a Docker (v2s2) manifest, which the container runtime cannot pull. This typically comes from an outdated CLI version. Please upgrade to the latest trigger.dev CLI and re-deploy."
+      );
+    }
+
     // Fail closed: if we can't confirm the image is present, don't promote a version
     // that might not start. Set DEPLOY_IMAGE_VERIFICATION_ENABLED=0 for out-of-band pushes.
     if (result === "unknown") {

apps/webapp/app/v3/services/verifyDeploymentImage.server.ts

@@ -5,6 +5,7 @@ import {
 } from "@aws-sdk/client-ecr";
 import { tryCatch } from "@trigger.dev/core";
 import pRetry, { AbortError } from "p-retry";
+import { z } from "zod";
 import { logger } from "~/services/logger.server";
 import {
   type AssumeRoleConfig,
@@ -16,7 +17,85 @@ import { type RegistryConfig } from "../registryConfig.server";
 
 const SHA256_DIGEST = /^sha256:[a-f0-9]{64}$/;
 
-export type ImageLookupResult = "found" | "missing" | "unknown";
+export type ImageLookupResult = "found" | "missing" | "unknown" | "nonconformant";
+
+// A zstd layer carried in a Docker (v2s2) manifest rather than an OCI manifest is
+// unpullable by cri-o/containerd/podman. OCI zstd (...tar+zstd) is fine - only this
+// Docker media type is rejected. An outdated CLI that predates OCI-media-type output
+// can emit it when reusing zstd layers from a prior build or the registry cache.
+const UNPULLABLE_LAYER_MEDIA_TYPE = "application/vnd.docker.image.rootfs.diff.tar.zstd";
+
+// Nested indexes are exotic (index -> per-platform image manifests is depth 1); cap the
+// walk so a pathological manifest can't fan out unbounded.
+const MAX_MANIFEST_DEPTH = 3;
+
+// Lenient: we only read what we need. An image manifest carries layers[]; a manifest list
+// / OCI index carries manifests[] (per-platform child pointers). Both optional so either
+// shape parses cleanly.
+const ManifestSchema = z.object({
+  layers: z.array(z.object({ mediaType: z.string().optional() })).optional(),
+  manifests: z.array(z.object({ digest: z.string() })).optional(),
+});
+
+// Inspect one raw manifest: does it directly carry an unpullable layer, and (if it's an
+// index) which child manifests should be walked. Fails open to empty on anything we can't
+// parse, so an unreadable manifest never blocks a deploy.
+export function inspectManifest(rawManifest: string | undefined): {
+  hasUnpullableLayer: boolean;
+  childDigests: string[];
+} {
+  const empty = { hasUnpullableLayer: false, childDigests: [] };
+
+  if (!rawManifest) {
+    return empty;
+  }
+
+  let json: unknown;
+  try {
+    json = JSON.parse(rawManifest);
+  } catch {
+    return empty;
+  }
+
+  const parsed = ManifestSchema.safeParse(json);
+  if (!parsed.success) {
+    return empty;
+  }
+
+  return {
+    hasUnpullableLayer:
+      parsed.data.layers?.some((layer) => layer.mediaType === UNPULLABLE_LAYER_MEDIA_TYPE) ?? false,
+    childDigests: parsed.data.manifests?.map((child) => child.digest) ?? [],
+  };
+}
+
+// Walk a manifest and, for a multi-arch index, its child manifests (fetched by digest).
+// Returns true if any layer anywhere in the tree uses the unpullable media type. A child
+// that can't be fetched (undefined) is treated as conformant - fail open.
+export async function treeHasUnpullableLayer(
+  rawManifest: string | undefined,
+  fetchManifest: (digest: string) => Promise<string | undefined>,
+  depth = 0
+): Promise<boolean> {
+  const { hasUnpullableLayer, childDigests } = inspectManifest(rawManifest);
+
+  if (hasUnpullableLayer) {
+    return true;
+  }
+
+  if (depth >= MAX_MANIFEST_DEPTH) {
+    return false;
+  }
+
+  for (const digest of childDigests) {
+    const childManifest = await fetchManifest(digest);
+    if (await treeHasUnpullableLayer(childManifest, fetchManifest, depth + 1)) {
+      return true;
+    }
+  }
+
+  return false;
+}
 
 /**
  * Split a stored ECR image reference into repository + tag.
@@ -86,8 +165,10 @@ const sendBatchGetImage: BatchGetImageSender = async ({
   imageIds,
 }) => {
   const ecr = await createEcrClient({ region, assumeRole });
-  // No acceptedMediaTypes: only the single-manifest types are valid enum values, and
-  // we only care whether the image exists, not its manifest format.
+  // Intentionally no acceptedMediaTypes: ECR returns the manifest as stored - which we
+  // rely on for the layer-media-type check - and omitting it avoids a multi-arch index
+  // being reported as a failure (i.e. misread as missing). BatchGetImage populates
+  // imageManifest by default when the image exists; the check fails open if it's absent.
   return ecr.send(new BatchGetImageCommand({ repositoryName, registryId, imageIds }));
 };
 
@@ -183,5 +264,46 @@ export async function ecrImageExists(
     return "unknown";
   }
 
-  return interpretBatchGetImageResponse(response);
+  const result = interpretBatchGetImageResponse(response);
+
+  if (result !== "found") {
+    return result;
+  }
+
+  // Image exists - now confirm the runtime can actually pull it. Follow index children by
+  // digest so multi-arch deploys are covered, not just single image manifests.
+  const fetchManifest = async (digest: string): Promise<string | undefined> => {
+    const [fetchError, childResponse] = await tryCatch(
+      _send({
+        region,
+        assumeRole,
+        registryId: accountId,
+        repositoryName: parsed.repositoryName,
+        imageIds: [{ imageDigest: digest }],
+      })
+    );
+
+    if (fetchError) {
+      logger.warn("Could not fetch child manifest for conformance check", {
+        imageReference,
+        digest,
+        error: fetchError.message,
+      });
+      return undefined; // fail open on this child
+    }
+
+    return childResponse.images?.[0]?.imageManifest;
+  };
+
+  const topManifest = response.images?.[0]?.imageManifest;
+
+  if (await treeHasUnpullableLayer(topManifest, fetchManifest)) {
+    logger.error("Deployment image has a runtime-incompatible layer media type", {
+      imageReference,
+      repositoryName: parsed.repositoryName,
+    });
+    return "nonconformant";
+  }
+
+  return "found";
 }

apps/webapp/test/verifyDeploymentImage.test.ts

@@ -2,14 +2,37 @@ import { RepositoryNotFoundException } from "@aws-sdk/client-ecr";
 import { describe, expect, it } from "vitest";
 import {
   ecrImageExists,
+  inspectManifest,
   interpretBatchGetImageResponse,
   parseEcrImageReference,
+  treeHasUnpullableLayer,
 } from "~/v3/services/verifyDeploymentImage.server";
 import { type RegistryConfig } from "~/v3/registryConfig.server";
 
 const ECR_HOST = "123456789012.dkr.ecr.us-east-1.amazonaws.com";
 const ecrConfig: RegistryConfig = { host: ECR_HOST, namespace: "deployments-test" };
 
+const DIGEST_A = `sha256:${"a".repeat(64)}`;
+const DIGEST_B = `sha256:${"b".repeat(64)}`;
+const ZSTD_DOCKER = "application/vnd.docker.image.rootfs.diff.tar.zstd";
+
+const imageManifest = (layerMediaTypes: string[]) =>
+  JSON.stringify({
+    schemaVersion: 2,
+    mediaType: "application/vnd.docker.distribution.manifest.v2+json",
+    layers: layerMediaTypes.map((mediaType) => ({ mediaType, digest: DIGEST_A })),
+  });
+
+const indexManifest = (childDigests: string[]) =>
+  JSON.stringify({
+    schemaVersion: 2,
+    mediaType: "application/vnd.oci.image.index.v1+json",
+    manifests: childDigests.map((digest) => ({
+      digest,
+      mediaType: "application/vnd.oci.image.manifest.v1+json",
+    })),
+  });
+
 describe("parseEcrImageReference", () => {
   it("splits repository and tag for a ref under the configured host", () => {
     const ref = `${ECR_HOST}/deployments-test/proj_abc:20240101.1.prod.a1b2c3d4`;
@@ -61,6 +84,73 @@ describe("interpretBatchGetImageResponse", () => {
   });
 });
 
+describe("inspectManifest", () => {
+  it("flags an unpullable zstd layer in an image manifest", () => {
+    const result = inspectManifest(
+      imageManifest(["application/vnd.docker.image.rootfs.diff.tar.gzip", ZSTD_DOCKER])
+    );
+    expect(result.hasUnpullableLayer).toBe(true);
+    expect(result.childDigests).toEqual([]);
+  });
+
+  it("passes OCI zstd and gzip layers (runtime-supported media types)", () => {
+    const result = inspectManifest(
+      imageManifest([
+        "application/vnd.oci.image.layer.v1.tar+gzip",
+        "application/vnd.oci.image.layer.v1.tar+zstd",
+      ])
+    );
+    expect(result.hasUnpullableLayer).toBe(false);
+  });
+
+  it("returns child digests for an index and does not flag it directly", () => {
+    const result = inspectManifest(indexManifest([DIGEST_A, DIGEST_B]));
+    expect(result.hasUnpullableLayer).toBe(false);
+    expect(result.childDigests).toEqual([DIGEST_A, DIGEST_B]);
+  });
+
+  it("fails open (empty) when the manifest is absent or unparseable", () => {
+    expect(inspectManifest(undefined)).toEqual({ hasUnpullableLayer: false, childDigests: [] });
+    expect(inspectManifest("not json")).toEqual({ hasUnpullableLayer: false, childDigests: [] });
+  });
+});
+
+describe("treeHasUnpullableLayer", () => {
+  const neverFetch = async () => undefined;
+
+  it("detects an unpullable layer in a flat image manifest", async () => {
+    expect(await treeHasUnpullableLayer(imageManifest([ZSTD_DOCKER]), neverFetch)).toBe(true);
+  });
+
+  it("follows an index and detects an unpullable layer in a child", async () => {
+    const children: Record<string, string> = {
+      [DIGEST_A]: imageManifest(["application/vnd.oci.image.layer.v1.tar+gzip"]),
+      [DIGEST_B]: imageManifest([ZSTD_DOCKER]),
+    };
+    const result = await treeHasUnpullableLayer(
+      indexManifest([DIGEST_A, DIGEST_B]),
+      async (digest) => children[digest]
+    );
+    expect(result).toBe(true);
+  });
+
+  it("returns false for an index whose children are all conformant", async () => {
+    const children: Record<string, string> = {
+      [DIGEST_A]: imageManifest(["application/vnd.oci.image.layer.v1.tar+zstd"]),
+      [DIGEST_B]: imageManifest(["application/vnd.docker.image.rootfs.diff.tar.gzip"]),
+    };
+    const result = await treeHasUnpullableLayer(
+      indexManifest([DIGEST_A, DIGEST_B]),
+      async (digest) => children[digest]
+    );
+    expect(result).toBe(false);
+  });
+
+  it("fails open when a child manifest can't be fetched", async () => {
+    expect(await treeHasUnpullableLayer(indexManifest([DIGEST_A]), neverFetch)).toBe(false);
+  });
+});
+
 describe("ecrImageExists", () => {
   it("returns unknown for a non-ECR registry without calling the registry", async () => {
     let called = false;
@@ -174,4 +264,54 @@ describe("ecrImageExists", () => {
     );
     expect(seen.imageIds).toEqual([{ imageTag: "v1.prod.a1b2c3d4" }]);
   });
+
+  // Resolve a manifest by tag or digest against a fixture map, mimicking BatchGetImage.
+  const sendFrom =
+    (byRef: Record<string, string>) =>
+    async (input: any): Promise<any> => {
+      const id = input.imageIds[0];
+      const manifest = byRef[id.imageDigest ?? id.imageTag];
+      return manifest ? { images: [{ imageManifest: manifest }] } : { images: [{}] };
+    };
+
+  it("returns nonconformant for a single-arch image with an unpullable zstd layer", async () => {
+    const result = await ecrImageExists(
+      {
+        imageReference: `${ECR_HOST}/deployments-test/proj_abc:v1.prod.a1b2c3d4`,
+        registryConfig: ecrConfig,
+      },
+      sendFrom({ "v1.prod.a1b2c3d4": imageManifest([ZSTD_DOCKER]) })
+    );
+    expect(result).toBe("nonconformant");
+  });
+
+  it("returns nonconformant when a multi-arch index has an unpullable child", async () => {
+    const result = await ecrImageExists(
+      {
+        imageReference: `${ECR_HOST}/deployments-test/proj_abc:v1.prod.a1b2c3d4`,
+        registryConfig: ecrConfig,
+      },
+      sendFrom({
+        "v1.prod.a1b2c3d4": indexManifest([DIGEST_A, DIGEST_B]),
+        [DIGEST_A]: imageManifest(["application/vnd.oci.image.layer.v1.tar+gzip"]),
+        [DIGEST_B]: imageManifest([ZSTD_DOCKER]),
+      })
+    );
+    expect(result).toBe("nonconformant");
+  });
+
+  it("returns found when a multi-arch index's children are all conformant", async () => {
+    const result = await ecrImageExists(
+      {
+        imageReference: `${ECR_HOST}/deployments-test/proj_abc:v1.prod.a1b2c3d4`,
+        registryConfig: ecrConfig,
+      },
+      sendFrom({
+        "v1.prod.a1b2c3d4": indexManifest([DIGEST_A, DIGEST_B]),
+        [DIGEST_A]: imageManifest(["application/vnd.oci.image.layer.v1.tar+zstd"]),
+        [DIGEST_B]: imageManifest(["application/vnd.docker.image.rootfs.diff.tar.gzip"]),
+      })
+    );
+    expect(result).toBe("found");
+  });
 });