From fb472b518ec248233ee3639dd9684740d3ea59fa Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Thu, 11 Jun 2026 07:28:33 +0200 Subject: [PATCH] refactor(agent-v2): group termination-handler ctor into 4 surfaces (ARCH-F2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit EnrollmentTerminationHandler's 25-parameter constructor (17 optional, 9 Func<> accessors, wired via ~80 lambda lines in AgentRuntimeHost) becomes 11 parameters: four cohesive surfaces plus direct infra deps. - TerminationSessionContext (concrete, pure data): configuration, stateDirectory, agentStartTimeUtc, agentVersion, sessionPersistence, dialogExePathOverride, lazy IsWhiteGlovePart2. - IAppTrackingReadModel: currentState, packageStates, appTimings, ignoredCount, promoteActiveInstallsToStuck, lastEspTerminalFailure. Production: OrchestratorAppTrackingReadModel over the orchestrator's CollectorSurfaces (builds on ARCH-F4). - IDrainStatus: spool/ingress drain counts behind CanObserve* capability flags (replaces the accessor-null wiring variants); production: OrchestratorDrainStatus with the V1-parity 10s budget. - IShutdownGate: signalShutdown, triggerReboot, writeCleanExitMarker, tryClaimShutdownEvent; production: RuntimeShutdownGate in the host project (owns the shutdown.exe logic formerly DefaultTriggerReboot). Per-parameter rationale comments (c117946b, 080edee9, 7dd4e593, Codex Finding 2, Option 1/2 hardening, plan §11) moved onto the interface members. Test rig builds recording fakes of the same interfaces, so the optional-default wiring variants production never ran are gone. No behavior change. V2.Core 1297/1297, DecisionCore 359/359, full solution build green. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../EnrollmentTerminationHandlerTests.cs | 114 +++++--- .../Periodic/AgentSelfMetricsCollector.cs | 6 +- .../EnrollmentTerminationHandler.cs | 247 +++++------------- .../Termination/IAppTrackingReadModel.cs | 77 ++++++ .../Termination/IDrainStatus.cs | 58 ++++ .../Termination/IShutdownGate.cs | 48 ++++ .../OrchestratorAppTrackingReadModel.cs | 43 +++ .../Termination/OrchestratorDrainStatus.cs | 35 +++ .../Termination/TerminationSessionContext.cs | 70 +++++ .../Runtime/AgentRuntimeHost.cs | 93 ++----- .../Runtime/RuntimeShutdownGate.cs | 54 ++++ .../buildcounter.txt | 2 +- 12 files changed, 557 insertions(+), 290 deletions(-) create mode 100644 src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/IAppTrackingReadModel.cs create mode 100644 src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/IDrainStatus.cs create mode 100644 src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/IShutdownGate.cs create mode 100644 src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/OrchestratorAppTrackingReadModel.cs create mode 100644 src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/OrchestratorDrainStatus.cs create mode 100644 src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/TerminationSessionContext.cs create mode 100644 src/Agent/AutopilotMonitor.Agent.V2/Runtime/RuntimeShutdownGate.cs diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Termination/EnrollmentTerminationHandlerTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Termination/EnrollmentTerminationHandlerTests.cs index e9a31cc3..7c2a7702 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Termination/EnrollmentTerminationHandlerTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Termination/EnrollmentTerminationHandlerTests.cs @@ -176,12 +176,16 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Termination { config ??= BuildConfig(); return new EnrollmentTerminationHandler( - configuration: config, + session: new TerminationSessionContext( + configuration: config, + stateDirectory: StateDir, + agentStartTimeUtc: StartUtc, + agentVersion: agentVersion, + sessionPersistence: SessionPersistence), + appTracking: new RigAppTracking(this), + drainStatus: new RigDrainStatus(this), + shutdownGate: new RigShutdownGate(this), logger: Logger, - stateDirectory: StateDir, - agentStartTimeUtc: StartUtc, - currentStateAccessor: () => State, - packageStatesAccessor: () => Packages, cleanupServiceFactory: () => CleanupService, uploadDiagnosticsAsync: (succeeded, suffix) => { @@ -190,44 +194,86 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Termination LastDiagnosticsSuffix = suffix; return Task.FromResult(DiagnosticsResult ?? new DiagnosticsUploadResult { BlobName = "blob" }); }, - signalShutdown: () => - { - Interlocked.Increment(ref ShutdownSignalled); - TerminationActionLog.Add("signalShutdown"); - }, analyzerManager: null, post: Post, - sessionPersistence: SessionPersistence, - triggerReboot: delay => - { - Interlocked.Increment(ref RebootInvocations); - RebootDelaySeconds = delay; - }, // Zero out the timing ceremony for tests — production paths are covered by // the dedicated V1-parity tests below which opt back in via their own Rig. - lateEventGracePeriod: TimeSpan.Zero, - spoolDrainPeriod: SpoolDrainPeriodOverride, - appTimingsAccessor: () => AppTimingsOverride ?? new Dictionary(), - agentVersion: agentVersion, - pendingItemCountAccessor: PendingItemCountAccessor, - writeCleanExitMarker: WriteCleanExitMarkerHook ?? new Action(() => - { - Interlocked.Increment(ref CleanExitMarkerWrites); - TerminationActionLog.Add("writeCleanExitMarker"); - }), - ingressPendingSignalCountAccessor: IngressPendingSignalCountAccessor, - promoteActiveInstallsToStuck: (failureType, message, errorCode) => - { - PromotionCalls.Add((failureType, message, errorCode)); - return PromotionReturnValue; - }, - tryClaimShutdownEvent: TryClaimShutdownEventOverride, - lastEspTerminalFailureAccessor: () => LastEspTerminalFailureOverride); + lateEventGracePeriod: TimeSpan.Zero); } public void Dispose() => Tmp.Dispose(); } + /// ARCH-F2 — grouped read-model fake over the rig's mutable test state. + private sealed class RigAppTracking : IAppTrackingReadModel + { + private readonly Rig _rig; + public RigAppTracking(Rig rig) { _rig = rig; } + + public DecisionState CurrentState => _rig.State; + public IReadOnlyList? PackageStates => _rig.Packages; + public IReadOnlyDictionary? AppTimings => + _rig.AppTimingsOverride ?? new Dictionary(); + public int IgnoredCount => 0; + + public IReadOnlyList PromoteActiveInstallsToStuck(string failureType, string message, string? errorCode) + { + _rig.PromotionCalls.Add((failureType, message, errorCode!)); + return _rig.PromotionReturnValue; + } + + public EspTerminalFailureSnapshot? LastEspTerminalFailure => _rig.LastEspTerminalFailureOverride; + } + + /// + /// ARCH-F2 — drain fake. The nullable rig accessors map onto the capability flags: + /// an unset accessor means the surface is unobservable (legacy blind-delay path). + /// + private sealed class RigDrainStatus : IDrainStatus + { + private readonly Rig _rig; + public RigDrainStatus(Rig rig) { _rig = rig; } + + public TimeSpan SpoolDrainPeriod => _rig.SpoolDrainPeriodOverride; + public bool CanObserveIngress => _rig.IngressPendingSignalCountAccessor != null; + public long IngressPendingSignalCount => _rig.IngressPendingSignalCountAccessor!(); + public bool CanObserveSpool => _rig.PendingItemCountAccessor != null; + public int SpoolPendingItemCount => _rig.PendingItemCountAccessor!(); + } + + /// ARCH-F2 — recording shutdown gate (no real shutdown.exe in tests). + private sealed class RigShutdownGate : IShutdownGate + { + private readonly Rig _rig; + public RigShutdownGate(Rig rig) { _rig = rig; } + + public void SignalShutdown() + { + Interlocked.Increment(ref _rig.ShutdownSignalled); + _rig.TerminationActionLog.Add("signalShutdown"); + } + + public void TriggerReboot(int delaySeconds) + { + Interlocked.Increment(ref _rig.RebootInvocations); + _rig.RebootDelaySeconds = delaySeconds; + } + + public void WriteCleanExitMarker() + { + if (_rig.WriteCleanExitMarkerHook != null) + { + _rig.WriteCleanExitMarkerHook(); + return; + } + Interlocked.Increment(ref _rig.CleanExitMarkerWrites); + _rig.TerminationActionLog.Add("writeCleanExitMarker"); + } + + // Null override = legacy always-emit behaviour (claim always succeeds). + public bool TryClaimShutdownEvent() => _rig.TryClaimShutdownEventOverride?.Invoke() ?? true; + } + private static EnrollmentTerminatedEventArgs Args( EnrollmentTerminationReason reason, EnrollmentTerminationOutcome outcome, diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Periodic/AgentSelfMetricsCollector.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Periodic/AgentSelfMetricsCollector.cs index afe320f0..3009e9b7 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Periodic/AgentSelfMetricsCollector.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Periodic/AgentSelfMetricsCollector.cs @@ -122,9 +122,9 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Periodic } // --- Spool stats (P2 telemetry) --- - // _telemetrySpool is the live transport-layer spool, plumbed through - // DefaultComponentFactory.SetTelemetrySpool from EnrollmentOrchestrator.Start - // step 3. Null only on test fakes that don't construct a real spool. + // _telemetrySpool is the live transport-layer spool, passed by + // EnrollmentOrchestrator.Start into IComponentFactory.CreateCollectorHosts + // (ARCH-F4). Null only on test fakes that don't construct a real spool. int pendingItemCount = 0; long spoolFileSizeBytes = 0L; if (_telemetrySpool != null) diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/EnrollmentTerminationHandler.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/EnrollmentTerminationHandler.cs index 0bfc62cd..d3c1c497 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/EnrollmentTerminationHandler.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/EnrollmentTerminationHandler.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.IO; using System.Threading; using System.Threading.Tasks; @@ -51,170 +50,73 @@ namespace AutopilotMonitor.Agent.V2.Core.Termination public sealed class EnrollmentTerminationHandler { private static readonly TimeSpan DefaultLateEventGrace = TimeSpan.FromMilliseconds(2000); - private static readonly TimeSpan DefaultSpoolDrain = TimeSpan.FromMilliseconds(10000); // V1 parity: up to 20 × 500ms drains before shutdown.exe // Option 1 (WG Part 1 graceful-exit hardening, 2026-04-30): poll cadence for the // active spool-empty drain. 50ms is short enough that we exit the bounded wait // within ~one cadence after the last upload is acknowledged, but long enough that // an empty spool only costs a single sleep call. private static readonly TimeSpan SpoolDrainPollInterval = TimeSpan.FromMilliseconds(50); + // ARCH-F2 (V2 agent review 2026-06-10): the former 25-parameter constructor is + // grouped into four cohesive surfaces (session context, app-tracking read model, + // drain status, shutdown gate) plus a handful of direct infrastructure parameters. + // The per-parameter rationale comments (c117946b, 080edee9, 7dd4e593, Codex + // Finding 2, Option 1/2 hardening, plan §11) moved onto the corresponding + // interface members. + private readonly TerminationSessionContext _session; + private readonly IAppTrackingReadModel _appTracking; + private readonly IDrainStatus _drain; + private readonly IShutdownGate _shutdownGate; + + // Session-context scalars copied to fields once — immutable for the handler's + // lifetime. Only IsWhiteGlovePart2 must be read lazily via _session, because the + // orchestrator resolves the Part-2 hint after this handler is constructed. private readonly AgentConfiguration _configuration; - private readonly AgentLogger _logger; private readonly string _stateDirectory; private readonly DateTime _agentStartTimeUtc; - private readonly Func _currentStateAccessor; - // F5 (debrief 7dd4e593) — accept any IReadOnlyList so the - // termination summary can iterate the union of phase-snapshotted apps + live - // _packageStates (V2's clear-on-phase-transition would otherwise drop the - // DeviceSetup apps from app_tracking_summary and the SummaryDialog). - private readonly Func> _packageStatesAccessor; - private readonly Func> _appTimingsAccessor; - // V1-parity field: count of IME apps the tracker has marked as "ignored" (e.g. uninstall - // intents that don't surface in the install pipeline). Lives on the live - // AppPackageStateList only, hence a separate accessor — the deduped phase-snapshot - // union accessed via doesn't carry it. - private readonly Func _ignoredCountAccessor; + private readonly string _agentVersion; + private readonly SessionIdPersistence _sessionPersistence; + private readonly string _dialogExePathOverride; + + private readonly AgentLogger _logger; private readonly Func _cleanupServiceFactory; private readonly Func> _uploadDiagnosticsAsync; - private readonly Action _signalShutdown; private readonly Action _stopPeripheralCollectors; - private readonly string _dialogExePathOverride; private readonly AgentAnalyzerManager _analyzerManager; private readonly InformationalEventPost _post; - private readonly SessionIdPersistence _sessionPersistence; - private readonly Action _triggerReboot; private readonly TimeSpan _lateEventGracePeriod; - private readonly TimeSpan _spoolDrainPeriod; - private readonly string _agentVersion; - // Option 1 (WG Part 1 graceful-exit hardening, 2026-04-30): when wired, the - // termination handler polls this accessor every - // during and exits the wait as soon as it returns 0 - // (spool fully acknowledged by the backend). Falls back to the bounded - // timeout if the spool never drains. Null = legacy - // blind-delay behaviour (kept for tests + safety on the off-chance the orchestrator - // wiring fails to provide it). - private readonly Func _pendingItemCountAccessor; - // Codex Finding 2 (2026-04-30): the termination handler is now dispatched off the - // ingress worker, so it can wait for the worker to actually process the lifecycle - // events the handler posts (agent_shutting_down, whiteglove_part1_complete, - // analyzer events) BEFORE polling the spool — without this the spool-empty check - // would trivially fire on already-uploaded items while the just-posted events are - // still in the ingress channel. Polled in as the first - // wait step. Null = legacy behaviour (no ingress-drain wait, just spool-drain). - private readonly Func _ingressPendingSignalCountAccessor; - // Option 2 (same hardening): writes the clean-exit.marker file directly, - // before hands control to the main thread. This wins - // the race against an admin-triggered reseal-reboot — without this hook the marker - // is only written by the AppDomain.ProcessExit handler, which Windows can pre-empt. - // Null = no early write (tests / parity with the original blind-exit path). - private readonly Action _writeCleanExitMarker; - // V1-symmetric Part-2 hint accessor (plan §11). A Part-2 resume runs as a fresh - // Classic enrollment after Archive-and-Reset, so the terminal Stage is - // Completed/Failed like any first-run completion. The shutdown - // analyzer pipeline still needs to know it was a Part-2 run so SoftwareInventoryAnalyzer - // can tag findings with phase=2 and the backend's vulnerability-correlation pipeline - // can filter Part-2 inventory out of the Part-1 set. Wired by AgentRuntimeHost as - // () => orchestrator.IsWhiteGlovePart2; null in tests = legacy (always - // treats as Part-1/null). - private readonly Func _isWhiteGlovePart2Accessor; - - // Shutdown-gap closure (2026-05-15): cross-path idempotency gate shared with the - // AgentRuntimeHost's gap emitters (Ctrl+C, ProcessExit, unhandled exception, - // runtime-host finally). The handler calls TryClaim() before emitting - // agent_shutting_down in so a Terminated - // event that races a Ctrl+C cannot produce two events on the wire. Null = legacy - // behaviour (always emit; backward compat for tests + standalone construction). - private readonly Func _tryClaimShutdownEvent; - - // c117946b debrief (2026-05-12): on terminal ESP-Apps failure, promote any apps the - // agent observed in to Error so the - // user sees a name (not just an opaque "installing: 1" counter) and the - // app_install_failed event carries the canonical failureType. Accessor is invoked - // ONLY when the discriminator in - // matches — every other terminal path leaves app states untouched. Returns the list - // of promoted appIds for logging; null = legacy (no promotion). - // - // Session 080edee9 follow-up (2026-05-28): third parameter carries the HRESULT from - // the failed Apps subcategory (e.g. 0x87D1041C) when available, so the - // promotion can stamp + emit an enriched - // app_install_failed event. Null = no HRESULT observed → fallback to the - // legacy "esp_apps_timeout" classification. - private readonly Func> _promoteActiveInstallsToStuck; - - // Session 080edee9 follow-up + Codex review (P2/P3, 2026-05-28): accessor - // returning the latest ESP failure context (HRESULT + failedSubcategory + - // category) observed by ProvisioningStatusTracker. Read once during - // . The classifier first - // checks snapshot.IsAppsSubcategory — only then does the HRESULT - // drive the failureType. A non-Apps ESP failure (DevicePreparation/*, - // DeviceSetup/SecurityPolicies, AccountSetup/CertificatesAccountSetup) - // falls through to the generic esp_apps_timeout wording so a - // non-app HRESULT cannot mis-classify in-flight installs. - // Null accessor or null snapshot = no HRESULT context → fallback. - private readonly Func _lastEspTerminalFailureAccessor; - private int _handled; public EnrollmentTerminationHandler( - AgentConfiguration configuration, + TerminationSessionContext session, + IAppTrackingReadModel appTracking, + IDrainStatus drainStatus, + IShutdownGate shutdownGate, AgentLogger logger, - string stateDirectory, - DateTime agentStartTimeUtc, - Func currentStateAccessor, - Func> packageStatesAccessor, Func cleanupServiceFactory, Func> uploadDiagnosticsAsync, - Action signalShutdown, - string dialogExePathOverride = null, AgentAnalyzerManager analyzerManager = null, InformationalEventPost post = null, - SessionIdPersistence sessionPersistence = null, - Action triggerReboot = null, - TimeSpan? lateEventGracePeriod = null, - TimeSpan? spoolDrainPeriod = null, - Func> appTimingsAccessor = null, - string agentVersion = null, Action stopPeripheralCollectors = null, - Func ignoredCountAccessor = null, - Func pendingItemCountAccessor = null, - Action writeCleanExitMarker = null, - Func ingressPendingSignalCountAccessor = null, - Func isWhiteGlovePart2Accessor = null, - Func> promoteActiveInstallsToStuck = null, - Func tryClaimShutdownEvent = null, - Func lastEspTerminalFailureAccessor = null) + TimeSpan? lateEventGracePeriod = null) { - _configuration = configuration ?? throw new ArgumentNullException(nameof(configuration)); + _session = session ?? throw new ArgumentNullException(nameof(session)); + _appTracking = appTracking ?? throw new ArgumentNullException(nameof(appTracking)); + _drain = drainStatus ?? throw new ArgumentNullException(nameof(drainStatus)); + _shutdownGate = shutdownGate ?? throw new ArgumentNullException(nameof(shutdownGate)); _logger = logger ?? throw new ArgumentNullException(nameof(logger)); - _stateDirectory = string.IsNullOrEmpty(stateDirectory) ? throw new ArgumentNullException(nameof(stateDirectory)) : stateDirectory; - _agentStartTimeUtc = agentStartTimeUtc; - _currentStateAccessor = currentStateAccessor ?? throw new ArgumentNullException(nameof(currentStateAccessor)); - _packageStatesAccessor = packageStatesAccessor ?? throw new ArgumentNullException(nameof(packageStatesAccessor)); - // Plan §5 Fix 4 — optional timing accessor (older call sites / tests without IME - // plumbing pass null → empty timings, which is handled below). - _appTimingsAccessor = appTimingsAccessor ?? (() => new Dictionary()); - // Default to 0 when the host doesn't supply an accessor (e.g. tests without an - // ImeLogTracker wired up). Live ignoredCount only matters when there's a real tracker. - _ignoredCountAccessor = ignoredCountAccessor ?? (() => 0); _cleanupServiceFactory = cleanupServiceFactory ?? throw new ArgumentNullException(nameof(cleanupServiceFactory)); _uploadDiagnosticsAsync = uploadDiagnosticsAsync ?? throw new ArgumentNullException(nameof(uploadDiagnosticsAsync)); - _signalShutdown = signalShutdown ?? throw new ArgumentNullException(nameof(signalShutdown)); - _stopPeripheralCollectors = stopPeripheralCollectors; - _dialogExePathOverride = dialogExePathOverride; _analyzerManager = analyzerManager; _post = post; - _sessionPersistence = sessionPersistence; - _triggerReboot = triggerReboot ?? DefaultTriggerReboot; + _stopPeripheralCollectors = stopPeripheralCollectors; _lateEventGracePeriod = lateEventGracePeriod ?? DefaultLateEventGrace; - _spoolDrainPeriod = spoolDrainPeriod ?? DefaultSpoolDrain; - _agentVersion = agentVersion ?? string.Empty; - _pendingItemCountAccessor = pendingItemCountAccessor; - _writeCleanExitMarker = writeCleanExitMarker; - _ingressPendingSignalCountAccessor = ingressPendingSignalCountAccessor; - _isWhiteGlovePart2Accessor = isWhiteGlovePart2Accessor; - _promoteActiveInstallsToStuck = promoteActiveInstallsToStuck; - _tryClaimShutdownEvent = tryClaimShutdownEvent; - _lastEspTerminalFailureAccessor = lastEspTerminalFailureAccessor; + + _configuration = session.Configuration; + _stateDirectory = session.StateDirectory; + _agentStartTimeUtc = session.AgentStartTimeUtc; + _agentVersion = session.AgentVersion; + _sessionPersistence = session.SessionPersistence; + _dialogExePathOverride = session.DialogExePathOverride; } /// @@ -337,14 +239,14 @@ namespace AutopilotMonitor.Agent.V2.Core.Termination } finally { - try { _signalShutdown(); } + try { _shutdownGate.SignalShutdown(); } catch (Exception ex) { _logger.Warning($"EnrollmentTerminationHandler: signalShutdown threw: {ex.Message}"); } } } private DecisionState TryGetCurrentState() { - try { return _currentStateAccessor(); } + try { return _appTracking.CurrentState; } catch (Exception ex) { _logger.Warning($"EnrollmentTerminationHandler: current state accessor threw: {ex.Message}"); @@ -354,7 +256,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Termination private IReadOnlyList TryGetPackageStates() { - try { return _packageStatesAccessor(); } + try { return _appTracking.PackageStates; } catch (Exception ex) { _logger.Warning($"EnrollmentTerminationHandler: package states accessor threw: {ex.Message}"); @@ -364,7 +266,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Termination private IReadOnlyDictionary TryGetAppTimings() { - try { return _appTimingsAccessor() ?? new Dictionary(); } + try { return _appTracking.AppTimings ?? new Dictionary(); } catch (Exception ex) { _logger.Warning($"EnrollmentTerminationHandler: app timings accessor threw: {ex.Message}"); @@ -397,7 +299,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Termination int? wgPart; if (args.StageName == SessionStage.WhiteGloveSealed.ToString()) wgPart = 1; - else if (_isWhiteGlovePart2Accessor != null && _isWhiteGlovePart2Accessor()) + else if (_session.IsWhiteGlovePart2) wgPart = 2; else wgPart = null; @@ -492,7 +394,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Termination private int TryGetIgnoredCount() { - try { return _ignoredCountAccessor(); } + try { return _appTracking.IgnoredCount; } catch (Exception ex) { _logger.Warning($"EnrollmentTerminationHandler: ignoredCount accessor threw: {ex.Message}"); @@ -525,20 +427,18 @@ namespace AutopilotMonitor.Agent.V2.Core.Termination /// deadline; the agent has stopped monitoring, not the ESP). /// /// - /// Best-effort: any accessor failure is logged and swallowed. Skipped silently when - /// the host did not wire (tests). + /// Best-effort: any accessor failure is logged and swallowed. /// /// private void MaybePromoteActiveInstallsAsStuck(DecisionState state, EnrollmentTerminatedEventArgs args) { - if (_promoteActiveInstallsToStuck == null) return; if (!ShouldPromoteActiveInstallsAsStuck(state, args)) return; try { var timeoutMinutes = state.ScenarioObservations?.EspSyncFailureTimeoutMinutes?.Value; EspTerminalFailureSnapshot failureContext = null; - try { failureContext = _lastEspTerminalFailureAccessor?.Invoke(); } + try { failureContext = _appTracking.LastEspTerminalFailure; } catch (Exception ex) { _logger.Warning($"EnrollmentTerminationHandler: lastEspTerminalFailureAccessor threw: {ex.Message}"); } // Codex review (P3, 2026-05-28): only let the HRESULT drive the per-app @@ -555,7 +455,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Termination var (failureType, message) = AppFailureTypes.ClassifyEspAppsFailure(effectiveErrorCode, timeoutMinutes); - var promoted = _promoteActiveInstallsToStuck(failureType, message, effectiveErrorCode); + var promoted = _appTracking.PromoteActiveInstallsToStuck(failureType, message, effectiveErrorCode); var count = promoted?.Count ?? 0; if (count > 0) { @@ -747,7 +647,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Termination try { - _triggerReboot(delay); + _shutdownGate.TriggerReboot(delay); _logger.Info($"EnrollmentTerminationHandler: standalone reboot queued via shutdown.exe /r /t {delay}."); } catch (Exception ex) @@ -797,14 +697,13 @@ namespace AutopilotMonitor.Agent.V2.Core.Termination // stage) and for diagnostics to show how long the agent ran. // // Shutdown-gap closure (2026-05-15): participates in the cross-path idempotency gate - // owned by AgentRuntimeHost so a Terminated event that races a Ctrl+C / ProcessExit - // does not produce two agent_shutting_down events on the wire. When the gate - // is not wired (tests / standalone construction), the legacy always-emit behaviour - // is preserved. + // owned by AgentRuntimeHost (via ) + // so a Terminated event that races a Ctrl+C / ProcessExit does not produce two + // agent_shutting_down events on the wire. // private void EmitAgentShuttingDown(EnrollmentTerminatedEventArgs args) { - if (_tryClaimShutdownEvent != null && !_tryClaimShutdownEvent()) + if (!_shutdownGate.TryClaimShutdownEvent()) { _logger.Debug("EnrollmentTerminationHandler: agent_shutting_down already emitted via gap-path — skipping duplicate."); return; @@ -864,19 +763,18 @@ namespace AutopilotMonitor.Agent.V2.Core.Termination } /// - /// Option 2 helper — writes the clean-exit.marker early via the host-supplied - /// hook so the next agent run classifies previousExit=clean instead of - /// reboot_kill, even when Windows kills the process between - /// and the legacy AppDomain.ProcessExit handler - /// (typical race in admin-triggered reseal-reboots). No-op when the host did not - /// wire the accessor (legacy call sites + tests). + /// Option 2 helper — writes the clean-exit.marker early via + /// so the next agent run classifies + /// previousExit=clean instead of reboot_kill, even when Windows kills + /// the process between and the legacy + /// AppDomain.ProcessExit handler (typical race in admin-triggered + /// reseal-reboots). /// private void TryWriteCleanExitMarker() { - if (_writeCleanExitMarker == null) return; try { - _writeCleanExitMarker(); + _shutdownGate.WriteCleanExitMarker(); _logger.Debug("EnrollmentTerminationHandler: clean-exit marker written (early)."); } catch (Exception ex) @@ -920,32 +818,33 @@ namespace AutopilotMonitor.Agent.V2.Core.Termination // Phase B — wait for the spool to acknowledge upload to the backend (the // pre-Codex-Finding-2 behaviour, kept). // - // When neither accessor is wired (tests / paranoid fallback), we keep V1 + // When neither surface is observable (tests / paranoid fallback), we keep V1 // parity and just sleep the bounded period. - if (_spoolDrainPeriod <= TimeSpan.Zero) return; + var drainPeriod = _drain.SpoolDrainPeriod; + if (drainPeriod <= TimeSpan.Zero) return; - if (_ingressPendingSignalCountAccessor == null && _pendingItemCountAccessor == null) + if (!_drain.CanObserveIngress && !_drain.CanObserveSpool) { - try { Task.Delay(_spoolDrainPeriod).Wait(); } + try { Task.Delay(drainPeriod).Wait(); } catch { /* best-effort */ } return; } - var deadline = DateTime.UtcNow + _spoolDrainPeriod; + var deadline = DateTime.UtcNow + drainPeriod; - if (_ingressPendingSignalCountAccessor != null) + if (_drain.CanObserveIngress) { WaitFor( label: "ingress", - pollAccessor: () => _ingressPendingSignalCountAccessor(), + pollAccessor: () => _drain.IngressPendingSignalCount, deadline: deadline); } - if (_pendingItemCountAccessor != null) + if (_drain.CanObserveSpool) { WaitFor( label: "spool", - pollAccessor: () => (long)_pendingItemCountAccessor(), + pollAccessor: () => (long)_drain.SpoolPendingItemCount, deadline: deadline); } } @@ -1002,17 +901,5 @@ namespace AutopilotMonitor.Agent.V2.Core.Termination try { _post.Emit(evt); } catch (Exception ex) { _logger.Debug($"EnrollmentTerminationHandler: event emission '{evt?.EventType}' threw: {ex.Message}"); } } - - private static void DefaultTriggerReboot(int delaySeconds) - { - var psi = new ProcessStartInfo - { - FileName = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.System), "shutdown.exe"), - Arguments = $"/r /t {delaySeconds} /c \"Autopilot enrollment completed - rebooting\"", - UseShellExecute = false, - CreateNoWindow = true, - }; - Process.Start(psi); - } } } diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/IAppTrackingReadModel.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/IAppTrackingReadModel.cs new file mode 100644 index 00000000..f534423a --- /dev/null +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/IAppTrackingReadModel.cs @@ -0,0 +1,77 @@ +#nullable enable +using System.Collections.Generic; +using AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.Ime; +using AutopilotMonitor.DecisionCore.State; + +namespace AutopilotMonitor.Agent.V2.Core.Termination +{ + /// + /// Read-only view over the enrollment + app-tracking state the + /// needs at terminal time (ARCH-F2 grouping — + /// replaces six loose Func<> constructor parameters). Production implementation: + /// ; tests supply a small fake over their rig + /// state. All members are read lazily inside Handle — never cached at construction — + /// because the collector surfaces only exist after EnrollmentOrchestrator.Start. + /// + public interface IAppTrackingReadModel + { + /// + /// Current snapshot. May throw before the orchestrator has + /// started — the handler guards every read with try/catch. + /// + DecisionState CurrentState { get; } + + /// + /// F5 (debrief 7dd4e593) — deduped union of phase-snapshotted apps + live package + /// states, so the termination summary can iterate the union of phase-snapshotted apps + /// + live _packageStates (V2's clear-on-phase-transition would otherwise drop + /// the DeviceSetup apps from app_tracking_summary and the SummaryDialog). May return + /// null when no IME surface exists. + /// + IReadOnlyList? PackageStates { get; } + + /// + /// Plan §5 Fix 4 — per-app install-lifecycle timings for FinalStatusBuilder + + /// app_tracking_summary emission. May return null (handler substitutes an empty map). + /// + IReadOnlyDictionary? AppTimings { get; } + + /// + /// V1-parity field: count of IME apps the tracker has marked as "ignored" (e.g. + /// uninstall intents that don't surface in the install pipeline). Lives on the live + /// AppPackageStateList only — the deduped phase-snapshot union in + /// doesn't carry it. + /// + int IgnoredCount { get; } + + /// + /// c117946b debrief (2026-05-12): on terminal ESP-Apps failure, promote any apps the + /// agent observed in Installing to Error so the user sees a name (not just an + /// opaque "installing: 1" counter) and the app_install_failed event carries the + /// canonical failureType. Invoked ONLY when the discriminator in + /// ShouldPromoteActiveInstallsAsStuck matches — every other terminal path + /// leaves app states untouched. Returns the list of promoted appIds for logging. + /// + /// Session 080edee9 follow-up (2026-05-28): carries the + /// HRESULT from the failed Apps subcategory (e.g. 0x87D1041C) when available, + /// so the promotion can stamp AppPackageState.ErrorCode + emit an enriched + /// app_install_failed event. Null = no HRESULT observed → fallback to the + /// legacy "esp_apps_timeout" classification. + /// + /// + IReadOnlyList PromoteActiveInstallsToStuck(string failureType, string message, string? errorCode); + + /// + /// Session 080edee9 follow-up + Codex review (P2/P3, 2026-05-28): latest ESP failure + /// context (HRESULT + failedSubcategory + category) observed by + /// ProvisioningStatusTracker. Read once during + /// MaybePromoteActiveInstallsAsStuck. The classifier first checks + /// snapshot.IsAppsSubcategory — only then does the HRESULT drive the + /// failureType. A non-Apps ESP failure (DevicePreparation/*, + /// DeviceSetup/SecurityPolicies, AccountSetup/CertificatesAccountSetup) falls through + /// to the generic esp_apps_timeout wording so a non-app HRESULT cannot + /// mis-classify in-flight installs. Null = no HRESULT context → fallback. + /// + EspTerminalFailureSnapshot? LastEspTerminalFailure { get; } + } +} diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/IDrainStatus.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/IDrainStatus.cs new file mode 100644 index 00000000..a2f39434 --- /dev/null +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/IDrainStatus.cs @@ -0,0 +1,58 @@ +#nullable enable +using System; + +namespace AutopilotMonitor.Agent.V2.Core.Termination +{ + /// + /// Drain observability for (ARCH-F2 + /// grouping — replaces the pendingItemCountAccessor / + /// ingressPendingSignalCountAccessor / spoolDrainPeriod constructor + /// parameters). Production implementation: . + /// + /// The two CanObserve* capability flags replace the old "accessor is null" wiring + /// variant: when neither surface is observable the handler keeps the legacy V1-parity + /// blind delay for the full instead of polling. + /// + /// + public interface IDrainStatus + { + /// + /// Bounded budget shared by the two drain phases (ingress, then spool). V1 parity: + /// production uses 10 s (up to 20 × 500 ms drains before shutdown.exe); + /// disables draining entirely (test default). + /// + TimeSpan SpoolDrainPeriod { get; } + + /// + /// Codex Finding 2 (2026-04-30): the termination handler is dispatched off the ingress + /// worker, so it can wait for the worker to actually process the lifecycle events the + /// handler posts (agent_shutting_down, whiteglove_part1_complete, analyzer events) + /// BEFORE polling the spool — without this the spool-empty check would trivially fire + /// on already-uploaded items while the just-posted events are still in the ingress + /// channel. false = ingress not observable → phase skipped. + /// + bool CanObserveIngress { get; } + + /// + /// Signals accepted by the ingress but not yet fully processed. Only read when + /// is true; a throwing getter degrades to a bounded + /// sleep inside the handler's poll loop. + /// + long IngressPendingSignalCount { get; } + + /// + /// Option 1 (WG Part 1 graceful-exit hardening, 2026-04-30): when observable, the + /// termination handler polls the spool's pending count during DrainSpool and + /// exits the wait as soon as it returns 0 (spool fully acknowledged by the backend), + /// falling back to the bounded timeout if the spool + /// never drains. false = legacy blind-delay behaviour. + /// + bool CanObserveSpool { get; } + + /// + /// Spool items not yet acknowledged by the backend. Only read when + /// is true. + /// + int SpoolPendingItemCount { get; } + } +} diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/IShutdownGate.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/IShutdownGate.cs new file mode 100644 index 00000000..0260445a --- /dev/null +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/IShutdownGate.cs @@ -0,0 +1,48 @@ +#nullable enable +namespace AutopilotMonitor.Agent.V2.Core.Termination +{ + /// + /// Host-owned shutdown actions invoked by the + /// (ARCH-F2 grouping — replaces the signalShutdown / triggerReboot / + /// writeCleanExitMarker / tryClaimShutdownEvent constructor parameters). + /// Production implementation lives in the runtime host project + /// (RuntimeShutdownGate) because every member touches host-process machinery + /// (the shutdown ManualResetEventSlim, Program.WriteCleanExitMarker, the + /// cross-path agent_shutting_down idempotency gate). + /// + public interface IShutdownGate + { + /// + /// Releases the host's shutdown wait — the handler's very last act (invoked in the + /// finally of Handle, best-effort, never allowed to prevent shutdown). + /// + void SignalShutdown(); + + /// + /// V1 parity — standalone-reboot flow (RebootOnComplete without self-destruct): + /// queue shutdown.exe /r /t <delaySeconds> so the user gets a visible + /// countdown. Tests record the call instead of rebooting the build machine. + /// + void TriggerReboot(int delaySeconds); + + /// + /// Option 2 (WG Part 1 graceful-exit hardening, 2026-04-30): writes the + /// clean-exit.marker file directly, before hands + /// control to the main thread. This wins the race against an admin-triggered + /// reseal-reboot — without this hook the marker is only written by the + /// AppDomain.ProcessExit handler, which Windows can pre-empt. + /// + void WriteCleanExitMarker(); + + /// + /// Shutdown-gap closure (2026-05-15): cross-path idempotency gate shared with the + /// runtime host's gap emitters (Ctrl+C, ProcessExit, unhandled exception, + /// runtime-host finally). The handler calls this before emitting + /// agent_shutting_down so a Terminated event that races a Ctrl+C cannot + /// produce two events on the wire. Returns false when another path already + /// claimed the slot (the handler then skips its emit). Implementations without a + /// shared gate return true (always-emit). + /// + bool TryClaimShutdownEvent(); + } +} diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/OrchestratorAppTrackingReadModel.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/OrchestratorAppTrackingReadModel.cs new file mode 100644 index 00000000..79426297 --- /dev/null +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/OrchestratorAppTrackingReadModel.cs @@ -0,0 +1,43 @@ +#nullable enable +using System; +using System.Collections.Generic; +using AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.Ime; +using AutopilotMonitor.Agent.V2.Core.Orchestration; +using AutopilotMonitor.DecisionCore.State; + +namespace AutopilotMonitor.Agent.V2.Core.Termination +{ + /// + /// Production over the live + /// : decision state from the processor, app-tracking + /// surfaces from (ARCH-F4). All + /// members delegate lazily — the surfaces only exist after Start step 14, while the + /// termination handler is constructed earlier inside the onIngressReady hook. + /// + public sealed class OrchestratorAppTrackingReadModel : IAppTrackingReadModel + { + private readonly EnrollmentOrchestrator _orchestrator; + + public OrchestratorAppTrackingReadModel(EnrollmentOrchestrator orchestrator) + { + _orchestrator = orchestrator ?? throw new ArgumentNullException(nameof(orchestrator)); + } + + public DecisionState CurrentState => _orchestrator.CurrentState; + + public IReadOnlyList? PackageStates => + _orchestrator.CollectorSurfaces?.AllKnownPackageStates; + + public IReadOnlyDictionary? AppTimings => + _orchestrator.CollectorSurfaces?.ImeAppTimings; + + public int IgnoredCount => _orchestrator.CollectorSurfaces?.ImeIgnoredCount ?? 0; + + public IReadOnlyList PromoteActiveInstallsToStuck(string failureType, string message, string? errorCode) => + _orchestrator.CollectorSurfaces?.PromoteActiveInstallsToStuck(failureType, message, errorCode) + ?? Array.Empty(); + + public EspTerminalFailureSnapshot? LastEspTerminalFailure => + _orchestrator.CollectorSurfaces?.LastEspTerminalFailure; + } +} diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/OrchestratorDrainStatus.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/OrchestratorDrainStatus.cs new file mode 100644 index 00000000..11d7a3ef --- /dev/null +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/OrchestratorDrainStatus.cs @@ -0,0 +1,35 @@ +#nullable enable +using System; +using AutopilotMonitor.Agent.V2.Core.Orchestration; + +namespace AutopilotMonitor.Agent.V2.Core.Termination +{ + /// + /// Production over the live . + /// Both drain surfaces are observable in production; the drain budget is the V1-parity + /// default (the old optional spoolDrainPeriod constructor parameter was never set + /// by the runtime host). + /// + public sealed class OrchestratorDrainStatus : IDrainStatus + { + /// V1 parity: up to 20 × 500 ms drains before shutdown.exe. + private static readonly TimeSpan DefaultSpoolDrain = TimeSpan.FromMilliseconds(10000); + + private readonly EnrollmentOrchestrator _orchestrator; + + public OrchestratorDrainStatus(EnrollmentOrchestrator orchestrator) + { + _orchestrator = orchestrator ?? throw new ArgumentNullException(nameof(orchestrator)); + } + + public TimeSpan SpoolDrainPeriod => DefaultSpoolDrain; + + public bool CanObserveIngress => true; + + public long IngressPendingSignalCount => _orchestrator.IngressPendingSignalCount; + + public bool CanObserveSpool => true; + + public int SpoolPendingItemCount => _orchestrator.PendingItemCount; + } +} diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/TerminationSessionContext.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/TerminationSessionContext.cs new file mode 100644 index 00000000..1292f895 --- /dev/null +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/TerminationSessionContext.cs @@ -0,0 +1,70 @@ +#nullable enable +using System; +using AutopilotMonitor.Agent.V2.Core.Configuration; +using AutopilotMonitor.Agent.V2.Core.Security; + +namespace AutopilotMonitor.Agent.V2.Core.Termination +{ + /// + /// Immutable session facts the needs at + /// terminal time (ARCH-F2 grouping — replaces seven loose constructor parameters). + /// A concrete class, not an interface: it is pure data with no behavioral seam, so + /// production and tests construct the same type and there is no fake-vs-prod divergence. + /// + public sealed class TerminationSessionContext + { + private readonly Func? _isWhiteGlovePart2; + + public AgentConfiguration Configuration { get; } + + /// State directory for final-status.json + enrollment-complete.marker. + public string StateDirectory { get; } + + /// Used for the uptimeMinutes field on agent_shutting_down. + public DateTime AgentStartTimeUtc { get; } + + /// Normalized to when unknown. + public string AgentVersion { get; } + + /// + /// Writes the whiteglove.complete marker on WG Part-1 exit so Part-2 resume is + /// detected on the next boot. Nullable — the handler warns and skips when absent. + /// + public SessionIdPersistence? SessionPersistence { get; } + + /// Test override for the SummaryDialog exe path; null in production. + public string? DialogExePathOverride { get; } + + /// + /// V1-symmetric Part-2 hint (plan §11). A Part-2 resume runs as a fresh Classic + /// enrollment after Archive-and-Reset, so the terminal Stage is + /// Completed/Failed like any first-run completion. The shutdown analyzer + /// pipeline still needs to know it was a Part-2 run so SoftwareInventoryAnalyzer can + /// tag findings with phase=2 and the backend's vulnerability-correlation pipeline can + /// filter Part-2 inventory out of the Part-1 set. Evaluated lazily at terminal time + /// (production wires () => orchestrator.IsWhiteGlovePart2); defaults to + /// false when no accessor was supplied. + /// + public bool IsWhiteGlovePart2 => _isWhiteGlovePart2?.Invoke() ?? false; + + public TerminationSessionContext( + AgentConfiguration configuration, + string stateDirectory, + DateTime agentStartTimeUtc, + string? agentVersion = null, + SessionIdPersistence? sessionPersistence = null, + string? dialogExePathOverride = null, + Func? isWhiteGlovePart2 = null) + { + Configuration = configuration ?? throw new ArgumentNullException(nameof(configuration)); + StateDirectory = string.IsNullOrEmpty(stateDirectory) + ? throw new ArgumentNullException(nameof(stateDirectory)) + : stateDirectory; + AgentStartTimeUtc = agentStartTimeUtc; + AgentVersion = agentVersion ?? string.Empty; + SessionPersistence = sessionPersistence; + DialogExePathOverride = dialogExePathOverride; + _isWhiteGlovePart2 = isWhiteGlovePart2; + } + } +} diff --git a/src/Agent/AutopilotMonitor.Agent.V2/Runtime/AgentRuntimeHost.cs b/src/Agent/AutopilotMonitor.Agent.V2/Runtime/AgentRuntimeHost.cs index 9e39a8b1..60751502 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2/Runtime/AgentRuntimeHost.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2/Runtime/AgentRuntimeHost.cs @@ -333,88 +333,37 @@ namespace AutopilotMonitor.Agent.V2.Runtime // emits through the same InformationalEventPost. The terminated- // dispatch wrapper registered above picks this instance up via // closure capture. + // + // ARCH-F2: the former ~80-line lambda wiring is grouped into four + // adapter objects. The per-parameter rationale comments live on + // the interface members (IAppTrackingReadModel / IDrainStatus / + // IShutdownGate / TerminationSessionContext). terminationHandler = new EnrollmentTerminationHandler( - configuration: agentConfig, + session: new TerminationSessionContext( + configuration: agentConfig, + stateDirectory: stateSubdir, + agentStartTimeUtc: agentStartTimeUtc, + agentVersion: agentVersion, + sessionPersistence: sessionPersistence, + // Read lazily — the orchestrator resolves the Part-2 hint + // during Start, after this handler is constructed. + isWhiteGlovePart2: () => orchestrator.IsWhiteGlovePart2), + appTracking: new OrchestratorAppTrackingReadModel(orchestrator), + drainStatus: new OrchestratorDrainStatus(orchestrator), + shutdownGate: new RuntimeShutdownGate( + signalShutdown: () => shutdown.Set(), + writeCleanExitMarker: () => Program.WriteCleanExitMarker(dataDirectory), + tryClaimShutdownEvent: TryClaimAgentShuttingDownEmit), logger: logger, - stateDirectory: stateSubdir, - agentStartTimeUtc: agentStartTimeUtc, - currentStateAccessor: () => orchestrator.CurrentState, - // F5 (debrief 7dd4e593) — pass the deduped phase-snapshot+live - // union so DeviceSetup apps cleared from _packageStates on the - // AccountSetup transition still appear in the SummaryDialog and - // app_tracking_summary event. - packageStatesAccessor: () => orchestrator.CollectorSurfaces?.AllKnownPackageStates, cleanupServiceFactory: bootstrap.CleanupServiceFactory, uploadDiagnosticsAsync: uploadDiagnosticsAsync, - signalShutdown: () => shutdown.Set(), analyzerManager: analyzerManager, post: lifecyclePost, - sessionPersistence: sessionPersistence, - // Plan §5 Fix 4 — per-app timing snapshot for FinalStatusBuilder + - // app_tracking_summary emission. Null-safe via the handler's default. - appTimingsAccessor: () => orchestrator.CollectorSurfaces?.ImeAppTimings, - agentVersion: agentVersion, // Stop periodic collectors before the diagnostics ZIP is // built, so no late `performance_snapshot` slips in after // `diagnostics_collecting`. Idempotent on the orchestrator // side — the full Stop() call later is a no-op for hosts. - stopPeripheralCollectors: () => orchestrator.StopCollectorHosts(), - // V1-parity ignoredCount for app_tracking_summary — lives on the - // live AppPackageStateList only (phase snapshots don't carry it). - ignoredCountAccessor: () => orchestrator.CollectorSurfaces?.ImeIgnoredCount ?? 0, - // Option 1 (WG Part 1 graceful-exit hardening, 2026-04-30) — - // active spool-empty polling so DrainSpool exits as soon as the - // backend ack'd the last lifecycle event instead of waiting the - // full 10s timeout. Critical when an admin reseal-reboot races - // termination. - pendingItemCountAccessor: () => orchestrator.PendingItemCount, - // Option 2 (same hardening) — write clean-exit.marker before - // _signalShutdown returns, instead of relying solely on - // AppDomain.ProcessExit which Windows can pre-empt during a - // shutdown.exe / reseal-triggered reboot. - writeCleanExitMarker: () => Program.WriteCleanExitMarker(dataDirectory), - // Codex Finding 2 (2026-04-30) — DrainSpool needs to wait - // for the ingress to finish processing the lifecycle events - // the handler just posted (agent_shutting_down, - // whiteglove_part1_complete, analyzer events) BEFORE polling - // spool-empty. Pair with off-worker dispatch in - // EnrollmentOrchestrator.OnDecisionTerminalStage so the - // wait can actually make progress. - ingressPendingSignalCountAccessor: () => orchestrator.IngressPendingSignalCount, - // V1-symmetric Part-2 hint accessor (plan §11). The orchestrator - // does not reach a dedicated terminal stage for Part 2; instead - // Part 2 runs as a fresh Classic enrollment after Archive-and-Reset - // and ends on Completed/Failed. The shutdown analyzer pipeline - // still needs the hint to tag findings with phase=2 so the backend - // vulnerability correlation pipeline can filter Part-2 inventory - // out of the Part-1 set. - isWhiteGlovePart2Accessor: () => orchestrator.IsWhiteGlovePart2, - // c117646b debrief (2026-05-12) — on terminal ESP-Apps failure, - // promote any apps in `Installing` to Error with the canonical - // `esp_apps_timeout` failureType so the user sees a name + a - // hedged "likely stuck" label, not just an opaque `installing: 1` - // counter. Discriminator inside the handler gates this to the - // EspTerminalFailure pathway only — other Failed paths leave the - // app list untouched. - promoteActiveInstallsToStuck: (failureType, message, errorCode) - => orchestrator.CollectorSurfaces?.PromoteActiveInstallsToStuck(failureType, message, errorCode) - ?? Array.Empty(), - // Shutdown-gap closure (2026-05-15) — share the cross-path - // idempotency gate so a Terminated event that races a Ctrl+C / - // ProcessExit gap-path does not emit two agent_shutting_down - // events on the wire. The handler skips its emit when the - // gate already claimed the slot. - tryClaimShutdownEvent: TryClaimAgentShuttingDownEmit, - // Session 080edee9 follow-up + Codex review (P2/P3, 2026-05-28) - // — feeds the latest ESP failure context (HRESULT + - // failedSubcategory + category) captured by - // ProvisioningStatusTracker (via - // EspAndHelloTracker.LastEspTerminalFailure) into the - // termination handler's classifier. The handler only treats - // the HRESULT as an app-level cause when the failure came - // from the Apps subcategory; non-Apps ESP failures fall - // back to the generic esp_apps_timeout classification. - lastEspTerminalFailureAccessor: () => orchestrator.CollectorSurfaces?.LastEspTerminalFailure); + stopPeripheralCollectors: () => orchestrator.StopCollectorHosts()); // ServerActionDispatcher (plan §5.3) — constructed inside this // hook so lifecyclePost + terminationHandler are guaranteed diff --git a/src/Agent/AutopilotMonitor.Agent.V2/Runtime/RuntimeShutdownGate.cs b/src/Agent/AutopilotMonitor.Agent.V2/Runtime/RuntimeShutdownGate.cs new file mode 100644 index 00000000..aa919c45 --- /dev/null +++ b/src/Agent/AutopilotMonitor.Agent.V2/Runtime/RuntimeShutdownGate.cs @@ -0,0 +1,54 @@ +using System; +using System.Diagnostics; +using System.IO; +using AutopilotMonitor.Agent.V2.Core.Termination; + +namespace AutopilotMonitor.Agent.V2.Runtime +{ + /// + /// Production for (ARCH-F2). + /// Wraps the host-process shutdown machinery that only exists as locals inside + /// AgentRuntimeHost.Run: the shutdown ManualResetEventSlim, the early + /// clean-exit.marker write (Program.WriteCleanExitMarker), and the + /// cross-path agent_shutting_down idempotency gate shared with the Ctrl+C / + /// ProcessExit / unhandled-exception gap emitters. + /// + internal sealed class RuntimeShutdownGate : IShutdownGate + { + private readonly Action _signalShutdown; + private readonly Action _writeCleanExitMarker; + private readonly Func _tryClaimShutdownEvent; + + public RuntimeShutdownGate( + Action signalShutdown, + Action writeCleanExitMarker, + Func tryClaimShutdownEvent) + { + _signalShutdown = signalShutdown ?? throw new ArgumentNullException(nameof(signalShutdown)); + _writeCleanExitMarker = writeCleanExitMarker ?? throw new ArgumentNullException(nameof(writeCleanExitMarker)); + _tryClaimShutdownEvent = tryClaimShutdownEvent ?? throw new ArgumentNullException(nameof(tryClaimShutdownEvent)); + } + + public void SignalShutdown() => _signalShutdown(); + + public void WriteCleanExitMarker() => _writeCleanExitMarker(); + + public bool TryClaimShutdownEvent() => _tryClaimShutdownEvent(); + + /// + /// V1 parity — queues shutdown.exe /r /t <delaySeconds> with a visible + /// countdown message. (Formerly EnrollmentTerminationHandler.DefaultTriggerReboot.) + /// + public void TriggerReboot(int delaySeconds) + { + var psi = new ProcessStartInfo + { + FileName = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.System), "shutdown.exe"), + Arguments = $"/r /t {delaySeconds} /c \"Autopilot enrollment completed - rebooting\"", + UseShellExecute = false, + CreateNoWindow = true, + }; + Process.Start(psi); + } + } +} diff --git a/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt b/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt index 0eb5aea3..9a9755e4 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt +++ b/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt @@ -1 +1 @@ -1047 +1058