From 42bd29d654bbf28af59c6ba7ae668ce78749ccfd Mon Sep 17 00:00:00 2001 From: Joakim Persson Date: Thu, 10 Sep 2026 23:39:40 +0200 Subject: [PATCH] =?UTF-8?q?fix(ci):=20unblock=20the=20release=20=E2=80=94?= =?UTF-8?q?=20npm=2011=20esbuild=20bloat=20+=20two=20self-inflicted=20asse?= =?UTF-8?q?rtions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v1.9.0 was tagged but never published: smoke failed 90-passed/3-failed and build-variant needs smoke, so nothing reached the registry. All three are fixed. 1. Size, 431 MB over threshold. Node 24 brings npm 11, which installs EVERY @esbuild/ optional binary instead of the matching one: 26 dirs, 284 MB per pi-coding-agent copy. Measured on pi-fork: npm 10.9.8 -> 165 MB (exactly what v1.8.14 shipped), npm 11.19.0 -> 449 MB. npm 11 ignores the os/cpu constraints AND --os/--cpu AND an npmrc carrying them, all measured, so Dockerfile.variant prunes explicitly, keeping linux-$(node -p process.arch) so one line is right on both arches. Pruned in the SAME layer as each install, or the bytes survive in the earlier layer. Three sites: global pi, pi-fork, pi-studio. Verified esbuild still transforms TS after. 2. om's node_modules assertion tested an npm artefact. om has zero runtime deps; its node_modules held ONE file (.package-lock.json) and 20 empty scope dirs. npm 11 stopped creating it. Now asserts the entry point pi actually loads, read from package.json -> pi.extensions. 3. The skill-source annotation added in v1.9.0 ("baked (package copy)") broke the assertion matching "baked$". Pattern now allows an optional suffix; which copy shipped stays authoritatively asserted against the manifest + tree hash. Also: a failed size check now prints the largest layers, largest directories and an @esbuild sentinel, so this class attributes itself next time instead of costing a CI dig plus a local npm bisect. Threshold stays 3800 MB: it caught a real regression and raising it would have thrown the signal away. No Dockerfile.base/rootfs change, so base-0fb1256c7f99 is reused and build-base is skipped. --- CHANGELOG.md | 81 +++++++++++++++++++++++++++++++++++++++++-- Dockerfile.variant | 33 ++++++++++++++++++ scripts/smoke-test.sh | 48 +++++++++++++++++++++++-- 3 files changed, 157 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c56fea0..b6abd44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,78 @@ Pre-v1.0.0 tags followed the pi npm version (`v{pi_version}[letter]`). --- -## Unreleased +## v1.9.1 — 2026-09-10 + +**v1.9.0 was tagged but never published: its own smoke gate stopped it, and it +was right to.** `build-base` succeeded, then `smoke` failed 90-passed/3-failed, +and because `build-variant` needs `smoke`, both variants, `promote-base-latest` +and `update-description` were skipped. No image reached the registry, so +`latest` still pointed at v1.8.14. v1.9.1 carries everything listed under v1.9.0 +below, plus the three fixes here. Two of the three failures were self-inflicted +by v1.9.0's own changes, and the third was a real regression that the Node bump +dragged in — which is the case for keeping the gate strict. + +**Failure 1 — the image was 431 MB over its size threshold, and npm 11 was the +cause.** Node 22 → 24 brings npm 10 → 11, and npm 11 installs **every** +`@esbuild/` optional binary rather than only the one matching the host: +26 platform directories covering aix-ppc64, android, darwin, freebsd, netbsd, +openbsd, win32, s390x, riscv64 and more, none of which this image can execute. +Measured on pi-fork's dependency tree, same repo and same command: + +| npm | packages | `node_modules` | +| --- | --- | --- | +| 10.9.8 | 136 | **165 MB** | +| 11.19.0 | 169 | **449 MB** | + +The 165 MB figure reproduces exactly what v1.8.14 shipped, which is what +identified npm rather than the image as the variable. esbuild declares those +binaries with `os`/`cpu` constraints, but npm 11 ignores them — and also ignores +`--os`/`--cpu` flags and an `.npmrc` carrying `os=`/`cpu=` (all three measured, +all three still produced 26 directories). So `Dockerfile.variant` now prunes +explicitly, keeping only `linux-$(node -p process.arch)` so one line is correct +on amd64 and arm64. Verified this removes dead weight and not function: after +pruning, `esbuild.transformSync` still compiles TypeScript. The prune runs in the +**same layer** as each `npm install` — deleting in a later `RUN` would leave the +bytes in the earlier layer and shrink the image by nothing. Three sites are +covered: the global pi install, pi-fork, and pi-studio (which pulls its own +pi-coding-agent copy), for roughly 548 MB recovered in the non-studio variant and +822 MB in studio. The threshold stays at 3800 MB deliberately: it caught a real +regression, and raising it to accommodate one would have discarded the signal. + +**Failure 2 — the om `node_modules` assertion was checking an npm artefact, not +the software.** `pi-observational-memory` declares **zero** runtime dependencies: +8 devDependencies (omitted by `--omit=dev`) and 4 peerDependencies, which pi +itself provides. npm 10 still materialised a `node_modules` for it, but that +directory contained exactly **one file** (`.package-lock.json`, 4 KB) and no +nested `package.json` — 20 empty scope directories. npm 11 stopped creating it, +so `test -d node_modules` went red while nothing about om had changed or broken. +The assertion now checks what must actually hold — that the entry point pi loads +exists — read out of the manifest pi itself reads (`package.json` → +`pi.extensions`) rather than a hardcoded path that could drift. pi-fork keeps its +`node_modules` check, because pi-fork has real dependencies where the directory's +absence would mean something. + +**Failure 3 — the skill-source annotation broke the assertion that reads it.** +v1.9.0 taught `pi-devbox-version` to say *which* pi-extensions copy shipped +(`baked (package copy)`, or a loud FALLBACK/MIXED marker). The smoke assertion +matched `^ $s +baked$`, anchored at the end, so the annotation failed it even +though the state reported was correct. The pattern now allows an optional +` (...)` suffix, matched loosely on purpose: *which* copy shipped is already +asserted authoritatively against the manifest field and its measured tree hash, +and re-encoding that wording in a second regex would just add a second place to +update. The lesson recorded rather than the fix alone: the display branches were +tested in an isolated harness that passed, but the assertion **consuming** them +was never run — harness-passes-therefore-consumer-passes was an assumption. + +**A red size assertion now carries its own diagnostic.** Attributing the 431 MB +took a full CI-log dig plus a local npm bisect, while the container knew where +its bytes were the whole time. On failure the check now prints the largest +layers, the largest directories, and a count of `@esbuild` platform directories +as a sentinel for this exact regression recurring — the same principle the `run()` +helper already applies to every other assertion. + +No `Dockerfile.base` or `rootfs/` change, so the base fingerprint is untouched +and `base-decide` reuses `base-0fb1256c7f99` built during the v1.9.0 attempt. **A gate for documentation drift, because five claims rotted in one release and one of them was published.** Preparing v1.9.0 turned up a cluster of stale @@ -94,7 +165,13 @@ is stale (a live client shows 45) but left alone rather than corrected on a guess, since that count cannot be attributed to the baked 3.9.0 server without measuring it. -## v1.9.0 — 2026-09-10 +## v1.9.0 — 2026-09-10 (tagged, never published — superseded by v1.9.1) + +> This tag exists in git but no image was ever pushed for it: `smoke` failed +> three assertions and skipped every downstream job. Everything below ships in +> **v1.9.1**, whose entry explains the three failures and their fixes. Kept as +> its own section rather than folded away, because the tag is real and someone +> will eventually find it and wonder why Docker Hub has no v1.9.0. **`shellcheck` is now in the image, because the release gate it depends on could not be run by anyone.** v1.8.14 made shell lint a release gate: `scripts/lint-shell.sh` diff --git a/Dockerfile.variant b/Dockerfile.variant index 74ae4ed..89d5b8b 100644 --- a/Dockerfile.variant +++ b/Dockerfile.variant @@ -196,6 +196,25 @@ RUN set -e && \ done; \ return 1; \ } && \ + # prune_foreign_esbuild: npm 11 (shipped with Node 24) installs EVERY + # @esbuild/ optional binary instead of only the one matching the + # host — 26 platform dirs, 284 MB, for aix-ppc64/android/darwin/freebsd/ + # netbsd/openbsd/win32/s390x/riscv64/... that this image can never execute. + # Measured on pi-fork's tree: npm 10.9.8 -> 165 MB, npm 11.19.0 -> 449 MB, + # and the 165 MB figure reproduces what v1.9.0's predecessor actually shipped. + # esbuild declares those with os/cpu constraints, but npm 11 ignores them and + # ALSO ignores --os/--cpu and an npmrc carrying os=/cpu= (all three measured). + # So prune explicitly, keeping only linux-$(node -p process.arch) so the same + # line is correct on amd64 and arm64. Verified after pruning that esbuild still + # works (transformSync compiles TS), i.e. this removes dead weight, not function. + # MUST run in the SAME layer as the npm installs above: deleting in a later RUN + # leaves the bytes in this layer and shrinks the image by nothing. + prune_foreign_esbuild() { \ + keep="linux-$(node -p process.arch)"; \ + find /usr/lib/node_modules /opt -type d -regex '.*/@esbuild/[^/]+' \ + ! -name "$keep" -prune -exec rm -rf {} + ; \ + echo "esbuild platform dirs kept: $(find /usr/lib/node_modules /opt -type d -regex '.*/@esbuild/[^/]+' -printf '%f\\n' 2>/dev/null | sort -u | tr '\\n' ' ')"; \ + } && \ if [ "${PI_VERSION}" = "latest" ]; then \ NPM_CONFIG_PREFIX=/usr npm install -g @earendil-works/pi-coding-agent ; \ else \ @@ -209,6 +228,7 @@ RUN set -e && \ git_fetch_ref "${PI_ATELIER_REPO}" "${PI_ATELIER_REF}" /opt/pi-atelier && \ (cd /opt/pi-fork && npm install --omit=dev --no-audit --no-fund) && \ (cd /opt/pi-observational-memory && npm install --omit=dev --no-audit --no-fund) && \ + prune_foreign_esbuild && \ echo "pi-toolkit at $(cd /opt/pi-toolkit && git rev-parse --short HEAD)" && \ echo "pi-extensions at $(cd /opt/pi-extensions && git rev-parse --short HEAD)" && \ echo "pi-fork at $(cd /opt/pi-fork && git rev-parse --short HEAD)" && \ @@ -309,6 +329,18 @@ ARG PI_STUDIO_REF=main ARG PI_STUDIO_VERSION=none RUN if [ "${INSTALL_STUDIO}" = "true" ]; then \ set -e; \ + # Same esbuild prune as the main install RUN — see the comment there. It has + # to be redefined because shell functions do not survive across layers, and + # it has to run in THIS layer because pi-studio's npm install happens here: + # deleting in a later RUN would leave the bytes in this layer and shrink + # nothing. pi-studio pulls its own pi-coding-agent copy, so it is a third + # ~274 MB site on top of the two in the non-studio variant. + prune_foreign_esbuild() { \ + keep="linux-$(node -p process.arch)"; \ + find /usr/lib/node_modules /opt -type d -regex '.*/@esbuild/[^/]+' \ + ! -name "$keep" -prune -exec rm -rf {} + ; \ + echo "esbuild platform dirs kept: $(find /usr/lib/node_modules /opt -type d -regex '.*/@esbuild/[^/]+' -printf '%f\\n' 2>/dev/null | sort -u | tr '\\n' ' ')"; \ + }; \ rm -rf /opt/pi-studio && mkdir -p /opt/pi-studio && \ git -C /opt/pi-studio init -q && \ git -C /opt/pi-studio remote add origin "${PI_STUDIO_REPO}" && \ @@ -320,6 +352,7 @@ RUN if [ "${INSTALL_STUDIO}" = "true" ]; then \ done; \ [ "$ok" = "1" ] && \ (cd /opt/pi-studio && npm install --omit=dev --no-audit --no-fund) && \ + prune_foreign_esbuild && \ echo "pi-studio at $(cd /opt/pi-studio && git rev-parse --short HEAD)"; \ fi diff --git a/scripts/smoke-test.sh b/scripts/smoke-test.sh index fbb7816..5f76de8 100755 --- a/scripts/smoke-test.sh +++ b/scripts/smoke-test.sh @@ -315,8 +315,24 @@ run "pi-toolkit clone" "test -d /opt/pi-toolkit && git -C /opt/pi-toolkit rev run "pi-extensions clone" "test -d /opt/pi-extensions && git -C /opt/pi-extensions rev-parse --short HEAD" run "pi-fork clone + node_modules" \ "test -f /opt/pi-fork/package.json && test -d /opt/pi-fork/node_modules" -run "pi-observational-memory clone + node_modules" \ - "test -f /opt/pi-observational-memory/package.json && test -d /opt/pi-observational-memory/node_modules" +# om is checked differently from pi-fork ON PURPOSE. It declares ZERO runtime +# dependencies: 8 devDependencies (omitted by --omit=dev) and 4 peerDependencies, +# which pi itself provides. npm 10 still materialised a node_modules for it, but +# that directory held exactly ONE file (.package-lock.json, 4 KB) and no nested +# package.json at all — 20 empty scope dirs. npm 11 stopped creating it, so the +# old `test -d node_modules` assertion went red on v1.9.0 while nothing about om +# had changed or broken. It was asserting an npm artefact, not a property of the +# shipped software. What actually has to hold is that the entry point pi loads +# exists, so assert THAT, straight out of the manifest pi reads +# (package.json -> pi.extensions), rather than a hardcoded path that could drift. +run "pi-observational-memory clone + declared pi entry point" \ + "test -f /opt/pi-observational-memory/package.json && \ + node -e 'const p=require(\"/opt/pi-observational-memory/package.json\"),f=require(\"fs\"),h=require(\"path\"); \ + const l=(p.pi&&p.pi.extensions)||[]; \ + if(!l.length){console.error(\"package.json declares no pi.extensions\");process.exit(1)} \ + for(const e of l){const t=h.resolve(\"/opt/pi-observational-memory\",e); \ + if(!f.existsSync(t)){console.error(\"declared entry missing: \"+t);process.exit(1)}} \ + console.log(\"entries ok: \"+l.join(\",\"))'" # ...and that the clone carries the AUTH FIX, not merely that it exists. om's # pre-flight hasUsableAuth() check silently disabled `recall` for ~8 weeks once # pi moved to request-time SigV4 signing and stopped exposing a static Bedrock @@ -697,9 +713,17 @@ exec_test "pi-devbox-version reports skill sources (all baked, no skillset here) 'out=$(pi-devbox-version) echo "$out" | grep -q "skills:" || { echo "no skills section" >&2; exit 1; } for s in mempalace pi-extensions pi-devbox-environment credential-incident-response; do - echo "$out" | grep -qE "^ $s +baked$" \ + echo "$out" | grep -qE "^ $s +baked( \([^)]*\))?$" \ || { echo "$s not reported as baked" >&2; exit 1; } done; echo ok' +# The optional " (...)" above is what pi-extensions now appends to say WHICH copy +# shipped — "baked (package copy)", or a loud FALLBACK/MIXED annotation. Without +# allowing it, adding that annotation turned this assertion red on v1.9.0 even +# though the state it reported was the correct one. The suffix is deliberately +# matched loosely rather than pinned to "(package copy)", because WHICH copy +# shipped is already asserted authoritatively above, against the manifest field +# and its measured tree hash, and duplicating that here in a regex would just +# create a second place to update whenever the wording changes. # The boot banner must NOT carry the section: entrypoint-user.sh prints the # version FIRST, before the baked links exist and long before the skillset # deploy + reconcile run last, so anything it said about skill sources would be @@ -882,6 +906,24 @@ elif [ "$SIZE_MB" -le "$SIZE_THRESHOLD_MB" ]; then printf " ✅ size: %d MB (threshold %d MB)\n" "$SIZE_MB" "$SIZE_THRESHOLD_MB"; PASS=$((PASS+1)) else printf " ❌ size: %d MB exceeds threshold %d MB\n" "$SIZE_MB" "$SIZE_THRESHOLD_MB"; FAIL=$((FAIL+1)) + # A bare "too big" verdict cost a full CI-log dig plus a local npm bisect to + # attribute the v1.9.0 overshoot (+431 MB, which turned out to be npm 11 + # installing 26 @esbuild platform binaries per pi-coding-agent copy). The + # container already knows where its bytes are, so make it say so: the biggest + # layers, and the biggest directories under the paths that historically grow. + # Same principle as the run() helper above — a red assertion should carry its + # own diagnostic rather than send the next reader spelunking. + echo " ── largest layers (docker history) ──" + docker history --format '{{.Size}}\t{{.CreatedBy}}' "$IMAGE" 2>/dev/null \ + | grep -vE '^0B' | head -12 | sed 's/^/ /' | cut -c1-160 + echo " ── largest directories in the image ──" + docker run --rm --entrypoint sh "$IMAGE" -c \ + 'du -sm /usr/lib/node_modules/* /opt/* /usr/local/share/ms-playwright 2>/dev/null | sort -rn | head -12' \ + 2>/dev/null | sed 's/^/ /' || echo " (could not inspect directories)" + echo " ── @esbuild platform dirs (npm 11 regression sentinel) ──" + docker run --rm --entrypoint sh "$IMAGE" -c \ + 'find /usr/lib/node_modules /opt -type d -regex ".*/@esbuild/[^/]+" -printf "%f\n" 2>/dev/null | sort | uniq -c | sort -rn | head' \ + 2>/dev/null | sed 's/^/ /' || true fi # ── Summary ───────────────────────────────────────────────────────────