Sign up
View stargazersSign up

fix(web): Update all says why it skips a machine that runs the WSL backend#32858

closed1
main
GitHub fork: fix/update-all-wsl-backend
opened3d agobyBbinjamshid
2 files changed
View on GitHubTwo-way sync enabledPR last imported 1d ago

Problem

"Update all" lists a machine whose desktop app runs the WSL backend as skipped with "needs a newer desktop app", even when that desktop app is current. The reason is wrong, so the user updates the desktop app and the machine is still skipped.

The skip comes from updateEverything.logic.ts: any desktop-managed server without desktopAppUpdate gets that text. A WSL server can't advertise desktopAppUpdate. The server only sets it when it received desktopTelemetryControlFd (ServerEnvironment.ts:206-207). The WSL bootstrap goes over stdin and gets no extra fds, because wsl.exe drops them (DesktopBackendConfiguration.ts:571-584, DesktopBackendManager.ts:456).

Change

A desktop-managed server with no desktop update capability that reports platform.os === "linux" and platform.machine === "linux" is now skipped with "runs in WSL, so update the T3 Code desktop app on that machine". The same applies to channel switches, with "switch" in place of "update". The server reports machine: "linux" only for WSL (ServerEnvironmentMachine.ts:138-140); DMI detection never yields that kind. Every other case keeps "needs a newer desktop app".

Why this doesn't make WSL machines updatable

A desktop app update is a two-way protocol over two fds. The server sends request/commit/cancel to the desktop on fd 5 (DesktopTelemetryReceiver.ts:450-468), and the desktop reports status back on fd 4, which DesktopAppUpdate.run waits for (DesktopAppUpdate.ts:156-174). A WSL server has neither direction. Stdin carries only the bootstrap line, and stdout/stderr only feed the backend log. Carrying that protocol over WSL stdio is a new transport, so it isn't part of this fix.

Verification

  • Extended the existing cases in updateEverything.logic.test.ts with an old desktop and a WSL desktop, in both collectServerUpdateTargets and collectChannelSwitchTargets. Without the fix, 2 of 9 tests fail on the WSL reason; with it, all 9 pass.
  • vp run check:changed origin/main: format, lint, knip, typecheck (2 packages), tests and React compiler pass.
  • Not checked on a real Windows + WSL machine.

Refs #32062. The issue isn't fully fixed: the machine is still skipped, but the reason is now true and says what to do.

Written by Claude Opus 5.5 (Claude Code harness, run inside T3 Code).

Checks

25 successful · 13 skipped
All checks passed
d77b465

Added 1 commit

  • fix(web): Update all says why it skips a machine that runs the WSL backendd77b465
MmacroscopeappBot3d ago
Synced from GitHub

Approvability

Verdict: Approved at d77b465

Macroscope's review found this PR approvable — WSL-backed machines now receive actionable skip messages directing users to update or switch the desktop app on that machine. Update eligibility and execution are unchanged.

You can add or adjust custom eligibility rules. Learn more.

CcoderabbitaiBot3d ago
Synced from GitHub

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The update logic now uses WSL-specific skip messages when a desktop-managed server lacks desktop-app update or channel-switch support. Tests cover the WSL and desktop skip reasons.

Changes: WSL update skip messages

Layer / File(s)Summary
WSL-specific skip reasons
apps/web/src/components/sidebar/updateEverything.logic.ts, apps/web/src/components/sidebar/updateEverything.logic.test.ts		The logic identifies WSL when the OS and machine are both linux. It uses WSL-specific skip messages for server updates and channel switches. Tests cover these messages and the existing desktop message.

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

Change: Bug fix

Possibly related issues

  • pingdotgg/t3code#32062 reports the WSL update skip message that this change updates. The underlying inability to update WSL machines remains.

Possibly related PRs

  • pingdotgg/t3code#26288: Adds the channel-switch planning and WSL refusal behavior that this change updates with a WSL-specific skip message.

Suggested reviewers: t3dotgg, juliusmarminge

Merge Risk: External image Minimal · up to d77b4

WSL skip messages now provide clearer guidance without enabling automatic updates. No material merge risk was identified.

Pre-merge checks: Passed 4

Check nameStatusExplanation
Title checkPassedThe title names the main change: the skip reason that "Update all" shows for a machine running the WSL backend is corrected. It follows conventional commit form and stays specific. It matches the raw …
Description checkPassedThe description follows the template structure. It has Problem, Change, and Verification sections with specific detail: the root cause, the new WSL-specific skip message, the reasoning for why WSL mac…
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the main change: the skip reason that "Update all" shows for a machine running the WSL backend is corrected. It follows conventional commit form and stays specific. It matches the raw summary, the PR objectives, and the description, which all describe the same reason-message change for WSL desktop-managed servers.

Full details: Description check

Explanation

The description follows the template structure. It has Problem, Change, and Verification sections with specific detail: the root cause, the new WSL-specific skip message, the reasoning for why WSL machines remain non-updatable, and test results with counts. The Scope and approval section is missing. It does not link a triaged issue or maintainer approval. It only has a reference to #32062 and an admission that the issue is not fully fixed. The template requires that section for a bug fix, so the description is mostly complete but incomplete on approval.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
External image Recent review info

External image Run configuration

External image Commits

Reviewing files that changed from the base of the PR and between 09a54f01bc0ae83865347a965ced50f8c8ec2953 and d77b465b259ec4ed9e29afb877c2d1b86f27fa1c.

External image Files selected for processing: 2

  • apps/web/src/components/sidebar/updateEverything.logic.test.ts
  • apps/web/src/components/sidebar/updateEverything.logic.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.

JJulius Marminge1d ago

Thanks for this. #33030 makes the same change and is the one being kept: Prefer #33030, which addresses the same issue and both affected paths with fewer production changes, without adding WSL detection. It also correctly distinguishes updating within the same track from changing tracks. All 29 reported checks pass, and its patch applies cleanly to the available main checkout. All 3,640 open PRs were paged and screened for related submissions. Main still contains both misleading messages; distinctive-line searches found no implementation, and no commits or merges since this PR's base touched either changed file. Fresh fetches failed with HTTP 503, so these main comparisons use the available remote-tracking checkout; the forge's latest conflict check for the alternative remains pending. No submitted code, tests, or scripts were run.

Closing as a duplicate.