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
45 changes: 45 additions & 0 deletions WitcherScriptMerger.Core/CLAUDE.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -205,6 +205,42 @@ inconsistently — full path for the regex, trimmed segment for the allowlist
related bug caught in the same review). Regression-tested via
`FileMergerTests.IsVanillaDlcBundleFolder_FolderNameMerelyEndsInPattern_ReturnsFalse`.

## The merged mod is excluded from the conflict scan

`ModFileIndex.BuildAsync` enumerates `Directory.GetDirectories(ModsDirectory, "mod*")`
and filters the result through `GetIgnoredModNames()`. That filter honors the
`IgnoreModNames` setting **and** always excludes the merged mod itself
(`MergedModName`, `mod0000_MergedFiles` by default).

Excluding it is not cosmetic. The merged mod is this tool's own *output*, but its
directory name starts with `mod`, so it matches the same glob as any source mod. Left in
the scan it becomes a merge input alongside the very mods it was built from, and each
subsequent run re-applies those mods' edits on top of already-merged text — **a
re-merge becomes cumulative instead of idempotent**. Inserted blocks accumulate one fresh
copy per run, and a losing most-distinct-from-vanilla tiebreak can additionally revert an
edit a previous run had kept. Confirmed on a real 249-mod install before the fix: a single
`modBloodAndSteel` insertion present 6× in `actor.ws` and a `modCriSlowMoCR` one 6× in
`damageManagerProcessor.ws` (each appears exactly once in the mod's own file), 37
duplicated mod-added lines across 11 of 42 merged files, and one `modTTMutagenSwap` edit
reverted outright. Nothing surfaced this as an error — the output stayed syntactically
valid and merged "successfully" every time, which is why it went unnoticed across
repeated merges.

The name-matching lives in `Paths.NormalizeMergedModName(string)` — a non-interactive,
argument-taking counterpart to `Paths.RetrieveMergedModName()`. The scan path must not use
the latter: it can prompt via `ConfirmInvalidModName` and message through
`AppState.Notifier`, neither of which may fire just because mod directories are being
enumerated. Both apply the same `Paths.MergedModNameMaxLength` (64) truncation, which is
what decides the directory name a merge actually writes — the two must agree or a scan
would fail to recognize the very directory the merge creates. `NormalizeMergedModName`
additionally trims, deliberately: its result is compared against a `DirectoryInfo.Name`,
which never carries surrounding whitespace.

`ModFileIndex.BuildIgnoredModNames(ignoreModNamesSetting, mergedModNameSetting)` is the
pure function behind `GetIgnoredModNames()`, split out so it's unit-testable without
touching `AppState.Settings` — see `WitcherScriptMerger.Tests/CLAUDE.md`'s
"`AppState.Settings`-safety constraints" and `FileIndex/ModFileIndexTests.cs`.

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

`Cli/MergeOperations.cs` is the scan-then-merge sequence shared by both hosts' `merge`
Expand DownExpand Up@@ -536,6 +572,15 @@ host's own `CLAUDE.md` for its own startup-level check; the MCP tools' own per-c
(`RequireDependenciesAndModsDirectory`, above) always uses the text-merge-only check
regardless of host.

Both hosts' `merge` CLI verbs now use the text-merge-only gate. The WinForms host's verb
previously used the combined `ValidateDependencyPaths()`, which made `merge` refuse to
start at all on an install whose conflicts were entirely flat-file (`.ws`/`.xml`) — the
common case, and the only category either headless path can resolve anyway. Since neither
QuickBMS nor wcc_lite is committed to this repo (see the root `CLAUDE.md`), a plain
clone-and-run hit that refusal every time, with an error message pointing at the GUI's
dependency setup for tooling the run didn't actually need. `ValidateDependencyPaths()` is
still there for callers that genuinely want the combined check.

## Hash format (`MergeInventory.xml`)

**Load-bearing.** `MergeInventory.xml` (including real, already-populated files on
Expand Down
35 changes: 32 additions & 3 deletions WitcherScriptMerger.Core/FileIndex/ModFileIndex.cs
Original file line numberDiff line numberDiff line change
Expand Up@@ -149,10 +149,39 @@ private List<ModFile> GetModFilesFromPaths(

private IEnumerable<string> GetIgnoredModNames()
{
var ignoredNames = AppState.Settings.Get("IgnoreModNames");
return ignoredNames.Split(',')
return BuildIgnoredModNames(
AppState.Settings.Get("IgnoreModNames"),
AppState.Settings.Get("MergedModName"));
}

// Split out from GetIgnoredModNames (above) as a pure function over the two raw
// setting values so it's unit-testable without touching AppState.Settings - see
// WitcherScriptMerger.Tests/CLAUDE.md's "AppState.Settings-safety constraints".
//
// The merged mod is always excluded, on top of whatever the user configured in
// IgnoreModNames. It is this tool's own *output*, not a source mod, but its
// directory name starts with "mod" and so matches BuildAsync's "mod*" glob like any
// other. Left in the scan it becomes a merge input alongside the very mods it was
// built from, and each subsequent merge re-applies those mods' edits on top of an
// already-merged file - inserted blocks accumulate a fresh copy per run (observed
// live: a single modBloodAndSteel insertion present 6 times in actor.ws, and a
// modCriSlowMoCR one 6 times in damageManagerProcessor.ws, after repeated merges),
// and a losing tiebreak can additionally revert an edit a previous run had kept.
// Excluding it by name is what makes a re-merge idempotent instead of cumulative.
public static List<string> BuildIgnoredModNames(string ignoreModNamesSetting, string mergedModNameSetting)
{
var ignoredNames = (ignoreModNamesSetting ?? string.Empty).Split(',')
.Where(name => !string.IsNullOrWhiteSpace(name))
.Select(name => name.Trim());
.Select(name => name.Trim())
.ToList();

var mergedModName = Paths.NormalizeMergedModName(mergedModNameSetting);
if (mergedModName != null &&
!ignoredNames.Any(name => name.EqualsIgnoreCase(mergedModName)))
{
ignoredNames.Add(mergedModName);
}
return ignoredNames;
}
}
}
34 changes: 32 additions & 2 deletions WitcherScriptMerger.Core/Paths.cs
Original file line numberDiff line numberDiff line change
Expand Up@@ -162,6 +162,36 @@ public static string RetrieveMergedBundlePath()
return null;
}

// The merged mod's directory name is capped at this length; anything longer in the
// MergedModName setting is truncated to it, and that truncation is what decides the
// directory actually written to. Named (rather than left as a literal in
// RetrieveMergedModName) so NormalizeMergedModName below can apply the same cap
// without restating it - the two must agree, or a scan would fail to recognize the
// very directory a merge writes.
public const int MergedModNameMaxLength = 64;

// Non-interactive counterpart to RetrieveMergedModName, for callers that only need
// to *recognize* the merged mod's directory name rather than resolve-and-validate a
// name to write to. Takes the raw setting value as an argument and touches neither
// AppState.Settings nor AppState.Notifier, so it's safe on a scan path (and directly
// unit-testable - see WitcherScriptMerger.Tests/CLAUDE.md's "AppState.Settings-safety
// constraints"): RetrieveMergedModName can prompt via ConfirmInvalidModName, which
// must never happen just because a scan is enumerating mod directories.
//
// Unlike RetrieveMergedModName this also trims, deliberately: the result is compared
// against a DirectoryInfo.Name, which never carries surrounding whitespace, so a
// setting value padded with spaces would otherwise silently fail to match and let the
// merged mod back into the scan - the exact failure this method exists to prevent.
public static string NormalizeMergedModName(string mergedModName)
{
if (string.IsNullOrWhiteSpace(mergedModName))
return null;
mergedModName = mergedModName.Trim();
return mergedModName.Length > MergedModNameMaxLength
? mergedModName.Substring(0, MergedModNameMaxLength)
: mergedModName;
}

public static string RetrieveMergedModName()
{
var mergedModName = AppState.Settings.Get("MergedModName");
Expand All@@ -170,8 +200,8 @@ public static string RetrieveMergedModName()
AppState.Notifier.ShowMessage("The MergedModName setting isn't configured in the .config file.");
return null;
}
if (mergedModName.Length > 64)
mergedModName = mergedModName.Substring(0, 64);
if (mergedModName.Length > MergedModNameMaxLength)
mergedModName = mergedModName.Substring(0, MergedModNameMaxLength);
if (!mergedModName.IsAlphaNumeric() || !mergedModName.StartsWith("mod"))
{
if (!ConfirmInvalidModName(mergedModName))
Expand Down
12 changes: 12 additions & 0 deletions WitcherScriptMerger.Tests/CLAUDE.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -45,6 +45,18 @@ does.
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.
- `FileIndex/ModFileIndexTests.cs` — `ModFileIndex.BuildIgnoredModNames`, the pure
function behind the mod-directory filter `BuildAsync` applies to its `"mod*"` glob.
Regression coverage for the merged mod being scanned as an ordinary source mod, which
made every re-merge cumulative rather than idempotent (duplicated insertions, and
occasionally a reverted edit) — see Core's `CLAUDE.md`'s "The merged mod is excluded
from the conflict scan" section for the mechanism and the real-install evidence. Covers
the exclusion happening with no `IgnoreModNames` configured at all (the bug itself), a
non-default `MergedModName`, user entries surviving alongside it, case-insensitive
de-duplication when the merged mod is already listed by hand (the pre-fix workaround),
a blank/unconfigured `MergedModName` adding no phantom entry, and the
`Paths.MergedModNameMaxLength` truncation and whitespace-trimming that keep the excluded
name equal to the directory name a merge actually writes.
- `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
154 changes: 154 additions & 0 deletions WitcherScriptMerger.Tests/FileIndex/ModFileIndexTests.cs
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
using WitcherScriptMerger.FileIndex;
using Xunit;

namespace WitcherScriptMerger.Tests.FileIndex
{
// Regression coverage for ModFileIndex.BuildIgnoredModNames - the mod-directory filter
// BuildAsync applies to Directory.GetDirectories(ModsDirectory, "mod*").
//
// Before this, that filter honored only the IgnoreModNames setting, so the merged mod
// (mod0000_MergedFiles by default) was scanned as an ordinary source mod: its name
// starts with "mod", so it matches the same glob. That made every re-merge cumulative
// rather than idempotent - the previous run's output became an input alongside the mods
// it was built from, so each run re-applied those mods' edits on top of already-merged
// text. Observed on a real 249-mod install: a single modBloodAndSteel insertion present
// 6 times in actor.ws and a modCriSlowMoCR one 6 times in damageManagerProcessor.ws
// (both appear exactly once in the mods' own files), 37 duplicated mod-added lines
// across 11 of 42 merged files, plus one modTTMutagenSwap edit reverted outright when
// the re-ingested output lost a most-distinct-from-vanilla tiebreak.
//
// These exercise the pure two-argument overload rather than GetIgnoredModNames, which
// reads AppState.Settings - see WitcherScriptMerger.Tests/CLAUDE.md's
// "AppState.Settings-safety constraints" for why tests must not touch that. Reaching
// Paths.NormalizeMergedModName/Paths.MergedModNameMaxLength through it is safe for the
// same reason those constraints exist: every static *field* initializer on Paths is
// settings-free by design (Path.Combine/literals only - see Paths.cs's own comment on
// why ScriptsDirectory et al. are properties, not cached fields), so touching a plain
// string helper there can't force AppState.Settings to construct and Environment.Exit
// the test host.
public class ModFileIndexTests
{
// The bug itself: nothing configured in IgnoreModNames must still not leave the
// merged mod in the scan.
[Fact]
public void BuildIgnoredModNames_NoIgnoreListConfigured_StillExcludesMergedMod()
{
var result = ModFileIndex.BuildIgnoredModNames("", "mod0000_MergedFiles");

Assert.Equal(new[] { "mod0000_MergedFiles" }, result);
}

[Fact]
public void BuildIgnoredModNames_NullIgnoreList_StillExcludesMergedMod()
{
var result = ModFileIndex.BuildIgnoredModNames(null, "mod0000_MergedFiles");

Assert.Equal(new[] { "mod0000_MergedFiles" }, result);
}

// A non-default MergedModName must be honored too - the exclusion follows the
// setting, not a hardcoded "mod0000_MergedFiles".
[Fact]
public void BuildIgnoredModNames_CustomMergedModName_ExcludesThatName()
{
var result = ModFileIndex.BuildIgnoredModNames("", "modAAA_MyMerges");

Assert.Equal(new[] { "modAAA_MyMerges" }, result);
}

// The user's own IgnoreModNames entries keep working alongside the added exclusion.
[Fact]
public void BuildIgnoredModNames_WithIgnoreList_KeepsBothUserEntriesAndMergedMod()
{
var result = ModFileIndex.BuildIgnoredModNames("modFoo,modBar", "mod0000_MergedFiles");

Assert.Equal(new[] { "modFoo", "modBar", "mod0000_MergedFiles" }, result);
}

[Fact]
public void BuildIgnoredModNames_IgnoreListEntriesAreTrimmed()
{
var result = ModFileIndex.BuildIgnoredModNames(" modFoo , modBar ", "mod0000_MergedFiles");

Assert.Equal(new[] { "modFoo", "modBar", "mod0000_MergedFiles" }, result);
}

[Fact]
public void BuildIgnoredModNames_BlankIgnoreListEntriesAreDropped()
{
var result = ModFileIndex.BuildIgnoredModNames("modFoo,, ,modBar,", "mod0000_MergedFiles");

Assert.Equal(new[] { "modFoo", "modBar", "mod0000_MergedFiles" }, result);
}

// Already listing the merged mod by hand (the pre-fix workaround) must not produce a
// duplicate entry, and must stay case-insensitive to match BuildAsync's own
// EqualsIgnoreCase comparison against DirectoryInfo.Name.
[Theory]
[InlineData("mod0000_MergedFiles")]
[InlineData("MOD0000_MERGEDFILES")]
[InlineData("Mod0000_mergedfiles")]
public void BuildIgnoredModNames_MergedModAlreadyInIgnoreList_NotDuplicated(string alreadyListed)
{
var result = ModFileIndex.BuildIgnoredModNames(alreadyListed, "mod0000_MergedFiles");

Assert.Single(result);
Assert.Equal(alreadyListed, result[0]);
}

[Fact]
public void BuildIgnoredModNames_MergedModAlreadyListedAmongOthers_NotDuplicated()
{
var result = ModFileIndex.BuildIgnoredModNames("modFoo,mod0000_MergedFiles,modBar", "mod0000_MergedFiles");

Assert.Equal(new[] { "modFoo", "mod0000_MergedFiles", "modBar" }, result);
Assert.Single(result, name => name.EqualsIgnoreCase("mod0000_MergedFiles"));
}

// An unconfigured/blank MergedModName must not add a phantom empty entry that would
// then match nothing (or, worse, everything).
[Theory]
[InlineData(null)]
[InlineData("")]
[InlineData(" ")]
public void BuildIgnoredModNames_NoMergedModNameConfigured_AddsNothing(string mergedModName)
{
var result = ModFileIndex.BuildIgnoredModNames("modFoo", mergedModName);

Assert.Equal(new[] { "modFoo" }, result);
}

[Theory]
[InlineData(null)]
[InlineData("")]
public void BuildIgnoredModNames_NothingConfiguredAtAll_ReturnsEmpty(string mergedModName)
{
Assert.Empty(ModFileIndex.BuildIgnoredModNames("", mergedModName));
}

// Surrounding whitespace in the setting must not stop the match: the comparison is
// against a DirectoryInfo.Name, which never carries any.
[Fact]
public void BuildIgnoredModNames_MergedModNamePadded_StillMatchesDirectoryName()
{
var result = ModFileIndex.BuildIgnoredModNames("", " mod0000_MergedFiles ");

Assert.Equal(new[] { "mod0000_MergedFiles" }, result);
}

// A MergedModName longer than the cap is truncated when the merge writes its output
// directory (Paths.RetrieveMergedModName), so the scan has to exclude the truncated
// name - excluding the untruncated one would miss the directory that actually exists.
[Fact]
public void BuildIgnoredModNames_OverlongMergedModName_ExcludesTheTruncatedName()
{
var overlong = "mod" + new string('A', WitcherScriptMerger.Paths.MergedModNameMaxLength);
var expected = overlong.Substring(0, WitcherScriptMerger.Paths.MergedModNameMaxLength);

var result = ModFileIndex.BuildIgnoredModNames("", overlong);

Assert.Equal(new[] { expected }, result);
Assert.Equal(WitcherScriptMerger.Paths.MergedModNameMaxLength, result[0].Length);
}
}
}
17 changes: 14 additions & 3 deletions WitcherScriptMerger/Program.cs
Original file line numberDiff line numberDiff line change
Expand Up@@ -180,11 +180,22 @@ static int RunCli(string[] args)
return 1;
}

if (!Paths.ValidateDependencyPaths())
// Only the text-merge engine (DiffPlexMergeEngine, always available - it's
// in-process) gates starting a merge run, matching WitcherScriptMerger.Headless's
// own merge verb. This deliberately does NOT also require QuickBMS/wcc_lite via
// Paths.ValidateDependencyPaths(): those are needed only for bundle-content
// conflicts, and requiring them up front made `merge` refuse to run at all on an
// install that has nothing but flat-file (.ws/.xml) conflicts - the common case,
// and the only case either headless path can resolve anyway. Neither binary is
// committed to this repo (see the root CLAUDE.md), so a plain clone-and-run hit
// this every time. Bundle-category conflicts still fail gracefully, per-conflict,
// when actually attempted without the tooling configured - see
// ModFileIndex.BuildAsync and FileMerger.GetUnpackedFiles (Core).
if (!Paths.ValidateTextMergeDependencies())
{
Notifier.ShowError(
"A required dependency (QuickBMS or wcc_lite) is missing. Configure its path " +
"in App.config, or run without arguments once to use the GUI's dependency setup.");
"The configured text-merge engine is missing or misconfigured. This shouldn't " +
"happen with the built-in DiffPlex engine - check for a corrupted install.");
return 1;
}

Expand Down
Loading