Skip to content

feat(lifecycle): notify and offer restart on in-place package upgrade#564

Open
lizthegrey wants to merge 2 commits intoaaddrick:mainfrom
lizthegrey:lizf.upgrade-restart-notification
Open

feat(lifecycle): notify and offer restart on in-place package upgrade#564
lizthegrey wants to merge 2 commits intoaaddrick:mainfrom
lizthegrey:lizf.upgrade-restart-notification

Conversation

@lizthegrey
Copy link
Copy Markdown
Contributor

Summary

Watch app.asar for in-place dpkg/rpm replacement; on detection, show a click-to-restart notification. Fixes the Quick Entry "raw JS" / About "minified source" / Ctrl+Q "intermittent" symptom cluster that traces to the running v(N) main process loading v(N+1) renderer assets after an upgrade. Linux only; AppImage and Nix immutable-store paths are inert as expected.

The bug

dpkg/rpm replace app.asar via rename() while the main process keeps its in-memory JS. Any BrowserWindow opened after the swap reads HTML/asset files fresh from disk, where the hashed asset filenames now point at v(N+1) bundles that the in-memory v(N) IPC and preload no longer match. Symptoms observed across recent triage:

Before this PR users had no signal that the running process was stale. The macOS / Windows clients get this from Squirrel; Linux deb/RPM has no equivalent.

The fix

Inserted in scripts/frame-fix-wrapper.js as a sibling to the autostart shim — same "main-process boot" surface, runs once on first require('electron').

  • Watch the parent dir, not the file. File-level fs.watch loses the inode across rename-replace; dir-level inotify reports the new entry via IN_MOVED_TO / IN_CREATE. Filename-filtered to app.asar so unrelated activity in the resources dir is ignored.
  • 5s debounce past the last event to clear dpkg's .dpkg-new → rename dance and similar multi-stage swaps in rpm / Nix.
  • Compare both ino and mtimeMs so the rename-replace path and the (rare) in-place-rewrite path both register as a real change.
  • Notification deferred behind whenReady if the app hasn't reached ready when the swap fires; click → app.relaunch(); app.quit(). Linux libnotify ignores Electron's actions array per the API docs, so whole-notification click is the only affordance — that matches what the macOS client does anyway.
  • Best-effort. Stat failures and missing-asar paths (AppImage's squashfs mount, dev runs, unusual installs) silently skip arming the watcher.

Test plan

  • Build deb v(N), install, run from terminal
  • sudo apt install ./claude-desktop_v(N+1).deb while it's running
  • Notification appears within ~5s
  • Click → app cleanly relaunches into v(N+1) (verify via launcher.log)
  • AppImage build: confirm watcher arms but never fires (squashfs mount stays pinned to running file's contents)
  • No tests added — frame-fix-wrapper.js has no existing test harness in this repo (the autostart shim it sits next to is also untested). Open to wiring up a node mock-test if the maintainer wants one before merge.

Generated with Claude Code
Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com
~100% AI / minimal Human
Claude: diagnosed root cause (running-process / on-disk asset version mismatch after upgrade), designed and implemented the watcher, wrote commit and PR body
Human: filed the bug report, ran the launch-context test that ruled out CLAUDE_TITLEBAR_STYLE as the culprit, pointed at the macOS restart-prompt UX as the model to follow

dpkg/rpm replace app.asar via rename() while the main process keeps
its in-memory JS. Any window opened after the swap loads HTML/asset
files fresh from disk, where the hashed asset filenames now point at
v(N+1) bundles that the in-memory v(N) IPC and preload layers don't
match. Symptoms observed across recent reports: Quick Entry rendering
as raw JS text, About dialog showing minified source, and Ctrl+Q
intermittently failing — anything where a post-swap window load
crosses the version boundary.

macOS / Windows clients get this from Squirrel; Linux deb/RPM has no
equivalent, so we watch the file ourselves and surface a click-to-
restart notification. AppImage is unaffected (squashfs mount stays
pinned to the running file's contents); Nix store paths are immutable
until GC, so the running inode also stays valid until explicit
relaunch. The watcher noop-quiet on those targets is deliberate.

Implementation: stat-baseline app.asar at first require('electron'),
watch the parent dir (file-level fs.watch loses the inode across
rename-replace; inotify on the dir reports the new entry via
IN_MOVED_TO), filename-filter, debounce 5s past the last event to
clear dpkg's .dpkg-new → rename dance, compare ino+mtime to confirm
a real change, then show a Notification deferred behind whenReady.
Click → app.relaunch(); app.quit().

Co-Authored-By: Claude <claude@anthropic.com>
@lizthegrey lizthegrey requested a review from aaddrick as a code owner May 3, 2026 02:20
Hoist the in-place-upgrade detection block in frame-fix-wrapper.js
into an armUpgradeWatcher() helper so the missing-baseline path
becomes an early return instead of an outer if (baseline) wrap.
Collapse the isReady() ? show() : whenReady().then(show) ternary
to plain whenReady().then(show) — Electron's whenReady() resolves
immediately when the app is already ready, so the branch was
dead. Trim narrative comments that duplicate the previous commit
message; keep the why-comments that earn their keep (parent-dir
watch, ino+mtime, 5s debounce, watcher.unref).

Behaviour preserved: filename filter, 5s debounce, ino+mtime
double-check, watcher.unref(), best-effort try/catch, idempotent
notified guard. Net -22 lines.

Co-Authored-By: Claude <claude@anthropic.com>
@aaddrick
Copy link
Copy Markdown
Owner

aaddrick commented May 3, 2026

I accidentally ran into something related to this earlier today while working on a testing suite to validate functionality between distros. Issue #567

I'll come back with more details momentarily

@aaddrick
Copy link
Copy Markdown
Owner

aaddrick commented May 3, 2026

Hey @lizthegrey! I dug into this further after the earlier review and wanted to share the research before we lock in a direction. Trying to figure out the right shape here together rather than render a verdict.

Context

I walked the upstream autoUpdater path in build-reference/app-extracted/.vite/build/index.js. One nuance worth being explicit about: Squirrel stages an update and fires update-downloaded once the swap is ready. dpkg and rpm apply the swap in-place under a running process. So the macOS/Windows signal isn't actually reproducible on Linux deb/rpm. By the time anything could fire, the file is already swapped. This isn't recreating Squirrel; it's filling a different gap. Post-hoc detection of a running stale process. The Quick Entry / About / Ctrl+Q symptom cluster matches the v(N)-process / v(N+1)-on-disk skew you described.

The implementation looks solid from static analysis. Parent-dir fs.watch is correct (file-level loses the inode across rename-replace), and the filename filter, 5s debounce, dual ino + mtimeMs check, watcher.unref(), and stat-failure early return all do the right thing. AppImage and Nix paths should be correctly inert — squashfs mount stays pinned to the running file's contents, Nix store is read-only.

I also did a quick ecosystem search to see if there's prior art. Nobody in Electron-land has a clean solution. Official autoUpdater is unimplemented on Linux. electron-updater explicitly punts on deb/rpm. VSCode doesn't auto-update on Linux at all. Discord polls a server for "update available" but doesn't detect the file swap. Slack relies entirely on apt-source integration. electron/electron#2198 has been open since 2015. So this approach appears genuinely novel, and probably worth a docs/learnings/ entry post-merge so future contributors don't go hunting for a standard library.

Options

There's a related issue (#567) covering a separate bug: Electron's autoUpdater is structurally enabled on Linux because ELECTRON_FORCE_IS_PACKAGED=true satisfies the eligibility gate. It's only saved from firing because Linux autoUpdater is unimplemented upstream. The plan for #567 is a defensive stub in frame-fix-wrapper.js — no-op setFeedURL / checkForUpdates, redirect quitAndInstall to app.relaunch(); app.quit(), forward listeners to a local EventEmitter.

That stub creates a hook point. There's a spectrum of how aggressively we use it:

Level 1 — pure stub. What #567 proposes standalone. Defensive against future Electron bumps that implement Linux autoUpdater. Doesn't pretend to be anything more.

Level 2 — stub plus synthetic emission. Same stub, but this PR's inotify watcher additionally emits a synthetic update-downloaded after firing libnotify. The upstream state machine (vc) flips to "ready", the in-app menu item becomes "Restart and Install", and desktop_update_downloaded telemetry fires. Linux installs start matching macOS/Windows on the upstream dashboards. Tiny diff over Level 1. Static analysis says no platform gating in the relevant code path.

Level 3 — full hijack. Substitute our own feed URL. We already run a Cloudflare Worker at pkg.claude-desktop-debian.dev fronting the apt/dnf metadata, and a latest.json endpoint would be a small addition. Implement the polling that Electron's Linux autoUpdater doesn't, so checkForUpdates() actually does something. Two-stage flow: update-available when the repo has a newer version (lights up the menu before the user runs apt upgrade), update-downloaded when inotify fires (after they have). Combined with a launcher-shim boot-time version check, every state of "running stale / running current / upgraded-while-closed / upgraded-while-running" has a first-class signal.

Caveats

Level 3 has an awkward "click does what?" question for the update-available state. Clicking the menu item there shouldn't relaunch — nothing to load yet. Options are shell.openExternal() to the system update center, copying the apt command to clipboard with a follow-up notification, or staying passive until the file actually swaps. Probably the last one, but I'm not certain.

Telemetry implication for Levels 2 and 3: the upstream update-* handlers all call It("desktop_update_*", ...). Going Level 2 or 3 means Linux installs start showing up on upstream's update dashboards. Arguably good — parity, gives them visibility — but it's a behavior change worth being explicit about. We'd be pumping events upstream currently doesn't see from Linux.

Coverage note

Inotify covers "upgraded while running." There's a sister gap for "upgraded while closed, opened cold" that nothing upstream handles either. I plan to file separately as a launcher-shim diff against a stored last-seen-version. The launcher already has clean hook points between the cleanup functions and setup_electron_env. Doesn't depend on this PR.

The ask

Which level feels right? A few possible sequences:

Open to whatever sequencing makes sense, and happy to write whichever piece you don't want to.


Written by Claude opus-4.7 via Claude Code

@aaddrick
Copy link
Copy Markdown
Owner

aaddrick commented May 3, 2026

I'd say, land this PR to shore up the immediate issue, then check in with the team about a path forward. Would be interested in your perspective as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants