joakimp/pi-devbox

Fork 0

Files

T

joakimp e8ddeaf89f

Lint / hadolint (push) Successful in 9s

Details

Lint / actionlint (push) Successful in 1m39s

Details

skills: a gate that could pass without checking, and a mailbox that never empties

Fixes the three blockers and seven should-fixes from pi@emb-7kj4vr4g's review
(logstream correlation skills-provenance-review, full text in
drawer_pi-devbox_reviews_e43e766641c9ec85217bc6ce). Every finding was
reproduced by execution here before being fixed; two were refined by that
reproduction rather than taken as given.

BLOCKER 1 — the provenance gate could print OK and exit 0 without verifying.
`git show <ref>:<path> | sha256sum` hashes EMPTY STDIN when the ref does not
resolve, so at_ref was never empty and the UNKNOWN branch was dead code.
Measured: a bogus ref reported MISMATCH — accusing the snapshot of lying when
the real cause was an incomplete clone, and the operator's natural remedy for
MISMATCH is to re-run the refresh, which rewrites provenance to silence the
complaint; and with a 0-byte snapshot against a 0-byte upstream file it printed
"OK: exactly skillset@aaaaaaa" with exit 0 for a ref that does not exist. The
script already had the sha_empty idiom and had applied it to blob_sha but not
to at_ref. Existence is now PROVEN with git cat-file -e before anything is
hashed, at two levels (ref resolves / path exists at it) because those deserve
different messages. Same defect class as the canary it replaces: a check that
can succeed without checking. A second, unflagged instance of the same pipeline
shape in blob_sha was found and fixed too.

Exit codes split, because the old contract failed the sanctioned case: 0
truthful (including stale, with a NOTICE), 1 a lying record only, 2 cannot
determine. AGENTS.md step 2 promised "the message distinguishes the two" and
was the thing this branch was breaking; rewritten to state all three.

BLOCKER 3 — VENDORED.md contradicted itself in the release whose stated
invariant is non-contradiction: its hand-maintained provenance line named
skillset 670f7f1, seven commits behind the ARG and itself the commit that told
agents to hand-stamp added_by — the withdrawn instruction this work exists to
stop shipping — while its cp recipe contradicted the "not cp" rule 20 lines
above. Line removed (nothing forced it to move when the ARGs did); 670f7f1 kept
only as a labelled cautionary example. The pi-extensions half was verified
redundant (CI require_sha resolves PI_EXTENSIONS_REF) before removal.

SHOULD-FIXES: `<root> --check`, the spelling VENDORED.md documented, silently
ran a REFRESH because only $1 was parsed (both tools now parse all args and
reject unknown ones); refresh at a detached/older HEAD silently rewound ref and
bytes (now refused unless the recorded ref is an ancestor, --force to override);
upstream_dirty was computed and never used in check mode; --no-skills --json
printed human text and broke jq; --help was a hardcoded sed range this branch
had already made stale; the fingerprint hashed SKILL.md alone so a live skill
differing only in a sibling file reported "identical", and pi-extensions already
ships two files, so it is now a per-skill TREE hash with the manifest field
renamed skillset_snapshot_tree_sha256; the --no-skills smoke assertion was
negative-only and passed on a crashed binary. mktemp+mv left files 0600 — CI was
unaffected since the index records 100644, so the blast radius was local builds
only, narrower than the review inferred.

Snapshot resynced c04cd15 -> 5fd0d5c so the no-clone fallback carries the
CORRECTED coordination protocol rather than the withdrawn one; --check is now OK
with no staleness notice, and the bidirectional canary re-verified against the
new bytes. Local validation is bash -n only (shellcheck, hadolint and actionlint
are all absent in this container) — CI remains the shellcheck gate.

2026-08-26 14:38:07 +02:00

8.7 KiB

Raw Blame History

Vendored fallback skills

Most directories here are image-baked skills that entrypoint-user.sh 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
`pi-devbox-environment`	pi-devbox (this repo)	authored here; the canonical copy
`pi-extensions`	the `pi-extensions` package repo (`skill/`)	vendored fallback + refreshed at build
`mempalace`	the `skillset` repo	vendored fallback (snapshot only)

Why fallbacks exist

The pi-toolkit global AGENTS.md tells every pi session to read ~/.agents/skills/pi-extensions/SKILL.md at start (to fix fork/recall under-utilisation). That pointer dangles in a container started without the private skillset repo mounted. Baking the skill closes that availability gap. mempalace is baked for the same reason (memory continuity); since nothing in pi-toolkit's AGENTS.md points to it, the pi-devbox managed block (pi-global-AGENTS.append.md) also adds the matching proactive-load directive ("load the mempalace skill at session start") so a new container actually picks it up rather than relying on description-matching. pi-extensions's directive already ships in pi-toolkit's AGENTS.md, so only its skill file needed baking.

Freshness model (layered — see Dockerfile.variant)

pi-extensions — Option 1 + Option 2. The committed copy here is the floor; at build time Dockerfile.variant copies /opt/pi-extensions/skill/ (the pinned, package-owned source) over it, so a normal build ships the fresh package copy and a stale-ref / mirror build still ships the snapshot. Keep evaluate-extension-usage.py alongside SKILL.md — the skill calls it via ./.

mempalace — Option 2 only. The mempalace consumer skill lives only in the private skillset repo (the mempalace-toolkit repo ships a 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 <skillset-root>, 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, 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:

user override — a real directory, or a symlink pointing outside the baked tree; never touched by anything
live skillset clone — but only for names in skillset-owned.txt
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 <repo> @ <sha> — 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 skill. Only mempalace is skillset-owned today.

Verify with readlink -f ~/.agents/skills/<skill> — 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 <pi-extensions-pkg>/skill/SKILL.md                    pi-extensions/SKILL.md
cp <pi-extensions-pkg>/skill/evaluate-extension-usage.py pi-extensions/

Copy pi-extensions from its owner in the table above — the package repo's skill/ (since a7f3044 co-located it there; skillset also carries a copy, but it is a downstream duplicate and can lag). Copying pi-extensions from skillset would regress the snapshot to whatever that repo last mirrored.

mempalace is not refreshed by cp — see the Freshness model section above: scripts/vendor-mempalace-skill.sh <skillset-root> is the only thing that should ever touch that snapshot, because a bare copy can update the bytes without updating the ref that claims to describe them, which produces a manifest that confidently lies.

Neither vendored skill has a hand-maintained "last refreshed at" line here on purpose — one previously existed (skillset 670f7f1, pi-extensions pkg e73cb9f) and went stale within hours, because nothing forced it to move when the ARGs did. 670f7f1 is now a cautionary example rather than a fact worth recording: it is the commit that told agents to hand-stamp added_by, which a later skillset commit (and the pi-devbox edge stamper) withdrew — so a reader trusting that line would have been pointed at superseded guidance. Both facts it tried to capture now live somewhere that cannot drift by hand:

Fact	Where
which skillset commit `mempalace`'s bytes came from	`ARG SKILLSET_SNAPSHOT_REF` (Dockerfile.variant) + `skillset_snapshot_ref` in `build-manifest.json`, written only by `vendor-mempalace-skill.sh`
which pi-extensions package commit was vendored	`ARG PI_EXTENSIONS_REF` (Dockerfile.variant, CI-resolved to a 40-hex commit) → OCI label `se.jordbo.pi-devbox.pi-extensions-ref` and `build-manifest.json`'s `components.pi-extensions`, both read from the actual `/opt/pi-extensions` checkout, not from intent

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.

8.7 KiB Raw Blame History