mirror of
https://github.com/okieselbach/AutopilotMonitor.git
synced 2026-10-24 04:32:32 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
+10
-3
@@ -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]
|
||||
|
||||
+22
@@ -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<ServerAction>
|
||||
{
|
||||
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<IReadOnlyDictionary<string, object>>(failed.TypedPayload!);
|
||||
Assert.Equal(DiagnosticsUploadResult.SkipModeOff, (string)typed["failureReason"]);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Dispatch_unknown_action_type_emits_failed_with_unknown_action_type_reason()
|
||||
{
|
||||
|
||||
+24
-7
@@ -45,10 +45,26 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Runtime
|
||||
/// </summary>
|
||||
public string SasUrlPrefix { get; set; }
|
||||
|
||||
/// <summary>Human-readable error code/message when upload failed. Null on success or skip.</summary>
|
||||
/// <summary>Human-readable error code/message when upload failed or was skipped. Null on success.</summary>
|
||||
public string ErrorCode { get; set; }
|
||||
|
||||
/// <summary>
|
||||
/// True when no package was built because a configuration gate said so (mode=Off,
|
||||
/// destination not configured, OnFailure + known success). <see cref="ErrorCode"/>
|
||||
/// then carries the machine-readable gate name so the emitted
|
||||
/// <c>server_action_failed</c> / <c>diagnostics_upload_failed</c> event names the
|
||||
/// actual reason instead of a generic "upload failed".
|
||||
/// </summary>
|
||||
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 };
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -191,7 +207,8 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Runtime
|
||||
/// <summary>
|
||||
/// 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 <see cref="DiagnosticsUploadResult.Skipped"/> and the gate name in ErrorCode.
|
||||
/// This method is non-fatal: all exceptions are caught and logged.
|
||||
/// </summary>
|
||||
/// <param name="enrollmentSucceeded">
|
||||
@@ -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}" : "")})...");
|
||||
|
||||
@@ -115,10 +115,11 @@ namespace AutopilotMonitor.Agent.V2.Runtime
|
||||
|
||||
/// <summary>
|
||||
/// 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 <c>rotate_config</c> ServerAction
|
||||
/// (<see cref="ServerControlPlane.BuildDispatcher"/>) so all three apply identical semantics.
|
||||
/// </summary>
|
||||
private static RemoteConfigMergeResult ApplyFetchedConfig(
|
||||
internal static RemoteConfigMergeResult ApplyFetchedConfig(
|
||||
AgentConfiguration agentConfig,
|
||||
AgentConfigResponse remoteConfig,
|
||||
BackendAuthBundle auth,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user