diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Integration/ShellCoreToEngineEspExitingTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Integration/ShellCoreToEngineEspExitingTests.cs index 9177b624..a09423db 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Integration/ShellCoreToEngineEspExitingTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Integration/ShellCoreToEngineEspExitingTests.cs @@ -16,17 +16,16 @@ using Xunit; namespace AutopilotMonitor.Agent.V2.Core.Tests.Integration { /// - /// Plan §EspExiting Adapter — wire-through verification on TWO topologies: - /// - /// Single-tracker wiring: ShellCoreTracker.EspExited → - /// ShellCoreTrackerAdapter → ingress (test-only scenario). - /// Production wiring: ShellCoreTracker.EspExited → inner re-raise on - /// EspAndHelloTracker.EspExited → EspAndHelloTrackerAdapter → ingress. - /// This is the path the live V2-Host uses; without the coordinator forward the - /// new EspExiting signal would never reach the engine in production. - /// - /// Both paths end with a reducer step that arms the HelloSafety deadline when - /// AccountSetup is already entered. + /// Plan §EspExiting Adapter — wire-through verification of the production EspExiting path: + /// ShellCoreTracker.HandleBackfillRecord → EspExited event → inner re-raise on + /// EspAndHelloTracker.EspExited → EspAndHelloTrackerAdapter → ingress → engine + /// reducer. This is the path the live V2-Host uses; without the coordinator forward the + /// EspExiting signal would never reach the engine in production. + /// + /// The first test isolates the tracker's EspExited contract (source-event timestamp + /// preserved through backfill); the coordinator tests drive the full production chain through + /// to the HelloSafety deadline when AccountSetup is already entered. + /// /// public sealed class ShellCoreToEngineEspExitingTests { @@ -35,12 +34,15 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Integration "CommercialOOBE_ESPProgress_Page_Exiting"; // matches EspExitingPattern in ShellCoreTracker [Fact] - public void Tracker_backfill_record_drives_adapter_to_post_EspExiting_signal() + public void Tracker_backfill_record_raises_EspExited_with_source_timestamp() { + // Tracker-level contract: HandleBackfillRecord raises EspExited carrying the source + // event time (not Clock.UtcNow). The EspExited→EspExiting signal mapping and SourceOrigin + // are covered by the coordinator tests below; here we isolate the backfill→event link + // that the production coordinator depends on. using var tmp = new TempDirectory(); var logger = new AgentLogger(tmp.Path, AgentLogLevel.Info); var clock = new VirtualClock(ClockNow); - var ingress = new FakeSignalIngressSink(); var trackerPost = new InformationalEventPost(new FakeSignalIngressSink(), clock); using var tracker = new ShellCoreTracker( sessionId: "S1", @@ -48,77 +50,17 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Integration post: trackerPost, logger: logger, helloTracker: null); - using var adapter = new ShellCoreTrackerAdapter(tracker, ingress, clock); - // Source time IS in the past relative to the clock — verify the adapter forwards - // the historical timestamp from the EventArgs rather than collapsing to Clock.UtcNow. + EspExitedEventArgs captured = null; + tracker.EspExited += (_, args) => captured = args; + + // Source time IS in the past relative to the clock — verify the tracker carries the + // historical timestamp on the event rather than collapsing to Clock.UtcNow. var sourceTime = ClockNow.AddMinutes(-2); tracker.HandleBackfillRecord(EspExitingDescription, sourceTime); - // One EspExiting on the ingress with the source timestamp preserved. - Assert.Contains(ingress.Posted, p => p.Kind == DecisionSignalKind.EspExiting); - var espExiting = ingress.Posted.FirstOrDefault(p => p.Kind == DecisionSignalKind.EspExiting)!; - Assert.Equal("ShellCoreTracker", espExiting.SourceOrigin); - Assert.Equal(sourceTime, espExiting.OccurredAtUtc); - } - - [Fact] - public void Tracker_backfill_then_reducer_step_arms_HelloSafety_when_AccountSetup_observed() - { - // End-to-end: real tracker event → adapter signal → engine reducer → - // HelloSafety deadline. Builds the seed state with AccountSetup pre-entered so the - // reducer's ShouldTransitionToAwaitingHello guard unlocks on the EspExiting forward. - using var tmp = new TempDirectory(); - var logger = new AgentLogger(tmp.Path, AgentLogLevel.Info); - var clock = new VirtualClock(ClockNow); - var ingress = new FakeSignalIngressSink(); - var trackerPost = new InformationalEventPost(new FakeSignalIngressSink(), clock); - using var tracker = new ShellCoreTracker( - sessionId: "S1", - tenantId: "T1", - post: trackerPost, - logger: logger, - helloTracker: null); - using var adapter = new ShellCoreTrackerAdapter(tracker, ingress, clock); - - var sourceTime = ClockNow.AddMinutes(-1); - tracker.HandleBackfillRecord(EspExitingDescription, sourceTime); - - var espExiting = ingress.Posted.FirstOrDefault(p => p.Kind == DecisionSignalKind.EspExiting); - Assert.NotNull(espExiting); - - // Build a Classic state already at AccountSetup AND with the strong post-AccountSetup - // gate satisfied (session 330f73f3 fix) so the EspExiting guard unlocks. Pre-fix this - // worked with only AccountSetupEnteredUtc; the new gate requires AccountSetupProvisioningSucceededUtc. - var engine = new DecisionEngine(); - var seed = DecisionState.CreateInitial("S1", "T1", agentBootUtc: sourceTime.AddMinutes(-5)) - .ToBuilder() - .WithStage(SessionStage.EspAccountSetup) - .WithStepIndex(2) - .WithLastAppliedSignalOrdinal(1); - seed.AccountSetupEnteredUtc = new SignalFact(sourceTime.AddMinutes(-2), 1); - seed.AccountSetupProvisioningSucceededUtc = new SignalFact(sourceTime.AddMinutes(-1).AddSeconds(-30), 1); - var state = seed.Build(); - - // Re-wrap the adapter-posted signal with the ordinal the engine expects. - var enrichedSignal = new DecisionSignal( - sessionSignalOrdinal: 2, - sessionTraceOrdinal: 2, - kind: espExiting!.Kind, - kindSchemaVersion: espExiting.KindSchemaVersion, - occurredAtUtc: espExiting.OccurredAtUtc, - sourceOrigin: espExiting.SourceOrigin, - evidence: espExiting.Evidence, - payload: espExiting.Payload); - - var step = engine.Reduce(state, enrichedSignal); - - Assert.Equal(SessionStage.AwaitingHello, step.NewState.Stage); - var helloSafety = Assert.Single(step.NewState.Deadlines, d => d.Name == DeadlineNames.HelloSafety); - Assert.Equal(sourceTime.AddSeconds(300), helloSafety.DueAtUtc); - Assert.Contains(step.Effects, - e => e.Kind == DecisionEffectKind.ScheduleDeadline - && e.Deadline?.Name == DeadlineNames.HelloSafety); + Assert.NotNull(captured); + Assert.Equal(sourceTime, captured!.OccurredAtUtc); } // -------------------------------------------- Coordinator-pfad (production wiring) -- diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/HelloTrackerPolicyDetectionTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/HelloTrackerPolicyDetectionTests.cs index 29d43b16..abc3bf10 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/HelloTrackerPolicyDetectionTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/HelloTrackerPolicyDetectionTests.cs @@ -113,6 +113,85 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Monitoring Assert.Null(f.Tracker.HelloOutcome); } + // ---- MON-C2: policy-not-yet-detected must not force premature not_configured ---- + + [Fact] + public void Wait_timeout_with_unknown_policy_grants_grace_and_defers_completion() + { + // Policy never detected (slow MDM/CSP sync). The first wait timeout must NOT + // resolve Hello to not_configured — it grants one bounded grace re-arm and keeps + // waiting so a late policy read can still land. + using var f = new Fixture(); + // Deliberately skip SetPolicyForTest so _isPolicyConfigured stays false (unknown). + f.Tracker.StartHelloWaitTimer(); + + f.Tracker.TriggerWaitTimeoutForTest(); + + Assert.False(f.Tracker.IsHelloCompleted); + Assert.Null(f.Tracker.HelloOutcome); + // Wait timer re-armed for the grace window; completion timer NOT started. + Assert.True(f.Tracker.IsWaitTimerActiveForTest); + Assert.False(f.Tracker.IsCompletionTimerActiveForTest); + } + + [Fact] + public void Wait_timeout_unknown_policy_after_grace_completes_not_configured() + { + // If the policy is STILL unknown after the one-shot grace window, the device is + // genuinely not configured (DetectHelloPolicy returns null forever) — resolve to + // not_configured so enrollment is not stranded. + using var f = new Fixture(); + f.Tracker.StartHelloWaitTimer(); + + f.Tracker.TriggerWaitTimeoutForTest(); // grace re-arm + Assert.False(f.Tracker.IsHelloCompleted); + + f.Tracker.TriggerWaitTimeoutForTest(); // grace exhausted, still unknown + + Assert.True(f.Tracker.IsHelloCompleted); + Assert.Equal("not_configured", f.Tracker.HelloOutcome); + } + + [Fact] + public void Wait_timeout_unknown_then_policy_enabled_during_grace_extends_wait() + { + // The slow-sync race the grace exists for: policy lands as ENABLED during the grace + // window. The next wait timeout must take the extended-wait path (long completion + // timer), never not_configured. + using var f = new Fixture(); + f.Tracker.StartHelloWaitTimer(); + + f.Tracker.TriggerWaitTimeoutForTest(); // unknown → grace re-arm + + f.Tracker.SetPolicyForTest(helloEnabled: true, source: "CSP/Intune (device-scoped)"); + f.Tracker.TriggerWaitTimeoutForTest(); // now known-enabled → extend + + Assert.False(f.Tracker.IsHelloCompleted); + Assert.Null(f.Tracker.HelloOutcome); + Assert.True(f.Tracker.IsCompletionTimerActiveForTest); + } + + [Fact] + public void Esp_resumption_reset_re_grants_the_unknown_policy_grace() + { + // The one-shot grace flag must reset on ESP resumption (hybrid-join mid-enrollment + // reboot). Without the reset a fresh post-resume wait that hits an undetected policy + // would skip its grace and resolve straight to not_configured. + using var f = new Fixture(); + f.Tracker.StartHelloWaitTimer(); + f.Tracker.TriggerWaitTimeoutForTest(); // unknown → grace consumed + Assert.False(f.Tracker.IsHelloCompleted); + + f.Tracker.ResetForEspResumption(); + + f.Tracker.StartHelloWaitTimer(); + f.Tracker.TriggerWaitTimeoutForTest(); // unknown again → must re-grant grace, not complete + + Assert.False(f.Tracker.IsHelloCompleted); + Assert.Null(f.Tracker.HelloOutcome); + Assert.True(f.Tracker.IsWaitTimerActiveForTest); + } + [Fact] public void HelloTerminalEvent_after_policy_disabled_emits_mismatch_warning() { diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Orchestration/EnrollmentOrchestratorEventBridgeTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Orchestration/EnrollmentOrchestratorEventBridgeTests.cs index 282d7293..a13f41cc 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Orchestration/EnrollmentOrchestratorEventBridgeTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Orchestration/EnrollmentOrchestratorEventBridgeTests.cs @@ -53,23 +53,26 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Orchestration public IReadOnlyCollection? CapturedWhiteGloveSealingPatternIds { get; private set; } public ISignalIngressSink? CapturedIngress { get; private set; } public IClock? CapturedClock { get; private set; } + public AutopilotMonitor.Agent.V2.Core.Transport.Telemetry.ITelemetrySpool? CapturedTelemetrySpool { get; private set; } - public IReadOnlyList CreateCollectorHosts( + public CollectorSurfaces CreateCollectorHosts( string sessionId, string tenantId, AgentLogger logger, IReadOnlyCollection whiteGloveSealingPatternIds, ISignalIngressSink ingress, - IClock clock) + IClock clock, + AutopilotMonitor.Agent.V2.Core.Transport.Telemetry.ITelemetrySpool? telemetrySpool) { CapturedWhiteGloveSealingPatternIds = whiteGloveSealingPatternIds; CapturedIngress = ingress; CapturedClock = clock; + CapturedTelemetrySpool = telemetrySpool; foreach (var name in HostNames) { Hosts.Add(new FakeCollectorHost(name)); } - return Hosts; + return new CollectorSurfaces(Hosts); } } @@ -124,6 +127,9 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Orchestration Assert.NotNull(factory.CapturedIngress); Assert.NotNull(factory.CapturedClock); Assert.Same(rig.Clock, factory.CapturedClock); + // ARCH-F4: the spool flows in as a CreateCollectorHosts parameter (replaces the + // old SetTelemetrySpool late-binding setter + concrete-factory downcast). + Assert.NotNull(factory.CapturedTelemetrySpool); sut.Stop(); } diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Orchestration/EnrollmentOrchestratorWhiteGlovePatternTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Orchestration/EnrollmentOrchestratorWhiteGlovePatternTests.cs index 020742c0..8e54f5eb 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Orchestration/EnrollmentOrchestratorWhiteGlovePatternTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Orchestration/EnrollmentOrchestratorWhiteGlovePatternTests.cs @@ -20,16 +20,17 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Orchestration { public IReadOnlyCollection? CapturedPatternIds { get; private set; } - public IReadOnlyList CreateCollectorHosts( + public CollectorSurfaces CreateCollectorHosts( string sessionId, string tenantId, AgentLogger logger, IReadOnlyCollection whiteGloveSealingPatternIds, ISignalIngressSink ingress, - IClock clock) + IClock clock, + AutopilotMonitor.Agent.V2.Core.Transport.Telemetry.ITelemetrySpool? telemetrySpool) { CapturedPatternIds = whiteGloveSealingPatternIds; - return Array.Empty(); + return new CollectorSurfaces(Array.Empty()); } } diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/SignalAdapters/EspAndHelloTrackerAdapterTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/SignalAdapters/EspAndHelloTrackerAdapterTests.cs index 1033957c..41644290 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/SignalAdapters/EspAndHelloTrackerAdapterTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/SignalAdapters/EspAndHelloTrackerAdapterTests.cs @@ -57,6 +57,22 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.SignalAdapters Assert.Equal("unknown", posted.Payload![SignalPayloadKeys.HelloOutcome]); } + [Theory] + [InlineData("completed")] + [InlineData("skipped")] + [InlineData("timeout")] + [InlineData("not_configured")] + [InlineData("wizard_not_started")] + public void HelloOutcome_propagates_verbatim_to_payload(string outcome) + { + using var f = new Fixture(); + using var adapter = new EspAndHelloTrackerAdapter(f.Coordinator, f.Ingress, f.Clock); + + adapter.TriggerHelloFromTest(outcome); + + Assert.Equal(outcome, f.Ingress.Posted[0].Payload![SignalPayloadKeys.HelloOutcome]); + } + [Fact] public void FinalizingEvent_emits_EspPhaseChanged_with_FinalizingSetup_payload() { @@ -97,6 +113,18 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.SignalAdapters Assert.Contains("merged", posted.Evidence.DerivationInputs!["subSource"]); } + [Fact] + public void EspFailureEvent_empty_type_falls_back_to_unknown() + { + using var f = new Fixture(); + using var adapter = new EspAndHelloTrackerAdapter(f.Coordinator, f.Ingress, f.Clock); + + // Empty failureType is normalized to "unknown" by EspFailureDetectedEventArgs. + adapter.TriggerEspFailureFromTest(""); + + Assert.Equal("unknown", f.Ingress.Posted[0].Payload!["failureType"]); + } + [Fact] public void DeviceSetupCompleteEvent_emits_DeviceSetupProvisioningComplete_with_snapshot_fallback() { diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/SignalAdapters/EspFailureDetectedHResultTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/SignalAdapters/EspFailureDetectedHResultTests.cs index b9226098..67f36a06 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/SignalAdapters/EspFailureDetectedHResultTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/SignalAdapters/EspFailureDetectedHResultTests.cs @@ -104,40 +104,5 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.SignalAdapters } finally { coordinator.Dispose(); } } - - // ------------------------------------------------------------------ ProvisioningStatusTrackerAdapter - - [Fact] - public void Provisioning_adapter_forwards_full_args_to_signal_payload() - { - using var tmp = new TempDirectory(); - var logger = new AgentLogger(tmp.Path, AgentLogLevel.Info); - var clock = new VirtualClock(Fixed); - var ingress = new FakeSignalIngressSink(); - var tracker = new ProvisioningStatusTracker( - sessionId: "S1", - tenantId: "T1", - post: new InformationalEventPost(new FakeSignalIngressSink(), clock), - logger: logger); - - try - { - using var adapter = new ProvisioningStatusTrackerAdapter(tracker, ingress, clock); - - adapter.TriggerEspFailureFromTest(new EspFailureDetectedEventArgs( - failureType: "Provisioning_DeviceSetup_Apps_Failed", - errorCode: "0x87d1041c", - failedSubcategory: "Apps", - category: "DeviceSetup")); - - var posted = Assert.Single(ingress.Posted); - Assert.Equal(DecisionSignalKind.EspTerminalFailure, posted.Kind); - Assert.Equal("Provisioning_DeviceSetup_Apps_Failed", posted.Payload!["failureType"]); - Assert.Equal("0x87d1041c", posted.Payload!["errorCode"]); - Assert.Equal("Apps", posted.Payload!["failedSubcategory"]); - Assert.Equal("DeviceSetup", posted.Payload!["category"]); - } - finally { tracker.Dispose(); } - } } } diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/SignalAdapters/HelloTrackerAdapterTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/SignalAdapters/HelloTrackerAdapterTests.cs deleted file mode 100644 index 44416260..00000000 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/SignalAdapters/HelloTrackerAdapterTests.cs +++ /dev/null @@ -1,112 +0,0 @@ -using System; -using System.Linq; -using AutopilotMonitor.Agent.V2.Core.Logging; -using AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals; -using AutopilotMonitor.Agent.V2.Core.Orchestration; -using AutopilotMonitor.Agent.V2.Core.SignalAdapters; -using AutopilotMonitor.Agent.V2.Core.Tests.Harness; -using AutopilotMonitor.Agent.V2.Core.Tests.Orchestration; -using AutopilotMonitor.DecisionCore.Engine; -using AutopilotMonitor.DecisionCore.Signals; -using Xunit; - -namespace AutopilotMonitor.Agent.V2.Core.Tests.SignalAdapters -{ - public sealed class HelloTrackerAdapterTests - { - private static readonly DateTime Fixed = new DateTime(2026, 4, 20, 10, 0, 0, DateTimeKind.Utc); - - private sealed class Fixture : IDisposable - { - public TempDirectory Tmp { get; } = new TempDirectory(); - public AgentLogger Logger { get; } - public HelloTracker Tracker { get; } - public FakeSignalIngressSink Ingress { get; } = new FakeSignalIngressSink(); - public VirtualClock Clock { get; } = new VirtualClock(Fixed); - - public Fixture() - { - Logger = new AgentLogger(Tmp.Path, AgentLogLevel.Info); - // Separate throwaway ingress for tracker emissions so the adapter's DecisionSignal - // assertions on Ingress.Posted stay unpolluted by InformationalEvent pass-through. - var trackerPost = new InformationalEventPost(new FakeSignalIngressSink(), Clock); - Tracker = new HelloTracker( - sessionId: "S1", - tenantId: "T1", - post: trackerPost, - logger: Logger); - } - - public void Dispose() - { - Tracker.Dispose(); - Tmp.Dispose(); - } - } - - [Fact] - public void TriggerFromTest_emits_HelloResolved_with_outcome_in_payload() - { - using var f = new Fixture(); - using var adapter = new HelloTrackerAdapter(f.Tracker, f.Ingress, f.Clock); - - adapter.TriggerFromTest("completed"); - - var posted = Assert.Single(f.Ingress.Posted); - Assert.Equal(DecisionSignalKind.HelloResolved, posted.Kind); - Assert.Equal(Fixed, posted.OccurredAtUtc); - Assert.Equal("HelloTracker", posted.SourceOrigin); - Assert.Equal("completed", posted.Payload![SignalPayloadKeys.HelloOutcome]); - Assert.Contains("completed", posted.Evidence.Summary); - } - - [Theory] - [InlineData("completed")] - [InlineData("skipped")] - [InlineData("timeout")] - [InlineData("not_configured")] - [InlineData("wizard_not_started")] - public void Outcome_propagates_verbatim_to_payload(string outcome) - { - using var f = new Fixture(); - using var adapter = new HelloTrackerAdapter(f.Tracker, f.Ingress, f.Clock); - - adapter.TriggerFromTest(outcome); - - Assert.Equal(outcome, f.Ingress.Posted[0].Payload![SignalPayloadKeys.HelloOutcome]); - } - - [Fact] - public void Null_or_empty_outcome_falls_back_to_unknown() - { - using var f = new Fixture(); - using var adapter = new HelloTrackerAdapter(f.Tracker, f.Ingress, f.Clock); - - adapter.TriggerFromTest(null!); - - Assert.Equal("unknown", f.Ingress.Posted[0].Payload![SignalPayloadKeys.HelloOutcome]); - } - - [Fact] - public void Duplicate_trigger_is_deduplicated() - { - using var f = new Fixture(); - using var adapter = new HelloTrackerAdapter(f.Tracker, f.Ingress, f.Clock); - - adapter.TriggerFromTest("completed"); - adapter.TriggerFromTest("timeout"); // even with different outcome, we're fire-once - - Assert.Single(f.Ingress.Posted); - Assert.Equal("completed", f.Ingress.Posted[0].Payload![SignalPayloadKeys.HelloOutcome]); - } - - [Fact] - public void Ctor_null_args_throw() - { - using var f = new Fixture(); - Assert.Throws(() => new HelloTrackerAdapter(null!, f.Ingress, f.Clock)); - Assert.Throws(() => new HelloTrackerAdapter(f.Tracker, null!, f.Clock)); - Assert.Throws(() => new HelloTrackerAdapter(f.Tracker, f.Ingress, null!)); - } - } -} diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/SignalAdapters/ProvisioningStatusTrackerAdapterTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/SignalAdapters/ProvisioningStatusTrackerAdapterTests.cs deleted file mode 100644 index e3fb2526..00000000 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/SignalAdapters/ProvisioningStatusTrackerAdapterTests.cs +++ /dev/null @@ -1,115 +0,0 @@ -using System; -using System.Linq; -using AutopilotMonitor.Agent.V2.Core.Logging; -using AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals; -using AutopilotMonitor.Agent.V2.Core.Orchestration; -using AutopilotMonitor.Agent.V2.Core.SignalAdapters; -using AutopilotMonitor.Agent.V2.Core.Tests.Harness; -using AutopilotMonitor.Agent.V2.Core.Tests.Orchestration; -using AutopilotMonitor.DecisionCore.Signals; -using Xunit; - -namespace AutopilotMonitor.Agent.V2.Core.Tests.SignalAdapters -{ - public sealed class ProvisioningStatusTrackerAdapterTests - { - private static readonly DateTime Fixed = new DateTime(2026, 4, 20, 10, 0, 0, DateTimeKind.Utc); - - private sealed class Fixture : IDisposable - { - public TempDirectory Tmp { get; } = new TempDirectory(); - public AgentLogger Logger { get; } - public ProvisioningStatusTracker Tracker { get; } - public FakeSignalIngressSink Ingress { get; } = new FakeSignalIngressSink(); - public VirtualClock Clock { get; } = new VirtualClock(Fixed); - - public Fixture() - { - Logger = new AgentLogger(Tmp.Path, AgentLogLevel.Info); - var trackerPost = new InformationalEventPost(new FakeSignalIngressSink(), Clock); - Tracker = new ProvisioningStatusTracker( - sessionId: "S1", - tenantId: "T1", - post: trackerPost, - logger: Logger); - } - - public void Dispose() - { - Tracker.Dispose(); - Tmp.Dispose(); - } - } - - [Fact] - public void DeviceSetupComplete_maps_to_DeviceSetupProvisioningComplete_signal() - { - using var f = new Fixture(); - using var adapter = new ProvisioningStatusTrackerAdapter(f.Tracker, f.Ingress, f.Clock); - - adapter.TriggerDeviceSetupCompleteFromTest(); - - var posted = Assert.Single(f.Ingress.Posted); - Assert.Equal(DecisionSignalKind.DeviceSetupProvisioningComplete, posted.Kind); - Assert.Equal(Fixed, posted.OccurredAtUtc); - Assert.Equal("ProvisioningStatusTracker", posted.SourceOrigin); - // Snapshot without real registry data → "unknown" - Assert.Equal("unknown", posted.Payload!["deviceSetupResolved"]); - } - - [Fact] - public void EspFailure_maps_to_EspTerminalFailure_with_distinct_sourceOrigin_from_ShellCore() - { - using var f = new Fixture(); - using var adapter = new ProvisioningStatusTrackerAdapter(f.Tracker, f.Ingress, f.Clock); - - adapter.TriggerEspFailureFromTest("DevicePreparation_Failed"); - - var posted = Assert.Single(f.Ingress.Posted); - Assert.Equal(DecisionSignalKind.EspTerminalFailure, posted.Kind); - // SourceOrigin distinguishes ShellCore vs. ProvisioningStatus — important for - // multi-source failure attribution in the journal. - Assert.Equal("ProvisioningStatusTracker", posted.SourceOrigin); - Assert.Equal("DevicePreparation_Failed", posted.Payload!["failureType"]); - } - - [Fact] - public void EspFailure_empty_type_falls_back_to_unknown() - { - using var f = new Fixture(); - using var adapter = new ProvisioningStatusTrackerAdapter(f.Tracker, f.Ingress, f.Clock); - - adapter.TriggerEspFailureFromTest(""); - - Assert.Equal("unknown", f.Ingress.Posted[0].Payload!["failureType"]); - } - - [Fact] - public void Each_signal_kind_is_deduplicated_independently() - { - using var f = new Fixture(); - using var adapter = new ProvisioningStatusTrackerAdapter(f.Tracker, f.Ingress, f.Clock); - - adapter.TriggerDeviceSetupCompleteFromTest(); - adapter.TriggerDeviceSetupCompleteFromTest(); // dedup - adapter.TriggerEspFailureFromTest("t1"); - adapter.TriggerEspFailureFromTest("t2"); // dedup - - Assert.Equal(2, f.Ingress.Posted.Count); - Assert.Contains(f.Ingress.Posted, p => p.Kind == DecisionSignalKind.DeviceSetupProvisioningComplete); - Assert.Contains(f.Ingress.Posted, p => p.Kind == DecisionSignalKind.EspTerminalFailure); - } - - [Fact] - public void Ctor_null_args_throw() - { - using var f = new Fixture(); - Assert.Throws( - () => new ProvisioningStatusTrackerAdapter(null!, f.Ingress, f.Clock)); - Assert.Throws( - () => new ProvisioningStatusTrackerAdapter(f.Tracker, null!, f.Clock)); - Assert.Throws( - () => new ProvisioningStatusTrackerAdapter(f.Tracker, f.Ingress, null!)); - } - } -} diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/SignalAdapters/ShellCoreTrackerAdapterTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/SignalAdapters/ShellCoreTrackerAdapterTests.cs deleted file mode 100644 index f3b141fb..00000000 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/SignalAdapters/ShellCoreTrackerAdapterTests.cs +++ /dev/null @@ -1,221 +0,0 @@ -using System; -using System.Linq; -using AutopilotMonitor.Agent.V2.Core.Logging; -using AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals; -using AutopilotMonitor.Agent.V2.Core.Orchestration; -using AutopilotMonitor.Agent.V2.Core.SignalAdapters; -using AutopilotMonitor.Agent.V2.Core.Tests.Harness; -using AutopilotMonitor.Agent.V2.Core.Tests.Orchestration; -using AutopilotMonitor.DecisionCore.Engine; -using AutopilotMonitor.DecisionCore.Signals; -using AutopilotMonitor.Shared.Models; -using Xunit; - -namespace AutopilotMonitor.Agent.V2.Core.Tests.SignalAdapters -{ - public sealed class ShellCoreTrackerAdapterTests - { - private static readonly DateTime Fixed = new DateTime(2026, 4, 20, 10, 0, 0, DateTimeKind.Utc); - - private sealed class Fixture : IDisposable - { - public TempDirectory Tmp { get; } = new TempDirectory(); - public AgentLogger Logger { get; } - public ShellCoreTracker Tracker { get; } - public FakeSignalIngressSink Ingress { get; } = new FakeSignalIngressSink(); - public VirtualClock Clock { get; } = new VirtualClock(Fixed); - - public Fixture() - { - Logger = new AgentLogger(Tmp.Path, AgentLogLevel.Info); - var trackerPost = new InformationalEventPost(new FakeSignalIngressSink(), Clock); - Tracker = new ShellCoreTracker( - sessionId: "S1", - tenantId: "T1", - post: trackerPost, - logger: Logger, - helloTracker: null); - } - - public void Dispose() - { - Tracker.Dispose(); - Tmp.Dispose(); - } - } - - [Fact] - public void FinalizingPhase_maps_to_EspPhaseChanged_with_Finalizing_payload() - { - using var f = new Fixture(); - using var adapter = new ShellCoreTrackerAdapter(f.Tracker, f.Ingress, f.Clock); - - adapter.TriggerFinalizingFromTest("shell-core-62404"); - - var posted = Assert.Single(f.Ingress.Posted); - Assert.Equal(DecisionSignalKind.EspPhaseChanged, posted.Kind); - Assert.Equal(Fixed, posted.OccurredAtUtc); - Assert.Equal("ShellCoreTracker", posted.SourceOrigin); - Assert.Equal( - EnrollmentPhase.FinalizingSetup.ToString(), - posted.Payload![SignalPayloadKeys.EspPhase]); - Assert.Equal("shell-core-62404", posted.Payload["reason"]); - } - - [Fact] - public void WhiteGloveCompleted_maps_to_WhiteGloveShellCoreSuccess() - { - using var f = new Fixture(); - using var adapter = new ShellCoreTrackerAdapter(f.Tracker, f.Ingress, f.Clock); - - adapter.TriggerWhiteGloveCompletedFromTest(); - - var posted = Assert.Single(f.Ingress.Posted); - Assert.Equal(DecisionSignalKind.WhiteGloveShellCoreSuccess, posted.Kind); - Assert.Equal("ShellCoreTracker", posted.SourceOrigin); - Assert.Equal(EvidenceKind.Derived, posted.Evidence.Kind); - Assert.Contains("sealing success", posted.Evidence.Summary); - } - - [Fact] - public void EspFailure_maps_to_EspTerminalFailure_with_failureType_in_payload() - { - using var f = new Fixture(); - using var adapter = new ShellCoreTrackerAdapter(f.Tracker, f.Ingress, f.Clock); - - adapter.TriggerEspFailureFromTest("AccountSetup_Timeout"); - - var posted = Assert.Single(f.Ingress.Posted); - Assert.Equal(DecisionSignalKind.EspTerminalFailure, posted.Kind); - Assert.Equal("AccountSetup_Timeout", posted.Payload!["failureType"]); - Assert.Equal("AccountSetup_Timeout", posted.Evidence.DerivationInputs!["failureType"]); - } - - [Fact] - public void EspFailure_empty_type_falls_back_to_unknown() - { - using var f = new Fixture(); - using var adapter = new ShellCoreTrackerAdapter(f.Tracker, f.Ingress, f.Clock); - - adapter.TriggerEspFailureFromTest(""); - - Assert.Equal("unknown", f.Ingress.Posted[0].Payload!["failureType"]); - } - - [Fact] - public void Each_signal_kind_is_deduplicated_independently() - { - using var f = new Fixture(); - using var adapter = new ShellCoreTrackerAdapter(f.Tracker, f.Ingress, f.Clock); - - adapter.TriggerFinalizingFromTest("r1"); - adapter.TriggerFinalizingFromTest("r2"); // dedup - adapter.TriggerWhiteGloveCompletedFromTest(); - adapter.TriggerWhiteGloveCompletedFromTest(); // dedup - adapter.TriggerEspFailureFromTest("t1"); - adapter.TriggerEspFailureFromTest("t2"); // dedup - - Assert.Equal(3, f.Ingress.Posted.Count); - Assert.Contains(f.Ingress.Posted, p => p.Kind == DecisionSignalKind.EspPhaseChanged); - Assert.Contains(f.Ingress.Posted, p => p.Kind == DecisionSignalKind.WhiteGloveShellCoreSuccess); - Assert.Contains(f.Ingress.Posted, p => p.Kind == DecisionSignalKind.EspTerminalFailure); - } - - [Fact] - public void All_three_events_can_fire_in_one_session() - { - using var f = new Fixture(); - using var adapter = new ShellCoreTrackerAdapter(f.Tracker, f.Ingress, f.Clock); - - adapter.TriggerFinalizingFromTest("phase-enter"); - adapter.TriggerWhiteGloveCompletedFromTest(); - adapter.TriggerEspFailureFromTest("Timeout"); - - Assert.Equal(3, f.Ingress.Posted.Count); - Assert.Equal(DecisionSignalKind.EspPhaseChanged, f.Ingress.Posted[0].Kind); - Assert.Equal(DecisionSignalKind.WhiteGloveShellCoreSuccess, f.Ingress.Posted[1].Kind); - Assert.Equal(DecisionSignalKind.EspTerminalFailure, f.Ingress.Posted[2].Kind); - } - - [Fact] - public void Ctor_null_args_throw() - { - using var f = new Fixture(); - Assert.Throws(() => new ShellCoreTrackerAdapter(null!, f.Ingress, f.Clock)); - Assert.Throws(() => new ShellCoreTrackerAdapter(f.Tracker, null!, f.Clock)); - Assert.Throws(() => new ShellCoreTrackerAdapter(f.Tracker, f.Ingress, null!)); - } - - // ---- EspExited → DecisionSignalKind.EspExiting (Plan §EspExiting Adapter) ---- - - [Fact] - public void EspExited_maps_to_EspExiting_signal() - { - using var f = new Fixture(); - using var adapter = new ShellCoreTrackerAdapter(f.Tracker, f.Ingress, f.Clock); - var sourceTime = new DateTime(2026, 5, 12, 17, 45, 30, DateTimeKind.Utc); - - adapter.TriggerEspExitingFromTest(sourceTime); - - var posted = Assert.Single(f.Ingress.Posted); - Assert.Equal(DecisionSignalKind.EspExiting, posted.Kind); - Assert.Equal("ShellCoreTracker", posted.SourceOrigin); - Assert.Equal(EvidenceKind.Derived, posted.Evidence.Kind); - Assert.Contains("ESP exiting", posted.Evidence.Summary); - Assert.Equal("Microsoft-Windows-Shell-Core", posted.Evidence.DerivationInputs!["eventSource"]); - Assert.Equal("62407", posted.Evidence.DerivationInputs["eventId"]); - } - - [Fact] - public void EspExited_posts_every_occurrence_no_dedup() - { - // Shell-Core 62407 fires at every ESP phase transition (Device→Account, Account→End). - // The adapter must NOT dedup — the reducer (ShouldTransitionToAwaitingHello) - // distinguishes the genuine post-AccountSetup exit from intermediate ones. - using var f = new Fixture(); - using var adapter = new ShellCoreTrackerAdapter(f.Tracker, f.Ingress, f.Clock); - - adapter.TriggerEspExitingFromTest(new DateTime(2026, 5, 12, 17, 20, 0, DateTimeKind.Utc)); - adapter.TriggerEspExitingFromTest(new DateTime(2026, 5, 12, 17, 45, 0, DateTimeKind.Utc)); - adapter.TriggerEspExitingFromTest(new DateTime(2026, 5, 12, 18, 10, 0, DateTimeKind.Utc)); - - Assert.Equal(3, f.Ingress.Posted.Count); - Assert.All(f.Ingress.Posted, p => Assert.Equal(DecisionSignalKind.EspExiting, p.Kind)); - } - - [Fact] - public void EspExited_preserves_source_event_timestamp_not_clock_now() - { - // OccurredAtUtc must come from the source event (live = log time, backfill = - // record.TimeCreated), not from the adapter's clock — otherwise the engine's - // EffectiveDeadlineBase would floor HelloSafety at "now" on a backfilled run. - using var f = new Fixture(); - using var adapter = new ShellCoreTrackerAdapter(f.Tracker, f.Ingress, f.Clock); - var pastTime = new DateTime(2026, 4, 1, 8, 15, 22, DateTimeKind.Utc); // before f.Clock = 2026-04-20 - - adapter.TriggerEspExitingFromTest(pastTime); - - var posted = Assert.Single(f.Ingress.Posted); - Assert.Equal(pastTime, posted.OccurredAtUtc); - Assert.NotEqual(f.Clock.UtcNow, posted.OccurredAtUtc); - } - - [Fact] - public void All_four_events_can_fire_in_one_session() - { - using var f = new Fixture(); - using var adapter = new ShellCoreTrackerAdapter(f.Tracker, f.Ingress, f.Clock); - - adapter.TriggerFinalizingFromTest("phase-enter"); - adapter.TriggerWhiteGloveCompletedFromTest(); - adapter.TriggerEspFailureFromTest("Timeout"); - adapter.TriggerEspExitingFromTest(Fixed); - - Assert.Equal(4, f.Ingress.Posted.Count); - Assert.Contains(f.Ingress.Posted, p => p.Kind == DecisionSignalKind.EspPhaseChanged); - Assert.Contains(f.Ingress.Posted, p => p.Kind == DecisionSignalKind.WhiteGloveShellCoreSuccess); - Assert.Contains(f.Ingress.Posted, p => p.Kind == DecisionSignalKind.EspTerminalFailure); - Assert.Contains(f.Ingress.Posted, p => p.Kind == DecisionSignalKind.EspExiting); - } - } -} 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/Enrollment/SystemSignals/HelloTracker.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/HelloTracker.cs index de98f5a3..60d12343 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/HelloTracker.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/HelloTracker.cs @@ -85,6 +85,9 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals private bool _isHelloCompleted; private bool _helloWizardStarted; private bool _espExitSeen; + // MON-C2 — one-shot guard so the "policy not yet detected" grace re-arm of the wait + // timer can fire at most once before falling through to the not-configured resolution. + private bool _helloUnknownGraceUsed; private readonly object _stateLock = new object(); public event EventHandler HelloCompleted; @@ -214,6 +217,10 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals _helloWizardStarted = false; _espExitSeen = false; HelloOutcome = null; + // MON-C2 — clear the one-shot unknown-policy grace flag too, otherwise a fresh + // post-resume wait that hits an undetected policy would skip its grace re-arm and + // resolve straight to not_configured. + _helloUnknownGraceUsed = false; } _logger.Info("HelloTracker: Reset for ESP resumption — Hello tracking restarted"); } @@ -955,11 +962,23 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals return; } - // Timeout expired without Hello wizard starting. - // If Hello policy is known to be ENABLED, the wizard simply hasn't appeared yet - // (e.g. Windows is still setting up prerequisites). Do NOT declare enrollment - // complete — start the long HelloCompletion timer and keep waiting. - if (_isHelloPolicyEnabled) + // Timeout expired without the Hello wizard starting. Resolve into one of three + // cases — only a *positively detected DISABLED* policy is a real "Hello not + // configured" signal (MON-C2): + // + // * policy known ENABLED — wizard simply hasn't appeared yet; keep waiting on + // the long completion timer. + // * policy NOT YET DETECTED — a slow MDM/CSP sync can land the policy read after + // this wait; force-completing to "not_configured" now + // would prematurely resolve Hello on a device that is + // actually configured. Grant one bounded grace re-arm + // for detection to catch up. + // * policy known DISABLED (or still unknown after the grace) — genuinely not + // configured / skipped; resolve completion. + var policyKnownEnabled = _isPolicyConfigured && _isHelloPolicyEnabled; + var policyUnknown = !_isPolicyConfigured; + + if (policyKnownEnabled) { _logger.Info($"Hello wait timeout ({_helloWaitTimeoutSeconds}s) expired but Hello policy is enabled — " + $"wizard not yet visible, starting long completion timer ({HelloCompletionTimeoutSeconds}s)"); @@ -979,6 +998,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals { "timeoutSeconds", _helloWaitTimeoutSeconds }, { "espExitDetected", _espExitSeen }, { "helloPolicyEnabled", true }, + { "policyDetected", true }, { "action", "extended_wait" } }, ImmediateUpload = true @@ -988,8 +1008,44 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals return; } + if (policyUnknown && !_helloUnknownGraceUsed) + { + _helloUnknownGraceUsed = true; + _logger.Info($"Hello wait timeout ({_helloWaitTimeoutSeconds}s) expired with Hello policy not yet detected — " + + $"granting a {_helloWaitTimeoutSeconds}s grace for a slow policy sync before assuming not configured"); + + _post.Emit(new EnrollmentEvent + { + SessionId = _sessionId, + TenantId = _tenantId, + EventType = Constants.EventTypes.HelloWaitTimeout, + Severity = EventSeverity.Info, + Source = "EspAndHelloTracker", + Phase = EnrollmentPhase.Unknown, + Message = $"Hello wizard did not start within {_helloWaitTimeoutSeconds}s after ESP exit and Hello policy " + + $"is not yet detected — waiting a further {_helloWaitTimeoutSeconds}s for a slow policy sync", + Data = new Dictionary + { + { "timeoutSeconds", _helloWaitTimeoutSeconds }, + { "espExitDetected", _espExitSeen }, + { "helloPolicyEnabled", false }, + { "policyDetected", false }, + { "action", "unknown_policy_grace" } + }, + ImmediateUpload = true + }); + + // Re-arm the existing one-shot wait timer for a single bounded grace window. + // The 10s-interval policy check timer keeps running and may flip the policy to + // enabled/disabled within this window; otherwise the next fire resolves it. + _helloWaitTimer?.Change(TimeSpan.FromSeconds(_helloWaitTimeoutSeconds), TimeSpan.FromMilliseconds(-1)); + return; + } + _logger.Info($"Hello wait timeout ({_helloWaitTimeoutSeconds}s) expired without Hello wizard starting"); - _logger.Info("Hello policy not detected as enabled — assuming Hello is not configured or was skipped"); + _logger.Info(_isPolicyConfigured + ? "Hello policy detected as disabled — assuming Hello is not configured or was skipped" + : "Hello policy still not detected after grace — assuming Hello is not configured or was skipped"); _isHelloCompleted = true; HelloOutcome = "not_configured"; @@ -1009,6 +1065,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals { "timeoutSeconds", _helloWaitTimeoutSeconds }, { "espExitDetected", _espExitSeen }, { "helloPolicyEnabled", false }, + { "policyDetected", _isPolicyConfigured }, { "action", "enrollment_complete" } }, ImmediateUpload = true diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/ShellCoreTracker.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/ShellCoreTracker.cs index d81bb736..742e8711 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/ShellCoreTracker.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/ShellCoreTracker.cs @@ -299,10 +299,10 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals catch (Exception ex) { _logger.Error($"EspFailureDetected handler failed for '{detectedFailureType}'", ex); } } - // Fire EspExited AFTER event emission. Subscribers (ShellCoreTrackerAdapter) post - // a DecisionSignalKind.EspExiting so the engine can arm HelloSafety on the genuine - // post-AccountSetup exit. Engine-side guard (ShouldTransitionToAwaitingHello) - // distinguishes intermediate exits from the real one. + // Fire EspExited AFTER event emission. The coordinator (EspAndHelloTracker) re-raises + // this and EspAndHelloTrackerAdapter posts a DecisionSignalKind.EspExiting so the + // engine can arm HelloSafety on the genuine post-AccountSetup exit. Engine-side guard + // (ShouldTransitionToAwaitingHello) distinguishes intermediate exits from the real one. if (eventType == Constants.EventTypes.EspExiting) { try { EspExited?.Invoke(this, new EspExitedEventArgs(timestamp)); } 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/Orchestration/CollectorSurfaces.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/CollectorSurfaces.cs new file mode 100644 index 00000000..a0af0869 --- /dev/null +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/CollectorSurfaces.cs @@ -0,0 +1,117 @@ +#nullable enable +using System; +using System.Collections.Generic; +using AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.Ime; +using AutopilotMonitor.Shared.Models; + +namespace AutopilotMonitor.Agent.V2.Core.Orchestration +{ + /// + /// Result of — the collector hosts + /// plus the typed read-model surfaces peripheral consumers need after startup + /// (ARCH-F4, V2 agent review 2026-06-10). + /// + /// Before this type existed, DefaultComponentFactory stashed host references in + /// mutable fields and exposed them through lazy properties, which turned the factory into + /// a service locator + state bag and forced EnrollmentOrchestrator to downcast the + /// seam. Now the factory returns everything in one + /// immutable bundle; the orchestrator stores it and exposes it via + /// EnrollmentOrchestrator.CollectorSurfaces for the runtime host. + /// + /// + /// The read-model properties are live delegating views over the underlying hosts — + /// not snapshots. They return null (or an empty fallback) when the corresponding + /// host was not created, e.g. in test fakes that only supply . + /// + /// + public sealed class CollectorSurfaces + { + private readonly ImeLogHost? _imeLogHost; + + /// + /// All collector hosts in creation order. The orchestrator starts them in this + /// order and stops/disposes them in the same order on shutdown. + /// + public IReadOnlyList Hosts { get; } + + /// + /// ESP/Hello coordinator host — the WhiteGlove-success + DeviceSetup-complete event + /// surface consumed by WhiteGloveInventoryTrigger / AutoLogonDeviceSetupTrigger. + /// internal because itself is internal sealed; + /// AutopilotMonitor.Agent.V2 (the runtime entry-point project) sees it via the + /// InternalsVisibleTo declared on this project's csproj. + /// + internal EspAndHelloHost? EspAndHelloHost { get; } + + /// + /// AAD-join watcher host — used by the runtime host to arm the Hybrid User-Driven + /// login-pending detector after observing a reboot-kill on a Hybrid-AAD device + /// (2026-05-01 completion-gap fix). + /// + internal AadJoinHost? AadJoinHost { get; } + + /// + /// Surfaces with hosts only — for test fakes that don't build the production + /// read-model hosts. All read-model properties return their empty fallbacks. + /// + public CollectorSurfaces(IReadOnlyList hosts) + : this(hosts, imeLogHost: null, espAndHelloHost: null, aadJoinHost: null) + { + } + + internal CollectorSurfaces( + IReadOnlyList hosts, + ImeLogHost? imeLogHost, + EspAndHelloHost? espAndHelloHost, + AadJoinHost? aadJoinHost) + { + Hosts = hosts ?? throw new ArgumentNullException(nameof(hosts)); + _imeLogHost = imeLogHost; + EspAndHelloHost = espAndHelloHost; + AadJoinHost = aadJoinHost; + } + + /// + /// F5 (debrief 7dd4e593) — deduped union of phase-snapshotted apps + the live + /// package-state list. Used by the termination summary path so DeviceSetup apps + /// cleared from the live list on the AccountSetup transition still reach the + /// SummaryDialog and app_tracking_summary event. + /// + public IReadOnlyList? AllKnownPackageStates => + _imeLogHost?.AllKnownPackageStates; + + /// + /// Plan §5 Fix 4c — per-app install-lifecycle timings (StartedAt / CompletedAt / + /// DurationSeconds) captured by ImeLogTrackerAdapter. + /// + public IReadOnlyDictionary? ImeAppTimings => _imeLogHost?.AppTimings; + + /// + /// V1-parity field: count of IME apps in the tracker's ignore list (e.g. uninstall + /// intents that don't surface in the install pipeline). Lives on the live + /// only — phase snapshots don't carry it. + /// + public int ImeIgnoredCount => _imeLogHost?.PackageStates?.IgnoreList?.Count ?? 0; + + /// + /// c117946b debrief (2026-05-12) — bridge for the V2 EnrollmentTerminationHandler + /// pre-hook. Delegates to ImeLogHost.PromoteActiveInstallsToStuck which calls + /// the tracker directly so the standard OnAppStateChanged path fires and the + /// adapter emits regular app_install_failed events for every promoted app. + /// Returns an empty list when no IME host exists. + /// + public IReadOnlyList PromoteActiveInstallsToStuck(string failureType, string message, string? errorCode = null) => + _imeLogHost?.PromoteActiveInstallsToStuck(failureType, message, errorCode) ?? Array.Empty(); + + /// + /// Session 080edee9 follow-up + Codex review (P2/P3, 2026-05-28) — last observed + /// ESP failure context (HRESULT + failedSubcategory + category). Read by + /// EnrollmentTerminationHandler.MaybePromoteActiveInstallsAsStuck so the + /// promotion can: (a) classify via HRESULT (detection-failure / install-failure), + /// AND (b) refuse to attach app-level classifications when the failure originated + /// outside the Apps subcategory. Returns null when no registry-derived ESP failure + /// was observed. + /// + public Termination.EspTerminalFailureSnapshot? LastEspTerminalFailure => EspAndHelloHost?.LastEspTerminalFailure; + } +} diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/DefaultComponentFactory.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/DefaultComponentFactory.cs index c088a5c0..b90b0b61 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/DefaultComponentFactory.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/DefaultComponentFactory.cs @@ -25,9 +25,9 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration /// EspAndHelloHost — coordinator /// (internally aggregates HelloTracker + ShellCoreTracker + ProvisioningStatusTracker /// + ModernDeploymentTracker) wired via . - /// This is the single production entry for the ESP+Hello signal surface — avoids - /// double emission that would happen if the sub-tracker adapters were also wired - /// in parallel (§4.x M4.3 tech-debt note about adapter duplication). + /// This is the single production entry for the ESP+Hello signal surface: the sub-trackers + /// are private to the coordinator and only their re-raised events are adapted here, so + /// there is exactly one DecisionSignal per source event (no double emission). /// DesktopArrivalHost — + . /// AadJoinHost — + . /// ImeLogHost — + @@ -68,87 +68,6 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration private readonly string _agentVersion; private readonly string _stateDirectory; - private ImeLogHost? _imeLogHost; - private EspAndHelloHost? _espAndHelloHost; - private AadJoinHost? _aadJoinHost; - private RealmJoinHost? _realmJoinHost; - private Transport.Telemetry.ITelemetrySpool? _telemetrySpool; - - /// - /// Exposes the WhiteGlove-success event surface from the ESP/Hello coordinator host so - /// downstream subscribers (e.g. WhiteGloveInventoryTrigger) can react to the - /// pre-provisioning success window without reaching into the host's internals. Returns - /// null before has been called. - /// - /// internal because itself is internal sealed. - /// AutopilotMonitor.Agent.V2 (the runtime entry-point project) sees it via the - /// InternalsVisibleTo declared on this project's csproj. - /// - /// - internal EspAndHelloHost? EspAndHelloHost => _espAndHelloHost; - - /// - /// Exposes the so the runtime host can arm the Hybrid - /// User-Driven login-pending detector after observing a reboot-kill on a Hybrid-AAD - /// device (2026-05-01 completion-gap fix). Returns null before - /// has been called. - /// - internal AadJoinHost? AadJoinHost => _aadJoinHost; - - /// - /// Exposes the IME tracker's package-state list to peripheral consumers such as the - /// FinalStatusBuilder in M4.6.β. Returns null before - /// has been called (Orchestrator start order). - /// - public AppPackageStateList? ImePackageStates => _imeLogHost?.PackageStates; - - /// - /// F5 (debrief 7dd4e593) — deduped union of phase-snapshotted apps + the live - /// . Use this from the termination summary path so - /// DeviceSetup apps cleared from _packageStates on the AccountSetup transition - /// still reach the SummaryDialog and app_tracking_summary event. Returns - /// null before has been called. - /// - public IReadOnlyList? AllKnownPackageStates => - _imeLogHost?.AllKnownPackageStates; - - /// - /// Plan §5 Fix 4c — per-app install-lifecycle timings (StartedAt / CompletedAt / - /// DurationSeconds) captured by ImeLogTrackerAdapter. Returns null before - /// has been called (Orchestrator start order). - /// - public IReadOnlyDictionary? ImeAppTimings => _imeLogHost?.AppTimings; - - /// - /// V1-parity field: count of IME apps in the tracker's ignore list (e.g. uninstall - /// intents that don't surface in the install pipeline). Lives on the live - /// only — phase snapshots don't carry it. Returns 0 - /// before has been called. - /// - public int ImeIgnoredCount => _imeLogHost?.PackageStates?.IgnoreList?.Count ?? 0; - - /// - /// c117946b debrief (2026-05-12) — bridge for the V2 EnrollmentTerminationHandler - /// pre-hook. Delegates to ImeLogHost.PromoteActiveInstallsToStuck which calls - /// the tracker directly so the standard OnAppStateChanged path fires and the - /// adapter emits regular app_install_failed events for every promoted app. - /// Returns an empty list when the host has not been created yet (start order). - /// - public IReadOnlyList PromoteActiveInstallsToStuck(string failureType, string message, string? errorCode = null) => - _imeLogHost?.PromoteActiveInstallsToStuck(failureType, message, errorCode) ?? Array.Empty(); - - /// - /// Session 080edee9 follow-up + Codex review (P2/P3, 2026-05-28) — last - /// observed ESP failure context (HRESULT + failedSubcategory + category). - /// Surfaced by and - /// read by EnrollmentTerminationHandler.MaybePromoteActiveInstallsAsStuck - /// so the promotion can: (a) classify via HRESULT (detection-failure / - /// install-failure), AND (b) refuse to attach app-level classifications - /// when the failure originated outside the Apps subcategory. Returns null - /// when no registry-derived ESP failure was observed. - /// - public Termination.EspTerminalFailureSnapshot? LastEspTerminalFailure => EspAndHelloHost?.LastEspTerminalFailure; - public DefaultComponentFactory( AgentConfiguration agentConfig, AgentConfigResponse remoteConfig, @@ -163,27 +82,14 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration _stateDirectory = stateDirectory ?? throw new ArgumentNullException(nameof(stateDirectory)); } - /// - /// Late-binding setter called by right after - /// the is constructed (Start step 3). - /// The spool reference flows from here into the - /// PeriodicCollectorLifecycleHost in and - /// from there into , - /// which surfaces spool.pendingItemCount / spool.fileSizeBytes on - /// every agent_metrics_snapshot. - /// - public void SetTelemetrySpool(Transport.Telemetry.ITelemetrySpool spool) - { - _telemetrySpool = spool; - } - - public IReadOnlyList CreateCollectorHosts( + public CollectorSurfaces CreateCollectorHosts( string sessionId, string tenantId, AgentLogger logger, IReadOnlyCollection whiteGloveSealingPatternIds, ISignalIngressSink ingress, - IClock clock) + IClock clock, + Transport.Telemetry.ITelemetrySpool? telemetrySpool) { if (string.IsNullOrEmpty(sessionId)) throw new ArgumentException("SessionId required.", nameof(sessionId)); if (string.IsNullOrEmpty(tenantId)) throw new ArgumentException("TenantId required.", nameof(tenantId)); @@ -196,7 +102,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration // ----- Kernel hosts (always-on; they produce decision signals) -------------------- - _espAndHelloHost = new EspAndHelloHost( + var espAndHelloHost = new EspAndHelloHost( sessionId: sessionId, tenantId: tenantId, logger: logger, @@ -209,7 +115,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration modernDeploymentBackfillLookbackMinutes: collectors.ModernDeploymentBackfillLookbackMinutes, modernDeploymentHarmlessEventIds: collectors.ModernDeploymentHarmlessEventIds, stateDirectory: _stateDirectory); - hosts.Add(_espAndHelloHost); + hosts.Add(espAndHelloHost); // Hybrid User-Driven completion-gap fix (2026-05-01): the AadJoinHost notifies // the DesktopArrivalHost when a real AAD user replaces the foouser/autopilot @@ -219,8 +125,9 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration // // RealmJoin hookup: the DesktopArrival observer feeds the resolved real-user owner // to the RealmJoinHost so it can arm its HKU-scope package watcher. The lambda - // captures `_realmJoinHost` by reference — null at this point, set a few lines - // later before the detector can ever fire. + // captures `realmJoinHost` by reference (closure over the local) — null at this + // point, set a few lines later before the detector can ever fire. + RealmJoinHost? realmJoinHost = null; var desktopArrivalHost = new DesktopArrivalHost( logger, ingress, @@ -230,10 +137,10 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration tenantId: tenantId, onRealUserOwnerObserved: owner => { - if (_realmJoinHost == null) return; + if (realmJoinHost == null) return; if (UserSidResolver.TryResolveSid(owner, out var sid) && !string.IsNullOrEmpty(sid)) { - _realmJoinHost.ArmHkuWatcher(sid!); + realmJoinHost.ArmHkuWatcher(sid!); } else { @@ -242,13 +149,13 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration }); hosts.Add(desktopArrivalHost); - _aadJoinHost = new AadJoinHost( + var aadJoinHost = new AadJoinHost( logger, ingress, clock, onRealUserJoined: desktopArrivalHost.RequestResetForRealUserSwitch); - hosts.Add(_aadJoinHost); + hosts.Add(aadJoinHost); - _realmJoinHost = new RealmJoinHost(logger, ingress, clock); - hosts.Add(_realmJoinHost); + realmJoinHost = new RealmJoinHost(logger, ingress, clock); + hosts.Add(realmJoinHost); // Single-rail refactor (plan §5.8) — DeviceInfoCollector existed in V2.Core but had // no host so the Device-Details UI block was empty in V2 sessions (V1-Parity Issue #2). @@ -281,7 +188,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration ? _agentConfig.ReplayLogDir : _agentConfig.ImeLogPathOverride; - _imeLogHost = new ImeLogHost( + var imeLogHost = new ImeLogHost( sessionId: sessionId, tenantId: tenantId, logger: logger, @@ -294,7 +201,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration whiteGloveSealingPatternIds: whiteGloveSealingPatternIds, simulationMode: simulationMode, simulationSpeedFactor: _agentConfig.ReplaySpeedFactor); - hosts.Add(_imeLogHost); + hosts.Add(imeLogHost); if (collectors.StallProbeEnabled) { @@ -333,7 +240,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration idleTimeoutMinutes: collectors.CollectorIdleTimeoutMinutes, networkMetrics: _networkMetrics, agentVersion: _agentVersion, - telemetrySpool: _telemetrySpool)); + telemetrySpool: telemetrySpool)); } // V1 parity (CollectorCoordinator.StartOptionalCollectors:375-382) — wire the @@ -369,7 +276,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration // tracker reports an app entering Downloading/Installing (AppStateChanged chain below) OR // an Office C2R install is active (officeHost signal) — the latter lets it capture Office's // DO CDN jobs and fold them into the office_install_* events. - if (collectors.EnableDeliveryOptimizationCollector && _imeLogHost != null) + if (collectors.EnableDeliveryOptimizationCollector) { var doHost = new DeliveryOptimizationHost( sessionId: sessionId, @@ -378,7 +285,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration clock: clock, logger: logger, intervalSeconds: collectors.DeliveryOptimizationIntervalSeconds, - imeHost: _imeLogHost, + imeHost: imeLogHost, officeHost: officeHost); hosts.Add(doHost); } @@ -403,7 +310,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration unrestrictedMode: _agentConfig.UnrestrictedMode)); } - return hosts; + return new CollectorSurfaces(hosts, imeLogHost, espAndHelloHost, aadJoinHost); } } } diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/EnrollmentOrchestrator.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/EnrollmentOrchestrator.cs index 8a00f7cd..82ce154e 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/EnrollmentOrchestrator.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/EnrollmentOrchestrator.cs @@ -299,6 +299,14 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration /// public long IngressPendingSignalCount => _ingress?.PendingSignalCount ?? 0L; + /// + /// Typed read-model surfaces returned by + /// (ARCH-F4). Set during step 14; null before Start and when + /// no factory was supplied. The runtime host reads peripheral surfaces (IME app tracking, + /// ESP/Hello coordinator, AAD-join host) from here instead of downcasting the factory seam. + /// + public CollectorSurfaces? CollectorSurfaces { get; private set; } + // ---------------------------------------------------------------- Lifecycle /// @@ -584,17 +592,9 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration _transport.BatchPoisoned += OnBatchPoisoned; _transport.UploadBlocked += OnUploadBlocked; - // Late-bind the spool reference into the component factory so peripheral - // collectors (AgentSelfMetricsCollector via PeriodicCollectorLifecycleHost) can - // read PendingItemCount / SpoolFileSizeBytes for the agent_metrics_snapshot - // payload. Done as a setter call rather than a CreateCollectorHosts parameter - // because IComponentFactory is a public seam with test fakes — adding a - // parameter would force every fake to be touched, and the spool stats are an - // optional capability that test fakes don't need to surface. - if (_componentFactory is DefaultComponentFactory defaultFactory) - { - defaultFactory.SetTelemetrySpool(_spool); - } + // The spool reference reaches peripheral collectors (AgentSelfMetricsCollector via + // PeriodicCollectorLifecycleHost) as a CreateCollectorHosts parameter at step 14, + // so agent_metrics_snapshot can surface PendingItemCount / SpoolFileSizeBytes. // 3a) Immediate-flush wakeup — lifecycle-critical items (agent_started, hello wizard, // auth-failure shutdown, version-check, enrollment_failed on max-lifetime, …) @@ -876,9 +876,11 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration // über den Ingress als InformationalEvent. if (_componentFactory != null) { - _collectorHosts = _componentFactory.CreateCollectorHosts( + var surfaces = _componentFactory.CreateCollectorHosts( _sessionId, _tenantId, _logger, _whiteGloveSealingPatternIds, - ingress: _ingress, clock: _clock); + ingress: _ingress, clock: _clock, telemetrySpool: _spool); + CollectorSurfaces = surfaces; + _collectorHosts = surfaces.Hosts; foreach (var host in _collectorHosts) { try diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/IComponentFactory.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/IComponentFactory.cs index eb97635e..66d0ca64 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/IComponentFactory.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/IComponentFactory.cs @@ -27,13 +27,26 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration /// Baut alle Collector-Hosts. Jeder Host ist selbst dafür zuständig, seine /// InformationalEventPost aus (ingress, clock) zu konstruieren und Collector-Events /// als InformationalEvent-Signals über zu posten. + /// + /// Returns a bundle (hosts + typed read-model surfaces) + /// instead of a bare host list (ARCH-F4): the factory holds no post-creation state and + /// the orchestrator never needs to downcast this seam. Test fakes return + /// new CollectorSurfaces(hosts). + /// + /// + /// flows into the PeriodicCollectorLifecycleHost + /// → AgentSelfMetricsCollector so agent_metrics_snapshot can surface + /// spool.pendingItemCount / spool.fileSizeBytes. Nullable — fakes and + /// spool-less configurations pass null and the metrics fields are simply absent. + /// /// - IReadOnlyList CreateCollectorHosts( + CollectorSurfaces CreateCollectorHosts( string sessionId, string tenantId, AgentLogger logger, IReadOnlyCollection whiteGloveSealingPatternIds, ISignalIngressSink ingress, - IClock clock); + IClock clock, + Transport.Telemetry.ITelemetrySpool? telemetrySpool); } } diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/SignalAdapters/HelloTrackerAdapter.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/SignalAdapters/HelloTrackerAdapter.cs deleted file mode 100644 index 1e384f02..00000000 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/SignalAdapters/HelloTrackerAdapter.cs +++ /dev/null @@ -1,73 +0,0 @@ -#nullable enable -using System; -using System.Collections.Generic; -using AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals; -using AutopilotMonitor.Agent.V2.Core.Orchestration; -using AutopilotMonitor.DecisionCore.Engine; -using AutopilotMonitor.DecisionCore.Signals; - -namespace AutopilotMonitor.Agent.V2.Core.SignalAdapters -{ - /// - /// Adapter for → . - /// Plan §2.1a / §2.2. - /// - /// Fires genau einmal, wenn der Tracker Hello-Abschluss feststellt. Payload enthält den - /// HelloOutcome-String (completed | skipped | timeout | not_configured | wizard_not_started), - /// der vom Reducer für die SignalFact<string> HelloOutcome-Fakt-Update verwendet wird. - /// - /// - internal sealed class HelloTrackerAdapter : IDisposable - { - private readonly HelloTracker _tracker; - private readonly ISignalIngressSink _ingress; - private readonly IClock _clock; - private bool _fired; - - public HelloTrackerAdapter( - HelloTracker tracker, - ISignalIngressSink ingress, - IClock clock) - { - _tracker = tracker ?? throw new ArgumentNullException(nameof(tracker)); - _ingress = ingress ?? throw new ArgumentNullException(nameof(ingress)); - _clock = clock ?? throw new ArgumentNullException(nameof(clock)); - - _tracker.HelloCompleted += OnHelloCompleted; - } - - public void Dispose() - { - _tracker.HelloCompleted -= OnHelloCompleted; - } - - private void OnHelloCompleted(object sender, EventArgs e) => EmitInternal(_tracker.HelloOutcome); - - internal void TriggerFromTest(string helloOutcome) => EmitInternal(helloOutcome); - - private void EmitInternal(string? helloOutcome) - { - if (_fired) return; - _fired = true; - - var outcome = string.IsNullOrEmpty(helloOutcome) ? "unknown" : helloOutcome!; - - _ingress.Post( - kind: DecisionSignalKind.HelloResolved, - occurredAtUtc: _clock.UtcNow, - sourceOrigin: "HelloTracker", - evidence: new Evidence( - kind: EvidenceKind.Derived, - identifier: "hello-tracker-v1", - summary: $"Hello resolved (outcome={outcome})", - derivationInputs: new Dictionary(StringComparer.Ordinal) - { - [SignalPayloadKeys.HelloOutcome] = outcome, - }), - payload: new Dictionary(StringComparer.Ordinal) - { - [SignalPayloadKeys.HelloOutcome] = outcome, - }); - } - } -} diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/SignalAdapters/ProvisioningStatusTrackerAdapter.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/SignalAdapters/ProvisioningStatusTrackerAdapter.cs deleted file mode 100644 index afe6d79f..00000000 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/SignalAdapters/ProvisioningStatusTrackerAdapter.cs +++ /dev/null @@ -1,173 +0,0 @@ -#nullable enable -using System; -using System.Collections.Generic; -using AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals; -using AutopilotMonitor.Agent.V2.Core.Orchestration; -using AutopilotMonitor.DecisionCore.Engine; -using AutopilotMonitor.DecisionCore.Signals; - -namespace AutopilotMonitor.Agent.V2.Core.SignalAdapters -{ - /// - /// Adapter for → 3 DecisionSignalKinds. - /// Plan §2.1a / §2.2. - /// - /// Event mapping: - /// - /// DeviceSetupProvisioningComplete → - /// AccountSetupProvisioningComplete → (session 330f73f3 fix) - /// EspFailureDetected → - /// - /// - /// - /// Duplicate EspTerminalFailure-Signals aus ShellCoreTracker + ProvisioningStatusTracker - /// sind erwartbar (zwei Detection-Quellen für dasselbe Outcome). Der Reducer handled das - /// idempotent — erste Failure gewinnt und setzt Stage auf Failed; weitere sind dann Dead-End. - /// Adapter führt nur lokale per-kind Dedup (innerhalb einer Tracker-Instanz). - /// - /// - internal sealed class ProvisioningStatusTrackerAdapter : IDisposable - { - private readonly ProvisioningStatusTracker _tracker; - private readonly ISignalIngressSink _ingress; - private readonly IClock _clock; - - private bool _deviceSetupCompletePosted; - private bool _accountSetupCompletePosted; - private bool _espFailurePosted; - - public ProvisioningStatusTrackerAdapter( - ProvisioningStatusTracker tracker, - ISignalIngressSink ingress, - IClock clock) - { - _tracker = tracker ?? throw new ArgumentNullException(nameof(tracker)); - _ingress = ingress ?? throw new ArgumentNullException(nameof(ingress)); - _clock = clock ?? throw new ArgumentNullException(nameof(clock)); - - _tracker.DeviceSetupProvisioningComplete += OnDeviceSetupComplete; - _tracker.AccountSetupProvisioningComplete += OnAccountSetupComplete; - _tracker.EspFailureDetected += OnEspFailure; - } - - public void Dispose() - { - _tracker.DeviceSetupProvisioningComplete -= OnDeviceSetupComplete; - _tracker.AccountSetupProvisioningComplete -= OnAccountSetupComplete; - _tracker.EspFailureDetected -= OnEspFailure; - } - - private void OnDeviceSetupComplete(object sender, EventArgs e) => EmitDeviceSetupComplete(); - private void OnAccountSetupComplete(object sender, EventArgs e) => EmitAccountSetupComplete(); - private void OnEspFailure(object sender, EspFailureDetectedEventArgs args) => EmitEspFailure(args); - - internal void TriggerDeviceSetupCompleteFromTest() => EmitDeviceSetupComplete(); - internal void TriggerAccountSetupCompleteFromTest() => EmitAccountSetupComplete(); - internal void TriggerEspFailureFromTest(string failureType) - => EmitEspFailure(new EspFailureDetectedEventArgs(failureType)); - internal void TriggerEspFailureFromTest(EspFailureDetectedEventArgs args) => EmitEspFailure(args); - - private void EmitDeviceSetupComplete() - { - if (_deviceSetupCompletePosted) return; - _deviceSetupCompletePosted = true; - - var snapshot = _tracker.GetProvisioningCategorySnapshot(); - var deviceSetupResolved = snapshot.TryGetValue("DeviceSetup", out var dsState) && dsState.HasValue - ? dsState.Value.ToString().ToLowerInvariant() - : "unknown"; - - _ingress.Post( - kind: DecisionSignalKind.DeviceSetupProvisioningComplete, - occurredAtUtc: _clock.UtcNow, - sourceOrigin: "ProvisioningStatusTracker", - evidence: new Evidence( - kind: EvidenceKind.Derived, - identifier: "provisioning-status-tracker-v1", - summary: "DeviceSetupCategory provisioning completed", - derivationInputs: new Dictionary(StringComparer.Ordinal) - { - ["registrySource"] = @"HKLM\SOFTWARE\Microsoft\Provisioning\AutopilotSettings\DeviceSetupCategory.Status", - ["deviceSetupResolved"] = deviceSetupResolved, - }), - payload: new Dictionary(StringComparer.Ordinal) - { - ["deviceSetupResolved"] = deviceSetupResolved, - }); - } - - private void EmitAccountSetupComplete() - { - if (_accountSetupCompletePosted) return; - _accountSetupCompletePosted = true; - - var snapshot = _tracker.GetProvisioningCategorySnapshot(); - var accountSetupResolved = snapshot.TryGetValue("AccountSetupCategory.Status", out var asState) && asState.HasValue - ? asState.Value.ToString().ToLowerInvariant() - : "unknown"; - - _ingress.Post( - kind: DecisionSignalKind.AccountSetupProvisioningComplete, - occurredAtUtc: _clock.UtcNow, - sourceOrigin: "ProvisioningStatusTracker", - evidence: new Evidence( - kind: EvidenceKind.Derived, - identifier: "provisioning-status-tracker-v1", - summary: "AccountSetupCategory provisioning completed", - derivationInputs: new Dictionary(StringComparer.Ordinal) - { - ["registrySource"] = @"HKLM\SOFTWARE\Microsoft\Provisioning\AutopilotSettings\AccountSetupCategory.Status", - ["accountSetupResolved"] = accountSetupResolved, - }), - payload: new Dictionary(StringComparer.Ordinal) - { - ["accountSetupResolved"] = accountSetupResolved, - }); - } - - private void EmitEspFailure(EspFailureDetectedEventArgs args) - { - if (_espFailurePosted) return; - _espFailurePosted = true; - - var safeFailureType = args?.FailureType ?? "unknown"; - var errorCode = args?.ErrorCode; - var failedSubcategory = args?.FailedSubcategory; - var category = args?.Category; - - var derivationInputs = new Dictionary(StringComparer.Ordinal) - { - ["registrySource"] = @"HKLM\SOFTWARE\Microsoft\Provisioning\AutopilotSettings\*.Status", - ["failureType"] = safeFailureType, - }; - if (!string.IsNullOrEmpty(errorCode)) - derivationInputs["errorCode"] = errorCode!; - if (!string.IsNullOrEmpty(failedSubcategory)) - derivationInputs["failedSubcategory"] = failedSubcategory!; - if (!string.IsNullOrEmpty(category)) - derivationInputs["category"] = category!; - - var payload = new Dictionary(StringComparer.Ordinal) - { - ["failureType"] = safeFailureType, - }; - if (!string.IsNullOrEmpty(errorCode)) - payload["errorCode"] = errorCode!; - if (!string.IsNullOrEmpty(failedSubcategory)) - payload["failedSubcategory"] = failedSubcategory!; - if (!string.IsNullOrEmpty(category)) - payload["category"] = category!; - - _ingress.Post( - kind: DecisionSignalKind.EspTerminalFailure, - occurredAtUtc: _clock.UtcNow, - sourceOrigin: "ProvisioningStatusTracker", - evidence: new Evidence( - kind: EvidenceKind.Derived, - identifier: "provisioning-status-tracker-v1", - summary: $"ESP terminal failure from provisioning registry (type={safeFailureType}, errorCode={errorCode ?? "n/a"})", - derivationInputs: derivationInputs), - payload: payload); - } - } -} diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/SignalAdapters/ShellCoreTrackerAdapter.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/SignalAdapters/ShellCoreTrackerAdapter.cs deleted file mode 100644 index eabf1bd1..00000000 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/SignalAdapters/ShellCoreTrackerAdapter.cs +++ /dev/null @@ -1,167 +0,0 @@ -#nullable enable -using System; -using System.Collections.Generic; -using AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals; -using AutopilotMonitor.Agent.V2.Core.Orchestration; -using AutopilotMonitor.DecisionCore.Engine; -using AutopilotMonitor.DecisionCore.Signals; -using AutopilotMonitor.Shared.Models; - -namespace AutopilotMonitor.Agent.V2.Core.SignalAdapters -{ - /// - /// Adapter for — translates its 4 events into Decision-Signals. - /// Plan §2.1a / §2.2. - /// - /// Event mapping (Plan §2.2 DecisionSignalKind): - /// - /// FinalizingSetupPhaseTriggered → (phase=Finalizing) - /// WhiteGloveCompleted → - /// EspFailureDetected → - /// EspExited → - /// - /// - /// - /// Finalizing / WhiteGloveSuccess / EspFailure are fire-once (ShellCoreTracker has Dedup-Guards; - /// the adapter adds per-kind flags so duplicate Subscribe-Calls or external re-entry can't - /// double-post). EspExiting is intentionally NOT deduped at the adapter — Shell-Core - /// 62407 fires at every ESP phase transition (Device→Account, Account→End), and the - /// reducer (HandleEspExitingV1 + ShouldTransitionToAwaitingHello) decides which occurrence - /// is the genuine post-AccountSetup exit that arms HelloSafety. - /// - /// - internal sealed class ShellCoreTrackerAdapter : IDisposable - { - private readonly ShellCoreTracker _tracker; - private readonly ISignalIngressSink _ingress; - private readonly IClock _clock; - - private bool _finalizingPosted; - private bool _whiteGloveSuccessPosted; - private bool _espFailurePosted; - - public ShellCoreTrackerAdapter( - ShellCoreTracker tracker, - ISignalIngressSink ingress, - IClock clock) - { - _tracker = tracker ?? throw new ArgumentNullException(nameof(tracker)); - _ingress = ingress ?? throw new ArgumentNullException(nameof(ingress)); - _clock = clock ?? throw new ArgumentNullException(nameof(clock)); - - _tracker.FinalizingSetupPhaseTriggered += OnFinalizing; - _tracker.WhiteGloveCompleted += OnWhiteGloveCompleted; - _tracker.EspFailureDetected += OnEspFailure; - _tracker.EspExited += OnEspExited; - } - - public void Dispose() - { - _tracker.FinalizingSetupPhaseTriggered -= OnFinalizing; - _tracker.WhiteGloveCompleted -= OnWhiteGloveCompleted; - _tracker.EspFailureDetected -= OnEspFailure; - _tracker.EspExited -= OnEspExited; - } - - private void OnFinalizing(object sender, string reason) => EmitFinalizing(reason); - private void OnWhiteGloveCompleted(object sender, EventArgs e) => EmitWhiteGloveSuccess(); - private void OnEspFailure(object sender, string failureType) => EmitEspFailure(failureType); - private void OnEspExited(object sender, EspExitedEventArgs args) => EmitEspExiting(args.OccurredAtUtc); - - internal void TriggerFinalizingFromTest(string reason) => EmitFinalizing(reason); - internal void TriggerWhiteGloveCompletedFromTest() => EmitWhiteGloveSuccess(); - internal void TriggerEspFailureFromTest(string failureType) => EmitEspFailure(failureType); - internal void TriggerEspExitingFromTest(DateTime occurredAtUtc) => EmitEspExiting(occurredAtUtc); - - private void EmitFinalizing(string reason) - { - if (_finalizingPosted) return; - _finalizingPosted = true; - - _ingress.Post( - kind: DecisionSignalKind.EspPhaseChanged, - occurredAtUtc: _clock.UtcNow, - sourceOrigin: "ShellCoreTracker", - evidence: new Evidence( - kind: EvidenceKind.Derived, - identifier: "shell-core-tracker-v1", - summary: "ESP Finalizing phase triggered (Shell-Core 62404/62407)", - derivationInputs: new Dictionary(StringComparer.Ordinal) - { - ["eventSource"] = "Microsoft-Windows-Shell-Core", - ["phaseReason"] = reason ?? string.Empty, - }), - payload: new Dictionary(StringComparer.Ordinal) - { - [SignalPayloadKeys.EspPhase] = EnrollmentPhase.FinalizingSetup.ToString(), - ["reason"] = reason ?? string.Empty, - }); - } - - private void EmitWhiteGloveSuccess() - { - if (_whiteGloveSuccessPosted) return; - _whiteGloveSuccessPosted = true; - - _ingress.Post( - kind: DecisionSignalKind.WhiteGloveShellCoreSuccess, - occurredAtUtc: _clock.UtcNow, - sourceOrigin: "ShellCoreTracker", - evidence: new Evidence( - kind: EvidenceKind.Derived, - identifier: "shell-core-tracker-v1", - summary: "WhiteGlove sealing success observed (Shell-Core)", - derivationInputs: new Dictionary(StringComparer.Ordinal) - { - ["eventSource"] = "Microsoft-Windows-Shell-Core", - ["detection"] = "WhiteGloveCompleted event", - })); - } - - private void EmitEspFailure(string failureType) - { - if (_espFailurePosted) return; - _espFailurePosted = true; - - var safeFailureType = string.IsNullOrEmpty(failureType) ? "unknown" : failureType!; - - _ingress.Post( - kind: DecisionSignalKind.EspTerminalFailure, - occurredAtUtc: _clock.UtcNow, - sourceOrigin: "ShellCoreTracker", - evidence: new Evidence( - kind: EvidenceKind.Derived, - identifier: "shell-core-tracker-v1", - summary: $"ESP terminal failure detected (type={safeFailureType})", - derivationInputs: new Dictionary(StringComparer.Ordinal) - { - ["eventSource"] = "Microsoft-Windows-Shell-Core", - ["failureType"] = safeFailureType, - }), - payload: new Dictionary(StringComparer.Ordinal) - { - ["failureType"] = safeFailureType, - }); - } - - // OccurredAtUtc comes from the tracker (live = log time, backfill = record.TimeCreated) - // rather than _clock.UtcNow so HandleEspExitingV1 can floor HelloSafety at the actual - // ESP-exit moment via EffectiveDeadlineBase, not at wall-clock-now on a backfilled run. - private void EmitEspExiting(DateTime occurredAtUtc) - { - _ingress.Post( - kind: DecisionSignalKind.EspExiting, - occurredAtUtc: occurredAtUtc, - sourceOrigin: "ShellCoreTracker", - evidence: new Evidence( - kind: EvidenceKind.Derived, - identifier: "shell-core-tracker-v1", - summary: "ESP exiting (Shell-Core 62407 OOBE_ESP*Exiting)", - derivationInputs: new Dictionary(StringComparer.Ordinal) - { - ["eventSource"] = "Microsoft-Windows-Shell-Core", - ["eventId"] = "62407", - })); - } - } -} 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 cbba6297..60751502 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2/Runtime/AgentRuntimeHost.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2/Runtime/AgentRuntimeHost.cs @@ -333,87 +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: () => componentFactory.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: () => componentFactory.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: () => componentFactory.ImeIgnoredCount, - // 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) - => componentFactory.PromoteActiveInstallsToStuck(failureType, message, errorCode), - // 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: () => componentFactory.LastEspTerminalFailure); + stopPeripheralCollectors: () => orchestrator.StopCollectorHosts()); // ServerActionDispatcher (plan §5.3) — constructed inside this // hook so lifecyclePost + terminationHandler are guaranteed @@ -442,15 +392,16 @@ namespace AutopilotMonitor.Agent.V2.Runtime shutdownComplete, logger); - // Wire the WhiteGlove Part-1 inventory trigger. componentFactory.EspAndHelloHost - // is set inside CreateCollectorHosts (orchestrator.Start step 14, runs after - // the onIngressReady hook above), so by here both it and analyzerManager are - // live. The Action closes over the local `analyzerManager` variable; null-safe - // in case a race ever inverts the order. - if (componentFactory.EspAndHelloHost != null) + // Wire the WhiteGlove Part-1 inventory trigger. The CollectorSurfaces are + // set inside CreateCollectorHosts (orchestrator.Start step 14, runs after + // the onIngressReady hook above), so by here both the EspAndHelloHost and + // analyzerManager are live. The Action closes over the local + // `analyzerManager` variable; null-safe in case a race ever inverts the order. + var espAndHelloHost = orchestrator.CollectorSurfaces?.EspAndHelloHost; + if (espAndHelloHost != null) { whiteGloveInventoryTrigger = new WhiteGloveInventoryTrigger( - host: componentFactory.EspAndHelloHost, + host: espAndHelloHost, onTrigger: () => analyzerManager?.RunWhiteGlovePart1InventorySnapshot(), logger: logger); @@ -459,13 +410,13 @@ namespace AutopilotMonitor.Agent.V2.Runtime // AutoLogon keys. Final shutdown re-scans via RunShutdown for user-phase // changes. Same host + lifecycle as the WG inventory trigger above. autoLogonDeviceSetupTrigger = new AutoLogonDeviceSetupTrigger( - host: componentFactory.EspAndHelloHost, + host: espAndHelloHost, onTrigger: () => analyzerManager?.RunDeviceSetupCompleteAutoLogonCheck(), logger: logger); } else { - logger.Warning("WhiteGloveInventoryTrigger / AutoLogonDeviceSetupTrigger not wired — componentFactory.EspAndHelloHost is null after orchestrator.Start."); + logger.Warning("WhiteGloveInventoryTrigger / AutoLogonDeviceSetupTrigger not wired — CollectorSurfaces.EspAndHelloHost is null after orchestrator.Start."); } // WhiteGlove Part-2 resume: EnrollmentOrchestrator.Start (PR-A) detected @@ -502,7 +453,7 @@ namespace AutopilotMonitor.Agent.V2.Runtime // reboots don't have the placeholder→real-user flow that motivates this. if (!isWhiteGloveResume && EnrollmentRegistryDetector.DetectHybridJoin()) { - try { componentFactory.AadJoinHost?.ArmHybridLoginPendingDetector(); } + try { orchestrator.CollectorSurfaces?.AadJoinHost?.ArmHybridLoginPendingDetector(); } catch (Exception ex) { logger.Warning($"ArmHybridLoginPendingDetector threw: {ex.Message}"); 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 b633d3d9..9a9755e4 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt +++ b/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt @@ -1 +1 @@ -1036 +1058 diff --git a/src/Agent/AutopilotMonitor.Agent/buildcounter.txt b/src/Agent/AutopilotMonitor.Agent/buildcounter.txt index 2a63ccf6..68abbd9a 100644 --- a/src/Agent/AutopilotMonitor.Agent/buildcounter.txt +++ b/src/Agent/AutopilotMonitor.Agent/buildcounter.txt @@ -1 +1 @@ -1439 +1440 diff --git a/src/Agent/AutopilotMonitor.DecisionCore.Tests/RealmJoinGateTests.cs b/src/Agent/AutopilotMonitor.DecisionCore.Tests/RealmJoinGateTests.cs index b61bac79..5d107afb 100644 --- a/src/Agent/AutopilotMonitor.DecisionCore.Tests/RealmJoinGateTests.cs +++ b/src/Agent/AutopilotMonitor.DecisionCore.Tests/RealmJoinGateTests.cs @@ -112,6 +112,25 @@ namespace AutopilotMonitor.DecisionCore.Tests Assert.NotNull(step.NewState.DesktopArrivedUtc); // fact still recorded } + [Fact] + public void Closed_gate_tags_the_deferred_transition_with_the_gate_name_suffix() + { + // ARCH-F1 forward-proof: the defer trigger suffix is derived from the closed gate's + // CompletionGate.Name (":Closed") inside CompleteThroughFinalizingOrDefer, not + // hardcoded at the call site. A new gate registered in s_completionGates inherits the + // same deferral shape with its own suffix — this test locks the derivation so a rename + // or a regression back to per-site hardcoding is caught. + var engine = new DecisionEngine(); + var state = PrimeClassicAwaitingDesktop(engine); + state = engine.Reduce(state, MakeSignal(5, DecisionSignalKind.RealmJoinDetected, T0.AddMinutes(5), + new Dictionary { [DecisionEngine.RealmJoinPayloadKeys.DeploymentPhase] = "100" })).NewState; + + var step = engine.Reduce(state, MakeSignal(6, DecisionSignalKind.DesktopArrived, T0.AddMinutes(6))); + + Assert.True(step.Transition.Taken); + Assert.Equal(nameof(DecisionSignalKind.DesktopArrived) + ":RealmJoinGateClosed", step.Transition.Trigger); + } + [Fact] public void RealmJoinResolved_after_hello_and_desktop_triggers_finalizing_via_classic_path() { diff --git a/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Classic.cs b/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Classic.cs index 2cb57b52..4b6bc61e 100644 --- a/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Classic.cs +++ b/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Classic.cs @@ -227,40 +227,23 @@ namespace AutopilotMonitor.DecisionCore.Engine // This lets the backend see phase_transition(FinalizingSetup) + enrollment_complete // before IsTerminal() fires and EnrollmentTerminationHandler tears down the spool. // - // RealmJoin gate: while RJ is detected and unresolved, the Finalizing transition - // is deferred. Hello+Desktop facts are still recorded; the next RealmJoinResolved - // or RealmJoinTimeout signal releases the deferred completion via + // Completion gates (ARCH-F1): while a gate (e.g. an active RealmJoin deployment) is + // closed the Finalizing transition is deferred. Hello+Desktop facts are still + // recorded; the gate's release handler re-attempts completion via // CompleteIfDeferredOrBookkeep(). - if (desktopAlreadyArrived && RealmJoinGateOpen(state)) + if (desktopAlreadyArrived) { - return TransitionToFinalizing( + return CompleteThroughFinalizingOrDefer( state: state, signal: signal, preparedBuilder: builder, nextStepIndex: nextStep, trigger: nameof(DecisionSignalKind.HelloResolved), - extraLeadingEffects: helloSafetyCancelEffect != null + leadingEffects: helloSafetyCancelEffect != null ? new[] { helloSafetyCancelEffect } : null); } - if (desktopAlreadyArrived) - { - // RJ gate closed: defer Finalizing. Stay in the current stage; the RJ - // resolved/timeout handler will route through CompleteIfDeferredOrBookkeep(). - var deferredState = builder.Build(); - var deferredTransition = BuildTakenTransition( - before: state, - signal: signal, - toStage: state.Stage, - nextStepIndex: nextStep, - trigger: nameof(DecisionSignalKind.HelloResolved) + ":RealmJoinGateClosed"); - var deferredEffects = helloSafetyCancelEffect != null - ? new[] { helloSafetyCancelEffect } - : Array.Empty(); - return new DecisionStep(deferredState, deferredTransition, deferredEffects); - } - builder.WithStage(SessionStage.AwaitingDesktop); var newState = builder.Build(); @@ -377,30 +360,15 @@ namespace AutopilotMonitor.DecisionCore.Engine }; } - if (RealmJoinGateOpen(state)) - { - return TransitionToFinalizing( - state: state, - signal: signal, - preparedBuilder: builder, - nextStepIndex: nextStep, - trigger: nameof(DecisionSignalKind.DesktopArrived) + ":HelloDisabledFastPath", - extraLeadingEffects: extraEffects); - } - - // RJ gate closed: synthetic Hello + Desktop are recorded; defer Finalizing - // until the RJ resolved/timeout handler releases the gate. - var fastPathDeferredState = builder.Build(); - var fastPathDeferredTransition = BuildTakenTransition( - before: state, + // Completion gates (ARCH-F1): synthetic Hello + Desktop are recorded; complete + // through Finalizing when all gates are open, else defer until a gate releases. + return CompleteThroughFinalizingOrDefer( + state: state, signal: signal, - toStage: state.Stage, + preparedBuilder: builder, nextStepIndex: nextStep, - trigger: nameof(DecisionSignalKind.DesktopArrived) + ":HelloDisabledFastPath:RealmJoinGateClosed"); - var fastPathDeferredEffects = extraEffects != null - ? extraEffects.ToArray() - : Array.Empty(); - return new DecisionStep(fastPathDeferredState, fastPathDeferredTransition, fastPathDeferredEffects); + trigger: nameof(DecisionSignalKind.DesktopArrived) + ":HelloDisabledFastPath", + leadingEffects: extraEffects); } // Plan §5 Fix 6: mirror of HandleHelloResolvedV1. When both prerequisites are in, @@ -420,31 +388,17 @@ namespace AutopilotMonitor.DecisionCore.Engine builder.CancelDeadline(DeadlineNames.HelloSafety); } - if (RealmJoinGateOpen(state)) - { - return TransitionToFinalizing( - state: state, - signal: signal, - preparedBuilder: builder, - nextStepIndex: nextStep, - trigger: nameof(DecisionSignalKind.DesktopArrived), - extraLeadingEffects: helloSafetyCancelEffect != null - ? new[] { helloSafetyCancelEffect } - : null); - } - - // RJ gate closed: Desktop fact is recorded; defer Finalizing. - var deferredState = builder.Build(); - var deferredTransition = BuildTakenTransition( - before: state, + // Completion gates (ARCH-F1): Desktop fact recorded; complete through Finalizing + // when all gates are open, else defer. + return CompleteThroughFinalizingOrDefer( + state: state, signal: signal, - toStage: state.Stage, + preparedBuilder: builder, nextStepIndex: nextStep, - trigger: nameof(DecisionSignalKind.DesktopArrived) + ":RealmJoinGateClosed"); - var deferredEffects = helloSafetyCancelEffect != null - ? new[] { helloSafetyCancelEffect } - : Array.Empty(); - return new DecisionStep(deferredState, deferredTransition, deferredEffects); + trigger: nameof(DecisionSignalKind.DesktopArrived), + leadingEffects: helloSafetyCancelEffect != null + ? new[] { helloSafetyCancelEffect } + : null); } // Desktop came first: keep current stage (AwaitingHello or EspAccountSetup) until diff --git a/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Completion.cs b/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Completion.cs new file mode 100644 index 00000000..510c5f8c --- /dev/null +++ b/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Completion.cs @@ -0,0 +1,138 @@ +using System; +using System.Collections.Generic; +using AutopilotMonitor.DecisionCore.Signals; +using AutopilotMonitor.DecisionCore.State; + +namespace AutopilotMonitor.DecisionCore.Engine +{ + // Completion-gate weaving (ARCH-F1). The cross-cutting "may the session complete now?" + // AND-gate used to be hand-inlined at every terminal site across the Classic / Shared / + // SelfDeploying / RealmJoin engine partials, each with its own near-identical + // RealmJoinGateOpen check + ":RealmJoinGateClosed" defer block. That duplication was the + // origin of the premature/duplicate enrollment_complete regressions (8b8d611d, 08c99638, + // 330f73f3). This partial centralizes the gate SET and the deferral shape for the + // completion-ATTEMPT sites — the four Classic/Shared handlers that route a "both + // prerequisites in" decision through CompleteThroughFinalizingOrDefer. Those sites inherit a + // new gate purely by it appearing in s_completionGates. + // + // Two paths intentionally do NOT consult the collection (single-gate-correct today; a second + // gate must wire them explicitly — they are the gate's release/origin, not attempt sites): + // * the RealmJoin release path (CompleteIfDeferredOrBookkeep, DecisionEngine.RealmJoin.cs) + // completes directly because it evaluates the PRE-resolution state where the gate it just + // opened still reads closed; a second gate that must re-block here needs a post-state + // re-check; + // * the SelfDeploying deadline path (HandleDeviceOnlyEspDetectionDeadlineFired) owns a + // gate-specific deferral (SelfDeployingDeferredCompletion on RealmJoinFacts, released by + // the RealmJoin handlers) and so checks RealmJoinGateOpen directly. + public sealed partial class DecisionEngine + { + /// + /// A cross-cutting completion precondition. Completion is deferred while any gate is + /// closed; the deferring transition is tagged :<Name>Closed until a later + /// signal re-opens the gate and a completion-release handler re-attempts. + /// + private readonly struct CompletionGate + { + public CompletionGate(string name, Func isOpen) + { + Name = name; + IsOpen = isOpen; + } + + /// Stable gate identifier; forms the :<Name>Closed trigger suffix. + public string Name { get; } + + /// Returns true when this precondition no longer blocks completion. + public Func IsOpen { get; } + } + + /// + /// The ordered set of completion preconditions. Append-only: a new enrollment + /// scenario registers one gate here and every completion-ATTEMPT site that routes through + /// inherits it with no per-site edits. + /// RealmJoin is the only gate today. Note this collection does NOT cover the RealmJoin + /// release path or the SelfDeploying deadline path — both are gate-specific by design + /// (see the class-level remarks); a second gate that must block there needs explicit + /// wiring, not just an entry here. + /// + private static readonly CompletionGate[] s_completionGates = + { + // An active RealmJoin deployment (detected, not yet resolved / timed-out) blocks + // completion until RealmJoinResolved (phase 110) or the 60-min hard timeout fires. + new CompletionGate("RealmJoinGate", RealmJoinGateOpen), + }; + + /// + /// Returns the of the first closed completion gate, or + /// null when every gate is open (completion may proceed). Centralizes the + /// cross-cutting AND-gate (ARCH-F1). + /// + private static string? FirstClosedCompletionGate(DecisionState state) + { + foreach (var gate in s_completionGates) + { + if (!gate.IsOpen(state)) return gate.Name; + } + return null; + } + + /// + /// Shared completion router for every "both prerequisites in → finish via Finalizing" + /// site (Classic Hello/Desktop handlers, the Hello-disabled fast-path, the Hello-safety + /// timeout, and the RealmJoin release path). When all completion gates are open this + /// routes through ; when a gate is closed it emits + /// the canonical defer step — current stage unchanged, trigger tagged + /// :<Gate>Closed, the same preserved, no + /// deadline armed — so a later gate-release handler re-attempts completion. + /// + /// The caller has already populated with its + /// signal-specific facts (HelloResolvedUtc / DesktopArrivedUtc / Hello-safety cancel). + /// + /// + private DecisionStep CompleteThroughFinalizingOrDefer( + DecisionState state, + DecisionSignal signal, + DecisionStateBuilder preparedBuilder, + int nextStepIndex, + string trigger, + IReadOnlyList? leadingEffects = null) + { + var closedGate = FirstClosedCompletionGate(state); + if (closedGate == null) + { + return TransitionToFinalizing( + state: state, + signal: signal, + preparedBuilder: preparedBuilder, + nextStepIndex: nextStepIndex, + trigger: trigger, + extraLeadingEffects: leadingEffects); + } + + // Gate closed: defer Finalizing. Stay in the current stage; the gate's resolved/ + // timeout handler routes back through completion once it re-opens. + var deferredState = preparedBuilder.Build(); + var deferredTransition = BuildTakenTransition( + before: state, + signal: signal, + toStage: state.Stage, + nextStepIndex: nextStepIndex, + trigger: trigger + ":" + closedGate + "Closed"); + return new DecisionStep(deferredState, deferredTransition, MaterializeEffects(leadingEffects)); + } + + /// + /// Materialize an optional effect list into the array shape the + /// defer path expects. Returns the shared empty array when there is nothing to emit, and + /// passes a backing array through untouched to avoid a copy. + /// + private static DecisionEffect[] MaterializeEffects(IReadOnlyList? effects) + { + if (effects == null || effects.Count == 0) return Array.Empty(); + if (effects is DecisionEffect[] arr) return arr; + var copy = new DecisionEffect[effects.Count]; + for (var i = 0; i < effects.Count; i++) copy[i] = effects[i]; + return copy; + } + } +} diff --git a/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.RealmJoin.cs b/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.RealmJoin.cs index d7b53712..dc32a255 100644 --- a/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.RealmJoin.cs +++ b/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.RealmJoin.cs @@ -412,6 +412,12 @@ namespace AutopilotMonitor.DecisionCore.Engine if (classicReady) { + // This IS the gate-release path: the RealmJoin gate has just opened (the caller + // wrote WithResolved / WithTimeoutOutcome into preparedBuilder), so complete + // directly. Note the completion gates read `state` (pre-resolution), where RJ is + // still closed — routing this through CompleteThroughFinalizingOrDefer would + // re-defer on the very gate we just released. A future second gate that must + // re-block here would re-check the *post* state (WDP-v2 follow-up, ARCH-F1). var extra = leadingEffect != null ? new[] { leadingEffect } : null; return TransitionToFinalizing( state: state, diff --git a/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Shared.cs b/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Shared.cs index 15ac0e49..5b36099b 100644 --- a/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Shared.cs +++ b/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Shared.cs @@ -409,12 +409,12 @@ namespace AutopilotMonitor.DecisionCore.Engine // synthetic-timeout terminal event shares the same grace window + phase-declaration // pathway as the happy-path handlers. AwaitingDesktop path is unchanged. // - // RealmJoin gate: while RJ is detected and unresolved, defer Finalizing. The - // synthetic Hello-timeout fact stays recorded; the RJ resolved/timeout handler - // routes through CompleteIfDeferredOrBookkeep when both gates are ready. - if (desktopAlreadyArrived && RealmJoinGateOpen(state)) + // Completion gates (ARCH-F1): while a gate (e.g. an active RealmJoin deployment) is + // closed, defer Finalizing. The synthetic Hello-timeout fact stays recorded; the + // gate's release handler routes through CompleteIfDeferredOrBookkeep. + if (desktopAlreadyArrived) { - return TransitionToFinalizing( + return CompleteThroughFinalizingOrDefer( state: state, signal: signal, preparedBuilder: builder, @@ -422,18 +422,6 @@ namespace AutopilotMonitor.DecisionCore.Engine trigger: $"DeadlineFired:{DeadlineNames.HelloSafety}"); } - if (desktopAlreadyArrived) - { - var deferredState = builder.Build(); - var deferredTransition = BuildTakenTransition( - before: state, - signal: signal, - toStage: state.Stage, - nextStepIndex: nextStep, - trigger: $"DeadlineFired:{DeadlineNames.HelloSafety}:RealmJoinGateClosed"); - return new DecisionStep(deferredState, deferredTransition, Array.Empty()); - } - builder.WithStage(SessionStage.AwaitingDesktop); var newState = builder.Build();