fix(desktop): stage and verify AppImage replacements by juliusmarminge · Pull Request #21621 · pingdotgg/t3code

juliusmarminge · 2026-10-19T09:43:44Z

A failed cross-filesystem AppImage update can remove the installed executable before the downloaded replacement finishes copying. Stage the replacement beside its destination, verify release size and SHA-512, set executable permissions, sync it, and commit with one rename. Cleanup runs after the commit and preserves aliases of the installed file.

This is the file-replacement layer rebuilt from @MarMarLabs's #20117. The dependent layer activates it in electron-updater. The original identified the failure correctly; the rebuild keeps its verified staging behavior and drops the separate shutdown/telemetry drain work.

Stack: #21622 is the dependent integration layer.

Verification:

This layer: 15 passing tests in the existing ElectronUpdater.test.ts (5 existing + 10 behavioral cases). Removing SHA verification makes the corruption regression fail.
The complete stack: 22 passing updater tests, plus 43 passing existing desktop update/lifecycle tests; desktop typecheck, scoped lint, and formatting pass.
Real Linux mount-namespace proof uses the actual AppImageUpdater.install with 40 MiB executable fixtures, separate tmpfs filesystems, and real ENOSPC. Stock updater leaves a 24 MiB partial executable that cannot run; the rebuilt stack keeps the runnable 40 MiB old executable and complete download. A separate real bind-mounted cache test verifies that cleanup preserves the committed executable.

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 live fixtures are shell executables, not packaged Electron/FUSE AppImages. Packaged Linux success/failure, real Electron shutdown/relaunch and single-instance locking still need human qualification before landing. macOS, Windows, iOS, Android and Electron desktop UI were not exercised. This nonvisual filesystem change has test/CLI evidence instead of screenshots.

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

Rebuilt from #20117 by MarMarLabs to preserve the installed AppImage until a verified replacement can be committed.

macroscopeapp · 2026-10-19T09:45:46Z

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — The helper stages and verifies AppImage replacements before atomic installation. Its new file-level static-analysis suppression requires human review to confirm the exception is justified.

^{You can add or adjust custom eligibility rules. Learn more.}

coderabbitai · 2026-10-19T09:59:33Z

No actionable comments were generated in the recent review.

Walkthrough

The change adds installAppImageUpdate to validate a downloaded AppImage, stage and verify it beside the destination, and replace the destination. Filesystem-backed tests cover successful installs, validation and commit failures, path aliases, and cleanup failures.

Changes: AppImage Update Installation

Layer / File(s)	Summary
Verified AppImage replacement `apps/desktop/src/electron/AppImageInstall.ts`, `apps/desktop/src/electron/AppImageInstall.test.ts`	The installer checks the download, stages it on the destination filesystem, verifies its size and SHA-512 digest, sets executable permissions, syncs it, and renames it into place. It handles obsolete paths and cleanup failures. Tests cover generic and versioned destinations, verification and rename failures, path aliases, and cleanup failure after commit.

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

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant installAppImageUpdate
  participant installerPath
  participant StagedFile
  participant Destination
  installAppImageUpdate->>installerPath: check download size
  installAppImageUpdate->>StagedFile: copy and verify staged file
  installAppImageUpdate->>StagedFile: set permissions and sync file
  installAppImageUpdate->>Destination: rename staged file into place
  installAppImageUpdate->>Destination: sync directory and remove obsolete paths

Merge Risk: Minimal · up to 4f41f

This change makes AppImage updates safer: the installed executable is kept until a verified replacement is ready. No blocking issues were found, and packaged-app qualification remains a normal follow-up.

Pre-merge checks: 3 · 1

Failed checks: 1 inconclusive

Check name	Status	Explanation	Resolution
Linked Issues check		Objectives from `#10685`, with stack status: 1. Stage the download beside the destination, then rename. Addressed in this PR. `installAppImageUpdate` in `apps/desktop/src/electron/AppImageInstall.ts` c…	Provide current review evidence for `#21622`. Show that the updater install path calls `installAppImageUpdate` in place of `unlinkSync` followed by `mv`. Show that install-step logging is emit…

Passed checks: 3 passed

Check name	Status	Explanation
Title check		The title "fix(desktop): stage and verify AppImage replacements" matches the main change. The changeset adds installAppImageUpdate, which stages the download beside the destination, verifies size and …
Description check		The description covers the problem, the change, scope with a linked issue (`#10685`), and verification with test results and manual-check limits. It follows the template's section structure. Two gaps: t…
Out of Scope Changes check		The current PR changes `apps/desktop/src/electron/AppImageInstall.ts` (new helper) and `apps/desktop/src/electron/AppImageInstall.test.ts` (filesystem tests). Every changed path implements the file-re…

Full details: Title check

Explanation

The title "fix(desktop): stage and verify AppImage replacements" matches the main change. The changeset adds installAppImageUpdate, which stages the download beside the destination, verifies size and SHA-512, and commits with a rename. It follows the conventional commit format from the template.

Full details: Description check

Explanation

The description covers the problem, the change, scope with a linked issue (#10685), and verification with test results and manual-check limits. It follows the template's section structure. Two gaps: the Problem section has no explicit reproduction steps or environment, and the Scope section does not explain why a stacked dependent PR is needed. The description also has no agent/model footer, though it names the models used in the Verification section, so the template's trailing agent line is covered in substance.

Full details: Out of Scope Changes check

Explanation

The current PR changes apps/desktop/src/electron/AppImageInstall.ts (new helper) and apps/desktop/src/electron/AppImageInstall.test.ts (filesystem tests). Every changed path implements the file-replacement objective of #10685 or tests that objective. The PR description says the shutdown and telemetry drain work from #20117 was dropped, and no such code appears in the reviewed summary. The test cases cover the staging, verification, rename, cleanup, and alias behavior described in the helper. No unrelated change was found.

Full details: Linked Issues check

Explanation

Objectives from #10685, with stack status:

Stage the download beside the destination, then rename. Addressed in this PR. installAppImageUpdate in apps/desktop/src/electron/AppImageInstall.ts creates a private stage directory in dirname(destination), copies with COPYFILE_EXCL, and calls renameSync (same filesystem, atomic). Evidence: current PR.
Verify size and SHA-512 before commit. Addressed in this PR. verifyAndSyncStage checks the staged size and the base64 SHA-512 digest before the rename. It also rejects empty or size-mismatched downloads. Evidence: current PR.
Keep the old AppImage until the replacement is verified. Addressed in the helper. The installed file is changed only by the final rename. The helper does not unlink it first. Replacing the unlinkSync-first call in electron-updater is not shown in this PR. Evidence: current PR plus #21622 (uncertain).
Route the real install path through the helper. Uncertain. The helper is not called from ElectronUpdater.ts or DesktopUpdates.ts in this PR. Its activation is in #21622, which has no current review evidence. Stack objective is Uncertain.
Log the install step and wire the updater logger into the trace. Partially addressed. The helper accepts an optional logger and logs only cleanup or directory-sync failures. It does not log the install step. Trace wiring is not shown in the reviewed evidence. Evidence: current PR plus #21622 (uncertain).

Unresolved: the stack-level objective 4 and the trace-wiring part of objective 5 cannot be established. The PR states that packaged Linux qualification is still outstanding. Those points are landing gates and are not assessed here.

Resolution

Provide current review evidence for #21622. Show that the updater install path calls installAppImageUpdate in place of unlinkSync followed by mv. Show that install-step logging is emitted and that the updater logger is connected to the desktop trace. Then reassess objectives 4 and 5.

Finishing Touches

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

Source mode: Auto

Repository: pingdotgg/t3code

Branch: desktop-app-12/appimage-replacement

File: .coderabbit.config.ts

Commit: 4f41f68

Layers: Repository TS config

Review profile: CHILL

Plan: Team

Run ID: `b89d1a5d-5e26-4d45-9857-e4aaca14c0ab`

Commits

Reviewing files that changed from the base of the PR and between `4e41d35` and `4f41f68`.

Files selected for processing: 2

`apps/desktop/src/electron/AppImageInstall.test.ts`

`apps/desktop/src/electron/AppImageInstall.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.

Dismissed: unsolicited automated review from a blocked account.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

Main rewrote ElectronUpdater.test.ts around the real electron-updater, so the helper's tests get their own file and mock copyFileSync instead of a test-only copyFile parameter. Overlapping failure cases are folded into one table. Co-Authored-By: Claude Opus 5.5 <[email protected]>

juliusmarminge · 2026-11-14T21:27:04Z

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

in the real app the problem is still there, and the rework changed nothing:

No code change. The reported failure is electron-updater's stock AppImage install path, which this layer does not change. On 4f41f68, installAppImageUpdate is imported only by AppImageInstall.test.ts. ElectronUpdater.ts and the rest of production never call it; the wiring is the dependent PR #21622, as this PR's scope already states. The packaged build therefore still runs electron-updater's own install, which deletes the old AppImage and then copies the new one. That produces exactly the reported 16,777,216-byte, mode-0700 partial file, and it is why both proof builds behaved identically. To check that the helper is right for this scenario, I mounted a real 16,777,216-byte tmpfs as the install directory, put a 315,634,226-byte update with a matching SHA-512 in a cache on ext4, and called installAppImageUpdate with the real fs. The staged copy hit ENOSPC and the helper threw. The installed AppImage was left as it was (mode 0755) and still ran (exit 0), the private staging directory was removed, and the cached download stayed complete. Packaged-app proof that #10685 is fixed has to run on a build that includes #21622's integration.

It stays open for a person to finish or close.

fix(desktop): stage and verify AppImage replacements

4e41d35

Rebuilt from #20117 by MarMarLabs to preserve the installed AppImage until a verified replacement can be committed.

juliusmarminge added this pull request to stack #21623 October 19, 2026 09:43

github-actions Bot added the vouch:trusted Author is trusted by repo permissions or the VOUCHED list. label Oct 19, 2026

juliusmarminge changed the title ~~desktop app 12/appimage replacement~~ fix(desktop): stage and verify AppImage replacements Oct 19, 2026

juliusmarminge mentioned this pull request Oct 19, 2026

fix(desktop): activate verified AppImage updates #21622

Open

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 Bot added the size:L 100-499 changed lines (additions + deletions). label Oct 19, 2026

juliusmarminge mentioned this pull request Oct 19, 2026

fix(desktop): a failed AppImage update no longer breaks the installed app #20117

Closed

only21mil mentioned this pull request Oct 20, 2026

fix(desktop): retry a timed-out Windows update signature check once #22357

Closed

juliusmarminge mentioned this pull request Oct 22, 2026

AppImage update deletes the old binary before mv; a failed mv (ENOTCONN from stale mount on PATH) leaves no app #23188

Closed

Derpedyea mentioned this pull request Oct 22, 2026

fix(desktop): a failed AppImage update no longer deletes the installed app #23204

Closed

t3dotgg mentioned this pull request Oct 22, 2026

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

Open

voltcrash mentioned this pull request Nov 4, 2026

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

Open

This comment was marked as spam.

Sign in to view

pingdotgg deleted a comment from Wladefant Nov 8, 2026

coderabbitai Bot mentioned this pull request Nov 11, 2026

fix(desktop): AppImage updates no longer leave old runtimes and mounts running #29672

Merged

juliusmarminge added the takeover:in-progress A takeover lead is working on this PR label Nov 14, 2026

juliusmarminge and others added 2 commits November 14, 2026 20:50

Merge origin/main into desktop-app-12/appimage-replacement

42ff030

Co-Authored-By: Claude Opus 5.5 <[email protected]>

github-actions Bot added size:M 30-99 changed lines (additions + deletions). and removed size:L 100-499 changed lines (additions + deletions). labels Nov 14, 2026

juliusmarminge added takeover:blocked Takeover blocked; reason in the PR body and removed takeover:in-progress A takeover lead is working on this PR labels Nov 14, 2026

Navigation Menu

Repository navigation

fix(desktop): stage and verify AppImage replacements - #21621

fix(desktop): stage and verify AppImage replacements#21621
juliusmarminge wants to merge 3 commits into
mainfrom
desktop-app-12/appimage-replacement

juliusmarminge commented Oct 19, 2026 •

edited

Loading

Uh oh!

macroscopeapp Bot commented Oct 19, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 19, 2026 •

edited

Loading

Sequence Diagram(s)

Uh oh!

This comment was marked as spam.

Uh oh!

juliusmarminge commented Nov 14, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

juliusmarminge commented Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

macroscopeapp Bot commented Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Approvability

Uh oh!

coderabbitai Bot commented Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Sequence Diagram(s)

Uh oh!

This comment was marked as spam.

Uh oh!

juliusmarminge commented Nov 14, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

juliusmarminge commented Oct 19, 2026 •

edited

Loading

macroscopeapp Bot commented Oct 19, 2026 •

edited

Loading

coderabbitai Bot commented Oct 19, 2026 •

edited

Loading