diff --git a/CLAUDE.md b/CLAUDE.md index 40448cc..9a01d0c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,161 +1,125 @@ -# CLAUDE.md +# CLAUDE.md -This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. +This file provides guidance to Claude Code (claude.ai/code) when working with code in +this repository. It's deliberately short: detailed, project-specific guidance lives in +each project's own `CLAUDE.md`, linked below. Read this file first for orientation, then +follow the pointer to whichever project you're actually changing. ## Project overview -Script Merger for The Witcher 3 — a Windows desktop tool (WinForms, not WPF) that detects and merges conflicting mod script files. It scans a mod folder, finds `.ws`/`.xml` files (including inside `.bundle` packages) that multiple mods modify, and drives a 3-way merge (vanilla + mod1 + mod2) via an in-process DiffPlex-based merge engine (`Tools/DiffPlexMergeEngine.cs`, Core). `.bundle` package contents are unpacked with QuickBMS and repacked with wcc_lite. KDiff3 was formerly used for text merges instead — it was retired; see `docs/decisions/kdiff3-retirement.md` for why and for the empirical KDiff3 process-behavior findings preserved there now that the code is gone. - -This is a fork of the upstream `AnotherSymbiote/WitcherScriptMerger` repo, currently mid-modernization. A separate fork, `TheValiantOne/WitcherScriptMerger`, now exists and is this project's actual `origin` remote (default branch `main`) — `upstream` is a second configured remote pointing at `AnotherSymbiote/WitcherScriptMerger` (default branch `master`), kept for reference/pulling upstream changes, not as a PR target. This distinction is load-bearing for tooling, not just trivia: an earlier version of this sentence claimed no separate fork existed and that `origin` was still the upstream repo - false by the time it was written, and it cost a real mistake when it caused `gh pr create` (run with no `--repo`/`--base`) to silently default to opening a PR against `AnotherSymbiote/WitcherScriptMerger`'s `master` instead of this fork's `main`, visible in that unrelated repo's history until closed. Always pass `--repo TheValiantOne/WitcherScriptMerger --base main` explicitly (or otherwise confirm the target) when opening a PR from this repo. See `HANDOFF.md` at the repo root for the full rationale behind the fork and detailed gotchas hit during the .NET modernization — read it before picking up follow-on work in this repo. Of its original list of open goals, whitespace/diff-noise and a CLI mode (see "CLI mode" below) are done; dependency-packaging/licensing decisions are still open. An MCP server mode (see "MCP mode" below) was added afterward, beyond that original list, to let an MCP client (e.g. Claude Code) drive merges directly instead of only through the CLI. +Script Merger for The Witcher 3 (WSM) — a mod-script-merging tool. It scans a mod +folder, finds `.ws`/`.xml` files (including inside `.bundle` packages) that multiple mods +modify, and drives a 3-way merge (vanilla + mod1 + mod2) via an in-process, +DiffPlex-based merge engine — no external merge tool is required for flat-file +conflicts. `.bundle` package contents are unpacked with QuickBMS and repacked with +wcc_lite (external, Windows-only tools — see "External tool dependencies" below). + +There are three ways to run it: a WinForms GUI, a headless CLI verb, and an MCP server +mode (so an MCP client, e.g. Claude Code, can drive merges directly). A fourth, +Linux-capable host offers the CLI and MCP modes without the GUI. See "Architecture" +below. + +## Fork history + +This is a fork of the upstream `AnotherSymbiote/WitcherScriptMerger` repo, currently +mid-modernization. A separate fork, `TheValiantOne/WitcherScriptMerger`, is this +project's actual `origin` remote (default branch `main`) — `upstream` is a second +configured remote pointing at `AnotherSymbiote/WitcherScriptMerger` (default branch +`master`), kept for reference/pulling upstream changes, not as a PR target. + +**This distinction is load-bearing for tooling, not just trivia.** An earlier version of +this file claimed no separate fork existed and that `origin` was still the upstream repo +— false by the time it was written, and it cost a real mistake: it caused +`gh pr create` (run with no `--repo`/`--base`) to silently default to opening a PR +against `AnotherSymbiote/WitcherScriptMerger`'s `master` instead of this fork's `main`, +visible in that unrelated repo's history until closed. **Always pass +`--repo TheValiantOne/WitcherScriptMerger --base main` explicitly** (or otherwise confirm +the target) when opening a PR from this repo. + +See the local, gitignored `HANDOFF.md` at the repo root (not present in a fresh clone — +it's session-continuity context, not committed) for the full rationale behind the fork +and detailed gotchas hit during the .NET modernization, if it's present in your working +copy. ## Build & run -- Build: `dotnet build WitcherScriptMerger.sln` from the repo root. -- Run (GUI, no args): launch the built `WitcherScriptMerger.exe`, or `dotnet run --project WitcherScriptMerger/WitcherScriptMerger.csproj`. At startup the app validates `QuickBmsPath`/`QuickBmsPluginPath`/`WccLitePath` from `App.config` (`Paths.ValidateDependencyPaths` in `WitcherScriptMerger.Core/Paths.cs`) and shows a blocking `DependencyForm` if any are missing — the external binaries (QuickBMS, wcc_lite) are **not** in source control (see "External tool dependencies" below), so a fresh checkout won't run end-to-end without sourcing them separately. (KDiff3 used to be a third dependency validated here; it's been retired — see `docs/decisions/kdiff3-retirement.md`.) -- Run (CLI, headless): `WitcherScriptMerger.exe merge [--order-file ]` — see "CLI mode" below. Any arguments at all route to the CLI path instead of the GUI. -- Run (MCP server): `WitcherScriptMerger.exe mcp` — see "MCP mode" below. Speaks MCP over stdio; not meant to be run interactively from a terminal. -- Run (Linux-capable CLI/MCP-only host): `WitcherScriptMerger.Headless.exe merge [--order-file ]` or `WitcherScriptMerger.Headless.exe mcp` — see "Headless host (WitcherScriptMerger.Headless)" below. Same verbs, no GUI, flat-file conflicts only. -- Four projects, one `.sln`: `WitcherScriptMerger.Core` (WinForms-free class library, `net10.0`), `WitcherScriptMerger` (the WinForms host — GUI + CLI + MCP entry points, `net10.0-windows7.0`, references Core), `WitcherScriptMerger.Headless` (the Linux-capable CLI/MCP-only host, `net10.0`, references Core only), and `WitcherScriptMerger.Tests` (xunit, `net10.0`, references Core only). See "Architecture" below for what lives where. -- **Publishing self-contained single-file binaries** (no existing `.pubxml` profiles in this repo — these commands are the documented convention instead): - - WinForms host, `win-x64` only (it's a WinForms app — never makes sense on Linux): - `dotnet publish WitcherScriptMerger/WitcherScriptMerger.csproj -r win-x64 --self-contained -p:PublishSingleFile=true -c Release` - - Headless host, `win-x64`: - `dotnet publish WitcherScriptMerger.Headless/WitcherScriptMerger.Headless.csproj -r win-x64 --self-contained -p:PublishSingleFile=true -c Release` - - Headless host, `linux-x64` (cross-compiles fine from Windows — producing the binary doesn't require a Linux machine, only *running* it does): - `dotnet publish WitcherScriptMerger.Headless/WitcherScriptMerger.Headless.csproj -r linux-x64 --self-contained -p:PublishSingleFile=true -c Release` - - Each publish's `.dll.config` (the `App.config` copy `System.Configuration.ConfigurationManager` actually reads) lands next to the executable — copy it there if deploying the exe on its own. Confirmed empirically that this resolves correctly even in a single-file publish, despite `Assembly.GetEntryAssembly().Location` being documented (and confirmed here too, via a real build's `IL3000` warning) to always return `""` for a single-file-bundled assembly: `ConfigurationManager.OpenExeConfiguration("")` still finds and reads the real `.dll.config` sitting beside the actual running executable, both with and without that file present (missing-config still correctly triggers `AppSettings`'s existing `Environment.Exit(1)` path) — no `AppSettings.cs` change was needed for single-file publishing to work. - -### Tests - -`WitcherScriptMerger.Tests` (xunit) covers `WitcherScriptMerger.Core` — run with `dotnet test WitcherScriptMerger.Tests/WitcherScriptMerger.Tests.csproj` (or `dotnet test WitcherScriptMerger.sln`). It does **not** reference the host project (no WinForms) - `DiffPlexMergeEngine` (the sole text-merge engine; KDiff3 was retired, see `docs/decisions/kdiff3-retirement.md`) and other Core-side logic (`Hasher`, `FileEncoding`) are covered directly. -- Tests never construct `FileMerger.MergeSource` via `MergeSource.FromFlatFile`/`FromBundle` (both call `ModFile.GetModNameFromPath` → `Paths.ModsDirectory` → `AppState.Settings`) or otherwise force `AppState.Settings` to construct outside a real GUI/CLI/MCP entry point: `AppSettings`'s constructor calls `Environment.Exit(1)` if it can't find a config file next to `Assembly.GetEntryAssembly().Location`, and in a `dotnet test` host (`testhost.dll`, no matching `.config`) that kills the entire test process, not just one test. `AppState.Settings` is a lazy property specifically so that merely touching `AppState.Notifier` (which Core code — e.g. `DiffPlexMergeEngine`'s headless skip/guard messages — legitimately does on its own) doesn't also force `Settings` to construct; see `AppState.cs` and "Startup flow" below. `Paths.cs`'s own properties (`ScriptsDirectory`/`ModsDirectory`/`IsScriptsDirectoryDerived`/`IsModsDirectoryDerived`) read `AppState.Settings.Get(...)` on every access rather than caching the result via a static field initializer, for the identical reason one layer further out: a field initializer there would've forced `Settings` to construct merely from touching an unrelated static member of `Paths` (e.g. `GetRelativePath`), via C#'s beforefieldinit semantics, which would've silently undermined `AppState.Settings`' own laziness. `Tools/DiffPlexMergeEngine.GetConflictMarkerPath` reads only the compile-time-literal `Paths.DiffPlexConflictsDirectory` const, which never triggers `Paths`' type initializer at all, so it's safe to call from tests unconditionally. Tests that need a `FileMerger.MergeSource` build it directly via object-initializer syntax instead (its fields are all public). -- A few tests optionally cross-check against a real Witcher 3 + WitcherScriptMerger install (a live `MergeInventory.xml`'s recorded hashes, or a real `KDiff3.exe` binary a developer happens to have locally, for an auto-solvable-only A/B check against `DiffPlexMergeEngine` — WSM no longer bundles or requires one itself, see `docs/decisions/kdiff3-retirement.md`) via `WitcherScriptMerger.Tests/LiveInstall.cs`, gated entirely on the `WSM_TEST_GAME_DIR` environment variable (unset by default) — never a hardcoded or scanned path, per this repo's "scrub machine-specific paths" rule (see `CONTRIBUTING.md`). They silently no-op when it's unset. -- For anything not covered by the test project — especially further hash-output or `MergeInventory.xml` schema changes — the precedent set in `HANDOFF.md` still applies: a disposable, non-committed `dotnet new console` scratch app, exercising synthetic edge cases plus a cross-check against a real value already recorded in a live `MergeInventory.xml`. Follow that pattern rather than assuming API docs alone are sufficient, particularly for anything hash- or serialization-related (see "Compatibility constraints" below). +- Build everything: `dotnet build WitcherScriptMerger.sln` from the repo root. Single + `.sln`, four projects (see "Architecture" below) — there's no independent build for + any one of them beyond `dotnet build .csproj`. +- Test: `dotnet test WitcherScriptMerger.sln` — see `WitcherScriptMerger.Tests/CLAUDE.md` + for what's covered and its constraints. +- Format check (required before a PR): `dotnet format whitespace WitcherScriptMerger.sln --verify-no-changes`. +- Run/publish each entry point — see that project's own `CLAUDE.md`: + - GUI + CLI + MCP (Windows only): `WitcherScriptMerger/CLAUDE.md`. + - CLI + MCP, Linux-capable: `WitcherScriptMerger.Headless/CLAUDE.md`. ## Architecture -Four projects as of the Core/host split, the later addition of a test project, and the still-later addition of the Linux-capable Headless host (see git history around "Split into WitcherScriptMerger.Core + host GUI/CLI/MCP project" for the Core/host rationale): - -- **`WitcherScriptMerger.Core`** (`WitcherScriptMerger.Core/WitcherScriptMerger.Core.csproj`, `net10.0`, no WinForms reference — deliberately cross-platform-capable, toward eventual Linux support) holds all domain logic: file scanning, merge orchestration, load-order handling, settings/paths, and the CLI/MCP entry-point logic. Nothing in Core references `System.Windows.Forms`. -- **`WitcherScriptMerger`** (`WitcherScriptMerger/WitcherScriptMerger.csproj`, `net10.0-windows7.0`, `WinExe`, `UseWindowsForms=true`) is the host: WinForms GUI (`Forms/`, `Controls/`) and the three entry points (`Program.cs`). References Core via `ProjectReference`. No longer has a `Tools/` folder of its own — its one Win32-P/Invoke-using file, `Tools/KDiff3.cs`, was deleted along with KDiff3 (see `docs/decisions/kdiff3-retirement.md`). -- **`WitcherScriptMerger.Headless`** (`WitcherScriptMerger.Headless/WitcherScriptMerger.Headless.csproj`, `net10.0` — no `-windows` suffix, `Exe`) is the Linux-capable CLI/MCP-only host — no GUI, no reference to the WinForms host project at all. References Core only. See "Headless host (WitcherScriptMerger.Headless)" below for full detail. -- **`WitcherScriptMerger.Tests`** (`WitcherScriptMerger.Tests/WitcherScriptMerger.Tests.csproj`, xunit, `net10.0`) references Core only — see "Tests" above for what it covers and its `AppState.Settings`-safety constraints. - -There is no MVC/MVP split within the host — `Forms/MainForm.cs` (~1000 lines) is a monolithic orchestrator that directly owns the tree controls, constructs `ModFileIndex`/drives merges, and wires up async callbacks. - -Domain code in Core doesn't call into WinForms directly — it goes through `AppState.Notifier` (an `IMergeNotifier`, defined against neutral `NotifyResult`/`NotifyButtons`/`DialogIcon` types so Core never needs `System.Windows.Forms`), which is what makes CLI mode possible. `MainForm` (host) implements `IMergeNotifier` too, translating those neutral types to/from real `MessageBox.Show(...)`/`DialogResult`. The host's `Program.Notifier`/`Settings`/`LoadOrder`/`Inventory` are pass-through properties onto `AppState` (Core) — see "Startup flow" below. See "CLI mode" below for more. - -Folder map — **Core**: -- `FileIndex/` — scans the mods folder and builds the conflict index: `ModFileIndex.cs` (`BuildAsync` → `Conflicts`), `ModFile.cs`, `ModFileCategory.cs`. -- `Inventory/` — core merge domain + persistence: `FileMerger.cs` (headless orchestration plus TreeNode-free interactive orchestration — see "Interactive vs. headless split" below), `Merge.cs`, `MergeInventory.cs`, `FileHash.cs`, `MergeProgressInfo.cs`. -- `LoadOrder/` — mod load-order logic: `CustomLoadOrder.cs`, `LoadOrderComparer.cs`, `LoadOrderValidator.cs`, `ModLoadSetting.cs`. -- `Tools/` — wrappers that shell out to bundled external executables, plus the sole text-merge engine: `QuickBms.cs`, `WccLite.cs`, `Hasher.cs`, `FileEncoding.cs` (UTF-16LE+BOM normalization used by `DiffPlexMergeEngine` — see "Text-merge input encoding" under Compatibility constraints), `DiffPlexMergeEngine.cs` (the sole text-merge engine — see "Interactive vs. headless split" below; KDiff3 and the `IMergeEngine` interface that used to sit in front of it were retired, see `docs/decisions/kdiff3-retirement.md`), `FileOpener.cs` (portable "open in the OS's default associated app" helper — the only call site is `DiffPlexMergeEngine.MergeHeadless`, opening a genuine conflict's marker sidecar). -- `Cli/` — `MergeOperations.cs`: the scan-then-merge sequence shared by the `merge` CLI verb and the MCP tools. -- `Mcp/` — `WsmMcpTools.cs`: the MCP server's tool implementations. See "MCP mode" below. -- Root: `AppState.cs` (shared mutable state — `Notifier`/`Settings`/`LoadOrder`/`Inventory`), `AppSettings.cs`, `Paths.cs`, `StringExtensions.cs`, `IMergeNotifier.cs`, `NotifyTypes.cs` (the neutral `NotifyResult`/`NotifyButtons`/`DialogIcon` enums), `HeadlessMergeNotifier.cs`. - -Folder map — **host**: -- `Forms/` — WinForms screens: `MainForm.cs` (the hub, also implements `IMergeNotifier`, translating to/from real WinForms types), `OptionsForm.cs`, `DependencyForm.cs` (startup blocker if tool paths are invalid), `MergeReportForm.cs`, `PackReportForm.cs`, `PriorityPrompt.cs`, `MessageBoxManager.cs`. -- `Controls/` — custom `TreeView` subclasses: `SMTree.cs` (base, metadata/context-menu logic), `ConflictTree.cs` (detected conflicts), `MergeTree.cs` (existing merges), `SMTreeSorter.cs`, `ToolStripRegion.cs`. -- `Inventory/` — `InteractiveMergeRunner.cs`: the host-side counterpart to Core's `FileMerger` for the interactive path (see "Interactive vs. headless split" below). -- `Tools/` — empty as of KDiff3's retirement (`docs/decisions/kdiff3-retirement.md`); used to hold `KDiff3.cs` (Win32 P/Invoke for window-title polling) and `KDiff3MergeEngine.cs`. The sole text-merge engine, `DiffPlexMergeEngine`, lives in Core — see "Interactive vs. headless split" below. -- Root: `Program.cs` (entry point: GUI, CLI, and MCP), `Extensions.cs` (WinForms-specific `TreeNode`/`TreeView` helpers and Win32 P/Invoke — pure string helpers live in Core's `StringExtensions.cs` instead), `TaskbarProgress.cs`, `App.config`. - -Folder map — **Headless**: just `Program.cs` (entry point: CLI and MCP, no GUI) and its own `App.config` — see "Headless host (WitcherScriptMerger.Headless)" below. - -### Interactive vs. headless split (`FileMerger` / `DiffPlexMergeEngine`) - -Core's `FileMerger` never sees a `TreeNode`, `BackgroundWorker`, or `Forms.*` type. Its headless methods (`MergeConflictsHeadless` et al.) are unchanged in shape from before the split. Its interactive methods (`MergeFilesInteractive`, `MergeFlatFileInteractive`, `MergeBundleFileInteractive`, `MergeTextInteractive`) take a plain `InteractiveMergeRequest` (relative path, bundle flag, vanilla file path, ordered `MergeSource[]`) instead of `TreeNode[]`, and report back through `OnMergeReport`/`OnPackReport` callbacks instead of constructing `MergeReportForm`/`PackReportForm` directly. The host's `Inventory/InteractiveMergeRunner.cs` is the thing `MainForm` actually talks to: it extracts `InteractiveMergeRequest`s from checked `TreeNode`s (inside its `BackgroundWorker`'s `DoWork`, matching the pre-split threading model — extracting outside `DoWork` let a bad node throw synchronously on the UI thread instead of being captured by `BackgroundWorker`), owns the `BackgroundWorker`, and supplies the `OnMergeReport`/`OnPackReport` callbacks (report forms, completion sounds). - -Both `FileMerger.MergeText*` methods talk to a private `DiffPlexMergeEngine` field (`Merge`/`MergeHeadless`) constructed by `FileMerger`'s own constructor. There used to be an `IMergeEngine` interface between them, with two implementations — `KDiff3MergeEngine` (host, wrapping `Tools/KDiff3.Run`/`KDiff3.RunHeadless`) and `DiffPlexMergeEngine` (Core) — selectable via a `MergeEngine` App.config key. KDiff3 was retired (see `docs/decisions/kdiff3-retirement.md` for the full rationale and the empirical KDiff3 process-behavior findings preserved there); with only one implementation ever going to remain, the interface indirection was deleted along with it as premature abstraction (an interface with exactly one implementation for its whole remaining life) — not a documented repo-wide rule (CONTRIBUTING.md doesn't state one; an earlier version of this sentence claimed it did, which code review caught as a false citation), just this deletion's own reasoning, matching what `IMergeEngine.cs`'s own former doc comment already said about itself. `FileMerger` now calls `DiffPlexMergeEngine` directly. - -**`DiffPlexMergeEngine`** (Core, `Tools/DiffPlexMergeEngine.cs`) is the sole text-merge engine — in-process, built on the DiffPlex NuGet package (MIT-licensed), needing no external binary. It builds its own merge loop around `DiffPlex.ThreeWayDiffer.CreateDiffs` rather than calling `ThreeWayDiffer.CreateMerge` directly, so it can intercept `ThreeWayChangeType.Conflict` blocks itself: a conflict whose two sides are equal once whitespace is collapsed (joined-and-collapsed comparison over the classic ASCII whitespace set — space/tab/CR/LF/form-feed/vertical-tab, deliberately narrower than .NET regex's Unicode-aware `\s` so a genuine content difference that happens to be NBSP-vs-space isn't misclassified as whitespace-only — not per-line `Trim()`, and never applied when either side has zero pieces, since a real deletion must never be conflated with "the surviving side happens to collapse to empty too") auto-resolves by taking the first mod's side verbatim, mirroring the now-retired KDiff3 engine's `--cs "WhiteSpace3FileMergeDefault=2"` (confirmed against the KDiff3 source, preserved in `docs/decisions/kdiff3-retirement.md`: value 2 means "always pick input B", and KDiff3's own file order — vanilla, source1, source2 — maps source1 to B). A genuine conflict instead produces git/diff3-style conflict markers (`<<<<<<< ` / `||||||| Vanilla` / `=======` / `>>>>>>> `) written to a **sidecar** file under `Paths.DiffPlexConflictsDirectory` (a dedicated top-level `DiffPlexConflicts` folder — via `GetConflictMarkerPath`, keyed by an `XxHash32` of the full output path plus its filename) rather than to `outputPath` itself — writing markers to `outputPath` would make `FileMerger`'s pre-merge `File.Exists(_outputPath)` overwrite guard treat it as an already-completed merge and permanently skip retrying, since `HeadlessMergeNotifier` always declines the overwrite prompt. The sidecar went through two prior locations before landing here, each ruled out by direct end-to-end testing against the real CLI rather than by inspection alone: it originally lived right beside `outputPath` (`.conflict`), which code review flagged for two real problems (a flat-file conflict's `outputPath` sits inside the live `Paths.ModsDirectory` tree, which nothing ever cleans up; a bundle-content conflict's `outputPath` sits inside `Paths.MergedBundleContent`, which `Tools/WccLite.PackBundle` packs *wholesale* with no filtering, so a leftover `.conflict` file there could get embedded as bogus content into a later-packed `blob0.bundle`); it then moved under `Paths.TempBundleContent`, which fixed both of those but broke immediately in end-to-end testing for a third reason neither review nor unit tests caught - `FileMerger.CleanUpTempFiles()` deletes the entire `TempBundleContent` tree wholesale at the end of every headless merge run (to clear QuickBMS-unpacked bundle scratch content), so the sidecar was gone by the time the CLI process exited, before a user could ever see it. `Paths.DiffPlexConflictsDirectory` is an unrelated top-level name specifically to avoid that collision - see its own comment in `Paths.cs` for the full story. A conflict-marker file that later becomes an auto-solve on retry has its stale sidecar deleted before writing the fresh output. - -**The sidecar is opened for the user, not just written.** `MergeHeadless` writes the sidecar, then (unless `openConflictMarkers` is `false` - see below) calls `Tools/FileOpener.Open` (a swappable static `Func`, defaulting to `Process.Start` with `UseShellExecute = true` so it resolves the OS's file association rather than trying to execute the sidecar as a process image) on the sidecar path, and only then reports the skip via `AppState.Notifier.ShowMessage` - the open happens *before* the message, not after (an earlier version of this method did it the other way around; code review caught that the message needs `FileOpener.Open`'s own bool return to say whether the file actually opened, which requires calling it first). That bool distinguishes only "`Process.Start` succeeded" from "`Process.Start` threw" (e.g. `ERROR_NO_ASSOCIATION`) - it is not a guarantee an editor actually came up. Confirmed empirically during this feature's own end-to-end verification: on the machine used for testing, the sidecar's `.conflict` extension had no registered file association, so `Process.Start` succeeded by launching `OpenWith.exe` (the OS's own "how do you want to open this file?" picker) - `opened` was `true`, and the message correctly said "opened it for review" by its own definition of "opened" (the call didn't fail), even though what the user actually sees is a picker dialog, not directly an editor. `Merge()` (interactive) just runs `MergeHeadless()` and maps `NeedsManualResolution` to `Failed` since there's no UI here at all - this also means the sidecar-write-and-open behavior, and the "merging an updated mod file into an existing merge chain" outdated-hash guard, both fire identically whether reached via the GUI's interactive path or the CLI/MCP headless path, since it's the exact same underlying call either way. `MergeHeadless`'s `openConflictMarkers` parameter (default `true`) is the one thing that differs by caller: `FileMerger.MergeTextHeadless` passes `openConflictMarkers: !dryRun`, so a dry run (`MergeConflictsHeadless(dryRun: true)`, including the MCP `merge_conflicts` tool's `dryRun` option) still writes a genuine conflict's sidecar - unchanged, pre-existing behavior - but never launches anything for it, since a preview must not have that kind of side effect; a real (non-dry-run) run still opens every genuine conflict's sidecar with no cap on how many. Deliberately not `Program.TryOpenFile` (the host's existing, WinForms-adjacent equivalent, used elsewhere for opening merged output files): that helper's non-`.exe` branch is a bare `Process.Start(path)` with no `UseShellExecute = true`, which on modern .NET (unlike .NET Framework, where `UseShellExecute` defaulted to true) throws for a non-executable path — silently swallowed by that method's surrounding `catch`, and out of scope to fix there, but not a pattern worth propagating into this new code path. `Tools/FileOpener.cs` exists specifically so both the GUI-interactive and CLI/MCP-headless paths can reach a correctly-implemented version of this without Core referencing `System.Windows.Forms`. - -Two loose ends worth stating plainly rather than leaving implicit: nothing automatically deletes the `DiffPlexConflicts` directory itself (the same "accumulates until manually cleared" property `TempBundleContent` has, and the CLI-mode section below already tells users to clear that one between runs - `DiffPlexConflicts` needs the same manual housekeeping, just without an automated sweep); and `Paths.DiffPlexConflictsDirectory` is a relative path, resolved against `Environment.CurrentDirectory` - `Program.RunCli` sets that to `AppContext.BaseDirectory` before anything else runs (see "Startup flow" below), so in CLI mode sidecars land predictably next to the installed exe, but the GUI path never does that reset, so a GUI-mode conflict's sidecar lands wherever the process's CWD happened to be at launch (typically the exe's own directory too, for a normal double-click launch, but not guaranteed). - -A 3-way merge with no vanilla version at all (expected mainly on the bundle-content path, when no matching vanilla bundle is found, but the guard applies unconditionally to any conflict missing one) is refused outright (`NeedsManualResolution`, nothing written) rather than attempted with an empty base string — `DiffPlex.ThreeWayDiffer` degrades silently to zero diff blocks and a "successful" empty merge in that case, confirmed empirically, which would otherwise produce a truncated output file; this is a deliberate divergence from the now-retired KDiff3 engine, which had no equivalent guard and always attempted a real (if vanilla-less, degraded 2-way) `--auto` merge instead, since KDiff3 had a coherent notion of a 2-file merge that DiffPlex's `ThreeWayDiffer`, as used here, does not (see `docs/decisions/kdiff3-retirement.md`). See the "DiffPlex's `ThreeWayDiffer` can produce internally inconsistent diff blocks" bullet under Compatibility constraints below for a confirmed upstream DiffPlex bug this engine has to defend against on every merge, regardless of any of the above — the primary reason this retirement accepted a real, measured reliability gap in exchange for dropping KDiff3, rather than treating DiffPlex as a strict improvement. - -`FileMerger`'s constructor (`public FileMerger(MergeInventory inventory)`) builds its own `DiffPlexMergeEngine` — there's no longer an engine selection step at startup. - -### Startup flow (`Program.cs`) - -The shared mutable state that used to be static fields directly on `Program` (`Notifier`/`Settings`/`LoadOrder`/`Inventory`) now lives on Core's `AppState` instead — domain code that moved to Core needs to read/write it, and Core can never reference the host assembly. `Program.Notifier`/`Settings`/`LoadOrder`/`Inventory` are pass-through properties onto `AppState` so every pre-existing host call site kept working unchanged. `AppState.Notifier` defaults to `HeadlessMergeNotifier` via field initializer, unconditionally, so any startup error is safe to report even before it's known whether this is a GUI or CLI run. `AppState.Settings` is a **lazy property**, not a field initializer (`_settings ?? (_settings = new AppSettings())`) — deliberately decoupled from `Notifier`'s eager init: `AppSettings`'s constructor calls `Environment.Exit(1)` if it can't find a config file next to the entry assembly, appropriate for the real GUI/CLI/MCP entry points (where that's genuinely fatal) but not for `WitcherScriptMerger.Tests`, whose `dotnet test` host has no matching `.config` — Core code legitimately reads `AppState.Notifier` on its own (e.g. `DiffPlexMergeEngine`'s headless skip/guard messages), and before this change that alone was enough to also force `Settings` to construct (C# runs all of a type's static field initializers together on first touch of *any* static member) and crash the whole test process. First real access to `Settings` still runs the identical `new AppSettings()` and identical crash-on-missing-config behavior for the real app, just deferred to that access instead of bundled with `Notifier`'s. `AppState` has an explicit (empty) static constructor so its own init ordering is deterministic rather than left to `beforefieldinit`'s discretion — `Program` needs the same treatment for its own remaining field initializer (`_consoleAttached = MaybeAttachConsole()`) for the identical reason, now that nothing in `Main()` necessarily touches a `Program`-owned field anymore (its former fields became properties). `MaybeAttachConsole()` runs as a field initializer, ahead of everything, so early failures are visible in the invoking terminal when there are CLI args. - -`[STAThread] Main(string[] args)`: if `args` is non-empty, hands off entirely to the CLI path (see below) and returns — the GUI is never touched. Otherwise: `Application.EnableVisualStyles()` → check `Settings.HasConfigFile` → `Paths.ValidateDependencyPaths()` (shows `DependencyForm` if QuickBMS/wcc_lite paths are invalid) → construct `MainForm`, reassign `Program.Notifier = MainForm`, `Application.Run(MainForm)`. (There used to be an engine-selection step here, setting `AppState.MergeEngine` to `KDiff3MergeEngine` or `DiffPlexMergeEngine` based on a `MergeEngine` App.config setting, before anything else ran — removed along with `IMergeEngine` itself; see "Interactive vs. headless split" above and `docs/decisions/kdiff3-retirement.md`.) - -### Merge flow - -No hand-rolled diff algorithm lives in this codebase — it's an orchestrator around an in-process DiffPlex-based engine for text merges (KDiff3 formerly filled this role; see `docs/decisions/kdiff3-retirement.md`) and QuickBMS/wcc_lite for `.bundle` archives: - -1. `FileIndex/ModFileIndex.BuildAsync` scans `Paths.ModsDirectory`, groups files by relative path, flags conflicts. -2. `MainForm` feeds the results into `ConflictTree`/`MergeTree`; the user checks nodes to merge. -3. The host's `Inventory/InteractiveMergeRunner` extracts an `InteractiveMergeRequest` per checked `TreeNode` and runs `FileMerger.MergeFilesInteractive` on a `BackgroundWorker`; `MergeFilesInteractive` builds/reuses an `Inventory/Merge` record per file and dispatches to `MergeFlatFileInteractive` (plain `.ws`/`.xml`) or `MergeBundleFileInteractive` (bundle-packed files, which first go through `Tools/QuickBms.UnpackFile`) — see "Interactive vs. headless split" above for the full Core/host breakdown. -4. `FileMerger.MergeTextInteractive` calls `DiffPlexMergeEngine.Merge(source1, source2, vanillaFile, outputPath)`, which merges in-process (`--auto`-equivalent auto-solving; a genuine conflict writes and opens a conflict-marker sidecar instead of opening a merge GUI - see "Interactive vs. headless split" above). Before merging, it normalizes each input's text to UTF-16LE (matching vanilla's encoding) via `Tools/FileEncoding.ReadAnyEncoding` - see "Text-merge input encoding" below for why. -5. On success, `Inventory/MergeInventory.AddModToMerge` hashes the result via `Tools/Hasher` (xxHash32, `System.IO.Hashing`) and persists the merge record to `MergeInventory.xml` (`XmlSerializer`). `MergeInventory.HasResolvedConflict` re-checks these hashes on refresh to detect merges made stale by upstream mod file changes. -6. Bundle content changes additionally go through `FileMerger.PackNewBundle` → `Tools/WccLite.PackBundle` + `GenerateMetadata` to repack `blob0.bundle`. - -The CLI path (`FileMerger.MergeConflictsHeadless`) is a separate, parallel orchestration over the same domain objects — see "CLI mode" below. - -### CLI mode - -`WitcherScriptMerger.exe merge [--order-file ]` merges every auto-solvable conflict without opening any merge-tool window, then exits (a conflict needing manual resolution does open its conflict-marker sidecar in the default editor - see "Interactive vs. headless split" above - but that's a text file opening in whatever app is associated with it, not a dedicated merge-tool UI). No-args still launches the GUI unchanged; passing `merge` is what selects the CLI path in `Program.cs`. - -- **Orchestration**: `Cli/MergeOperations.ScanConflicts()` runs `ModFileIndex.BuildAsync` synchronously (via a `ManualResetEventSlim`) and returns the built index; `MergeOperations.RunMerge(inventory, conflicts, mergedModName, orderOverrides)` calls `FileMerger.MergeConflictsHeadless`. Both the `merge` CLI verb and the MCP tools (see "MCP mode" below) call these instead of duplicating the scan/wait/merge sequence. `MergeConflictsHeadless` iterates `ModFileIndex.Conflicts` directly — those are plain `ModFile`/`FileHash` objects (relative path, category, per-mod name and hash), so no `TreeNode`/`ConflictTree` is ever constructed for this path. `MergeFlatConflictHeadless` and `MergeBundleConflictHeadless` mirror `MergeFlatFileNode`/`MergeBundleFileNode`'s logic against that plain data instead of `TreeNode.GetMetadata()`; bundle merges reuse `GetUnpackedFiles`/`PackNewBundle` unchanged. Per-file mod order defaults to `LoadOrderComparer` (matching `ConflictTree`'s own default sort, `Controls/SMTreeSorter.cs`); the `--order-file` JSON (`{"relative\\path.ws": ["modA", "modB"]}`) overrides specific files without requiring every conflict to be listed. Within a listed file's mod list, `FileMerger.ResolveMergeOrder` requires: no unknown mod names, no duplicate names, at least two entries, and every one of that file's *real* source mods present at least once — any violation rejects that one file with a clear error (via `AppState.Notifier.ShowError`) rather than silently merging an incomplete, self-paired, or single-entry (no-op) chain. This applies to both the CLI's `--order-file` and the MCP `merge_conflicts` tool's `orderOverrides`, since both funnel through this shared method. "Real source mods" deliberately excludes the configured merged-mod name itself: once a file has already been merged once, its own merged-mod folder re-enters `conflict.Mods` as if it were a source (see `scan_conflicts`'s tool description below), so re-merging after a source mod's file changes only needs to list the actual mods again, not the previous merge output too — the at-least-two-entries rule still applies, though, so listing only that one remaining real mod (with the merged-mod folder excluded from the requirement) is rejected rather than silently merged with nothing to merge against. -- **`IMergeNotifier`** (Core: `IMergeNotifier.cs`, `NotifyTypes.cs`, `HeadlessMergeNotifier.cs`; host: `MainForm.cs`'s implementation): replaces every direct `Program.MainForm.ShowMessage/ShowError/ShowModal` call in domain code with `AppState.Notifier.*` (host code still spells this `Program.Notifier.*`, via the pass-through property). The interface is defined against neutral `NotifyResult`/`NotifyButtons`/`DialogIcon` types, not `DialogResult`/`MessageBoxButtons`/`MessageBoxIcon` — Core can't reference `System.Windows.Forms` at all. `MainForm` translates those neutral types to/from real `MessageBox.Show(...)`/`DialogResult` calls; this is **not** a behavior-identical passthrough — one real prompt (`LoadOrderValidator`'s "Custom Load Order Problem" dialog) lost a `MessageBoxManager`-based custom button caption that used to mark its Cancel option as destructive/permanent (that caption mechanism was likely already silently broken pre-split — `MessageBoxManager.Register()`'s `AppDomain.GetCurrentThreadId()`-based hook doesn't reliably work on modern .NET — but the loss is real either way; the warning is now spelled out in the message body instead). `ShowModal(Form)` isn't part of `IMergeNotifier` at all — every call site is GUI-only, interactive code in the host project, which calls `MainForm.ShowModal` directly instead of going through the notifier abstraction. `HeadlessMergeNotifier` writes to the console and returns a fixed, non-destructive default for every decision: don't overwrite an existing merge output, don't use a merge name that's still conflicting, don't continue past a canceled/failed merge — except where a caller explicitly overrides that generic default via `ShowMessage`'s `defaultResult` parameter (added for `LoadOrderValidator`, whose YesNoCancel prompt has an inverted-from-usual safety shape: Cancel, not Yes/No, is the one destructive/permanent choice there). This is also what fixed a real null-ref hazard in `CustomLoadOrder.Refresh()`, which used to reach `Program.MainForm` at construction time. -- **`DiffPlexMergeEngine.MergeHeadless`**: see "Interactive vs. headless split" above for the full mechanics (whitespace-only auto-resolve, conflict-marker sidecar, `FileOpener.Open`). This used to be `KDiff3.RunHeadless`, which needed a window-persistence-detection poll loop, a scratch-output-then-copy-on-clean-exit pattern to guarantee a killed process could never leave a partial file at the real output path, and a best-effort foreground-focus restore after KDiff3's unsuppressible popup closed — none of that machinery is needed anymore, since this engine never opens a window at all. See `docs/decisions/kdiff3-retirement.md` for that machinery's full empirical writeup, preserved now that the code itself is gone. -- **Verification status**: the flat-file path (`Categories.Script`/`Categories.Xml`) is verified end-to-end against real conflicting files (including the encoding-mismatch scenario) and a synthetic guaranteed-conflict, in a scratch game/mods tree — never against a live install. The bundle path (`Categories.BundleText`) is code-reviewed and mirrors the proven flat-file orchestration, and its two building blocks (`GetUnpackedFiles`, `PackNewBundle`) are unchanged, already-exercised code — but it hasn't been round-tripped through a real bundle-vs-bundle conflict. If `tempbundlecontent` accumulates across many CLI runs during development, clear it between runs — one debugging session saw a single very slow run after a long buildup that didn't reproduce once it was cleared. - -### MCP mode - -`WitcherScriptMerger.exe mcp` runs an MCP (Model Context Protocol) server over stdio, using the official `ModelContextProtocol` NuGet package (`Host.CreateApplicationBuilder().Services.AddMcpServer().WithStdioServerTransport().WithToolsFromAssembly()`, wired up in `Program.cs`'s `RunMcp`). It's a third entry point alongside the GUI and the `merge` CLI verb — same `args.Length > 0` routing in `Main`, dispatched on `args[0] == "mcp"`. Lets an MCP client (e.g. a Claude Code session) inspect conflicts and drive merges directly instead of only through one-shot CLI invocations. - -- **Why stdio, and why logging goes to stderr**: an MCP client spawns WSM as a child process and communicates over its redirected stdin/stdout pipes, so stdout must stay reserved for protocol frames — `RunMcp` configures `builder.Logging.AddConsole(o => o.LogToStandardErrorThreshold = LogLevel.Trace)` to keep the SDK's own request-handler logging off stdout. `Program.MaybeAttachConsole()` also skips `AttachConsole` for the `mcp` verb specifically (checks `args[1]` before calling it) — there's no parent console to attach to in this scenario, and it's pointless at best. Verified: a hand-rolled stdio client (`initialize` → `tools/list` → `tools/call` for each tool) round-tripped clean JSON-RPC on stdout with all SDK logging correctly landing on stderr. -- **Tools** (`Mcp/WsmMcpTools.cs`, `[McpServerToolType]` static class): `scan_conflicts` (no args — scans and returns every conflict's relative path, category, per-mod hashes, default merge order, and whether it's already resolved), `merge_conflicts` (optional `relativePaths` to restrict to specific files, optional `orderOverrides` — same shape as the CLI's `--order-file`, minus the file, and validated per-file the same way (see "CLI mode" above — no duplicates, every real source mod covered), optional `dryRun` to preview which conflicts would auto-solve without writing merged output, repacking a bundle, or touching `MergeInventory.xml`; returns `{merged: [...], skipped: [...], unmatched: [...], dryRun}` — `unmatched` lists any requested `relativePaths` entry that's in-scope but no longer a detected conflict), `get_status` (dependency validation, resolved game/mods directories, configured merged-mod name, current conflict count), `list_merges` (enumerates `MergeInventory.xml`'s existing `Merge` records). All four reuse `Cli/MergeOperations` and the same `IMergeNotifier`/`HeadlessMergeNotifier` machinery as the `merge` CLI verb — merge decisions get the same safe non-destructive defaults either way. -- **Dependency gating only requires the text-merge engine, not QuickBMS/wcc_lite too**: `scan_conflicts`/`merge_conflicts` (via `WsmMcpTools.RequireDependenciesAndModsDirectory`) and `get_status`'s `conflictCount` computation all gate on `Paths.ValidateTextMergeDependencies()` rather than the combined `Paths.ValidateDependencyPaths()` — added for `WitcherScriptMerger.Headless` (see "Headless host" below), which has no QuickBMS/wcc_lite bundled at all, but this is a real behavior change for the WinForms host's MCP mode too, not just the new host: previously, a missing QuickBMS/wcc_lite path made every `scan_conflicts`/`merge_conflicts` call fail outright, even for a mods folder with zero bundle-category conflicts. `get_status` now reports `textMergeDependenciesValid`/`bundleDependenciesValid` separately alongside the original combined `dependenciesValid` (kept for existing callers). Bundle-category conflicts still fail per-conflict, gracefully, when QuickBMS/wcc_lite aren't available — see `Paths.ValidateBundleDependencies`, `Tools/QuickBms.IsAvailable`, and `FileIndex/ModFileIndex.BuildAsync`'s single up-front warning (not per-bundle) when bundle checking can't proceed. -- **Directory allow-listing**: `merge_conflicts`'s `relativePaths` and `orderOverrides` keys are validated (`WsmMcpTools.EnsureInScope`/`IsWithinModsDirectory`) to resolve inside `Paths.ModsDirectory` before any scan or merge runs — an absolute path, UNC path, or `..\`-escaping entry is rejected with a clear error instead of silently matching nothing. Neither value is actually joined into a filesystem path anywhere in this codebase today (`relativePaths` is only ever compared for equality against already-scanned `ModFile.RelativePath` values; `orderOverrides` values reach `Path.Combine` in `FileMerger.GetModFile` but only after being validated against `ModFile.ContainsMod`, a whitelist of real scanned mod folder names) — this check is defense-in-depth, not a fix for a live traversal. This check applies mods-directory-relative semantics uniformly to every category; for `Categories.BundleText` specifically, `conflict.RelativePath` is actually a path *internal to a bundle archive* (from `QuickBms.GetBundleContentPaths`), not one rooted at `Paths.ModsDirectory` — an ordinary internal path (e.g. `engine\foo.ws`) still validates fine, but a bundle whose internal listing itself contained a rooted or `..`-bearing entry would make that specific conflict unreachable via `relativePaths` (rejected as out-of-scope even though it's a legitimate conflict). Unexercised: every scratch config used to verify this unit has `CheckBundleContents=false`, consistent with the bundle path's existing "code-reviewed but not round-tripped" verification status below. See `Mcp/CLAUDE.md` for the minimal-permissions summary this unit added. -- **State per call, not cached across calls**: every tool call re-scans (`ModFileIndex.BuildAsync`) and re-loads `MergeInventory.Load(Paths.Inventory)` fresh rather than keeping a long-lived server-side cache, since the mods folder or `MergeInventory.xml` can change between calls (including from a concurrently-running GUI instance) and there's no test suite to catch a staleness bug. -- **Verification status**: smoke-tested end-to-end against the same scratch game/mods tree used for CLI mode verification — `initialize`, `tools/list`, and all four tools called via a hand-rolled stdio client, including a `merge_conflicts` call that exercised the (since-retired) `KDiff3.RunHeadless` path at the time (detected a genuine conflict, killed the stuck process, returned it in `skipped`) — the underlying engine has changed since (see `docs/decisions/kdiff3-retirement.md`), but the MCP-level behavior this verified (a conflict comes back in `skipped`, not silently dropped) is unaffected. Never run against a live install. The directory-allow-listing and `dryRun` additions were verified against a real KDiff3 stand-in (dependency paths present but not a real KDiff3.exe, so merges reliably fail/skip) via the same stdio-client approach, plus an in-process harness against `WitcherScriptMerger.Core` directly with a fake `IMergeEngine` (both now historical - `IMergeEngine` no longer exists; real KDiff3/QuickBMS/wcc_lite weren't available in that verification environment) — see the PR that introduced them for exactly what each covered. - -### Headless host (`WitcherScriptMerger.Headless`) - -`WitcherScriptMerger.Headless` (`WitcherScriptMerger.Headless/`, `net10.0`, `Exe`, no `-windows` TFM suffix) is a second, much smaller executable: only the `merge` CLI verb and `mcp` server mode exist here, with no WinForms reference anywhere in the project and no GUI code path to fall back to — a first concrete step toward "true headless operation for CLI/Agent interaction, focused on modded-gaming + Vortex workflows." References `WitcherScriptMerger.Core` only. Build/run/publish commands are under "Build & run" above. - -- **Routing** (`WitcherScriptMerger.Headless/Program.cs`): mirrors `WitcherScriptMerger/Program.cs`'s `args[0] == "merge"` / `args[0] == "mcp"` dispatch, but with no third (no-args-launches-GUI) branch — no args, or an unrecognized first argument, prints usage to stderr and exits 1. `AppState.MergeEngine` is set unconditionally to `new DiffPlexMergeEngine()` — there's no `KDiff3MergeEngine` available here (it needs `Tools/KDiff3.cs`'s Win32 P/Invoke, which stays host-only) and no `MergeEngine` App.config switch either, since this host has exactly one engine, always. `Environment.CurrentDirectory = AppContext.BaseDirectory` is set as the very first statement (before touching `AppState.Settings`/`Paths` at all) for the same reason `Program.RunCli` does it on the WinForms host — several Core paths (`Paths.Inventory`, `Paths.TempBundleContent`, `Paths.DiffPlexConflictsDirectory`, `Paths.MergedBundleContentAbsolute`'s field initializer) are relative to it. No `[STAThread]`, no `AttachConsole`/`MaybeAttachConsole` P/Invoke (`kernel32.dll`, Windows-only) — this project has nothing console-attach-shaped to do (it's always launched as a plain console app, and stdout is reserved for MCP protocol frames in `mcp` mode regardless), so that Windows-specific mechanism was left out entirely rather than ported. The actual scan/merge/MCP-tool orchestration is unchanged, shared Core code (`Cli/MergeOperations.cs`, `Mcp/WsmMcpTools.cs`) — this project only replicates the thin CLI argument-parsing/dispatch glue around it, which was small enough not to warrant extracting into Core too. -- **Flat-file conflicts only — bundle-content conflicts are unsupported, by design, not by oversight.** This host has no QuickBMS/wcc_lite bundled at all (see "External tool dependencies" below and `docs/decisions/bundle-format-replacement-spike.md` — no cross-platform replacement was found to exist). `App.config`'s `CheckBundleContents` defaults to `false` here (unlike the WinForms host's `true`) specifically so a normal run never touches bundle scanning at all. If a user turns it on anyway (or points `QuickBmsPath`/`WccLitePath` at real, sourced-separately Windows binaries — those settings still exist here and work if this host happens to be run on Windows), bundle-category conflicts fail gracefully rather than crashing: `FileIndex/ModFileIndex.BuildAsync` checks `Tools/QuickBms.IsAvailable` (exe + plugin both present) once per scan, not once per bundle, and if unavailable, prints one clear message ("Bundle-content conflicts aren't supported without QuickBMS and wcc_lite configured…") and skips bundle scanning entirely for that run instead of attempting it. This replaced a real crash this unit found by code inspection: `Tools/QuickBms.GetBundleContentPaths` used to return `null` when QuickBMS couldn't be found, and both `ModFileIndex.BuildAsync` and `Inventory/FileMerger.GetUnpackedFiles` enumerated that return value directly — reachable for the first time by this host, since the WinForms host always gates bundle-category scanning behind the combined `Paths.ValidateDependencyPaths()` (real QuickBMS guaranteed present) before it's ever reached. Fixed at the source: `GetBundleContentPaths` now returns `Array.Empty()` instead of `null`. `FileMerger.GetUnpackedFiles`'s vanilla-bundle search (`Directory.GetDirectories(Paths.BundlesDirectory)`/`Paths.DlcDirectory`) is also guarded against a missing `content`/`DLC` directory now (`DirectoryNotFoundException` otherwise) — for the same reason, a scratch/incomplete game tree can now reach this code without a full real Witcher 3 install backing it. Verified end-to-end in a scratch tree: a mod folder containing a junk `.bundle` file, with `CheckBundleContents=true` and no QuickBMS configured, scans and merges cleanly (flat-file conflicts still merge/skip correctly; the bundle file is never opened at all) with the one clear warning message and no exception, on both Windows and (see below) real Linux. -- **Cross-platform path-separator bugs found and fixed via real Linux testing, not just cross-compilation.** Two genuine bugs surfaced only by actually running the `linux-x64` publish under WSL2 (a real Linux kernel, not just a cross-compile target check) — building/publishing for `linux-x64` alone would not have caught either: - - `FileIndex/ModFile.GetModNameFromPath` used a hardcoded `'\\'` to find the mod-folder-name segment of a full path. On Linux, `Path.Combine`-built paths use `/`, so `IndexOf('\\')` always returned `-1`, and the subsequent `Substring(0, -1)` threw `ArgumentOutOfRangeException` on literally the first flat-file merge attempted — a hard crash on every `merge` invocation. Fixed to use `Path.DirectorySeparatorChar`. - - `Mcp/WsmMcpTools.cs`'s `merge_conflicts` normalized a client-supplied `relativePaths`/`orderOverrides` key by replacing `/` with a hardcoded `'\\'` to match `ModFile.RelativePath`'s separator convention — correct on the WinForms host (always Windows), silently wrong on Linux, where `ModFile.RelativePath` itself uses `/`: a client sending a `/`-separated path (the natural style on any OS) would get "normalized" to `\`-separated, never match, and land in `unmatched` looking like it wasn't a real conflict at all. Fixed to normalize both possible separators to `Path.DirectorySeparatorChar` instead of assuming `\`. - - Neither bug is specific to this new project — both live in shared `WitcherScriptMerger.Core` code — but neither was reachable before this unit, since the WinForms host is Windows-only. Grepped the rest of Core for the same hardcoded-`'\\'`/`"\\"` pattern after finding these two; no other occurrences remained. -- **Publish-time config loading verified safe for single-file publishing** — see "Build & run" above for the empirical finding (`ConfigurationManager.OpenExeConfiguration("")` still resolves the real `.dll.config` next to a single-file-bundled exe, despite `Assembly.GetEntryAssembly().Location` returning `""` there) — no `AppSettings.cs` change was needed. -- **Verification status**: `dotnet build WitcherScriptMerger.sln` and `dotnet test` (existing `WitcherScriptMerger.Tests` suite) both pass with these changes. Self-contained single-file `win-x64` and `linux-x64` publishes both succeed. Beyond that — unusually for a change in this repo, given no Linux machine is normally available in this environment — this unit was verified against a **real Linux runtime**, not just a successful cross-compile: WSL2 (Ubuntu 20.04, genuine Linux kernel, both `/mnt/c`-mounted and native `ext4`-backed scratch trees) was available in the development environment this time and used to actually run the published `linux-x64` binary. Confirmed there: the `merge` verb against synthetic scratch mods (one auto-solvable conflict, correctly merged with UTF-16LE+BOM output matching vanilla's encoding; one genuine conflict, correctly skipped with git/diff3-style conflict markers written under `DiffPlexConflicts/` and surviving process exit); the `mcp` verb's full stdio round-trip (`initialize` → `tools/list` → `tools/call` for all four tools, including a `merge_conflicts` call using a forward-slash `relativePaths` entry specifically to exercise the separator-normalization fix above); and the bundle-graceful-degradation path (junk `.bundle` file, `CheckBundleContents=true`, no QuickBMS — clean warning, no crash, exit code 2). The equivalent Windows-side checks (both verbs, both self-contained single-file publishes) were also run and matched in shape. Not verified: an actual bare-metal/native Linux distribution outside WSL2, and the bundle path was only exercised with a junk (non-POTATO70-format) `.bundle` file — a real bundle-vs-bundle conflict was judged impractical to construct without QuickBMS/wcc_lite (matching the WinForms host's own "bundle path is code-reviewed but not round-tripped" status — see "CLI mode" above), so that specific scenario relies on code inspection of the fixes described above, not an end-to-end run. - -### Compatibility constraints - -- **Hash format is load-bearing.** `MergeInventory.xml` (including real, already-populated files on developer machines) stores per-file hashes compared by string equality to detect when a mod source file has changed since it was last merged. Any change to `Tools/Hasher.cs` must produce byte-for-byte identical output to the current implementation, or every existing recorded merge silently "goes stale." Verify with the synthetic-edge-cases + real-recorded-hash cross-check pattern described under Tests. -- **TFM must keep the explicit `7.0` OS-version suffix.** The project uses `false` (to keep the hand-written `Properties/AssemblyInfo.cs`), which also suppresses the SDK's auto-generated `[assembly: SupportedOSPlatform("windows")]` attribute — that attribute is added manually in `AssemblyInfo.cs` instead. The TFM must stay `net10.0-windows7.0` (not bare `net10.0-windows`); dropping the `7.0` suffix reintroduces ~800 spurious `CA1416` platform-compatibility warnings. -- **Text-merge input encoding must stay normalized to UTF-16LE, never down to UTF-8.** Vanilla `.ws` files are UTF-16LE with a BOM; mod authors' files are often plain UTF-8/ASCII with no BOM (confirmed against real files on a live install). Mismatched encodings can make an entire file read as unmatchable against its counterpart, turning a false conflict into "needs manual resolution" that would otherwise have auto-solved cleanly — empirically confirmed real-world case: `baseEffect.ws` failed to auto-solve with mismatched encodings (against the now-retired KDiff3 engine, which had no per-input encoding flag at all - see `docs/decisions/kdiff3-retirement.md`) and succeeded cleanly once normalized, with correct merged output. `Tools/FileEncoding.cs` (Core) handles this: `ReadAnyEncoding`/`WriteUtf16` give `DiffPlexMergeEngine` UTF-16LE normalization without needing a temp file, since it merges in-process text directly (`EnsureUtf16File`, an on-disk-copy variant for a tool that needs a file path instead, has no in-repo caller since KDiff3's retirement but is kept, still unit-tested, for any future file-based tool) — never normalize toward UTF-8, since the game may not load a merged `.ws` file in that encoding. `ReadAnyEncoding` deliberately uses `File.ReadAllText(path)`'s built-in BOM auto-detection rather than decoding raw bytes with a fixed `Encoding` instance - the latter does not strip a detected BOM, leaving a stray U+FEFF glued to the first line, confirmed empirically to reproduce the exact `baseEffect.ws`-style false conflict this whole mechanism exists to avoid (see `WitcherScriptMerger.Tests`' `MergeHeadless_EncodingMismatch_...` fixture). -- **DiffPlex's `ThreeWayDiffer` (1.9.0) can produce internally inconsistent diff blocks - `DiffPlexMergeEngine.BuildMerge` must never trust its output without checking.** Confirmed as a genuine upstream library bug, not a defect in this repo's own merge loop: `BuildMerge`'s block-iteration loop is a faithful port of DiffPlex's own `ThreeWayDiffer.CreateMerge` (same index-chasing shape), and a throwaway scratch console app (per this repo's testing convention) calling DiffPlex's own `CreateMerge` directly - with both `LineChunker` (DiffPlex's own default, and the *only* chunker its own `Facts.DiffPlex/ThreeWayDifferFacts.cs` test suite ever exercises for 3-way diffs) and `LineEndingsPreservingChunker` (the one this engine actually uses) - reproduced the identical failure on the identical input either way. When old-side and new-side edits interleave/overlap relative to base in certain ways, `CreateThreeWayDiffBlocks` can emit a block list whose `OldCount`/`NewCount` don't actually correspond to the real `PiecesOld`/`PiecesNew` arrays. This surfaces two ways: an outright `ArgumentOutOfRangeException` from direct indexer access, or - confirmed via a minimal repro (base `"a();/b();/c();"`, one side inserts a line after `a()`, the other independently changes `b()` to `B()`) - no exception at all, but silently wrong output (content lost or duplicated), because the running `oldIndex`/`newIndex` end up not matching `PiecesOld.Count`/`PiecesNew.Count` even though no single block's own bookkeeping looked wrong in isolation. A large randomized stress test (varying edit density and file length, run against the real, fixed `BuildMerge`) measured combined failure rates of **0.35%** at one independent single-line edit per side on 50-200 line files (the closest analogue to a typical two-mod `.ws` conflict), rising to **0.88%** (1-2 edits/side), **2.65%** (2-3 edits/side), **4.99%** (1-6 edits/side on 50-200 line files), and **38.89%** on the original dense adversarial case (1-6 edits/side on 1-19 line files) - zero cases of any exception type other than the one this bug produces, across 100,000 total trials. `BuildMerge` defends against both failure modes: the block-processing loop is wrapped in `try`/`catch (ArgumentOutOfRangeException)`, and a post-loop check verifies `oldIndex`/`newIndex` actually reached `PiecesOld.Count`/`PiecesNew.Count` (accounting for a legitimate trailing-unchanged gap needing the exact same lockstep advance as the per-block gap-catchup above it - an early, incorrect version of this check that skipped that trailing advance produced a ~33% false-positive "inconsistent" rate on the exact same benign inputs). Either failure mode throws a `DiffPlexMergeEngine.DiffAlgorithmException`, which `MergeHeadless` catches and reports as `NeedsManualResolution` **without writing anything, including a conflict-marker sidecar** - the marker content itself would have been built from the same untrustworthy piece indices, so this is the one case where `DiffPlexMergeEngine` can't offer a conflict-marker starting point at all. This measured, non-negligible failure rate at realistic edit density is a real reliability gap the now-retired KDiff3 engine didn't share, and is the primary reason retiring KDiff3 was a deliberate tradeoff, not a strict improvement - see `docs/decisions/kdiff3-retirement.md`. Regression-tested via `DiffPlexMergeEngineTests`' `BuildMerge_InterleavedIndependentEdits_...`/`MergeHeadless_InterleavedIndependentEdits_...` fixtures (the minimal repro above). Do not "fix" this by switching chunkers - the bug reproduces under DiffPlex's own default/tested `LineChunker` too, just at a somewhat lower rate, so it isn't a `LineEndingsPreservingChunker`-specific problem and switching would trade a real, working byte-for-byte line-ending-preservation property for no actual safety gain. -- **KDiff3's empirically-discovered process behavior (window-title polling, poll-interval sensitivity, failed suppression attempts, unverified focus restoration) is preserved in `docs/decisions/kdiff3-retirement.md`, not here.** That code (`Tools/KDiff3.cs`, `Tools/KDiff3MergeEngine.cs`) is deleted; the findings remain relevant only as a warning against reintroducing a similar external-GUI-tool integration without re-deriving them. - -### Settings & persistence - -- App settings: `AppSettings.cs` wraps `System.Configuration.ConfigurationManager` over `App.config`'s `` block (`Get`/`Get`/`Set`/`Save`, cached `Configuration` object) — deliberately *not* `Properties.Settings` (that scaffolding was removed during the SDK-style migration). Settings are cached and require an explicit `Save()` call. -- Merge history: `MergeInventory.xml`, via `XmlSerializer` (`Inventory/MergeInventory.cs`). -- Game load order: `LoadOrder/CustomLoadOrder.cs` reads the game's own `mods.settings` file. - -### External tool dependencies - -Two bundled Windows executables are invoked via `Process.Start`, with relative paths configured in `App.config`'s `` (`QuickBmsPath`, `QuickBmsPluginPath`, `WccLitePath`): -- **QuickBMS** (`Tools\QuickBMS\quickbms.exe` + `witcher3.bms` plugin) — no license file found; do not add to source control. -- **wcc_lite** (`Tools\wcc_lite\bin\x64\wcc_lite.exe`) — no license file found; do not add to source control. - -Neither binary is committed to this repo (matches the original upstream project's precedent) — keep it that way; if packaging is tackled later, it belongs in a separate release artifact, not source control. - -KDiff3 (`Tools\KDiff3\KDiff3.exe`, GPL-licensed, safe to bundle into a release) used to be a third dependency here, for text merges — it was retired in favor of an in-process DiffPlex-based engine that needs no external binary at all; see `docs/decisions/kdiff3-retirement.md`. +Four projects, one `.sln`: + +- **`WitcherScriptMerger.Core`** (`net10.0`, no WinForms reference) — all domain logic: + file scanning, merge orchestration, load-order handling, settings/paths, the + DiffPlex-based merge engine, and the CLI/MCP entry-point logic shared by both hosts. + See `WitcherScriptMerger.Core/CLAUDE.md` (and `WitcherScriptMerger.Core/Mcp/CLAUDE.md` + for the MCP tools' minimal-permissions detail specifically). +- **`WitcherScriptMerger`** (`net10.0-windows7.0`, `WinExe`) — the original WinForms + host: GUI + CLI + MCP entry points, references Core. See `WitcherScriptMerger/CLAUDE.md`. +- **`WitcherScriptMerger.Headless`** (`net10.0`, `Exe`) — the Linux-capable CLI/MCP-only + host, no GUI, references Core only. See `WitcherScriptMerger.Headless/CLAUDE.md`. +- **`WitcherScriptMerger.Tests`** (xunit, `net10.0`) — covers Core only. See + `WitcherScriptMerger.Tests/CLAUDE.md`. + +Domain code in Core never calls into WinForms directly — it goes through +`AppState.Notifier` (an `IMergeNotifier`, defined against neutral types so Core never +needs `System.Windows.Forms`), which is what makes both headless hosts possible. Each +host's own `CLAUDE.md` covers its own startup flow, entry-point wiring, and +verification status in detail — this file doesn't restate it. + +## Architecture decisions (`docs/decisions/`) + +Bigger design decisions than fit comfortably in a `CLAUDE.md` note live here as their own +documents: + +- `docs/decisions/kdiff3-retirement.md` — why the external KDiff3 tool was retired in + favor of the in-process DiffPlex-based engine, including the full empirical writeup of + KDiff3's process behavior (window-title polling, poll-interval sensitivity, failed + suppression attempts, unverified focus restoration) now that the code itself is gone. +- `docs/decisions/bundle-format-replacement-spike.md` — a research spike into whether + `WolvenKit.Modkit` could replace QuickBMS/wcc_lite for `.bundle` handling + (cross-platform, clearly licensed); no follow-on implementation was recommended. + +## External tool dependencies & licensing + +WSM itself is **GPLv2-licensed** — see the root `LICENSE` file. + +Two bundled Windows executables are invoked via `Process.Start`, with relative paths +configured in each host's `App.config` (`QuickBmsPath`, `QuickBmsPluginPath`, +`WccLitePath`): + +- **QuickBMS** (`quickbms.exe` + `witcher3.bms` plugin) — no license file found; not + committed to source control. +- **wcc_lite** (`wcc_lite.exe`) — no license file found; not committed to source control. + +Neither binary is committed to this repo (matches the original upstream project's +precedent) — keep it that way; if packaging is tackled later, it belongs in a separate +release artifact, not source control. Both are required only for `.bundle`-content +conflicts — flat-file (`.ws`/`.xml`) conflicts need neither (see +`WitcherScriptMerger.Core/CLAUDE.md`'s "Dependency validation" section). + +KDiff3 (GPL-licensed, formerly a third such dependency, safe to bundle) was retired in +favor of an in-process engine built on **DiffPlex** (MIT-licensed) — an ordinary NuGet +package, not an external binary, so it carries none of the "not in source control" +concerns above. See `docs/decisions/kdiff3-retirement.md`. + +Of the fork's original list of open goals, whitespace/diff-noise (see +`WitcherScriptMerger.Core/CLAUDE.md`'s "Text-merge input encoding") and a CLI mode (see +each host's own `CLAUDE.md`) are done. **Dependency-packaging/licensing — specifically, +whether/how to ship QuickBMS/wcc_lite in a release build — is still an open decision**, +not something this batch resolved; re-confirm with the repo owner before changing the +"not in source control" policy above. An MCP server mode was added afterward, beyond +that original goals list. ## Coding standards & SOP -See `CONTRIBUTING.md` for observed code style (bracing, naming, region conventions) and repository process (branching, commit style, AI-assisted-development disclosure). +See `CONTRIBUTING.md` for observed code style (bracing, naming, region conventions) and +repository process (branching, commit style, AI-assisted-development disclosure). diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 742f29a..cd8e944 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,6 +1,6 @@ # Contributing -This is a public repository, and contributions — human or AI-agent-assisted — are welcome. See `CLAUDE.md` for build commands, architecture, and compatibility constraints; this file covers style and process. For the project's own fork lineage (this repo vs. the upstream `AnotherSymbiote/WitcherScriptMerger` project), see `CLAUDE.md`'s Project overview. +This is a public repository, and contributions — human or AI-agent-assisted — are welcome. See the root `CLAUDE.md` for build commands and architecture, and each project's own `CLAUDE.md` (`WitcherScriptMerger.Core/CLAUDE.md`, `WitcherScriptMerger/CLAUDE.md`, `WitcherScriptMerger.Headless/CLAUDE.md`, `WitcherScriptMerger.Tests/CLAUDE.md`) for that project's compatibility constraints; this file covers style and process. For the project's own fork lineage (this repo vs. the upstream `AnotherSymbiote/WitcherScriptMerger` project), see the root `CLAUDE.md`'s "Fork history" section. ## Code style @@ -19,22 +19,22 @@ 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. -- **PR description should cover**: what changed and why, and — given there's no test suite (see Testing below) — specifically *how you verified it*. "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 `CLAUDE.md`'s Compatibility constraints for why those are load-bearing, and its Tests section for the verification pattern this codebase uses in place of a test suite. +- **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. -- External binary dependencies (QuickBMS, wcc_lite — see `CLAUDE.md`'s External tool dependencies) aren't in source control, so a fresh clone needs them sourced separately before the app runs end-to-end. PRs that only touch code not exercising those tools don't need them to build and review. (KDiff3 used to be a third such dependency; it was retired — see `docs/decisions/kdiff3-retirement.md`.) +- External binary dependencies (QuickBMS, wcc_lite — see the root `CLAUDE.md`'s "External tool dependencies & licensing" section) aren't in source control, so a fresh clone needs them sourced separately before the app runs end-to-end. PRs that only touch code not exercising those tools don't need them to build and review. (KDiff3 used to be a third such dependency; it was retired — see `docs/decisions/kdiff3-retirement.md`.) ## Testing -`WitcherScriptMerger.Tests` (xunit) covers `WitcherScriptMerger.Core` — see `CLAUDE.md`'s Tests section for what it covers and its constraints. For anything not covered there — especially further hash output or `MergeInventory.xml` schema changes — use a disposable, non-committed scratch console app: exercise synthetic edge cases plus a cross-check against a real value already recorded in a live `MergeInventory.xml`. Describe what you actually ran in your PR description — see Repository SOP above. +`WitcherScriptMerger.Tests` (xunit) covers `WitcherScriptMerger.Core` — see `WitcherScriptMerger.Tests/CLAUDE.md` for what it covers and its constraints. For anything not covered there — especially further hash output or `MergeInventory.xml` schema changes — use a disposable, non-committed scratch console app: exercise synthetic edge cases plus a cross-check against a real value already recorded in a live `MergeInventory.xml`. Describe what you actually ran in your PR description — see Repository SOP above. ## AI-assisted development -This repository is developed with AI coding agents (Claude Code, and expect others), openly — that's not hidden, and it's not discouraged. `CLAUDE.md` carries the operational guidance these tools use when working in this repo, kept up to date as the codebase changes; read it before pointing an agent at this repo. If these guidelines are silent on something and you're using an agent, defer to the explicit rules below over whatever the agent proposes on its own. +This repository is developed with AI coding agents (Claude Code, and expect others), openly — that's not hidden, and it's not discouraged. The federated `CLAUDE.md` files (a short root one, plus one per project) carry the operational guidance these tools use when working in this repo, kept up to date as the codebase changes; read the root one, plus whichever project's you're touching, before pointing an agent at this repo. If these guidelines are silent on something and you're using an agent, defer to the explicit rules below over whatever the agent proposes on its own. - **Disclose it.** If a PR was substantially produced or assisted by an AI coding agent, say so in the PR description. Commits already carry a `Co-Authored-By` trailer when an agent is involved (Claude Code does this automatically) — that's necessary but not sufficient; the PR description is where a reviewer looks first. - **You own what you submit, regardless of how it was produced.** Be able to explain any part of your own PR if a reviewer asks — "the agent wrote it that way" isn't an answer to "why does this work." If you can't explain a change, that's a signal to understand it better before submitting, not to submit it anyway. -- **The verification bar doesn't move for AI-assisted changes — if anything, hold it higher.** This codebase has a thin, Core-only test suite and several genuinely load-bearing, non-obvious compatibility constraints (hash format, text-merge input encoding normalization, the DiffPlex upstream bug `DiffPlexMergeEngine` has to defend against on every merge — all documented in `CLAUDE.md`). Agents are good at producing code that looks plausible and compiles; they have no way to know these constraints exist unless `CLAUDE.md` tells them, and no way to know their fix actually works unless it's actually run against real data. "Should work" is not verification — see Testing above. +- **The verification bar doesn't move for AI-assisted changes — if anything, hold it higher.** This codebase has a thin, Core-only test suite and several genuinely load-bearing, non-obvious compatibility constraints (hash format, text-merge input encoding normalization, the DiffPlex upstream bug `DiffPlexMergeEngine` has to defend against on every merge — all documented in `WitcherScriptMerger.Core/CLAUDE.md`). Agents are good at producing code that looks plausible and compiles; they have no way to know these constraints exist unless `CLAUDE.md` tells them, and no way to know their fix actually works unless it's actually run against real data. "Should work" is not verification — see Testing above. - **Scrub machine-specific state before submitting.** Agent-assisted sessions tend to accumulate absolute local paths, scratch config pointing at a personal install, or test artifacts from the working process — check your diff for anything like a `G:\SteamLibrary\...`-style path or a personal game install location before opening a PR. `.gitignore` excludes common agent runtime-state directories (`.claude/`, `.cursor/`, etc.) and session handoff notes (`HANDOFF*.md`) for the same reason — extend it rather than working around it if your tool of choice uses a different local-state convention. -- **You're responsible for license compatibility of anything an agent produces**, same as for hand-written code — this project cares about this already (see `CLAUDE.md`'s External tool dependencies section on why QuickBMS/wcc_lite specifically aren't bundled). Don't accept agent output that reproduces code from a source with an incompatible license. +- **You're responsible for license compatibility of anything an agent produces**, same as for hand-written code — this project cares about this already (see the root `CLAUDE.md`'s "External tool dependencies & licensing" section on why QuickBMS/wcc_lite specifically aren't bundled). Don't accept agent output that reproduces code from a source with an incompatible license. - **Bulk or automated PRs still go through the same process.** A large refactor being agent-generated isn't a reason to skip branch-per-change, PR review, or the two-approval requirement — if anything, larger diffs benefit more from review, not less. diff --git a/README.md b/README.md index 9f24258..b7cdfc8 100644 --- a/README.md +++ b/README.md @@ -1,10 +1,81 @@ # Script Merger for The Witcher 3 -I threw together this tool because I got tired of manually merging script files. +A tool for detecting and merging conflicting Witcher 3 mod script files. It scans your +Mods folder, finds `.ws`/`.xml` files (including inside `.bundle` packages) that more +than one mod modifies, and drives a 3-way merge (vanilla + mod1 + mod2) to combine them. -- Checks your Mods folder for mod conflicts. Uses [QuickBMS](http://aluigi.altervista.org/quickbms.htm) to scan .bundle packages. -- Merges .ws scripts or .xml files inside bundle packages using an in-process 3-way merge engine built on [DiffPlex](https://github.com/mmanela/diffplex) — no external merge tool required. A conflict that can't be auto-solved is written to a conflict-marker file and opened for manual review instead. (This fork previously used the external tool KDiff3 for this; see `docs/decisions/kdiff3-retirement.md` for why it was retired.) -- Packages new .bundle packages using the official mod tool [wcc_lite](http://www.nexusmods.com/witcher3/news/12625/?). -- Detects updated merge source files using the [xxHash](https://github.com/Cyan4973/xxHash) algorithm by Yann Collet, [implemented in .NET](https://github.com/wilhelmliao/xxHash.NET) by Wilhelm Liao. +This is [`TheValiantOne/WitcherScriptMerger`](https://github.com/TheValiantOne/WitcherScriptMerger), +a fork of the original [`AnotherSymbiote/WitcherScriptMerger`](https://github.com/AnotherSymbiote/WitcherScriptMerger), +mid-modernization: a .NET modernization pass, an in-process merge engine replacing the +external KDiff3 tool, a headless CLI mode, an MCP server mode, and a Linux-capable host +have all landed since the fork. See `CLAUDE.md` at the repo root (and each project's own +`CLAUDE.md`) for full architecture detail if you're contributing. -**QuickBMS & wcc_lite aren't included in this source code.** +## What it does + +- Checks your Mods folder for mod conflicts. Uses [QuickBMS](http://aluigi.altervista.org/quickbms.htm) + to scan `.bundle` packages for conflicting internal content. +- Merges `.ws` scripts or `.xml` files (including those inside bundle packages) using an + in-process 3-way merge engine built on [DiffPlex](https://github.com/mmanela/diffplex) + — no external merge tool required. A conflict that can't be auto-solved is written to a + conflict-marker file (git/diff3-style markers) and opened for manual review instead. + (This fork previously used the external tool KDiff3 for this; see + [`docs/decisions/kdiff3-retirement.md`](docs/decisions/kdiff3-retirement.md) for why it + was retired.) +- Packages new `.bundle` packages using the official mod tool + [wcc_lite](http://www.nexusmods.com/witcher3/news/12625/?). +- Detects updated merge source files using the [xxHash](https://github.com/Cyan4973/xxHash) + algorithm (xxHash32), via the [`System.IO.Hashing`](https://www.nuget.org/packages/System.IO.Hashing) + NuGet package. + +## Ways to run it + +There are three entry points in the Windows GUI application, plus a fourth, +Linux-capable host that drops the GUI: + +- **GUI** (Windows only) — launch `WitcherScriptMerger.exe` with no arguments, or + `dotnet run --project WitcherScriptMerger/WitcherScriptMerger.csproj`. The familiar + point-and-click conflict tree and merge workflow. +- **Headless CLI** (Windows only, same executable) — + `WitcherScriptMerger.exe merge [--order-file ]` merges every + auto-solvable conflict with no window at all, then exits. +- **MCP server** (Windows only, same executable) — `WitcherScriptMerger.exe mcp` runs an + MCP (Model Context Protocol) server over stdio, so an MCP client (e.g. a Claude Code + session) can inspect conflicts and drive merges directly. +- **`WitcherScriptMerger.Headless`** (Windows or Linux) — a second, smaller executable + with the same `merge` and `mcp` verbs and no GUI dependency at all, for + CLI/agent-driven workflows on either OS. Supports flat-file (`.ws`/`.xml`) conflicts + only — see "Dependencies" below. + +## Building + +``` +dotnet build WitcherScriptMerger.sln +``` + +Single solution, four projects: `WitcherScriptMerger.Core` (shared domain logic), +`WitcherScriptMerger` (the WinForms host, all three entry points above), +`WitcherScriptMerger.Headless` (the Linux-capable CLI/MCP-only host), and +`WitcherScriptMerger.Tests`. See the root `CLAUDE.md` for the full breakdown and each +project's own `CLAUDE.md` for that project's build/run/publish details, including +self-contained single-file publish commands for both hosts (`win-x64`, plus `linux-x64` +for the headless host). + +## Dependencies + +**QuickBMS and wcc_lite aren't included in this source code.** Both are Windows-only +binaries with no license file in their own distribution, so they aren't committed here — +you'll need to source them separately and point `App.config`'s `QuickBmsPath`, +`QuickBmsPluginPath`, and `WccLitePath` settings at them. They're needed **only** for +`.bundle`-content conflicts; plain `.ws`/`.xml` file conflicts merge without them, on +either host. + +DiffPlex, the library behind the merge engine, is MIT-licensed and pulled in as an +ordinary NuGet package — no separate download or licensing concern, unlike QuickBMS/ +wcc_lite. (KDiff3, an earlier external dependency for merging, has been fully retired — +see [`docs/decisions/kdiff3-retirement.md`](docs/decisions/kdiff3-retirement.md).) + +## License + +Script Merger for The Witcher 3 is licensed under the **GNU General Public License v2.0** +— see [`LICENSE`](LICENSE). diff --git a/WitcherScriptMerger.Core/CLAUDE.md b/WitcherScriptMerger.Core/CLAUDE.md new file mode 100644 index 0000000..8719d90 --- /dev/null +++ b/WitcherScriptMerger.Core/CLAUDE.md @@ -0,0 +1,366 @@ +# CLAUDE.md — WitcherScriptMerger.Core + +Guidance for working in `WitcherScriptMerger.Core` (`net10.0`, no WinForms reference, +deliberately cross-platform-capable). This project holds all domain logic — file +scanning, merge orchestration, load-order handling, settings/paths, and the CLI/MCP +entry-point logic shared by both hosts. Nothing here references `System.Windows.Forms`. + +See the root `CLAUDE.md` for the overall project/fork context and the other three +projects. See `Mcp/CLAUDE.md` for the MCP tools' minimal-permissions detail (not +duplicated here). See `docs/decisions/kdiff3-retirement.md` for the full empirical +history of the KDiff3 engine this project's `DiffPlexMergeEngine` replaced, and +`docs/decisions/bundle-format-replacement-spike.md` for why QuickBMS/wcc_lite are +still external dependencies rather than an in-process replacement. + +## Folder map + +- `FileIndex/` — scans the mods folder and builds the conflict index: `ModFileIndex.cs` + (`BuildAsync` → `Conflicts`), `ModFile.cs`, `ModFileCategory.cs`. +- `Inventory/` — core merge domain + persistence: `FileMerger.cs` (headless orchestration + plus TreeNode-free interactive orchestration — see "FileMerger: interactive vs. + headless split" below), `Merge.cs`, `MergeInventory.cs`, `FileHash.cs`, + `MergeProgressInfo.cs`. +- `LoadOrder/` — mod load-order logic: `CustomLoadOrder.cs`, `LoadOrderComparer.cs`, + `LoadOrderValidator.cs`, `ModLoadSetting.cs`. +- `Tools/` — wrappers that shell out to bundled external executables, plus the sole + text-merge engine: `QuickBms.cs`, `WccLite.cs`, `Hasher.cs`, `FileEncoding.cs` + (UTF-16LE+BOM normalization — see "Text-merge input encoding" below), + `DiffPlexMergeEngine.cs` (see below), `FileOpener.cs` (portable "open in the OS's + default associated app" helper; the only call site is + `DiffPlexMergeEngine.MergeHeadless`, opening a genuine conflict's marker sidecar). +- `Cli/` — `MergeOperations.cs`: the scan-then-merge sequence shared by both hosts' + `merge` CLI verb and by the MCP tools (see "CLI & MCP orchestration" below). +- `Mcp/` — `WsmMcpTools.cs`: the MCP server's tool implementations (see below and + `Mcp/CLAUDE.md`). +- Root: `AppState.cs` (shared mutable state — see below), `AppSettings.cs`, `Paths.cs`, + `StringExtensions.cs`, `IMergeNotifier.cs`, `NotifyTypes.cs` (the neutral + `NotifyResult`/`NotifyButtons`/`DialogIcon` enums), `HeadlessMergeNotifier.cs`. + +## AppState & IMergeNotifier + +`AppState` (`AppState.cs`) holds the shared mutable state that used to be static fields +directly on the WinForms host's `Program` class — `Notifier`/`Settings`/`LoadOrder`/ +`Inventory`. It moved here because domain code that now lives in Core needs to read/write +it, and Core can never reference either host assembly (the dependency only flows +host → Core). Both hosts' `Program` classes re-expose these as pass-through properties +(the WinForms host keeps calling them `Program.Notifier` etc.) so their own call sites +didn't need to change. + +- **`AppState.Notifier`** defaults to `HeadlessMergeNotifier` via a field initializer, + unconditionally, so any startup error is safe to report even before it's known whether + a given run is GUI, CLI, or MCP. +- **`AppState.Settings`** is a **lazy property**, not a field initializer + (`LazyInitializer.EnsureInitialized`, not the simpler non-atomic + `_settings ?? (_settings = new AppSettings())`, for thread-safety against a hypothetical + future concurrent first-access caller) — deliberately decoupled from `Notifier`'s eager + init. `AppSettings`'s constructor calls `Environment.Exit(1)` if it can't find a config + file next to the entry assembly — correct for the real GUI/CLI/MCP entry points, where + that's genuinely fatal, but not for `WitcherScriptMerger.Tests`'s `dotnet test` host, + which has no matching `.config`. C# runs *all* of a type's static field initializers + together on first touch of *any* static member, so before this laziness existed, merely + reading `AppState.Notifier` — which Core code legitimately does on its own (e.g. + `DiffPlexMergeEngine`'s headless skip/guard messages) — silently also ran + `new AppSettings()` and killed the entire test process. See + `WitcherScriptMerger.Tests/CLAUDE.md` for the test-side constraints this laziness + exists to satisfy. +- `AppState` has an explicit (empty) static constructor so its own init ordering is + deterministic rather than left to `beforefieldinit`'s discretion. `Paths.cs`'s + `ScriptsDirectory`/`ModsDirectory`/`IsScriptsDirectoryDerived`/`IsModsDirectoryDerived` + read `AppState.Settings.Get(...)` on every access rather than caching the result via a + static field initializer, for the identical reason one layer further out: a field + initializer there would force `Settings` to construct merely from touching an unrelated + static member of `Paths` (e.g. `GetRelativePath`), via the same `beforefieldinit` + mechanism. + +**`IMergeNotifier`** (`IMergeNotifier.cs`, `NotifyTypes.cs`, `HeadlessMergeNotifier.cs`) +replaces every direct UI call in domain code with `AppState.Notifier.*`. It's defined +against neutral `NotifyResult`/`NotifyButtons`/`DialogIcon` types, not +`DialogResult`/`MessageBoxButtons`/`MessageBoxIcon` — Core can't reference +`System.Windows.Forms` at all. `ShowModal(Form)` isn't part of the interface: every call +site is GUI-only, interactive code living in the WinForms host, which calls +`MainForm.ShowModal` directly instead. `IsInteractive` was dropped too (confirmed dead +code, zero read call sites before or after the Core split). `ShowMessage`'s +`defaultResult` parameter is the caller's own answer for "which result is safe/ +non-destructive for this specific prompt" — added for `LoadOrderValidator`'s +`YesNoCancel` prompt, whose safety shape is inverted from usual (Cancel, not Yes/No, is +the one destructive/permanent choice there). `HeadlessMergeNotifier` writes to the +console and returns a fixed, non-destructive default per button set (don't overwrite, +don't use a still-conflicting merge name, don't continue past a failure) unless the +caller overrides it via `defaultResult`. The WinForms host's `MainForm` implements this +interface too, translating the neutral types to/from real WinForms calls — see +`WitcherScriptMerger/CLAUDE.md` for that translation and the one behavior change it +introduced. + +This indirection isn't abstraction for its own sake: it's also what fixed a real +null-ref hazard in `LoadOrder/CustomLoadOrder.Refresh()`, which used to reach +`Program.MainForm` directly at construction time — a problem for any code path (like the +CLI/MCP entry points) that constructs a `CustomLoadOrder` before a `MainForm` exists at +all. Going through `AppState.Notifier` (defaulting to `HeadlessMergeNotifier`, always +constructed) instead of the WinForms host's concrete form removed that ordering +dependency entirely. + +## FileMerger: interactive vs. headless split + +Core's `FileMerger` (`Inventory/FileMerger.cs`) never sees a `TreeNode`, +`BackgroundWorker`, or `Forms.*` type. + +- Its **headless** methods (`MergeConflictsHeadless`, `MergeFlatConflictHeadless`, + `MergeBundleConflictHeadless`, `ResolveMergeOrder`, ...) are unchanged in shape from + before the Core/host split. +- Its **interactive** methods (`MergeFilesInteractive`, `MergeFlatFileInteractive`, + `MergeBundleFileInteractive`, `MergeTextInteractive`) take a plain + `InteractiveMergeRequest` (relative path, bundle flag, vanilla file path, ordered + `MergeSource[]`) instead of `TreeNode[]`, and report back through `OnMergeReport`/ + `OnPackReport` callbacks instead of constructing report forms directly. The WinForms + host's `Inventory/InteractiveMergeRunner.cs` is the concrete thing that drives this + path (extracts `InteractiveMergeRequest`s from checked `TreeNode`s, owns the + `BackgroundWorker`, supplies the callbacks) — see `WitcherScriptMerger/CLAUDE.md`. + +There used to be an `IMergeEngine` interface between `FileMerger` and the actual +text-merge implementation, with two implementations — `KDiff3MergeEngine` (host, +wrapping the external KDiff3 process) and `DiffPlexMergeEngine` (Core, in-process). KDiff3 +was retired (see `docs/decisions/kdiff3-retirement.md` for the full rationale and the +empirical KDiff3 process-behavior findings preserved there); with only one implementation +ever going to remain, the interface indirection was deleted as premature abstraction. +`FileMerger`'s constructor (`public FileMerger(MergeInventory inventory)`) now builds its +own private `DiffPlexMergeEngine` field directly — there's no engine-selection step at +startup in either host anymore. + +## CLI & MCP orchestration (`Cli/`, `Mcp/`) + +`Cli/MergeOperations.cs` is the scan-then-merge sequence shared by both hosts' `merge` +CLI verb and by the MCP tools, so neither duplicates the scan/wait/merge sequence. +`ScanConflicts()` runs `ModFileIndex.BuildAsync` synchronously (via a +`ManualResetEventSlim`) and returns the built index; `RunMerge(inventory, conflicts, +mergedModName, orderOverrides, dryRun)` calls `FileMerger.MergeConflictsHeadless`, which +iterates `ModFileIndex.Conflicts` directly — plain `ModFile`/`FileHash` objects, so no +`TreeNode`/`ConflictTree` is ever constructed on this path. + +Per-file mod order defaults to `LoadOrderComparer` (matching the WinForms host's +`ConflictTree`'s own default sort). An `orderOverrides` map (`{"relative\\path.ws": +["modA", "modB"]}` — the CLI's `--order-file` JSON has the identical shape, minus the +file) overrides specific files without requiring every conflict to be listed. +`FileMerger.ResolveMergeOrder` validates a listed file's mod list: no unknown mod names, +no duplicates, at least two entries, and every one of that file's *real* source mods +present at least once — any violation rejects that one file with a clear error via +`AppState.Notifier.ShowError` rather than silently merging an incomplete, self-paired, or +no-op chain. "Real source mods" deliberately excludes the configured merged-mod name +itself, since a file that's already been merged once has its own merged-mod folder +re-enter `conflict.Mods` as if it were a source. + +`Mcp/WsmMcpTools.cs` (`[McpServerToolType]` static class) exposes four tools — +`scan_conflicts`, `merge_conflicts` (optional `relativePaths`/`orderOverrides`/`dryRun`; +returns `{merged, skipped, unmatched, dryRun}`), `get_status`, `list_merges` — all +reusing `MergeOperations` and the same `IMergeNotifier` machinery as the CLI verb. +`get_status` reports `textMergeDependenciesValid` and `bundleDependenciesValid` as two +independent fields (plus a combined `dependenciesValid`, kept for existing callers that +only checked one flag) — deliberately split rather than a single boolean, so a host with +no QuickBMS/wcc_lite (e.g. `WitcherScriptMerger.Headless`) doesn't report a +`conflictCount` of 0 just because bundle tooling is missing; `conflictCount` itself only +requires `textMergeDependenciesValid`. State is re-scanned/re-loaded on every call, never +cached server-side, since the mods folder or `MergeInventory.xml` can change between +calls. `merge_conflicts`'s +`relativePaths`/`orderOverrides` keys are validated to resolve inside `Paths.ModsDirectory` +before any scan or merge runs (`EnsureInScope`/`IsWithinModsDirectory`) — defense in +depth, since neither value is actually joined into a filesystem path anywhere today. +`ScanConflicts`/`MergeConflicts` gate on `Paths.ValidateTextMergeDependencies()` only +(via `RequireDependenciesAndModsDirectory`), not the combined +`Paths.ValidateDependencyPaths()` — see "Dependency validation" below for why, and for +the important caveat that this per-tool gate is not the only gate a given host applies; +each host's own `mcp` verb entry point has its own startup-level check, described in that +host's own `CLAUDE.md`. See `Mcp/CLAUDE.md` for the tools' filesystem-footprint and +permissions detail (not duplicated here). + +## DiffPlexMergeEngine (the text-merge engine) + +`Tools/DiffPlexMergeEngine.cs` is the sole text-merge engine — in-process, built on the +DiffPlex NuGet package (MIT-licensed, 1.9.0), needing no external binary. It builds its +own merge loop (`BuildMerge`) around `DiffPlex.ThreeWayDiffer.CreateDiffs` rather than +calling `ThreeWayDiffer.CreateMerge` directly, so it can intercept +`ThreeWayChangeType.Conflict` blocks itself. + +**Whitespace-only auto-resolve.** A conflict whose two sides are equal once whitespace is +collapsed (joined-and-collapsed comparison over the classic ASCII whitespace set — +space/tab/CR/LF/form-feed/vertical-tab, deliberately narrower than .NET regex's +Unicode-aware `\s` so a genuine NBSP-vs-space difference isn't misclassified as +whitespace-only — never applied when either side has zero pieces, since a real deletion +must never be conflated with "the surviving side happens to collapse to empty too") +auto-resolves by taking the first mod's side verbatim, mirroring the retired KDiff3 +engine's `--cs "WhiteSpace3FileMergeDefault=2"` behavior (confirmed against the KDiff3 +source, preserved in `docs/decisions/kdiff3-retirement.md`). + +**Conflict-marker sidecar.** A genuine conflict produces git/diff3-style conflict markers +(`<<<<<<< ` / `||||||| Vanilla` / `=======` / `>>>>>>> `) written to +a **sidecar** file under `Paths.DiffPlexConflictsDirectory` (a dedicated top-level +`DiffPlexConflicts` folder, via `GetConflictMarkerPath`, keyed by an `XxHash32` of the +full output path plus filename) rather than to `outputPath` itself — writing to +`outputPath` would make `FileMerger`'s pre-merge `File.Exists(_outputPath)` overwrite +guard treat it as an already-completed merge and permanently skip retrying, since +`HeadlessMergeNotifier` always declines the overwrite prompt. The sidecar went through +two prior locations before landing here, each ruled out by direct end-to-end testing +rather than inspection alone: beside `outputPath` (`.conflict` — leaves +untracked litter inside the live mods tree, and a bundle-content conflict's `outputPath` +sits inside `Paths.MergedBundleContent`, which `WccLite.PackBundle` packs *wholesale* +with no filtering, risking a leftover `.conflict` file getting embedded into a +later-packed `blob0.bundle`); then under `Paths.TempBundleContent`, which fixed both of +those but broke for a third reason neither review nor unit tests caught — +`FileMerger.CleanUpTempFiles()` deletes the entire `TempBundleContent` tree wholesale at +the end of every headless run, so the sidecar was gone before a user could ever see it. A +conflict-marker file that later becomes an auto-solve on retry has its stale sidecar +deleted before writing the fresh output. Nothing automatically deletes the +`DiffPlexConflicts` directory itself (the same "accumulates until manually cleared" +property `TempBundleContent` has) — needs the same manual housekeeping between runs, just +without an automated sweep. `Paths.DiffPlexConflictsDirectory` is a relative path, +resolved against `Environment.CurrentDirectory` — each host sets that to +`AppContext.BaseDirectory` before dispatching to `merge`/`mcp` (see that host's own +`CLAUDE.md`), so in CLI/MCP mode sidecars land predictably next to the installed exe; the +WinForms host's GUI path never does that reset, so a GUI-mode conflict's sidecar lands +wherever the process's CWD happened to be at launch (typically the exe's own directory +for a normal double-click launch, but not guaranteed). + +**The sidecar is opened for the user, not just written.** `MergeHeadless` writes the +sidecar, then (unless `openConflictMarkers` is `false`) calls `Tools/FileOpener.Open` (a +swappable static `Func`, defaulting to `Process.Start` with +`UseShellExecute = true` so it resolves the OS's file association) on the sidecar path, +and only then reports the skip via `AppState.Notifier.ShowMessage` — the open happens +*before* the message, since the message needs `FileOpener.Open`'s own bool return to say +whether the file actually opened. That bool distinguishes only "`Process.Start` succeeded" +from "`Process.Start` threw" — not a guarantee an editor actually came up; confirmed +empirically that on a machine with no file association for `.conflict`, `Process.Start` +still succeeds by launching the OS's own "how do you want to open this?" picker. `Merge()` +(interactive) just runs `MergeHeadless()` and maps `NeedsManualResolution` to `Failed` +since there's no UI here at all — the sidecar-write-and-open behavior fires identically +whether reached via the GUI's interactive path or the CLI/MCP headless path. +`MergeHeadless`'s `openConflictMarkers` parameter (default `true`) is the one thing that +differs by caller: `FileMerger.MergeTextHeadless` passes `openConflictMarkers: !dryRun`, +so a dry run still writes a genuine conflict's sidecar but never launches anything for it. +Deliberately not the WinForms host's existing `Program.TryOpenFile` helper: that helper's +non-`.exe` branch is a bare `Process.Start(path)` with no `UseShellExecute = true`, which +throws on modern .NET for a non-executable path (silently swallowed by that method's own +`catch`) — `Tools/FileOpener.cs` exists specifically so both hosts' paths can reach a +correctly-implemented version without Core referencing `System.Windows.Forms`. + +**Vanilla-less guard.** A 3-way merge with no vanilla version at all (expected mainly on +the bundle-content path, when no matching vanilla bundle is found, but the guard applies +unconditionally to any conflict missing one) is refused outright +(`NeedsManualResolution`, nothing written) rather than attempted with an empty base +string — `DiffPlex.ThreeWayDiffer` degrades silently to zero diff blocks and a +"successful" empty merge in that case, confirmed empirically, which would otherwise +produce a truncated output file. This is a deliberate divergence from the retired KDiff3 +engine, which had no equivalent guard and always attempted a real (if vanilla-less, +degraded 2-way) `--auto` merge instead — KDiff3 had a coherent notion of a 2-file merge +that DiffPlex's `ThreeWayDiffer`, as used here, does not. + +## Compatibility constraint: DiffPlex's `ThreeWayDiffer` can produce inconsistent diff blocks + +Confirmed as a genuine upstream library bug (DiffPlex 1.9.0), not a defect in this +repo's own merge loop: a throwaway scratch console app calling DiffPlex's own +`CreateMerge` directly — with both `LineChunker` (DiffPlex's own default and tested +chunker) and `LineEndingsPreservingChunker` (the one this engine actually uses) — +reproduced the identical failure on the identical input either way. When old-side and +new-side edits interleave/overlap relative to base in certain ways, +`CreateThreeWayDiffBlocks` can emit a block list whose `OldCount`/`NewCount` don't +actually correspond to the real `PiecesOld`/`PiecesNew` arrays. This surfaces two ways: +an outright `ArgumentOutOfRangeException`, or — confirmed via a minimal repro (base +`"a();/b();/c();"`, one side inserts a line after `a()`, the other independently changes +`b()` to `B()`) — no exception at all, but silently wrong output (content lost or +duplicated), because the running `oldIndex`/`newIndex` end up not matching +`PiecesOld.Count`/`PiecesNew.Count` even though no single block's own bookkeeping looked +wrong in isolation. + +A large randomized stress test (100,000 total trials, varying edit density and file +length, run against the real, fixed `BuildMerge`) measured combined failure rates of +**0.35%** at one independent single-line edit per side on 50–200 line files (the closest +analogue to a typical two-mod `.ws` conflict), rising to **0.88%** (1–2 edits/side), +**2.65%** (2–3 edits/side), **4.99%** (1–6 edits/side on 50–200 line files), and +**38.89%** on the original dense adversarial case (1–6 edits/side on 1–19 line files) — +zero cases of any exception type other than the one this bug produces. + +`BuildMerge` defends against both failure modes: the block-processing loop is wrapped in +`try`/`catch (ArgumentOutOfRangeException)`, and a post-loop check verifies +`oldIndex`/`newIndex` actually reached `PiecesOld.Count`/`PiecesNew.Count` (accounting for +a legitimate trailing-unchanged gap needing the exact same lockstep advance as the +per-block gap-catchup above it — an early, incorrect version of this check that skipped +that trailing advance produced a **~33% false-positive** "inconsistent" rate on +otherwise-benign inputs). Either failure mode throws +`DiffPlexMergeEngine.DiffAlgorithmException`, which `MergeHeadless` catches and reports as +`NeedsManualResolution` **without writing anything, including a conflict-marker +sidecar** — the marker content itself would have been built from the same untrustworthy +piece indices. + +This measured, non-negligible failure rate at realistic edit density is a real +reliability gap the retired KDiff3 engine didn't share, and is the primary reason +retiring KDiff3 was a deliberate tradeoff, not a strict improvement — see +`docs/decisions/kdiff3-retirement.md`. Regression-tested via +`DiffPlexMergeEngineTests`'s `BuildMerge_InterleavedIndependentEdits_...`/ +`MergeHeadless_InterleavedIndependentEdits_...` fixtures. **Do not "fix" this by +switching chunkers** — the bug reproduces under DiffPlex's own default/tested +`LineChunker` too, just at a somewhat lower rate, so switching would trade a real, +working byte-for-byte line-ending-preservation property for no actual safety gain. + +## Text-merge input encoding + +Vanilla `.ws` files are UTF-16LE with a BOM; mod authors' files are often plain +UTF-8/ASCII with no BOM (confirmed against real files on a live install). Mismatched +encodings can make an entire file read as unmatchable against its counterpart, turning a +false conflict into "needs manual resolution" that would otherwise have auto-solved +cleanly — empirically confirmed real-world case: `baseEffect.ws` failed to auto-solve +with mismatched encodings against the (now-retired) KDiff3 engine, and succeeded cleanly +once normalized, with correct merged output. `Tools/FileEncoding.cs` handles this: +`ReadAnyEncoding`/`WriteUtf16` give `DiffPlexMergeEngine` UTF-16LE normalization without +needing a temp file, since it merges in-process text directly (`EnsureUtf16File`, an +on-disk-copy variant for a file-based tool, has no in-repo caller since KDiff3's +retirement but is kept, still unit-tested, for any future file-based tool). **Never +normalize toward UTF-8** — the game may not load a merged `.ws` file in that encoding. +`ReadAnyEncoding` deliberately uses `File.ReadAllText(path)`'s built-in BOM +auto-detection rather than decoding raw bytes with a fixed `Encoding` instance — the +latter does not strip a detected BOM, leaving a stray U+FEFF glued to the first line, +confirmed empirically to reproduce the exact `baseEffect.ws`-style false conflict this +mechanism exists to avoid (see `WitcherScriptMerger.Tests`'s +`MergeHeadless_EncodingMismatch_...` fixture). + +## Dependency validation + +`Paths.ValidateTextMergeDependencies()` always returns `true` — `DiffPlexMergeEngine` is +in-process and needs no external binary (`DiffPlexMergeEngine.ValidateExePath()` always +returns `true` too). It's kept as a named method, rather than removed, because callers +(`WsmMcpTools`, `DependencyForm`) already call it by this name and it documents intent at +each call site. `Paths.ValidateBundleDependencies()` checks QuickBMS's exe + plugin and +wcc_lite's exe actually exist on disk. `Paths.ValidateDependencyPaths()` is just +`ValidateTextMergeDependencies() && ValidateBundleDependencies()`, kept for existing +callers that want the combined check. + +The split exists so a host that only supports flat-file (`.ws`/`.xml`) conflicts — +`WitcherScriptMerger.Headless`, which has no QuickBMS/wcc_lite bundled at all — can gate +starting a scan/merge run on just the text-merge engine, without also requiring bundle +tooling it deliberately doesn't ship. Bundle-category conflicts still fail gracefully +per-conflict when attempted without QuickBMS/wcc_lite regardless of which gate a given +entry point uses (see `Tools/QuickBms.cs`'s `IsAvailable` and its callers, +`FileIndex/ModFileIndex.BuildAsync`, `Inventory/FileMerger.GetUnpackedFiles`) — this split +only changes what gates a *run starting at all*, not the per-conflict bundle behavior. +**Which gate a given entry point actually uses differs by host and by call** — see each +host's own `CLAUDE.md` for its own startup-level check; the MCP tools' own per-call gate +(`RequireDependenciesAndModsDirectory`, above) always uses the text-merge-only check +regardless of host. + +## Hash format (`MergeInventory.xml`) + +**Load-bearing.** `MergeInventory.xml` (including real, already-populated files on +developer machines) stores per-file hashes (`Tools/Hasher.cs`, xxHash32 via +`System.IO.Hashing`) compared by string equality to detect when a mod source file has +changed since it was last merged. Any change to `Hasher.cs` must produce byte-for-byte +identical output to the current implementation, or every existing recorded merge +silently "goes stale." `MergeInventory.HasResolvedConflict` re-checks these hashes on +refresh to detect merges made stale by upstream mod file changes. Verify any change with +the synthetic-edge-cases + real-recorded-hash cross-check pattern described in +`WitcherScriptMerger.Tests/CLAUDE.md`. + +## Settings & persistence + +- App settings: `AppSettings.cs` wraps `System.Configuration.ConfigurationManager` over + `App.config`'s `` block (`Get`/`Get`/`Set`/`Save`, cached + `Configuration` object) — deliberately *not* `Properties.Settings` (removed during the + SDK-style migration). Settings are cached and require an explicit `Save()` call. +- Merge history: `MergeInventory.xml`, via `XmlSerializer` (`Inventory/MergeInventory.cs`). +- Game load order: `LoadOrder/CustomLoadOrder.cs` reads the game's own `mods.settings` + file. diff --git a/WitcherScriptMerger.Core/Mcp/CLAUDE.md b/WitcherScriptMerger.Core/Mcp/CLAUDE.md index f8266b2..5b5be4f 100644 --- a/WitcherScriptMerger.Core/Mcp/CLAUDE.md +++ b/WitcherScriptMerger.Core/Mcp/CLAUDE.md @@ -1,8 +1,11 @@ # CLAUDE.md — Mcp/ -Guidance specific to `WsmMcpTools.cs`. See the root `CLAUDE.md`'s "MCP mode" section for -the tool list, transport rationale, and per-call state model — this file covers only what -that section doesn't: exactly what the process touches, and at what privilege level. +Guidance specific to `WsmMcpTools.cs`. See `../CLAUDE.md`'s "CLI & MCP orchestration" +section for the tool list, transport rationale, and per-call state model — this file +covers only what that section doesn't: exactly what the process touches, and at what +privilege level. See each host's own `CLAUDE.md` (`WitcherScriptMerger/CLAUDE.md`, +`WitcherScriptMerger.Headless/CLAUDE.md`) for that host's own `mcp` verb startup gate, +which is not necessarily the same as the per-call gate described here. ## Minimal required permissions @@ -23,8 +26,9 @@ that section doesn't: exactly what the process touches, and at what privilege le - The app's own install directory — `Paths.Inventory` (`MergeInventory.xml`), `Paths.TempBundleContent` (`tempbundlecontent`), and `Paths.MergedBundleContent` (`Merged Bundle Content`) are all relative paths, resolved against - `Environment.CurrentDirectory`, not against the mods/game tree. `Program.RunCli` - pins `Environment.CurrentDirectory = AppContext.BaseDirectory` before dispatching to + `Environment.CurrentDirectory`, not against the mods/game tree. The WinForms host's + `Program.RunCli` and the Headless host's `Program.Main` both pin + `Environment.CurrentDirectory = AppContext.BaseDirectory` before dispatching to either the `merge` or `mcp` verb, so in practice this is always the directory the executable itself lives in, regardless of what directory an MCP client launches it from — verified empirically (a client-supplied working directory had no effect; @@ -32,14 +36,29 @@ that section doesn't: exactly what the process touches, and at what privilege le - `Paths.DiffPlexConflictsDirectory` (`DiffPlexConflicts`, next to the executable for the same `Environment.CurrentDirectory` reason as the previous bullet) — every genuine conflict `merge_conflicts` processes writes a git/diff3-style conflict-marker - sidecar file here (see the root `CLAUDE.md`'s "Interactive vs. headless split" + sidecar file here (see `../CLAUDE.md`'s "DiffPlexMergeEngine (the text-merge engine)" section), even for a `dryRun` call. Nothing sweeps this directory automatically, unlike `TempBundleContent`. - **`merge_conflicts`'s `relativePaths` and `orderOverrides` keys are validated against `Paths.ModsDirectory`** before any scan or merge runs (`WsmMcpTools.EnsureInScope` / `IsWithinModsDirectory`) — an entry that doesn't resolve inside that directory (absolute path, UNC path, or a `..\` escape) is rejected with a clear error rather than silently - matching nothing or being joined into a path outside the intended scope. + matching nothing or being joined into a path outside the intended scope. Neither value + is actually joined into a filesystem path anywhere in this codebase today + (`relativePaths` is only ever compared for equality against already-scanned + `ModFile.RelativePath` values; `orderOverrides` values reach `Path.Combine` only after + being validated against `ModFile.ContainsMod`, a whitelist of real scanned mod folder + names) — this check is defense-in-depth, not a fix for a live traversal. This applies + mods-directory-relative semantics uniformly to every category; for + `Categories.BundleText` specifically, `conflict.RelativePath` is actually a path + *internal to a bundle archive* (from `QuickBms.GetBundleContentPaths`), not one rooted + at `Paths.ModsDirectory` — an ordinary internal path (e.g. `engine\foo.ws`) still + validates fine, but a bundle whose internal listing itself contained a rooted or + `..`-bearing entry would make that specific conflict unreachable via `relativePaths` + (rejected as out-of-scope even though it's a legitimate conflict). Unexercised: every + scratch config used to verify this was built with `CheckBundleContents=false`, + consistent with the bundle path's "code-reviewed but not round-tripped" verification + status described in each host's own `CLAUDE.md`. - **No network access beyond the transport itself.** The MCP SDK speaks JSON-RPC over the process's stdin/stdout pipes to whatever spawned it (`WithStdioServerTransport()`) — that is the only inbound/outbound channel this process opens on its own. diff --git a/WitcherScriptMerger.Headless/CLAUDE.md b/WitcherScriptMerger.Headless/CLAUDE.md new file mode 100644 index 0000000..e0462a5 --- /dev/null +++ b/WitcherScriptMerger.Headless/CLAUDE.md @@ -0,0 +1,169 @@ +# CLAUDE.md — WitcherScriptMerger.Headless + +Guidance for working in `WitcherScriptMerger.Headless` (`net10.0` — no `-windows` +suffix, `Exe`), the Linux-capable CLI/MCP-only host. Only the `merge` CLI verb and `mcp` +server mode exist here — no WinForms reference anywhere in the project, no GUI code path +to fall back to. A first concrete step toward "true headless operation for CLI/Agent +interaction, focused on modded-gaming + Vortex workflows." References +`WitcherScriptMerger.Core` only — see `WitcherScriptMerger.Core/CLAUDE.md` for the domain +logic (`FileMerger`, `DiffPlexMergeEngine`, `Cli/MergeOperations`, `Mcp/WsmMcpTools`) this +project only adds thin routing around. See the root `CLAUDE.md` for overall project +context. + +The whole project is just `Program.cs` and its own `App.config`. + +## Routing (`Program.cs`) + +Mirrors `WitcherScriptMerger/Program.cs`'s `args[0] == "merge"` / `args[0] == "mcp"` +dispatch, but with no third (no-args-launches-GUI) branch — no args, or an unrecognized +first argument, prints usage to stderr and exits 1. + +`Environment.CurrentDirectory = AppContext.BaseDirectory` is set as the very first +statement in `Main`, before touching `AppState.Settings`/`Paths` at all — several Core +paths are relative to it (`Paths.Inventory`, `Paths.TempBundleContent`, +`Paths.DiffPlexConflictsDirectory`, `Paths.MergedBundleContentAbsolute`'s field +initializer; see Core's `CLAUDE.md`). This mirrors the WinForms host's `Program.RunCli` +doing the same as its own first statement, except unconditionally as the very first +thing here — this host has no no-args-launches-GUI branch to worry about leaving +unreset. + +## What this host deliberately omits + +- **No GUI.** No `System.Windows.Forms` reference at all, so there's nothing that + *could* launch one. +- **No console-attach machinery.** No `[STAThread]`, no `AttachConsole`/ + `MaybeAttachConsole` P/Invoke (`kernel32.dll`, Windows-only) — this project has + nothing console-attach-shaped to do (it's always launched as a plain console app, and + stdout is reserved for MCP protocol frames in `mcp` mode regardless), so that + Windows-specific mechanism was left out entirely rather than ported. +- **No KDiff3 — and never had it.** KDiff3 is retired repo-wide (see + `docs/decisions/kdiff3-retirement.md`), so this is true of the WinForms host too now, + but worth stating explicitly here: this project never had a `Tools/KDiff3.cs`-style + Win32 P/Invoke dependency to begin with, since it postdates the retirement. `Program.cs` + sets no engine at all — `FileMerger` builds its own `DiffPlexMergeEngine` directly, + identically to the WinForms host, with no `MergeEngine` App.config switch on either + host. + +The actual scan/merge/MCP-tool orchestration is unchanged, shared Core code +(`Cli/MergeOperations.cs`, `Mcp/WsmMcpTools.cs`) — this project only replicates the thin +CLI argument-parsing/dispatch glue around it, which was small enough not to warrant +extracting into Core too. + +## Dependency gating: text-merge engine only + +Both `RunMerge` and `RunMcp` here gate on `Paths.ValidateTextMergeDependencies()` only — +**not** the combined `Paths.ValidateDependencyPaths()` that the WinForms host's `merge` +and `mcp` verbs both require (see `WitcherScriptMerger/CLAUDE.md`). This host has no +QuickBMS/wcc_lite bundled at all (see "External tool dependencies" in the root +`CLAUDE.md` and `docs/decisions/bundle-format-replacement-spike.md` — no cross-platform +replacement was found to exist), so requiring the full combined check would mean this +host could never merge even its supported flat-file (`.ws`/`.xml`) conflicts. + +**Flat-file conflicts only — bundle-content conflicts are unsupported, by design, not by +oversight.** `App.config`'s `CheckBundleContents` defaults to `false` here (unlike the +WinForms host's `true`) specifically so a normal run never touches bundle scanning at +all. If a user turns it on anyway (or points `QuickBmsPath`/`WccLitePath` at real, +sourced-separately Windows binaries — those settings still exist here and work if this +host happens to be run on Windows), bundle-category conflicts fail gracefully rather than +crashing: `FileIndex/ModFileIndex.BuildAsync` (Core) checks `Tools/QuickBms.IsAvailable` +once per scan, not once per bundle, and if unavailable, prints one clear message and +skips bundle scanning entirely for that run instead of attempting it. + +This replaced a real crash found by code inspection when this host was first built: +`Tools/QuickBms.GetBundleContentPaths` used to return `null` when QuickBMS couldn't be +found, and both `ModFileIndex.BuildAsync` and `FileMerger.GetUnpackedFiles` enumerated +that return value directly — unreachable on the WinForms host (which always gates +bundle-category scanning behind the combined `ValidateDependencyPaths()`, guaranteeing +real QuickBMS present), but reachable here for the first time. Fixed at the source: +`GetBundleContentPaths` now returns `Array.Empty()` instead of `null`. +`FileMerger.GetUnpackedFiles`'s vanilla-bundle search (`Directory.GetDirectories` over +`Paths.BundlesDirectory`/`Paths.DlcDirectory`) is also now guarded against a missing +`content`/`DLC` directory (`DirectoryNotFoundException` otherwise), since a +scratch/incomplete game tree can reach this code here without a full real Witcher 3 +install backing it. Both fixes live in Core, not this project, but neither was +reachable before this host existed. + +Verified end-to-end in a scratch tree: a mod folder containing a junk `.bundle` file, +with `CheckBundleContents=true` and no QuickBMS configured, scans and merges cleanly +(flat-file conflicts still merge/skip correctly; the bundle file is never opened at all) +with one clear warning message and no exception, on both Windows and real Linux (see +below). + +## Two real cross-platform path-separator bugs, found via real Linux testing + +Found and fixed only by actually running the `linux-x64` publish under WSL2 (a real +Linux kernel, not just a cross-compile target check) — building/publishing for +`linux-x64` alone would not have caught either. Both live in shared +`WitcherScriptMerger.Core` code, not in this project, but neither was reachable before +this host existed, since the WinForms host is Windows-only: + +- `FileIndex/ModFile.GetModNameFromPath` used a hardcoded `'\\'` to find the + mod-folder-name segment of a full path. On Linux, `Path.Combine`-built paths use `/`, + so `IndexOf('\\')` always returned `-1`, and the subsequent `Substring(0, -1)` threw + `ArgumentOutOfRangeException` on the first flat-file merge attempted — a hard crash on + every `merge` invocation. Fixed to use `Path.DirectorySeparatorChar`. +- `Mcp/WsmMcpTools.cs`'s `merge_conflicts` normalized a client-supplied + `relativePaths`/`orderOverrides` key by replacing `/` with a hardcoded `'\\'` to match + `ModFile.RelativePath`'s separator convention — correct on the WinForms host (always + Windows), silently wrong on Linux, where `ModFile.RelativePath` itself uses `/`: a + client sending a `/`-separated path (the natural style on any OS) would get + "normalized" to `\`-separated, never match, and land in `unmatched` looking like it + wasn't a real conflict at all. Fixed to normalize both possible separators to + `Path.DirectorySeparatorChar` instead of assuming `\`. + +The rest of Core was grepped for the same hardcoded-`'\\'`/`"\\"` pattern after finding +these two; no other occurrences remained. + +## Publishing + +No existing `.pubxml` profiles in this repo — these commands are the documented +convention instead. Cross-compiling for `linux-x64` works fine from Windows — producing +the binary doesn't require a Linux machine, only *running* it does: + +``` +dotnet publish WitcherScriptMerger.Headless/WitcherScriptMerger.Headless.csproj -r win-x64 --self-contained -p:PublishSingleFile=true -c Release +dotnet publish WitcherScriptMerger.Headless/WitcherScriptMerger.Headless.csproj -r linux-x64 --self-contained -p:PublishSingleFile=true -c Release +``` + +Each publish's `.dll.config` (the `App.config` copy +`System.Configuration.ConfigurationManager` actually reads) lands next to the executable +— copy it there if deploying the exe on its own. Confirmed empirically that this +resolves correctly even in a single-file publish, despite +`Assembly.GetEntryAssembly().Location` being documented (and confirmed here too, via a +real build's `IL3000` warning) to always return `""` for a single-file-bundled assembly: +`ConfigurationManager.OpenExeConfiguration("")` still finds and reads the real +`.dll.config` sitting beside the actual running executable, both with and +without that file present (missing-config still correctly triggers `AppSettings`'s +existing `Environment.Exit(1)` path) — no `AppSettings.cs` change was needed for +single-file publishing to work. + +## Verification status + +`dotnet build WitcherScriptMerger.sln` and `dotnet test` (the `WitcherScriptMerger.Tests` +suite) both pass with this host's code present. Self-contained single-file `win-x64` and +`linux-x64` publishes both succeed. + +This host was verified against a **real Linux runtime**, not just a successful +cross-compile: WSL2 (Ubuntu 20.04, genuine Linux kernel, both `/mnt/c`-mounted and +native `ext4`-backed scratch trees) was used to actually run the published `linux-x64` +binary. Confirmed there: + +- The `merge` verb against synthetic scratch mods (one auto-solvable conflict, correctly + merged with UTF-16LE+BOM output matching vanilla's encoding; one genuine conflict, + correctly skipped with git/diff3-style conflict markers written under + `DiffPlexConflicts/` and surviving process exit). +- The `mcp` verb's full stdio round-trip (`initialize` → `tools/list` → `tools/call` for + all four tools, including a `merge_conflicts` call using a forward-slash + `relativePaths` entry specifically to exercise the separator-normalization fix above). +- The bundle-graceful-degradation path (junk `.bundle` file, `CheckBundleContents=true`, + no QuickBMS — clean warning, no crash, exit code 2). + +The equivalent Windows-side checks (both verbs, both self-contained single-file +publishes) were also run and matched in shape. + +**Not verified**: an actual bare-metal/native Linux distribution outside WSL2, and the +bundle path was only exercised with a junk (non-`POTATO70`-format) `.bundle` file — a +real bundle-vs-bundle conflict was judged impractical to construct without +QuickBMS/wcc_lite (matching the WinForms host's own "bundle path is code-reviewed but not +round-tripped" status — see `WitcherScriptMerger/CLAUDE.md`), so that specific scenario +relies on code inspection of the fixes above, not an end-to-end run. diff --git a/WitcherScriptMerger.Tests/CLAUDE.md b/WitcherScriptMerger.Tests/CLAUDE.md new file mode 100644 index 0000000..d8ca573 --- /dev/null +++ b/WitcherScriptMerger.Tests/CLAUDE.md @@ -0,0 +1,75 @@ +# CLAUDE.md — WitcherScriptMerger.Tests + +Guidance for working in `WitcherScriptMerger.Tests` (`net10.0`, xunit). This is the +only test project in the repo, and it covers `WitcherScriptMerger.Core` only — it does +**not** reference either host project (no WinForms). Run with +`dotnet test WitcherScriptMerger.Tests/WitcherScriptMerger.Tests.csproj` (or +`dotnet test WitcherScriptMerger.sln`). See the root `CLAUDE.md` for overall project +context and `WitcherScriptMerger.Core/CLAUDE.md` for what the code under test actually +does. + +## What's covered + +- `Tools/DiffPlexMergeEngineTests.cs` — `DiffPlexMergeEngine`'s `BuildMerge`/`Merge`/ + `MergeHeadless`: whitespace-only auto-resolve, conflict-marker sidecar behavior, the + vanilla-less guard, and the `DiffAlgorithmException` defense (`BuildMerge_ + InterleavedIndependentEdits_...`/`MergeHeadless_InterleavedIndependentEdits_...` — + regression tests for the confirmed upstream DiffPlex bug documented in Core's + `CLAUDE.md`). +- `Tools/FileEncodingTests.cs` — `FileEncoding`'s UTF-16LE normalization, including the + `MergeHeadless_EncodingMismatch_...` fixture reproducing the `baseEffect.ws`-style false + conflict that motivated it. +- `Tools/HasherTests.cs` — `Hasher`'s xxHash32 output, including synthetic edge cases. +- `Tools/KDiff3CrossCheckTests.cs` — an auto-solvable-only A/B check of + `DiffPlexMergeEngine` against a real `KDiff3.exe` binary, when a developer happens to + have one locally (WSM no longer bundles or requires KDiff3 itself — see + `docs/decisions/kdiff3-retirement.md`). +- `LiveInstall.cs` — see "Live-install cross-check tests" below. + +## `AppState.Settings`-safety constraints + +Tests never construct `FileMerger.MergeSource` via `MergeSource.FromFlatFile`/ +`FromBundle` (both call `ModFile.GetModNameFromPath` → `Paths.ModsDirectory` → +`AppState.Settings`) or otherwise force `AppState.Settings` to construct outside a real +GUI/CLI/MCP entry point. `AppSettings`'s constructor calls `Environment.Exit(1)` if it +can't find a config file next to `Assembly.GetEntryAssembly().Location`, and in a +`dotnet test` host (`testhost.dll`, no matching `.config`) that kills the entire test +process, not just one test. + +`AppState.Settings` is a lazy property specifically so that merely touching +`AppState.Notifier` (which Core code — e.g. `DiffPlexMergeEngine`'s headless skip/guard +messages — legitimately does on its own) doesn't also force `Settings` to construct; see +`WitcherScriptMerger.Core/CLAUDE.md`'s "AppState & IMergeNotifier" section for the full +mechanism. `Paths.cs`'s own properties +(`ScriptsDirectory`/`ModsDirectory`/`IsScriptsDirectoryDerived`/`IsModsDirectoryDerived`) +read `AppState.Settings.Get(...)` on every access rather than caching the result via a +static field initializer, for the identical reason one layer further out — a field +initializer there would've forced `Settings` to construct merely from touching an +unrelated static member of `Paths` (e.g. `GetRelativePath`), via C#'s `beforefieldinit` +semantics, silently undermining `AppState.Settings`'s own laziness. + +`Tools/DiffPlexMergeEngine.GetConflictMarkerPath` reads only the compile-time-literal +`Paths.DiffPlexConflictsDirectory` const, which never triggers `Paths`'s type initializer +at all, so it's safe to call from tests unconditionally. Tests that need a +`FileMerger.MergeSource` build it directly via object-initializer syntax instead (its +fields are all public) rather than going through `FromFlatFile`/`FromBundle`. + +## Live-install cross-check tests (`WSM_TEST_GAME_DIR`) + +A few tests optionally cross-check against a real Witcher 3 + WitcherScriptMerger +install — a live `MergeInventory.xml`'s recorded hashes, or a real `KDiff3.exe` binary a +developer happens to have locally (for `KDiff3CrossCheckTests.cs`'s auto-solvable-only +A/B check against `DiffPlexMergeEngine`) — via `LiveInstall.cs`, gated entirely on the +`WSM_TEST_GAME_DIR` environment variable (unset by default). This is **never** a +hardcoded or scanned path, per the repo's "scrub machine-specific paths" rule (see +`CONTRIBUTING.md`). These tests silently no-op when the variable is unset. + +## Beyond what this project covers + +For anything not covered here — especially further hash-output or `MergeInventory.xml` +schema changes — the precedent set in the (local, gitignored) `HANDOFF.md` still applies: +a disposable, non-committed `dotnet new console` scratch app, exercising synthetic edge +cases plus a cross-check against a real value already recorded in a live +`MergeInventory.xml`. Follow that pattern rather than assuming API docs alone are +sufficient, particularly for anything hash- or serialization-related — see Core's +`CLAUDE.md`'s "Hash format" section for why that's load-bearing. diff --git a/WitcherScriptMerger/CLAUDE.md b/WitcherScriptMerger/CLAUDE.md new file mode 100644 index 0000000..053a295 --- /dev/null +++ b/WitcherScriptMerger/CLAUDE.md @@ -0,0 +1,238 @@ +# CLAUDE.md — WitcherScriptMerger (WinForms host) + +Guidance for working in `WitcherScriptMerger`, the WinForms host project +(`net10.0-windows7.0`, `WinExe`, `UseWindowsForms=true`). This is the original, +full-featured entry point: GUI + CLI + MCP, all three dispatched from this project's +`Program.cs`. It references `WitcherScriptMerger.Core` for all domain logic — see +`WitcherScriptMerger.Core/CLAUDE.md` for `FileMerger`, `DiffPlexMergeEngine`, +`AppState`/`IMergeNotifier`, and the CLI/MCP orchestration shared with +`WitcherScriptMerger.Headless`. See the root `CLAUDE.md` for overall project context. + +There is no MVC/MVP split here — `Forms/MainForm.cs` (~1000 lines) is a monolithic +orchestrator that directly owns the tree controls, constructs `ModFileIndex`, drives +merges via `InteractiveMergeRunner`, and wires up async callbacks. + +## Build, run & publish + +- Run (GUI, no args): launch the built `WitcherScriptMerger.exe`, or + `dotnet run --project WitcherScriptMerger/WitcherScriptMerger.csproj`. +- Run (CLI/MCP): see "CLI mode" / "MCP mode" below. +- **Publish** (self-contained single-file, `win-x64` only — it's a WinForms app, never + makes sense on Linux; no existing `.pubxml` profile in this repo, this command is the + documented convention instead): + ``` + dotnet publish WitcherScriptMerger/WitcherScriptMerger.csproj -r win-x64 --self-contained -p:PublishSingleFile=true -c Release + ``` + The publish's `.dll.config` (the `App.config` copy + `System.Configuration.ConfigurationManager` actually reads, via Core's + `AppSettings.cs`) lands next to the executable — copy it there if deploying the exe on + its own. This resolves correctly even in a single-file publish despite + `Assembly.GetEntryAssembly().Location` being documented (and confirmed via a real + build's `IL3000` warning) to always return `""` for a single-file-bundled assembly — + `ConfigurationManager.OpenExeConfiguration("")` still finds and reads the real + `.dll.config` sitting beside the actual running executable, both with + and without that file present (missing-config still correctly triggers `AppSettings`'s + `Environment.Exit(1)` path). This finding applies identically to + `WitcherScriptMerger.Headless` — see that project's own `CLAUDE.md`'s "Publishing" + section for its own (`win-x64`/`linux-x64`) publish commands; the underlying mechanism + is shared Core behavior (`AppSettings.cs`), verified independently on both hosts. + +## Folder map + +- `Forms/` — WinForms screens: `MainForm.cs` (the hub, also implements + `IMergeNotifier`), `OptionsForm.cs`, `DependencyForm.cs` (startup blocker if tool + paths are invalid), `MergeReportForm.cs`, `PackReportForm.cs`, `PriorityPrompt.cs`, + `MessageBoxManager.cs` (see "MessageBoxManager" below). +- `Controls/` — custom `TreeView` subclasses: `SMTree.cs` (base, metadata/context-menu + logic), `ConflictTree.cs` (detected conflicts), `MergeTree.cs` (existing merges), + `SMTreeSorter.cs`, `ToolStripRegion.cs`. +- `Inventory/` — `InteractiveMergeRunner.cs`: the host-side counterpart to Core's + `FileMerger` for the interactive path (see "Interactive merge flow" below). +- `Tools/` — empty as of KDiff3's retirement (`docs/decisions/kdiff3-retirement.md`); + used to hold `KDiff3.cs` (Win32 P/Invoke for window-title polling) and + `KDiff3MergeEngine.cs`. The sole text-merge engine, `DiffPlexMergeEngine`, lives in + Core. +- Root: `Program.cs` (entry point: GUI, CLI, and MCP — see "Startup flow" below), + `Extensions.cs` (WinForms-specific `TreeNode`/`TreeView` helpers and Win32 P/Invoke; + pure string helpers live in Core's `StringExtensions.cs` instead), `TaskbarProgress.cs`, + `App.config`, `Properties/AssemblyInfo.cs` (see the TFM compatibility constraint below). + +## `MainForm`'s `IMergeNotifier` translation + +`MainForm` implements `IMergeNotifier`, translating Core's neutral +`NotifyResult`/`NotifyButtons`/`DialogIcon` types to/from real `MessageBox.Show(...)`/ +`DialogResult` calls. This is **not** a behavior-identical passthrough: one real prompt +(`LoadOrderValidator.PromptToPrioritizeMergedMod`'s "Custom Load Order Problem" dialog, +Core) lost a `MessageBoxManager`-based custom button caption that used to mark its +Cancel option as destructive/permanent (that relabeling mechanism was likely already +silently broken pre-split — see "MessageBoxManager" below — but the loss is real either +way; the warning is now spelled out in the message body instead, since `IMergeNotifier` +has no hook for relabeling a button). `ShowModal(Form)` isn't part of `IMergeNotifier` at +all — every call site is GUI-only, interactive code in this project, which calls +`MainForm.ShowModal` directly instead of going through the notifier abstraction. + +## MessageBoxManager + +`Forms/MessageBoxManager.cs` (from a 2010s CodeProject article) hooks +`SetWindowsHookEx(WH_CALLWNDPROCRET, ...)` on the current thread to relabel a standard +`MessageBox`'s buttons before it displays. It's still actively used at one call site — +`MainForm.PromptToDeleteForChangedHash`, a direct `MessageBox.Show(...)` call (not routed +through `IMergeNotifier`, since it's UI-only code) that relabels Cancel to `"Ne&ver"` via +`MessageBoxManager.Register()`/`.Unregister()` around the call. Its hook mechanism keys +off `AppDomain.GetCurrentThreadId()`, a deprecated API that doesn't reliably return the +real Win32 thread ID `SetWindowsHookEx` needs — it was likely already silently broken +before the Core/host split, independent of anything this split changed. This is why the +equivalent `LoadOrderValidator` prompt (see above) doesn't try to route a relabeled +button through `IMergeNotifier`: rather than propagate a mechanism that may not actually +work, that prompt spells the "Cancel is permanent" warning out in the message text +instead. + +## Interactive merge flow (`InteractiveMergeRunner`) + +`Inventory/InteractiveMergeRunner.cs` is the host-side counterpart to Core's +`Inventory/FileMerger.cs` for the interactive (GUI) path — see +`WitcherScriptMerger.Core/CLAUDE.md`'s "FileMerger: interactive vs. headless split" for +the Core side. Its public API (constructor shape, `MergeByTreeNodesAsync`, +`RepackBundleAsync`) deliberately mirrors the pre-split `FileMerger` so `MainForm`'s call +sites needed only a type-name change. + +1. `FileIndex/ModFileIndex.BuildAsync` (Core) scans `Paths.ModsDirectory`, groups files + by relative path, flags conflicts. +2. `MainForm` feeds the results into `ConflictTree`/`MergeTree`; the user checks nodes to + merge. +3. `InteractiveMergeRunner.MergeByTreeNodesAsync` extracts one `FileMerger. + InteractiveMergeRequest` per checked `TreeNode` (`ExtractRequest`) **inside its + `BackgroundWorker`'s `DoWork`**, not before starting it — this matches the pre-split + threading model, and matters beyond fidelity: `ExtractRequest` dereferences node + metadata and casts `Tag` with no null/type check, and `BackgroundWorker` captures any + exception from `DoWork` into `RunWorkerCompletedEventArgs.Error` instead of letting it + propagate. Extracting outside `DoWork` would let that same exception throw + synchronously on the UI thread instead, which modern .NET WinForms terminates the + process for by default (unlike .NET Framework's more forgiving behavior). +4. `FileMerger.MergeFilesInteractive` (Core) builds/reuses a `Merge` record per file and + dispatches to `MergeFlatFileInteractive` (plain `.ws`/`.xml`) or + `MergeBundleFileInteractive` (bundle-packed files, first unpacked via + `Tools/QuickBms.UnpackFile`). +5. `FileMerger.MergeTextInteractive` calls `DiffPlexMergeEngine.Merge(...)` (Core; + in-process, auto-solving, or a conflict-marker sidecar for a genuine conflict — see + Core's `CLAUDE.md`). +6. On success, `MergeInventory.AddModToMerge` (Core) hashes the result and persists the + merge record; bundle content changes additionally go through `FileMerger. + PackNewBundle` → `WccLite.PackBundle` + `GenerateMetadata` to repack `blob0.bundle`. +7. `InteractiveMergeRunner` owns the `BackgroundWorker` and supplies the `OnMergeReport`/ + `OnPackReport` callbacks that `FileMerger` invokes: `ShowMergeReport`/`ShowPackReport` + optionally play a completion sound (`Program.Settings.Get("PlayCompletionSounds")`) + and optionally pop up `MergeReportForm`/`PackReportForm` via `Program.MainForm.ShowModal` + (gated on `ReportAfterMerge`/`ReportAfterPack` settings). + +## Startup flow (`Program.cs`) + +`Program.Notifier`/`Settings`/`LoadOrder`/`Inventory` are pass-through properties onto +Core's `AppState` (see `WitcherScriptMerger.Core/CLAUDE.md`) — every pre-existing call +site in this project kept working unchanged. +`static readonly bool _consoleAttached = MaybeAttachConsole();` runs as a field +initializer, ahead of everything, so early startup failures are visible in the invoking +terminal when there are CLI args. `Program` has an explicit (empty) static constructor +for the same `beforefieldinit`-determinism reason `AppState` does (see Core's +`CLAUDE.md`) — now load-bearing here specifically because nothing in `Main()` necessarily +touches a `Program`-owned field anymore (its former fields became pass-through +properties), so without the explicit constructor the CLR could defer +`_consoleAttached`'s initializer arbitrarily; confirmed empirically with a minimal repro +mirroring this exact shape. + +`[STAThread] Main(string[] args)`: if `args` is non-empty, hands off entirely to +`RunCli(args)` and returns — the GUI is never touched. Otherwise: +`Application.EnableVisualStyles()` → check `Settings.HasConfigFile` → +`Paths.ValidateDependencyPaths()` (the **combined** check — QuickBMS *and* wcc_lite, +not just the text-merge engine; shows `DependencyForm` if either is missing) → construct +`MainForm`, reassign `Program.Notifier = MainForm`, `Application.Run(MainForm)`. + +### CLI mode (this host) + +`WitcherScriptMerger.exe merge [--order-file ]` merges every auto-solvable +conflict without opening any merge-tool window, then exits (a conflict needing manual +resolution opens its conflict-marker sidecar in the default editor instead — see Core's +`CLAUDE.md`). No-args still launches the GUI unchanged; passing `merge` (or any argument) +is what selects the CLI path. + +`RunCli` sets `Environment.CurrentDirectory = AppContext.BaseDirectory` as its first +statement (several Core paths are relative to it — see Core's `CLAUDE.md`'s +`DiffPlexConflictsDirectory` note), then dispatches on `args[0]`. **The `merge` verb +requires the full combined `Paths.ValidateDependencyPaths()`** (QuickBMS *and* wcc_lite, +not just the text-merge engine) before doing anything else — this host refuses to start +a merge run at all without full bundle tooling configured, unlike +`WitcherScriptMerger.Headless`'s `merge` verb, which only requires the text-merge engine +(see `WitcherScriptMerger.Headless/CLAUDE.md`). `--order-file ` is parsed into +the `orderOverrides` shape Core's `ResolveMergeOrder` expects (`{"relative\\path.ws": +["modA", "modB"]}`). Exit codes: 0 = every conflict merged, 1 = couldn't even start (bad +args/config/deps), 2 = ran, but one or more conflicts were skipped. + +- **Verification status**: the flat-file path (`Categories.Script`/`Categories.Xml`) is + verified end-to-end against real conflicting files (including the encoding-mismatch + scenario) and a synthetic guaranteed-conflict, in a scratch game/mods tree — never + against a live install. The bundle path (`Categories.BundleText`) is code-reviewed and + mirrors the proven flat-file orchestration, and its two building blocks + (`GetUnpackedFiles`, `PackNewBundle`) are unchanged, already-exercised code — but it + hasn't been round-tripped through a real bundle-vs-bundle conflict. If + `tempbundlecontent` accumulates across many CLI runs during development, clear it + between runs — one debugging session saw a single very slow run after a long buildup + that didn't reproduce once it was cleared. + +### MCP mode (this host) + +`WitcherScriptMerger.exe mcp` runs an MCP server over stdio (`ModelContextProtocol` +NuGet package, `Host.CreateApplicationBuilder().Services.AddMcpServer() +.WithStdioServerTransport().WithToolsFromAssembly(typeof(WsmMcpTools).Assembly)` — the +assembly must be passed explicitly since `WsmMcpTools` lives in Core, not this calling +assembly; the parameterless overload only scans the calling assembly and would silently +register zero tools). `RunMcp` **also gates on the full combined +`Paths.ValidateDependencyPaths()`** before starting the server at all — this host won't +even start an MCP server without QuickBMS/wcc_lite configured, regardless of whether the +client ever calls a bundle-touching tool. This is a stricter gate than +`WsmMcpTools.RequireDependenciesAndModsDirectory` itself applies per-call (text-merge +engine only — see Core's `CLAUDE.md`) and stricter than `WitcherScriptMerger.Headless`'s +own `mcp` verb (also text-merge-only) — the finer-grained per-tool distinction in Core +only actually matters for a host whose own startup gate doesn't already guarantee both. + +stdout must stay reserved for MCP protocol frames: `builder.Logging.AddConsole(o => +o.LogToStandardErrorThreshold = LogLevel.Trace)` keeps the SDK's own logging on stderr. +`MaybeAttachConsole()` reads its own `Environment.GetCommandLineArgs()` (whose index 0 is +the exe path itself, so index 1 is `Main`'s `args[0]`) and skips `AttachConsole` +specifically when that first real argument is `"mcp"` — there's no parent console to +attach to in this scenario (an MCP client spawns this process with its own redirected +pipes), and it would be pointless at best. + +- **Verification status**: smoke-tested end-to-end against a scratch game/mods tree — a + hand-rolled stdio client (`initialize` → `tools/list` → `tools/call` for each tool), + including a `merge_conflicts` call that exercised the (since-retired) `KDiff3. + RunHeadless` path at the time it was tested (detected a genuine conflict, killed the + stuck process, returned it in `skipped`) — the underlying engine has since changed to + `DiffPlexMergeEngine`, but the MCP-level behavior verified (a conflict comes back in + `skipped`, not silently dropped) is unaffected. The directory-allow-listing and + `dryRun` additions (Core's `EnsureInScope`/`IsWithinModsDirectory`, `MergeConflicts`'s + `dryRun` parameter) were separately verified via the same stdio-client approach plus an + in-process harness against `WitcherScriptMerger.Core` directly against a fake merge + engine — both that harness's fake-engine seam and the real KDiff3/QuickBMS/wcc_lite + binaries used at the time are historical now (the seam it used, `IMergeEngine`, no + longer exists post-retirement — see `docs/decisions/kdiff3-retirement.md`). Never run + against a live install. + +## Compatibility constraint: TFM must keep the explicit `7.0` OS-version suffix + +This project uses `false` (to keep the +hand-written `Properties/AssemblyInfo.cs`), which also suppresses the SDK's +auto-generated `[assembly: SupportedOSPlatform("windows")]` attribute — that attribute is +added manually in `AssemblyInfo.cs` instead. The TFM must stay `net10.0-windows7.0` (not +bare `net10.0-windows`); dropping the `7.0` suffix reintroduces ~800 spurious `CA1416` +platform-compatibility warnings. + +## Other host-only helpers + +`Program.TryOpenFile`/`TryOpenFileLocation`/`TryOpenDirectory` are this host's own +"open in the OS's default app" helpers, used for opening merged output files and mod +folders from the GUI. `TryOpenFile`'s non-`.exe` branch is a bare `Process.Start(path)` +with no `UseShellExecute = true`, which throws on modern .NET for a non-executable path +(silently swallowed by that method's own `catch`) — a known wart, not fixed here. Core's +`Tools/FileOpener.cs` (used by `DiffPlexMergeEngine` to open conflict-marker sidecars) is +a separately, correctly implemented equivalent — see Core's `CLAUDE.md`; the two are not +unified.