Repository navigation
feat(settings): lock the project runtime to a WSL storage distro - #22128
Bandersnatch0x wants to merge 6 commits into
Conversation
A project stored on \wsl.localhost\<distro> must run inside that distro — the storage lane (worktree mirror, path-keyed git routing) is fixed by the path, while a Windows-host runtime preference would push every git and terminal call across 9P. When the project's repo path resolves to an installed distro, the Project Runtime control now locks to that distro: the Windows/Default segments and the distro select are disabled, a hint explains the lock (move the project out of the distro to change it), and a legacy contradicting stored preference is normalized once. The lock releases when WSL goes unavailable or the storage distro is uninstalled, so the existing repair copy's 'switch to Windows' keeps working.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. ℹ️ Recent review info
Walkthrough
Priority: Merge Risk:
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 5 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
-
Check name Status Explanation Title check ✅ Passed The title clearly and concisely describes the main change: locking the project runtime to the WSL storage distro. Description check ✅ Passed The description covers the required sections, explains the user impact and implementation, links the related issue, documents visual proof as not applicable, lists testing, and addresses cross-platfor… Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request. Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
- 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: b38a1976-af25-402a-b3a8-1b7a0892eafb
📒 Files selected for processing (7)
src/renderer/src/components/settings/ProjectWindowsRuntimeSetting.test.tsxsrc/renderer/src/components/settings/ProjectWindowsRuntimeSetting.tsxsrc/renderer/src/components/settings/RepositoryPane.tsxsrc/renderer/src/components/settings/RepositoryWindowsRuntimeSection.tsxsrc/renderer/src/components/settings/project-runtime-setting-copy.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/zh.json
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
One logic gap in the lock's normalization can leave the stored preference contradicting the locked UI after the lock releases and re-engages. Details inline.
Reviewed changes
- Storage-derived runtime lock —
ProjectWindowsRuntimeSettingnow acceptsrepoPath; when it parses to an installed (or still-loading) WSL distro, theinherit-global/windows-hostsegments and the distroSelectare disabled and a hint line renders. - One-time normalization — a
useEffectrewrites a contradicting storedlocalWindowsRuntimePreferenceto{ kind: 'wsl', distro }through the existingprojects:updatepath. - Deliberate lock release — the lock disengages when WSL is unavailable or the storage distro is gone, preserving the existing repair copy.
- Helper extraction — the pure display/state helpers moved verbatim into the new
project-runtime-setting-copy.ts(no behavior change);RepositoryPanepassesrepo.paththroughRepositoryWindowsRuntimeSection. - Locales —
wslStorageLockedadded toen.jsonandzh.json; other locales fall back to English, which the catalog verifier permits.
ℹ️ Enforcement lives only in the settings renderer
The lock's guarantee reaches execution only after this effect has rewritten the stored preference. src/main/local-project-runtime-resolution.ts:62 resolves the runtime from project.localWindowsRuntimePreference alone, and src/shared/local-windows-terminal-runtime.ts:52 routes terminal shells off that resolution — neither consults the repo path. So a pre-existing project with a contradicting preference keeps running Windows over the 9P bridge until someone opens its settings pane.
I could not find the add-time pin referenced as #22020 in repo-add-actions.ts, so this may be the only normalization path for projects that already exist. Not necessarily a blocker, but worth deciding whether a one-time migration is intended or whether the settings-visit dependency should be called out.
Technical details
# Enforcement boundary of the storage lock
## Affected sites
- `src/main/local-project-runtime-resolution.ts:62` — resolves from `project.localWindowsRuntimePreference` only
- `src/shared/local-windows-terminal-runtime.ts:52` — terminal shell follows that resolution, not `repo.path`
- `src/renderer/src/components/settings/ProjectWindowsRuntimeSetting.tsx:75` — the only writer of the storage-derived preference
- `src/renderer/src/store/repos/repo-add-actions.ts:132` — add path emits the boundary advisory but does not pin a runtime
## Required outcome
- Decide whether existing projects need a one-time migration at load, or document that the storage/runtime agreement is only established when the project's settings pane mounts.
## Open questions for the human
- Is #22020 already merged on a branch this PR builds on? If so, only legacy projects are affected and this is informational.ℹ️ Nitpicks
lockedPreferenceusesas LocalWindowsRuntimePreference;const lockedPreference: LocalWindowsRuntimePreference | null = …avoids the assertion, whichAGENTS.mdreserves foras const.- The first new test enables fake timers and calls
vi.runAllTimers(), but the effect runs synchronously insideact— the timers never gate anything. - The hint's "Move the project out of the distro to change it" is slightly stronger than the behavior: moving releases the lock, but the normalized
{ wsl, distro }override persists, so the user still has to change the control manually.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
…L lock engages - Clear normalizedLockRef when the storage lock is off so a later re-engage re-normalizes instead of skipping on a stale project:distro key. - Retire a pending runtime change once the lock activates, so the Apply/Cancel banner cannot linger over a locked, non-editable control. Covered by test.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Changes since the prior pullfrog review (6966f05) are confined to the lock's useEffect and its test file:
- Lock-ref reset —
normalizedLockRefis cleared whenlockedWslDistrois null, so a lock that releases (distro gone / WSL unavailable) and later re-engages for the same distro re-normalizes the stored preference instead of early-returning on a staleproject.id:distrokey. Confirmed against a throwaway release → re-engage probe: without the reset the re-normalize assertion fails, with it it passes. - Pending-change retirement — the same effect now clears
pendingPreferencewhen the lock engages, so the Apply/Cancel banner cannot sit over the disabled, non-editable control. - Regression test — a new case drives a pending WSL selection with live sessions, re-renders with a WSL UNC
repoPath, and asserts the pending banner is dropped and the storage-lock hint is shown.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
Defense-in-depth for the storage lock: commitRuntimePreference now no-ops while the WSL storage lock is active, so a stale pending Apply can never overwrite the locked distro even if the banner is somehow reached.
There was a problem hiding this comment.
ℹ️ No critical issues — one coverage nit inline.
Reviewed changes
Changes since the prior pullfrog review (b4c243c) are a single guard in the lock's commit path:
- Locked commit rejection —
commitRuntimePreferencenow clearspendingPreferenceand returns early whenlockedWslDistrois set, so no runtime write can land while the storage lock is engaged.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
… case Renders locked with live sessions and a contradicting stored preference, forms a pending WSL choice and clicks Apply, asserting updateProject fires once (the normalization) — the guard removed, it fires twice. Reword the guard comment to match what it filters.
There was a problem hiding this comment.
ℹ️ No critical issues — one test-hardening nit inline.
Reviewed changes
Changes since the prior pullfrog review (e5f0a66) are confined to the test file and one comment:
- Guard regression test —
refuses to commit a pending change while the storage lock is activerenders the control locked with a contradictingwindows-hoststored preference, forms a pending WSL choice through the enabledWSLsegment, clicks Apply, and assertsupdateProjectwas called only by the mount normalization. I verified it fails without the guard (expected "vi.fn()" to be called 1 times, but got 2 times), so it pins thee5f0a66no-op as requested. - Guard comment — reworded from "reject any value that would contradict the lock" to "no commit is accepted", which matches the guard's actual all-values filter.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
Replace the optional if (applyButton) with an explicit expect(...).toBeTruthy() so a regressed pending flow fails the guard test instead of passing vacuously.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Changes since the prior pullfrog review (0b05a0e) are a single test-only hardening edit, addressing the prior test nit:
- Assert-then-click in the guard test —
refuses to commit a pending change while the storage lock is activenow asserts theApply runtime changebutton rendered before dispatching the click, matching the sibling test, so a regressed pending flow fails instead of passing vacuously.
I re-ran the file (pnpm vitest run --config config/vitest.config.ts src/renderer/src/components/settings/ProjectWindowsRuntimeSetting.test.tsx): 10/10 pass. The sole open Pullfrog thread on this nit is resolved.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
# Conflicts: # src/renderer/src/components/settings/ProjectWindowsRuntimeSetting.tsx
|
Closing this in favour of the upstream WSL-via-orcad series (#27792, #27800, #27819, #27834, #27916, #27933, #27934, #27955), which now owns this area of the product. The changes here were kept mergeable against main and still pass their checks, but the design they build on predates the orcad host architecture. Rather than maintain a parallel path, this work waits until the official WSL host implementation lands and can absorb it. No action needed on this PR. |
ELI5
A project whose files live inside a WSL distro must also run inside that distro. If the two disagree — files on
\wsl.localhost\Ubuntu, runtime set to Windows — every git and terminal call crosses the 9P file-sharing bridge and runs dramatically slower, or fails outright. The project-runtime control now locks to the distro the project is stored in, with a plain-language hint, instead of letting storage and execution drift apart.What Changed
ProjectWindowsRuntimeSettingaccepts the project's repo path and, when it resolves to an installed WSL distro (parseWslUncPath), locks the segmented control: the "Default"/"Windows" segments and the distro select are disabled, with a hint — "This project is stored inside {{distro}} (\wsl.localhost), so the runtime is locked to that distro. Move the project out of the distro to change it."projects:updatepath.Why
Storage and execution are two different axes (
resolveMirrorDistrokeys the worktree mirror off the repo path; the runtime preference keys git/terminal/agent execution). This PR makes the settings UI enforce their agreement instead of silently allowing the pathological combination. Alternatives: warn-only (user can still click through into the slow path), or a migration wizard to move the project (valuable but far larger; better as a follow-up). This is the third piece of the WSL first-class-host series (#22024, #22020): #22020 pins the runtime at add time; this locks the settings surface afterwards.Linked Issue
Follows #22024 (WSL first-class host series).
Visual Proof
N/A for before/after screenshots in this PR — the locked state is a disabled segmented control plus a hint line; a screenshot of the locked control is attached to the series issue as the panels land. No interactive flow changed other than disabling two options.
Testing
Automated tests: three new cases in
ProjectWindowsRuntimeSetting.test.tsx— lock engages (segments disabled viaaria-disabled, normalization write fired, hint rendered), the lock releases when the storage distro is uninstalled (repair copy preserved), and existing runtime-change tests still pass (8/8; settings suite otherwise green).tsc --noEmitall four projects;oxlintclean; localization gates pass (en + zh entries).Windows 11 with WSL2: no behavior change for non-UNC projects; UNC projects resolve through the same
parseWslUncPathused everywhere else.Automated tests added/updated, or explained why not below
Review
AI review summary: cross-platform — the lock reads
parseWslUncPath(Windows-only shape) and only engages on win32 data; non-WSL projects are untouched. SSH/remote —isLocalWindowsProjectalready gates this control to local projects. Performance — pure string parse per render, no probes. Security — no new IPC; the normalization write goes through the existingprojects:updatehandler. Agent compatibility — agent-neutral.Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Part of the WSL first-class-host series (#22024). A follow-up could offer a storage-migration wizard when the user wants to move a project between hosts; the lock here intentionally keeps that door labeled but closed.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)