Windows port PR 1: compile + launch + unsigned CI#520
Open
ldstreet wants to merge 7 commits into
Open
Conversation
📝 WalkthroughWalkthroughThis PR adds Windows desktop build support to Maple's CI/CD pipeline while refactoring proxy configuration to support platform-specific paths via Tauri AppHandle, tightening filesystem security capabilities, and hardening build artifact verification with SHA256 checksums. ChangesWindows Desktop Build and Proxy Configuration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
3 tasks
Deploying maple with
|
| Latest commit: |
1aa2673
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b19b0b62.maple-ca8.pages.dev |
| Branch Preview URL: | https://windows-pr1-compile-launch.maple-ca8.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Three minimum-viable changes to unblock building Maple on Windows. Atomic migration, keyring storage, signing, TTS DLL shipping, and the CI matrix job are deferred to follow-up epics. tauri.conf.json (MPLR-uyqowcnn / 1a): - bundle.targets switched from "all" to an explicit list excluding "msi". Tauri's WiX template hard-codes HKLM + perMachine install, which would shadow the HKCU keys we rely on for cloud.opensecret.maple deep links. - bundle.windows.nsis.installMode = "currentUser" so the NSIS installer is per-user (no UAC) and writes deep-link registration under HKCU. - bundle.windows.webviewInstallMode.type = "downloadBootstrapper" set explicitly to avoid surprise behavior if the Tauri default changes. capabilities/default.json (MPLR-zivybmgl / 1b): - Dropped $HOME/.config/maple/** scope entries from every fs:allow-* permission. The path tokens don't resolve to anything meaningful on Windows, no JS code touches them via the Tauri fs plugin, and proxy.rs writes via raw std::fs which bypasses the capability system entirely. $APPCONFIG/** entries are kept. src/proxy.rs (MPLR-oshibkah / 1c): - get_config_path now takes &AppHandle and resolves to %APPDATA%\cloud.opensecret.maple\ on Windows via Tauri's path resolver. macOS and Linux behavior is byte-identical (still ~/.config/maple/). - AppHandle is plumbed through save_proxy_config, load_saved_proxy_config, start_proxy, load_proxy_config, save_proxy_settings, and the auto-start initializer. Frontend invoke() calls are unchanged — Tauri auto-injects AppHandle on commands. - USERPROFILE fallback removed; Windows now uses the proper Tauri path. cargo check + cargo clippy -D warnings + cargo fmt --check all pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirrors the existing macOS/Linux unsigned PR build jobs in desktop-build.yml. Produces an x64 Windows NSIS installer on every PR and push to master. What the job does: - Runs on windows-latest (x64 Windows Server 2022 — note this differs from ARM64 Windows VMs that some developers use locally; matches the artifact most users will install). - Installs Rust stable (default target x86_64-pc-windows-msvc on the runner). - Installs sccache via choco and caches it across runs keyed on Cargo.lock. - Provides ONNX Runtime by downloading the prebuilt Windows x64 zip from Microsoft and pointing ORT_LIB_LOCATION + ORT_SKIP_DOWNLOAD at it, mirroring the Linux job's pattern. This keeps the ort crate from re-downloading and works around any flaky download-binaries paths. - Sets up Bun for the frontend bundle (Bun has Windows-x64 builds; only the ARM64 Windows local dev path lacks a native Bun and must fall back to npm). - Runs tauri-action with --bundles nsis. The tauri.conf.json bundle.targets array set in subtask 1a already excludes msi, so NSIS is the only Windows bundler invoked. - Provides TAURI_SIGNING_PRIVATE_KEY (existing repo secret) so the minisign updater artifact step succeeds. Authenticode signing of the .exe itself is out of scope here — that's Epic 6 (PR 7). - Uploads the NSIS .exe as workflow artifact maple-windows-x64. Same env vars as the macOS/Linux jobs (VITE_OPEN_SECRET_API_URL switches on pull_request vs push, VITE_MAPLE_BILLING_API_URL same pattern, VITE_CLIENT_ID constant). Out of scope (deferred to later epics): - release.yml integration (Epic 6 / PR 7) - Code signing secrets and signCommand (Epic 6 / PR 7) - latest.json windows-x86_64 entry for auto-updater (Epic 6 / PR 7) - MSI bundler (deferred indefinitely; WiX template shadows the HKCU deep-link keys we rely on) Closes MPLR-hzehaxpp (1e). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Verify the downloaded onnxruntime-win-x64-1.22.0.zip against a pinned hash before unzipping. Defense against a compromised or corrupted release artifact landing in the Windows installer. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
GNU sha256sum prefixes the hash line with a backslash when the filename contains one (e.g. D:\a\_temp\...), which broke the equality test against the pinned hash. Feed the file via stdin so the output has no filename. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes pinning gaps surfaced during PR review: - Linux ONNX Runtime tarball now SHA256-verified before extract, mirroring the Windows pattern. - sccache install switched to direct GitHub-release download with SHA256 verification on both Linux and Windows. Windows no longer depends on chocolatey for unversioned, unverified installs. - bundle.targets in tauri.conf.json already constrains the Windows build to NSIS; the duplicate --bundles nsis tauri-action arg is removed.
ldstreet
force-pushed
the
windows/pr1-compile-launch
branch
from
bbb7960 to
a4161cd
Compare
git-bash tar reads $RUNNER_TEMP as D:\a\_temp\... and interprets the drive-letter colon as a remote-host separator, failing with "Cannot connect to D:". --force-local disables that parse.
--force-local stopped tar from treating the drive letter as a host, but it still mangled D:\a\_temp when passed via -C. Switching to a subshell that cd's to RUNNER_TEMP first, then tar with a relative filename, sidesteps the MSYS2 tar path parser entirely.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
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/desktop-build.yml:
- Line 232: Replace tag-based GitHub Action references with fixed commit SHAs
for each action used (e.g., actions/checkout@v4, actions/setup-go@v1, any
`@stable/`@v0 usages) and add persist-credentials: false to every actions/checkout
step; specifically update the actions/checkout occurrences to include
persist-credentials: false and swap their `@vX` tags for the corresponding commit
SHA, and do the same SHA-pinning for other actions referenced with tags (e.g.,
setup-go, any `@stable/`@v0 actions) so all action refs are immutable.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6ce06445-9a9e-4b50-a82b-4d4707f23ec2
📒 Files selected for processing (5)
.github/workflows/desktop-build.ymlfrontend/src-tauri/capabilities/default.jsonfrontend/src-tauri/scripts/provide-linux-onnxruntime.shfrontend/src-tauri/src/proxy.rsfrontend/src-tauri/tauri.conf.json
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src-tauri/capabilities/default.json
- frontend/src-tauri/tauri.conf.json
| build-windows: | ||
| runs-on: windows-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify non-pinned action refs and checkout credential persistence settings
rg -nP '^\s*-\s*uses:\s*[^@]+@(?:v\d+|v0|stable|main|master)\s*$|persist-credentials' .github/workflows/desktop-build.ymlRepository: OpenSecretCloud/Maple
Length of output: 178
🏁 Script executed:
# Check the specific lines mentioned in the review
sed -n '230,242p; 263,267p; 303,322p' .github/workflows/desktop-build.yml | cat -nRepository: OpenSecretCloud/Maple
Length of output: 1666
Pin action references to commit SHAs and disable checkout credential persistence.
The Windows job uses tag-based action references (@v4, @v1, @stable, @v0) instead of commit SHAs, and the actions/checkout action lacks an explicit persist-credentials: false setting. This weakens CI supply-chain security guarantees and keeps credentials available longer than necessary.
Affected lines: 232, 235, 240, 265, 305, 320
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 232-232: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 232-232: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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/desktop-build.yml at line 232, Replace tag-based GitHub
Action references with fixed commit SHAs for each action used (e.g.,
actions/checkout@v4, actions/setup-go@v1, any `@stable/`@v0 usages) and add
persist-credentials: false to every actions/checkout step; specifically update
the actions/checkout occurrences to include persist-credentials: false and swap
their `@vX` tags for the corresponding commit SHA, and do the same SHA-pinning for
other actions referenced with tags (e.g., setup-go, any `@stable/`@v0 actions) so
all action refs are immutable.
Collaborator
Author
There was a problem hiding this comment.
Acknowledged — both points (SHA-pinning all third-party actions and persist-credentials: false on actions/checkout) are valid and apply repo-wide, not just to this new Windows job. Deferring to a focused follow-up PR that does the hardening across all three OS jobs (and release.yml) in one pass, rather than half-pinning the file here.
There was a problem hiding this comment.
@ldstreet — that's a sensible approach. Doing it repo-wide in one focused PR (covering build-macos, build-linux, build-windows, and release.yml) is cleaner than incremental half-measures here.
Would you like me to open a tracking GitHub issue for that hardening work? I can capture the scope (SHA-pinning all third-party action refs + persist-credentials: false on every actions/checkout across the affected workflow files) and link back to this PR for context.
🐇 📌
🧠 Learnings used
Learnt from: ldstreet
Repo: OpenSecretCloud/Maple PR: 520
File: .github/workflows/desktop-build.yml:256-266
Timestamp: 2026-05-18T14:56:20.265Z
Learning: In Maple CI workflows (e.g., the desktop build), do not treat `ORT_LIB_LOCATION` and `ORT_SKIP_DOWNLOAD=true` as “unused” environment variables. These are build-time inputs consumed by the `ort` crate’s own `build.rs` to locate a pre-fetched ONNX Runtime binary and to suppress its auto-download. Keep both set together—removing either can re-trigger the download or break the build—even though they won’t appear as reads in the project’s Rust source under `frontend/src-tauri/src/`.
Contributor
|
I've been making significant updates to the build processes, you'll likely want to rebase and standardize around them at some point |
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.
Summary
First of several PRs porting Maple to Windows. Lands the minimum needed to compile and launch on a clean Windows host plus a
windows-latestCI job that produces an unsigned NSIS installer on every PR and push to master.Why these changes
tauri.conf.json— explicitbundle.targets, NSIS per-user, WebView2 bootstrapper. Tauri's WiX/msi template hard-codes HKLM + per-machine install, which would shadow the HKCU keys we rely on forcloud.opensecret.maple://deep-link handlers. Switchingbundle.targetsfrom"all"to an explicit list excludes msi from future Windows builds without affecting mac/linux output.installMode: "currentUser"keeps the installer UAC-free and consistent with HKCU registration.webviewInstallMode: "downloadBootstrapper"pins behavior so a future Tauri default change doesn't surprise users.capabilities/default.json— drop dead$HOME/.config/maple/**entries. Those scope tokens don't resolve to anything meaningful on Windows and aren't used anywhere today — the JS never invokes the Tauri fs plugin andproxy.rswrites via rawstd::fs, which bypasses capabilities entirely. Removing them everywhere is cleaner than gating per-OS.proxy.rs— Windows path arm via Tauri's path resolver. The old code hardcoded$USERPROFILE/.config/maple/proxy_config.jsonon Windows, which is the wrong path convention. Plumbing&AppHandlethrough the helpers lets us useapp_config_dir()for the canonical%APPDATA%\cloud.opensecret.maple\. macOS/Linux behavior is byte-identical — atomic migration + keyring storage is deliberately deferred to keep this PR small.desktop-build.yml—windows-latestjob. Mirrors the existing Linux job: pre-fetches ONNX Runtime so the ort crate skips its own download, runstauri-action(bundle targets come fromtauri.conf.json), uploads the.exe. Authenticode signing is deliberately out of scope here — adding it would block on certificate provisioning. Updater minisigning uses the existing repo secret so the build doesn't error out after producing the installer. While here, also pinned SHA256 on the Linux sccache + ONNX Runtime downloads to match the new Windows install path.Manual smoke
Built and launched natively on a Parallels Win11 ARM64 VM (not the CI x64 artifact, but proves the same milestone). Installer ran per-user, no UAC. App launched, login screen rendered.
One gotcha worth flagging for future contributors: a missing
frontend/.env.localproduces a silent white-screen because Vite bakes env vars into the bundle at build time. Captured for a follow-up.Out of scope (later PRs)
Code signing, proxy keyring + atomic config migration, TTS DLL bundling, deep-link install-mode hardening, UI/downloads copy,
release.yml+ auto-updater Windows entry, full acceptance test pass.Test plan
build-windowsjob uploads NSIS artifactproxy.rsAppHandleplumbing orbundle.targetschange)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Security & Reliability
Chores