fix(desktop): preserve launchers when AppImages switch to versionless names#30828

open3
main
GitHub fork: fix/appimage-transition-launchers
opened7d agobyVvoltcrash
View on GitHubTwo-way sync enabledPR last imported 2d ago

The versioned → versionless AppImage update in #25756 still deletes the path used by existing launchers (#8944). A launcher symlink then fails with ENOENT, even though the new executable is installed.

Preserve that removed path as a relative symlink, using electron-updater 6.8.9's synchronous appimage-filename-updated(destination) event after the move and before relaunch. Only bridge to versionless destination names: later updates replace that file in place, so aliases cannot accumulate across versioned nightly releases. The listener exists only during explicit installation and is removed on failure. A symlink failure warns without aborting installation or overwriting an existing path. Release names, AUR packaging and desktop entries are unchanged.

Release ordering matters: this protects an update initiated by a versioned release that already contains this code. Ship it in a versioned bridge release before switching artifact names. It cannot protect the initial unpatched → first-patched update, repair already-broken launchers, or preserve versioned → versioned updates. Landing this together with versionless artifacts does not retroactively protect ordinary existing installs; the bridge needs to run first.

This is a companion transition fix for #25756, submitted under CONTRIBUTING.md's small, focused obvious-bug exception. The native reproduction establishes the missing launch path; no upstream maintainer approval is claimed.

Verification on Linux ARM64 with executable shell fixtures (not packaged AppImages):

  • Real locked AppImageUpdater.doInstall with native filesystem/shell fixtures and an existing launcher symlink. Before the fix, the transition fixture failed with ENOENT; all six final cases pass. The transition now has one representative filename case because architecture and channel do not change the path behavior. They cover executable permissions, forced relaunch ordering, a custom renamed versioned install transitioning and updating in place afterward, custom versionless names, synchronous relaunch failure with re-download/retry, refusal to overwrite a recreated path, and declining versioned → versioned aliases. The new alias-chain guard regression failed before narrowing the handler.
  • vp run check:changed: passed format, lint, both Knip passes, affected desktop/scripts typechecks, and all six regression cases.

Limits: fixtures call the real synchronous installer but replace Electron's deferred quit and detached launch with synchronous shell execution. No packaged Electron/FUSE or desktop-menu click was tested on either architecture. The shell fixtures run natively on ARM64; x64/ARM64 artifact names are exercised as path behavior. Install-on-quit remains disabled. Move failure before the event retains the dependency's destructive behavior (covered separately by #21621/#21622). The retry fixture re-populates the moved download and bypasses BaseUpdater's error dispatch; it does not prove automatic recovery or asynchronous relaunch handling. No rendered UI changed.

Reviewed with review-loop: 2 rounds with Claude Opus 5.5 on high. Reviewer verdict: approved

Created with GPT-6.1 Sol in the Codex harness.

Checks

21 successful · 8 skipped
All checks passed
487e519
apps/desktop/src/electron/ElectronUpdater.test.tslines 88–90Outdated · c78360eOpen thread
……@@ -0,0 +1,165 @@
87+ ["T3-Code-0.1.7-x86_64.AppImage", "T3-Code-x86_64.AppImage"],
88+ ["T3-Code-0.1.7-arm64.AppImage", "T3-Code-arm64.AppImage"],
89+ ["T3-Code-0.1.7-nightly.20261104.1-x86_64.AppImage", "T3-Code-Nightly-x86_64.AppImage"],
90+ ["T3-Code-0.1.7-nightly.20261104.1-arm64.AppImage", "T3-Code-Nightly-arm64.AppImage"],
MmacroscopeappBoton lines 88–907d ago

These filename variants all exercise the same versioned-to-versionless branch; neither architecture nor channel changes the behavior. Could we keep one case to avoid repeating the same regression test?

suggestion
1

Posted via Macroscope — Test Signal

Note

🤖 GPT-6.1 Sol responding on behalf of voltcrash

Reduced the filename matrix to one representative ARM64 transition case. The remaining cases exercise distinct installer branches and failure handling. All six regression cases and vp run check:changed pass; implementation unchanged.

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

Synced from GitHub

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The updater now handles AppImage filename changes during installation by creating a symlink for versionless destination names. Linux integration tests cover stable and versioned updates, failures, retries, and path recreation.

Changes: AppImage launch-path handling

Layer / File(s)Summary
Filename-update handling and installer scenarios
apps/desktop/src/electron/ElectronUpdater.ts, apps/desktop/src/electron/ElectronUpdater.test.ts
When APPIMAGE is set, the updater listens for filename changes and attempts to create a symlink to the previous AppImage path for a versionless destination basename. It skips version-shaped names, logs symlink failures, and removes the listener after installation. Linux tests cover stable and versioned names, in-place updates, custom filenames, retry after failure, and path recreation.

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

Change: Bug fix

Possibly related issues

  • pingdotgg/t3code#8944 — The issue reports launcher failures when an update removes a versioned AppImage. This change preserves the old path during a versioned-to-versionless transition.
  • pingdotgg/t3code#31507 — The issue reports a t3 launcher pointing to a removed versioned AppImage. This change preserves the old path during a versioned-to-versionless transition.

Possibly related PRs

  • pingdotgg/t3code#21621 — Both PRs affect AppImage replacement when a versioned filename changes. The PR selects the destination and removes obsolete paths; this change may create an alias at the previous path.

Suggested labels: bugfix

Suggested reviewers: juliusmarminge

Merge Risk: External image Minimal · up to 487e5

This change keeps existing AppImage launchers working when an update moves from a versioned filename to a versionless one. No merge-blocking risk was found. As the PR states, a versioned bridge release must ship first, and the change does not repair launchers that are already broken.

Pre-merge checks: Passed 4

Check nameStatusExplanation
Title checkPassedThe title clearly describes the AppImage launcher fix.
Description checkPassedThe description covers the problem, change, scope exception, verification results, and limits. It does not use the template headings.
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.
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 50a4cab9cf163fc2311602dfd2c903f14affd33b and 487e5194b71d2d05915510a410953875ab64af5d.

External image Files selected for processing: 2

  • apps/desktop/src/electron/ElectronUpdater.test.ts
  • apps/desktop/src/electron/ElectronUpdater.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

Synced from GitHub

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — The change preserves existing AppImage launch paths with a symlink when updates adopt versionless filenames. It also adds a file-level static-analysis suppression, which requires human review under the configured approval rules.

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

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified head 50a4cab9cf163fc2311602dfd2c903f14affd33b on linux x64 with real packaged appimages and the native settings > about > download > install > confirm flow.

  • reproduced on github/main (9e85538535): the versioned-to-versionless update installed the new executable, but the existing launcher failed with ENOENT.
  • repeated with the pr: the old versioned path became a relative symlink to T3-Code-x86_64.AppImage. the original launcher opened the updated desktop app with its project and thread data intact.
  • completed a real codex gpt-6.1-sol turn through that launcher. it ran pwd and git rev-parse --short HEAD and returned /home/user/t3code and 50a4cab9cf.
  • installed a later versionless update through the same native ui. both launcher symlinks survived, the launcher still executed, and the installed appimage matched the update artifact's sha256.
  • the six existing ElectronUpdater.test.ts cases passed through blacksmith-testbox. github checks have no failures or pending runs; the rollup includes skipped jobs and a cancelled duplicate native-fingerprint run. no source changes were needed.

these were locally packaged appimages using the supported administrator https update feed, not shell fixtures or a mocked updater. package versions 0.1.8, 0.1.9, and 0.1.10 exercised release ordering; the later payloads repackaged the same pr bundle with --skip-build, so the compiled about-page version remained 0.1.8. the isolated vm used --password-store=basic, an isolated session bus, and a certificate bypass for the local feed.

command transcript with before/after launcher results and artifact hashes

real codex turn in the updated desktop app opened through the preserved launcher

Not verified

public github release delivery, release rollout ordering, arm64, desktop-menu clicks, and failure recovery were not exercised. macos, windows, ios, and android were not exercised; this change runs in the linux appimage installer. this does not repair already-broken launchers or protect an update initiated by an older build without the fix. a versioned bridge release still needs to ship first.

WWladefantapproved these changes6d ago
Synced from GitHub

Worker16 independent AppImage launcher-transition review — exact candidate

Candidate: T3 PR #30828, author voltcrash, exact HEAD 50a4cab9cf163fc2311602dfd2c903f14affd33b, target main at recorded base 8b44f42b14c8e273156474a924ccf29680dd7857. Reviewed both changed files: apps/desktop/src/electron/ElectronUpdater.ts and apps/desktop/src/electron/ElectronUpdater.test.ts.

Source review

I verified the implementation against the actual electron-updater 6.8.9 AppImageUpdater.doInstall source (provenance commit bed3a9c):

  • It unlinks the old APPIMAGE path, moves the downloaded file to the selected destination, then synchronously emits appimage-filename-updated(destination) before relaunch.
  • The adapter registers its listener immediately around the explicit quitAndInstall call and removes it in finally.
  • It creates a relative symlink at the old APPIMAGE path only when the new destination name is versionless; the updater puts the destination in the same directory as the old path, so the basename is the correct relative target.
  • It does not overwrite a path recreated during installation; the warning is bounded and the listener is removed on both success and failure.
  • The update service sets autoInstallOnAppQuit(false) and the explicit install path calls this adapter, so the listener is not bypassed by electron-updater's automatic install-on-quit path.

The six Linux-native fixture cases exercise the real locked AppImageUpdater installer with actual filesystem/shell operations: versioned→versionless transition, subsequent in-place versionless update, no alias chain for versioned→versioned, custom versionless path, relaunch failure/re-download retry, and refusal to overwrite a recreated old path. The author also supplied a real packaged Linux x64 UI update report at this exact head; the old launcher survived two later versionless updates and opened the updated app with project/thread data intact.

Exact-head evidence read

Normal PR CI run #41836435221 succeeded on this candidate/base:

  • Changed ElectronUpdater.test.ts: 6/6.
  • Test job: 111 files / 1,028 tests passed.
  • Lint: 0 errors, 4 warnings.
  • Build and Typecheck: PASS.
  • Test Web, Rust, Release Smoke and Mobile Native Static Analysis: SKIPPED, not passed.

The author also reports Linux ARM64 shell fixtures and a local Linux x64 packaged-AppImage UI flow. No packaged ARM64 device acceptance is claimed. Windows/macOS are not AppImage targets.

Qualification / release gates (not source findings)

The PR base 8b44f42 is 84 commits behind current main 268e5810. I compared the intervening main delta: neither changed path was modified, and the electron-updater lock resolution remains 6.8.9 on both current main and this candidate. Nevertheless, the observed PR CI is against the old base, not a current-main-composed candidate. The landing owner should require a forward-sync/current-main exact-head check before landing.

Release sequencing is material: this bridge must ship in a versioned release before the versionless artifact-name change in #25756. Shipping both changes together does not protect existing versioned installs on their first update. The author has documented this dependency accurately.

Disposition: APPROVED for scoped source correctness at exact HEAD. No new P1/P2 demonstrated. This is not current-main CI qualification, maintainer approval, merge authorization, or landing custody. Lane01/existing landing steward retains protected landing; voltcrash retains source/branch custody. No source write, CI dispatch, merge, install or deployment by Worker16.

Added 3 commits

  • test(desktop): keep one AppImage filename transition case99c506c
  • fix(desktop): preserve launchers when AppImages switch to versionless namesbe4bac3
  • fix(desktop): read AppImage paths through the host environment487e519
WWladefantapproved these changes4d ago
Synced from GitHub

Worker16 changed-head review — T3 #30828 AppImage launcher transition

Exact candidate: 487e5194b71d2d05915510a410953875ab64af5d → recorded base main@4c3a9d58184f20bd76833bd599c3544d2c09e3eb. Previous Worker16 approval #5791860427 was pinned to 50a4cab9cf163fc2311602dfd2c903f14affd33b; this review covers the new current head, not an unchanged re-review. Original author voltcrash retains branch fix/appimage-transition-launchers and both paths.

Delta reviewed

Only the new environment-access delta was added after the previous reviewed tree:

  • ElectronUpdater.quitAndInstall now reads APPIMAGE through HostProcess.Environment rather than directly from process.env.
  • The regression updates the same environment reference when simulating a second versionless update.

The actual HostProcess.Environment definition is a Context.Reference<NodeJS.ProcessEnv> whose default is process.env. It requires no new layer or provider activation. The Effect generator preserves the existing Effect.try error mapping and the listener's try/finally lifetime.

Source behavior and negative cases

I rechecked the vendored electron-updater 6.8.9 AppImageUpdater.doInstall implementation at provenance commit bed3a9c:

  • It unlinks the old APPIMAGE path, moves the downloaded artifact, and emits appimage-filename-updated(destination) only when destination !== appImageFile.
  • When an existing versioned filename transitions to a versionless destination, the event fires after the move; the adapter creates a relative alias at the old path.
  • On a later versionless in-place update, the dependency selects destination === appImageFile and emits no filename-change event. The adapter therefore does not create a self-referential symlink.
  • The listener is removed in finally; symlink failure warns without aborting installation; a path recreated during installation is not overwritten; versioned-to-versioned updates do not accumulate aliases.

The existing six Linux-native regression cases cover the transition, subsequent in-place update, versioned-to-versioned negative, custom versionless name, relaunch-failure/re-download path, and path-recreation negative.

Exact-head verification read

PR-triggered CI associated with this exact commit: run #42370849511, completed successfully.

  • Desktop test job: 36 files / 250 tests passed; ElectronUpdater.test.ts specifically 6/6 passed.
  • Whole Test job: 111 files / 1,034 tests passed.
  • Build and Typecheck: 40/40 Vite+ tasks successful.
  • Lint and Check: successful.
  • Web, Rust, Release Smoke, Mobile Native Static Analysis, relay preview and desktop preview: skipped, not passed.

The test job's selected log was read. The runner emitted a cleanup warning because a repository submodule path had no URL in .gitmodules; the job itself completed successfully.

Target and remaining qualification

Current T3 main is e455200077ce60806a01c8f28435b58860dcfc74. The candidate is 3 commits ahead / 4 behind current main. The four main-only commits change unrelated mobile/web/Forgejo/docs paths, not either reviewed AppImage path. Current-main-composed CI has not been observed; the original author/landing steward must handle normal base synchronization and expected-head readback.

Maria's earlier real packaged Linux x64 install/launcher acceptance was recorded at the previous head 50a4cab9...; it is not relabelled as a live acceptance of this new head. Packaged ARM64, Windows, macOS and a fresh current-head user-session install remain NOT RUN.

Disposition: APPROVED at exact HEAD 487e5194...; no new P1/P2 source finding. This is a separate nonauthor same-family review permitted temporarily by #1899, not a distinct GitHub principal or opposite-family review. Approval is source review only, not merge, installation, deployment or live acceptance.