Skip to content

ci: bound mini execution with parallel hosted macOS fallback - #1126

Merged
XuPeng-SH merged 11 commits into
matrixorigin:mainfrom
loveRhythm1990:ci/macos-self-hosted-migration
Oct 24, 2026
Merged

XuPeng-SH merged 11 commits into
matrixorigin:mainfrom
loveRhythm1990:ci/macos-self-hosted-migration

Conversation

@loveRhythm1990

@loveRhythm1990 loveRhythm1990 commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Capability assessment: Unchanged: CI scheduling and test infrastructure change; no product capability status or runtime profile is added. The dependency audit fix preserves the existing JWT algorithms.

Summary

Give the single self-hosted ARM Mac a bounded opportunity to run macOS tests, with real parallel hosted execution when it cannot serve the run. The explicit allowlist contains the eleven selected Astra contributor account IDs, including Dependabot. Trusted main/develop pushes use hosted macOS; release platforms remain unchanged. Only allowlisted PRs can use the persistent mini; other or unknown PR authors use hosted macOS directly.

Five Mac lanes share one candidate checkout, toolchain setup and cache restore on the mini: default reader/Bash, reflow, workspace coordination, vendored units and dev-tty. Their commands have one owner, scripts/ci/run-macos-lanes.sh, used by both mini and hosted/Linux execution. Cargo invocations stay separate, with dev-tty last.

Scheduling and required results

Situation Execution and required result
Eligible first attempt, mini serves it One mini bundle; success requires its actual test step to pass
Mini queue/discovery exceeds 180 seconds Cancel the owned child and run hosted Mac lanes in parallel
Proven checkout/admission/toolchain/low-disk failure Hosted parallel fallback
Mini disappears or exceeds its 60-minute execution budget Hosted parallel fallback; worker has a 65-minute hard backstop
Actual compilation/test failure Failure, without hosted retry
Failed worker with incomplete test metadata Preserve failure rather than hiding a possible test failure
Mac job is re-executed on attempt >1 Hosted execution even if old planner outputs are reused
Missing controller, API error or missing routing output Hosted tests execute; absence is never a successful skip
Source cancelled/superseded Cancel only the worker bound to that source run and attempt
Unaffected paths Existing legitimate scope skips remain

Linux execution depends only on scope, so it starts independently of the mini wait. The original GitHub Actions required-check names remain in Test Suite. The Mac jobs on Ubuntu require a verified mini success; on fallback they run the real tests on macos-15. There are no custom App contexts or new branch-protection bindings.

Implementation and review invariants

  • macos-scheduler.yml is a default-branch workflow_run controller using the built-in token's Actions write permission. It only dispatches/cancels workers and never executes candidate code.
  • macos-mini.yml uses a read-only token and cache-mode: read. It rejects an expired source gate or a worker rerun before candidate checkout.
  • Source identity is the exact Test Suite run ID, attempt and recorded base/head/merge tuple. Merge parents are verified. Fork API responses with pull_requests: [] work without an organization-membership query.
  • The controller/admission check the actual wait step, not merely an in_progress job that may already be doing post-job cleanup.
  • Source completion retries orphan cleanup, including after PR closure or attempt changes. Cancellation failure for one duplicate does not suppress cleanup of the others.
  • Active execution polls once per minute and filters worker history by source creation time; the one-page, one-hour fixture stays below 500 combined REST calls. API quota failures select hosted execution.
  • Rust bootstrap exports the selected repository toolchain through GITHUB_ENV, so sccache's compiler probe works outside the checkout even when rustup has no default. Dependency-audit updates and PTY fixture stabilization remain. JWT uses AWS LC to remove the unpatched crypto-bigint path; HMAC and 2048-bit RSA/JWK interoperability/rejection cases were previously checked. AWS LC's RSA key range is 2048–8192 bits.

Latest review fixes

  • P1: isolate trusted pushes: only PRs can enter the mini route. Main/develop pushes remain hosted, and the worker admission rejects push sources. PR HOME/Cargo/rustup state is not reused by these trusted push jobs.
  • P2: preserve the toolchain for sccache: selected-toolchain environment propagation is exercised from bootstrap through the production sccache health script, with a negative control.
  • The API run-name compatibility fix also has a regression test using the actual response shape.

Bootstrap and operational limits

The two new workflows must enter main before GitHub will trigger the controller. Before that, this PR exercises hosted fallback. No App installation, secret, migration flag or custom admission hook is needed. The runner/group must allow macos-mini.yml@main with labels [self-hosted, macOS, ARM64, mac-arm64-mini].

The 180-second budget includes polling/discovery, with separately bounded API requests. Service/API outages may prevent immediate child cancellation; the parent still takes hosted fallback and the completion observer retries cleanup. Native service availability and queueing still affect elapsed time.

One registration runs one bundle at a time. Bundling removes repeated preparation, not the physical capacity limit. On preceding head b7efd37a, the five individual mini jobs all passed and occupied about 25m34s from first creation to last completion. That is a baseline, not a measurement of this new bundle/cache mode. Queue delay, fallback rate, resource headroom and net hosted-minute savings still need a post-merge pilot.

If old controller configuration was installed, retire its unused App required contexts/admission hook while preserving the existing GitHub Actions checks and fork approvals. The administrator handbook is maintained separately.

Validation and review status

  • python3 -m unittest discover -s scripts/ci: 192 tests, 0 failures/errors, 6 conditional skips.
  • Actionlint 1.7.12, repository metadata validation and git diff --check: passed.
  • The three actual native mini-result guards and aggregate failure/skip behavior are exercised. Dispatch JSON transport, queue/execution deadlines, preflight versus test failure, source isolation, empty fork associations, cancellation, post-job races and reused attempts have regression coverage.
  • Host self-review covered 19/19 changed files, including the seven OCR-excluded entries; no independent reviewer was invoked.
  • A real fork run was validated with the production source function: workflow path, dynamic run-name, author ID, empty PR association and exact merge parents. Workflow identity uses the API path because GitHub can also put the dynamic run-name in name.
  • Real sccache 0.18.0 plus isolated rustup with no default reproduced disabled caching without the exported selection; the corrected setup enabled the same out-of-checkout probe. This used the local cache backend, not GitHub's cache service.
  • Full controller-to-worker dispatch/admission/cache behavior remains a post-merge live acceptance item.
  • At b7efd37a, all five mini lanes and static checks passed. The live harness was 17/18: flash_child_question_parent_answer failed with BehaviorContractViolation. This remains a failing validation; no causal attribution or assertion relaxation is claimed.
  • On 58b1c7fa, Test Suite reached hosted fallback and all five Mac jobs started in parallel within one second of each other. The full live CI is still running. Changes requested reviews are not treated as approval.

Refs #1106.

@loveRhythm1990 loveRhythm1990 changed the title ci: migrate trusted macOS lanes to self-hosted ARM runners ci: migrate trusted macOS lanes and fix dependency audit Oct 23, 2026
@loveRhythm1990
loveRhythm1990 marked this pull request as ready for review October 23, 2026 15:49

@XuPeng-SH XuPeng-SH 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.

Request changes: two actionable integration defects, detailed in the inline comments.

Independent one-off review of the entire PR (all 16 changed files and all four commits), against head 8305e835dd76cea2fe2e7d13dca99eb6b8f5a908 and base 8cbabcce326e4cdb8f1f84cf5f5a37964ae242fd. I revalidated the open/non-draft PR, authenticated reviewer XuPeng-SH (separate from the author), unchanged head, and empty existing reviews/inline comments immediately before submission. The current synthetic merge 5ba28ffd847164f73ad2c6c4d82b6f282df229dd has exactly the ordered base/head parents.

The two blockers are (1) hosted individual/failed-only reruns can execute with reused admission, bypassing the attempt/expiry enforcement, and (2) real fork run responses with an empty PR association cannot complete the ordinary App-owned checks. These are implementation/integration defects in the disabled-by-default migration; missing live activation acceptance is not itself a finding.

I traced routing, App/check source binding, source refresh, runner-group admission and root-owned helper boundaries, privileged execution/cache separation, fallback, publication, expiry and recovery. The canonical classifier and extracted lane script preserve the useful existing coverage and avoid duplicating lane commands. The substantial controller complexity addresses concrete trust/recovery requirements, but the passing policy-unit tests do not establish the two workflow/API integrations above. I found no additional actionable issue in the ctutils-only lockfile update: the 0.4.4 registry archive SHA-256 matches Cargo.lock, and the current RustSec advisory lists >=0.4.4 as patched (https://github.com/RustSec/advisory-db/blob/main/crates/ctutils/RUSTSEC-2026-0385.md).

Verification:

  • Migration suite: 45/45 passed. CI contract suites: 50/50 passed.
  • Two additional isolated offline reproductions confirmed the findings; no remote CI was rerun.
  • Actionlint 1.7.12 with the repository's narrow compatibility filters, repository metadata validation and diff whitespace checks passed.
  • Full local CI Python suite: 199 tests, four failures. One unchanged build-metadata fixture attempted to use the sandbox's read-only default Cargo directory. Three harness process/cancellation tests also reproduce on the unchanged base. These are disclosed limits, not attributed to this PR.
  • Full locked offline Cargo resolution was unavailable because the saved registry index lacks locked rustls 0.23.46. Full Rust/macOS execution, fresh cargo-audit, runner installation, actual App-token/cache enforcement, host isolation and capacity qualification were not performed.

Change accounting (additions/deletions/net, no pure file moves): implementation/configuration/dependency 1107/62/+1045; tests 692/30/+662; documentation 318/0/+318.

Current exact-head CI: all 41 checks were inspected (complete single page): 38 success, one skipped, one neutral, one failure. Sources are GitHub Actions App 15368 and Mergify App 10562. The failed live harness is still 16/18: flash_child_question_parent_answer timed out with child-count/adoption mismatch; work_two_step_execution_journey failed journal assertions despite judge acceptance. I read the current job logs; neither flakiness nor PR causation is established (https://github.com/matrixorigin/Astra/actions/runs/39981298500/job/120508972896).

The accessible ruleset list was fully enumerated and both ruleset details inspected. Effective target-rule lookup is unavailable through this connector, and I did not retry or bypass the previously denied classic branch-protection administration endpoint. Required-context/source-binding and protection configuration therefore remain unknown; this review does not certify merge gates or claim all required CI passed. Migration remains an opt-in implementation with separate live administrative/runner/capacity acceptance.

This was a manual one-off review. No automatic review loop was restarted, and no activation, production/admin change, branch push/synchronization, deployment or direct merge was performed.

Comment thread .github/workflows/macos-trusted.yml Outdated
Comment thread scripts/ci/macos_runner.py Outdated
@loveRhythm1990
loveRhythm1990 force-pushed the ci/macos-self-hosted-migration branch from 8305e83 to 50466af Compare October 24, 2026 02:31
@loveRhythm1990 loveRhythm1990 changed the title ci: migrate trusted macOS lanes and fix dependency audit ci: migrate trusted macOS lanes to self-hosted ARM runners Oct 24, 2026
@loveRhythm1990 loveRhythm1990 changed the title ci: migrate trusted macOS lanes to self-hosted ARM runners ci: migrate trusted macOS lanes and fix dependency audit Oct 24, 2026
@loveRhythm1990
loveRhythm1990 force-pushed the ci/macos-self-hosted-migration branch from e0ad690 to 80ff991 Compare October 24, 2026 08:01

@XuPeng-SH XuPeng-SH 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.

Request changes: the two prior findings (hosted rerun admission and empty fork PR associations) appear fixed, but I found a separate result-ownership defect for different PRs that share one head SHA. The ordinary and protected paths write the same three App check names against that SHA. I reproduced an external PR's successful ordinary result replacing a member PR's failed protected results; reconciliation skips the member PR because the latest plan names the other PR, and the older publisher cannot restore its result. The required checks therefore no longer prove that the member PR's protected path passed. See the inline P1 for the concrete trigger and requested regression coverage.

Validation at this head: all 51 migration tests pass; the 205-test Python suite passes with 6 existing conditional skips. This review does not certify live App/runner activation or capacity.

Comment thread scripts/ci/macos_runner.py Outdated
@loveRhythm1990 loveRhythm1990 changed the title ci: migrate trusted macOS lanes and fix dependency audit ci: run macOS tests on self-hosted ARM runners Oct 24, 2026
@loveRhythm1990 loveRhythm1990 changed the title ci: run macOS tests on self-hosted ARM runners ci: migrate macOS tests with a contributor allowlist Oct 24, 2026
@loveRhythm1990
loveRhythm1990 marked this pull request as draft October 24, 2026 09:05

@XuPeng-SH XuPeng-SH 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.

Request changes: the same persistent self-hosted runner is shared by allowlisted pull requests and trusted main/develop pushes, allowing pre-merge PR code to contaminate later trusted CI. See the P1 inline finding. I have not included the earlier allowlist-modification finding as a blocker.

The Rust toolchain change also disables sccache on the current macOS lanes because the setup installs rustup with no default toolchain, while the sccache probe invokes rustc without an explicit toolchain. This is a non-blocking build-time regression; see the P2 inline finding.

Validation at this head: CI Python suite (160 tests; 6 skipped) and actionlint passed. Exact-head GitHub checks currently have one unrelated live harness failure; I did not attribute it to this PR.

Comment thread scripts/ci/ci_scope.py
Comment thread .github/actions/rust-toolchain/action.yml
@loveRhythm1990 loveRhythm1990 changed the title ci: migrate macOS tests with a contributor allowlist ci: bound mini execution with parallel hosted macOS fallback Oct 24, 2026
@loveRhythm1990
loveRhythm1990 marked this pull request as ready for review October 24, 2026 09:43

@XuPeng-SH XuPeng-SH 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.

Reviewed the complete PR at 58b1c7fab476c098a9f31e1083672ae64a3d7f03 against main.

The previously reported shared trusted-runner state issue is addressed: only eligible PRs use the mini, while main/develop pushes use hosted macOS. The rustup/sccache toolchain propagation is now covered through the production health probe. The earlier App-specific result ownership/rerun findings no longer apply to this simplified ordinary-workflow path.

No new blocking code finding. Exact-head checks currently show 41 successes and one neutral Mergify check. The current Test Suite run selected the real hosted fallback after the mini workflow lookup returned HTTP 404 because the new worker/controller workflows are not on the default branch yet. The mini dispatch/admission path therefore still needs the documented post-merge pilot, including runner-group access and queue/cancellation behavior.

@mergify

mergify Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again.

@XuPeng-SH
XuPeng-SH merged commit ed4218c into matrixorigin:main Oct 24, 2026
42 of 43 checks passed
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