Skip to content

ci: backport reliability fixes to v5 without changing runner images - #5185

Open
infraclaw-dash wants to merge 4 commits into
dashpay:v6.0-devfrom
infraclaw-dash:ci/smooth-ci-rollout-v50-20260929
Open

infraclaw-dash wants to merge 4 commits into
dashpay:v6.0-devfrom
infraclaw-dash:ci/smooth-ci-rollout-v50-20260929

Conversation

@infraclaw-dash

@infraclaw-dash infraclaw-dash commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Issue being fixed or feature implemented

infraclaw: carry the four already-landed v4.2 CI reliability fixes onto v5.0-dev, without changing contributors' branches, application code, test expectations, or the ordinary runner image currently required by v5.

What was done?

CI-only backports of #5165, #5171, #5174 and #5168:

  • Treat an optional S3 cache-export outage as optional; build, tests and image publication remain fatal on failure.
  • Pin both trusted publisher references to the merged, reviewed exact-contract reuse controller 1be03edb7f4f18548d525b17dfac6ee31db17c99.
  • Preserve current legacy runner labels; require distinct exact-contract labels if a later reviewed change adopts a new AMD64 image. No image rollout is performed here.
  • Recheck that the PR is still open and its head unchanged immediately before returning a candidate label.

Both requirements manifests, Rust setup, Rust/Kotlin/application workflows and all non-.github content remain unchanged from ad7874352c2a191ca25b6b87be12c487696758f5. Existing fork admission, immutable image provenance and cache-writer restrictions remain intact. This is not a full development-branch merge and does not include the new recipe from #5172.

How Has This Been Tested?

  • All 49 focused CI Python tests pass on the combined v5 tree.
  • git diff --check passes.
  • Verified both requirements manifests and consuming application workflow blobs are identical to the exact v5 base.
  • The four source PRs are already merged on v4.2; attributable checks for this target and exact head are still required. Local tests are not a claim of live image reuse or worker rollout.

Contributor impact

No author needs to change application code for these CI fixes. No contributor PR will be pushed, rebased, retargeted or mass-rerun by this change. Existing runner/image requirements stay the same. If an existing run used an older workflow, explain the specific failure and let its author update from the repaired target and trigger a fresh run when appropriate; blindly rerunning a pinned old workflow is not a repair.

Breaking Changes

None. No application changes, image requirement change, release publication, trust expansion or protection bypass.

Checklist:

  • I have performed a self-review of my own code
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding documentation changes where needed
  • Required independent reviews and attributable target checks complete

Summary by CodeRabbit

  • Bug Fixes
    • Temporary cache upload failures no longer interrupt builds. Build and image publication failures still stop the workflow.
    • Runner selection now checks that a pull request is still open and on the same revision before assigning a candidate runner.
  • Updates
    • AMD64 jobs now select compatible runner capacity based on the required image version. Jobs wait when matching capacity is unavailable.

@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: dashpay/platform/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 073066d4-d2b0-41dd-a97e-6fd9cced4eab

📥 Commits

Reviewing files that changed from the base of the PR and between ad78743 and d8b1d04.

📒 Files selected for processing (9)
  • .github/ORDINARY_RUNNER_IMAGES.md
  • .github/actions/s3-layer-cache-settings/action.yaml
  • .github/scripts/runner-image.py
  • .github/scripts/tests/fixtures/legacy-runner-requirements.json
  • .github/scripts/tests/test_candidate_controller_pin.py
  • .github/scripts/tests/test_ordinary_image_routing.py
  • .github/scripts/tests/test_runner_image.py
  • .github/scripts/tests/test_s3_layer_cache.py
  • .github/workflows/runner-image-candidate.yml

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The pull request updates ordinary AMD64 runner labels and candidate selection checks, documents the runner-image rollout, and configures S3 layer-cache exports to tolerate export errors.

Changes

Runner image selection

Layer / File(s) Summary
Ordinary runner label routing
.github/scripts/runner-image.py, .github/scripts/tests/fixtures/legacy-runner-requirements.json, .github/scripts/tests/test_ordinary_image_routing.py, .github/scripts/tests/test_runner_image.py
Ordinary AMD64 selection uses fingerprinted labels except for the designated legacy manifest. Tests cover manifest variants, architecture selection, key-order stability, and architecture-contract comparison.
Candidate selection freshness
.github/scripts/runner-image.py, .github/scripts/tests/test_runner_image.py, .github/workflows/runner-image-candidate.yml, .github/scripts/tests/test_candidate_controller_pin.py
After candidate publication, selection verifies that the pull request remains open at the original head SHA. Tests cover stale or failed checks, and the workflow and test pin the candidate controller revision.
Ordinary runner rollout
.github/ORDINARY_RUNNER_IMAGES.md
The documentation describes AMD64 label requirements and rollout steps, including retaining old capacity and verifying new capacity before merging the recipe change.

S3 layer-cache export

Layer / File(s) Summary
Cache export error handling
.github/actions/s3-layer-cache-settings/action.yaml, .github/scripts/tests/test_s3_layer_cache.py
Cache exports now set ignore-error=true. Tests check export and import settings, shared-write opt-in, and that build and registry publication failures are not ignored.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant RunnerImage as runner-image.py
  participant Publisher as Candidate publisher
  participant PullRequests as Pull request API
  RunnerImage->>Publisher: Publish candidate
  Publisher-->>RunnerImage: Publication succeeds
  RunnerImage->>PullRequests: Fetch pull request state and head SHA
  PullRequests-->>RunnerImage: Return open state and head SHA
  RunnerImage-->>RunnerImage: Select candidate label only when state and SHA match
Loading

Suggested reviewers: thepastaclaw

Merge Risk: ⚪ Minimal · up to d8b1d

No actionable merge-blocking issue was found. The ordinary runner jobs retain their existing labels; merge remains subject to the stated rollout hold and exact-head checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to d8b1d

The current runner image keeps its existing labels, while future images require distinct labels. Candidate selection adds a freshness check, and cache-export failures are configured separately from image publication. No introduced security issue is established, but publisher, runner-fleet, and cache failure behavior need live verification.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The changed cache setting can affect builds that receive the repository's S3 cache credentials, while the changed ordinary labels affect CI jobs using the selector. The available source does not establish S3-side authorization or the runner fleet's actual label-to-image binding.

Trust Boundaries and Controls

  • observed — The selector treats the PR head and candidate status as inputs requiring API freshness and publisher checks. The caller pins its privileged reusable publisher, but that external implementation's head binding and publication-completion guarantees are not established here.
  • observed — Shared name-manifest cache writes remain opt-in, while cache imports include SHA, head-ref, and name manifests. The new option applies to export; the available source does not prove what a later import does after an interrupted export.

Resilience and Maintainability Implications

  • inferred — Deterministic versioned labels and retaining old capacity support failure containment during a future rollout. The actual registration, interruption recovery, and rollback behavior are outside the demonstrated repository implementation.

Hardening Proposals

  • proposed — Before relying on the new cache failure policy, verify the deployed BuildKit version's partial-export and subsequent-import behavior under interruption, including recovery and repeated writes.
  • proposed — Before adopting new runner capacity, verify the pinned publisher's PR-head and publication-completion guarantees and exercise label registration and rollback against real capacity.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 3.13% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 32 functions across 5 files. (4 skipped: 4… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: backporting CI reliability fixes to v5 while preserving the runner images currently required by v5.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 3.13% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 32 functions across 5 files. (4 skipped: 4 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added this to the v5.0.0 milestone Sep 29, 2026
@github-actions github-actions Bot added the waiting-bots Waiting for the review bots to report on this head label Sep 29, 2026
@thepastaclaw

thepastaclaw commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

✅ Final review complete — no blockers (commit d8b1d04) · triage: normal

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Final validation — Phase 1 + Phase 2

The CI-only backport is consistent with its stated goal: cache-export failures are isolated, trusted publisher references are pinned, ordinary AMD64 contracts are separated from legacy labels, and candidate selection rechecks PR freshness. No concrete in-scope correctness, security, architecture, or test issue was identified in the exact reviewed head.

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: architecture-layering); reviewer 3: muse-spark-1.3-contributor (agent: phase1-reviewer, role: security-auditor); reviewer 4: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 5: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 6: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6-astra (agent: astra-verifier, role: final-verifier)

  • Triage: normal by gpt-6-astra (effort low) — The diff changes multiple interacting CI behaviors involving cache failure handling, controller pinning, runner-label routing, and candidate freshness checks, warranting ordinary review but touching none of the defined critical surfaces.
  • Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — architecture-layering (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — security-auditor (completed, effort xhigh); agent phase1-reviewer
  • Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 13% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
  • Fresh verifier: gpt-6-astra — final-verifier; agent astra-verifier
  • Phase 2 reviewers: gpt-6-astra — general (completed, effort high); agent phase2-reviewer, gpt-6-astra — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6-astra — security-auditor (completed, effort high); agent phase2-reviewer

@ktechmidas

Copy link
Copy Markdown
Collaborator

@coderabbitai review

infraclaw requesting one ordinary included review of exact head d8b1d0419024dac33429aab7c9626c34ce35e9ce, after rereading the live allowance receipts. Thepastaclaw completed an independent exact-head review at 11:35 UTC with no in-scope findings. Hosted contract and Tests workflows are successful; intentional fork/self-hosted skips are not worker-capacity proof.

This is the existing CI-only v5 backport: optional cache-export error tolerance, immutable controller adoption, contract-specific ordinary labels, and final PR-head recheck. Application workflows and manifests are unchanged. This is not a priority or premium request. The existing rollout hold remains: no merge, replacement PR, contributor-branch change, or claim of completed target adoption is authorized by this review request. Account attribution reflects the explicitly authorized protected transport, not latte's personal review or approval.

@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@github-actions github-actions Bot added ready-for-human Bots have reported, the author has self-reviewed, and the build is green: this needs a human. and removed waiting-bots Waiting for the review bots to report on this head labels Sep 29, 2026
@github-actions github-actions Bot removed the ready-for-human Bots have reported, the author has self-reviewed, and the build is green: this needs a human. label Sep 30, 2026

This branch has not been deployed

No deployments
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.

3 participants