fix(desktop): require user action before applying updates (#1820)

Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@sprout-oss.stage.blox.sqprod.co>

desktop/src/features/settings/SidebarUpdateCard.tsx

@@ -21,63 +21,63 @@ export function SidebarUpdateCompactCard({
   onDismiss,
   testId = "sidebar-update-card-compact",
 }: SidebarUpdateCompactCardProps) {
-  const { relaunch } = useUpdaterContext();
-  const [isRestartPending, setIsRestartPending] = React.useState(false);
-  const restartPendingRef = React.useRef(false);
-  const restartFrameRef = React.useRef<number | null>(null);
-  const restartTimeoutRef = React.useRef<number | null>(null);
+  const { installAndRelaunch } = useUpdaterContext();
+  const [isUpdatePending, setIsUpdatePending] = React.useState(false);
+  const updatePendingRef = React.useRef(false);
+  const updateFrameRef = React.useRef<number | null>(null);
+  const updateTimeoutRef = React.useRef<number | null>(null);
 
   React.useEffect(() => {
     return () => {
-      if (restartFrameRef.current !== null) {
-        window.cancelAnimationFrame(restartFrameRef.current);
+      if (updateFrameRef.current !== null) {
+        window.cancelAnimationFrame(updateFrameRef.current);
       }
-      if (restartTimeoutRef.current !== null) {
-        window.clearTimeout(restartTimeoutRef.current);
+      if (updateTimeoutRef.current !== null) {
+        window.clearTimeout(updateTimeoutRef.current);
       }
-      restartPendingRef.current = false;
+      updatePendingRef.current = false;
     };
   }, []);
 
-  const handleRestart = React.useCallback(() => {
-    if (restartPendingRef.current) {
+  const handleUpdate = React.useCallback(() => {
+    if (updatePendingRef.current) {
       return;
     }
 
-    restartPendingRef.current = true;
-    setIsRestartPending(true);
-    restartFrameRef.current = window.requestAnimationFrame(() => {
-      restartFrameRef.current = null;
-      restartTimeoutRef.current = window.setTimeout(() => {
-        restartTimeoutRef.current = null;
-        void relaunch()
+    updatePendingRef.current = true;
+    setIsUpdatePending(true);
+    updateFrameRef.current = window.requestAnimationFrame(() => {
+      updateFrameRef.current = null;
+      updateTimeoutRef.current = window.setTimeout(() => {
+        updateTimeoutRef.current = null;
+        void installAndRelaunch()
           .catch((error) => {
-            console.error("[SidebarUpdateCard] relaunch failed:", error);
+            console.error("[SidebarUpdateCard] update failed:", error);
           })
           .finally(() => {
-            restartPendingRef.current = false;
-            setIsRestartPending(false);
+            updatePendingRef.current = false;
+            setIsUpdatePending(false);
           });
       }, 0);
     });
-  }, [relaunch]);
+  }, [installAndRelaunch]);
 
   return (
     <SidebarCompactActionCard
-      actionAriaLabel="Restart now to apply update"
-      actionDisabled={isRestartPending}
+      actionAriaLabel="Update now"
+      actionDisabled={isUpdatePending}
       actionTestId={actionTestId}
-      description={isRestartPending ? "Restarting" : "Click to restart"}
+      description={isUpdatePending ? "Updating" : "Click to update"}
       dismissLabel="Dismiss update notification"
       icon={
-        isRestartPending ? (
+        isUpdatePending ? (
           <Spinner aria-hidden="true" className="h-5 w-5 border-2" />
         ) : (
           <CircleArrowUp aria-hidden="true" className="h-5 w-5" />
         )
       }
-      iconKey={isRestartPending ? "pending" : "idle"}
-      onAction={handleRestart}
+      iconKey={isUpdatePending ? "pending" : "idle"}
+      onAction={handleUpdate}
       onDismiss={onDismiss}
       testId={testId}
       title="Ready to update!"
@@ -111,7 +111,7 @@ export function SidebarUpdateCard({ onDismiss }: SidebarUpdateCardProps) {
 
   return (
     <SidebarUpdateCompactCard
-      actionTestId="sidebar-update-restart"
+      actionTestId="sidebar-update-now"
       onDismiss={onDismiss}
       testId="sidebar-update-card"
     />

desktop/src/features/settings/UpdateChecker.tsx

@@ -7,7 +7,7 @@ import {
 } from "./ui/SettingsOptionGroup";
 import { SettingsSectionHeader } from "./ui/SettingsSectionHeader";
 export function UpdateChecker() {
-  const { status, checkForUpdate, relaunch } = useUpdaterContext();
+  const { status, checkForUpdate, installAndRelaunch } = useUpdaterContext();
 
   return (
     <section className="min-w-0" data-testid="settings-updates">
@@ -129,11 +129,11 @@ export function UpdateChecker() {
             <div className="min-w-0">
               <p className="text-sm font-medium">Update status</p>
               <p className="text-sm font-normal text-muted-foreground">
-                Update installed. Restart to apply.
+                Update downloaded. Click to apply.
               </p>
             </div>
-            <Button size="sm" onClick={relaunch}>
-              Restart Now
+            <Button size="sm" onClick={installAndRelaunch}>
+              Update Now
             </Button>
           </SettingsOptionRow>
         )}

desktop/src/features/settings/UpdateIndicator.tsx

@@ -51,7 +51,7 @@ const variants: Record<
   },
   ready: {
     Icon: RotateCw,
-    label: "Restart to update",
+    label: "Update now",
     badgeColor: "bg-emerald-500",
   },
 };
@@ -70,7 +70,7 @@ function getVariant(state: UpdateStatus["state"]) {
 }
 
 export function UpdateIndicator({ className }: { className?: string }) {
-  const { status, downloadAndInstall, relaunch } = useUpdaterContext();
+  const { status, installAndRelaunch } = useUpdaterContext();
   const variant = getVariant(status.state);
 
   if (!variant) {
@@ -79,19 +79,15 @@ export function UpdateIndicator({ className }: { className?: string }) {
 
   const { Icon, iconClassName = "h-4 w-4", label, badgeColor } = variant;
   const isActionable =
-    status.state === "available" ||
-    status.state === "ready" ||
-    status.state === "manual-required";
+    status.state === "ready" || status.state === "manual-required";
   const handleClick =
     status.state === "ready"
-      ? relaunch
+      ? installAndRelaunch
       : status.state === "manual-required"
         ? () => {
             void openUrl(status.releaseUrl);
           }
-        : status.state === "available"
-          ? downloadAndInstall
-          : null;
+        : null;
 
   return (
     <Tooltip>

desktop/src/features/settings/hooks/use-updater.ts

@@ -57,6 +57,7 @@ export function useUpdater() {
   const updateRef = useRef<Update | null>(null);
   const checkInFlightRef = useRef(false);
   const downloadInFlightRef = useRef(false);
+  const installInFlightRef = useRef(false);
   const manualResultRequestedRef = useRef(false);
 
   const setStatus = useCallback((nextStatus: UpdateStatus) => {
@@ -65,7 +66,7 @@ export function useUpdater() {
   }, []);
 
   const closeUpdate = useCallback(async () => {
-    if (downloadInFlightRef.current) {
+    if (downloadInFlightRef.current || installInFlightRef.current) {
       return;
     }
     const current = updateRef.current;
@@ -75,7 +76,7 @@ export function useUpdater() {
     }
   }, []);
 
-  const downloadAndInstall = useCallback(async () => {
+  const downloadUpdate = useCallback(async () => {
     if (downloadInFlightRef.current) {
       return;
     }
@@ -88,14 +89,7 @@ export function useUpdater() {
       }
 
       setStatus({ state: "downloading" });
-
-      await update.downloadAndInstall((event) => {
-        if (event.event === "Finished") {
-          setStatus({ state: "installing" });
-        }
-      });
-
-      updateRef.current = null;
+      await update.download();
       setStatus({ state: "ready" });
     } catch (err) {
       setStatus({ state: "error", message: toErrorMessage(err) });
@@ -104,6 +98,29 @@ export function useUpdater() {
     }
   }, [setStatus]);
 
+  const installAndRelaunch = useCallback(async () => {
+    if (installInFlightRef.current) {
+      return;
+    }
+
+    const update = updateRef.current;
+    if (!update) {
+      return;
+    }
+
+    installInFlightRef.current = true;
+    try {
+      setStatus({ state: "installing" });
+      await update.install();
+      updateRef.current = null;
+      await relaunch();
+    } catch (err) {
+      setStatus({ state: "error", message: toErrorMessage(err) });
+    } finally {
+      installInFlightRef.current = false;
+    }
+  }, [setStatus]);
+
   const runUpdateCheck = useCallback(
     async ({ background }: { background: boolean }) => {
       if (checkInFlightRef.current) {
@@ -137,17 +154,16 @@ export function useUpdater() {
         if (update) {
           // Check support BEFORE exposing any actionable state — on a Linux
           // .deb, the window between "available" and "manual-required" would
-          // let a click reach downloadAndInstall on an un-updatable install.
+          // let a click reach an un-updatable install.
           const autoUpdateOk = await isAutoUpdateSupported();
           updateRef.current = update;
           if (autoUpdateOk) {
             setStatus({ state: "available", version: update.version });
-            // Start download automatically — user sees "restart" when done
-            void downloadAndInstall();
+            void downloadUpdate();
           } else {
             // .deb / non-AppImage: surface manual-download card instead.
             // updateRef is intentionally NOT retained — no install handle
-            // should be kept when we will never call downloadAndInstall.
+            // should be kept when we will never install in-app.
             updateRef.current = null;
             setStatus({
               state: "manual-required",
@@ -182,7 +198,7 @@ export function useUpdater() {
         checkInFlightRef.current = false;
       }
     },
-    [closeUpdate, downloadAndInstall, setStatus],
+    [closeUpdate, downloadUpdate, setStatus],
   );
 
   const checkForUpdate = useCallback(async () => {
@@ -193,14 +209,6 @@ export function useUpdater() {
     await runUpdateCheck({ background: true });
   }, [runUpdateCheck]);
 
-  const handleRelaunch = useCallback(async () => {
-    try {
-      await relaunch();
-    } catch (err) {
-      setStatus({ state: "error", message: toErrorMessage(err) });
-    }
-  }, [setStatus]);
-
   useEffect(() => {
     void checkForUpdateInBackground();
 
@@ -217,7 +225,6 @@ export function useUpdater() {
   return {
     status,
     checkForUpdate,
-    downloadAndInstall,
-    relaunch: handleRelaunch,
+    installAndRelaunch,
   };
 }

desktop/src/features/settings/sidebarUpdateCardVisibility.test.mjs

@@ -0,0 +1,30 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+
+import { shouldShowSidebarUpdateCard } from "./sidebarUpdateCardVisibility.ts";
+
+test("shows the card for states that require user action or feedback", () => {
+  assert.equal(shouldShowSidebarUpdateCard({ state: "ready" }), true);
+  assert.equal(shouldShowSidebarUpdateCard({ state: "installing" }), true);
+  assert.equal(shouldShowSidebarUpdateCard({ state: "manual-required" }), true);
+});
+
+test("hides the card for states with nothing actionable to show", () => {
+  assert.equal(shouldShowSidebarUpdateCard({ state: "idle" }), false);
+  assert.equal(shouldShowSidebarUpdateCard({ state: "checking" }), false);
+  assert.equal(shouldShowSidebarUpdateCard({ state: "up-to-date" }), false);
+  assert.equal(shouldShowSidebarUpdateCard({ state: "unavailable" }), false);
+  assert.equal(shouldShowSidebarUpdateCard({ state: "available" }), false);
+  assert.equal(shouldShowSidebarUpdateCard({ state: "downloading" }), false);
+});
+
+// Pre-existing behavior, unchanged by this fix: an install failure hides the
+// card rather than surfacing an error state in the sidebar. The background
+// check will retry and can re-show "available"/"ready" on its own schedule,
+// so this is left as-is rather than scoped into the installing-state fix.
+test("hides the card on error, matching pre-existing behavior", () => {
+  assert.equal(
+    shouldShowSidebarUpdateCard({ state: "error", message: "boom" }),
+    false,
+  );
+});

desktop/src/features/settings/sidebarUpdateCardVisibility.ts

@@ -1,3 +1,7 @@
 export function shouldShowSidebarUpdateCard(status: { state: string }) {
-  return status.state === "ready" || status.state === "manual-required";
+  return (
+    status.state === "ready" ||
+    status.state === "installing" ||
+    status.state === "manual-required"
+  );
 }

desktop/src/testing/e2eBridge.ts

@@ -5688,7 +5688,7 @@ function handleUpdaterCheck(config: E2eConfig | undefined) {
   };
 }
 
-async function handleUpdaterDownloadAndInstall(
+async function handleUpdaterDownload(
   payload: unknown,
   config: E2eConfig | undefined,
 ) {
@@ -5699,6 +5699,10 @@ async function handleUpdaterDownloadAndInstall(
   }
 
   notifyUpdaterFinished(payload);
+  return 43;
+}
+
+function handleUpdaterInstall() {
   return null;
 }
 
@@ -9352,8 +9356,10 @@ export function maybeInstallE2eTauriMocks() {
         return null;
       case "plugin:updater|check":
         return handleUpdaterCheck(activeConfig);
-      case "plugin:updater|download_and_install":
-        return handleUpdaterDownloadAndInstall(payload, activeConfig);
+      case "plugin:updater|download":
+        return handleUpdaterDownload(payload, activeConfig);
+      case "plugin:updater|install":
+        return handleUpdaterInstall();
       case "is_auto_update_supported":
         // Default true so all existing tests continue to use the auto-update
         // path. Set mock.autoUpdateSupported: false to simulate a .deb install.

desktop/tests/e2e/sidebar.spec.ts

@@ -263,23 +263,35 @@ test("shows a sidebar update card when an update is ready", async ({
   await page.getByTestId("settings-nav-updates").click();
   await page.getByRole("button", { name: "Check for Updates" }).click();
   await expect(page.getByTestId("settings-panel-updates")).toContainText(
-    "Update installed. Restart to apply.",
+    "Update downloaded. Click to apply.",
   );
+  await expect
+    .poll(() =>
+      page.evaluate(() => {
+        const commands =
+          (
+            window as Window & {
+              __BUZZ_E2E_COMMANDS__?: string[];
+            }
+          ).__BUZZ_E2E_COMMANDS__ ?? [];
+        return (
+          commands.includes("plugin:updater|install") ||
+          commands.includes("plugin:process|restart")
+        );
+      }),
+    )
+    .toBe(false);
 
   await page.getByTestId("settings-back-to-app").click();
 
   const updateCard = page.getByTestId("sidebar-update-card");
   await expect(updateCard).toBeVisible();
   await expect(updateCard).toContainText("Ready to update!");
-  await expect(updateCard).toContainText("Click to restart");
-  await expect(page.getByTestId("sidebar-update-restart")).toBeVisible();
-  const reservedCardHeight = await updateCard.evaluate(
-    (element) => (element as HTMLElement).offsetHeight,
-  );
-
-  await page.getByTestId("sidebar-update-restart").click();
-  await expect(updateCard).toContainText("Restarting");
-  await expect(page.getByTestId("sidebar-update-restart")).toBeDisabled();
+  await expect(updateCard).toContainText("Click to update");
+  await expect(page.getByTestId("sidebar-update-now")).toBeVisible();
+  await page.getByTestId("sidebar-update-now").click();
+  await expect(updateCard).toContainText("Updating");
+  await expect(page.getByTestId("sidebar-update-now")).toBeDisabled();
 
   await expect
     .poll(() =>
@@ -292,42 +304,34 @@ test("shows a sidebar update card when an update is ready", async ({
           ).__BUZZ_E2E_COMMANDS__ ?? [],
       ),
     )
-    .toContain("plugin:process|restart");
+    .toEqual(
+      expect.arrayContaining([
+        "plugin:updater|download",
+        "plugin:updater|install",
+        "plugin:process|restart",
+      ]),
+    );
 
-  const dismissButton = page.getByTestId("sidebar-update-dismiss");
-  await updateCard.hover();
-  const dismissButtonBox = await dismissButton.boundingBox();
-  expect(dismissButtonBox).not.toBeNull();
-  if (!dismissButtonBox) return;
-
-  await page.mouse.move(
-    dismissButtonBox.x + dismissButtonBox.width / 2,
-    dismissButtonBox.y + dismissButtonBox.height / 2,
+  const commands = await page.evaluate(
+    () =>
+      (
+        window as Window & {
+          __BUZZ_E2E_COMMANDS__?: string[];
+        }
+      ).__BUZZ_E2E_COMMANDS__ ?? [],
+  );
+  expect(commands.indexOf("plugin:updater|download")).toBeLessThan(
+    commands.indexOf("plugin:updater|install"),
+  );
+  expect(commands.indexOf("plugin:updater|install")).toBeLessThan(
+    commands.indexOf("plugin:process|restart"),
   );
-  await page.mouse.down();
-  await expect(page.locator(".buzz-poof-burst")).toHaveCount(1);
-  await expect(updateCard).toBeVisible();
-  await page.mouse.up();
-  await expect(updateCard).toHaveAttribute("data-dismissing", "true");
-  await expect
-    .poll(() =>
-      updateCard.evaluate((element) => (element as HTMLElement).offsetHeight),
-    )
-    .toBe(reservedCardHeight);
-  await expect
-    .poll(() =>
-      updateCard.evaluate((element) =>
-        Number.parseFloat(getComputedStyle(element).opacity),
-      ),
-    )
-    .toBeLessThan(0.05);
-  await expect(updateCard).toBeHidden();
 });
 
 // Regression test for the Linux .deb auto-update guard (PR #1535).
 // When auto-update is not supported (e.g. Linux .deb install), the update
 // check must surface a "manual-required" card with a GitHub link and
-// AppImage hint, and must NEVER invoke plugin:updater|download_and_install.
+// AppImage hint, and must NEVER invoke the in-app download or install commands.
 test("shows manual-required update card and never auto-downloads on non-AppImage installs", async ({
   page,
 }) => {
@@ -373,7 +377,7 @@ test("shows manual-required update card and never auto-downloads on non-AppImage
   await expect(updateCard).toBeVisible();
   await expect(updateCard).toContainText("AppImage");
 
-  // download_and_install must NEVER have been called.
+  // In-app download and install must NEVER have been called.
   const commands = await page.evaluate(
     () =>
       (
@@ -382,5 +386,6 @@ test("shows manual-required update card and never auto-downloads on non-AppImage
         }
       ).__BUZZ_E2E_COMMANDS__ ?? [],
   );
-  expect(commands).not.toContain("plugin:updater|download_and_install");
+  expect(commands).not.toContain("plugin:updater|download");
+  expect(commands).not.toContain("plugin:updater|install");
 });