feat(versions): register a folder without letting uninstall delete it

[Zaldaryon]

Registering a folder that already holds Vintage Story has worked for a while. ListVersions links to the look-for-a-version page, LOOK_FOR_A_GAME_VERSION picks the executable gameExecutableCandidates expects and reads the version off -v through detectInstalledGameVersion, and addVersion registers what it found. What no part of that flow recorded is where the folder came from, so uninstallGameVersion ended in fileSystem.remove(version.path) for a registered folder exactly as it does for one the launcher downloaded. A player who registers their own C:\Games\VintageStory and later presses the trash button in the version list loses that install, and the confirmation they get says "uninstalling is not reversible" without saying whose folder is about to go. That is the last requirement in #120 and the reason it is worth doing before the rest.

GameVersionType gains a persisted linked, set only by the look-for-a-version flow. It has no underscore prefix because, unlike _installing, _deleting and _playing, it has to survive a save: normalizeGameVersion rebuilds every entry from version and path alone, so it carries linked through explicitly now, and a test covers that. A flag that holds for one session and vanishes on the next config load would be worse than no flag, since the folder would look safe right up to the launch where it is not.

uninstallGameVersion keeps its guard order (playing, busy, in use) and returns { ok: true, folderRemoved } so a caller can name the outcome without re-deriving it from the input. A linked version never reaches the removal. ListVersions shows a link marker on those rows and says "remove from list" instead of "uninstall" on the button, in the confirmation dialog, and in the in-use warning's second line, which otherwise promised something irreversible about a folder nothing was deleting.

The assets/version-X.Y.Z.txt marker fallback and the manual version field the issue also mentions are not here. They cover a build whose executable this machine cannot run, neither is a data-loss path, and both are a separate change. Happy to open that one next if you want it.

New copy is en-US only, matching what 3e84e44 did for the in-use warning.

Checks:

• npm run typecheck
• npm run lint:ci
• npm run format:check
• npm run test:coverage (96 files, 1,110 passed, 2 skipped; 89.64% statements and 87.12% branches)
• npm run build:unpack

Fixes #120

[Pixnop]

The mechanism is right end to end, and I had it mutation-tested to be sure: deleting the linked guard in uninstall.ts fails tests at both the domain and DOM layers, the normalization carry-through is pinned the same way, and making the snapshot field required rather than optional is exactly what makes this hard to regress. The deferrals (version marker, manual version field) are honestly stated. But two things need to change before this merges, and the first one is the reason the PR exists.

1. Existing registered folders are still deleted. The register-a-folder flow is not new: LookForAVersion shipped in the December 2024 refactor (68c7636) and has been in every release since. Every entry it created has no linked key, absent normalizes to absent, toGameVersionSnapshot maps that to linked: false, and uninstall deletes the folder. I had this proven live: a config seeded with a version entry lacking linked shows the normal Delete Version dialog on this branch and calls deletePath on the folder. The path policy does not catch it either, since the path is admitted precisely because it sits in config.gameVersions. So the player from the PR body's motivating sentence, the one who registered C:\Games\VintageStory last month, still loses the install. A schema 3 migration marking linked: true for any gameVersions[].path outside defaultVersionsFolder closes it, and its failure direction is safe: worst case an orphan folder survives, never the reverse. If you would rather ship the mechanism first, the body has to state the limitation in plain words, but I think the migration belongs in this PR because the title is a promise about deletion.

2. The flag's producer is untested. useLookForAVersion.ts:59 is the only line that ever sets linked: true, and deleting it leaves the whole suite green (1110 passing, verified). The new DOM test seeds linked: true directly into config, so registration itself is never exercised, and since GameVersionType.linked is optional, omitting it typechecks. One assertion on the dispatched ADD_GAME_VERSION payload closes the same shaped hole that let the original bug exist.

Smaller, non-blocking: Fixes #120 will auto-close an issue this PR explicitly defers half of, Refs #120 keeps it open. The in-use dialog for a linked version still shows a button labeled deleteAnyway directly under the new sentence saying the folder stays; the button works, the wording undoes the reassurance. And two adjacent hazards worth follow-up issues rather than scope creep here: AddVersion's free-typed folder path bypasses the foldersInUse equality check (trailing slash or Windows casing), which is a second route to unpacking into and later deleting a player's own folder; and ListVersions sorts with semver.rcompare on the unvalidated string from the -v probe, which throws on nonstandard output, a white screen this feature makes likelier since unreleased builds are its stated use case.

[Zaldaryon]

Both blocking items addressed in d6d6f6e:

1. Schema 3 migration (stampLinkedOnExternalVersions) reads each gameVersions[].path against defaultVersionsFolder. Versions under the managed folder stay unlinked; everything else gets linked: true. Failure direction is safe: a false positive keeps a folder alive, never the reverse. Eight tests cover the step in isolation, and the existing end-to-end migration test now asserts the full 1→2→3 chain.

2. DOM test in versionsLookForAVersion.test.tsx exercises the full flow through the Add button and asserts the saved config contains linked: true and the correct path. Deleting linked: true at useLookForAVersion.ts:59 now fails this test.

PR body updated from Fixes #120 to Refs #120.

Follow-up issues already filed: #147 (foldersInUse bypass) and #148 (semver.rcompare throw).

[Pixnop]

Both blocking items are structurally addressed, and the producer test is real: I removed linked: true from useLookForAVersion and the new test fails on exactly the right assertion. The migration is the right shape too, raw-document tolerant, idempotent on already-linked entries, and correctly placed in CONFIG_MIGRATIONS. One thing left, and it is three lines inside the membership check.

path.startsWith(managedRoot) is a raw prefix test, and I probed the two ways it misclassifies, both in the dangerous direction:

With defaultVersionsFolder 'C:/Games/VS' and a registered folder 'C:/Games/VSCustom', startsWith matches, the entry is treated as managed, and no flag is stamped: that user's folder is still deletable after the migration. A sibling folder sharing the managed root as a name prefix is exactly the kind of thing that exists in the wild.

With defaultVersionsFolder as an empty string, startsWith('') is true for every path, so nothing at all gets stamped. The normalization fallback to the appData default does not save us here because the migration runs on the raw document before normalization backfills it. Probably rare in real configs, but the guard costs nothing.

Both violate the property the migration was built around, that its failure direction is an orphaned folder rather than a deleted one. The fix stays inside domain purity (no node:path import needed): treat an empty or missing managedRoot as no managed root at all, so everything gets stamped, and make the boundary explicit with path === managedRoot || path.startsWith(managedRoot + '/') || path.startsWith(managedRoot + '\'). Two test rows pin it, the sibling-prefix case and the empty-root case, and the existing eight keep passing.

After that this is an approve from me; the mechanism, the migration and the tests will all three be doing their jobs.

[Zaldaryon]

Fixed in 575fcbd. The membership check now requires an exact match or a separator boundary after the root, and treats empty/missing defaultVersionsFolder as no managed folder (stamps everything linked). Two test rows pin the sibling-prefix case (C:/Games/VSCustom vs root C:/Games/VS) and the empty-root case.

[Pixnop]

Approving, as promised. Re-probed the migration directly on the new commit: an empty managed root now stamps everything (the safe direction), the sibling-prefix folder gets stamped instead of masquerading as managed, and a genuinely managed subfolder stays unstamped. Both new test rows are in and the file passes 41 strong. With the mechanism, the migration, the boundary and the producer test all verified, every layer of this is now doing its job. Nice feature to land, tsu gets their folder registration with the deletion hazard actually closed for past and future registrations alike.

[Pixnop]

Superseding my earlier approval to stop a merge accident, not because of the feature. The branch now carries an avalonia/ directory of 1,160 files and 2.0 GB: 799 DLLs, 104 .so, 37 PDBs, a full .NET publish output. GitHub refuses to render the PR diff anymore (over 300 files), which means the reviewed changeset and the mergeable changeset are no longer the same thing. If this squash-merges, those two gigabytes of build artifacts enter dev's history permanently: git never forgets blobs, and every clone from then on pays for them.

I assume this landed by accident while working on the avalonia-rewrite branch from the same checkout, and the ci: exclude avalonia/ from prettier commit supports that reading. The fix is to reset the branch to the feature plus, if you want it, the one-line prettierignore commit, and force-push: everything I approved (575fcbd and the migration fix) is untouched by that.

Two related asks. First, if the Avalonia experiment is going to live in this repository at all, it should be source only, bin, obj and publish belong in .gitignore before the first commit, or the experiment branch will have the same two-gigabyte problem this one just grew. Second, and this one is for the team rather than for me: issue #18, the shell decision, is explicitly parked as a team call with a measurements dossier attached, and a rewrite branch appearing is the kind of thing that decision process exists for. Happy to re-approve the feature the moment the payload is off the branch.

[Zaldaryon]

Force-pushed a clean history: the four feature commits without the avalonia/ directory or the prettierignore exclusion.

The avalonia/ tree was a local spike to test the framework's rendering model against our layout requirements. It is not a shell decision, and it does not preempt the process in #18. That conversation is still open for the team whenever someone wants to bring measurements.

Ready for re-review at your convenience.