diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 3dc5a71..c88a947 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -150,3 +150,120 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} AUR_KEY: ${{ secrets.AUR_KEY }} + + # Mirrors skills/ into basecamp/skills, the distribution repo shared by every + # CLI. The script owns only this CLI's skills there (.managed-skills.fizzy-cli) + # and never touches a sibling's. continue-on-error: a stale skills repo must + # not fail a release that has already shipped; the failure step files an + # issue instead, and sync-skills.yml is the manual way back. + sync-skills: + name: Sync skills + needs: [release] + if: startsWith(github.ref, 'refs/tags/v') && !contains(github.ref_name, '-') + continue-on-error: true + concurrency: + group: sync-skills + cancel-in-progress: false + runs-on: ubuntu-latest + environment: release + timeout-minutes: 10 + permissions: + contents: read + issues: write + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + 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: + client-id: ${{ vars.RELEASE_CLIENT_ID }} + private-key: ${{ secrets.RELEASE_APP_PRIVATE_KEY }} + owner: basecamp + repositories: skills + 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 }} + RELEASE_TAG: ${{ github.ref_name }} + SOURCE_SHA: ${{ github.sha }} + run: scripts/sync-skills.sh + + - name: Notify on sync failure + if: failure() + env: + GH_TOKEN: ${{ github.token }} + REPO_SLUG: ${{ github.repository }} + REF_NAME: ${{ github.ref_name }} + RUN_ID: ${{ github.run_id }} + run: | + RUN_URL="https://github.com/${REPO_SLUG}/actions/runs/${RUN_ID}" + LABEL="skills-sync" + TITLE="Skills sync failure" + BODY="The automatic skills sync to basecamp/skills for [${REF_NAME}](${RUN_URL}) failed. Check the workflow run for details; once fixed, rerun the failed jobs in that run or dispatch the Sync skills workflow for ${REF_NAME}." + + # Filed on this repo (the only one github.token can write to), one + # open issue per outage. Deduped by label, not title: a title lookup + # misses the moment someone retitles the issue while triaging it. + # The lookup fails closed — an API error must not read as "no open + # issue" and file a duplicate; the annotation below still records + # the failure. + notify_status=0 + if ! matches=$(gh issue list --repo "$REPO_SLUG" --state open --label "$LABEL" \ + --json number --jq '[.[].number] | join(" ")'); then + echo "::error::Could not list open '${LABEL}' issues in ${REPO_SLUG}. Filing nothing rather than risk a duplicate." + notify_status=2 + else + read -ra issues <<< "$matches" + case "${#issues[@]}" in + 0) + echo "No open '${LABEL}' issue; filing one." + # --force makes this idempotent, so the label exists before the + # first issue carries it (gh issue create refuses an unknown label). + gh label create "$LABEL" --repo "$REPO_SLUG" --force \ + --color d93f0b --description "The release-time sync to basecamp/skills failed" \ + && gh issue create --repo "$REPO_SLUG" --title "$TITLE" --body "$BODY" --label "$LABEL" \ + || notify_status=$? + ;; + 1) + echo "Commenting on existing '${LABEL}' issue #${issues[0]}." + gh issue comment --repo "$REPO_SLUG" "${issues[0]}" --body "$BODY" || notify_status=$? + ;; + *) + # Picking one arbitrarily would scatter one outage's history + # across issues a human already decided to keep separate. + echo "::error::${#issues[@]} open issues carry the '${LABEL}' label (${issues[*]}). Refusing to guess which one to update — consolidate them, then re-run." + notify_status=3 + ;; + esac + fi + + echo "::error::Skills sync to basecamp/skills failed for ${REF_NAME}. See ${RUN_URL}" + exit "$notify_status" diff --git a/.github/workflows/sensitive-change-gate.yml b/.github/workflows/sensitive-change-gate.yml index d2023eb..78e18f5 100644 --- a/.github/workflows/sensitive-change-gate.yml +++ b/.github/workflows/sensitive-change-gate.yml @@ -12,6 +12,7 @@ jobs: with: extra-patterns: | scripts/publish-aur.sh + scripts/sync-skills.sh permissions: contents: read pull-requests: write diff --git a/.github/workflows/sync-skills.yml b/.github/workflows/sync-skills.yml new file mode 100644 index 0000000..82eb287 --- /dev/null +++ b/.github/workflows/sync-skills.yml @@ -0,0 +1,119 @@ +# Manual skills sync, for when the release-time sync could not run. +# +# The sync-skills job in release.yml is `needs: [release]`, so anything that +# fails the release job after publication skips the sync entirely and leaves +# basecamp/skills stale against a shipped release. A skipped or failed sync +# 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 — only this CLI's skills, tracked in its own manifest there +# (.managed-skills.fizzy-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: + workflow_dispatch: + inputs: + tag: + description: 'Release tag to sync from, with the v prefix (e.g. v4.1.0)' + required: true + type: string + dry_run: + description: 'Preview the sync without pushing' + required: false + default: false + type: boolean + +permissions: {} + +# Shared with release.yml's sync-skills job so a manual run cannot interleave +# with an automatic one. +concurrency: + group: sync-skills + cancel-in-progress: false + +jobs: + sync: + name: Sync skills + runs-on: ubuntu-latest + timeout-minutes: 10 + environment: release + permissions: + contents: read + steps: + - name: Validate tag input + env: + TAG: ${{ inputs.tag }} + GH_TOKEN: ${{ github.token }} + run: | + # No prerelease suffix: the automatic sync in release.yml excludes + # prereleases deliberately, and this path must not smuggle prerelease + # content into the distribution repo's main branch. + if [[ ! "$TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+$ ]]; then + echo "::error::Invalid tag '${TAG}' — expected a stable v-prefixed semver tag (e.g. v4.1.0)" + exit 1 + fi + + # 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) + if [ "$TAG" != "$latest" ]; then + echo "::error::Refusing to sync ${TAG}: the latest stable release is ${latest}. Syncing an older tag would roll basecamp/skills back." + exit 1 + fi + + # 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: + 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 + # cannot run tokenless. Dry runs get a read-only token, which is also what + # stops DRY_RUN=remote from pushing even if the script were wrong. + - name: Generate token for skills repo + id: skills-token + uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 + with: + client-id: ${{ vars.RELEASE_CLIENT_ID }} + private-key: ${{ secrets.RELEASE_APP_PRIVATE_KEY }} + owner: basecamp + 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 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 + env: + TAG: ${{ inputs.tag }} + run: echo "sha=$(git -C release rev-parse --verify "refs/tags/${TAG}^{commit}")" >> "$GITHUB_OUTPUT" + + # DRY_RUN=remote clones the actual target and stops before the push, so + # the preview shows the real diff including deletions. + - name: Sync skills to distribution repo + env: + 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 bc874b7..ae8c456 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -34,6 +34,9 @@ jobs: - name: Run tests run: go test -v ./... + - name: Test the skills sync + run: make test-sync-skills + - name: Build run: go build -o bin/fizzy ./cmd/fizzy diff --git a/AGENTS.md b/AGENTS.md index a7a74cd..1a057f2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,7 +13,7 @@ fizzy-cli/ │ ├── mcpserver/ # `fizzy mcp` MCP server (catalog/ synced from fizzy-mcp-server) │ └── render/ # Output rendering (styled, markdown, columns) ├── e2e/ # Go integration tests -├── skills/ # Agent skills +├── skills/ # Agent skills (mirrored to basecamp/skills on release) └── .claude-plugin/ # Claude Code integration ``` @@ -113,7 +113,7 @@ Token-based via personal access tokens. Run `fizzy setup` for interactive config ## Checks -`make check` runs `fmt-check vet lint tidy-check race-test`. There is no `surface-check` +`make check` runs `fmt-check vet lint tidy-check race-test test-sync-skills`. There is no `surface-check` in that list, but the surface gate still runs: `race-test` is `go test -race -count=1 ./internal/...`, which includes `internal/commands.TestSurfaceSnapshot`. diff --git a/Makefile b/Makefile index 29495b5..a8aaeac 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: test test-unit test-e2e e2e test-go test-file e2e-file test-run e2e-run build clean tidy help \ +.PHONY: test test-unit test-sync-skills test-e2e e2e test-go test-file e2e-file test-run e2e-run build clean tidy help \ check-toolchain fmt fmt-check vet lint tidy-check race-test vuln secrets \ replace-check security check release-check release tools \ surface-snapshot surface-check lint-actions @@ -28,6 +28,7 @@ help: @echo "Usage:" @echo " make build Build the CLI" @echo " make test-unit Run unit tests (no API required)" + @echo " make test-sync-skills Test the skills sync script (no API required)" @echo " make e2e Run owner-only CLI contract e2e tests" @echo " make test-e2e Alias for e2e" @echo " make test Alias for e2e" @@ -50,7 +51,7 @@ help: @echo "" @echo " make lint-actions Lint GitHub Actions workflows" @echo " make security lint + vuln + secrets" - @echo " make check fmt-check + vet + lint + test-unit + tidy-check" + @echo " make check fmt-check + vet + lint + tidy-check + race-test + test-sync-skills" @echo " make release-check check + replace-check + vuln + race-test" @echo " make release Run release preflight and tag" @echo " make tools Install dev tools" @@ -92,6 +93,10 @@ build: check-toolchain test-unit: check-toolchain go test -v ./internal/... +# Test the skills sync script against a throwaway basecamp/skills checkout (no network) +test-sync-skills: + EXPECTED_SOURCE=fizzy-cli scripts/test-sync-skills.sh + # Run e2e tests (requires API credentials) e2e: build @if [ -z "$$FIZZY_TEST_TOKEN" ]; then echo "Error: FIZZY_TEST_TOKEN not set"; exit 1; fi @@ -175,8 +180,8 @@ replace-check: # Security suite security: lint vuln secrets -# Local CI gate (fmt, vet, lint, tidy, race-test) -check: fmt-check vet lint tidy-check race-test +# Local CI gate (fmt, vet, lint, tidy, race-test, skills sync test) +check: fmt-check vet lint tidy-check race-test test-sync-skills # Release preflight release-check: check replace-check vuln diff --git a/RELEASING.md b/RELEASING.md index 46efbe4..977aac2 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -20,6 +20,7 @@ Pushing the tag triggers the GitHub Actions release workflow, which: 7. For stable tags only, publishes the Homebrew cask to `basecamp/homebrew-tap` 8. For stable tags only, publishes the Scoop manifest to `basecamp/homebrew-tap` 9. For stable tags only, publishes to AUR (if `AUR_KEY` configured) +10. For stable tags only, mirrors `skills/` into [basecamp/skills](https://github.com/basecamp/skills) ## Versioning @@ -45,18 +46,19 @@ Prerelease behavior is intentionally conservative so existing package-manager us | Homebrew | Updates the normal `basecamp/tap/fizzy` cask. `brew upgrade fizzy` can move users to `v4.0.0`. | Does not update the normal cask (`skip_upload: auto`). Existing `brew upgrade fizzy` users stay on the latest stable cask. | | Scoop | Updates the normal `fizzy` manifest. `scoop update fizzy` can move users to `v4.0.0`. | Does not update the normal manifest (`skip_upload: auto`). Existing Scoop users stay on the latest stable manifest. | | AUR | Updates the normal `fizzy-cli` package if `AUR_KEY` is configured. | Skips the AUR publish job. Existing AUR users stay on the latest stable package. | +| basecamp/skills | Mirrors `skills/fizzy` into the shared skills repo. | Skips the skills sync job. `npx skills add basecamp/skills` keeps serving the latest stable skill. | | Go install | The git tag exists for users who explicitly request it. | The prerelease tag exists for users who explicitly request it; no package-manager manifest is updated. | Technical testers can install prereleases explicitly from the GitHub release assets, for example by downloading the asset for their OS/architecture from `https://github.com/basecamp/fizzy-cli/releases/tag/v4.0.0-beta1`. ## CI Secrets -All release credentials live in the `release` environment (`Settings > Environments > release`), so they are only exposed to jobs that pass the environment's required-reviewer gate. There are no repository-level release secrets. `HOMEBREW_TAP_TOKEN` does not exist as a stored secret — it is minted per-run from the `cli-release-bot` GitHub App credentials. +All release credentials live in the `release` environment (`Settings > Environments > release`), so they are only exposed to jobs that pass the environment's required-reviewer gate. There are no repository-level release secrets. `HOMEBREW_TAP_TOKEN` and the skills sync token do not exist as stored secrets — each is minted per-run from the `cli-release-bot` GitHub App credentials, scoped to the one repo that job pushes to (`homebrew-tap`, `skills`). The app must be installed on both repos with contents write access. | Name | Type | Purpose | |------|------|---------| | `RELEASE_CLIENT_ID` | variable | GitHub App client ID for `cli-release-bot` | -| `RELEASE_APP_PRIVATE_KEY` | secret | GitHub App private key for tap push | +| `RELEASE_APP_PRIVATE_KEY` | secret | GitHub App private key for tap and skills push | | `AUR_KEY` | secret | ed25519 SSH private key for AUR (optional) | | `MACOS_SIGN_P12` | secret | Base64-encoded Developer ID Application .p12 | | `MACOS_SIGN_PASSWORD` | secret | Password for the .p12 certificate | @@ -74,6 +76,7 @@ Set a secret with `gh secret set --env release -R basecamp/fizzy-cli`; th | Homebrew | `basecamp/homebrew-tap` Casks/fizzy.rb | GoReleaser (stable tags only) | | Scoop | `basecamp/homebrew-tap` fizzy.json | GoReleaser (stable tags only) | | AUR | `aur.archlinux.org/packages/fizzy-cli` | `publish-aur.sh` (stable tags only) | +| Skills | `basecamp/skills` skills/fizzy | `sync-skills.sh` (stable tags only) | | Go install | `go install github.com/basecamp/fizzy-cli/cmd/fizzy@latest` | Go module proxy | | curl installer | `scripts/install.sh` | Manual | @@ -93,6 +96,45 @@ rm fizzy-tmp goreleaser release --snapshot --clean ``` +## Skills sync + +Stable releases mirror `skills/` into [basecamp/skills](https://github.com/basecamp/skills), +which several CLIs share. `scripts/sync-skills.sh` is the script every publishing CLI +runs (the copy in the `basecamp/cli` seed is canonical; only the `CLI_NAME` default +differs here). It owns only this CLI's skills there: it records the names it published +in `.managed-skills.fizzy-cli` at the target root, refuses to publish a name another +CLI's `.managed-skills.*` already holds, and removes a `skills/` only when its own +manifest lists it, the release no longer ships it, and no other manifest claims it (a +claimed name is warned about and left alone). A target with no `.managed-skills.fizzy-cli` +yet is a first run: nothing is removed. 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). When another CLI +pushes to basecamp/skills between the clone and the push, the sync is applied again +from the remote's new tip (ownership checks included) and pushed once more. +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` pins the contract, racing +publisher included, by running the script as both CLIs against a local bare repository +— real clones, commits and pushes, no network; it runs as `make test-sync-skills` (part +of `make check`) and in CI. + +The copy drops `*.go` and dotfiles, so `skills/embed.go` stays here and only +`skills/fizzy/**` is published. Preview what a release would publish, offline: + +```bash +DRY_RUN=local RELEASE_TAG=v0.0.0 SOURCE_SHA=$(git rev-parse HEAD) scripts/sync-skills.sh +``` + +The job is `continue-on-error`, so a failed sync never fails a release that has +already shipped. If it 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 master 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. + ## AUR Setup 1. Generate ed25519 SSH keypair: `ssh-keygen -t ed25519 -f aur_key` diff --git a/scripts/sync-skills.sh b/scripts/sync-skills.sh new file mode 100755 index 0000000..1e6f184 --- /dev/null +++ b/scripts/sync-skills.sh @@ -0,0 +1,339 @@ +#!/usr/bin/env bash +# 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 +# + +set -euo pipefail + +CLI_NAME="${CLI_NAME:-fizzy}" +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:-}" + +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 +} + +# 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_names=() +for skill_md in "$SKILLS_SOURCE"/*/SKILL.md; do + [[ -f "$skill_md" ]] || continue + name=$(basename "$(dirname "$skill_md")") + plain_name "$name" || die "skill directory '$name' is not a plain name" + skill_names+=("$name") +done + +[[ ${#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, excluding *.go and dotfiles, preserving subdirectories --- + +copy_skills() { + 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 +} + +tmpdir=$(mktemp -d) +trap 'rm -rf "$tmpdir"' EXIT + +# --- DRY_RUN=local: what would be published --- + +if [[ "$DRY_RUN" == "local" ]]; then + preview="${tmpdir}/preview" + echo "DRY_RUN=local: copying skills into ${preview}" + copy_skills "${preview}/${SKILLS_SUBDIR}" + echo "" + echo "=== Skills copied ===" + 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 "$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 + +# --- 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" < "${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 -C "$target" add -A +} + +commit_sync() { + git -C "$target" commit -q -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 "" +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 0000000..5e997b5 --- /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