SCMU does not set ServiceControlQueueAddress when only adding audit instances bug fix

docs/specs/audit-instance-servicecontrol-queue-address.md

@@ -0,0 +1,70 @@
+# Feature: Audit Instance ServiceControl Queue Address
+
+**As an operator adding instances through ServiceControl Management Utility (SCMU), I want every audit instance to be connected to a ServiceControl (error) instance so that messages the audit instance sends to the error instance always have a valid destination.**
+
+> Bug: [#4753 — SCMU does not set ServiceControlQueueAddress when only adding audit instances](https://github.com/Particular/ServiceControl/issues/4753).
+> When the `ServiceControl.Audit/ServiceControlQueueAddress` setting is missing from
+> `ServiceControl.Audit.exe.config`, the audit instance fails at runtime with
+> ["no destination specified for message"](https://docs.particular.net/servicecontrol/troubleshooting#no-destination-specified-for-message).
+
+## Rules and Examples
+
+### Rule 1: Must address the audit instance to the error instance installed in the same session
+
+When the user installs an error instance and an audit instance together, the audit
+instance's queue address is the name of the error instance being installed — never an
+already-installed one.
+
+- **Example:** The one where both instances are installed together and the audit
+  instance's queue address is the new error instance's name.
+- **Counter-example:** The one where other error instances already exist on the
+  machine, yet no choice is offered — the error instance being installed always wins.
+
+---
+
+### Rule 2: Should auto-detect the existing error instance when adding an audit instance alone
+
+- **Example:** The one where exactly one error instance exists on the machine and its
+  name is used as the queue address without any user input (no dropdown shown).
+
+---
+
+### Rule 3: Must require an explicit choice when multiple existing error instances are found
+
+Auto-detection cannot guess between several error instances — picking one silently
+risks routing messages to the wrong instance. The choice dropdown is shown **only** in
+this case.
+
+- **Example:** The one where two error instances exist and the dropdown offers both.
+- **Example:** The one where Save is blocked until the user picks one of the detected
+  instances, and unblocked once a choice is made.
+
+---
+
+### Rule 4: Must block installation when no error instance exists to connect to
+
+An audit instance without a reachable error instance is misconfigured by definition;
+SCMU must not produce it.
+
+- **Example:** The one where no error instance exists, the user adds an audit instance
+  alone, and a validation error prevents the installation from proceeding.
+- **Counter-example:** The one where only an error instance is being installed — the
+  queue address does not apply and no validation error is raised.
+
+## Resolved decisions (for implementation)
+
+- **Auto-detect source:** installed Windows error instances, discovered via
+  `InstanceFinder.ServiceControlInstances()`; exposed on the view model through a
+  `GetInstalledErrorInstanceNames` function seam (mirrors the existing
+  `GetWindowsServiceNames` pattern) so tests can substitute it.
+- **Multiple instances found:** user must choose from a dropdown that is visible only
+  when adding an audit instance alone **and** more than one error instance is detected.
+- **No instance found:** Save is blocked by a validation error; deploying with
+  PowerShell remains the path for advanced scenarios.
+- **Acceptance tier:** view model + validator observed through `INotifyDataErrorInfo`
+  — the same mechanism the UI uses to block Save. A full SCMU end-to-end test (install
+  a Windows service, inspect the written config file) is not automatable in this
+  repository's test suites.
+- **Out of scope:** registering the new audit instance as a remote of the existing
+  error instance (`AddRemoteInstance` is only called when both instances are installed
+  together) — candidate for a follow-up issue.

src/ServiceControl.Config.Tests/.editorconfig

@@ -2,3 +2,7 @@
 
 # Justification: Test project
 dotnet_diagnostic.CA2007.severity = none
+
+# Justification: Executable specifications intentionally assign properties after the
+# object initializer to mirror user interaction order (e.g. typing a name after load)
+dotnet_diagnostic.IDE0017.severity = none

src/ServiceControl.Config.Tests/AddInstance/AuditInstanceServiceControlQueueAddress.cs

@@ -0,0 +1,196 @@
+namespace ServiceControl.Config.Tests.AddInstance
+{
+    using System.ComponentModel;
+    using NUnit.Framework;
+    using ServiceControl.Config.UI.InstanceAdd;
+
+    /// <summary>
+    /// Executable specification for docs/specs/audit-instance-servicecontrol-queue-address.md
+    /// (bug https://github.com/Particular/ServiceControl/issues/4753).
+    ///
+    /// Organized as feature > rule > examples:
+    /// - this outer class is the feature,
+    /// - each nested fixture is one rule from the spec,
+    /// - each test is one example, named with the spec's "The one where ..." language.
+    ///
+    /// These tests are the OUTER loop of a double-loop TDD process. They observe the view
+    /// model and its validator through INotifyDataErrorInfo - the same mechanism the UI
+    /// uses to block Save - and reference members that do not exist yet:
+    ///   GetInstalledErrorInstanceNames, ServiceControlQueueAddress,
+    ///   ServiceControlQueueAddressOptions, ShowServiceControlQueueAddressSelection.
+    /// </summary>
+    public class AuditInstanceServiceControlQueueAddress
+    {
+        [TestFixture]
+        public class Rule_1_Must_address_the_audit_instance_to_the_error_instance_installed_in_the_same_session
+        {
+            [Test]
+            public void The_one_where_both_instances_are_installed_together_and_the_new_error_instance_name_is_used()
+            {
+                var viewModel = new ServiceControlAddViewModel
+                {
+                    InstallErrorInstance = true,
+                    InstallAuditInstance = true,
+                    SubmitAttempted = true,
+                    // No pre-existing error instances on the machine
+                    GetInstalledErrorInstanceNames = () => new string[0]
+                };
+
+                viewModel.ErrorInstanceName = "My.Error.Instance";
+
+                viewModel.NotifyOfPropertyChange(nameof(viewModel.ServiceControlQueueAddress));
+
+                var notifyErrorInfo = GetNotifyErrorInfo(viewModel);
+
+                using (Assert.EnterMultipleScope())
+                {
+                    Assert.That(viewModel.ServiceControlQueueAddress, Is.EqualTo("My.Error.Instance"));
+                    Assert.That(viewModel.ShowServiceControlQueueAddressSelection, Is.False);
+                    Assert.That(notifyErrorInfo.GetErrors(nameof(viewModel.ServiceControlQueueAddress)), Is.Empty);
+                }
+            }
+
+            [Test]
+            public void The_one_where_other_error_instances_already_exist_yet_no_choice_is_offered_because_the_instance_being_installed_wins()
+            {
+                var viewModel = new ServiceControlAddViewModel
+                {
+                    InstallErrorInstance = true,
+                    InstallAuditInstance = true,
+                    GetInstalledErrorInstanceNames = () => new string[] { "Particular.ServiceControl", "Particular.ServiceControl.2" }
+                };
+
+                Assert.That(viewModel.ShowServiceControlQueueAddressSelection, Is.False);
+            }
+        }
+
+        [TestFixture]
+        public class Rule_2_Should_auto_detect_the_existing_error_instance_when_adding_an_audit_instance_alone
+        {
+            [Test]
+            public void The_one_where_a_single_error_instance_exists_and_its_name_is_used_without_any_user_input()
+            {
+                var viewModel = new ServiceControlAddViewModel
+                {
+                    InstallErrorInstance = false,
+                    InstallAuditInstance = true,
+                    SubmitAttempted = true,
+                    GetInstalledErrorInstanceNames = () => new string[] { "Particular.ServiceControl" }
+                };
+
+                viewModel.NotifyOfPropertyChange(nameof(viewModel.ServiceControlQueueAddress));
+
+                var notifyErrorInfo = GetNotifyErrorInfo(viewModel);
+
+                using (Assert.EnterMultipleScope())
+                {
+                    Assert.That(viewModel.ServiceControlQueueAddress, Is.EqualTo("Particular.ServiceControl"));
+                    Assert.That(viewModel.ShowServiceControlQueueAddressSelection, Is.False, "Dropdown must not show when there is only one existing error instance");
+                    Assert.That(notifyErrorInfo.GetErrors(nameof(viewModel.ServiceControlQueueAddress)), Is.Empty);
+                }
+            }
+        }
+
+        [TestFixture]
+        public class Rule_3_Must_require_an_explicit_choice_when_multiple_existing_error_instances_are_found
+        {
+            [Test]
+            public void The_one_where_two_error_instances_exist_and_the_dropdown_offers_both()
+            {
+                var viewModel = new ServiceControlAddViewModel
+                {
+                    InstallErrorInstance = false,
+                    InstallAuditInstance = true,
+                    GetInstalledErrorInstanceNames = () => new string[] { "Particular.ServiceControl", "Particular.ServiceControl.2" }
+                };
+
+                using (Assert.EnterMultipleScope())
+                {
+                    Assert.That(viewModel.ShowServiceControlQueueAddressSelection, Is.True);
+                    Assert.That(viewModel.ServiceControlQueueAddressOptions, Is.EquivalentTo(new[]
+                    {
+                        "Particular.ServiceControl",
+                        "Particular.ServiceControl.2"
+                    }));
+                }
+            }
+
+            [Test]
+            public void The_one_where_save_is_blocked_until_the_user_picks_one_of_the_detected_instances()
+            {
+                var viewModel = new ServiceControlAddViewModel
+                {
+                    InstallErrorInstance = false,
+                    InstallAuditInstance = true,
+                    SubmitAttempted = true,
+                    GetInstalledErrorInstanceNames = () => new string[] { "Particular.ServiceControl", "Particular.ServiceControl.2" }
+                };
+
+                viewModel.NotifyOfPropertyChange(nameof(viewModel.ServiceControlQueueAddress));
+
+                var notifyErrorInfo = GetNotifyErrorInfo(viewModel);
+
+                // No selection made yet: must be blocked
+                Assert.That(notifyErrorInfo.GetErrors(nameof(viewModel.ServiceControlQueueAddress)), Is.Not.Empty,
+                    "A validation error is expected until the user picks one of the existing error instances");
+
+                // User picks an instance from the dropdown
+                viewModel.ServiceControlQueueAddress = "Particular.ServiceControl.2";
+
+                using (Assert.EnterMultipleScope())
+                {
+                    Assert.That(viewModel.ServiceControlQueueAddress, Is.EqualTo("Particular.ServiceControl.2"));
+                    Assert.That(notifyErrorInfo.GetErrors(nameof(viewModel.ServiceControlQueueAddress)), Is.Empty);
+                }
+            }
+        }
+
+        [TestFixture]
+        public class Rule_4_Must_block_installation_when_no_error_instance_exists_to_connect_to
+        {
+            [Test]
+            public void The_one_where_no_error_instance_exists_and_a_validation_error_prevents_the_installation_from_proceeding()
+            {
+                var viewModel = new ServiceControlAddViewModel
+                {
+                    InstallErrorInstance = false,
+                    InstallAuditInstance = true,
+                    SubmitAttempted = true,
+                    GetInstalledErrorInstanceNames = () => new string[0]
+                };
+
+                viewModel.NotifyOfPropertyChange(nameof(viewModel.ServiceControlQueueAddress));
+
+                var notifyErrorInfo = GetNotifyErrorInfo(viewModel);
+
+                using (Assert.EnterMultipleScope())
+                {
+                    Assert.That(viewModel.ServiceControlQueueAddress, Is.Null.Or.Empty);
+                    Assert.That(viewModel.ShowServiceControlQueueAddressSelection, Is.False);
+                    Assert.That(notifyErrorInfo.GetErrors(nameof(viewModel.ServiceControlQueueAddress)), Is.Not.Empty,
+                        "A validation error is expected so the user cannot proceed without an existing error instance to connect to");
+                }
+            }
+
+            [Test]
+            public void The_one_where_only_an_error_instance_is_installed_and_the_queue_address_does_not_apply()
+            {
+                var viewModel = new ServiceControlAddViewModel
+                {
+                    InstallErrorInstance = true,
+                    InstallAuditInstance = false,
+                    SubmitAttempted = true,
+                    GetInstalledErrorInstanceNames = () => new string[0]
+                };
+
+                viewModel.NotifyOfPropertyChange(nameof(viewModel.ServiceControlQueueAddress));
+
+                var notifyErrorInfo = GetNotifyErrorInfo(viewModel);
+
+                Assert.That(notifyErrorInfo.GetErrors(nameof(viewModel.ServiceControlQueueAddress)), Is.Empty);
+            }
+        }
+
+        static INotifyDataErrorInfo GetNotifyErrorInfo(object vm) => vm as INotifyDataErrorInfo;
+    }
+}

src/ServiceControl.Config.Tests/ServiceControlAddScreenLoadedTests.cs

@@ -153,7 +153,7 @@ public void Database_maintenance_port_number_are_set_to_defaults_with_no_validat
         [Test]
         public void Destination_path_is_null()
         {
-            var viewModel = new ServiceControlAddViewModel();
+            var viewModel = new ServiceControlAddViewModel(() => []);
 
             var errorInfo = (INotifyDataErrorInfo)viewModel;
 
@@ -169,7 +169,7 @@ public void Destination_path_is_null()
         [Test]
         public void Log_path_is_null()
         {
-            var viewModel = new ServiceControlAddViewModel();
+            var viewModel = new ServiceControlAddViewModel(() => []);
 
             var errorInfo = (INotifyDataErrorInfo)viewModel;
 
@@ -186,7 +186,7 @@ public void Log_path_is_null()
         [Test]
         public void Database_path_is_null()
         {
-            var viewModel = new ServiceControlAddViewModel();
+            var viewModel = new ServiceControlAddViewModel(() => []);
 
             var errorInfo = (INotifyDataErrorInfo)viewModel;
 

src/ServiceControl.Config/UI/InstanceAdd/ServiceControlAddAttachment.cs

@@ -100,7 +100,7 @@ async Task Add()
                 auditNewInstance.AuditRetentionPeriod = viewModel.ServiceControlAudit.AuditRetentionPeriod;
                 auditNewInstance.ServiceAccount = viewModel.ServiceControlAudit.ServiceAccount;
                 auditNewInstance.ServiceAccountPwd = viewModel.ServiceControlAudit.Password;
-                auditNewInstance.ServiceControlQueueAddress = serviceControlNewInstance == null ? string.Empty : serviceControlNewInstance.InstanceName;
+                auditNewInstance.ServiceControlQueueAddress = viewModel.ServiceControlQueueAddress;
                 auditNewInstance.EnableFullTextSearchOnBodies = viewModel.ServiceControlAudit.EnableFullTextSearchOnBodies.Value;
             }
 

src/ServiceControl.Config/UI/InstanceAdd/ServiceControlAddView.xaml

@@ -299,6 +299,21 @@
 
                                 <controls:FormTextBox Header="NAME / WINDOWS SERVICE NAME" Text="{Binding AuditInstanceName}" />
 
+                                <controls:FormComboBox HorizontalAlignment="Stretch"
+                                                       Header="FORWARD FAILED AUDIT IMPORTS TO SERVICECONTROL INSTANCE"
+                                                       ItemsSource="{Binding ServiceControlQueueAddressOptions}"
+                                                       SelectedItem="{Binding ServiceControlQueueAddress}"
+                                                       Visibility="{Binding ShowServiceControlQueueAddressSelection,
+                                                                            Converter={StaticResource boolToVis}}" />
+
+                                <TextBlock Padding="0,0,0,10"
+                                           FontSize="13px"
+                                           Foreground="{StaticResource ErrorBrush}"
+                                           Text="No ServiceControl (error) instance was found on this machine. The audit instance needs one to send messages to. Also install the ServiceControl instance above, or add one first."
+                                           TextWrapping="Wrap"
+                                           Visibility="{Binding ShowNoErrorInstanceFoundWarning,
+                                                                Converter={StaticResource boolToVis}}" />
+
                                 <GroupBox Header="USER ACCOUNT">
                                     <StackPanel>
                                         <RadioButton IsChecked="{Binding AuditUseSystemAccount}">