Skip to content

fix(server): only offer desktop app updates the desktop can install - #30804

Open
juliusmarminge wants to merge 2 commits into
mainfrom
fix/issue-30784
Open

juliusmarminge wants to merge 2 commits into
mainfrom
fix/issue-30784

Conversation

@juliusmarminge

Copy link
Copy Markdown
Member

Problem

A server run by the desktop app advertised desktopAppUpdate whenever the desktop's control fd was present, even when the desktop's own updater was disabled. On Linux installs that are neither an AppImage nor a .deb (for example a pacman package that ships the extracted AppImage), remote clients and the host's own UI offered an update that always failed with "Automatic updates on Linux require the AppImage or the .deb package."

Reproduced on main 8b44f42: I built the Linux AppImage, extracted it the way the pacman package does, and ran it without $APPIMAGE. /.well-known/t3/environment reported desktop-managed with desktopAppUpdate: true, and server.updateServerWithProgress from a second paired client failed with that error.

Change

  • The desktop now passes one more value in the bootstrap it gives its backend, desktopAppUpdate. DesktopUpdates.configure derives it from the existing resolveDisabledReason, so the rule for whether updates work still lives in one place. It sets the value on DesktopState before the first backend start, and every start and restart reads it again.
  • The server copies it into ServerConfig. ServerEnvironment advertises desktopAppUpdate, desktopAppChannelSwitch and serverSelfUpdateProgress only when the desktop reported that it can install updates.
  • No client changes were needed. Clients already handle a desktop-managed host without desktopAppUpdate by saying the desktop app on that machine has to be updated.
  • A nightly channel blocked by policy still counts as updatable, because switching back to stable still works.

Scope and approval

Fixes #30784

Triaged via the automated issue flow.

Verification

  • apps/server/src/environment/ServerEnvironment.test.ts, test advertises desktop app updates only when the desktop can update: the desktopAppUpdate: false row fails on main (advertises desktopAppUpdate although the desktop cannot update: expected { desktopAppUpdate: true, … }) and passes here. Result: 8/8 passed.

  • apps/desktop/src/backend/DesktopBackendConfiguration.test.ts checks that the primary bootstrap carries the updater's state. apps/desktop/src/updates/DesktopUpdates.test.ts checks that configure reports a .deb install as updatable and an unmarked Linux install without APPIMAGE as not. Result: 59/59 passed.

  • vp run check:changed passed: format, lint, knip, typecheck of 36 packages, and tests in 5 files.

  • Runtime transcript. I built a real vp run dist:desktop:linux AppImage, ran --appimage-extract, added a release-style resources/app-update.yml (GitHub feed), and launched it under Xvfb with an empty T3 home. Capabilities excerpt:

    # before: main 8b44f42b14, extracted payload, APPIMAGE unset
    {"serverSelfUpdate": "desktop-managed", "desktopAppUpdate": true, "desktopAppChannelSwitch": true}
    # second paired client -> /ws server.updateServerWithProgress {"targetVersion":"0.1.8"}
    {"_tag":"Exit","exit":{"_tag":"Failure","cause":[{"_tag":"Fail","error":{"_tag":"ServerSelfUpdateError","reason":"Automatic updates on Linux require the AppImage or the .deb package."}}]}}
    
    # after: 8ccb2b02f2, extracted payload, APPIMAGE unset
    {"serverSelfUpdate": "desktop-managed", "desktopAppUpdate": null, "desktopAppChannelSwitch": null, "serverSelfUpdateProgress": null}
    # after, same payload with APPIMAGE=<path to the .AppImage> (AppImage install keeps in-app updates)
    {"serverSelfUpdate": "desktop-managed", "desktopAppUpdate": true, "desktopAppChannelSwitch": true, "serverSelfUpdateProgress": true}
    

    The same mechanism also reproduced on main with a built but unpackaged desktop app: it advertised desktopAppUpdate: true, and the update failed with "Automatic updates are not available because no update feed is configured."

  • Not checked: macOS and Windows packaged builds. Their updater has no disabled reason, so the code keeps advertising the capability for them. Also not checked: a full desktop update handoff from a remote client. The fix does not change that flow.

  • There is no UI change. Clients render the existing "update the desktop app on that machine" state, so this PR has no captures.

AI-assisted; verified with the tests and tools listed above.

🤖 Generated with Claude Code

juliusmarminge and others added 2 commits November 4, 2026 11:30
…ithout updater

These tests reproduce #30784: a desktop-mode server with a control fd
advertises desktopAppUpdate and desktopAppChannelSwitch even when its
desktop reports it cannot update (ServerConfig desktopAppUpdate: false).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…install them

A desktop-managed server advertised desktopAppUpdate (plus the channel switch and
self-update progress capabilities) whenever it had a desktop control fd. Builds
whose updater is disabled, such as an extracted AppImage run without $APPIMAGE,
therefore offered remote updates that then failed.

DesktopUpdates.configure now records whether the updater has a disabled reason
in DesktopState. It runs before the first backend start. The primary backend
bootstrap carries that as desktopAppUpdate, and ServerEnvironment advertises the
capabilities only when it is true. A nightly channel blocked by policy still
counts as updatable, because the user can switch back to stable.

Fixes #30784

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Nov 4, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 8ccb2b0

Macroscope's review found this PR approvable — Desktop update offers now reflect whether the desktop’s existing updater can actually install them. The change preserves update policy and installation behavior, with a reported Linux runtime comparison confirming unsupported installs stop advertising updates while AppImage installs retain them.

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

@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 21.5 KiB 21.5 KiB 0 B (0.0%) 24.4 KiB ✅
Codex Live turn messages 1 1 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.9 KiB 21.9 KiB 0 B (0.0%) 24.4 KiB ✅
Claude Live turn messages 1 1 0 (0.0%) 8 ✅

Baseline: 8b44f42 · PR result: 8ccb2b0 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@juliusmarminge juliusmarminge added the bugfix Fix PR opened by an issue lead; enters the PR loop label Nov 4, 2026
@coderabbitai

coderabbitai Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The desktop app now passes its update-installation capability to the server. The server uses that value, along with the desktop telemetry control file descriptor, to determine whether it advertises desktop update capabilities.

Changes: Desktop Update Capability

Layer / File(s) Summary
Determine desktop install capability
apps/desktop/src/app/DesktopState.ts, apps/desktop/src/updates/DesktopUpdates.ts, apps/desktop/src/updates/DesktopUpdates.test.ts
DesktopState tracks whether the desktop app can install updates. DesktopUpdates.configure sets this value from the general update-disabled reason. Tests cover installable and disabled Linux packages.
Pass capability through bootstrap and server configuration
packages/contracts/src/desktopBootstrap.ts, apps/desktop/src/backend/DesktopBackendConfiguration.ts, apps/desktop/src/backend/DesktopBackendConfiguration.test.ts, apps/server/src/config.ts, apps/server/src/cli/config.ts
The desktop backend passes desktopAppUpdate in the bootstrap. The server config preserves the optional value. Backend tests check that the bootstrap value matches desktop state.
Gate server update capabilities
apps/server/src/environment/ServerEnvironment.ts, apps/server/src/environment/ServerEnvironment.test.ts
The server advertises desktop update capabilities only when desktopAppUpdate is true and the desktop telemetry control file descriptor is present. Tests cover enabled and disabled states.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant DesktopUpdates
  participant DesktopBackendConfiguration
  participant ServerConfig
  participant ServerEnvironment
  DesktopUpdates->>DesktopBackendConfiguration: set and read canInstallUpdates
  DesktopBackendConfiguration->>ServerConfig: pass desktopAppUpdate in bootstrap
  ServerConfig->>ServerEnvironment: provide desktopAppUpdate
  ServerEnvironment->>ServerEnvironment: gate advertised capabilities on desktopAppUpdate and control fd
Loading

Possibly related PRs

  • pingdotgg/t3code#27197: Adds managed update policy and channel-specific update blocking in DesktopUpdates, which relates to how this change determines desktop update capability.
  • pingdotgg/t3code#26288: Adds desktop update-track switching and the desktopAppChannelSwitch capability, which uses the same desktop update capability model.

Suggested reviewers: maria-rcks, t3dotgg

Merge Risk: Minimal · up to 8ccb2

Unsupported desktop installations no longer advertise update controls. No material merge risk was identified.

Pre-merge checks: Passed 3
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: the server offers desktop app updates only when the desktop can install them.
Linked Issues check Passed Issue #30784 is the only active direct link. Its coding requirement is that a desktop-managed server must not advertise a desktop app update when the desktop cannot install it. The diff implements thi…
Out of Scope Changes check Passed Every changed file lies on the capability path for desktopAppUpdate. These files are DesktopState.ts, DesktopUpdates.ts and its test, DesktopBackendConfiguration.ts and its test, `packages/con…
Full details: Linked Issues check

Explanation

Issue #30784 is the only active direct link. Its coding requirement is that a desktop-managed server must not advertise a desktop app update when the desktop cannot install it. The diff implements this chain:

  • DesktopUpdates.configure sets DesktopState.canInstallUpdates from the absence of the general resolveDisabledReason. The Linux non-AppImage, non-.deb case therefore yields false.
  • DesktopBackendConfiguration (resolvePrimaryStartConfig) reads that ref on each start and passes desktopAppUpdate to the backend bootstrap.
  • packages/contracts/src/desktopBootstrap.ts, apps/server/src/config.ts, and apps/server/src/cli/config.ts carry the value into ServerConfig.
  • ServerEnvironment.make advertises desktopAppUpdate, desktopAppChannelSwitch, and serverSelfUpdateProgress only when desktopAppUpdate is true.

Client behavior matches the issue. At head, packages/client-runtime/src/connection/outdatedHostUpdate.ts rejects a desktop-managed host without desktopAppUpdate === true with "Update T3 Code on <label> manually; it cannot update itself." The client no longer offers a failing update in that case, which is the requested outcome. updateEverything.logic.ts was not inspected directly.

Tests exist for the server capability matrix (ServerEnvironment.test.ts), the bootstrap value (DesktopBackendConfiguration.test.ts), and the Linux package-install state (DesktopUpdates.test.ts). The reported test results were not re-run here.

The issue's background-service and version-conflict observations are not requirements of this PR. The issue states that their cause was not established. The issue also reports that the host's own desktop UI showed an update banner. The diff does not change the desktop updater's own UI state, and the issue does not state that as a separate requirement. This is noted, not treated as a failure.

Full details: Out of Scope Changes check

Explanation

Every changed file lies on the capability path for desktopAppUpdate. These files are DesktopState.ts, DesktopUpdates.ts and its test, DesktopBackendConfiguration.ts and its test, packages/contracts/src/desktopBootstrap.ts, apps/server/src/config.ts, apps/server/src/cli/config.ts, ServerEnvironment.ts and its test. Each change either sets, carries, or consumes the flag, or tests that behavior. The diff adds no client, UI, or unrelated behavior.

The design choice to derive the flag from the general disabled reason, not from the selected channel's blocked reason, is explained in the PR description. It is part of the same capability decision and is not a separate change.

The diff of DesktopBackendConfiguration.ts shows the flag added to the generic resolvePrimaryStartConfig input. The AI summary's reference to a "Windows primary" bootstrap was not supported by the diff, so no platform-specific scope concern is raised.

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
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
Commits

Reviewing files that changed from the base of the PR and between 8b44f42 and 8ccb2b0.

Files selected for processing: 10
  • apps/desktop/src/app/DesktopState.ts
  • apps/desktop/src/backend/DesktopBackendConfiguration.test.ts
  • apps/desktop/src/backend/DesktopBackendConfiguration.ts
  • apps/desktop/src/updates/DesktopUpdates.test.ts
  • apps/desktop/src/updates/DesktopUpdates.ts
  • apps/server/src/cli/config.ts
  • apps/server/src/config.ts
  • apps/server/src/environment/ServerEnvironment.test.ts
  • apps/server/src/environment/ServerEnvironment.ts
  • packages/contracts/src/desktopBootstrap.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.

@juliusmarminge juliusmarminge added the pr:gate-failed A land gate failed label Nov 4, 2026

@Wladefant Wladefant left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worker16 — independent technical review (COMMENT; not APPROVED)

Receiving identity: setup marker HL-SB-20261102; registry task Superboard OS and desktop, task ID 6ae9048924888191ba619eb5249ca846; active invocation 0babc564-c830-8328-88ed-b3eabc0cdb74. Review scope recorded on Superboard #1778 comment #6630164022.

Candidate: T3 Code PR #30804, branch fix/issue-30784, exact HEAD 8ccb2b02f28c559cb6b78b05f2d37d4246d40f93 → main@8b44f42b14c8e273156474a924ccf29680dd7857; all ten changed paths were inspected.

Review result: no demonstrated scoped P1/P2 source defect.

Source/dataflow checks

  • DesktopUpdates.configure runs before desktop bootstrap in DesktopApp.ts, so the capability Ref is set before the first backend launch. The app foundation supplies one shared DesktopState; the backend reads it for each primary resolve/restart.
  • The updater's existing disabled-reason predicate covers missing feed config, development/unpackaged builds, managed-policy off, the disable env flag, and Linux installs that are neither AppImage nor .deb. The nightly-channel restriction remains separate, correctly preserving the ability to switch back to stable.
  • The WSL bootstrap path omits the desktop control fd as before, so it does not gain desktop-update authority from this flag.
  • Server config carries the optional bootstrap boolean; ServerEnvironment advertises desktop update/channel-switch/progress only for a desktop-managed server with desktopAppUpdate === true and the existing control fd. Omitted/false values fail closed.

CI evidence read independently: normal PR workflow #41824672117 completed successfully. The Test job recorded the focused desktop suite at 59/59 across six files; Test Server 7 recorded ServerEnvironment.test.ts 8/8. Lint, Build and Typecheck, selected server shards, and workflow summary also succeeded. The test/build jobs checked out the synthetic PR merge candidate 62aaae8a9a7b00c3709cb70b964d4765d9df2cbb (PR head merged with the then-current base), not a raw head-only checkout. Desktop macOS preview and other preview workflows were skipped.

Remaining evidence boundary: author-supplied Linux AppImage/extracted-install runtime transcript is useful but was not independently reproduced in this review. Packaged Windows/macOS behavior and a full remote-client update handoff remain unverified; native host acceptance is NOT RUN, not a blanket source-merge veto.

Nonblocking P3 coverage suggestion: add an undefined desktopAppUpdate case to the ServerEnvironment capability table while the control fd is present. The field is optional in the bootstrap schema, and the strict-true fail-closed behavior for an omitted/older payload is correct but not explicitly locked by the current false/true cases.

This is a technical COMMENT only. It is not formal APPROVED, a distinct-account endorsement, merge authorization, CI admission, or host/device acceptance. No source or branch changes were made.

}),
);
it.effect.each([
{ desktopAppUpdate: false, advertised: false },

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nonblocking P3 coverage suggestion: add desktopAppUpdate: undefined with desktopTelemetryControlFd present. The bootstrap field is optional, and this strict-true check intentionally fails closed for omitted/older payloads; the current false/true table does not explicitly lock that compatibility boundary. The implementation appears correct.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Fix PR opened by an issue lead; enters the PR loop pr:gate-failed A land gate failed size:S 10-29 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update is offered for desktop-managed servers that can't self-update on pacman-installed Linux

2 participants