diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 661c5654..2bf13ae5 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -549,7 +549,29 @@ jobs: with: persist-credentials: false + # An older stable release can reach this job after a newer one has + # published — its run stalled, or its failed sync was rerun later — and + # the script mirrors the skills wholesale, so it would roll basecamp/skills + # back. The concurrency group serialises the jobs but does not order them, + # so this is the check the manual Sync skills workflow makes: only the + # latest stable release publishes. A failed lookup fails the job, which + # files the issue below rather than skipping in silence. + - name: Confirm this is the latest stable release + id: latest + env: + GH_TOKEN: ${{ github.token }} + TAG: ${{ github.ref_name }} + run: | + latest=$(gh release view --repo "${GITHUB_REPOSITORY}" --json tagName --jq .tagName) + if [ "$TAG" = "$latest" ]; then + echo "publish=true" >> "$GITHUB_OUTPUT" + else + echo "::warning::Skipping the skills sync for ${TAG}: the latest stable release is ${latest}, and syncing an older tag would roll basecamp/skills back." + echo "publish=false" >> "$GITHUB_OUTPUT" + fi + - name: Generate token for skills repo + if: steps.latest.outputs.publish == 'true' id: skills-token uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 with: @@ -560,6 +582,7 @@ jobs: permission-contents: write - name: Sync skills to distribution repo + if: steps.latest.outputs.publish == 'true' id: sync env: SKILLS_TOKEN: ${{ steps.skills-token.outputs.token }} diff --git a/.github/workflows/sync-skills.yml b/.github/workflows/sync-skills.yml index 39dd0495..c33a2ca3 100644 --- a/.github/workflows/sync-skills.yml +++ b/.github/workflows/sync-skills.yml @@ -7,8 +7,10 @@ # a way back without cutting a new tag. This is it. # # scripts/sync-skills.sh mirrors the skills/ tree at the given ref into -# basecamp/skills and no-ops when the content already matches, so re-running it -# for an already-synced release is safe. +# basecamp/skills — only this CLI's skills, tracked in its own manifest there +# (.managed-skills.basecamp-cli); other CLIs' skills are never touched — and +# no-ops when the content already matches, so re-running it for an +# already-synced release is safe. name: Sync skills on: @@ -52,8 +54,8 @@ jobs: exit 1 fi - # sync-skills.sh mirrors the tree wholesale, so syncing an older tag - # would roll basecamp/skills back. Unlike the AUR, there is no + # sync-skills.sh mirrors this CLI's skills wholesale, so syncing an + # older tag would roll them back in basecamp/skills. Unlike the AUR, there is no # independent record of what the distribution repo currently holds, and # the only reason to run this by hand is that the newest release failed # to sync — so require exactly that release. @@ -63,11 +65,24 @@ jobs: exit 1 fi - # Check out the tag itself, not main: the sync must mirror the skills tree - # as it was released, even if main has moved on since. + # Two checkouts: sync logic from the dispatching ref, skills content from + # the release tag. A defect in scripts/sync-skills.sh is one of the ways + # the automatic sync fails, and running the tagged copy here would just + # re-run the defective script — recovery must be able to run a fix merged + # to main without cutting a new tag. - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: - ref: ${{ inputs.tag }} + persist-credentials: false + + # The content still mirrors the skills tree as it was released, even if + # main has moved on since: SKILLS_SOURCE points the script at this + # checkout. Fully qualified: actions/checkout resolves a bare name as a + # branch before a tag, so a branch sharing the tag's name would win and + # mirror unreleased content. + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: refs/tags/${{ inputs.tag }} + path: release persist-credentials: false # Needed for dry runs too: the honest preview clones the target, so it @@ -83,12 +98,14 @@ jobs: repositories: skills permission-contents: ${{ inputs.dry_run && 'read' || 'write' }} - # github.sha is the dispatching ref's SHA (main), not the tag's, so resolve - # the commit actually checked out — otherwise the sync records the wrong - # provenance for the release it claims to mirror. + # github.sha is the dispatching ref's SHA (main), not the tag's, so + # resolve the tagged commit from its own checkout — otherwise the sync + # records the wrong provenance for the release it claims to mirror. - name: Resolve the tagged commit id: source - run: echo "sha=$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT" + env: + TAG: ${{ inputs.tag }} + run: echo "sha=$(git -C release rev-parse --verify "refs/tags/${TAG}^{commit}")" >> "$GITHUB_OUTPUT" # DRY_RUN=remote, not local: the local path never clones basecamp/skills # and diffs against an empty repo, so every skill reads as newly added and @@ -100,5 +117,6 @@ jobs: SKILLS_TOKEN: ${{ steps.skills-token.outputs.token }} RELEASE_TAG: ${{ inputs.tag }} SOURCE_SHA: ${{ steps.source.outputs.sha }} + SKILLS_SOURCE: release/skills DRY_RUN: ${{ inputs.dry_run && 'remote' || '' }} run: scripts/sync-skills.sh diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 98b2def3..6630ef2f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -181,6 +181,9 @@ jobs: - name: Run BATS integration tests run: make test-e2e + - name: Test the skills sync + run: make test-sync-skills + cli-surface: name: CLI Surface Check runs-on: ubuntu-latest diff --git a/Makefile b/Makefile index 371ef099..fad4bb1d 100644 --- a/Makefile +++ b/Makefile @@ -383,7 +383,7 @@ check-smoke-coverage: build # Run all checks (local CI gate) .PHONY: check -check: fmt-check vet lint lint-actions test test-e2e check-naming check-surface check-skill-drift check-bare-groups check-lint-lockstep check-smoke-coverage provenance-check tidy-check +check: fmt-check vet lint lint-actions test test-e2e test-sync-skills check-naming check-surface check-skill-drift check-bare-groups check-lint-lockstep check-smoke-coverage provenance-check tidy-check # Lint GitHub Actions workflows (requires actionlint + zizmor) .PHONY: lint-actions @@ -563,6 +563,11 @@ sync-skills: @test -n "$(TAG)" || (echo "Usage: make sync-skills TAG=v1.2.3" && exit 1) RELEASE_TAG=$(TAG) SOURCE_SHA=$$(git rev-parse HEAD) DRY_RUN=local scripts/sync-skills.sh +# Run the skills sync against a throwaway basecamp/skills, as two CLIs publishing in turn +.PHONY: test-sync-skills +test-sync-skills: + EXPECTED_SOURCE=basecamp-cli scripts/test-sync-skills.sh + # Sync skills (dry-run against real target repo) # Usage: make sync-skills-remote TAG=v1.2.3 SKILLS_TOKEN=ghp_... .PHONY: sync-skills-remote @@ -647,6 +652,7 @@ help: @echo "" @echo "Skills:" @echo " sync-skills Local dry-run of skill sync (TAG=v1.2.3)" + @echo " test-sync-skills Run the skills sync as two CLIs against a throwaway target" @echo " sync-skills-remote Remote dry-run (TAG=v1.2.3 SKILLS_TOKEN=...)" @echo "" @echo " help Show this help" diff --git a/RELEASING.md b/RELEASING.md index 0607d1e6..3c9d64a2 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -70,6 +70,32 @@ basecamp skill basecamp skill install ``` +## Skills sync + +Stable releases mirror `skills/` into [basecamp/skills](https://github.com/basecamp/skills), +which several CLIs share. `scripts/sync-skills.sh` owns only this CLI's skills there: +it records the names it published in `.managed-skills.basecamp-cli` at the target root +and removes a `skills/` only when that manifest lists it, the release no longer +ships it, and no other CLI's `.managed-skills.*` claims it (a collision is warned about +and left alone), and refuses outright to publish a name another CLI's manifest holds. +A target with no `.managed-skills.basecamp-cli` yet is a first run: nothing is removed. +A push rejected because another CLI published first is retried by applying the whole +sync again from the remote's new tip, not by replaying the stale commit. The legacy shared `.managed-skills` is rewritten as a comment-only +tombstone so a CLI still on the pre-fix script — which deleted everything its own tree +lacked — deletes nothing (basecamp/skills#5). + +If the release-time sync fails, recover with the `Sync skills` workflow +(`workflow_dispatch`, stable tag, optional dry run). It refuses anything but the latest +stable release so it cannot roll the distribution repo back, and it runs the sync script +from the dispatching branch against the tag's skills tree — so when the failure was a +defect in `sync-skills.sh` itself, merge the fix to main and dispatch; no new release +needed. The release-time job makes the same check before it publishes, so an older +release whose run stalls, or whose failed sync is rerun after a newer release has +shipped, skips the sync instead of rolling it back. The script always clones the target fresh and pushes only the commit it made, +so there is no checkout to hand it. `scripts/test-sync-skills.sh` (`make test-sync-skills`, +in `bin/ci`) pins the ownership contract by running the script as both CLIs against a +local bare repository — real clones, commits and pushes, no network. + ## Requirements - On `main` branch with clean, synced working tree diff --git a/scripts/sync-skills.sh b/scripts/sync-skills.sh index ebe87680..fd626ccb 100755 --- a/scripts/sync-skills.sh +++ b/scripts/sync-skills.sh @@ -1,176 +1,288 @@ #!/usr/bin/env bash -set -euo pipefail - -# Sync skills from basecamp-cli to basecamp/skills distribution repo. +# sync-skills.sh — Publish this CLI's skills to the basecamp/skills distribution repo. +# +# Runs from CI on a release tag. Clones basecamp/skills fresh into a temp directory, +# mirrors each skills// tree (SKILL.md and its supporting files; no *.go, no +# dotfiles) into skills// at its root — the layout `npx skills add +# basecamp/skills` reads — then commits as [bot], pushes, and removes the +# clone. The script never adopts an existing checkout: the only commit it can push +# is the one it made, against the tip it cloned or fetched. +# +# Several CLIs publish into that one repo, so each owns a manifest of its own at the +# target root, .managed-skills., listing the skill names it has published, +# one per line. A skills/ directory is removed only when all of these hold: +# this source's manifest lists it, this source's current skill set no longer has +# it, and no other source's manifest claims it. A name two sources claim is a +# collision to settle upstream, never one a release resolves by deletion — the +# script warns and leaves the directory, and refuses outright to publish a name +# another source's manifest holds. Nothing else in the target is ever deleted: with +# no manifest yet, the first run publishes and removes nothing. +# +# The shared manifest the pre-fix scripts kept, .managed-skills, is rewritten on +# every run as a comment-only tombstone. The pre-fix script skips any line it cannot +# parse as a skill name, but treats a missing file as licence to own every skills/* +# directory — so the tombstone is what stops an un-upgraded sibling from deleting +# this source's skills, whichever CLI upgrades first (basecamp/skills#5). It shields +# only the sources that have upgraded: a pre-fix sibling still writes its own names +# to .managed-skills, and a second pre-fix sibling still deletes those — the #5 +# clobber, confined to the CLIs yet to upgrade and gone once each has; nothing the +# target holds can make that script delete less, since a file it cannot read widens +# its reach to every skills/* directory. One more case is accepted: a skill a +# still-pre-fix sibling drops after the tombstone exists stays behind in the target +# (that script has no names left to delete by, and the sibling's own first run here +# removes nothing) — a lingering directory to remove by hand, which beats guessing +# ownership from the legacy file. +# +# Required env vars: +# RELEASE_TAG — the release tag (e.g. v1.2.3) +# SOURCE_SHA — the source commit SHA +# SKILLS_TOKEN — GitHub token with push access to basecamp/skills; not needed +# for DRY_RUN=local, nor when SKILLS_REPO_URL is not on github.com +# +# Optional env vars: +# CLI_NAME — this CLI's name; the publishing source is -cli +# SYNC_SOURCE — the publishing repo's name (default: -cli). Names the +# manifest, the bot and the commit; the test sets it to play +# another CLI +# SKILLS_SOURCE — directory holding the skills tree (default: skills). A manual +# recovery workflow can point it at a checkout of the release tag +# so the sync logic comes from a newer ref than the content +# SKILLS_REPO_URL — where basecamp/skills is cloned from and pushed to (default: +# https://github.com/basecamp/skills.git). The test points it at +# a local bare repository so a real push lands somewhere it can +# read back +# DRY_RUN — "local": no network; copy into an empty tmpdir and print what +# would be published. +# "remote": clone, apply, print the diff, and stop before +# committing # -# Env vars: -# SKILLS_TOKEN - GitHub token with push access to basecamp/skills (required unless DRY_RUN=local) -# RELEASE_TAG - Release tag, e.g. v1.2.3 (required) -# SOURCE_SHA - Source commit SHA (required) -# DRY_RUN - Optional: "local" (no network) or "remote" (clone but skip push) +set -euo pipefail + +CLI_NAME="${CLI_NAME:-basecamp}" +SYNC_SOURCE="${SYNC_SOURCE:-${CLI_NAME}-cli}" RELEASE_TAG="${RELEASE_TAG:?RELEASE_TAG is required}" SOURCE_SHA="${SOURCE_SHA:?SOURCE_SHA is required}" +SKILLS_SOURCE="${SKILLS_SOURCE:-skills}" +SKILLS_TOKEN="${SKILLS_TOKEN:-}" DRY_RUN="${DRY_RUN:-}" -SKILLS_SOURCE="skills" TARGET_REPO="basecamp/skills" TARGET_BRANCH="main" +SKILLS_REPO_URL="${SKILLS_REPO_URL:-https://github.com/${TARGET_REPO}.git}" SKILLS_SUBDIR="skills" +LEGACY_MANIFEST=".managed-skills" +MANIFEST="${LEGACY_MANIFEST}.${SYNC_SOURCE}" +# The commit's provenance line; GITHUB_REPOSITORY is exact in CI, the default holds +# for the basecamp org's naming. +SOURCE_REPO="${GITHUB_REPOSITORY:-basecamp/${SYNC_SOURCE}}" # --- Helpers --- die() { echo "ERROR: $*" >&2; exit 1; } +warn() { echo "WARNING: $*" >&2; } + +# A skill directory name or a source name: nothing a path could smuggle in. +plain_name() { + [[ "$1" != "." && "$1" != ".." && "$1" =~ ^[a-zA-Z0-9._-]+$ ]] +} + +in_list() { + local needle="$1" item + shift + for item in "$@"; do + [[ "$item" == "$needle" ]] && return 0 + done + return 1 +} -assert_remote_url() { - local url - url=$(git -C "$1" remote get-url origin) - local stripped - stripped=$(echo "$url" | sed -E 's/\.git$//') - # Validate host + owner/repo for both HTTPS and SSH forms - case "$stripped" in - https://github.com/"$TARGET_REPO") ;; - https://x-access-token:*@github.com/"$TARGET_REPO") ;; - git@github.com:"$TARGET_REPO") ;; - *) die "origin remote '$(echo "$url" | sed -E 's#(https://[^:@]+:)[^@]*@#\1***@#')' does not point to github.com/$TARGET_REPO" ;; - esac +# Print the skill names a manifest lists, one per line. Blank and comment lines +# are skipped silently; anything else that is not a plain name, with a warning. +read_manifest() { + local file="$1" line + [[ -f "$file" ]] || return 0 + while IFS= read -r line || [[ -n "$line" ]]; do + [[ -z "$line" || "$line" == \#* ]] && continue + if plain_name "$line"; then + echo "$line" + else + warn "skipping invalid entry in ${file##*/}: $line" + fi + done < "$file" } -assert_branch() { - local branch - branch=$(git -C "$1" rev-parse --abbrev-ref HEAD) - [[ "$branch" == "$TARGET_BRANCH" ]] || die "checked-out branch is '$branch', expected '$TARGET_BRANCH'" +# Print the other source whose manifest claims a name, if any. +claimed_by_other() { + local name="$1" file other listed + for file in "${target}/${LEGACY_MANIFEST}".*; do + [[ -f "$file" ]] || continue + other="${file##*/"${LEGACY_MANIFEST}".}" + [[ "$other" == "$SYNC_SOURCE" ]] && continue + listed=$(read_manifest "$file") + if grep -qxF -- "$name" <<< "$listed"; then + echo "$other" + return 0 + fi + done + return 1 } +# --- Validate the knobs --- + +plain_name "$SYNC_SOURCE" || die "SYNC_SOURCE '$SYNC_SOURCE' is not a plain name" +case "$DRY_RUN" in + ""|local|remote) ;; + *) die "DRY_RUN must be unset, 'local' or 'remote', not '$DRY_RUN'" ;; +esac + # --- Discover skills --- -skill_dirs=() +skill_names=() for skill_md in "$SKILLS_SOURCE"/*/SKILL.md; do [[ -f "$skill_md" ]] || continue - skill_dirs+=("$(dirname "$skill_md")") + name=$(basename "$(dirname "$skill_md")") + plain_name "$name" || die "skill directory '$name' is not a plain name" + skill_names+=("$name") done -[[ ${#skill_dirs[@]} -gt 0 ]] || die "no skills found under $SKILLS_SOURCE/*/SKILL.md" -echo "Found ${#skill_dirs[@]} skill(s): ${skill_dirs[*]}" +[[ ${#skill_names[@]} -gt 0 ]] || die "no skills found under ${SKILLS_SOURCE}/*/SKILL.md" +echo "Found ${#skill_names[@]} skill(s) in ${SKILLS_SOURCE}/: ${skill_names[*]}" -# --- Copy skills into target, excluding *.go and dotfiles --- +# --- Copy skills, excluding *.go and dotfiles, preserving subdirectories --- copy_skills() { - local target_dir="$1" - for skill_dir in "${skill_dirs[@]}"; do - local name - name=$(basename "$skill_dir") - rm -rf "${target_dir:?}/${name}" - mkdir -p "$target_dir/$name" - # Copy files, excluding *.go and dotfiles - find "$skill_dir" -mindepth 1 \ - ! -name '*.go' \ - ! -name '.*' \ - ! -path '*/.*' \ - -type f \ - -exec bash -c ' - src="$1"; skill_dir="$2"; target_dir="$3" - rel="${src#$skill_dir/}" - mkdir -p "$(dirname "$target_dir/$rel")" - cp "$src" "$target_dir/$rel" - ' _ {} "$skill_dir" "$target_dir/$name" \; + local skills_dir="$1" name dest + for name in "${skill_names[@]}"; do + dest="${skills_dir}/${name}" + rm -rf "${dest:?}" + mkdir -p "$dest" + (cd "${SKILLS_SOURCE}/${name}" && find . -type f ! -name '*.go' ! -name '.*' ! -path '*/.*/*' -print0) | + while IFS= read -r -d '' file; do + mkdir -p "${dest}/$(dirname "$file")" + cp "${SKILLS_SOURCE}/${name}/${file}" "${dest}/${file}" + done done } -# --- DRY_RUN=local: copy into tmpdir, diff against empty baseline --- +tmpdir=$(mktemp -d) +trap 'rm -rf "$tmpdir"' EXIT + +# --- DRY_RUN=local: what would be published --- if [[ "$DRY_RUN" == "local" ]]; then - tmpdir=$(mktemp -d) - trap 'rm -rf "$tmpdir"' EXIT - echo "DRY_RUN=local: copying skills into $tmpdir" - copy_skills "$tmpdir/$SKILLS_SUBDIR" + preview="${tmpdir}/preview" + echo "DRY_RUN=local: copying skills into ${preview}" + copy_skills "${preview}/${SKILLS_SUBDIR}" echo "" echo "=== Skills copied ===" - find "$tmpdir" -type f | sort | while read -r f; do - echo " ${f#$tmpdir/}" + find "$preview" -type f | LC_ALL=C sort | while read -r file; do + echo " ${file#"${preview}/"}" done echo "" echo "=== Diff (against empty baseline) ===" - # Initialize as empty git repo to get a clean diff - git -C "$tmpdir" init -q - git -C "$tmpdir" add -A - git -C "$tmpdir" diff --cached --stat + git -C "$preview" init -q + git -C "$preview" add -A + git -C "$preview" diff --cached --stat echo "" echo "DRY_RUN=local complete. No network operations performed." exit 0 fi -# --- Clone target repo --- - -[[ -n "${SKILLS_TOKEN:-}" ]] || die "SKILLS_TOKEN is required (set DRY_RUN=local for offline testing)" - -tmpdir=$(mktemp -d) -trap 'rm -rf "$tmpdir"' EXIT - -echo "Cloning $TARGET_REPO into $tmpdir/skills..." -git clone --depth 1 --branch "$TARGET_BRANCH" \ - "https://x-access-token:${SKILLS_TOKEN}@github.com/${TARGET_REPO}.git" \ - "$tmpdir/skills" - -target="$tmpdir/skills" +# --- Git configuration for the target --- +# +# A private global config for every git call below: the bot is the identity for +# the commit, and for the one a rejected push makes again, and the token goes in as +# an Authorization header scoped to github.com, the way actions/checkout sends it — +# not as a URL rewrite, which git expands before handing the URL to git-remote-https +# in argv. The remote URL stays clean; the token is only in this file, mode 600, +# removed with the tmpdir. Only the user's global file is replaced (~/.gitconfig: +# identity, signing, credential helpers, hooks path); the system config and any +# GIT_CONFIG_COUNT/GIT_CONFIG_KEY_* settings in the environment still apply — the +# test's race case injects a hooks path that way. +export GIT_CONFIG_GLOBAL="${tmpdir}/gitconfig" +cat > "$GIT_CONFIG_GLOBAL" <> "$GIT_CONFIG_GLOBAL" <&2 + for name in ${previously_published[@]+"${previously_published[@]}"}; do + in_list "$name" "${skill_names[@]}" && continue + if other=$(claimed_by_other "$name"); then + warn "skills/${name} is no longer in ${SYNC_SOURCE}'s skills but ${other} lists it in ${LEGACY_MANIFEST}.${other}; leaving it in place" continue fi - previously_managed_names+=("$entry") - done < "$target/$MANIFEST" -else - # No manifest yet (first run). basecamp-cli is the source of truth, so - # treat all */SKILL.md dirs in the target as managed. Any skill not in the - # source set will be removed. Use DRY_RUN=remote to preview before pushing. - for candidate in "$target/$SKILLS_SUBDIR"/*/SKILL.md; do - [[ -f "$candidate" ]] || continue - previously_managed_names+=("$(basename "$(dirname "$candidate")")") + if [[ -d "${target}/${SKILLS_SUBDIR}/${name}" ]]; then + echo "Removing stale skill: ${name}" + rm -rf "${target:?}/${SKILLS_SUBDIR}/${name}" + fi done -fi -for previously_managed in "${previously_managed_names[@]}"; do - found=0 - for name in "${source_skill_names[@]}"; do - [[ "$name" == "$previously_managed" ]] && found=1 && break - done - if [[ "$found" -eq 0 && -d "$target/$SKILLS_SUBDIR/$previously_managed" ]]; then - echo "Removing stale skill: $previously_managed" - rm -rf "${target:?}/$SKILLS_SUBDIR/$previously_managed" - fi -done + # This source's manifest, and the legacy tombstone + printf '%s\n' "${skill_names[@]}" | LC_ALL=C sort > "${target}/${MANIFEST}" + cat > "${target}/${LEGACY_MANIFEST}" <<'TOMBSTONE' +# Superseded by the per-source manifests (.managed-skills.), one per publishing CLI. +# Each CLI deletes only the skill directories listed in its own manifest. +# Kept so a CLI still running the pre-fix sync script deletes nothing: that script skips +# every line it cannot parse as a skill name and only deletes names it can. +TOMBSTONE -# Write current manifest -printf '%s\n' "${source_skill_names[@]}" | sort > "$target/$MANIFEST" + git -C "$target" add -A +} -# --- Commit --- +commit_sync() { + git -C "$target" commit -q -m "$(cat <&1 } if ! output=$(push_target); then - if echo "$output" | grep -qi "non-fast-forward"; then - echo "Push rejected (non-fast-forward). Pulling with rebase and retrying..." - git -C "$target" pull --rebase origin "$TARGET_BRANCH" - if ! retry_output=$(push_target); then - echo "$retry_output" >&2 - die "Push failed after retry" - fi - else + if ! echo "$output" | grep -Eqi "fetch first|non-fast-forward"; then echo "$output" >&2 die "Push failed" fi + echo "Push rejected (the remote has moved). Applying the sync again from its new tip..." + git -C "$target" fetch -q origin "$TARGET_BRANCH" + git -C "$target" reset -q --hard FETCH_HEAD + apply_sync + if git -C "$target" diff --cached --quiet; then + echo "Nothing left to publish: the remote already holds these skills." + exit 0 + fi + commit_sync + if ! retry_output=$(push_target); then + echo "$retry_output" >&2 + die "Push failed after retry" + fi fi echo "" -echo "Skills synced to $TARGET_REPO ($TARGET_BRANCH) from $RELEASE_TAG" +echo "Skills synced to ${TARGET_REPO} (${TARGET_BRANCH}) from ${SYNC_SOURCE} ${RELEASE_TAG}" diff --git a/scripts/test-sync-skills.sh b/scripts/test-sync-skills.sh new file mode 100755 index 00000000..5e997b58 --- /dev/null +++ b/scripts/test-sync-skills.sh @@ -0,0 +1,391 @@ +#!/usr/bin/env bash +# test-sync-skills.sh — Run sync-skills.sh as two CLIs against one throwaway +# basecamp/skills and prove neither deletes the other's skills. +# +# The target is a local bare repository the script clones from and pushes to +# through SKILLS_REPO_URL, exactly as it would basecamp/skills — so every run +# here is a real clone, commit and push, and the test reads the result back +# from a clone of its own. It starts in the state basecamp/skills#5 left it: +# basecamp-cli's skills and the shared .managed-skills listing them. Then +# hey-cli and basecamp-cli sync in turn, one loses a skill, a pre-fix sibling +# rewrites the legacy manifest, two manifests claim one name, a sibling wins +# the race to push, and the script runs as the source its own CLI_NAME default +# names — after each step both sources' skills must be where they belong. No +# network and no token. +# +# Usage: scripts/test-sync-skills.sh (tests scripts/sync-skills.sh) +# SYNC_SCRIPT=path/to/sync-skills.sh scripts/test-sync-skills.sh +# EXPECTED_SOURCE=-cli scripts/test-sync-skills.sh +# (the source the script publishes as when nothing names one; a CLI's +# Makefile passes its own, the default is read off the script's CLI_NAME line) + +set -euo pipefail + +here=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +SYNC_SCRIPT="${SYNC_SCRIPT:-${here}/sync-skills.sh}" +[[ -x "$SYNC_SCRIPT" ]] || { echo "ERROR: ${SYNC_SCRIPT} is not an executable script" >&2; exit 1; } + +work=$(mktemp -d) +trap 'rm -rf "$work"' EXIT + +# The test's own git calls see only this config; the script brings its own. +export GIT_CONFIG_GLOBAL="${work}/gitconfig" GIT_CONFIG_NOSYSTEM=1 +printf '[user]\n\tname = test\n\temail = test@example.com\n' > "$GIT_CONFIG_GLOBAL" + +# The token-required case points the script at the real basecamp/skills; a token +# inherited from the caller's environment would let it clone and publish the +# fixtures there. +unset SKILLS_TOKEN + +# A file:// URL rather than a path: git ignores --depth for a path, and the +# script's clone is shallow, so the retry has to fetch as it would from GitHub. +origin="${work}/origin.git" +origin_url="file://${origin}" +target="${work}/target" +out="${work}/out" +failures=0 + +# --- Assertions --- + +ok() { echo "ok - $*"; } +not_ok() { echo "not ok - $*"; failures=$((failures + 1)); } + +assert() { # description, command... + local desc="$1" + shift + if "$@"; then ok "$desc"; else not_ok "$desc"; fi +} + +assert_skill() { assert "skills/$1 present" test -f "${target}/skills/$1/SKILL.md"; } +assert_no_skill() { assert "skills/$1 absent" test ! -e "${target}/skills/$1"; } +assert_no_path() { assert "$1 not published" test ! -e "${target}/skills/$1"; } +assert_content() { # path, expected content + assert "$1 holds '$2'" test "$(cat "${target}/$1")" = "$2" +} +assert_manifest() { # source, names... + local source="$1" want + shift + want=$(printf '%s\n' "$@") + assert ".managed-skills.${source} lists exactly: $*" test "$(cat "${target}/.managed-skills.${source}")" = "$want" +} +assert_no_manifest() { assert ".managed-skills.$1 absent" test ! -e "${target}/.managed-skills.$1"; } +assert_tombstone() { + if [[ -f "${target}/.managed-skills" ]] && ! grep -qv '^#' "${target}/.managed-skills" && grep -q 'Superseded' "${target}/.managed-skills"; then + ok ".managed-skills is the comment-only tombstone" + else + not_ok ".managed-skills is the comment-only tombstone" + fi +} +assert_author() { assert "last commit authored by $1[bot]" test "$(git -C "$target" log -1 --format=%an)" = "$1[bot]"; } +assert_output() { assert "output says: $1" grep -q -- "$1" "$out"; } +assert_head() { # expected sha, description + assert "$2" test "$(git -C "$origin" rev-parse main)" = "$1" +} + +origin_head() { git -C "$origin" rev-parse main; } + +# --- Running the script --- +# +# Every run clones origin afresh, as a release would clone basecamp/skills, and +# the target clone is brought to origin's tip afterwards for the assertions. + +refresh_target() { + git -C "$target" fetch -q origin main + git -C "$target" reset -q --hard FETCH_HEAD +} + +# The script against origin with the identifiers a release carries; the caller's +# VAR=value pairs go in front of the fixed ones, so only SKILLS_REPO_URL can be +# overridden (DRY_RUN=local points it nowhere to prove it is never reached). +run_sync() { # [VAR=value...] + env SKILLS_REPO_URL="$origin_url" "$@" RELEASE_TAG=v9.9.9 SOURCE_SHA=0123abcd "$SYNC_SCRIPT" > "$out" 2>&1 +} + +sync() { # source, fixture, [VAR=value...] + local source="$1" fixture="$2" + shift 2 + if run_sync "$@" SYNC_SOURCE="$source" SKILLS_SOURCE="${fixture}/skills"; then + ok "sync as ${source} succeeded" + else + not_ok "sync as ${source} succeeded" + sed 's/^/ /' "$out" + fi + refresh_target +} + +sync_expecting_failure() { # source, fixture, [VAR=value...] + local source="$1" fixture="$2" + shift 2 + if run_sync "$@" SYNC_SOURCE="$source" SKILLS_SOURCE="${fixture}/skills"; then + not_ok "sync as ${source} refused" + sed 's/^/ /' "$out" + else + ok "sync as ${source} refused" + fi + refresh_target +} + +# The script with neither SYNC_SOURCE nor CLI_NAME set, so the source is the one +# the CLI_NAME default line names — the line each CLI edits, which the other +# runs here never reach because they set SYNC_SOURCE to play another CLI. +sync_as_default() { # fixture + local fixture="$1" + if (unset CLI_NAME SYNC_SOURCE; run_sync SKILLS_SOURCE="${fixture}/skills"); then + ok "sync as the default source succeeded" + else + not_ok "sync as the default source succeeded" + sed 's/^/ /' "$out" + fi + refresh_target +} + +# Commit the target's working tree and push it, as a hand-made or pre-fix commit +publish() { # message + git -C "$target" add -A + git -C "$target" commit -q -m "$1" + git -C "$target" push -q origin main +} + +# --- Fixtures --- + +write_skill() { # dir, content + mkdir -p "$1" + echo "$2" > "$1/SKILL.md" +} + +a="${work}/hey-cli" +b="${work}/basecamp-cli" + +write_skill "${a}/skills/hey" "hey v2" +mkdir -p "${a}/skills/hey/reference" "${a}/skills/hey/.cache" +echo "nested" > "${a}/skills/hey/reference/commands.md" +echo "package hey" > "${a}/skills/hey/embed.go" +echo "secret" > "${a}/skills/hey/.env" +echo "cached" > "${a}/skills/hey/.cache/index" +write_skill "${a}/skills/hey-doctor" "hey-doctor v2" + +write_skill "${b}/skills/basecamp" "basecamp v2" +write_skill "${b}/skills/basecamp-doctor" "basecamp-doctor v2" + +# The target as basecamp/skills#5 left it: only basecamp-cli's skills survive, +# and the shared manifest names them. +git init -q --bare -b main "$origin" +git init -q -b main "$target" +git -C "$target" remote add origin "$origin_url" +write_skill "${target}/skills/basecamp" "basecamp v1" +write_skill "${target}/skills/basecamp-doctor" "basecamp-doctor v1" +printf 'basecamp\nbasecamp-doctor\n' > "${target}/.managed-skills" +echo "# skills" > "${target}/README.md" +publish "State after basecamp/skills#5" + +# --- Interleaved syncs: A, B, A, B --- + +echo "# hey-cli syncs first: restores its skills, touches nothing else" +sync hey-cli "$a" +assert_output "Skills synced to basecamp/skills" +assert_skill hey +assert_skill hey-doctor +assert_skill basecamp +assert_skill basecamp-doctor +assert_content skills/basecamp/SKILL.md "basecamp v1" +assert_content skills/hey/reference/commands.md "nested" +assert_no_path hey/embed.go +assert_no_path hey/.env +assert_no_path hey/.cache +assert_manifest hey-cli hey hey-doctor +assert_no_manifest basecamp-cli +assert_tombstone +assert_author hey-cli +assert_output "first run for hey-cli, removing nothing" +assert "origin main is the seed commit then hey-cli's sync" \ + test "$(git -C "$origin" log --format=%s -2 main | tr '\n' '|')" = "Sync skills from hey-cli v9.9.9|State after basecamp/skills#5|" + +echo "# basecamp-cli syncs: refreshes its skills, leaves hey-cli's" +sync basecamp-cli "$b" +assert_skill hey +assert_skill hey-doctor +assert_skill basecamp +assert_skill basecamp-doctor +assert_content skills/basecamp/SKILL.md "basecamp v2" +assert_manifest hey-cli hey hey-doctor +assert_manifest basecamp-cli basecamp basecamp-doctor +assert_tombstone +assert_author basecamp-cli + +echo "# both sync again with nothing new: no commits, nothing lost" +head_before=$(origin_head) +sync hey-cli "$a" +assert_output "No changes to commit" +sync basecamp-cli "$b" +assert_output "No changes to commit" +assert_head "$head_before" "origin main unchanged by the no-op syncs" +assert_skill hey +assert_skill hey-doctor +assert_skill basecamp +assert_skill basecamp-doctor +assert_manifest hey-cli hey hey-doctor +assert_manifest basecamp-cli basecamp basecamp-doctor +assert_tombstone + +# --- hey-cli drops a skill: only that directory goes --- + +echo "# hey-cli drops hey-doctor" +rm -rf "${a}/skills/hey-doctor" +sync hey-cli "$a" +assert_output "Removing stale skill: hey-doctor" +assert_no_skill hey-doctor +assert_skill hey +assert_skill basecamp +assert_skill basecamp-doctor +assert_manifest hey-cli hey +assert_manifest basecamp-cli basecamp basecamp-doctor +assert_author hey-cli + +# --- A pre-fix sibling rewrote the legacy manifest: still nothing of B's goes --- + +echo "# a pre-fix basecamp-cli rewrites .managed-skills with its own names" +printf 'basecamp\nbasecamp-doctor\n' > "${target}/.managed-skills" +publish "Sync skills from basecamp-cli v0.0.0 (pre-fix script)" +sync hey-cli "$a" +assert_skill basecamp +assert_skill basecamp-doctor +assert_skill hey +assert_tombstone +assert_manifest basecamp-cli basecamp basecamp-doctor + +# --- Two manifests claim one name: removal is refused with a warning --- + +echo "# hey-cli's manifest also lists basecamp, which basecamp-cli owns" +printf 'basecamp\nhey\n' > "${target}/.managed-skills.hey-cli" +publish "Collision: hey-cli claims basecamp" +sync hey-cli "$a" +assert_skill basecamp +assert_content skills/basecamp/SKILL.md "basecamp v2" +assert_output "WARNING: skills/basecamp is no longer in hey-cli's skills but basecamp-cli lists it" +assert_manifest hey-cli hey +assert_manifest basecamp-cli basecamp basecamp-doctor + +# --- Publishing a name another source owns is refused before anything changes --- + +echo "# hey-cli ships a skill named basecamp" +write_skill "${a}/skills/basecamp" "hey-cli's basecamp" +head_before=$(origin_head) +sync_expecting_failure hey-cli "$a" +assert_output "ERROR: skills/basecamp is published by basecamp-cli" +assert_content skills/basecamp/SKILL.md "basecamp v2" +assert_head "$head_before" "origin main unchanged by the refused sync" +rm -rf "${a}/skills/basecamp" + +# --- DRY_RUN=remote clones and shows the diff but commits nothing --- + +echo "# DRY_RUN=remote against origin" +echo "hey v3" > "${a}/skills/hey/SKILL.md" +head_before=$(origin_head) +sync hey-cli "$a" DRY_RUN=remote +assert_output "DRY_RUN=remote: skipping commit and push" +assert_output "+hey v3" +assert_head "$head_before" "origin main unchanged by DRY_RUN=remote" +assert_content skills/hey/SKILL.md "hey v2" + +# --- DRY_RUN=local: no clone at all, lists what would be published --- + +echo "# DRY_RUN=local never reaches the repository" +sync hey-cli "$a" DRY_RUN=local SKILLS_REPO_URL="file://${work}/nowhere.git" +assert_output "skills/hey/SKILL.md" +assert_output "skills/hey/reference/commands.md" +assert_output "No network operations performed" +if grep -q "embed.go" "$out"; then not_ok "preview leaves out embed.go"; else ok "preview leaves out embed.go"; fi + +# --- Without a token, github.com is refused before anything is cloned --- + +echo "# the default target needs SKILLS_TOKEN" +sync_expecting_failure hey-cli "$a" SKILLS_REPO_URL=https://github.com/basecamp/skills.git +assert_output "ERROR: SKILLS_TOKEN is required" +if grep -q "Cloning" "$out"; then not_ok "refused before cloning"; else ok "refused before cloning"; fi + +# --- Another publisher pushes first: the sync is applied again from its tip --- +# +# The script clones afresh, so the sibling's push has to land after that clone and +# before the push. A post-commit hook, reached through the GIT_CONFIG_* environment +# the script's private gitconfig cannot hide, pushes the sibling's commit at exactly +# that moment; the script's push is then rejected as "fetch first". + +sibling="${work}/sibling" +git clone -q "$origin_url" "$sibling" +hooks="${work}/hooks" +mkdir -p "$hooks" +printf '#!/usr/bin/env bash\ngit -C "%s" push -q origin main\n' "$sibling" > "${hooks}/post-commit" +chmod +x "${hooks}/post-commit" +racing() { # source, fixture: sync with the sibling pushing between clone and push + sync "$1" "$2" GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=core.hooksPath "GIT_CONFIG_VALUE_0=${hooks}" +} +racing_expecting_failure() { + sync_expecting_failure "$1" "$2" GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=core.hooksPath "GIT_CONFIG_VALUE_0=${hooks}" +} + +echo "# a concurrent publisher wins the race to origin" +echo "# skills (sibling)" > "${sibling}/README.md" +git -C "$sibling" commit -q -am "Sync skills from basecamp-cli v0.0.1 (concurrent)" +sibling_head=$(git -C "$sibling" rev-parse HEAD) +echo "hey v4" > "${a}/skills/hey/SKILL.md" +racing hey-cli "$a" +assert_output "Push rejected" +assert_output "Skills synced to basecamp/skills" +assert "origin main holds the sibling's commit then the sync" \ + test "$(git -C "$origin" log --format=%s -2 main | tr '\n' '|')" = "Sync skills from hey-cli v9.9.9|Sync skills from basecamp-cli v0.0.1 (concurrent)|" +assert "the sync commit was made on the sibling's tip" test "$(git -C "$origin" rev-parse main^)" = "$sibling_head" +assert_content skills/hey/SKILL.md "hey v4" +assert_content README.md "# skills (sibling)" +assert_author hey-cli + +echo "# a concurrent publisher claims a name this source ships: the retry refuses" +git -C "$sibling" pull -q origin main +printf 'basecamp\nbasecamp-doctor\nhey\n' > "${sibling}/.managed-skills.basecamp-cli" +git -C "$sibling" commit -q -am "Collision: basecamp-cli claims hey (concurrent)" +sibling_head=$(git -C "$sibling" rev-parse HEAD) +echo "hey v5" > "${a}/skills/hey/SKILL.md" +racing_expecting_failure hey-cli "$a" +assert_output "Push rejected" +assert_output "ERROR: skills/hey is published by basecamp-cli" +assert_head "$sibling_head" "origin main tip is the sibling's commit" +assert_content skills/hey/SKILL.md "hey v4" + +echo "# the sibling drops its claim: the next release publishes" +git -C "$sibling" checkout -q HEAD~1 -- .managed-skills.basecamp-cli +git -C "$sibling" commit -q -am "basecamp-cli drops its claim on hey" +git -C "$sibling" push -q origin main +sync hey-cli "$a" +assert_output "Skills synced to basecamp/skills" +assert_content skills/hey/SKILL.md "hey v5" +assert_manifest basecamp-cli basecamp basecamp-doctor +assert_manifest hey-cli hey + +# --- The CLI_NAME default: the one line each CLI's copy of the script changes --- +# +# A copy whose default names another CLI, or still names the seed's placeholder, +# fails here rather than publishing under that source's manifest and bot identity. +# The CLI's Makefile says which source to expect; the seed expects what its own +# CLI_NAME line says. Last, because in hey-cli's or basecamp-cli's repository this +# is that CLI's own sync, which rightly rewrites its manifest from the new tree. + +echo "# with nothing set, the script publishes as the source its CLI_NAME default names" +default_source="${EXPECTED_SOURCE:-$(sed -n 's/^CLI_NAME=.*CLI_NAME:-\([a-z0-9-]*\)}.*/\1/p' "$SYNC_SCRIPT")-cli}" +assert "a default source is known (${default_source})" test "$default_source" != "-cli" +c="${work}/default" +write_skill "${c}/skills/default-skill" "default-skill v1" +sync_as_default "$c" +assert_output "Skills synced to basecamp/skills (main) from ${default_source} v9.9.9" +assert_skill default-skill +assert_manifest "$default_source" default-skill +assert_author "$default_source" +assert_tombstone + +# --- Verdict --- + +echo "" +if [[ "$failures" -eq 0 ]]; then + echo "sync-skills: all assertions passed" +else + echo "sync-skills: ${failures} assertion(s) failed" >&2 + exit 1 +fi