fix(collections): ignore only the member a skip names

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

@@ -16,6 +16,7 @@ import { describe, expect, vi } from "vitest";
 
 import {
   makeCollectionModInfo,
+  makeDeterministicRef,
   makeDownload,
   makeFileListItem,
   makeInstallerChoices,
@@ -212,6 +213,79 @@ describe("InstallDriver optional default-skip", () => {
     expect(rules.find((r) => r.reference.tag === "req-a")?.ignored).toBeUndefined();
   });
 
+  // Nexus file names repeat across unrelated mods ("Main File"), so a required member listed before
+  // an optional can share its logical file name. Defaulting the optional must skip the optional,
+  // not the required member, which would then never be installed, with the collection reported
+  // complete.
+  test("defaults the optional, not a required member with the same file name", async ({
+    makeDriver,
+  }) => {
+    const sharedReq = makeRule({
+      type: "requires",
+      reference: makeReference({ tag: "req-main", logicalFileName: "Main File" }),
+    });
+    const sharedOpt = makeRule({
+      type: "recommends",
+      reference: makeReference({ tag: "opt-main", logicalFileName: "Main File" }),
+    });
+    const fixture = buildCollectionFixture("col-shared", [sharedReq, sharedOpt]);
+    const h = makeDriver({
+      mods: { [GAME_ID]: { [fixture.collection.id]: fixture.collection } },
+      downloads: { [fixture.download.id]: fixture.download },
+      profiles: { [profile.id]: profile },
+    });
+    await startWith(h, fixture);
+
+    const session = h.getState().session.collections.activeSession;
+    expect(session?.mods[modRuleId(sharedOpt)].status).toBe("ignored");
+    expect(session?.mods[modRuleId(sharedReq)].status).not.toBe("ignored");
+    const rules = h.getState().persistent.mods[GAME_ID]["col-shared"].rules ?? [];
+    expect(rules.find((r) => r.reference.tag === "opt-main")?.ignored).toBe(true);
+    expect(rules.find((r) => r.reference.tag === "req-main")?.ignored).toBeUndefined();
+  });
+
+  // A deterministic collection tags a fuzzy member by its mod page and install spec, so a required
+  // and an optional file from one page share a tag. Defaulting the optional must still skip it,
+  // not the required member.
+  test("defaults the optional, not a required member sharing its deterministic tag", async ({
+    makeDriver,
+  }) => {
+    const page = { repository: "nexus", gameId: GAME_ID, modId: "1234", fileId: "1" };
+    const sharedReq = makeRule({
+      type: "requires",
+      reference: makeDeterministicRef({
+        repo: page,
+        versionMatch: ">=1.0.0+prefer",
+        logicalFileName: "Main File",
+      }),
+    });
+    const sharedOpt = makeRule({
+      type: "recommends",
+      reference: makeDeterministicRef({
+        repo: { ...page, fileId: "2" },
+        versionMatch: ">=1.0.0+prefer",
+        logicalFileName: "Optional patch",
+      }),
+    });
+    // the fixture models the real rule: both members carry the same tag
+    expect(sharedReq.reference.tag).toBeDefined();
+    expect(sharedOpt.reference.tag).toBe(sharedReq.reference.tag);
+    const fixture = buildCollectionFixture("col-det", [sharedReq, sharedOpt]);
+    const h = makeDriver({
+      mods: { [GAME_ID]: { [fixture.collection.id]: fixture.collection } },
+      downloads: { [fixture.download.id]: fixture.download },
+      profiles: { [profile.id]: profile },
+    });
+    await startWith(h, fixture);
+
+    const session = h.getState().session.collections.activeSession;
+    expect(session?.mods[modRuleId(sharedOpt)].status).toBe("ignored");
+    expect(session?.mods[modRuleId(sharedReq)].status).not.toBe("ignored");
+    const rules = h.getState().persistent.mods[GAME_ID]["col-det"].rules ?? [];
+    expect(rules.find((r) => r.type === "recommends")?.ignored).toBe(true);
+    expect(rules.find((r) => r.type === "requires")?.ignored).toBeUndefined();
+  });
+
   test("does not re-default an optional the user already selected (ignored:false)", async ({
     makeDriver,
   }) => {

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

@@ -12,6 +12,7 @@ import type { IState } from "../../../types/IState";
 import {
   generateCollectionSessionId,
   isTerminalMemberStatus,
+  modRuleId,
 } from "../../../util/collectionInstallSession";
 import {
   getCollectionActiveSession,
@@ -997,7 +998,10 @@ class InstallDriver {
     // rule flag + session status) now that the session exists.
     for (const rule of optional) {
       if (rule.ignored === undefined) {
-        markCollectionMemberSkipped(this.mApi, { reference: rule.reference });
+        markCollectionMemberSkipped(this.mApi, {
+          reference: rule.reference,
+          ruleId: modRuleId(rule),
+        });
       }
     }
 

src/renderer/src/extensions/mod_management/InstallManager.ts

@@ -857,6 +857,22 @@ class InstallManager {
     this.maybeAdvancePhase(collectionId, api);
   }
 
+  /**
+   * Whether a dependency is a member of the collection being installed: it carries the member's
+   * session key, or it belongs to the collection itself rather than to one of its members. A
+   * member's own dependencies are installed with the member as their source and are not members.
+   */
+  private isCollectionMemberDependency(
+    api: IExtensionApi,
+    sourceModId: string,
+    dep: IDependency,
+  ): boolean {
+    return (
+      dep.sessionRuleId !== undefined ||
+      getCollectionActiveSession(api.getState())?.collectionId === sourceModId
+    );
+  }
+
   private handleDownloadSkipped(api: IExtensionApi, sourceModId: string, dep: IDependency) {
     if (!sourceModId || !dep) {
       return;
@@ -882,8 +898,11 @@ class InstallManager {
     }
 
     // Mark the skipped member ignored directly against the active session (collections is core
-    // now, so no event round-trip through the InstallDriver is needed).
-    markCollectionMemberSkipped(api, { reference: dep.reference });
+    // now, so no event round-trip through the InstallDriver is needed). A sub-dependency of a
+    // member is not a member itself, so its skip ignores nothing.
+    if (this.isCollectionMemberDependency(api, sourceModId, dep)) {
+      markCollectionMemberSkipped(api, { reference: dep.reference, ruleId: dep.sessionRuleId });
+    }
 
     // See if we can advance the phase
     this.maybeAdvancePhase(sourceModId, api);
@@ -2720,8 +2739,14 @@ class InstallManager {
           } else {
             this.mDependencyRetryCount.delete(installKey);
             if (recovery.action === "skip") {
-              // same settle the free-user skip performs: durable ignore + session "ignored"
-              markCollectionMemberSkipped(api, { reference: dep.reference });
+              // same settle the free-user skip performs: durable ignore + session "ignored".
+              // A sub-dependency of a member is not a member itself, so its skip ignores nothing.
+              if (this.isCollectionMemberDependency(api, sourceModId, dep)) {
+                markCollectionMemberSkipped(api, {
+                  reference: dep.reference,
+                  ruleId: dep.sessionRuleId,
+                });
+              }
             } else if (recovery.action === "fail") {
               // Retries exhausted: settle the member as failed (terminal) so the collection can
               // still complete and the member is not re-prompted. writeCollectionSession no-ops

src/renderer/src/test-utils/builders.ts

@@ -36,6 +36,7 @@ import type {
   ICollectionMod,
   ICollectionModRule,
 } from "../extensions/collections/types/ICollection";
+import { deterministicReferenceTag } from "../extensions/collections/util/deterministicReferenceTag";
 import type InstallDriver from "../extensions/collections/util/InstallDriver";
 import { stateReducer as downloadStateReducer } from "../extensions/download_management/reducers/state";
 import { downloadPathForGame } from "../extensions/download_management/selectors";
@@ -135,6 +136,14 @@ export function makeFuzzyRef(overrides: Partial<IModReference> = {}): IModRefere
   return makeExactRef({ versionMatch: "*", ...overrides });
 }
 
+// A reference tagged the way a deterministic collection tags its members (transformCollection):
+// the tag is derived from the reference, so two fuzzy files from one mod page share it, as they do
+// in real collections. Tests of member identity must use this rather than a hand-picked tag.
+export function makeDeterministicRef(overrides: Partial<IModReference> = {}): IModReference {
+  const reference = makeExactRef({ tag: undefined, ...overrides });
+  return { ...reference, tag: deterministicReferenceTag(reference) };
+}
+
 export function makeRule(overrides: Partial<IModRule> = {}): IModRule {
   return {
     type: "requires",

src/renderer/src/util/collectionSkip.test.ts

@@ -10,6 +10,7 @@ import { describe, expect } from "vitest";
 
 import type { IModRule } from "../extensions/mod_management/types/IMod";
 import {
+  makeDeterministicRef,
   makeInstallState,
   makeMod,
   makeModInstallInfo,
@@ -141,6 +142,123 @@ describe("markCollectionMemberSkipped - automatic skip (mod reference)", () => {
     expect(flagged).toEqual(["tag-x"]);
   });
 
+  // the member the tag names wins over one met earlier that only shares a weaker marker, in the
+  // session scan and in the live-rule scan alike
+  test.for([
+    ["file hash", { fileMD5: "shared-md5" }],
+    ["logical file name", { logicalFileName: "Main File" }],
+  ] as const)(
+    "ignores the tagged member, not an earlier one with the same %s",
+    ([, shared], { makeApi }) => {
+      const earlier = makeRule({
+        type: "requires",
+        reference: makeReference({ tag: "tag-earlier", ...shared }),
+      });
+      const skipped = makeRule({
+        type: "recommends",
+        reference: makeReference({ tag: "tag-skipped", ...shared }),
+      });
+      const h = makeApi({
+        mods: {
+          [GAME_ID]: {
+            [COLLECTION_ID]: makeMod({ id: COLLECTION_ID, rules: [earlier, skipped] }),
+          },
+        },
+        session: makeInstallState({
+          activeSession: makeSession({
+            sessionId: SESSION_ID,
+            collectionId: COLLECTION_ID,
+            gameId: GAME_ID,
+            mods: {
+              [modRuleId(earlier)]: makeModInstallInfo({ rule: earlier, status: "pending" }),
+              [modRuleId(skipped)]: makeModInstallInfo({ rule: skipped, status: "pending" }),
+            },
+          }),
+        }),
+      });
+
+      const matched = markCollectionMemberSkipped(h.api, { reference: skipped.reference });
+
+      expect(matched).toBe(true);
+      expect(statusOf(h, skipped)).toBe("ignored");
+      expect(statusOf(h, earlier)).toBe("pending");
+      const rules = h.getState().persistent.mods[GAME_ID][COLLECTION_ID].rules ?? [];
+      const flagged = rules
+        .filter((rule) => rule.ignored === true)
+        .map((rule) => rule.reference.tag);
+      expect(flagged).toEqual(["tag-skipped"]);
+    },
+  );
+
+  // two fuzzy files from one mod page share a deterministic tag; the skip site's rule id tells
+  // them apart where no marker can
+  test("ignores the member its rule id names when another shares its deterministic tag", ({
+    makeApi,
+  }) => {
+    const page = { repository: "nexus", gameId: GAME_ID, modId: "1234", fileId: "1" };
+    const required = makeRule({
+      type: "requires",
+      reference: makeDeterministicRef({ repo: page, versionMatch: ">=1.0.0+prefer" }),
+    });
+    const optional = makeRule({
+      type: "recommends",
+      reference: makeDeterministicRef({
+        repo: { ...page, fileId: "2" },
+        versionMatch: ">=1.0.0+prefer",
+      }),
+    });
+    expect(optional.reference.tag).toBe(required.reference.tag);
+    const h = makeApi({
+      mods: {
+        [GAME_ID]: {
+          [COLLECTION_ID]: makeMod({ id: COLLECTION_ID, rules: [required, optional] }),
+        },
+      },
+      session: makeInstallState({
+        activeSession: makeSession({
+          sessionId: SESSION_ID,
+          collectionId: COLLECTION_ID,
+          gameId: GAME_ID,
+          mods: {
+            [modRuleId(required)]: makeModInstallInfo({ rule: required, status: "pending" }),
+            [modRuleId(optional)]: makeModInstallInfo({ rule: optional, status: "pending" }),
+          },
+        }),
+      }),
+    });
+
+    const matched = markCollectionMemberSkipped(h.api, {
+      reference: optional.reference,
+      ruleId: modRuleId(optional),
+    });
+
+    expect(matched).toBe(true);
+    expect(statusOf(h, optional)).toBe("ignored");
+    expect(statusOf(h, required)).toBe("pending");
+    const rules = h.getState().persistent.mods[GAME_ID][COLLECTION_ID].rules ?? [];
+    expect(rules.filter((rule) => rule.ignored === true).map((rule) => rule.type)).toEqual([
+      "recommends",
+    ]);
+  });
+
+  // a tagged skip that names no member (a sub-dependency, say) must not land on a member that
+  // only shares its logical file name
+  test("does not fall back to the file name for a skip that carries a tag", ({ makeApi }) => {
+    const rule = makeRule({
+      type: "requires",
+      reference: makeReference({ tag: "mod-a", logicalFileName: "Main File" }),
+    });
+    const h = makeApi(ruleOverrides(rule));
+
+    const matched = markCollectionMemberSkipped(h.api, {
+      reference: makeReference({ tag: "not-a-member", logicalFileName: "Main File" }),
+    });
+
+    expect(matched).toBe(false);
+    expect(statusOf(h, rule)).toBe("pending");
+    expect(durableIgnored(h)).toBeUndefined();
+  });
+
   // a session can track a member the collection's current rules no longer carry (the rules were
   // replaced mid-install); the skip settles the session entry without re-adding the old rule
   test("settles a member whose rule left the collection without re-adding it", ({ makeApi }) => {