mirror of
https://github.com/okieselbach/AutopilotMonitor.git
synced 2026-10-24 12:42:38 +00:00
fix(agent/v2,web): recover terminal-stage on restart + quiet UI for terminal sessions
Agent V2: when RecoveryPath loads a snapshot already at Stage=Completed/Failed, no reducer transition fires so OnDecisionTerminalStage was never invoked — the agent idled until AgentMaxLifetime (default 6h), emitting periodic perf/metrics events the whole time. Fire it explicitly at the end of Start() so the EnrollmentTerminationHandler runs (marker write + diagnostics + self-destruct). The WhiteGloveSealed archive-and-reset at step 0 already neutralises the Part-2 case; non-terminal stages take the normal recovery path. Web: gate the SignalR eventStream refetch and the 30s fallback polling on non-terminal session status. Once Succeeded/Failed, late perf/metrics arrivals no longer trigger a full-page refetch + re-render that scroll-jumps the timeline while the user is reading. Observed in session 17ef0e0e-... (2026-05-19): 4x enrollment_complete, only 1x agent_shutting_down; reboot mid-shutdown left subsequent agent instances running for hours emitting noise. Tests: 3 new in RecoveryPathTests (Completed/Failed -> Terminated, non-terminal -> no Terminated), 4 in sessionStatus.test.ts. Suites grun: Web 384/384, Agent V2.Core 1095/1095, Backend 1844/1844. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -829,5 +829,132 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Orchestration
|
||||
var task = (System.Threading.Tasks.Task<DrainResult>)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(); }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
||||
@@ -1 +1 @@
|
||||
815
|
||||
819
|
||||
|
||||
@@ -1 +1 @@
|
||||
1383
|
||||
1384
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<string>;
|
||||
sessionRef: React.MutableRefObject<Session | null>;
|
||||
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);
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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";
|
||||
}
|
||||
Reference in New Issue
Block a user