Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions WitcherScriptMerger.Core/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,47 @@ Both are regression-tested in `WitcherScriptMerger.Tests`
(`LoadOrder/CustomLoadOrderTests.cs`, `Inventory/FileMergerTests.cs`) — see
`WitcherScriptMerger.Tests/CLAUDE.md`.

## Config-extensible vanilla-DLC-folder allowlist (`AdditionalVanillaDlcFolderNames`)

`IsVanillaDlcBundleFolder` has a second overload,
`IsVanillaDlcBundleFolder(string path, IEnumerable<string> additionalFolderNames)` — the
single-arg overload now just forwards to it with `Array.Empty<string>()`. The two-arg
overload ORs the built-in regex match with an exact, case-insensitive match of the
extracted trailing folder-name segment against `additionalFolderNames`, populated at
`GetUnpackedFiles`' one call site by parsing the `"AdditionalVanillaDlcFolderNames"`
App.config setting (comma-separated, trimmed of whitespace and stray trailing directory
separators — the same parse shape `FileIndex/ModFileIndex.GetIgnoredModNames` already
uses for the sibling `"IgnoreModNames"` setting). This exists so a future DLC/expansion
whose folder codename isn't recognized by the built-in regex yet (e.g. CD Projekt Red's
"Songs of the Past", announced in 2026 with no public folder name at time of writing)
doesn't need a code change — just a config entry.

**Deliberately stays an exact-match allowlist, never existence-based auto-discovery.**
Vortex's own `witcher3dlc` mod type deploys ordinary user mods into
`<GameDir>\DLC\<modname>\content\...` — the identical on-disk shape as real vanilla DLC
content — so treating "any folder under DLC" as a vanilla merge baseline would risk
silently merging a conflict against a mod's own bundle instead of vanilla's. The two-arg
overload deliberately takes the extra names as a plain parameter rather than reading
`AppState.Settings` itself, keeping it a pure, static, directly unit-testable function
with no config/`AppState` dependency (settings are read exactly once, at the
`GetUnpackedFiles` call site) — see this file's own "AppState & IMergeNotifier" section
above for why touching `AppState.Settings` from code a test exercises is a real hazard.

**The built-in regex itself is matched against the extracted folder-name segment, not
the raw path, and is anchored at both ends (`^...$`).** An earlier version matched an
end-anchor-only pattern (`"(DLC[0-9]*|ep[0-9]|bob)$"`) against the full path — since
.NET `Regex.IsMatch` has no implicit start anchor, that matched *any* folder name merely
*ending* in one of those substrings, not just a folder name that *is* one of them
(optionally + digits): e.g. `"ImmersiveDLC"` or `"Step1"` would have incorrectly
qualified as vanilla. Caught in code review while adding the allowlist above, since it's
exactly the same collision-with-an-arbitrary-mod-folder-name risk the allowlist's own
"never auto-discovery" rule exists to prevent. Fixed by extracting the folder-name
segment once, up front, and running both the regex check and the allowlist check against
that same normalized value (an earlier version also normalized the two checks
inconsistently — full path for the regex, trimmed segment for the allowlist — a second,
related bug caught in the same review). Regression-tested via
`FileMergerTests.IsVanillaDlcBundleFolder_FolderNameMerelyEndsInPattern_ReturnsFalse`.

## CLI & MCP orchestration (`Cli/`, `Mcp/`)

`Cli/MergeOperations.cs` is the scan-then-merge sequence shared by both hosts' `merge`
Expand Down
81 changes: 77 additions & 4 deletions WitcherScriptMerger.Core/Inventory/FileMerger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,20 @@ public class MergeReportData
bool _bundleChanged;
List<Merge> _pendingBundleMerges = new List<Merge>();

// Anchored at BOTH ends ("^...$") - see IsVanillaDlcBundleFolder's own comment
// below for why this matters: it's matched against just the extracted folder-name
// segment, not the full path, so a full-string match is required, not merely a
// suffix. Code review on the AdditionalVanillaDlcFolderNames addition caught that
// an earlier, end-anchor-only version of this pattern ("(DLC[0-9]*|ep[0-9]|bob)$",
// matched against the full path with no start anchor) would satisfy .NET Regex's
// "match anywhere in the string" default for ANY folder name merely ending in one
// of those substrings - confirmed to incorrectly match e.g. "ImmersiveDLC" (ends
// in "DLC") or "Step1" (ends in "ep1") - exactly the kind of arbitrary,
// attacker/mod-author-chosen folder name this allowlist's own doc comment warns
// about (a Vortex "witcher3dlc"-deployed mod folder with such a name would have
// silently qualified as a vanilla merge baseline).
static readonly Regex VanillaDlcBundleFolderPattern =
new Regex(@"(DLC[0-9]*|ep[0-9]|bob)$", RegexOptions.IgnoreCase | RegexOptions.Compiled);
new Regex(@"^(DLC[0-9]*|ep[0-9]|bob)$", RegexOptions.IgnoreCase | RegexOptions.Compiled);

// Matches a DLC-folder name that has its own "bundles" subfolder to search for a
// vanilla bundle: base-game expansions (DLC1, DLC2, ...), the "ep1"/"ep2" xpac
Expand All @@ -137,8 +149,52 @@ public class MergeReportData
// match could silently miss a real vanilla DLC folder on any platform, not just
// a case-sensitive one. Public and static (no instance state involved)
// specifically so it's directly unit testable, matching
// DiffPlexMergeEngine.BuildMerge's own reasoning for the same shape.
public static bool IsVanillaDlcBundleFolder(string path) => VanillaDlcBundleFolderPattern.IsMatch(path);
// DiffPlexMergeEngine.BuildMerge's own reasoning for the same shape. Forwards to
// the two-arg overload below with an empty extra-names list, so every existing
// caller/test keeps this regex-only behavior unchanged.
public static bool IsVanillaDlcBundleFolder(string path) =>
IsVanillaDlcBundleFolder(path, Array.Empty<string>());

// Extends the regex match above with an exact (case-insensitive) match of the
// trailing path segment against a caller-supplied allowlist, read from the
// "AdditionalVanillaDlcFolderNames" App.config setting (see GetUnpackedFiles'
// call site below) - the escape hatch for a future DLC/expansion whose folder
// codename isn't known yet (e.g. CD Projekt Red's "Songs of the Past", announced
// in 2026 with no folder name public as of this writing) without needing a code
// change here every time it happens. Deliberately stays an exact-match
// ALLOWLIST, never existence-based auto-discovery: Vortex's own "witcher3dlc" mod
// type deploys ordinary user mods into "<GameDir>\DLC\<modname>\content\..." -
// the identical on-disk shape as real vanilla DLC content - so treating "any
// folder under DLC" as a vanilla baseline would risk silently merging against a
// mod's own bundle instead of vanilla's, producing a silently wrong 3-way merge.
// Deliberately takes the extra names as a plain parameter rather than reading
// AppState.Settings itself, so this stays a pure, static, directly
// unit-testable function with no config/AppState dependency - see this project's
// CLAUDE.md and WitcherScriptMerger.Tests/CLAUDE.md for why touching
// AppState.Settings from code a test exercises is a real hazard (AppSettings'
// constructor calls Environment.Exit(1) when no config file is found next to the
// entry assembly, which is fatal to the whole `dotnet test` process, not just one
// test).
//
// The path is reduced to just its trailing folder-name segment ONCE, up front,
// and both the regex check and the allowlist check run against that same
// normalized value - deliberately not "regex against the raw path, allowlist
// against the trimmed segment" (an earlier version of this method did exactly
// that, an inconsistency code review also caught: a path with a trailing
// separator would normalize differently for each branch).
public static bool IsVanillaDlcBundleFolder(string path, IEnumerable<string> additionalFolderNames)
{
var folderName = Path.GetFileName(
path.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar));

if (VanillaDlcBundleFolderPattern.IsMatch(folderName))
return true;

if (additionalFolderNames == null)
return false;

return additionalFolderNames.Any(name => !string.IsNullOrEmpty(name) && name.EqualsIgnoreCase(folderName));
}

#endregion

Expand Down Expand Up @@ -724,14 +780,31 @@ bool GetUnpackedFiles(string contentRelativePath, ref MergeSource source1, ref M
// deliberately doesn't require QuickBMS/wcc_lite to attempt flat-file merges, so
// a bundle conflict can now reach this code without one. Flagged in code review,
// see CLAUDE.md.
// Read once per search, here rather than inside IsVanillaDlcBundleFolder
// itself, so that function stays a pure, static, AppState-free function
// safely callable from tests - see its own comment above. Comma-separated
// exact folder names (not regex fragments - see App.config's own
// description of this key), split/trimmed/filtered the same way
// ModFileIndex.GetIgnoredModNames already parses the pre-existing
// "IgnoreModNames" setting - plus an extra TrimEnd of stray directory
// separators a user might paste into the setting (e.g. "SongsOfThePast\"),
// since IsVanillaDlcBundleFolder compares against an already
// separator-trimmed folder name and would otherwise never match such an
// entry.
var additionalDlcFolderNames = AppState.Settings.Get("AdditionalVanillaDlcFolderNames")
.Split(',')
.Where(name => !string.IsNullOrWhiteSpace(name))
.Select(name => name.Trim().TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar))
.ToArray();

var bundleDirs =
(Directory.Exists(Paths.BundlesDirectory)
? Directory.GetDirectories(Paths.BundlesDirectory).Select(path => Path.Combine(path, "bundles"))
: Enumerable.Empty<string>())
.Concat(
Directory.Exists(Paths.DlcDirectory)
? Directory.GetDirectories(Paths.DlcDirectory)
.Where(IsVanillaDlcBundleFolder)
.Where(path => IsVanillaDlcBundleFolder(path, additionalDlcFolderNames))
.Select(path => Path.Combine(path, Paths.BundleBase, "bundles"))
: Enumerable.Empty<string>()
)
Expand Down
12 changes: 12 additions & 0 deletions WitcherScriptMerger.Headless/App.config
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,17 @@ CheckBundleContents Whether to check for mod conflicts in bundle file conten
than crashing if this is turned on regardless.
IgnoreModNames Which mod folders to ignore (separated by commas)

AdditionalVanillaDlcFolderNames
Extra vanilla DLC/expansion folder names (under GameDirectory\DLC)
to treat as a 3-way merge's vanilla baseline, in addition to the
built-in DLC1/DLC2/.../ep1/ep2/bob pattern - e.g. for a future
expansion whose folder codename isn't recognized yet. Comma-separated,
EXACT folder names only (not regex/wildcards) - this is a strict
allowlist, not existence-based auto-discovery, since some mod managers
(e.g. Vortex's "witcher3dlc" mod type) deploy ordinary user mods into
this same GameDirectory\DLC\<name>\content\... shape as real vanilla
DLC content. Leave blank unless you actually have such a folder.

MergedModName Which mod folder to save merges in (should be 1st alphabetically, so the game loads it before others)

QuickBmsPath Where quickbms.exe is located - Windows-only, has no effect unless this
Expand All @@ -39,6 +50,7 @@ since KDiff3MergeEngine needs Win32 P/Invoke that isn't available outside the Wi
<add key="CheckXmlFiles" value="true" />
<add key="CheckBundleContents" value="false" />
<add key="IgnoreModNames" value="" />
<add key="AdditionalVanillaDlcFolderNames" value="" />
<add key="MergedModName" value="mod0000_MergedFiles" />
<add key="QuickBmsPath" value="Tools\QuickBMS\quickbms.exe" />
<add key="QuickBmsPluginPath" value="Tools\QuickBMS\witcher3.bms" />
Expand Down
11 changes: 10 additions & 1 deletion WitcherScriptMerger.Tests/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,16 @@ does.
- `Tools/HasherTests.cs` — `Hasher`'s xxHash32 output, including synthetic edge cases.
- `Inventory/FileMergerTests.cs` — `FileMerger.IsVanillaDlcBundleFolder`: known vanilla
DLC-folder names, case-insensitivity, and non-matches (including anchoring) — see
Core's `CLAUDE.md`'s "Vortex-fork parity fixes" section.
Core's `CLAUDE.md`'s "Vortex-fork parity fixes" section. Also covers the two-arg
`IsVanillaDlcBundleFolder(path, additionalFolderNames)` overload backing the
`AdditionalVanillaDlcFolderNames` config allowlist: extra-name matches (including
case-insensitivity and a trailing path separator), the empty-list case still matching
everything the regex alone matches, an extra name absent from the list still returning
`false` (no accidental wildcard), a `null` extra-names list degrading to regex-only
instead of throwing, and a regression case for a real folder-name-anchoring bug caught
in code review (a folder name merely *ending* in a recognized substring, e.g.
`"ImmersiveDLC"`/`"Step1"`, must not match) — see Core's `CLAUDE.md`'s "Config-extensible
vanilla-DLC-folder allowlist" section.
- `LoadOrder/CustomLoadOrderTests.cs` — `CustomLoadOrder.ProcessLine`'s tolerance for
`mods.settings` "VK=" (VortexKey) lines, via reflection — see Core's `CLAUDE.md`'s
"Vortex-fork parity fixes" section.
Expand Down
97 changes: 96 additions & 1 deletion WitcherScriptMerger.Tests/Inventory/FileMergerTests.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using WitcherScriptMerger.Inventory;
using System;
using WitcherScriptMerger.Inventory;
using Xunit;

namespace WitcherScriptMerger.Tests.Inventory
Expand All @@ -10,6 +11,11 @@ namespace WitcherScriptMerger.Tests.Inventory
// internal folder codename) alternative, and a case-sensitive match that only ever
// worked by luck of Windows' case-insensitive filesystem - see
// FileMerger.cs's own comment on VanillaDlcBundleFolderPattern and Core's CLAUDE.md.
//
// The two-arg overload's own tests below additionally cover the
// "AdditionalVanillaDlcFolderNames" App.config setting (parsed and passed in by
// FileMerger.GetUnpackedFiles) - see IsVanillaDlcBundleFolder's own comment on why
// this must stay a strict allowlist, never existence-based auto-discovery.
public class FileMergerTests
{
[Theory]
Expand Down Expand Up @@ -52,5 +58,94 @@ public void IsVanillaDlcBundleFolder_NonVanillaFolders_ReturnsFalse(string path)
// substring anywhere earlier in a longer, unrelated folder name.
Assert.False(FileMerger.IsVanillaDlcBundleFolder(path));
}

[Theory]
[InlineData(@"C:\Witcher3\DLC\ImmersiveDLC")]
[InlineData(@"C:\Witcher3\DLC\Step1")]
[InlineData(@"C:\Witcher3\DLC\SomeBob")]
[InlineData(@"C:\Witcher3\DLC\PrefixDLC12")]
public void IsVanillaDlcBundleFolder_FolderNameMerelyEndsInPattern_ReturnsFalse(string path)
{
// Regression test for a real bug caught in code review while adding the
// two-arg overload below: VanillaDlcBundleFolderPattern used to be matched
// against the full path with only an end anchor ("(DLC[0-9]*|ep[0-9]|bob)$"),
// and .NET Regex.IsMatch has no implicit start anchor - so it matched ANY
// folder name merely ending in one of those substrings, not just a folder
// name that IS one of those substrings (optionally + digits). "ImmersiveDLC"
// (ends in "DLC"), "Step1" (ends in "ep1"), "SomeBob" (ends in "bob"), and
// "PrefixDLC12" (ends in "DLC12") would all have incorrectly matched under
// the old pattern. This is exactly the collision this whole feature's
// allowlist has to guard against - a Vortex "witcher3dlc"-deployed mod folder
// with an unlucky name would have silently qualified as a vanilla merge
// baseline. Fixed by matching a full "^...$"-anchored pattern against just the
// extracted folder-name segment instead of an end-anchored pattern against the
// raw path.
Assert.False(FileMerger.IsVanillaDlcBundleFolder(path));
Assert.False(FileMerger.IsVanillaDlcBundleFolder(path, Array.Empty<string>()));
}

[Fact]
public void IsVanillaDlcBundleFolder_NullAdditionalFolderNames_ReturnsRegexResult()
{
// The two-arg overload's additionalFolderNames is a public parameter a caller
// could pass null for - confirms that degrades gracefully to "regex-only"
// instead of throwing, for both a regex-matching and a non-matching path.
Assert.True(FileMerger.IsVanillaDlcBundleFolder(@"C:\Witcher3\DLC\DLC1", null));
Assert.False(FileMerger.IsVanillaDlcBundleFolder(@"C:\Witcher3\DLC\some_other_mod", null));
}

[Theory]
[InlineData(@"C:\Witcher3\DLC\DLC1")]
[InlineData(@"C:\Witcher3\DLC\DLC13")]
[InlineData(@"C:\Witcher3\DLC\DLC")]
[InlineData(@"C:\Witcher3\DLC\ep1")]
[InlineData(@"C:\Witcher3\DLC\ep2")]
[InlineData(@"C:\Witcher3\DLC\bob")]
public void IsVanillaDlcBundleFolder_TwoArgOverload_EmptyExtraNames_StillMatchesRegex(string path)
{
// The two-arg overload must keep matching everything the regex alone already
// matches when the extra-names list is empty - i.e. adding the overload must
// not regress the single-arg overload's existing behavior (which now forwards
// to this one with Array.Empty<string>()).
Assert.True(FileMerger.IsVanillaDlcBundleFolder(path, Array.Empty<string>()));
}

[Theory]
[InlineData(@"C:\Witcher3\DLC\SongsOfThePast", "SongsOfThePast")]
// Trailing separator on the path is trimmed before comparing the folder name.
[InlineData(@"C:\Witcher3\DLC\SongsOfThePast\", "SongsOfThePast")]
public void IsVanillaDlcBundleFolder_ExtraNameInAllowlist_ReturnsTrue(string path, string extraName)
{
// A synthetic future DLC/expansion folder name (not in the built-in regex at
// all) matches once it's supplied via the extra-names list - the escape hatch
// this overload exists for (e.g. CD Projekt Red's "Songs of the Past",
// announced in 2026 with no folder codename known yet).
Assert.True(FileMerger.IsVanillaDlcBundleFolder(path, new[] { extraName }));
}

[Theory]
[InlineData(@"C:\Witcher3\DLC\SongsOfThePast", "songsofthepast")]
[InlineData(@"C:\Witcher3\DLC\SONGSOFTHEPAST", "SongsOfThePast")]
[InlineData(@"C:\Witcher3\DLC\SoNgSoFtHePaSt", "sOnGsOfThEpAsT")]
public void IsVanillaDlcBundleFolder_ExtraNameCaseInsensitive_ReturnsTrue(string path, string extraName)
{
Assert.True(FileMerger.IsVanillaDlcBundleFolder(path, new[] { extraName }));
}

[Theory]
[InlineData(@"C:\Witcher3\DLC\SomeUnlistedMod", new[] { "SongsOfThePast" })]
[InlineData(@"C:\Witcher3\DLC\some_other_mod", new string[0])]
public void IsVanillaDlcBundleFolder_ExtraNameNotInAllowlist_ReturnsFalse(string path, string[] additionalNames)
{
// Confirms the extra-names list is a strict allowlist, not "treat any DLC
// subfolder as vanilla" - a folder that's neither in additionalFolderNames nor
// matched by the built-in regex must still return false. This is the case that
// matters most for the real-world risk this overload has to guard against:
// Vortex's "witcher3dlc" mod type deploys ordinary user mods into the identical
// GameDirectory\DLC\<modname>\content\... shape as real vanilla DLC content, so
// an accidental wildcard here would risk silently merging against a mod's own
// bundle instead of vanilla's.
Assert.False(FileMerger.IsVanillaDlcBundleFolder(path, additionalNames));
}
}
}
Loading
Loading