diff --git a/CHANGELOG.md b/CHANGELOG.md index b1c5fe5..bcbcda8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,77 @@ Pre-v1.0.0 tags followed the pi npm version (`v{pi_version}[letter]`). --- +## Unreleased + +### Fixed + +- **Four build-provenance smoke assertions verified the presence of a manifest + field name and never looked at its value.** The originals were literally: + + ```sh + run_expect "manifest records pi_version" "cat …build-manifest.json" '"pi_version"' + ``` + + which passes on `{"pi_version": ""}` and on `{"pi_version": null}`. The tell + was sitting in the passing output all along — `✅ manifest records pi_version + (got "pi_version")` echoes the *key* back as the thing it claims to have + found — and it was spotted while reading run 579's smoke log to confirm the + new v1.8.6 assertions had actually executed. + + Replaced with checks against the values, and against ground truth where + ground truth exists: + + | Assertion | What it now enforces | + |---|---| + | `manifest declares every required component key` | all seven components present by name, failing with *which* key vanished | + | `manifest component values are resolved 40-hex commits` | each value is a full 40-hex SHA; `null` allowed for `pi-studio` alone (absent in the non-studio variant) | + | `manifest pi_version matches the installed pi` | manifest value equals `pi --version`, same ground-truth shape as the mempalace check | + | `manifest top-level fields are well-formed, not merely present` | `release_tag` non-empty; `source_revision` 40-hex *when populated*; `build_date` ISO-8601 *when populated* | + | `pi-devbox-version --json round-trips the manifest byte-for-byte` | actual string equality with the file, since `--json` is a verbatim `cat` | + + **Why five value-checks replace four name-checks (total assertion count + unchanged at 61), and specifically why key presence and value shape are kept + apart:** an "every component value is a valid SHA" loop passes **vacuously** on `components:{}`, because jq's `all()` + over an empty list is true. A single combined check would therefore go green + on a manifest that had lost every component — which is the same shape of hole + as the three false greens already recorded in this file. They are separate on + purpose. + + Mutation-tested rather than reasoned about, twice: nine fabricated manifests + through the raw jq filters, then twelve through the shipped assertions using + the real `run` helper's `sh -c` quoting path (the quoting is load-bearing here + — a jq filter that dies on a quoting error exits non-zero and *looks* like a + caught defect). Measured against the old assertions on the same twelve + defects: **old caught 3, missed 9; new catches 12.** The three the old set + caught were key *disappearance* (grepping for a key name does fail when the + key is gone) and the literal string `"unknown"`; every value-level defect — + empty string, `null`, a 12-hex truncation, a wrong-but-plausible version, a + malformed `source_revision` — was invisible. Three legitimate variations are + correctly *not* flagged: empty `source_revision` and empty `build_date` (both + default empty on a plain local `docker build`, so demanding them would fail + honest local smoke runs) and `pi-studio: null`. + +- **Dropped the now-redundant `manifest has no unresolved ('unknown') + components` assertion.** The 40-hex value check strictly subsumes it: + `"unknown"` is not 40-hex, and only `rev()` in Dockerfile.variant ever emits + that string, feeding `components{}` exclusively. Removed rather than left in + place, because a redundant check that can never fail independently is one more + green tick that means nothing. + +- **Corrected a factually wrong "Still open" bullet in the v1.8.6 entry below** + (see the strikethrough there). It claimed `pi-devbox-version`'s human output + does not display `mempalace_version` and that only `--json` surfaces it. Both + halves are false — v1.8.6 shipped a `palace:` line with the same live-vs-baked + drift annotation `pi` already had. Verified by running the shipped script + against fabricated manifests: matching versions print `palace: 3.7.1`, a skew + prints `palace: 3.7.1 (baked as 3.8.0 — drift detected)`, and a pre-v1.8.6 + manifest with no baked field prints the live value un-annotated. The bullet + appears to describe an intermediate state of the working tree and was never + re-checked before tagging. Left visible as a struck-through correction rather + than deleted, since v1.8.6 is already published and someone may have read it. + +--- + ## v1.8.6 — 2026-08-25 Patch release. Adopts the drift that accumulated in the ~2 days since v1.8.5 @@ -144,7 +215,7 @@ and did not require a toolkit-side change. on drift; `MEMPALACE_VERSION` is a literal Dockerfile string with zero references in `.gitea/workflows/docker-publish.yml`. Flagged in v1.8.5's audit as a gap; still a gap. -- **`pi-devbox-version`'s human-readable output does not display +- ~~**`pi-devbox-version`'s human-readable output does not display `mempalace_version`.** Its render path is a fixed sequence (`release_tag`, `build_date`, `source_revision`, `pi`, then `components{}`) and the new top-level field isn't in it — only `--json` mode (which `cat`s @@ -152,7 +223,11 @@ and did not require a toolkit-side change. `rootfs/usr/local/bin/pi-devbox-version` would fix this; deferred since the field's stated purpose (correlating a palace bug to an image) is already served by `--json`, but worth doing in a follow-up if this becomes a - routine manual check. + routine manual check.~~ + **CORRECTION (2026-08-25, post-tag):** this bullet is wrong and was never + true of the tagged tree. `pi-devbox-version` *does* print a `palace:` line + in human mode, with live-vs-baked drift detection, degrading quietly on + pre-v1.8.6 manifests. Nothing is open here. See the Unreleased entry above. **Resolved during this release, not left open:** the feeder `--agent` default behavioural hook initially looked like it might need a diff --git a/scripts/smoke-test.sh b/scripts/smoke-test.sh index 07dbcc9..968d10e 100755 --- a/scripts/smoke-test.sh +++ b/scripts/smoke-test.sh @@ -348,12 +348,71 @@ echo "" echo "── Build provenance ──" run "/etc/pi-devbox/build-manifest.json present" \ "test -f /etc/pi-devbox/build-manifest.json" -run_expect "manifest records pi-extensions component" \ - "cat /etc/pi-devbox/build-manifest.json" '"pi-extensions"' -run_expect "manifest records pi-atelier" \ - "cat /etc/pi-devbox/build-manifest.json" '"pi-atelier"' -run_expect "manifest records pi_version" \ - "cat /etc/pi-devbox/build-manifest.json" '"pi_version"' +# These next checks replace three that grepped the manifest for the FIELD NAME +# and never looked at the value: +# +# run_expect "manifest records pi_version" "cat …manifest.json" '"pi_version"' +# +# which passes on {"pi_version": ""} and on {"pi_version": null}. The tell was +# visible in its own passing output — `✅ manifest records pi_version (got +# "pi_version")` echoes the key back as the thing it claims to have found. +# Two failure modes were therefore invisible: a key that survives with an empty +# or garbage value, and a key that vanishes from the manifest while every +# remaining value still looks fine. +# +# Those two need SEPARATE assertions, and the reason is a trap worth keeping in +# writing: an "every component value is a valid SHA" loop passes VACUOUSLY on +# components:{} — jq's all() over an empty list is true — so the value check +# alone would go green on a manifest that lost every component. Mutation-tested +# 2026-08-25 across nine fabricated manifests (empty map, deleted key, "", +# null, "unknown", 12-hex truncation, 40 non-hex chars, legit null pi-studio). +run "manifest declares every required component key" ' + req="pi-toolkit pi-extensions pi-fork pi-observational-memory pi-atelier mempalace-toolkit pi-studio" + for k in $req; do + jq -e --arg k "$k" "(.components|has(\$k))" /etc/pi-devbox/build-manifest.json >/dev/null \ + || { echo "manifest lost component key: $k" >&2; exit 1; } + done +' +# Subsumes the old `! grep -q \"unknown\"` check ("unknown" is not 40-hex), and +# also catches "", null and truncated SHAs, which that grep let through. null is +# legitimate for pi-studio alone: the non-studio variant has no such clone. +run "manifest component values are resolved 40-hex commits" ' + jq -e " + .components + | to_entries + | all(if .key == \"pi-studio\" and .value == null then true + else (.value|type) == \"string\" and (.value|test(\"^[0-9a-f]{40}\$\")) end) + " /etc/pi-devbox/build-manifest.json >/dev/null +' +# pi_version against ground truth, same shape as the mempalace check below. +# Chains with the "pi version matches build arg" assertion earlier in this file: +# together they tie build arg -> installed binary -> recorded manifest, so a +# manifest written from a stale variable cannot pass by agreeing with itself. +run "manifest pi_version matches the installed pi" ' + m=$(jq -r ".pi_version // empty" /etc/pi-devbox/build-manifest.json) + b=$(pi --version 2>/dev/null | head -n1 | tr -d "\r") + echo "manifest=[$m] installed=[$b]" >&2 + [ -n "$m" ] && [ "$m" = "$b" ] +' +# Top-level provenance fields: assert the SHAPE of each value, and only when the +# field is populated. source_revision and build_date legitimately default to +# empty (Dockerfile.variant ARGs) on a plain local `docker build`, so demanding +# them would fail honest local smoke runs; a populated-but-malformed value is +# the actual defect. release_tag defaults to "dev", so empty means a broken write. +run "manifest top-level fields are well-formed, not merely present" ' + j=/etc/pi-devbox/build-manifest.json + t=$(jq -r ".release_tag // empty" $j) + r=$(jq -r ".source_revision // empty" $j) + d=$(jq -r ".build_date // empty" $j) + echo "release_tag=[$t] source_revision=[$r] build_date=[$d]" >&2 + [ -n "$t" ] || { echo "release_tag empty (ARG default is dev)" >&2; exit 1; } + if [ -n "$r" ]; then + printf "%s" "$r" | grep -qxE "[0-9a-f]{40}" || { echo "source_revision not a 40-hex commit" >&2; exit 1; } + fi + if [ -n "$d" ]; then + printf "%s" "$d" | grep -qE "^[0-9]{4}-[0-9]{2}-[0-9]{2}T" || { echo "build_date not ISO-8601" >&2; exit 1; } + fi +' # mempalace CORE was absent from the manifest through v1.8.5: the toolkit SHA # was recorded but the palace version behind the MCP tools was not, so a palace # bug could not be correlated to an image version. Assert the field exists AND @@ -369,17 +428,23 @@ run "manifest mempalace_version matches the installed core" ' [ -n "$m" ] && [ "$m" = "$b" ] ' # Every component must be a resolved commit (or null for pi-studio in the -# non-studio variant) — 'unknown' means a clone silently failed to resolve. -run "manifest has no unresolved ('unknown') components" \ - "! grep -q '\"unknown\"' /etc/pi-devbox/build-manifest.json" -# pi-devbox-version wraps the manifest into a human-first command (this -# PR); verify the binary is present, executable, and both output modes work. +# non-studio variant) — now enforced by the 40-hex value check above, which +# strictly subsumes the old whole-file grep for '"unknown"'. Only rev() ever +# emits "unknown" and rev() feeds components only, so nothing is lost. +# pi-devbox-version wraps the manifest into a human-first command; verify the +# binary is present, executable, and that all three output modes work. run "pi-devbox-version binary present + executable" \ "test -x /usr/local/bin/pi-devbox-version" run_expect "pi-devbox-version human output shows release tag" \ "pi-devbox-version" "pi-devbox " -run_expect "pi-devbox-version --json round-trips the manifest" \ - "pi-devbox-version --json" '"release_tag"' +# --json is a verbatim `cat` of the manifest, so "round-trips" is assertable +# literally. The old form grepped the output for the string "release_tag" — the +# key name again — which would pass on a truncated or re-serialised dump. +run "pi-devbox-version --json round-trips the manifest byte-for-byte" ' + a=$(cat /etc/pi-devbox/build-manifest.json) + b=$(pi-devbox-version --json) + [ "$a" = "$b" ] || { echo "--json output differs from the manifest on disk" >&2; exit 1; } +' run_expect "pi-devbox-version --quiet is a compact one-liner" \ "pi-devbox-version --quiet | wc -l" "1" # OCI labels live in the image config, not the container fs — inspect them