Migrate pipeline from CircleCI to GitHub Actions#231
Conversation
Bump nokogiri 1.18.8 -> 1.19.4: 1.18.8 was yanked from RubyGems, so the locked version was no longer installable.
phelma
left a comment
phelma
left a comment
There was a problem hiding this comment.
Code Review: #231 - Migrate pipeline from CircleCI to GitHub Actions
Verdict: COMMENT
Six lenses (correctness, security, safety, standards, code quality, portability)
found no in-scope defects. The diff is a byte-faithful application of the
Variant B family plan §4 in both directions: every mandated change is present,
and nothing beyond the plan was touched. I independently re-ran the plan's §5
self-verification and all eight checks pass. Three lens findings — including two
rated critical — were disproven by direct verification and are recorded as
false positives below. Everything else that survived is a plan concern: a
challenge to something the plan or PR description documents as deliberate. Those
are worth a human's eye on the plan before this shape fans out to ~103 repos,
but none of them should block a plan-conformant diff.
Verification performed
| Check | Result |
|---|---|
§5.1 ./go library:check (rubocop) |
✅ 7 files, no offenses |
§5.3 bundle exec rake -T |
✅ all required tasks present; no circle_ci:*, no keys:deploy:*; pipeline:prepare does not depend on github:deploy_keys:ensure |
| §4.5 lockfile stability | ✅ bundle lock is a no-op under Ruby 3.2.11 |
| §5.5 both workflows parse | ✅ |
| §5.6 tree clean, no secret material | ✅ |
| §5.7 GPG key move byte-identical | ✅ R100 .circleci/gpg.private.enc → .github/gpg.private.enc |
| §5.8 decommission complete | ✅ git grep -i circleci matches nothing; config/secrets/ci/ holds only encryption.passphrase, gpg.private, gpg.public; circle_ci/ and github/ gone; dockerhub/ untouched |
| Rakefile loads | ✅ dynamic resolves (the rake_factory/kernel_extensions require is correct) and the provisioning raises do not fire at load |
False positives (disproven — counted nowhere)
- 🔴→❌ portability, ×2 (rated critical): "the docker-compose pin
v5.3.1
references a release tag that does not exist; docker/compose only publishes
v2.x, socurl -fsSL404s and everytestjob fails." Disproven.v5.3.1
is the current docker/compose release (releases/latest→v5.3.1); the
URL returns HTTP 200, and the pinned sha256
f9ebc6ebdb19d769b793c245a736caaeb198c62587f13b25c660c13b4987f959matches the
release's own.sha256asset byte-for-byte. The lens reasoned from a stale
assumption about the project's versioning. The pin is correct. - 🟡→❌ portability (rated major): "
PLATFORMSlistsx86_64-linuxwhile
the specs resolve-x86_64-linux-gnuvariants, so Bundler may re-resolve on
the runner and dirty the lock (issue 24)." Disproven.Gem::Platform.new('x86_64-linux-gnu') =~ Gem::Platform.new('x86_64-linux')returns
truthy for bothnokogiriandffi— Bundler normalises these as compatible,
and this is exactly the lockfile shape modern Bundler emits.bundle lock
under Ruby 3.2.11 is a verified no-op. The correctness lens independently
reached the same conclusion.
Cross-Cutting Themes
- The release approval is time-based, not content-based (flagged by:
correctness, security, safety) —releasechecks outref: mainthen
git pulls, so it publishesmainas of approval time, not the SHA
prereleasetested. All three lenses independently landed here. The plan and
PR description document it as parity with the oldrelease.sh, so it is a plan
concern — but it is the most security-meaningful of the documented hazards,
because it makes thereleaseenvironment gate an ordering control rather than
a review control. It also interacts with the documented dependabot auto-merge
hazard: an unreviewed base-image bump can reach a published release with no
human having inspected it. Worth revisiting at the fleet-wide hardening pass
(ref: ${{ github.sha }}, drop the pull). queue: maxmakes workflow-file validity a fleet-wide single point of
failure (flagged by: correctness, safety) — GHA rejects an entire workflow
file on an unrecognised key rather than degrading the one job. Ifqueueis
unavailable to this org/plan,main.yamlsilently stops running everything
(check, test, prerelease, release) whilepr.yamlstays green — so the failure
looks like "releases just stopped happening". Plan-mandated and cited to a
2026-05-07 GA changelog, so not a defect; but confirming a real push tomain
produces the expected job graph is a cheap gate before fanning out to 103 repos.- The compose guard is boilerplate this repo cannot use (flagged by:
standards, code-quality, portability) — nospec/dependencies.yml, no
dependencies:test:*;test:integrationdepends only onimage:build. Since
ubuntu-latestships only thedocker composeplugin,command -v docker-compose
fails and the step really doessudo-install a binary this repo never invokes,
on every run. Plan §4 accepts this explicitly ("one family shape, fewer
variants"), so it is a plan concern, not drift.
Tradeoff Analysis
- Family uniformity vs least-privilege: the plan mandates the compose guard
in everytestjob for one family shape; security/standards/portability all
note it means asudoinstall of a third-party binary on runners that later
hold secrets, in repos that never use it. The plan's own pinning + checksum
comment is the mitigation, and I verified both are correct. Recommendation:
keep the uniform shape; the pin is sound. - Parity vs safety on the release gate: the parity principle says reproduce
the CircleCI behaviour warts included; security says the approval should bind
to the tested SHA. Recommendation: hold parity for cutover (as the plan
directs), and carry the SHA-pinning change as a fleet-wide hardening item —
it is a two-line change with a real benefit. - Plan fidelity vs code quality: code-quality would extract a
git_crypt_locked?(path)helper (the"\x00GITCRYPT"magic and the re-read
appear twice, in two namespaces) and hoist the token resolution out of the
config block. Both are reasonable, and both target snippets the plan mandates
verbatim. Recommendation: if taken, they belong in the plan and land
fleet-wide — not as a one-repo deviation.
Strengths
- ✅ Byte-faithful to the plan's §4.1/§4.2 templates, mandated comments included
— no drift, no improvisation. Decommission is complete and bounded to §4.7. - ✅ Uses
pull_request, notpull_request_target— fork PRs get no secrets and
a read-only token, so the pwn-request class is structurally absent. The single
most important decision in the diff, and it is correct. - ✅ Script injection correctly avoided:
head_commit.messageandPR_URLboth
routed throughenv:and quoted, rather than interpolated into the shell body. - ✅ Least-privilege permissions: workflow-level
contents: read, elevated only
on the three jobs that need it. - ✅ The
dynamiccredentials guard is verified correct end-to-end (rake_factory
DynamicValue#evaluateresolves at task-run time;RakeDocker::Tasks::Publish
no-ops the write via itsrespond_to?guard), and the magic-header check is
encoding-safe —File.binreadreturns ASCII-8BIT but"\x00GITCRYPT"is
7-bit, sostart_with?compares byte-wise without anEncoding::CompatibilityError.
A genuinely easy thing to get wrong. - ✅ Detecting the git-crypt header explicitly rather than rescuing
Psych::SyntaxErroris the right call, and the comment states exactly why. - ✅ Both provisioning guards fail fast with actionable messages, converting an
opaque Octokit 401 and a much-later GPG unlock failure into local, named causes. - ✅ Skip-propagation is correct: no
always()leaks anywhere in the graph, so a
[skip ci]push cascades cleanly through toreleasewith no publish. The
guard also fails safe (unparseable message → CI runs). - ✅ The gate/slot concurrency split genuinely avoids the starvation deadlock a
naive workflow-level group would cause, and every job carries atimeout-minutes. - ✅ The
.tool-versions3.2.11 bump correctly pre-empts the issue-24 hazard the
new lock (activesupport 8.1.3) would otherwise have created. - ✅ Slack notify steps are uniformly
continue-on-error: true+if: ${{ !cancelled() }}
— observability can never become the reason a build fails. - ✅ Commit is a single commit with the exact §6 message, no tool attribution.
General Findings
All remaining findings are plan concerns — challenges to documented
deliberate decisions, recorded for a human to weigh against the plan rather than
against this diff.
- 🔵 Security:
actions/checkout@v4is a mutable tag in the jobs holding
ENCRYPTION_PASSPHRASEandcontents: write. D6 exemptsasdf_install@v1
explicitly but the plan is silent oncheckout. Low likelihood; if the org
wants SHA pinning it is a fleet-wide plan change, best landed with the
github-actionsDependabot ecosystem enabled. - 🔵 Security: the git-crypt unlock decrypts the whole secret tree
(includingci/gpg.privateand the passphrase) in jobs that then run
third-party gem code, when only the Docker Hub credentials are needed. Inherent
to D1; good input to the deferred OIDC/native-secrets follow-up. Verified not
a build-context leak:RakeDocker'scopy_speccopies only the Dockerfile. - 🔵 Security: CircleCI API token and stored PAT are removed from the tree but
remain live at their issuers and recoverable from public history. Correctly
scoped out (issue 20) — worth ensuring the sweep treats revocation as blocking,
since "deleted from the tree" reads as "handled" to anyone skimming later. - 🔵 Safety: this PR orphans a live, write-capable
CircleCIdeploy key that
nothing in the tree manages any more. Across 103 repos a missed sweep item is a
silently-forgotten write credential, not a visible failure — a scripted
gh api repos/infrablocks/<repo>/keysassertion would make omissions detectable. - 🔵 Safety:
release-gatecarriestimeout-minutes: 10alongside
environment: release. Waiting-for-approval jobs allocate no runner and the
timer should bound execution only — but if it did count, every out-of-hours
release would auto-cancel after the rc had already published. Cheap to confirm
once on the pilot. - 🔵 Standards:
gem 'rake_factory'is unconstrained while the lock pins
0.34.0.pre.2, a prerelease — the Gemfile and lock express different intents.
Harmless today (the lock wins, and 0.33.x still clears the plan's>= 0.32
floor);gem 'rake_factory', '>= 0.32'would be the honest fleet-wide form. - 🔵 Portability: dropping
arm64-darwin-*(per the plan's no-operator-local-
platforms rule) means macOS maintainers now compile nokogiri and ffi from
source, andrake_github 0.17adds anrbnacl/libsodium prerequisite. Worth
documenting the local prerequisites in the README development section. - 🔵 Portability: the compose URL hardcodes
x86_64with no arch detection —
latent only (the image is pinnedlinux/amd64), but an arm64 runner pool would
install a binary that fails withexec format errorrather than at download. - 🔵 Portability: Slack channel IDs and the
maintainersteam slug are
hardcoded per-repo (plan §3 mandates routing by ID). Flagged so the coupling is
conscious; the channel map is the natural thing to lift intorake_slack
defaults if the fleet ever needs it. - 🔵 Code quality (checked non-finding): shell robustness in
run:steps is
adequate. No-euo pipefail, but GHA defaults tobash -e {0}and the only
pipeline exits onsha256sum's status, which-ecatches.
Review generated by /accelerator:review-pr
| with: | ||
| ref: main | ||
| fetch-depth: 0 | ||
| - name: Pull latest main |
There was a problem hiding this comment.
🔵 Correctness + Safety — plan concern, not a defect of this diff
Two independent lenses landed on this step, so recording it together.
It is redundant. The step's comment says its purpose is that "approval can land long after the run starts; release publishes main as of approval time" — but the preceding actions/checkout@v4 with ref: main already achieves exactly that. release does not start until release-gate completes (i.e. post-approval), so its checkout resolves main at approval time regardless.
It is ordered before the git author is set. Set CI git author runs three steps later. CircleCI had this inverted — configure-git.sh ran during configure_tools, before release.sh reached its git pull. GitHub runners carry no global git identity, so a pull needing a merge commit aborts with unable to auto-detect email address. Low risk in practice (checkout at ref: main means the pull can only fast-forward, and fast-forwards need no identity) — but the safety of this step rests on an invariant established four steps away.
Impact: a redundant step whose stated rationale doesn't hold, contributing latent failure risk to the one job that pushes tags and publishes images. If an identity ever did exist, a non-fast-forward pull would silently create a merge commit that version:release's repo.push('origin', 'main', tags: true) would then push to main unreviewed.
This is the plan's exact YAML (§4.2) and the PR description documents the pull as parity with the old release.sh — so nothing to change here. Worth raising against the plan: git pull --ff-only with Set CI git author hoisted above it would make the intended invariant enforced rather than assumed, at zero parity cost.
| t.credentials = YAML.load_file( | ||
| 'config/secrets/dockerhub/credentials.yaml' | ||
| ) | ||
| t.credentials = dynamic do |
There was a problem hiding this comment.
🔵 Correctness + Code Quality — plan concern, not a defect of this diff
To be clear up front: I verified this guard is correct. rake_factory's DynamicValue#evaluate resolves the block at task-run time, RakeDocker::Tasks::Publish no-ops the write via its respond_to? guard, and File.binread returns ASCII-8BIT while "\x00GITCRYPT" is 7-bit ASCII, so start_with? compares byte-wise with no Encoding::CompatibilityError. Detecting the header explicitly rather than rescuing Psych::SyntaxError is the right call, and the comment says why. Two observations on shape only:
Double read. rake_docker's build and push tasks each do Docker.authenticate!(t.credentials) if t.credentials, which evaluates the block twice per task — so in the unlocked case File.binread + YAML.load_file both run twice, and in principle the guard and the argument could observe different file states. A TOCTOU window the domain makes vanishingly unlikely on a runner, but the duplicated I/O is real.
Duplicated concept. The "\x00GITCRYPT" magic and the "is this tree locked?" idea appear again ~60 lines up in the RakeGithub block, against a different path. Neither the magic string nor the concept has a name to grep for; if detection ever needs to change (header variants, symmetric-key mode) there are two sites and no single source of truth.
Also: if the file is ever absent rather than locked, File.binread raises Errno::ENOENT from inside rake_factory's Parameter#get — an opaque backtrace, which is the opposite of what the surrounding fail-fast comments aim for.
This Rakefile already establishes bare helpers at the top (repo, latest_tag, tmpdir), so the idiom exists:
def git_crypt_locked?(path)
File.binread(path).start_with?("\x00GITCRYPT")
endBoth guards would then read declaratively. The plan mandates this snippet verbatim and applies it family-wide, so if this is worth doing it belongs in the plan and lands across all ~103 repos — not as a one-repo deviation.
| needs: [check, test] | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 30 | ||
| concurrency: |
There was a problem hiding this comment.
🔵 Safety — plan concern, not a defect of this diff
The main-tags group serialises the publish jobs, and the header comment describes this as making close-together merges "serialise FIFO on tag operations". Worth noting the serialisation is narrower than that reads.
version:bump ultimately does repo.push('origin', 'main', tags: true) — i.e. git push origin main --tags, which pushes the main branch as well as the tag. actions/checkout on a push event creates local main at the tested SHA (git checkout --force -B main <github.sha>), and unlike release, prerelease deliberately does not pull. So when two merges land close together, the older run's prerelease pushes a stale main and git rejects it as non-fast-forward. Because git push updates refspecs independently (no --atomic), the tag lands anyway while the command exits non-zero — so && ./go image:publish never runs.
Impact: an rc tag on origin with no image behind it, plus a red prerelease. Same orphan-tag hazard the PR already documents, but reached via a benign cause. Serialising the jobs cannot prevent it, because the staleness comes from the checkout SHA rather than from job overlap.
Nothing to change — the push logic is untouched per D5. Two suggestions for the plan: narrow the concurrency comment to say it serialises tag creation, and log the branch-push-from-a-stale-SHA behaviour as a fleet-wide item (the fix is for version:bump to push the tag refspec alone, not main).
| .start_with?("\x00GITCRYPT") | ||
| nil # tree locked: skip Docker Hub auth (base image is public) | ||
| else | ||
| YAML.load_file('config/secrets/dockerhub/credentials.yaml') |
There was a problem hiding this comment.
🔵 Safety — plan concern, not a defect of this diff
The locked-tree guard returns nil credentials so image:build skips Docker.authenticate! — correct, since src/alpine-aws/Dockerfile starts FROM alpine:3.18.5, which is public, exactly as the plan reasons.
The consequence worth recording: the old scripts/ci/steps/test.sh ran git crypt unlock first, so the CircleCI test job pulled that base image authenticated. The new test jobs in both workflows never unlock, so every base-image pull is now anonymous.
Impact: anonymous Docker Hub pulls are rate-limited per source IP, and GitHub-hosted runners share heavily NAT'd egress across the whole platform. When the limit trips, image:build fails with toomanyrequests, failing test — and since merge-pull-request has needs: [check, test], dependabot auto-merge stalls with it. It fails safe (nothing published, nothing merged), but across ~103 repos this is a plausible source of correlated, hard-to-explain red PR checks that would look like flakiness.
The guard is the right shape and this is inherent to the plan's §4.4 2a design — nothing to change. Worth adding to the pilot watch-list (issue 06) alongside the D8 native-daemon validation, and it strengthens the case for the deferred Docker Hub token hygiene follow-up: a read-only Hub token in the Actions store (not git-crypt, so dependabot runs get it too) would restore the authenticated pull limit for test without reintroducing an unlock.
| # hold secrets; the checksum is from the release's own .sha256 asset. | ||
| run: | | ||
| if ! command -v docker-compose >/dev/null 2>&1; then | ||
| sudo curl -fsSL \ |
There was a problem hiding this comment.
🔵 Security — plan concern, not a defect of this diff
First, since another lens flagged this URL as a critical defect: I verified it and the pin is correct. v5.3.1 is the current docker/compose release, the URL returns HTTP 200, and the pinned sha256 matches the release's own .sha256 asset byte-for-byte. No action needed on the version or the checksum.
The shape observation: the unverified download is written straight to its final system path (sudo curl -o /usr/local/bin/docker-compose) and only checksummed afterwards. The step's own comment notes this "runs as root on runners that later hold secrets", so the ordering is worth tightening to match that stated intent.
Impact: low in practice — sha256sum -c - fails the step (GHA defaults to bash -e {0}) before chmod +x, so a mismatched binary is left non-executable and the job aborts. But the artefact does land at a root-owned path on PATH before verification, and the pattern invites regressions if someone later reorders the lines or adds a || true.
Download → verify → install would preserve the intent structurally rather than by step ordering:
tmp=$(mktemp)
curl -fsSL "$URL" -o "$tmp"
echo "$SHA $tmp" | sha256sum -c -
sudo install -m 0755 "$tmp" /usr/local/bin/docker-composeThis block is family-uniform boilerplate straight from the plan, so the fix belongs in the plan template rather than this repo alone.
| fetch-depth: 0 | ||
| - name: Install tools | ||
| uses: infrablocks/github-actions/asdf_install@v1 | ||
| - name: Ensure docker-compose is available |
There was a problem hiding this comment.
🔵 Code Quality — plan concern, not a defect of this diff
This 9-line install guard — including the version pin v5.3.1 and its sha256 — appears four times across pr.yaml and main.yaml in this repo, and will appear in every repo of the docker family.
This is distinct from the repeated checkout/install/notify scaffolding, which the plan mandates and I am explicitly not flagging: those are one-liners delegating to ./go, whereas this is embedded logic carrying a version constant that has to stay consistent across ~400 sites.
Impact: bumping the compose pin (or fixing a checksum) means a fleet-wide, four-sites-per-repo edit with no mechanism to detect a repo left behind on a stale pin — the exact drift the shared actions repo exists to prevent. The plan says bumping the pin "is a deliberate fleet-wide change", which is precisely the operation this shape makes expensive.
Worth noting this repo cannot even use the binary: there is no spec/dependencies.yml and no dependencies:test:*, so test:integration depends only on image:build. And because ubuntu-latest ships only the docker compose plugin, command -v docker-compose fails and the step really does download and sudo-install a binary that is never invoked. The plan accepts this ("one family shape, fewer variants"), so it is not drift.
The family already has the right idiom — infrablocks/github-actions/asdf_install@v1. A sibling infrablocks/github-actions/docker_compose_install@v1 would collapse each site to a single uses: line and make the pin a one-line change in one repo, while keeping the workflows as lean as the plan intends. Plan-level suggestion; not a blocker.
| 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.
🔵 Security — plan concern, not a defect of this diff
The test job runs ./go test:integration, which executes bundle install (compiling native extensions — arbitrary code) and builds/runs a Docker image from the PR's own Dockerfile. The same job holds SLACK_BOT_TOKEN.
For fork PRs this is harmless — the workflow correctly uses pull_request rather than pull_request_target, so GitHub withholds secrets entirely (this is the single most important decision in the diff, and it is right). The case worth recording is Dependabot PRs: there the token comes from the Dependabot secret store and is exposed to exactly the dependency code being bumped. .github/dependabot.yml runs both bundler and docker ecosystems daily, so a compromised gem release or base image is the realistic path.
Impact: a malicious upstream release could exfiltrate the SLACK_BOT_TOKEN from a Dependabot CI run. Because that token is provisioned org-wide (D2, issue 19), the blast radius is the whole fleet rather than this repo.
No change needed at cutover — the Dependabot store is deliberately unprovisioned (plan §7), so the token is empty today and this is latent. Worth recording against the issue-19 decision: when the Dependabot store is populated, consider omitting the notify steps on Dependabot runs, or moving notification to a workflow_run-triggered job that never shares a runner with dependency code.
| # the family migration plan). 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, not a defect of this diff
Verified non-issue for the record, since it looks alarming: rake_factory invokes this configuration block at task-invoke time (TaskSet::TaskDefinition#resolve_block), not at Rakefile load — so the raise correctly cannot fire during the unrelated ./go invocations in CI. I confirmed bundle exec rake -T loads cleanly.
Two readability observations:
The rescue scope is easy to misread. rescue Errno::ENOENT sits in the body of the ENV.fetch block (valid since Ruby 2.6, but an uncommon construct) — at a glance it reads as rescuing ENV.fetch rather than the backtick.
Three causes, one message. gh not installed, gh installed but not logged in, and GITHUB_TOKEN set to an empty string all funnel into the same empty-string sentinel and the same error — which tells you to run gh auth login even when gh is the thing that's missing, and to set GITHUB_TOKEN even when you just did (badly).
Impact: low. The message is broadly actionable and gh's own stderr leaks to the console alongside it, so an operator will usually get there — the cost is a few minutes of confusion on the gh-not-installed path.
Hoisting into a named helper would make both the rescue scope and the intent explicit:
def github_token
ENV.fetch('GITHUB_TOKEN') { `gh auth token`.strip }
rescue Errno::ENOENT
raise 'gh CLI not found: install gh, or set GITHUB_TOKEN'
endPlan-mandated verbatim, so this belongs upstream if taken.
| namespace :slack do | ||
| RakeSlack.define_notification_tasks do |t| | ||
| t.bot_token = ENV.fetch('SLACK_BOT_TOKEN', nil) | ||
| t.routing_rules = [ |
There was a problem hiding this comment.
🔵 Code Quality — plan concern, not a defect of this diff
rake_slack evaluates these routing rules first-match-wins, and the ordering here is doing real work: the dependabot[bot] + success rule must precede the bare outcome: 'success' rule, and both must precede the when: {} catch-all — or dependabot notifications silently reroute to builds. Nothing in the block says so; the constraint is only discoverable by reading the gem source.
Impact: a future edit that adds a rule, or alphabetises the list, would silently change notification routing with no test to catch it. The kind of change that looks harmless in review and is noticed weeks later when someone wonders why builds-dependabot went quiet.
One line above the array would carry it: # First match wins — most specific rules first. The existing per-rule channel-name comments are good and worth keeping; this just states the invariant they depend on.
The plan mandates this block verbatim including its comments, so adding a line here would be drift — the comment belongs in the plan so it lands fleet-wide.
| gem 'rake' | ||
| gem 'rake_circle_ci' | ||
| gem 'rake_docker' | ||
| gem 'rake_factory' |
There was a problem hiding this comment.
🔵 Standards
gem 'rake_factory' is now declared directly — correct per plan §4.4 step 2a, since dynamic needs it and it must not ride a transitive resolution. The observation is that Gemfile.lock pins it to rake_factory (0.34.0.pre.2), a prerelease. An unconstrained gem 'rake_factory' requirement would never select a prerelease in a from-scratch resolve, so the Gemfile and the lock express different intents. The prerelease pin is inherited from the pre-existing transitive lock rather than introduced by this PR.
Impact: cosmetic today — the lock wins, and I verified bundle lock is a no-op under Ruby 3.2.11. A future bundle lock after a lock deletion, or a dependabot resolution, could quietly drop to 0.33.x, which still clears the plan's >= 0.32 floor — so nothing breaks. It is a convention wrinkle, not a hazard.
No change needed for this PR. If the family wants the Gemfile to be honest about what is locked, gem 'rake_factory', '>= 0.32' would be the fleet-wide expression — a plan-level decision, not a per-repo one.
There was a problem hiding this comment.
Pull request overview
This PR completes the repository’s CI/CD migration from CircleCI to GitHub Actions for building, testing, prereleasing, and releasing the Docker image, while also removing CircleCI-era scripts/credentials and updating the Ruby/tooling stack used by the pipeline.
Changes:
- Adds GitHub Actions workflows for PR checks and
main(check/test/prerelease + environment-gated release), including Slack notifications and dependabot auto-merge. - Updates the build/provisioning Rake tasks to use GitHub Actions secrets/environments and adds git-crypt + Docker Hub credential handling for locked trees.
- Removes CircleCI configuration/scripts and refreshes toolchain dependencies (Ruby version bump and gem updates).
Reviewed changes
Copilot reviewed 20 out of 26 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
scripts/ci/steps/test.sh |
Removes CircleCI test step script (now handled by GitHub Actions). |
scripts/ci/steps/release.sh |
Removes CircleCI release step script (now handled by GitHub Actions). |
scripts/ci/steps/prerelease.sh |
Removes CircleCI prerelease step script (now handled by GitHub Actions). |
scripts/ci/steps/merge-pull-request.sh |
Removes CircleCI dependabot merge script (replaced by gh pr merge in Actions). |
scripts/ci/common/install-slack-deps.sh |
Removes CircleCI runner dependency installation script. |
scripts/ci/common/install-gpg-key.sh |
Removes CircleCI-specific GPG key installation script. |
scripts/ci/common/install-git-crypt.sh |
Removes CircleCI runner dependency installation script. |
scripts/ci/common/install-docker.sh |
Removes CircleCI runner dependency installation script. |
scripts/ci/common/install-docker-compose.sh |
Removes CircleCI runner dependency installation script. |
scripts/ci/common/install-asdf.sh |
Removes CircleCI asdf bootstrap script. |
scripts/ci/common/install-asdf-dependencies.sh |
Removes CircleCI asdf dependency installation script. |
scripts/ci/common/configure-git.sh |
Removes CircleCI git author configuration script (replaced by a rake task). |
scripts/ci/common/configure-asdf.sh |
Removes CircleCI asdf plugin configuration script. |
README.md |
Updates CI key management docs to reference GitHub Actions location for encrypted GPG key. |
Rakefile |
Replaces CircleCI tasks with GitHub Actions provisioning (secrets + environments), adds Slack tasks, adds Docker Hub credential guard when git-crypt is locked. |
Gemfile.lock |
Updates gems to new resolved versions and adjusts platforms for the refreshed toolchain. |
Gemfile |
Removes CircleCI-related gems and adds rake_factory / rake_slack. |
.tool-versions |
Bumps Ruby version used by the toolchain. |
.github/workflows/pr.yaml |
Adds PR workflow (check/test + dependabot auto-merge + Slack notifications). |
.github/workflows/main.yaml |
Adds main workflow (skip-ci guard, check/test, prerelease, release gate + release, Slack notifications). |
.github/gpg.private.enc |
Adds encrypted CI GPG key material under .github/ for Actions use. |
.circleci/config.yml |
Removes CircleCI pipeline config. |
💡 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 }} |
Cutover to GitHub Actions per the Variant B family plan (docker image).
Includes decommission — merging this PR completes the repo's migration.
releaseenvironment gate.github/CircleCI's
setup_remote_docker+ install scripts); the integration suitebuilds the image and runs its docker-compose dependencies on the runner
dynamicblockdetecting the git-crypt
\0GITCRYPTheader, nil credentials when locked)so PR checks run without unlocking git-crypt — the one deliberate change
inside the
imagenamespacerake_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:
version:bump[rc]/version:releasetag and push beforeimage:publishruns — a failed publish leaves a version tag with no imagebehind it. Pre-existing ordering inside the untouched release logic.
An rc image is published to Docker Hub on every push to
mainwith noapproval gate; only full releases are gated (
environment: release).version:bump[rc]on a released version increments minor and startsrc.1— every release cycle is a minor bump. Pre-existinglib/version.rblogic.Dependabot auto-merge accepts any update type that passes checks, and the
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.
The old pipeline ran CI only on
mainanddependabot/*branches;pr.yamlrunning on every pull request is a deliberate family-uniformimprovement.
The
releasejob pullsmainat approval time, so a delayed approvalreleases 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).
Version tags live in git only — there is no version-bump commit, so no
push-back commit and nothing for the loop-guard to catch;
skip-ci-checkstays as family-uniform belt-and-braces.
asdf_install@v1is our own action (infrablocks/github-actions); we arehappy tracking its major version tag.
Jobs that resolve version tags (
test,prerelease,release) check outwith
fetch-depth: 0— the Rakefile'slatest_tagneeds the full tag set.The
testjobs carry a guard step installing standalonedocker-composeonly if the runner image lacks it (the Rakefile shells out to the
standalone binary); it is a no-op where compose is present or unused.
Job scaffolding is repeated flat per job by design: the logic lives in the
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.Small hunks may appear where the refreshed toolchain's rubocop
autocorrects existing code — required by the
library:checkverificationgate, not drive-by refactoring.
Provisioning (
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.
Deleting
config/secrets/{circle_ci,github}/removes the CircleCI APItoken and stored PAT from the tree but does not revoke them — both
remain live at their issuers and recoverable (git-crypted) from history.
Revocation is a host-side step on the end-of-migration sweep checklist,
alongside disabling the CircleCI project and deleting the deploy key.
Do not merge manually — the pipeline merges once checks are green.
Disabling the CircleCI project, deleting the
CircleCIdeploy key, andrevoking the CircleCI API token + stored PAT are deferred to the
end-of-migration sweep.