From 424d22cf0f7162cf8907c62f3038a27b8753c729 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 30 Aug 2026 13:56:04 -0700 Subject: [PATCH] fix(skills): safely repair nested self-links Preserve real skill directories and files during ordinary sync. Add an explicit allowlisted nested-self-link repair that validates the entire batch before unlinking and reports partial progress on later failures. Detect this loop topology in the read-only fleet audit, document the scoped owner workflow, and cover preservation, refusals, idempotence, and recurrence with isolated HOME fixtures. --- .github/workflows/ci.yml | 8 + CHANGELOG.md | 1 + README.md | 16 + scripts/sync-skills | 139 +++++- scripts/test-sync-skills | 425 ++++++++++++++++++ skills/fleet-maintenance/SKILL.md | 2 +- .../references/fleet-schema.md | 6 +- .../scripts/agent-skill-links-audit.sh | 14 +- 8 files changed, 596 insertions(+), 15 deletions(-) create mode 100755 scripts/test-sync-skills diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a43cdc6..5b20465 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,6 +24,8 @@ jobs: - name: Check shell helper syntax run: | + bash -n scripts/sync-skills + bash -n scripts/test-sync-skills find skills -path '*/scripts/*' -type f -print0 | while IFS= read -r -d '' script; do if head -n 1 "$script" | grep -q 'bash'; then @@ -31,6 +33,12 @@ jobs: fi done + - name: Test skill link sync and audit + run: | + sudo apt-get update + sudo apt-get install -y ripgrep + /bin/bash scripts/test-sync-skills + - name: Test ClawSweeper status helper run: skills/clawsweeper-status/scripts/clawsweeper-status.test.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c8bbe9..bd35339 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ summary: Timeline of guardrail helper changes mirrored from Sweetistics and rela ## Unreleased +- Fixed skill sync nesting links inside locally owned directories; added bounded allowlisted self-link repair and read-only audit detection. - Removed the machine-specific 1Password skill from the public skill set; local discovery now uses its private owner. - Fixed `clawsweeper-status` aborting before activity sections on large workflow snapshots while preserving row caps and upstream errors. - Fixed `clawsweeper-status` public queue parsing, preserved optional health fields without shifted columns, and added a separate publication-tail summary. diff --git a/README.md b/README.md index 2cf0d08..2996879 100644 --- a/README.md +++ b/README.md @@ -30,6 +30,21 @@ Global discovery is built by `scripts/sync-skills` (idempotent; run on every Mac - Codex scans nested dirs, so it gets whole-root links: `~/.codex/skills/agent-scripts -> ~/Projects/agent-scripts/skills`, `~/.codex/skills/manager -> ~/Projects/manager/skills`. - Claude Code loads only `~/.claude/skills//SKILL.md` (exactly one level deep; per-entry symlinks are followed, category subfolders are not scanned — verified on 2.1.197). It gets a flat per-skill link mirror covering both repos plus machine-local `~/.codex/skills/` extras. - Name collisions resolve agent-scripts > manager > codex-local; the script prints skipped duplicates and prunes broken/stale managed links. +- Real destination files and directories are preserved. A real Claude skill directory with a Codex backlink to that same directory satisfies local ownership; other real destination conflicts are reported and make sync fail. + +For the specific legacy topology `~/.claude/skills/NAME/NAME -> ~/.codex/skills/NAME -> ~/.claude/skills/NAME`, invoke the sync owner directly with an explicit allowlist: + +```bash +/absolute/path/to/agent-scripts/scripts/sync-skills --repair-nested-self-links --dry-run -- boxd-cli boxd-setup-deploy +``` + +```bash +/absolute/path/to/agent-scripts/scripts/sync-skills --repair-nested-self-links -- boxd-cli boxd-setup-deploy +``` + +This mode validates every candidate before unlinking only the extra nested leaves. It preserves the real skill directories, assets, and valid Codex backlinks, and exits before creating roots, building mirrors, pruning, or touching instruction pointers. Names must start with an ASCII letter or digit and contain only letters, digits, `.`, `_`, or `-`; duplicates, missing names, and unknown arguments are rejected. A missing nested leaf is a no-op only with the expected surrounding topology. Redirected/inaccessible roots, unexpected objects or literal targets, and changed directory/link identities cause refusal. Rechecks before each unlink are not atomic concurrency protection; a later error stops the batch and reports removals already completed, without rollback. + +The read-only `skills/fleet-maintenance/scripts/agent-skill-links-audit.sh` reports same-name nested ancestor loops as `reason=nested-self-link`. This is narrow detection, not an exhaustive graph validator. Its `--repair` remains a broad sync through `~/Projects/agent-scripts/scripts/sync-skills`; it is not the scoped repair above. Run `scripts/test-sync-skills` for isolated fixture coverage; `scripts/test-sync-skills --recurrence-only /absolute/path/to/old-sync-skills` runs the unchanged recurrence assertion against an original helper. Shared personal skills live as real folders in `skills/`. Public OpenClaw shared skills live in `../agent-skills` and are exposed here with tracked relative symlinks. Repo-owned skills stay canonical in their repo and are exposed here the same way, for example: @@ -62,6 +77,7 @@ Repo-specific rules go below that pointer. Do not copy the shared blocks into do `scripts/sync-skills` - Builds the per-machine skill mirror: Codex whole-root links, Claude flat per-skill links, shared `AGENTS.MD` pointers. - Idempotent; prints changes only, prunes broken/stale managed links, never clobbers real files. +- No arguments runs ordinary sync; `--help` prints usage. Only the scoped `--repair-nested-self-links` mode accepts `--dry-run`. `scripts/validate-skills` - Checks every `skills/*/SKILL.md`. diff --git a/scripts/sync-skills b/scripts/sync-skills index cc4bdd4..957588e 100755 --- a/scripts/sync-skills +++ b/scripts/sync-skills @@ -8,17 +8,140 @@ # Collision priority: agent-scripts > manager > codex-local extras. set -eo pipefail +usage() { + printf 'usage: %s [--help | --repair-nested-self-links [--dry-run] -- NAME...]\n' "$0" +} + +repair=0 +dry_run=0 +if [ "$#" -gt 0 ]; then + case "$1" in + --help) [ "$#" -eq 1 ] || { usage >&2; exit 2; }; usage; exit 0 ;; + --repair-nested-self-links) + repair=1 + shift + if [ "${1:-}" = --dry-run ]; then dry_run=1; shift; fi + [ "${1:-}" = -- ] || { usage >&2; exit 2; } + shift + [ "$#" -gt 0 ] || { usage >&2; exit 2; } + repair_names=() + for name in "$@"; do + case "$name" in + ''|[!a-zA-Z0-9]*|*[!a-zA-Z0-9._-]*) + printf 'invalid skill basename: %s\n' "$name" >&2; exit 2 ;; + esac + for previous in "${repair_names[@]}"; do + [ "$name" != "$previous" ] || { printf 'duplicate skill name: %s\n' "$name" >&2; exit 2; } + done + repair_names+=("$name") + done + ;; + *) usage >&2; exit 2 ;; + esac +fi + AGENT_SKILLS="$HOME/Projects/agent-scripts/skills" MANAGER_SKILLS="$HOME/Projects/manager/skills" CODEX_ROOT="$HOME/.codex/skills" CLAUDE_ROOT="$HOME/.claude/skills" AGENTS_MD="$HOME/Projects/agent-scripts/AGENTS.MD" +# This mode exits before any ordinary mirror or instruction-pointer work. +if [ "$repair" -eq 1 ]; then + removed=0 + absent=0 + refuse() { + printf 'nested-self-links: refused: %s (removed=%s already-absent=%s)\n' "$*" "$removed" "$absent" >&2 + exit 1 + } + # BSD and GNU stat both default to the link itself, not its target. + if stat -c '%d:%i' "$HOME" >/dev/null 2>&1; then + stat_args=(-c '%d:%i') + else + stat_args=(-f '%d:%i') + fi + identity() { stat "${stat_args[@]}" "$1"; } + roots=("$HOME" "$HOME/.claude" "$HOME/.codex" "$CLAUDE_ROOT" "$CODEX_ROOT") + root_ids=() + check_roots() { + local i root physical current + for i in "${!roots[@]}"; do + root=${roots[$i]} + [ ! -L "$root" ] && [ -d "$root" ] && [ -r "$root" ] && [ -x "$root" ] || refuse "not an accessible real root: $root" + physical=$(cd "$root" && pwd -P) || refuse "cannot resolve root: $root" + [ "$physical" = "$root" ] || refuse "redirected root: $root" + current=$(identity "$root") || refuse "cannot identify root: $root" + [ "$current" = "${root_ids[$i]:-$current}" ] || refuse "root changed: $root" + root_ids[$i]=$current + done + } + check_candidate() { + local parent="$CLAUDE_ROOT/$1" backlink="$CODEX_ROOT/$1" nested="$CLAUDE_ROOT/$1/$1" target + [ ! -L "$parent" ] && [ -d "$parent" ] && [ -r "$parent" ] && [ -x "$parent" ] || refuse "not an accessible real skill directory: $parent" + [ -f "$parent/SKILL.md" ] && [ -r "$parent/SKILL.md" ] || refuse "missing or unreadable SKILL.md: $parent" + [ -L "$backlink" ] || refuse "not a Codex backlink: $backlink" + target=$(readlink "$backlink") || refuse "cannot read backlink: $backlink" + [ "$target" = "$parent" ] && [ "$parent" -ef "$backlink" ] || refuse "unexpected Codex backlink: $backlink" + candidate_state=absent + candidate_leaf_id=absent + if [ -L "$nested" ]; then + target=$(readlink "$nested") || refuse "cannot read nested link: $nested" + [ "$target" = "$backlink" ] && [ "$parent" -ef "$nested" ] && [ "$backlink" -ef "$nested" ] || refuse "unexpected nested link: $nested" + candidate_state=present + candidate_leaf_id=$(identity "$nested") || refuse "cannot identify nested link: $nested" + elif [ -e "$nested" ]; then + refuse "nested entry is not a symlink: $nested" + fi + candidate_parent_id=$(identity "$parent") || refuse "cannot identify skill directory: $parent" + candidate_backlink_id=$(identity "$backlink") || refuse "cannot identify backlink: $backlink" + } + check_roots + parent_ids=(); backlink_ids=(); leaf_ids=(); states=() + for i in "${!repair_names[@]}"; do + check_candidate "${repair_names[$i]}" + parent_ids[$i]=$candidate_parent_id + backlink_ids[$i]=$candidate_backlink_id + leaf_ids[$i]=$candidate_leaf_id + states[$i]=$candidate_state + done + for i in "${!repair_names[@]}"; do + name=${repair_names[$i]} + nested="$CLAUDE_ROOT/$name/$name" + # Recheck the roots and this candidate immediately before unlinking. These + # shell checks detect drift but cannot make pathname-based unlink atomic. + check_roots + check_candidate "$name" + [ "$candidate_parent_id" = "${parent_ids[$i]}" ] && + [ "$candidate_backlink_id" = "${backlink_ids[$i]}" ] && + [ "$candidate_leaf_id" = "${leaf_ids[$i]}" ] && + [ "$candidate_state" = "${states[$i]}" ] || refuse "topology changed: $name" + if [ "$candidate_state" = absent ]; then + absent=$((absent + 1)) + printf 'already-absent %s\n' "$nested" + elif [ "$dry_run" -eq 1 ]; then + printf 'would-remove %s\n' "$nested" + else + unlink "$nested" || refuse "unlink failed: $nested" + removed=$((removed + 1)) + printf 'removed %s\n' "$nested" + fi + done + [ "$dry_run" -eq 1 ] || printf 'nested-self-links: removed=%s already-absent=%s\n' "$removed" "$absent" + exit 0 +fi + changed=0 +conflicts=0 note() { printf '%s\n' "$*"; changed=1; } # link : create/retarget symlink, quiet when already right. link() { + if [ ! -L "$2" ] && [ -e "$2" ]; then + [ -d "$1" ] && [ -d "$2" ] && [ "$1" -ef "$2" ] && return 0 + printf 'WARN: real destination preserved: %s (expected -> %s)\n' "$2" "$1" >&2 + conflicts=$((conflicts + 1)) + return 0 + fi [ "$(readlink "$2" 2>/dev/null)" = "$1" ] && return 0 ln -sfn "$1" "$2" note "link $2 -> $1" @@ -87,17 +210,11 @@ for entry in "$CLAUDE_ROOT"/* "$CLAUDE_ROOT"/.[!.]*; do esac done -# --- Global instruction pointers; never clobber a real file. -ensure_md() { - if [ -e "$1" ] && [ ! -L "$1" ]; then - printf 'WARN: %s is a real file, not linking (expected -> %s)\n' "$1" "$AGENTS_MD" - return 0 - fi - link "$AGENTS_MD" "$1" -} -ensure_md "$HOME/.claude/CLAUDE.md" -ensure_md "$HOME/.claude/AGENTS.md" -ensure_md "$HOME/.codex/AGENTS.md" +# --- Global instruction pointers use the same real-destination protection. +link "$AGENTS_MD" "$HOME/.claude/CLAUDE.md" +link "$AGENTS_MD" "$HOME/.claude/AGENTS.md" +link "$AGENTS_MD" "$HOME/.codex/AGENTS.md" +[ "$conflicts" -eq 0 ] || { printf 'skills mirror has %s real destination conflict(s)\n' "$conflicts" >&2; exit 1; } [ "$changed" = 0 ] && echo "skills mirror up to date (${#names[@]} skills)" exit 0 diff --git a/scripts/test-sync-skills b/scripts/test-sync-skills new file mode 100755 index 0000000..09bf874 --- /dev/null +++ b/scripts/test-sync-skills @@ -0,0 +1,425 @@ +#!/bin/bash +# Standalone, disposable-HOME coverage. --recurrence-only accepts an unmodified +# old helper so the same assertion can prove the patch's regression provenance. +set -euo pipefail + +script_dir=$(cd "$(dirname "$0")" && pwd -P) +sync="$script_dir/sync-skills" +audit="$script_dir/../skills/fleet-maintenance/scripts/agent-skill-links-audit.sh" +recurrence_only=0 +if [ "$#" -gt 0 ]; then + if [ "$#" -ne 2 ] || [ "$1" != --recurrence-only ] || [ ! -f "$2" ]; then + printf 'usage: %s [--recurrence-only /absolute/path/to/sync-skills]\n' "$0" >&2 + exit 2 + fi + case "$2" in /*) sync=$2 ;; *) exit 2 ;; esac + recurrence_only=1 +fi +scratch=$(mktemp -d "${TMPDIR:-/tmp}/sync-skills-test.XXXXXX") +cleanup() { + local cleanup_exit_code=$? + trap - EXIT + case "$scratch" in + "${TMPDIR:-/tmp}"/sync-skills-test.*) + chmod -R u+rwX "$scratch" + rm -rf "$scratch" + ;; + *) printf 'refusing unexpected temporary path: %s\n' "$scratch" >&2 ;; + esac + exit "$cleanup_exit_code" +} +trap cleanup EXIT + +names=(boxd-cli boxd-setup-deploy boxd-setup-golden boxd-setup-preview boxd-setup-fix boxd-setup-hermes) +fail() { printf 'FAIL: %s\n' "$*" >&2; exit 1; } +contains() { rg -Fq -- "$1" "$output" || { cat "$output" >&2; fail "missing output: $1"; }; } +new_case() { + case_root="$scratch/$1" + mkdir -p "$case_root/home" + fixture_home=$(cd "$case_root/home" && pwd -P) + claude="$fixture_home/.claude/skills" + codex="$fixture_home/.codex/skills" + agent="$fixture_home/Projects/agent-scripts/skills" + manager="$fixture_home/Projects/manager/skills" + output="$case_root/output" +} +run_sync() { HOME="$fixture_home" /bin/bash "$sync" "$@"; } +run_audit() { HOME="$fixture_home" /bin/bash "$audit" "$@"; } +make_skill() { + mkdir -p "$1/assets" + printf '# fixture skill\n' >"$1/SKILL.md" + printf 'preserved asset\n' >"$1/assets/data" +} +base_fixture() { + mkdir -p "$claude" "$codex" "$agent" "$manager" + printf 'fixture instructions\n' >"$fixture_home/Projects/agent-scripts/AGENTS.MD" + ln -s "$agent" "$codex/agent-scripts" + ln -s "$manager" "$codex/manager" + ln -s "$fixture_home/Projects/agent-scripts/AGENTS.MD" "$fixture_home/.codex/AGENTS.md" + ln -s "$fixture_home/Projects/agent-scripts/AGENTS.MD" "$fixture_home/.claude/AGENTS.md" + ln -s "$fixture_home/Projects/agent-scripts/AGENTS.MD" "$fixture_home/.claude/CLAUDE.md" +} +six_skills() { + local name + for name in "${names[@]}"; do + make_skill "$claude/$name" + ln -s "$claude/$name" "$codex/$name" + if [ "$1" = loops ]; then ln -s "$codex/$name" "$claude/$name/$name"; fi + done +} +# lstat snapshots never follow links: preserve inode/device, mode, file hashes, +# and literal link targets, including hidden entries. Ignore directory mtimes. +snapshot() { + python3 - "$1" <<'PY' +import hashlib, json, os, stat, sys +root = sys.argv[1] +rows = {} +def visit(path): + info = os.lstat(path) + row = [info.st_dev, info.st_ino, info.st_mode] + if stat.S_ISLNK(info.st_mode): + row.append(os.readlink(path)) + elif stat.S_ISREG(info.st_mode): + with open(path, 'rb') as stream: + row.append(hashlib.sha256(stream.read()).hexdigest()) + rows[os.path.relpath(path, root)] = row + if stat.S_ISDIR(info.st_mode): + for name in sorted(os.listdir(path)): + visit(os.path.join(path, name)) +visit(root) +print(json.dumps(rows, sort_keys=True, indent=2)) +PY +} +unchanged() { + snapshot "$fixture_home" >"$case_root/after" + cmp "$case_root/before" "$case_root/after" || fail 'unexpected HOME mutation' +} +expect_rejected() { + local code=0 + snapshot "$fixture_home" >"$case_root/before" + "$@" >"$output" 2>&1 || code=$? + [ "$code" -ne 0 ] || fail "expected rejection: $*" + unchanged +} +expect_usage() { + local code=0 + snapshot "$fixture_home" >"$case_root/before" + "$@" >"$output" 2>&1 || code=$? + [ "$code" -eq 2 ] || fail "expected usage exit 2, got $code: $*" + unchanged +} +assert_registrations() { + local name + for name in "${names[@]}"; do + [ ! -L "$claude/$name" ] && [ -d "$claude/$name" ] || fail "lost real directory: $name" + [ -L "$codex/$name" ] && [ "$(readlink "$codex/$name")" = "$claude/$name" ] && + [ "$claude/$name" -ef "$codex/$name" ] || fail "lost Codex backlink: $name" + done +} +recurrence() { + new_case recurrence + base_fixture + six_skills clean + snapshot "$claude" >"$case_root/skills-before" + run_sync >"$output" 2>&1 + local name + for name in "${names[@]}"; do + [ ! -L "$claude/$name/$name" ] || fail "ordinary sync recreated nested self-link: $name" + done + snapshot "$claude" >"$case_root/skills-after" + cmp "$case_root/skills-before" "$case_root/skills-after" || fail 'ordinary sync changed locally owned skills' + assert_registrations + contains 'skills mirror up to date (6 skills)' + printf 'ordinary-sync recurrence: PASS\n' +} +recurrence +[ "$recurrence_only" -eq 0 ] || exit 0 + +new_case six-loop-lifecycle +base_fixture +six_skills loops +expect_rejected run_audit +[ "$(rg -c 'reason=nested-self-link' "$output")" -eq 6 ] || fail 'audit missed loops' +for registry in "$claude" "$codex"; do + code=0 + rg --files --follow --hidden -g SKILL.md "$registry" >"$case_root/rg-paths" 2>"$case_root/rg-errors" || code=$? + [ "$code" -eq 2 ] || fail 'rg did not report loops' + [ "$(rg -c 'loop' "$case_root/rg-errors")" -eq 6 ] || fail 'expected six rg loop errors' +done +snapshot "$fixture_home" >"$case_root/before" +run_sync --repair-nested-self-links --dry-run -- "${names[@]}" >"$output" +[ "$(rg -c '^would-remove ' "$output")" -eq 6 ] || fail 'dry-run operations' +[ "$(wc -l <"$output")" -eq 6 ] || fail 'dry-run extra output' +unchanged +run_sync --repair-nested-self-links -- "${names[@]}" >"$output" +contains 'nested-self-links: removed=6 already-absent=0' +[ "$(rg -c '^removed ' "$output")" -eq 6 ] || fail 'removal operations' +snapshot "$fixture_home" >"$case_root/after" +python3 - "$case_root/before" "$case_root/after" "${names[@]}" <<'PY' +import json, sys +before, after = (json.load(open(path)) for path in sys.argv[1:3]) +for name in sys.argv[3:]: + del before[f'.claude/skills/{name}/{name}'] +assert before == after, 'repair changed more than the six nested links' +PY +assert_registrations +run_audit >"$output" +contains 'status=current' +for registry in "$claude" "$codex"; do + rg --files --follow --hidden -g SKILL.md "$registry" >"$case_root/rg-paths" 2>"$case_root/rg-errors" + [ ! -s "$case_root/rg-errors" ] || fail 'rg errors after repair' +done +snapshot "$fixture_home" >"$case_root/before" +run_sync --repair-nested-self-links -- "${names[@]}" >"$output" +contains 'nested-self-links: removed=0 already-absent=6' +unchanged +run_sync --repair-nested-self-links --dry-run -- "${names[@]}" >"$output" +[ "$(rg -c '^already-absent ' "$output")" -eq 6 ] || fail 'dry-run missing no-ops' +unchanged +run_sync >"$output" +unchanged +run_audit >"$output" +printf 'six-loop lifecycle, snapshots and rg traversal: PASS\n' + +new_case scoped-preservation +base_fixture +six_skills loops +ln -s "$agent" "$claude/healthy" +ln -s "$fixture_home/missing" "$claude/broken" +ln -s "$agent" "$claude/.hidden-healthy" +ln -s "$fixture_home/missing" "$claude/.hidden-broken" +ln -s "$agent" "$codex/.hidden-healthy" +ln -s "$fixture_home/missing" "$codex/.hidden-broken" +ln -s "$agent" "$claude/boxd-cli/unrelated" +ln -s "$fixture_home/missing" "$claude/boxd-cli/.unrelated-broken" +printf 'nested file\n' >"$claude/boxd-cli/other-file" +# Scoped mode needs neither canonical repo nor shared instruction pointers. +unlink "$fixture_home/.claude/AGENTS.md" +printf 'real instructions\n' >"$fixture_home/.claude/AGENTS.md" +mv "$fixture_home/Projects" "$fixture_home/Projects-unavailable" +snapshot "$fixture_home" >"$case_root/before" +run_sync --repair-nested-self-links --dry-run -- boxd-cli >"$output" +unchanged +run_sync --repair-nested-self-links -- boxd-cli >"$output" +snapshot "$fixture_home" >"$case_root/after" +python3 - "$case_root/before" "$case_root/after" <<'PY' +import json, sys +before, after = (json.load(open(path)) for path in sys.argv[1:]) +del before['.claude/skills/boxd-cli/boxd-cli'] +assert before == after, 'scoped repair touched unrelated entries or instruction pointers' +PY +printf 'scoped preservation: PASS\n' + +new_case parsing +# Empty HOME proves invalid invocations cannot bootstrap the mirror. +expect_usage run_sync --unknown +expect_usage run_sync extra +expect_usage run_sync '' +expect_usage run_sync --dry-run +expect_usage run_sync --help extra +expect_usage run_sync --repair-nested-self-links +expect_usage run_sync --repair-nested-self-links -- +expect_usage run_sync --repair-nested-self-links --dry-run -- +expect_usage run_sync --repair-nested-self-links boxd-cli +expect_usage run_sync --repair-nested-self-links --dry-run --dry-run -- boxd-cli +expect_usage run_sync --repair-nested-self-links --unknown -- boxd-cli +expect_usage run_sync --repair-nested-self-links -- boxd-cli boxd-cli +for bad in '' . .. ../boxd-cli boxd-cli/child /absolute .hidden -option 'two words' 'bad*' 'bad?' 'bad[1]' 'bad\\name' $'bad\nname'; do + expect_usage run_sync --repair-nested-self-links -- "$bad" +done +expect_usage run_sync --repair-nested-self-links -- boxd-cli --dry-run +expect_usage run_audit --unknown +expect_usage run_audit '' +expect_usage run_audit --repair extra +expect_usage run_audit --repair --repair +snapshot "$fixture_home" >"$case_root/before" +run_sync --help >"$output" +unchanged +expect_rejected run_sync --repair-nested-self-links -- boxd-cli +printf 'strict argument parsing before mutation: PASS\n' + +# Every invalid final candidate must prevent removal of all earlier valid ones. +for variant in nested-real-dir nested-real-file nested-wrong nested-relative nested-dangling backlink-real backlink-wrong backlink-relative backlink-dangling parent-link missing-skill absent-invalid inaccessible-parent; do + new_case "$variant" + base_fixture + six_skills loops + last=${names[5]} + parent="$claude/$last" + nested="$parent/$last" + case "$variant" in + nested-real-dir) unlink "$nested"; mkdir "$nested" ;; + nested-real-file) unlink "$nested"; printf 'keep\n' >"$nested" ;; + nested-wrong) unlink "$nested"; ln -s "$agent" "$nested" ;; + nested-relative) unlink "$nested"; ln -s . "$nested" ;; + nested-dangling) unlink "$nested"; ln -s "$fixture_home/missing" "$nested" ;; + backlink-real) unlink "$codex/$last"; make_skill "$codex/$last" ;; + backlink-wrong) unlink "$codex/$last"; ln -s "$agent" "$codex/$last" ;; + backlink-relative) unlink "$codex/$last"; ln -s "../../.claude/skills/$last" "$codex/$last" ;; + backlink-dangling) unlink "$codex/$last"; ln -s "$fixture_home/missing" "$codex/$last" ;; + parent-link) mv "$parent" "$fixture_home/other-parent"; ln -s "$fixture_home/other-parent" "$parent" ;; + missing-skill) mv "$parent/SKILL.md" "$parent/not-a-skill" ;; + absent-invalid) unlink "$nested"; unlink "$codex/$last" ;; + inaccessible-parent) + # CI and normal macOS runs are non-root; root bypasses mode restrictions. + if [ "$(id -u)" -eq 0 ]; then printf 'SKIP: inaccessible directory as root\n'; continue; fi + chmod 000 "$parent" + # Snapshot cannot read it either; capture with access, then restore mode + # after the rejected operation to compare every entry and inode. + chmod 700 "$parent" + snapshot "$fixture_home" >"$case_root/before" + chmod 000 "$parent" + code=0 + run_sync --repair-nested-self-links -- "${names[@]}" >"$output" 2>&1 || code=$? + chmod 700 "$parent" + [ "$code" -ne 0 ] || fail 'inaccessible parent accepted' + unchanged + continue + ;; + esac + expect_rejected run_sync --repair-nested-self-links -- "${names[@]}" + contains 'removed=0 already-absent=0' + expect_rejected run_sync --repair-nested-self-links --dry-run -- "${names[@]}" +done +for component in .claude .codex .claude/skills .codex/skills; do + new_case "redirect-${component//\//-}" + base_fixture + six_skills loops + mv "$fixture_home/$component" "$fixture_home/redirected" + ln -s "$fixture_home/redirected" "$fixture_home/$component" + expect_rejected run_sync --repair-nested-self-links -- "${names[@]}" +done +new_case redirected-home +base_fixture +six_skills loops +ln -s "$fixture_home" "$case_root/alias" +expect_rejected env HOME="$case_root/alias" /bin/bash "$sync" --repair-nested-self-links -- "${names[@]}" +printf 'topology refusals and whole-batch preflight: PASS\n' + +# Inject a deterministic failure/change after the first unlink, without races +# or production test hooks. Only the temporary fixture is reachable by the stub. +real_unlink=$(command -v unlink) +for variant in io-error changed-parent changed-root; do + new_case "$variant" + base_fixture + six_skills loops + mkdir "$case_root/bin" + cat >"$case_root/bin/unlink" <<'STUB' +#!/bin/bash +set -eu +case "$1" in "$HOME/.claude/skills/"*) ;; *) exit 90 ;; esac +if [ "$1" = "$HOME/.claude/skills/boxd-cli/boxd-cli" ]; then + "$TEST_REAL_UNLINK" "$1" + case "$TEST_VARIANT" in + changed-parent) + mv "$HOME/.claude/skills/boxd-setup-deploy" "$HOME/saved-parent" + mkdir "$HOME/.claude/skills/boxd-setup-deploy" + cp "$HOME/saved-parent/SKILL.md" "$HOME/.claude/skills/boxd-setup-deploy/SKILL.md" + ln -s "$HOME/.codex/skills/boxd-setup-deploy" "$HOME/.claude/skills/boxd-setup-deploy/boxd-setup-deploy" + ;; + changed-root) + mv "$HOME/.codex/skills" "$HOME/saved-root" + mkdir "$HOME/.codex/skills" + for name in boxd-cli boxd-setup-deploy boxd-setup-golden boxd-setup-preview boxd-setup-fix boxd-setup-hermes; do + ln -s "$HOME/.claude/skills/$name" "$HOME/.codex/skills/$name" + done + ;; + esac +else + printf 'injected unlink I/O error\n' >&2 + exit 1 +fi +STUB + chmod 755 "$case_root/bin/unlink" + code=0 + HOME="$fixture_home" TEST_REAL_UNLINK="$real_unlink" TEST_VARIANT="$variant" PATH="$case_root/bin:$PATH" \ + /bin/bash "$sync" --repair-nested-self-links -- "${names[@]}" >"$output" 2>&1 || code=$? + [ "$code" -ne 0 ] || fail 'partial failure reported success' + contains 'removed=1 already-absent=0' + case "$variant" in + io-error) contains 'unlink failed:' ;; + changed-parent) contains 'topology changed:' ;; + changed-root) contains 'root changed:' ;; + esac + [ ! -L "$claude/boxd-cli/boxd-cli" ] || fail 'first removal was rolled back' + for name in "${names[@]:1}"; do + [ -L "$claude/$name/$name" ] || fail 'removed a later candidate after failure' + done +done +printf 'revalidation and partial progress: PASS\n' + +new_case audit-identity +base_fixture +make_skill "$claude/direct" +ln -s . "$claude/direct/direct" +make_skill "$claude/.hidden" +ln -s . "$claude/.hidden/.hidden" +make_skill "$claude/healthy" +ln -s "$agent" "$claude/healthy/healthy" +ln -s "$claude/healthy" "$claude/alias" +expect_rejected run_audit +[ "$(rg -c 'reason=nested-self-link' "$output")" -eq 2 ] || fail 'audit identity detection' +unlink "$claude/direct/direct" +unlink "$claude/.hidden/.hidden" +run_audit >"$output" +contains 'status=current' +printf 'bounded audit identity detection: PASS\n' + +new_case ordinary-sync +base_fixture +make_skill "$agent/winner" +make_skill "$manager/winner" +make_skill "$codex/winner" +make_skill "$manager/manager-only" +make_skill "$codex/local-only" +make_skill "$agent/already-correct" +ln -s "$agent/already-correct" "$claude/already-correct" +snapshot "$claude/already-correct" >"$case_root/link-before" +ln -s "$agent" "$claude/foreign" +ln -s "$agent" "$claude/.foreign" +ln -s "$fixture_home/missing" "$claude/broken" +ln -s "$fixture_home/missing" "$claude/.broken" +mkdir "$agent/stale" +ln -s "$agent/stale" "$claude/stale" +run_sync >"$output" +contains 'skip winner (manager): name taken by' +contains 'skip winner (codex-local): name taken by' +[ "$(readlink "$claude/winner")" = "$agent/winner" ] || fail 'precedence' +[ "$(readlink "$claude/manager-only")" = "$manager/manager-only" ] || fail 'manager mirror' +[ "$(readlink "$claude/local-only")" = "$codex/local-only" ] || fail 'local mirror' +[ -L "$claude/foreign" ] && [ -L "$claude/.foreign" ] || fail 'healthy foreign link pruned' +for name in broken .broken stale; do [ ! -L "$claude/$name" ] || fail 'stale link not pruned'; done +snapshot "$claude/already-correct" >"$case_root/link-after" +cmp "$case_root/link-before" "$case_root/link-after" || fail 'healthy symlink recreated' +snapshot "$fixture_home" >"$case_root/before" +run_sync >"$output" +contains 'skills mirror up to date' +unchanged +# Real files, real dirs and a FIFO must survive every ordinary link() call. +make_skill "$agent/file-conflict" +make_skill "$agent/dir-conflict" +make_skill "$agent/fifo-conflict" +printf 'keep file\n' >"$claude/file-conflict" +make_skill "$claude/dir-conflict" +mkfifo "$claude/fifo-conflict" +unlink "$codex/agent-scripts" +mkdir "$codex/agent-scripts" +unlink "$codex/manager" +printf 'keep root file\n' >"$codex/manager" +unlink "$fixture_home/.codex/AGENTS.md" +printf 'keep instructions\n' >"$fixture_home/.codex/AGENTS.md" +expect_rejected run_sync +contains 'skills mirror has 6 real destination conflict(s)' +if rg -q 'skills mirror up to date|^link ' "$output"; then fail 'conflicts falsely reported linked/current'; fi +printf 'ordinary sync ownership, conflicts and precedence: PASS\n' + +new_case broad-audit-repair +base_fixture +make_skill "$agent/mirrored" +mkdir -p "$fixture_home/Projects/agent-scripts/scripts" +cp "$sync" "$fixture_home/Projects/agent-scripts/scripts/sync-skills" +chmod 755 "$fixture_home/Projects/agent-scripts/scripts/sync-skills" +run_audit --repair >"$output" +[ "$(readlink "$claude/mirrored")" = "$agent/mirrored" ] || fail 'broad repair did not use canonical fixture owner' +contains 'status=current' +printf 'canonical broad audit repair routing: PASS\n' +printf 'sync-skills tests: ok (Bash %s)\n' "$BASH_VERSION" diff --git a/skills/fleet-maintenance/SKILL.md b/skills/fleet-maintenance/SKILL.md index ac3bed7..34d7a1a 100644 --- a/skills/fleet-maintenance/SKILL.md +++ b/skills/fleet-maintenance/SKILL.md @@ -18,7 +18,7 @@ Maintain Peter's Macs while protecting ambiguous local work. Package updates are - Keep passwords, recovery keys, and private keys in 1Password. The inventory stores opaque item IDs only. Invoke `$one-password` before any `op` command; a `pending` reference is not an error during package maintenance. - Require the classic OpenSSH mesh named by `ssh_mesh` on both profiles. The manager fleet setup document owns the canonical peer list and live proof. Use the symmetric Tailscale TCP 22 grant plus per-host `authorized_keys`; macOS GUI Tailscale clients cannot act as Tailscale SSH servers. Distribute public keys only, keep private keys host-local, verify both directions with `BatchMode=yes` and a finite timeout, and leave offline or provider-blocked directions pending. - Require the stable 1Password CLI integrity baseline on every fleet Mac. Require the file-backed service-account profile block unless that host has a documented `requirement_exceptions` security boundary in inventory. Audit eligible hosts with `scripts/op-profile-audit.sh`; audit token-exempt hosts with `scripts/op-profile-audit.sh --cli-only`. Repair only after `$one-password` is loaded and the mode-0600 token file is provisioned; never print or store the token in inventory. -- Require the agent skill mirror on every fleet Mac. Audit it with `scripts/agent-skill-links-audit.sh`; use `--repair` only when both canonical repos exist. This owns the Codex root links, Claude flat mirror, and shared instruction pointers documented in `references/fleet-schema.md` and `~/Projects/manager/docs/fleet-setup.md`. +- Require the agent skill mirror on every fleet Mac. Audit it with `scripts/agent-skill-links-audit.sh`; its `--repair` invokes broad canonical sync only when both canonical repos exist. Sync preserves real directories and files, accepting same-directory local ownership and reporting other real destination conflicts. For `reason=nested-self-link`, invoke the reviewed sync owner directly by absolute path with `--repair-nested-self-links --dry-run -- NAME...`, then omit `--dry-run` to remove only validated nested leaves; preserve the real Claude skill directories and Codex backlinks. This narrow loop check is not an exhaustive graph validator. See `references/fleet-schema.md` for scope and `~/Projects/manager/docs/fleet-setup.md` for fleet setup. - Require the shared global Git ignore on every fleet Mac. Audit it with `scripts/global-gitignore-audit.sh`; `--repair` creates `~/.config/git/ignore`, preserves unrelated entries, adds the inventory's macOS metadata patterns, and points `core.excludesFile` at it. An already-configured alternate excludes file requires manual review so existing rules are never discarded. - Require Claude Code and Claude Desktop coding sessions to omit AI attribution. Audit `~/.claude/settings.json` with `scripts/claude-attribution-audit.sh`; `--repair` preserves unrelated settings while disabling commit trailers, pull-request footers, and remote-session links. - Require the official Codex and Claude Code CLIs on both profiles. Package ownership comes from the profile's `codex` and `claude-code` Homebrew casks; the separate `claude` cask is Claude Desktop and does not satisfy the CLI requirement. Audit versions and non-interactive authentication with `scripts/agent-cli-audit.sh`; use `--live` for bounded, tool-free, non-persistent model turns. Never copy normal Claude OAuth credentials between Macs: refresh each host independently through `$anthropic` and leave locked-Keychain, account-selection, or offline cases pending. diff --git a/skills/fleet-maintenance/references/fleet-schema.md b/skills/fleet-maintenance/references/fleet-schema.md index 4743501..3b4433d 100644 --- a/skills/fleet-maintenance/references/fleet-schema.md +++ b/skills/fleet-maintenance/references/fleet-schema.md @@ -39,10 +39,12 @@ Every fleet Mac must have: - `~/.codex/skills/agent-scripts -> ~/Projects/agent-scripts/skills` - `~/.codex/skills/manager -> ~/Projects/manager/skills` -- a real `~/.claude/skills` directory containing flat per-skill links, with agent-scripts winning name collisions +- a real `~/.claude/skills` directory containing flat per-skill links, with agent-scripts winning name collisions; locally owned real skill directories and valid Codex backlinks to them are preserved - `~/.codex/AGENTS.md`, `~/.claude/CLAUDE.md`, and `~/.claude/AGENTS.md` pointing to `~/Projects/agent-scripts/AGENTS.MD` -Run `scripts/agent-skill-links-audit.sh` read-only. Run it with `--repair` to invoke the canonical idempotent `~/Projects/agent-scripts/scripts/sync-skills`; never replace a conflicting real instruction file automatically. +Run `scripts/agent-skill-links-audit.sh` read-only. Run it with `--repair` to invoke the canonical idempotent `~/Projects/agent-scripts/scripts/sync-skills` for broad mirror, pruning, and instruction-pointer work. Sync preserves every real destination, treats the same directory identity as satisfied local ownership, and fails on other real destination conflicts. + +The audit also reports `reason=nested-self-link` for same-name symlinks inside real top-level Claude directories that resolve back to their parent. This bounded read-only check is not an exhaustive graph validator. For this topology, call the reviewed sync owner directly by absolute path with `--repair-nested-self-links [--dry-run] -- NAME...`; the audit has no scoped repair wrapper or owner override. The owner requires a nonempty, unique safe-basename allowlist, accessible real registration roots, a real Claude skill directory with `SKILL.md`, and exact literal Codex/nested targets with matching directory identities. It validates the entire batch before unlinking only nested leaves and exits before ordinary sync. Already-absent leaves are no-ops only with valid surrounding topology. Revalidation catches drift but is not atomic against concurrent writers; later failures report partial progress without rollback. Real directories, contents, valid backlinks, unrelated entries, and shared instruction pointers remain untouched by scoped repair. Homebrew dependencies do not belong in `formulae`; declare intentionally installed leaves. Homebrew is rolling-release state, not a version lock. diff --git a/skills/fleet-maintenance/scripts/agent-skill-links-audit.sh b/skills/fleet-maintenance/scripts/agent-skill-links-audit.sh index 7b94162..a38b91e 100755 --- a/skills/fleet-maintenance/scripts/agent-skill-links-audit.sh +++ b/skills/fleet-maintenance/scripts/agent-skill-links-audit.sh @@ -9,7 +9,7 @@ agents_md="$HOME/Projects/agent-scripts/AGENTS.MD" repair=${1:-} failures=0 -if [ -n "$repair" ] && [ "$repair" != "--repair" ]; then +if [ "$#" -gt 1 ] || { [ "$#" -eq 1 ] && [ "$repair" != "--repair" ]; }; then printf 'usage: %s [--repair]\n' "$0" >&2 exit 2 fi @@ -84,6 +84,18 @@ for link_path in "$claude_root"/* "$claude_root"/.[!.]*; do done failures=$((failures + broken)) +# Narrow ancestor-loop detection, not an exhaustive symlink graph validator. +# A nested link resolving successfully to its real parent is still a loop. +for entry in "$claude_root"/* "$claude_root"/.[!.]* "$claude_root"/..?*; do + [ ! -L "$entry" ] && [ -d "$entry" ] || continue + name=${entry##*/} + nested="$entry/$name" + if [ -L "$nested" ] && [ "$nested" -ef "$entry" ]; then + printf 'skill-links-drift\tpath=%s\treason=nested-self-link\n' "$nested" + failures=$((failures + 1)) + fi +done + claude_count=$(find "$claude_root" -mindepth 1 -maxdepth 1 -type l 2>/dev/null | wc -l | tr -d ' ') if [ "$failures" -eq 0 ]; then printf 'skill-links\tstatus=current\tclaude_links=%s\n' "$claude_count"