Uh oh!
There was an error while loading. Please reload this page.
feat(desktop): make app updates background and task-aware - #1992
Conversation
Astro-Han
left a comment
There was a problem hiding this comment.
Review findings (5 independent deepseek-v4-flash perspectives)
The direction (updater lifecycle into main, auto-download, single-button install) is right, and the wiring against electron-updater@6.8.9 was verified correct (autoDownload/autoInstallOnAppQuit semantics, macOS delayed-install, cache sha512 re-validation, single-instance lock, fail-closed snapshot). Two blockers and several scope questions below.
P1 — Background shell/PTY tasks are invisible to the activity snapshot: an install silently kills them with no confirmation
- Predicted failure: a user runs a long
run_in_background=truetask (build/download/server — the feature's typical use), the turn ends, the user clicks Restart:getActiveTaskCount()returns 0 → the first request{maxInterruptibleActiveTasks: 0}passes →quitAndInstall(false, true)→shellRuns.terminateAll()kills the process tree. No dialog ever appears. This is exactly the "no work interruption without confirmation" scenario the PR claims to eliminate. - Evidence: snapshot composes only two sources —
boot.ts:648-650(sessionActivities.activeTaskCount(inFlightAutomationIds().map(automationActivityKey))). Background shell runs hold no activity lease (all threereserve(call sites are turn-scoped and unrelated to shell-run-manager). But background tasks do survive the turn:shell-run-manager.ts:72LifecycleCause = 'timeout' | 'cancel' | 'shutdown'has no turn-end cause;runBackgroundBash(:219) doesn't joinlive.finished. And they are killed on the update quit:app-lifecycle.ts:398shellRuns.terminateAll()runs in the shutdown path on both platforms (Windows:quitAndInstall→app.quit()→before-quit→ quit coordinator; macOS: Squirrel[NSApp terminate]→ Electron overrides-terminate:→Browser::Quit→HandleBeforeQuit→ same coordinator).liveCount()/livePtyCount()accessors already exist (shell-run-manager.ts:567-571). - Fix (pick one): (a) fold
shellRuns.liveCount()into the snapshot (one line + a test), or (b) explicitly narrow scope — state in the PR body and copy that background shell tasks are NOT protected by the confirmation.
P2 — Scope: the task-aware mechanism grew well beyond the issue's minimal design
Issue #1931 asks for "inject hasActiveTasks" — a boolean composed from existing main-process activity, plus a confirmation dialog; "if nothing is running, install immediately". The PR instead built a full authorization machine: precise counting with stable task-identity dedup (activityKey plumbing ×3, taskLeaseCounts parallel map, additionalTaskKeys, inFlightAutomationIds() lifecycle), a maxInterruptibleActiveTasks ceiling ("first request authorizes zero; follow-ups limited to that count"), and an escalation loop re-confirming when the count grows.
From first principles: after the user confirms, installUpdate calls quitAndInstall in the same synchronous tick — there is no "afterwards" window for a ceiling to protect. The only escalation-relevant window is "tasks start while the dialog is open", which a fresh count read handles without ceiling semantics. The precision machinery is also where the bugs live (this P1 and the per-session undercount below). Please choose: return to the minimal boolean + count-display design (which also makes the P1 fix trivial), or keep the ceiling mechanism with a written rationale for what it protects that a fresh re-read does not.
P2 — Download stall/failure leaves no in-app recovery path
checkForUpdates short-circuits on downloading/downloaded (app-update-service.ts:256), the sidebar button is disabled for available/downloading (session-sidebar-nav.tsx:116), and the click is a no-op in those states (app-shell.tsx:438-439). The old error/available click-triggered downloadUpdate() retry was removed. A download that stalls without an error event (hung socket/proxy) permanently freezes the surface until restart, and a failed auto-download is silent with a misleading "Update available" title. (Confirmed independently by 3 of 5 perspectives.)
P2 — installUpdate returns {ok:true} even when no install will occur
BaseUpdater.install reports failure only via dispatchError, never by throwing (BaseUpdater.js:13-27,51-53), and on macOS MacUpdater.quitAndInstall is fire-and-forget (MacUpdater.js:240-256) — a Squirrel-side failure (code-sign validation, proxy fetch) surfaces only as a forwarded 'error' event, after the IPC reply already returned ok. The renderer toasts only on kind === 'failed', so the user confirms interrupting work and nothing happens, silently. Suggest an installing status that maps a subsequent 'error' to install_failed.
P2 — Merge conflict with current main requires a semantic decision
git merge-tree shows a real conflict in app-lifecycle.ts: main (#1994) deleted project-startup-migration.ts (runProjectStartupMigration has zero references on main) while the PR head still calls it at app-lifecycle.ts:175; mechanical resolution produces TS2305. Rebase must decide whether to drop the migration call, then re-run desktop typecheck/build.
P3 (non-blocking)
- The boot.ts
getActiveTaskCountcomposition (the PR's headline invariant) has no integration test — this is why the P1 slipped through. - Same-session concurrent turns share
session:{id}key → the confirmation count understates streams killed (runtime-kernel.ts:166 explicitly supports concurrent runs;sessions:sendhas no idle gate). MAKA_UPDATE_MOCK_STATE=availableis now a dead-end (mock publishesavailableonce, install requiresdownloaded, button disabled).- Dead params
platform/arch(declaredapp-update-service.ts:59-60, passedboot.ts:641-642, never read). - Install authorization is renderer-trusted consent, not a main-side security boundary — acceptable given sandboxing, worth one sentence in the PR body.
Gate: FAIL (P1 open). The P1 must be fixed or explicitly narrowed before merge; P2s need handling or a written deferral with reason.
中文说明关于 task-aware 的设计，我有一点个人的观察，供你参考：issue #1931 原意是一个布尔判断 + 安装前确认，现在实现成了"精确计数 + 授权上限 + 重新确认"的状态机。确认后安装是立即执行的，这套机制实际保护的窗口很小，复杂度却主要集中在计数逻辑上——P1 的 shell 任务漏计也源于此。 只是建议，你可以按自己的判断来：如果回归到最小实现（布尔 + 并入 |
me2seeks
commented
Aug 3, 2026
感谢建议，这个分析很有帮助。我重新顺着安装时序看了一遍，ceiling 实际保护的主要是确认框打开期间“任务数量增加”这一小段，而且也无法识别数量不变时的任务替换，确实不值得为此保留整套计数状态机。 我会回到 issue 的最小方案：使用 boolean consent，把 |
0c0eaf4 to
2428c06Compare2428c06 to
fc1991dCompareAstro-Han
left a comment
There was a problem hiding this comment.
Approving. The two blockers from the previous round are genuinely fixed: hasInterruptibleUpdateWork is now a 17-line boolean union of session / Automation / shell-run activity, and the counting-plus-ceiling state machine (activityKey plumbing, taskLeaseCounts, escalation loop) is gone. Download stall/failure now has a real recovery path via retryUpdateDownload with cancellation-token semantics. CI is green and the branch merges cleanly with current main.
I re-reviewed the head through three independent perspectives (lifecycle correctness, design minimality, test quality); all three came back with no P0/P1. The main-process half reads as close to minimal: one timer, one boolean, three two-line read-only accessors, with the injected clock, operation-tagged error states, and mock fixture all load-bearing.
Four non-blocking items below. None gate this PR — fix, defer, or close them as you see fit.
P2 — installing has no exit when Squirrel stalls silently. On macOS with autoInstallOnAppQuit = false, update-downloaded is dispatched when the local proxy server starts, so squirrelDownloadedUpdate is still false at click time and MacUpdater.quitAndInstall takes the else branch (MacUpdater.js:240-256): it registers a listener, kicks off an async checkForUpdates(), and returns without quitting. Status stays installing, which short-circuits both checkForUpdates (app-update-service.ts:304) and retryUpdateDownload (:351), and updateReminder (app-shell.tsx:416-420) excludes installing, so the button disappears. A genuine Squirrel error is fine — MacUpdater.js:18-21 forwards it and you map it to error/install — so this is only the silent-hang path. Smallest fix: stop short-circuiting on installing and fall back to downloaded on the next scheduled check.
P3 — openUpdateDownload is now dead end to end. The sidebar button only renders for available | downloading | downloaded | error-with-latestVersion, and all four are routed to install or retry, so the else branch at app-shell.tsx:483 is unreachable — along with the preload method, the app:openUpdateDownload handler (app-ipc-main.ts:84), and the service method. Either delete the chain or wire it to a state that can actually reach it.
P3 — Retry is unguarded where install is guarded.isDisabled was removed from the sidebar button, and a click during downloading cancels the in-flight download and restarts from 0%. Install has updateInstallInFlightRef; retry has no equivalent, so the one destructive action in the surface is the least protected.
P3 — The renderer half has no tests.app-update-install.ts is covered as a pure function, but the wiring is not: the receivedPush race guard, the click-branch routing, and the double-click guard would all survive being reverted. AGENTS.md asks for one representative Playwright journey when a flow crosses renderer and main, and the deterministic seam (MAKA_UPDATE_MOCK_STATE, with installUpdate returning ok without quitting in mock mode) already exists and is unused.
中文说明
已 approve。上一轮的两个阻塞项确实修掉了：活动判断回到 17 行的布尔并集，计数 + ceiling 那套状态机整体删除，下载停滞也有了真实的取消重下路径。CI 全绿，与 main 无冲突。
我从生命周期正确性、设计最小性、测试质量三个独立视角重新过了一遍，都没有 P0/P1。主进程这一半基本已是最小解。
上面四条都不阻塞合并，是否处理你自己判断。P2 是 macOS 上 Squirrel 静默挂起时 installing 没有出口（真正报错的路径你已经兜住了）；两个 P3 分别是 openUpdateDownload 整条链路已不可达，以及 downloading 时点按钮会取消并从 0 重下且无保护；最后一个 P3 是 renderer 那一半没有测试，而确定性 mock seam 已经存在。
Uh oh!
There was an error while loading. Please reload this page.
Summary
Update discovery used to depend on the renderer staying alive, and installing a downloaded update had no view of work still running in the main process.
This moves the updater lifecycle into Electron main: the first check runs about 10 seconds after app readiness, later checks run four hours after the previous check settles, and
electron-updaterdownloads an available release automatically. The renderer now only projects the current status and listens for pushes; the existing sidebar button remains the only update surface, with no download-complete popup and no install-on-quit behavior.Restart/install is guarded by a main-owned activity snapshot. Session turns are counted by stable task identity, and Automation scheduler activity is merged with stream activity without double-counting the same fire. The first install request authorizes interrupting zero tasks. After the user confirms the reported count, the follow-up request is limited to that count; if more tasks start while the dialog is open, the new count is shown for confirmation instead of silently widening the authorization.
Closes#1931
中文说明
本 PR 将更新检查和自动下载的生命周期移到 Electron 主进程：应用 ready 约 10 秒后首次检查，之后在上一次检查结束 4 小时后再次检查。渲染进程只读取状态并订阅推送；下载完成后仍只通过现有侧栏按钮提示，不弹窗，也不会在退出时自动安装。
重启安装前，主进程会合并会话 turn 与 Automation scheduler 的活动状态，并按稳定任务标识去重。首次请求不允许中断任何任务；用户确认 N 个任务后，后续请求最多只能中断这 N 个任务。如果确认期间又启动了任务，会显示新的数量并再次询问，而不是扩大原授权。
Verification
@maka/runtimefull suite — 2,762 passed, 3 skipped@maka/desktopfull main suite — 1,354 passed@maka/uifull suite — 256 passednpm run typechecknpm run lint -- --error-on-warningsnpm run format:checknpm --workspace @maka/desktop run buildapps/desktopandpackages/uidownloaded, the sidebar exposed Restart without a completion popup, and the no-active-work path proceeded without confirmationReview focus
SessionActivityRegistry.activeTaskCount()andAutomationScheduler.inFlightAutomationIds()form the main-process activity snapshot; the shared automation key prevents a scheduler fire and its stream lease from counting twice.maxInterruptibleActiveTasksis an authorization ceiling, not a stale informational count. A higher current count returns to the confirmation flow.