From 431f03d884b55215573d740d782edfbad381bde2 Mon Sep 17 00:00:00 2001 From: Chris Knight Date: Fri, 7 Aug 2026 12:48:48 -0400 Subject: [PATCH 1/2] Route LoadOrderValidator's prompt through Program.Notifier PromptToPrioritizeMergedMod called MessageBox.Show directly instead of Program.Notifier.ShowMessage, unlike every other domain call site. Its sole caller today is invoked only from MainForm.cs, so it was harmless in practice, but it was a landmine for any future headless (CLI/MCP) load-order validation path, which would otherwise hit an unmediated WinForms MessageBox.Show with no message pump watching it. IMergeNotifier.ShowMessage gained a trailing optional MessageBoxDefaultButton parameter (default Button1, matching MessageBox.Show's own default) so the prompt's Button2 (No) default survives the move - dropping it silently would have flipped the default action from "leave load order alone" to "rewrite mods.settings". MainForm.ShowMessage forwards it to the 6-arg MessageBox.Show overload; HeadlessMergeNotifier ignores it. The MessageBoxManager relabeling of the Cancel button to "Ne&ver" is removed rather than preserved: it depends on a SetWindowsHookEx hook registered on the calling thread, but Program.Notifier.ShowMessage (MainForm.ShowMessage) marshals the actual MessageBox.Show call onto the UI thread via Invoke when called off-thread - as this call always is, via MainForm's Task.Run - so the hook would never see the dialog's window messages once routed through the notifier. Kept as dead code it would look functional without being so. The Cancel button now reads "Cancel" instead of "Never"; the DialogResult value and its handling in ValidateAndFix are unchanged. Also guarded ValidateAndFix's Cancel branch on Program.Notifier.IsInteractive: HeadlessMergeNotifier's fixed non-destructive default for YesNoCancel is Cancel, which previously mapped to "Never" here and would have silently persisted ValidateCustomLoadOrder=false to App.config on any future headless run that reaches this code path - exactly the landmine this change exists to defuse. Verified: Program.Notifier is reassigned to MainForm in Program.cs before Application.Run, and the only path that reaches PromptToPrioritizeMergedMod (MainForm_Shown -> RefreshMergeInventory -> LoadOrderValidator.ValidateAndFix) runs after Shown, never from MainForm's constructor - so this change doesn't introduce a window where the prompt silently goes to a HeadlessMergeNotifier instead of the GUI. No GUI automation harness is available in this environment; verified by dotnet build (no new warnings), dotnet format whitespace --verify-no-changes, and code inspection confirming button set, icon, message text, and DialogResult handling are unchanged from the original MessageBox.Show call. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01GXAuGMLB44T5Zv5o5ZzKah --- WitcherScriptMerger/Forms/MainForm.cs | 7 +++--- WitcherScriptMerger/HeadlessMergeNotifier.cs | 3 ++- WitcherScriptMerger/IMergeNotifier.cs | 3 ++- .../LoadOrder/LoadOrderValidator.cs | 23 ++++++++++--------- 4 files changed, 20 insertions(+), 16 deletions(-) diff --git a/WitcherScriptMerger/Forms/MainForm.cs b/WitcherScriptMerger/Forms/MainForm.cs index 8897f40..06a9cb9 100644 --- a/WitcherScriptMerger/Forms/MainForm.cs +++ b/WitcherScriptMerger/Forms/MainForm.cs @@ -882,18 +882,19 @@ void HideProgressScreen() public DialogResult ShowMessage(string text, string title = "", MessageBoxButtons buttons = MessageBoxButtons.OK, - MessageBoxIcon icon = MessageBoxIcon.None) + MessageBoxIcon icon = MessageBoxIcon.None, + MessageBoxDefaultButton defaultButton = MessageBoxDefaultButton.Button1) { this.ActivateSafely(); if (this.InvokeRequired) { return (DialogResult)this.Invoke(new Func( - () => { return MessageBox.Show(this, text, title, buttons, icon); })); + () => { return MessageBox.Show(this, text, title, buttons, icon, defaultButton); })); } else { - return MessageBox.Show(this, text, title, buttons, icon); + return MessageBox.Show(this, text, title, buttons, icon, defaultButton); } } diff --git a/WitcherScriptMerger/HeadlessMergeNotifier.cs b/WitcherScriptMerger/HeadlessMergeNotifier.cs index 8212f24..3d941a6 100644 --- a/WitcherScriptMerger/HeadlessMergeNotifier.cs +++ b/WitcherScriptMerger/HeadlessMergeNotifier.cs @@ -14,7 +14,8 @@ class HeadlessMergeNotifier : IMergeNotifier public DialogResult ShowMessage(string text, string title = "", MessageBoxButtons buttons = MessageBoxButtons.OK, - MessageBoxIcon icon = MessageBoxIcon.None) + MessageBoxIcon icon = MessageBoxIcon.None, + MessageBoxDefaultButton defaultButton = MessageBoxDefaultButton.Button1) { Write(text, title, icon); diff --git a/WitcherScriptMerger/IMergeNotifier.cs b/WitcherScriptMerger/IMergeNotifier.cs index c66d0dd..0eb3034 100644 --- a/WitcherScriptMerger/IMergeNotifier.cs +++ b/WitcherScriptMerger/IMergeNotifier.cs @@ -9,7 +9,8 @@ interface IMergeNotifier DialogResult ShowMessage(string text, string title = "", MessageBoxButtons buttons = MessageBoxButtons.OK, - MessageBoxIcon icon = MessageBoxIcon.None); + MessageBoxIcon icon = MessageBoxIcon.None, + MessageBoxDefaultButton defaultButton = MessageBoxDefaultButton.Button1); DialogResult ShowError(string text, string title = "Error"); diff --git a/WitcherScriptMerger/LoadOrder/LoadOrderValidator.cs b/WitcherScriptMerger/LoadOrder/LoadOrderValidator.cs index 29e88b5..124dc9e 100644 --- a/WitcherScriptMerger/LoadOrder/LoadOrderValidator.cs +++ b/WitcherScriptMerger/LoadOrder/LoadOrderValidator.cs @@ -21,8 +21,11 @@ public static void ValidateAndFix(CustomLoadOrder loadOrder) { PrioritizeMergedMod(loadOrder, mergedMod); } - else if (choice == DialogResult.Cancel) // Never + else if (choice == DialogResult.Cancel && Program.Notifier.IsInteractive) // Never { + // IsInteractive guard: HeadlessMergeNotifier's fixed default for + // YesNoCancel is Cancel, which would otherwise persist this setting + // on every headless run that reaches here. Program.Settings.Set("ValidateCustomLoadOrder", false); Program.Settings.Save(); } @@ -30,20 +33,18 @@ public static void ValidateAndFix(CustomLoadOrder loadOrder) static DialogResult PromptToPrioritizeMergedMod(string modsSettingsPath) { - MessageBoxManager.Cancel = "Ne&ver"; - MessageBoxManager.Register(); - - var choice = MessageBox.Show( + // Cancel button reads "Cancel", not "Never" - the old MessageBoxManager + // relabel hook was thread-affine and can't survive ShowMessage's Invoke. + // DialogResult semantics (and PromptToPrioritizeMergedMod's return value) + // are unchanged. + return Program.Notifier.ShowMessage( $"{modsSettingsPath}\n\n" + "Detected custom load order in the file above, and merged files aren't configured to load first.\n\n" + "Would you like Script Merger to modify your custom load order so that your merged files have top priority?", "Custom Load Order Problem", - MessageBoxButtons.YesNoCancel, - MessageBoxIcon.Exclamation, - MessageBoxDefaultButton.Button2); - - MessageBoxManager.Unregister(); - return choice; + System.Windows.Forms.MessageBoxButtons.YesNoCancel, + System.Windows.Forms.MessageBoxIcon.Exclamation, + System.Windows.Forms.MessageBoxDefaultButton.Button2); } static void PrioritizeMergedMod(CustomLoadOrder loadOrder, ModLoadSetting mergedModSetting) From bff23d30c54a50963a9f9791f83a4f6c33a06b86 Mon Sep 17 00:00:00 2001 From: Chris Knight Date: Fri, 7 Aug 2026 13:07:44 -0400 Subject: [PATCH 2/2] Address code-review findings on the notifier-routing change - LoadOrderValidator.cs: simplify back to unqualified MessageBoxButtons/ MessageBoxIcon/MessageBoxDefaultButton now that the fully-qualified form (matched literally to the task's example) reads as inconsistent next to the same file's own unqualified DialogResult usage and the rest of the codebase's convention wherever `using System.Windows.Forms;` is already present. - LoadOrderValidator.cs: expand the comment on the dropped "Ne&ver" relabel. Review correctly pointed out the old MessageBoxManager hook genuinely worked before this change (MessageBox.Show ran directly on the same background thread Register() hooked, no Invoke involved) - this is a real, disclosed regression in how the Cancel button reads, not a no-op cleanup, and the comment now says so plainly along with why it can't be preserved through Program.Notifier without extending IMergeNotifier with custom button-text support (out of scope here). - HeadlessMergeNotifier.cs: comment on why defaultButton is accepted but not consulted when choosing the headless DialogResult, so it doesn't read as an oversight to a future caller relying on it. Not addressed here, flagged for other units instead: HeadlessMergeNotifier .Write already routes MessageBoxIcon.None messages to stdout, which is a pre-existing MCP stdout-hygiene risk unrelated to this change (belongs to the MCP-hardening unit); MainForm.cs's PromptToDeleteForChangedHash has an analogous still-"Ne&ver"-labeled prompt that now reads inconsistently with this one, but that method is GUI-layer code outside LoadOrderValidator.cs's scope. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01GXAuGMLB44T5Zv5o5ZzKah --- WitcherScriptMerger/HeadlessMergeNotifier.cs | 8 +++++++ .../LoadOrder/LoadOrderValidator.cs | 23 +++++++++++++------ 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/WitcherScriptMerger/HeadlessMergeNotifier.cs b/WitcherScriptMerger/HeadlessMergeNotifier.cs index 3d941a6..a76ce0b 100644 --- a/WitcherScriptMerger/HeadlessMergeNotifier.cs +++ b/WitcherScriptMerger/HeadlessMergeNotifier.cs @@ -19,6 +19,14 @@ public DialogResult ShowMessage(string text, { Write(text, title, icon); + // defaultButton is intentionally unused here: it only affects which + // button has interactive UI focus (Enter-key behavior), and headless + // mode has no UI to focus. The DialogResult below is chosen per + // buttons set instead, per this class's own fixed, non-destructive + // default for each verb - not by the caller's requested default + // button. A call site relying on defaultButton to signal "this is the + // safe answer" for a MessageBoxButtons combination not covered below + // will get whatever the catch-all case returns instead. return buttons switch { MessageBoxButtons.OK => DialogResult.OK, diff --git a/WitcherScriptMerger/LoadOrder/LoadOrderValidator.cs b/WitcherScriptMerger/LoadOrder/LoadOrderValidator.cs index 124dc9e..d99f184 100644 --- a/WitcherScriptMerger/LoadOrder/LoadOrderValidator.cs +++ b/WitcherScriptMerger/LoadOrder/LoadOrderValidator.cs @@ -33,18 +33,27 @@ public static void ValidateAndFix(CustomLoadOrder loadOrder) static DialogResult PromptToPrioritizeMergedMod(string modsSettingsPath) { - // Cancel button reads "Cancel", not "Never" - the old MessageBoxManager - // relabel hook was thread-affine and can't survive ShowMessage's Invoke. - // DialogResult semantics (and PromptToPrioritizeMergedMod's return value) - // are unchanged. + // Known, accepted regression: the Cancel button used to be relabeled + // "Ne&ver" via MessageBoxManager.Register()/Unregister(), which worked + // only because the old MessageBox.Show call ran on the same background + // thread (Register()'s SetWindowsHookEx is thread-affine) as this + // method. Program.Notifier.ShowMessage (MainForm.ShowMessage) Invokes + // the actual MessageBox.Show onto the UI thread, so that hook can no + // longer see the dialog's window messages - relabeling can't be + // preserved without adding custom button-text support to + // IMergeNotifier, which is out of scope here. The Cancel button now + // reads "Cancel"; clicking it still permanently disables this check + // (see the IsInteractive-guarded branch above), just without a label + // saying so. DialogResult semantics and this method's return value are + // otherwise unchanged. return Program.Notifier.ShowMessage( $"{modsSettingsPath}\n\n" + "Detected custom load order in the file above, and merged files aren't configured to load first.\n\n" + "Would you like Script Merger to modify your custom load order so that your merged files have top priority?", "Custom Load Order Problem", - System.Windows.Forms.MessageBoxButtons.YesNoCancel, - System.Windows.Forms.MessageBoxIcon.Exclamation, - System.Windows.Forms.MessageBoxDefaultButton.Button2); + MessageBoxButtons.YesNoCancel, + MessageBoxIcon.Exclamation, + MessageBoxDefaultButton.Button2); } static void PrioritizeMergedMod(CustomLoadOrder loadOrder, ModLoadSetting mergedModSetting)