Repository navigation
Conversation
…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>
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Head:
1c0bc4a9748597b8bf5165a399138c9ec1567b83(#17, branchfix/collection-retry, based on upstream/masterc8ea03d00)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 durablerule.ignoredflag, 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:deterministicReferenceTag), so a required and an optional file from one mod page share a tag.InstallDriver.startdefaults 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 servedhandleDownloadSkippedand the Skip recovery inInstallManager, 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 (
qdurkxrevision 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 countsignored 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:ruleId, the member's session key (modRuleId). When the session has that member, it is used directly, with no marker scan.collections/util/InstallDriver.ts: the optional default passesmodRuleId(rule).mod_management/InstallManager.ts:handleDownloadSkippedand the instructions-Skip recovery passdep.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:makeDeterministicReftags a reference the way deterministic collections do, so identity tests meet the real collision (review lesson 4).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.InstallDriver.ts:1000andInstallManager.ts,handleDownloadSkippedand the recovery.Exit paths:
Behaviour changes
Evidence
Regression tests. Each fails on master and passes on the head:
collectionSkip.test.tscollectionSkip.test.ts5ab74f3collectionSkip.test.ts5ab74f3InstallDriver.test.tsInstallDriver.test.tsmakeDeterministicRef)5ab74f3Negative control.
ai:preflight --checkout <worktree>reverted the three source files toc8ea03d: the tests pass on the branch and fail reverted (6 failed, 42 passed). Reverting them to5ab74f3instead 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:c8ea03d5ab74f31c0bc4aThe user's failure modes, run on
5ab74f3:scenarios.mtsandtwice.mtsexercise them with direct members. In every one the member was never ignored and was installed once the server recovered:pnpm run verifypassed on1c0bc4a(build, lint, typecheck, tests). The build rewroteetc/vortex.api.md(line endings), which was restored.pnpm run formatleft 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:
1c0bc4aby passing the rule id.1c0bc4a: a tagged skip no longer falls back to the name, and sub-dependencies mark nothing.Count: 0 preflight, 2 author, 1 new, 1 judgment.
Not covered
markCollectionMemberSkippedlogs nothing on success, and no dependency skip line. Next steps:state.v2or a list of their collection's rules to see which carryignored;Resuming paused downloadlines 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 withscenarios.mts pause. Needs its own issue.modRuleIdistype_tag), and an installed member satisfies an optional that shares its tag. This is pre-existing and needs a follow-up.ai:test:bethesdapassed on that merge.Written with doodlebot.