Repository navigation
Conversation
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (1)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between 3771e28dc374c795a4cefd6b0c9200fef276297d and 0f6d9f3. 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review. 📝 WalkthroughWalkthroughThe WSL launcher now changes the converted bridge script path from the Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The change narrowly normalizes the bridge script path while preserving other paths. No actionable merge-blocking risk is identified; Windows Group Policy validation remains unperformed. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change is narrowly scoped to the script path and preserves the existing launcher, arguments, and execution options. No exploitable security regression was established, but the effects on Windows trust decisions have not been validated under the affected Group Policy. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Hardening Proposals
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description explains the problem, focused fix, scope, behavior, issue reference, and test evidence. However, it does not follow the repository template and omits required sections such as ELI5, What Changed, Why, Visual Proof, Testing checkboxes, Review, Agent skill upstream boundary, Notes, and Checklist. Resolution Restructure the description using the repository template. Add the missing required sections, mark Visual Proof as N/A with a reason if applicable, complete the Testing and Checklist items, and retain the existing technical explanation and test evidence.
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 |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This PR works around #20082: on Windows machines whose PowerShell policy zones \\wsl.localhost\ as Internet, executing the Orca bridge with powershell.exe -File fails with AuthorizationManager check failed (or prompts interactively) before the bridge starts. The fix rewrites only the bridge script path to the legacy \\wsl$\ share, which resolves to the same running distro but is classified as intranet.
- Bridge UNC rewrite — after
wslpath -wresolves the bridge path,buildLauncherrewrites a leading\\wsl.localhost\to\\wsl$\. A path already on\\wsl$\or a Windows drive path is left unchanged, andORCA_WSL_CWD_WIN(passed only as-WslCwd, never executed as a script) keeps the modern spelling. - Test coverage — the launcher test pins the exact generated rewrite line, executes it under real
bashagainst\\wsl.localhost\Ubuntu-24.04\..., and asserts the cwd UNC is not rewritten.
I verified the change is scoped to the one script-execution path (only wsl-cli-scripts.ts emits wslpath -w), that the focused suite passes (24/24), and that oxlint/oxfmt are clean. I also confirmed the load-bearing external contract: \\wsl$ is a supported, non-deprecated compatibility alias for \\wsl.localhost (Microsoft WSL release notes Build 21354, and microsoft/WSL src/linux/init/util.cpp still defines PLAN9_RDR_COMPAT_PREFIX). The rewrite is a no-op on non-matching paths, so it cannot regress a setup where the modern spelling already worked.
Two notes for the record, neither blocking:
- The PR body says this was not validated on a real Windows host with the offending GPO; the correctness argument rests on the issue's reproduction plus the zone/spelling analysis. Worth a manual smoke test on a GPO-managed machine if one is available.
- The only documented asymmetry —
\\wsl$may not auto-start a stopped distro while\\wsl.localhostcan — does not apply, because this launcher runs from inside the distro.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
PowerShell's AuthorizationManager rejects noninteractive -File on \\wsl.localhost\ script paths. Rewrite only that executed bridge UNC.
The launcher test only executed the \\wsl.localhost rewrite, so a later change could rewrite those two paths and still pass. Co-authored-by: Cursor <cursoragent@cursor.com>
3771e28 to
0f6d9f3
Compare
Sync update (
|
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run re-reviewed the delta since the prior pullfrog review (3771e28). The only change is test-only: it closes the pass-through coverage gap raised on the greptile thread.
- Added pass-through assertions — the launcher test now builds a
rewriteBridgePathhelper that re-runs the generated UNC substitution under realbashfor an already-\\wsl$path and aC:\...drive path, asserting both are returned unchanged. The existing\\wsl.localhost\rewrite assertion is untouched.
The pass-through cases can genuinely fail (they assert exact equality against the substitution's real bash output, not a loose match), and the focused suite passes 24/24 locally.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Description
The managed WSL CLI launcher passes
wslpath -woutput straight topowershell.exe -File. On Windows that path is\\wsl.localhost\<distro>\.... PowerShell's AuthorizationManager then refuses the script before the bridge starts, either with an interactive trust prompt or, noninteractively,AuthorizationManager check failed. The same script on the\\wsl$\share runs.Focused fix
wslpath -wof the bridge script, rewrite a leading\\wsl.localhost\to\\wsl$\and pass that path to-File.-WslCwd),wslpathitself, execution policy, and any other UNC consumer.Preserves
\\wsl$\, or a normal Windows drive path, is left unchanged.-WslCwdstays the modern\\wsl.localhost\spelling.Evidence
node node_modules/vitest/vitest.mjs run --config config/vitest.config.ts src/main/cli/wsl-cli-installer.test.ts\\wsl.localhost\Ubuntu-24.04\tmp\orca-wsl-bridge.ps1becomes\\wsl$\Ubuntu-24.04\tmp\orca-wsl-bridge.ps1.\\wsl.localhost\.User-regression-tradeoffs
Fixes #20082