ci: schedule Rust workspace tests on macOS or Linux self-hosted runners - #4287
Merged
Merged
Conversation
`runs-on` label arrays are AND-ed, so `[self-hosted, macOS, ARM64]` could never also match the new Linux runner. Select on a shared custom label instead: `[self-hosted, rust-ci]`, applied to both mac runners and the Linux one. Keeping `self-hosted` in the list means GitHub-hosted runners still can't pick the job up. Making the job portable: - Homebrew setup is gated to macOS; Linux gets an apt equivalent for what ./.github/actions/rust doesn't already install (it handles clang, llvm and libsnappy), plus gh for the immutable-structure check and rustup for dtolnay/rust-toolchain. Every branch is a no-op after the first run. - The disk prune clamps its absolute caps to a share of the volume, so a smaller runner doesn't purge target/ on every run and cold-rebuild. - The GPG keyboxd reset is a macOS runner workaround, so it's gated too. Linux builds rocksdb from source like macOS does rather than linking the prebuilt static lib the old Ubuntu jobs used. The persistent target/ makes that a one-time cost, and it avoids their sed pass over Cargo.toml crate types, which static rocksdb forced. The three Ubuntu backup jobs and their poll-the-Mac-job dance are removed. UBUNTU_BACKUP_ENABLED has been false, so they were dead code, and a second self-hosted runner is the redundancy they were standing in for. That empties tests-rs-doctests.yml; doctests already run inside both Rust jobs under `doctests-changed`. The job is renamed Tests (macOS) -> Tests. No branch protection rule references it. Requires the `rust-ci` label on all three runners. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRust CI now runs through self-hosted ChangesRust CI workflow migration
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Collaborator
|
⛔ Blockers found — Sonnet deferred (commit 0444662) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In @.github/workflows/tests.yml:
- Around line 157-162: Update the workflow scope comment near the fork pull
request handling to say that untrusted fork PRs must use the full workspace
path, while acknowledging the trusted thepastaclaw fork exception that may use
the wallet scope and reach tests-rs-wallet.yml. Preserve the existing
distinctions for same-repository pull requests, push runs, and scheduled runs.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 44d48aaf-5c35-4c0a-8958-6b21dcabd387
📒 Files selected for processing (5)
.github/actionlint.yaml.github/workflows/tests-rs-doctests.yml.github/workflows/tests-rs-wallet.yml.github/workflows/tests-rs-workspace.yml.github/workflows/tests.yml
💤 Files with no reviewable changes (1)
- .github/workflows/tests-rs-doctests.yml
Comment on lines
157
to
+162
| # The fast path applies only to same-repo pull requests. Fork PRs | ||
| # must take the full workspace path (whose Ubuntu backup jobs cover | ||
| # them when UBUNTU_BACKUP_ENABLED is set, while the wallet | ||
| # workflow's only job skips fork PRs). Push and schedule runs have | ||
| # no reliable base SHA — scope computed from the last commit alone | ||
| # could silently downgrade the nightly / post-merge full runs. | ||
| # must take the full workspace path (both Rust workflows skip fork | ||
| # PRs outright, since neither may run untrusted code on a persistent | ||
| # runner). Push and schedule runs have no reliable base SHA — scope | ||
| # computed from the last commit alone could silently downgrade the | ||
| # nightly / post-merge full runs. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Describe the trusted fork exception in the scope comment.
The comment says that fork pull requests use the full workspace path. Lines [168]-[170] treat a fork owned by thepastaclaw as trusted, so that fork may use scope=wallet and reach tests-rs-wallet.yml. Change the comment to say Untrusted fork PRs.
Proposed wording
- # The fast path applies only to same-repo pull requests. Fork PRs
- # must take the full workspace path (both Rust workflows skip fork
- # PRs outright, since neither may run untrusted code on a persistent
- # runner). Push and schedule runs have no reliable base SHA — scope
+ # The fast path applies only to same-repo pull requests. Untrusted
+ # fork PRs are classified as full-scope changes. Both Rust workflows
+ # skip those PRs, since neither may run untrusted code on a
+ # persistent runner. Push and schedule runs have no reliable base SHA📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # The fast path applies only to same-repo pull requests. Fork PRs | |
| # must take the full workspace path (whose Ubuntu backup jobs cover | |
| # them when UBUNTU_BACKUP_ENABLED is set, while the wallet | |
| # workflow's only job skips fork PRs). Push and schedule runs have | |
| # no reliable base SHA — scope computed from the last commit alone | |
| # could silently downgrade the nightly / post-merge full runs. | |
| # must take the full workspace path (both Rust workflows skip fork | |
| # PRs outright, since neither may run untrusted code on a persistent | |
| # runner). Push and schedule runs have no reliable base SHA — scope | |
| # computed from the last commit alone could silently downgrade the | |
| # nightly / post-merge full runs. | |
| # The fast path applies only to same-repo pull requests. Untrusted | |
| # fork PRs are classified as full-scope changes. Both Rust workflows | |
| # skip those PRs, since neither may run untrusted code on a | |
| # persistent runner. Push and schedule runs have no reliable base SHA | |
| # computed from the last commit alone could silently downgrade the | |
| # nightly / post-merge full runs. |
🤖 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/tests.yml around lines 157 - 162, Update the workflow
scope comment near the fork pull request handling to say that untrusted fork PRs
must use the full workspace path, while acknowledging the trusted thepastaclaw
fork exception that may use the wallet scope and reach tests-rs-wallet.yml.
Preserve the existing distinctions for same-repository pull requests, push runs,
and scheduled runs.
thepastaclaw
suggested changes
Aug 5, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Preliminary review — Codex only
The shared runner label and OS-specific workflow gates correctly allow the workspace job to target either macOS or Linux. However, the Linux bootstrap omits both the OpenSSL development files required by the workspace dependency graph and the cargo-nextest executable required by the test command, so a newly provisioned runner cannot reliably complete the job. The Rust scope comment also inaccurately describes the explicitly trusted fork exception.
Source: reviewers codex/general=gpt-5.6-sol(completed); verifier=codex/verifier=gpt-5.6-sol(completed); coordinator=openclaw-agent/cliproxy/gpt-5.6-sol(orchestration-only).
Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 2 blocking | 💬 1 nitpick(s)
1 additional finding(s) omitted (not in diff).
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `.github/workflows/tests-rs-workspace.yml`:
- [BLOCKING] .github/workflows/tests-rs-workspace.yml:84: Install OpenSSL headers on the Linux runner
The Linux provisioning list installs `pkg-config` but not the OpenSSL development package. The Linux workspace build activates the default, non-vendored `openssl-sys` feature through `native-tls` and reqwest, including during `cargo clippy --workspace --all-features`. On a fresh Ubuntu runner, `openssl-sys` therefore cannot find the required OpenSSL headers and `openssl.pc` metadata. Add `libssl-dev` to the packages installed during first-run provisioning.
- [BLOCKING] .github/workflows/tests-rs-workspace.yml:118-119: Provision cargo-nextest before invoking llvm-cov nextest
The test step runs `cargo llvm-cov nextest`, which delegates execution to the separately installed `cargo-nextest` binary. Installing `cargo-llvm-cov` does not provide that binary. The installer was previously removed from the macOS path only because nextest was already installed on those persistent runners, while the deleted Ubuntu fallback installed it explicitly. The new Linux first-run setup does neither, so a fresh Linux runner will fail when it reaches the non-shielded test step.
Comment on lines
157
to
+162
| # The fast path applies only to same-repo pull requests. Fork PRs | ||
| # must take the full workspace path (whose Ubuntu backup jobs cover | ||
| # them when UBUNTU_BACKUP_ENABLED is set, while the wallet | ||
| # workflow's only job skips fork PRs). Push and schedule runs have | ||
| # no reliable base SHA — scope computed from the last commit alone | ||
| # could silently downgrade the nightly / post-merge full runs. | ||
| # must take the full workspace path (both Rust workflows skip fork | ||
| # PRs outright, since neither may run untrusted code on a persistent | ||
| # runner). Push and schedule runs have no reliable base SHA — scope | ||
| # computed from the last commit alone could silently downgrade the | ||
| # nightly / post-merge full runs. |
Collaborator
There was a problem hiding this comment.
💬 Nitpick: Document the trusted fork exception in the scope comment
The comment says the fast path applies only to same-repository pull requests and that both Rust workflows skip fork PRs, but the condition immediately below explicitly trusts forks owned by thepastaclaw. Those pull requests continue through wallet-scope classification and can reach tests-rs-wallet.yml. Update the comment to distinguish untrusted forks from this trusted exception.
Suggested change
| # The fast path applies only to same-repo pull requests. Fork PRs | |
| # must take the full workspace path (whose Ubuntu backup jobs cover | |
| # them when UBUNTU_BACKUP_ENABLED is set, while the wallet | |
| # workflow's only job skips fork PRs). Push and schedule runs have | |
| # no reliable base SHA — scope computed from the last commit alone | |
| # could silently downgrade the nightly / post-merge full runs. | |
| # must take the full workspace path (both Rust workflows skip fork | |
| # PRs outright, since neither may run untrusted code on a persistent | |
| # runner). Push and schedule runs have no reliable base SHA — scope | |
| # computed from the last commit alone could silently downgrade the | |
| # nightly / post-merge full runs. | |
| # The fast path applies to same-repo pull requests and to the trusted | |
| # thepastaclaw fork. Untrusted fork PRs are classified as full-scope | |
| # changes, and both Rust workflows skip them because neither may run | |
| # untrusted code on a persistent runner. Push and schedule runs have | |
| # no reliable base SHA — scope computed from the last commit alone | |
| # could silently downgrade the nightly / post-merge full runs. |
source: ['coderabbit']
Co-authored-by: Lil Claw <pasta+claw@dashboost.org>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v4.2-dev #4287 +/- ##
============================================
+ Coverage 87.54% 87.57% +0.03%
============================================
Files 2679 2700 +21
Lines 341312 344254 +2942
============================================
+ Hits 298798 301492 +2694
- Misses 42514 42762 +248
🚀 New features to boost your workflow:
|
QuantumExplorer
added a commit
that referenced
this pull request
Aug 5, 2026
One textual conflict and one semantic one. `.github/workflows/tests-rs-workspace.yml`: #4287 collapsed the macOS job and the Ubuntu backup job into a single `[self-hosted, rust-ci]` job and dropped the cdylib->rlib strip step entirely. Took that structure, so this branch's edits to the strip step fall away with the step itself (the surviving job already runs cdylib crates such as rs-sdk-ffi unstripped). Kept this branch's addition of platform-encryption and rs-unified-sdk-jni to the package list; the resolved file is now exactly the base plus those two lines. The wallet workflow's rs-unified-sdk-jni entry merged cleanly. `DocumentQuery` gained an `offset` field in #4266 (ranked aggregate indexes, protocol v14), so the three txMetadata query builders no longer compiled. All three set `offset: None`: the encrypted-document scan pages by insertion-order cursor, and `offset` is served only on the ranked surface — a skip-count page would silently drop documents whenever the owner writes between round-trips. The two test builders mirror the production query, so they take the same value. cargo test -p platform-wallet -p platform-wallet-ffi -p rs-unified-sdk-jni: 710 / 267+26+6+9 / 52 passed, 0 failed. cargo fmt --all -- --check and clippy --all-targets on the three crates: clean.
6 tasks
bfoss765
added a commit
to bfoss765/platform
that referenced
this pull request
Aug 5, 2026
…b into v41-keystore-qa5 Reconstruction of PR dashpay#4301 (feat/shielded-two-note-invites @d6610262b0) on top of qa5's tip. dashpay#4301 is based on a current v4.2-dev point; merging it directly would have dragged in 9 unrelated commits (dashpay#4287, dashpay#4266, dashpay#4279, dashpay#4276, dashpay#4278, dashpay#4277, and duplicate dashpay#4183/dashpay#4191/dashpay#4251), including dashpay#4277's competing encrypted-txMetadata implementation that collides with qa5's dashpay#4186. The reconstruction cherry-picks ONLY dashpay#4301's own commit; the resulting delta is byte-identical to the original (1256 insertions, 8 deletions across the same 11 files) — only hunk offsets differ. Verified: platform-wallet 774/774, platform-wallet-ffi 284/284, dpp shielded 216/216 (incl. both multi/single-output fee-vs-action-count tests), the four dashpay#4204 security tests, and dashpay#4301's three note-selection tests all pass. rustfmt clean; no new clippy warnings.
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
We added a self-hosted Linux runner, but
Rust workspace tests / Tests (macOS)was pinned toruns-on: [self-hosted, macOS, ARM64]and could never schedule onto it. Labels in aruns-onarray are AND-ed — there is no OR syntax — so no edit to that list alone can express "macOS or Linux".Current runners on this repo:
self-hosted, macOS, ARM64self-hosted, Linux, X64, ubuntu, ms-a2What was done?
Select on a shared custom label instead:
Whichever of the three runners is idle takes the job. Keeping
self-hostedin the list means a GitHub-hosted Linux VM can never match it, so Linux stays self-hosted-only.Making the job body portable:
./.github/actions/rustdoesn't already install (it handles clang, llvm and libsnappy on Linux), plusghfor the immutable-structure check andrustupfordtolnay/rust-toolchain. Each branch is guarded, so it's a no-op once the persistent runner is provisioned by the first run.TOTAL/3andTOTAL/5). On the large macOS volumes this is exactly today's behavior; it stops a smaller volume from tripping the floor on every run and forcing a cold rebuild each time.Linux builds rocksdb from source the same way macOS already does, rather than linking the prebuilt static lib the old Ubuntu jobs used. The persistent
target/makes that a one-time cost and it avoids theirsedpass rewriting cdylib crate-types inCargo.toml, which static rocksdb forced.Removed the three Ubuntu backup jobs (
test-ubuntu,lint-ubuntu, and the whole oftests-rs-doctests.yml) along with theirsleep 15+ poll-the-Mac-job-status dance.UBUNTU_BACKUP_ENABLEDisfalseon this repo, so they have been dead code; a second self-hosted runner is the redundancy they were standing in for. Doctests already run inside both Rust jobs underdoctests-changed, so dropping the standaloners-doctestsjob loses no coverage.Also added
.github/actionlint.yamldeclaringrust-ci, so localactionlintruns don't flag it as an unknown label.Required before merge
The
rust-cilabel must be added to all three runners (Settings → Actions → Runners → runner → Labels). Until then the job has nothing to schedule onto and will sit queued.How Has This Been Tested?
actionlint(with shellcheck) clean across the three touched workflows; the remaining shellcheck notes are pre-existing and unrelated — the sameSC2086fires on the untouched Homebrew line intests-rs-wallet.yml.Tests (macOS)job name, andUBUNTU_BACKUP_ENABLED; the three comment mentions left behind were updated.v4.2-devhas no required status checks configured, so the job rename breaks no gate.Breaking Changes
None.
Notes for reviewers
tests-rs-wallet.ymlstays pinned to the macOS runners. That fast path is short and mac-only by design; floating it too is a reasonable follow-up but I kept this PR to the workspace job. Its header comment is updated to say so.UBUNTU_BACKUP_ENABLEDis now unreferenced and can be deleted from repo variables.if:guard blocked them from the mac runners before and now guards both platforms; the Ubuntu backup that could theoretically have covered them was disabled.Checklist:
🤖 Generated with Claude Code
Summary by CodeRabbit
rust-ci.