fix: linked desktops keep a plugin when its update from the server fails

src/backend/sync/client/plugins.ts

@@ -16,10 +16,12 @@ import { sendCoreAlert } from "../../notify/core-notify.js";
 import {
   getPluginRuntime,
   installPluginArtifact,
+  registerLoadedPlugins,
   setPluginEnabled,
   unloadPlugin,
 } from "../../plugins/index.js";
 import { getPluginsDir } from "../../plugins/paths.js";
+import type { LoadedPlugin } from "../../plugins/loader.js";
 import {
   createCurrentPluginPermissionGrantRepository,
   createCurrentPluginRepository,
@@ -56,6 +58,7 @@ async function isEnabledLocally(pluginId: string): Promise<boolean> {
 
 const PLUGIN_ID_PATTERN = /^[a-z0-9][a-z0-9-]*$/;
 
+/** Downloads next to the installed copy, which stays until the swap. */
 async function download(link: SyncLink, plugin: RemotePlugin): Promise<string> {
   const response = await remoteFetch(
     link,
@@ -68,7 +71,7 @@ async function download(link: SyncLink, plugin: RemotePlugin): Promise<string> {
   const buffer = Buffer.from(await response.arrayBuffer());
   const dir = getPluginsDir();
   await fs.promises.mkdir(dir, { recursive: true });
-  const file = path.join(dir, `${plugin.id}.tmxplug`);
+  const file = path.join(dir, `${plugin.id}.tmxplug.download`);
   await fs.promises.writeFile(file, buffer);
   const signature = response.headers.get("x-termix-signature");
   const sigFile = `${file}.sig`;
@@ -83,6 +86,68 @@ async function download(link: SyncLink, plugin: RemotePlugin): Promise<string> {
   return file;
 }
 
+async function moveIfThere(from: string, to: string): Promise<void> {
+  await fs.promises.rm(to, { force: true });
+  if (fs.existsSync(from)) await fs.promises.rename(from, to);
+}
+
+/**
+ * Swaps a downloaded copy in for the installed one. If it fails to install,
+ * the old copy goes back on disk and is loaded again, so a bad download never
+ * leaves the desktop without the plugin.
+ */
+async function replaceWithDownload(
+  pluginId: string,
+  downloaded: string,
+  previous: LoadedPlugin | undefined,
+  wasEnabled: boolean,
+): Promise<LoadedPlugin> {
+  const file = path.join(getPluginsDir(), `${pluginId}.tmxplug`);
+  const backup = `${file}.previous`;
+  await moveIfThere(file, backup);
+  await moveIfThere(`${file}.sig`, `${backup}.sig`);
+  await moveIfThere(downloaded, file);
+  await moveIfThere(`${downloaded}.sig`, `${file}.sig`);
+  if (previous) await unloadPlugin(pluginId);
+  try {
+    const installed = await installPluginArtifact(file);
+    await fs.promises.rm(backup, { force: true });
+    await fs.promises.rm(`${backup}.sig`, { force: true });
+    return installed;
+  } catch (error) {
+    await moveIfThere(backup, file);
+    await moveIfThere(`${backup}.sig`, `${file}.sig`);
+    if (previous) await restorePrevious(pluginId, previous, file, wasEnabled);
+    throw error;
+  }
+}
+
+async function restorePrevious(
+  pluginId: string,
+  previous: LoadedPlugin,
+  file: string,
+  wasEnabled: boolean,
+): Promise<void> {
+  const { loader } = getPluginRuntime();
+  if (loader.get(pluginId)) return;
+  try {
+    if (fs.existsSync(file)) {
+      await installPluginArtifact(file);
+    } else if (previous.source === "bundled") {
+      await registerLoadedPlugins([await loader.loadBundled(pluginId)]);
+    }
+    if (wasEnabled && loader.get(pluginId)) {
+      await setPluginEnabled(pluginId, true);
+    }
+  } catch (error) {
+    syncLogger.warn("Could not restore a feature after a failed update", {
+      operation: "sync_plugin_restore",
+      pluginId,
+      error: error instanceof Error ? error.message : String(error),
+    });
+  }
+}
+
 async function matchGrants(plugin: RemotePlugin): Promise<void> {
   const repository = createCurrentPluginPermissionGrantRepository();
   const local = getPluginRuntime().loader.get(plugin.id);
@@ -161,9 +226,14 @@ async function mirrorOne(link: SyncLink, plugin: RemotePlugin): Promise<void> {
       // A signed update of a bundled plugin. The loader checks the signature
       // and that the version really is newer.
       const wasEnabled = await isEnabledLocally(plugin.id);
-      if (local) await unloadPlugin(plugin.id);
       try {
-        local = await installPluginArtifact(await download(link, plugin));
+        const downloaded = await download(link, plugin);
+        local = await replaceWithDownload(
+          plugin.id,
+          downloaded,
+          local,
+          wasEnabled,
+        );
       } catch (error) {
         await alertOlderDesktop(plugin);
         throw error;
@@ -180,9 +250,13 @@ async function mirrorOne(link: SyncLink, plugin: RemotePlugin): Promise<void> {
       local.manifest.version !== plugin.version;
     if (outdated && (!local || local.source === "user")) {
       const wasEnabled = local ? await isEnabledLocally(plugin.id) : false;
-      if (local) await unloadPlugin(plugin.id);
-      const file = await download(link, plugin);
-      local = await installPluginArtifact(file);
+      const downloaded = await download(link, plugin);
+      local = await replaceWithDownload(
+        plugin.id,
+        downloaded,
+        local,
+        wasEnabled,
+      );
       if (wasEnabled && !plugin.enabled) {
         await setPluginEnabled(plugin.id, false);
       }

src/backend/tests/sync/client/plugins.test.ts

@@ -0,0 +1,157 @@
+import fs from "fs";
+import os from "os";
+import path from "path";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+
+const state = vi.hoisted(() => ({
+  dir: "",
+  loaded: new Map<string, Record<string, unknown>>(),
+  enabled: new Set<string>(),
+  install: vi.fn(),
+  fetch: vi.fn(),
+  loadBundled: vi.fn(),
+}));
+
+vi.mock("../../../utils/logger.js", () => ({
+  syncLogger: { warn: vi.fn(), info: vi.fn(), error: vi.fn() },
+}));
+vi.mock("../../../notify/core-notify.js", () => ({
+  sendCoreAlert: vi.fn(async () => {}),
+}));
+vi.mock("../../../plugins/paths.js", () => ({
+  getPluginsDir: () => state.dir,
+}));
+vi.mock("../../../database/repositories/factory.js", () => ({
+  createCurrentPluginRepository: () => ({
+    findById: async (id: string) => ({
+      state: state.enabled.has(id) ? "enabled" : "disabled",
+    }),
+  }),
+  createCurrentPluginPermissionGrantRepository: () => ({}),
+}));
+vi.mock("../../../sync/client/http.js", () => ({
+  remoteFetch: state.fetch,
+  remoteJson: vi.fn(),
+}));
+vi.mock("../../../plugins/index.js", () => ({
+  getPluginRuntime: () => ({
+    loader: {
+      get: (id: string) => state.loaded.get(id),
+      list: () => [...state.loaded.values()],
+      loadBundled: state.loadBundled,
+    },
+  }),
+  installPluginArtifact: state.install,
+  registerLoadedPlugins: vi.fn(async () => {}),
+  setPluginEnabled: vi.fn(async (id: string, on: boolean) => {
+    if (on) state.enabled.add(id);
+    else state.enabled.delete(id);
+  }),
+  unloadPlugin: vi.fn(async (id: string) => {
+    state.loaded.delete(id);
+  }),
+}));
+
+const { mirrorPlugins } = await import("../../../sync/client/plugins.js");
+const { remoteJson } = await import("../../../sync/client/http.js");
+
+const bundled = {
+  id: "ssh-terminal",
+  source: "bundled",
+  state: "active",
+  manifest: { id: "ssh-terminal", version: "1.0.0", capabilities: [] },
+};
+
+function serverHas(version: string) {
+  vi.mocked(remoteJson).mockResolvedValue({
+    plugins: [
+      {
+        id: "ssh-terminal",
+        name: "SSH Terminal",
+        version,
+        source: "bundled",
+        artifact: true,
+        enabled: true,
+        active: true,
+        desktop: "mirror",
+        syncEntities: [],
+        granted: [],
+      },
+    ],
+  });
+}
+
+beforeEach(() => {
+  state.dir = fs.mkdtempSync(path.join(os.tmpdir(), "termix-mirror-"));
+  state.loaded = new Map([["ssh-terminal", { ...bundled }]]);
+  state.enabled = new Set(["ssh-terminal"]);
+  state.install.mockReset();
+  state.fetch.mockReset();
+  state.loadBundled.mockReset();
+  state.loadBundled.mockImplementation(async (id: string) => {
+    const plugin = { ...bundled, id };
+    state.loaded.set(id, plugin);
+    return plugin;
+  });
+});
+
+afterEach(() => {
+  fs.rmSync(state.dir, { recursive: true, force: true });
+});
+
+describe("mirrorPlugins", () => {
+  it("keeps the installed copy when the download fails", async () => {
+    serverHas("1.1.0");
+    state.fetch.mockResolvedValue({ ok: false, status: 502 });
+
+    await mirrorPlugins({} as never);
+
+    expect(state.loaded.has("ssh-terminal")).toBe(true);
+    expect(state.enabled.has("ssh-terminal")).toBe(true);
+    expect(state.install).not.toHaveBeenCalled();
+  });
+
+  it("puts the shipped copy back when the new one fails to install", async () => {
+    serverHas("1.1.0");
+    state.fetch.mockResolvedValue({
+      ok: true,
+      arrayBuffer: async () => new ArrayBuffer(4),
+      headers: new Headers(),
+    });
+    state.install.mockRejectedValue(new Error("signature check failed"));
+
+    await mirrorPlugins({} as never);
+
+    expect(state.loadBundled).toHaveBeenCalledWith("ssh-terminal");
+    expect(state.loaded.has("ssh-terminal")).toBe(true);
+    expect(state.enabled.has("ssh-terminal")).toBe(true);
+    expect(fs.existsSync(path.join(state.dir, "ssh-terminal.tmxplug"))).toBe(
+      false,
+    );
+  });
+
+  it("restores an earlier update file when a newer one fails", async () => {
+    serverHas("1.2.0");
+    const file = path.join(state.dir, "ssh-terminal.tmxplug");
+    fs.writeFileSync(file, "old");
+    state.fetch.mockResolvedValue({
+      ok: true,
+      arrayBuffer: async () => new ArrayBuffer(4),
+      headers: new Headers(),
+    });
+    state.install
+      .mockRejectedValueOnce(new Error("bad archive"))
+      .mockImplementationOnce(async () => {
+        const plugin = { ...bundled };
+        state.loaded.set("ssh-terminal", plugin);
+        return plugin;
+      });
+
+    await mirrorPlugins({} as never);
+
+    expect(fs.readFileSync(file, "utf8")).toBe("old");
+    expect(state.install).toHaveBeenCalledTimes(2);
+    expect(state.loaded.has("ssh-terminal")).toBe(true);
+    expect(state.enabled.has("ssh-terminal")).toBe(true);
+  });
+});