refactor(v2): extract BackendClientFactory from Program.RunAgent (PR3)

Phase 3 (BackendApiClient + DistressReporter + EmergencyReporter + AuthFailureTracker + AuthCertificateMissing distress emit) and Phase 5 (mTLS HttpClient + BackendTelemetryUploader) move into Runtime/BackendClientFactory.cs. BackendAuthBundle holds the four clients plus the cached manufacturer/model/serial from HardwareInfo so RunAgent + Phase 5 read the same hardware identity. TelemetryClientResult is the discriminated outcome for Phase 5 — Continue with mTLS+uploader, or Exit with the V1-parity codes (4 = mTLS construction failure, 5 = uploader construction failure). Phase 4 (RemoteConfig fetch + merge + binary-integrity + bootstrap- config cleanup) stays inline in RunAgent for PR4. RunAgent reads the Phase 3 clients via local aliases so Phase 4 wiring is unchanged. RunAgent's Phase 3+5 blocks shrink from ~80 lines to ~12 lines of factory invocations + result destructuring. No behavioural change: constructor argument lists, distress-emit guard, exit-code mapping and the disposal-on-failure semantics are preserved. Tests: 4 new BackendClientFactoryTests (auth-bundle smoke, no-cert-lookup-on-bootstrap-auth, TelemetryClientResult.Exit, TelemetryClientResult.Continue). V2.Core 747/750 without the 3 known EnrollmentOrchestrator threading flakes (pass solo). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-10-24 20:53:06 +00:00 · 2026-04-26 11:50:01 +02:00
parent 9b9ad61315
commit 1785854ac7
5 changed files with 337 additions and 86 deletions
@@ -0,0 +1,100 @@
+using System.IO;
+using AutopilotMonitor.Agent.V2.Core.Configuration;
+using AutopilotMonitor.Agent.V2.Core.Logging;
+using AutopilotMonitor.Agent.V2.Core.Tests.Harness;
+using AutopilotMonitor.Agent.V2.Runtime;
+using Xunit;
+
+namespace AutopilotMonitor.Agent.V2.Core.Tests.Program
+{
+    /// <summary>
+    /// Tests for <see cref="BackendClientFactory"/>: Phase 3 (auth-clients bundle) and
+    /// Phase 5 (telemetry-clients result) extracts from <c>Program.RunAgent</c>.
+    /// </summary>
+    public sealed class BackendClientFactoryTests
+    {
+        private static AgentLogger NewLogger(string path)
+            => new AgentLogger(Path.Combine(path, "logs"), AgentLogLevel.Info);
+
+        private static AgentConfiguration MinimalBootstrapTokenConfig()
+            => new AgentConfiguration
+            {
+                ApiBaseUrl = "https://example.invalid",
+                TenantId = "00000000-0000-0000-0000-000000000123",
+                SessionId = "11111111-1111-1111-1111-111111111111",
+                BootstrapToken = "test-token",
+                UseBootstrapTokenAuth = true,
+                UseClientCertAuth = false,
+                MaxAuthFailures = 3,
+                AuthFailureTimeoutMinutes = 30,
+            };
+
+        // ============================================================ BuildAuthClients
+
+        [Fact]
+        public void BuildAuthClients_returns_bundle_with_all_clients_populated()
+        {
+            using var tmp = new TempDirectory();
+            var logger = NewLogger(tmp.Path);
+            var config = MinimalBootstrapTokenConfig();
+
+            var bundle = BackendClientFactory.BuildAuthClients(config, agentVersion: "1.2.3.4", logger: logger);
+
+            Assert.NotNull(bundle);
+            Assert.NotNull(bundle.BackendApiClient);
+            Assert.NotNull(bundle.DistressReporter);
+            Assert.NotNull(bundle.EmergencyReporter);
+            Assert.NotNull(bundle.AuthFailureTracker);
+            // HardwareInfo.GetHardwareInfo always returns string fields (possibly empty on
+            // non-WMI environments) — the bundle must surface whatever it produced.
+            Assert.NotNull(bundle.Manufacturer);
+            Assert.NotNull(bundle.Model);
+            Assert.NotNull(bundle.SerialNumber);
+        }
+
+        [Fact]
+        public void BuildAuthClients_does_not_attempt_cert_lookup_when_bootstrap_auth_used()
+        {
+            using var tmp = new TempDirectory();
+            var logger = NewLogger(tmp.Path);
+            var config = MinimalBootstrapTokenConfig();
+
+            var bundle = BackendClientFactory.BuildAuthClients(config, agentVersion: "1.2.3.4", logger: logger);
+
+            // With UseClientCertAuth=false, BackendApiClient skips the cert-store search
+            // entirely → ClientCertificate is null and no AuthCertificateMissing distress
+            // is dispatched. Asserts the bundle is consistent with that path.
+            Assert.Null(bundle.BackendApiClient.ClientCertificate);
+        }
+
+        // =========================================================== TelemetryClientResult
+
+        [Fact]
+        public void TelemetryClientResult_Exit_carries_exit_code_and_no_clients()
+        {
+            var result = TelemetryClientResult.Exit(4);
+
+            Assert.True(result.ShouldExit);
+            Assert.Equal(4, result.ExitCode);
+            Assert.Null(result.MtlsHttpClient);
+            Assert.Null(result.Uploader);
+        }
+
+        [Fact]
+        public void TelemetryClientResult_Continue_carries_clients_and_zero_exit_code()
+        {
+            using var tmp = new TempDirectory();
+            using var fakeHttpClient = new System.Net.Http.HttpClient();
+
+            // BackendTelemetryUploader is hard to construct purely (needs auth tracker etc.) —
+            // pass null, the Continue() factory does not validate. The Result-type contract
+            // is what we exercise here.
+            var result = TelemetryClientResult.Continue(fakeHttpClient, uploader: null);
+
+            Assert.False(result.ShouldExit);
+            Assert.Equal(0, result.ExitCode);
+            Assert.Same(fakeHttpClient, result.MtlsHttpClient);
+            Assert.Null(result.Uploader);
+        }
+    }
+}
@@ -197,53 +197,13 @@ namespace AutopilotMonitor.Agent.V2
            var isWhiteGloveResume = bootstrap.IsWhiteGloveResume;
            var cleanupServiceFactory = bootstrap.CleanupServiceFactory;

-            var backendApiClient = new BackendApiClient(
-                baseUrl: agentConfig.ApiBaseUrl,
-                configuration: agentConfig,
-                logger: logger,
-                agentVersion: GetAgentVersion());
-
-            // M4.6.γ — Emergency + Distress reporters. Plumbed into RemoteConfigService so
-            // Config-fetch failures (auth vs network) flow to the correct channel. Also fires
-            // an initial AuthCertificateMissing distress when the MDM cert was expected but
-            // not found (Legacy parity — surfaces pre-MDM-enrollment dead-ends to the backend
-            // via the cert-less distress channel).
-            var hardwareForReporters = HardwareInfo.GetHardwareInfo(logger);
-            var distressReporter = new DistressReporter(
-                baseUrl: agentConfig.ApiBaseUrl,
-                tenantId: agentConfig.TenantId,
-                manufacturer: hardwareForReporters.Manufacturer,
-                model: hardwareForReporters.Model,
-                serialNumber: hardwareForReporters.SerialNumber,
-                agentVersion: GetAgentVersion(),
-                logger: logger);
-
-            var emergencyReporter = new EmergencyReporter(
-                apiClient: backendApiClient,
-                sessionId: agentConfig.SessionId,
-                tenantId: agentConfig.TenantId,
-                agentVersion: GetAgentVersion(),
-                logger: logger);
-
-            if (agentConfig.UseClientCertAuth && backendApiClient.ClientCertificate == null)
-            {
-                _ = distressReporter.TrySendAsync(
-                    DistressErrorType.AuthCertificateMissing,
-                    "MDM certificate not found in LocalMachine or CurrentUser store");
-            }
-
-            // Central observer for consecutive 401/403 responses. Initialised with the CLI/bootstrap
-            // defaults on AgentConfiguration; UpdateLimits is called after RemoteConfigMerger.Merge
-            // so tenant-policy overrides take effect. ThresholdExceeded is wired below, once the
-            // shutdown signal is available, to trigger a clean agent exit when the limit is hit.
-            // V1 parity — the distress reporter is plumbed in at construction so the tracker is
-            // the single dispatch point for auth-failure distress (first failure only).
-            var authFailureTracker = new AuthFailureTracker(
-                maxFailures: agentConfig.MaxAuthFailures,
-                timeoutMinutes: agentConfig.AuthFailureTimeoutMinutes,
-                clock: SystemClock.Instance,
-                logger: logger,
-                distressReporter: distressReporter);
+            // Phase 3 (BackendApiClient + reporters + auth-failure tracker) is encapsulated
+            // in BackendClientFactory; see Runtime/BackendClientFactory.cs.
+            var auth = Runtime.BackendClientFactory.BuildAuthClients(agentConfig, GetAgentVersion(), logger);
+            var backendApiClient = auth.BackendApiClient;
+            var distressReporter = auth.DistressReporter;
+            var emergencyReporter = auth.EmergencyReporter;
+            var authFailureTracker = auth.AuthFailureTracker;

            var remoteConfigService = new RemoteConfigService(
                backendApiClient, agentConfig.TenantId, logger, emergencyReporter, distressReporter, authFailureTracker);
@@ -315,45 +275,16 @@ namespace AutopilotMonitor.Agent.V2
                logger.Debug($"BootstrapConfigCleanup outer exception: {ex.Message}");
            }

-            HttpClient mtlsHttpClient;
-            BackendTelemetryUploader uploader;
-            try
+            // Phase 5 (mTLS HttpClient + BackendTelemetryUploader) is encapsulated in
+            // BackendClientFactory. Construction failures map to V1-parity exit codes
+            // (4 = mTLS, 5 = uploader). See Runtime/BackendClientFactory.cs.
+            var telemetry = Runtime.BackendClientFactory.BuildTelemetryClients(agentConfig, auth, GetAgentVersion(), logger);
+            if (telemetry.ShouldExit)
            {
-                // Share BackendApiClient.NetworkMetrics so the mTLS pipeline records every
-                // outbound request — most importantly BackendTelemetryUploader's POST
-                // /api/agent/telemetry, which dominates per-session traffic. Without this the
-                // agent_metrics_snapshot net_total_requests undercounts ~25-30x (only the
-                // legacy BackendApiClient calls would show up).
-                mtlsHttpClient = MtlsHttpClientFactory.Create(
-                    resolver: new DefaultCertificateResolver(),
-                    logger: logger,
-                    metrics: backendApiClient.NetworkMetrics);
-            }
-            catch (InvalidOperationException ex)
-            {
-                logger.Error("mTLS HttpClient creation failed — cannot upload telemetry.", ex);
-                return 4;
-            }
-
-            try
-            {
-                uploader = new BackendTelemetryUploader(
-                    httpClient: mtlsHttpClient,
-                    baseUrl: agentConfig.ApiBaseUrl,
-                    tenantId: agentConfig.TenantId,
-                    manufacturer: hardwareForReporters.Manufacturer,
-                    model: hardwareForReporters.Model,
-                    serialNumber: hardwareForReporters.SerialNumber,
-                    bootstrapToken: agentConfig.UseBootstrapTokenAuth ? agentConfig.BootstrapToken : null,
-                    agentVersion: GetAgentVersion(),
-                    authFailureTracker: authFailureTracker,
-                    logger: logger);     // Plan §5 Fix 5 — upload-cadence logging
-            }
-            catch (Exception ex)
-            {
-                logger.Error("BackendTelemetryUploader construction failed.", ex);
-                return 5;
+                return telemetry.ExitCode;
            }
+            var mtlsHttpClient = telemetry.MtlsHttpClient;
+            var uploader = telemetry.Uploader;

            // V1 parity (MonitoringService.Start, MonitoringService.RegisterSessionAsync) —
            // POST /api/agent/register-session with 5-retry exponential backoff (2s/4s/8s/16s)
@@ -0,0 +1,220 @@
+using System;
+using System.Net.Http;
+using AutopilotMonitor.Agent.V2.Core.Configuration;
+using AutopilotMonitor.Agent.V2.Core.Logging;
+using AutopilotMonitor.Agent.V2.Core.Monitoring.Transport;
+using AutopilotMonitor.Agent.V2.Core.Orchestration;
+using AutopilotMonitor.Agent.V2.Core.Security;
+using AutopilotMonitor.Agent.V2.Core.Transport.Telemetry;
+using AutopilotMonitor.DecisionCore.Engine;
+using AutopilotMonitor.Shared.Models;
+
+namespace AutopilotMonitor.Agent.V2.Runtime
+{
+    /// <summary>
+    /// Phase 3 + Phase 5 of <see cref="Program"/>'s <c>RunAgent</c>: backend HTTP-clients
+    /// (BackendApiClient + reporters + auth-failure tracker) and the mTLS-backed telemetry
+    /// uploader. Phase 4 (RemoteConfig fetch + merge + binary-integrity check) lives in
+    /// <c>RunAgent</c> between the two factory calls and consumes Phase 3's clients to
+    /// produce the merged <see cref="AgentConfigResponse"/>.
+    /// </summary>
+    internal static class BackendClientFactory
+    {
+        /// <summary>
+        /// Phase 3 — construct the backend HTTP clients, the distress / emergency reporters
+        /// and the auth-failure tracker. Fires a one-shot <c>AuthCertificateMissing</c>
+        /// distress when client-cert auth is configured but no cert was found in either
+        /// LocalMachine or CurrentUser store (Legacy parity — pre-MDM-enrollment dead-end
+        /// surface). Tracker ceilings reflect the CLI/bootstrap defaults; tenant-policy
+        /// overrides are applied later via <see cref="AuthFailureTracker.UpdateLimits"/>
+        /// once <c>RemoteConfigMerger</c> has run.
+        /// </summary>
+        public static BackendAuthBundle BuildAuthClients(
+            AgentConfiguration agentConfig,
+            string agentVersion,
+            AgentLogger logger)
+        {
+            if (agentConfig == null) throw new ArgumentNullException(nameof(agentConfig));
+            if (logger == null) throw new ArgumentNullException(nameof(logger));
+
+            var backendApiClient = new BackendApiClient(
+                baseUrl: agentConfig.ApiBaseUrl,
+                configuration: agentConfig,
+                logger: logger,
+                agentVersion: agentVersion);
+
+            // M4.6.γ — Emergency + Distress reporters. Plumbed into RemoteConfigService so
+            // Config-fetch failures (auth vs network) flow to the correct channel. Also fires
+            // an initial AuthCertificateMissing distress when the MDM cert was expected but
+            // not found (Legacy parity — surfaces pre-MDM-enrollment dead-ends to the backend
+            // via the cert-less distress channel).
+            var hardware = HardwareInfo.GetHardwareInfo(logger);
+            var distressReporter = new DistressReporter(
+                baseUrl: agentConfig.ApiBaseUrl,
+                tenantId: agentConfig.TenantId,
+                manufacturer: hardware.Manufacturer,
+                model: hardware.Model,
+                serialNumber: hardware.SerialNumber,
+                agentVersion: agentVersion,
+                logger: logger);
+
+            var emergencyReporter = new EmergencyReporter(
+                apiClient: backendApiClient,
+                sessionId: agentConfig.SessionId,
+                tenantId: agentConfig.TenantId,
+                agentVersion: agentVersion,
+                logger: logger);
+
+            if (agentConfig.UseClientCertAuth && backendApiClient.ClientCertificate == null)
+            {
+                _ = distressReporter.TrySendAsync(
+                    DistressErrorType.AuthCertificateMissing,
+                    "MDM certificate not found in LocalMachine or CurrentUser store");
+            }
+
+            // Central observer for consecutive 401/403 responses. Initialised with the CLI/bootstrap
+            // defaults on AgentConfiguration; UpdateLimits is called after RemoteConfigMerger.Merge
+            // so tenant-policy overrides take effect. ThresholdExceeded is wired in RunAgent, once
+            // the shutdown signal is available, to trigger a clean agent exit when the limit is hit.
+            // V1 parity — the distress reporter is plumbed in at construction so the tracker is
+            // the single dispatch point for auth-failure distress (first failure only).
+            var authFailureTracker = new AuthFailureTracker(
+                maxFailures: agentConfig.MaxAuthFailures,
+                timeoutMinutes: agentConfig.AuthFailureTimeoutMinutes,
+                clock: SystemClock.Instance,
+                logger: logger,
+                distressReporter: distressReporter);
+
+            return new BackendAuthBundle(
+                backendApiClient: backendApiClient,
+                manufacturer: hardware.Manufacturer,
+                model: hardware.Model,
+                serialNumber: hardware.SerialNumber,
+                distressReporter: distressReporter,
+                emergencyReporter: emergencyReporter,
+                authFailureTracker: authFailureTracker);
+        }
+
+        /// <summary>
+        /// Phase 5 — construct the mTLS-backed <see cref="HttpClient"/> and the
+        /// <see cref="BackendTelemetryUploader"/>. The uploader's <see cref="NetworkMetrics"/>
+        /// instance is shared with the legacy <see cref="BackendApiClient"/>'s pipeline so
+        /// the agent_metrics_snapshot net_total_requests reflects every outbound HTTP call,
+        /// including the dominant <c>/api/agent/telemetry</c> POST (Plan §5 Fix 5 — without
+        /// this the counter undercounts ~25-30x).
+        /// </summary>
+        public static TelemetryClientResult BuildTelemetryClients(
+            AgentConfiguration agentConfig,
+            BackendAuthBundle auth,
+            string agentVersion,
+            AgentLogger logger)
+        {
+            if (agentConfig == null) throw new ArgumentNullException(nameof(agentConfig));
+            if (auth == null) throw new ArgumentNullException(nameof(auth));
+            if (logger == null) throw new ArgumentNullException(nameof(logger));
+
+            HttpClient mtlsHttpClient;
+            try
+            {
+                // Share BackendApiClient.NetworkMetrics so the mTLS pipeline records every
+                // outbound request — most importantly BackendTelemetryUploader's POST
+                // /api/agent/telemetry, which dominates per-session traffic. Without this the
+                // agent_metrics_snapshot net_total_requests undercounts ~25-30x (only the
+                // legacy BackendApiClient calls would show up).
+                mtlsHttpClient = MtlsHttpClientFactory.Create(
+                    resolver: new DefaultCertificateResolver(),
+                    logger: logger,
+                    metrics: auth.BackendApiClient.NetworkMetrics);
+            }
+            catch (InvalidOperationException ex)
+            {
+                logger.Error("mTLS HttpClient creation failed — cannot upload telemetry.", ex);
+                return TelemetryClientResult.Exit(4);
+            }
+
+            BackendTelemetryUploader uploader;
+            try
+            {
+                uploader = new BackendTelemetryUploader(
+                    httpClient: mtlsHttpClient,
+                    baseUrl: agentConfig.ApiBaseUrl,
+                    tenantId: agentConfig.TenantId,
+                    manufacturer: auth.Manufacturer,
+                    model: auth.Model,
+                    serialNumber: auth.SerialNumber,
+                    bootstrapToken: agentConfig.UseBootstrapTokenAuth ? agentConfig.BootstrapToken : null,
+                    agentVersion: agentVersion,
+                    authFailureTracker: auth.AuthFailureTracker,
+                    logger: logger);     // Plan §5 Fix 5 — upload-cadence logging
+            }
+            catch (Exception ex)
+            {
+                logger.Error("BackendTelemetryUploader construction failed.", ex);
+                return TelemetryClientResult.Exit(5);
+            }
+
+            return TelemetryClientResult.Continue(mtlsHttpClient, uploader);
+        }
+    }
+
+    /// <summary>
+    /// Phase 3 outcome — the four backend-facing clients and the cached hardware identity
+    /// they were built with. RunAgent retains this bundle through the rest of startup so
+    /// the lifecycle / termination wiring can read each client without re-construction.
+    /// </summary>
+    internal sealed class BackendAuthBundle
+    {
+        public BackendApiClient BackendApiClient { get; }
+        public string Manufacturer { get; }
+        public string Model { get; }
+        public string SerialNumber { get; }
+        public DistressReporter DistressReporter { get; }
+        public EmergencyReporter EmergencyReporter { get; }
+        public AuthFailureTracker AuthFailureTracker { get; }
+
+        public BackendAuthBundle(
+            BackendApiClient backendApiClient,
+            string manufacturer,
+            string model,
+            string serialNumber,
+            DistressReporter distressReporter,
+            EmergencyReporter emergencyReporter,
+            AuthFailureTracker authFailureTracker)
+        {
+            BackendApiClient = backendApiClient;
+            Manufacturer = manufacturer;
+            Model = model;
+            SerialNumber = serialNumber;
+            DistressReporter = distressReporter;
+            EmergencyReporter = emergencyReporter;
+            AuthFailureTracker = authFailureTracker;
+        }
+    }
+
+    /// <summary>
+    /// Phase 5 outcome — either an early exit (V1 parity: 4 = mTLS construction failure,
+    /// 5 = uploader construction failure) or a Continue payload carrying the live mTLS
+    /// <see cref="HttpClient"/> and <see cref="BackendTelemetryUploader"/>.
+    /// </summary>
+    internal sealed class TelemetryClientResult
+    {
+        public bool ShouldExit { get; }
+        public int ExitCode { get; }
+        public HttpClient MtlsHttpClient { get; }
+        public BackendTelemetryUploader Uploader { get; }
+
+        private TelemetryClientResult(bool shouldExit, int exitCode, HttpClient mtls, BackendTelemetryUploader uploader)
+        {
+            ShouldExit = shouldExit;
+            ExitCode = exitCode;
+            MtlsHttpClient = mtls;
+            Uploader = uploader;
+        }
+
+        public static TelemetryClientResult Exit(int code)
+            => new TelemetryClientResult(true, code, null, null);
+
+        public static TelemetryClientResult Continue(HttpClient mtls, BackendTelemetryUploader uploader)
+            => new TelemetryClientResult(false, 0, mtls, uploader);
+    }
+}
@@ -1 +1 @@
-367
+370
@@ -1 +1 @@
-1177
+1179