Repository navigation
Migrate pipeline from CircleCI to GitHub Actions - #275
Conversation
phelma
left a comment
There was a problem hiding this comment.
Code Review: #275 - Migrate pipeline from CircleCI to GitHub Actions
Verdict: COMMENT
A faithful, plan-conformant CircleCI → GitHub Actions cutover. Independent conformance-checking against family plan §4 found every required change present and correctly shaped, and nothing changed beyond the plan. All five lenses (correctness, security, safety, standards, code-quality) returned findings, but every in-scope finding challenges a decision the plan or PR description documents as deliberate — so they are recorded as plan concerns for a human to revisit at the plan level, not as blocking defects. One correctness finding was disproven (false positive).
Plan-conformance (independent, both directions)
- Required changes present: pr.yaml + main.yaml, byte-identical gpg key move, Rakefile (require swaps, RakeCircleCI/keys:deploy removal, ambient-token + passphrase-guard provisioning, rake_slack routing, set_ci_author, library:build, pipeline:prepare, prerelease:publish), gemspec dep swap, README retitle, full CircleCI decommission. All match §4.
- Nothing beyond the plan: lockfile carries only expected transitive bumps; rake_github 0.17.0, rake_git_crypt 0.4.0, rake_slack 0.3.0 as required.
- Verified
spec.name = 'ruby-terraform', sogem_file = "ruby-terraform-#{version}.gem"correctly matchesgem buildoutput. - Zero in-scope defects. No plan violations found.
False positive (investigated, disproven)
- 🔵 Correctness flagged the
main.yamlprerelease job's detached-HEAD checkout +git pushas guaranteed to fail. Disproven: the canonical already-liverake_slackmain.yaml uses the identical bareactions/checkout@v4+git pushshape in production. Plan §4.2 deliberately omitsref:on prerelease. Not counted.
Plan concerns (documented-deliberate; do not block)
- 🟡
queue: maxconcurrency key (correctness, standards ×2) — flagged as an unrecognised Actions key; plan §4.2 specifies it explicitly with a GA changelog cited 2026-05-07. Plan's exact YAML → plan concern. A human should confirm the changelog (reviewer schema knowledge may be stale). - 🟡 Publish-before-push ordering (safety ×2) —
gem release --tag --pushprecedesgit push; §1 parity hazard + PR description. - 🟡 PR prerelease exposes RubyGems key + passphrase to PR-branch code (security) — inherent to D8; fork/Dependabot guard is the accepted mitigation.
- 🔵 PR pre-releases accumulate in the public namespace (safety) — explicitly D8-accepted.
- 🔵 Release publishes main-as-of-approval, not the tested SHA (safety) — PR-documented deliberate parity.
- 🔵 Dependabot merge under GITHUB_TOKEN restrictions (security) — challenges D3; fails safe; relates to deferred issue 19.
- 🔵 openssl
-md sha1weak KDF (security) — inherited, only relocated; README openssl left untouched per §4.6/§4.8. - 🔵 Gem filename hardcoded (code-quality) — plan §4.4 step 8 prescribes the exact literal; verified correct.
- 🔵 Provisioning validation embedded +
Metrics/BlockLengthdisable (code-quality) — plan §4.4 step 3 prescribes this inline; "do not restructure". - 🔵
.yamlvs.yml(standards) — plan-specified; no in-repo convention violated. - 🔵
git pull --ff-onlytracking (correctness, low confidence) — plan's exact §4.2 YAML; reference flow works.
Strengths
- ✅ Untrusted PR inputs passed via
env:, never interpolated intorun:— no workflow injection. - ✅ Workflow-level
contents: read; write scopes granted narrowly per-job. - ✅ Provisioning fails fast on empty token and git-crypt ciphertext before uploading a secret.
- ✅
prerelease:publishrestoresversion.rband removes the gem in anensureblock;run_attemptprevents re-run collisions. - ✅ Dependabot merge uses
--match-head-commitso a post-checks commit fails the merge rather than landing unchecked.
Review generated by /accelerator:review-pr
| concurrency: | ||
| group: main | ||
| cancel-in-progress: false | ||
| queue: max |
There was a problem hiding this comment.
🟡 Plan concern (correctness, standards — lenses assigned major)
Three lenses flag queue: max as not a recognised GitHub Actions concurrency key (only group and cancel-in-progress are documented). The plan §4.2 specifies it explicitly and cites a GA changelog dated 2026-05-07. This is the plan's exact, documented-deliberate YAML → recorded as a plan concern, not a defect. A human should confirm the cited changelog is real (the reviewing models' schema knowledge predates the GA date). Applies identically to the release job at line 99.
| - name: Bump version | ||
| run: ./go "version:bump[pre]" | ||
| - name: Release | ||
| run: ./go release |
There was a problem hiding this comment.
🟡 Plan concern (safety — lens assigned major)
./go release (gem release --tag --push) publishes to RubyGems before the separate git push of the bump commit. A push failure after a successful publish desyncs RubyGems from git and blocks future releases. Explicitly listed in plan §1 parity hazards and the PR description as inherited, out-of-scope behaviour — recorded for a human to weigh as post-migration hardening.
| # Same-repo human PRs only: fork and Dependabot PRs carry no secrets, so | ||
| # git-crypt unlock / the RubyGems publish would fail. user.login is the | ||
| # immutable PR author, not github.actor. | ||
| if: >- |
There was a problem hiding this comment.
🟡 Plan concern (security — lens assigned major)
The prerelease job unlocks git-crypt and configures RubyGems credentials, then runs ./go/Rakefile tasks checked out from the PR branch — before merge. A collaborator could alter build tasks to exfiltrate the RubyGems key / passphrase. This is inherent to D8 (PR-CI prerelease publish); the same-repo + non-Dependabot guard is the accepted mitigation → plan concern. Worth confirming the RubyGems key is scoped to this gem only.
| run: ./scripts/ci/common/configure-rubygems.sh | ||
| - name: Publish prerelease | ||
| # Facts via env, never interpolated | ||
| run: ./go "prerelease:publish[$PR_NUMBER,$RUN_NUMBER,$RUN_ATTEMPT]" |
There was a problem hiding this comment.
🔵 Plan concern (safety — lens assigned minor)
ruby-terraform-<ver>.pr<PR>.<run>.<attempt> is published to the public namespace on every PR push and accumulates permanently; a consumer using --pre could resolve an unmerged build. Explicitly accepted in D8 → plan concern.
|
|
||
| version = "#{base}.pr#{args.pr_number}" \ | ||
| ".#{args.run_number}.#{args.run_attempt}" | ||
| gem_file = "ruby-terraform-#{version}.gem" |
There was a problem hiding this comment.
🔵 Plan concern (code-quality — lens assigned minor)
gem_file = "ruby-terraform-#{version}.gem" duplicates spec.name; a gemspec rename would break gem push with an opaque file-not-found. Verified correct today (spec.name is ruby-terraform). The plan §4.4 step 8 prescribes this exact literal → plan concern. Deriving the filename from the spec would decouple it.
| end | ||
|
|
||
| RakeGithub.define_repository_tasks( | ||
| RakeGithub.define_repository_tasks( # rubocop:disable Metrics/BlockLength |
There was a problem hiding this comment.
🔵 Plan concern (code-quality — lens assigned minor)
Token-fallback and git-crypt ciphertext checks live inline in the define_repository_tasks block, now large enough to need a Metrics/BlockLength disable, and can only be exercised by running the whole task. Extracting helpers would make them testable — but the plan §4.4 step 3 prescribes this exact inline code and "do not restructure anything else in the Rakefile" → plan concern.
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