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".
Mirrors the seed (basecamp/cli#78 at 1df3bfe5). GIT_CONFIG_GLOBAL stands in for the user's global file only. The system config and any GIT_CONFIG_COUNT settings in the environment still reach every git call the script makes — the race test injects its hooks path through exactly that — so the comment no longer claims the ambient environment is kept out.
…release The sync-skills job's concurrency group serialises the syncs but does not order them by version. An older stable release's job can reach the sync after a newer one has published — its run stalled, or its failed sync was rerun later, which is what the failure issue tells the operator to do — and the script mirrors the skills wholesale, so that run would roll basecamp/skills back to the older skill. The job now makes the check the manual Sync skills workflow already makes: the tag must be the repository's latest stable release, or the sync is skipped with a warning. A failed lookup fails the job, so the failure issue is filed rather than the sync skipped in silence.
There was a problem hiding this comment.
🟡 Changes recommended
The repository-specific fizzy-cli identity default is not exercised by the integration suite.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 1
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
|
Review threads: 1 resolved (1 fixed, 0 declined). |
Every case set SYNC_SOURCE to play hey-cli or basecamp-cli, so the one line this copy of the script changes — the CLI_NAME default — never ran. A last case runs the script with neither SYNC_SOURCE nor CLI_NAME set and checks the manifest it writes, the bot it commits as and the summary line against the source the Makefile expects, fizzy-cli. A copy left at the seed's placeholder, or naming another CLI, fails here instead of publishing under that source's manifest and identity. Mirrors seed/scripts/test-sync-skills.sh from basecamp/cli@19b52ed.
There was a problem hiding this comment.
🟢 Approved
The sync behavior is defensively implemented, comprehensively tested, and consistently integrated into release and recovery workflows.
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 0 new
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a917e941c9
ℹ️ 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".
… rewrite Mirrors the seed (basecamp/cli#78 at 0649a4d); scripts/sync-skills.sh stays byte-identical apart from the CLI_NAME default, scripts/test-sync-skills.sh byte-identical. The insteadOf rewrite kept the token out of the outer git argv and the remote URL, but git expands the rewrite before invoking git-remote-https, so that helper still received https://x-access-token:<token>@github.com/... as an argument. The private gitconfig now carries http.https://github.com/.extraheader with the basic-auth form actions/checkout writes: the helper sees the plain URL, and the token lives only in the mode-600 file until the tmpdir goes. The test unsets SKILLS_TOKEN up front: its token-required case points the script at the real basecamp/skills, and a token inherited from the caller's environment would have let it clone and publish the fixtures there. The header now says exactly what the tombstone shields — the sources that have upgraded — rather than "anyone's skills": a pre-fix sibling still rewrites .managed-skills with its own names and a second pre-fix sibling still deletes those, the basecamp/skills#5 clobber confined to the CLIs yet to upgrade.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 14d039fbee
ℹ️ 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".
|
Review threads: 2 resolved (2 fixed, 0 declined). 14d039f mirrors the seed (basecamp/cli#78 at 0649a4d): |
|
Review threads: 1 more resolved (0 fixed, 1 declined with the reasoning in the thread) — 9 of 9 resolved. Declined:
Bots on 14d039f: Codex reviewed it (the thread above). Copilot last approved a917e94 and has not reported on 14d039f after 35 minutes, though it reviewed each earlier push within minutes; a re-review cannot be requested by API, so a human can request it from the Reviewers panel if wanted. The diff from a917e94 is the seed mirror plus one AGENTS.md line — the same script bytes Copilot approved on basecamp-cli#708 at 4ec7f53. |
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 at1df3bfe5apart 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; and the token reaches git as a github.com-scopedhttp.extraheaderin a private mode-600GIT_CONFIG_GLOBALinstead of the remote URL or aninsteadOfrewrite (which git expands into git-remote-https's argv), so it never appears in argv or the clone's config. The script always clones the target fresh and pushes only the commit it made, so there is no checkout to hand it.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, and the check the manual workflow makes — the tag must be the repository's latest stable release, or the sync is skipped — so an older release whose run stalls, or whose failed sync is rerun after a newer one has shipped, cannot roll basecamp/skills back (the concurrency group serialises the syncs but does not order them), 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 a local bare basecamp/skills repository seeded with the state The hey-cli bot deleted the Basecamp skills skills#5 left — real clones, commits and pushes, no network: 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); the sibling dropping that claim (the next release publishes); and a github.com target without a token refused before anything is cloned. 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.shat1df3bfe5: only the# TODOline and theCLI_NAMEdefault differactionlint,zizmorandshellcheckover the edited workflow and script: cleanscripts/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)