Publish the fizzy skill to basecamp/skills on release - #214
Conversation
basecamp/skills is becoming the skills repo for every 37signals product, and Fizzy was the one CLI not publishing there. Stable releases now mirror skills/ into it with the same sync-skills.sh basecamp-cli and hey-cli run, which owns only this CLI's skills via a per-source manifest (.managed-skills.fizzy-cli) and never touches a sibling's (basecamp/skills#5). The job is continue-on-error and files a skills-sync issue on failure; sync-skills.yml is the manual recovery. The sibling repos' BATS test is ported to plain bash, since this repo's toolchain has no BATS, and runs as make test-scripts (part of make check) and in CI.
Sensitive Change Detection (shadow mode)This PR modifies control-plane files:
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🟡 Changes recommended
Invalid DRY_RUN values can unexpectedly execute the production push path instead of failing safely.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Publishes Fizzy’s agent skill to basecamp/skills during stable releases, with safe ownership tracking and manual recovery.
Changes:
- Adds skill synchronization and release workflows.
- Adds shell-based ownership and safety tests to CI.
- Documents the publishing and recovery process.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
File summaries
| File | Description |
|---|---|
scripts/sync-skills.sh |
Implements synchronization and ownership manifests. |
e2e/sync_skills_test.sh |
Tests synchronization behavior. |
.github/workflows/release.yml |
Publishes skills after stable releases. |
.github/workflows/sync-skills.yml |
Adds manual recovery workflow. |
.github/workflows/test.yml |
Runs shell tests in CI. |
.github/workflows/sensitive-change-gate.yml |
Protects the publishing script. |
Makefile |
Adds script-test targets. |
RELEASING.md |
Documents publishing and recovery. |
AGENTS.md |
Notes the external skills mirror. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 2
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ed19d80c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The seed (basecamp/cli#78) went further than the hey-cli copy taken first: a rejected push re-applies the sync from the remote's new tip (matching git's "fetch first" as well as "non-fast-forward"), a name another source's manifest holds is refused before anything is copied, SYNC_SOURCE and DRY_RUN are validated, a dirty checkout is refused, and the token reaches git through a private config rather than the remote URL. basecamp-cli and hey-cli are converging on the same file, so every publisher runs one script; only the CLI_NAME default differs here. The seed's test-sync-skills.sh replaces the bash port of the BATS file: it covers the same ownership cases plus the racing publisher, the publish-side refusal, DRY_RUN=remote and the dirty-tree guard.
…ards Mirrors basecamp/cli@966966e (the seed is the source of truth; only the CLI_NAME default differs here), and picks up the two seed commits this copy was behind: the remote-URL assert that read every configured URL, and two comment rewordings — both now moot, since the assert is gone. SKILLS_TARGET let the script adopt an existing checkout of basecamp/skills. Every release path clones its own target, and each review round found another corner of "any checkout" to guard. The script now always clones into a temp directory from SKILLS_REPO_URL (default https://github.com/basecamp/skills.git, the token carried through the same insteadOf rewrite as before), applies, pushes and cleans up, so the only commit it can push is the one it made. The remote-URL, branch and clean-tree asserts are gone with the knob; the retry from the fetched tip, the per-source manifests, the collision guard, the tombstone and DRY_RUN validation are unchanged. The test points SKILLS_REPO_URL at a local bare repository and reads every result back from a clone of its own; the race is staged with a post-commit hook that pushes the sibling's commit between the script's clone and push.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed326de328
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| sync-skills: | ||
| name: Sync skills | ||
| needs: [release] | ||
| if: startsWith(github.ref, 'refs/tags/v') && !contains(github.ref_name, '-') |
There was a problem hiding this comment.
When an older stable release finishes after a newer one—or its workflow is rerun later—this condition still runs the sync for the older tag, and scripts/sync-skills.sh replaces the published skill wholesale. The release workflow permits different tags to run concurrently via the per-tag group at lines 8-10, while this job's sync-skills concurrency group only serializes jobs and does not enforce version order, so the older job can permanently roll basecamp/skills back after the newer job. Add the same latest-stable-release guard used by the manual workflow before publishing.
AGENTS.md reference: AGENTS.md:L16-L16
Useful? React with 👍 / 👎.
Why
basecamp/skills is being repositioned as the skills repo for all 37signals products (basecamp/skills#6), installable with one
npx skills add basecamp/skills. basecamp-cli and hey-cli already publish their skills there on every stable release; Fizzy is the one CLI that does not, soskills/fizzyonly ever reaches agents through the binary or this repo's tree.What
scripts/sync-skills.sh— byte-identical to the seed'sseed/scripts/sync-skills.shin Seed the skills sync with per-source manifests and the skills/ layout cli#78 at5aec9a1apart from theCLI_NAMEdefault (fizzy, soSYNC_SOURCEisfizzy-cli: botfizzy-cli[bot], commitSync skills from fizzy-cli <tag>, provenancebasecamp/fizzy-cli@<sha>fromGITHUB_REPOSITORY) and the seed's# TODOline. basecamp-cli#708 and hey-cli#434 are converging on the same file, so every publisher runs one script..managed-skills.fizzy-cliat the target root and only ever removes askills/<name>that its own manifest lists, the release no longer ships, and no other CLI's manifest claims. A target with no.managed-skills.fizzy-cliyet — which is what the first Fizzy release will find — removes nothing. The manifest is per-source because the shared one let each CLI's release delete the others' skills (The hey-cli bot deleted the Basecamp skills skills#5: hey-cli's sync deletedskills/basecampandskills/basecamp-doctor, basecamp-cli's deletedskills/heytwice). The legacy.managed-skillsis rewritten as a comment-only tombstone so a sibling still on the pre-fix script deletes nothing either.fetch firstornon-fast-forward) resets to the remote's new tip, re-applies the sync there with the ownership checks, and pushes again;SYNC_SOURCEandDRY_RUNare validated; a checkout with uncommitted changes is refused; and the token reaches git through a privateGIT_CONFIG_GLOBALinsteadOfrewrite instead of the remote URL, so it never appears in argv or the checkout's config.sync-skillsjob inrelease.yml— mirrors hey-cli's: stable tags only,needs: [release],continue-on-error: trueso a stale skills repo never fails a release that has already shipped, concurrency groupsync-skills,releaseenvironment, token minted per-run and scoped tobasecamp/skillswith contents write. On failure it files or comments on askills-sync-labeled issue here (label-keyed and fail-closed so a retitled issue or an API error cannot spawn duplicates; the label is created idempotently on first use since this repo does not have it yet).sync-skills.yml— hey-cli's manual recovery workflow:workflow_dispatchwith a stable tag and an optional dry run, refusing anything but the latest stable release so it cannot roll the distribution repo back, running the sync logic from the dispatching ref against the tag's skills tree.scripts/test-sync-skills.sh— the seed's test, unchanged. Plain bash (BATS is not in this repo's toolchain:make toolsand CI install none). It runs the script as two CLIs against one throwaway basecamp/skills checkout seeded with the state The hey-cli bot deleted the Basecamp skills skills#5 left: interleaved A, B, A, B asserting both sources' skills, both manifests and the tombstone after each step; the copy filter dropping*.go, dotfiles and dot-directories; a dropped skill removing only its own directory; a pre-fix sibling's rewrite of the legacy manifest deleting nothing; a name in two manifests surviving with a warning; publishing a sibling-owned name refused with nothing changed;DRY_RUN=remoteshowing the diff without committing; the offlineDRY_RUN=localpreview; a real push against a local bare origin that a sibling moved first (retry lands on top of the sibling's commit) and one where the sibling claims the name mid-race (retry refuses, origin untouched); and the dirty-tree, wrong-remote, wrong-pushurl and wrong-branch refusals. Wired asmake test-sync-skills(now part ofmake check) and as a "Test the skills sync" step intest.yml'stestjob, whichrelease.ymlalso calls — the same wiring as the seed's Makefile and test.yml.scripts/sync-skills.shadded to the sensitive-change gate's extra patterns alongsidepublish-aur.sh, since it pushes to another repo with a minted token.cli-release-bot, each scoped to its one repo), and a new "Skills sync" section covering ownership, the offline preview, the failure issue and the recovery workflow. AGENTS.md's tree comment notes the mirror.What gets published
DRY_RUN=localfrom this branch's root:skills/holds onlyfizzy/SKILL.mdandembed.go; the copy filter drops*.goand dotfiles, soSKILL.mdis the whole of it.For the maintainer to confirm: the app on basecamp/skills
The job mints its token from
vars.RELEASE_CLIENT_ID/secrets.RELEASE_APP_PRIVATE_KEYin this repo'sreleaseenvironment, exactly as the existing tap push does, but asks forrepositories: skills. That only works if the app behind those credentials is installed on basecamp/skills. What I could verify:RELEASE_CLIENT_IDis the same value (Iv23liAbERu6z8d7t5A8,cli-release-bot) in thereleaseenvironments of fizzy-cli, basecamp-cli and hey-cli, and basecamp/skills' history carries pushes from bothbasecamp-cli[bot]andhey-cli[bot], so that app is installed there with contents write. This repo'sRELEASE_APP_PRIVATE_KEYalready mints a working token forhomebrew-tapon every release, so the key is that app's. What I cannot exercise from here is the mint forrepositories: skillsfrom this repo's environment, so please confirm it on the first stable release. The job iscontinue-on-error, so if the mint fails the release still ships; theskills-syncissue and theSync skillsworkflow are the way back.Testing
diff scripts/sync-skills.sh <seed>/seed/scripts/sync-skills.shat5aec9a1: only the# TODOline and theCLI_NAMEdefault differscripts/test-sync-skills.sh: all assertions passed (locally and in CI'stestjob)shellcheck scripts/sync-skills.sh scripts/test-sync-skills.sh: cleanactionlintandzizmor .(whatmake lint-actionsand thelint-actionsjob run): no findingsmake checkpasses (fmt, vet, lint, tidy, race-test, test-sync-skills)