Skip to content

Migrate pipeline from CircleCI to GitHub Actions#274

Closed
phelma wants to merge 1 commit into
mainfrom
gha-migration
Closed

Migrate pipeline from CircleCI to GitHub Actions#274
phelma wants to merge 1 commit into
mainfrom
gha-migration

Conversation

@phelma

@phelma phelma commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Cutover to GitHub Actions per the Variant A family plan (gem pilot).
Includes decommission — merging this PR completes the repo's migration.

  • main + pr workflows: loop-guard, check/test, prerelease, release environment gate
  • git-crypt unlock on the runner; encrypted CI GPG key moved to .github/
  • Slack notifications via rake_slack; dependabot auto-merge job
  • Rakefile provisioning swapped to rake_github secrets/environments; rake_circle_ci dropped
  • CircleCI pipeline removed: .circleci/, scripts/ci/, the CI SSH deploy
    key pair and its keys:deploy/deploy_keys provisioning, and the stored
    CircleCI/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 release publishes to RubyGems before the version-bump commit is
    pushed — pre-existing ordering inside the untouched release logic.
  • Prerelease publishes on every push to main with no approval gate; only
    full releases are gated (environment: release).
  • 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 next
    human-triggered release.
  • The release job pulls main at approval time, so a delayed approval
    publishes main as it stands then, not the SHA this run tested — parity with
    the old release.sh (which also pulled; prerelease.sh did not, so the
    prerelease job has no pull).
  • asdf_install@v1 is our own action (infrablocks/github-actions); we are
    happy tracking its major version tag.
  • 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 and
    supplies secrets/context.
  • Gemfile.lock carries transitive major bumps — the unavoidable resolution
    of the targeted bundle lock --update, not scope creep.
  • Small library hunks may appear where the refreshed toolchain's rubocop
    autocorrects existing code (e.g. Style/ArgumentsForwarding) — required
    by the library:check verification gate, not drive-by refactoring.
  • Provisioning (pipeline:prepare) authenticates with the operator's ambient
    gh login (GITHUB_TOKEN fallback) instead of a stored PAT — a deliberate
    parity deviation; the stored token in config/secrets/github/config.yaml
    is 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 CircleCI deploy key are
deferred to the end-of-migration sweep.

@phelma phelma left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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: max is not a valid GitHub Actions concurrency key (flagged by: correctness, safety, standards) — concurrency supports only group and cancel-in-progress. With cancel-in-progress: false GitHub 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: read default-deny, with write scopes widened only on the jobs that need them.
  • ✅ Untrusted github.event.head_commit.message passed via the HEAD_COMMIT_MESSAGE env var and quoted in bash — avoids the ${{ }}-into-run: injection sink. Same pattern for the dependabot PR_URL.
  • ✅ Uses pull_request (not pull_request_target) so fork PRs run read-only without secrets.
  • ✅ Secret deletions correctly scoped: rubygems/credentials and ci/encryption.passphrase retained; 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_TOKEN no-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@v4 is 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.status interpolated into run: 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_path is never configured after the key moved to .github/. Verified false: rake_git_crypt 0.4.0 defaults encrypted_key_path to .github/gpg.private.enc and passphrase_env_var_name to ENCRYPTION_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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

@phelma
phelma marked this pull request as ready for review July 14, 2026 15:37
Copilot AI review requested due to automatic review settings July 14, 2026 15:37

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +10 to +17
# 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
Comment thread .github/workflows/pr.yaml
Comment on lines +62 to +66
- 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 phelma left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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: max concurrency key (correctness + safety) — both lenses flagged main.yaml:17 as 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 + needs chaining 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.
  • release gated behind environment: 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 the rescue Errno::ENOENT => '' that plan §4.4 step 3 specifies. A missing gh binary raises a raw Errno::ENOENT instead of the friendly guard the comment promises. In-scope defect (major).
  • 🔵 Documentation / plan-conformance (main.yaml:9-12): keeps queue: max but drops the plan's GA 2026-05-07 justification comment — the load-bearing evidence for the unusual key. In-scope defect (minor).
  • 🔵 Security (plan concern): PR check/test/build jobs run PR-supplied ./go tasks and expose SLACK_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:186 trailing-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

Comment thread Rakefile Outdated
# 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 }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 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.

Comment thread .github/workflows/pr.yaml
continue-on-error: true
run: ./go "slack:notify[${{ job.status }}]"
env:
SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 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 phelma left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.yaml match §4.1/§4.2 verbatim (permissions, concurrency with queue: max, continue-on-error notifies, plain --merge, release runs documentation:update, no git pull on 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, trimmed pipeline:prepare.
  • ✅ Gemspec dev-deps swapped (alphabetical); lockfile drops rake_circle_ci/rake_ssh; PLATFORMS unchanged from prior commit (no new darwin platform; ruby+x86_64-linux retained).
  • ✅ README CI-neutral wording + .github/ paths, no badge invented.
  • ✅ §4.7 deletions all present; git grep -i circleci outside 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, .yaml naming and job names match sibling + plan verbatim.
  • Security: untrusted head_commit.message routed via env var (correct injection mitigation); least-privilege token scoping; release gated behind reviewed environment; chmod 0600 credential; safe pull_request trigger.
  • Correctness: CI-loop guard sound end-to-end; != 'true' fails open (safe).
  • Code quality: fail-fast github_token resolution; purposeful "why" comments.

General Findings (all non-blocking plan concerns)

  • 🔵 Correctness: queue: max is a recently-GA key; a live green main run confirms the runner accepts it. Documented (with changelog link).
  • 🔵 Correctness: skip-ci-check inspects 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 main push, ungated — documented deliberate parity.
  • 🔵 Safety: no timeout-minutes on check/test/build — matches prescribed shape.
  • 🔵 Safety: a long-pending release approval holds the serialised main group — documented intent of queue: 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

Comment thread .github/workflows/pr.yaml
pull-requests: write
steps:
- name: Merge pull request
run: gh pr merge --merge "$PR_URL"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 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.

Comment thread Rakefile
{ when: { type: 'on_hold' },
channel: 'C038EDCRSQJ', format: :on_hold }, # release
{ when: { actor: 'dependabot[bot]', outcome: 'success' },
channel: 'C03N711HVDG', format: :success }, # builds-dependabot

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 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.

Comment thread Rakefile
# 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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 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.

@phelma phelma closed this Jul 17, 2026
@phelma
phelma deleted the gha-migration branch July 17, 2026 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants