From 56d378f2b261c2d253e8b834b22d72091b8bcc51 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 8 Jul 2026 14:17:18 +0000 Subject: [PATCH] =?UTF-8?q?feat(backend):=20PR2=20=E2=80=94=20reclassify?= =?UTF-8?q?=20timed-out=20sessions=20in=20the=20sweep?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wire EnrollmentTimeoutClassifier into MarkStalledSessionsAsTimedOutAsync so the 5h inactivity sweep stops blindly failing silent-but-provisioned sessions (docs/design/enrollment-status-reclassification.md). Forward-only. - TenantConfiguration.SessionGraceHours (default 72h) + default config. - Sweep Stage 2: classify from the ESP subcategory rollup instead of hard Failed. Fast-path skips within-grace AwaitingUser sessions with NO event read (grace is time-based; late completions are caught by ingest). Per-outcome counters; audit + ops event only when the pass actually terminalized/transitioned something. - session_timeout event + auto-analyze kept ONLY for Failed/Incomplete (parity with the agent max-lifetime path); AwaitingUser is non-terminal, Succeeded is a reconcile. - GetStalledSessionsAsync includes AwaitingUser so it graduates to Incomplete once grace elapses; agent-silent (Stage 1) stays InProgress-only. - UpdateSessionStatusAsync: Incomplete is terminal (idempotency); AwaitingUser can't regress a terminal; CompletedAt stamped for Incomplete (no skewing duration); reason + snapshot persisted for Incomplete/AwaitingUser. Full Functions.Tests suite: 2670 pass. --- ...enrollment-status-reclassification-plan.md | 30 ++--- .../Services/MaintenanceService.cs | 114 ++++++++++++------ .../TableStorageService.Maintenance.cs | 9 +- .../Services/TableStorageService.Sessions.cs | 47 ++++++-- .../Models/Config/TenantConfiguration.cs | 12 ++ 5 files changed, 147 insertions(+), 65 deletions(-) diff --git a/docs/design/enrollment-status-reclassification-plan.md b/docs/design/enrollment-status-reclassification-plan.md index 42ce90c2..57cbcf73 100644 --- a/docs/design/enrollment-status-reclassification-plan.md +++ b/docs/design/enrollment-status-reclassification-plan.md @@ -23,21 +23,23 @@ Backend + stats only — no agent changes, no heartbeat. - [x] Unit tests `EnrollmentTimeoutClassifierTests` (12) — rollup extraction (0/5, 1–4/5, 5/5, fallback, missing) + every classification branch. **Full Functions.Tests: 2670 pass.** -## PR2 — Sweep uses the classifier + grace window +## PR2 — Sweep uses the classifier + grace window ✅ done -- [ ] `TenantConfiguration.SessionGraceHours` (default 72) + config plumbing. -- [ ] In `MarkStalledSessionsAsTimedOutAsync` (Stage 2, `MaintenanceService.cs:~360`): - replace the hard-coded `SessionStatus.Failed` with `ClassifyTimedOutSession(...)`. - - 5h reached, DeviceSetup succeeded, within grace → `AwaitingUser` (non-terminal). - - grace expired → `Incomplete`. - - AccountSetup all-succeeded / `enrollment_complete` seen → `Succeeded`. - - explicit failure event → `Failed` (unchanged). -- [ ] Keep the synthetic `session_timeout` event + analyze enqueue ONLY for the real - `Failed`/`Incomplete` terminal transitions (not for `AwaitingUser`). -- [ ] `GetStalledSessionsAsync` / `GetAgentSilentSessionsAsync` must re-scan `AwaitingUser` - so the grace→`Incomplete` transition fires on a later pass. -- [ ] Tests: `SessionTimeoutRuleTests` / `MaintenanceServiceSessionTimeoutEventTests` extended - for the new target states + grace boundary. +- [x] `TenantConfiguration.SessionGraceHours` (default 72) + default-config plumbing. +- [x] `MarkStalledSessionsAsTimedOutAsync` (Stage 2): replaced hard-coded `Failed` with + `EnrollmentTimeoutClassifier.ClassifyTimedOutSession(...)`. Fast-path skips within-grace + `AwaitingUser` sessions with NO event read (grace is time-based). Per-outcome counters + + audit/ops only when the pass did something. +- [x] Synthetic `session_timeout` event + analyze enqueue kept ONLY for `Failed`/`Incomplete` + terminal transitions (not `AwaitingUser`/`Succeeded`). +- [x] `GetStalledSessionsAsync` now includes `AwaitingUser` so grace→`Incomplete` fires later. + `GetAgentSilentSessionsAsync` left InProgress-only (no regress). +- [x] `UpdateSessionStatusAsync`: `Incomplete` treated as terminal (idempotency guard); + `AwaitingUser` never regresses a terminal; `CompletedAt` stamped for `Incomplete` (no + duration); reason + snapshot persisted for `Incomplete`/`AwaitingUser`. +- [x] Decision logic + grace boundary covered by `EnrollmentTimeoutClassifierTests` (pure — + the sweep itself has ~17 deps, same rationale as `MaintenanceServiceSessionTimeoutEventTests`). + **Full Functions.Tests: 2670 pass.** End-to-end validated via the live forward-only watch. ## PR3 — Reconciliation of late completions diff --git a/src/Backend/AutopilotMonitor.Functions/Services/MaintenanceService.cs b/src/Backend/AutopilotMonitor.Functions/Services/MaintenanceService.cs index 5ec45385..1a9dca97 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/MaintenanceService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/MaintenanceService.cs @@ -279,6 +279,7 @@ namespace AutopilotMonitor.Functions.Services { var config = await _tenantConfigService.GetConfigurationAsync(tenantId); var timeoutHours = config?.SessionTimeoutHours ?? 5; + var graceHours = config?.SessionGraceHours ?? 72; const int agentSilenceHours = 2; // fixed policy: 2h silence → Stalled intermediate var now = DateTime.UtcNow; var cutoffTime = now.AddHours(-timeoutHours); @@ -331,19 +332,23 @@ namespace AutopilotMonitor.Functions.Services continue; } - int sessionCount = 0; + int terminalizedCount = 0; // Failed + Incomplete — the real "timed out" outcomes + int awaitingCount = 0; // reached timeout but Device Setup done → AwaitingUser (non-terminal) + int reconciledCount = 0; // late completion detected at the sweep → Succeeded foreach (var session in stalledSessions) { - // Hybrid User-Driven completion-gap fix (2026-05-01): before - // graduating the session to terminal Failed, build a compact - // snapshot of "what we last knew" so operators don't have to - // scroll through hundreds of events to reconstruct where the - // session was stuck. Best-effort — a snapshot read failure - // must never block the timeout transition itself. + // Fast path: an AwaitingUser session still inside the grace window needs no + // work and — crucially — no event read (grace is purely time-based). A late + // completion is picked up by the ingest path, so we don't re-scan every pass. + var elapsedHours = (now - session.StartedAt).TotalHours; + if (session.Status == SessionStatus.AwaitingUser && elapsedHours < graceHours) + continue; + + // Build a compact snapshot of "what we last knew" (Hybrid User-Driven + // completion-gap fix, 2026-05-01) plus the ESP rollup that drives the + // reclassification. Best-effort — a read failure must never block the sweep. string? snapshotJson = null; - // Reused below to assign the synthetic session_timeout a Sequence after the - // session's last event so it sorts last in the canonical Sequence order. List sessionEvents = new(); try { @@ -354,26 +359,47 @@ namespace AutopilotMonitor.Functions.Services catch (Exception snapEx) { _logger.LogWarning(snapEx, - $"Failed to build failure snapshot for session {session.SessionId}; proceeding with timeout transition without snapshot"); + $"Failed to read events for session {session.SessionId}; proceeding with timeout classification without snapshot"); } + // Timeout ≠ failure. Classify from the ESP subcategory rollup the agent already + // emits instead of blindly failing every silent session + // (docs/design/enrollment-status-reclassification.md): explicit failure → Failed; + // Account Setup all-succeeded / enrollment_complete → Succeeded (reconcile); + // Device Setup done + within grace → AwaitingUser, else Incomplete; silent before + // Device Setup → Incomplete. Never Failed without an explicit failure signal. + var rollup = EnrollmentTimeoutClassifier.ExtractRollup(sessionEvents); + var (targetStatus, reason) = EnrollmentTimeoutClassifier.ClassifyTimedOutSession( + rollup, session.StartedAt, now, graceHours); + + // No-op if the verdict equals the current (non-terminal) state. + if (targetStatus == session.Status) + continue; + var transitioned = await _sessionRepo.UpdateSessionStatusAsync( session.TenantId, session.SessionId, - SessionStatus.Failed, - failureReason: $"Session timed out after {timeoutHours} hours (started at {session.StartedAt:yyyy-MM-dd HH:mm:ss} UTC)", + targetStatus, + failureReason: reason, failureSnapshotJson: snapshotJson ); - sessionCount++; - // Parity with the agent max-lifetime path: the status flip alone is invisible to the - // analyze pipeline (it fires on stream events, not on a repo status change), so a - // server-timed-out session would otherwise analyze to totalIssues:0. Emit a terminal - // session_timeout event into the stream and enqueue auto-analyze so ANALYZE-ENRL-002 - // fires. Gated on the real first transition (transitioned==true) so a re-run or an - // already-terminal session can never produce a duplicate event. Best-effort — a - // failure here must never break the maintenance sweep. - if (transitioned) + if (!transitioned) + continue; + + switch (targetStatus) + { + case SessionStatus.AwaitingUser: awaitingCount++; break; + case SessionStatus.Succeeded: reconciledCount++; break; + default: terminalizedCount++; break; // Failed / Incomplete + } + + // Only the terminal "sweep-terminalized" outcomes (Failed / Incomplete) get the + // synthetic session_timeout event + auto-analyze, so the analyze pipeline + // (ANALYZE-ENRL-002) still has a terminal event to fire on — parity with the agent + // max-lifetime path. AwaitingUser is non-terminal and Succeeded is a reconcile, so + // neither emits a timeout artifact. Best-effort — a failure here must not break the sweep. + if (targetStatus == SessionStatus.Failed || targetStatus == SessionStatus.Incomplete) { try { @@ -391,28 +417,42 @@ namespace AutopilotMonitor.Functions.Services catch (Exception emitEx) { _logger.LogWarning(emitEx, - $"Failed to emit session_timeout event / enqueue analyze for session {session.SessionId}; timeout transition stands"); + $"Failed to emit session_timeout event / enqueue analyze for session {session.SessionId}; transition stands"); } } } - totalSessionsTimedOut += sessionCount; + totalSessionsTimedOut += terminalizedCount; - await _maintenanceRepo.LogAuditEntryAsync( - tenantId, - "SessionTimeout", - "Session", - $"{sessionCount} sessions", - "System.Maintenance", - new Dictionary - { - { "SessionsTimedOut", sessionCount.ToString() }, - { "TimeoutHours", timeoutHours.ToString() }, - { "CutoffTime", cutoffTime.ToString("yyyy-MM-dd HH:mm:ss") } - }); + // Only record when the sweep actually did something this pass — otherwise every + // 2h tick over a backlog of within-grace AwaitingUser sessions would spam a 0-count + // audit entry + ops event. + if (terminalizedCount + awaitingCount + reconciledCount > 0) + { + await _maintenanceRepo.LogAuditEntryAsync( + tenantId, + "SessionTimeout", + "Session", + $"{terminalizedCount} sessions", + "System.Maintenance", + new Dictionary + { + { "SessionsTimedOut", terminalizedCount.ToString() }, + { "SessionsAwaitingUser", awaitingCount.ToString() }, + { "SessionsReconciled", reconciledCount.ToString() }, + { "TimeoutHours", timeoutHours.ToString() }, + { "GraceHours", graceHours.ToString() }, + { "CutoffTime", cutoffTime.ToString("yyyy-MM-dd HH:mm:ss") } + }); - _logger.LogInformation($"Tenant {tenantId}: Marked {sessionCount} stalled sessions as timed out (timeout: {timeoutHours}h)"); - await _opsEventService.RecordSessionTimeoutsAsync(tenantId, sessionCount, timeoutHours); + _logger.LogInformation( + $"Tenant {tenantId}: timeout sweep — {terminalizedCount} terminalized (Failed/Incomplete), " + + $"{awaitingCount} held AwaitingUser, {reconciledCount} reconciled to Succeeded " + + $"(timeout: {timeoutHours}h, grace: {graceHours}h)"); + + if (terminalizedCount > 0) + await _opsEventService.RecordSessionTimeoutsAsync(tenantId, terminalizedCount, timeoutHours); + } } catch (Exception ex) { diff --git a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Maintenance.cs b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Maintenance.cs index 6b7003f1..16ff3a45 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Maintenance.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Maintenance.cs @@ -560,12 +560,15 @@ namespace AutopilotMonitor.Functions.Services var tableClient = _tableServiceClient.GetTableClient(Constants.TableNames.Sessions); // Eligible for the 5h-timeout sweep: InProgress or Stalled sessions (Stalled is a - // non-terminal intermediate state set earlier by either the agent or the 2h sweep; - // when the 5h mark is reached without healing, the session graduates to Failed). + // non-terminal intermediate state set earlier by either the agent or the 2h sweep). + // At the 5h mark the session is reclassified rather than blindly failed — see + // EnrollmentTimeoutClassifier. AwaitingUser is included too so it keeps being + // re-evaluated each pass and graduates to Incomplete once SessionGraceHours elapses + // (docs/design/enrollment-status-reclassification.md). // IsPreProvisioned ne true → WhiteGlove sessions are intentionally long-lived and // must never be timed out, even after months in storage. var filter = $"PartitionKey eq '{tenantId}' " + - $"and (Status eq 'InProgress' or Status eq 'Stalled') " + + $"and (Status eq 'InProgress' or Status eq 'Stalled' or Status eq 'AwaitingUser') " + $"and StartedAt lt datetime'{cutoffTime:yyyy-MM-ddTHH:mm:ss}Z' " + $"and IsPreProvisioned ne true"; var query = tableClient.QueryAsync(filter: filter); diff --git a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Sessions.cs b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Sessions.cs index 397dfd35..bcda14a9 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Sessions.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Sessions.cs @@ -1363,16 +1363,27 @@ namespace AutopilotMonitor.Functions.Services var entity = await tableClient.GetEntityAsync(tenantId, sessionId); var session = entity.Value; - // Idempotency: if the session is already in a terminal state (Succeeded/Failed), - // do not overwrite it with another terminal state to prevent duplicate notifications. + // Idempotency: never overwrite one terminal state with another. Terminal states + // are Succeeded, Failed and Incomplete (docs/design/enrollment-status-reclassification.md). + // Reconciling a Failed/Incomplete session UP to Succeeded on a late completion is + // done on the dedicated reconcile path, not here. var existingStatusStr = session.GetString("Status"); - if (status == SessionStatus.Succeeded || status == SessionStatus.Failed) + bool incomingTerminal = status == SessionStatus.Succeeded + || status == SessionStatus.Failed + || status == SessionStatus.Incomplete; + bool existingTerminal = existingStatusStr == SessionStatus.Succeeded.ToString() + || existingStatusStr == SessionStatus.Failed.ToString() + || existingStatusStr == SessionStatus.Incomplete.ToString(); + if (incomingTerminal && existingTerminal) { - if (existingStatusStr == SessionStatus.Succeeded.ToString() || existingStatusStr == SessionStatus.Failed.ToString()) - { - _logger.LogInformation($"Session {sessionId} already in terminal state '{existingStatusStr}', skipping status update to '{status}'"); - return false; - } + _logger.LogInformation($"Session {sessionId} already in terminal state '{existingStatusStr}', skipping status update to '{status}'"); + return false; + } + // AwaitingUser is non-terminal but must never regress a terminal session. + if (status == SessionStatus.AwaitingUser && existingTerminal) + { + _logger.LogInformation($"Session {sessionId} already terminal '{existingStatusStr}', skipping AwaitingUser update"); + return false; } // Build a Merge update with only the fields that actually change @@ -1487,6 +1498,16 @@ namespace AutopilotMonitor.Functions.Services update["DurationSeconds"] = (int)(latestEventTimestamp.Value - durationStart).TotalSeconds; } } + // Incomplete is terminal but not a completion — stamp a terminal timestamp so the + // session reads as closed, but do NOT compute DurationSeconds (a ~grace-length span + // would skew duration stats, and Incomplete is excluded from those anyway). + else if (status == SessionStatus.Incomplete) + { + update["CompletedAt"] = EnsureUtc(ResolveCompletionTimestamp( + completedAt, + session.GetDateTimeOffset("LastEventAt")?.UtcDateTime, + DateTime.UtcNow)); + } // Track the most recent event timestamp for excessive data sender detection if (latestEventTimestamp.HasValue) @@ -1496,8 +1517,11 @@ namespace AutopilotMonitor.Functions.Services update["LastEventAt"] = EnsureUtc(latestEventTimestamp.Value); } - // Set failure reason if failed - if (status == SessionStatus.Failed && !string.IsNullOrEmpty(failureReason)) + // Set the reason string for the sweep-authored states. Failed keeps its failure + // reason; Incomplete and AwaitingUser carry the informational classification reason + // (same field reuse as the Stalled path below), so the UI can explain the state. + if ((status == SessionStatus.Failed || status == SessionStatus.Incomplete || status == SessionStatus.AwaitingUser) + && !string.IsNullOrEmpty(failureReason)) { update["FailureReason"] = failureReason; } @@ -1507,7 +1531,8 @@ namespace AutopilotMonitor.Functions.Services // fix, 2026-05-01); other terminal paths leave the field untouched, which // is fine — null/empty means "no snapshot available" and the UI falls back // to the existing FailureReason rendering. - if (status == SessionStatus.Failed && !string.IsNullOrEmpty(failureSnapshotJson)) + if ((status == SessionStatus.Failed || status == SessionStatus.Incomplete || status == SessionStatus.AwaitingUser) + && !string.IsNullOrEmpty(failureSnapshotJson)) { update["FailureSnapshotJson"] = failureSnapshotJson; } diff --git a/src/Shared/AutopilotMonitor.Shared/Models/Config/TenantConfiguration.cs b/src/Shared/AutopilotMonitor.Shared/Models/Config/TenantConfiguration.cs index cf0722db..9ffdec1c 100644 --- a/src/Shared/AutopilotMonitor.Shared/Models/Config/TenantConfiguration.cs +++ b/src/Shared/AutopilotMonitor.Shared/Models/Config/TenantConfiguration.cs @@ -184,6 +184,17 @@ namespace AutopilotMonitor.Shared.Models /// public int SessionTimeoutHours { get; set; } = 5; + /// + /// Grace window in hours for a session that reached the inactivity timeout with Device Setup + /// already provisioned but no completion signal yet (docs/design/enrollment-status-reclassification.md). + /// At such a session becomes AwaitingUser (non-terminal) instead + /// of Failed; only after this longer window elapses without a completion does it graduate to the + /// terminal, non-failure Incomplete state. The user/Account-Setup phase legitimately spans hours or + /// overnight, so this must be comfortably larger than SessionTimeoutHours. + /// Default: 72 hours. + /// + public int SessionGraceHours { get; set; } = 72; + // ===== PAYLOAD SETTINGS ===== /// @@ -832,6 +843,7 @@ namespace AutopilotMonitor.Shared.Models AllowInsecureAgentRequests = false, DataRetentionDays = 90, SessionTimeoutHours = 5, + SessionGraceHours = 72, MaxNdjsonPayloadSizeMB = 5, EnablePerformanceCollector = true, PerformanceCollectorIntervalSeconds = 30,