From 0859c05f4a66d994b893e2e6dfa42eef533462ec Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Thu, 28 May 2026 14:09:48 +0200 Subject: [PATCH] fix(agent-v2): adapter surfaces all three AppFailureTypes on app_install_failed Codex review (P1): ImeLogTrackerAdapter.BuildAppStatePayload only set DataJson.failureType for `esp_apps_timeout`. After the HRESULT-aware classifier landed (4dd050e0), apps promoted with `esp_apps_detection_failure` or `esp_apps_install_failure` still landed with errorPatternId set but failureType missing. Downstream consequences: - ANALYZE-APP-013 (dataField: failureType, value: esp_apps_detection_failure) never matched. - EventIngestProcessor.AppInstall set AppInstallSummary.FailureCode to the HRESULT instead of the canonical failureType. - InstallProgress.tsx fell back to the generic "Failed" badge. Extend the gate to cover all three AppFailureTypes constants and stamp DataJson.failureType from app.ErrorPatternId. Adds a Theory test covering all three flavours and a regression assertion that an unrelated IME-ERROR-* pattern does NOT spuriously promote to failureType. Coverage: 108/108 ImeLogTrackerAdapter tests, 1176/1176 V2.Core. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../ImeLogTrackerAdapterTests.cs | 57 +++++++++++++++++++ .../SignalAdapters/ImeLogTrackerAdapter.cs | 21 ++++--- .../buildcounter.txt | 2 +- .../AutopilotMonitor.Agent/buildcounter.txt | 2 +- 4 files changed, 72 insertions(+), 10 deletions(-) diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/SignalAdapters/ImeLogTrackerAdapterTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/SignalAdapters/ImeLogTrackerAdapterTests.cs index afc45bd1..59bb2ee3 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/SignalAdapters/ImeLogTrackerAdapterTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/SignalAdapters/ImeLogTrackerAdapterTests.cs @@ -341,6 +341,63 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.SignalAdapters Assert.Equal("60001", info.Payload["errorCode"]); Assert.Equal("-2146964895", info.Payload["hresultFromWin32"]); Assert.Equal(EventSeverity.Error.ToString(), info.Payload[SignalPayloadKeys.Severity]); + // Codex review follow-up (P1): an unrelated ErrorPatternId (here a regular + // IME-ERROR-* pattern from the per-app installer) MUST NOT promote to a + // failureType tag — that field is reserved for the three canonical + // termination-handler-promoted classifications. + Assert.False(info.Payload.ContainsKey("failureType")); + } + + [Theory] + [InlineData(AutopilotMonitor.Shared.Constants.AppFailureTypes.EspAppsTimeout, null)] + [InlineData(AutopilotMonitor.Shared.Constants.AppFailureTypes.EspAppsDetectionFailure, "0x87d1041c")] + [InlineData(AutopilotMonitor.Shared.Constants.AppFailureTypes.EspAppsInstallFailure, "0x80070643")] + public void AppStateChange_Error_promoted_carries_failureType_and_errorCode(string failureType, string errorCode) + { + // Codex review follow-up (P1) — when the V2 EnrollmentTerminationHandler + // promotes an app from Installing -> Error via + // ImeLogTracker.PromoteActiveInstallsToStuck, AppPackageState.ErrorPatternId + // is one of the three canonical AppFailureTypes constants and + // AppPackageState.ErrorCode carries the ESP HRESULT (when known). The adapter + // MUST surface BOTH on the emitted app_install_failed event so the analyze + // rule engine, the AppInstallSummary FailureCode column, and the UI badge + // logic can all distinguish detection-failure / install-failure / likely-stuck. + using var f = new ImeLogTrackerAdapterFixture(); + using var adapter = new ImeLogTrackerAdapter(f.Tracker, f.Ingress, f.Clock); + var app = AppPackageState.Restore( + id: "app-promoted", + listPos: 0, + name: "Microsoft Apps for Enterprise - 64Bit", + runAs: AppRunAs.System, + intent: AppIntent.Install, + targeted: AppTargeted.Device, + dependsOn: new HashSet(), + installationState: AppInstallationState.Error, + downloadingOrInstallingSeen: true, + progressPercent: 0, + bytesDownloaded: 10580224, + bytesTotal: 10580224, + errorPatternId: failureType, + errorDetail: "promoted by termination handler", + errorCode: errorCode, + attemptNumber: 1, + detectionResult: failureType == AutopilotMonitor.Shared.Constants.AppFailureTypes.EspAppsDetectionFailure ? "NotDetected" : null); + + adapter.TriggerAppStateFromTest(app, AppInstallationState.Installing, AppInstallationState.Error); + + var info = Assert.Single(f.InfoEvents(SharedEventTypes.AppInstallFailed)); + Assert.Equal(failureType, info.Payload!["failureType"]); + Assert.Equal(failureType, info.Payload["errorPatternId"]); + Assert.Equal("presumed", info.Payload["confidence"]); + Assert.Equal("EspTerminalFailure", info.Payload["terminationTrigger"]); + if (errorCode == null) + { + Assert.False(info.Payload.ContainsKey("errorCode")); + } + else + { + Assert.Equal(errorCode, info.Payload["errorCode"]); + } } [Fact] diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/SignalAdapters/ImeLogTrackerAdapter.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/SignalAdapters/ImeLogTrackerAdapter.cs index 48c3ff2b..198116df 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/SignalAdapters/ImeLogTrackerAdapter.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/SignalAdapters/ImeLogTrackerAdapter.cs @@ -1062,15 +1062,20 @@ namespace AutopilotMonitor.Agent.V2.Core.SignalAdapters if (!string.IsNullOrEmpty(app.ExitCode)) data["exitCode"] = app.ExitCode!; if (!string.IsNullOrEmpty(app.HResultFromWin32)) data["hresultFromWin32"] = app.HResultFromWin32!; - // Likely-stuck classification: when the agent itself promoted this app from - // Installing -> Error (because the ESP gave up before IME reported a final - // status), tag the event so the backend's AppInstallSummary carries the - // canonical failureType and the UI can render hedged "likely stuck" wording - // instead of a confirmed-failure label. See EnrollmentTerminationHandler for - // the discriminator that gates the promotion. - if (string.Equals(app.ErrorPatternId, AutopilotMonitor.Shared.Constants.AppFailureTypes.EspAppsTimeout, StringComparison.Ordinal)) + // Likely-stuck / detection-failure / install-failure classification: when the + // agent itself promoted this app from Installing -> Error (because the ESP + // terminated), tag the event with the canonical failureType so the backend's + // AppInstallSummary carries it as FailureCode and the UI / analyze rules can + // distinguish a confirmed detection-rule mismatch (esp_apps_detection_failure + // / HRESULT 0x87D1041C) or installer error (esp_apps_install_failure) from + // the genuine "no HRESULT available" timeout (esp_apps_timeout). See + // EnrollmentTerminationHandler.MaybePromoteActiveInstallsAsStuck + + // AppFailureTypes.ClassifyEspAppsFailure for the assignment. + if (string.Equals(app.ErrorPatternId, AutopilotMonitor.Shared.Constants.AppFailureTypes.EspAppsTimeout, StringComparison.Ordinal) + || string.Equals(app.ErrorPatternId, AutopilotMonitor.Shared.Constants.AppFailureTypes.EspAppsDetectionFailure, StringComparison.Ordinal) + || string.Equals(app.ErrorPatternId, AutopilotMonitor.Shared.Constants.AppFailureTypes.EspAppsInstallFailure, StringComparison.Ordinal)) { - data["failureType"] = AutopilotMonitor.Shared.Constants.AppFailureTypes.EspAppsTimeout; + data["failureType"] = app.ErrorPatternId!; data["confidence"] = "presumed"; data["terminationTrigger"] = "EspTerminalFailure"; } diff --git a/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt b/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt index ed8cc244..7b8d0843 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt +++ b/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt @@ -1 +1 @@ -902 +905 diff --git a/src/Agent/AutopilotMonitor.Agent/buildcounter.txt b/src/Agent/AutopilotMonitor.Agent/buildcounter.txt index 08507bb2..26e58c77 100644 --- a/src/Agent/AutopilotMonitor.Agent/buildcounter.txt +++ b/src/Agent/AutopilotMonitor.Agent/buildcounter.txt @@ -1 +1 @@ -1409 +1411