fix(1b-iii): address PR review (resilient consumers, set-based delete, DI forwarding, migration orphan guard)

Author: HandyS11

src/RustPlusBot.Features.Connections/Hosting/ConnectionHostedService.cs

@@ -76,22 +76,48 @@ await Task.WhenAll(
 
     private async Task ConsumeRegisteredAsync(CancellationToken cancellationToken)
     {
-        await foreach (var registered in eventBus.SubscribeAsync<ServerRegisteredEvent>(cancellationToken)
-                           .ConfigureAwait(false))
+        try
         {
-            await supervisor.EnsureConnectionAsync(registered.GuildId, registered.ServerId, cancellationToken)
-                .ConfigureAwait(false);
+            await foreach (var registered in eventBus.SubscribeAsync<ServerRegisteredEvent>(cancellationToken)
+                               .ConfigureAwait(false))
+            {
+                await supervisor.EnsureConnectionAsync(registered.GuildId, registered.ServerId, cancellationToken)
+                    .ConfigureAwait(false);
+            }
         }
+        catch (OperationCanceledException)
+        {
+            // Shutting down.
+        }
+#pragma warning disable CA1031 // Broad catch is intentional: one faulting consumer must not stop the other or crash the host.
+        catch (Exception ex)
+        {
+            LogLoopFaulted(logger, ex);
+        }
+#pragma warning restore CA1031
     }
 
     private async Task ConsumeCredentialsChangedAsync(CancellationToken cancellationToken)
     {
-        await foreach (var changed in eventBus.SubscribeAsync<ServerCredentialsChangedEvent>(cancellationToken)
-                           .ConfigureAwait(false))
+        try
         {
-            await supervisor.EnsureConnectionAsync(changed.GuildId, changed.ServerId, cancellationToken)
-                .ConfigureAwait(false);
+            await foreach (var changed in eventBus.SubscribeAsync<ServerCredentialsChangedEvent>(cancellationToken)
+                               .ConfigureAwait(false))
+            {
+                await supervisor.EnsureConnectionAsync(changed.GuildId, changed.ServerId, cancellationToken)
+                    .ConfigureAwait(false);
+            }
         }
+        catch (OperationCanceledException)
+        {
+            // Shutting down.
+        }
+#pragma warning disable CA1031 // Broad catch is intentional: one faulting consumer must not stop the other or crash the host.
+        catch (Exception ex)
+        {
+            LogLoopFaulted(logger, ex);
+        }
+#pragma warning restore CA1031
     }
 
     [LoggerMessage(Level = LogLevel.Error, Message = "Connection hosted-service loop faulted.")]

src/RustPlusBot.Features.Workspace/Messages/ServerInfoMessageRenderer.cs

@@ -77,13 +77,15 @@ public async ValueTask<MessagePayload> RenderAsync(MessageRenderContext context,
                 select.AddOption(label, credential.Id.ToString(), isDefault: credential.Id == active?.Id);
             }
 
-            builder.WithSelectMenu(select);
+            builder.WithSelectMenu(select, row: 0);
         }
 
+        // Keep the remove button on its own row: Discord rejects an action row that mixes a select with buttons.
         builder.WithButton(
             localizer.Get("server.info.remove.button", context.Culture),
             $"{WorkspaceComponentIds.ServerInfoRemovePrefix}{serverId}",
-            ButtonStyle.Danger);
+            ButtonStyle.Danger,
+            row: eligible.Count > 0 ? 1 : 0);
 
         return new MessagePayload(null, embed.Build(), builder.Build());
     }

src/RustPlusBot.Features.Workspace/WorkspaceServiceCollectionExtensions.cs

@@ -41,10 +41,12 @@ public static IServiceCollection AddWorkspace(this IServiceCollection services)
         services.AddScoped<IMessageRenderer, SettingsMessageRenderer>();
         services.AddScoped<IMessageRenderer, ServerInfoMessageRenderer>();
 
-        // Reconciler + teardown (scoped).
+        // Reconciler + teardown (scoped). Register the teardown service once and expose both interfaces
+        // off the same scoped instance, so resolving either does not create a second instance.
         services.AddScoped<IWorkspaceReconciler, WorkspaceReconciler>();
-        services.AddScoped<IWorkspaceTeardownService, WorkspaceTeardownService>();
-        services.AddScoped<IServerWorkspaceRemover, WorkspaceTeardownService>();
+        services.AddScoped<WorkspaceTeardownService>();
+        services.AddScoped<IWorkspaceTeardownService>(sp => sp.GetRequiredService<WorkspaceTeardownService>());
+        services.AddScoped<IServerWorkspaceRemover>(sp => sp.GetRequiredService<WorkspaceTeardownService>());
 
         // Options (Host binds the "Workspace" section; default = danger commands off).
         services.AddOptions<WorkspaceOptions>();

src/RustPlusBot.Persistence/Credentials/CredentialStore.cs

@@ -68,18 +68,22 @@ public async Task<IReadOnlyList<Guid>> RemoveForOwnerAsync(
         ulong ownerUserId,
         CancellationToken cancellationToken = default)
     {
-        var owned = await context.PlayerCredentials
+        // Project the affected server ids first (no token material loaded), then delete set-based.
+        var serverIds = await context.PlayerCredentials
             .Where(c => c.GuildId == guildId && c.OwnerUserId == ownerUserId)
+            .Select(c => c.RustServerId)
+            .Distinct()
             .ToListAsync(cancellationToken)
             .ConfigureAwait(false);
-        if (owned.Count == 0)
+        if (serverIds.Count == 0)
         {
             return [];
         }
 
-        var serverIds = owned.Select(c => c.RustServerId).Distinct().ToList();
-        context.PlayerCredentials.RemoveRange(owned);
-        await context.SaveChangesAsync(cancellationToken).ConfigureAwait(false);
+        await context.PlayerCredentials
+            .Where(c => c.GuildId == guildId && c.OwnerUserId == ownerUserId)
+            .ExecuteDeleteAsync(cancellationToken)
+            .ConfigureAwait(false);
         return serverIds;
     }
 

src/RustPlusBot.Persistence/Migrations/20260615183313_ServerRemovalCascade.cs

@@ -10,6 +10,11 @@ public partial class ServerRemovalCascade : Migration
         /// <inheritdoc />
         protected override void Up(MigrationBuilder migrationBuilder)
         {
+            // Defensive: remove any ConnectionStates orphaned before this FK existed so adding the
+            // constraint cannot fail. No server-removal path existed before 1b-iii, so this is normally a no-op.
+            migrationBuilder.Sql(
+                "DELETE FROM \"ConnectionStates\" WHERE \"RustServerId\" NOT IN (SELECT \"Id\" FROM \"RustServers\");");
+
             migrationBuilder.AddForeignKey(
                 name: "FK_ConnectionStates_RustServers_RustServerId",
                 table: "ConnectionStates",

tests/RustPlusBot.Features.Workspace.Tests/Messages/RendererTests.cs

@@ -225,4 +225,54 @@ public async Task ServerInfo_HasRemoveServerButton()
             .SelectMany(r => r.Components).OfType<ButtonComponent>();
         Assert.Contains(buttons, b => b.CustomId == $"workspace:info:remove:{serverId}");
     }
+
+    [Fact]
+    public async Task ServerInfo_SwapSelectAndRemoveButton_AreInSeparateActionRows()
+    {
+        var serverId = Guid.NewGuid();
+        var credId = Guid.NewGuid();
+        var servers = Substitute.For<IServerService>();
+        servers.GetAsync(1, serverId, Arg.Any<CancellationToken>())
+            .Returns(new RustServer
+            {
+                Id = serverId,
+                GuildId = 1,
+                Name = "S",
+                Ip = "1.2.3.4",
+                Port = 28015
+            });
+        var connections = Substitute.For<IConnectionStore>();
+        connections.GetStateAsync(1, serverId, Arg.Any<CancellationToken>())
+            .Returns(new DomainConnectionState
+            {
+                RustServerId = serverId,
+                GuildId = 1,
+                ActiveCredentialId = credId,
+                Status = ConnectionStatus.Connected,
+                PlayerCount = 1,
+            });
+        connections.ListPoolAsync(1, serverId, Arg.Any<CancellationToken>())
+            .Returns(new List<PlayerCredential>
+            {
+                new()
+                {
+                    Id = credId,
+                    GuildId = 1,
+                    RustServerId = serverId,
+                    OwnerUserId = 7,
+                    SteamId = 5UL,
+                    Status = CredentialStatus.Active
+                },
+            });
+        var renderer = new ServerInfoMessageRenderer(servers, connections, Loc);
+
+        var payload = await renderer.RenderAsync(new MessageRenderContext(1, serverId, "en"), default);
+
+        var rows = payload.Components!.Components.OfType<ActionRowComponent>().ToList();
+        // Discord rejects an action row that mixes a select menu with buttons.
+        Assert.DoesNotContain(rows, r =>
+            r.Components.OfType<SelectMenuComponent>().Any() && r.Components.OfType<ButtonComponent>().Any());
+        Assert.Contains(rows, r => r.Components.OfType<SelectMenuComponent>().Any());
+        Assert.Contains(rows, r => r.Components.OfType<ButtonComponent>().Any());
+    }
 }