Say which plane decided the MCP endpoint is on, and on what port (#2389)

Darling/Darling.Tests/DarlingHostBindingTests.cs

@@ -6,6 +6,7 @@
  * Licensed under the MIT License. See LICENSE file in the project root for full license information.
  */
 
+using System;
 using System.Net;
 using Microsoft.Extensions.Logging;
 using PerformanceMonitor.Darling.Service.Hosting;
@@ -247,4 +248,153 @@ public void NestedMcpEnums_MirrorSharedBindEnums_1To1_ForTheAdapterCast()
         Assert.Equal((int)DarlingHostBinding.BindReason.TokenMissing, (int)DarlingMcpHostService.McpBindReason.TokenMissing);
         Assert.Equal((int)DarlingHostBinding.BindReason.AllowFromInvalid, (int)DarlingMcpHostService.McpBindReason.AllowFromInvalid);
     }
+
+    /* ================================================================================================
+       #2389: WHICH PLANE decided enabled/port. One mcp object in darling.json has two owners —
+       enabled/port are a seed the store overrides forever after, network.* is file-only and restart-only —
+       and before this the supervisor's `published?.Enabled ?? config.Mcp.Enabled` could not say which side
+       it had taken. The resolution now carries its provenance, so the start line names it and a
+       disagreement is reported at the point of override.
+       ================================================================================================ */
+
+    /// <summary>Nothing published yet (worker bootstrapping, or a store it never reached): the FILE values
+    /// apply, unchanged — and are flagged as such, because the control plane can still contradict them.</summary>
+    [Fact]
+    public void ResolveEndpointToggle_Unpublished_TakesTheFileValues_AndSaysSo()
+    {
+        var toggle = DarlingHostBinding.ResolveEndpointToggle(published: null, fileEnabled: true, filePort: 5152);
+
+        Assert.True(toggle.Enabled);
+        Assert.Equal(5152, toggle.Port);
+        Assert.Equal(DarlingHostBinding.EndpointToggleOrigin.File, toggle.Origin);
+
+        /* Nothing is published, so there is nothing to disagree WITH — an unpublished read is not an override. */
+        Assert.False(toggle.EnabledOverridden);
+        Assert.False(toggle.PortOverridden);
+        Assert.Null(DarlingHostBinding.DescribeToggleOverride(toggle, "mcp", "MCP", fileEnabled: true, filePort: 5152));
+    }
+
+    /// <summary>The published row wins — the pre-existing behavior, pinned so the diagnostic did not change it.</summary>
+    [Theory]
+    [InlineData(true, 5152, false, 5199)]
+    [InlineData(false, 5199, true, 5152)]
+    public void ResolveEndpointToggle_Published_AlwaysWins(bool storeEnabled, int storePort, bool fileEnabled, int filePort)
+    {
+        var toggle = DarlingHostBinding.ResolveEndpointToggle((storeEnabled, storePort), fileEnabled, filePort);
+
+        Assert.Equal(storeEnabled, toggle.Enabled);
+        Assert.Equal(storePort, toggle.Port);
+        Assert.Equal(DarlingHostBinding.EndpointToggleOrigin.ControlPlane, toggle.Origin);
+    }
+
+    /// <summary>Agreeing planes are silent: the warning fires on the MISMATCH, not on every start.</summary>
+    [Fact]
+    public void DescribeToggleOverride_PlanesAgree_IsSilent()
+    {
+        var toggle = DarlingHostBinding.ResolveEndpointToggle((true, 5152), fileEnabled: true, filePort: 5152);
+
+        Assert.False(toggle.EnabledOverridden);
+        Assert.False(toggle.PortOverridden);
+        Assert.Null(DarlingHostBinding.DescribeToggleOverride(toggle, "mcp", "MCP", fileEnabled: true, filePort: 5152));
+    }
+
+    /// <summary>
+    /// The reported case (#2389): darling.json says enabled, the store says disabled, the store wins and the
+    /// server is stopped five seconds after announcing a successful start. The message has to name BOTH sides
+    /// with the key/column an operator can act on, say which one wins, and disclose the opposite ownership of
+    /// the network block — otherwise the reader concludes their file edit worked.
+    /// </summary>
+    [Fact]
+    public void DescribeToggleOverride_FileEnabledButStoreDisabled_NamesBothPlanesAndTheWinner()
+    {
+        var toggle = DarlingHostBinding.ResolveEndpointToggle((false, 5152), fileEnabled: true, filePort: 5152);
+        var report = DarlingHostBinding.DescribeToggleOverride(toggle, "mcp", "MCP", fileEnabled: true, filePort: 5152);
+
+        Assert.NotNull(report);
+        Assert.Contains("true in darling.json (mcp.enabled)", report, StringComparison.Ordinal);
+        Assert.Contains("false in config.config_service.mcp_enabled", report, StringComparison.Ordinal);
+        Assert.Contains("CONTROL PLANE WINS", report, StringComparison.Ordinal);
+        Assert.Contains("--enable-mcp/--disable-mcp", report, StringComparison.Ordinal);
+        /* The other half of the confusion: network.* is file-authoritative and no store setting can move it. */
+        Assert.Contains("mcp.network", report, StringComparison.Ordinal);
+        Assert.Contains("cannot change where this endpoint binds", report, StringComparison.Ordinal);
+        /* The endpoint IS down here, so the consequence names that state — and only in that state. */
+        Assert.Contains("not what is keeping this endpoint down", report, StringComparison.Ordinal);
+        /* Only the field that actually DIFFERS gets a clause — the ports agree here, so no port clause.
+           (The closing advice still names mcp.port, which is why this pins the clause form, not the key.) */
+        Assert.DoesNotContain("port is 5152 in darling.json", report, StringComparison.Ordinal);
+    }
+
+    /// <summary>The port has the identical shape (the issue's second half): a server on an unexpected port
+    /// with no explanation. Both fields differing are reported together, in one line.</summary>
+    [Fact]
+    public void DescribeToggleOverride_ReportsThePortToo_AndBothFieldsAtOnce()
+    {
+        var toggle = DarlingHostBinding.ResolveEndpointToggle((true, 5199), fileEnabled: false, filePort: 5152);
+        var report = DarlingHostBinding.DescribeToggleOverride(toggle, "mcp", "MCP", fileEnabled: false, filePort: 5152);
+
+        Assert.NotNull(report);
+        Assert.Contains("false in darling.json (mcp.enabled)", report, StringComparison.Ordinal);
+        Assert.Contains("true in config.config_service.mcp_enabled", report, StringComparison.Ordinal);
+        Assert.Contains("port is 5152 in darling.json (mcp.port) but 5199 in config.config_service.mcp_port", report, StringComparison.Ordinal);
+
+        /* Review catch: the control plane turned this endpoint ON despite the file, so the closing consequence
+           must not claim it is being kept off — a diagnostic that exists to stop operators being misled about
+           the effective state cannot itself misstate it. */
+        Assert.DoesNotContain("keeping this endpoint down", report, StringComparison.Ordinal);
+        Assert.Contains("what this RUNNING endpoint is bound and gated by", report, StringComparison.Ordinal);
+    }
+
+    /// <summary>
+    /// The likeliest real mismatch, and the one that reads least like a bug: both planes agree the endpoint
+    /// runs and only the PORT differs, because someone moved it in the Viewer's Settings. It is still a
+    /// reportable disagreement — the file value is dead and the firewall rule is named off it (#2414) — but the
+    /// endpoint is up, so nothing in the message may say otherwise.
+    /// </summary>
+    [Fact]
+    public void DescribeToggleOverride_PortOnlyMismatch_ReportsIt_WithoutClaimingTheEndpointIsDown()
+    {
+        var toggle = DarlingHostBinding.ResolveEndpointToggle((true, 5199), fileEnabled: true, filePort: 5152);
+        var report = DarlingHostBinding.DescribeToggleOverride(toggle, "mcp", "MCP", fileEnabled: true, filePort: 5152);
+
+        Assert.NotNull(report);
+        Assert.True(toggle.PortOverridden);
+        Assert.False(toggle.EnabledOverridden);
+        Assert.Contains("port is 5152 in darling.json (mcp.port) but 5199 in config.config_service.mcp_port", report, StringComparison.Ordinal);
+        /* The planes agree on enabled, so no enabled clause. */
+        Assert.DoesNotContain("in darling.json (mcp.enabled)", report, StringComparison.Ordinal);
+        Assert.DoesNotContain("keeping this endpoint down", report, StringComparison.Ordinal);
+    }
+
+    /// <summary>The web dashboard is the same defect on the same seam, so it shares the resolver: one
+    /// <c>section</c> argument drives the file key, the store column and the CLI verb, which is what stops the
+    /// two surfaces' wordings from drifting apart.</summary>
+    [Fact]
+    public void DescribeToggleOverride_WebSurface_NamesTheWebKeysAndVerbs()
+    {
+        var toggle = DarlingHostBinding.ResolveEndpointToggle((false, 5153), fileEnabled: true, filePort: 5153);
+        var report = DarlingHostBinding.DescribeToggleOverride(toggle, "web", "Web dashboard", fileEnabled: true, filePort: 5153);
+
+        Assert.NotNull(report);
+        Assert.Contains("web.enabled", report, StringComparison.Ordinal);
+        Assert.Contains("config.config_service.web_enabled", report, StringComparison.Ordinal);
+        Assert.Contains("--enable-web/--disable-web", report, StringComparison.Ordinal);
+        Assert.DoesNotContain("mcp", report, StringComparison.Ordinal);
+    }
+
+    /// <summary>The start line's provenance clause distinguishes the two planes, and the file case admits it
+    /// is provisional — that line is the one the operator greps and stops reading.</summary>
+    [Fact]
+    public void DescribeToggleOrigin_DistinguishesThePlanes_AndFlagsTheFileCaseAsProvisional()
+    {
+        var fromStore = DarlingHostBinding.DescribeToggleOrigin(
+            DarlingHostBinding.ResolveEndpointToggle((true, 5152), fileEnabled: true, filePort: 5152));
+        var fromFile = DarlingHostBinding.DescribeToggleOrigin(
+            DarlingHostBinding.ResolveEndpointToggle(published: null, fileEnabled: true, filePort: 5152));
+
+        Assert.Contains("config.config_service", fromStore, StringComparison.Ordinal);
+        Assert.Contains("darling.json", fromFile, StringComparison.Ordinal);
+        Assert.Contains("PROVISIONAL", fromFile, StringComparison.Ordinal);
+        Assert.NotEqual(fromStore, fromFile);
+    }
 }

Darling/Darling.Tests/DarlingMcpSupervisorTests.cs

@@ -7,6 +7,7 @@
  */
 
 using System;
+using System.IO;
 using PerformanceMonitor.Darling.Service.Mcp;
 using Xunit;
 
@@ -68,4 +69,58 @@ public void SupervisorCadences_ArePinned()
         Assert.Equal(TimeSpan.FromSeconds(5), DarlingMcpHostService.SupervisorPollInterval);
         Assert.Equal(TimeSpan.FromSeconds(30), DarlingMcpHostService.FailedStartBackoff);
     }
+
+    /* ================================================================================================
+       #2389: the SOURCE pin. The defect was not in a decision function — it was that the supervisor's
+       `published?.Enabled ?? config.Mcp.Enabled` silently took a side and had no way to say so, while
+       the MCP network block beside it stayed file-authoritative. A behavioural test of the old code
+       passes: it resolved the right value, it just could not report it. So pin the CALL SITE, which is what a
+       future "simplification" back to the null-coalesce would break.
+       ================================================================================================ */
+
+    [Fact]
+    public void TheSupervisor_ResolvesThroughTheProvenanceAwareHelper_NotASilentNullCoalesce()
+    {
+        var source = ReadHostSource("DarlingMcpHostService.cs");
+
+        /* The silent form, gone from both halves of the resolution. */
+        Assert.DoesNotContain("published?.Enabled ??", source, StringComparison.Ordinal);
+        Assert.DoesNotContain("published?.Port ??", source, StringComparison.Ordinal);
+
+        /* Replaced by the shared resolver, and BOTH of its diagnostics wired up: the override report (the
+           disagreement, at the point of override) and the origin clause (on the start line the operator
+           greps). Either one missing leaves half the confusion in place. */
+        Assert.Contains("DarlingHostBinding.ResolveEndpointToggle(", source, StringComparison.Ordinal);
+        Assert.Contains("DarlingHostBinding.DescribeToggleOverride(", source, StringComparison.Ordinal);
+        Assert.Contains("DarlingHostBinding.DescribeToggleOrigin(", source, StringComparison.Ordinal);
+    }
+
+    /// <summary>
+    /// The disagreement is a WARNING and it is deduplicated, not emitted on every 5s poll tick: the report is
+    /// compared to the last one emitted, and cleared when the planes agree again so a LATER re-divergence is
+    /// still reported. An undeduplicated warning would be 17,280 lines a day and get filtered out, which is the
+    /// same silence in a different costume.
+    /// </summary>
+    [Fact]
+    public void TheOverrideReport_IsWarnedOncePerDistinctState()
+    {
+        var source = ReadHostSource("DarlingMcpHostService.cs");
+
+        Assert.Contains("_logger.LogWarning(\"{Report}\", overrideReport);", source, StringComparison.Ordinal);
+        Assert.Contains("!string.Equals(overrideReport, lastOverrideReport, StringComparison.Ordinal)", source, StringComparison.Ordinal);
+        Assert.Contains("lastOverrideReport = overrideReport;", source, StringComparison.Ordinal);
+    }
+
+    /// <summary>Reads the real host source, copied beside the test binary by the csproj (the same fixture the
+    /// Host-header guard pins parse).</summary>
+    private static string ReadHostSource(string fileName)
+    {
+        var path = Path.Combine(AppContext.BaseDirectory, "Fixtures", fileName);
+        Assert.True(File.Exists(path), $"{fileName} was not copied beside the test binary — check the csproj None/Link item.");
+
+        var source = File.ReadAllText(path);
+        /* Guard the guard: an unrecognizable restructure must fail loudly, not pass vacuously. */
+        Assert.Contains("var published = _state.Read();", source, StringComparison.Ordinal);
+        return source;
+    }
 }

Darling/Darling.Tests/DarlingWebSupervisorTests.cs

@@ -7,6 +7,7 @@
  */
 
 using System;
+using System.IO;
 using PerformanceMonitor.Darling.Service;
 using PerformanceMonitor.Darling.Service.Hosting;
 using PerformanceMonitor.Darling.Service.Mcp;
@@ -125,4 +126,58 @@ public void SupervisorCadences_ArePinned()
         Assert.Equal(TimeSpan.FromSeconds(5), DarlingWebHostService.SupervisorPollInterval);
         Assert.Equal(TimeSpan.FromSeconds(30), DarlingWebHostService.FailedStartBackoff);
     }
+
+    /* ================================================================================================
+       #2389: the SOURCE pin. The defect was not in a decision function — it was that the supervisor's
+       `published?.Enabled ?? config.Web.Enabled` silently took a side and had no way to say so, while
+       the web network block beside it stayed file-authoritative. A behavioural test of the old code
+       passes: it resolved the right value, it just could not report it. So pin the CALL SITE, which is what a
+       future "simplification" back to the null-coalesce would break.
+       ================================================================================================ */
+
+    [Fact]
+    public void TheSupervisor_ResolvesThroughTheProvenanceAwareHelper_NotASilentNullCoalesce()
+    {
+        var source = ReadHostSource("DarlingWebHostService.cs");
+
+        /* The silent form, gone from both halves of the resolution. */
+        Assert.DoesNotContain("published?.Enabled ??", source, StringComparison.Ordinal);
+        Assert.DoesNotContain("published?.Port ??", source, StringComparison.Ordinal);
+
+        /* Replaced by the shared resolver, and BOTH of its diagnostics wired up: the override report (the
+           disagreement, at the point of override) and the origin clause (on the start line the operator
+           greps). Either one missing leaves half the confusion in place. */
+        Assert.Contains("DarlingHostBinding.ResolveEndpointToggle(", source, StringComparison.Ordinal);
+        Assert.Contains("DarlingHostBinding.DescribeToggleOverride(", source, StringComparison.Ordinal);
+        Assert.Contains("DarlingHostBinding.DescribeToggleOrigin(", source, StringComparison.Ordinal);
+    }
+
+    /// <summary>
+    /// The disagreement is a WARNING and it is deduplicated, not emitted on every 5s poll tick: the report is
+    /// compared to the last one emitted, and cleared when the planes agree again so a LATER re-divergence is
+    /// still reported. An undeduplicated warning would be 17,280 lines a day and get filtered out, which is the
+    /// same silence in a different costume.
+    /// </summary>
+    [Fact]
+    public void TheOverrideReport_IsWarnedOncePerDistinctState()
+    {
+        var source = ReadHostSource("DarlingWebHostService.cs");
+
+        Assert.Contains("_logger.LogWarning(\"{Report}\", overrideReport);", source, StringComparison.Ordinal);
+        Assert.Contains("!string.Equals(overrideReport, lastOverrideReport, StringComparison.Ordinal)", source, StringComparison.Ordinal);
+        Assert.Contains("lastOverrideReport = overrideReport;", source, StringComparison.Ordinal);
+    }
+
+    /// <summary>Reads the real host source, copied beside the test binary by the csproj (the same fixture the
+    /// Host-header guard pins parse).</summary>
+    private static string ReadHostSource(string fileName)
+    {
+        var path = Path.Combine(AppContext.BaseDirectory, "Fixtures", fileName);
+        Assert.True(File.Exists(path), $"{fileName} was not copied beside the test binary — check the csproj None/Link item.");
+
+        var source = File.ReadAllText(path);
+        /* Guard the guard: an unrecognizable restructure must fail loudly, not pass vacuously. */
+        Assert.Contains("var published = _state.Read();", source, StringComparison.Ordinal);
+        return source;
+    }
 }