Sign up
View stargazersSign up

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

open5
main
GitHub fork: fix/appimage-transition-launchers
opened10d agobyVvoltcrash
2 files changed
View on GitHubTwo-way sync enabledPR last imported 6h 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

20 successful · 20 skipped
All checks passed
d3f30ff
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–9010d 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

Vvoltcrash10d ago

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.

MmacroscopeappBot10d ago

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

CcoderabbitaiBot10d ago
Synced from GitHub

Explain Change →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

  • Last reviewed commit: d3f30ff0f571cf54e1d31e174f47d4c1de7418e9

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • Resume reviews
  • Trigger review

No actionable comments were generated in the recent review.

Walkthrough

The updater now handles AppImage filename-update events during installation. Linux tests cover launcher paths across stable and versioned updates, including a path recreated during installation.

Changes: AppImage launch-path handling

Layer / File(s)Summary
Handle AppImage filename changes
apps/desktop/src/electron/ElectronUpdater.ts		When APPIMAGE is set, the updater listens for filename-update events and attempts a symlink unless the destination basename contains a version-like pattern. It removes the listener after installation and logs symlink failures as warnings.
Verify launch paths during updates
apps/desktop/src/electron/ElectronUpdater.launchPaths.test.ts		Linux live tests check stable and versioned update paths, removal of the old path between versioned releases, and preservation of a path recreated during installation.

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

Change: Bug fix

Sequence Diagram(s)

mermaid
1sequenceDiagram
2 participant ElectronUpdater
3 participant AppImageUpdater
4 participant Filesystem
5 ElectronUpdater->>AppImageUpdater: quitAndInstall()
6 AppImageUpdater->>ElectronUpdater: appimage-filename-updated(destination)
7 ElectronUpdater->>Filesystem: attempt symlink for eligible destination
8 ElectronUpdater->>ElectronUpdater: remove listener after installation

Suggested reviewers: juliusmarminge, wladefant

Merge Risk: External image Minimal · up to d3f30

This change keeps existing AppImage launchers working when an update switches to a versionless filename. No actionable merge-blocking risk was found. The change does not repair launchers that are already broken or protect updates started from older builds, so a bridge release must ship first.

Pre-merge checks: Passed 4

Check nameStatusExplanation
Title checkPassedThe title names the main change: AppImage launchers are preserved when AppImages switch to versionless names. The raw summary shows a symlink from the prior versioned launch path to the versionless de…
Description checkPassedThe description covers the problem, the change, scope, and verification. It cites the related issue and PR, explains the CONTRIBUTING.md small-fix exception, and lists the test environment, limits, an…
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: AppImage launchers are preserved when AppImages switch to versionless names. The raw summary shows a symlink from the prior versioned launch path to the versionless destination, which matches the title. The title is specific and follows the conventional commit format used in the repository template.

Full details: Description check

Explanation

The description covers the problem, the change, scope, and verification. It cites the related issue and PR, explains the CONTRIBUTING.md small-fix exception, and lists the test environment, limits, and results in detail. The template asks for a conventional title, a problem section, a change section, a scope section, and a verification section, and the author provides each of them in plain prose instead of the template headings. The description also ends with the model and harness, as the template requests. It is long and dense, but it is substantively complete. The author did not add the explicit section headings from the template, which a reviewer may want. The description also reports verification run on Linux ARM64 only, which the author states openly.

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 9863c3b87398f8c4d40382a62b858101425c5d8d and d3f30ff0f571cf54e1d31e174f47d4c1de7418e9.

External image Files selected for processing: 2

  • apps/desktop/src/electron/ElectronUpdater.launchPaths.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; 7 remain after this review.

MmacroscopeappBot10d ago
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.

Mmaria10d ago

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 changes9d 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.

WWladefantapproved these changes7d 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.

Added 5 commits

  • fix(desktop): preserve launchers when AppImages switch to versionless names7db3505
  • fix(desktop): read AppImage paths through the host environment9863c3b
  • test(desktop): keep one AppImage filename transition casebc91c91
  • Merge remote-tracking branch 'origin/main' into fix/appimage-transition-launchers1b4f536
  • test(desktop): keep AppImage launcher tests that exercise the aliasd3f30ff