From fd378f3d03aab4cf7e455ec77c02defff70b0ab6 Mon Sep 17 00:00:00 2001 From: doodlum <15017472+doodlum@users.noreply.github.com> Date: Mon, 5 Oct 2026 12:33:45 +0100 Subject: [PATCH 1/3] fix(collections): re-download a member whose same-named archive has the wrong hash A collection member that pins one exact file (a non-fuzzy version with a file hash) was satisfied by any archive in the download folder with the expected file name. The download adapter reuses such a file by name alone, and doDownload then tagged it as the member, so a truncated or different file was installed, failed and was retried against the same file until the user deleted it. Verify an archive before reusing it for such a member: the recorded hash, or when none is recorded the size and then the file's hash (recorded for next time). On a mismatch remove that download and fetch the file again. The tag fallback in findDownloadByReferenceTag no longer resolves the member to an archive whose recorded hash contradicts the rule. Fuzzy members are exempt, and a matching same-named archive is still reused without a download. Fixes LAZ-1286 Co-Authored-By: Claude Opus 5.5 --- .../InstallManager.reusedArchive.test.ts | 350 ++++++++++++++++++ .../mod_management/InstallManager.ts | 84 ++++- .../util/archiveMatchesReference.ts | 79 ++++ 3 files changed, 509 insertions(+), 4 deletions(-) create mode 100644 src/renderer/src/extensions/mod_management/InstallManager.reusedArchive.test.ts create mode 100644 src/renderer/src/extensions/mod_management/util/archiveMatchesReference.ts diff --git a/src/renderer/src/extensions/mod_management/InstallManager.reusedArchive.test.ts b/src/renderer/src/extensions/mod_management/InstallManager.reusedArchive.test.ts new file mode 100644 index 0000000000..7a20336201 --- /dev/null +++ b/src/renderer/src/extensions/mod_management/InstallManager.reusedArchive.test.ts @@ -0,0 +1,350 @@ +/** + * A collection member whose archive name is already taken in the download folder (LAZ-1286). + * + * The download adapter reuses an on-disk file by name alone. For a member that pins one exact file + * (a non-fuzzy version with a file hash) a same-named file with another hash, such as a truncated + * copy, must not be reused or tagged as the member, or every retry installs the same bad file. A + * same-named copy that is the right file must still be reused without a download (LAZ-972). + */ +import { AlreadyDownloaded } from "@vortex/shared/errors"; +import { beforeEach, describe, expect, vi } from "vitest"; + +import { + makeDownload, + makeExactRef, + makeFuzzyRef, + makeInstallState, + makeMod, + makeModInstallInfo, + makeProfile, + makeRule, + makeSession, + managerInternals as internals, +} from "../../test-utils/builders"; +import type { IInstallManagerHarness } from "../../test-utils/harnessTypes"; +import { test as imTest } from "../../test-utils/installManagerTest"; +import { generateCollectionSessionId, modRuleId } from "../../util/collectionInstallSession"; +import { MOD_TYPE } from "../collections/constants"; +import type { IDownload } from "../download_management/types/IDownload"; +import type { IModReference } from "./types/IMod"; +import { downloadReferenceTags } from "./util/testModReference"; + +vi.mock("../../logging", () => ({ log: vi.fn() })); + +const hashFile = vi.hoisted(() => vi.fn<(filePath: string) => Promise>()); +vi.mock("../../util/checksum", async (importOriginal) => ({ + ...(await importOriginal()), + fileMD5: hashFile, +})); + +beforeEach(() => { + hashFile.mockReset(); +}); + +const GAME = "skyrimse"; +const PROFILE = "prof-1"; +const COLLECTION = "col-1"; +const ARCHIVE = "Member-100-1-0.7z"; +const GOOD_MD5 = "good-md5"; +const GOOD_SIZE = 1000; +const TAG = "member-tag"; + +const exactRef = makeExactRef({ + tag: TAG, + gameId: GAME, + fileMD5: GOOD_MD5, + fileSize: GOOD_SIZE, + versionMatch: "1.0.0", +}); + +interface IFakeDisk { + files: Set; + starts: Array<{ fileName: string; redownload: string }>; + removed: string[]; +} + +/** + * Stands in for IPCDownloadAdapter's start-download and remove-download: a name already on disk is + * reused with AlreadyDownloaded and the record that tracks it (the adapter checks neither size nor + * hash), anything else downloads the good file under that name; remove-download deletes the file + * and its record. + */ +function fakeAdapter(h: IInstallManagerHarness, onDisk: string[]): IFakeDisk { + const disk: IFakeDisk = { files: new Set(onDisk), starts: [], removed: [] }; + h.api.events.on( + "start-download", + ( + _urls: unknown, + _modInfo: unknown, + fileName: string, + callback: (err: Error | null, id?: string) => void, + redownload: string, + ) => { + disk.starts.push({ fileName, redownload }); + const files = h.getState().persistent.downloads.files; + const existing = Object.keys(files).find((id) => files[id].localPath === fileName); + if (redownload !== "replace" && disk.files.has(fileName) && existing !== undefined) { + callback(new AlreadyDownloaded(fileName, existing)); + return; + } + const id = `dl-fresh-${disk.starts.length}`; + h.setState((draft) => { + draft.persistent.downloads.files[id] = makeDownload({ + id, + state: "finished", + game: [GAME], + localPath: fileName, + size: GOOD_SIZE, + fileMD5: GOOD_MD5, + }); + }); + disk.files.add(fileName); + callback(null, id); + }, + ); + h.api.events.on("remove-download", (id: string, callback?: (err: Error | null) => void) => { + disk.removed.push(id); + disk.files.delete(h.getState().persistent.downloads.files[id]?.localPath); + h.setState((draft) => { + delete draft.persistent.downloads.files[id]; + }); + callback?.(null); + }); + return disk; +} + +function makeInstall( + makeInstallManager: (overrides?: object) => IInstallManagerHarness, + reference: IModReference, + existing: IDownload, +) { + const rule = makeRule({ type: "requires", reference }); + const h = makeInstallManager({ + profiles: { [PROFILE]: makeProfile({ id: PROFILE, gameId: GAME }) }, + mods: { + [GAME]: { + [COLLECTION]: makeMod({ + id: COLLECTION, + type: MOD_TYPE, + rules: [rule], + attributes: { collectionId: 1 }, + }), + }, + }, + downloads: { [existing.id]: existing }, + session: makeInstallState({ + activeSession: makeSession({ + sessionId: generateCollectionSessionId(COLLECTION, PROFILE), + collectionId: COLLECTION, + profileId: PROFILE, + gameId: GAME, + mods: { + [modRuleId(rule)]: makeModInstallInfo({ + rule, + type: "requires", + status: "pending", + phase: 0, + }), + }, + totalRequired: 1, + }), + }), + }); + h.setState((draft) => { + draft.settings.profiles.activeProfileId = PROFILE; + }); + const phaseState = h.phaseTracker.ensure(COLLECTION); + phaseState.allowedPhase = 0; + const queued = vi + .spyOn(h.manager as unknown as { queueInstallation: () => void }, "queueInstallation") + .mockImplementation(() => undefined); + return { h, rule, queued }; +} + +/** Run one dependency round for the member until it queued an install, then unwind it. */ +async function installMember( + h: IInstallManagerHarness, + rule: ReturnType, + queued: { mock: { calls: unknown[][] } }, + download?: string, +): Promise { + const installing = internals(h.manager).doInstallDependencies( + h.api, + GAME, + COLLECTION, + [ + { + reference: rule.reference, + sessionRuleId: modRuleId(rule), + download, + phase: 0, + lookupResults: [ + { + key: "member", + value: { + sourceURI: "https://files.example/Member.7z", + logicalFileName: "Member", + domainName: GAME, + source: "nexus", + }, + }, + ], + extra: { fileName: ARCHIVE }, + }, + ], + false, + true, + ); + try { + await vi.waitFor(() => expect(queued.mock.calls.length).toBeGreaterThan(0)); + return queued.mock.calls[0][2] as string; + } finally { + internals(h.manager).mDependencyInstalls[COLLECTION]?.(); + delete internals(h.manager).mDependencyInstalls[COLLECTION]; + await installing.catch(() => undefined); + } +} + +const onDisk = (overrides: Partial): IDownload => + makeDownload({ + id: "dl-existing", + state: "finished", + game: [GAME], + localPath: ARCHIVE, + size: GOOD_SIZE, + ...overrides, + }); + +const tagsOf = (h: IInstallManagerHarness, id: string) => + downloadReferenceTags(h.getState().persistent.downloads.files[id]); + +describe("a same-named archive already in the download folder", () => { + imTest( + "is re-downloaded when its recorded hash isn't the member's", + async ({ makeInstallManager }) => { + const { h, rule, queued } = makeInstall( + makeInstallManager, + exactRef, + onDisk({ fileMD5: "truncated-md5", size: GOOD_SIZE / 2 }), + ); + const disk = fakeAdapter(h, [ARCHIVE]); + + const installed = await installMember(h, rule, queued); + + expect(installed).not.toBe("dl-existing"); + expect(h.getState().persistent.downloads.files[installed].fileMD5).toBe(GOOD_MD5); + expect(tagsOf(h, installed)).toEqual([TAG]); + // the bad copy is gone, so no later lookup can resolve the member to it + expect(disk.removed).toEqual(["dl-existing"]); + expect(h.getState().persistent.downloads.files["dl-existing"]).toBeUndefined(); + expect(hashFile).not.toHaveBeenCalled(); + }, + ); + + imTest( + "is re-downloaded without hashing when its size isn't the member's", + async ({ makeInstallManager }) => { + // adopted by name from the folder, so no hash is recorded yet + const { h, rule, queued } = makeInstall( + makeInstallManager, + exactRef, + onDisk({ size: GOOD_SIZE / 2 }), + ); + const disk = fakeAdapter(h, [ARCHIVE]); + + const installed = await installMember(h, rule, queued); + + expect(installed).not.toBe("dl-existing"); + expect(disk.removed).toEqual(["dl-existing"]); + expect(hashFile).not.toHaveBeenCalled(); + }, + ); + + imTest( + "is hashed when nothing is recorded, and re-downloaded on a mismatch", + async ({ makeInstallManager }) => { + hashFile.mockResolvedValue("other-md5"); + const { h, rule, queued } = makeInstall(makeInstallManager, exactRef, onDisk({})); + const disk = fakeAdapter(h, [ARCHIVE]); + + const installed = await installMember(h, rule, queued); + + expect(hashFile).toHaveBeenCalledTimes(1); + expect(installed).not.toBe("dl-existing"); + expect(disk.removed).toEqual(["dl-existing"]); + }, + ); + + // LAZ-972: an archive another collection (or an earlier install) fetched is reused as it is + imTest( + "is reused and tagged, without a download, when it is the member's file", + async ({ makeInstallManager }) => { + const { h, rule, queued } = makeInstall( + makeInstallManager, + exactRef, + onDisk({ fileMD5: GOOD_MD5, modInfo: { referenceTags: ["other"], referenceTag: "other" } }), + ); + const disk = fakeAdapter(h, [ARCHIVE]); + + const installed = await installMember(h, rule, queued); + + expect(installed).toBe("dl-existing"); + expect(tagsOf(h, "dl-existing")).toEqual(["other", TAG]); + expect(disk.removed).toEqual([]); + expect(Object.keys(h.getState().persistent.downloads.files)).toEqual(["dl-existing"]); + }, + ); + + imTest( + "is reused after hashing it when nothing is recorded and it matches", + async ({ makeInstallManager }) => { + hashFile.mockResolvedValue(GOOD_MD5); + const { h, rule, queued } = makeInstall(makeInstallManager, exactRef, onDisk({})); + const disk = fakeAdapter(h, [ARCHIVE]); + + const installed = await installMember(h, rule, queued); + + expect(installed).toBe("dl-existing"); + expect(disk.removed).toEqual([]); + // the hash is recorded, so the next lookup doesn't hash again + expect(h.getState().persistent.downloads.files["dl-existing"].fileMD5).toBe(GOOD_MD5); + }, + ); + + // a fuzzy version resolves to files with other hashes by design, so the hash isn't checked + imTest("is reused for a fuzzy member whatever its hash", async ({ makeInstallManager }) => { + const fuzzyRef = makeFuzzyRef({ tag: TAG, gameId: GAME, fileMD5: GOOD_MD5 }); + const { h, rule, queued } = makeInstall( + makeInstallManager, + fuzzyRef, + onDisk({ fileMD5: "other-md5", modInfo: { nexus: { ids: { modId: 100, fileId: 5 } } } }), + ); + const disk = fakeAdapter(h, [ARCHIVE]); + + const installed = await installMember(h, rule, queued, "dl-existing"); + + expect(installed).toBe("dl-existing"); + expect(disk.removed).toEqual([]); + }); +}); + +describe("a member already resolved to an archive with another hash", () => { + // an earlier version tagged the bad copy as the member, so the gather resolves the rule to it + imTest( + "downloads the member's file instead of installing the tagged copy", + async ({ makeInstallManager }) => { + const { h, rule, queued } = makeInstall( + makeInstallManager, + exactRef, + onDisk({ fileMD5: "truncated-md5", modInfo: { referenceTag: TAG, referenceTags: [TAG] } }), + ); + const disk = fakeAdapter(h, [ARCHIVE]); + + const installed = await installMember(h, rule, queued, "dl-existing"); + + expect(installed).not.toBe("dl-existing"); + expect(h.getState().persistent.downloads.files[installed].fileMD5).toBe(GOOD_MD5); + expect(disk.removed).toEqual(["dl-existing"]); + }, + ); +}); diff --git a/src/renderer/src/extensions/mod_management/InstallManager.ts b/src/renderer/src/extensions/mod_management/InstallManager.ts index 90074e251b..cc45b7309b 100644 --- a/src/renderer/src/extensions/mod_management/InstallManager.ts +++ b/src/renderer/src/extensions/mod_management/InstallManager.ts @@ -186,6 +186,7 @@ import type { IModInstaller, ISupportedInstaller } from "./types/IModInstaller"; import type { IInstallationDetails, InstallFunc } from "./types/InstallFunc"; import type { IReplaceChoice, ReplaceChoice } from "./types/IReplaceChoice"; import type { ISupportedResult, ITestSupportedDetails, TestSupported } from "./types/TestSupported"; +import { archiveMatchesReference, contradictsReference } from "./util/archiveMatchesReference"; import { getCSharpScriptAllowListForGame } from "./util/cSharpScriptAllowList"; import gatherDependencies, { findDownloadByRef, @@ -288,8 +289,10 @@ function findDownloadByReferenceTag( downloads: Record, reference: IModReference, ): string | null { + // an archive whose hash isn't the one a non-fuzzy reference pins is never the member, even when + // it carries the member's tag const dlId = findDownloadByRef(reference, downloads); - if (dlId) { + if (dlId && !contradictsReference(downloads[dlId], reference)) { return dlId; } @@ -300,7 +303,8 @@ function findDownloadByReferenceTag( return ( Object.keys(downloads).find( (id) => - downloadHasReferenceTag(downloads[id], reference.tag) || + (downloadHasReferenceTag(downloads[id], reference.tag) && + !contradictsReference(downloads[id], reference)) || (reference.md5Hint && downloads[id].fileMD5 === reference.md5Hint), ) || null ); @@ -308,7 +312,7 @@ function findDownloadByReferenceTag( function getReadyDownloadId( downloads: Record, - reference: { tag?: string; md5Hint?: string }, + reference: IModReference, hasActiveOrPendingCheck: (downloadId: string) => boolean, ): string | null { const downloadId = findDownloadByReferenceTag(downloads, reference); @@ -5419,6 +5423,7 @@ class InstallManager { campaign?: string, fileName?: string, parentCollection?: IParentCollection, + expected?: IModReference, ): Promise { const call = (input: string | (() => PromiseLike)): Promise => input !== undefined && typeof input === "function" @@ -5473,7 +5478,21 @@ class InstallManager { if (error == null) { return resolve(id); } else if (error instanceof AlreadyDownloaded) { - return resolve(error.downloadId); + // the adapter reuses a file by its name alone; without `expected` the + // download below doesn't check again, so a replaced file can't loop + return resolve( + this.reuseExistingArchive(api, error.downloadId, expected, () => + this.downloadURL( + api, + lookupResult, + wasCanceled, + referenceTag, + campaign, + fileName, + parentCollection, + ), + ), + ); } else if (parseError(error).data.kind === "download:is-html") { // If this is a google drive link and the file exceeds the // virus testing limit, Google will return an HTML page asking @@ -5519,6 +5538,43 @@ class InstallManager { ); } + /** + * Resolves to the download the adapter reused by file name, unless it is a finished archive that + * isn't the file `expected` pins. That one is removed and the file downloaded again. + */ + private async reuseExistingArchive( + api: IExtensionApi, + downloadId: string, + expected: IModReference | undefined, + redownload: () => Promise, + ): Promise { + const download = api.getState().persistent.downloads.files[downloadId]; + if ( + expected === undefined || + download?.state !== "finished" || + (await archiveMatchesReference(api, download, expected)) + ) { + return downloadId; + } + log("warn", "archive on disk is not the file the rule pins, downloading it again", { + downloadId, + fileName: download.localPath, + size: download.size, + fileMD5: api.getState().persistent.downloads.files[downloadId]?.fileMD5, + expectedSize: expected.fileSize, + expectedMD5: expected.fileMD5, + }); + await new Promise((resolve, reject) => { + api.events.emit( + "remove-download", + downloadId, + (err: Error | null) => (err ? reject(err) : resolve()), + { confirmed: true, silent: true }, + ); + }); + return redownload(); + } + private downloadMatching( api: IExtensionApi, lookupResult: IModInfoEx, @@ -5659,6 +5715,7 @@ class InstallManager { campaign, fileName, parentCollection, + requirement, ) : res, ); @@ -5671,6 +5728,7 @@ class InstallManager { campaign, fileName, parentCollection, + requirement, ).catch((err) => { if (err instanceof UserCanceled || err instanceof ProcessCanceled) { return Promise.reject(err); @@ -6502,6 +6560,24 @@ class InstallManager { } else { dlPromise = Promise.resolve(dep.download); } + } else if ( + downloads[dep.download].state === "finished" && + dep.extra?.localPath === undefined + ) { + // resolved by tag or by mod/file id, neither of which proves it's the pinned file + const existing = downloads[dep.download]; + const hasSource = (dep.lookupResults[0]?.value?.sourceURI ?? "") !== ""; + dlPromise = archiveMatchesReference(api, existing, dep.reference).then((matches) => { + if (matches || !hasSource) { + return dep.download; + } + log("warn", "resolved archive is not the file the rule pins, downloading it", { + downloadId: dep.download, + fileName: existing.localPath, + expectedMD5: dep.reference.fileMD5, + }); + return queueDownload(dep); + }); } return dlPromise .catch((err: unknown) => { diff --git a/src/renderer/src/extensions/mod_management/util/archiveMatchesReference.ts b/src/renderer/src/extensions/mod_management/util/archiveMatchesReference.ts new file mode 100644 index 0000000000..5aa9c4f8fd --- /dev/null +++ b/src/renderer/src/extensions/mod_management/util/archiveMatchesReference.ts @@ -0,0 +1,79 @@ +/** + * Whether an archive already in the download folder is the file a dependency rule asks for. + * + * A non-fuzzy reference with a file hash names one exact file, so an archive that merely shares its + * file name (a truncated copy, or a different file from a browser download) must not stand in for + * it. Fuzzy references (`+prefer`, ranges, `*`) legitimately resolve to files with other hashes, so + * they are never checked. + */ +import * as path from "path"; + +import { log } from "../../../logging"; +import type { IExtensionApi } from "../../../types/IExtensionContext"; +import { fileMD5 } from "../../../util/checksum"; +import { setDownloadHash } from "../../download_management/actions/state"; +import { downloadPathForGame } from "../../download_management/selectors"; +import type { IDownload } from "../../download_management/types/IDownload"; +import { knownGames } from "../../gamemode_management/selectors"; +import { convertGameIdReverse } from "../../nexus_integration/util/convertGameId"; +import type { IModReference } from "../types/IMod"; +import { isFuzzyVersion } from "./isFuzzyVersion"; + +function pinsFileHash(reference: IModReference | undefined): boolean { + return !!reference?.fileMD5 && !isFuzzyVersion(reference.versionMatch); +} + +/** + * True when the download's recorded hash is known and isn't the one the reference pins. Cheap: it + * never touches the disk, so a download whose hash isn't known yet doesn't contradict. + */ +export function contradictsReference( + download: IDownload | undefined, + reference: IModReference | undefined, +): boolean { + return ( + pinsFileHash(reference) && + download?.fileMD5 !== undefined && + download.fileMD5 !== reference.fileMD5 + ); +} + +async function hashDownload(api: IExtensionApi, download: IDownload): Promise { + if (!download.localPath) { + return undefined; + } + const state = api.getState(); + const gameId = convertGameIdReverse(knownGames(state), download.game[0]) || download.game[0]; + const filePath = path.join(downloadPathForGame(state, gameId), download.localPath); + try { + const hash = await fileMD5(filePath); + api.store.dispatch(setDownloadHash(download.id, hash)); + return hash; + } catch (err) { + log("warn", "failed to hash existing archive", { downloadId: download.id, err }); + return undefined; + } +} + +/** + * Whether the finished download can be reused for the reference. Uses the recorded hash when there + * is one; otherwise rejects on a known size mismatch before hashing the file (and recording the + * hash). An archive that can't be hashed keeps the old behaviour and is reused. + */ +export async function archiveMatchesReference( + api: IExtensionApi, + download: IDownload, + reference: IModReference, +): Promise { + if (!pinsFileHash(reference)) { + return true; + } + let hash = download.fileMD5; + if (hash === undefined) { + if (reference.fileSize > 0 && download.size > 0 && download.size !== reference.fileSize) { + return false; + } + hash = await hashDownload(api, download); + } + return hash === undefined || hash === reference.fileMD5; +} From e520a62c05a3c7ed56c393aceb901495d9a5d780 Mon Sep 17 00:00:00 2001 From: doodlum <15017472+doodlum@users.noreply.github.com> Date: Mon, 5 Oct 2026 13:46:07 +0100 Subject: [PATCH 2/3] fix(collections): check an archive's size on disk and failed archives too A recorded hash describes the file as it was when it was hashed, so an archive cut short afterwards still matched. Compare the member's file size with the size on disk first. An archive whose install failed is marked as a failed download, which the reuse check skipped, so a resumed collection stalled on it; verify those as well. Co-Authored-By: Claude Opus 5.5 --- .../InstallManager.reusedArchive.test.ts | 55 +++++++++++++++++++ .../mod_management/InstallManager.ts | 9 ++- .../util/archiveMatchesReference.ts | 39 +++++++------ 3 files changed, 84 insertions(+), 19 deletions(-) diff --git a/src/renderer/src/extensions/mod_management/InstallManager.reusedArchive.test.ts b/src/renderer/src/extensions/mod_management/InstallManager.reusedArchive.test.ts index 7a20336201..68a321b9c5 100644 --- a/src/renderer/src/extensions/mod_management/InstallManager.reusedArchive.test.ts +++ b/src/renderer/src/extensions/mod_management/InstallManager.reusedArchive.test.ts @@ -6,6 +6,9 @@ * copy, must not be reused or tagged as the member, or every retry installs the same bad file. A * same-named copy that is the right file must still be reused without a download (LAZ-972). */ +import { mkdir, writeFile } from "node:fs/promises"; +import * as path from "node:path"; + import { AlreadyDownloaded } from "@vortex/shared/errors"; import { beforeEach, describe, expect, vi } from "vitest"; @@ -23,8 +26,10 @@ import { } from "../../test-utils/builders"; import type { IInstallManagerHarness } from "../../test-utils/harnessTypes"; import { test as imTest } from "../../test-utils/installManagerTest"; +import { makeTempDir } from "../../test-utils/tempDir"; import { generateCollectionSessionId, modRuleId } from "../../util/collectionInstallSession"; import { MOD_TYPE } from "../collections/constants"; +import { downloadPathForGame } from "../download_management/selectors"; import type { IDownload } from "../download_management/types/IDownload"; import type { IModReference } from "./types/IMod"; import { downloadReferenceTags } from "./util/testModReference"; @@ -348,3 +353,53 @@ describe("a member already resolved to an archive with another hash", () => { }, ); }); + +describe("an archive whose record no longer describes the file", () => { + // the hash was recorded when the download finished; the file was cut short afterwards + imTest( + "is re-downloaded when its size on disk isn't the member's", + async ({ makeInstallManager }) => { + const { h, rule, queued } = makeInstall( + makeInstallManager, + exactRef, + onDisk({ fileMD5: GOOD_MD5, modInfo: { referenceTag: TAG, referenceTags: [TAG] } }), + ); + // the real file, cut to half the size its record still says + const root = await makeTempDir("laz1286-"); + h.setState((draft) => { + draft.settings.downloads.path = root; + }); + const dlPath = downloadPathForGame(h.getState(), GAME); + await mkdir(dlPath, { recursive: true }); + await writeFile(path.join(dlPath, ARCHIVE), Buffer.alloc(GOOD_SIZE / 2)); + const disk = fakeAdapter(h, [ARCHIVE]); + + const installed = await installMember(h, rule, queued, "dl-existing"); + + expect(installed).not.toBe("dl-existing"); + expect(disk.removed).toEqual(["dl-existing"]); + expect(hashFile).not.toHaveBeenCalled(); + }, + ); + + // an earlier attempt installed the bad copy, which marked the download failed; the adapter still + // hands it back by name + imTest("is re-downloaded after it failed to install", async ({ makeInstallManager }) => { + const { h, rule, queued } = makeInstall( + makeInstallManager, + exactRef, + onDisk({ + state: "failed", + fileMD5: "truncated-md5", + size: GOOD_SIZE / 2, + modInfo: { referenceTag: TAG, referenceTags: [TAG] }, + }), + ); + const disk = fakeAdapter(h, [ARCHIVE]); + + const installed = await installMember(h, rule, queued); + + expect(installed).not.toBe("dl-existing"); + expect(disk.removed).toEqual(["dl-existing"]); + }); +}); diff --git a/src/renderer/src/extensions/mod_management/InstallManager.ts b/src/renderer/src/extensions/mod_management/InstallManager.ts index cc45b7309b..4ed305b174 100644 --- a/src/renderer/src/extensions/mod_management/InstallManager.ts +++ b/src/renderer/src/extensions/mod_management/InstallManager.ts @@ -5539,8 +5539,9 @@ class InstallManager { } /** - * Resolves to the download the adapter reused by file name, unless it is a finished archive that - * isn't the file `expected` pins. That one is removed and the file downloaded again. + * Resolves to the download the adapter reused by file name, unless it is a settled archive (finished, + * or failed to install) that isn't the file `expected` pins. That one is removed and the file + * downloaded again. */ private async reuseExistingArchive( api: IExtensionApi, @@ -5549,9 +5550,11 @@ class InstallManager { redownload: () => Promise, ): Promise { const download = api.getState().persistent.downloads.files[downloadId]; + // a download still in progress or paused is left to the resume handling + const settled = download?.state === "finished" || download?.state === "failed"; if ( expected === undefined || - download?.state !== "finished" || + !settled || (await archiveMatchesReference(api, download, expected)) ) { return downloadId; diff --git a/src/renderer/src/extensions/mod_management/util/archiveMatchesReference.ts b/src/renderer/src/extensions/mod_management/util/archiveMatchesReference.ts index 5aa9c4f8fd..fa11ec44d4 100644 --- a/src/renderer/src/extensions/mod_management/util/archiveMatchesReference.ts +++ b/src/renderer/src/extensions/mod_management/util/archiveMatchesReference.ts @@ -6,6 +6,7 @@ * it. Fuzzy references (`+prefer`, ranges, `*`) legitimately resolve to files with other hashes, so * they are never checked. */ +import { stat } from "node:fs/promises"; import * as path from "path"; import { log } from "../../../logging"; @@ -38,15 +39,15 @@ export function contradictsReference( ); } -async function hashDownload(api: IExtensionApi, download: IDownload): Promise { - if (!download.localPath) { - return undefined; - } +function archivePath(api: IExtensionApi, download: IDownload): string { const state = api.getState(); const gameId = convertGameIdReverse(knownGames(state), download.game[0]) || download.game[0]; - const filePath = path.join(downloadPathForGame(state, gameId), download.localPath); + return path.join(downloadPathForGame(state, gameId), download.localPath); +} + +async function hashDownload(api: IExtensionApi, download: IDownload): Promise { try { - const hash = await fileMD5(filePath); + const hash = await fileMD5(archivePath(api, download)); api.store.dispatch(setDownloadHash(download.id, hash)); return hash; } catch (err) { @@ -55,25 +56,31 @@ async function hashDownload(api: IExtensionApi, download: IDownload): Promise { + return stat(archivePath(api, download)).then( + (stats) => stats.size, + () => download.size, + ); +} + /** - * Whether the finished download can be reused for the reference. Uses the recorded hash when there - * is one; otherwise rejects on a known size mismatch before hashing the file (and recording the - * hash). An archive that can't be hashed keeps the old behaviour and is reused. + * Whether the download can be reused for the reference. A known size that isn't the reference's + * rejects it first; then the recorded hash decides, or, when none is recorded, the file is hashed + * (and the hash recorded). An archive that can't be hashed keeps the old behaviour and is reused. */ export async function archiveMatchesReference( api: IExtensionApi, download: IDownload, reference: IModReference, ): Promise { - if (!pinsFileHash(reference)) { + if (!pinsFileHash(reference) || !download.localPath) { return true; } - let hash = download.fileMD5; - if (hash === undefined) { - if (reference.fileSize > 0 && download.size > 0 && download.size !== reference.fileSize) { - return false; - } - hash = await hashDownload(api, download); + const size = (reference.fileSize ?? 0) > 0 ? await sizeOnDisk(api, download) : undefined; + if (size !== undefined && size > 0 && size !== reference.fileSize) { + return false; } + const hash = download.fileMD5 ?? (await hashDownload(api, download)); return hash === undefined || hash === reference.fileMD5; } From 861de8777e3f2f8a545cfec3f4737951461c8a24 Mon Sep 17 00:00:00 2001 From: doodlum <15017472+doodlum@users.noreply.github.com> Date: Mon, 5 Oct 2026 22:40:56 +0100 Subject: [PATCH 3/3] test(collections): give the reused-archive rounds time on a loaded machine The first dependency round in the file loads the manager's lazy modules and outlasted waitFor's one-second default under pnpm run verify. Co-Authored-By: Claude Opus 5.5 --- .../mod_management/InstallManager.reusedArchive.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/renderer/src/extensions/mod_management/InstallManager.reusedArchive.test.ts b/src/renderer/src/extensions/mod_management/InstallManager.reusedArchive.test.ts index 68a321b9c5..7bd6efcda2 100644 --- a/src/renderer/src/extensions/mod_management/InstallManager.reusedArchive.test.ts +++ b/src/renderer/src/extensions/mod_management/InstallManager.reusedArchive.test.ts @@ -201,7 +201,11 @@ async function installMember( true, ); try { - await vi.waitFor(() => expect(queued.mock.calls.length).toBeGreaterThan(0)); + // the first round in a file loads the manager's lazy modules, which can outlast waitFor's default + // second on a loaded machine + await vi.waitFor(() => expect(queued.mock.calls.length).toBeGreaterThan(0), { + timeout: 15_000, + }); return queued.mock.calls[0][2] as string; } finally { internals(h.manager).mDependencyInstalls[COLLECTION]?.();