diff --git a/.gitignore b/.gitignore index 95ae484..744a094 100644 --- a/.gitignore +++ b/.gitignore @@ -237,3 +237,6 @@ HANDOFF*.md # Local build/packaging scratch + editor config dist-local/ .vscode/ + +# CDPR-derived bug-repro artifacts - never commit (third-party game content) +docs/bugs/artifacts/ diff --git a/WitcherScriptMerger.Core/Cli/MergeOperations.cs b/WitcherScriptMerger.Core/Cli/MergeOperations.cs index ab9be28..b253770 100644 --- a/WitcherScriptMerger.Core/Cli/MergeOperations.cs +++ b/WitcherScriptMerger.Core/Cli/MergeOperations.cs @@ -30,10 +30,11 @@ public static FileMerger.HeadlessMergeSummary RunMerge( IEnumerable conflicts, string mergedModName, IReadOnlyDictionary orderOverrides, - bool dryRun = false) + bool dryRun = false, + bool overwrite = false) { var merger = new FileMerger(inventory); - return merger.MergeConflictsHeadless(conflicts, mergedModName, orderOverrides, dryRun); + return merger.MergeConflictsHeadless(conflicts, mergedModName, orderOverrides, dryRun, overwrite); } } } diff --git a/WitcherScriptMerger.Core/Inventory/FileMerger.cs b/WitcherScriptMerger.Core/Inventory/FileMerger.cs index d4a1acd..ad3ee66 100644 --- a/WitcherScriptMerger.Core/Inventory/FileMerger.cs +++ b/WitcherScriptMerger.Core/Inventory/FileMerger.cs @@ -482,7 +482,8 @@ public HeadlessMergeSummary MergeConflictsHeadless( IEnumerable conflicts, string mergedModName, IReadOnlyDictionary orderOverrides, - bool dryRun = false) + bool dryRun = false, + bool overwrite = false) { var summary = new HeadlessMergeSummary(); @@ -519,8 +520,8 @@ public HeadlessMergeSummary MergeConflictsHeadless( var isBundle = conflict.Category == Categories.BundleText; var fullyMerged = isBundle - ? MergeBundleConflictHeadless(conflict, merge, orderedNames, dryRun) - : MergeFlatConflictHeadless(conflict, merge, mergedModName, orderedNames, dryRun); + ? MergeBundleConflictHeadless(conflict, merge, orderedNames, dryRun, overwrite) + : MergeFlatConflictHeadless(conflict, merge, mergedModName, orderedNames, dryRun, overwrite); if (!fullyMerged) { @@ -587,7 +588,7 @@ public HeadlessMergeSummary MergeConflictsHeadless( return summary; } - bool MergeFlatConflictHeadless(ModFile conflict, Merge merge, string mergedModName, string[] orderedNames, bool dryRun) + bool MergeFlatConflictHeadless(ModFile conflict, Merge merge, string mergedModName, string[] orderedNames, bool dryRun, bool overwrite) { var firstHash = conflict.Mods.First(h => h.Name.EqualsIgnoreCase(orderedNames[0])); var source1 = MergeSource.FromFlatFile(new FileInfo(conflict.GetModFile(orderedNames[0])), firstHash); @@ -597,14 +598,22 @@ bool MergeFlatConflictHeadless(ModFile conflict, Merge merge, string mergedModNa Path.Combine(Paths.ModsDirectory, source1.Name)); var realOutputPath = Path.Combine(Paths.ModsDirectory, mergedModName, relPath); - // Checked against the real would-be output path regardless of dryRun: a real - // run always declines to overwrite an existing output (HeadlessMergeNotifier's - // fixed default), so a dry run needs to predict that same "already exists, - // would be skipped" outcome rather than only ever reporting whether the text - // itself would auto-solve - otherwise a preview and the real run it's meant to - // predict could disagree on a conflict whose output already exists. - if (File.Exists(realOutputPath) && !ConfirmOutputOverwrite(realOutputPath)) + // Checked against the real would-be output path regardless of dryRun, so a + // preview predicts the same outcome the real run it previews would hit. + // Without overwrite, an existing output is a clearly-reported skip (never a + // YesNo prompt: headless runs have no one to answer it, and + // HeadlessMergeNotifier's fixed non-destructive default made the old prompt + // an unconditional, near-silent "no" - the skip reason lived only in stderr + // prompt text, and headless/MCP callers could never refresh an existing + // merge at all; see docs/bugs/function-level-merge-gap-handling.md's + // "Related, lesser finding"). With overwrite, the existing output is + // refreshed - the semantic a mod manager re-merging after a mod update + // actually needs. + if (!overwrite && File.Exists(realOutputPath)) + { + ReportOutputExistsSkip(conflict.RelativePath, realOutputPath); return false; + } // KDiff3 always physically writes its -o target on a successful solve - there's // no "check without writing" mode - so a dry run still needs somewhere real to @@ -646,17 +655,19 @@ static string DescribeAccumulated(IEnumerable namesSoFar) return names.Count > 1 ? "accumulated merge (" + string.Join(", ", names) + ")" : names[0]; } - bool MergeBundleConflictHeadless(ModFile conflict, Merge merge, string[] orderedNames, bool dryRun) + bool MergeBundleConflictHeadless(ModFile conflict, Merge merge, string[] orderedNames, bool dryRun, bool overwrite) { merge.BundleName = Path.GetFileName(Paths.RetrieveMergedBundlePath()); var realOutputPath = Path.Combine(Paths.MergedBundleContent, conflict.RelativePath); - // See the matching comment in MergeFlatConflictHeadless - same reasoning: check - // against the real would-be output regardless of dryRun, so a preview predicts - // the same "already exists, declined" outcome a real run would hit. - if (File.Exists(realOutputPath) && !ConfirmOutputOverwrite(realOutputPath)) + // See the matching comment in MergeFlatConflictHeadless - same reasoning and + // same overwrite semantics. + if (!overwrite && File.Exists(realOutputPath)) + { + ReportOutputExistsSkip(conflict.RelativePath, realOutputPath); return false; + } // Rooted under TempBundleContent instead of MergedBundleContent for a dry run so // its intermediate merge text can never linger there either - see the matching @@ -824,6 +835,13 @@ void RecordMergedSources(Merge merge, MergeSource source1, MergeSource source2) } } + // Interactive-path prompt (MergeFlatFileInteractive/MergeBundleFileInteractive) + // - a GUI user is present to actually answer it. The headless paths use + // ReportOutputExistsSkip below instead: under HeadlessMergeNotifier this + // prompt's fixed non-destructive default silently declined every overwrite, + // which meant headless/MCP merge_conflicts could never refresh an existing + // merge, with the reason buried in stderr prompt text (see docs/bugs/ + // function-level-merge-gap-handling.md's "Related, lesser finding"). bool ConfirmOutputOverwrite(string outputPath) { return (NotifyResult.Yes == AppState.Notifier.ShowMessage( @@ -833,6 +851,19 @@ bool ConfirmOutputOverwrite(string outputPath) DialogIcon.Exclamation)); } + // Headless-path counterpart to ConfirmOutputOverwrite: no prompt (there's no + // one to answer it) - states the skip and the way out. The overwrite + // parameter on MergeConflictsHeadless is what actually authorizes a refresh. + static void ReportOutputExistsSkip(string relativePath, string outputPath) + { + AppState.Notifier.ShowMessage( + "Skipped " + relativePath + ": merged output already exists.\n" + outputPath + + "\nPass --overwrite (CLI) or overwrite: true (MCP merge_conflicts) to refresh it.", + "Already Merged", + NotifyButtons.OK, + DialogIcon.Warning); + } + bool GetUnpackedFiles(string contentRelativePath, ref MergeSource source1, ref MergeSource source2) { if (_vanillaFile == null) diff --git a/WitcherScriptMerger.Core/Mcp/WsmMcpTools.cs b/WitcherScriptMerger.Core/Mcp/WsmMcpTools.cs index a25b0ea..450624d 100644 --- a/WitcherScriptMerger.Core/Mcp/WsmMcpTools.cs +++ b/WitcherScriptMerger.Core/Mcp/WsmMcpTools.cs @@ -70,7 +70,11 @@ public static object MergeConflicts( "listed, even if it shows up as a source because the file was already merged " + "once).")] Dictionary orderOverrides = null, [Description("If true, evaluates which conflicts would auto-solve without writing any " + - "merged output, repacking any bundle, or modifying MergeInventory.xml.")] bool dryRun = false) + "merged output, repacking any bundle, or modifying MergeInventory.xml.")] bool dryRun = false, + [Description("If true, refreshes conflicts whose merged output already exists (e.g. " + + "after a source mod updated). Without it, an already-merged conflict is skipped " + + "with a clear reason rather than silently rebuilt - and a dry run can only " + + "predict that skip, not whether the refreshed merge would auto-solve.")] bool overwrite = false) { // Validated before touching the mods folder or dependency state: this is pure // input validation the caller controls, so it should fail fast and @@ -139,7 +143,7 @@ public static object MergeConflicts( kv => kv.Value, StringComparer.OrdinalIgnoreCase); - var summary = MergeOperations.RunMerge(AppState.Inventory, conflicts, mergedModName, normalizedOrderOverrides, dryRun); + var summary = MergeOperations.RunMerge(AppState.Inventory, conflicts, mergedModName, normalizedOrderOverrides, dryRun, overwrite); // FileMerger guarantees a dry run never adds or updates records in the // loaded inventory (see MergeConflictsHeadless), but skipping the disk @@ -148,7 +152,7 @@ public static object MergeConflicts( if (!dryRun) AppState.Inventory.Save(); - return new { merged = summary.Merged, skipped = summary.Skipped, unmatched, dryRun, functionLevelDecisions = summary.FunctionLevelDecisions }; + return new { merged = summary.Merged, skipped = summary.Skipped, unmatched, dryRun, overwrite, functionLevelDecisions = summary.FunctionLevelDecisions }; } } diff --git a/WitcherScriptMerger.Headless/Program.cs b/WitcherScriptMerger.Headless/Program.cs index 3f9a141..98172ce 100644 --- a/WitcherScriptMerger.Headless/Program.cs +++ b/WitcherScriptMerger.Headless/Program.cs @@ -80,7 +80,7 @@ static void PrintUsage() Console.Error.WriteLine("WitcherScriptMerger.Headless - CLI/MCP-only host (no GUI)."); Console.Error.WriteLine(); Console.Error.WriteLine("Usage:"); - Console.Error.WriteLine(" WitcherScriptMerger.Headless merge [--order-file ]"); + Console.Error.WriteLine(" WitcherScriptMerger.Headless merge [--order-file ] [--overwrite]"); Console.Error.WriteLine(" WitcherScriptMerger.Headless mcp"); Console.Error.WriteLine(" WitcherScriptMerger.Headless --version"); Console.Error.WriteLine(); @@ -117,10 +117,13 @@ static int RunMerge(string[] args) } string orderFilePath = null; + var overwrite = false; for (int i = 1; i < args.Length; ++i) { if (args[i] == "--order-file" && i + 1 < args.Length) orderFilePath = args[++i]; + else if (args[i] == "--overwrite") + overwrite = true; else { Console.Error.WriteLine($"Unknown argument: {args[i]}"); @@ -150,7 +153,7 @@ static int RunMerge(string[] args) return 0; } - var summary = MergeOperations.RunMerge(AppState.Inventory, modIndex.Conflicts, mergedModName, orderOverrides); + var summary = MergeOperations.RunMerge(AppState.Inventory, modIndex.Conflicts, mergedModName, orderOverrides, dryRun: false, overwrite: overwrite); AppState.Inventory.Save(); diff --git a/WitcherScriptMerger/Program.cs b/WitcherScriptMerger/Program.cs index e8e9d11..b40ceae 100644 --- a/WitcherScriptMerger/Program.cs +++ b/WitcherScriptMerger/Program.cs @@ -189,10 +189,13 @@ static int RunCli(string[] args) } string orderFilePath = null; + var overwrite = false; for (int i = 1; i < args.Length; ++i) { if (args[i] == "--order-file" && i + 1 < args.Length) orderFilePath = args[++i]; + else if (args[i] == "--overwrite") + overwrite = true; else { Console.Error.WriteLine($"Unknown argument: {args[i]}"); @@ -222,7 +225,7 @@ static int RunCli(string[] args) return 0; } - var summary = MergeOperations.RunMerge(Inventory, modIndex.Conflicts, mergedModName, orderOverrides); + var summary = MergeOperations.RunMerge(Inventory, modIndex.Conflicts, mergedModName, orderOverrides, dryRun: false, overwrite: overwrite); Inventory.Save(); diff --git a/vortex-extension/src/mcpClient.ts b/vortex-extension/src/mcpClient.ts index de465e7..3500915 100644 --- a/vortex-extension/src/mcpClient.ts +++ b/vortex-extension/src/mcpClient.ts @@ -88,6 +88,17 @@ export interface MergeConflictsArgs { relativePaths?: string[]; orderOverrides?: Record; dryRun?: boolean; + /** + * Refresh conflicts whose merged output already exists (e.g. after a source mod + * updated). Without it the server skips already-merged conflicts with a clear + * reason - it never silently rebuilds them - which is the wrong default for a mod + * manager's own re-merge flow: a Vortex deployment changing a source mod is exactly + * the situation where the stale merged output SHOULD be rebuilt. Requires a WSM + * build with `merge_conflicts` overwrite support (> 0.6.2); an older server ignores + * unknown tool arguments, degrading to its previous skip behavior rather than + * erroring. + */ + overwrite?: boolean; } export interface MergeConflictsResult { diff --git a/vortex-extension/src/resolveAction.ts b/vortex-extension/src/resolveAction.ts index ceb9499..c6821fe 100644 --- a/vortex-extension/src/resolveAction.ts +++ b/vortex-extension/src/resolveAction.ts @@ -130,6 +130,11 @@ export async function resolveScriptConflicts( try { preview = await runMergeConflictsWorkflow(api, connect, tool.path, env, { dryRun: true, + // overwrite so the preview answers "would this auto-solve?" for + // already-merged conflicts too - without it, the server can only predict + // "skipped: output already exists" for them, and this preview exists to show + // what the real (also-overwrite) run below will actually do. + overwrite: true, activityMessage: 'Scanning for mergeable script conflicts...', }); } catch (err) { @@ -162,6 +167,11 @@ export async function resolveScriptConflicts( try { result = await runMergeConflictsWorkflow(api, connect, tool.path, env, { dryRun: false, + // A Vortex-driven re-merge is exactly the "source mod updated, stale merged + // output should be rebuilt" case overwrite exists for - see + // MergeConflictsArgs.overwrite's own doc comment (including version-skew + // behavior against a pre-overwrite WSM build). + overwrite: true, activityMessage: 'Merging script conflicts...', }); } catch (err) { diff --git a/vortex-extension/test/mcpClient.integration.test.ts b/vortex-extension/test/mcpClient.integration.test.ts index 011a5f1..a7b7f36 100644 --- a/vortex-extension/test/mcpClient.integration.test.ts +++ b/vortex-extension/test/mcpClient.integration.test.ts @@ -346,4 +346,49 @@ describe('WsmMcpClient integration - real merge round trip (auto-solve + genuine await client.close(); } }, 30_000); + + // Depends on the previous test having really merged itemA.ws - vitest runs tests in + // a file in declaration order, and this whole describe block already relies on that + // (the dry-run test asserts the merged file does NOT exist yet). + it('re-merging skips an already-merged file without overwrite, and refreshes it with overwrite: true', async () => { + const client = await WsmMcpClient.connect({ exePath: mergeExePath }); + try { + const mergedScriptPath = path.join( + mergeScratchDir, + 'Mods', + 'mod0000_MergedFiles', + 'content', + 'scripts', + 'game', + 'itemA.ws', + ); + expect(fs.existsSync(mergedScriptPath)).toBe(true); + const beforeMtime = fs.statSync(mergedScriptPath).mtimeMs; + + // Without overwrite: the already-merged file is a reported skip (the server + // never silently rebuilds an existing merge), and a dry run predicts the same. + const skippedPreview = await client.mergeConflicts({ dryRun: true }); + expect(skippedPreview.merged).toEqual([]); + expect(skippedPreview.skipped).toContain(MERGED_SCRIPT_RELATIVE_PATH); + + const skippedRun = await client.mergeConflicts({ dryRun: false }); + expect(skippedRun.merged).toEqual([]); + expect(skippedRun.skipped).toContain(MERGED_SCRIPT_RELATIVE_PATH); + expect(fs.statSync(mergedScriptPath).mtimeMs).toBe(beforeMtime); + + // With overwrite: the dry run can now answer "would this auto-solve?" for the + // already-merged file, and the real run actually refreshes it. + const overwritePreview = await client.mergeConflicts({ dryRun: true, overwrite: true }); + expect(overwritePreview.merged).toEqual([MERGED_SCRIPT_RELATIVE_PATH]); + expect(fs.statSync(mergedScriptPath).mtimeMs).toBe(beforeMtime); + + const overwriteRun = await client.mergeConflicts({ dryRun: false, overwrite: true }); + expect(overwriteRun.merged).toEqual([MERGED_SCRIPT_RELATIVE_PATH]); + const refreshedText = fs.readFileSync(mergedScriptPath, 'utf16le'); + expect(refreshedText).toContain('a = 100;'); + expect(refreshedText).toContain('b = 200;'); + } finally { + await client.close(); + } + }, 30_000); });