diff --git a/.github/workflows/cloud-vm-image-contract.yml b/.github/workflows/cloud-vm-image-contract.yml index 928091bb65bb..994626c0087c 100644 --- a/.github/workflows/cloud-vm-image-contract.yml +++ b/.github/workflows/cloud-vm-image-contract.yml @@ -1,25 +1,10 @@ name: Cloud VM image contract on: + # No path filter on pull_request: a required check that never runs blocks a + # PR forever, so the job always reports and decides internally what to run. pull_request: branches: [main] - paths: - - web/package.json - - web/bun.lock - - web/scripts/devbox-image-common.ts - - web/scripts/derive-devbox-sizes.ts - - web/scripts/promote-devbox-image.ts - - web/scripts/validate-devbox-ladder.ts - - web/scripts/build-devbox-freestyle.ts - - web/scripts/verify-devbox-image.ts - - web/scripts/devbox-agent-launch.* - - web/tests/devbox-agent-launch-test.py - - web/scripts/check-devbox-agent-pins.ts - - web/services/vms/images/manifest.json - - web/services/vms/images/sizes.ts - - web/services/vms/images/devbox/** - - web/tests/vm-image-*.test.ts - - .github/workflows/cloud-vm-image-contract.yml push: branches: [main] paths: @@ -59,6 +44,9 @@ jobs: uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: persist-credentials: false + # The drift check reads the image inputs at each default entry's + # bake commit, which a shallow clone does not carry. + fetch-depth: 0 - name: Setup Bun uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2 @@ -71,5 +59,21 @@ jobs: - name: Validate the checked-in image ladder run: bun run devbox:manifest:check + # The manifest is a build artifact that a promotion rewrites twelve + # entries of at once, so two promotions in flight collide in one file. + # These are the states no bake can fix: a ladder assembled from two + # bakes (what hand-resolving that conflict produces), or a default baked + # from a commit that is not on main (a branch bake, whose lineage + # disappears with the branch). + - name: Validate the image pin + run: bun run devbox:drift:check --pin + - name: Test image contracts - run: bun test tests/vm-image-manifest.test.ts tests/vm-image-sizes.test.ts tests/vm-devbox-image.test.ts + run: bun test tests/vm-image-manifest.test.ts tests/vm-image-sizes.test.ts tests/vm-devbox-image.test.ts tests/vm-image-drift.test.ts + + # A merged edit to the image source ships nothing until a snapshot is + # baked and the manifest bump lands. This step is the only place that + # says so. It does not gate the merge (the workflow is not required): + # source and image are allowed to move apart, but never silently. + - name: Report devbox image drift + run: bun run devbox:drift:check diff --git a/skills/cmux-backend/references/devbox-image-deploys.md b/skills/cmux-backend/references/devbox-image-deploys.md new file mode 100644 index 000000000000..57673f6305e5 --- /dev/null +++ b/skills/cmux-backend/references/devbox-image-deploys.md @@ -0,0 +1,70 @@ +# Shipping a devbox image + +Two things ship on different clocks, and only one of them ships by merging. + +**Code ships on merge.** `web/services/vms/images/manifest.json` is the source of +truth for the image a machine boots. `resolver.ts` reads the `defaultForKind` +entry for the requested kind and size; no environment variable selects an image +(the `envVar` field is legacy). Merging a manifest bump to main deploys it with +the next Vercel deploy. + +**Image content ships on a bake.** Everything under +`web/services/vms/images/devbox/` plus `scripts/build-devbox-freestyle.ts` is +input to a Freestyle snapshot. Editing those files and merging changes nothing +on any machine: the manifest still points at the snapshot baked before the edit. + +## Baking and promoting + +From `web/`, with the deployment account's `FREESTYLE_API_KEY`: + +```bash +bun run devbox:promote freestyle --no-desktop # base ladder +bun run devbox:promote freestyle # desktop ladder +``` + +`promote-devbox-image.ts` runs a stale-checkout preflight (HEAD must equal +`origin/main`, or `CMUX_BAKE_ALLOW_BRANCH=1`), bakes, verifies by booting a real +VM, derives the size ladder, and writes the manifest entries. A failed verify +writes nothing. The output is a manifest diff: open it as a PR, and merging that +PR is the promotion. Twelve entries move at once (base and desktop, six sizes). + +## Drift + +`bun run devbox:drift:check` compares each default entry's `repoCommit` against +the image inputs in the working tree and names the files that changed since the +bake. The `Cloud VM image contract` workflow runs it on every PR and push that +touches image files. It is a report, not a gate: source and image are allowed to +move apart (a bake needs a provider credential and real VMs), but never +silently. When it fires, either bake and promote, or accept that the change is +queued for the next bake. + +## Two promotions at once + +A promotion rewrites twelve entries in one file, so two agents promoting in +parallel collide there. Never hand-resolve a `manifest.json` conflict: it is a +build artifact, and the merge you would write by hand has no bake behind it. +Reset the file to main and re-run the promotion against the new main, adopting +the snapshot you already baked: + +```bash +git checkout origin/main -- services/vms/images/manifest.json +bun run devbox:promote freestyle --image --no-desktop +bun run devbox:drift:check +``` + +If the drift check then fires, your snapshot predates image source that has +since landed, and it must be rebaked rather than promoted. The losing promotion +is always the one that rebakes. + +`--pin` covers the two states no bake can fix, and is the part worth making a +required check: a ladder assembled from two bakes (what hand-resolving that +conflict produces, valid JSON with sm and md on different images), and a default +whose `repoCommit` is not on main. The second is not hypothetical: bakes taken +from a feature branch with `CMUX_BAKE_ALLOW_BRANCH=1` record a commit that squash +merging never puts on main, and that disappears when the branch is deleted, so +the image's lineage becomes unverifiable. Bake defaults from main. + +A bake is not reproducible: agent CLIs, apt, and the ble.sh nightly all resolve +at bake time, so every promotion carries unrelated upgrades. The manifest entry +records `agentToolResolvedVersions`, and the promotion PR diff is where those +upgrades get reviewed. diff --git a/web/package.json b/web/package.json index b31177839b93..5130e18a4f41 100644 --- a/web/package.json +++ b/web/package.json @@ -27,6 +27,7 @@ "devbox:verify:private-link": "bun scripts/verify-devbox-private-link.ts", "devbox:promote": "bun scripts/promote-devbox-image.ts", "devbox:manifest:check": "bun scripts/validate-devbox-ladder.ts", + "devbox:drift:check": "bun scripts/check-devbox-image-drift.ts", "devbox:pins:check": "bun scripts/check-devbox-agent-pins.ts", "devbox:probe:busybox": "bun scripts/probe-busybox-cmux-tui.ts", "db:check": "bunx drizzle-kit check --config drizzle.config.ts", diff --git a/web/scripts/check-devbox-image-drift.ts b/web/scripts/check-devbox-image-drift.ts new file mode 100644 index 000000000000..5a4426bc2af9 --- /dev/null +++ b/web/scripts/check-devbox-image-drift.ts @@ -0,0 +1,224 @@ +#!/usr/bin/env bun +/** + * Reports whether the devbox image the manifest serves was baked from the + * devbox source that is checked in now. + * + * The manifest is the source of truth for the image users boot, and merging a + * manifest bump ships it. Editing the image SOURCE (`services/vms/images/devbox` + * or the builder script) ships nothing on its own: a snapshot has to be baked + * and promoted first. Without this check that gap is invisible, and a merged + * change to the shell config or the Dockerfile silently never reaches a machine. + * + * Each default manifest entry records the `repoCommit` it was baked from, so + * drift is the diff between the image inputs at that commit and the ones in the + * working tree. No manifest schema change and no provider credential needed. + * + * Two failures live here, and they are not the same severity: + * + * PIN the manifest itself is wrong. A default was baked from a commit that + * is not in main's history (a branch bake), or one kind's size ladder + * mixes bakes (the shape a hand-resolved manifest conflict takes). Both + * mean the deployed image is not the one main describes, so `--pin` + * is safe to make a required check. + * DRIFT main's image source moved past the pinned bake. Expected right after + * an image source PR merges, and only a bake clears it, so this reports + * and never gates. + * + * Usage: + * bun scripts/check-devbox-image-drift.ts # pin + drift, exit 1 on either + * bun scripts/check-devbox-image-drift.ts --pin # pin only + * bun scripts/check-devbox-image-drift.ts --warn # always exit 0 + */ +import { execFileSync } from "node:child_process"; +import { existsSync, readFileSync } from "node:fs"; +import path from "node:path"; +import { + DEVBOX_DESKTOP_FILES, + DEVBOX_TEMPLATE_FILES, + devboxDesktopDir, + devboxDir, + readImageManifest, + repoRoot, + webRoot, +} from "./devbox-image-common"; + +/** + * Every file whose content ends up in a baked image, as repo-relative paths. + * The builder script is included because it writes files into the image that + * the devbox directory does not carry (the ble.sh install, the cache bake). + */ +export function devboxImageInputPaths(): string[] { + const rel = (dir: string, name: string) => path.relative(repoRoot, path.join(dir, name)); + return [ + ...DEVBOX_TEMPLATE_FILES.map((name) => rel(devboxDir, name)), + ...DEVBOX_DESKTOP_FILES.map((name) => rel(devboxDesktopDir, name)), + path.relative(repoRoot, path.join(webRoot, "scripts/build-devbox-freestyle.ts")), + ].sort(); +} + +/** Reads one input from a commit; a file the commit predates reads as absent. */ +function readAtCommit(commit: string, relPath: string): string | null { + try { + return execFileSync("git", ["show", `${commit}:${relPath}`], { + cwd: repoRoot, + encoding: "utf8", + maxBuffer: 32 * 1024 * 1024, + }); + } catch { + return null; + } +} + +function readFromTree(relPath: string): string | null { + const full = path.join(repoRoot, relPath); + return existsSync(full) ? readFileSync(full, "utf8") : null; +} + +/** Input paths whose content differs between a baked commit and the tree. */ +export function driftedInputs( + baked: ReadonlyMap, + tree: ReadonlyMap, +): string[] { + const paths = new Set([...baked.keys(), ...tree.keys()]); + return [...paths].filter((p) => baked.get(p) !== tree.get(p)).sort(); +} + +/** + * The ref a bake must have landed on. A feature branch is usually behind main, + * so testing against HEAD alone would call every recent bake a branch bake. + * CI checks out the PR's merge commit, where HEAD already contains the base. + */ +function landedRef(): string { + for (const ref of [process.env.CMUX_DEVBOX_BASE_REF, "origin/main", "main"]) { + if (!ref) continue; + try { + execFileSync("git", ["rev-parse", "--verify", "--quiet", `${ref}^{commit}`], { cwd: repoRoot, stdio: "ignore" }); + return ref; + } catch { + continue; + } + } + return "HEAD"; +} + +/** True when `commit` is in the landed history (or in this checkout's HEAD). */ +function isLanded(commit: string, ref = landedRef()): boolean | null { + for (const target of [ref, "HEAD"]) { + try { + execFileSync("git", ["merge-base", "--is-ancestor", commit, target], { cwd: repoRoot, stdio: "ignore" }); + return true; + } catch { + continue; + } + } + try { + execFileSync("git", ["cat-file", "-e", `${commit}^{commit}`], { cwd: repoRoot, stdio: "ignore" }); + } catch { + // A commit this clone does not have is a shallow checkout, not a branch + // bake, and must not be reported as one. + return null; + } + return false; +} + +/** + * Manifest problems that no bake can fix, so they must not reach main. + * + * A promote PR writes twelve entries at once. Two agents promoting in parallel + * therefore collide in one file, and resolving that conflict by hand is how a + * ladder ends up half from each bake: valid JSON, one default per kind and + * size, and machines of different sizes running different images. + */ +export function pinProblems( + defaults: readonly { version: string; kind?: string; repoCommit?: string }[], + ancestry: (commit: string) => boolean | null, +): string[] { + const problems: string[] = []; + const byKind = new Map>(); + for (const entry of defaults) { + const kind = entry.kind ?? "base"; + if (!entry.repoCommit) continue; + const lineages = byKind.get(kind) ?? new Map(); + lineages.set(entry.repoCommit, [...(lineages.get(entry.repoCommit) ?? []), entry.version]); + byKind.set(kind, lineages); + } + for (const [kind, lineages] of byKind) { + if (lineages.size > 1) { + const shown = [...lineages] + .map(([commit, versions]) => `${commit.slice(0, 10)} (${versions.join(", ")})`) + .join(" and "); + problems.push( + `${kind}: the size ladder mixes bakes: ${shown}. ` + + "One bake feeds one ladder; re-run the promotion instead of merging two.", + ); + } + for (const commit of lineages.keys()) { + if (ancestry(commit) === false) { + problems.push( + `${kind}: default baked from ${commit.slice(0, 10)}, which is not in this history. ` + + "A branch bake (CMUX_BAKE_ALLOW_BRANCH=1) must not be promoted: rebake from main.", + ); + } + } + } + return problems; +} + +function main(): void { + const warnOnly = process.argv.includes("--warn"); + const manifest = readImageManifest(); + const defaults = manifest.images.filter((entry) => entry.defaultForKind); + if (defaults.length === 0) { + console.error("devbox image drift: the manifest has no default entry to check"); + process.exit(1); + } + const pin = pinProblems(defaults, (commit) => isLanded(commit)); + if (pin.length > 0) { + console.error(`devbox image pin is invalid:\n ${pin.join("\n ")}`); + if (!warnOnly) process.exit(1); + } else { + console.log(`devbox image pin ok: ${defaults.length} defaults, one bake per kind, all in this history`); + } + if (process.argv.includes("--pin")) return; + + const inputs = devboxImageInputPaths(); + const tree = new Map(inputs.map((p) => [p, readFromTree(p)] as const)); + + const bakedCommits = [...new Set(defaults.map((entry) => entry.repoCommit).filter((c): c is string => !!c))]; + const missing = defaults.filter((entry) => !entry.repoCommit); + if (missing.length > 0) { + console.warn( + `devbox image drift: ${missing.length} default entr${missing.length === 1 ? "y has" : "ies have"} no repoCommit; ` + + "rebake to record one (pre-2026-09 bakes did not).", + ); + } + + let drifted = false; + for (const commit of bakedCommits) { + const baked = new Map(inputs.map((p) => [p, readAtCommit(commit, p)] as const)); + if ([...baked.values()].every((value) => value === null)) { + console.warn(`devbox image drift: commit ${commit.slice(0, 10)} is not in this checkout; skipping (fetch depth?)`); + continue; + } + const changed = driftedInputs(baked, tree); + const versions = defaults.filter((entry) => entry.repoCommit === commit).map((entry) => entry.version); + if (changed.length === 0) { + console.log(`devbox image ok: ${versions.length} default(s) baked from ${commit.slice(0, 10)} match the tree`); + continue; + } + drifted = true; + console.error( + `devbox image drift: the default image(s) baked from ${commit.slice(0, 10)} predate ${changed.length} ` + + `image input change(s), so these edits are NOT on any machine:\n ${changed.join("\n ")}\n` + + ` defaults: ${versions.join(", ")}\n` + + " Bake and promote (from web/, with the deployment's FREESTYLE_API_KEY):\n" + + " bun run devbox:promote freestyle --no-desktop # base ladder\n" + + " bun run devbox:promote freestyle # desktop ladder\n" + + " then merge the manifest diff. Until then main's devbox source is ahead of production.", + ); + } + + if (drifted && !warnOnly) process.exit(1); +} + +if (import.meta.main) main(); diff --git a/web/tests/vm-image-drift.test.ts b/web/tests/vm-image-drift.test.ts new file mode 100644 index 000000000000..7966a6d6fe90 --- /dev/null +++ b/web/tests/vm-image-drift.test.ts @@ -0,0 +1,76 @@ +import { describe, expect, test } from "bun:test"; +import { devboxImageInputPaths, driftedInputs, pinProblems } from "../scripts/check-devbox-image-drift"; + +describe("devbox image drift", () => { + test("every file the image is built from is an input", () => { + const paths = devboxImageInputPaths(); + // The shell config is the case that motivated the check: it merged to main + // and reached no machine, because nothing rebaked the snapshot. + expect(paths).toContain("web/services/vms/images/devbox/cmux-bashrc"); + expect(paths).toContain("web/services/vms/images/devbox/Dockerfile"); + expect(paths).toContain("web/scripts/build-devbox-freestyle.ts"); + expect(paths).toContain("web/services/vms/images/devbox/desktop/cmux-desktop-boot"); + expect(new Set(paths).size).toBe(paths.length); + }); + + test("an input that changed since the bake is drift", () => { + const baked = new Map([["a", "one"], ["b", "two"]]); + const tree = new Map([["a", "one"], ["b", "two-changed"]]); + expect(driftedInputs(baked, tree)).toEqual(["b"]); + }); + + test("an input added or deleted since the bake is drift", () => { + expect(driftedInputs(new Map([["a", null]]), new Map([["a", "new file"]]))).toEqual(["a"]); + expect(driftedInputs(new Map([["a", "was there"]]), new Map([["a", null]]))).toEqual(["a"]); + }); + + test("an unchanged tree is not drift", () => { + const inputs = new Map([["a", "one"], ["b", "two"]]); + expect(driftedInputs(inputs, new Map(inputs))).toEqual([]); + }); +}); + +describe("devbox image pin", () => { + const landed = () => true; + + test("one bake feeds one size ladder", () => { + // The shape a hand-resolved manifest conflict takes: valid JSON, one + // default per size, and sm running a different image from md. + const problems = pinProblems( + [ + { version: "base-sm", kind: "base", repoCommit: "aaaaaaaaaa" }, + { version: "base-md", kind: "base", repoCommit: "bbbbbbbbbb" }, + ], + landed, + ); + expect(problems).toHaveLength(1); + expect(problems[0]).toContain("mixes bakes"); + }); + + test("kinds may sit on different bakes", () => { + // Base and desktop are promoted separately by design. + expect( + pinProblems( + [ + { version: "base-sm", kind: "base", repoCommit: "aaaaaaaaaa" }, + { version: "desktop-sm", kind: "desktop", repoCommit: "bbbbbbbbbb" }, + ], + landed, + ), + ).toEqual([]); + }); + + test("a default baked off main is rejected", () => { + const problems = pinProblems( + [{ version: "base-sm", kind: "base", repoCommit: "cccccccccc" }], + () => false, + ); + expect(problems).toHaveLength(1); + expect(problems[0]).toContain("not in this history"); + }); + + test("a commit this clone cannot see is not called a branch bake", () => { + // A shallow checkout must report nothing rather than a false accusation. + expect(pinProblems([{ version: "base-sm", kind: "base", repoCommit: "dddddddddd" }], () => null)).toEqual([]); + }); +});