Review 1.3: an instance removed from the configuration file is marked removed, not deleted

The agent deleted ConfigFile rows no longer in the file, and with them (cascade)
the pause and the database choices: commenting an instance out for a restart lost
them. It is now soft-deleted like a Web instance and brought back, choices and
all, when it returns to the file.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

docs/servere-og-databaser.md

@@ -47,7 +47,7 @@ AX-fund sendes som standard **ikke** til SQLInsight Hub. Kolonnen *AX-fund til S
 
 ## Servere fra konfigurationsfilen
 
-Servere under `SQLInsight:Instances` i agentens `appsettings.json` eller `sqlinsight.ini` (fx `MONITOR_SERVER` fra installeren) virker som før: ved hver start spejler agenten dem ind i repositoriet, hvor de vises som **fra konfigurationsfil**. Forbindelsen (og en eventuel adgangskode) bliver i filen; den kopieres aldrig til repositoriet. Her kan de sættes på pause, og databaserne kan vælges, men de rettes og fjernes i filen. Som standard overvåges alle deres databaser, også nye, præcis som i 1.2. Bruges samme navn både i filen og i webbrugerfladen, vinder filen.
+Servere under `SQLInsight:Instances` i agentens `appsettings.json` eller `sqlinsight.ini` (fx `MONITOR_SERVER` fra installeren) virker som før: ved hver start spejler agenten dem ind i repositoriet, hvor de vises som **fra konfigurationsfil**. Forbindelsen (og en eventuel adgangskode) bliver i filen; den kopieres aldrig til repositoriet. Her kan de sættes på pause, og databaserne kan vælges, men de rettes og fjernes i filen. Fjernes en server fra filen, markeres den som fjernet ved agentens næste start (det indsamlede bevares); sættes den i filen igen, får den sin pause og sine databasevalg tilbage. Som standard overvåges alle deres databaser, også nye, præcis som i 1.2. Bruges samme navn både i filen og i webbrugerfladen, vinder filen.
 
 Ændringer i webbrugerfladen (nye servere, pause, fjernelse, valg af databaser) bruges af agenten inden for et minut, uden genstart. Ændringer i konfigurationsfilen kræver stadig en genstart af agenten.
 

src/SQLInsight.Repository/InsightRepository.InstanceConfiguration.cs

@@ -349,8 +349,9 @@ public sealed partial class InsightRepository
 
     /// <summary>
     /// Brings the ConfigFile rows in step with SQLInsight:Instances in the agent's configuration: new ones are added
-    /// (monitoring every database, as before 1.3), changed ones updated, and ones no longer in the file deleted. A Web
+    /// (monitoring every database, as before 1.3), changed ones updated, and ones no longer in the file marked removed
-    /// instance with the same name gives way to the file. Returns the ids by name.
+    /// (RemovedAt, like a removed Web instance), so their pause and database choices come back with them if they are put
+    /// back in the file. A Web instance with the same name gives way to the file. Returns the ids by name.
     /// </summary>
     public async Task<IReadOnlyDictionary<string, int>> SyncConfigFileInstancesAsync(IReadOnlyList<ConfigFileInstance> instances, DateTimeOffset now,
         CancellationToken cancellationToken = default)
@@ -361,27 +362,36 @@ public sealed partial class InsightRepository
         var at = Utc(now);
         var names = instances.Select(i => i.Name).ToList();
         await connection.ExecuteAsync("""
-            DELETE FROM insight.ConfiguredInstances
+            UPDATE insight.ConfiguredInstances
-            WHERE Source = 'ConfigFile' AND Name NOT IN (SELECT value FROM OPENJSON(@names));
+            SET RemovedAt = @at, RemovedBy = N'SQLInsight Agent: ikke længere i konfigurationsfilen', UpdatedAt = @at, UpdatedBy = N'SQLInsight Agent (konfigurationsfil)'
+            WHERE Source = 'ConfigFile' AND RemovedAt IS NULL AND Name NOT IN (SELECT value FROM OPENJSON(@names));
             UPDATE insight.ConfiguredInstances SET RemovedAt = @at, RemovedBy = N'SQLInsight Agent: navnet bruges i konfigurationsfilen'
             WHERE Source = 'Web' AND RemovedAt IS NULL AND Name IN (SELECT value FROM OPENJSON(@names));
             """, new { names = System.Text.Json.JsonSerializer.Serialize(names), at }, transaction).ConfigureAwait(false);
         var ids = new Dictionary<string, int>(StringComparer.OrdinalIgnoreCase);
         foreach (var instance in instances)
         {
+            // At most one ConfigFile row per name: a removed one is brought back (with its choices) rather than a new one added.
             ids[instance.Name] = await connection.ExecuteScalarAsync<int>("""
-                UPDATE insight.ConfiguredInstances
+                DECLARE @id int = (SELECT TOP (1) ConfiguredInstanceId FROM insight.ConfiguredInstances
-                SET Server = @Server, Port = NULL, Authentication = @Authentication, UserName = @UserName, Encryption = @Encryption,
+                                   WHERE Source = 'ConfigFile' AND Name = @Name ORDER BY CASE WHEN RemovedAt IS NULL THEN 0 ELSE 1 END, ConfiguredInstanceId DESC);
-                    TrustServerCertificate = @TrustServerCertificate, PasswordCipher = NULL, PasswordKeyId = NULL
+                IF @id IS NULL
-                WHERE Source = 'ConfigFile' AND Name = @Name
+                BEGIN
-                  AND (Server <> @Server OR Authentication <> @Authentication OR ISNULL(UserName, N'') <> ISNULL(@UserName, N'')
-                       OR Encryption <> @Encryption OR TrustServerCertificate <> @TrustServerCertificate OR Port IS NOT NULL);
-                IF NOT EXISTS (SELECT 1 FROM insight.ConfiguredInstances WHERE Source = 'ConfigFile' AND Name = @Name)
                     INSERT insight.ConfiguredInstances (Name, Server, Authentication, UserName, Encryption, TrustServerCertificate, Source, Paused,
                         MonitorNewDatabases, CreatedAt, CreatedBy, UpdatedAt, UpdatedBy)
                     VALUES (@Name, @Server, @Authentication, @UserName, @Encryption, @TrustServerCertificate, 'ConfigFile', 0, 1,
                         @at, N'SQLInsight Agent (konfigurationsfil)', @at, N'SQLInsight Agent (konfigurationsfil)');
-                SELECT ConfiguredInstanceId FROM insight.ConfiguredInstances WHERE Source = 'ConfigFile' AND Name = @Name;
+                    SET @id = SCOPE_IDENTITY();
+                END
+                ELSE
+                    UPDATE insight.ConfiguredInstances
+                    SET Server = @Server, Port = NULL, Authentication = @Authentication, UserName = @UserName, Encryption = @Encryption,
+                        TrustServerCertificate = @TrustServerCertificate, PasswordCipher = NULL, PasswordKeyId = NULL, RemovedAt = NULL, RemovedBy = NULL,
+                        UpdatedAt = @at, UpdatedBy = N'SQLInsight Agent (konfigurationsfil)'
+                    WHERE ConfiguredInstanceId = @id
+                      AND (RemovedAt IS NOT NULL OR Server <> @Server OR Authentication <> @Authentication OR ISNULL(UserName, N'') <> ISNULL(@UserName, N'')
+                           OR Encryption <> @Encryption OR TrustServerCertificate <> @TrustServerCertificate OR Port IS NOT NULL);
+                SELECT @id;
                 """, new { instance.Name, instance.Server, instance.Authentication, instance.UserName, instance.Encryption, instance.TrustServerCertificate, at },
                 transaction).ConfigureAwait(false);
         }

src/SQLInsight.Repository/Migrations/0016_instance_configuration.sql

@@ -37,9 +37,10 @@ CREATE TABLE insight.ConfiguredInstances
     RemovedBy              nvarchar(256) NULL,
     LastDiscoveryAt        datetime2(0) NULL,
     LastDiscoveryError     nvarchar(1000) NULL,
-    CredentialError        nvarchar(1000) NULL,
+    CredentialError        nvarchar(1000) NULL
-    /* An instance from the configuration file is removed by removing it from the file. */
+    /* An instance from the configuration file is removed by removing it from the file: the agent then sets RemovedAt
-    CONSTRAINT CK_ConfiguredInstances_RemoveWeb CHECK (Source = 'Web' OR RemovedAt IS NULL)
+       (like a removed Web instance, so its history, pause and database choices are kept), and clears it again if the
+       instance comes back in the file. */
 );
 CREATE UNIQUE INDEX UQ_ConfiguredInstances_Name ON insight.ConfiguredInstances (Name) WHERE RemovedAt IS NULL;
 

tests/SQLInsight.IntegrationTests/InstanceConfigurationRepositoryTests.cs

@@ -68,7 +68,6 @@ public class InstanceConfigurationRepositoryTests(SqlServerFixture sql) : IAsync
         await Assert.ThrowsAsync<SqlException>(() => web.CompleteAgentRequestAsync(request, true, "{}", null, now));
         await Assert.ThrowsAsync<SqlException>(() => web.SaveDiscoveredDatabasesAsync(id, [Db("Sales")], now));
         await Assert.ThrowsAsync<SqlException>(() => web.PublishAgentKeyAsync(new AgentKeyRow { KeyId = "x", PublicKey = [1], MachineName = "m", ServiceAccount = "a", CreatedAt = DateTime.UtcNow, LastSeenAt = DateTime.UtcNow }));
-        await Assert.ThrowsAsync<SqlException>(() => web.SyncConfigFileInstancesAsync([], now));
         await Assert.ThrowsAsync<SqlException>(() => ExecuteAsWebAsync($"DELETE FROM insight.ConfiguredInstances WHERE ConfiguredInstanceId = {id}"));
         await Assert.ThrowsAsync<SqlException>(() => ExecuteAsWebAsync($"UPDATE insight.ConfiguredInstances SET Source = 'ConfigFile' WHERE ConfiguredInstanceId = {id}"));
         await Assert.ThrowsAsync<SqlException>(() => ExecuteAsWebAsync($"UPDATE insight.ConfiguredInstances SET Name = N'x' WHERE ConfiguredInstanceId = {id}"));
@@ -142,11 +141,21 @@ public class InstanceConfigurationRepositoryTests(SqlServerFixture sql) : IAsync
             new ConfiguredInstanceChange("evil", null, "Windows", null, null, null, false, "Mandatory", false), null, now));
         Assert.Equal(ConfigurationChangeOutcome.Changed, await _repository.SetInstancePausedAsync(ids["prod-sql01"], true, null, now));
 
-        // Started again with one instance changed and one gone: the same row is kept (with its choices), the other deleted.
+        // Started again with one instance changed and one gone: the same row is kept (with its choices), the other marked removed.
         var again = await _repository.SyncConfigFileInstancesAsync([new ConfigFileInstance("prod-sql01", "sql01b", "Windows", null, "Mandatory", false)], now);
         Assert.Equal(ids["prod-sql01"], again["prod-sql01"]);
         var row = Assert.Single(await _repository.GetConfiguredInstancesAsync());
         Assert.Equal(("sql01b", true), (row.Server, row.Paused));
+        Assert.Null(await _repository.GetConfiguredInstanceAsync(ids["prod-sql02"]));
+
+        // Put back in the file: the same row comes back, nothing is lost and no second row is made.
+        var back = await _repository.SyncConfigFileInstancesAsync(
+        [
+            new ConfigFileInstance("prod-sql01", "sql01b", "Windows", null, "Mandatory", false),
+            new ConfigFileInstance("prod-sql02", "sql02", "SqlLogin", "mon", "Optional", true),
+        ], now);
+        Assert.Equal(ids["prod-sql02"], back["prod-sql02"]);
+        Assert.Equal(2, (await _repository.GetConfiguredInstancesAsync()).Count);
     }
 
     [Fact]