Skip to content

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

Closed
doodlum wants to merge 2 commits into
masterfrom
fix/collection-retry
Closed

doodlum wants to merge 2 commits into
masterfrom
fix/collection-retry

Conversation

@doodlum

@doodlum doodlum commented Sep 27, 2026 •

Copy link
Copy Markdown
Owner

Head: 1c0bc4a9748597b8bf5165a399138c9ec1567b83 (#17, branch fix/collection-retry, based on upstream/master c8ea03d00)

Problem

markCollectionMemberSkipped (src/renderer/src/util/collectionSkip.ts) is the one function that turns a skip into an ignored member. It writes the session status "ignored" and the durable rule.ignored flag, which keeps the member out of every later resume. It finds the member by scanning the session and accepts the first entry whose tag, file hash or logical file name matches the skip's reference. None of these is unique:

  • A deterministic collection tags a fuzzy member by its mod page and install spec (deterministicReferenceTag), so a required and an optional file from one mod page share a tag.
  • Two members installing the same archive share a file hash.
  • Unrelated files share logical names such as "Main File".

InstallDriver.start defaults every optional member with no ignore choice to skipped through this function. So when a required member listed before such an optional shares one of these markers, the required member is ignored instead, for good. The optional is installed, and the review reports the collection complete. The same scan served handleDownloadSkipped and the Skip recovery in InstallManager, including for a member's own sub-dependencies, which are not collection members at all.

What this does not explain. The report behind this work: Vortex 2.7.1, Gate to Sovngarde (qdurkx revision 117, 1,846 required and 119 optional members), from a user whose downloads failed and were retried, and who then found members ignored and not installed. The members concerned are VIGILANT SE v180, Beyond Skyrim Assets 1.6.3 and Bruma 1.6.3, Skyland bits and Bobs, Book Covers Skyrim SE, GTS Grass, A. Vigilant Armors, and Caranthir Tower Reborn. Every one of them is required. Their downloads failed in the resumed session of 10:51 to 11:56. From the 12:00 start onwards no session touched them again, and the completed run of 13:01 counts ignored 11. They were therefore durably ignored between 11:11 and 12:00. That collection has no optional member sharing a file name or file id with a required one, so the collision fixed here is not what happened to that user. I have not found the path that did it. See Not covered.

Change

  • util/collectionSkip.ts:
    • A reference skip may carry ruleId, the member's session key (modRuleId). When the session has that member, it is used directly, with no marker scan.
    • Without a rule id, each marker is tried across all members before the next, weaker one. A skip that carries a tag tries the tag and then the file hash, never the logical file name. A tagless skip tries the file hash, then the logical file name.
    • The free-user identifier match is unchanged.
  • collections/util/InstallDriver.ts: the optional default passes modRuleId(rule).
  • mod_management/InstallManager.ts: handleDownloadSkipped and the instructions-Skip recovery pass dep.sessionRuleId. They mark nothing for a dependency that is not a collection member: one without a session key, whose source is a member mod rather than the collection being installed (isCollectionMemberDependency).
  • test-utils/builders.ts: makeDeterministicRef tags a reference the way deterministic collections do, so identity tests meet the real collision (review lesson 4).
  • Tests: collectionSkip.test.ts, InstallDriver.test.ts.

Callers outside the diff (preflight WARN, 9 entries), each checked:

  • nxmProtocol.ts:439: the free-user identifiers skip, whose path is unchanged.
  • The three changed skip sites: InstallDriver.ts:1000 and InstallManager.ts, handleDownloadSkipped and the recovery.
  • The rest are the touched test builders and helpers.

Exit paths:

  • A rule id the session doesn't have falls back to the marker scan.
  • No match logs and returns false, as before.
  • No active session returns early, as before.

Behaviour changes

  • When an optional member is defaulted to skipped, or when a member is skipped at a download or instructions prompt, only that member is ignored. Before, an earlier required member sharing its tag, file hash or file name could be ignored in its place.
  • A skip carrying a tag no longer ignores a member that only shares its logical file name. A tagless skip still matches on file hash, then on file name. Only the free-user path and callers without a rule id can send a tagless skip.
  • Skipping one of a member's own sub-dependencies no longer ignores any collection member.
  • Members already wrongly ignored keep the flag. "Stop Ignoring" on the collection page clears it.

Evidence

Regression tests. Each fails on master and passes on the head:

File Test Fails on
collectionSkip.test.ts the tagged member beats an earlier one sharing its file hash, and one sharing its file name master
collectionSkip.test.ts the rule id names the member when another shares its deterministic tag master and 5ab74f3
collectionSkip.test.ts a tagged skip doesn't fall back to the file name master and 5ab74f3
InstallDriver.test.ts the optional, not the required member, is defaulted when they share a file name master
InstallDriver.test.ts the optional, not the required member, is defaulted when they share a deterministic tag (built with makeDeterministicRef) master and 5ab74f3

Negative control. ai:preflight --checkout <worktree> reverted the three source files to c8ea03d: the tests pass on the branch and fail reverted (6 failed, 42 passed). Reverting them to 5ab74f3 instead fails the three deterministic-tag and fallback tests. Preflight overall: 0 fail, 1 warn (the callers above).

In the app. Source build, fake Fallout 4, slot 6. The script is harness/.artifacts/collection-retry/alias.mts: an offline collection with a bundled member and two direct members served locally, required listed first, then optional. One run per cell:

Scenario master c8ea03d 5ab74f3 head 1c0bc4a
Both called "Main File" required ignored, never downloaded; optional installed; review "complete" required installed, optional ignored required installed, optional ignored
Both share one tag (a stand-in for the deterministic collision) not run required ignored; the install stalled with the optional "downloading" and never reached the review required installed, optional ignored, review reached
Required download reset once, then Resume not run installed by the resume not run

The user's failure modes, run on 5ab74f3: scenarios.mts and twice.mts exercise them with direct members. In every one the member was never ignored and was installed once the server recovered:

  • a transfer stalled after a third of the bytes until Vortex's 15-second stall timeout;
  • a transfer reset mid-way;
  • a failure followed by an optionals-only pass from the collection page, then Resume;
  • a failure that repeats in a resumed session, then a Vortex restart and a second Resume.

pnpm run verify passed on 1c0bc4a (build, lint, typecheck, tests). The build rewrote etc/vortex.api.md (line endings), which was restored. pnpm run format left the tree clean.

E2E: not run. Vortex's own suite needs a packaged app.

CI: checks on #17 not yet read for this head.

Review

Round 1 QA found:

  • The root cause claimed for the report didn't fit the user's collection. Judgment. The Problem section is rewritten, and the claim is withdrawn.
  • Deterministic tags defeated the tag-first match. New. Fixed in 1c0bc4a by passing the rule id.
  • The file-name fallback could still ignore an unrelated member. Author. Fixed in 1c0bc4a: a tagged skip no longer falls back to the name, and sub-dependencies mark nothing.
  • The description had stale lines and a wrong comment. Author. Fixed.

Count: 0 preflight, 2 author, 1 new, 1 judgment.

Not covered

  • The user's actual cause is unknown. Eight required members whose downloads failed in the 10:51 to 11:56 session were durably ignored before the 12:00 start. What those eight have in common: each of their downloads failed in the resumed session, and seven of them were in flight when the user paused at 10:55. Downloads that succeeded after the pause (New Legion, Bonemold) were not ignored. The logs show no call into this code, because markCollectionMemberSkipped logs nothing on success, and no dependency skip line. Next steps:
    • get the user's state.v2 or a list of their collection's rules to see which carry ignored;
    • add a log line on every durable ignore (not in this PR).
  • Found while reproducing, separate bug. Pausing a collection while a download is in flight, then resuming, resumes that download twice at once: two Resuming paused download lines and two overlapping range requests. The archive is corrupted ("CRC Failed"), the install is canceled, and the member is requeued and left on "installing", so the collection never reaches its review. Reproduced once with scenarios.mts pause. Needs its own issue.
  • Two members from one mod page still share a session key when they have the same type (modRuleId is type_tag), and an installed member satisfies an optional that shares its tag. This is pre-existing and needs a follow-up.
  • Missing Masters after a collection is fix(collections): keep running checks after a collection installs #1 (upstream fix(collections): keep running checks after a collection installs Nexus-Mods/Vortex#24282). It merges cleanly with the first commit of this branch, and ai:test:bethesda passed on that merge.
  • The review says the collection is complete while required members are ignored. The user is told nothing about them.

Written with doodlebot.

doodlum and others added 2 commits September 27, 2026 16:37
…ile name

markCollectionMemberSkipped matched a skipped reference against the session
members one by one, accepting a tag, file hash or logical file name on the
first member it met. At install start every untouched optional member is
defaulted to skipped through it, so an optional that shared a file hash or a
logical file name ("Main File") with a required member listed before it
marked the required member ignored instead. The flag is durable: the
required member was never downloaded on this or any later resume, the
collection reported complete, and the optional the user never chose was
installed.

Try each identity marker across every member before the next, weaker one,
so the tagged member wins and the hash and file name stay fallbacks.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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, and
the tag-first match from the previous commit still ignored the required
member when the optional was defaulted to skipped.

The skip sites that have the member's rule now pass its session key
(modRuleId) and markCollectionMemberSkipped uses it directly: the optional
default in InstallDriver.start, handleDownloadSkipped and the instructions
Skip recovery in InstallManager. The marker scan remains for a skip with no
rule id, and a skip that carries a tag no longer falls back to the logical
file name. A member's own sub-dependency is not a collection member, so its
skip no longer ignores whichever member shares its markers.

makeDeterministicRef builds references tagged the way deterministic
collections tag them, so identity tests meet the real collision.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@doodlum doodlum changed the title fix(collections): skip the member a skip names, not one sharing its file name fix(collections): ignore only the member a skip names Sep 27, 2026
@doodlum

doodlum commented Sep 27, 2026

Copy link
Copy Markdown
Owner Author

Closing: not enough information to reproduce the reported failure (the member was likely cancelled rather than failing). The skip-matcher change can be revisited if it recurs.

@doodlum doodlum closed this Sep 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant