Add Vortex coexistence & Collections handling (Unit K) - #25
Merged
Conversation
Detect, warn, and reconcile when Vortex's built-in game-witcher3 Script Merger integration modifies WSM merge state out from under this companion extension - Collections import (importScriptMerges) and per-profile merge backup/restore (mergeBackup.ts's storeToProfile/restoreFromProfile) can both overwrite merge output with no way for this extension to intercept them (vortex-api has no extension-blocking API). New src/coexistenceGuard.ts snapshots two independent signals (a list_merges-derived signature, and a plain recursive listing of the real merged-mod folder - the latter needed because each companion binary keeps its own separate MergeInventory.xml, so the folder is the one resource genuinely shared with game-witcher3's own discovered tool) and compares them at three trigger points wired from index.ts: gamemode-activated, profile-did-change (not profile-will-change, which Vortex never awaits), and did-deploy - positioned above the isModOrDependencyInstallActive gate so a Collection install is still observed. resolveAction.ts's own merge workflow reconciles the baseline afterward: silently for a real merge (this extension's own write), but through the same compare-and-warn path for a dry-run preview, so a preview-then-cancel can't silently erase evidence of undetected drift. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXAuGMLB44T5Zv5o5ZzKah
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.
Summary
Unit K of the Vortex companion extension: detect, warn, reconcile when Vortex's built-in
game-witcher3Script Merger integration modifies WSM merge state out from under this extension. Perdocs/vortex-extension-design.md§0/Open Question 1, there is novortex-apimechanism to intercept, disable, or block another extension's registrations or event handlers (re-confirmed against the publishedlib/api.d.ts), so this unit cannot prevent either hazard - only notice, warn distinctly, and point the user at existing remediation (the merge-history dashlet, "Resolve Script Conflicts"). Per the task's instructions,docs/vortex-extension-design.mditself is intentionally untouched (a parallel unit is reconciling it separately) - the design rationale below, and in code comments, is the substitute channel.Two mechanistic corrections to the design doc's own framing
Found by fetching and reading the real
Nexus-Mods/Vortexmonorepo source directly (gh api repos/Nexus-Mods/Vortex/contents/extensions/games/game-witcher3/src/{eventHandlers,mergeBackup,common,index}.ts), not carried over from the design doc's summary. Both changed how detection had to work here, so they're recorded here and insrc/coexistenceGuard.ts's own header doc comment with the same citations:MergeInventory.xmlby default.mergeBackup.ts'shandleMergedScriptsresolvesMERGE_INV_MANIFEST("MergeInventory.xml",common.ts) againstpath.dirname(scriptMergerTool.path), wherescriptMergerToolis the discovery entry forSCRIPT_MERGER_ID = "W3ScriptMerger"-game-witcher3's own discovered tool (typically theIDCs/WitcherScriptMergerfork it auto-downloads). This extension registers a distinct tool id,WitcherScriptMergerEnhanced(discoveredTool.ts), at its own acquired path. Per WSM's ownPaths.Inventory, resolved againstEnvironment.CurrentDirectory(pinned toAppContext.BaseDirectorybefore dispatching tomerge/mcp), each binary'sMergeInventory.xmllives next to that binary - not a shared file. What genuinely is shared:handleMergedScripts'smergedScriptsPath = path.join(gamePath, "Mods", mergedModName), the real, physical merged-mod-content folder every WSM instance reads/writes via identicalGameDirectory/ModsDirectoryresolution. This is whycoexistenceGuard.tssnapshots a plain recursive listing of that real folder alongside alist_merges-derived signature, rather than relying onlist_merges/MergeInventory.xmlalone - the folder listing is the one signal that also catches drift in the default (separate-binaries) configuration;list_mergesalone would only catch it if a user pointedgame-witcher3's ownW3ScriptMergerdiscovery at the exact same binary this extension acquired.mergeBackup.ts'sgenBasePropsreturnsundefined(a no-op) unlessstate.persistent.profiles[profileId].features.local_mergesistrue, andgame-witcher3's ownindex.tsregisters that feature (context.registerProfileFeature("local_merges", "boolean", "settings", "Profile Data", "This profile will store and restore profile specific data (merged scripts, loadorder, etc) when switching profiles", ...)) as an ordinary Vortex profile toggle defaulting to unset/false. So hazard 2 does not fire "on every profile switch" as the design doc's own phrasing suggests - only for a profile the user has explicitly opted into that feature.Design
index.ts):gamemode-activated(session start / switching into Witcher 3 mid-session),profile-did-change, anddid-deploy- the last positioned above the existingisModOrDependencyInstallActiveearly-return, since installing a Collection is exactly the windowimportScriptMerges()(hazard 1) fires in; a check gated behind that same early-return would never see the deployment where the overwrite happened.profile-will-change. That event is a plain, synchronousevents.onemit (EVENTS.mdmarkswill-deploy/did-deploy"Async.", not this one), andgame-witcher3's ownonProfileWillChangehandler is registered the same synchronous way (confirmed in itsindex.ts) - Vortex never awaits it, so there's no reliable way to bracket its file moves from a second listener on the same event.profile-did-change(after the switch) is used instead; every trigger point re-observes settled state rather than trying to catch a change mid-flight.did-deploy, not amortized ontoconflictScan.ts'sscanWsmConflictsclient. Amortizing would either widen that module's own established, separately-tested return shape andinFlightScancoalescing, or bypass it with a second connect anyway - more coupling than a rare-event detector (not a per-deployment hot path) justifies.computeMergeStateSnapshot's two independent MCP calls (get_status,list_merges) run concurrently viaPromise.all, and the connect uses a bounded 15s per-request timeout (matchingconflictScan.ts's ownPOST_DEPLOY_SCAN_TIMEOUT_MS) since this sits inside Vortex's awaiteddid-deploywindow on that trigger path.resolveAction.ts'srunMergeConflictsWorkflowcallscoexistenceGuard.tsafter everymergeConflictscall, but not the same way for both: a real merge (dryRun !== true) silently callsrecordOwnMergeStateSnapshot(this extension's own write, known-good by definition); a dry-run preview instead goes throughcheckCoexistenceDrift(compare-and-warn), since a preview performs no write - silently re-baselining there (an earlier version of this branch did exactly that) would let a preview-then-cancel workflow permanently erase evidence of real, undetected drift with no notification ever shown.conflictNotifications.ts's own suppression signature - otherwise a change that fully resolved every conflict would never get its stale "N unresolved" notification cleared, since the reset alone produces the same empty-signature value a fresh session starts with.Testing
src/coexistenceGuard.test.ts(new, 26 tests): signature helpers, folder-listing walk (including a symlink-handling regression test, self-skipping onEPERM),computeMergeStateSnapshot,checkCoexistenceDrift(including the fixed baseline-commit-after-success ordering and the stale-notification dismissal),refreshCoexistenceState.test/coexistenceGuard.integration.test.ts(new): spawns a realWitcherScriptMerger.Headless.exe, drives a genuine auto-solving merge, and provescomputeMergeStateSnapshot/checkCoexistenceDriftreact correctly to real before/after state.src/index.test.ts/src/resolveAction.test.ts: updated/extended for the new wiring and the preview-vs-real-merge branch.Full pipeline (
typecheck && build && lint && test && test:integration) passes: 214 tests total.Review
Ran
/code-review; addressed all findings before this PR:dryRun.gamemode-activatedwas documented as a wired trigger point but never actually connected torefreshCoexistenceState- fixed, with a regression test that would have caught it.refreshCoexistenceStaterejects, while its own neighboring comment (and the actual code) said the opposite - corrected.computeMergeStateSnapshot's two independent MCP calls parallelized viaPromise.all.walkFilesRecursivesilently skipped symlinked entries - fixed to record them vialstat, with a self-skipping-on-EPERMregression test.A second, independent pass via the
advisortool caught two more before commit:checkCoexistenceDriftcommitting its baseline before the notification succeeded (should retry on failure, mirroringnotifyConflictsIfChanged's own documented reasoning) - fixed; andconflictNotifications.ts'sresetConflictNotificationStatedoc comment claiming "no production caller should ever need it," now false sincecoexistenceGuard.tsis a real production caller - fixed.🤖 Generated with Claude Code
https://claude.ai/code/session_01GXAuGMLB44T5Zv5o5ZzKah