Skip to content

fix(desktop): activate verified AppImage updates - #21622

Open
juliusmarminge wants to merge 3671 commits into
desktop-app-12/appimage-replacementfrom
desktop-app-12/appimage-updater
Open

juliusmarminge wants to merge 3671 commits into
desktop-app-12/appimage-replacementfrom
desktop-app-12/appimage-updater

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Oct 19, 2026 •

Copy link
Copy Markdown
Member

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.
cross-filesystem ENOSPC: stock installedBytes=25165824 oldAppRuns=false
cross-filesystem ENOSPC: rebuilt installedBytes=41943040 oldAppRuns=true
bind-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 https://git.cafe/pingdotgg/t3code/issues/10685

Fixes #10685

@juliusmarminge juliusmarminge changed the title fix(desktop): use verified AppImage installs in the updater fix(desktop): activate verified AppImage updates Oct 19, 2026
@github-actions github-actions Bot added the vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. label Oct 19, 2026
@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 19, 2026
@github-actions github-actions Bot added the size:L 100-499 changed lines (additions + deletions). label Oct 19, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — The changes include extensive release and preview automation plus a new pnpm hook. That hook adds a lint suppression at .pnpmfile.cjs:31, which requires human review even with its documented justification.

Needs human approval because:

  • 5 blocking correctness issues found at or above your repo's Minimum Blocking Severity

Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Review skipped

We couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting @coderabbitai full review.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review

No actionable comments were generated in the recent review.

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.

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

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant ElectronUpdater
  participant AppImageUpdater
  participant verifyAppImageInstalls
  participant AppImageProcess
  ElectronUpdater->>AppImageUpdater: call quitAndInstall
  AppImageUpdater->>verifyAppImageInstalls: invoke doInstall
  verifyAppImageInstalls->>verifyAppImageInstalls: validate APPIMAGE path and update metadata
  verifyAppImageInstalls->>AppImageUpdater: install update and emit filename event when changed
  verifyAppImageInstalls->>AppImageProcess: relaunch with silent-install flag
Loading

Suggested reviewers: maria-rcks

Merge Risk: Minimal · previous review 74d7f

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

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Description check Warning The description gives detailed implementation and verification information, but it does not state the underlying problem or expected behavior in a dedicated Problem section. It also does not provide e… Add a Problem section that states the issue, expected behavior, and relevant reproduction context. Add the triaged issue or approval discussion with the maintainer’s explicit approval comment, or explain why this is a small, focused fix tha…
Passed Passed checks: 3 passed

Check name Status Explanation
Title check Passed The title is concise, conventional, and accurately describes the main change: activating verified AppImage updates in the desktop updater.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description gives detailed implementation and verification information, but it does not state the underlying problem or expected behavior in a dedicated Problem section. It also does not provide explicit maintainer approval for the scope; closing a linked issue does not show that approval.

Resolution

Add a Problem section that states the issue, expected behavior, and relevant reproduction context. Add the triaged issue or approval discussion with the maintainer’s explicit approval comment, or explain why this is a small, focused fix that qualifies for the exception.

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

  • 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 4e41d35 and 74d7f48.

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

Wladefant

This comment was marked as spam.

@t3dotgg
t3dotgg dismissed Wladefant’s stale review November 8, 2026 23:27

Dismissed: unsolicited automated review from a blocked account.

@pingdotgg pingdotgg deleted a comment from Wladefant Nov 8, 2026
juliusmarminge and others added 11 commits November 9, 2026 17:53
…w at rest (#33599)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Julius Marminge <julius0216@outlook.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: maria-rcks <maria@kuuro.net>
…bar changes (#33611)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…rashpad pipe (#26374)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
Co-authored-by: PR Batch Tester <agent@local.test>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: maria-rcks <maria@kuuro.net>
…ab (#23589)

Co-authored-by: PR Batch Tester <agent@local.test>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: PR Batch Tester <agent@local.test>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: maria-rcks <maria@kuuro.net>
Add a Details action for pending background commands and subagents. Preserve Claude command metadata across roster updates and bind output visibility to the explicit command item. Keep internal task IDs, duplicate command labels, and empty output headings out of the dialog.

Validation: check:changed passed with 371 tests; CI and agentic reviews completed on a0326ab. Verified command details in real Codex and Claude runs.

Implemented and verified with GPT 6.1 Sol in the Codex harness.
…anup (#33664)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
juliusmarminge and others added 28 commits November 11, 2026 04:48
…ock (#23001)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…ar file (#26594)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Julius Marminge <julius0216@outlook.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…ds the inspection limit (#26593)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…eckpoints (#27425)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…4170)

Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
Co-authored-by: Sinan Turna <189102964+Sinan53x@users.noreply.github.com>
Co-authored-by: Julius Marminge <julius0216@outlook.com>
…10495)

Co-authored-by: Julius Marminge <julius0216@outlook.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: maria-rcks <maria@kuuro.net>
)

Co-authored-by: Julius Marminge <julius0216@outlook.com>
…at t3_thread_list takes allProjects (#26525)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: Julius Marminge <julius0216@outlook.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Julius Marminge <julius0216@outlook.com>
…ll other git work (#26601)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Julius Marminge <julius0216@outlook.com>
Co-authored-by: maria-rcks <maria@kuuro.net>
Co-authored-by: Julius Marminge <julius0216@outlook.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…inks off (#21641)

Co-authored-by: Julius Marminge <julius0216@outlook.com>
…6311)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: Vladislav Nikonorov <vladislav@nikonorov.dev>
…8379)

Co-authored-by: Julius Marminge <julius0216@outlook.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Drop the test-only copyFile hook in favour of mocking node:fs, and cut
the tests to cases that fail when the verified install is not active,
plus the alias cases that guard the committed destination.

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

Copy link
Copy Markdown
Member Author

Evidence

Ran the real packaged Linux x64 desktop on both commits with fresh isolated state, light theme, identical layout, and the same fully built update payload. All captures use Xvfb and a 1920×840 recording region. The black rectangle redacts an unrelated local CLI path.

Steps: launch t3code.AppImage, select Stable, download 0.1.10 completely, leave exactly 8 MiB free on its filesystem (separate from the cache), invoke install/restart, inspect the executable and cache, then close and reopen the installed filename. Repeat with 600 MiB free to verify installation and automatic relaunch.

Before After
31eea883c7 5cf4f63505
8 MiB free: executable truncated to 8,388,608 bytes. Before: truncated executable 8 MiB free: original SHA-512 and complete cached download preserved. After: original and cache preserved
Reopening fails, exit 127. Before: AppImage cannot reopen Original reopens as 0.1.9. After: original reopens
Before low-space recording After low-space recording
600 MiB free: full payload installed and 0.1.10 automatically relaunched. Before: adequate-space control 600 MiB free: full payload installed and 0.1.10 automatically relaunched. After: adequate-space control
Before adequate-space recording After adequate-space recording

Driver · Build and reproduction instructions · Sanitized runtime transcript

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

Not shown:

  • Rejection of deliberately corrupted or wrong-size cached downloads was not exercised; the prescribed reproduction used a valid release payload.
  • Same-filesystem installation, versioned filenames, alias paths, and interruption or power-loss atomicity were outside this reproduction.

@juliusmarminge

Copy link
Copy Markdown
Member Author

The automated pipeline stopped on this pull request and will not merge it:

not finished after 6 rounds

It stays open for a person to finish or close.

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 macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews pr:needs-julius Needs Julius: route gate, unverifiable surface, or blocked size:L 100-499 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.

AppImage auto-update leaves a 0-byte binary and the app never launches again