mirror of
https://github.com/okieselbach/AutopilotMonitor.git
synced 2026-10-24 20:53:06 +00:00
fix(health-scripts): stop compliant detections being stamped script_failed/Error
Intune health-script detections (scriptType=remediation, scriptPart=detection/ post-detection) that report complianceResult=True, exitCode=0 were emitted as script_failed/Severity=Error whenever the detection PowerShell leaked a benign probe line to stderr (e.g. Get-ChildItem path-not-found). On green sessions this inflated errorCount (8 on a successful session) and ranked the rows #1 in search_events, diluting usage. Root cause: the hasStderr check short-circuited the failure decision before the existing isHealthComplianceReport exemption. Exit-code was already exempt for detection phases; stderr was not. The flawed rule was hand-duplicated in the agent emit and the web reducer, and propagated into MCP errorCount + ranking. Fix (verdict is authoritative for detection/post-detection phases): - Agent ImeLogTrackerAdapter.EmitScriptCompleted: fold hasStderr under !isHealthComplianceReport. Platform scripts, the remediation phase itself, and explicit result="Failed" still route to script_failed/Error. - Web reducer scriptExecutions.ts: same exemption (kept in sync). Non-compliant detections render amber via isNonCompliantReport, not red. - MCP read-side guard isBenignHealthDetectionReport() in tools/shared.ts, applied in search.ts severityIntentFactor (damp 0.6, no failure boost) and sessions.ts errorCount/keyEvents. Non-destructive; de-pollutes already-stored rows before the agent build rolls out per-enrollment. Tests: Agent 70 pass (retargeted enrichment test + new compliant-stderr regression), MCP search 61 pass (helper + ranker damp), Web reducer 66 pass (flipped detection-stderr case). MCP + Web tsc clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
+31
-2
@@ -1080,8 +1080,11 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.SignalAdapters
|
||||
stdout: "Inventory Updated 2026-05-11",
|
||||
stderr: "Invoke-WebRequest : The remote name could not be resolved");
|
||||
|
||||
// stderr present → adapter routes to script_failed (stderr-as-failure rule).
|
||||
var info = Assert.Single(f.InfoEvents(SharedEventTypes.ScriptFailed));
|
||||
// Detection is a compliance report — stderr (and exit code) are exempt from forcing
|
||||
// failure, so this stays script_completed. Non-compliance is carried by
|
||||
// complianceResult=False (rendered amber via isNonCompliantReport), not as an Error.
|
||||
Assert.Empty(f.InfoEvents(SharedEventTypes.ScriptFailed));
|
||||
var info = Assert.Single(f.InfoEvents(SharedEventTypes.ScriptCompleted));
|
||||
Assert.Equal("e39d92ae-20bd-4b37-b36a-838abe50ac89", info.Payload!["policyId"]);
|
||||
Assert.Equal("detection", info.Payload["scriptPart"]);
|
||||
Assert.Equal("False", info.Payload["complianceResult"]);
|
||||
@@ -1091,6 +1094,32 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.SignalAdapters
|
||||
Assert.Contains("Invoke-WebRequest", info.Payload["stderr"]);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void HealthScriptDetectionResult_compliant_with_stderr_stays_script_completed_Info()
|
||||
{
|
||||
// Regression for the search_events / errorCount pollution: a fully compliant health
|
||||
// detection (complianceResult=True, exit 0) whose PowerShell leaked a benign probe
|
||||
// error to stderr (e.g. Get-ChildItem path-not-found) MUST NOT be stamped
|
||||
// script_failed / Error. The compliance verdict is authoritative for detection phases.
|
||||
using var f = new ImeLogTrackerAdapterFixture();
|
||||
using var adapter = new ImeLogTrackerAdapter(f.Tracker, f.Ingress, f.Clock);
|
||||
|
||||
f.Tracker.HandleHealthScriptDetectionResultForTest(
|
||||
"55f2e743-dc79-44b8-a32e-f58db396abf7",
|
||||
"True", "pre",
|
||||
exitCode: 0,
|
||||
runContext: "User",
|
||||
stdout: "InventoryDate: 18-05 20:07 WUInventory:OK",
|
||||
stderr: "Get-ChildItem : Cannot find path 'HKEY_LOCAL_MACHINE\\SYSTEM\\...\\JoinInfo' because it does not exist.");
|
||||
|
||||
Assert.Empty(f.InfoEvents(SharedEventTypes.ScriptFailed));
|
||||
var info = Assert.Single(f.InfoEvents(SharedEventTypes.ScriptCompleted));
|
||||
Assert.Equal("detection", info.Payload!["scriptPart"]);
|
||||
Assert.Equal("True", info.Payload["complianceResult"]);
|
||||
Assert.Equal("0", info.Payload["exitCode"]);
|
||||
Assert.Equal(EventSeverity.Info.ToString(), info.Payload[SignalPayloadKeys.Severity]);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void HealthScriptDetectionResult_post_part_maps_to_post_detection_phase()
|
||||
{
|
||||
|
||||
@@ -757,27 +757,30 @@ namespace AutopilotMonitor.Agent.V2.Core.SignalAdapters
|
||||
{
|
||||
if (script == null || string.IsNullOrEmpty(script.PolicyId)) return;
|
||||
|
||||
// Outcome classification — three rules in priority order:
|
||||
// 1. stderr present → script_failed. Per user UX preference (debrief 2026-05-11):
|
||||
// consistent failure semantics across script types — any time a script writes
|
||||
// to stderr the user wants visibility, even if exit was 0 and IME reported
|
||||
// compliant. Eliminates the confusing case where a platform script with stderr
|
||||
// shows red but a remediation detection with stderr shows green.
|
||||
// 2. Phase-aware exit handling: detection / post-detection use exit code as a
|
||||
// compliance verdict (not a crash signal), so non-zero exit alone is NOT
|
||||
// failure for those phases. Only remediation phase + platform scripts route
|
||||
// non-zero exit to script_failed.
|
||||
// 3. Defensive: explicit Result=="Failed" → script_failed (V1 parity).
|
||||
// Outcome classification — rules in priority order:
|
||||
// 1. Phase-aware exemption: health-script detection / post-detection phases are
|
||||
// *compliance reports*, not crash signals. Their authoritative outcome is the
|
||||
// compliance verdict (complianceResult / RemediationStatus), so NEITHER a
|
||||
// non-zero exit code NOR stderr forces script_failed for those phases. Detection
|
||||
// PowerShell routinely leaks benign probe errors to stderr (e.g. Get-ChildItem
|
||||
// path-not-found) while still returning a valid compliant verdict — those must
|
||||
// not be stamped Error (it inflated errorCount and ranked #1 in search_events
|
||||
// on otherwise-green sessions). Non-compliance is carried by complianceResult and
|
||||
// rendered amber by the Web reducer (isNonCompliantReport), not as a failure.
|
||||
// 2. For everything else (platform scripts, the remediation phase itself) stderr OR
|
||||
// a non-zero exit → script_failed. Per user UX preference (debrief 2026-05-11):
|
||||
// any time such a script writes to stderr the user wants visibility.
|
||||
// 3. Defensive: explicit Result=="Failed" → script_failed (V1 parity), all phases.
|
||||
var isHealthScript = IsRemediation(script.ScriptType);
|
||||
var isHealthComplianceReport = isHealthScript
|
||||
&& (string.Equals(script.ScriptPart, "detection", StringComparison.OrdinalIgnoreCase)
|
||||
|| string.Equals(script.ScriptPart, "post-detection", StringComparison.OrdinalIgnoreCase));
|
||||
|
||||
var hasStderr = !string.IsNullOrWhiteSpace(script.Stderr);
|
||||
var isFailure = hasStderr
|
||||
|| string.Equals(script.Result, "Failed", StringComparison.OrdinalIgnoreCase)
|
||||
var isFailure = string.Equals(script.Result, "Failed", StringComparison.OrdinalIgnoreCase)
|
||||
|| (!isHealthComplianceReport
|
||||
&& script.ExitCode.HasValue && script.ExitCode.Value != 0);
|
||||
&& (hasStderr
|
||||
|| (script.ExitCode.HasValue && script.ExitCode.Value != 0)));
|
||||
var severity = isFailure ? EventSeverity.Error : EventSeverity.Info;
|
||||
var eventType = isFailure ? SharedEventTypes.ScriptFailed : SharedEventTypes.ScriptCompleted;
|
||||
|
||||
|
||||
@@ -1 +1 @@
|
||||
918
|
||||
920
|
||||
|
||||
@@ -18,6 +18,38 @@ import {
|
||||
} from '../tools/search.js';
|
||||
import { scanLexical } from '../search-provider.js';
|
||||
import type { SearchDocument } from '../search-provider.js';
|
||||
import { isBenignHealthDetectionReport } from '../tools/shared.js';
|
||||
|
||||
describe('isBenignHealthDetectionReport', () => {
|
||||
const det = (over: Record<string, unknown> = {}) =>
|
||||
({ scriptType: 'remediation', scriptPart: 'detection', complianceResult: 'True', ...over });
|
||||
|
||||
it('true for a compliant remediation detection stamped script_failed', () => {
|
||||
expect(isBenignHealthDetectionReport('script_failed', det())).toBe(true);
|
||||
});
|
||||
it('true for a non-compliant detection too (verdict, not crash)', () => {
|
||||
expect(isBenignHealthDetectionReport('script_failed', det({ complianceResult: 'False', exitCode: '1' }))).toBe(true);
|
||||
});
|
||||
it('true for post-detection phase', () => {
|
||||
expect(isBenignHealthDetectionReport('script_failed', det({ scriptPart: 'post-detection' }))).toBe(true);
|
||||
});
|
||||
it('false when IME explicitly reported result=Failed (authoritative failure)', () => {
|
||||
expect(isBenignHealthDetectionReport('script_failed', det({ result: 'Failed' }))).toBe(false);
|
||||
});
|
||||
it('false for the remediation phase itself (a real crash there is a failure)', () => {
|
||||
expect(isBenignHealthDetectionReport('script_failed', det({ scriptPart: 'remediation' }))).toBe(false);
|
||||
});
|
||||
it('false for platform scripts', () => {
|
||||
expect(isBenignHealthDetectionReport('script_failed', { scriptType: 'platform', exitCode: '1' })).toBe(false);
|
||||
});
|
||||
it('false for non-failure event types (only guards mis-stamped script_failed rows)', () => {
|
||||
expect(isBenignHealthDetectionReport('script_completed', det())).toBe(false);
|
||||
});
|
||||
it('tolerates snake_case keys and missing data', () => {
|
||||
expect(isBenignHealthDetectionReport('script_failed', { script_type: 'remediation', script_part: 'detection' })).toBe(true);
|
||||
expect(isBenignHealthDetectionReport('script_failed', undefined)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
type Page = { events?: Array<Record<string, unknown>>; nextLink?: string };
|
||||
|
||||
@@ -338,6 +370,38 @@ describe('scoreEvent — severity-intent alignment', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('scoreEvent — benign health-detection report is damped, not boosted', () => {
|
||||
// A compliant health-script detection mis-stamped script_failed/Error must not ride the
|
||||
// failure boost: it should rank like an Info event so it stops polluting problem queries.
|
||||
const keywords = ['compliance', 'script', 'failed'];
|
||||
const benignDetection = {
|
||||
eventType: 'script_failed', severity: 'Error',
|
||||
data: { scriptType: 'remediation', scriptPart: 'detection', complianceResult: 'True', exitCode: '0' },
|
||||
message: 'Remediation script 55f2e743: compliance=True (exit: 0)',
|
||||
};
|
||||
const realScriptFailure = {
|
||||
eventType: 'script_failed', severity: 'Error',
|
||||
data: { scriptType: 'remediation', scriptPart: 'remediation', exitCode: '1' },
|
||||
message: 'Remediation script crashed',
|
||||
};
|
||||
|
||||
it('ranks the benign compliant detection below a real script failure on a problem query', () => {
|
||||
const benign = scoreEvent(benignDetection, keywords, true)!;
|
||||
const real = scoreEvent(realScriptFailure, keywords, true)!;
|
||||
expect(benign.score).toBeLessThan(real.score);
|
||||
});
|
||||
|
||||
it('damps the benign detection by the Info factor (0.6) not the failure factor (1.5)', () => {
|
||||
// Single low-weight keyword keeps the unboosted score < 1 so the ×0.6 damp is exact
|
||||
// (a multi-keyword match would clamp the unboosted score to 1.0 first).
|
||||
const kw = ['compliance'];
|
||||
const boosted = scoreEvent(benignDetection, kw, true)!;
|
||||
const unboosted = scoreEvent(benignDetection, kw, false)!;
|
||||
expect(unboosted.score).toBeLessThan(1);
|
||||
expect(boosted.score).toBeCloseTo(unboosted.score * 0.6, 5);
|
||||
});
|
||||
});
|
||||
|
||||
describe('extractEventTypeCandidates — ranking + broad-catalog coverage', () => {
|
||||
it('ranks the type matching the most keywords first', () => {
|
||||
const ranked = extractEventTypeCandidates(['hello', 'provisioning', 'failed']);
|
||||
|
||||
@@ -3,7 +3,7 @@ import { z } from 'zod';
|
||||
import { apiFetch, buildQuery, followNextLink, pickGlobalOrTenantPath } from '../client.js';
|
||||
import { withToolTelemetry } from '../telemetry.js';
|
||||
import type { SearchProvider } from '../search-provider.js';
|
||||
import { READ_ONLY, MAX_RESULT_SIZE_CHARS, toolResultText, SessionIdSchema } from './shared.js';
|
||||
import { READ_ONLY, MAX_RESULT_SIZE_CHARS, toolResultText, SessionIdSchema, isBenignHealthDetectionReport } from './shared.js';
|
||||
import { toolError } from './error-handler.js';
|
||||
import { ALL_EVENT_TYPES } from '../resource-catalog.js';
|
||||
|
||||
@@ -631,6 +631,9 @@ type ScoredEvent = {
|
||||
*/
|
||||
function severityIntentFactor(e: EventEntry, boostFailures: boolean): number {
|
||||
if (!boostFailures) return 1;
|
||||
// A compliant health-script detection mis-stamped script_failed/Error is benign — damp it
|
||||
// like an Info event so it can't outrank (or sit alongside) real failures on a problem query.
|
||||
if (isBenignHealthDetectionReport(e.eventType, e.data)) return 0.6;
|
||||
const sev = (e.severity ?? '').toLowerCase();
|
||||
const type = (e.eventType ?? '').toLowerCase();
|
||||
if (sev === 'error' || sev === 'critical' || type.includes('failed') || type === 'error_detected') return 1.5;
|
||||
|
||||
@@ -2,7 +2,7 @@ import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
|
||||
import { z } from 'zod';
|
||||
import { apiFetch, buildQuery, followNextLink, pickGlobalOrTenantPath, scanUntilMatch } from '../client.js';
|
||||
import { withToolTelemetry } from '../telemetry.js';
|
||||
import { READ_ONLY, MAX_RESULT_SIZE_CHARS, toolResultText, SessionIdSchema } from './shared.js';
|
||||
import { READ_ONLY, MAX_RESULT_SIZE_CHARS, toolResultText, SessionIdSchema, isBenignHealthDetectionReport } from './shared.js';
|
||||
import { toolError } from './error-handler.js';
|
||||
import { assertKnownEventType, assertKnownDevicePropertyKeys } from '../resource-catalog.js';
|
||||
|
||||
@@ -349,7 +349,11 @@ export function registerSessionTools(server: McpServer, ga: boolean): void {
|
||||
let appSkipped = 0;
|
||||
for (const e of allEvents) {
|
||||
const sev = String(e.severity ?? '');
|
||||
if (sev === 'Error' || sev === 'Critical') errorCount++;
|
||||
// A compliant health-script detection mis-stamped script_failed/Error is benign —
|
||||
// exclude it so a green session's errorCount isn't inflated by routine compliance reports.
|
||||
const benign = isBenignHealthDetectionReport(
|
||||
String(e.eventType ?? ''), e.data as Record<string, unknown> | undefined);
|
||||
if (!benign && (sev === 'Error' || sev === 'Critical')) errorCount++;
|
||||
if (sev === 'Warning') warningCount++;
|
||||
const et = String(e.eventType ?? '');
|
||||
if (et === 'app_install_started') appTotal++;
|
||||
@@ -376,6 +380,9 @@ export function registerSessionTools(server: McpServer, ga: boolean): void {
|
||||
});
|
||||
|
||||
const relevanceScore = (e: Record<string, unknown>): number => {
|
||||
// Benign compliant detection mis-stamped Error → rank as info-level, not top.
|
||||
if (isBenignHealthDetectionReport(
|
||||
String(e.eventType ?? ''), e.data as Record<string, unknown> | undefined)) return 10;
|
||||
const sev = SEVERITY_RANK[String(e.severity ?? '')] ?? -1;
|
||||
if (sev >= 3) return 100; // Error/Critical
|
||||
if (PHASE_EVENT_TYPES.has(String(e.eventType ?? ''))) return 60;
|
||||
|
||||
@@ -16,6 +16,34 @@ export const SessionIdSchema = z
|
||||
'sessionId must be a UUID (e.g. "e259c121-1234-4abc-9def-0123456789ab")',
|
||||
);
|
||||
|
||||
/**
|
||||
* Read-side guard for the historical / in-flight `script_failed` rows that the agent
|
||||
* mis-stamped `Severity=Error`. A health-script DETECTION / post-detection event is a
|
||||
* *compliance report*, not a crash: its authoritative outcome is the compliance verdict,
|
||||
* and detection PowerShell routinely leaks benign probe errors to stderr while still
|
||||
* reporting compliant. The agent historically routed those to `script_failed` / Error,
|
||||
* which inflated `errorCount` and ranked them #1 in `search_events` on green sessions.
|
||||
*
|
||||
* Returns true when an event is one of those benign detection/post-detection reports and
|
||||
* should therefore NOT be treated as an error for ranking or counting. Mirrors the agent's
|
||||
* post-fix routing (detection/post-detection are non-failures unless IME reported
|
||||
* `result === "Failed"`). The agent emit was corrected at the source; this guard de-pollutes
|
||||
* the rows already stored before that build rolls out per-enrollment — without mutating them.
|
||||
*/
|
||||
export function isBenignHealthDetectionReport(
|
||||
eventType: string | undefined,
|
||||
data: Record<string, unknown> | undefined,
|
||||
): boolean {
|
||||
if (eventType !== 'script_failed' || !data) return false;
|
||||
const scriptType = String(data.scriptType ?? data.script_type ?? '').toLowerCase();
|
||||
if (scriptType !== 'remediation') return false;
|
||||
const scriptPart = String(data.scriptPart ?? data.script_part ?? '').toLowerCase();
|
||||
if (scriptPart !== 'detection' && scriptPart !== 'post-detection') return false;
|
||||
// Explicit IME failure verdict is authoritative — keep it as an error.
|
||||
if (String(data.result ?? '').toLowerCase() === 'failed') return false;
|
||||
return true;
|
||||
}
|
||||
|
||||
/** Read-only query tool — no side effects, idempotent, closed-world (our backend only). */
|
||||
export const READ_ONLY: ToolAnnotations = {
|
||||
readOnlyHint: true,
|
||||
|
||||
@@ -523,10 +523,13 @@ describe("reduceScriptEvents", () => {
|
||||
expect(items[0].state).toBe("Failed");
|
||||
});
|
||||
|
||||
it("HEALTH-SCRIPT detection with stderr present renders as Failed (consistency over compliance verdict)", () => {
|
||||
// Earlier behavior: stderr-on-detection was just amber non-compliant. User wanted
|
||||
// consistency: stderr present → Failed everywhere. This is the case from session
|
||||
// e2929c97 (Inventory script with Invoke-WebRequest DNS error).
|
||||
it("HEALTH-SCRIPT detection with stderr present is NOT Failed (compliance verdict is authoritative)", () => {
|
||||
// Detection PowerShell routinely leaks benign probe errors to stderr (path-not-found,
|
||||
// service-not-startable) while still returning a valid compliant verdict. Stamping those
|
||||
// Failed inflated errorCount and ranked them #1 in search_events on green sessions, so
|
||||
// stderr is exempt for detection/post-detection phases — the verdict (complianceResult)
|
||||
// is authoritative. Real script crashes still surface via the remediation phase / platform
|
||||
// scripts / explicit result="Failed".
|
||||
const items = reduceScriptEvents([
|
||||
finalEvent({
|
||||
eventType: "script_completed", ts: 0,
|
||||
@@ -536,11 +539,29 @@ describe("reduceScriptEvents", () => {
|
||||
scriptPart: "detection",
|
||||
exitCode: 0,
|
||||
complianceResult: "True",
|
||||
stderr: "Get-ChildItem : Cannot find path '...JoinInfo' because it does not exist.",
|
||||
},
|
||||
}),
|
||||
]);
|
||||
expect(items[0].state).toBe("Success");
|
||||
});
|
||||
|
||||
it("HEALTH-SCRIPT non-compliant detection with stderr is a NonCompliant report, not Failed", () => {
|
||||
const items = reduceScriptEvents([
|
||||
finalEvent({
|
||||
eventType: "script_completed", ts: 0,
|
||||
data: {
|
||||
policyId: "p1",
|
||||
scriptType: "remediation",
|
||||
scriptPart: "detection",
|
||||
exitCode: 1,
|
||||
complianceResult: "False",
|
||||
stderr: "Invoke-WebRequest : The remote name could not be resolved",
|
||||
},
|
||||
}),
|
||||
]);
|
||||
expect(items[0].state).toBe("Failed");
|
||||
expect(items[0].state).toBe("Success");
|
||||
expect(isNonCompliantReport(items[0])).toBe(true);
|
||||
});
|
||||
|
||||
it("whitespace-only stderr does not flip state to Failed", () => {
|
||||
|
||||
@@ -215,22 +215,20 @@ export function reduceScriptEvents(events: ScriptInputEvent[]): ScriptItem[] {
|
||||
const stderr = typeof d.stderr === "string" ? d.stderr : undefined;
|
||||
const hasStderr = !!stderr && stderr.trim().length > 0;
|
||||
|
||||
// State derivation — three rules in priority order:
|
||||
// 1. stderr present → Failed. Consistent across script types: any time a script
|
||||
// writes to stderr the user wants visibility, even if exit was 0. Per user
|
||||
// preference (debrief 2026-05-11): "exit 0 bedeutet zwar success aber wenn auf
|
||||
// stderr was gezeigt wird sollte es failed sein".
|
||||
// 2. Phase-aware exit handling: detection / post-detection use exit code as a
|
||||
// compliance verdict (not a crash signal), so non-zero exit alone is NOT
|
||||
// failure for those phases. Only remediation phase + platform scripts route
|
||||
// non-zero exit to Failed.
|
||||
// State derivation — rules in priority order (mirrors the agent's EmitScriptCompleted):
|
||||
// 1. Phase-aware exemption: detection / post-detection are *compliance reports*, not
|
||||
// crash signals. Their outcome is the compliance verdict, so NEITHER a non-zero exit
|
||||
// NOR stderr forces Failed for those phases — detection PowerShell routinely leaks
|
||||
// benign probe errors to stderr while still reporting compliant. Non-compliance is
|
||||
// conveyed by complianceResult and rendered amber via isNonCompliantReport, not Failed.
|
||||
// 2. Otherwise (platform scripts, the remediation phase) stderr OR non-zero exit → Failed.
|
||||
// Per user preference (debrief 2026-05-11): stderr on those scripts wants visibility.
|
||||
// 3. Defensive: explicit script_failed eventType OR result === "Failed" → Failed.
|
||||
const isHealthComplianceReport = scriptType === "remediation"
|
||||
&& (scriptPart === "detection" || scriptPart === "post-detection");
|
||||
const isFailureSignal = evt.eventType === "script_failed"
|
||||
|| hasStderr
|
||||
|| (!isHealthComplianceReport && exitCode != null && exitCode !== 0)
|
||||
|| d.result === "Failed";
|
||||
|| d.result === "Failed"
|
||||
|| (!isHealthComplianceReport && (hasStderr || (exitCode != null && exitCode !== 0)));
|
||||
|
||||
const candidate: ScriptItem = {
|
||||
policyId,
|
||||
|
||||
Reference in New Issue
Block a user