Fix for issue 4820 - #6053
Conversation
There was a problem hiding this comment.
Reviewed the new WSL detection logic in apps/server/src/environment/ServerEnvironmentLabel.ts against the Effect service conventions. Two findings: the filesystem failure is swallowed untyped instead of being modeled/logged like the adjacent probe, and the new backend behavior has no focused test coverage.
Posted via Macroscope — Effect Service Conventions
| const isWsl = yield* isWslEnvironment(); | ||
| if (isWsl) { | ||
| const env = yield* HostProcessEnvironment; | ||
| const distroName = env["WSL_DISTRO_NAME"]; | ||
| if (distroName && distroName.trim().length > 0) { | ||
| return `${hostname} (WSL: ${distroName.trim()})`; | ||
| } | ||
| return `${hostname} (WSL)`; | ||
| } |
There was a problem hiding this comment.
This changes the resolved server environment label (a backend behavior change) but ServerEnvironmentLabel.test.ts is untouched. Consider adding focused cases there using the existing FileSystem.layerNoop seam plus Layer.succeed(HostProcessEnvironment, { WSL_DISTRO_NAME: "Ubuntu" }): WSL with a distro name, WSL without one, and a non-WSL Linux host that still returns the plain hostname.
Posted via Macroscope — Effect Service Conventions
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
ApprovabilityVerdict: Needs human review This PR introduces new runtime behavior by adding WSL detection to server environment labels. While well-tested and self-contained, it adds new capability rather than fixing a bug, and the author is new to this file. Human review recommended to validate the approach. You can customize Macroscope's approvability policy. Learn more. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4a4420a. Configure here.
|
Verified on Linux/WSL against this PR's head (tested 2026-08-11):
I hit the same bug first-hand on a stock Ubuntu WSL2 install before this PR existed: A few notes from reading the approach:
I am not filing a competing PR for #4820. |
CDVolvik
left a comment
There was a problem hiding this comment.
Verified on Linux (WSL2 Ubuntu, Node 24).
Branch as submitted:
✓ src/environment/ServerEnvironmentLabel.test.ts (10 tests)
Tests 10 passed (10)
Swapping only ServerEnvironmentLabel.ts for the current main copy, keeping this PR's tests:
× appends WSL distro name when WSL_DISTRO_NAME is present
AssertionError: expected 'WINDOWS-HOST' to be 'WINDOWS-HOST (WSL: Ubuntu-22.04)'
× appends generic WSL label when WSL_DISTRO_NAME is absent
AssertionError: expected 'WINDOWS-HOST' to be 'WINDOWS-HOST (WSL)'
Tests 2 failed | 8 passed (10)
Both new tests fail without the source change and pass with it, and the other eight are untouched, so they pin the added behaviour specifically.
Worth adding that the premise holds on a real setup, which is the part unit tests cannot show. On a Windows host running a WSL2 distro, hostname inside the distro returns the Windows machine name verbatim, and WSL_DISTRO_NAME is populated. So before this change the two environments are genuinely indistinguishable by label, and WSL_DISTRO_NAME is the right discriminator to reach for. That matches what #4820 describes.
One thing to confirm: WSL_DISTRO_NAME is set by the WSL init path, so it is present in an interactive shell and in anything launched from one, but it is not guaranteed for a process started through other entry points. The generic (WSL) fallback covers that, which is the right call, though it does mean the distro name is best-effort rather than always present.
|
Written by inayayousfi, typed by gpt-5.6-sol running in OpenCode. PR #7267 is a related broader naming implementation that is now ready for review. It lets users assign a server-owned name from web, desktop, or mobile, including for remote environments, then propagates that name to connected clients and T3 Connect. It handles the WSL collision once the user chooses distinct names, and it also covers hostname collisions outside WSL. This PR still provides something #7267 does not: automatic WSL detection and a zero-configuration distro suffix. I hope this focused fix can land alongside the manual rename support. |

Fixes issue #4820
Note
Low Risk
Display-only labeling change with file read and env lookup; no auth, security, or data-path impact.
Overview
Server environment labels now distinguish Windows Subsystem for Linux when friendly host names are unavailable and the resolver falls back to the hostname.
On Linux, a new
isWslEnvironmentcheck reads/proc/sys/kernel/osreleaseformicrosoftorwsl. If WSL is detected, the label becomeshostname (WSL: <distro>)whenWSL_DISTRO_NAMEis set and non-empty, orhostname (WSL)otherwise.HostProcessEnvironmentis wired in for distro name lookup; tests stubHostProcessEnvironmentvia an extendedwithHostPlatformhelper and cover both named and generic WSL cases.Reviewed by Cursor Bugbot for commit 84346fe. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Append WSL distro info to server environment label when running under WSL
isWslEnvironmentinServerEnvironmentLabel.tsthat detects WSL by checking/proc/sys/kernel/osreleasefor 'microsoft' or 'wsl' (case-insensitive); file errors are logged at debug level and treated as non-WSL.resolveServerEnvironmentLabelto append(WSL: <distro>)whenWSL_DISTRO_NAMEis set, or(WSL)when not, for Linux hosts running under WSL.WslLayerandHostProcessEnvironment.Macroscope summarized 84346fe.