diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index da12acce..dc0a2fc5 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -66,7 +66,7 @@ jobs: - name: Run release quality gate run: | - make check-release-lockstep fmt-check vet lint test test-e2e tidy-check check-surface + make check-release-lockstep fmt-check vet lint test test-e2e test-sync-skills tidy-check check-surface go test -race -count=1 ./... - name: Run govulncheck @@ -565,7 +565,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: @@ -576,6 +598,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 4a387664..cb8b91af 100644 --- a/.github/workflows/sync-skills.yml +++ b/.github/workflows/sync-skills.yml @@ -6,8 +6,10 @@ # needs 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.hey-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: @@ -53,8 +55,8 @@ jobs: exit 1 fi - # sync-skills.sh mirrors the tree wholesale, so syncing an older tag - # would roll basecamp/skills back. The only reason to run this by + # sync-skills.sh mirrors this CLI's skills wholesale, so syncing an + # older tag would roll them back in basecamp/skills. The only reason to run this by # hand is that the newest release failed to sync — so require exactly # that release. latest=$(gh release view --repo "${GITHUB_REPOSITORY}" --json tagName --jq .tagName) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 0cc80074..e1e1a812 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -171,6 +171,9 @@ jobs: - name: Run bats suite run: make test-e2e + - name: Test the skills sync + run: make test-sync-skills + installer-bash32: name: Installer (bash 3.2) runs-on: macos-latest diff --git a/Makefile b/Makefile index c6f7e56c..f65605a9 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: build test test-unit test-e2e test-smoke preview-callback coverage fmt fmt-check vet lint tidy tidy-check \ +.PHONY: build test test-unit test-e2e test-sync-skills test-smoke preview-callback coverage fmt fmt-check vet lint tidy tidy-check \ race-test vuln gosec secrets replace-check check-toolchain check security \ release-check release test-release bench bench-save bench-compare \ check-surface update-surface check-surface-compat check-size check-lint-lockstep \ @@ -25,6 +25,7 @@ help: @echo " make test-unit Run unit tests" @echo " make test Alias for test-unit" @echo " make test-e2e Run the bats suite (installer and script contracts)" + @echo " make test-sync-skills Run the skills sync as two CLIs against a throwaway target" @echo " make test-smoke Run smoke tests against a live server" @echo " make preview-callback Preview the OAuth callback screens in a browser" @echo " make coverage Run cross-package coverage and enforce the 70.8% floor" @@ -43,7 +44,7 @@ help: @echo " make secrets Run gitleaks secret scan" @echo " make replace-check Guard against replace directives in go.mod" @echo "" - @echo " make check fmt-check + vet + lint + test-unit + tidy-check" + @echo " make check fmt-check + vet + lint + test-unit + test-sync-skills + tidy-check" @echo " make security lint + vuln + gosec + secrets" @echo " make release-check check + replace-check + vuln + gosec + race-test" @echo " make release Run release preflight and tag (VERSION=v1.2.3 [DRY_RUN=1])" @@ -103,6 +104,10 @@ preview-callback: check-toolchain test-e2e: @./tests/e2e/run.sh +# Run the skills sync against a throwaway basecamp/skills, as two CLIs publishing in turn +test-sync-skills: + EXPECTED_SOURCE=hey-cli scripts/test-sync-skills.sh + # Run smoke tests against a live HEY server. # Requires: a running server (default http://app.hey.localhost:3003) and Chrome. # Override defaults: make test-smoke HEY_SMOKE_BASE_URL=... HEY_SMOKE_EMAIL=... HEY_SMOKE_PASSWORD=... @@ -171,7 +176,7 @@ replace-check: fi # Local CI gate -check: fmt-check vet lint test-unit tidy-check check-surface check-release-lockstep +check: fmt-check vet lint test-unit test-sync-skills tidy-check check-surface check-release-lockstep # Verify every workflow lints with the same golangci-lint version check-lint-lockstep: diff --git a/RELEASING.md b/RELEASING.md index 13d41d65..cfe73a09 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -216,13 +216,30 @@ account, store the private key as `AUR_KEY`. ## Skills sync -Stable releases mirror `skills/` into `basecamp/skills`. If that job fails, a -`skills-sync`-labeled issue is filed; 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. +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.hey-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.hey-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). 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 `make check`) pins the contract by running the script as both +CLIs against a local bare repository — real clones, commits and pushes, no network. + +If that job fails, a `skills-sync`-labeled issue is filed; 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. ## Local dry runs diff --git a/scripts/sync-skills.sh b/scripts/sync-skills.sh index d44a63bc..953ad2b0 100755 --- a/scripts/sync-skills.sh +++ b/scripts/sync-skills.sh @@ -1,172 +1,288 @@ #!/usr/bin/env bash -set -euo pipefail - -# Sync skills from hey-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) -# SKILLS_SOURCE - Optional: directory holding the skills tree (default: skills) -# DRY_RUN - Optional: "local" (no network) or "remote" (clone but skip push) +set -euo pipefail + +CLI_NAME="${CLI_NAME:-hey}" +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:-}" -# The skills tree to mirror. The manual recovery workflow (sync-skills.yml) -# points this at a separate checkout of the release tag, so the sync logic can -# come from a newer ref (with fixes) than the content it mirrors. -SKILLS_SOURCE="${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; } -assert_remote_url() { - local url - url=$(git -C "$1" remote get-url origin) - local stripped - stripped=$(echo "$url" | sed -E 's/\.git$//') - 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 +# A skill directory name or a source name: nothing a path could smuggle in. +plain_name() { + [[ "$1" != "." && "$1" != ".." && "$1" =~ ^[a-zA-Z0-9._-]+$ ]] } -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'" +in_list() { + local needle="$1" item + shift + for item in "$@"; do + [[ "$item" == "$needle" ]] && return 0 + done + return 1 } +# 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" +} + +# 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" - 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) ===" - 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 - 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 diff --git a/tests/e2e/sync_skills.bats b/tests/e2e/sync_skills.bats index 34c52c77..99215cde 100644 --- a/tests/e2e/sync_skills.bats +++ b/tests/e2e/sync_skills.bats @@ -31,7 +31,7 @@ teardown() { @test "defaults to the skills/ tree and excludes *.go and dotfiles" { run "$SYNC" [ "$status" -eq 0 ] - [[ "$output" == *"Found 1 skill(s): skills/hey"* ]] + [[ "$output" == *"Found 1 skill(s) in skills/: hey"* ]] [[ "$output" == *"skills/hey/SKILL.md"* ]] [[ "$output" == *"skills/hey/reference/api.md"* ]] [[ "$output" != *"tool.go"* ]] @@ -41,7 +41,7 @@ teardown() { @test "SKILLS_SOURCE points the sync at a tagged checkout" { SKILLS_SOURCE=release/skills run "$SYNC" [ "$status" -eq 0 ] - [[ "$output" == *"Found 1 skill(s): release/skills/hey"* ]] + [[ "$output" == *"Found 1 skill(s) in release/skills/: hey"* ]] [[ "$output" == *"skills/hey/SKILL.md"* ]] [[ "$output" != *"reference/api.md"* ]] }