Correct the review count, give the extension a stable id, and put it under CI - #44
Open
TheValiantOne wants to merge 2 commits into
Open
Correct the review count, give the extension a stable id, and put it under CI#44TheValiantOne wants to merge 2 commits into
TheValiantOne wants to merge 2 commits into
Conversation
…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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FP8H6rBLCGPBFRSVsF3Kgw
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Four small pre-publication fixes. Bundled into one PR deliberately — review bandwidth is the current bottleneck (five PRs already open against a one-reviewer rule), and none of these touch code any other open PR touches.
1.
CONTRIBUTING.mdsaid 2 reviews; it's 1Verified against the real setting:
Worth fixing because it misrepresents what it actually takes to land anything — I quoted the stale number myself earlier in this work before checking the API.
2. The extension had no
idVortex uses
info.json'sidas the extension's stable identity. Without one, identity and the installed folder name derive from the archive filename, so an update can present as a different extension.package.mjsnow also fails wheninfo.json's version disagrees withpackage.json's. The zip is named from one and the manifest Vortex reads is the other, with nothing reconciling them — a one-sided bump ships an archive whose filename contradicts the version Vortex reports. Verified the guard fires by deliberately desyncing them:3. The extension had no CI at all
build.ymlran only .NET steps, whilevortex-extension/is roughly half of recent activity here — a TypeScript compile error, lint failure, or broken test could land onmainunnoticed. Added a job runningnpm ci+ typecheck + lint + test, on ubuntu, gated to runs where the extension changed.npm cirather thannpm installso a drifted lockfile fails instead of being silently rewritten. Thetest/integration suite is deliberately excluded — it spawns a real WSM binary CI has no copy of.4. Install docs were wrong in two ways
The README claimed the extension was "not yet published anywhere" — the
companion-0.1.0release exists. And it gave%APPDATA%\Vortex\pluginsas the install path, which is right only for a default per-user install; with shared storage Vortex usesC:\ProgramData\vortex, which is where this extension is actually installed on the machine this was written on. Both the README andpackage.mjs's own printed instructions now name both and say how to check.Verification
npm run typecheckandnpm run lintclean, 218 extension tests pass,npm run packageproduces a working zip, and the new version guard correctly rejects a desyncedinfo.json. The workflow YAML parses and both jobs are present.Context
These came out of a review pass over the repo's publication-readiness. Two other findings from that pass were fixed in their own branches rather than here: an unrelated
discoveredTool.tschange that an over-broadgit add -Ahad swept into #42 (removed — it's a real behaviour change to what a Tools-dashboard click does, and deserves its own review), and a comparer-dropping bug in #43.AI-assisted development
Produced by Claude Code (Opus 5). Every claim above was verified against the live repo/API rather than taken from documentation — which is how the review-count discrepancy surfaced in the first place.