From 852f900b53426b77a07031a5e93f8bfc550c11b4 Mon Sep 17 00:00:00 2001 From: Joakim Persson Date: Fri, 11 Sep 2026 11:06:18 +0200 Subject: [PATCH] =?UTF-8?q?test(smoke):=20make=20CI=20prove=20the=20native?= =?UTF-8?q?s=20still=20work=20=E2=80=94=20the=20runbook=20check=20didn't?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The post-boot check v1.9.1 left for the next machine was node -e 'require("esbuild").transformSync("const x:number=1",{loader:"ts"})' with "if this fails, the prune removed something needed on arm64 -> revert to v1.8.14 and fix" attached. Run from /workspace on the freshly recreated v1.9.1 image it fails with MODULE_NOT_FOUND — on a perfectly healthy image. require() resolves by walking up from the CURRENT DIRECTORY, esbuild is nested inside the two pi trees, and a global install is not on node's require path (NODE_PATH is unset). The version that "verified the prune" last night only passed because the shell happened to sit inside the tree. So the check was CWD-dependent, its red meant nothing, and the remedy it prescribed was reverting a good release. Path-qualified, both sites compile TS today. Rather than leave a fragile command in a note for a human to paste, CI now owns it: - esbuild must transformSync at EVERY install site found in the image - @mariozechner/clipboard must load with its native binding attached at every site — for the clipboard prune that IS the proof, since napi-rs resolves the platform package at require() time Sites are discovered with find, so the studio variant's third site is covered without naming it. Verified as a four-way matrix, not just a green run: both assertions pass against the real image FROM /workspace (the cwd that broke the old command), and both fail (exit 1, "The package \"@esbuild/linux-arm64\" could not be found, and is needed by esbuild") against copies of the same packages with the host platform binary removed. An assertion never observed failing is not evidence. Also documents both failure shapes in AGENTS.md next to the size gate: this one, and the mode-700 /root one where `test ! -d` passes on a permission error. --- AGENTS.md | 24 +++++++++++++++++------- CHANGELOG.md | 18 ++++++++++++++++++ scripts/smoke-test.sh | 22 ++++++++++++++++++++++ 3 files changed, 57 insertions(+), 7 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 8d83632..5417b5a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -294,13 +294,23 @@ upstream bumps. **The size gate is not a substitute for naming the residue.** It carries ~225 MB of deliberate margin, so v1.9.1 shipped +131 MB of pure build residue — 110 MB of it npm's own download cache under `/root/.npm`, the -rest foreign platform packages — and stayed green. Two named assertions -now cover exactly that: no foreign npm-11 platform packages beyond the -host arch (`@esbuild/*`, `@mariozechner/clipboard-*`), and no -`/root/.npm` in the image. Note the second one refuses to run as -non-root: `test ! -d /root/.npm` on mode-700 `/root` would otherwise -pass for the wrong reason, which is the failure shape to watch for in -any assertion about a path you may not be able to read. +rest foreign platform packages — and stayed green. Four named assertions +now cover that ground: no foreign npm-11 platform packages beyond the +host arch (`@esbuild/*`, `@mariozechner/clipboard-*`), no `/root/.npm` in +the image, and — because the prune's real risk is *removing something +needed*, not size — esbuild must compile TS and clipboard must load its +native binding at **every** install site. + +Two failure shapes to copy from those, both of which bit here: +- `test ! -d /root/.npm` on mode-700 `/root` passes for a **permission** + error, so the cache assertion refuses to run as non-root. Watch for + this in any assertion about a path you may not be allowed to read. +- `node -e 'require("esbuild")'` resolves by walking up from the CURRENT + DIRECTORY, so it fails with `MODULE_NOT_FOUND` from `/workspace` on a + perfectly healthy image (esbuild is nested inside the pi trees; + `NODE_PATH` is unset). Always path-qualify: `require("/esbuild")`. + A runbook shipped the bare form with "if this fails, revert the + release" attached, and it duly went red for the wrong reason. ## Build pipeline notes diff --git a/CHANGELOG.md b/CHANGELOG.md index f3ed983..82f2629 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,24 @@ RUN at the end of `Dockerfile.variant` calls `pi --version` again, so deleting it earlier only relocates those bytes into that layer — today's manifest layer is 128 kB precisely because it finds the cache warm. +**Two functional assertions as well, after the runbook command left for the +next machine failed for the wrong reason.** v1.9.1's open item prescribed +`node -e 'require("esbuild").transformSync(...)'` as the post-boot check, with +"if this fails, the prune removed something needed → revert to v1.8.14". Run +from `/workspace` it fails with `MODULE_NOT_FOUND` on a perfectly good image: +`require` resolves by walking up from the current directory, esbuild lives +nested inside the two pi trees, and global installs are not on node's require +path (`NODE_PATH` is unset). The check that verified the prune last time only +passed because the shell happened to be inside the tree. Smoke now does it +properly and CI owns it: for every install site found in the image (so the +studio variant's third site is covered automatically), esbuild must compile TS +and `@mariozechner/clipboard` must load with its native binding attached — the +latter is the real proof for the clipboard prune, since napi-rs resolves the +platform package at `require()` time. Both were verified as a four-way matrix: +green on the real image *from `/workspace`*, and red against copies of the same +packages with the host platform binary removed (`The package +"@esbuild/linux-arm64" could not be found`). + Touches `Dockerfile.variant` and `scripts/smoke-test.sh` only: `Dockerfile.base` is unchanged, so this needs no base rebuild and should **ride the next release** rather than burn a cycle of its own. diff --git a/scripts/smoke-test.sh b/scripts/smoke-test.sh index 3298322..4813c84 100755 --- a/scripts/smoke-test.sh +++ b/scripts/smoke-test.sh @@ -30,6 +30,7 @@ # client bundle present + registered via `pi install` # - no foreign npm-11 platform packages (@esbuild, clipboard) beyond the host # - no build-time npm cache (/root/.npm) shipped in the image +# - esbuild compiles + clipboard native loads at every install site # - image size within threshold set -euo pipefail @@ -909,6 +910,27 @@ run "no foreign platform packages (npm 11 sentinel)" \ run "no build-time npm cache shipped (/root/.npm)" \ 'test "$(id -u)" = "0" || { echo "assertion needs root to read /root" >&2; exit 1; }; if [ -e /root/.npm ]; then echo "/root/.npm shipped: $(du -sm /root/.npm | cut -f1) MB" >&2; exit 1; fi; echo ok' +# The prune's risk is not "too big" but "removed something needed", and only a +# FUNCTIONAL check covers that. These load the natives from every install site +# found in the image, so they also scale to the studio variant's third site. +# +# NOTE THE PATH-QUALIFIED require(). The obvious form, `node -e +# 'require("esbuild")...'`, resolves by walking up from the CURRENT DIRECTORY — +# so it fails with MODULE_NOT_FOUND from /workspace on a perfectly good image, +# because esbuild lives nested inside the pi trees and global installs are not +# on node's require path (NODE_PATH is unset). That exact command was left in a +# runbook as "if this fails, revert the release", and it duly failed for the +# wrong reason on the first machine that ran it. A check must fail only for the +# thing it is checking. +run "esbuild works at every install site (prune removed weight, not function)" \ + 'sites=$(find /usr/lib/node_modules /opt -type d -path "*/node_modules/esbuild" -prune 2>/dev/null); if [ -z "$sites" ]; then echo "no esbuild install found at all" >&2; exit 1; fi; for d in $sites; do node -e "require(\"$d\").transformSync(\"const x:number=1\",{loader:\"ts\"})" || { echo "esbuild broken at $d" >&2; exit 1; }; done; echo ok' + +# Clipboard is the family pruned second, and its napi-rs loader picks its native +# binding at require() time — so a successful load IS the proof that the kept +# platform package is the one this image needs. +run "clipboard native loads at every install site" \ + 'sites=$(find /usr/lib/node_modules /opt -type d -path "*/node_modules/@mariozechner/clipboard" -prune 2>/dev/null); if [ -z "$sites" ]; then echo "no @mariozechner/clipboard install found at all" >&2; exit 1; fi; for d in $sites; do node -e "var c=require(\"$d\"); if (typeof c.setText !== \"function\") { throw new Error(\"native binding missing\"); }" || { echo "clipboard native broken at $d" >&2; exit 1; }; done; echo ok' + # ── Image size ──────────────────────────────────────────────────────── echo "" echo "── Image size ──"