Migrate pipeline from CircleCI to GitHub Actions#274
Conversation
phelma
left a comment
There was a problem hiding this comment.
Code Review: #274 - Migrate pipeline from CircleCI to GitHub Actions
Verdict: COMMENT
This is a clean, plan-conformant cutover from CircleCI to GitHub Actions. The diff reproduces the Variant A family plan §4 exactly in both directions — every required change is present and nothing beyond the plan was altered. The security posture is strong (least-privilege token scoping, env-var indirection for untrusted input, pull_request rather than pull_request_target). The one finding raised by four separate lenses — queue: max under concurrency — is a concern against the plan's own authoritative YAML, not a deviation the implementer introduced, so it does not block this diff; it is surfaced so a human can revisit the plan fleet-wide.
Cross-Cutting Themes
queue: maxis not a valid GitHub Actionsconcurrencykey (flagged by: correctness, safety, standards) —concurrencysupports onlygroupandcancel-in-progress. Withcancel-in-progress: falseGitHub keeps only the newest pending run, so the "keep every queued run" guarantee the comment promises does not hold, and an unknown key may fail workflow validation. This is written verbatim in the family plan's §4.2 authoritative YAML, so it is a plan concern to escalate, not a defect of this PR — the implementer correctly reproduced the spec. Recommend the plan owner correct it fleet-wide (drop the key + fix the comment, or serialise via a supported mechanism).
Strengths
- ✅ Top-level
permissions: contents: readdefault-deny, with write scopes widened only on the jobs that need them. - ✅ Untrusted
github.event.head_commit.messagepassed via theHEAD_COMMIT_MESSAGEenv var and quoted in bash — avoids the${{ }}-into-run:injection sink. Same pattern for the dependabotPR_URL. - ✅ Uses
pull_request(notpull_request_target) so fork PRs run read-only without secrets. - ✅ Secret deletions correctly scoped:
rubygems/credentialsandci/encryption.passphraseretained; only decommissioned CircleCI SSH/API credentials removed. - ✅ Git-token resolution resolves once and fails fast with an actionable message.
- ✅ Requires and gemspec dev-dependencies remain strictly alphabetical after the rake_circle_ci/rake_ssh → rake_slack swap.
General Findings
- 🔵 Safety: Dependabot auto-merge parity relies on the implicit
GITHUB_TOKENno-retrigger behaviour rather than the old[skip ci]marker — a documented deliberate decision (D3); a clarifying comment would harden it against a future PAT swap. - 🔵 Standards:
actions/checkout@v4is a moving major tag; third-party actions are conventionally SHA-pinned. Minor, widely-optional hardening note. - 🔵 Code Quality: RubyGems credential setup is inline shell duplicated in the prerelease and release jobs (plan's exact YAML); could be extracted to a rake task mirroring
repository:set_ci_author. - 🔵 Security:
job.statusinterpolated intorun:blocks is safe (fixed GitHub-controlled enum); noted only so the pattern isn't later extended to user-controlled values.
Verification notes (false positive)
- The correctness lens raised that the git-crypt
encrypted_key_pathis never configured after the key moved to.github/. Verified false:rake_git_crypt0.4.0 defaultsencrypted_key_pathto.github/gpg.private.encandpassphrase_env_var_nametoENCRYPTION_PASSPHRASE(unlock_with_encrypted_gpg_key.rb:18-19). The §4.3 move is precisely to match the default, so no explicit config is required.
Review generated by /accelerator:review-pr
| # prereleases instead of cancelling them. | ||
| concurrency: | ||
| group: main | ||
| cancel-in-progress: false |
There was a problem hiding this comment.
🟡 Plan concern (does not block this diff) — queue: max is not a valid GitHub Actions concurrency option
The concurrency block only supports group and cancel-in-progress. queue: max is not a recognised key — the accompanying comment describes queue-retention semantics GitHub Actions does not provide. An unknown key may be rejected as an invalid workflow; if silently ignored, the intended "keep every queued run" guarantee does not hold — with cancel-in-progress: false GitHub keeps only the newest pending run, so a burst of merges can drop intermediate prereleases (silently skipped version bumps).
Important: this line is written verbatim in the family plan's §4.2 authoritative YAML, so it is faithfully reproduced here — a concern with the plan, not a defect the implementer introduced. It is flagged for a human to revisit fleet-wide and does not block merging this plan-conformant diff.
Suggested fix (at the plan level): drop queue: max and rely on cancel-in-progress: false; if strict "retain all queued runs" is a hard requirement, implement it via a supported serialisation mechanism (external lock / merge queue). Update the comment to match the real semantics.
There was a problem hiding this comment.
Pull request overview
Migrates this repository’s CI/CD pipeline from CircleCI to GitHub Actions, including decommissioning CircleCI-specific scripts/config and reworking Rake provisioning to manage GitHub Actions secrets/environments and Slack notifications.
Changes:
- Added GitHub Actions workflows for PR checks and main branch publish flow (prerelease + gated release).
- Replaced CircleCI tooling/provisioning (CircleCI config + scripts + deploy key tasks) with GitHub Actions equivalents in the Rakefile.
- Updated dependencies and docs to support the new pipeline (Slack notifications, encrypted GPG key location, lockfile refresh).
Reviewed changes
Copilot reviewed 19 out of 25 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/pr.yaml |
New PR workflow running check/test/build plus dependabot merge job. |
.github/workflows/main.yaml |
New main workflow running check/test then prerelease and environment-gated release with git-crypt/GPG unlock. |
.github/gpg.private.enc |
Adds encrypted CI GPG private key under .github/ for Actions use. |
Rakefile |
Drops CircleCI/deploy-key provisioning; adds GitHub secrets/environments provisioning, Slack notify tasks, CI author task, and library:build. |
ruby_terraform.gemspec |
Replaces rake_circle_ci/rake_ssh with rake_slack. |
Gemfile.lock |
Dependency resolution refresh; removes CircleCI-related gems and adds rake_slack (plus transitive updates). |
README.md |
Updates CI key management docs from CircleCI paths to GitHub Actions paths. |
.circleci/config.yml |
Removes CircleCI pipeline configuration. |
scripts/ci/steps/build.sh |
Removed CircleCI build step wrapper. |
scripts/ci/steps/test.sh |
Removed CircleCI test step wrapper. |
scripts/ci/steps/prerelease.sh |
Removed CircleCI prerelease step wrapper. |
scripts/ci/steps/release.sh |
Removed CircleCI release step wrapper. |
scripts/ci/steps/merge-pull-request.sh |
Removed CircleCI dependabot merge step wrapper. |
scripts/ci/common/install-slack-deps.sh |
Removed CircleCI runner provisioning script. |
scripts/ci/common/install-gpg-key.sh |
Removed CircleCI GPG install/decrypt script (replaced by Actions steps). |
scripts/ci/common/install-git-crypt.sh |
Removed CircleCI git-crypt install script (replaced by Actions steps). |
scripts/ci/common/install-asdf.sh |
Removed CircleCI asdf install script (replaced by asdf_install@v1 action). |
scripts/ci/common/install-asdf-dependencies.sh |
Removed CircleCI asdf dependency install script. |
scripts/ci/common/configure-rubygems.sh |
Removed CircleCI RubyGems credential configuration script (replaced by Actions steps). |
scripts/ci/common/configure-git.sh |
Removed CircleCI git author configuration script (replaced by repository:set_ci_author). |
scripts/ci/common/configure-asdf.sh |
Removed CircleCI asdf plugin configuration script. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Every run publishes to RubyGems; serialise so close-together merges cannot | ||
| # race on version numbers. queue: max keeps every queued run (the default | ||
| # keeps only the newest pending), so a slow release approval delays later | ||
| # prereleases instead of cancelling them. | ||
| concurrency: | ||
| group: main | ||
| cancel-in-progress: false | ||
| queue: max |
| - 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 }} |
phelma
left a comment
There was a problem hiding this comment.
Code Review: #274 - Migrate pipeline from CircleCI to GitHub Actions
Verdict: COMMENT
Clean, highly plan-conformant Variant A cutover: the CircleCI pipeline is faithfully re-expressed as GHA workflows, the decommission (scripts, deploy keys, stored PAT, CircleCI secrets) is complete, and the Rakefile/gemspec/lockfile changes match family-plan §4. The one genuine in-scope defect is a divergence from the plan's specified token-resolution code (a dropped rescue Errno::ENOENT). The most severe-looking lens findings target the plan's own documented deliberate decisions (notably queue: max) and are recorded as plan concerns, not blockers.
Cross-Cutting Theme
queue: maxconcurrency key (correctness + safety) — both lenses flaggedmain.yaml:17as an invalid GHA key. It is the plan's exact specified YAML (§4.2), documented with a GA citation dated 2026-05-07 (github.blog "concurrency groups now allow larger queues"). The feature post-dates the reviewers' knowledge cutoff, so "invalid key" reflects stale training data, not a defect. Recorded as a plan concern.
Strengths
- ✅ Faithful task-triggering parity: skip-ci-check +
needschaining correctly propagate[ci skip]. - ✅ Least-privilege tokens: top-level
contents: read, escalating only where needed. - ✅ Untrusted input handled via env vars (
HEAD_COMMIT_MESSAGE,PR_URL) — no shell/template injection. - ✅
releasegated behindenvironment: release(maintainers reviewer); git-crypt/RubyGems creds confined to prerelease/release. - ✅ Alphabetical ordering preserved; lines within the 80-char rubocop limit.
General Findings
- 🟡 Code Quality / plan-conformance (
Rakefile:153): the token resolution drops therescue Errno::ENOENT => ''that plan §4.4 step 3 specifies. A missingghbinary raises a rawErrno::ENOENTinstead of the friendly guard the comment promises. In-scope defect (major). - 🔵 Documentation / plan-conformance (
main.yaml:9-12): keepsqueue: maxbut drops the plan'sGA 2026-05-07justification comment — the load-bearing evidence for the unusual key. In-scope defect (minor). - 🔵 Security (plan concern): PR check/test/build jobs run PR-supplied
./gotasks and exposeSLACK_BOT_TOKEN(plan's exact §4.1 shape; fork-PR secret withholding mitigates). - 🔵 Safety (plan concern): dependabot merge no longer stamps
[skip ci]; no-release-on-merge relies on the GITHUB_TOKEN-does-not-trigger-workflows rule (D3's documented mechanism). - 🔵 Safety (plan concern): publish-before-push has no recovery path if the push fails — documented §1 parity hazard, deferred.
- 🔵 Standards (plan concern):
Rakefile:186trailing-comment spacing reproduces the plan verbatim.
Plan-conformance (§4, both directions)
All required changes present (workflows, gpg rename at 100% similarity, Rakefile/gemspec/lockfile edits, README retitle, all §4.7 deletions, dependabot.yml untouched); nothing beyond the plan changed; commit message matches §6. Only deviations are the two in-scope defects above.
Review generated by /accelerator:review-pr
| # deliberate-decisions list). Resolve once and fail fast: a missing or | ||
| # unauthenticated gh yields an empty string, which would otherwise surface | ||
| # later as an opaque Octokit 401. | ||
| github_token = ENV.fetch('GITHUB_TOKEN') { `gh auth token`.strip } |
There was a problem hiding this comment.
🟡 Major — dropped rescue Errno::ENOENT; diverges from plan §4.4 step 3
The comment states "a missing or unauthenticated gh yields an empty string", but `gh auth token` raises Errno::ENOENT when the gh binary is absent — it does not return "". Plan §4.4 step 3 specifies the do … rescue Errno::ENOENT ; '' end form precisely to collapse the missing-binary case into the friendly github_token.empty? guard; the brace-block form here drops it, so a host without gh aborts with a raw "No such file or directory - gh" instead of the intended message.
Restore the rescue, e.g. ENV.fetch('GITHUB_TOKEN') { (\gh auth token` rescue '').strip }`.
| concurrency: | ||
| group: main | ||
| cancel-in-progress: false | ||
| queue: max |
There was a problem hiding this comment.
🔵 Minor — queue: max is plan-conformant (GA 2026-05-07); keep its justification comment
queue: max is the plan's specified YAML (§4.2), enabled by the 2026-05-07 GHA change allowing larger concurrency queues — it is valid and does not fail validation (automated reviewers with pre-May-2026 cutoffs will incorrectly flag it as an unknown key). The diff kept the key but dropped the plan's # 'queue: max' — GA 2026-05-07: <changelog URL> comment. Please restore that citation so the unusual key carries its own justification for future readers.
| 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.
🔵 Minor (plan concern) — SLACK_BOT_TOKEN present in jobs that run PR code
The check/test/build jobs execute PR-supplied ./go tasks and also inject SLACK_BOT_TOKEN. This is the plan's exact §4.1 shape and mitigated by GitHub withholding secrets from fork PRs, so it does not block the cutover — raising it only so the plan can revisit isolating the notify step in a post-migration hardening pass.
phelma
left a comment
There was a problem hiding this comment.
Code Review: #274 — Migrate pipeline from CircleCI to GitHub Actions
Verdict: APPROVE-equivalent (posted as COMMENT — GitHub blocks self-approval)
This PR is a faithful, near-verbatim application of the Variant A gem family plan: both workflow files match the plan's authoritative YAML, the Rakefile provisioning/Slack/token changes match §4.4 exactly, the GPG key move is byte-identical, the CircleCI pipeline and its secrets are cleanly decommissioned, and the lockfile carries the expected rake_slack / rake_git_crypt >= 0.4 / rake_github >= 0.17 resolution. Across five lenses (correctness, security, safety, standards, code-quality) no in-scope defect — no plan deviation, no genuine defect in unspecified territory — was found. Every substantive observation challenges a decision the plan or PR description documents as deliberate, so all are recorded below as non-blocking plan concerns for a human to revisit fleet-wide.
Plan-conformance (independent check, plan §4)
Verified both directions — all required §4 changes present, nothing beyond the plan changed:
- ✅
pr.yaml/main.yamlmatch §4.1/§4.2 verbatim (permissions,concurrencywithqueue: max,continue-on-errornotifies, plain--merge,releaserunsdocumentation:update, nogit pullon prerelease). - ✅ GPG key moved
.circleci/ → .github/gpg.private.enc, byte-identical (rename + matching shasums). - ✅ Rakefile: requires re-ordered,
rake_circle_ci/rake_ssh+ deploy-key machinery removed, ambient-auth token block,t.secrets/t.environments, Slack routing (builds-not-team-dev comment),set_ci_author,library:build, trimmedpipeline:prepare. - ✅ Gemspec dev-deps swapped (alphabetical); lockfile drops
rake_circle_ci/rake_ssh;PLATFORMSunchanged from prior commit (no new darwin platform;ruby+x86_64-linuxretained). - ✅ README CI-neutral wording +
.github/paths, no badge invented. - ✅ §4.7 deletions all present;
git grep -i circlecioutside docs/lockfile is clean.
Cross-Cutting Theme — Dependabot auto-merge
Two lenses probed the merge-pull-request job. One concern is real-but-out-of-scope: the org-level Dependabot/Actions write settings must be enabled for gh pr merge to succeed under a Dependabot run's read-only default token (akin to the deferred issue-19 provisioning). The other — that dropping [skip ci] would publish a prerelease on every merge — is a false positive: per D3, a merge performed with GITHUB_TOKEN triggers no workflow, so main.yaml never runs. Parity with the old [skip ci] merge is achieved by a different mechanism.
Strengths
- ✅ Standards: exemplary conformance — alphabetised requires/deps,
.yamlnaming and job names match sibling + plan verbatim. - ✅ Security: untrusted
head_commit.messagerouted via env var (correct injection mitigation); least-privilege token scoping; release gated behind reviewed environment;chmod 0600credential; safepull_requesttrigger. - ✅ Correctness: CI-loop guard sound end-to-end;
!= 'true'fails open (safe). - ✅ Code quality: fail-fast
github_tokenresolution; purposeful "why" comments.
General Findings (all non-blocking plan concerns)
- 🔵 Correctness:
queue: maxis a recently-GA key; a live greenmainrun confirms the runner accepts it. Documented (with changelog link). - 🔵 Correctness:
skip-ci-checkinspects only the head commit — matches prescribed shape and the intended release flow. - 🔵 Security: actions pinned to mutable tags (
asdf_install@v1,checkout@v4) not SHAs; tracking the org action's major tag is a documented §1 decision. - 🔵 Security: RubyGems credential not removed after publish (low risk on ephemeral runners; plan's exact YAML has no cleanup).
- 🔵 Security: prerelease publishes on every
mainpush, ungated — documented deliberate parity. - 🔵 Safety: no
timeout-minuteson check/test/build — matches prescribed shape. - 🔵 Safety: a long-pending
releaseapproval holds the serialisedmaingroup — documented intent ofqueue: max. - 🔵 Code Quality: Slack channel IDs are repeated opaque literals (prescribed block).
- 🔵 Code Quality: token fallback rescues only
Errno::ENOENT(prescribed verbatim).
Review generated by /accelerator:review-pr — full artifact at meta/reviews/prs/274-review-3.md
| pull-requests: write | ||
| steps: | ||
| - name: Merge pull request | ||
| run: gh pr merge --merge "$PR_URL" |
There was a problem hiding this comment.
🔵 Correctness/Safety — plan concern (non-blocking)
For Dependabot-triggered pull_request runs, GITHUB_TOKEN is read-only by default and secrets.* resolve from the separate Dependabot store, regardless of the job-level permissions: write. Unless "Allow GitHub Actions to create and approve pull requests" and Dependabot write access are enabled org/repo-wide, gh pr merge --merge will fail authorization — the same class of host-side precondition as the deferred issue-19 Dependabot secret store. This is the plan's prescribed D3 shape, so it does not block the diff; verify on a real Dependabot PR before the CircleCI project is disabled.
False positive, for the record: a lens also flagged that this merge drops the old [skip ci] marker and would publish a prerelease on every merge. It will not — per D3, a merge performed with GITHUB_TOKEN triggers no workflow, so main.yaml never runs.
| { when: { type: 'on_hold' }, | ||
| channel: 'C038EDCRSQJ', format: :on_hold }, # release | ||
| { when: { actor: 'dependabot[bot]', outcome: 'success' }, | ||
| channel: 'C03N711HVDG', format: :success }, # builds-dependabot |
There was a problem hiding this comment.
🔵 Code Quality — plan concern (non-blocking)
The routing rules embed raw channel IDs as literals, several repeated (C03N711HVDG twice, C023XUE76GH twice), with the trailing comment as the only human-readable meaning. Named constants would make a mismatched ID harder to introduce. This is the plan's prescribed §4.4 block verbatim, so it's a fleet-wide plan revisit, not a change for this diff.
| # deliberate-decisions list). Resolve once and fail fast: a missing, | ||
| # unauthenticated, or absent gh yields an empty string, which would | ||
| # otherwise surface later as an opaque Octokit 401. | ||
| github_token = ENV.fetch('GITHUB_TOKEN') do |
There was a problem hiding this comment.
🔵 Code Quality — plan concern (non-blocking)
The fallback rescues only Errno::ENOENT; a gh present-but-not-authenticated (or transient API failure) yields an empty string and is reported as the generic "run gh auth login" error, which may misdirect in the rare non-ENOENT case. Prescribed verbatim by plan §4.4 step 3; noted for the plan, not blocking.
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.