From 2758a11f0b237d507643f85d959ab3c2120ddede Mon Sep 17 00:00:00 2001 From: doodlum <15017472+doodlum@users.noreply.github.com> Date: Wed, 23 Sep 2026 23:57:10 +0100 Subject: [PATCH 1/3] fix(collections): release the check suppression when an install completes startInstall suppresses the plugins-changed, mod-installed, mod-activated and settings-changed checks while a collection installs, and only onStop (cancel, pause) released them. A successful install ends through the review screen's close(), which never did, so from then on none of those checks ran again for the rest of the session. The Missing Masters check is one of them: a user who finished a collection and then fixed or broke a load order was never told. - close() releases the hold, as onStop does. - startInstall releases a hold it still owns before taking another, so consecutive collections cannot stack. - Once released, the held-off checks run once. Suppressed events are dropped, not queued, so a missing master the collection itself brought in would otherwise go unreported until something else changed the plugins. Reproduced in a source build against a disposable Fallout 4 fixture and an offline (bundled-member) collection, counting check runs with a probe check registered through registerTest. After the collection closes, installing and deploying a plugin with a missing master: - unpatched: plugins-changed checks run 0 times, no missing-master flag - patched: checks re-run on close, plugins-changed runs 2 times, the plugin is flagged missing-master Co-Authored-By: Claude Opus 5.5 --- .../util/InstallDriver.suppression.test.ts | 106 ++++++++++++++++++ .../collections/util/InstallDriver.ts | 48 ++++++-- 2 files changed, 142 insertions(+), 12 deletions(-) create mode 100644 src/renderer/src/extensions/collections/util/InstallDriver.suppression.test.ts diff --git a/src/renderer/src/extensions/collections/util/InstallDriver.suppression.test.ts b/src/renderer/src/extensions/collections/util/InstallDriver.suppression.test.ts new file mode 100644 index 0000000000..a3f18d25b6 --- /dev/null +++ b/src/renderer/src/extensions/collections/util/InstallDriver.suppression.test.ts @@ -0,0 +1,106 @@ +/** + * 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 Bluebird from "bluebird"; +import { describe, expect } 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() { + return { + downloads: { + [ARCHIVE]: makeDownload({ + id: ARCHIVE, + state: "finished", + modInfo: makeCollectionModInfo({ collectionId: 1, revisionId: 2, gameId: GAME }), + }), + }, + }; +} + +/** Stands in for the test runner: counts holds per event and records re-run requests. */ +function trackSuppression(h: ICollectionHarness) { + const held: Record = {}; + const reruns: string[] = []; + (h.api.ext as Record).withSuppressedTests = ( + tests: string[], + cb: () => Bluebird, + ) => { + tests.forEach((test) => (held[test] = (held[test] ?? 0) + 1)); + return cb().finally(() => tests.forEach((test) => (held[test] -= 1))); + }; + h.api.events.on("trigger-test-run", (event: string) => reruns.push(event)); + return { held, reruns }; +} + +/** 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( + expect.arrayContaining([ + "plugins-changed", + "mod-installed", + "mod-activated", + "settings-changed", + ]), + ); + }); + + 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("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); + }); +}); diff --git a/src/renderer/src/extensions/collections/util/InstallDriver.ts b/src/renderer/src/extensions/collections/util/InstallDriver.ts index b4e2e2a333..5d889cdc8f 100644 --- a/src/renderer/src/extensions/collections/util/InstallDriver.ts +++ b/src/renderer/src/extensions/collections/util/InstallDriver.ts @@ -76,6 +76,14 @@ export type Step = export type UpdateCB = () => void; +/** Test events whose checks are held off while a collection installs. */ +const SUPPRESSED_TEST_EVENTS = [ + "plugins-changed", + "settings-changed", + "mod-activated", + "mod-installed", +]; + class InstallDriver { private mApi: IExtensionApi; private mProfile: IProfile; @@ -796,18 +804,29 @@ 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?.( + 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(() => { + SUPPRESSED_TEST_EVENTS.forEach((event) => this.mApi.events.emit("trigger-test-run", event)); + }); return this.startImpl(); }; @@ -1105,6 +1124,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(); }; From f3e46dd136310762513147d1aeaef4ca0d663f7d Mon Sep 17 00:00:00 2001 From: doodlum <15017472+doodlum@users.noreply.github.com> Date: Thu, 24 Sep 2026 01:34:27 +0100 Subject: [PATCH 2/3] fix(collections): release the check hold when an install never starts An install that returns early from startImpl (no archive or profile, the game-version prompt cancelled) or throws reached neither onStop nor close, so the hold startInstall took on the test runner leaked. Release it there. Re-run the held-off checks with the delays the test runner itself uses for each event (500 ms for plugins-changed and settings-changed, 5000 ms for mod-installed and mod-activated) rather than the runner's 500 ms default. The suppression tests now use native promises, record how many holds were outstanding when each re-run was requested (a re-run emitted while held is dropped), and cover the game-version cancel, pause then resume, and installing the optional mods from the review screen more than once. Co-Authored-By: Claude Opus 5.5 --- .../util/InstallDriver.suppression.test.ts | 150 +++++++++++++++--- .../collections/util/InstallDriver.ts | 36 +++-- 2 files changed, 154 insertions(+), 32 deletions(-) diff --git a/src/renderer/src/extensions/collections/util/InstallDriver.suppression.test.ts b/src/renderer/src/extensions/collections/util/InstallDriver.suppression.test.ts index a3f18d25b6..e79c6b4fdb 100644 --- a/src/renderer/src/extensions/collections/util/InstallDriver.suppression.test.ts +++ b/src/renderer/src/extensions/collections/util/InstallDriver.suppression.test.ts @@ -8,8 +8,7 @@ * the hold, so no Missing Masters (or other plugins-changed) check ran again until Vortex * restarted. */ -import Bluebird from "bluebird"; -import { describe, expect } from "vitest"; +import { describe, expect, vi } from "vitest"; import { makeCollectionModInfo, makeDownload, makeRevision } from "../../../test-utils/builders"; import { test } from "../../../test-utils/collectionTest"; @@ -19,31 +18,69 @@ const GAME = "skyrimse"; const COLLECTION = "col-1"; const ARCHIVE = "dl-col-1"; -function downloadOverride() { +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: makeCollectionModInfo({ collectionId: 1, revisionId: 2, gameId: GAME }), - }), + [ARCHIVE]: makeDownload({ id: ARCHIVE, state: "finished", modInfo }), }, }; } -/** Stands in for the test runner: counts holds per event and records re-run requests. */ +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 = {}; - const reruns: string[] = []; + 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).withSuppressedTests = ( tests: string[], - cb: () => Bluebird, + cb: () => PromiseLike, ) => { tests.forEach((test) => (held[test] = (held[test] ?? 0) + 1)); - return cb().finally(() => tests.forEach((test) => (held[test] -= 1))); + return Promise.resolve(cb()).finally(() => tests.forEach(release)); }; - h.api.events.on("trigger-test-run", (event: string) => reruns.push(event)); - return { held, reruns }; + 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. */ @@ -64,14 +101,12 @@ describe("InstallDriver check suppression", () => { expect(held["plugins-changed"]).toBe(0); expect(held["mod-installed"]).toBe(0); - expect(reruns).toEqual( - expect.arrayContaining([ - "plugins-changed", - "mod-installed", - "mod-activated", - "settings-changed", - ]), - ); + 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 }) => { @@ -85,6 +120,77 @@ describe("InstallDriver check suppression", () => { 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("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, }) => { diff --git a/src/renderer/src/extensions/collections/util/InstallDriver.ts b/src/renderer/src/extensions/collections/util/InstallDriver.ts index 5d889cdc8f..8bb4a604d0 100644 --- a/src/renderer/src/extensions/collections/util/InstallDriver.ts +++ b/src/renderer/src/extensions/collections/util/InstallDriver.ts @@ -76,13 +76,16 @@ export type Step = export type UpdateCB = () => void; -/** Test events whose checks are held off while a collection installs. */ -const SUPPRESSED_TEST_EVENTS = [ - "plugins-changed", - "settings-changed", - "mod-activated", - "mod-installed", -]; +/** + * 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 = { + "plugins-changed": 500, + "settings-changed": 500, + "mod-activated": 5000, + "mod-installed": 5000, +}; class InstallDriver { private mApi: IExtensionApi; @@ -812,7 +815,7 @@ class InstallDriver { // releases them this.mApi.ext .withSuppressedTests?.( - SUPPRESSED_TEST_EVENTS, + Object.keys(SUPPRESSED_TEST_EVENTS), () => new Bluebird((resolve) => { this.mOnStop = () => { @@ -825,10 +828,23 @@ class InstallDriver { // problem the collection itself brought in (a missing master) goes unreported until // something else happens to change the plugins. ?.then(() => { - SUPPRESSED_TEST_EVENTS.forEach((event) => this.mApi.events.emit("trigger-test-run", event)); + 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 () => { From d23b5a2e678f129faa9c34f3b4f964e834b1e381 Mon Sep 17 00:00:00 2001 From: doodlum <15017472+doodlum@users.noreply.github.com> Date: Thu, 24 Sep 2026 03:20:47 +0100 Subject: [PATCH 3/3] test(collections): cover resuming an install that ended incomplete A required pass that ends with a member still pending leaves the driver on "installing" with the collection set and the check hold still taken. Resuming it calls start again; the guard at the top of startInstall must release that hold before taking a new one, or the first is never released. Nothing covered the guard: deleting it kept every test green. Co-Authored-By: Claude Opus 5.5 --- .../util/InstallDriver.suppression.test.ts | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/renderer/src/extensions/collections/util/InstallDriver.suppression.test.ts b/src/renderer/src/extensions/collections/util/InstallDriver.suppression.test.ts index e79c6b4fdb..b5bd439494 100644 --- a/src/renderer/src/extensions/collections/util/InstallDriver.suppression.test.ts +++ b/src/renderer/src/extensions/collections/util/InstallDriver.suppression.test.ts @@ -160,6 +160,46 @@ describe("InstallDriver check suppression", () => { 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, }) => {