Recommendations skip databases that are a secondary copy on this node

Darling/Darling.Tests/AnalysisFindingsTabTests.cs

@@ -140,6 +140,17 @@ public void TheReadCapNote_AndThePageNote_AreBothShown()
         Assert.Contains("PAGE-NOTE", text, StringComparison.Ordinal);
     }
 
+    [Fact]
+    public void TheSecondaryReplicaNote_ShowsWithFindingsAndWithNone_AndIsAbsentWhenTheReadCarriesNone()
+    {
+        var r = Run("replicaNote");
+        Assert.Contains("REPLICA-NOTE", r.GetProperty("withFindings").GetString(), StringComparison.Ordinal);
+        Assert.Contains("REPLICA-NOTE", r.GetProperty("noFindings").GetString(), StringComparison.Ordinal);
+        Assert.DoesNotContain("REPLICA-NOTE", r.GetProperty("without").GetString(), StringComparison.Ordinal);
+        Assert.DoesNotContain("REPLICA-NOTE", r.GetProperty("nullNote").GetString(), StringComparison.Ordinal);
+        Assert.DoesNotContain("null", r.GetProperty("nullNote").GetString(), StringComparison.Ordinal);
+    }
+
     [Fact]
     public void TheTabsAbortSignal_ReachesTheRead()
     {

Darling/Darling.Tests/DarlingMcpFinOpsRecommendationsToolsTests.cs

@@ -88,7 +88,7 @@ public void Envelope_WithNoMonthlyCost_NullsTheCostAndSaysWhy()
     {
         var env = Json(DarlingMcpFinOpsRecommendationsTools.Envelope("alpha", 0m, ["Storage tier"], [Rec(null, "B"), Rec(1m, "A")]));
         Assert.Equal(
-            new[] { "server", "monthly_cost_usd", "cost_reason", "recommendation_count", "skipped_checks", "recommendations" },
+            new[] { "server", "monthly_cost_usd", "cost_reason", "recommendation_count", "skipped_checks", "secondary_replica_note", "recommendations" },
             env.EnumerateObject().Select(p => p.Name).ToArray());
         Assert.Equal("alpha", env.GetProperty("server").GetString());
         Assert.Equal(JsonValueKind.Null, env.GetProperty("monthly_cost_usd").ValueKind);
@@ -98,6 +98,24 @@ public void Envelope_WithNoMonthlyCost_NullsTheCostAndSaysWhy()
         Assert.Equal(new[] { "B", "A" }, env.GetProperty("recommendations").EnumerateArray().Select(e => e.GetProperty("category").GetString()).ToArray());
     }
 
+    [Fact]
+    public void Envelope_WithASecondaryNote_WritesItBetweenSkippedChecksAndTheRows_AndWithoutOneWritesNull()
+    {
+        var note = AgReplicaScope.SkippedNote(2);
+        var with = Json(DarlingMcpFinOpsRecommendationsTools.Envelope("alpha", 1000m, [], [Rec(1m, "A")], note));
+        Assert.Equal(
+            new[] { "server", "monthly_cost_usd", "cost_reason", "recommendation_count", "skipped_checks", "secondary_replica_note", "recommendations" },
+            with.EnumerateObject().Select(p => p.Name).ToArray());
+        Assert.Equal(note, with.GetProperty("secondary_replica_note").GetString());
+
+        /* #5558: one convention across every MCP note: the key is always there, null when nothing was skipped. */
+        var without = Json(DarlingMcpFinOpsRecommendationsTools.Envelope("alpha", 1000m, [], [Rec(1m, "A")], null));
+        Assert.Equal(JsonValueKind.Null, without.GetProperty("secondary_replica_note").ValueKind);
+        Assert.Equal(
+            new[] { "server", "monthly_cost_usd", "cost_reason", "recommendation_count", "skipped_checks", "secondary_replica_note", "recommendations" },
+            without.EnumerateObject().Select(p => p.Name).ToArray());
+    }
+
     [Fact]
     public void Envelope_WithAMonthlyCost_CarriesItAndNoReason()
     {

Darling/Darling.Tests/FinOpsRecommendationsReadsLiveTests.cs

@@ -332,7 +332,7 @@ public async Task Composer_ReportsAFailedCheck_ToTheHook_AndKeepsTheRest()
             {
                 Assert.NotNull(ex);
                 failed.Add(label);
-            }, ct);
+            }, cancellationToken: ct);
 
         Assert.Equal(new[] { "Maintenance window" }, failed.ToArray());
         Func<FinOpsRecommendation, string> key = r => $"{r.Severity}|{r.Category}|{r.Finding}|{r.Detail}|{r.EstMonthlySavings}";
@@ -404,7 +404,7 @@ public async Task Composer_DoesNotReportTheVmCpuFallback_WhileTheVmRowsStillEmit
 
         var failed = new System.Collections.Generic.List<string>();
         var after = await DarlingFinOpsRecommendationsReader.GetRecommendationsAsync(
-            dataSource, idA, 1000m, TimeoutSeconds, (label, _) => failed.Add(label), ct);
+            dataSource, idA, 1000m, TimeoutSeconds, (label, _) => failed.Add(label), cancellationToken: ct);
 
         Assert.DoesNotContain("VM right-sizing", failed);
         Assert.Contains(after, r => r.Category == "Hardware" && r.Finding.StartsWith("CPU:", StringComparison.Ordinal));

Darling/Darling.Tests/FinOpsRecommendationsSecondaryReplicaLiveTests.cs

@@ -0,0 +1,199 @@
+/*
+ * Copyright (c) 2026 Erik Darling, Darling Data LLC
+ *
+ * This file is part of the SQL Server Performance Monitor.
+ *
+ * Licensed under the MIT License. See LICENSE file in the project root for full license information.
+ */
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text.Json;
+using System.Threading;
+using System.Threading.Tasks;
+using Npgsql;
+using PerformanceMonitor.Collectors;
+using PerformanceMonitor.Common;
+using PerformanceMonitor.Darling.Service.Mcp;
+using PerformanceMonitor.Darling.Storage;
+using PerformanceMonitor.Darling.Storage.FinOps;
+using PerformanceMonitor.Darling.Viewer;
+using Xunit;
+
+namespace Darling.Tests;
+
+/* #1776 own-store: every test seeds its own scratch database, so nothing here shares rows with another test. */
+/// <summary>
+/// #5558: the FinOps Recommendations list leaves the per-database findings of a database this node holds only as a
+/// secondary copy in an Availability Group to the primary. Over the golden store, server A has an idle database
+/// (ArchiveA), two dev/test names (app_dev_a, qa1_a), two TDE databases (SalesA, OrdersA), a 2 GB uncompressed index in
+/// SalesA, and a low-latency database (OrdersA). Each test seeds the stored Availability Group snapshots for server A,
+/// reads through the storage composer, the viewer and the MCP tool, and compares with the same read over no snapshots.
+/// </summary>
+public sealed class FinOpsRecommendationsSecondaryReplicaLiveTests
+{
+    private const int TimeoutSeconds = 30;
+    private const string BaseSkip = "Set DARLING_TEST_PG to a Postgres connection string to run the live FinOps secondary replica test.";
+
+    private static int ServerId => FinOpsRecommendationsGoldenLiveTests.ServerIdA;
+
+    private static async Task<ScratchPostgres> SeedStoreAsync(string baseConnectionString, CancellationToken ct)
+    {
+        var scratch = await ScratchPostgres.CreateAsync(baseConnectionString, ct);
+        await using var connection = new NpgsqlConnection(scratch.ConnectionString);
+        await connection.OpenAsync(ct);
+        await PgMigrations.MigrateAsync(connection, ct);
+        await FinOpsRecommendationsGoldenLiveTests.SeedAsync(connection, ct);
+        return scratch;
+    }
+
+    /// <summary>One AG whose local replica has <paramref name="localRole"/>, holding <paramref name="databases"/>; snapshots taken <paramref name="age"/> ago.</summary>
+    private static async Task SeedAgAsync(string connectionString, string localRole, TimeSpan age, CancellationToken ct, params string[] databases)
+    {
+        await using var connection = new NpgsqlConnection(connectionString);
+        await connection.OpenAsync(ct);
+        var at = DateTime.SpecifyKind(DateTime.UtcNow - age, DateTimeKind.Unspecified);
+        await DarlingMcpTestData.ExecAsync(connection, ct,
+            "INSERT INTO ag_replica_states (collection_id, collection_time, server_id, server_name, ag_name, replica_server_name, role_desc, is_local) VALUES ($1,$2,$3,$4,'AG1','NODE1',$5,TRUE)",
+            CollectionIdGenerator.Next(), at, ServerId, FinOpsRecommendationsGoldenLiveTests.ServerNameA, localRole);
+        foreach (var database in databases)
+            await DarlingMcpTestData.ExecAsync(connection, ct,
+                "INSERT INTO ag_database_replica_states (collection_id, collection_time, server_id, server_name, ag_name, database_name, replica_server_name, is_local) VALUES ($1,$2,$3,$4,'AG1',$5,'NODE1',TRUE)",
+                CollectionIdGenerator.Next(), at, ServerId, FinOpsRecommendationsGoldenLiveTests.ServerNameA, database);
+    }
+
+    private static FinOpsRecommendation? Find(IEnumerable<FinOpsRecommendation> rows, string findingContains) =>
+        rows.FirstOrDefault(r => r.Finding.Contains(findingContains, StringComparison.Ordinal));
+
+    [Fact]
+    public async Task ASecondaryCopy_IsDroppedFromIdleDevTestTdeAndCompression_ButKeptInLowIoLatency()
+    {
+        var baseConnectionString = Environment.GetEnvironmentVariable("DARLING_TEST_PG");
+        Assert.SkipWhen(string.IsNullOrEmpty(baseConnectionString), BaseSkip);
+        var ct = TestContext.Current.CancellationToken;
+        await using var scratch = await SeedStoreAsync(baseConnectionString!, ct);
+        await using var dataSource = NpgsqlDataSource.Create(scratch.ConnectionString);
+
+        var baseline = await DarlingFinOpsRecommendationsReader.GetRecommendationsAsync(dataSource, ServerId, 1000m, TimeoutSeconds, cancellationToken: ct);
+        Assert.NotNull(Find(baseline, "idle database"));
+        Assert.Contains("2 possible dev/test", Find(baseline, "possible dev/test")!.Finding, StringComparison.Ordinal);
+        Assert.Contains("SalesA", Find(baseline, "TDE in use")!.Detail, StringComparison.Ordinal);
+        Assert.Contains("OrdersA", Find(baseline, "TDE in use")!.Detail, StringComparison.Ordinal);
+        Assert.NotNull(Find(baseline, "uncompressed object"));
+        Assert.Contains("OrdersA", Find(baseline, "low IO latency")!.Detail, StringComparison.Ordinal);
+
+        /* ArchiveA (idle), app_dev_a (dev/test) and SalesA (TDE and the 2 GB index) are secondary copies. Names are matched
+           exactly (#5558 round 2): the set and the stored names both come from sys.databases.name, so the casing is shared. */
+        var secondary = new HashSet<string>(StringComparer.Ordinal) { "ArchiveA", "app_dev_a", "SalesA" };
+        var rows = await DarlingFinOpsRecommendationsReader.GetRecommendationsAsync(
+            dataSource, ServerId, 1000m, TimeoutSeconds, secondaryDatabases: secondary, cancellationToken: ct);
+
+        Assert.Null(Find(rows, "idle database"));
+        Assert.Contains("1 possible dev/test", Find(rows, "possible dev/test")!.Finding, StringComparison.Ordinal);
+        Assert.DoesNotContain("app_dev_a", Find(rows, "possible dev/test")!.Detail, StringComparison.Ordinal);
+        var tde = Find(rows, "TDE in use")!;
+        Assert.Contains("OrdersA", tde.Detail, StringComparison.Ordinal);
+        Assert.DoesNotContain("SalesA", tde.Detail, StringComparison.Ordinal);
+        Assert.Null(Find(rows, "uncompressed object"));
+        /* Low IO latency is unchanged, and so is every finding that is not about one database. */
+        Assert.Equal(Find(baseline, "low IO latency")!.Detail, Find(rows, "low IO latency")!.Detail);
+        static bool PerDatabase(FinOpsRecommendation r) =>
+            r.Finding.Contains("idle database", StringComparison.Ordinal) || r.Finding.Contains("possible dev/test", StringComparison.Ordinal)
+            || r.Finding.Contains("TDE in use", StringComparison.Ordinal) || r.Finding.Contains("uncompressed object", StringComparison.Ordinal);
+        Assert.Equal(
+            baseline.Where(r => !PerDatabase(r)).Select(r => (r.Finding, r.Detail)).ToArray(),
+            rows.Where(r => !PerDatabase(r)).Select(r => (r.Finding, r.Detail)).ToArray());
+    }
+
+    [Fact]
+    public async Task EveryTdeDatabaseASecondary_SaysNothingAboutTdeAndNeverClaimsNoEnterpriseFeatures()
+    {
+        var baseConnectionString = Environment.GetEnvironmentVariable("DARLING_TEST_PG");
+        Assert.SkipWhen(string.IsNullOrEmpty(baseConnectionString), BaseSkip);
+        var ct = TestContext.Current.CancellationToken;
+        await using var scratch = await SeedStoreAsync(baseConnectionString!, ct);
+        await using var dataSource = NpgsqlDataSource.Create(scratch.ConnectionString);
+
+        var rows = await DarlingFinOpsRecommendationsReader.GetRecommendationsAsync(
+            dataSource, ServerId, 1000m, TimeoutSeconds, secondaryDatabases: new[] { "SalesA", "OrdersA" }, cancellationToken: ct);
+
+        Assert.DoesNotContain(rows, r => r.Category == "Licensing");
+        Assert.NotNull(Find(rows, "low IO latency"));
+    }
+
+    [Fact]
+    public async Task NullOrEmptySet_ChangesNothing()
+    {
+        var baseConnectionString = Environment.GetEnvironmentVariable("DARLING_TEST_PG");
+        Assert.SkipWhen(string.IsNullOrEmpty(baseConnectionString), BaseSkip);
+        var ct = TestContext.Current.CancellationToken;
+        await using var scratch = await SeedStoreAsync(baseConnectionString!, ct);
+        await using var dataSource = NpgsqlDataSource.Create(scratch.ConnectionString);
+
+        var baseline = await DarlingFinOpsRecommendationsReader.GetRecommendationsAsync(dataSource, ServerId, 1000m, TimeoutSeconds, cancellationToken: ct);
+        var empty = await DarlingFinOpsRecommendationsReader.GetRecommendationsAsync(
+            dataSource, ServerId, 1000m, TimeoutSeconds, secondaryDatabases: new HashSet<string>(), cancellationToken: ct);
+
+        Assert.Equal(baseline.Select(r => (r.Finding, r.Detail)).ToArray(), empty.Select(r => (r.Finding, r.Detail)).ToArray());
+    }
+
+    [Fact]
+    public async Task TheViewerAndTheMcpTool_ReadTheStoredSnapshots_AndFailOpenWhenTheyAreStaleOrTheNodeIsPrimary()
+    {
+        var baseConnectionString = Environment.GetEnvironmentVariable("DARLING_TEST_PG");
+        Assert.SkipWhen(string.IsNullOrEmpty(baseConnectionString), BaseSkip);
+        var ct = TestContext.Current.CancellationToken;
+        await using var scratch = await SeedStoreAsync(baseConnectionString!, ct);
+        await using var dataSource = NpgsqlDataSource.Create(scratch.ConnectionString);
+        await using var viewer = new ViewerDataService(scratch.ConnectionString);
+
+        /* No snapshots: no note, and the MCP payload carries the field as null. */
+        var (plainRows, plainNote) = await viewer.GetRecommendationsWithNoteAsync(ServerId, 1000m, ct);
+        Assert.Null(plainNote);
+        var plainRead = await FinOpsAsync(dataSource, ct);
+        Assert.Equal(System.Text.Json.JsonValueKind.Null, plainRead.GetProperty("secondary_replica_note").ValueKind);
+
+        /* A stale secondary snapshot (a collector that stopped hours ago) vouches for nothing. */
+        await SeedAgAsync(scratch.ConnectionString, "SECONDARY", TimeSpan.FromHours(6), ct, "ArchiveA");
+        var (staleRows, staleNote) = await viewer.GetRecommendationsWithNoteAsync(ServerId, 1000m, ct);
+        Assert.Null(staleNote);
+        Assert.Equal(plainRows.Select(r => r.Finding).ToArray(), staleRows.Select(r => r.Finding).ToArray());
+
+        /* A fresh secondary snapshot of ArchiveA drops its idle finding on both surfaces and writes the note. */
+        await SeedAgAsync(scratch.ConnectionString, "SECONDARY", TimeSpan.FromMinutes(2), ct, "ArchiveA");
+        var (rows, note) = await viewer.GetRecommendationsWithNoteAsync(ServerId, 1000m, ct);
+        Assert.Equal(AgReplicaScope.SkippedNote(1), note);
+        Assert.DoesNotContain(rows, r => r.Finding.Contains("idle database", StringComparison.Ordinal));
+        Assert.Contains(plainRows, r => r.Finding.Contains("idle database", StringComparison.Ordinal));
+
+        var payload = await FinOpsAsync(dataSource, ct);
+        Assert.Equal(AgReplicaScope.SkippedNote(1), payload.GetProperty("secondary_replica_note").GetString());
+        Assert.DoesNotContain(payload.GetProperty("recommendations").EnumerateArray(),
+            r => r.GetProperty("finding").GetString()!.Contains("idle database", StringComparison.Ordinal));
+    }
+
+    [Fact]
+    public async Task OnThePrimary_NothingIsSkipped_AndThereIsNoNote()
+    {
+        var baseConnectionString = Environment.GetEnvironmentVariable("DARLING_TEST_PG");
+        Assert.SkipWhen(string.IsNullOrEmpty(baseConnectionString), BaseSkip);
+        var ct = TestContext.Current.CancellationToken;
+        await using var scratch = await SeedStoreAsync(baseConnectionString!, ct);
+        await using var viewer = new ViewerDataService(scratch.ConnectionString);
+
+        var (plainRows, _) = await viewer.GetRecommendationsWithNoteAsync(ServerId, 1000m, ct);
+        await SeedAgAsync(scratch.ConnectionString, "PRIMARY", TimeSpan.FromMinutes(2), ct, "ArchiveA", "app_dev_a", "SalesA");
+        var (rows, note) = await viewer.GetRecommendationsWithNoteAsync(ServerId, 1000m, ct);
+
+        Assert.Null(note);
+        Assert.Equal(plainRows.Select(r => (r.Finding, r.Detail)).ToArray(), rows.Select(r => (r.Finding, r.Detail)).ToArray());
+    }
+
+    private static async Task<JsonElement> FinOpsAsync(NpgsqlDataSource dataSource, CancellationToken ct)
+    {
+        var json = await DarlingMcpFinOpsRecommendationsTools.GetFinOpsRecommendations(
+            dataSource, FinOpsRecommendationsGoldenLiveTests.ServerNameA, ct);
+        return JsonDocument.Parse(json).RootElement.Clone();
+    }
+}

Darling/Darling.Tests/LiteAnalysisServiceCancellationSourceTests.cs

@@ -39,7 +39,7 @@ public sealed class LiteAnalysisServiceCancellationSourceTests
     private const string CollectConfigAuditFactsAsyncSignature =
         "public async Task<List<Fact>> CollectConfigAuditFactsAsync(";
     private const string ComparePeriodsAsyncSignature =
-        "public async Task<(List<Fact> BaselineFacts, List<Fact> ComparisonFacts, WindowCoverage? BaselineCoverage, WindowCoverage? ComparisonCoverage, IReadOnlyDictionary<string, BaselineBucket> Dispersion)> ComparePeriodsAsync(";
+        "public async Task<(List<Fact> BaselineFacts, List<Fact> ComparisonFacts, WindowCoverage? BaselineCoverage, WindowCoverage? ComparisonCoverage, IReadOnlyDictionary<string, BaselineBucket> Dispersion, IReadOnlySet<string>? BaselineSecondaries, IReadOnlySet<string>? ComparisonSecondaries)> ComparePeriodsAsync(";
     private const string LookUpDispersionAsyncSignature =
         "private async Task<IReadOnlyDictionary<string, BaselineBucket>> LookUpDispersionAsync(";
     // The next declaration after LookUpDispersionAsync in file order (#4203's four end here); anchors the

Darling/Darling.Tests/McpPayloadContractCensusTests.cs

@@ -2305,7 +2305,7 @@ public static readonly (string File, string Member, int Limit)[] StatementTermin
     [
         ("DarlingFinOpsRecommendationsReader.cs", "(file scope)", 10),
         ("LocalDataService.Blocking.cs", "GetBlockingPairRowsAsync", 5000),
-        ("LocalDataService.FinOps.Recommendations.cs", "GetRecommendationsAsync", 10),
+        ("LocalDataService.FinOps.Recommendations.cs", "GetRecommendationsWithNoteAsync", 10),
         ("LocalDataService.PlanCache.cs", "GetPlanCacheSnapshotAsync", 30),
         ("LocalDataService.RunningJobs.cs", "GetAnomalousJobsAsync", 5),
         ("LocalDataService.WaitStats.cs", "GetAllQuerySnapshotsInRangeAsync", 2000),