Migrate pipeline from CircleCI to GitHub Actions#106
Conversation
phelma
left a comment
There was a problem hiding this comment.
Code Review: #106 - Migrate pipeline from CircleCI to GitHub Actions
Verdict: COMMENT
This is a clean, plan-conformant CircleCI → GitHub Actions cutover with notably strong security hygiene (all untrusted PR-controlled values passed via env: rather than interpolated into run:, least-privilege permissions:, a correct fork/Dependabot guard on the prerelease job, and explicit git-crypt-ciphertext guards in provisioning). Across five lenses (correctness, security, safety, standards, code quality) no in-scope critical or major defects were found; the substantive findings are documented deliberate decisions from the family plan (plan concerns) plus two small in-scope defence-in-depth nits. The diff faithfully implements every §4 change and nothing beyond the plan was altered.
Strengths
- ✅ Untrusted PR data (title, number, head SHA, URL) is passed exclusively via
env:and referenced as shell variables — closing the classic Actions script-injection vector. - ✅ The prerelease job guard (
head.repo.full_name == github.repository && user.login != 'dependabot[bot]') correctly excludes fork and Dependabot PRs and uses the immutableuser.login, not the spoofablegithub.actor. - ✅ Least-privilege permissions: top-level
contents: readwith per-job elevation only where needed. - ✅ Provisioning refuses to upload the passphrase if the file is missing or still git-crypt ciphertext (
\x00GITCRYPT), and resolves the GitHub token fail-fast from ambient auth rather than a checked-in PAT. - ✅
prerelease:publishvalidates all three args and restoresversion.rbin anensureblock; the release job usesgit pull --ff-onlyto fail loud. - ✅ Full decommission verified: deleted secrets/scripts orphan nothing still referenced; retained rubygems credentials, passphrase and GPG keys remain in use.
General Findings
- 🔵 Security (minor):
scripts/ci/common/configure-rubygems.shno longer runsgit crypt unlock(unlock is now an explicit earlier workflow step) but still blindly copiesconfig/secrets/rubygems/credentialsto~/.gem/credentialswith no ciphertext guard — unlike the Rakefile passphrase handling this PR hardens. Not exploitable today (all jobs unlock first); defence-in-depth only. Consider mirroring the\x00GITCRYPTguard. - 🔵 Security (suggestion): the Slack notify steps interpolate
${{ job.status }}intorun:— safe here (GitHub-controlled enum) but inconsistent with the "never interpolate into run:" convention the rest of the diff follows. Could pass viaenv:for uniformity.
Plan Concerns (documented-deliberate; non-blocking — for the human to revisit the plan)
- 🔵
queue: maxin theconcurrency:blocks (correctness lens rated critical). Flagged as an unknown key that would make Actions reject the workflow. However this is prescribed verbatim by plan §4.2, which cites a GitHub changelog (2026-05-07: concurrency groups allow larger queues). The diff reproduces the plan's authoritative YAML exactly, so this is a plan concern, not a defect of the diff. Worth a human double-check thatqueue: maxis a currently-valid concurrency key before merge — an invalid key would fail the whole main pipeline — but any fix belongs in the plan, fleet-wide. - 🔵 Provisioning block embeds procedural logic (code-quality). The
RakeGithubblock inlines token resolution + git-crypt guard, needing a# rubocop:disable Metrics/BlockLength. Extracting helpers is the natural quality move but the plan prescribes this inline code and says "do not restructure anything else in the Rakefile". - 🔵 Duplicated argument-name list in
prerelease:publish(code-quality) — reproduced verbatim from plan §4.4 step 8. - 🔵 Slack routing inline-comment spacing (standards) — one space vs two, reproduced exactly from the plan's template text.
False Positives (verified, counted nowhere)
- GPG key path discovery (safety): concern that
.github/gpg.private.encmight not be found by the unlock task. Verified againstrake_git_crypt0.4.0:encrypted_key_pathdefaults to.github/gpg.private.enc— exactly the new location. Correct as-is.
Review generated by /accelerator:review-pr
| concurrency: | ||
| group: main | ||
| cancel-in-progress: false | ||
| queue: max |
There was a problem hiding this comment.
🔵 Plan concern / verify (correctness) — The concurrency: block uses queue: max (also in the release job). The correctness lens flagged this as an unknown key that would make Actions reject the workflow — historically concurrency: only accepts group and cancel-in-progress. This is prescribed verbatim by the family plan (§4.2, citing a 2026-05-07 GitHub changelog adding larger queues), so it is treated as a documented deliberate decision rather than a defect of this diff. Flagging for a human to confirm queue: max is a currently-valid concurrency option before merge — if it is not, the entire main pipeline would fail to run. Any change belongs in the plan (fleet-wide), not this PR.
| end | ||
|
|
||
| RakeGithub.define_repository_tasks( | ||
| RakeGithub.define_repository_tasks( # rubocop:disable Metrics/BlockLength |
There was a problem hiding this comment.
🔵 Plan concern (code-quality) — The RakeGithub.define_repository_tasks block now inlines token resolution and the git-crypt passphrase guard, which pushed it over the length threshold and required the # rubocop:disable Metrics/BlockLength. Extracting resolve_github_token / read_ci_passphrase helpers would make the routines independently testable and likely remove the suppression — but the plan prescribes this inline code and instructs "do not restructure anything else in the Rakefile", so this is a plan concern for a future fleet-wide revisit, not a change for this PR.
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.
PR-CI prerelease publish (deliberate, permanent)
pr.yamlhas aprereleasejob that publishes a namespaced pre-release ofthis gem to RubyGems from the PR branch — a permanent CI feature, not
migration-only. This is a deliberate deviation from CircleCI (which published
nothing pre-merge): it proves the publish path before merge instead of
discovering it broken on
main. The version is<committed-version>.pr<PR>.<run>.<attempt>(via the newprerelease:publishRakefile task), so it can never collide with
main'sversion:bump[pre]sequence; the task builds the gem and pushes it straight to RubyGems, then
restores
version.rb, so nothing is committed, tagged, or pushed(
gem releaseis not used — it aborts on the uncommitted version rewrite).The job is skipped for fork
and Dependabot PRs (they hold no secrets), and
merge-pull-requestdoes notdepend on it. PR pre-release versions accumulate permanently on RubyGems —
accepted.
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.
🏭 This PR was opened by Foundry, Atomic's AI software development
factory. Implementation, review, and fixes are performed by AI agents;
merges happen automatically once the review and checks gates pass.
This task migrates a Ruby gem's CI from CircleCI to GitHub Actions.
migratemigrate-gem2026-07-23T15-01-57-210Zatomic-foundry-pr · foundry-pipeline: migrate · foundry-task: migrate-gem · foundry-run: 2026-07-23T15-01-57-210Z