fix(collections): keep running checks after a collection installs

src/renderer/src/extensions/collections/util/InstallDriver.suppression.test.ts

@@ -0,0 +1,252 @@
+/**
+ * A collection install holds off the expensive checks (plugins-changed, mod-installed, …) while
+ * it runs, through the test runner's withSuppressedTests. These tests drive the REAL driver
+ * through the collection harness and check the hold is released however the install ends — and
+ * that the checks it held off then run once, since suppressed events are dropped, not queued.
+ *
+ * The regression they pin: a *completed* install, closed from the review screen, never released
+ * the hold, so no Missing Masters (or other plugins-changed) check ran again until Vortex
+ * restarted.
+ */
+import { describe, expect, vi } from "vitest";
+
+import { makeCollectionModInfo, makeDownload, makeRevision } from "../../../test-utils/builders";
+import { test } from "../../../test-utils/collectionTest";
+import type { ICollectionHarness } from "../../../test-utils/harnessTypes";
+
+const GAME = "skyrimse";
+const COLLECTION = "col-1";
+const ARCHIVE = "dl-col-1";
+
+function downloadOverride(gameVersions?: string[]) {
+  const modInfo = makeCollectionModInfo({ collectionId: 1, revisionId: 2, gameId: GAME });
+  if (gameVersions !== undefined) {
+    // revision info carried on the download, so the driver reads it without a network fetch
+    modInfo.nexus.revisionInfo = {
+      modFiles: [],
+      gameVersions: gameVersions.map((reference) => ({ reference })),
+    };
+  }
+  return {
+    downloads: {
+      [ARCHIVE]: makeDownload({ id: ARCHIVE, state: "finished", modInfo }),
+    },
+  };
+}
+
+interface IRerun {
+  event: string;
+  delay: number | undefined;
+  // holds still outstanding on that event when the re-run was requested. The real runner drops
+  // a run while any hold is outstanding, so anything but 0 means the re-run is lost.
+  heldAtEmit: number;
+}
+
+/**
+ * Stands in for the test runner: counts holds per event and records re-run requests. `lowest`
+ * is the smallest count any event reached; below 0 means a hold was released twice.
+ */
+function trackSuppression(h: ICollectionHarness) {
+  const held: Record<string, number> = {};
+  const reruns: IRerun[] = [];
+  const counts = { lowest: 0 };
+  const release = (test: string) => {
+    held[test] -= 1;
+    counts.lowest = Math.min(counts.lowest, held[test]);
+  };
+  (h.api.ext as Record<string, unknown>).withSuppressedTests = (
+    tests: string[],
+    cb: () => PromiseLike<void>,
+  ) => {
+    tests.forEach((test) => (held[test] = (held[test] ?? 0) + 1));
+    return Promise.resolve(cb()).finally(() => tests.forEach(release));
+  };
+  h.api.events.on("trigger-test-run", (event: string, delay?: number) => {
+    // only the events a hold covered; the driver emits others (collections-changed) itself
+    if (event in held) {
+      reruns.push({ event, delay, heldAtEmit: held[event] });
+    }
+  });
+  return { held, reruns, counts };
+}
+
+/** Mark every member installed and let the driver reach its review step, without closing it. */
+async function reachReview(h: ICollectionHarness, collectionId: string, recommendations: boolean) {
+  h.setState((draft) => {
+    const session = draft.session.collections.activeSession;
+    for (const id of Object.keys(session?.mods ?? {})) {
+      session.mods[id].status = "installed";
+    }
+  });
+  h.emit("did-install-dependencies", GAME, collectionId, recommendations);
+  // the recommendations pass sets the review step without firing onUpdate, so poll the step
+  await vi.waitFor(() => expect(h.driver.step).toBe("review"));
+}
+
+/** Let the released hold's promise chain settle. */
+const settle = () => new Promise((resolve) => setTimeout(resolve, 0));
+
+describe("InstallDriver check suppression", () => {
+  test("a completed install releases the hold and re-runs the held-off checks", async ({
+    makeCollection,
+  }) => {
+    const h = makeCollection(downloadOverride());
+    const { held, reruns } = trackSuppression(h);
+
+    await h.installRevision(makeRevision(1, [{ tag: "a" }], { collectionId: COLLECTION }));
+    expect(held["plugins-changed"]).toBe(1);
+
+    await h.completeActiveInstall();
+    await settle();
+
+    expect(held["plugins-changed"]).toBe(0);
+    expect(held["mod-installed"]).toBe(0);
+    expect(reruns).toEqual([
+      { event: "plugins-changed", delay: 500, heldAtEmit: 0 },
+      { event: "settings-changed", delay: 500, heldAtEmit: 0 },
+      { event: "mod-activated", delay: 5000, heldAtEmit: 0 },
+      { event: "mod-installed", delay: 5000, heldAtEmit: 0 },
+    ]);
+  });
+
+  test("a cancelled install releases the hold", async ({ makeCollection }) => {
+    const h = makeCollection(downloadOverride());
+    const { held } = trackSuppression(h);
+
+    await h.installRevision(makeRevision(1, [{ tag: "a" }], { collectionId: COLLECTION }));
+    h.driver.cancel();
+    await settle();
+
+    expect(held["plugins-changed"]).toBe(0);
+  });
+
+  test("an install cancelled at the game-version prompt releases the hold", async ({
+    makeCollection,
+  }) => {
+    const h = makeCollection(downloadOverride(["9.9.9"]));
+    const { held, reruns } = trackSuppression(h);
+    h.setNextDialog({ action: "Cancel", input: {} });
+
+    await h.installRevision(makeRevision(1, [{ tag: "a" }], { collectionId: COLLECTION }));
+    await settle();
+
+    expect(h.dialogCalls.map((call) => call.title)).toContain("Game version mismatch");
+    expect(held["plugins-changed"]).toBe(0);
+    expect(reruns.map((rerun) => rerun.heldAtEmit)).toEqual([0, 0, 0, 0]);
+  });
+
+  test("a pause releases the hold once and resuming takes it again", async ({ makeCollection }) => {
+    const h = makeCollection(downloadOverride());
+    const { held, reruns, counts } = trackSuppression(h);
+    const revision = makeRevision(1, [{ tag: "a" }], { collectionId: COLLECTION });
+
+    await h.installRevision(revision);
+    h.driver.pause("user");
+    // a paused install that is then removed goes through onStop a second time
+    h.driver.cancel();
+    await settle();
+
+    expect(held["plugins-changed"]).toBe(0);
+    expect(reruns.map((rerun) => rerun.heldAtEmit)).toEqual([0, 0, 0, 0]);
+
+    // Resume (the notification action and resume-collection) is driver.start again
+    await h.installRevision(revision);
+    expect(held["plugins-changed"]).toBe(1);
+
+    await h.completeActiveInstall();
+    await settle();
+
+    expect(held["plugins-changed"]).toBe(0);
+    expect(counts.lowest).toBe(0);
+  });
+
+  test("resuming an install that ended incomplete does not stack a second hold", async ({
+    makeCollection,
+  }) => {
+    const h = makeCollection(downloadOverride());
+    const { held, reruns, counts } = trackSuppression(h);
+    const revision = makeRevision(1, [{ tag: "a" }], { collectionId: COLLECTION });
+
+    await h.installRevision(revision);
+    await h.driver.continue();
+    expect(h.driver.step).toBe("installing");
+    // the required pass ends with a member still pending: the driver marks the install done but
+    // keeps the collection and stays on "installing", so neither close nor onStop runs
+    h.setState((draft) => {
+      draft.session.collections.activeSession.mods["requires_a"].status = "pending";
+    });
+    h.emit("did-install-dependencies", GAME, revision.collection.id, false);
+    await vi.waitFor(() => expect(h.driver.installDone).toBe(true));
+    expect(h.driver.step).toBe("installing");
+    expect(h.driver.collection).toBeDefined();
+    expect(held["plugins-changed"]).toBe(1);
+
+    // Resume (resume-collection, or the restart after a premium change) is driver.start again
+    await h.installRevision(revision);
+    await settle();
+    expect(held["plugins-changed"]).toBe(1);
+
+    await h.completeActiveInstall();
+    await settle();
+
+    expect(held["plugins-changed"]).toBe(0);
+    expect(counts.lowest).toBe(0);
+    // the completed install's release is the one that re-runs the checks with no hold left
+    expect(reruns.slice(-4)).toEqual([
+      { event: "plugins-changed", delay: 500, heldAtEmit: 0 },
+      { event: "settings-changed", delay: 500, heldAtEmit: 0 },
+      { event: "mod-activated", delay: 5000, heldAtEmit: 0 },
+      { event: "mod-installed", delay: 5000, heldAtEmit: 0 },
+    ]);
+  });
+
+  test("installing the optional mods from the review screen keeps the one hold", async ({
+    makeCollection,
+  }) => {
+    const h = makeCollection(downloadOverride());
+    const { held, reruns, counts } = trackSuppression(h);
+    const revision = makeRevision(1, [{ tag: "a" }], { collectionId: COLLECTION });
+
+    await h.installRevision(revision);
+    await reachReview(h, revision.collection.id, false);
+
+    // the review screen offers the optional mods again after each pass
+    for (let pass = 0; pass < 2; pass += 1) {
+      h.driver.installRecommended();
+      expect(held["plugins-changed"]).toBe(1);
+      await reachReview(h, revision.collection.id, true);
+    }
+    expect(reruns).toEqual([]);
+
+    await h.driver.continue();
+    await settle();
+
+    expect(held["plugins-changed"]).toBe(0);
+    expect(counts.lowest).toBe(0);
+    expect(reruns.map((rerun) => rerun.event)).toEqual([
+      "plugins-changed",
+      "settings-changed",
+      "mod-activated",
+      "mod-installed",
+    ]);
+  });
+
+  test("installing a second collection after the first does not stack holds", async ({
+    makeCollection,
+  }) => {
+    const h = makeCollection(downloadOverride());
+    const { held } = trackSuppression(h);
+
+    await h.installRevision(makeRevision(1, [{ tag: "a" }], { collectionId: COLLECTION }));
+    await h.completeActiveInstall();
+    await settle();
+    await h.installRevision(makeRevision(2, [{ tag: "b" }], { collectionId: COLLECTION }));
+
+    expect(held["plugins-changed"]).toBe(1);
+
+    await h.completeActiveInstall();
+    await settle();
+
+    expect(held["plugins-changed"]).toBe(0);
+  });
+});

src/renderer/src/extensions/collections/util/InstallDriver.ts

@@ -76,6 +76,17 @@ export type Step =
 
 export type UpdateCB = () => void;
 
+/**
+ * Test events whose checks are held off while a collection installs, with the delay the test
+ * runner itself uses for each, so the re-run on release waits as long as a natural one would.
+ */
+const SUPPRESSED_TEST_EVENTS: Record<string, number> = {
+  "plugins-changed": 500,
+  "settings-changed": 500,
+  "mod-activated": 5000,
+  "mod-installed": 5000,
+};
+
 class InstallDriver {
   private mApi: IExtensionApi;
   private mProfile: IProfile;
@@ -796,20 +807,44 @@ class InstallDriver {
   }
 
   private startInstall = async () => {
-    // suppress plugins-changed event to avoid constantly running expensive callbacks
-    // until onStop gets called
-    this.mApi.ext.withSuppressedTests?.(
-      ["plugins-changed", "settings-changed", "mod-activated", "mod-installed"],
-      () =>
-        new Bluebird((resolve) => {
-          this.mOnStop = () => {
-            resolve(undefined);
-            this.mOnStop = undefined;
-          };
-        }),
-    );
+    // A start while this driver still holds a suppression (a second collection straight after
+    // the first) must not stack another one on top of it.
+    this.mOnStop?.();
+
+    // suppress the expensive checks while the collection installs, until onStop or close
+    // releases them
+    this.mApi.ext
+      .withSuppressedTests?.(
+        Object.keys(SUPPRESSED_TEST_EVENTS),
+        () =>
+          new Bluebird((resolve) => {
+            this.mOnStop = () => {
+              resolve(undefined);
+              this.mOnStop = undefined;
+            };
+          }),
+      )
+      // Suppressed events are dropped, not queued: re-run their checks once released, or a
+      // problem the collection itself brought in (a missing master) goes unreported until
+      // something else happens to change the plugins.
+      ?.then(() => {
+        Object.entries(SUPPRESSED_TEST_EVENTS).forEach(([event, delay]) =>
+          this.mApi.events.emit("trigger-test-run", event, delay),
+        );
+      });
 
-    return this.startImpl();
+    // An install that never starts (no archive or profile, the game-version prompt cancelled,
+    // or an error) reaches neither onStop nor close, so release the hold here.
+    try {
+      const started = await this.startImpl();
+      if (started === false) {
+        this.mOnStop?.();
+      }
+      return started;
+    } catch (err) {
+      this.mOnStop?.();
+      throw err;
+    }
   };
 
   private startImpl = async () => {
@@ -1105,6 +1140,11 @@ class InstallDriver {
     this.mCollection = undefined;
     this.setDependentMods([]);
     this.mInstallDone = true;
+    // The review's close is how a successful install ends, so it has to release the checks
+    // startInstall suppressed, as onStop does for a cancel or pause. Without it every check on
+    // plugins-changed, mod-installed, mod-activated and settings-changed (Missing Masters among
+    // them) stops running for the rest of the session.
+    this.mOnStop?.();
     this.triggerUpdate();
   };