ci: cut CI time with trusted routing and Fireactions caches - #2926
ci: cut CI time with trusted routing and Fireactions caches#2926UnArbosFive wants to merge 1 commit into
Conversation
UnArbosFive
added
the
fireactions-cache-benchmark
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🛡️ AI Review — Skeptic (security review)VERDICT: SAFE VERY HIGH account-age scrutiny, mitigated by admin permission and substantive merged contributions; no known Gittensor association; codex/fireactions-host-cache-pilot → main. Trusted AI-review instructions are unchanged. Static analysis found no malicious behavior or security vulnerability in the current diff. FindingsNo findings. ConclusionThe previously addressed E2E bootstrap concern remains resolved, and no new security issues were found. 🔍 AI Review — Auditor (domain review)VERDICT: 👍 Gittensor association UNKNOWN; the recently created account has admin repository permission and substantive merged CI contributions. The implementation remains consistent with the substantive PR description. Trusted-base classifiers fail closed, required check contexts are preserved, and cache/artifact reuse validates provenance and exact revisions. The overlapping PRs cover unrelated PyPI, TypeScript SDK, and governance work; none is a duplicate. This CI-only PR carries FindingsNo findings. ConclusionThe CI optimization retains asserted coverage and adds focused validation for routing, sharding, artifacts, and caches. I found no substantive domain issue blocking merge. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| payload="$RUNNER_TEMP/artifacts.json" | ||
| for attempt in 1 2 3 4 5; do | ||
| gh api "repos/$GITHUB_REPOSITORY/actions/runs/$PRODUCER_RUN_ID/artifacts?per_page=100" > "$payload" | ||
| count=$(jq '[.artifacts[] | select(.name == "mainnet-snapshot" or (.name | startswith("try-runtime-snap-v0.10.1-")))] | length' "$payload") |
There was a problem hiding this comment.
[LOW] Count only artifacts the warming loop can process
This readiness count includes expired artifacts, but the processing query below excludes them. If a run contains only expired matching artifacts, the retry loop exits and the job succeeds after warming nothing. Apply the same expiry filter here so maintenance fails or retries instead of reporting a false success.
| count=$(jq '[.artifacts[] | select(.name == "mainnet-snapshot" or (.name | startswith("try-runtime-snap-v0.10.1-")))] | length' "$payload") | |
| count=$(jq '[.artifacts[] | select(.expired == false) | select(.name == "mainnet-snapshot" or (.name | startswith("try-runtime-snap-v0.10.1-")))] | length' "$payload") |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
|
Benchmark results on the standard 8 vCPU / 28 GiB turbo runner shape:
Exact remote compiler keys refresh every six hours. Scheduled clone snapshots are produced daily with a 36-hour freshness gate and now publish an immutable R2 mirror manifest for per-host polling. There is no random-runner local maintenance job because one selected VM cannot warm a shared-label fleet. Runs: exact-key verification https://github.com/RaoFoundation/subtensor/actions/runs/29456620982 ; paired/artifact benchmark https://github.com/RaoFoundation/subtensor/actions/runs/29456911456 ; cold + warm PR attempts https://github.com/RaoFoundation/subtensor/actions/runs/29456621901 |
| payload="$RUNNER_TEMP/artifacts.json" | ||
| for attempt in 1 2 3 4 5; do | ||
| gh api "repos/$GITHUB_REPOSITORY/actions/runs/$PRODUCER_RUN_ID/artifacts?per_page=100" > "$payload" | ||
| count=$(jq '[.artifacts[] | select(.name == "mainnet-snapshot" or (.name | startswith("try-runtime-snap-v0.10.1-")))] | length' "$payload") |
There was a problem hiding this comment.
[LOW] Count only artifacts the warming loop can process
This count includes expired artifacts and any name with the try-runtime prefix. Such an artifact can end polling even though the loop skips it or download-artifact.sh rejects its name, producing misleading maintenance results. Match the downloader’s exact allowlist and the loop’s expiry filter.
| count=$(jq '[.artifacts[] | select(.name == "mainnet-snapshot" or (.name | startswith("try-runtime-snap-v0.10.1-")))] | length' "$payload") | |
| count=$(jq '[.artifacts[] | select(.expired == false) | select(.name == "mainnet-snapshot" or .name == "try-runtime-snap-v0.10.1-mainnet" or .name == "try-runtime-snap-v0.10.1-testnet" or .name == "try-runtime-snap-v0.10.1-devnet")] | length' "$payload") |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
|
Final validation note: a subsequent run reached 597 Rust hits / 0 misses, which exposed that sccache omits the language-specific miss row at zero. The gate initially parsed that omission as empty and failed despite a perfect cache result. Commit e339b14 normalizes omitted counters to zero; the final validation is green with 596 hits / 1 miss on the fill pass and 597 hits / 0 misses (100%) after the second cargo clean. Final validation: https://github.com/RaoFoundation/subtensor/actions/runs/29458291945 |
| payload="$RUNNER_TEMP/artifacts.json" | ||
| for attempt in 1 2 3 4 5; do | ||
| gh api "repos/$GITHUB_REPOSITORY/actions/runs/$PRODUCER_RUN_ID/artifacts?per_page=100" > "$payload" | ||
| count=$(jq '[.artifacts[] | select(.name == "mainnet-snapshot" or (.name | startswith("try-runtime-snap-v0.10.1-")))] | length' "$payload") |
There was a problem hiding this comment.
[LOW] Count only artifacts the warming loop can process
This readiness check counts expired artifacts and any name beginning with try-runtime-snap-v0.10.1-, while the loop excludes expired entries and download-artifact.sh accepts only three exact network names. An unusable artifact can therefore stop polling before a usable one exists. Apply the processing loop/downloader’s expiry and exact-name filters here.
| count=$(jq '[.artifacts[] | select(.name == "mainnet-snapshot" or (.name | startswith("try-runtime-snap-v0.10.1-")))] | length' "$payload") | |
| count=$(jq '[.artifacts[] | select(.expired == false) | select(.name == "mainnet-snapshot" or .name == "try-runtime-snap-v0.10.1-mainnet" or .name == "try-runtime-snap-v0.10.1-testnet" or .name == "try-runtime-snap-v0.10.1-devnet")] | length' "$payload") |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
UnArbosFive
removed
the
fireactions-cache-benchmark
UnArbosFive
changed the title
| payload="$RUNNER_TEMP/artifacts.json" | ||
| for attempt in 1 2 3 4 5; do | ||
| gh api "repos/$GITHUB_REPOSITORY/actions/runs/$PRODUCER_RUN_ID/artifacts?per_page=100" > "$payload" | ||
| count=$(jq '[.artifacts[] | select(.name == "mainnet-snapshot" or (.name | startswith("try-runtime-snap-v0.10.1-")))] | length' "$payload") |
There was a problem hiding this comment.
[LOW] Count only artifacts the warming loop can process
This count still includes expired artifacts and any name beginning with the try-runtime prefix, while the loop excludes expired entries and download-artifact.sh accepts only three exact try-runtime names. An expired or similarly prefixed artifact can make the retry loop stop despite having nothing usable, or make the warming step fail. Apply the same expired == false and exact-name allowlist predicates here and in the selection at line 69.
| count=$(jq '[.artifacts[] | select(.name == "mainnet-snapshot" or (.name | startswith("try-runtime-snap-v0.10.1-")))] | length' "$payload") | |
| count=$(jq '[.artifacts[] | select(.expired == false) | select(.name == "mainnet-snapshot" or .name == "try-runtime-snap-v0.10.1-mainnet" or .name == "try-runtime-snap-v0.10.1-testnet" or .name == "try-runtime-snap-v0.10.1-devnet")] | length' "$payload") |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
| publisher.put( | ||
| f"{MIRROR_PREFIX}/{args.artifact_name}/latest.json", |
There was a problem hiding this comment.
[LOW] Prevent stale writers from replacing the latest manifest
Both the scheduled snapshot producer and per-PR prewarm job call this publisher. If a prewarm selects artifact A, then a scheduled refresh publishes newer artifact B before the prewarm finishes, this unconditional final PUT replaces latest.json with A and gives it a new published_at. Hosts can consequently prefetch an older snapshot until another publication. Preserve monotonic freshness—for example, publish latest.json only from the scheduled producer, or conditionally replace it only when the existing manifest has an older artifact/production timestamp.
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
| # One-time bootstrap until this PR lands. This is the same strict | ||
| # all-112 round-robin matrix enforced by the repository test. | ||
| .rust-sdk-e2e-manifest/.github/scripts/build-bittensor-e2e-matrix.py \ | ||
| "$manifest" "$GITHUB_OUTPUT" 32 |
There was a problem hiding this comment.
[MEDIUM] [BOOTSTRAP] PR-controlled builder can select an incomplete E2E matrix
The base revision does not contain the trusted matrix builder, so this PR’s initial CI run executes the builder checked out from the PR head. That script controls GITHUB_OUTPUT and can omit tests from the generated matrix.
This path becomes structurally unreachable after merge, when the builder exists on the trusted base. The setup-time mitigation is the nucleus-approved CI run plus heightened human scrutiny of .github/ changes; any future reintroduction of this fallback would be a strong red flag.
|
🔄 AI review updated — Skeptic: VULNERABLE |
| # One-time bootstrap until this PR lands. This is the same strict | ||
| # all-112 round-robin matrix enforced by the repository test. | ||
| .rust-sdk-e2e-manifest/.github/scripts/build-bittensor-e2e-matrix.py \ | ||
| "$manifest" "$GITHUB_OUTPUT" 32 |
There was a problem hiding this comment.
[MEDIUM] [BOOTSTRAP] PR-controlled builder can select an incomplete E2E matrix
The trusted base revision still lacks build-bittensor-e2e-matrix.py, so this fallback executes the builder checked out from the PR head. A malicious builder can emit a valid-looking but incomplete matrix and suppress E2E coverage during this PR’s approved CI run. This is bootstrap-only: after merge, the base copy exists and the trusted branch is always used. The mitigating controls are the one nucleus-approved run and heightened human scrutiny of .github/; any future reintroduction of this fallback would be a strong red flag. Remove the fallback and make absence of the trusted builder fail closed, or have the workflow construct and validate the fixed 112-test matrix independently of PR-controlled code.
|
🔄 AI review updated — Skeptic: VULNERABLE |
| # One-time bootstrap until this PR lands. This is the same strict | ||
| # all-112 round-robin matrix enforced by the repository test. | ||
| .rust-sdk-e2e-manifest/.github/scripts/build-bittensor-e2e-matrix.py \ | ||
| "$manifest" "$GITHUB_OUTPUT" 32 |
There was a problem hiding this comment.
[MEDIUM] [BOOTSTRAP] PR-controlled builder can select an incomplete E2E matrix
The trusted base revision does not contain build-bittensor-e2e-matrix.py, so this PR’s approved CI run necessarily executes the builder from the PR head. A hostile builder could emit an incomplete matrix while the required check passes.
This path becomes structurally unreachable after merge because subsequent PRs will use the base-checked-out builder. The setup-time mitigation is the nucleus-approved CI run plus heightened human scrutiny of .github/ changes. Any future PR reintroducing this fallback condition should be treated as a strong red flag.
|
🔄 AI review updated — Skeptic: VULNERABLE |
| # One-time bootstrap until this PR lands. This is the same strict | ||
| # all-112 round-robin matrix enforced by the repository test. | ||
| .rust-sdk-e2e-manifest/.github/scripts/build-bittensor-e2e-matrix.py \ | ||
| "$manifest" "$GITHUB_OUTPUT" 32 |
There was a problem hiding this comment.
[MEDIUM] [BOOTSTRAP] PR-controlled builder can select an incomplete E2E matrix
When the trusted base lacks the matrix builder, this fallback executes the PR-head copy, allowing this PR to choose an incomplete E2E matrix despite the stated 112-test requirement.
This path is structurally unreachable after merge once the builder exists on main. The setup-time mitigation is the nucleus-approved CI run plus heightened human scrutiny of .github/ changes. Any future PR recreating this fallback path should be treated as a strong red flag.
|
🔄 AI review updated — Skeptic: VULNERABLE |
| # One-time bootstrap until this PR lands. This is the same strict | ||
| # all-112 round-robin matrix enforced by the repository test. | ||
| .rust-sdk-e2e-manifest/.github/scripts/build-bittensor-e2e-matrix.py \ | ||
| "$manifest" "$GITHUB_OUTPUT" 32 |
There was a problem hiding this comment.
[MEDIUM] [BOOTSTRAP] PR-controlled builder can select an incomplete E2E matrix
The base revision still lacks this builder, so the introducing run executes the PR-controlled copy. A hostile builder can report test_count=112 while emitting duplicates or omitting tests, bypassing the downstream count check. This path becomes structurally unreachable after merge; the mitigation is the one nucleus-approved CI run plus heightened human scrutiny of .github/. Any future reintroduction of this fallback should be treated as a strong red flag.
|
🔄 AI review updated — Skeptic: VULNERABLE |
| .rust-sdk-e2e-manifest/.github/scripts/build-bittensor-e2e-matrix.py \ | ||
| "$manifest" "$GITHUB_OUTPUT" 32 |
There was a problem hiding this comment.
[MEDIUM] [BOOTSTRAP] PR-controlled builder can select an incomplete E2E matrix
When the trusted base lacks this builder, the workflow executes the PR-head copy. The later compile-manifest comparison proves all 112 tests compiled, but does not prove the generated shards schedule all 112, so a modified builder can silently omit tests.
This unsafe path is structurally unreachable after merge. Its setup-time mitigation is the one nucleus-approved CI run plus heightened human scrutiny of .github/ changes; any future PR reintroducing this fallback would be a strong red flag. Add an independent matrix-completeness check that compares every generated test assignment against the manifest.
|
🔄 AI review updated — Skeptic: VULNERABLE |
| .rust-sdk-e2e-manifest/.github/scripts/build-bittensor-e2e-matrix.py \ | ||
| "$manifest" "$GITHUB_OUTPUT" 32 |
There was a problem hiding this comment.
[MEDIUM] [BOOTSTRAP] PR-controlled builder can select an incomplete E2E matrix
When the trusted base lacks this newly introduced builder, the workflow executes the PR-head copy. That script controls test_count, shard_count, and test_matrix; the later test_count == '112' condition therefore does not independently prove that all 112 tests appear exactly once in the matrix. A hostile version could report 112 while omitting selected tests.
Validate the emitted matrix independently in trusted workflow code (or construct it with a fixed inline routine) before scheduling shards. This path is structurally unreachable after merge because the builder will then exist at the base revision; the mitigation for this introducing run is nucleus approval plus heightened human scrutiny of .github/. A future PR reintroducing a PR-head fallback would be a strong red flag.
|
🔄 AI review updated — Skeptic: VULNERABLE |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
624a562 to
1013624
Compare
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
Reduce CI wall time while retaining fail-closed test ownership and required check reporting. - route Rust, runtime, Docker, clone, Rust SDK, and TypeScript coverage from trusted change classifiers - share production builds and snapshots across consumers - compile the Rust SDK E2E harness once and execute its verified 112-test manifest in balanced shards - shard TypeScript E2Es with explicit suite ownership and actionable validation failures - reuse immutable localnet artifacts where the PR cannot affect chain behavior - add bounded R2-backed compiler and artifact caching with idle-host proactive warming - keep assertion-bearing clone coverage required while making the reporting-only deprecated-stake histogram manual
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
|
cherry picked into #2949 |
1 participant
Important
One reporting-only clone test is now manual.
test-alpha-deprecated-stake-histogram.tsreads all legacy Alpha stake entries and prints a distribution chart. It does not check the result against an expected value, so it could not catch a regression by failing CI; it only made every PR wait about 49 seconds for a report nobody normally inspected. The script remains available throughnpm run test:alpha-deprecated-stake-histogramwhenever that report is useful. All eight clone tests with real pass/fail assertions remain required on relevant PRs.What changed
This PR is a coordinated CI runtime and reliability pass:
Coverage policy
No Rust SDK E2E test was removed. The 112 individual cases still receive isolated localnet containers; only compilation, image pulls, and job orchestration are shared.
Cache and failure behavior
Bootstrap trust note
The target branch does not yet contain the trusted Rust E2E matrix builder, so this introducing PR necessarily exercises the reviewed copy from the PR head. The workflow independently proves the complete 112-test manifest before sharding. After merge, subsequent PRs use the builder checked out from the trusted base and the bootstrap fallback is structurally unreachable.
Validation and measured impact
Current-head measurements from the final full run:
Because this PR changes the TypeScript shard topology itself, it also ran the canonical unsharded ownership audit. That one-time validation extended this PR's TypeScript workflow to 21m48s, primarily due to the unsharded Shield audit. Ordinary PRs do not pay for that audit unless they change the shard manifest, suite ownership, canonical Moonwall configuration, or the routing/validation infrastructure.
The latest
clone-upgrade (remaining)rerun is red after the refreshed mainnet snapshot changed the selected subnet. The failing balancer edge-emission test and its incomplete candidate-selection predicate already exist on the target branch; this PR does not change runtime, pallet, node, or that test's selection logic. Earlier runs of the same PR head passed against the previous snapshot. This is a snapshot-sensitive baseline test issue to fix separately, not a regression introduced by these CI changes.The runner restart used to deploy the final cache-affinity metadata interrupted an earlier attempt; GitHub marked those orphaned jobs failed after its ten-minute runner-loss timeout. A clean rerun acquired replacement runners in roughly 4–8 seconds.
Expected warm behavior after the scheduled warm-set generation has reached idle hosts:
The projection is intentionally separated from observed timings until the scheduled fleet refresh has run after merge and been measured end to end.