Migrate pipeline from CircleCI to GitHub Actions#118
Conversation
phelma
left a comment
phelma
left a comment
There was a problem hiding this comment.
Code Review: #118 - Migrate pipeline from CircleCI to GitHub Actions
Verdict: COMMENT
This PR is a faithful, plan-conformant implementation of the Variant A gem migration plan (§4). Independent conformance checking (both directions) found every required change present and correctly parameterised, and nothing changed beyond the plan. All five lenses (correctness, security, safety, standards, code-quality) ran clean of in-scope defects: every substantive finding either targets something the plan or PR description documents as a deliberate decision (the plan's exact YAML, D3, the §1 parity list) — recorded below as plan concerns for a human to revisit the plan, not as defects of the diff — or is a false positive disproven by the D3 token mechanism. Verdict is COMMENT (not APPROVE) solely to draw a human's eye to the queue: max concern before merge; it does not request changes to this diff.
Plan-conformance check (independent of lenses)
- §4.1/§4.2 workflows match the plan's authoritative YAML. The
releasejob correctly parameterises toversion:bump[minor]and omitsdocumentation:update, mirroring this repo's oldrelease.shexactly as §4.2 requires. - §4.3 GPG key
git mvis byte-identical; the repo's own unlock-task default is already.github/gpg.private.enc, so the moved key resolves (verified). - §4.4 Rakefile: all edits present. §4.5: dev deps live in the Gemfile here (gemspec is runtime-only), so the Gemfile edit is the correct locus; lock drops
rake_circle_ci/rake_ssh, addsrake_slack 0.3.0/rake_github 0.17.0. - §4.6 README updated, no badge (correct). §4.7 decommission complete; git-crypt secrets retained.
No plan violations found.
Cross-cutting theme (plan concern)
queue: maxis not a valid GitHub Actionsconcurrencykey (correctness, safety, standards, code-quality). Nativeconcurrencysupports onlygroup+cancel-in-progress; the "keeps every queued run" semantics are not achievable. This is verbatim from the plan's §4.2 YAML (plan lines 292–295) → plan concern, not a defect of this diff, but the highest-value item to fix in the plan fleet-wide.
False positive (counted nowhere)
- "Dependabot merge lacks
[skip ci]→ unintended prerelease" (correctness, major) — disproven. The merge runs withGH_TOKEN: secrets.GITHUB_TOKEN, and GITHUB_TOKEN-authored pushes do not trigger newpushruns (D3), somain.yamlnever starts. No skip-ci marker needed.
Strengths
- ✅ Faithful, complete plan implementation with correct per-repo parameterisation.
- ✅
github_tokenresolution fails fast with an actionable message. - ✅ Least-privilege token model (top-level
contents: read, per-job elevation). - ✅ Skip-ci check avoids the
${{ github.event... }}shell-injection sink (env var + quoted[[ ]]). - ✅ git-crypt unlock /
ENCRYPTION_PASSPHRASEconfined tomain.yaml, never exposed to PR runs. - ✅ Slack notify steps are
continue-on-error+!cancelled().
General findings (all plan concerns — do not block this diff)
- 🟡 Security/Safety: Dependabot auto-merge accepts any update type (D3 / §1 parity — no semver filter by design).
- 🔵 Security/Safety:
releasepublishes post-approvalmainHEAD, not the tested SHA (§1 parity with oldrelease.sh). - 🟡 Safety: RubyGems publish-before-push inside untouched
./go release(D5 / §1 parity). - 🔵 Security:
SLACK_BOT_TOKENin scope for PR jobs running PR-supplied./gocode (inherent to the plan's flat notify-per-job YAML). - 🔵 Code Quality: RubyGems credential setup inlined/duplicated across prerelease+release (verbatim plan §4.2 YAML; extracting would deviate).
- 🔵 Safety: No post-publish smoke test (beyond the plan's specified shape).
- 🔵 Standards:
.yamlvs retired.yml— plan-mandated filenames; conformant.
Review generated by /accelerator:review-pr
| concurrency: | ||
| group: main | ||
| cancel-in-progress: false | ||
| queue: max |
There was a problem hiding this comment.
🟡 Plan concern — not a defect of this diff.
The concurrency mapping only accepts group and cancel-in-progress; queue is not part of the GitHub Actions schema, and the "keeps every queued run" guarantee the comment describes is not achievable via native concurrency (at most one running + one pending per group). Depending on parser strictness the key is either silently ignored (serialisation via group still works; lossless queuing does not) or rejected.
This YAML is reproduced verbatim from the family plan's authoritative §4.2 block (plan lines 292–295), so the diff is plan-conformant and this is not an implementation defect. Flagging it as the highest-value item for a human to correct in the plan before fleet-wide rollout: drop queue: max and reword the comment, or implement lossless queuing outside native concurrency.
| continue-on-error: true | ||
| run: ./go "slack:notify[${{ job.status }}]" | ||
| env: | ||
| SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }} |
There was a problem hiding this comment.
🔵 Plan concern — not a defect of this diff.
The check/test/build PR jobs execute ./go/Rakefile code from the PR head while SLACK_BOT_TOKEN is injected into the notify step. Same-repo and dependabot branches receive secrets, so a crafted PR that edits the slack:notify task could read the token (fork PRs are protected). This is inherent to the plan's mandated flat notify-per-job structure; changing it would deviate from the plan. Recorded for the plan owners (e.g. run PR notifications in a workflow_run job that does not execute PR code).
| pull-requests: write | ||
| steps: | ||
| - name: Merge pull request | ||
| run: gh pr merge --merge "$PR_URL" |
There was a problem hiding this comment.
🟡 Plan concern — not a defect of this diff.
The merge-pull-request job auto-merges any dependabot[bot] PR once checks pass, with no semver-type filter. This is documented deliberate in D3 and the §1 parity list ("No update-type filter, also parity; post-migration hardening may add one"). Recorded for the plan owners; consider gating on dependabot/fetch-metadata update-type in post-migration hardening.
False-positive note (counted nowhere): a correctness finding claimed this merge triggers an unintended prerelease because the merge commit lacks [skip ci]. That is disproven — the merge runs with GH_TOKEN: secrets.GITHUB_TOKEN, and GITHUB_TOKEN-authored pushes do not trigger new push workflow runs (D3), so main.yaml never starts. No skip-ci marker is needed.
There was a problem hiding this comment.
Pull request overview
Migrates the repository’s CI/CD from CircleCI to GitHub Actions, including removal of CircleCI-specific scripts/config and updating the build/provisioning tooling to use GitHub Actions-compatible tasks and secrets.
Changes:
- Added GitHub Actions workflows for PR validation and
mainpublishing (prerelease + gated release). - Removed CircleCI pipeline configuration and associated
scripts/ci/**helper scripts. - Updated Ruby tooling and provisioning: switched from CircleCI-era tasks to
rake_github+ Slack notifications, and refreshed gem dependencies.
Reviewed changes
Copilot reviewed 19 out of 25 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/ci/steps/test.sh | Removed CircleCI test step wrapper (now handled by GitHub Actions). |
| scripts/ci/steps/release.sh | Removed CircleCI release step wrapper (now handled by GitHub Actions). |
| scripts/ci/steps/prerelease.sh | Removed CircleCI prerelease step wrapper (now handled by GitHub Actions). |
| scripts/ci/steps/merge-pull-request.sh | Removed CircleCI dependabot merge helper (replaced with gh pr merge job). |
| scripts/ci/steps/build.sh | Removed CircleCI build step wrapper (now handled by GitHub Actions). |
| scripts/ci/common/install-slack-deps.sh | Removed CircleCI apt-based Slack deps install (not used in Actions). |
| scripts/ci/common/install-gpg-key.sh | Removed CircleCI GPG key install/decrypt script (replaced by rake task + .github/ encrypted key). |
| scripts/ci/common/install-git-crypt.sh | Removed CircleCI git-crypt install script (Actions installs via apt). |
| scripts/ci/common/install-asdf.sh | Removed CircleCI asdf installer script (replaced by asdf_install@v1 action). |
| scripts/ci/common/install-asdf-dependencies.sh | Removed CircleCI asdf install script (handled by asdf_install@v1). |
| scripts/ci/common/configure-rubygems.sh | Removed CircleCI RubyGems credential setup script (Actions configures directly). |
| scripts/ci/common/configure-git.sh | Removed CircleCI git author config script (replaced by repository:set_ci_author). |
| scripts/ci/common/configure-asdf.sh | Removed CircleCI asdf plugin config script (handled by asdf_install@v1). |
| README.md | Updated documentation to reflect CI key management in GitHub Actions (.github/gpg.private.enc). |
| Rakefile | Dropped CircleCI/SSH provisioning, added GitHub secrets/environments provisioning, Slack notifications, and library:build. |
| Gemfile.lock | Updated dependency set (removed CircleCI/SSH gems, added Slack support, upgraded other dependencies). |
| Gemfile | Removed rake_circle_ci/rake_ssh, added rake_slack. |
| .github/workflows/pr.yaml | Added PR workflow (check/test/build + dependabot auto-merge). |
| .github/workflows/main.yaml | Added main-branch workflow (check/test + prerelease + environment-gated release). |
| .github/gpg.private.enc | Added encrypted GPG private key artifact for CI git-crypt unlock flow. |
| .circleci/config.yml | Removed CircleCI configuration (migration/decommission). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Merge pull request | ||
| run: gh pr merge --merge "$PR_URL" | ||
| env: | ||
| PR_URL: ${{ github.event.pull_request.html_url }} | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} |
| - name: Notify Slack | ||
| if: ${{ !cancelled() }} | ||
| continue-on-error: true | ||
| run: ./go "slack:notify[${{ job.status }}]" | ||
| env: | ||
| SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }} |
| - name: Notify Slack | ||
| if: ${{ !cancelled() }} | ||
| continue-on-error: true | ||
| run: ./go "slack:notify[${{ job.status }}]" | ||
| env: | ||
| SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }} |
| - name: Notify Slack | ||
| if: ${{ !cancelled() }} | ||
| continue-on-error: true | ||
| run: ./go "slack:notify[${{ job.status }}]" | ||
| env: | ||
| SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }} |
Cutover to GitHub Actions per the Variant A family plan (gem pilot).
Includes decommission — merging this PR completes the repo's migration.
releaseenvironment gate.github/rake_slack; dependabot auto-merge jobrake_githubsecrets/environments;rake_circle_cidropped.circleci/,scripts/ci/, the CI SSH deploykey pair and its
keys:deploy/deploy_keysprovisioning, and the storedCircleCI/GitHub API credentials (
config/secrets/{circle_ci,github}/)Deliberate decisions (not defects)
This cutover reproduces the CircleCI pipeline's behaviour, warts included;
fixing inherited hazards is post-migration work. In particular:
./go releasepublishes to RubyGems before the version-bump commit ispushed — pre-existing ordering inside the untouched release logic.
mainwith no approval gate; onlyfull releases are gated (
environment: release).merge does not trigger a release build — on CircleCI the merge commit
carried
[skip ci], so this matches. Updates ship with the nexthuman-triggered release.
releasejob pullsmainat approval time, so a delayed approvalpublishes main as it stands then, not the SHA this run tested — parity with
the old
release.sh(which also pulled;prerelease.shdid not, so theprerelease job has no pull).
asdf_install@v1is our own action (infrablocks/github-actions); we arehappy tracking its major version tag.
build system (
./go/rake) and CI stays lean — it just triggers tasks andsupplies secrets/context.
Gemfile.lockcarries transitive major bumps — the unavoidable resolutionof the targeted
bundle lock --update, not scope creep.autocorrects existing code (e.g.
Style/ArgumentsForwarding) — requiredby the
library:checkverification gate, not drive-by refactoring.pipeline:prepare) authenticates with the operator's ambientghlogin (GITHUB_TOKENfallback) instead of a stored PAT — a deliberateparity deviation; the stored token in
config/secrets/github/config.yamlis deleted with the rest of the CircleCI-era credentials.
Do not merge manually — the pipeline merges once checks are green.
Disabling the CircleCI project and deleting the
CircleCIdeploy key aredeferred to the end-of-migration sweep.