Sign up
View stargazersSign up

fix(desktop): activate verified AppImage updates#21622

open1
desktop-app-12/appimage-replacement
desktop-app-12/appimage-updater
opened10d agobyJJulius Marminge
4 files changed
View on GitHubTwo-way sync enabledPR last imported 2h ago

Use verified replacement for explicit installation and defensively cover install-on-quit if enabled; T3 currently disables automatic installation on quit. Activate it lazily on the real AppImage updater and preserve filename events, forced relaunch, and synchronous exit-after-install behavior. Bridge dependency logs into the existing desktop updater tracing context and restore the prior logger when the service closes.

This is the integration layer rebuilt from @MarMarLabs's #20117, above the verified file-replacement layer. It deliberately excludes the original shutdown wrappers and OTLP/file-drain changes. It does not guarantee delivery of final logs at process exit. Raw updater messages remain log messages, outside span attributes. The logger bridge also affects macOS and Windows: updater logs use the existing Effect logging configuration, including its minimum level; those native updater flows were not exercised.

Stack: #21621 is the required file-replacement layer.

Verification:

  • The original had 47 passing tests and 1 skipped across two files. The rebuilt stack has 22 passing tests in one existing file, including tests against the real AppImageUpdater, lazy activation, metadata verification, both launch modes, and the configured logger. Before integration, 6 new cases failed; bypassing production hook activation makes all four targeted integration cases fail.
  • 43 existing desktop update/lifecycle tests pass. Desktop typecheck, scoped lint and formatting pass.
  • Live Linux: actual updater installation under real cross-filesystem ENOSPC preserves the runnable 40 MiB installed executable; stock updater leaves an unrunnable 24 MiB partial file. A real bind-mounted cache alias retains the new executable after cleanup. These use shell executable fixtures, not packaged Electron/FUSE.
text
1cross-filesystem ENOSPC: stock installedBytes=25165824 oldAppRuns=false
2cross-filesystem ENOSPC: rebuilt installedBytes=41943040 oldAppRuns=true
3bind-mounted cache alias: rebuilt newAppRuns=true destinationSurvivesCleanup=true

The boundary override relies on electron-updater 6.8.9 internals (doInstall, downloaded metadata, spawnLog); upgrades require revalidation. Packaged Linux success/failure, Electron shutdown/relaunch, and single-instance locking require human qualification before landing. An upstream electron-builder fix would remove the need for this local override; no upstream change was submitted. macOS, Windows, iOS, Android and Electron desktop UI were not exercised. No UI changed, so evidence is test/CLI output.

Staging always copies the download so a later verification, sync, or final-rename failure leaves the cached download available. This requires another AppImage-sized allocation even on one filesystem, and copy/hash/sync run synchronously during installation. Rename-first staging would need rollback handling to preserve a valid cache after later failures; that optimization is outside this repair.

Two independent full-stack review rounds (Claude Opus 5.5 and GPT-6-Sol) found no remaining code defects. The full-copy allocation/I/O cost is accepted and documented; packaged qualification remains a landing gate.

Implementation and tests: GPT-6.1-Sol and GPT-6-Astra through Codex in T3 Code; independent review: Claude Opus 5.5 and GPT-6-Sol in T3 Code.

Closes #10685

Checks

22 successful · 9 skipped · 1 neutral
All checks passed
74d7f48

Added 1 commit

  • fix(desktop): use verified AppImage installs in the updater74d7f48
MmacroscopeappBot10d ago
Synced from GitHub

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — This production change wires verified AppImage replacement and relaunch behavior into the desktop updater, including a dependency-internal override and a new lifecycle/logging arrangement. The executable replacement and cross-platform updater effects are substantial enough to require human review despite the accompanying integration tests.

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

CcoderabbitaiBot10d ago
Synced from GitHub

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Team
  • Run ID: 4a5a6d14-6bde-4db6-81bb-70f67320e2b9
📥 Commits

Reviewing files that changed from the base of the PR and between 4e41d353b0eb7b5096d064a9323521146d382d1a and 74d7f48541b5d3831e784633a890ce2d9a4aab24.

📒 Files selected for processing: 4
  • apps/desktop/src/electron/AppImageInstall.ts
  • apps/desktop/src/electron/ElectronUpdater.test.ts
  • apps/desktop/src/electron/ElectronUpdater.ts
  • apps/desktop/src/main.ts

Limit details: You’ve used all 10 included reviews currently available.

📝 Walkthrough

The desktop updater now initializes its shared updater when service operations run. It adds scoped logging and tracing, and verifies AppImage update metadata and installation before relaunching.

Changes: Desktop updater

Layer / File(s)Summary
AppImage installation verification
apps/desktop/src/electron/AppImageInstall.ts, apps/desktop/src/electron/ElectronUpdater.test.ts		The AppImage install hook validates the image path and update metadata, installs the update, emits a filename-change event when needed, and relaunches the destination. Integration tests cover invalid or incomplete updates, versioned filenames, and custom filenames.
Effectful updater lifecycle
apps/desktop/src/electron/ElectronUpdater.ts, apps/desktop/src/electron/ElectronUpdater.test.ts, apps/desktop/src/main.ts		The updater service initializes the shared updater when operations run. It adds a scoped component logger, registers and removes listeners within a scope, and receives desktop observability through the foundation layer. Tests cover deferred access and logger and tracing capture.

Priority: ➖ Normal

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

Change: Bug fix

Sequence Diagram(s)

mermaid
1sequenceDiagram
2 participant ElectronUpdater
3 participant AppImageUpdater
4 participant verifyAppImageInstalls
5 participant AppImageProcess
6 ElectronUpdater->>AppImageUpdater: call quitAndInstall
7 AppImageUpdater->>verifyAppImageInstalls: invoke doInstall
8 verifyAppImageInstalls->>verifyAppImageInstalls: validate APPIMAGE path and update metadata
9 verifyAppImageInstalls->>AppImageUpdater: install update and emit filename event when changed
10 verifyAppImageInstalls->>AppImageProcess: relaunch with silent-install flag

Suggested reviewers: maria-rcks

Merge Risk: ⚪ Minimal · up to 74d7f

No actionable defect remains from the reviewed changes; the PR is mergeable after normal checks.

Pre-merge checks: Passed 4
Check nameStatusExplanation
Description checkPassedThe description clearly explains the problem, implementation, dependencies, scope exclusions, verification results, known limitations, and remaining qualification gates. It does not use the template h…
Title checkPassedThe title is concise, conventional, and accurately describes the main change: activating verified AppImage updates in the desktop updater.
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: Description check

Explanation

The description clearly explains the problem, implementation, dependencies, scope exclusions, verification results, known limitations, and remaining qualification gates. It does not use the template headings, but it includes the required information, including issue references and detailed verification.

✨ 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

Usage-based review receipt

Note

This review was completed with usage-based billing: files reviewed beyond your plan's included limits are billed at $0.25/file. View usage-based billing.

Comment @coderabbitai help to get the list of available commands.