Skip to content

fix(collections): keep running checks after a collection installs - #24282

Closed
doodlum wants to merge 3 commits into
Nexus-Mods:masterfrom
doodlum:fix/collection-test-suppression
Closed

doodlum wants to merge 3 commits into
Nexus-Mods:masterfrom
doodlum:fix/collection-test-suppression

Conversation

@doodlum

@doodlum doodlum commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Problem

After a collection install completes, Vortex stops running the checks registered on plugins-changed, mod-installed, mod-activated and settings-changed until it restarts. Missing Masters is one of them, so a collection that brings in a plugin with a missing master never warns about it. This matches Linear LAZ-1022 ("Warnings intermittently fail to be displayed — especially missing masters… only seen on collections"), and a report on the Fallout 4 collection 5atq9t against 2.7 and 2.8 beta: no missing-master warning with Vortex open for two hours.

InstallDriver.startInstall holds those checks off through the test runner's withSuppressedTests while a collection installs. Only onStop() (cancel, pause) released the hold. A successful install ends through the review screen's close(), which never did, so the counter stayed above zero and runChecks returned early from then on. Suppressed events are dropped, not queued, and nothing is logged. The same code is on v2.7.0, release/v2.8 and master.

Change

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

  • close() releases the hold, as onStop() does.
  • When the hold is released, the held-off checks run once through trigger-test-run, at the delays the test runner itself uses (500 ms for plugins-changed and settings-changed, 5000 ms for mod-installed and mod-activated). Without this, a missing master the collection itself introduced would stay unreported until something else changed the plugins.
  • startInstall releases a hold this driver still owns before taking a new one, so resuming an install that ended incomplete cannot stack a second hold.
  • An install that never starts (no archive or profile, Cancel at the game-version prompt, or a throw in startImpl) releases the hold. Before, it stayed held for the session.

InstallDriver.suppression.test.ts (new) drives the real driver through the collection harness, with a stand-in for withSuppressedTests that counts holds the way the test runner does. It records the hold count when each re-run is emitted: the real runner drops a run while any hold is outstanding.

Behaviour changes

  • Missing Masters and the other held-off checks keep working after a collection completes. A missing master brought in by the collection is reported about 0.7–1 s after Done, with no further action.
  • The held-off checks now re-run once on every release: completion, cancel, pause (including logout, game switch and the free-user cancel), Cancel at the game-version prompt, and a start that fails early. Before, cancel and pause released the hold without re-running anything.
  • Resuming an incomplete install briefly holds two suppressions (the old one is released in a later .finally). Re-runs from that first release are dropped by the runner, and the final release re-runs them.
  • A 5000 ms re-run can fire inside the next collection's hold, because the runner checks suppression when a run is scheduled, not when it fires. It costs one extra check pass.
  • new Bluebird stays in startInstall, although CODESTYLE.md discourages it, because withSuppressedTests is typed () => PromiseBB<void> (test_runner/index.ts:205).

Evidence

Head d23b5a2, on master 031b81d. Tested on Windows 10.

In the app (independent QA, production builds, fake Fallout 4 via the kit's --bethesda-sandbox, offline collections, --fresh profile per run):

Scenario master 031b81d this PR
Collection member with a missing master, no action after Done never flagged in 60 s; plugins-changed ran 0 times flagged 0.7 s after Done (0.7, 0.7, 1.0 s over 3 runs); one notification
Missing-master plugin installed and deployed after the collection never flagged flagged in 0.5 s
Checks during the install 0 runs 0 runs (still held off)
Pause mid-install flagged 1.5 s after flagged 0.7 s after; one notification
Cancel mid-install – flagged 1.0 s after; checks keep working
Two collections back to back, missing master in the second – flagged 0.7 s after the second Done; holds don't stack
Cancel at the game-version prompt checks suppressed for the rest of the session hold released and checks re-run (see Not covered)

Regression tests: 7 tests in InstallDriver.suppression.test.ts: completed, cancelled, game-version Cancel, pause then resume, optional mods from review, consecutive collections, and resuming an install that ended incomplete. Negative control (pr-preflight revert check): 6 of 7 fail on master. The cancelled-install case already worked there. With only the stack guard in startInstall deleted, only its test fails.

Scoped suites: vitest run src/extensions/collections src/extensions/test_runner: 17 files, 265 tests pass. Renderer typecheck and lint pass. oxfmt is clean.

pnpm run verify on d23b5a2: fails only where master fails. Master fails in 7 Windows icon-extraction tests (src/index.test.ts), which read notepad.exe on this machine. prepareSupportBundle > kills the archiver and cleans up when aborted mid run also failed under full-verify load. That code is untouched here, and it passes 5 of 5 in isolation on both master and this branch. The formatter left the tree clean.

E2E (packages/e2e, the kit's runner, which fixes the fixture's main-window startup race for the run only): 24 passed and 4 failed, with 38 skipped for lack of Nexus test-account credentials. Master: 25 passed, 3 failed (QA-106, QA-113, QA-128), 38 skipped. The only difference is Dashboard - Getting Started Videos > video player popup can be closed. It is flaky on master too: it failed in 1 of 4 master runs and 2 of 3 runs here, and this change doesn't touch the dashboard.

CI: all checks pass on d23b5a2. An earlier ubuntu failure on 2758a11 was infrastructure: loot failed to link libloot during pnpm install, and a rerun passed.

pr-preflight: size 318 lines in 2 files. It flags InstallStartDialog.startInstall as a caller outside the diff. That is the dialog's own method with the same name, not a caller. close is private.

Review

Round 1 (read-only adversarial review) found:

  • the hold leaking when the install never starts: fixed in f3e46dd;
  • a test fake that didn't check hold counts at emit time: fixed in f3e46dd;
  • evidence that didn't exercise the re-run: covered by QA's no-action scenario above;
  • the undisclosed pause behaviour and the wrong delays: fixed in f3e46dd;
  • the claim that switching the game also reset the suppression: it doesn't, and the claim is removed.

Round 2 (QA in the app, then review) found nothing blocking. Its one medium finding, the stack guard being untested, is fixed in d23b5a2. Findings by the layer that should have caught them: round 1, 3 author, 2 new, 1 preflight, 1 judgment; round 2, 1 author, 3 judgment.

Not covered

  • Cancel at the game-version prompt doesn't cancel. The next driver update installs the collection anyway, without a review screen. This predates this PR and is confirmed in the app on master. It has a separate fix on branch fix/collection-game-version-cancel. That fix merges cleanly with this one, and together the double release becomes a no-op.
  • An install that ends incomplete keeps the hold until the user resumes or pauses it.
  • With the game-version Cancel fix also applied, one near-unreachable edge remains. If an install attempt is paused while its game-version prompt is open, and a newer attempt starts before that prompt is answered, answering the old prompt makes this PR's release-on-false release the newer attempt's hold. The prompt is modal, so this needs a pause (logout or game switch) and a scripted start behind it.
  • Installing a bundled optional member from the review screen stalls in the offline setup QA used. That is unrelated to this change.
  • Other ways Missing Masters can fail silently, found while tracing: pluginList changes don't re-trigger the check; a plugin header that fails to parse is treated as having no masters; and a check that throws is only logged.

🤖 Generated with Claude Code

…etes

startInstall suppresses the plugins-changed, mod-installed, mod-activated and
settings-changed checks while a collection installs, and only onStop (cancel,
pause) released them. A successful install ends through the review screen's
close(), which never did, so from then on none of those checks ran again for
the rest of the session. The Missing Masters check is one of them: a user who
finished a collection and then fixed or broke a load order was never told.

- close() releases the hold, as onStop does.
- startInstall releases a hold it still owns before taking another, so
  consecutive collections cannot stack.
- Once released, the held-off checks run once. Suppressed events are dropped,
  not queued, so a missing master the collection itself brought in would
  otherwise go unreported until something else changed the plugins.

Reproduced in a source build against a disposable Fallout 4 fixture and an
offline (bundled-member) collection, counting check runs with a probe check
registered through registerTest. After the collection closes, installing and
deploying a plugin with a missing master:
- unpatched: plugins-changed checks run 0 times, no missing-master flag
- patched: checks re-run on close, plugins-changed runs 2 times, the plugin
  is flagged missing-master

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
doodlum added a commit to doodlum/vortex-doodlebot that referenced this pull request Sep 23, 2026
Diagnose Vortex issues that only large libraries and Bethesda games show,
without a game install and without changing Vortex.

Fake Fallout 4 (`up --dev-dir <checkout> --bethesda-sandbox`):
- A stand-in executable and generated plugins with real TES4 headers
  (bethesdaSandbox.ts).
- Private LocalAppData and Documents for plugins.txt, loadorder.txt and INI
  files. LOCALAPPDATA is redirected by environment, Documents by a
  NODE_OPTIONS preload that sets app.setPath on the app's first
  require("electron") (mainPreload.ts).
- The harness refuses to manage the game unless automation_status.paths
  shows both redirects, and kills an instance whose preload did not run
  before a game can activate. Packaged builds ignore NODE_OPTIONS, so this
  runs on source builds.
- --isolate-user-folders gives any game the same private folders.
- --inspect-brk was tried and rejected: Node workers inherit
  break-on-start, and Vortex's archive hashing worker then hangs every
  install.

Offline collections (offlineCollection.ts): members bundled in the archive,
installed through Install Now, review and Done, with no Nexus access.

Instrumentation:
- perf_trace_start/stop: dispatch timing per action type, long tasks, heap.
- check_probe_counts: probe checks registered through registerTest count
  how often Vortex runs checks per event (harness instances only).
- profiling.ts: renderer CPU profiles over CDP, summarised by function and
  file.
- vortexLog.ts: main-process timings Vortex already logs (persist diffs,
  slow writes, sorts, backups), read for a window.
- automation_status reports the documents/localAppData paths Vortex
  resolved.

`ai:test:bethesda` checks Missing Masters on the fake game before and after
a completed offline collection. It reproduced Vortex never re-running its
checks after a collection install (Nexus-Mods/Vortex#24282).

Also:
- A reset profile now clears the disposable game's deployed files. Stale
  manifests blocked deploys on External Changes.
- The External Changes policy refuses only "Links were deleted", whose
  default deletes staging files, and still confirms deleted sources.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@doodlum
doodlum requested a lite review from Copilot September 24, 2026 00:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

No unresolved review issues were identified, and regression coverage is included.

Review effort: Lite
Findings: None

What changed in this PR

Fixes collection-install test suppression so checks resume after installation completes.

Changes:

  • Releases suppression when the review screen closes.
  • Re-runs affected checks after release.
  • Adds regression coverage for completion, cancellation, and consecutive installs.
File Description
src/​renderer/​src/​extensions/​collections/​util/​InstallDriver.ts Manages suppression release and post-install check reruns.
src/​renderer/​src/​extensions/​collections/​util/​InstallDriver.suppression.test.ts Covers suppression lifecycle scenarios.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

doodlum and others added 2 commits September 24, 2026 01:34
An install that returns early from startImpl (no archive or profile, the
game-version prompt cancelled) or throws reached neither onStop nor close,
so the hold startInstall took on the test runner leaked. Release it there.

Re-run the held-off checks with the delays the test runner itself uses for
each event (500 ms for plugins-changed and settings-changed, 5000 ms for
mod-installed and mod-activated) rather than the runner's 500 ms default.

The suppression tests now use native promises, record how many holds were
outstanding when each re-run was requested (a re-run emitted while held is
dropped), and cover the game-version cancel, pause then resume, and
installing the optional mods from the review screen more than once.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A required pass that ends with a member still pending leaves the driver
on "installing" with the collection set and the check hold still taken.
Resuming it calls start again; the guard at the top of startInstall
must release that hold before taking a new one, or the first is never
released. Nothing covered the guard: deleting it kept every test green.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@doodlum doodlum changed the title fix(collections): release the check suppression when an install completes fix(collections): keep running checks after a collection installs Sep 24, 2026
@doodlum

doodlum commented Sep 24, 2026

Copy link
Copy Markdown
Contributor Author

Moved to doodlum#1 for testing.

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.

2 participants