Skip to content

fix: validate runner candidates using full commit statuses - #5156

Merged
ktechmidas merged 1 commit into
dashpay:v4.2-devfrom
infraclaw-dash:fix/runner-status-provenance-20260928
Sep 28, 2026
Merged

ktechmidas merged 1 commit into
dashpay:v4.2-devfrom
infraclaw-dash:fix/runner-status-provenance-20260928

Conversation

@infraclaw-dash

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

Copy link
Copy Markdown
Contributor

Issue being fixed or feature implemented

PR #5151 cannot select its successfully published runner image. The Kotlin selector job fails with Candidate status must come from the trusted publisher; the Rust and NPM selectors fail identically. The candidate publisher completed successfully.

The selector reads GitHub's combined /commits/{head}/status response, whose individual status objects omit creator, then requires creator.login == github-actions[bot]. The full /commits/{head}/statuses response includes that provenance. The old tests incorrectly manufactured the missing field in the combined response.

What was done?

  • Read the full commit-status history for the exact PR head, with explicit 100-entry pagination.
  • Select the newest matching context before validation, relying on GitHub's newest-first ordering. Stop pagination as soon as it is found; never fall back to an older success when the latest matching status is pending, failed, untrusted, or invalid.
  • Match GitHub's case-insensitive context identity while retaining the exact canonical publisher-context requirement. A newer differently-cased status cannot expose an older canonical success.
  • Preserve creator, immutable-digest, publishing-run URL, workflow/event/conclusion, current PR-head and manifest checks. Missing/null/malformed creator objects fail closed with the existing readable publisher error.
  • Replace inaccurate API mocks with a small projection of the public PR chore!: merge v4.2-dev into v4.3-dev #5151 responses: head a02b1460736e18b6345bb4722c622e55e787371d, status 55116170875, publisher run 36474975257. The combined status genuinely lacks creator; the full status identifies github-actions[bot] (ID 41898282).
  • Add regression tests for Kotlin/Rust/NPM success, status-history precedence, case variants, pagination/exhaustion, API errors, rejected provenance/digests/URLs/workflows, and existing pending-status/publisher wait behavior.

Companion controller promotion fix: dashpay/dash-selfhosted-image#8.

Integration / adoption

This PR targets the current v4.2-dev branch. Its selector fix must be included when refreshing the v4.2-dev → v4.3-dev merge PR #5151; rerunning #5151's unchanged head will still execute the old selector.

The controller pin remains aaea7df12716c386db223ea67a853b6b0efbd45a. Do not replace it with the standalone controller #8 head: that head does not include the separate legacy-template compatibility repair in controller #7. After review/merge, the controller pin should advance to a revision containing both repairs. This PR changes no workflow permissions, image manifest, recipe, live runner, or controller pin.

How Has This Been Tested?

  • python3 -m unittest discover -s .github/scripts/tests -p 'test_runner_image.py' -v — 27 passed.
  • python3 -m unittest discover -s .github/scripts/tests -v — 33 passed, including release-boundary tests.
  • Python compilation and git diff --check — passed.
  • Independent code review of the implementation and regression fixtures — no blocking findings.
  • Read-only live API replay against exact PR chore!: merge v4.2-dev into v4.3-dev #5151 head/status/publisher: the original selector reproduces the publisher error for all three kinds; the repaired selector selects the exact head/digest-specific Kotlin, Rust and NPM labels. Captured fixture fields match the live responses. This verifies selection, not execution of application builds or runner provisioning.

Normal PR CI remains separate from the local evidence above. No manual workflow dispatch, runner rollout, image publication, or merge was performed.

Breaking Changes

None. CI selection repair only; no application, dependency, consensus, or protocol changes.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed
  • If I added or changed GroveDB structure, I described it in the area's structure.rs, regenerated grovedb-structure.json, and checked the structure viewer link posted on this pull request

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

Summary by CodeRabbit

  • Bug Fixes
    • Runner image candidate checks now use the newest matching status and require an exact publisher-context match. This prevents an older successful status from overriding a newer result and improves validation consistency across supported image types.

PR Hygiene · 217167d

  • Bots — coderabbitai not yet · thepastaclaw not yet — /skip-bots proceeds without the ones not yet reported
  • Self-review — not asked of a bot author
  • Within your 5 open PRs
  • Build running
  • Approvals
    • files with no dedicated owner (.github/scripts/runner-image.py, .github/scripts/tests/fixtures/candidate-status-pr5151.json, .github/scripts/tests/test_runner_image.py) — QuantumExplorer or shumkov

When every box is checked the PR Hygiene check passes and this can merge.

@coderabbitai

coderabbitai Bot commented Sep 28, 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: f7e25b25-789f-4ff6-ab89-b1199b852390

📥 Commits

Reviewing files that changed from the base of the PR and between c795f81 and 217167d.

📒 Files selected for processing (3)
  • .github/scripts/runner-image.py
  • .github/scripts/tests/fixtures/candidate-status-pr5151.json
  • .github/scripts/tests/test_runner_image.py

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 runner-image selector now paginates commit statuses, selects the newest matching context, and validates successful candidates against publisher-run details. Tests cover status ordering and pagination, candidate validation, retries, runner labels, and ARM64 selection.

Changes

Runner-image status selection

Layer / File(s) Summary
Find the newest matching status
.github/scripts/runner-image.py, .github/scripts/tests/test_runner_image.py
A new helper scans status pages and returns the first case-insensitive context match. Tests check that a newer matching status takes precedence and that pagination stops at a match or exhausts without one.
Validate and select candidates
.github/scripts/runner-image.py, .github/scripts/tests/fixtures/candidate-status-pr5151.json, .github/scripts/tests/test_runner_image.py
Candidate polling uses the status helper and requires an exact context match for a successful candidate. Tests cover publisher-run validation, invalid candidate details, retries, candidate labels, and ARM64 selection.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: quantumexplorer, pastapastapasta

Merge Risk: ⚪ Minimal · up to 21716

The selector change has no identified merge-blocking issue; it is ready for normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 21716

The revised selection path retains the publisher and image checks, and no new bypass was established. Some integration and security coverage remains unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The affected decision is which published image-backed, self-hosted runner labels are emitted for PR jobs using the selector; it does not itself change runner provisioning.

Trust Boundaries and Controls

  • observed — A status returned by GitHub is not sufficient to select a candidate: the selector checks PR-head identity, status provenance, the immutable digest, and the publishing workflow before emitting candidate labels. A newer case-variant context cannot expose an older canonical success.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 30 functions across 2 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: validating runner candidates using full commit statuses.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 30 functions across 2 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
🧪 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.

@thepastaclaw

thepastaclaw commented Sep 28, 2026 •

Copy link
Copy Markdown
Collaborator

🕓 Queued for automated review — 5th in line, estimated start in ~25 min (commit 217167d)
Estimated review time once started: ~55 min (two-phase automated review; median of recent runs).

  • Request priority review — click to move this review to the front of the queue.

@infraclaw-dash infraclaw-dash changed the title fix(ci): validate runner candidates using full commit statuses fix: validate runner candidates using full commit statuses Sep 28, 2026
@github-actions github-actions Bot added the waiting-bots Waiting for the review bots to report on this head label Sep 28, 2026
@ktechmidas
ktechmidas merged commit b81ed54 into dashpay:v4.2-dev Sep 28, 2026
29 of 32 checks passed
PastaPastaPasta added a commit that referenced this pull request Sep 28, 2026
Pick up #5156 (validate runner candidates using full commit statuses),
#5153 and #5147.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting-bots Waiting for the review bots to report on this head

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants