Repository navigation
Validate the devbox image pin and report image drift #12117
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 <snapshot-id> --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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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", | ||
|
There was a problem hiding this comment. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Compare image inputs as bytes.
Read files as Also applies to: 62-62 🤖 Prompt for AI Agents |
||
| 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<string, string | null>, | ||
| tree: ReadonlyMap<string, string | null>, | ||
| ): 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; | ||
| } | ||
|
There was a problem hiding this comment. HEAD fallback hides branch bakesMedium Severity
  Reviewed by Cursor Bugbot for commit d25b8f7. Configure here. |
||
|
|
||
| /** | ||
| * 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<string, Map<string, string[]>>(); | ||
| for (const entry of defaults) { | ||
| const kind = entry.kind ?? "base"; | ||
| if (!entry.repoCommit) continue; | ||
| const lineages = byKind.get(kind) ?? new Map<string, string[]>(); | ||
| 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.", | ||
| ); | ||
| } | ||
|
There was a problem hiding this comment. Same-commit mix evades pin checkMedium Severity
Reviewed by Cursor Bugbot for commit d25b8f7. Configure here. |
||
| 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" + | ||
|
There was a problem hiding this comment. 🔒 Security & Privacy | 🛡️ Analyzed with Security Review | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🤖 get_repo_knowledge executed:
Length of output: 47552 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- target script ---'
sed -n '90,125p' web/scripts/check-devbox-image-drift.ts
printf '%s\n' '--- relevant conventions references ---'
rg -n -i -C 3 'environment variable|env(ironment)? variable|user-facing|command output|error output' . --glob '!node_modules' --glob '!dist' --glob '!build' | head -200Repository: manaflow-ai/cmux Length of output: 22294 Information Disclosure (CWE-200): Exposure of Sensitive Information to an Unauthorized Actor Reachability: Internal · Exploitability: Theoretical Remove the deployment environment-variable name from command output. Line 112 exposes the internal 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| " 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(); | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.