Repository navigation
Migrate pipeline from CircleCI to GitHub Actions - #196
Conversation
phelma
left a comment
There was a problem hiding this comment.
Code Review: #196 — Migrate pipeline from CircleCI to GitHub Actions
Verdict: COMMENT
A clean, plan-conformant CircleCI → GitHub Actions cutover for rake_terraform. The diff matches the Variant A family plan §4 in both directions: every required change is present and nothing beyond the plan was touched. The new Ruby (ambient-token resolution, git-crypt ciphertext guards, prerelease:publish) is careful, fails fast with actionable messages, and cleans up after itself. Almost every lens finding challenges a decision the plan or PR description documents as deliberate — recorded below as plan concerns for a human to revisit at the fleet level, not blocking defects.
Cross-Cutting Themes
- Inherited release-ordering hazards (safety, correctness) — both the
mainprereleaseandreleasejobs publish to RubyGems before the version-bump commit/tag is pushed, andprerelease(unlikerelease) does notgit pullfirst. Documented deliberate inherited parity (§1, PR description). queue: maxconcurrency key (correctness) — the plan's authoritative §4.2 YAML specifiesqueue: max, citing a GitHub changelog (GA 2026-05-07). A lens flagged it as an invalid key that would reject the workflow. Treated as a plan concern — the diff faithfully reproduces the plan's exact YAML; if the feature isn't actually GA, the fix belongs in the family plan. Worth anactionlint/live-run confirmation before fleet rollout.
Strengths
- ✅ Secret-safe design:
on: pull_request(neverpull_request_target); secret-bearingprereleaseguarded to same-repo, non-Dependabot PRs via immutableuser.login; attacker inputs viaenv:not${{ }}; least-privilegecontents: readdefault. - ✅ Provisioning guards detect a locked git-crypt clone (
\x00GITCRYPT) and a missing passphrase before uploading — no silent ciphertext-as-secret. - ✅
prerelease:publishusesbegin/ensureto restoreversion.rband remove the built gem. - ✅ Dependabot auto-merge uses
--match-head-commit "$HEAD_SHA". - ✅ Rakefile/gemspec/lockfile ordering stays alphabetical and in sync; new tasks follow existing conventions.
Plan Conformance (independent §4 check)
Verified both directions — all required changes present, nothing extra:
- §4.1
pr.yaml, §4.2main.yaml(theUpdate documentationstep correctly omitted — the deletedrelease.shnever ran it), §4.3 byte-identical GPG rename, §4.4 Rakefile, §4.5 gemspec+lockfile (no.pregems; PLATFORMSruby+x86_64-linux, no darwin), §4.6 README, §4.7 decommission (scripts trimmed as specified). No plan violations found.
General Findings
- 🔵 code-quality (in-scope minor):
RakeGithubblock bundles token/passphrase/secret/environment concerns inline and carries a# rubocop:disable Metrics/BlockLength. Extracting helpers would clear the smell (but the plan says not to restructure the Rakefile — a fleet-plan conversation). - 🔵 correctness:
releasejobgit pull --ff-onlyon a default shallow checkout — low confidence, plan-specified shape. - 🔵 security (plan concern): dependabot any-update auto-merge with
[skip ci](D3); PR-CI RubyGems credential exposed on every internal PR — ensure the key is gem-scoped/push-only (D8); inline${{ job.status }}inrun:(safe enum, plan-verbatim). - 🔵 safety (plan concern): publish-before-push; prerelease push without pull; shared
mainconcurrency group across the release approval wait; permanent PR-CI prerelease gems — all documented deliberate/inherited parity. - 🔵 standards (plan concern):
.yamlvs repo.yml; verbatimcheck/testduplication across workflows (deliberate flat scaffolding). - 🔵 code-quality (plan concern): inline
version.rbregex vsgem bumpcoupling (plan-verbatim §4.4).
Review via /accelerator:review-pr — lenses: correctness, security, safety, standards, code-quality.
| concurrency: | ||
| group: main | ||
| cancel-in-progress: false | ||
| queue: max |
There was a problem hiding this comment.
🔴→🔵 Correctness (plan concern)
The concurrency block uses queue: max (here and on release, line 99). A lens flagged this as an unrecognised concurrency key that would make GitHub reject the workflow as invalid, breaking all of main.yaml.
However, the family plan §4.2 specifies queue: max deliberately, citing a GitHub changelog that GA'd queue-depth control for concurrency groups on 2026-05-07. So this is reproducing the plan's authoritative YAML, not a diff defect — recorded as a plan concern.
Action: before the fleet rollout, confirm the feature is live (e.g. actionlint + a real run). If queue is not a valid key, the fix belongs in the family plan (fleet-wide), not this PR.
| - name: Bump version | ||
| run: ./go "version:bump[patch]" | ||
| - name: Release | ||
| run: ./go release |
There was a problem hiding this comment.
🔵 Safety (plan concern)
./go release runs gem release --tag --push (publishing the immutable gem) before the following git push / git push --tags. If the push fails (e.g. main advanced), the version is permanently on RubyGems with no matching git state, and the next run re-bumps from a stale version.
The PR description acknowledges this as inherited pre-existing ordering (untouched ./go release logic, D5) — no change requested here; flagged so a human can pick it up as post-migration fleet work.
| # [skip ci] stops the merge commit triggering a release build | ||
| # PR title via env, never interpolated | ||
| run: gh pr merge --merge --match-head-commit "$HEAD_SHA" "$PR_URL" --subject "$PR_TITLE [skip ci]" | ||
| env: |
There was a problem hiding this comment.
🔵 Security (plan concern)
The merge-pull-request job auto-merges any Dependabot PR that passes check/test/build — including major transitive bumps — and the [skip ci] subject means the merge runs no further verification. A compromised or typo-squatted upstream release that still builds and passes tests would land without human review.
Documented as deliberate parity (D3): the old CircleCI flow merged the same way with [skip ci]. No change requested; consider an update-type constraint as post-migration hardening.
Part of PP-709.
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-22T17-48-59-192Zatomic-foundry-pr · foundry-pipeline: migrate · foundry-task: migrate-gem · foundry-run: 2026-07-22T17-48-59-192Z