fix(plugins): prevent stale version writes during updates

src/main/services/PluginManager.ts

@@ -164,12 +164,14 @@ export class PluginManager {
   private readonly hookRegistryPath: string;
   private readonly plugins = new Map<string, InstalledPlugin>();
   private readonly pending = new Map<string, PendingInstall>();
+  private readonly updatingPlugins = new Map<string, Promise<InstalledPlugin>>();
   private readonly storageWrites = new Map<string, Promise<void>>();
   private readonly downloadRepository: DownloadRepository;
   private readonly downloadFullRepository: DownloadRepository;
   private readonly downloadModuleFiles: DownloadModuleFiles;
   private tokenProvider: () => Promise<string | null>;
   private registryWrite = Promise.resolve();
+  private versionsWrite = Promise.resolve();
 
   constructor(
     userDataPath: string,
@@ -647,38 +649,54 @@ export class PluginManager {
   }
 
   async checkForUpdates(): Promise<PluginUpdateStatus[]> {
-    const installed = [...this.plugins.values()]
+    const sources = [...this.plugins.values()]
       .filter((plugin) => plugin.enabled && plugin.sourceUrl)
       .sort((left, right) => left.manifest.id.localeCompare(right.manifest.id));
-    const versions = await this.readVersions();
-    const updates: PluginUpdateStatus[] = [];
     // Batch: one GraphQL metadata round-trip for all manifests, then raw
     // fetches for present files — far fewer requests than one per plugin.
-    const remoteVersions = await fetchRemoteManifestVersions(installed.map((plugin) => plugin.sourceUrl));
-    for (const plugin of installed) {
-      const latest = remoteVersions.get(plugin.sourceUrl);
-      if (latest === undefined) {
-        console.warn(`CanvasTTY could not check plugin update: ${plugin.manifest.id}.`);
-        continue;
-      }
-      versions[plugin.manifest.id] = {
-        installedVersion: plugin.manifest.version,
-        latestVersion: latest,
-        checkedAt: Date.now()
-      };
-      if (latest !== plugin.manifest.version) {
-        updates.push({
-          pluginId: plugin.manifest.id,
+    const remoteVersions = await fetchRemoteManifestVersions(sources.map((plugin) => plugin.sourceUrl));
+    return this.withVersionsLock((versions) => {
+      const installed = [...this.plugins.values()]
+        .filter((plugin) => plugin.enabled && plugin.sourceUrl)
+        .sort((left, right) => left.manifest.id.localeCompare(right.manifest.id));
+      const updates: PluginUpdateStatus[] = [];
+      for (const plugin of installed) {
+        const latest = remoteVersions.get(plugin.sourceUrl);
+        if (latest === undefined) {
+          console.warn(`CanvasTTY could not check plugin update: ${plugin.manifest.id}.`);
+          continue;
+        }
+        versions[plugin.manifest.id] = {
           installedVersion: plugin.manifest.version,
-          latestVersion: latest
-        });
+          latestVersion: latest,
+          checkedAt: Date.now()
+        };
+        if (latest !== plugin.manifest.version) {
+          updates.push({
+            pluginId: plugin.manifest.id,
+            installedVersion: plugin.manifest.version,
+            latestVersion: latest
+          });
+        }
       }
-    }
-    await this.persistVersions(versions);
-    return updates;
+      return updates;
+    });
+  }
+
+  updatePlugin(pluginId: string): Promise<InstalledPlugin> {
+    const inFlight = this.updatingPlugins.get(pluginId);
+    if (inFlight) return inFlight;
+
+    const update = this.performPluginUpdate(pluginId);
+    this.updatingPlugins.set(pluginId, update);
+    const clearInFlight = () => {
+      if (this.updatingPlugins.get(pluginId) === update) this.updatingPlugins.delete(pluginId);
+    };
+    void update.then(clearInFlight, clearInFlight);
+    return update;
   }
 
-  async updatePlugin(pluginId: string): Promise<InstalledPlugin> {
+  private async performPluginUpdate(pluginId: string): Promise<InstalledPlugin> {
     const plugin = this.requirePlugin(pluginId);
     if (plugin.enabledHooks.length > 0) {
       plugin.enabledHooks = [];
@@ -729,13 +747,13 @@ export class PluginManager {
       };
       this.plugins.set(pluginId, updated);
       await this.persistRegistry();
-      const versions = await this.readVersions();
-      versions[pluginId] = {
-        installedVersion: manifest.version,
-        latestVersion: manifest.version,
-        checkedAt: Date.now()
-      };
-      await this.persistVersions(versions);
+      await this.withVersionsLock((versions) => {
+        versions[pluginId] = {
+          installedVersion: manifest.version,
+          latestVersion: manifest.version,
+          checkedAt: Date.now()
+        };
+      });
       return structuredClone(activePlugin(updated));
     } catch (error) {
       if (currentBackedUp) {
@@ -786,6 +804,19 @@ export class PluginManager {
     await rename(temporaryPath, this.versionsPath);
   }
 
+  private withVersionsLock<T>(
+    update: (versions: Record<string, StoredVersionRecord>) => T | Promise<T>
+  ): Promise<T> {
+    const operation = this.versionsWrite.catch(() => undefined).then(async () => {
+      const versions = await this.readVersions();
+      const result = await update(versions);
+      await this.persistVersions(versions);
+      return result;
+    });
+    this.versionsWrite = operation.then(() => undefined, () => undefined);
+    return operation;
+  }
+
   contribution(pluginId: string, contributionId: string): PluginContribution {
     const plugin = activePlugin(this.requireEnabledPlugin(pluginId));
     const contribution = plugin.manifest.contributions.find((candidate) => candidate.id === contributionId);

tests/plugin-manager.test.mjs

@@ -1317,6 +1317,148 @@ test("updatePlugin restores the previous package when metadata persistence fails
   }
 });
 
+test("concurrent updatePlugin calls for one plugin share one in-flight update", async () => {
+  const userData = await mkdtemp(join(tmpdir(), "canvastty-plugin-update-singleflight-"));
+  const fixture = await mkdtemp(join(tmpdir(), "canvastty-plugin-update-singleflight-fixture-"));
+  let version = "1.0.0";
+  let updateDownloads = 0;
+  const writeFixture = async () => {
+    await rm(fixture, { recursive: true, force: true });
+    await mkdir(fixture, { recursive: true });
+    await writeFile(join(fixture, "app.html"), `<h1>${version}</h1>`, "utf8");
+    await writeFile(join(fixture, "canvastty.plugin.json"), JSON.stringify({
+      apiVersion: 1,
+      id: "com.example.update-singleflight",
+      name: "Update Singleflight",
+      version,
+      description: "Concurrent update fixture.",
+      permissions: [],
+      contributions: [{
+        id: "app",
+        kind: "canvas-app",
+        title: "App",
+        entry: "app.html",
+        defaultSize: { width: 480, height: 300 }
+      }]
+    }), "utf8");
+  };
+  await writeFixture();
+  let trackUpdateDownloads = false;
+  const manager = new PluginManager(userData, async (_url, destination) => {
+    if (trackUpdateDownloads) updateDownloads += 1;
+    await cp(fixture, destination, { recursive: true });
+  });
+  try {
+    await manager.load();
+    await manager.install((await manager.previewInstall("https://github.com/example/update-singleflight")).token);
+    version = "2.0.0";
+    await writeFixture();
+    trackUpdateDownloads = true;
+
+    const results = await Promise.allSettled([
+      manager.updatePlugin("com.example.update-singleflight"),
+      manager.updatePlugin("com.example.update-singleflight")
+    ]);
+
+    assert.deepEqual(results.map((result) => result.status), ["fulfilled", "fulfilled"]);
+    const [first, second] = results.map((result) => result.value);
+    assert.equal(updateDownloads, 1);
+    assert.equal(first.manifest.version, "2.0.0");
+    assert.equal(second.manifest.version, "2.0.0");
+    assert.equal(manager.list()[0].manifest.version, "2.0.0");
+    const versions = JSON.parse(await readFile(join(userData, "plugin-versions.json"), "utf8"));
+    assert.equal(versions["com.example.update-singleflight"].installedVersion, "2.0.0");
+  } finally {
+    await manager.dispose();
+    await rm(userData, { recursive: true, force: true });
+    await rm(fixture, { recursive: true, force: true });
+  }
+});
+
+test("checkForUpdates keeps installed version current when an update finishes during its fetch", async () => {
+  const originalFetch = globalThis.fetch;
+  const previousToken = process.env.GITHUB_TOKEN;
+  const previousCanvasToken = process.env.CANVASTTY_GITHUB_TOKEN;
+  delete process.env.GITHUB_TOKEN;
+  delete process.env.CANVASTTY_GITHUB_TOKEN;
+  const userData = await mkdtemp(join(tmpdir(), "canvastty-plugin-update-check-race-"));
+  const fixture = await mkdtemp(join(tmpdir(), "canvastty-plugin-update-check-race-fixture-"));
+  let version = "1.0.0";
+  let remoteVersion = "1.0.0";
+  let pauseRemoteFetch = false;
+  let signalRemoteStarted;
+  const remoteStarted = new Promise((resolve) => { signalRemoteStarted = resolve; });
+  let releaseRemoteFetch;
+  const remoteGate = new Promise((resolve) => { releaseRemoteFetch = resolve; });
+  const manifestFor = (manifestVersion) => ({
+    apiVersion: 1,
+    id: "com.example.update-check-race",
+    name: "Update Check Race",
+    version: manifestVersion,
+    description: "Version state race fixture.",
+    permissions: [],
+    contributions: [{
+      id: "app",
+      kind: "canvas-app",
+      title: "App",
+      entry: "app.html",
+      defaultSize: { width: 480, height: 300 }
+    }]
+  });
+  const writeFixture = async () => {
+    await rm(fixture, { recursive: true, force: true });
+    await mkdir(fixture, { recursive: true });
+    await writeFile(join(fixture, "app.html"), `<h1>${version}</h1>`, "utf8");
+    await writeFile(join(fixture, "canvastty.plugin.json"), JSON.stringify(manifestFor(version)), "utf8");
+  };
+  await writeFixture();
+  const manager = new PluginManager(userData, async (_url, destination) => {
+    await cp(fixture, destination, { recursive: true });
+  });
+  try {
+    globalThis.fetch = async (url) => {
+      const text = String(url);
+      if (text === "https://api.github.com/repos/example/update-check-race") {
+        return Response.json({ default_branch: "main" });
+      }
+      if (text === "https://raw.githubusercontent.com/example/update-check-race/main/canvastty.plugin.json") {
+        if (pauseRemoteFetch) {
+          signalRemoteStarted();
+          await remoteGate;
+        }
+        return Response.json(manifestFor(remoteVersion));
+      }
+      return new Response("missing", { status: 404 });
+    };
+    await manager.load();
+    await manager.install((await manager.previewInstall("https://github.com/example/update-check-race")).token);
+
+    version = "2.0.0";
+    remoteVersion = "2.0.0";
+    await writeFixture();
+    pauseRemoteFetch = true;
+    const checking = manager.checkForUpdates();
+    await remoteStarted;
+    await manager.updatePlugin("com.example.update-check-race");
+    releaseRemoteFetch();
+
+    assert.deepEqual(await checking, []);
+    const versions = JSON.parse(await readFile(join(userData, "plugin-versions.json"), "utf8"));
+    assert.equal(versions["com.example.update-check-race"].installedVersion, "2.0.0");
+    assert.equal(versions["com.example.update-check-race"].latestVersion, "2.0.0");
+  } finally {
+    releaseRemoteFetch();
+    globalThis.fetch = originalFetch;
+    if (previousToken === undefined) delete process.env.GITHUB_TOKEN;
+    else process.env.GITHUB_TOKEN = previousToken;
+    if (previousCanvasToken === undefined) delete process.env.CANVASTTY_GITHUB_TOKEN;
+    else process.env.CANVASTTY_GITHUB_TOKEN = previousCanvasToken;
+    await manager.dispose();
+    await rm(userData, { recursive: true, force: true });
+    await rm(fixture, { recursive: true, force: true });
+  }
+});
+
 test("validatePluginManifest accepts icon and localized descriptions", () => {
   const valid = validatePluginManifest({
     apiVersion: 1,