Repository navigation
refactor(preflight): rename execCommandInWsl to execCommandInWslOrThrow - #17375
Conversation
Pure rename, no behavior change. Callers like isCommandAvailable and
isCommandOnPath wrap this in try { ... return true } catch { return false },
collapsing "distro unreachable" and "command absent" into the same value —
a recurring bug class in this subsystem (see
docs/reference/wsl-probe-failure-semantics.md). The OrThrow suffix makes
that swallow visible at the call site instead of implicit in the function
name, so a reviewer notices when a new caller does the same collapse.
Greptile SummaryThis PR mechanically renames the internal WSL command executor to make its throwing contract explicit, without changing runtime behavior.
Confidence Score: 5/5The PR appears safe to merge because the rename is complete and introduces no behavioral change. All repository references to the internal executor use the new name, with no stale imports, re-exports, mocks, configuration references, or public API exposure identified.
|
| Filename | Overview |
|---|---|
| src/main/ipc/preflight-command-exec.ts | Renames the exported WSL executor and its two internal references while preserving its implementation and signature. |
| src/main/preflight/agent-detection.ts | Updates the import and both authentication-probe call sites to use the renamed executor. |
| docs/reference/wsl-probe-failure-semantics.md | Updates the documented failure-semantics example to use the new symbol name. |
Reviews (1): Last reviewed commit: "refactor(preflight): rename execCommandI..." | Re-trigger Greptile
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe WSL command helper is renamed to Merge Risk: ⚪ Minimal · up to This PR only renames a throwing WSL command helper and updates its references and documentation without changing behavior; the reported checks are clean, and no actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description clearly explains the motivation, scope, affected files, follow-up boundary, and verification. However, it omits several required template sections, including ELI5, Linked Issue with a Fixes # entry, Visual Proof or N/A, AI Disclosure, Review, Agent skill upstream boundary, Notes, and the Checklist. Resolution Complete the required template sections. Add an ELI5 summary, a Linked Issue using the required Fixes # format, Visual Proof with N/A and a reason if applicable, AI Disclosure, Review, Agent skill upstream boundary, Notes, and the Checklist. Keep the existing change and verification details. Full details: Docstring CoverageExplanation Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Export renamed to
execCommandInWslOrThrow— the throwing contract is now in the name, with a concise doc comment stating that an unreachable distro and a genuinely missing command both throw and must not be collapsed into "absent". - Call sites updated — both internal call sites in
preflight-command-exec.ts(isCommandAvailable,isCommandOnPath) and the twogh/glabauth-status call sites inagent-detection.tsnow use the new name. - Reference doc aligned — the canonical example in
docs/reference/wsl-probe-failure-semantics.mdshows the name a reader will actually grep for.
I confirmed the rename is complete (no remaining references to execCommandInWsl), the new doc comment matches the throwing behavior in runPreflightCommandInWsl (throws on timeout and non-zero exit), and no tests reference the symbol directly — the mocks target ./preflight-wsl-command, so the claim that no test changes are needed holds.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
…17380) Sibling of execCommandInWslOrThrow (#17375) with the identical throwing contract, sitting in the same `try { ... } catch { return false }` blocks. After that rename the pair read inconsistently — one announced that it throws, the other did not, while both collapse to a silent false at the call site. Also states the contract in a doc comment: it rejects rather than reporting "absent", so a caller that swallows the rejection makes "not installed" and "could not run it" the same answer.
…ow (stablyai#17375) Pure rename, no behavior change. Callers like isCommandAvailable and isCommandOnPath wrap this in try { ... return true } catch { return false }, collapsing "distro unreachable" and "command absent" into the same value — a recurring bug class in this subsystem (see docs/reference/wsl-probe-failure-semantics.md). The OrThrow suffix makes that swallow visible at the call site instead of implicit in the function name, so a reviewer notices when a new caller does the same collapse.
…tablyai#17380) Sibling of execCommandInWslOrThrow (stablyai#17375) with the identical throwing contract, sitting in the same `try { ... } catch { return false }` blocks. After that rename the pair read inconsistently — one announced that it throws, the other did not, while both collapse to a silent false at the call site. Also states the contract in a doc comment: it rejects rather than reporting "absent", so a caller that swallows the rejection makes "not installed" and "could not run it" the same answer.
Why
Orca has a recurring WSL bug class: a probe that cannot reach its distro returns the same value as one that asked and got "no". It has shipped three times — the preflight CLI cache (#17350), the
glab auth statusVM wakeups (#8941), andlistRunningWslDistrosAsyncfailing closed (#17072).execCommandInWslthrows on failure, and its callers wrap it like this:Naming it
...OrThrowputs the throwing contract in front of whoever writes thatcatch, so the swallow is visible at the call site rather than something you have to go read the callee to discover. It pairs with the ratchet and reference doc added in #17352.Change
Pure mechanical rename — no behaviour, signature, or error-handling changes.
src/main/ipc/preflight-command-exec.ts— export renamed, both internal call sites updated, doc comment now states the throwing contract and that callers must distinguish "absent" from "unreachable".src/main/preflight/agent-detection.ts— import + 2 call sites (gh auth status,glab auth status).docs/reference/wsl-probe-failure-semantics.md— the canonical example now shows the name a reader will actually grep for.5 call sites total. No test changes needed — mocks target the module, not this symbol.
Follow-up, deliberately not here
execLocalPreflightCommand(same file) has the identical throwing contract and sits in the sametry { … } catch { return false }blocks. After this rename the pair reads inconsistently — one saysOrThrow, the other doesn't, while both collapse to a silentfalse. Worth its own small PR; kept out to keep this diff mechanical.Verification
pnpm run typecheck(full, not justtc:node) — cleanpnpm test src/main/ipc/preflight src/main/wsl/wsl-probe-failure-semantics.test.ts— 83 passed / 1 skippedoxlinton both changed source files — clean