ci: add windows-latest build job so Windows compile/link breaks fail PRs - #6610
Conversation
📝 WalkthroughWalkthroughAdds a Windows-only GitHub Actions job that installs stable Rust, enables caching, and builds the compiler, runtime, standard library, and Windows UI crates with the ChangesWindows build CI
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.github.com/in/347564?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/test.yml:
- Line 625: Update the actions/checkout step in the PR build job to set
persist-credentials to false, preventing the GitHub token from being stored in
local Git configuration while leaving the existing checkout behavior unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6d6b436a-2028-4ce2-956b-24dbe34c25af
📒 Files selected for processing (1)
.github/workflows/test.yml
| # timeout on a required-path job is a deterministic PR blocker). | ||
| timeout-minutes: 75 | ||
| steps: | ||
| - uses: actions/checkout@v7 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant workflow section around the reported line.
sed -n '600,650p' .github/workflows/test.yml
# Show the workflow triggers and top-level permissions if present.
python3 - <<'PY'
from pathlib import Path
import re
p = Path('.github/workflows/test.yml')
text = p.read_text()
for key in ['on:', 'permissions:']:
m = re.search(rf'(^\s*{key}.*$)', text, re.M)
print(f"\n== first {key} line ==")
print(m.group(1) if m else 'not found')
PYRepository: PerryTS/perry
Length of output: 3005
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map all uses of actions/checkout in workflows to see if this repo consistently disables credentials.
rg -n "actions/checkout@v7|persist-credentials" .github/workflows -SRepository: PerryTS/perry
Length of output: 3401
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the workflow header to inspect triggers and permissions.
sed -n '1,120p' .github/workflows/test.yml
# Inspect the exact job block around the reported checkout step.
sed -n '610,640p' .github/workflows/test.ymlRepository: PerryTS/perry
Length of output: 6956
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the event names used by this workflow.
rg -n "^\s*(pull_request|pull_request_target|push|schedule|workflow_dispatch|workflow_call)\b" .github/workflows/test.yml
# Show the job name that contains the reported checkout step.
python3 - <<'PY'
from pathlib import Path
text = Path('.github/workflows/test.yml').read_text().splitlines()
for i in range(620, 633):
print(f"{i+1}: {text[i]}")
PYRepository: PerryTS/perry
Length of output: 735
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,90p' .github/workflows/test.ymlRepository: PerryTS/perry
Length of output: 3980
🏁 Script executed:
#!/bin/bash
set -euo pipefail
nl -ba .github/workflows/test.yml | sed -n '618,633p'Repository: PerryTS/perry
Length of output: 191
Disable checkout credential persistence for the PR build job.
actions/checkout writes ${{ github.token }} into the local Git config by default. Since this job runs Cargo build scripts and procedural macros on pull requests, set persist-credentials: false so untrusted build code can’t reuse the token.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 625-625: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 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/test.yml at line 625, Update the actions/checkout step in
the PR build job to set persist-credentials to false, preventing the GitHub
token from being stored in local Git configuration while leaving the existing
checkout behavior unchanged.
Source: Linters/SAST tools
actions/checkout leaves the workflow token in .git/config by default; the windows-build job compiles PR-controlled build scripts and proc macros, so it should not have the token on disk. Flagged by zizmor (artipacked) via the CodeRabbit review on #6610. The 16 pre-existing checkout steps in this workflow share the same default; sweeping them is left as a maintainer decision since some jobs may rely on the persisted token. Co-authored-by: Ralph Kuepper <ralph@skelpo.com>
1 participant
What
Adds a
windows-buildjob to.github/workflows/test.yml(the required-checks workflow). It runs onwindows-latestand builds:That package set covers the compiler binary (so
perry.exeactually links), the runtime/stdlib pair (the usual home ofcfg(windows)externs), and both Windows UI crates — exactly the cratescargo-test's ubuntu runner must--excludeand therefore never compiles. Setup mirrors the workflow's existing conventions:actions/checkout@v7,dtolnay/rust-toolchain@stable,Swatinem/rust-cache@v2withshared-key: "${{ runner.os }}-perry"(auto-keyed per OS) and the same main-onlysave-if(the nightly cron warms it, sincescheduleruns onrefs/heads/main).Why
Every job in this workflow runs exclusively on ubuntu/macos, so Windows builds of
mainbreak silently. Two real, current examples that shipped through green required checks:perry-runtime— theExitProcessextern declaration incrates/perry-runtime/src/process/env_misc.rs(Windows-onlycfg, so Unix CI never type-checks it). Fix in flight onfix/windows-exitprocess-never.js_crypto_ed25519_verifyunresolved when linkingperry.exe— the extern is declared inperry-updaterbut only defined inperry-stdlib; Unix linkers dead-strip the unused reference, MSVC'slink.exehard-errors. Fix in a parallel PR.This job would have turned both into a red PR check instead of a broken
main.Why the perry-dev profile
perry-dev(opt-level 1, codegen-units 16, no LTO — see[profile.perry-dev]inCargo.toml) builds in minutes instead of the ~30+ a--release/LTO build takes, while still driving the full compile and link ofperry.exe— so both the E0308 class and the LNK2019 class are caught.--releasewould be far too slow for per-PR CI and adds nothing for this gate.Notes
timeout-minutes: 75: cold Windows builds are slow and this workflow never runs on pushes to main, so PR runs after a cache-less night can be near-cold; a timeout on a required-path job is a deterministic PR blocker (same reasoning as the conformance-smoke bump in CI: conformance-smoke shards hit the 60-min timeout-minutes cap; slowest shard cancels and blocks merges #6456).Summary by CodeRabbit