From e070e0bcbfbd809036c7381cd88f1e92ca5c3d9c Mon Sep 17 00:00:00 2001 From: Joakim Persson Date: Wed, 26 Aug 2026 10:27:00 +0200 Subject: [PATCH] skills: record the vendored snapshot's provenance, and report which copy wins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found while verifying v1.8.7 from inside a fresh container: the baked mempalace snapshot is read by no host on this fleet. devbox-skill-reconcile repoints ~/.agents/skills/mempalace at the mounted live clone (the v1.8.5 fix working as designed), and all four compose stacks mount a workspace containing the skillset. So the phrase canary that blocked v1.8.7's first tag polices a file nobody opens, while the drift that could actually mislead an agent — a git pull nobody ran in /workspace/skillset — was invisible from inside the container and is invisible to CI by construction. Record provenance instead of policing it, and move the check to where the skillset actually is: - Dockerfile.variant: ARG SKILLSET_SNAPSHOT_REF (the claim) + a sha256 of the shipped bytes measured in the manifest layer (the fact), as manifest siblings rather than components{} members, plus an OCI label. An ARG default, not a CI-resolved output: no credential for the private skillset, no change at any of the four variant build call sites, and a local docker build records what CI does. Variant-only, so no base rebuild — check-base-hash.sh scans Dockerfile.base alone, verified by running it. - pi-devbox-version: a skills: section naming baked vs live @ per vendored skill, and for mempalace whether the live copy is identical to the baked fingerprint, at the same commit with uncommitted edits, or divergent. entrypoint-user.sh passes the new --no-skills, because the banner prints before the links exist and long before the reconcile runs. - scripts/vendor-mempalace-skill.sh: refresh the file and rewrite the ref together (a cp without an ARG bump makes the manifest lie, which is worse than anonymity); --check verifies the claim against a real clone. - 5 new smoke assertions (78 -> 83), mutation-tested through the real sh -c path: 6 fabricated manifests, where a well-formed hash of the wrong file proves the two manifest assertions are not redundant; the all-baked reporting test verified to FAIL against a live-skillset environment. Reviewed mid-flight by pi@emb-7kj4vr4g over the logstream (correlation skillset-vendor-drift), which retracted its own earlier recommendation of a build-time byte-compare against skillset HEAD and supplied the better framing: the invariant is NON-CONTRADICTION, not currency. Byte parity on a fallback would have cost a resync commit plus a ~67-min base rebuild for each of the four skillset commits pushed in one evening. Its warning also found a real bug here: the script now CONSTRUCTS the snapshot from `git show HEAD:` instead of copying the working tree, because a clean `git diff` says nothing about an untracked file — the one input the first draft would have recorded a false ref for. Tested: untracked, unstaged and staged-but-uncommitted all refuse, atomically. Also fixes three stale in-repo markers of the same class the canary belongs to (true when written, silently false at release): two dangling "Unreleased" pointers and a typst line still marked Unreleased five releases after v1.4.0. --- CHANGELOG.md | 180 +++++++++++++++++- Dockerfile.variant | 55 +++++- entrypoint-user.sh | 6 +- rootfs/usr/local/bin/pi-devbox-version | 95 ++++++++- .../local/share/pi-devbox/skills/VENDORED.md | 42 ++++ scripts/smoke-test.sh | 74 ++++++- scripts/vendor-mempalace-skill.sh | 159 ++++++++++++++++ 7 files changed, 604 insertions(+), 7 deletions(-) create mode 100755 scripts/vendor-mempalace-skill.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index b316933..c789553 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,184 @@ Pre-v1.0.0 tags followed the pi npm version (`v{pi_version}[letter]`). --- +## Unreleased + +The vendored `mempalace` skill snapshot stops being anonymous, and the +container starts saying which copy of each skill it is actually reading. + +**Also carried, previously undocumented:** `dbb7879` resynced the vendored +`mempalace` snapshot to skillset `c04cd15` ("the withdrawal only holds where the +bridge is live"), landed after the v1.8.7 tag and so absent from that image. +⚠️ **A base rebuild is forced** (~67 min): both that resync and the +`pi-devbox-version` / `entrypoint-user.sh` changes below touch inputs to +`base_tag` (`rootfs/` and `entrypoint*.sh`). The provenance recording itself +adds nothing to that cost — it lives entirely in `Dockerfile.variant`. + +Both come from one finding, made while verifying v1.8.7 from inside a freshly +recreated container: **the baked `mempalace` snapshot is read by no host on this +fleet.** `~/.agents/skills/mempalace` is a symlink to `/workspace/skillset/skills/mempalace` +— `entrypoint-user.sh` links the baked skill only `if [ ! -e ]`, and +`devbox-skill-reconcile` then repoints the skillset-owned ones at the live clone +(that is the v1.8.5 fix working as designed). All four compose stacks in +`docker-compose-repo` mount a workspace containing the skillset, so the vendored +copy is a CI/no-mount **fallback** and nothing else. Which means the +`mempalace skill snapshot is current` canary — the assertion that blocked +v1.8.7's first tag — polices a file that no agent on this fleet ever opens, +while the drift that *could* actually mislead an agent (a `git pull` nobody ran +in `/workspace/skillset`) was invisible from inside the container and is +invisible to CI by construction. + +**The rejected fix is worth recording, because it was the obvious one.** The +old comment in `scripts/smoke-test.sh` said the real answer was "a CI job +diffing this file against the skillset repo". It isn't: + +| Objection | Detail | +|---|---| +| needs a credential CI does not have | the skillset is **private** (`ssh://git@gitea.jordbo.se:2222/joakimp/skillset.git`); every build-time clone in this image uses anonymous HTTPS, and `resolve-versions`' `gitea_sha()` is explicitly documented as public-repo-only — its 401/403 path exists to survive a *stale token against a public repo*, so a private 403 would return empty and `require_sha` would hard-abort the release | +| makes another repo's branch able to fail this build | the same pi-devbox commit would go green today and red tomorrow, and a release could be blocked by an edit in an unrelated repo — precisely the shape of the run 589 failure, but automated and permanent | +| pure churn, and it is measurable | pi@emb-7kj4vr4g pushed **four** skillset commits in one evening (`d9dbbbd`, `b740d51`, `3324bd0`, `c04cd15`); a byte-parity gate would have demanded a pi-devbox resync commit **and a ~67-minute base rebuild for each one**, to keep current a copy almost nobody resolves | +| guards the wrong artefact | see above: on this fleet, nobody reads it | + +**The invariant is not currency, it is non-contradiction** — the framing comes +from pi@emb-7kj4vr4g's review (logstream `project/pi-devbox`, correlation +`skillset-vendor-drift`, which also **retracted** its own earlier build-time +byte-compare recommendation). A stale-but-self-consistent fallback is harmless; +a stale fallback carrying a **withdrawn instruction** is a live footgun, and +this project has already paid for that one — through v1.8.4 the baked snapshot +*shadowed* the live clone, which is how superseded attribution guidance kept +reaching agents. That is precisely what the bidirectional canary asserts, and +why it stays. + +So provenance is **recorded** rather than policed, and the check moves to where +the skillset actually is — a maintainer's clone, or any running container. + +### Added + +- **`build-manifest.json` now records the vendored snapshot's provenance: + `skillset_snapshot_ref` (which skillset commit the bytes are claimed to come + from) and `skillset_snapshot_sha256` (the bytes that actually shipped).** The + ref is a plain `ARG` **default in `Dockerfile.variant`**, deliberately not a + CI-resolved output, which buys three things at once: it needs no credential + for a private repo; it keeps a local `docker build` and CI identical by + construction (the same reasoning that put `MEMPALACE_VERSION` in + `Dockerfile.base` rather than duplicating it in the workflow); and it requires + **no change at any of the four `Dockerfile.variant` call sites** (`smoke`, + `smoke-studio`, `build-variant`, `build-variant-studio`), whose `--build-arg` + lists are hand-duplicated and therefore easy to under-apply to only two. + Also emitted as OCI label `se.jordbo.pi-devbox.skillset-snapshot-ref`, so it + is readable off the registry without pulling the image. + + Two design points, each arrived at from the file's own rules: + + - **The ref is a claim; the hash is measured.** `Dockerfile.variant` writes + the manifest from ground truth (`rev()` on each `/opt` clone, the live + `pi --version`), so the snapshot hash is computed with `sha256sum` in that + same layer rather than passed in. A build where the two disagree is exactly + what the new smoke assertions catch. + - **They are siblings, not members of `components{}`.** That map means "HEAD + of a clone present in this image" and the skillset is not cloned here — + calling it a component would be a lie a future reader would act on. It is + also load-bearing mechanically: `pi-devbox-version` renders every + `components{}` value with `.value[0:12]`, which would truncate a 64-hex + digest into something that looks like a short commit. Same reasoning as + `mempalace_version`'s existing comment. + + ⚠️ **Costs no base rebuild.** `base_tag` hashes `Dockerfile.base` + `rootfs/` + + `entrypoint*.sh` + the mempalace-toolkit SHA; `Dockerfile.variant` is in + none of it. `scripts/check-base-hash.sh` scans `Dockerfile.base` **only** + (`DF="Dockerfile.base"`, single hardcoded path), so a new `*_REF` ARG in the + variant is invisible to that guard — correctly, since it changes nothing + about the base's contents. + +- **`pi-devbox-version` gained a `skills:` section** reporting, per vendored + skill, whether the live copy is `baked` or a `live @ ` clone — + and for `mempalace`, whether that live copy matches the baked fingerprint: + `(identical to baked snapshot)`, `(baked snapshot + uncommitted edits)` + when the clone is at the recorded commit but the bytes differ, or + `(baked snapshot — live copy differs)`. Same live-vs-baked shape as the + existing `pi:`/`palace:` drift annotations. **This is the check CI cannot do + and a container can, for free**, since every host that matters already has the + skillset mounted. The list iterates the baked tree rather than a hardcoded + name list, so vendoring a fourth skill needs no edit here. + + `entrypoint-user.sh` calls it with the new **`--no-skills`** flag: the banner + is printed FIRST, before the baked links exist and long before the skillset + deploy and reconcile run last, so anything it said about skill sources would + describe a state that is about to change. Wrong-but-plausible is worse than + absent. (This is the one part of the change that touches `rootfs/` and + `entrypoint-user.sh`, so it does cost a base rebuild — already sunk, since + `dbb7879` refreshed the vendored snapshot.) + +- **`scripts/vendor-mempalace-skill.sh`** — refreshes the snapshot and rewrites + the recorded ref *together*, because a `cp` without a matching ARG bump + produces a manifest that confidently lies, which is worse than the anonymous + snapshot it replaced. Refuses to record a ref when the upstream file has + uncommitted modifications (no commit describes those bytes, so recording one + would be a fabrication) — checked on that one file, not the whole tree, so + unrelated work in progress in the skillset does not block a vendoring. + `--check` answers "is the committed snapshot really `skillset@`?" and separately reports staleness against the clone's HEAD. + + Counterfactual-tested rather than reasoned about, against throwaway clones: + a tampered snapshot reports `MISMATCH` **and** `STALE` (rc 1); a ref rolled + back to the previous skillset commit reports `MISMATCH` with content + unchanged (rc 1) and a subsequent refresh fixes only the ref, leaving the + bytes alone; unstaged and staged-but-uncommitted upstream edits are refused + with distinct messages and the snapshot left byte-identical, i.e. the refusal + is atomic. + + **Hardened after review** by pi@emb-7kj4vr4g, whose warning was that a resync + script must "write the ref it ACTUALLY copied from, or the provenance field + inherits the same class of bug the canary just had". The first draft copied + the working tree and guarded it with `git diff` — which says nothing about an + **untracked** file, and can be clean on a detached or behind checkout while + `HEAD` names something else. The snapshot is now *constructed* from + `git show HEAD:`, so the recorded pair cannot be a lie by construction, + and the untracked case is refused explicitly (tested: it was the one input the + first draft would have silently recorded a false ref for). Both new scripts are + `bash -n` clean and `shellcheck -S error` clean — the gate v1.8.7 added. + +### Fixed + +- **Three stale in-repo markers, all the same failure class.** Two "Unreleased" + pointers — `scripts/smoke-test.sh` pointed the reader at "the Unreleased + changelog note", and the v1.8.6 correction at "the Unreleased entry above"; + that section became the `## v1.8.7` heading at release time and neither + back-reference was updated. The third: `scripts/smoke-test.sh`'s own coverage + list still advertised "typst PDF engine for pandoc **(Unreleased)**", five + releases after typst shipped in v1.4.0. Same class as the canary they sit + next to: true when written, silently false at release, with nothing checking + them. The smoke comment now describes the mechanism that actually shipped + (and why the CI-diff idea it advertised was rejected); the changelog one names + v1.8.7; the typst line names v1.4.0. + +### Not fixed, deliberately + +- **CI still cannot tell you the vendored snapshot is behind `skillset` main.** + That needs a read-only deploy key for a private repo threaded into + `resolve-versions`, to warn about a file no host on this fleet reads. Revisit + when a no-skillset container becomes a real deployment (shipping the image + outside the fleet, or a CI-only agent) — at which point the honest gate is a + **warning**, matching the existing `PI_VERSION`/`MEMPALACE_VERSION` policy + (concreteness → error, newer-release-exists → warning), never a build + failure. +- **The phrase canary stays.** It is orthogonal and free: it pins *content* + where the new fields pin *provenance*, so it still catches a re-vendored + snapshot whose ref was bumped correctly but whose bytes came from the wrong + place — and, per the review above, asserting the **absence of withdrawn + guidance** is the half of it that earns its keep. Its comment now states the + limit instead of promising a fix. +- **v1.8.7's published image has no recorded ref**, and that is expected: the + field arrives here. Worth knowing when reading one, since the tag move + `ebd0de0` → `f645e66` means the published v1.8.7 carries a pre-`dbb7879` + snapshot, i.e. its baked mempalace skill lacks c04cd15's "confirm the bridge + actually stamps" caveat. Harmless — on v1.8.7 the bridge *is* live, so that + caveat self-retires, and every enrolled host reads the live clone anyway. + `pi-devbox-version` degrades quietly on such an image: no fingerprint, no + annotation, verified against the real v1.8.7 manifest. + +--- + ## v1.8.7 — 2026-08-25 Patch release, and the fastest turnaround in the series (~9 h after v1.8.6) for @@ -457,7 +635,7 @@ and did not require a toolkit-side change. **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. + pre-v1.8.6 manifests. Nothing is open here. See the v1.8.7 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/Dockerfile.variant b/Dockerfile.variant index 671a565..88f495e 100644 --- a/Dockerfile.variant +++ b/Dockerfile.variant @@ -277,6 +277,36 @@ ARG SOURCE_REVISION= # MEMPALACE_TOOLKIT_REF is consumed in Dockerfile.base; re-declared here # only so its intended ref lands in the label set alongside the others. ARG MEMPALACE_TOOLKIT_REF=main +# ── Vendored skill provenance ───────────────────────────────────────── +# The vendored mempalace SKILL.md is the ONLY baked artefact with no /opt +# clone behind it: its upstream (the skillset repo) is PRIVATE, so the +# image cannot clone it and CI cannot resolve its HEAD (see VENDORED.md). +# Consequence through v1.8.7: the snapshot was ANONYMOUS — nothing in the +# image or the repo recorded which skillset commit it was taken from, so +# the only staleness check available was a hand-maintained phrase canary in +# scripts/smoke-test.sh, which by construction can only detect "older than +# what I remembered to pin", never "older than skillset main". +# +# Recording the ref costs nothing and makes the question answerable. It is +# deliberately a plain ARG DEFAULT rather than a CI-resolved output: +# * the value is a fact about the committed snapshot, so it belongs in +# the tree next to it — not in a workflow that a local `docker build` +# never runs (same reasoning as MEMPALACE_VERSION living in +# Dockerfile.base rather than being duplicated in docker-publish.yml); +# * CI therefore needs NO new build-arg at any of its four +# Dockerfile.variant call sites (smoke, smoke-studio, build-variant, +# build-variant-studio) — a plumbing change that is easy to +# under-apply to only two of them; +# * and it needs no credential for a private repo. +# Bump it with scripts/vendor-mempalace-skill.sh, which refreshes the file +# and rewrites this line together, so the pair cannot drift apart by hand. +# This ARG lives in Dockerfile.variant ON PURPOSE: Dockerfile.base and +# rootfs/ are both hashed into base_tag, so recording provenance here costs +# no ~67-minute base rebuild. (scripts/check-base-hash.sh scans only +# Dockerfile.base, so no folding into the base hash is required — nor would +# it be correct, since this ARG changes nothing about the base's contents.) +ARG SKILLSET_SNAPSHOT_REF=c04cd156592decf8258cfce7aa5e123ac0a5c7e2 + # Dockerfile.base sets description="pi-devbox — base image (variant-independent)" # and every variant INHERITS it, so both published images used to advertise # themselves on Docker Hub as the base image. A LABEL cannot branch on @@ -301,7 +331,8 @@ LABEL org.opencontainers.image.version="${RELEASE_TAG}" \ se.jordbo.pi-devbox.pi-atelier-version="${PI_ATELIER_VERSION}" \ se.jordbo.pi-devbox.mempalace-toolkit-ref="${MEMPALACE_TOOLKIT_REF}" \ se.jordbo.pi-devbox.pi-studio-ref="${PI_STUDIO_REF}" \ - se.jordbo.pi-devbox.pi-studio-version="${PI_STUDIO_VERSION}" + se.jordbo.pi-devbox.pi-studio-version="${PI_STUDIO_VERSION}" \ + se.jordbo.pi-devbox.skillset-snapshot-ref="${SKILLSET_SNAPSHOT_REF}" # The manifest is written from GROUND TRUTH — the actual checked-out HEAD # of each /opt clone and the live `pi --version` — not merely the intended @@ -327,6 +358,20 @@ RUN set -e; \ case "$MP_V" in [0-9]*) MP_CORE="\"${MP_V}\"" ;; *) MP_CORE='null' ;; esac; \ STUDIO_REV='null'; \ if [ -d /opt/pi-studio/.git ]; then STUDIO_REV="\"$(rev /opt/pi-studio)\""; fi; \ + # The vendored skill snapshot's fingerprint is MEASURED here, not passed + # in as a build-arg, per the ground-truth rule above: SKILLSET_SNAPSHOT_REF + # is a CLAIM about which skillset commit the file came from, while this + # hash is what the image actually ships. Recorded together they let any + # reader with the skillset checked out — which on this fleet is every + # host, since all four compose stacks mount it — verify the claim at + # RUNTIME, without CI ever needing access to the private repo. Degrades + # to JSON null rather than failing the build if the file is absent; the + # smoke assertion is what turns that into a loud failure. + SKILL_SNAP='null'; \ + _snap_file=/usr/local/share/pi-devbox/skills/mempalace/SKILL.md; \ + if [ -f "$_snap_file" ]; then \ + SKILL_SNAP="\"$(sha256sum "$_snap_file" | cut -d' ' -f1)\""; \ + fi; \ { \ echo '{'; \ echo " \"release_tag\": \"${RELEASE_TAG}\","; \ @@ -337,6 +382,14 @@ RUN set -e; \ # SHAs and `pi-devbox-version` renders it with .value[0:12], which would # silently truncate a longer version string. echo " \"mempalace_version\": ${MP_CORE},"; \ + # Siblings, NOT members of components{}, for two independent reasons: + # that map means "HEAD of a clone present in this image" and the + # skillset is not cloned here (calling it a component would be a + # lie a future reader would act on), and `pi-devbox-version` renders + # every components{} value with .value[0:12] — which would truncate + # a 64-hex sha256 into something that looks like a short commit. + echo " \"skillset_snapshot_ref\": \"${SKILLSET_SNAPSHOT_REF}\","; \ + echo " \"skillset_snapshot_sha256\": ${SKILL_SNAP},"; \ echo " \"components\": {"; \ echo " \"pi-toolkit\": \"$(rev /opt/pi-toolkit)\","; \ echo " \"pi-extensions\": \"$(rev /opt/pi-extensions)\","; \ diff --git a/entrypoint-user.sh b/entrypoint-user.sh index ddbc9b7..3683d54 100755 --- a/entrypoint-user.sh +++ b/entrypoint-user.sh @@ -7,7 +7,11 @@ set -euo pipefail # so this reaches the same stream as the interactive shell the user lands # in). Reads the ground-truth manifest baked in Dockerfile.variant; a no-op # with a short stderr notice on images built before it existed. -command -v pi-devbox-version >/dev/null 2>&1 && pi-devbox-version || true +# `--no-skills`: this runs FIRST, before the baked skill links are created +# below and long before the skillset deploy + devbox-skill-reconcile run at the +# end of this script, so the skill-source section would report a pre-reconcile +# state that is about to change. Wrong-but-plausible is worse than absent. +command -v pi-devbox-version >/dev/null 2>&1 && pi-devbox-version --no-skills || true # ── SSH ControlMaster socket dir ──────────────────────────────── # Companion to /etc/ssh/ssh_config.d/00-devbox-controlmaster.conf in the diff --git a/rootfs/usr/local/bin/pi-devbox-version b/rootfs/usr/local/bin/pi-devbox-version index 3e655b8..b83c117 100755 --- a/rootfs/usr/local/bin/pi-devbox-version +++ b/rootfs/usr/local/bin/pi-devbox-version @@ -14,6 +14,8 @@ # pi-devbox-version human-readable summary (default) # pi-devbox-version --json raw manifest JSON (for scripting) # pi-devbox-version --quiet one-line "release_tag (source_revision)" form +# pi-devbox-version --no-skills skip the skill-source section (used at +# container start, where it would be premature) # # EXIT STATUS # 0 on success. 1 if the manifest is missing (e.g. an image built before @@ -24,12 +26,14 @@ set -euo pipefail MANIFEST=/etc/pi-devbox/build-manifest.json MODE="human" +SHOW_SKILLS="yes" case "${1:-}" in --json) MODE="json" ;; --quiet|-q) MODE="quiet" ;; + --no-skills) SHOW_SKILLS="no" ;; --help|-h) - sed -n '2,20p' "$0" | sed 's/^# \?//' + sed -n '2,22p' "$0" | sed 's/^# \?//' exit 0 ;; esac @@ -105,3 +109,92 @@ fi printf ' components:\n' jq -r '.components | to_entries[] | select(.value != null) | " \(.key): \(.value[0:12])"' "$MANIFEST" + +# ── Which copy of each vendored skill is actually being read? ───────── +# The image bakes fallback skills under /usr/local/share/pi-devbox/skills/, +# but for skills the skillset repo OWNS (skillset-owned.txt) a mounted live +# clone takes over at container start via devbox-skill-reconcile. Nothing +# reported which copy won, so a stale baked snapshot and a current live clone +# looked identical from inside — and on this fleet the baked mempalace copy is +# read by NOBODY (all four compose stacks mount a workspace containing the +# skillset), which is exactly the sort of fact that should be visible rather +# than reasoned about. Same "drift detected" shape as the pi/palace lines +# above: what is live, annotated with what was baked, when they disagree. +# +# Skipped with --no-skills at container start (entrypoint-user.sh calls this +# FIRST, before the baked links exist and long before the skillset deploy and +# reconcile run last), because a section that is accurate only after boot +# finishes is worse than no section at all. +BAKED_SKILLS=/usr/local/share/pi-devbox/skills +SKILLS_DIR="${HOME:-/home/developer}/.agents/skills" + +if [ "$SHOW_SKILLS" = "yes" ] && [ -d "$BAKED_SKILLS" ] && [ -d "$SKILLS_DIR" ]; then + # Recorded provenance of the vendored mempalace snapshot (absent on images + # built before this existed — `// empty` so a JSON null never prints as the + # 4-char string "null", the same trap noted for mempalace_version above). + snap_ref=$(jq -r '.skillset_snapshot_ref // empty' "$MANIFEST") + snap_sha=$(jq -r '.skillset_snapshot_sha256 // empty' "$MANIFEST") + + # Iterate the baked tree rather than a hardcoded name list, so vendoring a + # fourth skill needs no edit here. The header prints only if the tree is + # non-empty, so this can never emit a dangling "skills:" label. + _printed_header="no" + for _dir in "$BAKED_SKILLS"/*/; do + [ -d "$_dir" ] || continue + if [ "$_printed_header" = "no" ]; then + printf ' skills:\n' + _printed_header="yes" + fi + _name=$(basename "$_dir") + _link="$SKILLS_DIR/$_name" + + if [ ! -e "$_link" ]; then + printf ' %-22s not linked\n' "$_name" + continue + fi + + _target=$(readlink -f "$_link" 2>/dev/null || echo "$_link") + case "$_target" in + "$BAKED_SKILLS"/*|"$BAKED_SKILLS") + printf ' %-22s baked\n' "$_name" + continue + ;; + esac + + # Outside the baked tree: a mounted skillset clone, or a user override. + # The link target is /skills/, so the repo root is two up. + # Everything here is guarded: this script runs on the container-start path + # and must never fail, and `set -e` is in force. + _root=$(cd "$_target/../.." 2>/dev/null && pwd) || _root="" + _head="" + if [ -n "$_root" ]; then + _head=$(git -C "$_root" rev-parse HEAD 2>/dev/null || echo "") + fi + _where="live ${_root:-$_target}" + [ -n "$_head" ] && _where="$_where @ ${_head:0:7}" + + # For the one skill whose baked fingerprint we recorded, say plainly + # whether the live copy differs from what shipped. This is the check CI + # cannot perform (the skillset is private) and the container can, free. + _live_sha="" + if [ -n "$snap_sha" ] && [ "$_name" = "mempalace" ] && [ -f "$_target/SKILL.md" ]; then + _live_sha=$(sha256sum "$_target/SKILL.md" 2>/dev/null | cut -d' ' -f1 || echo "") + fi + if [ -z "$_live_sha" ]; then + printf ' %-22s %s\n' "$_name" "$_where" + elif [ "$_live_sha" = "$snap_sha" ]; then + printf ' %-22s %s (identical to baked snapshot)\n' "$_name" "$_where" + elif [ -n "$_head" ] && [ "$_head" = "$snap_ref" ]; then + # Same commit, different bytes — i.e. uncommitted edits in the live + # checkout. Distinguished from plain drift because otherwise the line + # reads as a self-contradiction ("@ c04cd15 ... baked snapshot c04cd15 + # — live copy differs") and a reader would suspect the tool, not the + # working tree. + printf ' %-22s %s \033[33m(baked snapshot %s + uncommitted edits)\033[0m\n' \ + "$_name" "$_where" "${snap_ref:0:7}" + else + printf ' %-22s %s \033[33m(baked snapshot %s — live copy differs)\033[0m\n' \ + "$_name" "$_where" "${snap_ref:0:7}" + fi + done +fi diff --git a/rootfs/usr/local/share/pi-devbox/skills/VENDORED.md b/rootfs/usr/local/share/pi-devbox/skills/VENDORED.md index ac9c3f6..838d14b 100644 --- a/rootfs/usr/local/share/pi-devbox/skills/VENDORED.md +++ b/rootfs/usr/local/share/pi-devbox/skills/VENDORED.md @@ -39,6 +39,33 @@ its skill file needed baking. *different* skill, `opencode-mempalace-bridge`), so there is no public package source to copy from. This snapshot is refreshed manually per release. + **Refresh it with `scripts/vendor-mempalace-skill.sh `, not + `cp`.** Because the image cannot clone the private upstream, the snapshot used + to be *anonymous* — nothing recorded which skillset commit the bytes came + from, so the only staleness check possible was a hand-maintained phrase canary + in `scripts/smoke-test.sh`, which by construction detects "older than the + phrase I remembered to pin", never "older than skillset main". Two facts now + travel with the file: + + | Fact | Where | Kind | + |---|---|---| + | `ARG SKILLSET_SNAPSHOT_REF` in `Dockerfile.variant` | manifest `skillset_snapshot_ref` + OCI label `se.jordbo.pi-devbox.skillset-snapshot-ref` | a **claim** about which commit these bytes are | + | `sha256sum` of this file, measured in the manifest layer | manifest `skillset_snapshot_sha256` | the bytes that **actually shipped** | + + The script writes both together, refuses when the upstream file has + uncommitted modifications (no commit describes those bytes), and + `--check` verifies the claim against a real clone. Deliberately an `ARG` + default rather than a CI-resolved value: no credential for a private repo, no + change at any of the four `Dockerfile.variant` build call sites, and a local + `docker build` records the same thing CI does. + + Verifying "is this snapshot current?" is **not** a CI job and was deliberately + not made one — see the Unreleased CHANGELOG entry for why (private repo; + another repo's branch must not be able to fail this build; and the artefact it + would guard is read by no host on this fleet). The check belongs where the + skillset actually is: `vendor-mempalace-skill.sh --check` for a maintainer, + and `pi-devbox-version`'s `skills:` section for an agent inside a container. + ## Runtime precedence (v1.8.5+) The baked links are created **early** in `entrypoint-user.sh` (before pi-deploy, @@ -59,6 +86,21 @@ repoints the links for skills the **skillset owns**, listed one per line in 3. **baked snapshot** — everything else, and every skill when no skillset is mounted +**Which one won is now reportable from inside the container:** +`pi-devbox-version` prints a `skills:` section naming, per vendored skill, +`baked` or `live @ ` — and for `mempalace` whether that live copy is +identical to the baked fingerprint, at the same commit but with uncommitted +edits, or genuinely divergent. Before that, a stale baked snapshot and a current +live clone were indistinguishable from inside, which is how the freshness of +this file went unexamined for three releases. The section is suppressed with +`--no-skills` on the container-start banner, because `entrypoint-user.sh` prints +the version *before* the links exist and long before the reconcile below runs. + +On this fleet, precedence 2 wins for `mempalace` on **every** host — all four +compose stacks mount a workspace containing the skillset — so the baked copy is +exercised only by CI and by a hypothetical no-mount container. Worth +remembering before spending effort on its freshness. + Ownership is per-skill on purpose: `pi-extensions`' authoritative source is the package repo (copied over the snapshot at build), and `skillset` carries a downstream copy that can lag, so handing it to the clone would *regress* the diff --git a/scripts/smoke-test.sh b/scripts/smoke-test.sh index 315e0c2..f8b5de1 100755 --- a/scripts/smoke-test.sh +++ b/scripts/smoke-test.sh @@ -7,7 +7,7 @@ # - pi binary present and (if EXPECTED_PI_VERSION set) matches CI's resolved version # - 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 (Unreleased) — `pandoc --pdf-engine=typst` +# - typst PDF engine for pandoc (v1.4.0) — `pandoc --pdf-engine=typst` # - non-modal editors nano + micro (alongside nvim) # - terminfo for modern emulators: xterm-kitty, xterm-ghostty, wezterm, # alacritty, foot (kitty-terminfo + ncurses-term + compiled ghostty alias) @@ -463,6 +463,38 @@ run "pi-devbox-version --json round-trips the manifest byte-for-byte" ' ' run_expect "pi-devbox-version --quiet is a compact one-liner" \ "pi-devbox-version --quiet | wc -l" "1" +# ── Vendored skill snapshot provenance ───────────────────────────────── +# The vendored mempalace skill is the one baked artefact with no /opt clone +# behind it (private upstream — see VENDORED.md), so until now the manifest +# could not say which skillset commit it came from. Two fields now travel with +# it: the CLAIMED ref (ARG default in Dockerfile.variant) and the MEASURED +# sha256 of the shipped bytes. Assert both are well-formed, and — separately — +# that the measurement still describes the file in the image. +# +# Kept as two assertions for the same reason the component checks are: one +# proves the fields are not empty/garbage, the other proves they are not merely +# self-consistent. A single combined check could pass on a manifest whose hash +# was computed from a file that was later overwritten (the pi-extensions skill +# copy at Dockerfile.variant:165 does exactly that kind of overwrite, one stage +# earlier), which is the failure this second one exists to catch. +run "manifest records the vendored skill snapshot provenance" ' + j=/etc/pi-devbox/build-manifest.json + r=$(jq -r ".skillset_snapshot_ref // empty" $j) + s=$(jq -r ".skillset_snapshot_sha256 // empty" $j) + echo "ref=[$r] sha256=[$s]" >&2 + printf "%s" "$r" | grep -qxE "[0-9a-f]{40}" \ + || { echo "skillset_snapshot_ref is not a 40-hex commit" >&2; exit 1; } + printf "%s" "$s" | grep -qxE "[0-9a-f]{64}" \ + || { echo "skillset_snapshot_sha256 is not a 64-hex digest" >&2; exit 1; } +' +run "manifest skill fingerprint matches the baked snapshot" ' + j=/etc/pi-devbox/build-manifest.json + f=/usr/local/share/pi-devbox/skills/mempalace/SKILL.md + m=$(jq -r ".skillset_snapshot_sha256 // empty" $j) + a=$(sha256sum "$f" | cut -d" " -f1) + echo "manifest=[$m] actual=[$a]" >&2 + [ -n "$m" ] && [ "$m" = "$a" ] +' # OCI labels live in the image config, not the container fs — inspect them # from the host docker rather than via `docker run`. LBL=$(docker inspect --format '{{ index .Config.Labels "se.jordbo.pi-devbox.pi-extensions-ref" }}' "$IMAGE" 2>/dev/null || true) @@ -539,8 +571,24 @@ exec_test "mempalace skill linked (fallback)" 'test -L $HOME/.agents/skills # one absent, so a re-vendored stale snapshot fails just as loudly as a # forgotten bump. A one-way canary only catches half the drift. # * a phrase canary can only ever detect "older than what I remembered to pin", -# never "older than skillset main". The real fix is a CI job diffing this -# file against the skillset repo — see the Unreleased changelog note. +# never "older than skillset main". +# +# That structural limit is now addressed, but NOT by the "CI job diffing this +# file against the skillset repo" this comment used to point at (that pointer +# also dangled: it referenced an Unreleased changelog note that had become the +# v1.8.7 heading). A CI diff cannot be done without granting CI a credential +# for the PRIVATE skillset repo, and it would guard a file that on this fleet +# NO host reads — all four compose stacks mount a workspace containing the +# skillset, so devbox-skill-reconcile repoints this link at the live clone and +# the baked copy is a CI/no-mount fallback only. Instead the snapshot now +# carries its provenance (skillset_snapshot_ref + a measured +# skillset_snapshot_sha256 in build-manifest.json, written by +# scripts/vendor-mempalace-skill.sh), which moves the check to where the +# skillset actually IS: `scripts/vendor-mempalace-skill.sh --check` for a +# maintainer, and `pi-devbox-version` for an agent inside any container. +# This assertion is kept because it is orthogonal and free: it pins content, +# not provenance, so it still catches a re-vendored snapshot whose ref was +# bumped correctly but whose bytes came from the wrong place. exec_test "mempalace skill snapshot is current" 'f=$HOME/.agents/skills/mempalace/SKILL.md; grep -q "Provenance is stamped for you" "$f" && ! grep -q "Attribute what you file yourself" "$f" && echo ok' # Link TARGETS, not just link existence: with no skillset mounted (as here) the # baked tree must be what resolves, for all three vendored skills. @@ -551,6 +599,26 @@ exec_test "vendored skills resolve to the baked tree (no skillset mounted)" \ *) echo "$s resolves to $(readlink -f $HOME/.agents/skills/$s)" >&2; exit 1 ;; esac done; echo ok' +# ... and that the tool REPORTS that resolution, which is the half that was +# missing: a stale baked snapshot and a current live clone were +# indistinguishable from inside the container. CI mounts no skillset, so every +# vendored skill must report "baked" here — which also makes this a real test of +# the fallback path rather than of the environment it happens to run in. +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; do + echo "$out" | grep -qE "^ $s +baked$" \ + || { echo "$s not reported as baked" >&2; exit 1; } + done; echo ok' +# 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 +# a pre-reconcile state that is about to change. +exec_test "pi-devbox-version --no-skills omits the skills section" \ + '! pi-devbox-version --no-skills | grep -q "skills:"' +exec_test "entrypoint prints the version banner with --no-skills" \ + 'grep -q "pi-devbox-version --no-skills" /usr/local/bin/entrypoint-user.sh' # The handover path itself. CI never mounts a skillset, so without this the # v1.8.5 fix would ship untested: fabricate a skillset + a skills dir holding # baked-style links, run the reconciler, and assert all three outcomes — diff --git a/scripts/vendor-mempalace-skill.sh b/scripts/vendor-mempalace-skill.sh new file mode 100755 index 0000000..fd90ccf --- /dev/null +++ b/scripts/vendor-mempalace-skill.sh @@ -0,0 +1,159 @@ +#!/usr/bin/env bash +# vendor-mempalace-skill.sh — refresh the vendored mempalace skill snapshot +# AND its recorded provenance, together, so the two cannot drift apart. +# +# WHY THIS EXISTS +# --------------- +# rootfs/usr/local/share/pi-devbox/skills/mempalace/SKILL.md is a snapshot of a +# file owned by the PRIVATE skillset repo (see VENDORED.md). Because the image +# cannot clone that repo, refreshing the snapshot was a manual `cp` — and the +# result was anonymous: nothing recorded WHICH skillset commit the bytes came +# from. The only staleness check available was a hand-maintained phrase canary +# in scripts/smoke-test.sh, which by construction detects "older than the phrase +# I remembered to pin", never "older than skillset main". +# +# Two facts now travel with the snapshot: the skillset commit it was taken from +# (ARG SKILLSET_SNAPSHOT_REF in Dockerfile.variant) and the sha256 of the bytes +# themselves (measured at build time into build-manifest.json). This script is +# the only thing that should ever write the first one, because a `cp` without a +# matching ARG bump produces a manifest that CONFIDENTLY LIES — worse than the +# anonymous snapshot it replaced. +# +# USAGE +# scripts/vendor-mempalace-skill.sh [skillset-root] refresh + record +# scripts/vendor-mempalace-skill.sh --check [root] verify, write nothing +# +# skillset-root defaults to /workspace/skillset, then $HOME/skillset. +# +# --check answers "is the committed snapshot really skillset@?" +# — the question CI cannot answer without a credential for the private repo, +# and which anyone with the skillset checked out can answer for free. Exit 0 +# when the snapshot is honest and current, 1 when it is not. +# +# EXIT STATUS +# 0 refreshed (or already up to date; --check: snapshot verified) +# 1 refused: dirty upstream file, missing repo, or --check mismatch +set -euo pipefail + +cd "$(dirname "$0")/.." + +DOCKERFILE="Dockerfile.variant" +VENDORED="rootfs/usr/local/share/pi-devbox/skills/mempalace/SKILL.md" +ARG_NAME="SKILLSET_SNAPSHOT_REF" +REL_PATH="skills/mempalace/SKILL.md" + +MODE="refresh" +if [ "${1:-}" = "--check" ]; then + MODE="check" + shift +fi + +ROOT="${1:-}" +if [ -z "$ROOT" ]; then + for candidate in /workspace/skillset "$HOME/skillset"; do + if [ -d "$candidate/.git" ]; then + ROOT="$candidate" + break + fi + done +fi + +die() { printf '%s: %s\n' "$(basename "$0")" "$1" >&2; exit 1; } + +[ -n "$ROOT" ] || die "no skillset clone found (pass one: $(basename "$0") /path/to/skillset)" +[ -d "$ROOT/.git" ] || die "not a git clone: $ROOT" +[ -f "$ROOT/$REL_PATH" ] || die "no $REL_PATH in $ROOT" +[ -f "$VENDORED" ] || die "vendored snapshot missing: $VENDORED" + +head_sha=$(git -C "$ROOT" rev-parse HEAD 2>/dev/null) || die "cannot read HEAD of $ROOT" +recorded=$(grep -oE "^ARG ${ARG_NAME}=[0-9a-f]{40}$" "$DOCKERFILE" | cut -d= -f2 || true) +[ -n "$recorded" ] || die "no 'ARG ${ARG_NAME}=<40-hex>' line in $DOCKERFILE" + +sha_of() { sha256sum "$1" | cut -d' ' -f1; } +sha_empty=$(printf '' | sha256sum | cut -d' ' -f1) +vendored_sha=$(sha_of "$VENDORED") +upstream_sha=$(sha_of "$ROOT/$REL_PATH") + +# The recorded ref must PROVABLY describe the recorded bytes. Reviewed by +# pi@emb-7kj4vr4g (logstream correlation skillset-vendor-drift, 2026-08-26): +# "make sure the resync script writes the ref it ACTUALLY copied from, or the +# provenance field inherits the same class of bug the canary just had." So this +# does not copy the working tree and then hope a `git diff` was enough — a +# clean-looking `git diff` says nothing about an UNTRACKED file, and a detached +# or behind checkout can be clean while HEAD names something else. Instead the +# snapshot is CONSTRUCTED from the ref being recorded (below), and the working +# tree is compared only so a local edit produces a refusal rather than a silent +# surprise. Order matters here: everything this block reads is defined above it. +blob_sha=$(git -C "$ROOT" show "HEAD:$REL_PATH" 2>/dev/null | sha256sum | cut -d' ' -f1 || true) +upstream_dirty="" +if [ -z "$blob_sha" ] || [ "$blob_sha" = "$sha_empty" ]; then + upstream_dirty="not present at HEAD (untracked, or absent at this commit)" +elif [ "$blob_sha" != "$upstream_sha" ]; then + if ! git -C "$ROOT" diff --quiet -- "$REL_PATH" 2>/dev/null; then + upstream_dirty="modified but not committed" + elif ! git -C "$ROOT" diff --cached --quiet -- "$REL_PATH" 2>/dev/null; then + upstream_dirty="staged but not committed" + else + upstream_dirty="different at HEAD than in the working tree" + fi +fi + +if [ "$MODE" = "check" ]; then + # Compare the committed snapshot against the file AT THE RECORDED REF, not + # against the working tree: the question is whether the record is truthful, + # which is independent of how current it is. Both are reported. + at_ref=$(git -C "$ROOT" show "${recorded}:${REL_PATH}" 2>/dev/null | sha256sum | cut -d' ' -f1 || true) + printf 'recorded ref: %s\n' "$recorded" + printf 'vendored sha256: %s\n' "$vendored_sha" + printf 'sha256 at ref: %s\n' "${at_ref:-}" + printf 'skillset HEAD: %s (%s)\n' "$head_sha" "$(sha_of "$ROOT/$REL_PATH")" + rc=0 + if [ -z "$at_ref" ]; then + printf 'UNKNOWN: %s is not in this clone — fetch, or check against a complete one\n' "$recorded" >&2 + rc=1 + elif [ "$at_ref" != "$vendored_sha" ]; then + printf 'MISMATCH: the vendored snapshot is NOT the file at the recorded ref\n' >&2 + rc=1 + else + printf 'OK: the vendored snapshot is exactly skillset@%s:%s\n' "${recorded:0:7}" "$REL_PATH" + fi + if [ "$vendored_sha" != "$upstream_sha" ]; then + printf 'STALE: the working tree of %s differs from the snapshot (HEAD %s)\n' \ + "$ROOT" "${head_sha:0:7}" >&2 + rc=1 + fi + exit "$rc" +fi + +[ -z "$upstream_dirty" ] || die "$ROOT/$REL_PATH is $upstream_dirty — commit it first, or the recorded ref would not describe these bytes" + +if [ "$vendored_sha" = "$upstream_sha" ] && [ "$recorded" = "$head_sha" ]; then + printf 'already current: snapshot == skillset@%s\n' "${head_sha:0:7}" + exit 0 +fi + +# Written FROM THE REF, not copied from the working tree, so the pair cannot be +# a lie by construction. Via a temp file so a failed write cannot leave a +# half-vendored snapshot behind. +snap_tmp=$(mktemp) +if ! git -C "$ROOT" show "HEAD:$REL_PATH" > "$snap_tmp" 2>/dev/null; then + rm -f -- "$snap_tmp" + die "cannot read HEAD:$REL_PATH from $ROOT" +fi +mv -- "$snap_tmp" "$VENDORED" +[ "$(sha_of "$VENDORED")" = "$blob_sha" ] \ + || die "internal: written snapshot does not match HEAD:$REL_PATH" +# In-place, and only the exact pinned line: a broad sed on this Dockerfile could +# rewrite one of the other *_REF ARGs. +tmp=$(mktemp) +sed "s|^ARG ${ARG_NAME}=.*$|ARG ${ARG_NAME}=${head_sha}|" "$DOCKERFILE" > "$tmp" +mv -- "$tmp" "$DOCKERFILE" + +new_recorded=$(grep -oE "^ARG ${ARG_NAME}=[0-9a-f]{40}$" "$DOCKERFILE" | cut -d= -f2 || true) +[ "$new_recorded" = "$head_sha" ] || die "failed to rewrite ${ARG_NAME} in $DOCKERFILE" + +printf 'snapshot: %s -> %s\n' "${vendored_sha:0:12}" "$(sha_of "$VENDORED" | cut -c1-12)" +printf 'ref: %s -> %s\n' "${recorded:0:7}" "${head_sha:0:7}" +printf '\nNOTE: %s is hashed into base_tag, so this costs a base rebuild\n' "$VENDORED" +printf 'on the next tag (~67 min). Also re-pin the phrase canary in\n' +printf 'scripts/smoke-test.sh if the section it names changed.\n'