From 5972a2c535fb49e628794c99e3bd1f8384a581f5 Mon Sep 17 00:00:00 2001 From: Joakim Persson Date: Mon, 7 Sep 2026 21:05:24 +0200 Subject: [PATCH] test+docs: assert the node major in smoke, and correct v1.8.13's agent-browser version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings from a delegated read-only audit of this repo, both verified from the filesystem before patching. 1. No test asserted the node major, so a node-24 bump would have passed the smoke suite SILENTLY. scripts/smoke-test.sh:94 was a bare `run "node" "node --version"` — exit-0 and non-empty output only, the printed version compared to nothing — while the line above it uses run_expect against $EXPECTED_PI_VERSION for pi. A reader skimming the suite would reasonably assume node regressions were covered. Worse, this is where the "node v22.23.2 verified" line in the v1.8.13 recreate notes came from: printed output, not an assertion. Now gated on EXPECTED_NODE_MAJOR, which CI derives from Dockerfile.base's ARG NODE_VERSION — the single source of truth (Dockerfile.base:557 is the ONLY hard pin in the repo; Dockerfile.variant has no node install at all). That also catches a stale cached layer whose node disagrees with the declared ARG. Unset => previous behaviour, so this is backward compatible. Verified two-sided rather than assumed: the sed derivation yields 22 (empty would have silently disabled the assertion, reintroducing the bug); grep -Fq "v22." matches v22.23.2; "v24." does NOT match, so a wrong major is caught; and "v2." does not prefix-collide. Workflow YAML re-parsed after editing (9 jobs). 2. The v1.8.13 entry claimed "the image's own 0.35.2" for agent-browser. The image ships 0.36.0: /usr/lib/node_modules/agent-browser/package.json says version 0.36.0, engines.node >=24.0.0, and no 0.35.2 exists anywhere in the image. The claim was also internally incoherent, contrasting 0.36.0 against a version that is not present. Corrected in place with a visible note, since the entry is already released. The reasoning survives untouched: the engines floor really is vestigial, because /usr/bin/agent-browser is a prebuilt aarch64 ELF invoked directly and never through node — which is why 0.36.0 runs fine on 22.23.2. --- .gitea/workflows/docker-publish.yml | 14 ++++++++++++-- CHANGELOG.md | 12 +++++++++--- scripts/smoke-test.sh | 14 +++++++++++++- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/.gitea/workflows/docker-publish.yml b/.gitea/workflows/docker-publish.yml index ff65090..986f729 100644 --- a/.gitea/workflows/docker-publish.yml +++ b/.gitea/workflows/docker-publish.yml @@ -543,7 +543,12 @@ jobs: env: EXPECTED_PI_VERSION: ${{ needs.resolve-versions.outputs.pi_version }} EXPECTED_MEMPALACE_VERSION: ${{ needs.resolve-versions.outputs.mempalace_version }} - run: bash scripts/smoke-test.sh pi-devbox:smoke + run: | + # Single source of truth for the node major is Dockerfile.base's ARG. + # Asserting the BUILT image matches it also catches a stale cached layer. + EXPECTED_NODE_MAJOR=$(sed -n 's/^ARG NODE_VERSION=\([0-9][0-9]*\).*/\1/p' Dockerfile.base) + export EXPECTED_NODE_MAJOR + bash scripts/smoke-test.sh pi-devbox:smoke # ── Phase 3b: amd64 smoke for the studio variant ──────────────────── # Additive + independent of the core `smoke` job: gates ONLY @@ -606,7 +611,12 @@ jobs: env: EXPECTED_PI_VERSION: ${{ needs.resolve-versions.outputs.pi_version }} EXPECTED_MEMPALACE_VERSION: ${{ needs.resolve-versions.outputs.mempalace_version }} - run: bash scripts/smoke-test.sh pi-devbox:smoke-studio + run: | + # Single source of truth for the node major is Dockerfile.base's ARG. + # Asserting the BUILT image matches it also catches a stale cached layer. + EXPECTED_NODE_MAJOR=$(sed -n 's/^ARG NODE_VERSION=\([0-9][0-9]*\).*/\1/p' Dockerfile.base) + export EXPECTED_NODE_MAJOR + bash scripts/smoke-test.sh pi-devbox:smoke-studio # ── Phase 4: multi-arch publish ───────────────────────────────────── build-variant: diff --git a/CHANGELOG.md b/CHANGELOG.md index 0af97aa..7dd88da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,9 +56,15 @@ identically to the 0.84.4 control, so "alive" could be distinguished from safe: all five prebuilt native addons in pi use NAPI (ABI-stable, no NODE_MODULE_VERSION lock, no binding.gyp), nothing in the image declares a node CEILING, and the install is one token (`setup_${NODE_VERSION}.x`). agent-browser -0.36.0 declares `engines.node >=24`, but that field is vestigial for the -artifact actually shipped: the image's own 0.35.2 declares the same floor and -runs fine on 22.23.2 as a prebuilt aarch64 ELF. The reason to wait is +0.36.0 declares `engines.node >=24.0.0`, but that field is vestigial for the +artifact actually shipped: `/usr/bin/agent-browser` is the prebuilt aarch64 ELF +`bin/agent-browser-linux-arm64`, invoked directly and never through node, so npm's +engines floor is never enforced at runtime — verified running under 22.23.2 in +this image. (Corrected 2026-09-07: this paragraph originally said "the image's own +0.35.2 declares the same floor". That was wrong and incoherent — it contrasted +0.36.0 against a 0.35.2 that does not exist in the image. There is exactly one +agent-browser present, `/usr/lib/node_modules/agent-browser` at 0.36.0. The +argument is unaffected; only the version was wrong.) The reason to wait is attribution, not compatibility — this release already moves pi a minor, mempalace a minor and bakes a Studio RC, so adding a node major would leave four suspects if the image misbehaves. Worth doing as its own release with the smoke diff --git a/scripts/smoke-test.sh b/scripts/smoke-test.sh index 5c91d61..4eb1516 100755 --- a/scripts/smoke-test.sh +++ b/scripts/smoke-test.sh @@ -5,6 +5,7 @@ # # Verifies: # - pi binary present and (if EXPECTED_PI_VERSION set) matches CI's resolved version +# - node MAJOR matches Dockerfile.base's ARG NODE_VERSION (if EXPECTED_NODE_MAJOR set) # - mempalace core matches the audited pin (if EXPECTED_MEMPALACE_VERSION set) # - new v1.0.0 base additions (pandoc, graphviz, imagemagick, yq, tealdeer) # - typst PDF engine for pandoc (v1.4.0) — `pandoc --pdf-engine=typst` @@ -91,7 +92,18 @@ if [ -n "${EXPECTED_PI_VERSION:-}" ]; then else run "pi" "pi --version" fi -run "node" "node --version" +# Until 2026-09-07 this was a bare `run "node" "node --version"`, which asserts +# only that the binary exists and exits 0 — the printed version was never +# compared to anything. A node major bump would therefore have passed this suite +# SILENTLY, while a reader skimming it would reasonably assume node regressions +# were covered. EXPECTED_NODE_MAJOR closes that: CI derives it from +# Dockerfile.base's ARG NODE_VERSION (the single source of truth), so this also +# catches a stale cached layer whose node does not match the declared ARG. +if [ -n "${EXPECTED_NODE_MAJOR:-}" ]; then + run_expect "node major matches Dockerfile ARG" "node --version" "v${EXPECTED_NODE_MAJOR}." +else + run "node" "node --version" +fi run "git" "git --version" run "aws" "aws --version" run "uv" "uv --version"