diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 802bee8..1d9f4c4 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -170,7 +170,6 @@ jobs: - name: Sync skills if: steps.check.outputs.ready == 'true' env: - CLI_NAME: cli SKILLS_TOKEN: ${{ steps.skills-token.outputs.token }} RELEASE_TAG: ${{ github.ref_name }} SOURCE_SHA: ${{ github.sha }} diff --git a/.github/workflows/sensitive-change-gate.yml b/.github/workflows/sensitive-change-gate.yml index 0af3330..2e8a075 100644 --- a/.github/workflows/sensitive-change-gate.yml +++ b/.github/workflows/sensitive-change-gate.yml @@ -12,6 +12,7 @@ jobs: with: extra-patterns: | scripts/sync-skills.sh + seed/scripts/sync-skills.sh permissions: contents: read pull-requests: write diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8f1e1e6..704135f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -38,6 +38,9 @@ jobs: - name: Test run: go test -v ./... + - name: Test the skills sync + run: seed/scripts/test-sync-skills.sh + lint: name: Lint runs-on: ubuntu-latest diff --git a/AGENTS.md b/AGENTS.md index ab31a8c..854241c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -53,6 +53,25 @@ When authoring new seed templates: - Keep generated code minimal — point to shared packages where possible - Test by running the `prompts/seed-cli.md` prompt end-to-end +## Skills sync + +Every CLI publishes its `skills//` trees into `skills//` at the root of +`basecamp/skills` (the layout `npx skills add basecamp/skills` reads). The one +implementation is `seed/scripts/sync-skills.sh`; `scripts/sync-skills.sh` here execs it +with `SYNC_SOURCE=cli`, and `actions/sync-skills` runs it from the action's checkout. +Change the seed script, never a copy. + +Several CLIs share that target, so each one owns `.managed-skills.` there +(`` is the publishing repo: `hey-cli`, `basecamp-cli`, `cli`) and removes only +skill directories its own manifest lists, that its skill set no longer has, and that no +other manifest claims. The legacy shared `.managed-skills` is rewritten as a comment-only +tombstone so a sibling still on the pre-fix script deletes nothing (basecamp/skills#5). +The script always clones the target fresh (from `SKILLS_REPO_URL`, default +`https://github.com/basecamp/skills.git`) and pushes only the commit it made, so there is +no checkout to hand it. `seed/scripts/test-sync-skills.sh` runs the script as two CLIs +against a local bare repository — real clones, commits and pushes, no network — and is +part of `make check`. + ## Rubric [RUBRIC.md](RUBRIC.md) defines the quality standard for 37signals Go CLIs. Two profiles: diff --git a/Makefile b/Makefile index 008dcad..1c56bee 100644 --- a/Makefile +++ b/Makefile @@ -1,14 +1,19 @@ .DEFAULT_GOAL := check -.PHONY: check test test-race vet lint fmt fmt-check bench check-all \ +.PHONY: check test test-sync-skills test-race vet lint fmt fmt-check bench check-all \ tidy tidy-check replace-check vuln secrets security release-check release # Default target: fast checks for inner-loop dev. -check: fmt-check vet test +check: fmt-check vet test test-sync-skills test: go test ./... +# The skills sync (seed/scripts/sync-skills.sh, which scripts/sync-skills.sh runs) +# against a throwaway basecamp/skills, as two CLIs publishing in turn +test-sync-skills: + seed/scripts/test-sync-skills.sh + test-race: go test -race ./... @@ -79,7 +84,7 @@ lint-actions: zizmor . # Full suite: everything CI runs. -check-all: fmt-check vet lint lint-actions test-race bench tidy-check +check-all: fmt-check vet lint lint-actions test-race test-sync-skills bench tidy-check # Full pre-flight for release release-check: check-all replace-check vuln secrets diff --git a/README.md b/README.md index 558495a..92c1431 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,7 @@ Reusable composite actions in `actions/`: |--------|-------------| | `rubric-check` | Score a built CLI binary against the 37signals CLI rubric | | `surface-compat` | Fail CI if CLI flags or subcommands were removed (breaking change) | -| `sync-skills` | Sync embedded SKILL.md files to the `basecamp/skills` distribution repo on release | +| `sync-skills` | Publish embedded skills to the `basecamp/skills` distribution repo on release (runs `seed/scripts/sync-skills.sh`) | Usage in a workflow: @@ -86,12 +86,18 @@ The `skills/` directory contains agent skills distributed via `basecamp/skills`: - `rubric-audit` — Audit a Go CLI against the rubric +On release, `scripts/sync-skills.sh` publishes each one to `skills//` in +`basecamp/skills`, where every 37signals CLI publishes its own. Each publisher owns a +manifest there, `.managed-skills.`, and only ever removes skills it listed — +the scheme, and the seed script every CLI runs, are described in +`seed/scripts/sync-skills.sh`. + ## Development Requires Go 1.24+. ``` -make check # fmt-check + vet + test (inner-loop dev) +make check # fmt-check + vet + test + test-sync-skills (inner-loop dev) make test # go test ./... make test-race # go test -race ./... make lint # golangci-lint run diff --git a/actions/sync-skills/action.yml b/actions/sync-skills/action.yml index b8de04c..5fd19f2 100644 --- a/actions/sync-skills/action.yml +++ b/actions/sync-skills/action.yml @@ -1,5 +1,9 @@ name: Sync Skills -description: Sync embedded SKILL.md files to basecamp/skills distribution repo +description: Publish embedded skills to the basecamp/skills distribution repo + +# Runs seed/scripts/sync-skills.sh from this action's own checkout, so the sync +# logic lives in one place. The script's header documents the manifest scheme and +# every env var; the inputs here map onto those one to one. inputs: skills-token: @@ -12,10 +16,18 @@ inputs: description: The source commit SHA required: true cli-name: - description: The CLI name (used as directory prefix in skills repo) + description: The CLI name; the publishing source is -cli required: true + source: + description: Override the publishing source name (default -cli) + required: false + default: "" + skills-source: + description: Directory holding the skills tree + required: false + default: skills dry-run: - description: '"local" to skip push, "remote" to skip commit+push, empty for real run' + description: '"local" to skip the push, "remote" to skip commit and push, empty for a real run' required: false default: "" @@ -29,105 +41,15 @@ runs: RELEASE_TAG: ${{ inputs.release-tag }} SOURCE_SHA: ${{ inputs.source-sha }} CLI_NAME: ${{ inputs.cli-name }} + SYNC_SOURCE: ${{ inputs.source }} + SKILLS_SOURCE: ${{ inputs.skills-source }} DRY_RUN: ${{ inputs.dry-run }} + ACTION_PATH: ${{ github.action_path }} run: | - set -euo pipefail - SKILLS_REPO="basecamp/skills" - SKILLS_DIR="skills" - MANAGED_MANIFEST=".managed-skills" - - # Clone the skills repo - WORK_DIR=$(mktemp -d) - trap 'rm -rf "$WORK_DIR"' EXIT - - echo "::group::Clone skills repo" - if ! git clone "https://x-access-token:${SKILLS_TOKEN}@github.com/${SKILLS_REPO}.git" "$WORK_DIR/skills-repo" 2>&1 | grep -v 'x-access-token'; then - echo "::error::Failed to clone ${SKILLS_REPO}" - exit 1 - fi - echo "::endgroup::" - - TARGET_DIR="${WORK_DIR}/skills-repo" - - # Collect skill directories - SKILL_DIRS=() - for skill_dir in ${SKILLS_DIR}/*/; do - if [[ -f "${skill_dir}/SKILL.md" ]]; then - SKILL_DIRS+=("$skill_dir") - fi - done - - if [[ ${#SKILL_DIRS[@]} -eq 0 ]]; then - echo "No skills found in ${SKILLS_DIR}/" + # The release workflows check this before generating a token; the action + # has no step in front of it, so a CLI with no skills yet is a no-op here. + if ! compgen -G "${SKILLS_SOURCE}/*/SKILL.md" > /dev/null; then + echo "No skill files found under ${SKILLS_SOURCE}/ — skipping sync" exit 0 fi - - echo "Found ${#SKILL_DIRS[@]} skill(s) to sync" - - # Copy skills - MANAGED_SKILLS=() - for skill_dir in "${SKILL_DIRS[@]}"; do - skill_name=$(basename "$skill_dir") - dest="${TARGET_DIR}/${CLI_NAME}/${skill_name}" - echo " Syncing ${skill_name}..." - mkdir -p "$dest" - # Preserve subdirectory structure; exclude Go sources and dotfiles - (cd "$skill_dir" && find . -type f ! -name '*.go' ! -name '.*' | while read -r f; do - mkdir -p "$dest/$(dirname "$f")" - cp "$f" "$dest/$f" - done) - MANAGED_SKILLS+=("${CLI_NAME}/${skill_name}") - done - - # Update manifest - MANIFEST_PATH="${TARGET_DIR}/${MANAGED_MANIFEST}" - if [[ -f "$MANIFEST_PATH" ]]; then - grep -v "^${CLI_NAME}/" "$MANIFEST_PATH" > "${MANIFEST_PATH}.tmp" || true - mv "${MANIFEST_PATH}.tmp" "$MANIFEST_PATH" - fi - for skill in "${MANAGED_SKILLS[@]}"; do - echo "$skill" >> "$MANIFEST_PATH" - done - sort -u -o "$MANIFEST_PATH" "$MANIFEST_PATH" - - # Remove stale skills - if [[ -d "${TARGET_DIR}/${CLI_NAME}" ]]; then - for existing in "${TARGET_DIR}/${CLI_NAME}"/*/; do - existing_name=$(basename "$existing") - found=false - for skill_dir in "${SKILL_DIRS[@]}"; do - [[ "$(basename "$skill_dir")" == "$existing_name" ]] && found=true && break - done - if [[ "$found" == "false" ]]; then - echo " Removing stale: ${existing_name}" - rm -rf "$existing" - fi - done - fi - - [[ "$DRY_RUN" == "remote" ]] && echo "DRY_RUN=remote: done" && exit 0 - - # Commit - cd "$TARGET_DIR" - git add -A - if git diff --cached --quiet; then - echo "No changes to commit" - exit 0 - fi - - git config user.name "${CLI_NAME}-cli[bot]" - git config user.email "${CLI_NAME}-cli[bot]@users.noreply.github.com" - git commit -m "Sync ${CLI_NAME} skills from ${RELEASE_TAG} - - Source: ${SOURCE_SHA}" - - [[ "$DRY_RUN" == "local" ]] && echo "DRY_RUN=local: done" && exit 0 - - # Push - echo "::group::Push to ${SKILLS_REPO}" - if ! git push origin main; then - git pull --rebase origin main - git push origin main - fi - echo "::endgroup::" - echo "Skills synced successfully" + bash "${ACTION_PATH}/../../seed/scripts/sync-skills.sh" diff --git a/prompts/seed-cli.md b/prompts/seed-cli.md index 487efff..9ff3244 100644 --- a/prompts/seed-cli.md +++ b/prompts/seed-cli.md @@ -96,7 +96,8 @@ You are creating a new Go CLI for a 37signals product using the seed templates. - `seed/scripts/check-cli-surface-diff.sh` → `scripts/check-cli-surface-diff.sh` (copy; chmod +x) - `seed/scripts/collect-profile.sh` → `scripts/collect-profile.sh` (copy; chmod +x) - `seed/scripts/publish-aur.sh` → `scripts/publish-aur.sh` (copy; chmod +x) - - `seed/scripts/sync-skills.sh` → `scripts/sync-skills.sh` (copy; chmod +x) + - `seed/scripts/sync-skills.sh` → `scripts/sync-skills.sh` (copy; chmod +x; set `CLI_NAME`) + - `seed/scripts/test-sync-skills.sh` → `scripts/test-sync-skills.sh` (copy; chmod +x) **GitHub infra (copy as-is unless .tmpl):** - `seed/.github/workflows/test.yml` → `.github/workflows/test.yml` (update env vars, GOPRIVATE) diff --git a/scripts/sync-skills.sh b/scripts/sync-skills.sh index 5c383e6..fb7c007 100755 --- a/scripts/sync-skills.sh +++ b/scripts/sync-skills.sh @@ -1,157 +1,12 @@ #!/usr/bin/env bash -# sync-skills.sh — Sync embedded skills to basecamp/skills distribution repo. +# sync-skills.sh — Publish this repo's skills (skills/rubric-audit) to basecamp/skills. # -# Run from CI on release (tag push). Copies skills/*/SKILL.md to the -# basecamp/skills repo, commits, and pushes. -# -# Required env vars: -# SKILLS_TOKEN — GitHub token with push access to basecamp/skills -# RELEASE_TAG — The release tag (e.g., v1.2.3) -# SOURCE_SHA — The source commit SHA -# -# Optional env vars: -# DRY_RUN — "local" to skip push, "remote" to skip commit+push +# The implementation is the seed's, seed/scripts/sync-skills.sh, run as it is: one +# script, exercised here before any CLI inherits it. Only the identity differs — +# this repo is basecamp/cli, so the publishing source is `cli`, not `-cli`. +# Knobs and env vars are documented in the seed script's header. set -euo pipefail -CLI_NAME="${CLI_NAME:-cli}" -SKILLS_REPO="basecamp/skills" -SKILLS_DIR="skills" -MANAGED_MANIFEST=".managed-skills" - -: "${RELEASE_TAG:?RELEASE_TAG is required}" -: "${SOURCE_SHA:?SOURCE_SHA is required}" - -DRY_RUN="${DRY_RUN:-}" -SKILLS_TOKEN="${SKILLS_TOKEN:-}" - -# SKILLS_TOKEN is required unless running a local dry-run -if [ -z "$SKILLS_TOKEN" ] && [ "$DRY_RUN" != "local" ]; then - echo "Error: SKILLS_TOKEN is required (set DRY_RUN=local to skip clone)" >&2 - exit 1 -fi - -# Clone the skills repo (skipped for local dry-run) -WORK_DIR=$(mktemp -d) -trap 'rm -rf "$WORK_DIR"' EXIT - -if [ "$DRY_RUN" = "local" ] && [ -z "$SKILLS_TOKEN" ]; then - echo "Local dry-run: creating stub target directory..." - mkdir -p "$WORK_DIR/skills-repo" - (cd "$WORK_DIR/skills-repo" && git init -q) -else - echo "Cloning ${SKILLS_REPO}..." - # Use a temp gitconfig so the token never appears in process args - TEMP_GITCONFIG="${WORK_DIR}/.gitconfig" - cat > "$TEMP_GITCONFIG" < "${MANIFEST_PATH}.tmp" || true - mv "${MANIFEST_PATH}.tmp" "$MANIFEST_PATH" -fi - -# Append current skills -for skill in "${MANAGED_SKILLS[@]}"; do - echo "$skill" >> "$MANIFEST_PATH" -done -sort -u -o "$MANIFEST_PATH" "$MANIFEST_PATH" - -# Check for stale skills to remove -if [[ -d "${TARGET_DIR}/${CLI_NAME}" ]]; then - for existing in "${TARGET_DIR}/${CLI_NAME}"/*/; do - existing_name=$(basename "$existing") - found=false - for skill_dir in "${SKILL_DIRS[@]}"; do - if [[ "$(basename "$skill_dir")" == "$existing_name" ]]; then - found=true - break - fi - done - if [[ "$found" == "false" ]]; then - echo " Removing stale skill: ${existing_name}" - rm -rf "$existing" - fi - done -fi - -if [[ "$DRY_RUN" == "remote" ]]; then - echo "DRY_RUN=remote: skipping commit and push" - exit 0 -fi - -# Commit and push -cd "$TARGET_DIR" -git add -A - -if git diff --cached --quiet; then - echo "No changes to commit" - exit 0 -fi - -git config user.name "${CLI_NAME}-cli[bot]" -git config user.email "${CLI_NAME}-cli[bot]@users.noreply.github.com" - -git commit -m "$(cat </ 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: -# SKILLS_TOKEN — GitHub token with push access to basecamp/skills -# RELEASE_TAG — The release tag (e.g., v1.2.3) -# SOURCE_SHA — The source commit SHA +# 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: -# DRY_RUN — "local" to skip push, "remote" to skip commit+push +# 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 +# # # TODO: Replace CLI_NAME with your CLI name. set -euo pipefail CLI_NAME="${CLI_NAME:-mycli}" -SKILLS_REPO="basecamp/skills" -SKILLS_DIR="skills" -MANAGED_MANIFEST=".managed-skills" +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:-}" -: "${RELEASE_TAG:?RELEASE_TAG is required}" -: "${SOURCE_SHA:?SOURCE_SHA is required}" +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}}" -DRY_RUN="${DRY_RUN:-}" -SKILLS_TOKEN="${SKILLS_TOKEN:-}" +# --- Helpers --- -# SKILLS_TOKEN is required unless running a local dry-run -if [ -z "$SKILLS_TOKEN" ] && [ "$DRY_RUN" != "local" ]; then - echo "Error: SKILLS_TOKEN is required (set DRY_RUN=local to skip clone)" >&2 - exit 1 -fi +die() { echo "ERROR: $*" >&2; exit 1; } +warn() { echo "WARNING: $*" >&2; } -# Clone the skills repo (skipped for local dry-run) -WORK_DIR=$(mktemp -d) -trap 'rm -rf "$WORK_DIR"' EXIT - -if [ "$DRY_RUN" = "local" ] && [ -z "$SKILLS_TOKEN" ]; then - echo "Local dry-run: creating stub target directory..." - mkdir -p "$WORK_DIR/skills-repo" - (cd "$WORK_DIR/skills-repo" && git init -q) -else - echo "Cloning ${SKILLS_REPO}..." - # Use a temp gitconfig so the token never appears in process args - TEMP_GITCONFIG="${WORK_DIR}/.gitconfig" - cat > "$TEMP_GITCONFIG" < "${MANIFEST_PATH}.tmp" || true - mv "${MANIFEST_PATH}.tmp" "$MANIFEST_PATH" +# --- 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" <> "$MANIFEST_PATH" -done -sort -u -o "$MANIFEST_PATH" "$MANIFEST_PATH" - -# Check for stale skills to remove -if [[ -d "${TARGET_DIR}/${CLI_NAME}" ]]; then - for existing in "${TARGET_DIR}/${CLI_NAME}"/*/; do - existing_name=$(basename "$existing") - found=false - for skill_dir in "${SKILL_DIRS[@]}"; do - if [[ "$(basename "$skill_dir")" == "$existing_name" ]]; then - found=true - break - fi - done - if [[ "$found" == "false" ]]; then - echo " Removing stale skill: ${existing_name}" - rm -rf "$existing" +# --- Clone the target --- + +target="${tmpdir}/skills" +echo "Cloning ${TARGET_REPO} into ${target}..." +git clone -q --depth 1 --branch "$TARGET_BRANCH" "$SKILLS_REPO_URL" "$target" + +# --- Apply the sync to the target's working tree --- +# +# Every decision here is made against the tree as it stands, so a retry after a +# rejected push runs this again from the remote's new tip instead of replaying +# decisions made against a stale one. + +apply_sync() { + local name other + local previously_published=() + + # Refuse a name another source has published + for name in "${skill_names[@]}"; do + if other=$(claimed_by_other "$name"); then + die "skills/${name} is published by ${other} (listed in ${LEGACY_MANIFEST}.${other}); rename the skill or settle ownership upstream" fi done -fi -if [[ "$DRY_RUN" == "remote" ]]; then - echo "DRY_RUN=remote: skipping commit and push" - exit 0 -fi + echo "Copying skills into ${target}/${SKILLS_SUBDIR}/..." + copy_skills "${target}/${SKILLS_SUBDIR}" -# Commit and push -cd "$TARGET_DIR" -git add -A + # Remove what this source published before and no longer has + while IFS= read -r name; do + previously_published+=("$name") + done < <(read_manifest "${target}/${MANIFEST}") -if git diff --cached --quiet; then - echo "No changes to commit" - exit 0 -fi + if [[ ! -f "${target}/${MANIFEST}" ]]; then + echo "No ${MANIFEST} yet: first run for ${SYNC_SOURCE}, removing nothing" + fi + + 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 + if [[ -d "${target}/${SKILLS_SUBDIR}/${name}" ]]; then + echo "Removing stale skill: ${name}" + rm -rf "${target:?}/${SKILLS_SUBDIR}/${name}" + fi + done -git config user.name "${CLI_NAME}-cli[bot]" -git config user.email "${CLI_NAME}-cli[bot]@users.noreply.github.com" + # 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 -git commit -m "$(cat <&1 +} + +if ! output=$(push_target); then + 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 "Skills synced successfully" +echo "" +echo "Skills synced to ${TARGET_REPO} (${TARGET_BRANCH}) from ${SYNC_SOURCE} ${RELEASE_TAG}" diff --git a/seed/scripts/test-sync-skills.sh b/seed/scripts/test-sync-skills.sh new file mode 100755 index 0000000..5e997b5 --- /dev/null +++ b/seed/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