t3codePublic Synced with GitHub

fix(desktop): keep WSL distros whose names start or end with a dot or hyphen #35971

Mergedjuliusmarmingemerged 2 commits intomain fromrefs/pull/35971/headmerged by juliusmarminge 22 minutes agoView on GitHub

juliusmarmingecommented 40 minutes ago edited

The single DISTRO_NAME_PATTERN in apps/desktop/src/wsl/wslPathParsing.ts (used by isValidDistroName -> DesktopAppSettings.normalizeWslDistro, extractDistroFromUncPath and wslUncPathToLinuxPath) required \w at both ends, so WSL-valid names like "Ubuntu-", "Ubuntu.", ".Ubuntu", "-Ubuntu" were rejected: the settings setter stored null (default distro) and UNC paths in those distros mapped to null. The pattern is now /^(?!.{1,2}$)[\w.-](?:[\w .-]*[\w.-])?$/: dot and hyphen are allowed at either end, while a leading or trailing space and any other character (controls, /, !, ;) are still rejected. The names "." and ".." are explicitly excluded so the UNC distro segment can never act as a relative path segment; one assertion for that was added to the existing rejection test. The repro branch's tests (including the updated #2751 isValidDistroName test) are kept and pass. Worst case on adversarial input: linear. There is one bounded lookahead (at most 3 characters) and a single variable-length character class followed by one terminal character class, so there is no nested or overlapping quantifier and no catastrophic backtracking. Not verified on a Windows host (native picker, WSL backend launch).

Fixes pingdotgg/t3code#35955

Reproduction on main

On origin/main 0c4b1d8f27 I ran the desktop app on Linux with T3CODE_DEV_SEED=0 ELECTRON_DISABLE_SANDBOX=1 T3CODE_DESKTOP_REMOTE_DEBUGGING_PORT=<port> headless-desktop vp run dev:desktop (fresh worktree database). Over CDP I called the renderer's real window.desktopBridge.setWslDistro IPC, the same call the Settings distro picker makes, selecting "Debian" first and then each name. Result: setWslDistro("Ubuntu-24.04") returned distro "Ubuntu-24.04" (control), while setWslDistro("Ubuntu-"), ("Ubuntu."), (".Ubuntu") and ("-Ubuntu") each returned distro null, so the earlier explicit selection was replaced by the system default. Unit level: wslUncPathToLinuxPath(\\wsl.localhost\Ubuntu-\home\josh) returns null and isValidDistroName("Ubuntu-") returns false. Two new tests on factory/repro-35955 fail on main. Without a Windows host I could not observe the native picker listing these distros or the WSL backend launching on the default distro.

Evidence

Select Debian, then each of Ubuntu-24.04, Ubuntu-, Ubuntu., .Ubuntu, and -Ubuntu through the real desktopBridge setter; read the selection back through IPC. Linux Electron, fresh databases, identical 1200×850 viewport. Captures show a labeled diagnostic overlay, not the Windows picker.

Before	After
0c4b1d8f27	ed92374614

Before recording	After recording

Identical CDP driver · Base responses · Head responses

Not shown:

Permanent blocker: A Windows host with installed edge-case WSL distros is needed to demonstrate native picker enumeration, backend launch into the selected distro, and UNC-path mapping in the real workflow; this VM is Linux.

Tests

vp test run src/wsl/wslPathParsing.test.ts src/settings (apps/desktop): 4 files, 39 tests passed. vp run check:changed: format, lint, knip, typecheck (2 packages), and tests all passed.

Who did the work

Written by Claude Opus 5.5, in Claude Code
Reviewed by GPT-6.1 Sol, in Codex, and by Claude Opus 5.5, each on its own
Proof captured by GPT-6.1 Sol and checked by Claude Opus 5.5

Run by the t3code factory in T3 Code.

github-actions[bot](GitHub profile, opens in a new tab) added labelsvouch:trusted39 minutes ago
github-actions[bot](GitHub profile, opens in a new tab) added labelssize:XS39 minutes ago
macroscopeapp[bot](GitHub profile, opens in a new tab) reviewed38 minutes ago
apps/desktop/src/wsl/wslPathParsing.test.ts lines 80 to 84
@@ -77,3 +77,8 @@
7777 expect(wslUncPathToLinuxPath("\\\\wsl.localhost\\Debian\\")).toBe("/");
7878 });
7979
80+ it("maps UNC paths of distros whose names start or end with a dot or hyphen", () => {
81+ for (const name of ["Ubuntu-", "Ubuntu.", ".Ubuntu", "-Ubuntu"]) {
82+ expect(wslUncPathToLinuxPath(`\\\\wsl.localhost\\${name}\\home\\josh`)).toBe("/home/josh");
83+ }
84+ });
macroscopeapp[bot](GitHub profile, opens in a new tab)commented 38 minutes ago edited
These cases repeat the same name validation already tested at the lower isValidDistroName layer. Could you remove this wrapper test and keep the cases at that layer?

^{Posted via Macroscope — Test Signal}
macroscopeapp[bot](GitHub profile, opens in a new tab) approved these changes37 minutes ago
macroscopeapp[bot](GitHub profile, opens in a new tab)commented 37 minutes ago
macroscopeapp[bot](GitHub profile, opens in a new tab)commented 37 minutes ago

Approvability

Verdict: Approved at ed92374

Macroscope's review found this PR approvable — This preserves WSL distro names with leading or trailing dots or hyphens in settings and path handling, while still rejecting '.' and '..'. The compatibility change is narrowly scoped and leaves product defaults unchanged.
^{You can add or adjust custom eligibility rules. [Learn more](https://docs.macroscope.com/approvability#example-custom-eligibility-rules).}

coderabbitai[bot]commented 35 minutes ago edited

No actionable comments were generated in the recent review.

Walkthrough

The WSL distro-name pattern now accepts names beginning or ending with a dot or hyphen. Tests cover UNC path mapping for those names and confirm that . and .. remain invalid.

Changes: WSL distro-name validation

Layer / File(s)	Summary
Distro-name validation and path tests `apps/desktop/src/wsl/wslPathParsing.ts`, `apps/desktop/src/wsl/wslPathParsing.test.ts`	The pattern accepts leading or trailing dots and hyphens while still rejecting `.` and `..`. Tests cover UNC path mapping, valid dot- and hyphen-prefixed or suffixed names, and rejection of trailing whitespace.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested labels: bugfix

Suggested reviewers: maria-rcks, t3dotgg

Merge Risk: Minimal · up to ed923

WSL distros with names that start or end with a dot or hyphen can now be selected and keep their name in settings, and distro-scoped path mapping works for them. No new risk was found. The native picker and backend launch have not been checked on a Windows host.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title names the main change: WSL distro names that start or end with a dot or hyphen are now kept. The changeset updates DISTRO_NAME_PATTERN and its tests to allow those names, which matches the t…
Linked Issues check		Directly linked issue `#35955` has three coding requirements. (1) Accept WSL-valid names with a leading or trailing dot or hyphen: the head replaces DISTRO_NAME_PATTERN in apps/desktop/src/wsl/wslPathPa…
Out of Scope Changes check		The whole-PR diff against the merge base 0c4b1d8f277d touches two files. wslPathParsing.ts changes only the DISTRO_NAME_PATTERN regex and adds one comment. wslPathParsing.test.ts updates the rejection…
Description check		The description covers the problem (DISTRO_NAME_PATTERN rejects WSL-valid names such as Ubuntu-, Ubuntu., .Ubuntu, -Ubuntu), the change (new pattern allowing dots and hyphens at either end, with . and…

Full details: Title check

Explanation

The title names the main change: WSL distro names that start or end with a dot or hyphen are now kept. The changeset updates DISTRO_NAME_PATTERN and its tests to allow those names, which matches the title's fix(desktop) scope.

Full details: Linked Issues check

Explanation

Directly linked issue #35955 has three coding requirements. (1) Accept WSL-valid names with a leading or trailing dot or hyphen: the head replaces DISTRO_NAME_PATTERN in apps/desktop/src/wsl/wslPathParsing.ts with /^(?!.{1,2}$)[\w.-](?:[\w .-]*[\w.-])?$/, which accepts Ubuntu-, Ubuntu., .Ubuntu and -Ubuntu. The new test in wslPathParsing.test.ts asserts isValidDistroName returns true for these names. (2) Preserve the selected name in settings: the PR description states DesktopAppSettings.normalizeWslDistro uses isValidDistroName, so the fixed predicate also fixes the settings fallback. I did not read the settings source at head; this rests on the PR description and the src/settings tests reported as passing. (3) Distro-scoped UNC helpers keep the distro and Linux path: the new test asserts wslUncPathToLinuxPath(\wsl.localhost<name>\home\josh) returns /home/josh for all four names. Leading or trailing spaces and other disallowed characters remain rejected, and the names '.' and '..' are excluded. The Windows native picker and WSL backend launch are not verified, as the issue also notes. Those are runtime checks outside the coding requirements above.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff against the merge base 0c4b1d8f277d touches two files. wslPathParsing.ts changes only the DISTRO_NAME_PATTERN regex and adds one comment. wslPathParsing.test.ts updates the rejection test to drop the dot and hyphen cases, adds accepting tests for the four names, and adds a rejection assertion for '..'. All changes serve issue #35955. The pattern is also linear on adversarial input, because the ^ anchor limits matching to one start position. The PR description's claim of 'no overlapping quantifiers' is imprecise, since [\w .-]* and the terminal [\w.-] overlap, but this does not change the result. No unrelated changes were found.

Full details: Description check

Explanation

The description covers the problem (DISTRO_NAME_PATTERN rejects WSL-valid names such as Ubuntu-, Ubuntu., .Ubuntu, -Ubuntu), the change (new pattern allowing dots and hyphens at either end, with . and .. excluded), and scope (links issue pingdotgg/t3code#35955 and fixes it). Verification is detailed: a before/after reproduction on main through the real desktopBridge IPC, unit test results (39 tests passed), and a check:changed run. It also states what was not verified (Windows host picker and WSL backend launch). The template's Problem, Change, Scope, and Verification sections are all present and substantively filled. The model and harness attribution line is also present. Minor gap: the description uses a long reproduction log and extra sections beyond the template, but the required information is complete.

Finishing Touches

Generate unit tests (beta)

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Recent review info

Run configuration

Source mode: Auto

Repository: pingdotgg/t3code

Branch: factory/issue-35955

File: .coderabbit.config.ts

Commit: ed92374

Layers: Repository TS config

Review profile: CHILL

Plan: Team

Run ID: `de34793d-a9ed-4bbc-8ae0-7a9a9f0c15bb`

Commits

Reviewing files that changed from the base of the PR and between 99224f098d5d4d223ab1d04f02935288fe2651cc and ed92374614b3acf6eed2231553d0f36093ddab6f.

Files selected for processing: 2

`apps/desktop/src/wsl/wslPathParsing.test.ts`

`apps/desktop/src/wsl/wslPathParsing.ts`

Included review availability: This review used your included allowance. 2 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 10 reviews per hour.

juliusmarminge merged thise01856322 minutes ago
juliusmarminge closed this22 minutes ago

fix(desktop): keep WSL distros whose names start or end with a dot or hyphen #35971

Reproduction on main

Evidence

Tests

Who did the work

Reviewers

Assignees

Labels

Fixes

Approvability

Reproduction on main#

Evidence#

Tests#

Who did the work#

Approvability#

Reproduction on main

Evidence

Tests

Who did the work

Approvability