diff --git a/CHANGELOG.md b/CHANGELOG.md index c048dfb..a2ebd4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,43 +13,116 @@ Pre-v1.0.0 tags followed the pi npm version (`v{pi_version}[letter]`). ## Unreleased -Docs only so far, but one of the two files ships **inside** the image. +### Fixed -### Known issue (proposed for v1.8.5, not yet fixed) - -- **Vendored skills silently shadow their live skillset counterparts — an - ordering bug, and the code comment claims the opposite.** `~/.agents/skills` - is *asymmetric*: `mempalace`, `pi-devbox-environment` and `pi-extensions` - resolve to the baked `/usr/local/share/pi-devbox/skills/…`, while every other - skill resolves to the live `/workspace/skillset/skills/…`. Root cause is - precedence-by-ordering in `entrypoint-user.sh`: the baked links are created at - **line 65** (deliberately early, to close a smoke-test readiness race) with - `[ ! -e … ]` so they are "created only when absent", and the skillset deploy - runs **last** at line 387, where it classifies the existing links as - foreign and leaves them alone. The comment at line 61 states the goal as "a - same-named skillset skill … is never clobbered" — but with baked-first plus - create-when-absent, the *skillset* skill is precisely the one that loses. +- **Vendored skills no longer silently shadow their live skillset + counterparts.** `~/.agents/skills` was *asymmetric*: `mempalace`, + `pi-devbox-environment` and `pi-extensions` resolved to the baked + `/usr/local/share/pi-devbox/skills/…`, while every other skill resolved to the + live `/workspace/skillset/skills/…`. Root cause was precedence-by-ordering in + `entrypoint-user.sh`: the baked links are created **early** (line 65 in + v1.8.4; the loop moved down as this fix added comments) — deliberately so, to + close a smoke-test readiness race — with `[ ! -e … ]` so they are + "created only when absent", and the skillset deploy runs **last**, where it + classifies the existing links as foreign and leaves them alone. The comment at + line 61 claimed the goal was "a same-named skillset skill … is never + clobbered" — but with baked-first plus create-when-absent, the *skillset* skill + was precisely the one that lost. Comment now describes the actual behaviour. Observed cost, on two hosts independently: an edit to `skillset/skills/mempalace/SKILL.md` (adding a drawer-attribution rule) was pushed and present in the live clone (`md5 129bcc4752`), yet both the EMB-7KJ4VR4G and tor-ms22 containers kept loading the baked copy (`md5 5236024fef`) with zero occurrences of the new rule. The tor-ms22 agent - had to fetch the rule from the Gitea API to read it at all. Editing a - skillset skill therefore *appears* to work and silently does nothing until an - image rebuild — for exactly the three skills most likely to be iterated on, - since they are the pi-devbox-specific ones. + had to fetch the rule from the Gitea API to read it at all. Editing a skillset + skill therefore *appeared* to work and silently did nothing until an image + rebuild — for exactly the three skills most likely to be iterated on. - Proposed fix (surgical, keeps the race fix): leave the early baked links as - the fallback, and let the skillset deploy at line 387 **replace** links that - point into `/usr/local/share/pi-devbox/skills/` when it ships a skill of the - same name — i.e. baked = default, live clone = preferred when present. User - overrides (a real directory, or a symlink pointing elsewhere) must still win - over both. Verify after the change with - `readlink -f ~/.agents/skills/mempalace`, not by reading the entrypoint. + **The fix is not "the skillset always wins"**, because ownership is per-skill + (`rootfs/usr/local/share/pi-devbox/skills/VENDORED.md`): `pi-extensions`' + authoritative source is the *package* repo, copied over the snapshot at build + time, and `skillset` carries a downstream copy that can lag — handing that one + to the clone would regress the skill. So a new helper + `devbox-skill-reconcile` runs immediately after the skillset deploy and + repoints only the skills named in `skills/skillset-owned.txt` (today: + `mempalace`). Precedence is now user override → live skillset clone (owned + names only) → baked snapshot, with the early links untouched as the fallback, + so the readiness race stays closed. It only ever replaces a symlink that + points into the baked tree, so a real directory or a link pointing elsewhere + is never disturbed. Verify with `readlink -f ~/.agents/skills/mempalace`, not + by reading the entrypoint. + +- **A latent boot-abort in the baked-link block, found while reviewing the fix + above and fixed with it.** `[ ! -e "$link" ]` is TRUE for a *dangling* symlink + (`-e` follows the link), so once a link may point into `/workspace/skillset` + — which the fix above makes possible — a vanished mount turns the guard into + "create over a broken link", and plain `ln -s` then fails with `File exists`. + Under the entrypoint's `set -euo pipefail` that **aborts container start** + before `exec "$@"`, with a cryptic `ln` error and no pi. Reachable on a + `docker restart` or a host reboot under `restart: unless-stopped` (the writable + layer survives and `~/.agents` is not a volume on any host), though not on a + `compose up -d` recreate. Now `ln -sfn`, which heals the broken link back to + the baked fallback; the reconciler re-points it in the same boot if the clone + is back. A comment at the call site records why the `-f` must stay. + +- **README's skill-precedence documentation was wrong** in the same way the + entrypoint comment was: it claimed baked skills are "created only when absent + so a same-named skillset skill … is never clobbered" and that "a mounted + skillset always overrides them". Rewritten to state the real, per-skill + precedence and to name `skillset-owned.txt` and `devbox-skill-reconcile`. + +- **The smoke canary for a stale `mempalace` snapshot could not detect + staleness.** It grepped `"Shared palace: multiple harnesses"` — a phrase + present in *both* the stale and the fresh copy, so it passed throughout the + shadowing bug above. It now pins the newest section + (`"Attribute what you file yourself"`), and `VENDORED.md` records that + updating this string is part of refreshing the snapshot. Three further + assertions close the gaps that let the bug ship: skill link **targets** are + asserted (not merely `test -L`), the `skillset-owned.txt` list is asserted to + contain `mempalace` and *not* `pi-extensions`, and the reconciler's replace + path — which CI never exercises, since no smoke container mounts a skillset — + is covered by fabricating a skillset and asserting all three outcomes (owned + skill repointed, unowned skill left baked, user override untouched) — plus a + second case that a mutation test proved necessary: with the reconciler's + "is this link ours?" guard deleted, all three of those assertions still + passed, so the discriminating case is an *owned* name whose link is a user + override pointing outside the baked tree. ### Changed +- **Vendored `mempalace` skill snapshot refreshed** from `skillset` `936fed8` → + `670f7f1` (`md5 5236024fef` → `129bcc4752`), which adds the "Attribute what + you file yourself" rule: hand-filed drawers should carry + `added_by="@"`. Without this refresh the symlink fix above + would only help hosts that mount `skillset`; a bare container would still ship + the pre-attribution-rule skill. + +- **Component audit for this release — no pin edits needed.** Every component + except `pi`/`pi-atelier` is pinned to a moving ref that CI resolves at build + time, and each was checked against upstream on 2026-08-23: `pi` `0.84.2` + (still npm latest, published 2026-08-14), `pi-atelier` `v0.8.2` (newest tag; + the `≥0.7.1` floor for `pi ≥ 0.84` is satisfied), `pi-fork` `f1ff8087`, + `pi-observational-memory` `ce9fc982`, `pi-toolkit` `0e1369e6`, + `pi-extensions` `20228878`, `pi-studio` `v0.9.48` → `c3b83680` — all + **byte-identical to what v1.8.4 shipped**. `MEMPALACE_VERSION` stays `3.7.1` + (still PyPI latest, and the version the central palace serves, so no + client/server skew). The one component that moved is `mempalace-toolkit` + `fd8b15f5` → `0fe64c4`, which is this release's other payload: the feeder now + defaults `--agent` to `pi@$MEMPALACE_PI_DEVICE` so palace writes carry + provenance, with `$USER` still the fallback when the variable is unset + (`AGENT="${MEMPALACE_PI_DEVICE:+pi@${MEMPALACE_PI_DEVICE}}"`), so un-enrolled + hosts are unaffected. Nothing landed upstream after the + `pi-observational-memory` merge `ce9fc982`, so the eight-week-bug fix in + v1.8.4 is not destabilised. + + Two notes for whoever runs the build. This release changes + `entrypoint-user.sh`, `rootfs/**` and the resolved toolkit SHA — all three feed + the base-image hash — so expect a **full multi-arch base rebuild** (~95 min, + as on v1.8.3/CI 562), not a fast variant-only publish. And that rebuild + re-resolves the ~14 base-tooling `ARG *_VERSION=latest` pins; measured drift + on 2026-08-23 was one patch (`nvim v0.12.4 → v0.12.5`), so the window is + favourable, but it is not covered by version assertions. + - **`pi-devbox-environment` skill — new §2 subsection "A negative result is usually your own filter", plus ControlMaster masking in §3.** This is baked (`rootfs/usr/local/share/pi-devbox/skills/`, symlinked to @@ -80,6 +153,24 @@ Docs only so far, but one of the two files ships **inside** the image. --- +### Still open + +- `build-manifest.json` records the `mempalace-toolkit` SHA but **not** the + mempalace **core** version, so a palace bug cannot be correlated with an + image. `mempalace --version` prints it; adding it is a one-line change to the + manifest `RUN` in `Dockerfile.variant` plus one smoke assertion, and is + variant-only (no base rebuild cost). +- Smoke asserts the `pi-observational-memory` clone **exists** but not that it + contains the ambient-auth fix. npm still ships pre-fix `3.0.4`, so an + accidental switch from the `/opt` clone to an npm install would be a silent + regression. Cheap guard: `grep -rl availability_recheck` must be ≥1. +- The feeder's new `pi@` default has no behavioural test hook + (`--dry-run` never prints the agent; `--self-test` only covers the remote-mine + response classifier). Cheapest available check is a source-shape grep for + `MEMPALACE_PI_DEVICE:+pi@`. + +--- + ## v1.8.4 — 2026-08-22 Patch release, and the one that ends an eight-week bug: **the baked diff --git a/Dockerfile.base b/Dockerfile.base index 89980c9..8ebfd73 100644 --- a/Dockerfile.base +++ b/Dockerfile.base @@ -686,12 +686,14 @@ COPY rootfs/usr/local/share/pi-devbox/ /usr/local/share/pi-devbox/ COPY rootfs/usr/local/bin/studio-expose /usr/local/bin/studio-expose COPY rootfs/usr/local/bin/dot-watch /usr/local/bin/dot-watch COPY rootfs/usr/local/bin/pi-devbox-version /usr/local/bin/pi-devbox-version +COPY rootfs/usr/local/bin/devbox-skill-reconcile /usr/local/bin/devbox-skill-reconcile COPY entrypoint.sh /usr/local/bin/entrypoint.sh COPY entrypoint-user.sh /usr/local/bin/entrypoint-user.sh RUN chmod +x /usr/local/bin/entrypoint.sh /usr/local/bin/entrypoint-user.sh \ /usr/local/bin/studio-expose \ /usr/local/bin/dot-watch \ /usr/local/bin/pi-devbox-version \ + /usr/local/bin/devbox-skill-reconcile \ /usr/local/lib/pi-devbox/*.sh 2>/dev/null || true # Start as root — entrypoint adjusts UID/GID then drops to developer diff --git a/README.md b/README.md index 379a883..7b3b95e 100644 --- a/README.md +++ b/README.md @@ -547,7 +547,8 @@ directory, and they compose: `~/.agents/skills/` by `entrypoint-user.sh` on every start. They need no external mount, survive volume recreate (the source is an image path, not a home dir a named volume would shadow), and are created only when absent so a - same-named skillset skill or user override is never clobbered. The bundled + user override is never clobbered. Precedence against a mounted `skillset` repo + is per-skill, not blanket — see *Skillset repo* below. The bundled **`pi-devbox-environment`** skill is delivered this way — it teaches agents the container's persistence model, host/LAN SSH reachability, split-DNS mechanisms, the interactive-vs-tool-shell alias gotcha (`dssh`/`dscp`), @@ -558,8 +559,11 @@ directory, and they compose: pi session to read `~/.agents/skills/pi-extensions/SKILL.md` at start (to fix fork/recall under-utilisation). That pointer would dangle in a container started *without* the private `skillset` repo, so the image also bakes - fallback copies of **`pi-extensions`** and **`mempalace`**. They are - symlinked only when absent, so a mounted skillset always overrides them. The + fallback copies of **`pi-extensions`** and **`mempalace`**. Whether a mounted + skillset overrides them depends on who *owns* the skill (see *Skillset repo*): + `mempalace` is skillset-owned, so the live clone wins; `pi-extensions` is + owned by its package repo, so the baked copy keeps winning — the skillset's + copy of it is a downstream duplicate that can lag. The `pi-extensions` skill is *layered*: a committed snapshot in `rootfs/` is the floor, and `Dockerfile.variant` copies the canonical, package-owned copy from the pinned `pi-extensions` clone (`/opt/pi-extensions/skill/`) over it at @@ -573,7 +577,16 @@ directory, and they compose: - **Skillset repo (optional).** If a `skillset` repo is mounted (at `$HOME/skillset` or `/workspace/skillset`, or via `SKILLSET_CONTAINER_PATH`), `deploy-skills.sh` symlinks its skills in too. Image-baked skills are - classified as foreign-links by its `--prune-stale` pass and left untouched. + classified as foreign-links by its `--prune-stale` pass and left untouched — + which through v1.8.4 meant the baked copy *always* won, so an edit pushed to a + skillset-owned skill was invisible until the next image build. Since v1.8.5 + `devbox-skill-reconcile` runs right after the deploy and repoints the links for + skills the skillset owns, listed in + `/usr/local/share/pi-devbox/skills/skillset-owned.txt` (today: `mempalace`). + Effective precedence, highest first: **user override** (a real directory, or a + symlink pointing outside the baked tree) → **live skillset clone** (owned names + only) → **baked snapshot** (everything else, and every skill when no skillset + is mounted). Check with `readlink -f ~/.agents/skills/`. To make agents *proactively* load a baked skill at session start (rather than only on description match), the image appends a short, gated pointer to the diff --git a/entrypoint-user.sh b/entrypoint-user.sh index e546bb2..ddbc9b7 100755 --- a/entrypoint-user.sh +++ b/entrypoint-user.sh @@ -58,10 +58,17 @@ fi # the runtime skill-link assertion. Pointing at the image path (/usr/local/...) # keeps the skill fresh from the image and surviving volume recreate (unlike # anything baked under a home dir, which a named volume would shadow). Created -# only when absent, so a same-named skillset skill (deployed later, at the end -# of this script) or a user override is never clobbered; the skillset deploy -# classifies these as foreign-links and its --prune-stale pass leaves them -# alone (only dangling symlinks are pruned). +# only when absent, so a user override is never clobbered. +# +# NB: "created only when absent" does NOT hand a same-named skillset skill +# priority — the opposite. The skillset deploy runs at the end of this script +# and classifies these links as foreign, so through v1.8.4 the BAKED copy +# always won and an edit pushed to a skillset-owned skill was invisible until +# the next image build. The links below are therefore the FALLBACK only; +# devbox-skill-reconcile (invoked right after the skillset deploy) hands the +# skillset-OWNED skills back to the live clone. Ownership is per-skill, listed +# in skills/skillset-owned.txt — see VENDORED.md for why pi-extensions must +# keep losing to the baked copy. DEVBOX_SKILLS_SRC=/usr/local/share/pi-devbox/skills if [ -d "$DEVBOX_SKILLS_SRC" ]; then mkdir -p "$HOME/.agents/skills" @@ -69,7 +76,16 @@ if [ -d "$DEVBOX_SKILLS_SRC" ]; then [ -d "$_sk" ] || continue _skname=$(basename "$_sk") if [ ! -e "$HOME/.agents/skills/$_skname" ]; then - ln -s "${_sk%/}" "$HOME/.agents/skills/$_skname" + # -sfn, not -s: `[ ! -e ]` is TRUE for a DANGLING symlink (-e follows the + # link), and since v1.8.5 these links can point into /workspace/skillset + # (see devbox-skill-reconcile, invoked after the skillset deploy). If that + # mount vanishes while the writable layer survives — a `docker restart` or + # a host reboot under restart: unless-stopped, as opposed to a recreate — + # plain `ln -s` fails with "File exists" and, under `set -e`, aborts + # container start before `exec "$@"`. With -f the broken link heals back to + # the baked fallback, and the reconciler re-points it in the same boot if + # the clone is back. + ln -sfn "${_sk%/}" "$HOME/.agents/skills/$_skname" fi done fi @@ -385,6 +401,13 @@ elif [ -x /workspace/skillset/deploy-skills.sh ]; then fi if [ -n "$SKILLSET_DEPLOY" ]; then "$SKILLSET_DEPLOY" --bootstrap --prune-stale >/dev/null 2>&1 || true + # The deploy leaves the early baked links (above) in place as foreign links, + # which silently shadows the live clone for skills the skillset OWNS. Repoint + # just those; baked stays the fallback, user overrides still win. `|| true`: + # a skill-link refinement must never break container start. + if command -v devbox-skill-reconcile >/dev/null 2>&1; then + devbox-skill-reconcile "$(dirname "$SKILLSET_DEPLOY")" || true + fi fi # ── Execute command ────────────────────────────────────────────────── diff --git a/rootfs/usr/local/bin/devbox-skill-reconcile b/rootfs/usr/local/bin/devbox-skill-reconcile new file mode 100755 index 0000000..79faccf --- /dev/null +++ b/rootfs/usr/local/bin/devbox-skill-reconcile @@ -0,0 +1,91 @@ +#!/bin/sh +# devbox-skill-reconcile — hand skillset-OWNED skills back to the live clone. +# +# WHY THIS EXISTS +# --------------- +# entrypoint-user.sh links the image-baked skills into ~/.agents/skills/ EARLY +# (before pi-deploy), because the smoke readiness probe gates on markers that +# only land later, and a link created after that gate produced a flaky +# assertion. Those links are created with a `[ ! -e ]` guard — "only when +# absent" — and the skillset deploy runs LAST, treating already-present links +# as foreign and leaving them alone. Net effect through v1.8.4: the baked copy +# always won, so an edit pushed to a skillset-owned skill was invisible in +# every container until the next image build (measured on two hosts: live +# skillset md5 129bcc4752 vs baked 5236024fef, the new section absent). +# +# The fix is NOT "the skillset always wins". Ownership is per-skill (see +# rootfs/usr/local/share/pi-devbox/skills/VENDORED.md): +# +# pi-devbox-environment authored in pi-devbox → baked IS canonical +# pi-extensions owned by the package repo, copied over the snapshot +# at build time; skillset carries a DOWNSTREAM copy +# that can lag → baked must keep winning +# mempalace owned by the skillset repo; baked is a snapshot +# fallback for containers with no skillset mounted +# → the live clone must win when it is present +# +# So only skills listed in skills/skillset-owned.txt are handed over. Baked +# links stay as the fallback (the early-link race fix is untouched), and a user +# override always beats both: a real directory is never replaced, and neither is +# a symlink that already points somewhere other than the baked tree. +# +# Usage: devbox-skill-reconcile [skills-dir] [baked-src] +# skillset-root the mounted skillset repo (contains skills//) +# skills-dir default $HOME/.agents/skills +# baked-src default /usr/local/share/pi-devbox/skills +# +# Idempotent, and silent unless it changes something. Exits 0 when there is +# nothing to do (no skillset, no list) so the entrypoint never fails on it. +set -eu + +SKILLSET_ROOT="${1:-}" +SKILLS_DIR="${2:-$HOME/.agents/skills}" +BAKED_SRC="${3:-/usr/local/share/pi-devbox/skills}" +BAKED_SRC="${BAKED_SRC%/}" # a trailing slash would make the prefix + # match below ("$BAKED_SRC"/*) match nothing + +[ -n "$SKILLSET_ROOT" ] || exit 0 +[ -d "$SKILLSET_ROOT/skills" ] || exit 0 +[ -d "$SKILLS_DIR" ] || exit 0 + +# Absolutise BOTH roots before they are used, because each has its own way of +# failing silently when relative: a relative symlink TARGET is resolved against +# the link's directory (~/.agents/skills), not $PWD, so it would dangle on +# creation; and a relative BAKED_SRC would never prefix-match the absolute +# target that `readlink` reports, so every skill would be skipped and the fix +# would look like it had simply done nothing. +SKILLSET_ROOT=$(CDPATH= cd -- "$SKILLSET_ROOT" 2>/dev/null && pwd) || exit 0 +BAKED_SRC=$(CDPATH= cd -- "$BAKED_SRC" 2>/dev/null && pwd) || exit 0 +OWNED_LIST="$BAKED_SRC/skillset-owned.txt" +[ -f "$OWNED_LIST" ] || exit 0 + +while IFS= read -r _line || [ -n "$_line" ]; do + # strip comments and surrounding whitespace; skip blanks + _name=$(printf '%s\n' "$_line" | sed -e 's/#.*$//' -e 's/^[[:space:]]*//' -e 's/[[:space:]]*$//') + [ -n "$_name" ] || continue + # defensive: a list entry must be a plain skill name, never a path + case "$_name" in */*|.*) continue ;; esac + + _live="$SKILLSET_ROOT/skills/$_name" + _link="$SKILLS_DIR/$_name" + + # the skillset does not ship it → the baked fallback is all there is + [ -d "$_live" ] || continue + # a real directory is a user override → never touch + [ -L "$_link" ] || continue + + # only ever replace OUR OWN link. readlink is deliberate: `readlink -f` + # would resolve a link that already points into the skillset clone and, + # since both trees hold a same-named skill, could not tell them apart. + _target=$(readlink "$_link" 2>/dev/null || true) + case "$_target" in + "$BAKED_SRC"/*|"$BAKED_SRC") ;; # baked link → ours to replace + *) continue ;; # user/foreign target → leave alone + esac + + # -n so an existing symlink-to-directory is replaced rather than followed + # (without it, ln would create $_link/$_name inside the baked tree). + if ln -sfn "$_live" "$_link" 2>/dev/null; then + printf 'skill %s: baked snapshot -> live skillset (%s)\n' "$_name" "$_live" + fi +done < "$OWNED_LIST" diff --git a/rootfs/usr/local/share/pi-devbox/skills/VENDORED.md b/rootfs/usr/local/share/pi-devbox/skills/VENDORED.md index 17d952a..ac9c3f6 100644 --- a/rootfs/usr/local/share/pi-devbox/skills/VENDORED.md +++ b/rootfs/usr/local/share/pi-devbox/skills/VENDORED.md @@ -1,9 +1,10 @@ # Vendored fallback skills Most directories here are **image-baked skills** that `entrypoint-user.sh` -symlinks into `~/.agents/skills/` on container start (only when a skill of the -same name is not already present, so a mounted `skillset` repo or a user -override always wins). +symlinks into `~/.agents/skills/` on container start. They are the **fallback** +layer: see *Runtime precedence* below for which copy actually wins when a +`skillset` repo is mounted (through v1.8.4 the answer was "always the baked +one", which was a bug). | skill | owner | how it gets here | |-------|-------|------------------| @@ -38,6 +39,35 @@ 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. +## Runtime precedence (v1.8.5+) + +The baked links are created **early** in `entrypoint-user.sh` (before pi-deploy, +to close a smoke readiness race) with a create-only-when-absent guard, and the +skillset deploy runs **last** and treats them as foreign links. Through v1.8.4 +that combination meant the baked snapshot always won: an edit pushed to +`skillset/skills/mempalace/SKILL.md` was invisible in every container until the +next image build (measured on two hosts — live `md5 129bcc4752` vs baked +`5236024fef`, new section absent). Editing those skills *appeared* to work. + +`devbox-skill-reconcile` now runs immediately after the skillset deploy and +repoints the links for skills the **skillset owns**, listed one per line in +`skillset-owned.txt`. Precedence, highest first: + +1. **user override** — a real directory, or a symlink pointing outside the baked + tree; never touched by anything +2. **live skillset clone** — but only for names in `skillset-owned.txt` +3. **baked snapshot** — everything else, and every skill when no skillset is + mounted + +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 +skill. Only `mempalace` is skillset-owned today. + +Verify with `readlink -f ~/.agents/skills/` — not by reading the +entrypoint. Smoke covers both directions (baked resolution with no skillset +mounted, plus a fabricated-skillset run of the reconciler). + ## Refreshing the snapshots cp /skill/SKILL.md pi-extensions/SKILL.md @@ -50,4 +80,9 @@ also carries a copy, but it is a downstream duplicate and can lag), and `mempalace` from `skillset`. Copying `pi-extensions` from `skillset` would regress the snapshot to whatever that repo last mirrored. -Snapshot provenance at last refresh: skillset `936fed8`, pi-extensions pkg `e73cb9f`. +Snapshot provenance at last refresh: skillset `670f7f1`, pi-extensions pkg `e73cb9f`. + +When you refresh the `mempalace` snapshot, also update the phrase asserted by +the "mempalace skill snapshot is current" smoke test — it deliberately pins the +**newest** section, because the previous canary grepped a phrase that survived +the very edit that made the snapshot stale, and so passed on stale content. diff --git a/rootfs/usr/local/share/pi-devbox/skills/mempalace/SKILL.md b/rootfs/usr/local/share/pi-devbox/skills/mempalace/SKILL.md index e97e6eb..3f809d2 100644 --- a/rootfs/usr/local/share/pi-devbox/skills/mempalace/SKILL.md +++ b/rootfs/usr/local/share/pi-devbox/skills/mempalace/SKILL.md @@ -293,6 +293,7 @@ Zechner's pi-coding-agent). Implications: When the palace is **central** (shared across machines), five more things apply: - **Check which machine a conversation came from.** Transcripts are fed per device, so `source_path` reads `…/mempalace-feed//pi_.jsonl` while the displayed `source_file` is only the basename. One search can legitimately return hits from several machines at once — look at the device segment before attributing a decision to *this* project. +- **Attribute what you file yourself.** Drawers now carry `device` and `agent_kind` metadata (plus `device_source`/`agent_kind_source` recording *how* each was determined, so an inference is never mistaken for a fact). Mined content gets these for free — the inbox path gives the device, the filename shape gives the harness — and a timer on the palace host re-stamps hourly, because live re-mining replaces metadata rows and silently drops earlier stamps. But for anything **you** file by hand, the only signal is what you pass: set `added_by="@"` (e.g. `pi@emb-7kj4vr4g`, from `$MEMPALACE_PI_DEVICE`) on `add_drawer`/`checkpoint`/`mine`. Skip it and your drawer joins the ~16k historic `/workspace` project mines that are permanently unattributable, because `/workspace` exists identically on every devbox. Note the palace preserves `source_file` in full (see `source_path`) but *displays* only the basename — so a device prefix there survives storage even though it looks stripped. - **Mined drawers carry the MINE date, not the session date.** When history is imported, or re-mined on the palace host, `filed_at`/`created_at` is the *import* time — so sorting by them does not give chronological order. Real session time is recoverable from the UUIDv7 in `pi_.jsonl`: the first 12 hex digits are milliseconds since the epoch (and UUIDv7 sorts lexicographically in time order, so a plain filename sort is already chronological). Agent-authored drawers and diaries have no such backdoor — for those `filed_at` is the only chronology, which is why it must never be restamped. - **Beware the timezone mismatch when you combine those.** Palace `filed_at`/`created_at` are naive timestamps in the palace host's local time, while a UUIDv7 decodes to UTC. Comparing them directly introduces a silent offset (2 h for a CEST host). Normalise before drawing conclusions about ordering. - **`agent_name` is not device-scoped.** `mempalace_diary_read(agent_name="pi")` returns *every* machine's `pi` diary, interleaved. Read the entry before assuming it is your own history. diff --git a/rootfs/usr/local/share/pi-devbox/skills/skillset-owned.txt b/rootfs/usr/local/share/pi-devbox/skills/skillset-owned.txt new file mode 100644 index 0000000..0362e49 --- /dev/null +++ b/rootfs/usr/local/share/pi-devbox/skills/skillset-owned.txt @@ -0,0 +1,16 @@ +# Skills in this directory whose OWNER is the skillset repo. +# +# Read by devbox-skill-reconcile, which runs after the skillset deploy in +# entrypoint-user.sh: for each name below, if the mounted skillset ships a +# skill of that name, the baked link in ~/.agents/skills/ is repointed at the +# live clone. The baked copy remains the fallback for containers started +# WITHOUT a skillset mount, and a user override always wins over both. +# +# Add a name here ONLY if the skillset repo is the authoritative source (see +# the ownership table in VENDORED.md). Do NOT add: +# pi-devbox-environment — authored in this repo; baked IS canonical +# pi-extensions — owned by the pi-extensions package repo and copied +# over the snapshot at build time; the skillset copy +# is a downstream duplicate that can lag, so letting +# it win would regress the skill. +mempalace diff --git a/scripts/smoke-test.sh b/scripts/smoke-test.sh index fab4522..b743a9d 100755 --- a/scripts/smoke-test.sh +++ b/scripts/smoke-test.sh @@ -235,6 +235,16 @@ run "image-baked mempalace fallback skill" \ # baked copy must be the fresh package copy (Option 1), not the stale snapshot. run "pi-extensions skill refreshed from package when present" \ "if [ -f /opt/pi-extensions/skill/SKILL.md ]; then cmp -s /opt/pi-extensions/skill/SKILL.md /usr/local/share/pi-devbox/skills/pi-extensions/SKILL.md; else true; fi" +# Runtime ownership handover (v1.8.5): the baked links are a FALLBACK, and +# skillset-OWNED skills must be repointed at the live clone when one is mounted. +# The list is data, so assert its content, not just its presence: mempalace in, +# pi-extensions deliberately out (its skillset copy is a lagging duplicate). +run "devbox-skill-reconcile helper present + executable" \ + "test -x /usr/local/bin/devbox-skill-reconcile" +run "skillset-owned list ships and names mempalace" \ + "grep -qx 'mempalace' /usr/local/share/pi-devbox/skills/skillset-owned.txt" +run "skillset-owned list excludes pi-extensions (ownership)" \ + "! grep -qx 'pi-extensions' /usr/local/share/pi-devbox/skills/skillset-owned.txt" # ── tmux 0-indexing (required for pi-studio variants) ───────────────── echo "" @@ -369,10 +379,50 @@ exec_test "pi-devbox-environment skill linked" 'test -L $HOME/.agents/skills exec_test "pi-extensions skill linked (fallback)" 'test -L $HOME/.agents/skills/pi-extensions && test -f $HOME/.agents/skills/pi-extensions/SKILL.md && echo ok' exec_test "mempalace skill linked (fallback)" 'test -L $HOME/.agents/skills/mempalace && test -f $HOME/.agents/skills/mempalace/SKILL.md && echo ok' # The vendored mempalace snapshot is refreshed MANUALLY per release (see -# rootfs/usr/local/share/pi-devbox/skills/VENDORED.md). It silently shadows the -# skillset copy in a devbox container, so a stale snapshot is invisible: assert -# the multi-machine shared-palace guidance is actually present, not just the file. -exec_test "mempalace skill snapshot is current" 'grep -q "Shared palace: multiple harnesses" $HOME/.agents/skills/mempalace/SKILL.md && echo ok' +# rootfs/usr/local/share/pi-devbox/skills/VENDORED.md). Through v1.8.4 it also +# silently SHADOWED the live skillset copy, so staleness was invisible — and the +# canary that was supposed to catch it could not: it grepped "Shared palace: +# multiple harnesses", a phrase present in BOTH the stale and the fresh copy. +# A snapshot canary must pin the NEWEST section, so update this string whenever +# the snapshot is refreshed — that is the point of it. +exec_test "mempalace skill snapshot is current" 'grep -q "Attribute what you file yourself" $HOME/.agents/skills/mempalace/SKILL.md && 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. +exec_test "vendored skills resolve to the baked tree (no skillset mounted)" \ + 'for s in mempalace pi-extensions pi-devbox-environment; do + case "$(readlink -f $HOME/.agents/skills/$s)" in + /usr/local/share/pi-devbox/skills/$s) ;; + *) echo "$s resolves to $(readlink -f $HOME/.agents/skills/$s)" >&2; exit 1 ;; + esac + done; echo ok' +# 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 — +# owned skill repointed, unowned skill left baked, user override untouched. +exec_test "reconciler: owned skill handed to live clone, others untouched" \ + 'set -e; t=$(mktemp -d); mkdir -p $t/ss/skills/mempalace $t/ss/skills/pi-extensions $t/skills + echo LIVE > $t/ss/skills/mempalace/SKILL.md; echo LIVE > $t/ss/skills/pi-extensions/SKILL.md + ln -s /usr/local/share/pi-devbox/skills/mempalace $t/skills/mempalace + ln -s /usr/local/share/pi-devbox/skills/pi-extensions $t/skills/pi-extensions + mkdir -p $t/skills/mine; echo MINE > $t/skills/mine/SKILL.md + devbox-skill-reconcile $t/ss $t/skills >/dev/null + devbox-skill-reconcile $t/ss $t/skills >/dev/null # idempotent + [ "$(readlink $t/skills/mempalace)" = "$t/ss/skills/mempalace" ] || { echo "owned skill NOT repointed" >&2; exit 1; } + [ "$(readlink $t/skills/pi-extensions)" = /usr/local/share/pi-devbox/skills/pi-extensions ] || { echo "unowned skill was repointed" >&2; exit 1; } + [ "$(cat $t/skills/mine/SKILL.md)" = MINE ] || { echo "user override clobbered" >&2; exit 1; } + rm -rf $t; echo ok' +# The case above cannot fail if the reconciler stops checking WHERE a link +# points — a mutation test showed all three of its assertions still passing with +# that guard deleted, which is the same false-green shape as the old snapshot +# canary. This one discriminates: an OWNED name (so it is considered) whose link +# is a user override pointing outside the baked tree (so it must be left alone). +exec_test "reconciler: user override on an owned name is left alone" \ + 'set -e; t=$(mktemp -d); mkdir -p $t/ss/skills/mempalace $t/skills $t/mine-skill + echo LIVE > $t/ss/skills/mempalace/SKILL.md; echo USERLINK > $t/mine-skill/SKILL.md + ln -sfn $t/mine-skill $t/skills/mempalace + devbox-skill-reconcile $t/ss $t/skills >/dev/null + [ "$(cat $t/skills/mempalace/SKILL.md)" = USERLINK ] || { echo "user symlink override clobbered" >&2; exit 1; } + rm -rf $t; echo ok' # mempalace-census gained a /usr/local/bin symlink in v1.8.3; its three siblings # had one since they were added, so this asserts the set stays complete. exec_test "mempalace-census on PATH" 'command -v mempalace-census >/dev/null && mempalace-census --help >/dev/null && echo ok'