fix(agent-v2): gate HRESULT-based app classification on failedSubcategory=Apps

Codex review (P3): the V2 EnrollmentTerminationHandler previously cached
only the HRESULT from the last registry-derived ESP failure. A non-Apps
ESP failure carrying its own HRESULT (e.g. DeviceSetup/SecurityPolicies
or AccountSetup/CertificatesAccountSetup with their own status codes)
would then mis-classify still-installing apps as
`esp_apps_install_failure` — the HRESULT describes the failed category,
not the in-flight installs.

Replace `LastEspTerminalErrorCode` (string) with a new
`EspTerminalFailureSnapshot` value carrying errorCode + failedSubcategory
+ category as a single atomic snapshot. Plumb through the host stack
(EspAndHelloTracker -> EspAndHelloHost -> DefaultComponentFactory ->
EnrollmentTerminationHandler) and gate the HRESULT-driven classifier on
`snapshot.IsAppsSubcategory == true`. Non-Apps failures fall through to
the generic `esp_apps_timeout` classification with no HRESULT in the
promoted app's DataJson.

Coverage: 2 new gating tests + the 2 existing HRESULT tests updated to
supply the full snapshot (errorCode + "Apps" subcategory).
1176/1176 V2.Core, 2058/2058 backend.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Oliver Kieselbach
2026-05-28 14:10:22 +02:00
parent 6007b9ecc5
commit dc0b9d7652
7 changed files with 216 additions and 71 deletions
@@ -151,11 +151,14 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Termination
public List<(string failureType, string message, string errorCode)> PromotionCalls { get; } =
new List<(string failureType, string message, string errorCode)>();
public IReadOnlyList<string> PromotionReturnValue { get; set; } = Array.Empty<string>();
// Session 080edee9 follow-up (2026-05-28) — feeds the
// lastEspTerminalErrorCodeAccessor in the production wiring. Defaults to null
// (no HRESULT observed) so existing tests fall through to the
// `esp_apps_timeout` classification.
public string LastEspTerminalErrorCodeOverride { get; set; } = null;
// Session 080edee9 follow-up + Codex review (P2/P3, 2026-05-28) — feeds the
// lastEspTerminalFailureAccessor in the production wiring. Defaults to null
// (no ESP failure context observed) so existing tests fall through to the
// `esp_apps_timeout` classification. Tests for HRESULT-driven classification
// assign a fully-populated snapshot (errorCode + Apps subcategory); the
// non-Apps gating test assigns a snapshot whose FailedSubcategory is NOT
// "Apps".
public EspTerminalFailureSnapshot? LastEspTerminalFailureOverride { get; set; } = null;
public List<string> TerminationActionLog { get; } = new List<string>();
public TimeSpan SpoolDrainPeriodOverride { get; set; } = TimeSpan.Zero;
// Shutdown-gap closure (2026-05-15) — when set, the constructed handler shares
@@ -219,7 +222,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Termination
return PromotionReturnValue;
},
tryClaimShutdownEvent: TryClaimShutdownEventOverride,
lastEspTerminalErrorCodeAccessor: () => LastEspTerminalErrorCodeOverride);
lastEspTerminalFailureAccessor: () => LastEspTerminalFailureOverride);
}
public void Dispose() => Tmp.Dispose();
@@ -1121,7 +1124,10 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Termination
using var rig = new Rig();
rig.State = BuildEspTerminalFailureState();
rig.PromotionReturnValue = new[] { "office-365" };
rig.LastEspTerminalErrorCodeOverride = "0x87d1041c";
rig.LastEspTerminalFailureOverride = new EspTerminalFailureSnapshot(
errorCode: "0x87d1041c",
failedSubcategory: "Apps",
category: "DeviceSetup");
rig.Build().Handle(sender: null!,
Args(EnrollmentTerminationReason.DecisionTerminalStage, EnrollmentTerminationOutcome.Failed, SessionStage.Failed));
@@ -1143,7 +1149,10 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Termination
using var rig = new Rig();
rig.State = BuildEspTerminalFailureState();
rig.PromotionReturnValue = new[] { "vpn-client" };
rig.LastEspTerminalErrorCodeOverride = "0x80070643";
rig.LastEspTerminalFailureOverride = new EspTerminalFailureSnapshot(
errorCode: "0x80070643",
failedSubcategory: "Apps",
category: "DeviceSetup");
rig.Build().Handle(sender: null!,
Args(EnrollmentTerminationReason.DecisionTerminalStage, EnrollmentTerminationOutcome.Failed, SessionStage.Failed));
@@ -1155,5 +1164,57 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Termination
Assert.Contains("0x80070643", rig.PromotionCalls[0].message);
Assert.Equal("0x80070643", rig.PromotionCalls[0].errorCode);
}
[Fact]
public void PromoteActiveInstalls_falls_back_to_timeout_when_failure_subcategory_is_not_Apps()
{
// Codex review (P3): a non-Apps ESP failure with HRESULT (here:
// DeviceSetup/SecurityPolicies failing with a policy-related code) MUST NOT
// mis-classify the still-installing apps as `esp_apps_install_failure` —
// the HRESULT describes the SecurityPolicies subcategory, not the
// in-flight installs. The classifier must drop the HRESULT and fall back
// to the generic `esp_apps_timeout` classification.
using var rig = new Rig();
rig.State = BuildEspTerminalFailureState();
rig.PromotionReturnValue = new[] { "in-flight-app" };
rig.LastEspTerminalFailureOverride = new EspTerminalFailureSnapshot(
errorCode: "0x8007064a",
failedSubcategory: "SecurityPolicies",
category: "DeviceSetup");
rig.Build().Handle(sender: null!,
Args(EnrollmentTerminationReason.DecisionTerminalStage, EnrollmentTerminationOutcome.Failed, SessionStage.Failed));
Assert.Single(rig.PromotionCalls);
Assert.Equal(
AutopilotMonitor.Shared.Constants.AppFailureTypes.EspAppsTimeout,
rig.PromotionCalls[0].failureType);
Assert.DoesNotContain("0x8007064a", rig.PromotionCalls[0].message);
Assert.Null(rig.PromotionCalls[0].errorCode);
}
[Fact]
public void PromoteActiveInstalls_falls_back_to_timeout_when_failure_snapshot_has_no_subcategory()
{
// Defense-in-depth: if the snapshot somehow carries a HRESULT but no
// failedSubcategory (registry shape regression), we cannot prove the
// failure came from Apps — must NOT classify as detection / install failure.
using var rig = new Rig();
rig.State = BuildEspTerminalFailureState();
rig.PromotionReturnValue = new[] { "in-flight-app" };
rig.LastEspTerminalFailureOverride = new EspTerminalFailureSnapshot(
errorCode: "0x87d1041c",
failedSubcategory: null,
category: "DeviceSetup");
rig.Build().Handle(sender: null!,
Args(EnrollmentTerminationReason.DecisionTerminalStage, EnrollmentTerminationOutcome.Failed, SessionStage.Failed));
Assert.Single(rig.PromotionCalls);
Assert.Equal(
AutopilotMonitor.Shared.Constants.AppFailureTypes.EspAppsTimeout,
rig.PromotionCalls[0].failureType);
Assert.Null(rig.PromotionCalls[0].errorCode);
}
}
}
@@ -123,17 +123,18 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals
public DateTime? LastEventOccurredAtUtc { get; private set; }
/// <summary>
/// Session 080edee9 follow-up (2026-05-28) — last HRESULT observed on an ESP failure
/// raised by <see cref="ProvisioningStatusTracker"/> (registry-derived, e.g.
/// <c>0x87D1041C</c> from a failed Apps subcategory). Read by
/// <c>EnrollmentTerminationHandler</c> via <see cref="EspAndHelloHost"/> +
/// <c>DefaultComponentFactory</c> so the late "Installing-→-Error" promotion can
/// classify the failure correctly (detection-failure vs. install-failure vs. genuine
/// timeout) instead of always claiming an ESP timeout. Null when no
/// HRESULT-carrying ESP failure has fired (or all observed failures came from
/// ShellCoreTracker, which has no HRESULT surface).
/// Session 080edee9 follow-up + Codex review (P2/P3, 2026-05-28) — last
/// ESP failure context observed via <see cref="ProvisioningStatusTracker"/>
/// (registry-derived; carries HRESULT, failed subcategory, and category as
/// a single immutable snapshot). Read by <c>EnrollmentTerminationHandler</c>
/// via <see cref="EspAndHelloHost"/> + <c>DefaultComponentFactory</c> so the
/// late "Installing-→-Error" promotion can classify the failure correctly
/// AND refuse to promote when the failure originated outside the Apps
/// subcategory (a non-Apps HRESULT does not describe per-app outcome).
/// Null when no HRESULT-carrying ESP failure has fired (or all observed
/// failures came from ShellCoreTracker, which has no HRESULT surface).
/// </summary>
public string LastEspTerminalErrorCode { get; private set; }
public Termination.EspTerminalFailureSnapshot LastEspTerminalFailure { get; private set; }
/// <summary>
/// Outcome of Hello provisioning. Set when Hello resolves (via events, timeout, or not configured).
@@ -506,14 +507,22 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals
private void OnProvisioningEspFailureDetected(object sender, EspFailureDetectedEventArgs args)
{
LastEventOccurredAtUtc = null;
// Session 080edee9 follow-up — stash the HRESULT before forwarding so
// EnrollmentTerminationHandler can read it on the terminal-failure pathway even
// if the downstream signal flow is asynchronous. Only updates on non-empty values
// so a later ShellCore-derived failure (which carries no HRESULT) cannot wipe
// out the previously captured registry value.
if (!string.IsNullOrEmpty(args?.ErrorCode))
// Session 080edee9 follow-up + Codex review (P2/P3) — snapshot the full
// failure context (HRESULT + failedSubcategory + category) before
// forwarding so EnrollmentTerminationHandler can read it on the terminal-
// failure pathway. Only update when the args carry at least one of the
// three fields, so a later ShellCore-derived failure (which has none of
// them) cannot wipe out the registry snapshot. We deliberately do NOT
// gate on ErrorCode alone — a non-Apps subcategory failure without
// HRESULT is still useful for downstream "should we promote?" gating.
if (!string.IsNullOrEmpty(args?.ErrorCode)
|| !string.IsNullOrEmpty(args?.FailedSubcategory)
|| !string.IsNullOrEmpty(args?.Category))
{
LastEspTerminalErrorCode = args.ErrorCode;
LastEspTerminalFailure = new Termination.EspTerminalFailureSnapshot(
errorCode: args!.ErrorCode,
failedSubcategory: args.FailedSubcategory,
category: args.Category);
}
try
{
@@ -138,16 +138,16 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration
_imeLogHost?.PromoteActiveInstallsToStuck(failureType, message, errorCode) ?? Array.Empty<string>();
/// <summary>
/// Session 080edee9 follow-up (2026-05-28) — last observed HRESULT from the ESP Apps
/// subcategory failure (e.g. <c>0x87D1041C</c>). Surfaced by
/// <see cref="EspAndHelloHost.LastEspTerminalErrorCode"/> and read by
/// <c>EnrollmentTerminationHandler.MaybePromoteActiveInstallsAsStuck</c> so the
/// promotion can classify the failure as <c>esp_apps_detection_failure</c> /
/// <c>esp_apps_install_failure</c> instead of always claiming a generic timeout.
/// Returns null when no ESP failure with a HRESULT was observed (or the host has not
/// been created yet).
/// Session 080edee9 follow-up + Codex review (P2/P3, 2026-05-28) — last
/// observed ESP failure context (HRESULT + failedSubcategory + category).
/// Surfaced by <see cref="EspAndHelloHost.LastEspTerminalFailure"/> and
/// read by <c>EnrollmentTerminationHandler.MaybePromoteActiveInstallsAsStuck</c>
/// 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.
/// </summary>
public string? LastEspTerminalErrorCode => EspAndHelloHost?.LastEspTerminalErrorCode;
public Termination.EspTerminalFailureSnapshot? LastEspTerminalFailure => EspAndHelloHost?.LastEspTerminalFailure;
public DefaultComponentFactory(
AgentConfiguration agentConfig,
@@ -29,14 +29,16 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration
public event EventHandler? WhiteGloveCompleted;
/// <summary>
/// Session 080edee9 follow-up (2026-05-28) — forwards
/// <see cref="EspAndHelloTracker.LastEspTerminalErrorCode"/>. Read by
/// <see cref="DefaultComponentFactory.LastEspTerminalErrorCode"/> which is in turn
/// queried by <c>EnrollmentTerminationHandler.MaybePromoteActiveInstallsAsStuck</c>
/// to classify ESP Apps-subcategory failures (detection-failure vs install-failure
/// vs genuine timeout). Null until the first HRESULT-carrying ESP failure fires.
/// Session 080edee9 follow-up + Codex review (P2/P3, 2026-05-28) — forwards
/// <see cref="EspAndHelloTracker.LastEspTerminalFailure"/>. Read by
/// <see cref="DefaultComponentFactory.LastEspTerminalFailure"/> which is in
/// turn queried by
/// <c>EnrollmentTerminationHandler.MaybePromoteActiveInstallsAsStuck</c> to
/// classify ESP failures correctly (HRESULT-based detection-failure /
/// install-failure ONLY when the failure was raised against the Apps
/// subcategory). Null until the first registry-derived ESP failure fires.
/// </summary>
public string? LastEspTerminalErrorCode => _tracker.LastEspTerminalErrorCode;
public Termination.EspTerminalFailureSnapshot? LastEspTerminalFailure => _tracker.LastEspTerminalFailure;
public EspAndHelloHost(
string sessionId,
@@ -141,15 +141,17 @@ namespace AutopilotMonitor.Agent.V2.Core.Termination
// legacy "esp_apps_timeout" classification.
private readonly Func<string, string, string, IReadOnlyList<string>> _promoteActiveInstallsToStuck;
// Session 080edee9 follow-up (2026-05-28): accessor returning the latest HRESULT
// observed on an ESP failure (typically extracted by
// <see cref="ProvisioningStatusTracker.TryExtractErrorCode"/> from the failed
// subcategory's <c>statusText</c>). Read once during
// <see cref="MaybePromoteActiveInstallsAsStuck"/> and passed to
// <see cref="AppFailureTypes.ClassifyEspAppsFailure"/> so the failureType +
// message accurately reflect "detection failure" / "install failure" vs. the
// generic timeout fallback. Null accessor or null result = no HRESULT → fallback.
private readonly Func<string> _lastEspTerminalErrorCodeAccessor;
// 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
// <see cref="MaybePromoteActiveInstallsAsStuck"/>. The classifier first
// checks <c>snapshot.IsAppsSubcategory</c> — only then does the HRESULT
// drive the failureType. A non-Apps ESP failure (DevicePreparation/*,
// DeviceSetup/SecurityPolicies, AccountSetup/CertificatesAccountSetup)
// falls through to the generic <c>esp_apps_timeout</c> wording so a
// non-app HRESULT cannot mis-classify in-flight installs.
// Null accessor or null snapshot = no HRESULT context → fallback.
private readonly Func<EspTerminalFailureSnapshot> _lastEspTerminalFailureAccessor;
private int _handled;
@@ -180,7 +182,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Termination
Func<bool> isWhiteGlovePart2Accessor = null,
Func<string, string, string, IReadOnlyList<string>> promoteActiveInstallsToStuck = null,
Func<bool> tryClaimShutdownEvent = null,
Func<string> lastEspTerminalErrorCodeAccessor = null)
Func<EspTerminalFailureSnapshot> lastEspTerminalFailureAccessor = null)
{
_configuration = configuration ?? throw new ArgumentNullException(nameof(configuration));
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
@@ -212,7 +214,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Termination
_isWhiteGlovePart2Accessor = isWhiteGlovePart2Accessor;
_promoteActiveInstallsToStuck = promoteActiveInstallsToStuck;
_tryClaimShutdownEvent = tryClaimShutdownEvent;
_lastEspTerminalErrorCodeAccessor = lastEspTerminalErrorCodeAccessor;
_lastEspTerminalFailureAccessor = lastEspTerminalFailureAccessor;
}
/// <summary>
@@ -535,23 +537,34 @@ namespace AutopilotMonitor.Agent.V2.Core.Termination
try
{
var timeoutMinutes = state.ScenarioObservations?.EspSyncFailureTimeoutMinutes?.Value;
string errorCode = null;
try { errorCode = _lastEspTerminalErrorCodeAccessor?.Invoke(); }
catch (Exception ex) { _logger.Warning($"EnrollmentTerminationHandler: lastEspTerminalErrorCodeAccessor threw: {ex.Message}"); }
EspTerminalFailureSnapshot failureContext = null;
try { failureContext = _lastEspTerminalFailureAccessor?.Invoke(); }
catch (Exception ex) { _logger.Warning($"EnrollmentTerminationHandler: lastEspTerminalFailureAccessor threw: {ex.Message}"); }
// Session 080edee9 follow-up (2026-05-28): classify the ESP Apps-subcategory
// failure via the shared helper so the failureType + message accurately
// reflect the HRESULT (detection-failure / install-failure) instead of
// claiming a timeout for every ESP terminal pathway.
var (failureType, message) = AppFailureTypes.ClassifyEspAppsFailure(errorCode, timeoutMinutes);
// Codex review (P3, 2026-05-28): only let the HRESULT drive the per-app
// classification when the ESP failure actually came from the Apps
// subcategory. A non-Apps failure (DevicePreparation/*,
// DeviceSetup/SecurityPolicies, AccountSetup/CertificatesAccountSetup …)
// would happily carry its own HRESULT but that HRESULT describes the
// category, NOT the in-flight installs. Falling back to the generic
// `esp_apps_timeout` classification here preserves the hedged "Likely
// stuck" UX without lying about the cause.
var effectiveErrorCode = (failureContext != null && failureContext.IsAppsSubcategory)
? failureContext.ErrorCode
: null;
var promoted = _promoteActiveInstallsToStuck(failureType, message, errorCode);
var (failureType, message) = AppFailureTypes.ClassifyEspAppsFailure(effectiveErrorCode, timeoutMinutes);
var promoted = _promoteActiveInstallsToStuck(failureType, message, effectiveErrorCode);
var count = promoted?.Count ?? 0;
if (count > 0)
{
var ecSuffix = string.IsNullOrEmpty(errorCode) ? string.Empty : $", errorCode={errorCode}";
var ecSuffix = string.IsNullOrEmpty(effectiveErrorCode) ? string.Empty : $", errorCode={effectiveErrorCode}";
var ctxSuffix = failureContext == null
? string.Empty
: $", failedSubcategory={failureContext.FailedSubcategory ?? "n/a"}";
_logger.Info(
$"EnrollmentTerminationHandler: promoted {count} Installing app(s) to Error (failureType={failureType}{ecSuffix}).");
$"EnrollmentTerminationHandler: promoted {count} Installing app(s) to Error (failureType={failureType}{ecSuffix}{ctxSuffix}).");
}
else
{
@@ -0,0 +1,58 @@
namespace AutopilotMonitor.Agent.V2.Core.Termination
{
/// <summary>
/// Read-only snapshot of the last ESP terminal failure observed by the agent,
/// captured from <c>ProvisioningStatusTracker.EspFailureDetected</c> args and
/// forwarded up to <see cref="EnrollmentTerminationHandler"/> via the host
/// stack. All three fields share the same source event so the consumer can
/// safely correlate them (e.g. only treat an HRESULT as an app-detection or
/// app-install failure when <see cref="FailedSubcategory"/> is also "Apps").
/// <para>
/// Session 080edee9 follow-up + Codex review (P2/P3): an earlier shape carried
/// only the HRESULT, which let a non-Apps ESP failure (e.g.
/// DevicePreparation/* or DeviceSetup/SecurityPolicies) classify still-installing
/// apps as <c>esp_apps_install_failure</c>. The subcategory + category fields
/// are now part of the snapshot so the classifier can refuse that misattribution.
/// </para>
/// </summary>
public sealed class EspTerminalFailureSnapshot
{
/// <summary>
/// HRESULT extracted from the failed subcategory's <c>statusText</c>
/// (lower-case, e.g. <c>0x87d1041c</c>). Null when no parseable HRESULT
/// was present.
/// </summary>
public string ErrorCode { get; }
/// <summary>
/// Failed ESP subcategory short name (e.g. <c>Apps</c>,
/// <c>SecurityPolicies</c>, <c>CertificatesAccountSetup</c>). Null when the
/// failure is recorded at category level (no specific subcategory).
/// </summary>
public string FailedSubcategory { get; }
/// <summary>
/// ESP category short name (<c>DevicePreparation</c> | <c>DeviceSetup</c>
/// | <c>AccountSetup</c>). Null when not available.
/// </summary>
public string Category { get; }
public EspTerminalFailureSnapshot(string errorCode, string failedSubcategory, string category)
{
ErrorCode = string.IsNullOrEmpty(errorCode) ? null : errorCode;
FailedSubcategory = string.IsNullOrEmpty(failedSubcategory) ? null : failedSubcategory;
Category = string.IsNullOrEmpty(category) ? null : category;
}
/// <summary>
/// True when the failure was emitted against the ESP <c>Apps</c>
/// subcategory. This is the only failure shape where promoting still-
/// installing apps with a HRESULT-based classification is correct;
/// non-Apps failures (security policies, certificate enrolment, etc.)
/// must fall back to the generic <c>esp_apps_timeout</c> wording because
/// the HRESULT does not describe per-app outcome.
/// </summary>
public bool IsAppsSubcategory =>
string.Equals(FailedSubcategory, "Apps", System.StringComparison.OrdinalIgnoreCase);
}
}
@@ -403,14 +403,16 @@ namespace AutopilotMonitor.Agent.V2.Runtime
// events on the wire. The handler skips its emit when the
// gate already claimed the slot.
tryClaimShutdownEvent: TryClaimAgentShuttingDownEmit,
// Session 080edee9 follow-up (2026-05-28) — feeds the latest
// HRESULT captured by ProvisioningStatusTracker (via
// EspAndHelloTracker.LastEspTerminalErrorCode) into the
// termination handler's classifier so promoted apps land
// with the right failureType (esp_apps_detection_failure /
// esp_apps_install_failure) instead of always claiming a
// generic timeout.
lastEspTerminalErrorCodeAccessor: () => componentFactory.LastEspTerminalErrorCode);
// 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);
// ServerActionDispatcher (plan §5.3) — constructed inside this
// hook so lifecyclePost + terminationHandler are guaranteed