Collect PostgreSQL configuration and what changed in it (#2658), and register ten MCP tools that shipped unreachable (#2659)

.github/workflows/build.yml

@@ -725,8 +725,8 @@ jobs:
       #
       # The values are the product's own derivation from the live hypertable count
       # (TimescaleSupport.HypertableCount = the 62-collector catalog + collection_log = 63):
-      #   timescaledb.max_background_workers = HypertableCount + 2       = 69
-      #   max_worker_processes = 3 + (HypertableCount + 2) + 8           = 80
+      #   timescaledb.max_background_workers = HypertableCount + 2       = 70
+      #   max_worker_processes = 3 + (HypertableCount + 2) + 8           = 81
       # Hard-coded here because a workflow cannot call into the product — so
       # CiClusterWorkerSizingTests parses THIS FILE and fails the build if either number stops
       # matching the formula as collectors are added, and CiClusterWorkerSizingLiveTests asserts
@@ -744,8 +744,8 @@ jobs:
           Add-Content -Path "$dataDir\postgresql.conf" -Value "shared_preload_libraries = 'timescaledb'"
           Add-Content -Path "$dataDir\postgresql.conf" -Value "port = 5541"
           Add-Content -Path "$dataDir\postgresql.conf" -Value "listen_addresses = '127.0.0.1'"
-          Add-Content -Path "$dataDir\postgresql.conf" -Value "timescaledb.max_background_workers = 69"
-          Add-Content -Path "$dataDir\postgresql.conf" -Value "max_worker_processes = 80"
+          Add-Content -Path "$dataDir\postgresql.conf" -Value "timescaledb.max_background_workers = 70"
+          Add-Content -Path "$dataDir\postgresql.conf" -Value "max_worker_processes = 81"
           & "$bin\pg_ctl.exe" -D $dataDir -l $logFile -w start
           if ($LASTEXITCODE -ne 0) { if (Test-Path $logFile) { Get-Content $logFile -Tail 50 }; throw "pg_ctl start failed ($LASTEXITCODE)" }
           & "$bin\createdb.exe" -h 127.0.0.1 -p 5541 -U darling darling

.github/workflows/nightly.yml

@@ -369,8 +369,8 @@ jobs:
       # customer runs and made scheduler-racing failures luck-of-the-slot instead of reproducible.
       # Values are the product's own derivation from the live hypertable count
       # (TimescaleSupport.HypertableCount = the 62-collector catalog + collection_log = 63):
-      #   timescaledb.max_background_workers = HypertableCount + 2  = 69
-      #   max_worker_processes = 3 + (HypertableCount + 2) + 8      = 80
+      #   timescaledb.max_background_workers = HypertableCount + 2  = 70
+      #   max_worker_processes = 3 + (HypertableCount + 2) + 8      = 81
       # Kept honest by CiClusterWorkerSizingTests (parses this file against the formula) and
       # CiClusterWorkerSizingLiveTests (asserts the running cluster serves them). Must configure
       # the cluster identically to build.yml's darling-pg job: that guard parses the appended
@@ -391,8 +391,8 @@ jobs:
           Add-Content -Path "$dataDir\postgresql.conf" -Value "shared_preload_libraries = 'timescaledb'"
           Add-Content -Path "$dataDir\postgresql.conf" -Value "port = 5541"
           Add-Content -Path "$dataDir\postgresql.conf" -Value "listen_addresses = '127.0.0.1'"
-          Add-Content -Path "$dataDir\postgresql.conf" -Value "timescaledb.max_background_workers = 69"
-          Add-Content -Path "$dataDir\postgresql.conf" -Value "max_worker_processes = 80"
+          Add-Content -Path "$dataDir\postgresql.conf" -Value "timescaledb.max_background_workers = 70"
+          Add-Content -Path "$dataDir\postgresql.conf" -Value "max_worker_processes = 81"
           & "$bin\pg_ctl.exe" -D $dataDir -l $logFile -w start
           if ($LASTEXITCODE -ne 0) { if (Test-Path $logFile) { Get-Content $logFile -Tail 50 }; throw "pg_ctl start failed ($LASTEXITCODE)" }
           & "$bin\createdb.exe" -h 127.0.0.1 -p 5541 -U darling darling

CHANGELOG.md

@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 
 ### Added
+- **PostgreSQL configuration, and what changed in it** ([#2658]) - `pg_settings` was never collected, so neither "what is `work_mem` set to on this server" nor "what changed last Tuesday" had an answer, and the second is the kind that cannot be recovered later at any price: a configuration history nobody recorded is not sitting on the server waiting to be read. `get_pg_server_config` reports the settings somebody actually chose, non-default first, with where each value came from and whether changing it needs a restart or a reload. `get_pg_server_config_changes` reports value changes between snapshots, old beside new. Both name `pending_restart` loudly - the state where `postgresql.conf` has been edited and reloaded but the running server is still on the old value, so the file and the server disagree with no symptom until a restart months later changes behaviour during someone else's incident.
 - **Test an index from the predicate grid** ([#2612]) - right-click a row in PostgreSQL predicate statistics and ask whether the planner would actually use an index on that column. The command shipped with no caller: the only way to reach it was hand-writing a row into the command queue, which is how it was tested and is not a feature. It hangs off that grid and nowhere else, which is the shape it was scoped to - on demand only, never scheduled, driven from a row somebody is already looking at. The confirmation says what it costs the server before it runs (nothing executed, no index built, session reset), and a predicate whose estimate error is already large is flagged BEFORE the round trip, because an index does not fix a plan built on a wrong row count.
 - **Azure SQL DB now reports every database's size, not just the connected one** ([#2643], raised from the field) - `sys.database_files` is database-scoped, so a Viewer pointed at `master` showed `master`'s two files and nothing else, which is correct and reads exactly like a broken collector. `sys.resource_stats` is a master-only view carrying `storage_in_megabytes` per database, so from a `master` connection the siblings now appear too - as one row each, labelled `(whole database)` with a NULL `file_id`, because that view has no per-file breakdown and a fabricated file name would make the grid look complete and be wrong. The sibling read runs through `sp_executesql`: the view does not exist in a user database and name resolution happens at parse time, so a guarded UNION still fails with 208 everywhere else. Verified against a live Azure SQL Database from both a `master` and a user-database connection.
 - **Mark rows in grids** ([#2645], requested from the field) - right-click any FinOps grid and mark the selected rows **Done**, **To Do** or **Do Not Do**, so you can work through a result set and remember which rows you have dealt with and which you have decided against. Asked for on Index Analysis, where you decide index by index. Marks are held against the row objects, so they last exactly as long as the result set does: on a run-on-demand grid until you run it again, on a live grid until the next refresh. They are painted from `LoadingRow`, so no row model gained a property and no grid gained a column.
@@ -77,6 +78,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **Lite's portable ZIP is self-contained, which HALVED it** ([#2501]) - `Publish Lite` is now `-r win-x64 --self-contained` in both `build.yml` and `nightly.yml`, so neither Lite artifact has a .NET prerequisite any more and the failure [#2489] documented stops existing: a tester who unzips onto a stock Windows Server no longer meets the .NET host's bare `You must install .NET to run this application` before a line of our code runs. **The size went the opposite way from what bundling a runtime suggests.** The old publish was RID-agnostic, so it copied every platform its packages ship - **537 MB of `runtimes\` on a 565 MB tree** (osx 130, linux-x64 116, linux-arm64 70, win-arm64 56, then win-x86, musl, loongarch64 and riscv64), of which only the **52 MB `win-x64`** folder could ever load on Windows. `DuckDB.NET.Bindings.Full` is most of it, SkiaSharp and SqlClient behind it. Dropping ~485 MB of unloadable native payload beats the cost of bundling .NET, WPF and ASP.NET Core by roughly two to one: measured on one commit and one SDK, **565 MB tree / 212.7 MB zipped becomes 277 MB / 114.2 MB**. It matters most for the **nightly** ZIP, which is the UAT download and is not offered as a `Setup.exe` at all. **A RID-specific publish needed two more files than the flag.** `Lite/packages.lock.json` had only a `net10.0-windows7.0` target, and a RID restore adds `net10.0-windows7.0/win-x64` to it - after which the `dotnet restore --locked-mode` that BOTH workflows run before the publish fails `NU1004: the project's runtime identifiers have changed`, because locked mode compares the PROJECT's RID set (empty) against the lock file's (win-x64). Reproduced locally; that is a red CI run on every PR, not the future `--no-restore` trap it was filed as. The fix is `<RuntimeIdentifiers>win-x64</RuntimeIdentifiers>` in `PerformanceMonitorLite.csproj`, so the project itself asks for that graph and one committed lock file satisfies the RID-less locked-mode restore and the RID publish alike; `RuntimeIdentifiers` (plural) sets no RID on the build, so a plain `dotnet build` stays RID-agnostic and `Lite.Tests` is untouched. **SignPath needed nothing** - the `Lite` artifact-configuration slug already receives both shapes today, and the signed re-zip reads `signed/Lite/*`, inheriting whatever shape `publish/Lite` has. Auto-update is unaffected; the ZIP is not a Velopack channel. `LiteRuntimePrerequisiteDocsTests` went red on the flag alone (3 of its 7 facts) and was rewritten to state every claim BOTH ways round: [#2499]'s version asserted only that the docs DID name the runtimes, so two of its facts stayed green while the prose went stale. It now also derives the lock file's RID coverage from the `-r` flags in the workflows, and every new assertion was proven red with its fix reverted.
 
 ### Fixed
+- **Ten PostgreSQL MCP tools were never registered with the host, so no agent could call them** ([#2659]) - `get_pg_write_stats`, `get_pg_buffer_usage`, `get_pg_extensions`, `get_pg_lock_stats`, `get_pg_index_bloat`, `get_pg_column_stats`, `get_pg_kernel_stats`, `get_pg_predicate_stats`, `get_pg_replication_stats` and `get_pg_wait_sampling` were implemented, documented, dispatched by the web dashboard and counted in the instructions census, and `tools/list` answered 116 tools where the census claimed 126. Registration is per class and explicit, and nothing failed when a class was left out: the inventory pin checks tool NAMES, which exist either way, and the tab pin exists to stop a read shipping reachable only through MCP - this was the exact inverse. A reflection-derived pin now asserts every `[McpServerToolType]` class is registered, so it fails when someone adds a class rather than when an agent next reaches for the tool.
 - **PostgreSQL 18 silently lost every I/O byte figure, and the estimate it replaced was off by an order of magnitude** ([#2655]) - 18 removed `op_bytes` from `pg_stat_io`, and both byte figures `get_pg_io_stats` serves were derived from it, so on 18 they came back null with no note, no status and nothing to distinguish a version change from a collector that had stopped. The columns 18 replaced it with are better than what was lost: `op_bytes` was the per-operation block size that the read multiplied by a count to ESTIMATE volume, while `read_bytes`/`write_bytes`/`extend_bytes` are measured totals. 18 also introduced vectored reads, so one entry in `reads` can cover several blocks and the old estimate undercounts - measured through the running service against a real 18.6 target, one combination reported 4,742 reads against 448,724,992 bytes where the estimate would have said 38,846,464, an 11.6x undercount, and three combinations ran 10x to 16x. They are now collected and served, with `bytes_source` on every row and on the envelope saying whether a figure was measured or estimated, because the two are not comparable and must never share a name silently.
 - **A PostgreSQL column that the server's VERSION removed read as a missing measurement** ([#2653]) - PostgreSQL 17 removed `buffers_backend` and `buffers_backend_fsync` from `pg_stat_bgwriter` outright, and `get_pg_write_stats` returned both as bare nulls under a note that went on explaining `buffers_backend` as a live backpressure signal. The collector was right - it emits NULL for them deliberately from 17 on - but nothing anywhere recorded the target's PostgreSQL major, so no read could tell a column the version does not have from one nothing collected. Seven PostgreSQL collectors gate on that version and the read layer had no access to it at all. The registry now carries it, stamped on every connect like the engine kind next door, and this read spends it: on 17 and later it names the removal, says it is not a measurement gap, and points at `get_pg_io_stats`, where the fact actually lives now.
 - **Self-hosted PostgreSQL had no query TEXT, so `test_hypothetical_index` could never work there** ([#2651]) - the statement-text store read `aurora_stat_statements()` with no vanilla path, so off Aurora `collect.pg_statement_text` was never populated. Two things failed silently: `get_pg_top_queries` returned `query_text: null` on every row forever, while that field's own documentation says null means "not captured YET" - true on Aurora, a lie here; and #2612's `test_hypothetical_index` resolves its statement from that table, so it always answered "no statement text is stored" and blamed a refresh cadence for a missing source. Fixing it exposed a second defect immediately: `pg_stat_statements` keys on `(queryid, userid, dbid, toplevel)`, so one queryid returns once per user and database, and the upsert - which keys on `(server_id, queryid)` - met those duplicates as `21000: ON CONFLICT DO UPDATE command cannot affect row a second time` and abandoned every batch. Both fixed and verified against a real self-hosted PostgreSQL: 47 statement texts stored where there were zero, and the hypothetical-index command then answered end to end for the first time - cost 1,059.34 to 438.71, a 58.6% reduction.
@@ -3011,3 +3013,5 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 [#2564]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2564
 [#2653]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2653
 [#2655]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2655
+[#2658]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2658
+[#2659]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2659

Darling/Darling.Tests/McpToolTypeRegistrationTests.cs

@@ -0,0 +1,113 @@
+// Copyright (c) Erik Darling Data. All rights reserved.
+// Licensed under the terms in the LICENSE file in the repository root.
+
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using System.Reflection;
+using System.Text.RegularExpressions;
+using ModelContextProtocol.Server;
+using PerformanceMonitor.Darling.Service.Mcp;
+using Xunit;
+
+namespace Darling.Tests;
+
+/// <summary>
+/// Every <see cref="McpServerToolTypeAttribute"/> class must actually be registered with the MCP host
+/// (#2659).
+///
+/// <para><b>Six were not.</b> Ten shipped PostgreSQL reads — <c>get_pg_write_stats</c>,
+/// <c>get_pg_buffer_usage</c>, <c>get_pg_extensions</c>, <c>get_pg_lock_stats</c>,
+/// <c>get_pg_index_bloat</c>, <c>get_pg_column_stats</c>, <c>get_pg_kernel_stats</c>,
+/// <c>get_pg_predicate_stats</c>, <c>get_pg_replication_stats</c>, <c>get_pg_wait_sampling</c> — were
+/// implemented, documented, dispatched by the web API, counted in the instructions census and covered by
+/// the name-based inventory pin, and an agent could not call any of them. Asked of the running service,
+/// <c>tools/list</c> answered 116 tools where the census claimed 126.</para>
+///
+/// <para><b>Why the existing guards could not see it.</b> The inventory pin checks tool NAMES, and the
+/// names exist — the attribute is on the method whether or not the class is registered. The
+/// <c>POSTGRES_TABS</c> pin asserts every <c>get_pg_*</c> read reaches a web tab, and its own header says
+/// it exists so a new read "cannot ship reachable only through MCP". This is the exact inverse, and there
+/// was no pin for it: these shipped reachable only through the WEB.</para>
+///
+/// <para><b>Derived, not enumerated.</b> The check walks the assembly for the attribute and the host source
+/// for its registrations, so it cannot go stale the way a hand-kept list does — and it fails the moment
+/// someone adds a class, rather than whenever an agent next reaches for the tool. That is the same
+/// reasoning as the tab pin being derived from the dispatch.</para>
+/// </summary>
+public sealed class McpToolTypeRegistrationTests
+{
+    [Fact]
+    public void EveryMcpServerToolTypeClass_IsRegisteredWithTheHost()
+    {
+        var declared = typeof(DarlingMcpHostService).Assembly
+            .GetTypes()
+            .Where(t => t.GetCustomAttribute<McpServerToolTypeAttribute>() is not null)
+            .Select(t => t.Name)
+            .OrderBy(n => n, StringComparer.Ordinal)
+            .ToList();
+
+        Assert.NotEmpty(declared);
+
+        var registered = RegisteredToolTypeNames();
+
+        var missing = declared.Where(n => !registered.Contains(n)).ToList();
+
+        Assert.True(
+            missing.Count == 0,
+            "These [McpServerToolType] classes are never registered with the MCP host, so every tool they "
+            + "declare is unreachable over MCP even though its name exists and the web API dispatches it: "
+            + string.Join(", ", missing)
+            + ". Add a .WithGeminiCompatibleTools<T>() line in DarlingMcpHostService.");
+    }
+
+    /// <summary>
+    /// Reads the registrations out of the host SOURCE rather than by invoking the builder, because the
+    /// builder needs a host, a store and a live configuration, and this is a wiring question that should be
+    /// answerable without any of them.
+    /// </summary>
+    private static HashSet<string> RegisteredToolTypeNames()
+    {
+        var path = HostSourcePath();
+        var source = File.ReadAllText(path);
+
+        var names = Regex
+            .Matches(source, @"WithGeminiCompatibleTools<(\w+)>")
+            .Select(m => m.Groups[1].Value)
+            .ToHashSet(StringComparer.Ordinal);
+
+        Assert.True(
+            names.Count > 0,
+            $"Found no .WithGeminiCompatibleTools<T>() registrations in {path}. If the registration style "
+            + "changed, this test needs to learn the new one rather than be deleted — it is the only thing "
+            + "standing between a new tools class and shipping unreachable.");
+
+        return names;
+    }
+
+    private static string HostSourcePath()
+    {
+        var dir = new DirectoryInfo(AppContext.BaseDirectory);
+
+        while (dir is not null)
+        {
+            var candidate = Path.Combine(
+                dir.FullName,
+                "Darling",
+                "PerformanceMonitor.Darling.Service",
+                "Mcp",
+                "DarlingMcpHostService.cs");
+
+            if (File.Exists(candidate))
+            {
+                return candidate;
+            }
+
+            dir = dir.Parent;
+        }
+
+        throw new FileNotFoundException(
+            "Could not locate DarlingMcpHostService.cs by walking up from the test output directory.");
+    }
+}