Repository navigation
feat(wsl): Add WSL distro host UI and status (WSL-via-orcad C7b, experimental) - #27934
Conversation
748bb69 to
cba0769
Compare
dd1f760 to
e03eef2
Compare
cba0769 to
6168b4a
Compare
6168b4a to
e4f518b
Compare
e03eef2 to
893f58f
Compare
54961ca to
c167054
Compare
678f9e4 to
82b4a9c
Compare
3387656 to
c6bd817
Compare
cec0e91 to
84d5cc7
Compare
… e2e (WSL-via-orcad C7b) Experimental toggle (Windows only), an Add WSL distro choice beside SSH host and remote server, the WSL host's own row in host pickers with its liveness state, a Connect action that starts a stopped distro only on request, a WSL hosts list in Settings, and the renderer report of which WSL hosts an open project or terminal needs.
…and confirm their removal With Experimental → WSL distros as hosts off, a WSL host keeps its row so its projects never seem to vanish, says it is turned off, and offers Turn on (opening that setting) instead of Connect; Settings lists it the same way with Connect disabled. Removing a WSL host now asks first in the dialog SSH hosts use, with WSL copy. The e2e covers both.
…g an add The UI reads main's template capability once. Without it, the add dialog says this build can't run WSL hosts and disables Add, existing hosts' rows and Settings say the same with Connect disabled, and the Experimental toggle notes it. The e2e covers a fake build without the files.
…turns, and on Refresh The add dialog read the registered distros once, so one imported while it was open never appeared. It now re-reads when the window regains focus or becomes visible, and from an explicit Refresh button, never on a timer. The selection is derived from the current list, so a refresh cannot leave a stale pick. The e2e imports a distro mid-dialog.
…ote servers Settings → Remote Orca Servers, the server-update checks and the status bar's server list now leave out environments whose deployment record is a WSL host (by kind, never by name). The WSL hosts section reads them from the store itself.
…t it left running The confirmation now says removing ends the host's open terminals as well as Orca's server, and a removal main could not complete in the distro (a terminal or the daemon kept running) is reported instead of passing silently.
The build resolves copy from the English catalog, which still held the old text under the reused key, so the new inline default never reached the dialog. The copy gets its own key, the stale entry is dropped from both catalogs, and a test reads the shipped catalog.
…up, and prove removal in e2e After removal the active workspace is left when it lived on that host, so nothing keeps calling a host that no longer exists, and a warning about what still runs in the distro stays until dismissed. The e2e now checks that removal ends the host's terminals on the server standing in for orcad, shows no warning, leaves the workspace, and starts nothing for the host afterwards.
…ithout reflection
…connecting' With the experiment off (or without server files) nothing retries, so a WSL host's terminal now says so, with Turn on, rather than claiming Orca is reconnecting.
…n e2e The headless server standing in for orcad now carries a project whose terminal must end when the project is removed (and leave the host's own catalog), and an orphan terminal whose project the host already forgot, which removing the host must still end. Both checks fail without the fixes.
…ors; offer Turn on when WSL hosts are off The file explorer showed "Error invoking remote method 'runtimeEnvironments:call': Error: ..." for a WSL workspace whose host was turned off. Explorer read errors now go through the shared IPC-error unwrapping, and when the workspace's WSL host is turned off the empty pane shows the same turned-off copy and Turn on button as the terminal banner instead of a read error and a Retry that cannot work. The editor's file-load error view drops the same wrapper at render time (stored text is unchanged for retry classifiers).
…losed gate shows nothing The fake-transport e2e launches its desktop with ORCA_INTERNAL_WSL_HOSTS=1. A new case leaves the gate closed with a WSL host already saved and the Experimental setting on: no WSL host API, list entry, host row, Add WSL distro action, Experimental toggle or Settings section appears, the fake never starts a distro, and the saved host stays on disk.
c6bd817 to
3fb1e7a
Compare
|
Merge-train review (blast radius outside WSL): LOW. With the internal gate closed the preload API is absent, so no WSL bridge listener or call runs. Two ungated notes:
While rebasing, I dropped the stale copy of "stop a project's terminals before the host forgets the project", since #28024 already landed it on main. The status bar resolution keeps main's managed-host dedupe (#28277) and filters with |
📝 Walkthrough
Priority: ➖ Normal Merge Risk: 🔵 Low · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 51.52% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 66 functions across 52 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly identifies the main change: experimental WSL distro host UI and status support. The extra milestone context does not obscure the primary change. Description check The description is mostly complete. It includes the user impact, implementation scope, rationale, testing details, visual proof, follow-up changes, and compatibility notes. The Linked Issue field is n… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
- Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
-
📝 Generate docstrings 💡
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks 💡
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
cc930c32-4834-4db9-9364-7f59c6559a30
📒 Files selected for processing (57)
src/renderer/src/components/editor/EditorFileLoadErrorView.test.tsxsrc/renderer/src/components/editor/EditorFileLoadErrorView.tsxsrc/renderer/src/components/right-sidebar/FileExplorer.tsxsrc/renderer/src/components/right-sidebar/FileExplorerFilesTreePane.errors.test.tsxsrc/renderer/src/components/right-sidebar/FileExplorerFilesTreePane.tsxsrc/renderer/src/components/right-sidebar/FileExplorerTreeStatus.tsxsrc/renderer/src/components/right-sidebar/file-explorer-expanded-dirs-refresh.tssrc/renderer/src/components/right-sidebar/file-explorer-read-error.test.tssrc/renderer/src/components/right-sidebar/file-explorer-read-error.tssrc/renderer/src/components/right-sidebar/useFileExplorerTree.tssrc/renderer/src/components/settings/ExperimentalPane.tsxsrc/renderer/src/components/settings/ManagedServersSection.tsxsrc/renderer/src/components/settings/WslHostsExperimentalSetting.tsxsrc/renderer/src/components/settings/WslHostsSection.tsxsrc/renderer/src/components/settings/experimental-search.tssrc/renderer/src/components/settings/use-runtime-environment-catalog.tssrc/renderer/src/components/settings/wsl-hosts-experimental-search.tssrc/renderer/src/components/sidebar/AddRemoteHostDialog.tsxsrc/renderer/src/components/sidebar/AddRepoHostSelector.test.tsxsrc/renderer/src/components/sidebar/AddRepoHostSelector.tsxsrc/renderer/src/components/sidebar/AddRepoHostSelectorSlot.tsxsrc/renderer/src/components/sidebar/AddWslHostDialogBody.tsxsrc/renderer/src/components/sidebar/AddWslHostFormPanel.test.tsxsrc/renderer/src/components/sidebar/AddWslHostFormPanel.tsxsrc/renderer/src/components/sidebar/HostRemoveDialog.tsxsrc/renderer/src/components/sidebar/add-repo-host-availability.tssrc/renderer/src/components/sidebar/sidebar-host-options.tssrc/renderer/src/components/sidebar/use-add-repo-host-selection.tssrc/renderer/src/components/sidebar/use-sidebar-host-scope-options.tssrc/renderer/src/components/sidebar/use-wsl-distro-list.test.tssrc/renderer/src/components/sidebar/use-wsl-distro-list.tssrc/renderer/src/components/sidebar/use-wsl-hosts-enabled.tssrc/renderer/src/components/sidebar/worktree-list/listing/use-section-rows.tssrc/renderer/src/components/sidebar/wsl-host-copy-catalog.test.tssrc/renderer/src/components/sidebar/wsl-host-removal.test.tssrc/renderer/src/components/sidebar/wsl-host-removal.tssrc/renderer/src/components/sidebar/wsl-host-row-status.test.tssrc/renderer/src/components/sidebar/wsl-host-row-status.tssrc/renderer/src/components/status-bar/SshStatusSegment.tsxsrc/renderer/src/components/terminal-pane/TerminalPaneRuntimePortals.tsxsrc/renderer/src/components/terminal-pane/TerminalRemoteRuntimeReconnectBanner.tsxsrc/renderer/src/components/terminal-pane/WslHostUnavailableBanner.tsxsrc/renderer/src/components/terminal-pane/use-wsl-host-unavailable-reason.test.tssrc/renderer/src/components/terminal-pane/use-wsl-host-unavailable-reason.tssrc/renderer/src/hooks/ipc-events/app-lifetime-ipc-bridge.tssrc/renderer/src/hooks/ipc-events/wsl-host-ipc-bridge.test.tssrc/renderer/src/hooks/ipc-events/wsl-host-ipc-bridge.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/lib/ipc-error.tssrc/renderer/src/store/index.tssrc/renderer/src/store/slices/remote-server-updates.tssrc/renderer/src/store/slices/store-test-helpers.tssrc/renderer/src/store/slices/wsl-hosts.tssrc/renderer/src/store/types.tssrc/shared/runtime-environments-listed-remote-server.test.tssrc/shared/runtime-environments.tstests/e2e/wsl-host-fake-transport.spec.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.
| /** Mirrors WSL host status into the store and tells main which hosts the user has open. */ | ||
| export function registerWslHostIpcBridge(unsubs: (() => void)[]): void { | ||
| const api = window.api.runtimeEnvironments?.wslHosts | ||
| // An older preload, or a partial test bridge, lacks the WSL host namespace. | ||
| if (typeof api?.onStatusChanged !== 'function') { | ||
| return | ||
| } | ||
| const apply = (status: WslHostStatus): void => useAppStore.getState().applyWslHostStatus(status) | ||
| // A preload from before the capability call leaves it unknown, which the UI treats as present. | ||
| if (typeof api.getCapability === 'function') { | ||
| void api | ||
| .getCapability() | ||
| .then(({ serverFiles }) => useAppStore.getState().setWslHostServerFiles(serverFiles)) | ||
| .catch((error) => console.error('Failed to read WSL host capability:', error)) | ||
| } | ||
| let stopped = false | ||
| unsubs.push(api.onStatusChanged(apply), () => { | ||
| stopped = true | ||
| }) | ||
| void api | ||
| .getStatuses() | ||
| .then((statuses) => { | ||
| if (!stopped) { | ||
| statuses.forEach(apply) | ||
| } | ||
| }) | ||
| .catch((error) => console.error('Failed to read WSL host statuses:', error)) | ||
|
|
||
| let reported = '' |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '55,118p' src/renderer/src/hooks/ipc-events/wsl-host-ipc-bridge.ts
sed -n '1,55p' src/renderer/src/store/slices/wsl-hosts.ts
rg -n 'getStatuses|onStatusChanged' src/main/wsl src/main/ipc src/preloadRepository: stablyai/orca
Length of output: 4533
🏁 Script executed:
printf '%s\n' '--- service ---'
sed -n '1,190p' src/main/wsl/wsl-host-service.ts
printf '%s\n' '--- preload API ---'
sed -n '1,110p' src/preload/api/wsl-hosts-api.ts
printf '%s\n' '--- handlers ---'
sed -n '1,70p' src/main/ipc/wsl-host-handlers.ts
sed -n '155,195p' src/main/ipc/wsl-host-handlers.ts
printf '%s\n' '--- status type and references ---'
rg -n -F --glob '*.{ts,tsx}' -e 'WslHostStatus' -e 'applyWslHostStatus' -e 'statusChanged' src/shared src/main src/preload src/renderer tests || test "$?" -eq 1
printf '%s\n' '--- bridge tests/usages ---'
rg -n -F --glob '*.{ts,tsx}' -e 'registerWslHostIpcBridge' -e 'wslHostStatuses' -e 'onStatusChanged(' src tests || test "$?" -eq 1Repository: stablyai/orca
Length of output: 21131
🏁 Script executed:
printf '%s\n' '--- shared status contract ---'
nl -ba src/shared/wsl-host-types.ts | sed -n '1,70p'
printf '%s\n' '--- service status publication ---'
nl -ba src/main/wsl/wsl-host-service.ts | sed -n '300,345p'
printf '%s\n' '--- status conversion ---'
nl -ba src/main/wsl/wsl-host-connection-record.ts | sed -n '1,60p'
printf '%s\n' '--- relevant service tests ---'
rg -n -F --glob 'src/main/wsl/wsl-host-service.test.ts' -e 'getStatuses' -e 'publish' -e 'handleConnectionChange' -e 'ready' src/main/wsl/wsl-host-service.test.ts
nl -ba src/main/wsl/wsl-host-service.test.ts | sed -n '55,150p'
printf '%s\n' '--- bridge and slice with line numbers ---'
nl -ba src/renderer/src/hooks/ipc-events/wsl-host-ipc-bridge.ts | sed -n '55,88p'
nl -ba src/renderer/src/store/slices/wsl-hosts.ts | sed -n '1,38p'Repository: stablyai/orca
Length of output: 13818
🏁 Script executed:
printf '%s\n' '--- host picker consumer ---'
nl -ba src/renderer/src/components/sidebar/sidebar-host-options.ts | sed -n '105,145p'
printf '%s\n' '--- status labels ---'
nl -ba src/renderer/src/components/sidebar/wsl-host-row-status.ts | sed -n '1,75p'
printf '%s\n' '--- bridge registration ---'
nl -ba src/renderer/src/hooks/ipc-events/app-lifetime-ipc-bridge.ts | sed -n '68,96p'Repository: stablyai/orca
Length of output: 6640
🌐 Web query:
official Electron documentation ordering guarantee ipcRenderer.invoke reply webContents.send event
💡 Result:
**The official Electron docs do not establish a delivery-order guarantee between an `ipcRenderer.invoke()` result and a separate event sent with `webContents.send()`.**
They document that `invoke()` returns a Promise for the main-process handler’s result, and that `webContents.send()` sends an asynchronous message to the renderer—but don’t specify which the renderer receives or processes first when both are sent around the same time. ([github.com](https://github.com/electron/electron/blob/main/docs/api/ipc-renderer.md?plain=1&utm_source=openai))
Electron does document an ordering rule for certain *renderer-to-main* messages (`send`, `sendSync`, and `postMessage`), but that rule does not cover `invoke()` replies versus main-to-renderer `webContents.send()` events. ([github.com](https://github.com/electron/electron/blob/main/docs/api/web-contents.md?utm_source=openai))
So, if your design depends on their relative order, treat it as **unspecified** and add explicit sequencing or correlation in your application. That is a conservative inference from what the docs do—and do not—guarantee; it is not a quoted Electron guarantee.
Citations:
- 1: https://github.com/electron/electron/blob/main/docs/api/ipc-renderer.md?plain=1&utm_source=openai
- 2: https://github.com/electron/electron/blob/main/docs/api/web-contents.md?utm_source=openai
Queue status events during initial hydration.
onStatusChanged applies events immediately, while getStatuses() applies its snapshot when the Promise resolves. These separate IPC operations have no ordering guarantee. A newer event can therefore be overwritten by an older snapshot for the same environmentId, leaving the host picker stale until another event arrives.
Suggested fix
const apply = (status: WslHostStatus): void => useAppStore.getState().applyWslHostStatus(status)
+ let hydrating = true
+ const pending: WslHostStatus[] = []
+ const applyEvent = (status: WslHostStatus): void => {
+ if (hydrating) {
+ pending.push(status)
+ return
+ }
+ apply(status)
+ }
// A preload from before the capability call leaves it unknown, which the UI treats as present.
...
let stopped = false
- unsubs.push(api.onStatusChanged(apply), () => {
+ unsubs.push(api.onStatusChanged(applyEvent), () => {
stopped = true
+ hydrating = false
+ pending.length = 0
})
void api
.getStatuses()
.then((statuses) => {
if (!stopped) {
statuses.forEach(apply)
+ pending.splice(0).forEach(apply)
}
+ hydrating = false
})
- .catch((error) => console.error('Failed to read WSL host statuses:', error))
+ .catch((error) => {
+ hydrating = false
+ if (!stopped) {
+ pending.splice(0).forEach(apply)
+ }
+ console.error('Failed to read WSL host statuses:', error)
+ })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** Mirrors WSL host status into the store and tells main which hosts the user has open. */ | |
| export function registerWslHostIpcBridge(unsubs: (() => void)[]): void { | |
| const api = window.api.runtimeEnvironments?.wslHosts | |
| // An older preload, or a partial test bridge, lacks the WSL host namespace. | |
| if (typeof api?.onStatusChanged !== 'function') { | |
| return | |
| } | |
| const apply = (status: WslHostStatus): void => useAppStore.getState().applyWslHostStatus(status) | |
| // A preload from before the capability call leaves it unknown, which the UI treats as present. | |
| if (typeof api.getCapability === 'function') { | |
| void api | |
| .getCapability() | |
| .then(({ serverFiles }) => useAppStore.getState().setWslHostServerFiles(serverFiles)) | |
| .catch((error) => console.error('Failed to read WSL host capability:', error)) | |
| } | |
| let stopped = false | |
| unsubs.push(api.onStatusChanged(apply), () => { | |
| stopped = true | |
| }) | |
| void api | |
| .getStatuses() | |
| .then((statuses) => { | |
| if (!stopped) { | |
| statuses.forEach(apply) | |
| } | |
| }) | |
| .catch((error) => console.error('Failed to read WSL host statuses:', error)) | |
| let reported = '' | |
| /** Mirrors WSL host status into the store and tells main which hosts the user has open. */ | |
| export function registerWslHostIpcBridge(unsubs: (() => void)[]): void { | |
| const api = window.api.runtimeEnvironments?.wslHosts | |
| // An older preload, or a partial test bridge, lacks the WSL host namespace. | |
| if (typeof api?.onStatusChanged !== 'function') { | |
| return | |
| } | |
| const apply = (status: WslHostStatus): void => useAppStore.getState().applyWslHostStatus(status) | |
| let hydrating = true | |
| const pending: WslHostStatus[] = [] | |
| const applyEvent = (status: WslHostStatus): void => { | |
| if (hydrating) { | |
| pending.push(status) | |
| return | |
| } | |
| apply(status) | |
| } | |
| // A preload from before the capability call leaves it unknown, which the UI treats as present. | |
| if (typeof api.getCapability === 'function') { | |
| void api | |
| .getCapability() | |
| .then(({ serverFiles }) => useAppStore.getState().setWslHostServerFiles(serverFiles)) | |
| .catch((error) => console.error('Failed to read WSL host capability:', error)) | |
| } | |
| let stopped = false | |
| unsubs.push(api.onStatusChanged(applyEvent), () => { | |
| stopped = true | |
| hydrating = false | |
| pending.length = 0 | |
| }) | |
| void api | |
| .getStatuses() | |
| .then((statuses) => { | |
| if (!stopped) { | |
| statuses.forEach(apply) | |
| pending.splice(0).forEach(apply) | |
| } | |
| hydrating = false | |
| }) | |
| .catch((error) => { | |
| hydrating = false | |
| if (!stopped) { | |
| pending.splice(0).forEach(apply) | |
| } | |
| console.error('Failed to read WSL host statuses:', error) | |
| }) | |
| let reported = '' |
| const app = await electron.launch({ | ||
| args: getOrcaElectronLaunchArgs(path.join(process.cwd(), 'out', 'main', 'index.js'), false), | ||
| env: { ...isolation.env, NODE_ENV: 'development', ORCA_E2E_HEADLESS: '1' } | ||
| }) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Pass ORCA_BACKGROUND_LAUNCH: '1' to the raw electron.launch() call.
launchDesktopWithFakeWsl launches Electron directly. It does not use tests/e2e/helpers/orca-app.ts. The repository rule requires every raw launch outside those helpers to pass ORCA_BACKGROUND_LAUNCH: '1'. This env only sets ORCA_E2E_HEADLESS: '1'. On a developer desktop, the second app can therefore reveal or focus its window.
Proposed fix
- env: { ...isolation.env, NODE_ENV: 'development', ORCA_E2E_HEADLESS: '1' }
+ env: {
+ ...isolation.env,
+ NODE_ENV: 'development',
+ ORCA_E2E_HEADLESS: '1',
+ ORCA_BACKGROUND_LAUNCH: '1'
+ }As per coding guidelines: "A raw electron.launch() outside those helpers must pass ORCA_BACKGROUND_LAUNCH: '1'."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const app = await electron.launch({ | |
| args: getOrcaElectronLaunchArgs(path.join(process.cwd(), 'out', 'main', 'index.js'), false), | |
| env: { ...isolation.env, NODE_ENV: 'development', ORCA_E2E_HEADLESS: '1' } | |
| }) | |
| const app = await electron.launch({ | |
| args: getOrcaElectronLaunchArgs(path.join(process.cwd(), 'out', 'main', 'index.js'), false), | |
| env: { | |
| ...isolation.env, | |
| NODE_ENV: 'development', | |
| ORCA_E2E_HEADLESS: '1', | |
| ORCA_BACKGROUND_LAUNCH: '1' | |
| } | |
| }) |
Source: Coding guidelines
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
a755184f-e87f-4127-acef-eea68e4a7f13
📒 Files selected for processing (2)
src/renderer/src/store/repos/runtime-project-terminal-stop.tstests/e2e/wsl-host-fake-transport.spec.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.
| }, | ||
| { repoPath: projectPath, environmentId: WSL_ENVIRONMENT_ID } | ||
| ) | ||
| await page.waitForTimeout(5_000) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '285,340p' tests/e2e/wsl-host-fake-transport.spec.tsRepository: stablyai/orca
Length of output: 2592
🏁 Script executed:
set -e
printf '%s\n' '--- surrounding test and helpers ---'
sed -n '1,380p' tests/e2e/wsl-host-fake-transport.spec.ts
printf '%s\n' '--- exact change against merge base ---'
git diff --unified=30 a65fdc887fe9fc584ded4233515e78792c28bf54 7ee1cb96d7a3e9975fd746b314c152c0d287f7e1 -- tests/e2e/wsl-host-fake-transport.spec.tsRepository: stablyai/orca
Length of output: 40166
Remove the fixed five-second wait.
terminal.list only produces diagnostic output. The existing expect.poll calls already wait for terminal and repository cleanup. Move the diagnostic after those polls, or remove it, so each run does not incur an unconditional five-second delay.
Suggested fix
--- "a/tests/e2e/wsl-host-fake-transport.spec.ts"
+++ "b/tests/e2e/wsl-host-fake-transport.spec.ts"
@@ -317,13 +317,12 @@
},
{ repoPath: projectPath, environmentId: WSL_ENVIRONMENT_ID }
)
- await page.waitForTimeout(5_000)
- console.log(
- '[e2e-diag] hub terminals after',
- JSON.stringify((await hubRuntime.call('terminal.list')).result)
- )
await expect.poll(() => hubTerminalsIn(projectPath), { timeout: 30_000 }).toBe(0)
await expect.poll(() => hubHasRepo(projectPath), { timeout: 30_000 }).toBe(false)
+ console.log(
+ '[e2e-diag] hub terminals after',
+ JSON.stringify((await hubRuntime.call('terminal.list')).result)
+ )
// An orphan: a terminal whose project the host already forgot (left by older builds).
// realpath: the host reports workspace paths resolved (/private/var on macOS).…rminals ended On Linux CI the hub keeps a pty-less tab record for the removed worktree after terminal.stop ended its shell, so counting every terminal.list row never reached zero.
7ee1cb9 to
ee68d80
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/e2e/wsl-host-fake-transport.spec.ts (1)
79-82: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winPass
ORCA_BACKGROUND_LAUNCH: '1'to the rawelectron.launch()call.
launchDesktopWithFakeWslcallselectron.launch()directly instead of going throughtests/e2e/helpers/orca-app.ts. Its env sets onlyORCA_E2E_HEADLESS: '1'. When the spec runs on a developer desktop, the second app can show or focus its window.Proposed fix
- env: { ...isolation.env, NODE_ENV: 'development', ORCA_E2E_HEADLESS: '1' } + env: { + ...isolation.env, + NODE_ENV: 'development', + ORCA_E2E_HEADLESS: '1', + ORCA_BACKGROUND_LAUNCH: '1' + }As per coding guidelines: "A raw
electron.launch()outside those helpers must passORCA_BACKGROUND_LAUNCH: '1'."Source: Coding guidelines
🔇 Additional comments (1)
tests/e2e/wsl-host-fake-transport.spec.ts (1)
373-376: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.Make the fixed 3-second wait explicit.
Line 375 waits 3 seconds unconditionally. The spec uses this wait to check that no respawn happens after removal. A negative check needs some observation window, so this is acceptable. Define the duration as a named constant, such as
RESPAWN_OBSERVATION_MS, so the intent of the wait is clear.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
12ea83a5-5c03-4be3-b017-623a37f4c5b2
📒 Files selected for processing (1)
tests/e2e/wsl-host-fake-transport.spec.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.
|
@nwparker Small thing I noticed while translating the new WSL host strings for the Russian language pack (checked on
Mapping the three codes to catalog strings in |
Stacked on #27933 (C7 main-process wiring), which is stacked on #27916 and folds in #27789. Part 2 of 2.
ELI5
On Windows, with Experimental → "WSL distros as hosts" on, the Add-a-project host picker gets an "Add WSL distro" choice next to "Add SSH host" and "Add remote server". Pick a distro, click Add host, and it shows up as its own host; projects and terminals you open on it run inside Linux.
What Changed
Before: no way to make a WSL distro a host; WSL projects went through the
\\wsl.localhostbridge.After (all hidden unless the Experimental switch is on, and only on Windows):
Selectprimitive; it preselects only when exactly one distro can be added. A stopped distro's button reads Start and add, so starting it is always the user's click. Refusals (WSL 1, R25 "already runs the Orca server for …") show inline; setup progress shows while C4 runs.runtime:<envId>row labelled<distro> (WSL), detail "WSL distro", plus the C6 state when it is not plain: Starting, Reconnecting, Distro stopped, Can’t verify, Idle, Failed to start. Health comes from the C6 verdict until the server itself answers. A stopped or failed WSL host gets the same Connect action SSH hosts have, which may start the distro.Why
Reusing the add-host dialog, picker rows and Connect affordance keeps WSL hosts learnable for anyone who has added an SSH host, and keeps the code in one place. Making "start the distro" a labelled button (rather than a silent boot) follows the background-distro rule. Reporting open work from the renderer, routed by each workspace's owner, is what lets main honour "no background boots" without guessing.
Linked Issue
Track C of WSL via orcad (C7).
Visual Proof
CDP screenshots of hidden windows from the fake-transport e2e (no window was shown or focused):
Add WSL distro dialog (Ubuntu running and preselected; a WSL 1 distro is listed but not selectable):
The new host's row in the Add a project host picker:
A project added on the WSL host, with its terminal running there:
Before: the picker had no WSL choice (the "Add WSL distro" entry and row are new; nothing else in the picker changed).
Testing
I manually tested these changes locally
Automated tests added/updated, or explained why not below
tests/e2e/wsl-host-fake-transport.spec.ts(macOS,ORCA_BACKGROUND_LAUNCH=1, passes): a fake WSL transport points the "distro" at a headless Orca server; the test turns on the switch, adds the distro through the real UI, sees the row reach Connected, adds a git project through the host's folder browser, and runs a command in a terminal whose PTY lives on that host.Renderer tests:
AddWslHostFormPanel.test.tsx,wsl-host-row-status.test.ts(health/detail mapping, Connect eligibility, own row),wsl-host-ipc-bridge.test.ts(open-work set, owner-routed),AddRepoHostSelector.test.tsx(entry shown only when enabled).Gates:
pnpm tc, sidebar and settings suites (581 files), oxlint,check:code-quality:changed(design system, React Doctor), focus-read and layout-writer ratchets, localization catalogs/extraction/coverage,pnpm format.Not tested on a real Windows desktop (see feat(wsl): WSL distro hosts, main-process wiring and persistence (WSL-via-orcad C7a, experimental) #27933).
AI Disclosure
Review
Agent skill upstream boundary
Notes
Selectinside a<form>reports''from its hidden native select and wiped the preselection; the panel has no form and ignores empty values.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)Follow-up commit (review decisions):
cba0769b96cExperiment off with existing WSL hosts. The rows stay, so projects never seem to vanish. The host picker row reads "Turned off in Settings → Experimental" with a Turn on link (opens that setting) instead of Connect. Settings lists the hosts with "WSL hosts are turned off in Settings → Experimental", Connect disabled, and a Turn on in Experimental button. No background work runs (main refuses too, #27933).
Remove asks first, in the same
HostRemoveDialogSSH hosts use, with WSL copy: it stops Orca's server in that distro; files in the distro are untouched.The fake-transport e2e now also covers both (turned-off row and settings, then confirm and remove).
Follow-up commits (win-lowspec smoke)
B2,
61aab89de2f: The UI reads main's template capability once. Without it, the add dialog shows "This build of Orca can’t run WSL hosts (server files missing)." with the distro picker and Add disabled, and runs no wsl.exe listing. Existing hosts' rows say "This build can’t run WSL hosts" (Settings uses the full sentence) with Connect disabled and no Turn on. The Experimental toggle notes it too. A second e2e covers a fake build without the files, including main's refusal.B3,
bb1fc63f2c8: The add dialog re-reads the distro list when its window regains focus or becomes visible, and from a new Refresh button; it never polls. The selection is now derived from the current list instead of being set by an effect. The e2e starts with only a WSL 1 distro, "imports" Ubuntu while the dialog is open, clicks Refresh, and sees Ubuntu preselected.Follow-up commits (win-lowspec smoke 2)
B2,
a913f615d41: Settings → Remote Orca Servers (connect list and default-host picker), the server-update checks and the status bar's server list now useisListedRemoteServer, which leaves out environments whose deployment record is a WSL host (by kind, never by name). WSL hosts appear only in the WSL hosts section, which reads them from the store. The e2e checks the generic list stays empty.B1,
83b1b27a25b: The confirmation now says removal ends the host's open terminals too; a removal that left something running is reported as a warning.Follow-up commits (awin smoke)
89e2f14035eThe new removal copy never shipped: the build reads en.json, which kept the old text under the reused key. The copy now has its own key, the stale entry is gone from both catalogs, and a test reads the shipped catalog.704281db6b0After removal, a workspace that lived on that host is no longer active, so nothing keeps calling a host that no longer exists. A warning about what still runs in the distro now stays until dismissed. The e2e checks that removal ends the host's terminal on the server standing in for orcad, shows no warning, leaves the workspace, and starts nothing for the host afterwards.54961ca0ab9Tests read the catalog and the fake's connect log without reflection.Follow-up commits (awin re-smoke)
ede1aed3021Project removal on any runtime host stops terminals beforerepo.rm. The host resolvesterminal.stopthrough the project's workspace rows, whichrepo.rmdeletes, so the shell survived. This affected SSH-managed and paired servers too, not only WSL hosts. The removal toast drops the IPC wrapper.fe5a4ec8134A WSL host's terminal says it is turned off (or that this build can't run WSL hosts), with Turn on, instead of "Reconnecting to remote runtime".c1670545a2cThe e2e reproduces both: the removed project's shell on the stand-in orcad must end and the project must leave the host's catalog, and an orphan terminal must end when the host is removed. Each check fails with its fix reverted.Follow-up commit (Windows smoke: explorer error)
3387656a41fWith WSL hosts turned off, the file explorer showed "Could not load files for this workspace: Error invoking remote method 'runtimeEnvironments:call': Error: WSL hosts are turned off...". Explorer read errors now drop the IPC wrapper (same unwrapping as the project-removal toast), and when the workspace's WSL host is turned off the empty pane shows the terminal banner's turned-off copy and Turn on button instead of an error and a Retry that cannot succeed. The editor's "Unable to load file" view drops the same wrapper at render time; the stored text is unchanged so retry classification still sees it. File search, source-control toasts, and the name filter already unwrapped.