Conversation
There was a problem hiding this comment.
Pull request overview
This PR switches the Windows build pipeline from Ubuntu-based cross-compilation (MinGW/gnu) to native windows-latest builds targeting x86_64-pc-windows-msvc, updating both the desktop bundling and CLI build workflows accordingly.
Changes:
- Update Windows GitHub Actions workflows to build Rust artifacts natively on Windows using the MSVC target and adjust packaging/signing steps.
- Remove the
bundle:windowsnpm script and inline the equivalent Windows bundling commands in the workflow. - Replace
zipusage with7zfor Windows packaging and adjust signature verification to useGet-AuthenticodeSignature.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| ui/desktop/package.json | Removes the bundle:windows script (workflow now runs the commands directly). |
| .github/workflows/bundle-desktop-windows.yml | Switches desktop bundling to windows-latest + MSVC target; updates copying, signing verification, and zipping steps. |
| .github/workflows/build-cli.yml | Switches Windows CLI builds to windows-latest + MSVC target and updates Windows packaging to use 7z. |
| - name: Set up Node.js | ||
| uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0 | ||
| with: | ||
| node-version: 22 | ||
| node-version: 24.10.0 | ||
|
|
There was a problem hiding this comment.
This workflow updates node-version to 24.10.0, but the npm cache key/restore-keys below still hardcode node22, which can lead to reusing caches across incompatible Node versions (especially for native deps). Update the cache key/restore-keys to match Node 24 (or derive it dynamically) to avoid cross-version cache contamination.
| # Windows builds (only x86_64 supported) | ||
| - os: windows | ||
| architecture: x86_64 | ||
| target-suffix: pc-windows-gnu | ||
| build-on: ubuntu-latest | ||
| target-suffix: pc-windows-msvc | ||
| build-on: windows-latest |
There was a problem hiding this comment.
Now that Windows builds run on windows-latest, the earlier Update version in Cargo.toml step will run under PowerShell on Windows (default shell) and its current sed invocation relies on bash-style quoting/concatenation, which will fail when inputs.version is set. Make that version-update step explicitly use shell: bash on Windows (or rewrite it in PowerShell) so tagged/release builds don’t break.
| on: | ||
| workflow_dispatch: | ||
| workflow_call: |
There was a problem hiding this comment.
workflow_dispatch is enabled, so this workflow can now be run directly; please update the header comment that says it “doesn't get triggered on its own” to avoid misleading future maintainers.
There was a problem hiding this comment.
workflow_dispatch: this was for testing. i have removed it after the test
| # Download jsign | ||
| echo "📥 Downloading jsign..." | ||
| wget -q https://github.com/ebourg/jsign/releases/download/6.0/jsign-6.0.jar -O jsign.jar | ||
| curl -sL https://github.com/ebourg/jsign/releases/download/6.0/jsign-6.0.jar -o jsign.jar | ||
| echo "05ca18d4ab7b8c2183289b5378d32860f0ea0f3bdab1f1b8cae5894fb225fa8a jsign.jar" | sha256sum -c | ||
|
|
There was a problem hiding this comment.
In the signing step, the --tsaurl later in this script uses plain HTTP; use an HTTPS timestamp authority endpoint to avoid MITM tampering of timestamp responses.
There was a problem hiding this comment.
DigiCert's official timestamp URL is HTTP only: http://timestamp.digicert.com. Changing to HTTPS would break it.
This is actually safe because timestamp responses are cryptographically signed by the TSA (Time Stamp Authority) themselves. jsign verifies the TSA's signature before accepting the timestampThe response includes a digital signature, jsign verifies the TSA's signature before accepting the timestamp, so even over HTTP, it can't be tampered with. HTTPS isn't needed for integrity here - the protocol itself handles it.
This reverts commit 364ae9d.
| - name: Setup Rust (Windows) | ||
| if: matrix.os == 'windows' | ||
| shell: bash | ||
| run: | | ||
| echo "🚀 Building Windows CLI executable with enhanced GitHub Actions caching..." | ||
|
|
||
| # Create cache directories | ||
| mkdir -p ~/.cargo/registry ~/.cargo/git | ||
|
|
||
| # Use enhanced caching with GitHub Actions cache mounts | ||
| docker run --rm \ | ||
| -v "$(pwd)":/usr/src/myapp \ | ||
| -v "$HOME/.cargo/registry":/usr/local/cargo/registry \ | ||
| -v "$HOME/.cargo/git":/usr/local/cargo/git \ | ||
| -w /usr/src/myapp \ | ||
| rust:latest \ | ||
| bash -c " | ||
| set -e | ||
| echo '=== Setting up Rust environment with caching ===' | ||
| export CARGO_HOME=/usr/local/cargo | ||
| export PATH=/usr/local/cargo/bin:\$PATH | ||
|
|
||
| # Check if Windows target is already installed in cache | ||
| if rustup target list --installed | grep -q x86_64-pc-windows-gnu; then | ||
| echo '✅ Windows cross-compilation target already installed' | ||
| else | ||
| echo '📦 Installing Windows cross-compilation target...' | ||
| rustup target add x86_64-pc-windows-gnu | ||
| fi | ||
|
|
||
| echo '=== Setting up build dependencies ===' | ||
| apt-get update | ||
| apt-get install -y mingw-w64 protobuf-compiler cmake time | ||
|
|
||
| echo '=== Setting up cross-compilation environment ===' | ||
| export CC_x86_64_pc_windows_gnu=x86_64-w64-mingw32-gcc | ||
| export CXX_x86_64_pc_windows_gnu=x86_64-w64-mingw32-g++ | ||
| export AR_x86_64_pc_windows_gnu=x86_64-w64-mingw32-ar | ||
| export CARGO_TARGET_X86_64_PC_WINDOWS_GNU_LINKER=x86_64-w64-mingw32-gcc | ||
| export PKG_CONFIG_ALLOW_CROSS=1 | ||
| export PROTOC=/usr/bin/protoc | ||
|
|
||
| echo '=== Optimized Cargo configuration ===' | ||
| mkdir -p .cargo | ||
| echo '[build]' > .cargo/config.toml | ||
| echo 'jobs = 4' >> .cargo/config.toml | ||
| echo '' >> .cargo/config.toml | ||
| echo '[target.x86_64-pc-windows-gnu]' >> .cargo/config.toml | ||
| echo 'linker = \"x86_64-w64-mingw32-gcc\"' >> .cargo/config.toml | ||
| echo '' >> .cargo/config.toml | ||
| echo '[net]' >> .cargo/config.toml | ||
| echo 'git-fetch-with-cli = true' >> .cargo/config.toml | ||
| echo 'retry = 3' >> .cargo/config.toml | ||
| echo '' >> .cargo/config.toml | ||
| echo '[profile.release]' >> .cargo/config.toml | ||
| echo 'codegen-units = 1' >> .cargo/config.toml | ||
| echo 'lto = false' >> .cargo/config.toml | ||
| echo 'panic = \"abort\"' >> .cargo/config.toml | ||
| echo 'debug = false' >> .cargo/config.toml | ||
| echo 'opt-level = 2' >> .cargo/config.toml | ||
| echo '' >> .cargo/config.toml | ||
| echo '[registries.crates-io]' >> .cargo/config.toml | ||
| echo 'protocol = \"sparse\"' >> .cargo/config.toml | ||
|
|
||
| echo '=== Building with cached dependencies ===' | ||
| # Check if we have cached build artifacts | ||
| if [ -d target/x86_64-pc-windows-gnu/release/deps ] && [ \"\$(ls -A target/x86_64-pc-windows-gnu/release/deps)\" ]; then | ||
| echo '✅ Found cached build artifacts, performing incremental build...' | ||
| CARGO_INCREMENTAL=1 | ||
| else | ||
| echo '🔨 No cached artifacts found, performing full build...' | ||
| CARGO_INCREMENTAL=0 | ||
| fi | ||
|
|
||
| echo '🔨 Building Windows CLI executable...' | ||
| CARGO_INCREMENTAL=\$CARGO_INCREMENTAL \ | ||
| CARGO_NET_RETRY=3 \ | ||
| CARGO_HTTP_TIMEOUT=60 \ | ||
| RUST_BACKTRACE=1 \ | ||
| cargo build --release --target x86_64-pc-windows-gnu -p goose-cli --jobs 4 | ||
|
|
||
| echo '=== Copying Windows runtime DLLs ===' | ||
| GCC_DIR=\$(ls -d /usr/lib/gcc/x86_64-w64-mingw32/*/ | head -n 1) | ||
| cp \"\$GCC_DIR/libstdc++-6.dll\" target/x86_64-pc-windows-gnu/release/ | ||
| cp \"\$GCC_DIR/libgcc_s_seh-1.dll\" target/x86_64-pc-windows-gnu/release/ | ||
| cp /usr/x86_64-w64-mingw32/lib/libwinpthread-1.dll target/x86_64-pc-windows-gnu/release/ | ||
|
|
||
| echo '✅ Build completed successfully!' | ||
| ls -la target/x86_64-pc-windows-gnu/release/ | ||
| " | ||
| rustup show | ||
| rustup target add x86_64-pc-windows-msvc | ||
|
|
||
| sudo chown -R $USER:$USER target/ | ||
| - name: Build CLI (Windows) | ||
| if: matrix.os == 'windows' | ||
| shell: bash | ||
| run: | | ||
| echo "🚀 Building Windows CLI executable..." | ||
| cargo build --release --target x86_64-pc-windows-msvc -p goose-cli |
There was a problem hiding this comment.
These Windows steps call rustup/cargo without first installing a toolchain (unlike CI which uses actions-rust-lang/setup-rust-toolchain@v1), so the workflow is dependent on whatever happens to be preinstalled on windows-latest; add an explicit Rust toolchain setup step (or activate hermit, if supported) to make builds reproducible and avoid runner-image regressions.
| - name: Setup Rust | ||
| shell: bash | ||
| run: | | ||
| echo "🚀 Building Windows executable with enhanced GitHub Actions caching..." | ||
|
|
||
| # Create cache directories | ||
| mkdir -p ~/.cargo/registry ~/.cargo/git | ||
|
|
||
| # Use enhanced caching with GitHub Actions cache mounts | ||
| docker run --rm \ | ||
| -v "$(pwd)":/usr/src/myapp \ | ||
| -v "$HOME/.cargo/registry":/usr/local/cargo/registry \ | ||
| -v "$HOME/.cargo/git":/usr/local/cargo/git \ | ||
| -w /usr/src/myapp \ | ||
| rust:latest \ | ||
| bash -c " | ||
| set -e | ||
| echo '=== Setting up Rust environment with caching ===' | ||
| export CARGO_HOME=/usr/local/cargo | ||
| export PATH=/usr/local/cargo/bin:\$PATH | ||
|
|
||
| # Check if Windows target is already installed in cache | ||
| if rustup target list --installed | grep -q x86_64-pc-windows-gnu; then | ||
| echo '✅ Windows cross-compilation target already installed' | ||
| else | ||
| echo '📦 Installing Windows cross-compilation target...' | ||
| rustup target add x86_64-pc-windows-gnu | ||
| fi | ||
|
|
||
| echo '=== Setting up build dependencies ===' | ||
| apt-get update | ||
| apt-get install -y mingw-w64 protobuf-compiler cmake time | ||
|
|
||
| echo '=== Setting up cross-compilation environment ===' | ||
| export CC_x86_64_pc_windows_gnu=x86_64-w64-mingw32-gcc | ||
| export CXX_x86_64_pc_windows_gnu=x86_64-w64-mingw32-g++ | ||
| export AR_x86_64_pc_windows_gnu=x86_64-w64-mingw32-ar | ||
| export CARGO_TARGET_X86_64_PC_WINDOWS_GNU_LINKER=x86_64-w64-mingw32-gcc | ||
| export PKG_CONFIG_ALLOW_CROSS=1 | ||
| export PROTOC=/usr/bin/protoc | ||
|
|
||
| echo '=== Optimized Cargo configuration ===' | ||
| mkdir -p .cargo | ||
| echo '[build]' > .cargo/config.toml | ||
| echo 'jobs = 4' >> .cargo/config.toml | ||
| echo '' >> .cargo/config.toml | ||
| echo '[target.x86_64-pc-windows-gnu]' >> .cargo/config.toml | ||
| echo 'linker = \"x86_64-w64-mingw32-gcc\"' >> .cargo/config.toml | ||
| echo '' >> .cargo/config.toml | ||
| echo '[net]' >> .cargo/config.toml | ||
| echo 'git-fetch-with-cli = true' >> .cargo/config.toml | ||
| echo 'retry = 3' >> .cargo/config.toml | ||
| echo '' >> .cargo/config.toml | ||
| echo '[profile.release]' >> .cargo/config.toml | ||
| echo 'codegen-units = 1' >> .cargo/config.toml | ||
| echo 'lto = false' >> .cargo/config.toml | ||
| echo 'panic = \"abort\"' >> .cargo/config.toml | ||
| echo 'debug = false' >> .cargo/config.toml | ||
| echo 'opt-level = 2' >> .cargo/config.toml | ||
| echo '' >> .cargo/config.toml | ||
| echo '[registries.crates-io]' >> .cargo/config.toml | ||
| echo 'protocol = \"sparse\"' >> .cargo/config.toml | ||
|
|
||
| echo '=== Building with cached dependencies ===' | ||
| # Check if we have cached build artifacts | ||
| if [ -d target/x86_64-pc-windows-gnu/release/deps ] && [ \"\$(ls -A target/x86_64-pc-windows-gnu/release/deps)\" ]; then | ||
| echo '✅ Found cached build artifacts, performing incremental build...' | ||
| CARGO_INCREMENTAL=1 | ||
| else | ||
| echo '🔨 No cached artifacts found, performing full build...' | ||
| CARGO_INCREMENTAL=0 | ||
| fi | ||
|
|
||
| echo '🔨 Building Windows executable...' | ||
| CARGO_INCREMENTAL=\$CARGO_INCREMENTAL \ | ||
| CARGO_NET_RETRY=3 \ | ||
| CARGO_HTTP_TIMEOUT=60 \ | ||
| RUST_BACKTRACE=1 \ | ||
| cargo build --release --target x86_64-pc-windows-gnu --jobs 4 | ||
|
|
||
| echo '=== Copying Windows runtime DLLs ===' | ||
| GCC_DIR=\$(ls -d /usr/lib/gcc/x86_64-w64-mingw32/*/ | head -n 1) | ||
| cp \"\$GCC_DIR/libstdc++-6.dll\" target/x86_64-pc-windows-gnu/release/ | ||
| cp \"\$GCC_DIR/libgcc_s_seh-1.dll\" target/x86_64-pc-windows-gnu/release/ | ||
| cp /usr/x86_64-w64-mingw32/lib/libwinpthread-1.dll target/x86_64-pc-windows-gnu/release/ | ||
| rustup show | ||
| rustup target add x86_64-pc-windows-msvc | ||
|
|
||
| echo '✅ Build completed successfully!' | ||
| ls -la target/x86_64-pc-windows-gnu/release/ | ||
| " | ||
| - name: Build Windows executable | ||
| shell: bash | ||
| run: | | ||
| echo "🚀 Building Windows executable..." | ||
| cargo build --release --target x86_64-pc-windows-msvc |
There was a problem hiding this comment.
This workflow also relies on rustup/cargo being present on windows-latest but never installs Rust; add an explicit toolchain setup step (or activate hermit) before rustup target add/cargo build to avoid failures if the runner image changes.
| @@ -201,55 +118,60 @@ jobs: | |||
|
|
|||
| if [ -d "./ui/desktop/src/platform/windows/bin/goose-npm" ]; then | |||
| echo "Setting up npm environment..." | |||
| rsync -a --delete ./ui/desktop/src/platform/windows/bin/goose-npm/ ./ui/desktop/src/bin/goose-npm/ | |||
| cp -r ./ui/desktop/src/platform/windows/bin/goose-npm/ ./ui/desktop/src/bin/goose-npm/ | |||
| fi | |||
| echo "Windows-specific files copied successfully" | |||
There was a problem hiding this comment.
Windows platform files are copied into ui/desktop/src/bin here and then scripts/prepare-platform-binaries.js runs later and copies/cleans the same directory again, which duplicates logic and can drift; consider relying on the JS script for platform-file copying/cleanup and keep this step focused on only copying the freshly built goosed.exe.
alexhancock
left a comment
There was a problem hiding this comment.
This would be a great one to post a test build of in discord and get a few people to validate it on different windows machines
|
.bundle |
|
There is an alternative option where we build v8 ourselves: https://github.com/denoland/rusty_v8/blob/8b232b48a3378f933f56d5d8d6711e5906168409/README.md?plain=1#L45 but that would mean a slower and more expensive build of course. How much slower, I don't know. |
* origin/main: Docs: require auth optional for custom providers (#7098) fix: improve text-muted contrast for better readability (#7095) Always sync bundled extensions (#7057) feat: Add tom (Top Of Mind) platform extension (#7073) chore(docs): update GOOSE_SESSION_ID -> AGENT_SESSION_ID (#6669) fix(ci): switch from cargo-audit to cargo-deny for advisory scanning (#7032) chore(deps): bump @isaacs/brace-expansion from 5.0.0 to 5.0.1 in /evals/open-model-gym/suite (#7085) chore(deps): bump @modelcontextprotocol/sdk from 1.25.3 to 1.26.0 in /evals/open-model-gym/mcp-harness (#7086) fix: switch to windows msvc (#7080) fix: allow unlisted models for CLI providers (#7090) Use goose port (#7089) chore: strip posthog for sessions/models/daily only (#7079) tidy: clean up old benchmark and add gym (#7081) fix: use command.process_group(0) for CLI providers, not just MCP (#7083) added build notify (#6891)
* main: (125 commits) chore: add a new scenario (#7107) fix: Goose Desktop missing Calendar and Reminders entitlements (#7100) Fix 'Edit In Place' and 'Fork Session' features (#6970) Fix: Only send command content to command injection classifier (excluding part of tool call dict) (#7082) Docs: require auth optional for custom providers (#7098) fix: improve text-muted contrast for better readability (#7095) Always sync bundled extensions (#7057) feat: Add tom (Top Of Mind) platform extension (#7073) chore(docs): update GOOSE_SESSION_ID -> AGENT_SESSION_ID (#6669) fix(ci): switch from cargo-audit to cargo-deny for advisory scanning (#7032) chore(deps): bump @isaacs/brace-expansion from 5.0.0 to 5.0.1 in /evals/open-model-gym/suite (#7085) chore(deps): bump @modelcontextprotocol/sdk from 1.25.3 to 1.26.0 in /evals/open-model-gym/mcp-harness (#7086) fix: switch to windows msvc (#7080) fix: allow unlisted models for CLI providers (#7090) Use goose port (#7089) chore: strip posthog for sessions/models/daily only (#7079) tidy: clean up old benchmark and add gym (#7081) fix: use command.process_group(0) for CLI providers, not just MCP (#7083) added build notify (#6891) test(mcp): add image tool test and consolidate MCP test fixtures (#7019) ...
I see! I guess we can build v8 ourselves targeting It seems most of crates support |
|
@lifeizhou-ap I think your solution is the right way to go-- just wanted to mention the other one in case some other reason comes up to go back to the gnu build toolchain. |
* main: (85 commits) Fix 'Edit In Place' and 'Fork Session' features (#6970) Fix: Only send command content to command injection classifier (excluding part of tool call dict) (#7082) Docs: require auth optional for custom providers (#7098) fix: improve text-muted contrast for better readability (#7095) Always sync bundled extensions (#7057) feat: Add tom (Top Of Mind) platform extension (#7073) chore(docs): update GOOSE_SESSION_ID -> AGENT_SESSION_ID (#6669) fix(ci): switch from cargo-audit to cargo-deny for advisory scanning (#7032) chore(deps): bump @isaacs/brace-expansion from 5.0.0 to 5.0.1 in /evals/open-model-gym/suite (#7085) chore(deps): bump @modelcontextprotocol/sdk from 1.25.3 to 1.26.0 in /evals/open-model-gym/mcp-harness (#7086) fix: switch to windows msvc (#7080) fix: allow unlisted models for CLI providers (#7090) Use goose port (#7089) chore: strip posthog for sessions/models/daily only (#7079) tidy: clean up old benchmark and add gym (#7081) fix: use command.process_group(0) for CLI providers, not just MCP (#7083) added build notify (#6891) test(mcp): add image tool test and consolidate MCP test fixtures (#7019) fix: remove Option from model listing return types, propagate errors (#7074) fix: lazy provider creation for goose acp (#7026) (#7066) ...
Summary
Switched our Windows released artifacts from
windows gnutowindows msvcand this fixed generating bundle Windows artifactsWith this PR #6765, the bundle windows artifacts failed, as the dependency (
pctx_code_mode) that introduced in the PR only can work inwindows msvc.Options to fix are:
windows msvcIt seems that
windows msvcworks better with more compatibility with Windows system library, and most of the window specific crates supportmsvc. So Option 2 seems to be a better solutionType of Change
Testing
Tested on a real Windows machine.
Tried a couple of ways such as using parallel or virtual box. Either does not work. I cannot download Parallel on the company laptop. For virtual box, I cannot download microsoft windows preview image.
I feel in the future we should create a virtual windows machine in Cloud such as AWS to make the testing easier.