From b32b36412910e9fd75905167abf4a3e9732c4e7f Mon Sep 17 00:00:00 2001 From: doodlum <15017472+doodlum@users.noreply.github.com> Date: Thu, 24 Sep 2026 03:30:43 +0100 Subject: [PATCH 1/2] fix(collections): let Cancel at the game-version prompt end the install When a collection revision lists game versions that do not match the installed game, the driver asks whether to continue. It had already moved to its "start" step, and Cancel only returned early, leaving the step and the collection in place. The collections extension continues any driver update that finds it on "start", so the next update, a second "Install Now", or (when resuming) the driver's own update straight after, began the install anyway. An update while the prompt was still open did the same. Cancel now cancels the driver, as the install dialog's "Later" does, which also releases the check suppression the install took. The driver moves to "start" only once nothing is left to ask, and an attempt that was paused or cancelled while the prompt was open stops whatever the answer. Co-Authored-By: Claude Opus 5.5 --- .../util/InstallDriver.gameVersion.test.ts | 173 ++++++++++++++++++ .../collections/util/InstallDriver.ts | 14 +- 2 files changed, 185 insertions(+), 2 deletions(-) create mode 100644 src/renderer/src/extensions/collections/util/InstallDriver.gameVersion.test.ts diff --git a/src/renderer/src/extensions/collections/util/InstallDriver.gameVersion.test.ts b/src/renderer/src/extensions/collections/util/InstallDriver.gameVersion.test.ts new file mode 100644 index 0000000000..e6871cdbf4 --- /dev/null +++ b/src/renderer/src/extensions/collections/util/InstallDriver.gameVersion.test.ts @@ -0,0 +1,173 @@ +/** + * When a collection revision lists game versions and none matches the installed game, the driver + * asks whether to go on ("Game version mismatch", Cancel / Continue). These tests drive the REAL + * driver through the collection harness, with the collections extension's own onUpdate handler + * (index.ts), which continues the driver whenever it sits on the "start" step. + * + * The regression they pin: Cancel left the driver on "start" with the collection still set, so + * the next driver update, or a second "Install Now", began the install anyway. + */ +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"; +import type { IDialogResult } from "../../../types/IDialog"; +import type { IProfile } from "../../profile_management/types/IProfile"; + +const GAME = "skyrimse"; +const COLLECTION = "col-1"; +const ARCHIVE = `dl-${COLLECTION}`; + +/** The collection download, carrying revision info that asks for a game version not installed. */ +function mismatchedGameVersion() { + const modInfo = makeCollectionModInfo({ collectionId: 1, revisionId: 2, gameId: GAME }); + // revision info on the download, so the driver reads it without a network fetch; the + // harness game reports 1.0.0 + modInfo.nexus.revisionInfo = { modFiles: [], gameVersions: [{ reference: "9.9.9" }] }; + return { downloads: { [ARCHIVE]: makeDownload({ id: ARCHIVE, state: "finished", modInfo }) } }; +} + +/** + * Wire up what the collections extension adds around the driver: its onUpdate handler, which + * continues from "start" (index.ts, "currently no UI associated with the start step"), and a + * count of the install-dependencies events that actually begin an install. + */ +function withExtensionHandler(h: ICollectionHarness) { + const begun: string[][] = []; + h.api.events.on("install-dependencies", (_profileId: string, _gameId: string, ids: string[]) => + begun.push(ids), + ); + let updates = 0; + h.driver.onUpdate(() => { + updates += 1; + if (h.driver.step === "start") { + void h.driver.continue(); + } + }); + return { begun, updates: () => updates }; +} + +/** Put the collection in state and open the install dialog, as a freshly added collection does. */ +async function openInstallDialog(h: ICollectionHarness) { + const revision = makeRevision(1, [{ tag: "a" }], { collectionId: COLLECTION }); + h.setState((draft) => { + (draft.persistent.mods[GAME] ??= {})[COLLECTION] = revision.collection; + }); + const profile: IProfile = h.getState().persistent.profiles["prof-1"]; + await h.driver.query(profile, revision.collection); + expect(h.driver.step).toBe("query"); + return revision; +} + +/** Let the driver's async continue() calls, started from onUpdate, run to completion. */ +const settle = () => new Promise((resolve) => setTimeout(resolve, 0)); + +describe("InstallDriver game-version prompt", () => { + test("Cancel from the install dialog ends the install and nothing later begins it", async ({ + makeCollection, + }) => { + const h = makeCollection(mismatchedGameVersion()); + const { begun, updates } = withExtensionHandler(h); + await openInstallDialog(h); + h.setNextDialog({ action: "Cancel", input: {} }); + + // "Install Now" + await h.driver.continue(); + await settle(); + + expect(h.dialogCalls.map((call) => call.title)).toEqual(["Game version mismatch"]); + // back to the state a collection nobody is installing leaves, as "Later" does; the update + // lets the install dialog close + expect(h.driver.step).toBe("prepare"); + expect(h.driver.collection).toBeUndefined(); + const updatesAtCancel = updates(); + expect(updatesAtCancel).toBeGreaterThan(1); + + // an unrelated mod install updates the driver, then "Install Now" is pressed again + h.emit("did-install-mod", GAME, "other-archive", "other-mod"); + await h.driver.continue(); + await settle(); + + expect(updates()).toBeGreaterThan(updatesAtCancel); + expect(begun).toEqual([]); + expect(h.getState().session.collections.activeSession).toBeUndefined(); + }); + + test("Cancel when resuming an install does not begin it", async ({ makeCollection }) => { + const h = makeCollection(mismatchedGameVersion()); + const { begun } = withExtensionHandler(h); + h.setNextDialog({ action: "Cancel", input: {} }); + + // resume (the notification, resume-collection) is driver.start + await h.installRevision(makeRevision(1, [{ tag: "a" }], { collectionId: COLLECTION })); + await settle(); + + expect(h.dialogCalls.map((call) => call.title)).toEqual(["Game version mismatch"]); + expect(begun).toEqual([]); + expect(h.driver.step).toBe("prepare"); + expect(h.driver.collection).toBeUndefined(); + }); + + test("Continue installs the collection", async ({ makeCollection }) => { + const h = makeCollection(mismatchedGameVersion()); + const { begun } = withExtensionHandler(h); + await openInstallDialog(h); + h.setNextDialog({ action: "Continue", input: {} }); + + await h.driver.continue(); + await settle(); + + expect(h.dialogCalls.map((call) => call.title)).toEqual(["Game version mismatch"]); + expect(begun).toEqual([[COLLECTION]]); + expect(h.driver.step).toBe("installing"); + expect(h.getState().session.collections.activeSession?.collectionId).toBe(COLLECTION); + }); + + test("an update while the prompt is open does not begin the install", async ({ + makeCollection, + }) => { + const h = makeCollection(mismatchedGameVersion()); + const { begun } = withExtensionHandler(h); + await openInstallDialog(h); + let answer: (result: IDialogResult) => void = () => undefined; + const showDialog = vi.fn(() => new Promise((resolve) => (answer = resolve))); + (h.api as { showDialog: unknown }).showDialog = showDialog; + + const installNow = h.driver.continue(); + await vi.waitFor(() => expect(showDialog).toHaveBeenCalled()); + h.emit("did-install-mod", GAME, "other-archive", "other-mod"); + await settle(); + + expect(begun).toEqual([]); + + answer({ action: "Continue", input: {} }); + await installNow; + await settle(); + + expect(begun).toEqual([[COLLECTION]]); + }); + + test("Continue after a pause while the prompt was open does not start the install", async ({ + makeCollection, + }) => { + const h = makeCollection(mismatchedGameVersion()); + const { begun } = withExtensionHandler(h); + await openInstallDialog(h); + let answer: (result: IDialogResult) => void = () => undefined; + const showDialog = vi.fn(() => new Promise((resolve) => (answer = resolve))); + (h.api as { showDialog: unknown }).showDialog = showDialog; + + const installNow = h.driver.continue(); + await vi.waitFor(() => expect(showDialog).toHaveBeenCalled()); + // logout and game switch pause the install through pauseCollection + h.driver.pause("logout"); + answer({ action: "Continue", input: {} }); + await installNow; + await settle(); + + expect(begun).toEqual([]); + expect(h.driver.step).toBe("prepare"); + expect(h.getState().session.collections.activeSession).toBeUndefined(); + }); +}); diff --git a/src/renderer/src/extensions/collections/util/InstallDriver.ts b/src/renderer/src/extensions/collections/util/InstallDriver.ts index b4e2e2a333..1ed22a936a 100644 --- a/src/renderer/src/extensions/collections/util/InstallDriver.ts +++ b/src/renderer/src/extensions/collections/util/InstallDriver.ts @@ -820,7 +820,6 @@ class InstallDriver { this.mInstalledMods = []; this.mInstallingMod = undefined; this.mInstallDone = false; - this.mStep = "start"; const collection = this.mCollection; const profile = this.mProfile; @@ -898,12 +897,23 @@ class InstallDriver { }, [{ label: "Cancel" }, { label: "Continue" }], ); + if (this.mCollection !== collection) { + // the install was paused or cancelled while the prompt was open, which already reset + // the driver; whatever the answer, this attempt is over + return false; + } if (choice.action === "Cancel") { - this.mInstallDone = true; + // end the attempt as the install dialog's "Later" does, so the driver is idle again + this.cancel(); return false; } } + // Only now, with nothing left to ask, is the install ready to begin: the collections + // extension continues any update that finds the driver on "start", so setting it before the + // prompt let an update while it was open, or after Cancel, begin the install. + this.mStep = "start"; + this.mApi.events.emit("will-install-collection", gameId, collection.id); this.mApi.events.emit("view-collection", collection.id); From ac1f7d0205282c64df61b9671c19203fe766c5aa Mon Sep 17 00:00:00 2001 From: doodlum <15017472+doodlum@users.noreply.github.com> Date: Thu, 24 Sep 2026 14:51:45 +0100 Subject: [PATCH 2/2] fix(collections): keep the step on "start" while the game-version prompt is open Setting "start" only after the prompt left whatever step the driver was on before visible while the revision info loaded and the prompt was open. After a finished install that is "review", so a collection resumed right after one showed "Collection installation complete" behind the prompt, got an installCompleted stamp on the next driver update, and could keep that dialog after Cancel. Set "start" where it always was, and instead make continue() wait at "start" until the attempt that set it has prepared the install and the prompt is answered. That is what stops the extension's update handler beginning the install early. Each attempt keeps its own mark, so answering a paused attempt's prompt cannot release a newer one. The mark is set around startInstall's callers, leaving startInstall itself as it was. Move the extension's driver update handler into InstallDriver.ts as makeDriverUpdateHandler, so the tests run the real handler, including its "review" branch. Co-Authored-By: Claude Opus 5.5 --- .../src/extensions/collections/index.ts | 19 +-- .../util/InstallDriver.gameVersion.test.ts | 141 ++++++++++++++++-- .../collections/util/InstallDriver.ts | 53 ++++++- 3 files changed, 180 insertions(+), 33 deletions(-) diff --git a/src/renderer/src/extensions/collections/index.ts b/src/renderer/src/extensions/collections/index.ts index 53777be40e..cc85858bba 100644 --- a/src/renderer/src/extensions/collections/index.ts +++ b/src/renderer/src/extensions/collections/index.ts @@ -73,7 +73,7 @@ import { cloneCollection } from "./util/cloneCollection"; import { createCollection } from "./util/createCollection"; import { genDefaultsAction } from "./util/defaults"; import { addExtension } from "./util/extension"; -import InstallDriver from "./util/InstallDriver"; +import InstallDriver, { makeDriverUpdateHandler } from "./util/InstallDriver"; import { readCollection } from "./util/readCollection"; import { getActiveInstallSession } from "./util/selectors"; import { makeCollectionId } from "./util/transformCollection"; @@ -1268,22 +1268,7 @@ function once(api: IExtensionApi, collectionsCB: () => ICallbackMap) { driver = new InstallDriver(api); - driver.onUpdate(() => { - // currently no UI associated with the start step - if (driver.step === "start") { - driver.continue(); - } - - if (driver.step === "review") { - // this is called a few times so we need to check if collection is undefined or not so we only write timestamp once - if (driver.collection === undefined) return; - - const gameId = driver.profile.gameId; - const modId = driver.collection.id; - - api.store.dispatch(actions.setModAttribute(gameId, modId, "installCompleted", Date.now())); - } - }); + driver.onUpdate(makeDriverUpdateHandler(api, driver)); // Pause collection installation if user becomes unauthenticated api.onStateChange(["persistent", "nexus", "userInfo"], (oldValue, newValue) => { diff --git a/src/renderer/src/extensions/collections/util/InstallDriver.gameVersion.test.ts b/src/renderer/src/extensions/collections/util/InstallDriver.gameVersion.test.ts index e6871cdbf4..853159ea42 100644 --- a/src/renderer/src/extensions/collections/util/InstallDriver.gameVersion.test.ts +++ b/src/renderer/src/extensions/collections/util/InstallDriver.gameVersion.test.ts @@ -2,10 +2,12 @@ * When a collection revision lists game versions and none matches the installed game, the driver * asks whether to go on ("Game version mismatch", Cancel / Continue). These tests drive the REAL * driver through the collection harness, with the collections extension's own onUpdate handler - * (index.ts), which continues the driver whenever it sits on the "start" step. + * (makeDriverUpdateHandler), which continues the driver whenever it sits on the "start" step and + * stamps installCompleted when it sits on "review". * - * The regression they pin: Cancel left the driver on "start" with the collection still set, so - * the next driver update, or a second "Install Now", began the install anyway. + * The regressions they pin: Cancel left the driver on "start" with the collection still set, so + * the next driver update, or a second "Install Now", began the install anyway; and a start right + * after a finished install must not leave that install's "review" showing for the new collection. */ import { describe, expect, vi } from "vitest"; @@ -14,10 +16,14 @@ import { test } from "../../../test-utils/collectionTest"; import type { ICollectionHarness } from "../../../test-utils/harnessTypes"; import type { IDialogResult } from "../../../types/IDialog"; import type { IProfile } from "../../profile_management/types/IProfile"; +import type { IRevisionEx } from "../types/IRevisionEx"; +import { makeDriverUpdateHandler } from "./InstallDriver"; const GAME = "skyrimse"; const COLLECTION = "col-1"; const ARCHIVE = `dl-${COLLECTION}`; +// a collection installed and reviewed before the one under test +const PREVIOUS = "col-0"; /** The collection download, carrying revision info that asks for a game version not installed. */ function mismatchedGameVersion() { @@ -29,9 +35,9 @@ function mismatchedGameVersion() { } /** - * Wire up what the collections extension adds around the driver: its onUpdate handler, which - * continues from "start" (index.ts, "currently no UI associated with the start step"), and a - * count of the install-dependencies events that actually begin an install. + * Wire up what the collections extension adds around the driver: its own onUpdate handler, which + * continues from "start" and stamps installCompleted at "review", and a count of the + * install-dependencies events that actually begin an install. */ function withExtensionHandler(h: ICollectionHarness) { const begun: string[][] = []; @@ -41,10 +47,8 @@ function withExtensionHandler(h: ICollectionHarness) { let updates = 0; h.driver.onUpdate(() => { updates += 1; - if (h.driver.step === "start") { - void h.driver.continue(); - } }); + h.driver.onUpdate(makeDriverUpdateHandler(h.api, h.driver)); return { begun, updates: () => updates }; } @@ -63,6 +67,29 @@ async function openInstallDialog(h: ICollectionHarness) { /** Let the driver's async continue() calls, started from onUpdate, run to completion. */ const settle = () => new Promise((resolve) => setTimeout(resolve, 0)); +/** + * Install another collection to its review and press Done there. That leaves the driver on + * "review" with no collection, as it stays until something else starts. + */ +async function completeAnotherInstall(h: ICollectionHarness) { + await h.installRevision(makeRevision(1, [{ tag: "a" }], { collectionId: PREVIOUS })); + await settle(); + await h.completeActiveInstall(); + expect(h.driver.step).toBe("review"); + expect(h.driver.collection).toBeUndefined(); +} + +/** When the collection's review last opened, as the extension's update handler stamps it. */ +const installCompleted = (h: ICollectionHarness) => + h.getState().persistent.mods[GAME][COLLECTION]?.attributes?.installCompleted; + +/** + * What the install-finished dialog shows for: a collection with the driver on "review" + * (InstallFinishedDialog's `show`). + */ +const reviewShown = (h: ICollectionHarness) => + h.driver.collection !== undefined && h.driver.step === "review"; + describe("InstallDriver game-version prompt", () => { test("Cancel from the install dialog ends the install and nothing later begins it", async ({ makeCollection, @@ -170,4 +197,100 @@ describe("InstallDriver game-version prompt", () => { expect(h.driver.step).toBe("prepare"); expect(h.getState().session.collections.activeSession).toBeUndefined(); }); + + test("answering a paused attempt's prompt does not let a newer attempt begin early", async ({ + makeCollection, + }) => { + const h = makeCollection(mismatchedGameVersion()); + const { begun } = withExtensionHandler(h); + const revision = await openInstallDialog(h); + const answers: Array<(result: IDialogResult) => void> = []; + const showDialog = vi.fn(() => new Promise((resolve) => answers.push(resolve))); + (h.api as { showDialog: unknown }).showDialog = showDialog; + + const installNow = h.driver.continue(); + await vi.waitFor(() => expect(showDialog).toHaveBeenCalledTimes(1)); + h.driver.pause("logout"); + // resumed (resume-collection) while the first prompt is still open, which asks again + const resume = h.installRevision(revision); + await vi.waitFor(() => expect(showDialog).toHaveBeenCalledTimes(2)); + + answers[0]({ action: "Continue", input: {} }); + await installNow; + h.emit("did-install-mod", GAME, "other-archive", "other-mod"); + await settle(); + + expect(begun).toEqual([]); + + answers[1]({ action: "Continue", input: {} }); + await resume; + await settle(); + + expect(begun).toEqual([[COLLECTION]]); + }); + + test("a resume right after a finished install does not review it while the prompt is open", async ({ + makeCollection, + }) => { + const h = makeCollection(mismatchedGameVersion()); + const { begun } = withExtensionHandler(h); + await completeAnotherInstall(h); + let answer: (result: IDialogResult) => void = () => undefined; + const showDialog = vi.fn(() => new Promise((resolve) => (answer = resolve))); + (h.api as { showDialog: unknown }).showDialog = showDialog; + + const resume = h.installRevision(makeRevision(1, [{ tag: "b" }], { collectionId: COLLECTION })); + await vi.waitFor(() => expect(showDialog).toHaveBeenCalled()); + h.emit("did-install-mod", GAME, "other-archive", "other-mod"); + await settle(); + + // the previous install's "review" must not carry over to this collection + expect(reviewShown(h)).toBe(false); + expect(installCompleted(h)).toBeUndefined(); + expect(begun).toEqual([[PREVIOUS]]); + expect(h.driver.step).toBe("start"); + + answer({ action: "Cancel", input: {} }); + await resume; + await settle(); + + expect(h.driver.step).toBe("prepare"); + expect(reviewShown(h)).toBe(false); + expect(installCompleted(h)).toBeUndefined(); + expect(begun).toEqual([[PREVIOUS]]); + }); + + test("a resume right after a finished install waits for its revision info", async ({ + makeCollection, + }) => { + const modInfo = makeCollectionModInfo({ collectionId: 1, revisionId: 2, gameId: GAME }); + const h = makeCollection({ + downloads: { [ARCHIVE]: makeDownload({ id: ARCHIVE, state: "finished", modInfo }) }, + }); + const { begun } = withExtensionHandler(h); + await completeAnotherInstall(h); + let deliver: (revision: IRevisionEx) => void = () => undefined; + const fetch = vi + .spyOn(h.driver.infoCache, "getRevisionInfo") + .mockImplementation(() => new Promise((resolve) => (deliver = resolve))); + + const resume = h.installRevision(makeRevision(1, [{ tag: "b" }], { collectionId: COLLECTION })); + await vi.waitFor(() => expect(fetch).toHaveBeenCalled()); + h.emit("did-install-mod", GAME, "other-archive", "other-mod"); + await settle(); + + expect(reviewShown(h)).toBe(false); + expect(installCompleted(h)).toBeUndefined(); + expect(begun).toEqual([[PREVIOUS]]); + expect(h.driver.step).toBe("start"); + + // a revision the installed game matches: no prompt, so the install begins + deliver({ modFiles: [], gameVersions: [] } as unknown as IRevisionEx); + await resume; + await settle(); + + expect(h.dialogCalls).toEqual([]); + expect(begun).toEqual([[PREVIOUS], [COLLECTION]]); + expect(h.driver.step).toBe("installing"); + }); }); diff --git a/src/renderer/src/extensions/collections/util/InstallDriver.ts b/src/renderer/src/extensions/collections/util/InstallDriver.ts index 1ed22a936a..d0c8475807 100644 --- a/src/renderer/src/extensions/collections/util/InstallDriver.ts +++ b/src/renderer/src/extensions/collections/util/InstallDriver.ts @@ -100,6 +100,9 @@ class InstallDriver { private mPrepare: Bluebird = Bluebird.resolve(); private mTimeStarted: number; private mPostprocessing: boolean = false; + // set while an attempt to start is still preparing the install (revision info, the game-version + // prompt); the step is already "start" then, but nothing may begin the install yet + private mStarting: object | undefined; // Throttle the progress notification to avoid flooding Redux/UI on every single mod // event. (Session status writes are dispatched directly now - InstallManager is the @@ -407,7 +410,7 @@ class InstallDriver { this.mLastCollection = this.mCollection = collection; this.mGameId = profile?.gameId ?? activeGameId(this.mApi.getState()); - await this.startInstall(); + await this.startAttempt(); await this.initCollectionInfo(); this.triggerUpdate(); @@ -579,7 +582,7 @@ class InstallDriver { await this.initCollectionInfo(); const steps = { - query: this.startInstall, + query: this.startAttempt, start: this.begin, disclaimer: this.closeDisclaimers, installing: this.finishInstalling, @@ -600,6 +603,10 @@ class InstallDriver { return this.mInstallDone; } else if (this.mStep === "disclaimer") { return this.mInstalledMods.length > 0 || this.mInstallDone; + } else if (this.mStep === "start") { + // the collections extension continues every update that finds the driver on "start", so + // "start" waits until the install is prepared and the game-version prompt answered + return this.mStarting === undefined; } else { return true; } @@ -613,6 +620,20 @@ class InstallDriver { return ["disclaimer", "installing"].indexOf(this.mStep) !== -1; } + /** startInstall, marked as preparing until it returns, which "start" waits for (canContinue) */ + private startAttempt = async () => { + const attempt = {}; + this.mStarting = attempt; + try { + return await this.startInstall(); + } finally { + // a later attempt, begun while this one's prompt was open, keeps its own mark + if (this.mStarting === attempt) { + this.mStarting = undefined; + } + } + }; + public get currentSessionId(): string | undefined { return this.mCurrentSessionId; } @@ -820,6 +841,7 @@ class InstallDriver { this.mInstalledMods = []; this.mInstallingMod = undefined; this.mInstallDone = false; + this.mStep = "start"; const collection = this.mCollection; const profile = this.mProfile; @@ -909,11 +931,6 @@ class InstallDriver { } } - // Only now, with nothing left to ask, is the install ready to begin: the collections - // extension continues any update that finds the driver on "start", so setting it before the - // prompt let an update while it was open, or after Cancel, begin the install. - this.mStep = "start"; - this.mApi.events.emit("will-install-collection", gameId, collection.id); this.mApi.events.emit("view-collection", collection.id); @@ -1164,4 +1181,26 @@ class InstallDriver { } } +/** + * The collections extension's handler for every driver update: nothing on screen belongs to the + * "start" step, so it continues from there, and it records when the review of a collection opens. + */ +export function makeDriverUpdateHandler(api: IExtensionApi, driver: InstallDriver): UpdateCB { + return () => { + if (driver.step === "start") { + driver.continue(); + } + + if (driver.step === "review") { + // this is called a few times so we need to check if collection is undefined or not so we only write timestamp once + if (driver.collection === undefined) return; + + const gameId = driver.profile.gameId; + const modId = driver.collection.id; + + api.store.dispatch(setModAttribute(gameId, modId, "installCompleted", Date.now())); + } + }; +} + export default InstallDriver;