Add Vortex extension tool acquisition and env-var configuration (Unit F) - #17
Merged
Conversation
Adds the piece that makes the Vortex companion extension actually usable: downloading a WSM release build from GitHub Releases, verifying/extracting it, and registering it as a discovered Vortex tool (WitcherScriptMergerEnhanced, distinct from game-witcher3's built-in W3ScriptMerger), configured entirely via WSM_<KeyName> environment variables rather than by editing .exe.config XML. New src/ modules: storage.ts (extension-private storage layout, including the QuickBMS/wcc_lite storage convention a future bundle-tooling unit will reuse), wsmEnv.ts (WSM_ env-var builder), githubRelease.ts (download logic behind an injectable HttpClient seam), archiveExtractor.ts (wraps Vortex's own api.openArchive instead of a hand-rolled zip parser or new dependency), discoveredTool.ts, and toolAcquisition.ts (orchestration + a network-free local re-registration path wired into index.ts, now re-checked live on gamemode-activated rather than only once at load). Verified via a mocked-HTTP unit test for the download logic (no real GitHub Release exists yet - no tag has been pushed) and a real, no-mocks integration test that publishes WitcherScriptMerger.Headless with release.yml's exact profile invocation and proves WSM_* env vars override a deliberately-wrong scratch XML config in a real spawned MCP process. Ran /code-review before finalizing; fixed 13 of 15 findings for real (redirect handling, request timeout, write-stream close-vs-finish, stale-install wipe on re-acquire, repo-aware idempotency, correct discoveryByGame selector, ENOENT-only error swallowing, concurrent-call coalescing, silent dispatch no-op) - see PR description for full detail and the two findings deliberately left as-is. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXAuGMLB44T5Zv5o5ZzKah
README's test:integration paragraph still described only the pre-existing dotnet build path; it now also documents toolAcquisition.integration.test.ts's dotnet publish -p:PublishProfile=win-x64 invocation (slower, produces a self-contained single-file exe) so a contributor isn't surprised by it on a cold checkout. archiveExtractor.ts's verify:true comment overstated what's actually known about Vortex's archive-handler behavior; softened to match the file's own "unverified" disclosure. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXAuGMLB44T5Zv5o5ZzKah
8 tasks
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 F: tool acquisition & configuration for the Vortex companion extension. Builds on
Unit E's scaffold (
src/mcpClient.ts,src/gating.ts,src/index.ts) to add the piecethat makes the extension actually usable: acquiring a WSM binary and telling Vortex
about it as a discovered tool, configured entirely via the
WSM_<KeyName>environment-variable mechanism (
WitcherScriptMerger.Core/AppSettings.cs) rather thanby editing
.exe.config/.dll.configXML.New files under
vortex-extension/src/:storage.ts- extension-private storage layout under Vortex'suserDatadirectory.wsmEnv.ts- buildsWSM_<KeyName>env-var overrides; the single mechanism both theMCP spawn path (
mcpClient.ts) and a future one-shotmergeCLI invocation pathshould use.
githubRelease.ts- GitHub Releases download logic (asset resolution + download),parameterized on repo/tag, real logic behind an injectable
HttpClientseam.archiveExtractor.ts- wraps Vortex's ownapi.openArchive/extractAllfor zipextraction (no hand-rolled parser, no new npm dependency).
discoveredTool.ts- builds and registers theIDiscoveredToolviaactions.addDiscoveredTool.toolAcquisition.ts- orchestrates download -> verify -> extract -> register(
acquireWsmTool) and a network-free local re-registration path(
ensureWsmToolRegistered), wired intoindex.ts'scontext.once.Ran
/code-reviewagainst the diff before finalizing; 15 findings came back, and thisPR fixes 13 of them for real (not just acknowledged) - see "Fixes from code review"
below for the two deliberately left as-is and why.
Tool ID choice
WitcherScriptMergerEnhanced- deliberately distinct from Vortex's own built-ingame-witcher3extension'sW3ScriptMerger(confirmed via direct source review,documented in
docs/vortex-extension-design.mdsection 0). There is no Vortex API tohide/disable another extension's tool registration, so this ships as a clearly-labeled
alternative alongside it, not a replacement - both tools will show up in Vortex's Tools
dashboard.
Env-var configuration convention
wsmEnv.ts'sbuildWsmEnv(config)maps{gameDirectory, modsDirectory, mergedModName, quickBmsPath, quickBmsPluginPath, wccLitePath}toWSM_GameDirectory,WSM_ModsDirectory, etc. - exactly theWSM_<KeyName>prefixAppSettings.EnvironmentVariablePrefixexpects.mergeWithProcessEnv(overrides)spreadsthese on top of
process.envforchild_process.spawn'senvoption (which replacesthe child's entire environment when set, rather than augmenting it). This is meant to be
the single source of truth for both the MCP spawn path (
mcpClient.ts'sWsmMcpClientOptions.env, demonstrated end-to-end below) and the not-yet-built one-shotmergeCLI invocation path (a later "merge panel" unit) - neither should duplicate theWSM_prefix mapping independently.Never reads or writes
WitcherScriptMerger.exe.config/WitcherScriptMerger.Headless.dll.configanywhere in this unit's code.
QuickBMS/wcc_lite storage convention (for the future bundle-tooling unit)
storage.ts'sgetBundleToolsDir(api)returns<userData>/witcherscriptmerger-vortex/bundle-tools/- exported now, unused by anythingin this unit (bundle-tooling acquisition hasn't landed), specifically so that later unit
doesn't have to re-derive where this extension keeps its own files. The convention: once
that unit exists, QuickBMS (
quickbms.exe+witcher3.bms) andwcc_lite.exeshould beacquired into subdirectories under this path, and
WSM_QuickBmsPath/WSM_QuickBmsPluginPath/WSM_WccLitePath(already accepted bywsmEnv.ts'sWsmEnvConfig, just unpopulated by anything yet) should point at the resulting filesinside it.
Full storage layout (
storage.ts):What was/wasn't verified against a real GitHub Release
Not verified: the actual GitHub-Releases download path end-to-end against a real
release (
githubRelease.ts'snodeHttpsClientimplementation - the redirect-following,timeout, and stream-lifecycle logic in particular). No version tag has been pushed to
this repo, so no GitHub Release exists yet - that's a deliberate, separate decision for
the repo owner to make later, not part of this unit. That logic is code-reviewed and
fixed against Node's own documented HTTP/stream semantics (see "Fixes from code review"),
not verified via a live socket test - a real HTTPS test server was judged not worth the
added complexity for this unit given no real release exists yet to validate against
regardless.
Verified:
githubRelease.test.ts- the download/asset-resolution orchestration logic againsta mocked
HttpClient(no real network calls anywhere in this repo's tests): correctrelease-by-tag URL construction, correct asset selected by exact name match
(matching
release.yml'sWitcherScriptMerger.Headless-<version>-win-x64.zipnaming), clear errors when the release/asset isn't found, the download-size integrity
check (downloaded byte count vs. GitHub's reported asset
size- the only integritycheck available, since
release.ymlpublishes no checksum manifest), and that acorrupt/truncated download is deleted rather than left behind.
test/toolAcquisition.integration.test.ts- a real, no-mocks integration test:WitcherScriptMerger.Headlesswith the exact profile invocationrelease.ymlitself uses (dotnet publish ... -c Release -p:PublishProfile=win-x64- self-contained, single-file), standing in for "thedownloaded-and-extracted binary."
acquireWsmToolwould have (same directory,same
installed-version.txtmarker format).ensureWsmToolRegisteredfor real and asserts the dispatchedaddDiscoveredToolaction's shape (game ID, tool ID, path)..dll.configwith deliberately wrongplaceholder
ModsDirectory/MergedModNamevalues, then spawns the real exe viaWsmMcpClient.connect({ exePath, env: mergeWithProcessEnv(buildWsmEnv(...)) })andasserts a real
get_statusMCP call reports back the env-var values, explicitlyasserting they are not the XML's values - not just "some value came back", but
specifically that the env var won over an explicit, non-blank config value.
writing the test) that this exact scenario works, including surfacing a pre-existing,
out-of-scope minor wart: WSM's headless MCP server writes one non-JSON diagnostic
line (
[WSM] Can't find any mods in the Mods directory.) directly to stdout whenthe mods directory is empty, technically violating "stdout carries protocol frames
only" - harmless here since
mcpClient.ts's line parser already defensivelyignores non-JSON lines, but worth flagging for whoever owns
WsmMcpTools.cs/CoreConsole output next.
archiveExtractor.ts's real implementation (Vortex's ownapi.openArchive/extractAll) is not exercised by any test - doing so would need a real Vortex hostproviding a real archive-handler extension, which nothing in this repo's test setup can
provide. It's tested only at the seam (
archiveExtractor.test.tswith a fakeapi),matching how
mcpClient.ts's own MCP-frame-shape assumptions were flagged as"genuinely unverified until exercised" in Unit E.
Fixes from code review
Ran
/code-reviewbefore finalizing. Fixed for real (not just noted):index.tsonly checkedisWitcher3Activeonce, atcontext.oncetime -contradicted this file's own doc comment about live game-mode switches. Fixed:
re-checks on every
'gamemode-activated'event too (confirmed real via@nexusmods/vortex-api's own README), proven by a newindex.test.ts(this unit'sfirst test for
index.ts- it had no real logic to test before).exception, not a Promise rejection (the callback runs outside
new Promise's ownsynchronous try/catch). Fixed: wrapped in try/catch, rejects properly.
acquireWsmToolforever. Fixed: 30s timeout viahttps.get's owntimeoutoptionplus a
'timeout'handler that destroys the request.'finish'instead of'close'-'finish'doesn't guarantee the OS file handle is actually released yet (Node's own docs),
risking a Windows sharing violation when
archiveExtractor.tsimmediately re-opensthe same file for extraction. Fixed.
pipe()doesn't auto-cascade destructionbetween source/destination. Fixed: both streams explicitly destroyed on either's
error.
failed. Fixed: best-effort unlink before throwing.
.zipwas never deleted from the cache after a successfulextraction, despite
storage.ts's own doc comment calling that directorydisposable. Fixed.
it first, so stale files from a prior version (or a prior failed/partial extraction)
could persist indefinitely - contradicted
storage.ts's own "overwrites" doc comment.Fixed:
installDiris wiped (fs.rm(..., {recursive:true, force:true})) immediatelybefore extraction. If a WSM process is actively running out of that directory, this
now fails loudly (Windows won't delete a running exe's backing file) instead of
silently corrupting a running install.
repo, so requesting the same version from adifferent repo silently reused the old repo's binary. Fixed: the installed-version
marker now records
<repo>@<version>, and both must match to skip re-acquisition.registerAcquiredToolderivedWSM_GameDirectoryfromselectors.currentGameDiscovery(whichever game happens to be active right now)even though the tool is always registered under a hardcoded
WITCHER3_GAME_ID.Fixed: uses
selectors.discoveryByGame(state, WITCHER3_GAME_ID)instead - Witcher 3'sown discovery, not whatever's active when an async call happens to resolve.
pathExists/readInstalledVersionswallowed every filesystem error, not just"doesn't exist" - a permission error or locked file would silently read as "nothing
installed," triggering a doomed re-download instead of surfacing the real problem.
Fixed: only
ENOENTis treated as "not found"; everything else propagates. Covered bya new test that injects an
EACCESerror viavi.spyOn(fs.promises, 'access').acquireWsmToolcalls for the same install(e.g. a double-clicked "Get/Update" action) - both would race the same download/extract
target. Fixed: concurrent calls for the same
installDirnow coalesce onto thefirst call's in-flight promise (documented limitation: a second call requesting
different
version/repowhile the first is in flight silently gets the firstcall's result - acceptable for this unit's only real trigger shape; see
acquireWsmTool's own doc comment).registerWsmDiscoveredToolsilently no-op'd ifapi.storewas undefined(
api.store?.dispatch(...)), while callers still reported success. Fixed: throwsinstead, so
ensureWsmToolRegistered/acquireWsmToolcan't reporttruewhennothing was actually dispatched.
util.writeFileAtomic(a real@nexusmods/vortex-apiexport, confirmedin its typings) for the installed-version marker write specifically - the one write
where a crash mid-write producing a corrupt/truncated marker would actually matter
for correctness.
Deliberately left as-is (both noted in code comments rather than silently dropped):
vortex-api'sfsnamespace (ensureDirAsyncetc.) throughout, insteadof plain
fs.promises- only partially adopted (seewriteFileAtomicabove).githubRelease.tsintentionally has zerovortex-apidependency for testability(confirmed: its unit tests run without needing the
vortex-apistub at all); thereviewer's own note confirms its hand-rolled HTTPS/JSON client isn't actually
redundant with
vortex-api's request helpers either way. The remaining plainfs.promises.mkdir/rm/unlinkcalls intoolAcquisition.tsdon't carry the sameatomicity hazard the marker-file write did (idempotent/best-effort operations), so
switching them was judged not worth the added coupling for this unit.
mkdirbetweentoolAcquisition.tsandarchiveExtractor.ts- nolonger purely redundant now that
installDiris wiped before extraction (see above):toolAcquisition.ts'smkdirrecreates a fresh directory right after the wipe;archiveExtractor.ts's ownmkdiris a generalArchiveExtractorinterfacecontract, independent of what a given caller already did. Left as two call sites each
owning their own precondition, with a comment explaining why.
Other design notes
WitcherScriptMerger.Headless-<version>-win-x64.zip),not the WinForms host's -
WitcherScriptMerger.Headless/CLAUDE.md's "Dependencygating" section: the WinForms host's
mergeandmcpverbs both gate on thecombined
ValidateDependencyPaths()(QuickBMS + wcc_lite), so it refuses to evenstart without bundle tooling this unit doesn't acquire. Headless gates on
ValidateTextMergeDependencies()only, so it works for flat-file conflicts withnothing else installed.
index.ts'scontext.oncecallsensureWsmToolRegistered(network-free) on load andon every
'gamemode-activated'event, notacquireWsmTool(the actual download) - aneager background download at every Vortex startup/game switch would be a guaranteed,
noisy failure while no release exists.
acquireWsmToolis exported for a later unit'sown explicit-user-action UI trigger (e.g. a "Get WitcherScriptMerger" button) to call
instead - not added here, since it's unverifiable without a running Vortex host and not
asked for by this unit's scope.
WitcherScriptMerger.Headless.exewith no argumentsprints usage and exits 1, so manually double-clicking this tool's tile in Vortex's
Tools dashboard does nothing useful today.
discoveredTool.tsdeliberately sets nodefault
parametersrather than picking an arbitrary verb that would be equallyunhelpful for a human - every actual invocation (this unit's integration test,
mcpClient.ts, a future CLI-invocation unit) always passes its own explicit args.ITool.executableis typed as a function, which cannotsurvive Vortex's Redux-state persistence to disk - the same shape
game-witcher3'sown
W3ScriptMergerregistration uses in production, not a new risk this unitintroduces.
discoveredTool.test.tsasserts every other field round-trips throughJSON.parse(JSON.stringify(...))correctly, andensureWsmToolRegisteredre-registerson every load/game-switch regardless, which papers over this if persistence does drop
it.
webpack.config.cjs'snodeBuiltinsexternals list gainedhttps/http/stream(for
githubRelease.ts's download logic).test/testUtils/vortexApiStub.tsgainedactions.addDiscoveredTool,selectors.discoveryByGame,util.writeFileAtomic, andlog- real (if simplified)fakes, since production code now calls each of them as values at runtime, unlike the
typesnamespace (compile-time only, elided from emitted JS, so it's never needed inthis stub - see
gating.test.ts's existing comment on why).Verification
npm run build(typecheck + webpack) - passes.npm run lint- clean.npm test(45 unit tests across 8 files, all fast/Node-only, no network) - passes.npm run test:integration(adds 4 real, no-mocks integration tests, 49 total) -passes.
dotnet build WitcherScriptMerger.slnanddotnet format whitespace WitcherScriptMerger.sln --verify-no-changesat the repo root - both unaffected,confirming
vortex-extension/stays fully outside the .NET solution's reach.AI-assisted development: this PR was substantially produced by Claude Code, per this
repo's
CONTRIBUTING.mddisclosure requirement.