From b3bbcf1300aa5bb9f38887f3ec8b1a2b01646ba7 Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Sat, 29 Aug 2026 19:30:00 +0200 Subject: [PATCH] fix(agent): rotate_config applies the fetched config; diagnostics skip gates report their reason rotate_config only stored the fetched config in RemoteConfigService and never merged it into the running AgentConfiguration, so the portal quick-config flow (enable diagnostics upload, then collect) ran the diagnostics gate against the stale mode=Off and failed with a generic diagnostics_upload_failed 4 ms later. The action now runs the same ApplyFetchedConfig path as the startup fetch and reports config_fetch_failed when the live fetch fell back to cache/defaults. Skipped uploads return a DiagnosticsUploadResult.Skipped with the gate name (diagnostics_skipped_mode_off / _not_configured / _onfailure_success) instead of null, so server_action_failed names the actual reason. Co-Authored-By: Claude Fable 5 --- .../Runtime/DiagnosticsPackageServiceTests.cs | 13 ++++++-- .../Runtime/ServerActionDispatcherTests.cs | 22 +++++++++++++ .../Runtime/DiagnosticsPackageService.cs | 31 ++++++++++++++----- .../Runtime/AgentRuntimeConfig.cs | 7 +++-- .../Runtime/AgentRuntimeHost.cs | 3 ++ .../Runtime/ServerControlPlane.cs | 27 +++++++++++++++- 6 files changed, 89 insertions(+), 14 deletions(-) diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Runtime/DiagnosticsPackageServiceTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Runtime/DiagnosticsPackageServiceTests.cs index 35817b02..befd0039 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Runtime/DiagnosticsPackageServiceTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Runtime/DiagnosticsPackageServiceTests.cs @@ -527,7 +527,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Monitoring.Runtime { // On-demand mid-session (no outcome yet): the OnFailure gate must NOT skip — // there is no known success to skip on. Reaching the archive build (sentinel - // throw → error result) proves the gate was passed; a skip would return null. + // throw → error result) proves the gate was passed; a skip would return Skipped. using var rig = new Rig(); var result = await BuildGateProbe(rig.Logger, "OnFailure") .CreateAndUploadAsync(enrollmentSucceeded: null, fileNameSuffix: "server-requested"); @@ -543,7 +543,10 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Monitoring.Runtime var result = await BuildGateProbe(rig.Logger, "OnFailure") .CreateAndUploadAsync(enrollmentSucceeded: true); - Assert.Null(result); + Assert.NotNull(result); + Assert.True(result!.Skipped); + Assert.False(result.Success); + Assert.Equal(DiagnosticsUploadResult.SkipOnFailureSucceeded, result.ErrorCode); } [Fact] @@ -553,7 +556,11 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Monitoring.Runtime var result = await BuildGateProbe(rig.Logger, "Off") .CreateAndUploadAsync(enrollmentSucceeded: null); - Assert.Null(result); + // The gate name travels in ErrorCode so the dispatcher's server_action_failed + // event says "mode_off" instead of a generic "diagnostics_upload_failed". + Assert.NotNull(result); + Assert.True(result!.Skipped); + Assert.Equal(DiagnosticsUploadResult.SkipModeOff, result.ErrorCode); } [Theory] diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Runtime/ServerActionDispatcherTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Runtime/ServerActionDispatcherTests.cs index 95b3949f..12a372aa 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Runtime/ServerActionDispatcherTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Runtime/ServerActionDispatcherTests.cs @@ -122,6 +122,28 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Monitoring.Runtime Assert.Equal("config_fetch_failed", (string)typed["failureReason"]); } + [Fact] + public async Task Dispatch_request_diagnostics_skipped_by_gate_surfaces_gate_name_as_failure_reason() + { + // Session 2550bcea (2026-08-29): the on-demand collection was refused by the + // mode=Off gate and the portal only saw a generic "diagnostics_upload_failed". + // The gate name must travel through to the event so the operator can tell a + // config gate from a real upload error. + using var rig = new Rig(); + rig.DiagnosticsResult = DiagnosticsUploadResult.SkippedBy(DiagnosticsUploadResult.SkipModeOff); + + await rig.Build().DispatchAsync(new List + { + new ServerAction { Type = ServerActionTypes.RequestDiagnostics, QueuedAt = DateTime.UtcNow }, + }); + + Assert.Equal(1, rig.DiagnosticsCalls); + Assert.Empty(rig.EventsOfType("server_action_executed")); + var failed = rig.EventsOfType("server_action_failed").Single(); + var typed = Assert.IsAssignableFrom>(failed.TypedPayload!); + Assert.Equal(DiagnosticsUploadResult.SkipModeOff, (string)typed["failureReason"]); + } + [Fact] public async Task Dispatch_unknown_action_type_emits_failed_with_unknown_action_type_reason() { diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Runtime/DiagnosticsPackageService.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Runtime/DiagnosticsPackageService.cs index 744354fb..a78c3d48 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Runtime/DiagnosticsPackageService.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Runtime/DiagnosticsPackageService.cs @@ -45,10 +45,26 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Runtime /// public string SasUrlPrefix { get; set; } - /// Human-readable error code/message when upload failed. Null on success or skip. + /// Human-readable error code/message when upload failed or was skipped. Null on success. public string ErrorCode { get; set; } + /// + /// True when no package was built because a configuration gate said so (mode=Off, + /// destination not configured, OnFailure + known success). + /// then carries the machine-readable gate name so the emitted + /// server_action_failed / diagnostics_upload_failed event names the + /// actual reason instead of a generic "upload failed". + /// + public bool Skipped { get; set; } + public bool Success => BlobName != null; + + public const string SkipModeOff = "diagnostics_skipped_mode_off"; + public const string SkipNotConfigured = "diagnostics_skipped_not_configured"; + public const string SkipOnFailureSucceeded = "diagnostics_skipped_onfailure_success"; + + public static DiagnosticsUploadResult SkippedBy(string gate) => + new DiagnosticsUploadResult { Skipped = true, ErrorCode = gate }; } /// @@ -191,7 +207,8 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Runtime /// /// Creates a diagnostics ZIP and uploads it to the configured Blob Storage container. /// Returns a DiagnosticsUploadResult with BlobName set on success, or ErrorCode set on failure. - /// Returns null if the upload was skipped (mode=Off, not configured, or OnFailure+succeeded). + /// A configuration gate (mode=Off, not configured, OnFailure+succeeded) returns a result + /// with and the gate name in ErrorCode. /// This method is non-fatal: all exceptions are caught and logged. /// /// @@ -214,14 +231,14 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Runtime var mode = _configuration.DiagnosticsUploadMode ?? "Off"; if (string.Equals(mode, "Off", StringComparison.OrdinalIgnoreCase)) { - _logger.Debug("Diagnostics upload disabled (mode=Off)"); - return null; + _logger.Info("Diagnostics upload skipped: DiagnosticsUploadMode=Off on the running agent."); + return DiagnosticsUploadResult.SkippedBy(DiagnosticsUploadResult.SkipModeOff); } if (!_configuration.DiagnosticsUploadEnabled) { - _logger.Debug("Diagnostics upload skipped: not configured for this tenant"); - return null; + _logger.Info($"Diagnostics upload skipped: no upload destination configured for this tenant (mode={mode})."); + return DiagnosticsUploadResult.SkippedBy(DiagnosticsUploadResult.SkipNotConfigured); } // Null outcome (on-demand mid-session) passes through: the OnFailure gate only @@ -229,7 +246,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Runtime if (string.Equals(mode, "OnFailure", StringComparison.OrdinalIgnoreCase) && enrollmentSucceeded == true) { _logger.Info("Diagnostics upload skipped: enrollment succeeded and mode=OnFailure"); - return null; + return DiagnosticsUploadResult.SkippedBy(DiagnosticsUploadResult.SkipOnFailureSucceeded); } _logger.Info($"Creating diagnostics package (mode={mode}, enrollmentSucceeded={(enrollmentSucceeded.HasValue ? enrollmentSucceeded.ToString() : "n/a")}{(fileNameSuffix != null ? $", suffix={fileNameSuffix}" : "")})..."); diff --git a/src/Agent/AutopilotMonitor.Agent.V2/Runtime/AgentRuntimeConfig.cs b/src/Agent/AutopilotMonitor.Agent.V2/Runtime/AgentRuntimeConfig.cs index d67504d0..ba9d8f8b 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2/Runtime/AgentRuntimeConfig.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2/Runtime/AgentRuntimeConfig.cs @@ -115,10 +115,11 @@ namespace AutopilotMonitor.Agent.V2.Runtime /// /// Applies a live-fetched config to the runtime: merge, tracker ceilings, log level, - /// integrity SHA + check, bootstrap-config cleanup. Shared by the Phase 4 startup path - /// and the post-registration recovery path so both apply identical semantics. + /// integrity SHA + check, bootstrap-config cleanup. Shared by the Phase 4 startup path, + /// the post-registration recovery path and the live rotate_config ServerAction + /// () so all three apply identical semantics. /// - private static RemoteConfigMergeResult ApplyFetchedConfig( + internal static RemoteConfigMergeResult ApplyFetchedConfig( AgentConfiguration agentConfig, AgentConfigResponse remoteConfig, BackendAuthBundle auth, diff --git a/src/Agent/AutopilotMonitor.Agent.V2/Runtime/AgentRuntimeHost.cs b/src/Agent/AutopilotMonitor.Agent.V2/Runtime/AgentRuntimeHost.cs index b2397cb3..310c083a 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2/Runtime/AgentRuntimeHost.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2/Runtime/AgentRuntimeHost.cs @@ -420,6 +420,9 @@ namespace AutopilotMonitor.Agent.V2.Runtime terminationHandler: terminationHandler, remoteConfigService: remoteConfigService, diagnosticsService: diagnosticsService, + auth: auth, + agentVersion: agentVersion, + consoleMode: consoleMode, shutdown: shutdown, shutdownComplete: shutdownComplete, post: lifecyclePost, diff --git a/src/Agent/AutopilotMonitor.Agent.V2/Runtime/ServerControlPlane.cs b/src/Agent/AutopilotMonitor.Agent.V2/Runtime/ServerControlPlane.cs index ae18a3ed..885e4197 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2/Runtime/ServerControlPlane.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2/Runtime/ServerControlPlane.cs @@ -33,6 +33,9 @@ namespace AutopilotMonitor.Agent.V2.Runtime EnrollmentTerminationHandler terminationHandler, RemoteConfigService remoteConfigService, DiagnosticsPackageService diagnosticsService, + BackendAuthBundle auth, + string agentVersion, + bool consoleMode, ManualResetEventSlim shutdown, ManualResetEventSlim shutdownComplete, InformationalEventPost post, @@ -41,9 +44,31 @@ namespace AutopilotMonitor.Agent.V2.Runtime return new ServerActionDispatcher( configuration: agentConfig, logger: logger, + // rotate_config = fetch AND apply. FetchConfigAsync only stores the response + // in RemoteConfigService.CurrentConfig; the projection onto the live + // AgentConfiguration (DiagnosticsUploadMode, LogLevel, SelfDestruct, ...) + // is ApplyFetchedConfig — the same path the startup fetch uses. Without it + // the portal's "enable diagnostics, then collect" flow fetched the new + // config and still ran the diagnostics gate against the stale mode. + // FetchConfigAsync never throws: a failed fetch falls back to cache/defaults + // and reports it via LastFetchOutcome — that is NOT an executed rotation. rotateConfigAsync: async () => { - try { var _ = await remoteConfigService.FetchConfigAsync(); return true; } + try + { + var fetched = await remoteConfigService.FetchConfigAsync(); + if (remoteConfigService.LastFetchOutcome != RemoteConfigFetchOutcome.Succeeded) + { + logger.Warning($"ServerAction rotate_config: live fetch failed (outcome={remoteConfigService.LastFetchOutcome}, {remoteConfigService.LastFetchFailureMessage}) — runtime config unchanged."); + return false; + } + + var mergeResult = AgentRuntimeConfig.ApplyFetchedConfig( + agentConfig, fetched, auth, agentVersion, consoleMode, logger); + LifecycleEmitters.EmitUnrestrictedModeAuditIfChanged(post, agentConfig, mergeResult, logger); + logger.Info($"ServerAction rotate_config: applied ConfigVersion={fetched.ConfigVersion} to the running agent."); + return true; + } catch (Exception ex) { logger.Warning($"ServerAction rotate_config failed: {ex.Message}"); return false; } }, // On-demand request mid-enrollment — the session has no outcome yet. Null keeps