diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Orchestration/RecoveryPathTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Orchestration/RecoveryPathTests.cs index e03c1ac7..1060f2b9 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Orchestration/RecoveryPathTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Orchestration/RecoveryPathTests.cs @@ -829,5 +829,132 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Orchestration var task = (System.Threading.Tasks.Task)method!.Invoke(sut, new object?[] { CancellationToken.None })!; return task.GetAwaiter().GetResult(); } + + // ============================================================ Terminal-stage-on-load recovery + // + // Bug captured in session 17ef0e0e-… (2026-05-19): an agent killed mid-shutdown + // restarts, RecoveryPath loads a snapshot already at Stage=Completed, but the + // DecisionStepProcessor's OnDecisionTerminalStage callback only fires on a real + // reducer TRANSITION into a terminal stage. A pre-terminal load produces no + // transition → no callback → EnrollmentTerminationHandler never runs → marker + // never written → SelfDestruct never invoked → agent idles for the full + // AgentMaxLifetime (default 6h) emitting periodic perf/metrics events. + // + // The fix at the end of EnrollmentOrchestrator.Start synthesises the missing + // invocation when initialState.Stage ∈ {Completed, Failed}, so the regular + // termination pipeline runs immediately. + + [Fact] + public void Completed_snapshot_fires_Terminated_with_succeeded_outcome_on_start() + { + using var rig = new Rig(); + Directory.CreateDirectory(rig.StateDir); + + // Seed a snapshot whose Stage is already Completed — simulates an agent + // killed by reboot/Ctrl+C between agent_shutting_down and marker write. + var terminalState = DecisionState.CreateInitial("S1", "T1") + .ToBuilder() + .WithStage(SessionStage.Completed) + .WithStepIndex(42) + .Build(); + new SnapshotPersistence( + Path.Combine(rig.StateDir, "snapshot.json"), + () => rig.Clock.UtcNow).Save(terminalState); + + var sut = rig.Build(); + + EnrollmentTerminatedEventArgs? captured = null; + using var fired = new ManualResetEventSlim(false); + sut.Terminated += (_, args) => + { + captured = args; + fired.Set(); + }; + + sut.Start(); + try + { + Assert.True(fired.Wait(TimeSpan.FromSeconds(5)), + "Terminated event did not fire within 5s after Start with terminal initial state."); + Assert.NotNull(captured); + Assert.Equal(EnrollmentTerminationReason.DecisionTerminalStage, captured!.Reason); + Assert.Equal(EnrollmentTerminationOutcome.Succeeded, captured.Outcome); + Assert.Equal(SessionStage.Completed.ToString(), captured.StageName); + } + finally { sut.Stop(); } + } + + [Fact] + public void Failed_snapshot_fires_Terminated_with_failed_outcome_on_start() + { + using var rig = new Rig(); + Directory.CreateDirectory(rig.StateDir); + + var terminalState = DecisionState.CreateInitial("S1", "T1") + .ToBuilder() + .WithStage(SessionStage.Failed) + .WithStepIndex(17) + .Build(); + new SnapshotPersistence( + Path.Combine(rig.StateDir, "snapshot.json"), + () => rig.Clock.UtcNow).Save(terminalState); + + var sut = rig.Build(); + + EnrollmentTerminatedEventArgs? captured = null; + using var fired = new ManualResetEventSlim(false); + sut.Terminated += (_, args) => + { + captured = args; + fired.Set(); + }; + + sut.Start(); + try + { + Assert.True(fired.Wait(TimeSpan.FromSeconds(5)), + "Terminated event did not fire within 5s after Start with Failed initial state."); + Assert.NotNull(captured); + Assert.Equal(EnrollmentTerminationReason.DecisionTerminalStage, captured!.Reason); + Assert.Equal(EnrollmentTerminationOutcome.Failed, captured.Outcome); + Assert.Equal(SessionStage.Failed.ToString(), captured.StageName); + } + finally { sut.Stop(); } + } + + [Fact] + public void Non_terminal_snapshot_does_NOT_fire_Terminated_on_start() + { + // Negative control: a mid-enrollment snapshot must keep the normal recovery + // path (no synthetic Terminated fire). Without this guard the recovery hook + // would short-circuit legitimate in-flight sessions on every restart. + using var rig = new Rig(); + Directory.CreateDirectory(rig.StateDir); + + var midState = DecisionState.CreateInitial("S1", "T1") + .ToBuilder() + .WithStage(SessionStage.EspDeviceSetup) + .WithStepIndex(3) + .Build(); + new SnapshotPersistence( + Path.Combine(rig.StateDir, "snapshot.json"), + () => rig.Clock.UtcNow).Save(midState); + + var sut = rig.Build(); + + using var fired = new ManualResetEventSlim(false); + sut.Terminated += (_, _) => fired.Set(); + + sut.Start(); + try + { + // 500ms is plenty: the recovery path fires synchronously off Task.Run. + // If it were going to fire, it would have hit within tens of milliseconds. + Assert.False(fired.Wait(TimeSpan.FromMilliseconds(500)), + "Terminated event fired despite non-terminal initial state — recovery hook over-fires."); + Assert.Equal(SessionStage.EspDeviceSetup, sut.CurrentState.Stage); + } + finally { sut.Stop(); } + } } } diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/EnrollmentOrchestrator.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/EnrollmentOrchestrator.cs index 5e7ce3f9..1931dd02 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/EnrollmentOrchestrator.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/EnrollmentOrchestrator.cs @@ -814,6 +814,34 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration var hostCount = _collectorHosts?.Count ?? 0; var maxLifetimeMin = _agentMaxLifetime.HasValue ? _agentMaxLifetime.Value.TotalMinutes.ToString("F0") + "min" : "off"; _logger.Info($"EnrollmentOrchestrator: started (sessionId={sessionShort}, initialStage={initialStage}, hosts={hostCount}, maxLifetime={maxLifetimeMin})."); + + // 17) Terminal-stage-on-load recovery. When recovery yields an initialState whose + // Stage is already terminal (Completed/Failed), no further reducer transition + // fires, so the regular OnDecisionTerminalStage path on the DecisionStepProcessor + // never triggers. Without this branch the agent would sit idle past completion + // until the AgentMaxLifetime watchdog (default 6h) trips — emitting periodic + // performance_snapshot / agent_metrics_snapshot events the whole time + // (observed in session 17ef0e0e-… 2026-05-19). Symmetric to the WhiteGlove + // Part-1 archive-and-reset at step 0: that path is handled there and the + // snapshot is gone by the time we reach this point, so the Completed/Failed + // check below is safe (cannot accidentally tear down a Part-2 resume). + // + // We reuse OnDecisionTerminalStage rather than re-implementing the dispatch + // so the existing _terminatedFired interlock, max-lifetime-timer disposal, + // and Task.Run off-loader all apply. The Terminated handler (terminationHandler + // in AgentRuntimeHost) is wired BEFORE Start() and the EnrollmentTerminationHandler + // itself is constructed inside the onIngressReady hook (step 13b), which has + // already fired by this point — so when our recovery-fire propagates through + // Task.Run, the handler is fully wired. + if (initialState.Stage == SessionStage.Completed || initialState.Stage == SessionStage.Failed) + { + _logger.Warning( + $"EnrollmentOrchestrator: post-recovery initialState already terminal " + + $"(stage={initialState.Stage}, stepIndex={initialState.StepIndex}) — " + + $"invoking OnDecisionTerminalStage directly. Prior run died before the " + + $"normal termination handler could complete (no transition will fire on its own)."); + OnDecisionTerminalStage(initialState); + } } /// diff --git a/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt b/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt index 5fa2986d..b3fa96d2 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt +++ b/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt @@ -1 +1 @@ -815 +819 diff --git a/src/Agent/AutopilotMonitor.Agent/buildcounter.txt b/src/Agent/AutopilotMonitor.Agent/buildcounter.txt index a9b14017..6ee29474 100644 --- a/src/Agent/AutopilotMonitor.Agent/buildcounter.txt +++ b/src/Agent/AutopilotMonitor.Agent/buildcounter.txt @@ -1 +1 @@ -1383 +1384 diff --git a/src/Web/autopilot-monitor-web/app/sessions/[sessionId]/hooks/useSessionEvents.ts b/src/Web/autopilot-monitor-web/app/sessions/[sessionId]/hooks/useSessionEvents.ts index 21412a60..3771a71b 100644 --- a/src/Web/autopilot-monitor-web/app/sessions/[sessionId]/hooks/useSessionEvents.ts +++ b/src/Web/autopilot-monitor-web/app/sessions/[sessionId]/hooks/useSessionEvents.ts @@ -5,6 +5,7 @@ import { api } from "@/lib/api"; import { authenticatedFetch, TokenExpiredError } from "@/lib/authenticatedFetch"; import { extractContinuation, MAX_EAGER_PAGES } from "@/lib/paginationLink"; import { isGuid } from "@/utils/inputValidation"; +import { isTerminalStatus } from "@/utils/sessionStatus"; import { EnrollmentEvent, Session } from "@/types"; import type { NotificationType } from "@/contexts/NotificationContext"; @@ -225,11 +226,16 @@ export function useSessionEvents({ }, []); // Fallback polling only while SignalR is disconnected. + // Symmetric with the SignalR eventStream gate: once the session has reached a terminal + // status, suppress the 30s refetch loop too — late agent perf/metrics snapshots would + // otherwise still trigger refetches, defeating the SignalR-side gate. useEffect(() => { const effectiveTenantId = resolveEffectiveTenantId(); if (!sessionId || !effectiveTenantId || isConnected) return; const interval = setInterval(() => { - if (document.visibilityState === "visible") fetchEvents(); + if (document.visibilityState !== "visible") return; + if (isTerminalStatus(sessionRef.current?.status)) return; + fetchEvents(); }, 30_000); return () => clearInterval(interval); // eslint-disable-next-line react-hooks/exhaustive-deps diff --git a/src/Web/autopilot-monitor-web/app/sessions/[sessionId]/hooks/useSessionSignalR.ts b/src/Web/autopilot-monitor-web/app/sessions/[sessionId]/hooks/useSessionSignalR.ts index 33683493..5593ce21 100644 --- a/src/Web/autopilot-monitor-web/app/sessions/[sessionId]/hooks/useSessionSignalR.ts +++ b/src/Web/autopilot-monitor-web/app/sessions/[sessionId]/hooks/useSessionSignalR.ts @@ -2,6 +2,7 @@ import { useEffect, useRef } from "react"; import { Session, RuleResult } from "@/types"; +import { isTerminalStatus } from "@/utils/sessionStatus"; interface SignalRApi { // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -20,6 +21,7 @@ interface UseSessionSignalRParams { sessionTenantIdFromSession: string | undefined; globalAdminMode: boolean; sessionIdRef: React.MutableRefObject; + sessionRef: React.MutableRefObject; resolveEffectiveTenantId: () => string | null; signalR: SignalRApi; scheduleFetchEvents: (delayMs?: number) => void; @@ -42,6 +44,7 @@ export function useSessionSignalR({ sessionTenantIdFromSession, globalAdminMode, sessionIdRef, + sessionRef, resolveEffectiveTenantId, signalR, scheduleFetchEvents, @@ -97,10 +100,19 @@ export function useSessionSignalR({ console.log('Event stream signal received via SignalR:', data); if (data.sessionId !== sessionIdRef.current) return; - // Fetch full events from storage (single source of truth), but debounce bursts. - // Session updates arrive via the "newevents" message (tenant group) — no session - // object in this signal to keep payloads minimal. - scheduleFetchEvents(); + // Skip the full event refetch once the session has reached a terminal status — + // agents in the post-completion grace period keep emitting performance/metrics + // snapshots every 15-30s, which would otherwise cause the timeline to thrash and + // scroll-jump while the user is reading. The initial page load already grabbed + // everything; the user can refresh manually if they want to see late trailing events. + // Rule-result + tenant-id side effects still run. + const status = sessionRef.current?.status; + if (!isTerminalStatus(status)) { + // Fetch full events from storage (single source of truth), but debounce bursts. + // Session updates arrive via the "newevents" message (tenant group) — no session + // object in this signal to keep payloads minimal. + scheduleFetchEvents(); + } if (data.tenantId) { setSessionTenantId(prev => prev || data.tenantId); diff --git a/src/Web/autopilot-monitor-web/app/sessions/[sessionId]/page.tsx b/src/Web/autopilot-monitor-web/app/sessions/[sessionId]/page.tsx index ef122911..735acfb6 100644 --- a/src/Web/autopilot-monitor-web/app/sessions/[sessionId]/page.tsx +++ b/src/Web/autopilot-monitor-web/app/sessions/[sessionId]/page.tsx @@ -91,6 +91,7 @@ export default function SessionDetailPage() { sessionTenantIdFromSession: detail.session?.tenantId, globalAdminMode, sessionIdRef: detail.sessionIdRef, + sessionRef: detail.sessionRef, resolveEffectiveTenantId: detail.resolveEffectiveTenantId, signalR: { on, off, isConnected, joinGroup, leaveGroup }, scheduleFetchEvents: eventsApi.scheduleFetchEvents, diff --git a/src/Web/autopilot-monitor-web/lib/__tests__/sessionStatus.test.ts b/src/Web/autopilot-monitor-web/lib/__tests__/sessionStatus.test.ts new file mode 100644 index 00000000..2bad16f3 --- /dev/null +++ b/src/Web/autopilot-monitor-web/lib/__tests__/sessionStatus.test.ts @@ -0,0 +1,27 @@ +import { describe, it, expect } from "vitest"; +import { isTerminalStatus } from "../../utils/sessionStatus"; + +describe("isTerminalStatus", () => { + it("returns true for terminal statuses", () => { + expect(isTerminalStatus("Succeeded")).toBe(true); + expect(isTerminalStatus("Failed")).toBe(true); + }); + + it("returns false for non-terminal statuses (session can still receive events)", () => { + expect(isTerminalStatus("InProgress")).toBe(false); + expect(isTerminalStatus("Pending")).toBe(false); + expect(isTerminalStatus("Stalled")).toBe(false); + expect(isTerminalStatus("Unknown")).toBe(false); + }); + + it("returns false for null / undefined / empty (status not yet hydrated)", () => { + expect(isTerminalStatus(null)).toBe(false); + expect(isTerminalStatus(undefined)).toBe(false); + expect(isTerminalStatus("")).toBe(false); + }); + + it("is case-sensitive (matches backend enum casing)", () => { + expect(isTerminalStatus("succeeded")).toBe(false); + expect(isTerminalStatus("FAILED")).toBe(false); + }); +}); diff --git a/src/Web/autopilot-monitor-web/utils/sessionStatus.ts b/src/Web/autopilot-monitor-web/utils/sessionStatus.ts new file mode 100644 index 00000000..bfdc8598 --- /dev/null +++ b/src/Web/autopilot-monitor-web/utils/sessionStatus.ts @@ -0,0 +1,8 @@ +// Session status helpers. Mirror of the backend SessionStatus enum +// (src/Shared/AutopilotMonitor.Shared/Models/SessionApiModels.cs). +// Non-terminal: InProgress, Pending, Stalled, Unknown. +// Terminal: Succeeded, Failed. + +export function isTerminalStatus(status: string | null | undefined): boolean { + return status === "Succeeded" || status === "Failed"; +}