Skip to content

fix(ci): install libatomic before Node 26 on self-hosted ARM runners - #248

Merged
Atom-oh merged 2 commits into
devfrom
fix/runner-libatomic-20261108
Nov 8, 2026
Merged

Atom-oh merged 2 commits into
devfrom
fix/runner-libatomic-20261108

Conversation

@Atom-oh

@Atom-oh Atom-oh commented Nov 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Node 26 on linux-arm64 links libatomic.so.1, which the sample-awsops self-hosted runner image does not ship. Every self-hosted job that runs node exits 127 (error while loading shared libraries: libatomic.so.1); Deploy Web has failed on dev since the runtime refresh.
  • Adds an "Ensure Node runtime libraries" step before each actions/setup-node in the 9 self-hosted jobs (build-runtime-images, collect-runtime, deploy-agentcore, deploy-migrations ×2, deploy-web ×2, terraform ×2). It installs libatomic1 with non-interactive sudo -n apt-get only when the library is missing, and fails with an explicit error if it is still unavailable.
  • GitHub-hosted (ubuntu-latest) jobs are unchanged. The runner image itself is not modified.

Test plan

  • All workflow files parse as YAML
  • python3 -m pytest -q scripts/v2/test_ci_web_workflow.py (6 passed)
  • node --test scripts/v2/ci/*.test.mjs: 151 pass / 5 fail, identical to the unchanged baseline (local Terraform/browser prerequisites)
  • Deploy Web on dev passes the Build & push (arm64) job after merge

🤖 Generated with Claude Code

Node 26 linux-arm64 links libatomic.so.1, which the sample-awsops runner
image lacks, so every self-hosted job that runs node exited 127 (Deploy Web
has failed on dev since the runtime refresh). Install it before setup-node
when missing, and fail with an explicit error if it is still unavailable.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 8, 2026 •

Copy link
Copy Markdown

🤖 AI Code Review (two independent reviewers)

Reviewer responses: codex/ALL claude/ALL

Status: PASSED — No blocking issues found

Chair Synthesis — PR #248: fix(ci): install libatomic before Node 26 on self-hosted ARM runners

Summary

The PR inserts an identical "Ensure Node runtime libraries" step before every actions/setup-node@v4 (Node 26) step in the nine self-hosted (sample-awsops) jobs across six workflows, probing for libatomic.so.1, attempting a best-effort sudo -n apt-get install libatomic1 when absent, and failing closed with a clear ::error:: if the library is still missing; it also updates the terraform.yml structure test to cover the new step. I verified placement, gating conditions, and the test rewrite against the base workflows: the step is correctly omitted from all ubuntu-latest Node jobs, correctly gated in terraform.yml (dispatch-only in plan, approved_host in apply, with the approved_host step id defined before the insertion point), and the test change widens rather than weakens coverage. The two MAJORs raised by one panel do not survive verification; only MINOR hardening/doc items remain.

Issues per lens

L2 — Code correctness

  • MINOR (downgraded from one model's MAJOR; the other model found no issue) — apt/dpkg lock contention between concurrent jobs is swallowed, then the recheck hard-fails. The claude panel flagged this as MAJOR; codex found no correctness issue. On verification I downgrade it: the install path executes only while libatomic.so.1 is absent, which is a one-time host-provisioning condition — once any job installs it, the race ceases to exist for the lifetime of the runner image. Even during the window, the final has_libatomic recheck will pass if a sibling job completed the install, and the failure mode otherwise is a clear fail-closed error that is safe to retry. Adding -o DPkg::Lock::Timeout=60 to both apt invocations is cheap and worth doing, but this is not a gating defect.
  • MINOR (one model) — apt-get update failure short-circuits the install. { update && install; } || true skips the install entirely on a mirror/update failure even when the package would install from the local cache. Running the install after a best-effort update (apt-get update -qq || true; apt-get install …) is strictly more robust.
  • MINOR (one model) — the ldconfig probe is architecture-agnostic. grep -q 'libatomic\.so\.1' would be satisfied by a non-aarch64 multiarch entry, in which case the job regresses to the original opaque Node loader error. Low likelihood on this dedicated ARM64 runner, but matching the ABI ((libc6,AArch64)) or testing the aarch64 library path directly would make the gate precise.
  • MINOR (one model) — duplicated five-clause gate in terraform.yml plan job. The new step restates setup-node's condition (terraform.yml:215) verbatim rather than sharing it; the updated test asserts only the workflow_dispatch clause, so the other clauses can drift silently. Drift is low-harm (worst case: apt runs on a plan that skips Node, or Node fails later with the pre-existing opaque error), hence MINOR.
  • The test rewrite in scripts/v2/ci/prepare-runtime-host.test.mjs:128-131 is correct: the first Ensure Node runtime libraries occurrence is in the plan job and precedes its setup-node, the ordering assertion is new coverage, and the split(' - ') tokenization is unaffected by the new run body (all body lines are indented past the split token and contain no - substring). Both models agree the step is fail-closed and correctly ordered.

L3 — Security / AWS mutation safety

  • No blocking finding. The models disagreed: claude flagged a MAJOR for "root-level, unpinned package installation in credentialed jobs"; codex found no security issue. I side with codex after verification. The step touches no AWS API, IAM policy, permissions: block, or secret, and the ADR-005 mutation freeze is not implicated — those are the repo's L3 criteria. The "unpinned content from the mirror" framing overstates the risk: apt enforces the distro signing chain, so the mirror cannot serve arbitrary code, and version drift of libatomic1 is immaterial. sudo -n fails fast without a password and the step degrades gracefully (if the package is pre-provisioned, no sudo is exercised at all), so it does not require passwordless sudo; moreover this host already assumes sudo in deployment flows (scripts/CLAUDE.md: the DOCKER env defaults to sudo docker), so no new trust boundary is created. sudo's default env_reset means the OIDC AWS credentials exported by configure-aws-credentials (terraform.yml:156-162) do not reach the apt child; only DEBIAN_FRONTEND is passed explicitly. Downgraded to the suggestion below (bake the package into the runner image and keep only the check in CI), which is the right long-term posture but not a merge blocker.
  • Positive (both models): in terraform.yml the privileged step is gated behind workflow_dispatch, so PR-triggered read-only plans never execute sudo, and the rewritten test now enforces that gate on the new step. All checkouts retain persist-credentials: false.

L4 — Observability / data-integration correctness

  • No blocking finding (both models). No Steampipe query, CIS check, or AgentCore diagnosis logic changes; the patch touches only workflow YAML and one structure test. The step improves diagnosability of the runtime-collection workflows by converting a dynamic-linker crash into an explicit error before any collection begins, and it fails closed.
  • MINOR (one model) — -qq plus || true discards the apt failure evidence, so the operator cannot distinguish no-sudo vs. dpkg lock vs. mirror outage from the final generic error. Capturing apt output and emitting it as a ::warning:: before the final check would keep fail-fast behavior while making the host state observable.

L5 — Docs/ADR consistency

  • No blocking finding (both models). No ADR boundary is touched; no ADR update is required. merge-verify.yml (the README's node-version source of truth) runs on ubuntu-latest and is correctly left unpatched. Omitting a CHANGELOG bullet is correct per the root CLAUDE.md convention (runner-library CI fix, not user-visible behavior).
  • MINOR (one model) — docs/runbooks/dev-repo-setup.md:73-76 ("### 1. Runner … Nothing to do unless runs queue forever") is now incomplete. Verified: the runner section documents no package prerequisites, yet a missing libatomic1 now hard-fails every self-hosted Node job with an error the runbook does not explain. Add the prerequisite and remediation.
  • MINOR (one model) — the repo-level agent/host prerequisite documentation lists Node 26 and other host requirements but not the new native-library requirement for self-hosted ARM64 runners; recording it alongside the existing Node 26 entry keeps the prerequisite list authoritative.
  • Noted, non-blocking: only terraform.yml has a test guarding the step's presence/ordering; the other five workflow copies are unguarded against future deletion.

Suggestions

  1. Preferred long-term fix: provision libatomic1 in the runner image/bootstrap and keep only the has_libatomic || exit 1 verification in the workflows — this removes sudo and apt from CI entirely while preserving the fail-fast diagnostic (addresses the L3 concern and most L2 minors at once).
  2. If the install stays in CI: add -o DPkg::Lock::Timeout=60 to both apt calls, run the install even when update fails, and surface captured apt output as a ::warning:: before the final check.
  3. Tighten the ldconfig probe to the aarch64 ABI, and consider deriving the duplicated terraform.yml plan-job condition from a single source (or extend the test to assert condition equality with setup-node).
  4. Document the libatomic1 prerequisite in docs/runbooks/dev-repo-setup.md §1 Runner (English-only per docs conventions).

Verdict

Both panels agree the change is fail-closed, correctly placed, correctly gated in terraform.yml, and free of AWS-mutation/ADR-005, IAM, or secret concerns. The single panel's two MAJORs (dpkg lock race; unpinned sudo apt in credentialed jobs) were verified against the base workflows and repo conventions and downgraded to MINOR/suggestions: the race is transient and self-healing with a safe fail-closed retry, and the install is signed-distro content with no credential exposure, no new sudo requirement, and no repo security rule implicated. No CRITICAL or MAJOR finding survives verification. The image manifest lists no images and no unavailable/omitted entries.

IMAGE_COVERAGE: NOT_REQUIRED


Triggered by commit 98a21196475971492daf1435e5b98da66330afc6 · workflow: .github/workflows/pr-review.yml

…licitly

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Atom-oh
Atom-oh merged commit f75e7d7 into dev Nov 8, 2026
9 checks passed

This branch was previously deployed

1 inactive deployment
ci-review-auto — 98a21196 Deployed Nov 8, 2026 by Atom-oh via AI Code Review #988
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.

1 participant