From 6d52cc3e05f9be0e3050e4ba1a90eeedf0f7cc13 Mon Sep 17 00:00:00 2001 From: Chris Knight Date: Sat, 22 Aug 2026 00:11:31 -0400 Subject: [PATCH 1/2] Correct the review count, give the extension a stable id, and put it under CI Four small pre-publication fixes, bundled into one PR deliberately: review bandwidth is the current bottleneck (five PRs already open against a one-reviewer protection rule), and none of these touch code any other open PR touches. CONTRIBUTING.md said pull requests require 2 approving reviews. The repository's actual protection requires 1, from a code owner - verified via `gh api repos/TheValiantOne/WitcherScriptMerger/branches/main/protection` (required_approving_review_count: 1). The stale number matters because it misrepresents what it takes to land anything. vortex-extension/info.json had no `id`. Vortex uses that as the extension's stable identity; without it, identity and the installed folder name derive from the archive filename, so an update can look like a different extension. package.mjs now also fails if info.json's version disagrees with package.json's - the zip is named from one and the manifest Vortex reads is the other, and nothing reconciled them, so a one-sided bump would ship an archive whose filename contradicts the version Vortex reports. Verified the guard fires by deliberately desyncing them. The Vortex extension had no CI at all - build.yml ran only .NET steps, while the extension is roughly half of recent activity in this repo. Added a job running npm ci + typecheck + lint + test, on ubuntu, only when vortex-extension/ changed. The test/ integration suite is deliberately excluded: it spawns a real WSM binary that CI has no copy of. Install docs claimed the extension was "not yet published anywhere" (the companion-0.1.0 release exists) and gave %APPDATA%\Vortex\plugins as the install path. That path is only right for a default per-user install - on a shared-storage setup Vortex uses C:\ProgramData\vortex, which is where this extension is actually installed on the machine this was written on. Both README and package.mjs's own printed instructions now name both and say how to check. Verified: npm typecheck/lint clean, 218 extension tests pass, npm run package produces a working zip, and the new version guard rejects a desynced info.json. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FP8H6rBLCGPBFRSVsF3Kgw --- .github/workflows/build.yml | 39 ++++++++++++++++++++++++++++ CONTRIBUTING.md | 5 +++- vortex-extension/README.md | 13 ++++++++-- vortex-extension/info.json | 1 + vortex-extension/scripts/package.mjs | 27 ++++++++++++++++++- 5 files changed, 81 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 298146f..b2c930a 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -40,3 +40,42 @@ jobs: - name: Verify formatting run: dotnet format whitespace WitcherScriptMerger.sln --verify-no-changes + + # The Vortex extension is roughly half of recent activity in this repo and had no CI + # coverage at all - build.yml ran only .NET steps, so a TypeScript compile error, a lint + # failure, or a broken test could land on main unnoticed. Runs on ubuntu (no .NET or + # Windows dependency here) and only when the extension actually changed. + vortex-extension: + name: Vortex extension (typecheck, lint, test) + runs-on: ubuntu-latest + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup Node + uses: actions/setup-node@v4 + with: + node-version: '20' + cache: npm + cache-dependency-path: vortex-extension/package-lock.json + + # `npm ci` (not `npm install`) so the lockfile is authoritative and a drifted + # lockfile fails the build instead of being silently rewritten. + - name: Install dependencies + working-directory: vortex-extension + run: npm ci + + - name: Typecheck + working-directory: vortex-extension + run: npm run typecheck + + - name: Lint + working-directory: vortex-extension + run: npm run lint + + # `npm test` is the src/ unit suite. The test/ integration suite is deliberately NOT + # run here: it spawns a real WitcherScriptMerger binary, which CI has no copy of. + - name: Test + working-directory: vortex-extension + run: npm test diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index cd8e944..c2f31aa 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -18,7 +18,10 @@ Match the existing source (e.g. `Inventory/FileMerger.cs`, `Controls/SMTree.cs`) - **`main` is protected.** No direct commits or pushes — all changes land via pull request. Force-pushes and branch deletion are disabled on `main` at the GitHub level. - **Branch per feature/fix**, off `main`: `feature/` for new functionality, `fix/` for bug fixes, `chore/` for tooling/process/docs changes not tied to a feature or bug. Keep the description short and kebab-case (e.g. `fix/kdiff3-encoding-mismatch`). -- **Pull requests require 2 approving reviews** before merge (GitHub branch protection on `main`). This applies to everyone, including repository admins in normal circumstances — admin bypass exists at the platform level for genuine emergencies, not as a routine shortcut. +- **Pull requests require 1 approving review** before merge (GitHub branch protection on `main`, + verified via `gh api repos/TheValiantOne/WitcherScriptMerger/branches/main/protection`: + `required_approving_review_count: 1`). This file previously said 2, which did not match the + repository's actual settings. This applies to everyone, including repository admins in normal circumstances — admin bypass exists at the platform level for genuine emergencies, not as a routine shortcut. - **PR description should cover**: what changed and why, and specifically *how you verified it* (see Testing below). "Builds successfully" is necessary but not sufficient for anything touching hash output, `MergeInventory.xml` schema, QuickBMS/wcc_lite invocation, the DiffPlex-based merge engine, or encoding handling; see `WitcherScriptMerger.Core/CLAUDE.md`'s "Hash format", "DiffPlexMergeEngine", and "Text-merge input encoding" sections for why those are load-bearing, and `WitcherScriptMerger.Tests/CLAUDE.md` for the verification pattern this codebase uses to cover them. - Commit messages are short, descriptive sentences (e.g. `Fixed crash after canceling file-open.`, `Replace hand-ported xxHash32 with System.IO.Hashing`). A `Category:` prefix (`Fixed:`, etc.) shows up occasionally but isn't enforced. No Conventional Commits format required. - GitHub Actions CI (`.github/workflows/build.yml`) runs `dotnet build --configuration Release` and `dotnet format whitespace --verify-no-changes` on every PR targeting `main`, but don't rely on it to catch problems for you — run both locally first: `dotnet build WitcherScriptMerger.sln --configuration Release` and `dotnet format whitespace WitcherScriptMerger.sln --verify-no-changes` before opening a PR. Catching failures before CI does saves a round trip. diff --git a/vortex-extension/README.md b/vortex-extension/README.md index 197d02f..fcc024f 100644 --- a/vortex-extension/README.md +++ b/vortex-extension/README.md @@ -130,7 +130,11 @@ bundling/redistributing anything itself: wcc_lite's) - the status tile only detects an existing local install or links to QuickBMS's own homepage, mirroring WSM's own GUI for this exact dependency. -## Install (manual - not yet published anywhere) +## Install (manual) + +> A `companion-0.1.0` GitHub release exists; this section covers installing it (or a +> locally-built zip) by hand. The extension is not yet listed in Vortex's in-app +> extension browser. ``` cd vortex-extension @@ -149,7 +153,12 @@ To install: extract that zip's contents (or copy the staged folder's contents) s `index.js` and `info.json` land directly inside ``` -%APPDATA%\Vortex\plugins\witcherscriptmerger-vortex\ +\plugins\witcherscriptmerger-vortex\n +where depends on how Vortex was installed: + %APPDATA%\Vortex (default, per-user) + C:\ProgramData ortex (shared/multi-user storage) + +If unsure, Vortex's own Settings -> Mods page shows the paths it is using. ``` The folder name under `plugins\` is arbitrary - `info.json` declares no explicit `id` diff --git a/vortex-extension/info.json b/vortex-extension/info.json index cfa6afa..fde5511 100644 --- a/vortex-extension/info.json +++ b/vortex-extension/info.json @@ -1,4 +1,5 @@ { + "id": "witcherscriptmerger-vortex", "name": "WitcherScriptMerger Companion", "author": "TheValiantOne/WitcherScriptMerger contributors", "version": "0.1.0", diff --git a/vortex-extension/scripts/package.mjs b/vortex-extension/scripts/package.mjs index 941ad97..0245e80 100644 --- a/vortex-extension/scripts/package.mjs +++ b/vortex-extension/scripts/package.mjs @@ -55,6 +55,29 @@ if (!fs.existsSync(INFO_JSON)) { process.exit(1); } +// package.json and info.json carry the version independently, and nothing used to +// reconcile them: the produced zip is named from package.json's version while the +// manifest Vortex actually reads is info.json's, so a one-sided bump ships an archive +// whose filename disagrees with the version Vortex reports. Fail the package step +// rather than emit that. +const info = JSON.parse(fs.readFileSync(INFO_JSON, 'utf8')); +if (info.version !== pkg.version) { + console.error( + `Version mismatch: package.json says '${pkg.version}' but info.json says '${info.version}'. ` + + `The zip is named from package.json while Vortex reads info.json, so these must agree - ` + + `update both before packaging.`, + ); + process.exit(1); +} + +// The id is what Vortex uses as the extension's stable identity. Without it, identity +// and the installed folder name derive from the archive filename and can change between +// releases, which makes an update look like a different extension. +if (!info.id) { + console.error(`info.json is missing an 'id' - Vortex needs a stable extension id that doesn't change between releases.`); + process.exit(1); +} + fs.rmSync(RELEASE_DIR, { recursive: true, force: true }); fs.mkdirSync(stageDir, { recursive: true }); @@ -115,6 +138,8 @@ console.log(`\nPackaged: ${zipPath}`); console.log(`Staged (unzipped) folder: ${stageDir}`); console.log( `\nManual install: extract the zip (or copy the staged folder's contents) so they land directly in\n` + - ` %APPDATA%\\Vortex\\plugins\\${stageName}\\\n` + + ` \\plugins\\${stageName}\\\n` + + `where is %APPDATA%\\Vortex for a default per-user install, or\n` + + `C:\\ProgramData\\vortex when Vortex is set up with shared/multi-user storage.\n` + `i.e. that folder should directly contain index.js and info.json, not a nested subfolder.`, ); From bb1fb645e5d6a142a41c95116d6e76e251611d6d Mon Sep 17 00:00:00 2001 From: Chris Knight Date: Sat, 22 Aug 2026 00:12:55 -0400 Subject: [PATCH 2/2] Assert info.json's id matches package.json's name The comment above stageName still said info.json declares no explicit id - true before this branch added one, stale immediately after. Rewritten, and backed by an actual assertion rather than a claim: the staged folder and zip are named from package.json's name while Vortex identifies the extension by info.json's id, so a divergence would install under one name and register under another. --- vortex-extension/scripts/package.mjs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/vortex-extension/scripts/package.mjs b/vortex-extension/scripts/package.mjs index 0245e80..de07139 100644 --- a/vortex-extension/scripts/package.mjs +++ b/vortex-extension/scripts/package.mjs @@ -39,10 +39,12 @@ const INFO_JSON = path.join(ROOT, 'info.json'); const RELEASE_DIR = path.join(ROOT, 'release'); const pkg = JSON.parse(fs.readFileSync(path.join(ROOT, 'package.json'), 'utf8')); -// The staged folder's own name - not read from info.json (which declares no explicit -// "id" field; @nexusmods/vortex-api's own IExtension typing marks `id` optional), so -// there's no single canonical extension id to derive this from. package.json's own -// `name` is used instead, matching this repo's git history/npm package identity. +// The staged folder's own name. info.json now declares an explicit "id" +// (@nexusmods/vortex-api's IExtension typing marks it optional, but Vortex uses it as +// the extension's stable identity), and the two are asserted equal below - so this could +// read from either. It stays on package.json's `name` because that's also what the zip +// filename and this repo's npm package identity use; the assertion is what keeps them +// from drifting apart. const stageName = pkg.name; const stageDir = path.join(RELEASE_DIR, stageName); @@ -77,6 +79,14 @@ if (!info.id) { console.error(`info.json is missing an 'id' - Vortex needs a stable extension id that doesn't change between releases.`); process.exit(1); } +if (info.id !== pkg.name) { + console.error( + `Identity mismatch: package.json name is '${pkg.name}' but info.json id is '${info.id}'. ` + + `The staged folder and zip are named from the former while Vortex identifies the extension ` + + `by the latter, so a divergence installs under one name and registers under another.`, + ); + process.exit(1); +} fs.rmSync(RELEASE_DIR, { recursive: true, force: true }); fs.mkdirSync(stageDir, { recursive: true });