From 090b532ce42354b61c32144de22e4758fed71955 Mon Sep 17 00:00:00 2001 From: Chris Knight Date: Sun, 9 Aug 2026 23:29:14 -0400 Subject: [PATCH 1/2] Add Vortex extension tool acquisition and env-var configuration (Unit F) 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_ 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 Claude-Session: https://claude.ai/code/session_01GXAuGMLB44T5Zv5o5ZzKah --- vortex-extension/README.md | 24 +- vortex-extension/package-lock.json | 2 + vortex-extension/src/archiveExtractor.test.ts | 56 ++++ vortex-extension/src/archiveExtractor.ts | 38 +++ vortex-extension/src/discoveredTool.test.ts | 79 ++++++ vortex-extension/src/discoveredTool.ts | 99 +++++++ vortex-extension/src/githubRelease.test.ts | 161 +++++++++++ vortex-extension/src/githubRelease.ts | 267 ++++++++++++++++++ vortex-extension/src/index.test.ts | 118 ++++++++ vortex-extension/src/index.ts | 59 +++- vortex-extension/src/storage.test.ts | 31 ++ vortex-extension/src/storage.ts | 61 ++++ vortex-extension/src/toolAcquisition.test.ts | 249 ++++++++++++++++ vortex-extension/src/toolAcquisition.ts | 236 ++++++++++++++++ vortex-extension/src/wsmEnv.test.ts | 68 +++++ vortex-extension/src/wsmEnv.ts | 77 +++++ .../test/testUtils/vortexApiStub.ts | 53 +++- .../test/toolAcquisition.integration.test.ts | 204 +++++++++++++ vortex-extension/webpack.config.cjs | 5 +- 19 files changed, 1870 insertions(+), 17 deletions(-) create mode 100644 vortex-extension/src/archiveExtractor.test.ts create mode 100644 vortex-extension/src/archiveExtractor.ts create mode 100644 vortex-extension/src/discoveredTool.test.ts create mode 100644 vortex-extension/src/discoveredTool.ts create mode 100644 vortex-extension/src/githubRelease.test.ts create mode 100644 vortex-extension/src/githubRelease.ts create mode 100644 vortex-extension/src/index.test.ts create mode 100644 vortex-extension/src/storage.test.ts create mode 100644 vortex-extension/src/storage.ts create mode 100644 vortex-extension/src/toolAcquisition.test.ts create mode 100644 vortex-extension/src/toolAcquisition.ts create mode 100644 vortex-extension/src/wsmEnv.test.ts create mode 100644 vortex-extension/src/wsmEnv.ts create mode 100644 vortex-extension/test/toolAcquisition.integration.test.ts diff --git a/vortex-extension/README.md b/vortex-extension/README.md index 8187700..72f959a 100644 --- a/vortex-extension/README.md +++ b/vortex-extension/README.md @@ -33,9 +33,21 @@ this extension's own TypeScript. ## Status -This is the foundation scaffold (info.json manifest, build tooling, the `init(context)` -entry point with only game-activity gating wired up, and the shared MCP stdio client in -`src/mcpClient.ts`). No actual features - tool acquisition, conflict scanning, the merge -panel, dashlets - are implemented here; those are separate, later units built on top of -this scaffold. See `docs/vortex-extension-design.md` (once merged) for the fuller design -context this scaffold follows. +The foundation scaffold (info.json manifest, build tooling, the `init(context)` entry +point, and the shared MCP stdio client in `src/mcpClient.ts`) is in place, plus one real +feature: **tool acquisition**. `src/toolAcquisition.ts` downloads a WSM release build +from GitHub Releases, verifies/extracts it, and registers it as a discovered Vortex tool +(`src/discoveredTool.ts`, tool ID `WitcherScriptMergerEnhanced` - distinct from Vortex's +own built-in `game-witcher3` extension's `W3ScriptMerger`). `src/wsmEnv.ts` builds the +`WSM_` environment-variable overrides (see +`WitcherScriptMerger.Core/AppSettings.cs`) used to configure a spawned WSM process - +never by editing its `.exe.config`/`.dll.config` XML. **The actual GitHub-Releases +download path is unverified against a real release** - no version tag has been pushed to +this repo yet, so no release exists; see `src/githubRelease.ts`'s own doc comment and +this feature's own PR description for exactly what was verified instead (a mocked-HTTP +unit test for the download logic, plus a full acquisition/registration/env-var-config +integration test using a locally-built binary standing in for a downloaded one). + +Conflict scanning, the merge panel, and dashlets are separate, later units not yet built +on top of this scaffold. See `docs/vortex-extension-design.md` for the fuller design +context this scaffold and the tool-acquisition unit follow. diff --git a/vortex-extension/package-lock.json b/vortex-extension/package-lock.json index 82a5c1f..d9b379d 100644 --- a/vortex-extension/package-lock.json +++ b/vortex-extension/package-lock.json @@ -4562,6 +4562,7 @@ "integrity": "sha512-o5a9xKjbtuhY6Bi5S3+HvbRERmouabWbyUcpXXUA1u+GNUKoROi9byOJ8M0nHbHYHkYICiMlqxkg1KkYmm25Sw==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "esbuild": "^0.21.3", "postcss": "^8.4.43", @@ -4789,6 +4790,7 @@ "integrity": "sha512-pIDJHIEI9LR0yxHXQ+Qh95k2EvXpWzZ5l+d+jIo+RdSm9MiHfzazIxwwni/p7+x4eJZuvG1AJwgC4TNQ7NRgsg==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@discoveryjs/json-ext": "^0.5.0", "@webpack-cli/configtest": "^2.1.1", diff --git a/vortex-extension/src/archiveExtractor.test.ts b/vortex-extension/src/archiveExtractor.test.ts new file mode 100644 index 0000000..4575408 --- /dev/null +++ b/vortex-extension/src/archiveExtractor.test.ts @@ -0,0 +1,56 @@ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { createVortexArchiveExtractor } from './archiveExtractor'; + +// This only tests createVortexArchiveExtractor's own glue (destDir creation, delegating +// to api.openArchive/archive.extractAll, error handling when extractAll is missing) - +// the real archive-handler behavior behind api.openArchive is Vortex's own, and isn't +// something this repo can exercise without a real Vortex host (see archiveExtractor.ts's +// own doc comment). +function fakeApi(openArchive: (archivePath: string, options?: unknown) => Promise<{ extractAll?: (dest: string) => Promise }>) { + return { openArchive } as unknown as Parameters[0]; +} + +describe('createVortexArchiveExtractor', () => { + let scratchDir: string; + + beforeEach(() => { + scratchDir = fs.mkdtempSync(path.join(os.tmpdir(), 'wsm-vortex-extract-test-')); + }); + + afterEach(() => { + fs.rmSync(scratchDir, { recursive: true, force: true }); + }); + + it('creates destDir, opens the archive, and calls extractAll(destDir)', async () => { + const destDir = path.join(scratchDir, 'nested', 'destination'); + const extractAllCalls: string[] = []; + let openArchiveCall: { archivePath: string; options: unknown } | undefined; + + const extractor = createVortexArchiveExtractor( + fakeApi(async (archivePath, options) => { + openArchiveCall = { archivePath, options }; + return { + extractAll: async (dest: string) => { + extractAllCalls.push(dest); + }, + }; + }), + ); + + await extractor.extractAll('C:\\fake\\asset.zip', destDir); + + expect(fs.existsSync(destDir)).toBe(true); + expect(openArchiveCall?.archivePath).toBe('C:\\fake\\asset.zip'); + expect(extractAllCalls).toEqual([destDir]); + }); + + it('throws a clear error when the opened archive has no extractAll', async () => { + const destDir = path.join(scratchDir, 'destination'); + const extractor = createVortexArchiveExtractor(fakeApi(async () => ({}))); + + await expect(extractor.extractAll('C:\\fake\\asset.zip', destDir)).rejects.toThrow(/does not support/); + }); +}); diff --git a/vortex-extension/src/archiveExtractor.ts b/vortex-extension/src/archiveExtractor.ts new file mode 100644 index 0000000..d004c06 --- /dev/null +++ b/vortex-extension/src/archiveExtractor.ts @@ -0,0 +1,38 @@ +import * as fs from 'fs'; +import { types } from 'vortex-api'; + +/** + * Extracts a downloaded WSM release archive. Deliberately goes through Vortex's own + * `api.openArchive`/`Archive.extractAll` (backed by the 7-zip archive handler Vortex + * itself ships and uses for every mod archive it installs - `@nexusmods/vortex-api`'s + * own `lib/api.d.ts` documents `openArchive(archivePath, options?, extension?): + * Promise` with an `extractAll(outputPath): Promise` member) rather than + * a hand-rolled zip reader or a new npm dependency - this is the idiomatic mechanism a + * Vortex extension already has for exactly this job, and it's what `IArchiveOptions` + * exists for. + * + * Behind a one-function interface (`ArchiveExtractor`) so `toolAcquisition.ts` stays + * unit-testable with extraction stubbed - this real implementation is never exercised + * by any test in this repo, since doing so would need a real Vortex host providing a + * real archive-handler extension (`api.openArchive` has no meaningful behavior outside + * one). See this unit's PR description for what was/wasn't verified. + */ +export interface ArchiveExtractor { + extractAll(archivePath: string, destDir: string): Promise; +} + +export function createVortexArchiveExtractor(api: types.IExtensionApi): ArchiveExtractor { + return { + async extractAll(archivePath: string, destDir: string): Promise { + await fs.promises.mkdir(destDir, { recursive: true }); + + const archive = await api.openArchive(archivePath, { verify: true }); + if (!archive.extractAll) { + throw new Error( + `Vortex's archive handler for '${archivePath}' does not support extracting the whole archive (extractAll is undefined).`, + ); + } + await archive.extractAll(destDir); + }, + }; +} diff --git a/vortex-extension/src/discoveredTool.test.ts b/vortex-extension/src/discoveredTool.test.ts new file mode 100644 index 0000000..ea40004 --- /dev/null +++ b/vortex-extension/src/discoveredTool.test.ts @@ -0,0 +1,79 @@ +import * as path from 'path'; +import { describe, expect, it, vi } from 'vitest'; +import { WITCHER3_GAME_ID } from './gating'; +import { buildWsmDiscoveredTool, registerWsmDiscoveredTool, WSM_TOOL_ID } from './discoveredTool'; + +const EXE_PATH = path.join('C:', 'fake', 'tool', 'WitcherScriptMerger.Headless.exe'); + +describe('WSM_TOOL_ID', () => { + it('does not collide with game-witcher3\'s own built-in tool ID', () => { + // See docs/vortex-extension-design.md section 0: game-witcher3 already registers + // 'W3ScriptMerger'. There is no API to hide/replace another extension's tool + // registration, so this must be a distinct ID. + expect(WSM_TOOL_ID).not.toBe('W3ScriptMerger'); + }); +}); + +describe('buildWsmDiscoveredTool', () => { + it('builds a tool pointing at the given exe path, custom and visible', () => { + const tool = buildWsmDiscoveredTool({ exePath: EXE_PATH }); + + expect(tool.id).toBe(WSM_TOOL_ID); + expect(tool.path).toBe(EXE_PATH); + expect(tool.custom).toBe(true); + expect(tool.hidden).toBe(false); + expect(tool.requiredFiles).toEqual([]); + expect(tool.workingDirectory).toBe(path.dirname(EXE_PATH)); + expect(tool.executable()).toBe('WitcherScriptMerger.Headless.exe'); + }); + + it('defaults environment to an empty object when none is given', () => { + const tool = buildWsmDiscoveredTool({ exePath: EXE_PATH }); + expect(tool.environment).toEqual({}); + }); + + it('carries through a supplied environment map unchanged', () => { + const tool = buildWsmDiscoveredTool({ exePath: EXE_PATH, environment: { WSM_ModsDirectory: 'C:\\Mods' } }); + expect(tool.environment).toEqual({ WSM_ModsDirectory: 'C:\\Mods' }); + }); + + it('round-trips every field except the known-non-serializable executable function through JSON', () => { + // ITool.executable is typed as a function - it cannot survive JSON.stringify, a + // known, documented, unavoidable limitation shared with game-witcher3's own + // W3ScriptMerger registration (see this module's own doc comment). Every other + // field must survive, since Vortex persists discovered-tools state to disk. + const tool = buildWsmDiscoveredTool({ exePath: EXE_PATH, environment: { WSM_ModsDirectory: 'C:\\Mods' } }); + const roundTripped = JSON.parse(JSON.stringify(tool)) as Record; + + const expectedWithoutExecutable: Record = { ...tool }; + delete expectedWithoutExecutable.executable; + expect(roundTripped).toEqual(expectedWithoutExecutable); + }); +}); + +describe('registerWsmDiscoveredTool', () => { + it('dispatches addDiscoveredTool for witcher3 with the given tool, marked custom/manual', () => { + const dispatch = vi.fn(); + const api = { store: { dispatch } } as unknown as Parameters[0]; + const tool = buildWsmDiscoveredTool({ exePath: EXE_PATH }); + + registerWsmDiscoveredTool(api, tool); + + expect(dispatch).toHaveBeenCalledTimes(1); + const dispatchedAction = dispatch.mock.calls[0][0] as { payload: { gameId: string; toolId: string; result: unknown; manual: boolean } }; + expect(dispatchedAction.payload.gameId).toBe(WITCHER3_GAME_ID); + expect(dispatchedAction.payload.toolId).toBe(WSM_TOOL_ID); + expect(dispatchedAction.payload.result).toBe(tool); + expect(dispatchedAction.payload.manual).toBe(true); + }); + + it('throws rather than silently no-oping when api.store is unavailable', () => { + // Callers (toolAcquisition.ts's ensureWsmToolRegistered/acquireWsmTool) treat this + // function completing without throwing as proof the tool was actually registered - + // a silent no-op here would make them report success with nothing really dispatched. + const api = { store: undefined } as unknown as Parameters[0]; + const tool = buildWsmDiscoveredTool({ exePath: EXE_PATH }); + + expect(() => registerWsmDiscoveredTool(api, tool)).toThrow(/store is unavailable/); + }); +}); diff --git a/vortex-extension/src/discoveredTool.ts b/vortex-extension/src/discoveredTool.ts new file mode 100644 index 0000000..deba641 --- /dev/null +++ b/vortex-extension/src/discoveredTool.ts @@ -0,0 +1,99 @@ +import * as path from 'path'; +import { actions, types } from 'vortex-api'; +import { WITCHER3_GAME_ID } from './gating'; + +/** + * There is no `context.registerTool` API in `vortex-api` (re-confirmed against + * `lib/api.d.ts` - `docs/vortex-extension-design.md` section 1) - tool discovery is + * always `actions.addDiscoveredTool(gameId, toolId, toolDetails, isCustom)`, dispatched + * via `api.store.dispatch(...)`, exactly like Vortex's own built-in `game-witcher3` + * extension already registers its `W3ScriptMerger` tool (`docs/vortex-extension-design.md` + * section 0). + * + * Deliberately a **different** tool ID from that one - `W3ScriptMerger` belongs to + * `game-witcher3`, a separate extension this one is a companion to, not a replacement + * of (see `gating.ts`'s own doc comment). There is no API to hide/disable another + * extension's existing tool registration, so both tools coexist in Vortex's Tools + * dashboard: `game-witcher3`'s `W3ScriptMerger` (which downloads and launches the GUI of + * a different, older WSM fork - `IDCs/WitcherScriptMerger` - per the design doc's + * research) and this one, clearly and distinctly labeled, pointing at a build of *this* + * repo instead. + */ +export const WSM_TOOL_ID = 'WitcherScriptMergerEnhanced'; + +export interface WsmDiscoveredToolOptions { + /** Absolute path to the acquired `WitcherScriptMerger.Headless.exe`. */ + exePath: string; + /** + * `WSM_` environment-variable overrides (see `wsmEnv.ts`'s `buildWsmEnv`) to + * attach to this tool's registration, applied by Vortex if the user launches it + * manually from the Tools dashboard. This is a secondary use of `buildWsmEnv`'s + * output - the primary one, per this unit's own instructions, is passing it straight + * into a spawned child process's `env` (`mcpClient.ts`'s `WsmMcpClientOptions.env`, + * demonstrated in `test/toolAcquisition.integration.test.ts`), not this static field. + */ + environment?: Record; +} + +/** + * Builds the `IDiscoveredTool` object `registerWsmDiscoveredTool` dispatches. + * + * **Known, unavoidable serialization caveat, not unique to this extension:** + * `ITool.executable` (which `IDiscoveredTool` inherits) is typed as a function + * (`(discoveredPath?: string) => string`), and Vortex's discovered-tools state is + * ordinarily persisted to disk across restarts. A function cannot survive a + * `JSON.stringify` round-trip - the same shape `game-witcher3`'s own `scriptmerger.ts` + * uses in production for `W3ScriptMerger` (per the design doc's direct source review), + * so this isn't a novel risk this unit introduces, just an inherited one. Untested here + * against real Vortex persistence (no real Vortex host in this repo's test setup) - + * `discoveredTool.test.ts` instead asserts every *other* field round-trips through + * `JSON.parse(JSON.stringify(...))` correctly, and re-registration happens on every + * `index.ts` `context.once` regardless (see that file), which would paper over a stale + * persisted `executable` field even if persistence does drop it. + */ +export function buildWsmDiscoveredTool(options: WsmDiscoveredToolOptions): types.IDiscoveredTool { + return { + id: WSM_TOOL_ID, + name: 'WitcherScriptMerger (Enhanced)', + shortName: 'WSM+', + // We already know the exact acquired path - no on-disk discovery scan needed, so + // requiredFiles (which drives that scan) is deliberately empty. + requiredFiles: [], + executable: () => path.basename(options.exePath), + // No default `parameters`: WitcherScriptMerger.Headless.exe with no args prints + // usage and exits 1 rather than doing anything useful (see + // WitcherScriptMerger.Headless/CLAUDE.md's routing section) - there's no verb that's + // meaningfully "the default" for a human clicking this tile in Vortex's Tools + // dashboard (`mcp` mode just sits waiting for JSON-RPC on stdin, which looks hung to + // a human; `merge` needs GameDirectory/ModsDirectory already configured). Documented + // limitation, not an oversight - a later unit driving this programmatically + // (mcpClient.ts, or a future one-shot `merge` CLI invocation) always passes its own + // explicit `args`, bypassing this default entirely. + environment: options.environment ?? {}, + path: options.exePath, + hidden: false, + custom: true, + workingDirectory: path.dirname(options.exePath), + }; +} + +/** + * Dispatches `actions.addDiscoveredTool` for Witcher 3 specifically - this extension + * never registers a tool for any other game. + * + * `IExtensionApi.store` is typed optional, but this extension only ever calls this from + * inside `index.ts`'s `context.once` (or code reachable from it), by which point Vortex + * guarantees a real store exists - a missing store there would be a genuine, unexpected + * problem, not a normal condition to swallow. Throwing here (rather than the previous + * `api.store?.dispatch(...)`, a silent no-op) matters concretely: `ensureWsmToolRegistered`/ + * `acquireWsmTool` (`toolAcquisition.ts`) both treat this call completing without + * throwing as "the tool is now registered" and return `true` accordingly - a swallowed + * no-op here would make both of those report success while nothing was actually + * dispatched to the Redux store. + */ +export function registerWsmDiscoveredTool(api: types.IExtensionApi, tool: types.IDiscoveredTool): void { + if (!api.store) { + throw new Error('Cannot register the WSM discovered tool: api.store is unavailable.'); + } + api.store.dispatch(actions.addDiscoveredTool(WITCHER3_GAME_ID, WSM_TOOL_ID, tool, true)); +} diff --git a/vortex-extension/src/githubRelease.test.ts b/vortex-extension/src/githubRelease.test.ts new file mode 100644 index 0000000..a481cdd --- /dev/null +++ b/vortex-extension/src/githubRelease.test.ts @@ -0,0 +1,161 @@ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { + buildAssetFileName, + downloadReleaseAsset, + HttpClient, + resolveReleaseAsset, +} from './githubRelease'; + +// No test in this file makes a real network call - every HttpClient here is a fake, per +// this unit's own verification requirement ("a unit test against a mocked HTTP response +// for the download logic itself"). githubRelease.ts's real `nodeHttpsClient` +// implementation is only exercised (against a locally-built stand-in, never a real +// GitHub Release) by test/toolAcquisition.integration.test.ts's setup, not here. + +function fakeClient(overrides: Partial = {}): HttpClient { + return { + getJson: async () => { + throw new Error('getJson was not expected to be called in this test'); + }, + downloadToFile: async () => { + throw new Error('downloadToFile was not expected to be called in this test'); + }, + ...overrides, + }; +} + +describe('buildAssetFileName', () => { + it('matches release.yml\'s package-release job naming exactly', () => { + expect(buildAssetFileName('0.6.2')).toBe('WitcherScriptMerger.Headless-0.6.2-win-x64.zip'); + }); +}); + +describe('resolveReleaseAsset', () => { + it('fetches the release-by-tag URL and returns the matching asset\'s download URL and size', async () => { + const requestedUrls: string[] = []; + const client = fakeClient({ + getJson: async (url: string) => { + requestedUrls.push(url); + return { + tag_name: 'v0.6.2', + assets: [ + { name: 'WitcherScriptMerger-0.6.2-win-x64.zip', browser_download_url: 'https://example.invalid/gui.zip', size: 111 }, + { + name: 'WitcherScriptMerger.Headless-0.6.2-win-x64.zip', + browser_download_url: 'https://example.invalid/headless-win.zip', + size: 222, + }, + { + name: 'WitcherScriptMerger.Headless-0.6.2-linux-x64.tar.gz', + browser_download_url: 'https://example.invalid/headless-linux.tar.gz', + size: 333, + }, + ], + }; + }, + }); + + const result = await resolveReleaseAsset({ + repo: 'TheValiantOne/WitcherScriptMerger', + tag: 'v0.6.2', + assetFileName: 'WitcherScriptMerger.Headless-0.6.2-win-x64.zip', + client, + }); + + expect(requestedUrls).toEqual([ + 'https://api.github.com/repos/TheValiantOne/WitcherScriptMerger/releases/tags/v0.6.2', + ]); + expect(result).toEqual({ downloadUrl: 'https://example.invalid/headless-win.zip', size: 222 }); + }); + + it('throws a clear error when the requested asset is not present on the release', async () => { + const client = fakeClient({ + getJson: async () => ({ tag_name: 'v0.6.2', assets: [{ name: 'something-else.zip', browser_download_url: 'x', size: 1 }] }), + }); + + await expect( + resolveReleaseAsset({ + repo: 'TheValiantOne/WitcherScriptMerger', + tag: 'v0.6.2', + assetFileName: 'WitcherScriptMerger.Headless-0.6.2-win-x64.zip', + client, + }), + ).rejects.toThrow(/no asset named 'WitcherScriptMerger\.Headless-0\.6\.2-win-x64\.zip'/); + }); + + it('wraps a failure fetching the release itself (e.g. tag not found) in a clear error', async () => { + const client = fakeClient({ + getJson: async () => { + throw new Error("GET '...' failed with HTTP 404: Not Found"); + }, + }); + + await expect( + resolveReleaseAsset({ + repo: 'TheValiantOne/WitcherScriptMerger', + tag: 'v9.9.9', + assetFileName: 'WitcherScriptMerger.Headless-9.9.9-win-x64.zip', + client, + }), + ).rejects.toThrow(/Could not fetch release 'v9\.9\.9'/); + }); +}); + +describe('downloadReleaseAsset', () => { + let scratchDir: string; + + beforeEach(() => { + scratchDir = fs.mkdtempSync(path.join(os.tmpdir(), 'wsm-vortex-download-test-')); + }); + + afterEach(() => { + fs.rmSync(scratchDir, { recursive: true, force: true }); + }); + + it('resolves when the downloaded byte count matches the expected size', async () => { + const destPath = path.join(scratchDir, 'asset.zip'); + const client = fakeClient({ + downloadToFile: async (_url: string, dest: string) => { + fs.writeFileSync(dest, Buffer.alloc(42, 1)); + return { bytesWritten: 42 }; + }, + }); + + await expect( + downloadReleaseAsset({ downloadUrl: 'https://example.invalid/asset.zip', destPath, expectedSize: 42, client }), + ).resolves.toBeUndefined(); + }); + + it('rejects with a clear error when the downloaded byte count does not match', async () => { + const destPath = path.join(scratchDir, 'asset.zip'); + const client = fakeClient({ + downloadToFile: async (_url: string, dest: string) => { + fs.writeFileSync(dest, Buffer.alloc(10, 1)); + return { bytesWritten: 10 }; + }, + }); + + await expect( + downloadReleaseAsset({ downloadUrl: 'https://example.invalid/asset.zip', destPath, expectedSize: 42, client }), + ).rejects.toThrow(/got 10 bytes, expected 42/); + }); + + it('deletes the truncated/corrupt file from destPath when the byte count does not match', async () => { + const destPath = path.join(scratchDir, 'asset.zip'); + const client = fakeClient({ + downloadToFile: async (_url: string, dest: string) => { + fs.writeFileSync(dest, Buffer.alloc(10, 1)); + return { bytesWritten: 10 }; + }, + }); + + await expect( + downloadReleaseAsset({ downloadUrl: 'https://example.invalid/asset.zip', destPath, expectedSize: 42, client }), + ).rejects.toThrow(); + + expect(fs.existsSync(destPath)).toBe(false); + }); +}); diff --git a/vortex-extension/src/githubRelease.ts b/vortex-extension/src/githubRelease.ts new file mode 100644 index 0000000..7d4421f --- /dev/null +++ b/vortex-extension/src/githubRelease.ts @@ -0,0 +1,267 @@ +import * as fs from 'fs'; +import * as https from 'https'; + +/** + * Download-from-GitHub-Releases logic for acquiring a WSM build. Parameterized on + * `repo`/`tag` (never hardcoded beyond `DEFAULT_WSM_REPO`'s default) so it's genuinely + * functional once a release actually exists - **as of this unit, no version tag has + * been pushed to this repo, so no GitHub Release exists yet and this code path has + * never been exercised against a real release; see this unit's PR description.** + * Verified here only via `githubRelease.test.ts`'s mocked `HttpClient` - no real network + * call is made by any test. + * + * Asset naming matches `.github/workflows/release.yml`'s `package-release` job exactly: + * `WitcherScriptMerger.Headless--win-x64.zip` (the CLI/MCP-only, no-GUI host - + * see `buildAssetFileName`'s own comment for why this asset, not the WinForms host's, + * is the one this extension wants). + */ + +export const DEFAULT_WSM_REPO = 'TheValiantOne/WitcherScriptMerger'; + +/** + * Windows-only for now, matching Vortex itself being Windows-only today (see + * `docs/vortex-extension-design.md`, Open Question 8) - not a hardcoded assumption + * baked in silently, just the only platform this extension can actually run on right + * now. A future Linux/SteamOS Vortex would need a second case here, not a rewrite. + */ +export type WsmAssetPlatform = 'win-x64'; + +/** + * The Headless host (CLI + MCP, no GUI) rather than the WinForms host's own win-x64 + * asset: `WitcherScriptMerger.Headless/CLAUDE.md`'s "Dependency gating" section - the + * WinForms host's `merge` *and* `mcp` verbs both gate on the combined + * `ValidateDependencyPaths()` (QuickBMS + wcc_lite), so it refuses to even start + * without bundle tooling this unit doesn't acquire. The Headless host gates on + * `ValidateTextMergeDependencies()` only, so it works for flat-file conflicts with + * nothing else installed - exactly what `mcpClient.ts` and a future one-shot `merge` + * CLI invocation both need. + */ +export function buildAssetFileName(version: string, platform: WsmAssetPlatform = 'win-x64'): string { + return `WitcherScriptMerger.Headless-${version}-${platform}.zip`; +} + +export interface GitHubReleaseAsset { + name: string; + browser_download_url: string; + size: number; +} + +interface GitHubReleaseResponse { + tag_name: string; + assets: GitHubReleaseAsset[]; +} + +/** + * Low-level HTTP transport, injectable for testing. `nodeHttpsClient` (below) is the + * real, default implementation; every test in `githubRelease.test.ts` supplies its own + * fake instead, so no test ever makes a real network call. + */ +export interface HttpClient { + /** GETs `url`, follows redirects, parses the body as JSON. Rejects on a non-2xx final + * status or invalid JSON. */ + getJson(url: string): Promise; + /** GETs `url`, follows redirects, streams the body to `destPath`. Rejects on a + * non-2xx final status or any I/O error; resolves with the number of bytes written. */ + downloadToFile(url: string, destPath: string): Promise<{ bytesWritten: number }>; +} + +const USER_AGENT = 'witcherscriptmerger-vortex'; +const MAX_REDIRECTS = 5; +/** Applies to each individual request in a redirect chain (connect-through-response-headers), + * not the whole chain/download - a slow-but-progressing large-asset download isn't cut off + * by this, only a connection that is accepted but never sends anything back at all. */ +const REQUEST_TIMEOUT_MS = 30_000; + +function requestFollowingRedirects( + url: string, + redirectsLeft: number, +): Promise<{ statusCode: number; response: import('http').IncomingMessage }> { + return new Promise((resolve, reject) => { + const req = https.get( + url, + { + headers: { 'User-Agent': USER_AGENT, Accept: 'application/vnd.github+json, application/octet-stream' }, + timeout: REQUEST_TIMEOUT_MS, + }, + (response) => { + // A throw in here would otherwise escape as an uncaught exception rather than a + // Promise rejection - this callback runs asynchronously, outside the executor's + // own synchronous try/catch, so `new Promise` cannot catch it for us. + try { + const statusCode = response.statusCode ?? 0; + + if (statusCode >= 300 && statusCode < 400 && response.headers.location) { + response.resume(); // discard this response's body before following the redirect + if (redirectsLeft <= 0) { + reject(new Error(`Too many redirects while fetching '${url}'.`)); + return; + } + resolve(requestFollowingRedirects(new URL(response.headers.location, url).toString(), redirectsLeft - 1)); + return; + } + + resolve({ statusCode, response }); + } catch (err) { + response.resume(); + reject(err); + } + }, + ); + req.on('error', reject); + // 'timeout' alone doesn't abort the request or reject anything by itself - per Node's + // own docs, it only fires; the caller is expected to act on it (here, destroying the + // request, which then emits 'error' and reaches the handler above). + req.on('timeout', () => { + req.destroy(new Error(`Request to '${url}' timed out after ${REQUEST_TIMEOUT_MS}ms.`)); + }); + }); +} + +async function nodeGetJson(url: string): Promise { + const { statusCode, response } = await requestFollowingRedirects(url, MAX_REDIRECTS); + + const chunks: Buffer[] = []; + for await (const chunk of response) { + chunks.push(chunk as Buffer); + } + const body = Buffer.concat(chunks).toString('utf8'); + + if (statusCode < 200 || statusCode >= 300) { + throw new Error(`GET '${url}' failed with HTTP ${statusCode}: ${body.slice(0, 500)}`); + } + + try { + return JSON.parse(body); + } catch (err) { + throw new Error( + `GET '${url}' returned a non-JSON body (HTTP ${statusCode}): ${err instanceof Error ? err.message : String(err)}`, + ); + } +} + +async function nodeDownloadToFile(url: string, destPath: string): Promise<{ bytesWritten: number }> { + const { statusCode, response } = await requestFollowingRedirects(url, MAX_REDIRECTS); + + if (statusCode < 200 || statusCode >= 300) { + response.resume(); + throw new Error(`Download of '${url}' failed with HTTP ${statusCode}.`); + } + + let bytesWritten = 0; + await new Promise((resolve, reject) => { + const out = fs.createWriteStream(destPath); + + // Node's pipe() does not auto-destroy the destination when the source errors (or vice + // versa) - without this, a failed download leaks an open file handle on `out` on top + // of leaving a partial file behind. + const onError = (err: Error) => { + out.destroy(); + response.destroy(); + reject(err); + }; + + response.on('data', (chunk: Buffer) => { + bytesWritten += chunk.length; + }); + response.on('error', onError); + out.on('error', onError); + // 'finish' fires once all data has been flushed to the stream's internal buffer, but + // does *not* guarantee the underlying file descriptor has actually been closed yet + // (per Node's own docs, 'close' is the event that guarantees that) - resolving on + // 'finish' left a real, if narrow, window where a caller that immediately re-opens + // destPath (e.g. archiveExtractor.ts extracting it right after) could race an + // OS-level handle that isn't released yet, particularly on Windows. + out.on('close', resolve); + response.pipe(out); + }); + + return { bytesWritten }; +} + +/** The real transport - Node's own `https`, no third-party HTTP dependency. */ +export const nodeHttpsClient: HttpClient = { + getJson: nodeGetJson, + downloadToFile: nodeDownloadToFile, +}; + +export interface ResolveReleaseAssetOptions { + /** `"owner/repo"`, e.g. `DEFAULT_WSM_REPO`. */ + repo: string; + /** A real release tag, e.g. `"v0.6.2"` (including the leading "v" - matches + * `.github/workflows/release.yml`'s own `push: tags: - 'v*'` trigger). */ + tag: string; + assetFileName: string; + client?: HttpClient; +} + +export interface ResolvedReleaseAsset { + downloadUrl: string; + size: number; +} + +/** + * Looks up a release by tag and finds the asset matching `assetFileName` by exact + * name. Throws a clear, specific error (naming the repo/tag/asset actually looked for) + * if the release or the asset within it doesn't exist - the expected outcome against + * this repo today, since no release has been tagged yet. + */ +export async function resolveReleaseAsset(options: ResolveReleaseAssetOptions): Promise { + const client = options.client ?? nodeHttpsClient; + const url = `https://api.github.com/repos/${options.repo}/releases/tags/${options.tag}`; + + let release: GitHubReleaseResponse; + try { + release = (await client.getJson(url)) as GitHubReleaseResponse; + } catch (err) { + throw new Error( + `Could not fetch release '${options.tag}' for '${options.repo}' (${url}): ${err instanceof Error ? err.message : String(err)}`, + ); + } + + const asset = (release.assets ?? []).find((a) => a.name === options.assetFileName); + if (!asset) { + const available = (release.assets ?? []).map((a) => a.name).join(', ') || '(none)'; + throw new Error( + `Release '${options.tag}' for '${options.repo}' has no asset named '${options.assetFileName}'. Available assets: ${available}.`, + ); + } + + return { downloadUrl: asset.browser_download_url, size: asset.size }; +} + +export interface DownloadReleaseAssetOptions { + downloadUrl: string; + destPath: string; + /** The size GitHub's API reported for this asset (`ResolvedReleaseAsset.size`). + * Compared against the actual downloaded byte count as this download's only + * integrity check - `release.yml` publishes no checksum manifest, so this is a + * transfer-completeness check, not a cryptographic verification. */ + expectedSize: number; + client?: HttpClient; +} + +/** + * Downloads a release asset to `destPath` and verifies the downloaded byte count + * matches the size GitHub's API reported for it. Throws if they don't match (a + * truncated/corrupted download) rather than silently handing a caller a bad file - and + * removes the bad file from `destPath` first (best-effort - a failure removing it is + * logged nowhere and swallowed, since the size-mismatch error is the one that actually + * matters to the caller), rather than leaving corrupt/truncated debris behind in what's + * meant to be a disposable download cache (`storage.ts`'s `getDownloadCacheDir`). + */ +export async function downloadReleaseAsset(options: DownloadReleaseAssetOptions): Promise { + const client = options.client ?? nodeHttpsClient; + const { bytesWritten } = await client.downloadToFile(options.downloadUrl, options.destPath); + + if (bytesWritten !== options.expectedSize) { + try { + await fs.promises.unlink(options.destPath); + } catch { + // Best-effort cleanup only - the error below is what actually matters here. + } + + throw new Error( + `Downloaded '${options.downloadUrl}' to '${options.destPath}' but got ${bytesWritten} bytes, expected ${options.expectedSize} (GitHub's reported asset size). The download may be incomplete or corrupted.`, + ); + } +} diff --git a/vortex-extension/src/index.test.ts b/vortex-extension/src/index.test.ts new file mode 100644 index 0000000..84f1454 --- /dev/null +++ b/vortex-extension/src/index.test.ts @@ -0,0 +1,118 @@ +import { describe, expect, it, vi } from 'vitest'; + +// `vi.mock` factories are hoisted above imports, so anything they reference has to come +// from `vi.hoisted` rather than an ordinary outer-scope `const` - this isolates index.ts's +// own wiring (what this file actually tests) from toolAcquisition.ts's real behavior +// (already thoroughly covered by toolAcquisition.test.ts). +const { ensureWsmToolRegisteredMock } = vi.hoisted(() => ({ + ensureWsmToolRegisteredMock: vi.fn(), +})); + +vi.mock('./toolAcquisition', () => ({ + ensureWsmToolRegistered: ensureWsmToolRegisteredMock, +})); + +import main from './index'; +import { WITCHER3_GAME_ID } from './gating'; + +/** A minimal stand-in for IExtensionContext - just enough surface for index.ts's own + * logic (context.once, context.api.getState/events.on), matching gating.test.ts's own + * fakeApi philosophy: a simplified fake, not a replica of Vortex's real context shape. */ +function fakeContext(initialActiveGameId: string | undefined) { + const state = { activeGameId: initialActiveGameId }; + let onceCallback: (() => void) | undefined; + const eventListeners = new Map void>>(); + + const context = { + once: (callback: () => void) => { + onceCallback = callback; + }, + api: { + getState: () => state, + events: { + on: (eventName: string, listener: () => void) => { + const listeners = eventListeners.get(eventName) ?? []; + listeners.push(listener); + eventListeners.set(eventName, listeners); + }, + }, + }, + }; + + return { + context: context as unknown as Parameters[0], + fireOnce: () => onceCallback?.(), + fireEvent: (eventName: string) => eventListeners.get(eventName)?.forEach((listener) => listener()), + setActiveGame: (gameId: string | undefined) => { + state.activeGameId = gameId; + }, + }; +} + +describe('main (index.ts)', () => { + it('returns true (Vortex extension init contract)', () => { + const { context } = fakeContext(undefined); + expect(main(context)).toBe(true); + }); + + it('calls ensureWsmToolRegistered when witcher3 is already active at context.once time', () => { + ensureWsmToolRegisteredMock.mockClear().mockResolvedValue(false); + const { context, fireOnce } = fakeContext(WITCHER3_GAME_ID); + + main(context); + fireOnce(); + + expect(ensureWsmToolRegisteredMock).toHaveBeenCalledTimes(1); + }); + + it('does not call ensureWsmToolRegistered when witcher3 is not active at context.once time', () => { + ensureWsmToolRegisteredMock.mockClear().mockResolvedValue(false); + const { context, fireOnce } = fakeContext('skyrimse'); + + main(context); + fireOnce(); + + expect(ensureWsmToolRegisteredMock).not.toHaveBeenCalled(); + }); + + it('re-checks on a live "gamemode-activated" switch into witcher3, without requiring a restart', () => { + // This is the fix for a real gap: previously, isWitcher3Active was only checked + // once inside context.once, so a user switching into Witcher 3 mid-session (no + // Vortex restart) would never trigger registration for the rest of that session. + ensureWsmToolRegisteredMock.mockClear().mockResolvedValue(false); + const { context, fireOnce, fireEvent, setActiveGame } = fakeContext('skyrimse'); + + main(context); + fireOnce(); + expect(ensureWsmToolRegisteredMock).not.toHaveBeenCalled(); + + setActiveGame(WITCHER3_GAME_ID); + fireEvent('gamemode-activated'); + + expect(ensureWsmToolRegisteredMock).toHaveBeenCalledTimes(1); + }); + + it('does nothing on "gamemode-activated" when the newly-active game still is not witcher3', () => { + ensureWsmToolRegisteredMock.mockClear().mockResolvedValue(false); + const { context, fireOnce, fireEvent, setActiveGame } = fakeContext('skyrimse'); + + main(context); + fireOnce(); + + setActiveGame('fallout4'); + fireEvent('gamemode-activated'); + + expect(ensureWsmToolRegisteredMock).not.toHaveBeenCalled(); + }); + + it('does not throw when ensureWsmToolRegistered rejects', async () => { + ensureWsmToolRegisteredMock.mockClear().mockRejectedValue(new Error('boom')); + const { context, fireOnce } = fakeContext(WITCHER3_GAME_ID); + + main(context); + expect(() => fireOnce()).not.toThrow(); + + // Let the rejected promise's .catch() handler actually run before the test ends. + await new Promise((resolve) => setTimeout(resolve, 0)); + }); +}); diff --git a/vortex-extension/src/index.ts b/vortex-extension/src/index.ts index abdaef8..773c254 100644 --- a/vortex-extension/src/index.ts +++ b/vortex-extension/src/index.ts @@ -1,28 +1,71 @@ import { log, types } from 'vortex-api'; import { isWitcher3Active } from './gating'; +import { ensureWsmToolRegistered } from './toolAcquisition'; /** * Extension entry point (Vortex looks for a default export named `init`, or a function * named `init`, per `@nexusmods/vortex-api`'s own documented extension structure). * - * This is the foundation scaffold unit: no feature registration lives here yet. Later - * units (tool acquisition, conflict scanning, the merge panel, dashlets) each add their - * own `context.register*` calls inside the `context.once(...)` callback below, gated on + * This unit (tool acquisition) is the first to add real registration: re-registering a + * previously-acquired WSM binary as a discovered tool, via `ensureWsmToolRegistered` + * (`./toolAcquisition`) - a **local-only, network-free** check, safe to run + * unconditionally on every load. Deliberately not an eager background *download* here: + * as of this unit, no GitHub Release exists on this repo yet (see + * `githubRelease.ts`'s own doc comment), so attempting one on every Vortex startup + * would just be a guaranteed, noisy failure with nothing to show for it. + * `./toolAcquisition`'s `acquireWsmTool` (the actual download/verify/extract/register + * pipeline) is exported for a later unit's own UI trigger (a "Get WitcherScriptMerger" + * action, once one exists) to call on explicit user request instead. + * + * Re-evaluated on every `'gamemode-activated'` event (`@nexusmods/vortex-api`'s own + * README documents this event, firing with the newly-active game's id), not just once + * at `context.once` time - a user can switch the active game without restarting Vortex, + * and without this, registering a previously-acquired tool would only ever happen if + * Witcher 3 already happened to be active the moment Vortex loaded this extension. + * + * Later units (conflict scanning, the merge panel, dashlets) each add their own + * `context.register*` calls inside the `context.once(...)` callback below, gated on * `isWitcher3Active` (imported from `./gating`) - preferably via each registration API's * own `condition` callback, so a live game-mode switch is honored without requiring a - * Vortex restart, rather than only checked once here. + * Vortex restart, the same way `tryRegisterWsmTool` below re-checks it on every + * `'gamemode-activated'` event rather than only once. * * This extension must never call `context.registerGame('witcher3', ...)` - Vortex's own * built-in `game-witcher3` extension already owns that registration; this extension is a * companion to it, not a replacement. */ function main(context: types.IExtensionContext): boolean { - context.once(() => { - if (isWitcher3Active(context.api)) { - log('info', 'witcherscriptmerger-vortex: witcher3 is the active game, extension ready'); - } else { + function tryRegisterWsmTool(): void { + if (!isWitcher3Active(context.api)) { log('debug', 'witcherscriptmerger-vortex: active game is not witcher3, extension is idle'); + return; } + + log('info', 'witcherscriptmerger-vortex: witcher3 is the active game, extension ready'); + + ensureWsmToolRegistered(context.api) + .then((registered) => { + if (registered) { + log('info', 'witcherscriptmerger-vortex: re-registered a previously acquired WSM tool'); + } else { + log('debug', 'witcherscriptmerger-vortex: no previously acquired WSM tool found - nothing to register yet'); + } + }) + .catch((err: unknown) => { + // Must never throw out of an event handler / context.once - an uncaught + // rejection here would be an unhandled promise rejection in Vortex's own + // process, not a contained extension failure. Local-only re-registration + // failing is unexpected (it does no network I/O), so this is logged at 'warn' + // rather than swallowed silently. + log('warn', 'witcherscriptmerger-vortex: failed to re-register a previously acquired WSM tool', { + error: err instanceof Error ? err.message : String(err), + }); + }); + } + + context.once(() => { + tryRegisterWsmTool(); + context.api.events.on('gamemode-activated', tryRegisterWsmTool); }); return true; diff --git a/vortex-extension/src/storage.test.ts b/vortex-extension/src/storage.test.ts new file mode 100644 index 0000000..843e9f1 --- /dev/null +++ b/vortex-extension/src/storage.test.ts @@ -0,0 +1,31 @@ +import * as path from 'path'; +import { describe, expect, it } from 'vitest'; +import { getBundleToolsDir, getDownloadCacheDir, getExtensionStorageDir, getWsmToolDir } from './storage'; + +// A minimal stand-in for IExtensionApi - only getPath is used by storage.ts, matching +// gating.test.ts's own fakeApi pattern for the same reason. +function fakeApi(userDataDir: string) { + return { + getPath: (name: string) => (name === 'userData' ? userDataDir : `/unexpected/${name}`), + } as unknown as Parameters[0]; +} + +describe('storage', () => { + const api = fakeApi(path.join('C:', 'fake', 'userData')); + + it('roots the extension storage dir under userData with a recognizable name', () => { + expect(getExtensionStorageDir(api)).toBe(path.join('C:', 'fake', 'userData', 'witcherscriptmerger-vortex')); + }); + + it('gives each subdirectory a distinct, stable path under the storage root', () => { + const root = getExtensionStorageDir(api); + expect(getWsmToolDir(api)).toBe(path.join(root, 'tool')); + expect(getDownloadCacheDir(api)).toBe(path.join(root, 'downloads')); + expect(getBundleToolsDir(api)).toBe(path.join(root, 'bundle-tools')); + }); + + it('never collides two subdirectories on the same path', () => { + const dirs = [getWsmToolDir(api), getDownloadCacheDir(api), getBundleToolsDir(api)]; + expect(new Set(dirs).size).toBe(dirs.length); + }); +}); diff --git a/vortex-extension/src/storage.ts b/vortex-extension/src/storage.ts new file mode 100644 index 0000000..62f4361 --- /dev/null +++ b/vortex-extension/src/storage.ts @@ -0,0 +1,61 @@ +import * as path from 'path'; +import { types } from 'vortex-api'; + +/** + * Extension-private storage layout, all rooted under Vortex's own `userData` directory + * (`api.getPath('userData')` - the same "always use the appropriate folder location" + * mechanism `@nexusmods/vortex-api`'s own `IExtensionApi.getPath` doc comment + * recommends, rather than this extension inventing its own path). Every helper here + * takes `api` rather than reading a module-level constant so it's trivially testable + * with a fake `{ getPath: () => tmpDir }` object, the same pattern `gating.ts` already + * uses for `isWitcher3Active(api)`. + * + * Layout: + * + * /witcherscriptmerger-vortex/ + * tool/ <- the acquired WSM Headless build (see toolAcquisition.ts). + * Flat, single "current" install, not versioned side-by-side + * installs - re-acquiring a different version overwrites it. + * `installed-version.txt` (INSTALLED_VERSION_FILENAME) + * records which version is currently unpacked here. + * downloads/ <- scratch .zip downloads before extraction; safe to delete + * entirely at any time (toolAcquisition.ts treats it as a + * cache, not a source of truth). + * bundle-tools/ <- CONVENTION for a later unit (bundle-tooling acquisition, + * not implemented here - see this unit's PR description): + * QuickBMS (quickbms.exe + witcher3.bms) and wcc_lite should + * land under here once that unit exists, and + * WSM_QuickBmsPath/WSM_QuickBmsPluginPath/WSM_WccLitePath + * (see wsmEnv.ts) should point inside it, e.g. + * path.join(getBundleToolsDir(api), 'QuickBMS', 'quickbms.exe'). + * Exported now, specifically so that later unit doesn't have + * to re-derive where this extension keeps its own files. + */ +const EXTENSION_STORAGE_DIRNAME = 'witcherscriptmerger-vortex'; +const TOOL_SUBDIR = 'tool'; +const DOWNLOAD_CACHE_SUBDIR = 'downloads'; +const BUNDLE_TOOLS_SUBDIR = 'bundle-tools'; + +/** Records which WSM version is currently unpacked in getWsmToolDir(api) - see that + * function's doc comment. Plain text, just the version string (e.g. "0.6.2"), no + * surrounding JSON/XML - deliberately trivial to read/write without a parser. */ +export const INSTALLED_VERSION_FILENAME = 'installed-version.txt'; + +export function getExtensionStorageDir(api: types.IExtensionApi): string { + return path.join(api.getPath('userData'), EXTENSION_STORAGE_DIRNAME); +} + +/** Where the acquired WSM Headless build's files (exe, its .dll.config, etc.) live. */ +export function getWsmToolDir(api: types.IExtensionApi): string { + return path.join(getExtensionStorageDir(api), TOOL_SUBDIR); +} + +/** Scratch directory for in-progress .zip downloads - a cache, not persistent state. */ +export function getDownloadCacheDir(api: types.IExtensionApi): string { + return path.join(getExtensionStorageDir(api), DOWNLOAD_CACHE_SUBDIR); +} + +/** See this module's own doc comment above - convention for a later unit, unused here. */ +export function getBundleToolsDir(api: types.IExtensionApi): string { + return path.join(getExtensionStorageDir(api), BUNDLE_TOOLS_SUBDIR); +} diff --git a/vortex-extension/src/toolAcquisition.test.ts b/vortex-extension/src/toolAcquisition.test.ts new file mode 100644 index 0000000..4c77eec --- /dev/null +++ b/vortex-extension/src/toolAcquisition.test.ts @@ -0,0 +1,249 @@ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { ArchiveExtractor } from './archiveExtractor'; +import { WSM_TOOL_ID } from './discoveredTool'; +import { DEFAULT_WSM_REPO, HttpClient } from './githubRelease'; +import { getDownloadCacheDir, INSTALLED_VERSION_FILENAME } from './storage'; +import { acquireWsmTool, AcquiredWsmTool, ensureWsmToolRegistered, WSM_HEADLESS_EXE_NAME } from './toolAcquisition'; + +const RELEASE_JSON = { + tag_name: 'v0.6.2', + assets: [ + { + name: 'WitcherScriptMerger.Headless-0.6.2-win-x64.zip', + browser_download_url: 'https://example.invalid/headless-win.zip', + size: 999, + }, + ], +}; + +function fakeApi(userDataDir: string, dispatch: (action: unknown) => void = vi.fn()) { + return { + getPath: (name: string) => (name === 'userData' ? userDataDir : `/unexpected/${name}`), + getState: () => ({}), + store: { dispatch }, + } as unknown as Parameters[0]['api']; +} + +/** A fake extractor that simulates "successful extraction" by writing the exe (and + * nothing else) directly into destDir - good enough to prove acquireWsmTool's own + * orchestration without a real archive. */ +function fakeExtractorThatProducesExe(): ArchiveExtractor { + return { + extractAll: async (_archivePath: string, destDir: string) => { + await fs.promises.mkdir(destDir, { recursive: true }); + await fs.promises.writeFile(path.join(destDir, WSM_HEADLESS_EXE_NAME), 'fake exe bytes', 'utf8'); + }, + }; +} + +function fakeClientForRelease(): HttpClient { + return { + getJson: async () => RELEASE_JSON, + downloadToFile: async (_url: string, destPath: string) => { + fs.writeFileSync(destPath, Buffer.alloc(RELEASE_JSON.assets[0].size, 1)); + return { bytesWritten: RELEASE_JSON.assets[0].size }; + }, + }; +} + +describe('acquireWsmTool', () => { + let userDataDir: string; + + beforeEach(() => { + userDataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'wsm-vortex-acquire-test-')); + }); + + afterEach(() => { + fs.rmSync(userDataDir, { recursive: true, force: true }); + }); + + it('downloads, verifies, extracts, records the installed repo+version, and registers the tool on a fresh install', async () => { + const dispatch = vi.fn(); + const api = fakeApi(userDataDir, dispatch); + const client = fakeClientForRelease(); + const extractor = fakeExtractorThatProducesExe(); + + const getJsonSpy = vi.spyOn(client, 'getJson'); + const downloadSpy = vi.spyOn(client, 'downloadToFile'); + const extractSpy = vi.spyOn(extractor, 'extractAll'); + + const result = await acquireWsmTool({ api, version: '0.6.2', client, extractor }); + + expect(getJsonSpy).toHaveBeenCalledTimes(1); + expect(downloadSpy).toHaveBeenCalledTimes(1); + expect(extractSpy).toHaveBeenCalledTimes(1); + expect(fs.existsSync(result.exePath)).toBe(true); + expect(fs.readFileSync(path.join(result.installDir, INSTALLED_VERSION_FILENAME), 'utf8')).toBe( + `${DEFAULT_WSM_REPO}@0.6.2`, + ); + + expect(dispatch).toHaveBeenCalledTimes(1); + const action = dispatch.mock.calls[0][0] as { payload: { toolId: string; result: { path: string } } }; + expect(action.payload.toolId).toBe(WSM_TOOL_ID); + expect(action.payload.result.path).toBe(result.exePath); + }); + + it('deletes the downloaded zip from the download cache after a successful extraction', async () => { + const api = fakeApi(userDataDir); + const client = fakeClientForRelease(); + const extractor = fakeExtractorThatProducesExe(); + + await acquireWsmTool({ api, version: '0.6.2', client, extractor }); + + const cacheDir = getDownloadCacheDir(api); + expect(fs.readdirSync(cacheDir)).toEqual([]); + }); + + it('is idempotent: does no network/extraction work when the requested repo+version is already installed, but still registers', async () => { + const dispatch = vi.fn(); + const api = fakeApi(userDataDir, dispatch); + const client = fakeClientForRelease(); + const extractor = fakeExtractorThatProducesExe(); + const getJsonSpy = vi.spyOn(client, 'getJson'); + const extractSpy = vi.spyOn(extractor, 'extractAll'); + + const first = await acquireWsmTool({ api, version: '0.6.2', client, extractor }); + dispatch.mockClear(); + getJsonSpy.mockClear(); + extractSpy.mockClear(); + + const second: AcquiredWsmTool = await acquireWsmTool({ api, version: '0.6.2', client, extractor }); + + expect(second).toEqual(first); + expect(getJsonSpy).not.toHaveBeenCalled(); + expect(extractSpy).not.toHaveBeenCalled(); + // Still (re-)registers even though nothing was downloaded - see acquireWsmTool's own + // doc comment for why that's not wasted work. + expect(dispatch).toHaveBeenCalledTimes(1); + }); + + it('re-acquires when a different version is requested than what is currently installed, wiping stale files from the old install', async () => { + const api = fakeApi(userDataDir); + const client = fakeClientForRelease(); + const extractor = fakeExtractorThatProducesExe(); + + const first = await acquireWsmTool({ api, version: '0.6.2', client, extractor }); + // A file that belonged only to the old (v0.6.2) install - e.g. a stray .pdb the new + // release's zip doesn't contain. If re-acquiring only extracts on top without first + // clearing installDir, this would survive indefinitely. + const staleFile = path.join(first.installDir, 'only-in-old-version.pdb'); + fs.writeFileSync(staleFile, 'stale', 'utf8'); + + const newerRelease = { + tag_name: 'v0.7.0', + assets: [{ name: 'WitcherScriptMerger.Headless-0.7.0-win-x64.zip', browser_download_url: 'https://example.invalid/newer.zip', size: 50 }], + }; + const newerClient: HttpClient = { + getJson: async () => newerRelease, + downloadToFile: async (_url, destPath) => { + fs.writeFileSync(destPath, Buffer.alloc(50, 2)); + return { bytesWritten: 50 }; + }, + }; + + const result = await acquireWsmTool({ api, version: '0.7.0', client: newerClient, extractor }); + + expect(fs.readFileSync(path.join(result.installDir, INSTALLED_VERSION_FILENAME), 'utf8')).toBe( + `${DEFAULT_WSM_REPO}@0.7.0`, + ); + expect(fs.existsSync(staleFile)).toBe(false); + }); + + it('re-acquires (rather than silently reusing the old install) when the same version is requested from a different repo', async () => { + const api = fakeApi(userDataDir); + const client = fakeClientForRelease(); + const extractor = fakeExtractorThatProducesExe(); + const extractSpy = vi.spyOn(extractor, 'extractAll'); + + await acquireWsmTool({ api, version: '0.6.2', repo: 'SomeFork/WitcherScriptMerger', client, extractor }); + extractSpy.mockClear(); + + const result = await acquireWsmTool({ api, version: '0.6.2', repo: DEFAULT_WSM_REPO, client, extractor }); + + expect(extractSpy).toHaveBeenCalledTimes(1); + expect(fs.readFileSync(path.join(result.installDir, INSTALLED_VERSION_FILENAME), 'utf8')).toBe( + `${DEFAULT_WSM_REPO}@0.6.2`, + ); + }); + + it('coalesces concurrent calls targeting the same install onto a single download/extract', async () => { + const dispatch = vi.fn(); + const api = fakeApi(userDataDir, dispatch); + const client = fakeClientForRelease(); + const extractor = fakeExtractorThatProducesExe(); + const getJsonSpy = vi.spyOn(client, 'getJson'); + const extractSpy = vi.spyOn(extractor, 'extractAll'); + + const [first, second] = await Promise.all([ + acquireWsmTool({ api, version: '0.6.2', client, extractor }), + acquireWsmTool({ api, version: '0.6.2', client, extractor }), + ]); + + expect(second).toEqual(first); + expect(getJsonSpy).toHaveBeenCalledTimes(1); + expect(extractSpy).toHaveBeenCalledTimes(1); + }); + + it('throws a clear error when extraction reports success but the expected exe is missing afterward', async () => { + const api = fakeApi(userDataDir); + const client = fakeClientForRelease(); + const noOpExtractor: ArchiveExtractor = { extractAll: async () => undefined }; + + await expect(acquireWsmTool({ api, version: '0.6.2', client, extractor: noOpExtractor })).rejects.toThrow( + /expected executable .* was not found/, + ); + }); +}); + +describe('ensureWsmToolRegistered', () => { + let userDataDir: string; + + beforeEach(() => { + userDataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'wsm-vortex-ensure-test-')); + }); + + afterEach(() => { + fs.rmSync(userDataDir, { recursive: true, force: true }); + }); + + it('returns false and dispatches nothing when no tool has been acquired yet', async () => { + const dispatch = vi.fn(); + const api = fakeApi(userDataDir, dispatch); + + await expect(ensureWsmToolRegistered(api)).resolves.toBe(false); + expect(dispatch).not.toHaveBeenCalled(); + }); + + it('returns true and registers the tool when one was already acquired locally', async () => { + const dispatch = vi.fn(); + const api = fakeApi(userDataDir, dispatch); + + // Simulate a prior acquireWsmTool run's on-disk result directly, without any + // network/extraction machinery - this function must do none of that. + const toolDir = path.join(userDataDir, 'witcherscriptmerger-vortex', 'tool'); + await fs.promises.mkdir(toolDir, { recursive: true }); + const exePath = path.join(toolDir, WSM_HEADLESS_EXE_NAME); + await fs.promises.writeFile(exePath, 'fake exe bytes', 'utf8'); + + await expect(ensureWsmToolRegistered(api)).resolves.toBe(true); + expect(dispatch).toHaveBeenCalledTimes(1); + const action = dispatch.mock.calls[0][0] as { payload: { toolId: string; result: { path: string } } }; + expect(action.payload.toolId).toBe(WSM_TOOL_ID); + expect(action.payload.result.path).toBe(exePath); + }); + + it('propagates a non-ENOENT filesystem error rather than silently treating it as "nothing installed"', async () => { + const api = fakeApi(userDataDir); + const accessError = Object.assign(new Error('EACCES: permission denied'), { code: 'EACCES' }); + const accessSpy = vi.spyOn(fs.promises, 'access').mockRejectedValueOnce(accessError); + + try { + await expect(ensureWsmToolRegistered(api)).rejects.toThrow(/EACCES/); + } finally { + accessSpy.mockRestore(); + } + }); +}); diff --git a/vortex-extension/src/toolAcquisition.ts b/vortex-extension/src/toolAcquisition.ts new file mode 100644 index 0000000..81740f9 --- /dev/null +++ b/vortex-extension/src/toolAcquisition.ts @@ -0,0 +1,236 @@ +import * as fs from 'fs'; +import * as path from 'path'; +import { selectors, types, util } from 'vortex-api'; +import { ArchiveExtractor, createVortexArchiveExtractor } from './archiveExtractor'; +import { buildWsmDiscoveredTool, registerWsmDiscoveredTool } from './discoveredTool'; +import { WITCHER3_GAME_ID } from './gating'; +import { buildAssetFileName, DEFAULT_WSM_REPO, downloadReleaseAsset, HttpClient, resolveReleaseAsset } from './githubRelease'; +import { getDownloadCacheDir, getWsmToolDir, INSTALLED_VERSION_FILENAME } from './storage'; +import { buildWsmEnv } from './wsmEnv'; + +/** + * Orchestrates the full acquisition pipeline (download from GitHub Releases -> verify -> + * extract -> register as a discovered tool) and a lighter local-only re-registration + * path used at every Vortex startup. See this unit's PR description for exactly what's + * verified end-to-end (a locally-built WSM binary standing in for a downloaded one, per + * `test/toolAcquisition.integration.test.ts`) versus what's real-but-unexercised code + * (the actual GitHub download - no release exists on this repo yet). + */ + +export const WSM_HEADLESS_EXE_NAME = 'WitcherScriptMerger.Headless.exe'; + +export interface AcquireWsmToolOptions { + api: types.IExtensionApi; + /** e.g. `"0.6.2"` - no leading "v" (matches `WitcherScriptMerger.Headless.csproj`'s + * own ``; the release tag itself, per `release.yml`, is this value with a + * "v" prefixed back on). */ + version: string; + repo?: string; + /** Test-only seam - see `githubRelease.ts`'s `HttpClient`. */ + client?: HttpClient; + /** Test-only seam - see `archiveExtractor.ts`'s `ArchiveExtractor`. */ + extractor?: ArchiveExtractor; +} + +export interface AcquiredWsmTool { + exePath: string; + version: string; + installDir: string; +} + +function isEnoent(err: unknown): boolean { + return typeof err === 'object' && err !== null && (err as NodeJS.ErrnoException).code === 'ENOENT'; +} + +async function pathExists(target: string): Promise { + try { + await fs.promises.access(target); + return true; + } catch (err) { + if (isEnoent(err)) { + return false; + } + // Anything other than "doesn't exist" (permission denied, locked by another process, + // a transient antivirus scan, etc.) is a real problem the caller needs to see, not + // something that should be silently treated as "nothing installed yet" - that would + // both hide the actual error and risk kicking off a doomed, unnecessary re-download. + throw err; + } +} + +/** `@` - GitHub owner/repo names never contain "@", so this is an + * unambiguous, trivially-parseable single-line marker; no JSON needed for two fields. */ +function formatInstalledMarker(repo: string, version: string): string { + return `${repo}@${version}`; +} + +async function readInstalledMarker(installDir: string): Promise<{ repo: string; version: string } | undefined> { + let content: string; + try { + content = await fs.promises.readFile(path.join(installDir, INSTALLED_VERSION_FILENAME), 'utf8'); + } catch (err) { + if (isEnoent(err)) { + return undefined; + } + throw err; + } + + const trimmed = content.trim(); + const separatorIndex = trimmed.lastIndexOf('@'); + if (separatorIndex <= 0) { + // Malformed or from an older marker format - treat as "no confident match", which + // safely falls through to a fresh acquire/overwrite rather than trusting a value we + // can't actually parse. + return undefined; + } + + return { repo: trimmed.slice(0, separatorIndex), version: trimmed.slice(separatorIndex + 1) }; +} + +/** + * Downloads (if not already present locally at the requested repo+version), verifies, + * extracts, and installs the WSM Headless build, then registers it as a discovered + * Vortex tool. Idempotent on the download/extract step: if `getWsmToolDir(api)` already + * contains this exact repo+version (per `INSTALLED_VERSION_FILENAME`), no network + * activity happens at all. Registration always happens regardless, even on that + * idempotent path - the exe being present locally doesn't guarantee it's currently + * registered (e.g. a fresh Vortex session after a restart whose discovered-tools + * persistence didn't survive this tool's non-serializable `executable` field - see + * `discoveredTool.ts`), and registering an already-registered tool is a harmless no-op + * dispatch. + * + * **Concurrency**: calls sharing the same `api` (and therefore the same `installDir`) + * that overlap in time coalesce onto whichever call started first - a second call + * arriving while the first is still in flight gets the *first* call's result rather than + * starting an independent download/extract into the same directory (which would race + * both the download and the extraction). This is a deliberate simplification, not a + * full per-argument dedup: if the second call actually requested a different + * `version`/`repo` than the first, it silently receives the first call's result instead + * of its own request. Acceptable for this unit's only real trigger shape (a user + * re-clicking the same "Get/Update WitcherScriptMerger" action while a request is + * already in flight) - a later unit adding that UI action should be aware of this if it + * ever needs to let a user cancel/redirect an in-flight acquisition. + */ +export async function acquireWsmTool(options: AcquireWsmToolOptions): Promise { + const installDir = getWsmToolDir(options.api); + const existing = inFlightAcquisitions.get(installDir); + if (existing) { + return existing; + } + + const promise = acquireWsmToolUncoordinated(options, installDir); + inFlightAcquisitions.set(installDir, promise); + try { + return await promise; + } finally { + inFlightAcquisitions.delete(installDir); + } +} + +/** Keyed by installDir - see `acquireWsmTool`'s own doc comment for the coalescing + * behavior this backs. */ +const inFlightAcquisitions = new Map>(); + +async function acquireWsmToolUncoordinated(options: AcquireWsmToolOptions, installDir: string): Promise { + const { api, version } = options; + const repo = options.repo ?? DEFAULT_WSM_REPO; + const exePath = path.join(installDir, WSM_HEADLESS_EXE_NAME); + + const installedMarker = await readInstalledMarker(installDir); + if ((await pathExists(exePath)) && installedMarker?.repo === repo && installedMarker?.version === version) { + registerAcquiredTool(api, exePath); + return { exePath, version, installDir }; + } + + const assetFileName = buildAssetFileName(version); + const asset = await resolveReleaseAsset({ repo, tag: `v${version}`, assetFileName, client: options.client }); + + const cacheDir = getDownloadCacheDir(api); + await fs.promises.mkdir(cacheDir, { recursive: true }); + const zipPath = path.join(cacheDir, assetFileName); + await downloadReleaseAsset({ + downloadUrl: asset.downloadUrl, + destPath: zipPath, + expectedSize: asset.size, + client: options.client, + }); + + // Wipe whatever's currently in installDir before extracting: re-acquiring a different + // version should actually overwrite (per storage.ts's own doc comment), not merge + // files from two different releases together, and a prior failed/partial extraction's + // debris shouldn't survive into this attempt either. `force: true` only suppresses the + // "doesn't exist yet" case (a fresh install with no prior installDir at all) - if a WSM + // process is actively running out of this directory, removal genuinely fails (Windows + // won't let a running exe's backing file be deleted), which is the correct outcome + // here: a clear error, not a silently corrupted running install. + await fs.promises.rm(installDir, { recursive: true, force: true }); + await fs.promises.mkdir(installDir, { recursive: true }); + + // extractAll's own implementations also ensure destDir exists (see archiveExtractor.ts's + // ArchiveExtractor contract) - that's not redundant with the mkdir just above so much as + // each call site owning its own precondition: this mkdir exists specifically to leave a + // fresh, empty directory right after the rm above, independent of whatever a particular + // extractor implementation does or doesn't assume about its destDir argument. + const extractor = options.extractor ?? createVortexArchiveExtractor(api); + await extractor.extractAll(zipPath, installDir); + + if (!(await pathExists(exePath))) { + throw new Error( + `Extracted '${zipPath}' into '${installDir}' but expected executable '${WSM_HEADLESS_EXE_NAME}' was not found there afterward. The release asset's internal layout may not match what this extension expects.`, + ); + } + + await util.writeFileAtomic(path.join(installDir, INSTALLED_VERSION_FILENAME), formatInstalledMarker(repo, version)); + + // The downloaded .zip has done its job once extraction succeeded - getDownloadCacheDir + // is documented (storage.ts) as a disposable cache, so don't leave it accumulating one + // full release archive per acquisition/upgrade forever. Best-effort: a failure to clean + // up the cache is not worth failing an otherwise-successful acquisition over. + try { + await fs.promises.unlink(zipPath); + } catch { + // Ignored - see comment above. + } + + registerAcquiredTool(api, exePath); + + return { exePath, version, installDir }; +} + +function registerAcquiredTool(api: types.IExtensionApi, exePath: string): void { + // Specifically Witcher 3's own discovered install path, not selectors.currentGameDiscovery + // (whichever game happens to be active *right now*) - this function always registers the + // tool under WITCHER3_GAME_ID a few lines down, so the environment attached to it must be + // scoped to that same game, not whatever's currently active (which can race a live + // game-mode switch, since every caller of this function is async). + const gameDirectory = selectors.discoveryByGame(api.getState(), WITCHER3_GAME_ID)?.path; + const tool = buildWsmDiscoveredTool({ exePath, environment: buildWsmEnv({ gameDirectory }) }); + registerWsmDiscoveredTool(api, tool); +} + +/** + * Registration-only path, with **no network activity whatsoever** - safe to call + * unconditionally on every extension load (see `index.ts`). If a WSM build was already + * acquired in a previous session (via `acquireWsmTool` above, triggered by a later + * unit's own UI action - this unit doesn't add one; see this unit's PR description for + * why an eager background download at every startup would be actively wrong while no + * GitHub Release exists yet), this re-registers it as a discovered tool; Vortex's own + * discovered-tools persistence for an `IDiscoveredTool` carrying a function field + * (`executable`) is unverified against a real Vortex host (see `discoveredTool.ts`), so + * re-registering on every startup is the safe, idempotent default rather than assuming + * a prior registration survived. + * + * Returns `false` (not an error) when nothing has been acquired yet - that's the + * expected, normal state for as long as no GitHub Release exists. + */ +export async function ensureWsmToolRegistered(api: types.IExtensionApi): Promise { + const installDir = getWsmToolDir(api); + const exePath = path.join(installDir, WSM_HEADLESS_EXE_NAME); + + if (!(await pathExists(exePath))) { + return false; + } + + registerAcquiredTool(api, exePath); + return true; +} diff --git a/vortex-extension/src/wsmEnv.test.ts b/vortex-extension/src/wsmEnv.test.ts new file mode 100644 index 0000000..8151afd --- /dev/null +++ b/vortex-extension/src/wsmEnv.test.ts @@ -0,0 +1,68 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { buildWsmEnv, mergeWithProcessEnv, WSM_ENV_PREFIX } from './wsmEnv'; + +describe('buildWsmEnv', () => { + it('maps each provided config key to its WSM_ environment variable', () => { + const env = buildWsmEnv({ + gameDirectory: 'C:\\Games\\Witcher3', + modsDirectory: 'C:\\Games\\Witcher3\\mods', + mergedModName: 'mod0000_MergedFiles', + quickBmsPath: 'C:\\Tools\\quickbms.exe', + quickBmsPluginPath: 'C:\\Tools\\witcher3.bms', + wccLitePath: 'C:\\Tools\\wcc_lite.exe', + }); + + expect(env).toEqual({ + WSM_GameDirectory: 'C:\\Games\\Witcher3', + WSM_ModsDirectory: 'C:\\Games\\Witcher3\\mods', + WSM_MergedModName: 'mod0000_MergedFiles', + WSM_QuickBmsPath: 'C:\\Tools\\quickbms.exe', + WSM_QuickBmsPluginPath: 'C:\\Tools\\witcher3.bms', + WSM_WccLitePath: 'C:\\Tools\\wcc_lite.exe', + }); + }); + + it('uses the exact "WSM_" prefix AppSettings.cs expects', () => { + expect(WSM_ENV_PREFIX).toBe('WSM_'); + }); + + it('omits keys whose value is undefined rather than setting them to an empty string', () => { + const env = buildWsmEnv({ modsDirectory: 'C:\\Mods' }); + + expect(env).toEqual({ WSM_ModsDirectory: 'C:\\Mods' }); + expect('WSM_GameDirectory' in env).toBe(false); + expect('WSM_MergedModName' in env).toBe(false); + }); + + it('does set a key when its value is deliberately the empty string', () => { + const env = buildWsmEnv({ modsDirectory: '' }); + expect(env).toEqual({ WSM_ModsDirectory: '' }); + }); + + it('returns an empty object for an empty config', () => { + expect(buildWsmEnv({})).toEqual({}); + }); +}); + +describe('mergeWithProcessEnv', () => { + const ORIGINAL_ENV = { ...process.env }; + + beforeEach(() => { + process.env.WSM_TEST_PROBE = 'from-process-env'; + process.env.PATH_LIKE_PROBE = 'still-present'; + }); + + afterEach(() => { + process.env = { ...ORIGINAL_ENV }; + }); + + it('keeps existing process.env entries the overrides do not mention', () => { + const merged = mergeWithProcessEnv({ WSM_ModsDirectory: 'C:\\Mods' }); + expect(merged.PATH_LIKE_PROBE).toBe('still-present'); + }); + + it('lets overrides win over an existing process.env value with the same key', () => { + const merged = mergeWithProcessEnv({ WSM_TEST_PROBE: 'from-override' }); + expect(merged.WSM_TEST_PROBE).toBe('from-override'); + }); +}); diff --git a/vortex-extension/src/wsmEnv.ts b/vortex-extension/src/wsmEnv.ts new file mode 100644 index 0000000..09e5ba7 --- /dev/null +++ b/vortex-extension/src/wsmEnv.ts @@ -0,0 +1,77 @@ +/** + * Builds the `WSM_` environment-variable overrides described in + * `WitcherScriptMerger.Core/AppSettings.cs` (`AppSettings.EnvironmentVariablePrefix` / + * `GetEnvironmentOverride`) - the *only* sanctioned way this extension configures a + * spawned WSM process. Never read or write `WitcherScriptMerger.exe.config` / + * `WitcherScriptMerger.Headless.dll.config` XML directly - see + * `docs/vortex-extension-design.md` section 4.1 for why (a cached-`Configuration`, + * explicit-`Save()`-only object on the .NET side makes hand-editing that file while a + * WSM process is already running against it a real race). + * + * This module has no dependency on 'vortex-api' at all - it's plain data in, plain data + * out - so it's reusable by every future caller that spawns a WSM process: the MCP path + * (`mcpClient.ts`'s `WsmMcpClientOptions.env`, wired in `test/toolAcquisition.integration.test.ts` + * as the proof this actually works end-to-end) and, per this unit's own instructions, the + * as-yet-unbuilt one-shot `merge` CLI invocation path (a later "merge panel" unit) - + * both should build their spawn `env` from this same function rather than duplicating + * the `WSM_` prefix/key-name mapping independently. + */ + +/** Mirrors `AppSettings.EnvironmentVariablePrefix` on the .NET side exactly. */ +export const WSM_ENV_PREFIX = 'WSM_'; + +export interface WsmEnvConfig { + gameDirectory?: string; + modsDirectory?: string; + mergedModName?: string; + /** + * Not consumed by anything in this unit (bundle-tooling acquisition hasn't landed + * yet - see `storage.ts`'s `getBundleToolsDir` doc comment for the storage + * convention a later unit should use to produce these three paths). Accepted here + * now so that later unit only has to supply values, not invent the env-var mapping. + */ + quickBmsPath?: string; + quickBmsPluginPath?: string; + wccLitePath?: string; +} + +const CONFIG_KEY_TO_SETTING_NAME: Record = { + gameDirectory: 'GameDirectory', + modsDirectory: 'ModsDirectory', + mergedModName: 'MergedModName', + quickBmsPath: 'QuickBmsPath', + quickBmsPluginPath: 'QuickBmsPluginPath', + wccLitePath: 'WccLitePath', +}; + +/** + * Builds the `WSM_` environment-variable map for the given config values. + * Keys whose value is `undefined` are omitted entirely (not set to `""`) so a caller + * can pass a partial config without accidentally overriding an unrelated setting with + * an empty string - `AppSettings.GetEnvironmentOverride` treats "env var not set" and + * "env var set to empty string" differently (the latter is itself a real, if unusual, + * override value), so omission has to be a real absence of the key, not `""`. + */ +export function buildWsmEnv(config: WsmEnvConfig): Record { + const env: Record = {}; + + for (const key of Object.keys(CONFIG_KEY_TO_SETTING_NAME) as (keyof WsmEnvConfig)[]) { + const value = config[key]; + if (value !== undefined) { + env[`${WSM_ENV_PREFIX}${CONFIG_KEY_TO_SETTING_NAME[key]}`] = value; + } + } + + return env; +} + +/** + * Merges WSM_* overrides on top of the current process's own environment, for passing + * directly as `child_process.spawn`'s `env` option (which, when set at all, replaces + * the child's entire environment rather than augmenting it - so a caller that wants the + * spawned WSM process to still see a normal PATH etc. needs to spread `process.env` + * itself; this is that spread, done once, in one place). + */ +export function mergeWithProcessEnv(overrides: Record): NodeJS.ProcessEnv { + return { ...process.env, ...overrides }; +} diff --git a/vortex-extension/test/testUtils/vortexApiStub.ts b/vortex-extension/test/testUtils/vortexApiStub.ts index d8dd542..8e150a7 100644 --- a/vortex-extension/test/testUtils/vortexApiStub.ts +++ b/vortex-extension/test/testUtils/vortexApiStub.ts @@ -1,3 +1,5 @@ +import * as fs from 'fs'; + // Minimal runtime stand-in for the real `vortex-api` module. At Vortex runtime, that bare // specifier is injected by Vortex's own extension loader (see webpack.config.cjs's // `externals` comment) - there is no installable runtime package to resolve it against. @@ -8,8 +10,55 @@ // Deliberately not an attempt to replicate Vortex's real, much larger Redux state shape // (confirmed via the actual Vortex monorepo source that `selectors.activeGameId` really // derives from `state.settings.profiles.activeProfileId` via a profile lookup) - tests -// using this stub exercise this extension's own gating logic (`gating.ts`), not Vortex's -// selector implementation, so a simplified fake state shape is intentional here. +// using this stub exercise this extension's own gating/tool-acquisition logic +// (`gating.ts`, `toolAcquisition.ts`), not Vortex's selector implementation, so a +// simplified fake state shape is intentional here. `discoveryByGame` (real signature: +// `(state, gameId) => IDiscoveryResult`, per @nexusmods/vortex-api's own +// `re-reselect`-based `ParametricSelector` type) added alongside `toolAcquisition.ts`, +// same simplified-fake-state philosophy - deliberately keyed by gameId (unlike a +// same-shape "whichever game is active" selector) since `toolAcquisition.ts` always +// registers its tool under a fixed game id and needs that game's own discovery +// specifically, not whatever happens to be active when it runs. export const selectors = { activeGameId: (state: { activeGameId?: string }): string | undefined => state?.activeGameId, + discoveryByGame: ( + state: { discoveryByGame?: Record }, + gameId: string, + ): { path?: string } | undefined => state?.discoveryByGame?.[gameId], +}; + +// `actions.addDiscoveredTool` needs a real (if simplified) implementation, not just a +// type, because `discoveredTool.ts`'s `registerWsmDiscoveredTool` calls it as a value at +// runtime (`api.store.dispatch(actions.addDiscoveredTool(...))`) - unlike a type-only +// import (e.g. `types`, deliberately never exported by this stub - see `gating.test.ts`'s +// own comment on why that's safe), vitest actually executes this call, so it needs +// something real to invoke. Shape matches the real `ComplexActionCreator4`'s payload +// closely enough for `discoveredTool.test.ts`'s dispatch-argument assertions; the actual +// action `type` string is never asserted on since it's an internal Vortex implementation +// detail this extension has no business depending on. +export const actions = { + addDiscoveredTool: (gameId: string, toolId: string, result: unknown, manual: boolean) => ({ + type: 'ADD_DISCOVERED_TOOL', + payload: { gameId, toolId, result, manual }, + }), +}; + +// `util.writeFileAtomic` needs a real implementation for the same reason as +// `actions.addDiscoveredTool` above - `toolAcquisition.ts` calls it as a value at +// runtime to write its installed-version marker. A plain (non-atomic) write is a +// perfectly adequate fake here: these tests exercise `toolAcquisition.ts`'s own +// orchestration, not vortex-api's atomicity guarantee, which this extension trusts +// rather than re-verifies. +export const util = { + writeFileAtomic: async (filePath: string, input: string | Buffer): Promise => { + await fs.promises.writeFile(filePath, input); + }, +}; + +// `log` needs a real (no-op) implementation because `index.ts` calls it as a value at +// every branch of its own logic - now that index.test.ts actually executes index.ts's +// `main()` (rather than only wiring/untested code, as when this scaffold had no real +// registration logic), this stub must resolve it rather than leaving it undefined. +export const log = (_level: string, _message: string, _metadata?: unknown): void => { + // Intentionally a no-op in tests - nothing here asserts on log output. }; diff --git a/vortex-extension/test/toolAcquisition.integration.test.ts b/vortex-extension/test/toolAcquisition.integration.test.ts new file mode 100644 index 0000000..269ae74 --- /dev/null +++ b/vortex-extension/test/toolAcquisition.integration.test.ts @@ -0,0 +1,204 @@ +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; +import { spawnSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { fileURLToPath } from 'url'; +import { WITCHER3_GAME_ID } from '../src/gating'; +import { WSM_TOOL_ID } from '../src/discoveredTool'; +import { DEFAULT_WSM_REPO } from '../src/githubRelease'; +import { getWsmToolDir, INSTALLED_VERSION_FILENAME } from '../src/storage'; +import { ensureWsmToolRegistered, WSM_HEADLESS_EXE_NAME } from '../src/toolAcquisition'; +import { buildWsmEnv, mergeWithProcessEnv } from '../src/wsmEnv'; +import { WsmMcpClient } from '../src/mcpClient'; + +// Real, end-to-end integration test for this unit's acquisition -> registration -> +// env-var-config pipeline. **Does not exercise the actual GitHub-Releases download +// path** (src/githubRelease.ts) - no version tag has been pushed to this repo, so no +// GitHub Release exists yet (see githubRelease.ts's own doc comment and this unit's PR +// description). Instead, this test stands a *locally-built* WSM binary in for "the +// downloaded-and-extracted one": it publishes WitcherScriptMerger.Headless with the +// exact same profile release.yml itself uses (`-p:PublishProfile=win-x64` - self- +// contained, single-file), then lays it out on disk exactly the way `acquireWsmTool` +// would have (see src/toolAcquisition.ts's `getWsmToolDir` layout), so everything +// *downstream* of the download - local-only registration (`ensureWsmToolRegistered`) +// and, critically, the WSM_* env-var configuration mechanism actually reaching a real, +// spawned WSM process - is proven for real, not mocked. +// +// The core proof (see the second `it` below): the scratch `.dll.config` this test +// writes deliberately sets ModsDirectory/MergedModName to *wrong* placeholder values +// nothing else in this test uses, and the env vars this unit's own `buildWsmEnv` builds +// are asserted to win over them - not just "some value came back", but specifically +// *not* the XML's value. That distinguishes "the mechanism works" from "some value +// happened to end up populated by coincidence". + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const REPO_ROOT = path.resolve(__dirname, '..', '..'); +const HEADLESS_CSPROJ = path.join( + REPO_ROOT, + 'WitcherScriptMerger.Headless', + 'WitcherScriptMerger.Headless.csproj', +); +// Matches release.yml's own `build` job matrix entry for +// "WitcherScriptMerger.Headless (win-x64)" exactly - see that job's `publish-dir`. +const PUBLISH_DIR = path.join( + REPO_ROOT, + 'WitcherScriptMerger.Headless', + 'bin', + 'Release', + 'net10.0', + 'win-x64', + 'publish', +); +const PUBLISHED_EXE = path.join(PUBLISH_DIR, WSM_HEADLESS_EXE_NAME); + +// Deliberately wrong values, distinct from anything this test's own assertions use for +// the "real" (env-var) side - see this file's own top comment. +const WRONG_XML_MODS_DIRECTORY = 'C:\\this-is-the-WRONG-xml-value\\mods'; +const WRONG_XML_MERGED_MOD_NAME = 'WRONG_XML_MergedModName'; + +// Same escaping rationale as mcpClient.integration.test.ts's own helper of the same +// name - kept local rather than shared/exported since it's test-fixture plumbing, not +// extension code (this test writes a scratch .dll.config as *test setup*, standing in +// for what a real `acquireWsmTool` extraction would have produced; the extension's own +// production code, per this unit's own instructions, never reads or writes this file). +function escapeXmlAttribute(value: string): string { + return value + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); +} + +function buildScratchConfig(modsDirectory: string, mergedModName: string): string { + return ` + + + + + + + + + + + + + + + + + + +`; +} + +let userDataDir: string; +let scratchModsDir: string; +let exePath: string; +let dispatch: ReturnType; +let fakeApi: Parameters[0]; + +beforeAll(() => { + if (!fs.existsSync(PUBLISHED_EXE)) { + // Exact same invocation as .github/workflows/release.yml's "Publish" step for the + // "WitcherScriptMerger.Headless (win-x64)" matrix entry (minus that workflow's + // -p:Version=, irrelevant here) - see WitcherScriptMerger.Headless/CLAUDE.md's + // "Publishing" section. + const result = spawnSync('dotnet', ['publish', HEADLESS_CSPROJ, '-c', 'Release', '-p:PublishProfile=win-x64'], { + cwd: REPO_ROOT, + stdio: 'inherit', + }); + if (result.status !== 0) { + throw new Error( + `dotnet publish (win-x64 profile) of WitcherScriptMerger.Headless failed (required to run this ` + + `integration test) - exit code ${result.status}`, + ); + } + } + + if (!fs.existsSync(PUBLISHED_EXE)) { + throw new Error(`Expected published exe not found at ${PUBLISHED_EXE} even after publishing.`); + } + + userDataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'wsm-vortex-tool-acquisition-test-')); + dispatch = vi.fn(); + fakeApi = { + getPath: (name: string) => (name === 'userData' ? userDataDir : `/unexpected/${name}`), + getState: () => ({}), + store: { dispatch }, + } as unknown as Parameters[0]; + + // Lay out this test's scratch "already acquired" install exactly the way + // acquireWsmTool (src/toolAcquisition.ts) would have after downloading and + // extracting a real release asset - same directory (getWsmToolDir), same files + // (the whole publish output, including the un-doctored WitcherScriptMerger.Core.pdb/ + // WitcherScriptMerger.Headless.pdb alongside the exe, matching a real zip's + // contents), same INSTALLED_VERSION_FILENAME marker. + const toolDir = getWsmToolDir(fakeApi); + fs.mkdirSync(toolDir, { recursive: true }); + fs.cpSync(PUBLISH_DIR, toolDir, { recursive: true }); + fs.writeFileSync(path.join(toolDir, INSTALLED_VERSION_FILENAME), `${DEFAULT_WSM_REPO}@0.6.2`, 'utf8'); + + // Overwrite the copied .dll.config with deliberately wrong placeholder values (see + // this file's own top comment) - test setup only, never done by extension code. + fs.writeFileSync( + path.join(toolDir, 'WitcherScriptMerger.Headless.dll.config'), + buildScratchConfig(WRONG_XML_MODS_DIRECTORY, WRONG_XML_MERGED_MOD_NAME), + 'utf8', + ); + + exePath = path.join(toolDir, WSM_HEADLESS_EXE_NAME); + + scratchModsDir = path.join(userDataDir, 'IntegrationTestMods'); + fs.mkdirSync(scratchModsDir, { recursive: true }); +}, 300_000); + +afterAll(() => { + if (userDataDir) { + fs.rmSync(userDataDir, { recursive: true, force: true }); + } +}); + +describe('tool acquisition end-to-end (locally-built binary standing in for a downloaded one)', () => { + it('ensureWsmToolRegistered finds the locally-installed binary and registers it, with no network activity', async () => { + const registered = await ensureWsmToolRegistered(fakeApi); + + expect(registered).toBe(true); + expect(dispatch).toHaveBeenCalledTimes(1); + + const action = dispatch.mock.calls[0][0] as { + payload: { gameId: string; toolId: string; manual: boolean; result: { path: string; id: string } }; + }; + expect(action.payload.gameId).toBe(WITCHER3_GAME_ID); + expect(action.payload.toolId).toBe(WSM_TOOL_ID); + expect(action.payload.manual).toBe(true); + expect(action.payload.result.path).toBe(exePath); + expect(action.payload.result.id).toBe(WSM_TOOL_ID); + }); + + it('WSM_* env vars built by buildWsmEnv override the XML config in a real spawned WSM MCP process', async () => { + const env = mergeWithProcessEnv( + buildWsmEnv({ modsDirectory: scratchModsDir, mergedModName: 'IntegrationTestMergedMod' }), + ); + + const client = await WsmMcpClient.connect({ exePath, env }); + try { + const status = await client.getStatus(); + + // The real proof: not the XML's values... + expect(status.modsDirectory).not.toBe(WRONG_XML_MODS_DIRECTORY); + expect(status.mergedModName).not.toBe(WRONG_XML_MERGED_MOD_NAME); + // ...but exactly what this unit's own env-var builder supplied. + expect(status.modsDirectory).toBe(scratchModsDir); + expect(status.mergedModName).toBe('IntegrationTestMergedMod'); + + expect(status.modsDirectoryExists).toBe(true); + expect(status.textMergeDependenciesValid).toBe(true); + expect(status.conflictCount).toBe(0); + } finally { + await client.close(); + } + }, 30_000); +}); diff --git a/vortex-extension/webpack.config.cjs b/vortex-extension/webpack.config.cjs index 5aa6bc2..c9b4751 100644 --- a/vortex-extension/webpack.config.cjs +++ b/vortex-extension/webpack.config.cjs @@ -30,7 +30,10 @@ const vortexApiPackageJsonPath = path.join( ); const { peerDependencies } = JSON.parse(fs.readFileSync(vortexApiPackageJsonPath, 'utf8')); -const nodeBuiltins = ['fs', 'path', 'os', 'child_process', 'net', 'util']; +// 'https' and 'http' (githubRelease.ts's download logic) and 'stream' (piping an https +// response into a write stream) added alongside toolAcquisition.ts's tool-acquisition +// unit; the rest predate it. +const nodeBuiltins = ['fs', 'path', 'os', 'child_process', 'net', 'util', 'https', 'http', 'stream']; function asExternals(names) { return names.reduce((acc, name) => { From e553d2daf856854641bfdd56a397d28205a0ebca Mon Sep 17 00:00:00 2001 From: Chris Knight Date: Sun, 9 Aug 2026 23:31:52 -0400 Subject: [PATCH 2/2] Fix stale README claim and overstated archiveExtractor comment 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 Claude-Session: https://claude.ai/code/session_01GXAuGMLB44T5Zv5o5ZzKah --- vortex-extension/README.md | 20 +++++++++++++------- vortex-extension/src/archiveExtractor.ts | 17 ++++++++++------- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/vortex-extension/README.md b/vortex-extension/README.md index 72f959a..2c46222 100644 --- a/vortex-extension/README.md +++ b/vortex-extension/README.md @@ -23,13 +23,19 @@ npm test # fast, Node-only unit tests ``` `npm test` only runs the fast, Node-only unit tests (`src/**/*.test.ts`) - no .NET SDK -needed. `src/mcpClient.ts`'s real, spawned-process integration test -(`test/mcpClient.integration.test.ts`) is a separate script, `npm run test:integration`, -since it needs a local .NET SDK and a buildable `WitcherScriptMerger.Headless` (it will -run `dotnet build` itself if the exe isn't already present) - kept out of the default -`npm test` so a Node-only environment (e.g. a contributor machine or CI runner without -the .NET SDK on `PATH`) isn't forced through a multi-minute .NET build just to iterate on -this extension's own TypeScript. +needed. The real, spawned-process integration tests are a separate script, `npm run +test:integration`, since they need a local .NET SDK and a built/published +`WitcherScriptMerger.Headless` - kept out of the default `npm test` so a Node-only +environment (e.g. a contributor machine or CI runner without the .NET SDK on `PATH`) +isn't forced through a multi-minute .NET build just to iterate on this extension's own +TypeScript. Two different `WitcherScriptMerger.Headless` invocations are involved: +`test/mcpClient.integration.test.ts` runs a plain `dotnet build` itself if the exe isn't +already present (framework-dependent, fast); `test/toolAcquisition.integration.test.ts` +instead runs `dotnet publish -c Release -p:PublishProfile=win-x64` (self-contained, +single-file, matching `.github/workflows/release.yml`'s own publish step exactly) if +that specific publish output isn't already present - slower on a cold run (produces a +~78 MB standalone exe) since it stands in for a downloaded-and-extracted release asset, +which the plain `dotnet build` output doesn't represent. ## Status diff --git a/vortex-extension/src/archiveExtractor.ts b/vortex-extension/src/archiveExtractor.ts index d004c06..d489bcc 100644 --- a/vortex-extension/src/archiveExtractor.ts +++ b/vortex-extension/src/archiveExtractor.ts @@ -3,13 +3,12 @@ import { types } from 'vortex-api'; /** * Extracts a downloaded WSM release archive. Deliberately goes through Vortex's own - * `api.openArchive`/`Archive.extractAll` (backed by the 7-zip archive handler Vortex - * itself ships and uses for every mod archive it installs - `@nexusmods/vortex-api`'s - * own `lib/api.d.ts` documents `openArchive(archivePath, options?, extension?): - * Promise` with an `extractAll(outputPath): Promise` member) rather than - * a hand-rolled zip reader or a new npm dependency - this is the idiomatic mechanism a - * Vortex extension already has for exactly this job, and it's what `IArchiveOptions` - * exists for. + * `api.openArchive`/`Archive.extractAll` (`@nexusmods/vortex-api`'s own `lib/api.d.ts` + * documents `openArchive(archivePath, options?, extension?): Promise` with an + * `extractAll(outputPath): Promise` member, backed by whatever archive-handler + * extension Vortex has registered for the file's format) rather than a hand-rolled zip + * reader or a new npm dependency - this is the idiomatic mechanism a Vortex extension + * already has for exactly this job. * * Behind a one-function interface (`ArchiveExtractor`) so `toolAcquisition.ts` stays * unit-testable with extraction stubbed - this real implementation is never exercised @@ -26,6 +25,10 @@ export function createVortexArchiveExtractor(api: types.IExtensionApi): ArchiveE async extractAll(archivePath: string, destDir: string): Promise { await fs.promises.mkdir(destDir, { recursive: true }); + // verify: true requests whatever integrity check Vortex's own archive handler + // supports (a CRC pass, or possibly nothing, depending on the handler) - its exact + // behavior is unverified here, like the rest of this real implementation (see this + // function's own doc comment above). const archive = await api.openArchive(archivePath, { verify: true }); if (!archive.extractAll) { throw new Error(