-
Notifications
You must be signed in to change notification settings - Fork 1.6k
cloudflare-warp: Add version 2025.9.558.0 #16945
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThis PR adds Cloudflare WARP support to the package manager. It introduces a manifest file defining package metadata, versioning, download source, and auto-update logic for the Windows MSI installer, along with a PowerShell script that configures the Cloudflare WARP Windows service, including privilege validation, service lifecycle management, and startup logic. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
|
All changes look good. Wait for review from human collaborators. cloudflare-warp
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
bucket/cloudflare-warp.json (1)
6-6: Improve user experience by clarifying service setup requirements.The note implies the service requires manual fixing after installation, which creates friction. Consider rewording to be clearer about when and why this step is needed.
✍️ Suggested rewording
- "notes": "Fix service by running: $dir\\service.ps1", + "notes": "To set up the Windows service, run as administrator: $dir\\service.ps1",scripts/cloudflare-warp/service.ps1 (1)
28-28: Consider using PowerShell cmdlets consistently.The script mixes
sc.exe(line 28) with PowerShell cmdlets likeStop-ServiceandNew-Service. Whilesc.exe deleteworks, usingRemove-Service(available in PowerShell 6+) would be more consistent. However, if backward compatibility with Windows PowerShell 5.1 is required,sc.exeis acceptable.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
bucket/cloudflare-warp.jsonscripts/cloudflare-warp/service.ps1
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-29T22:02:34.868Z
Learnt from: Ra2-IFV
Repo: ScoopInstaller/Extras PR: 16672
File: bucket/chromium-clang.json:12-12
Timestamp: 2025-11-29T22:02:34.868Z
Learning: In the chromium-clang.json manifest (bucket/chromium-clang.json), the extract_dir value "chrome-win32" is a fixed name determined by the upstream Chromium_Clang archive structure and should not be changed, even though the manifest targets 64-bit architecture.
Applied to files:
bucket/cloudflare-warp.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: WindowsPowerShell
🔇 Additional comments (12)
bucket/cloudflare-warp.json (7)
14-14: LGTM!The post_install correctly copies the service script to the installation directory, making it accessible to users at the documented path.
15-20: LGTM!The shortcuts configuration correctly maps the executable to a user-friendly name in the Start Menu.
21-24: LGTM!The checkver configuration uses the appropriate Sparkle update feed to detect new versions.
25-31: LGTM!The autoupdate configuration correctly constructs version-specific download URLs using the same pattern as the main download URL.
9-9: No change needed—the download URL is correct.The
downloads.cloudflareclient.comdomain is the direct, official download endpoint maintained by Cloudflare. Testing confirms it returns HTTP 200 with the MSI file. The1111-releases.cloudflareclient.comdomain is a redirect that ultimately points to the samedownloads.cloudflareclient.comURL, making it less efficient. The manifest correctly uses the direct download URL.Likely an incorrect or invalid review comment.
3-3: No changes needed. The URLhttps://1.1.1.1is the correct and official consumer-facing homepage for Cloudflare's 1.1.1.1/WARP application. The official Cloudflare WARP product documentation is athttps://www.cloudflare.com/warp/, but for a cloudflare-warp package manifest, the direct download/consumer hub URL athttps://1.1.1.1is appropriate.Likely an incorrect or invalid review comment.
13-13: MSI extract_dir verification requires manual inspection of the installer.The
extract_dirvalue"Cloudflare\\Cloudflare WARP"cannot be verified without downloading and inspecting the MSI file's internal directory structure. While the syntax is correct and the shortcut reference aligns with the path structure, only the package maintainer or direct inspection of the installer can confirm the path matches what the MSI actually contains.scripts/cloudflare-warp/service.ps1 (5)
1-8: LGTM!The administrator rights check correctly validates elevation before attempting service operations, with clear error messaging.
16-21: LGTM!The script properly validates that
warp-svc.exeexists before attempting service creation, preventing cryptic failures later.
23-30: LGTM!The cleanup of existing services is handled properly with appropriate error suppression and a delay to ensure cleanup completes.
42-50: LGTM!The service startup includes proper error handling with informative messages, gracefully handling startup failures while still reporting success for the overall setup.
35-35: Reconsider removing the WlanSvc dependency—it appears to be required for Cloudflare WARP to function.While Cloudflare's official documentation doesn't explicitly list WlanSvc as a requirement, community reports and GitHub issues confirm that WARP fails to start or register when WLAN AutoConfig is unavailable. This dependency is not specific to systems with WiFi hardware; it's a runtime requirement for WARP itself. Removing it would likely break the service on many Windows installations.
The
-StartupType Manualdefault may warrant discussion for a VPN-like service, but that's a separate design decision independent of the WlanSvc dependency.Likely an incorrect or invalid review comment.
Closes #10810
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.