joakimp
b5810654f6
Baked skill links won over the live skillset clone for all three vendored skills, so a pushed edit to skills/mempalace/SKILL.md was invisible in every container until the next image build -- measured on two hosts (live md5 129bcc4752 vs baked 5236024fef). Cause was ordering, not intent: the baked links are created early with a create-only-when-absent guard to close a smoke readiness race, and the skillset deploy runs last and treats them as foreign. The comment claimed the opposite of the behaviour. The fix is not "skillset always wins". Ownership is per-skill: pi-extensions is owned by its package repo and copied over the snapshot at build time, so the skillset's lagging duplicate must keep losing; pi-devbox-environment is authored here. Only mempalace is skillset-owned. devbox-skill-reconcile therefore runs after the deploy and repoints only the names in skills/skillset-owned.txt, replacing a link solely when it points into the baked tree, so a real directory or a link pointing elsewhere is never disturbed. Precedence is now user override -> live clone (owned names) -> baked snapshot, with the early links intact as the fallback so the readiness race stays closed. Reviewing that turned up a latent boot-abort in the pre-existing baked-link block: `[ ! -e "$link" ]` is TRUE for a dangling symlink, so once a link can point into /workspace/skillset, a vanished mount makes plain `ln -s` fail with "File exists" -- and under `set -euo pipefail` that aborts container start before `exec "$@"`. Reachable on `docker restart` or a host reboot, not on a recreate, since ~/.agents is not a volume on any host. Now `ln -sfn`, which heals the link back to the baked fallback. Smoke additions cover what let this ship: the stale-snapshot canary grepped a phrase present in BOTH the stale and fresh copies, so it passed throughout; it now pins the newest section. Link targets are asserted, not just `test -L`; the owned-list content is asserted both ways; and the reconciler's replace path -- which no CI container exercises, since none mounts a skillset -- is covered by fabricating one. A mutation test showed the obvious three assertions still pass with the "is this link ours?" guard deleted, so a discriminating case was added: an owned name whose link is a user override outside the baked tree. Also refreshes the mempalace snapshot to skillset 670f7f1 (without it the fix helps only hosts that mount skillset) and corrects README, which documented the old, wrong precedence in three places. Verified with 12 fixture cases plus 2 mutants: ownership respected against the real trees, user overrides preserved, relative/trailing-slash/CRLF/space/glob inputs handled, dangling link healed, read-only skills dir exits 0, idempotent.
92 lines
4.5 KiB
Bash
Executable File
92 lines
4.5 KiB
Bash
Executable File
#!/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 <skillset-root> [skills-dir] [baked-src]
|
|
# skillset-root the mounted skillset repo (contains skills/<name>/)
|
|
# 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"
|