Skip to content

ci(desktop): gate the Rust shell on macOS + Windows too - #61

Merged
marcelxpfeifer merged 1 commit into
mainfrom
ci/desktop-ci-multi-os
Jul 2, 2026
Merged

ci(desktop): gate the Rust shell on macOS + Windows too#61
marcelxpfeifer merged 1 commit into
mainfrom
ci/desktop-ci-multi-os

Conversation

@marcelxpfeifer

Copy link
Copy Markdown
Contributor

What

Turns the single rust job in .github/workflows/desktop-ci.yml (previously ubuntu-22.04 only) into a matrix over ubuntu-22.04, macos-14, and windows-latest with strategy.fail-fast: false and runs-on: ${{ matrix.os }}.

Why

The desktop Rust shell (incl. the vendored OpenSSL/libssh2 build for ssh2) was only compiled on Linux in CI, so a Windows/macOS-only compile break only surfaced at release-tag time in desktop-release.yml. This adds a fast per-PR compile+test gate on all three desktop OSes.

Changes

  • Gate the Install Linux dependencies apt step with if: runner.os == 'Linux'.
  • Add a Windows-only Install Windows build dependencies (NASM) step (if: runner.os == 'Windows') using ilammy/setup-nasm@v1 — the vendored OpenSSL/libssh2 build needs an assembler on Windows; Strawberry Perl is already preinstalled on windows-latest. macOS needs no extra deps.
  • Add shell: bash to the Stub frontend dist step so its mkdir -p / redirect run under Git Bash on windows-latest instead of PowerShell.

Preserving macOS/Linux behavior

The Linux job is byte-for-byte the same steps as before (the apt step now just carries an always-true runner.os == 'Linux' guard). cargo fmt --check, cargo test, and the bun lint/typecheck/test bridge steps are all cross-platform and unchanged. swatinem/rust-cache workspaces stay apps/desktop/src-tauri. The full signed tauri build remains in desktop-release.yml — this stays a fast compile+test gate.

Test coverage

The workflow going green on all three OSes on this PR is the test (desktop-ci.yml is in its own path filter, so the new macOS/Windows jobs run here).

Auto-merge pipeline: squash-merges on 3/3 lens approvals + green CI.

Turn the single ubuntu-22.04 rust job into a fail-fast:false matrix over
ubuntu-22.04, macos-14 and windows-latest so a Windows/macOS-only compile
break in the Rust shell or the vendored OpenSSL/libssh2 build surfaces on the
PR instead of at release-tag time.

- Gate the apt install step with runner.os == 'Linux'.
- Add a Windows-only ilammy/setup-nasm step (Strawberry Perl is preinstalled)
  for the vendored ssh2 OpenSSL/libssh2 build.
- Run the Stub frontend dist step under shell: bash so the mkdir -p / redirect
  works via Git Bash on windows-latest.

macOS/Linux behavior is unchanged; the fmt/test and bun bridge steps are
already cross-platform. The full tauri build stays in desktop-release.yml.
@marcelxpfeifer

Copy link
Copy Markdown
Contributor Author

Security review — round 1

Verdict: APPROVE

CI-only change to .github/workflows/desktop-ci.yml; no changes to install.sh, scripts/owlat, or ssh.rs, so the Linux-server install trust boundary and the SSH transport are untouched. No new remote-exec surface.

Checked against the Security lens:

  • harden-runner preserved (.github/workflows/desktop-ci.yml:31) — the step-security/harden-runner@v2 egress guard (egress-policy: audit) is unchanged and now runs on all three OS legs. Not dropped or weakened.
  • No secrets exposed / no fork-secret widening — the rust job references no secrets.*; this is a pull_request-triggered compile+test gate. Turning it into a 3-OS matrix does not grant forks access to any secret they couldn't already reach.
  • No untrusted input / injection — the Stub frontend dist step (.github/workflows/desktop-ci.yml:70-75) runs static content under shell: bash; no interpolation of untrusted PR data into shell. The if: guards use runner.os, a trusted runner-provided value.
  • Nothing secret logged.
  • New third-party action ilammy/setup-nasm@v1 (.github/workflows/desktop-ci.yml:52) is Windows-gated and pinned to a tag — consistent with the file's existing tag-pinned actions (checkout@v7, setup-bun@v2, rust-cache@v2). Non-blocking nit: repo convention here is tag-pinning, so this matches; a future hardening pass could SHA-pin all Actions uniformly, but that is out of scope for this piece.

Functional gate: all three matrix legs are green on this PR — ubuntu-22.04 (5m9s), macos-14 (3m17s), windows-latest (14m23s). Linux steps are byte-for-byte equivalent (apt step now carries an always-true runner.os == 'Linux' guard), so no macOS/Linux regression.

No blocking Security issues.

@marcelxpfeifer

Copy link
Copy Markdown
Contributor Author

Code Quality review — round 1

Verdict: APPROVE

Reviewed the full diff against the piece intent. This genuinely delivers a cross-platform compile+test gate without relocating the problem or regressing macOS/Linux.

  • .github/workflows/desktop-ci.yml:24-31 — clean matrix (ubuntu-22.04, macos-14, windows-latest) with fail-fast: false and runs-on: ${{ matrix.os }}; job name interpolates ${{ matrix.os }} for readable per-OS status. Correct.
  • desktop-ci.yml:39 — apt step gated if: runner.os == 'Linux'. Linux path is byte-for-byte the prior behavior plus an always-true guard — no macOS/Linux regression.
  • desktop-ci.yml:47-52 — Windows-only NASM via ilammy/setup-nasm@v1 gated if: runner.os == 'Windows', for the vendored OpenSSL/libssh2 (ssh2) build; Strawberry Perl is preinstalled on windows-latest, so no Perl step needed. macOS needs nothing extra — correct.
  • desktop-ci.yml:70-72 — the only bash-ism step (Stub frontend dist: mkdir -p, > redirect) now carries shell: bash, so it runs under Git Bash on windows-latest instead of PowerShell. No remaining POSIX FOO=bar cmd prefixes anywhere. The remaining run: steps are single bun run / cargo invocations that are shell-agnostic — no gratuitous shell: churn.
  • Scope respected: cargo fmt --check, cargo test --manifest-path ..., the bun lint/typecheck/test bridge steps, and swatinem/rust-cache workspaces are all unchanged and cross-platform; no full tauri build added (that stays in desktop-release.yml).
  • Commit is atomic, conventional (ci(desktop): ...), no AI/Claude attribution, focused diff, no dead code.

CI: all three Rust build + tests jobs (ubuntu-22.04, macos-14, windows-latest) went green on this PR, along with the rest of the suite. The new macOS/Windows jobs pass, satisfying the "must go green" requirement.

No blocking Code Quality issues.

@marcelxpfeifer

Copy link
Copy Markdown
Contributor Author

Functionality & Tests review — round 1

Verdict: APPROVE

Judged against the piece intent: turn the single ubuntu-22.04 rust job into a cross-OS matrix without regressing Linux behavior.

Cross-OS delivery — verified green on THIS PR

All three matrix legs of the path-filtered desktop-ci.yml ran and passed on this PR:

  • Rust build + tests (ubuntu-22.04) — pass
  • Rust build + tests (macos-14) — pass
  • Rust build + tests (windows-latest) — pass (14m23s; the vendored OpenSSL/libssh2 ssh2 build compiled with NASM + preinstalled Strawberry Perl)

This is the acceptance test for the piece and it is satisfied.

Spec compliance (.github/workflows/desktop-ci.yml)

  • strategy.fail-fast: false + matrix.os: [ubuntu-22.04, macos-14, windows-latest] + runs-on: ${{ matrix.os }} — correct.
  • Install Linux dependencies apt step gated if: runner.os == 'Linux' — correct; the Linux job runs the byte-identical step it ran before, so macOS/Linux behavior is preserved (guard is always-true on Linux).
  • Install Windows build dependencies (NASM) via ilammy/setup-nasm@v1, gated if: runner.os == 'Windows', placed before the Rust build — correct; macOS needs no extra deps.
  • Stub frontend dist now has shell: bash so mkdir -p/redirect run under Git Bash on windows-latest — correct.
  • cargo fmt --check, cargo test --manifest-path apps/desktop/src-tauri/Cargo.toml, and the bun lint/typecheck/test bridge steps are unchanged and cross-platform; swatinem/rust-cache workspaces: apps/desktop/src-tauri preserved.
  • No full tauri build added — stays a fast compile+test gate, per spec.

CI

gh pr checks 61 — all checks pass; no FAILED checks. Scope is confined to the workflow file; no product code touched, so no macOS/Linux runtime regression risk.

No blocking Functionality & Tests issues.

@marcelxpfeifer

Copy link
Copy Markdown
Contributor Author

Author response — round 2

All three round-1 reviews (Security, Code Quality, Functionality & Tests) returned APPROVE with no blocking findings, and every CI check is green — including all three Rust matrix legs on this PR: ubuntu-22.04 (5m9s), macos-14 (3m17s), windows-latest (14m23s, vendored OpenSSL/libssh2 ssh2 built via NASM + preinstalled Strawberry Perl).

Because there are no blocking comments, no code changes are made in round 2 — the workflow is left exactly as reviewed and passing, to avoid invalidating the green matrix.

Intentionally unchanged

  • SHA-pinning all Actions (Security review non-blocking nit): left as-is. ilammy/setup-nasm@v1 is tag-pinned, matching this file's existing convention (checkout@v7, setup-bun@v2, rust-cache@v2). A uniform SHA-pinning hardening pass is out of scope for this piece, as the reviewer noted.

Nothing else was raised. The piece delivers the cross-OS compile+test gate (macOS + Windows added to Linux) with the Linux path byte-for-byte preserved (apt step now carries an always-true runner.os == 'Linux' guard).

@marcelxpfeifer

Copy link
Copy Markdown
Contributor Author

Security review — round 2

Verdict: APPROVE

Re-review of the unchanged diff at head 89414a82. My round-1 blocking findings: there were none. The author correctly made no round-2 code changes (avoiding invalidating the green matrix), and the sole round-1 note (uniform SHA-pinning) was explicitly non-blocking and out of scope for this piece.

Confirmed against the Security lens on the current head:

  • harden-runner preserved (.github/workflows/desktop-ci.yml:31-33) — step-security/harden-runner@v2 with egress-policy: audit is unchanged and runs on all three OS legs. Not dropped or weakened.
  • No secrets exposed / no fork-secret widening — the rust job references no secrets.*; pull_request-triggered compile+test gate. Expanding to a 3-OS matrix grants forks no secret they couldn't already reach.
  • No untrusted input / injectionStub frontend dist (desktop-ci.yml:71-77) runs static content under shell: bash; no interpolation of untrusted PR data. if: guards use runner.os, a trusted runner-provided value; ${{ matrix.os }} interpolation is over a fixed literal list, not attacker-controlled.
  • Trust boundary intact — no changes to install.sh, scripts/owlat, or ssh.rs; host-key verification, tarball safety, and the Linux-server install boundary are untouched. No new remote-exec surface.
  • Nothing secret logged.
  • ilammy/setup-nasm@v1 (desktop-ci.yml:52) — Windows-gated, tag-pinned consistent with the file's existing convention (checkout@v7, setup-bun@v2, rust-cache@v2). Non-blocking.

Functional gate: all three matrix legs green on this PR — ubuntu-22.04 (5m9s), macos-14 (3m17s), windows-latest (14m23s) — plus the full suite. Linux steps are byte-for-byte equivalent (apt step carries an always-true runner.os == 'Linux' guard); no macOS/Linux regression.

No blocking Security issues.

@marcelxpfeifer

Copy link
Copy Markdown
Contributor Author

Code Quality review — round 2

Verdict: APPROVE

Re-reviewed at PR head (origin/ci/desktop-ci-multi-os, commit 89414a8). Round 1 (all three lenses) approved with no blocking findings; the author correctly made no round-2 code changes since nothing blocking was raised, leaving the green matrix intact. Confirmed the piece still delivers its spec cross-platform without regressing macOS/Linux:

  • .github/workflows/desktop-ci.yml:23-30 — matrix over ubuntu-22.04, macos-14, windows-latest with strategy.fail-fast: false and runs-on: ${{ matrix.os }}; job name interpolates ${{ matrix.os }} for per-OS status. Valid YAML.
  • desktop-ci.yml:39 — apt step gated if: runner.os == 'Linux'; the Linux path is byte-for-byte the prior behavior plus an always-true guard. No macOS/Linux regression.
  • desktop-ci.yml:47-52 — Windows-only NASM via ilammy/setup-nasm@v1 gated if: runner.os == 'Windows', placed before the Rust build, for the vendored OpenSSL/libssh2 (ssh2) build; Strawberry Perl preinstalled on windows-latest, macOS needs nothing extra. Correct.
  • desktop-ci.yml:70-75 — the only bash-ism step (Stub frontend dist: mkdir -p, > redirect) carries shell: bash so it runs under Git Bash on windows-latest. No remaining POSIX FOO=bar cmd prefixes; other run: steps are shell-agnostic single bun/cargo invocations with no gratuitous shell: churn.
  • Scope respected: cargo fmt --check, cargo test --manifest-path apps/desktop/src-tauri/Cargo.toml, the bun lint/typecheck/test bridge steps, and swatinem/rust-cache workspaces: apps/desktop/src-tauri are unchanged and cross-platform; no full tauri build added (stays in desktop-release.yml).
  • Single atomic conventional commit (ci(desktop): ...), no AI/Claude attribution, focused diff, no dead code.

CI: all three Rust build + tests legs green on this PR — ubuntu-22.04 (5m9s), macos-14 (3m17s), windows-latest (14m23s) — plus the rest of the suite. The must-go-green requirement for the new macOS/Windows jobs is satisfied.

No blocking Code Quality issues.

@marcelxpfeifer

Copy link
Copy Markdown
Contributor Author

Functionality & Tests review — round 2

Verdict: APPROVE

Re-reviewed against the piece intent (turn the single ubuntu-22.04 rust job into a cross-OS matrix without regressing Linux). Round 1 had no blocking findings; round 2 made no code changes (the author intentionally left the green matrix untouched), so there was nothing to re-verify beyond CI freshness.

Cross-OS delivery — green on THIS PR

gh pr checks 61 shows all three path-filtered matrix legs passing on this very PR:

  • Rust build + tests (ubuntu-22.04) — pass (5m9s)
  • Rust build + tests (macos-14) — pass (3m17s)
  • Rust build + tests (windows-latest) — pass (14m23s; vendored OpenSSL/libssh2 ssh2 compiled via NASM + preinstalled Strawberry Perl)

This is the acceptance test for the piece and it is satisfied. Entire suite (35 checks) is green; no FAILED or pending checks.

Spec compliance (.github/workflows/desktop-ci.yml)

  • .github/workflows/desktop-ci.yml:24-30strategy.fail-fast: false + matrix.os: [ubuntu-22.04, macos-14, windows-latest] + runs-on: ${{ matrix.os }} — correct.
  • desktop-ci.yml:39 — apt Install Linux dependencies gated if: runner.os == 'Linux'; Linux path is byte-for-byte the prior step plus an always-true guard — no macOS/Linux regression.
  • desktop-ci.yml:47-52Install Windows build dependencies (NASM) via ilammy/setup-nasm@v1, gated if: runner.os == 'Windows', placed before the Rust build; macOS needs no extra deps — correct.
  • desktop-ci.yml:70-72Stub frontend dist now carries shell: bash so mkdir -p/redirect run under Git Bash on windows-latest — correct.
  • cargo fmt --check, cargo test --manifest-path apps/desktop/src-tauri/Cargo.toml, and the bun lint/typecheck/test bridge steps are unchanged and cross-platform; swatinem/rust-cache workspaces: apps/desktop/src-tauri preserved. No full tauri build added — stays a fast compile+test gate, per spec.

Scope note

This piece touches only desktop-ci.yml; the ssh.rs exec-bit / cross-env / quickstart Docker-Desktop items are separate pieces in the family and are not part of this PR.

No blocking Functionality & Tests issues.

@marcelxpfeifer
marcelxpfeifer merged commit f019118 into main Jul 2, 2026
35 checks passed
@marcelxpfeifer
marcelxpfeifer deleted the ci/desktop-ci-multi-os branch July 2, 2026 10:41
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.

1 participant