Repository navigation
fix(OMN-13946): add libatomic1 to runner image (Pyright node runtime defect) - #2207
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 15 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR adds retry-with-backoff logic to ChangesCI Workflow Retry Logic
Runner Image libatomic1 Fix
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
17a6d3e to
375ed99
Compare
375ed99 to
32e96b5
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/validate-validator-requirements.yml (2)
54-60: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider also setting
UV_HTTP_TIMEOUThere for consistency.
url-authority-gate.ymlbumpsUV_HTTP_TIMEOUTto"600"alongside its retry loop, but this workflow'suv sync --all-extrasstill relies on uv's default 30s HTTP read timeout. If the underlying flakiness is timeout-related, this workflow could still fail on slow reads before the retry loop even gets a chance to matter.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/validate-validator-requirements.yml around lines 54 - 60, The retry loop around uv sync in the workflow should also set UV_HTTP_TIMEOUT for consistency with the other workflow. Update the validate-validator-requirements job so the environment used by the uv sync --all-extras step includes UV_HTTP_TIMEOUT set to 600, matching the retry behavior and preventing uv’s default read timeout from causing premature failures.
54-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRetry loop is duplicated across three workflows; consider extracting.
The identical 6-line retry block is now copy-pasted in
url-authority-gate.yml,validate-validator-requirements.yml, andvalidator-runtime-profiles.yml. Consider extracting it into a reusable composite action (e.g..github/actions/uv-sync-with-retry) to avoid drift when backoff/attempt tuning changes later.♻️ Example composite action
# .github/actions/uv-sync-with-retry/action.yml name: uv sync with retry runs: using: composite steps: - shell: bash run: | for attempt in 1 2 3; do uv sync --all-extras && break if [ "$attempt" = "3" ]; then exit 1 fi sleep "$((attempt * 10))" done🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/validate-validator-requirements.yml around lines 54 - 60, The retry loop for `uv sync --all-extras` is duplicated and should be centralized. Extract the shared logic from the workflow step into a reusable composite action such as `.github/actions/uv-sync-with-retry/action.yml`, then update the workflow that contains this block to call that action instead of inlining the loop. Keep the behavior and symbols around the `uv sync` step, but make the backoff/attempt handling live in one place so the other workflows can reuse it consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/validate-validator-requirements.yml:
- Around line 54-60: The retry loop around uv sync in the workflow should also
set UV_HTTP_TIMEOUT for consistency with the other workflow. Update the
validate-validator-requirements job so the environment used by the uv sync
--all-extras step includes UV_HTTP_TIMEOUT set to 600, matching the retry
behavior and preventing uv’s default read timeout from causing premature
failures.
- Around line 54-60: The retry loop for `uv sync --all-extras` is duplicated and
should be centralized. Extract the shared logic from the workflow step into a
reusable composite action such as
`.github/actions/uv-sync-with-retry/action.yml`, then update the workflow that
contains this block to call that action instead of inlining the loop. Keep the
behavior and symbols around the `uv sync` step, but make the backoff/attempt
handling live in one place so the other workflows can reuse it consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b96e6c7c-a750-4739-a9a4-b2f1b8651987
📒 Files selected for processing (6)
.github/workflows/url-authority-gate.yml.github/workflows/validate-validator-requirements.yml.github/workflows/validator-runtime-profiles.ymldocker/runners/Dockerfiledocker/runners/runner-image.lock.jsontests/integration/infra/test_omn_12765_release_backmerge_identity.py
…defect) The self-hosted CI runner image's bundled GitHub Actions node runtime (used by node-based actions, e.g. pyright) dlopens libatomic.so.1 at load time. Ubuntu 22.04 does not pull libatomic1 in transitively, so every node-based action fails with "node: libatomic.so.1: cannot open shared object file" — deterministic, not transient. Blocks omniclaude PR #1860 (OMN-13502) and any omniclaude PR that runs pyright. Add libatomic1 to the runner image apt install list, bump image_version 5->6 (real image content change), and regenerate the bound identity (docker/runners/runner-image.lock.json). The Dockerfile apt-list is not itself a binding input, so identity_digest only moves because image_version does; shared_env_digest is unchanged. Updates the OMN-12765 release-backmerge identity-pin test to the regenerated digest. Rebuild + fleet redeploy on .201 is a separate gated live-runtime step, out of scope here. Evidence-Source: OCC#TBD Evidence-Ticket: OMN-13946
32ea48e to
eb557d3
Compare
Summary
libatomic.so.1at load time; Ubuntu 22.04 doesn't pulllibatomic1in transitively. Every node-based action on the runner image fails deterministically withnode: libatomic.so.1: cannot open shared object file— real, not transient. Blocks omniclaude PR chore(OMN-12667): bump zone-filter workflow ref #1860 (OMN-13502) and any omniclaude PR that runs pyright.libatomic1todocker/runners/Dockerfileapt install list.image_version5->6 indocker/runners/runner-image.lock.json(real image content change) and regenerates the boundidentity_digestviascripts/ci/runner_image_identity.py --mode generate. The Dockerfile apt-list is not itself a binding input, soidentity_digestonly moves becauseimage_versiondoes;shared_env_digestis unchanged.tests/integration/infra/test_omn_12765_release_backmerge_identity.py) to the regenerated digest.Scope
Code fix + identity lock only. Image rebuild + fleet redeploy on
.201is a separate gated live-runtime step, out of scope here.Evidence-Ticket: OMN-13946
Evidence-Source: OCC#3608
Test plan
uv run pytest tests/ci/test_runner_image_identity.py tests/ci/test_validate_runner_image.py tests/ci/test_runner_image_node24_floor.py -v— 36 passeduv run pytest tests/integration/infra/test_omn_12765_release_backmerge_identity.py -v— 2 passeduv run pytest tests/ -n auto— 24808 passed / 19 failed / 78 errors, all pre-existing environment-dependent (live Kafka/Postgres/Infisical/runtime container) failures unrelated to this changeuv run mypy src/omnibase_infra/— no issuespre-commit run --files docker/runners/Dockerfile docker/runners/runner-image.lock.json— all applicable hooks passSummary by CodeRabbit