From 47bd94712ca0e89359084fbc051dca6ba1076d1e Mon Sep 17 00:00:00 2001 From: Codex Date: Thu, 2 Jul 2026 11:06:41 +0000 Subject: [PATCH] fix: classify quick verify observer cleanup --- ...hwlab-node-web-sentinel-p5-observe.test.ts | 78 +++++++++ .../src/hwlab-node-web-sentinel-p5-observe.ts | 158 ++++++++++++++++-- .../src/hwlab-node-web-sentinel-service.ts | 19 +++ scripts/src/hwlab-node/web-probe-observe.ts | 10 +- 4 files changed, 251 insertions(+), 14 deletions(-) create mode 100644 scripts/src/hwlab-node-web-sentinel-p5-observe.test.ts diff --git a/scripts/src/hwlab-node-web-sentinel-p5-observe.test.ts b/scripts/src/hwlab-node-web-sentinel-p5-observe.test.ts new file mode 100644 index 00000000..312458c9 --- /dev/null +++ b/scripts/src/hwlab-node-web-sentinel-p5-observe.test.ts @@ -0,0 +1,78 @@ +import assert from "node:assert/strict"; +import { test } from "bun:test"; + +import { classifyQuickVerifyCleanupStep, quickVerifyCleanupFindings } from "./hwlab-node-web-sentinel-p5-observe"; + +function forceStopStep(observer: Record): Record { + return { + phase: "observe-stop-after-terminal", + ok: false, + payload: { + ok: false, + status: "blocked", + forceReason: "graceful-stop-not-consumed", + observer: { + command: "web-probe-observe force-stop", + ...observer, + }, + valuesRedacted: true, + }, + valuesRedacted: true, + }; +} + +test("quick verify cleanup downgrades abandoned graceful stop when force stop confirms not alive", () => { + const step = forceStopStep({ + aliveBefore: true, + aliveAfter: false, + termSent: true, + killSent: true, + pendingAbandoned: 1, + processingAbandoned: 0, + }); + + const cleanup = classifyQuickVerifyCleanupStep(step); + assert.equal(cleanup.cleanupClass, "bounded-force-stop"); + assert.equal(cleanup.gracefulStopOk, false); + assert.equal(cleanup.forceStopOk, true); + assert.equal(cleanup.aliveAfter, false); + assert.equal(cleanup.pendingAbandoned, 1); + assert.deepEqual(quickVerifyCleanupFindings(step), []); +}); + +test("quick verify cleanup emits WBC-096 when force stop leaves observer alive", () => { + const step = forceStopStep({ + aliveBefore: true, + aliveAfter: true, + termSent: true, + killSent: false, + pendingAbandoned: 0, + processingAbandoned: 1, + }); + + const cleanup = classifyQuickVerifyCleanupStep(step); + const findings = quickVerifyCleanupFindings(step); + assert.equal(cleanup.cleanupClass, "observer-stop-failed"); + assert.equal(cleanup.forceStopOk, false); + assert.equal(cleanup.aliveAfter, true); + assert.equal(findings.length, 1); + assert.equal(findings[0]?.id, "quick-verify-observer-stop-failed"); + assert.match(String(findings[0]?.evidenceSummary), /aliveAfter=true/u); +}); + +test("quick verify cleanup emits WBC-096 when stopped state cannot be confirmed", () => { + const step = forceStopStep({ + aliveBefore: true, + termSent: true, + killSent: true, + pendingAbandoned: 1, + processingAbandoned: 0, + }); + + const cleanup = classifyQuickVerifyCleanupStep(step); + const findings = quickVerifyCleanupFindings(step); + assert.equal(cleanup.cleanupClass, "observer-stop-failed"); + assert.equal(cleanup.aliveAfter, null); + assert.equal(findings.length, 1); + assert.equal(findings[0]?.id, "quick-verify-observer-stop-failed"); +}); diff --git a/scripts/src/hwlab-node-web-sentinel-p5-observe.ts b/scripts/src/hwlab-node-web-sentinel-p5-observe.ts index a94a7e84..62cc20ee 100644 --- a/scripts/src/hwlab-node-web-sentinel-p5-observe.ts +++ b/scripts/src/hwlab-node-web-sentinel-p5-observe.ts @@ -326,6 +326,7 @@ export function runSentinelQuickVerify(state: SentinelCicdState, reason: string, const artifactFindings = Array.isArray(artifactSummaryRecord.findings) ? artifactSummaryRecord.findings.map(record) : []; const visibilityFindings = quickVerifyAnalysisVisibilityFindings(analysis, artifactSummary); const cleanupStep = stopQuickVerifyObserver(state, observerId, "observe-stop-after-terminal"); + const cleanup = classifyQuickVerifyCleanupStep(cleanupStep); const cleanupFindings = quickVerifyCleanupFindings(cleanupStep); const findings = mergeFindingRecords( mergeFindingRecords( @@ -361,17 +362,18 @@ export function runSentinelQuickVerify(state: SentinelCicdState, reason: string, promptSource: prompts.summary, accountEnv: accountEnv.summary, steps: [...steps, cleanupStep], + cleanup, analysis: artifactSummary, views: { - summary: { renderedText: renderQuickVerifySummary({ runId, scenarioId, observerId, artifactSummary, findings, findingCount: findings.length, steps, publicOrigin: stringAt(state.publicExposure, "publicBaseUrl") }) }, - "auth-session-switch-summary": { renderedText: renderAuthSessionSwitchQuickVerifySummary({ runId, scenarioId, observerId, artifactSummary, steps, findings, accountEnv: accountEnv.summary, publicOrigin: stringAt(state.publicExposure, "publicBaseUrl") }) }, + summary: { renderedText: renderQuickVerifySummary({ runId, scenarioId, observerId, artifactSummary, findings, findingCount: findings.length, steps, cleanup, publicOrigin: stringAt(state.publicExposure, "publicBaseUrl") }) }, + "auth-session-switch-summary": { renderedText: renderAuthSessionSwitchQuickVerifySummary({ runId, scenarioId, observerId, artifactSummary, steps, cleanup, findings, accountEnv: accountEnv.summary, publicOrigin: stringAt(state.publicExposure, "publicBaseUrl") }) }, "turn-summary": { renderedText: typeof turnSummary.renderedText === "string" ? turnSummary.renderedText : null, ok: turnSummary.ok }, "trace-frame": { renderedText: typeof traceFrame.renderedText === "string" ? traceFrame.renderedText : null, ok: traceFrame.ok }, }, findings, screenshot: record(artifactSummary).screenshot, publicOrigin: stringAt(state.publicExposure, "publicBaseUrl"), - warnings: mergeWarnings(analysisWarnings, nonBlockingCanaryWarnings, cleanupFindings.length > 0 ? ["quick verify observer stop failed; runner lifecycle cleanup is a blocking finding."] : [], elapsedWarnings()), + warnings: mergeWarnings(analysisWarnings, nonBlockingCanaryWarnings, quickVerifyCleanupWarnings(cleanup), elapsedWarnings()), valuesRedacted: true, }); } @@ -570,9 +572,14 @@ function finalizeQuickVerifyFailure(state: SentinelCicdState, input: { const artifactSummaryRecord = record(artifactSummary); const artifactFindings = Array.isArray(artifactSummaryRecord.findings) ? artifactSummaryRecord.findings.map(record) : []; const visibilityFindings = quickVerifyAnalysisVisibilityFindings(analysis, artifactSummary); + const cleanup = quickVerifyCleanupFromSteps(cleanupSteps); + const cleanupFindings = cleanupSteps.flatMap((step) => quickVerifyCleanupFindings(step)); const findings = mergeFindingRecords( - mergeFindingRecords(artifactFindings, controlFindings), - visibilityFindings, + mergeFindingRecords( + mergeFindingRecords(artifactFindings, controlFindings), + visibilityFindings, + ), + cleanupFindings, ); const blockingFindings = findings.filter(isQuickVerifyBlockingFinding); const recoveredWaitFailure = durableBusinessTurn @@ -602,10 +609,11 @@ function finalizeQuickVerifyFailure(state: SentinelCicdState, input: { runnerFailure: recoveredWaitFailure ? null : input.failure, promptSource: input.promptSource, steps: [...input.steps, ...cleanupSteps], + cleanup, analysis: artifactSummary, views: { - summary: { renderedText: renderQuickVerifySummary({ runId: input.runId, scenarioId: input.scenarioId, observerId: input.observerId, artifactSummary, findings, findingCount: findings.length, steps: input.steps, publicOrigin: stringAt(state.publicExposure, "publicBaseUrl"), failure: recoveredWaitFailure ? null : input.failure, failureTitleZh: recoveredWaitFailure ? null : failureTitle, timeoutDisplay: recoveredWaitFailure ? null : failureDisplay }) }, - "auth-session-switch-summary": { renderedText: renderAuthSessionSwitchQuickVerifySummary({ runId: input.runId, scenarioId: input.scenarioId, observerId: input.observerId, artifactSummary, steps: input.steps, findings, publicOrigin: stringAt(state.publicExposure, "publicBaseUrl"), failure: recoveredWaitFailure ? null : input.failure, failureTitleZh: recoveredWaitFailure ? null : failureTitle, timeoutDisplay: recoveredWaitFailure ? null : failureDisplay }) }, + summary: { renderedText: renderQuickVerifySummary({ runId: input.runId, scenarioId: input.scenarioId, observerId: input.observerId, artifactSummary, findings, findingCount: findings.length, steps: input.steps, cleanup, publicOrigin: stringAt(state.publicExposure, "publicBaseUrl"), failure: recoveredWaitFailure ? null : input.failure, failureTitleZh: recoveredWaitFailure ? null : failureTitle, timeoutDisplay: recoveredWaitFailure ? null : failureDisplay }) }, + "auth-session-switch-summary": { renderedText: renderAuthSessionSwitchQuickVerifySummary({ runId: input.runId, scenarioId: input.scenarioId, observerId: input.observerId, artifactSummary, steps: input.steps, cleanup, findings, publicOrigin: stringAt(state.publicExposure, "publicBaseUrl"), failure: recoveredWaitFailure ? null : input.failure, failureTitleZh: recoveredWaitFailure ? null : failureTitle, timeoutDisplay: recoveredWaitFailure ? null : failureDisplay }) }, "turn-summary": { renderedText: typeof turnSummary.renderedText === "string" ? turnSummary.renderedText : null, ok: turnSummary.ok }, "trace-frame": { renderedText: typeof traceFrame.renderedText === "string" ? traceFrame.renderedText : null, ok: traceFrame.ok }, }, @@ -615,6 +623,7 @@ function finalizeQuickVerifyFailure(state: SentinelCicdState, input: { warnings: mergeWarnings( Array.isArray(input.warnings) ? input.warnings : [], recoveredWaitFailure ? ["quick verify wait command timed out, but collected turn-summary/trace-frame artifacts show a durable completed business turn; treating the wait timeout as a non-blocking tool finding."] : [], + quickVerifyCleanupWarnings(cleanup), targetValidationElapsedWarnings(input.elapsedMs ?? null, "quick verify confirm-wait", targetValidationSeconds), ), valuesRedacted: true, @@ -627,24 +636,125 @@ function stopQuickVerifyObserver(state: SentinelCicdState, observerId: string, p "--node", state.spec.nodeId, "--lane", state.spec.lane, "--force", + "--raw", "--wait-ms", "55000", "--command-timeout-seconds", "55", ], 60); - return { phase, ok: stop.ok, result: stop.result }; + const step = { phase, ok: stop.ok, payload: cliDataPayload(stop.parsed), result: stop.result }; + return { ...step, cleanup: classifyQuickVerifyCleanupStep(step) }; } -function quickVerifyCleanupFindings(cleanupStep: Record): Record[] { - if (cleanupStep.ok === true) return []; +export function quickVerifyCleanupFindings(cleanupStep: Record): Record[] { + const cleanup = classifyQuickVerifyCleanupStep(cleanupStep); + if (stringAtNullable(cleanup, "cleanupClass") !== "observer-stop-failed") return []; return [{ id: "quick-verify-observer-stop-failed", severity: "red", count: 1, summary: "quick verify completed target analysis but failed to stop its observer runner; this can leak Chrome process trees on cadence runs.", cleanupPhase: cleanupStep.phase, + cleanup, + evidenceSummary: formatQuickVerifyCleanupLine(cleanup), valuesRedacted: true, }]; } +export function classifyQuickVerifyCleanupStep(cleanupStep: Record): Record { + const explicit = record(cleanupStep.cleanup); + if (stringAtNullable(explicit, "cleanupClass") !== null) return explicit; + const payload = quickVerifyCleanupPayload(cleanupStep); + const observer = record(payload.observer); + const forceResult = record(payload.forceResult); + const forceReason = stringAtNullable(payload, "forceReason"); + const status = stringAtNullable(payload, "status"); + const forceStopAttempted = forceReason !== null + || status === "forced-stopped" + || stringAtNullable(observer, "command") === "web-probe-observe force-stop" + || numberAtNullable(forceResult, "exitCode") !== null; + const aliveBefore = booleanAtNullable(observer, "aliveBefore"); + const aliveAfter = booleanAtNullable(observer, "aliveAfter"); + const termSent = booleanAtNullable(observer, "termSent"); + const killSent = booleanAtNullable(observer, "killSent"); + const pendingAbandoned = numberAtNullable(observer, "pendingAbandoned"); + const processingAbandoned = numberAtNullable(observer, "processingAbandoned"); + const forceStopOk = forceStopAttempted && aliveAfter === false; + const gracefulStopOk = !forceStopAttempted && (cleanupStep.ok === true || payload.ok === true); + const cleanupClass = gracefulStopOk + ? "stopped" + : forceStopOk + ? "bounded-force-stop" + : "observer-stop-failed"; + return { + cleanupClass, + phase: stringAtNullable(cleanupStep, "phase"), + commandOk: cleanupStep.ok === true ? true : cleanupStep.ok === false ? false : null, + status, + forceReason, + gracefulStopOk, + forceStopOk, + aliveBefore, + aliveAfter, + termSent, + killSent, + pendingAbandoned, + processingAbandoned, + valuesRedacted: true, + }; +} + +function quickVerifyCleanupPayload(cleanupStep: Record): Record { + const payload = record(cleanupStep.payload); + if (Object.keys(payload).length > 0) return payload; + const result = record(cleanupStep.result); + for (const key of ["stdoutPreview", "stdoutTail"]) { + const parsed = parseJsonObject(stringAtNullable(result, key) ?? ""); + const data = cliDataPayload(parsed); + if (Object.keys(data).length > 0) return data; + } + return {}; +} + +function quickVerifyCleanupFromSteps(steps: readonly Record[]): Record | null { + for (const step of steps.slice().reverse()) { + const phase = stringAtNullable(step, "phase"); + if (phase !== null && phase.includes("observe-stop")) return classifyQuickVerifyCleanupStep(step); + } + return null; +} + +function quickVerifyCleanupWarnings(cleanup: Record | null): string[] { + if (cleanup === null) return []; + const cleanupClass = stringAtNullable(cleanup, "cleanupClass"); + if (cleanupClass === "bounded-force-stop") { + return [`quick verify observer graceful stop did not drain, but force stop completed with aliveAfter=false; recorded bounded cleanup evidence (${formatQuickVerifyCleanupLine(cleanup)}).`]; + } + if (cleanupClass === "observer-stop-failed") { + return ["quick verify observer stop failed; runner lifecycle cleanup is a blocking finding."]; + } + return []; +} + +function formatQuickVerifyCleanupLine(cleanup: Record | null): string { + if (cleanup === null) return "cleanup=-"; + const fields = [ + ["cleanup", stringAtNullable(cleanup, "cleanupClass")], + ["aliveBefore", cleanup.aliveBefore], + ["aliveAfter", cleanup.aliveAfter], + ["termSent", cleanup.termSent], + ["killSent", cleanup.killSent], + ["pendingAbandoned", cleanup.pendingAbandoned], + ["processingAbandoned", cleanup.processingAbandoned], + ["gracefulStopOk", cleanup.gracefulStopOk], + ["forceStopOk", cleanup.forceStopOk], + ]; + return fields.map(([key, value]) => `${key}=${value ?? "-"}`).join(" "); +} + +function booleanAtNullable(value: Record, key: string): boolean | null { + const raw = value[key]; + return raw === true ? true : raw === false ? false : null; +} + function quickVerifyAnalysisVisibilityFindings(analysis: ChildCliResult, artifactSummary: Record): Record[] { if (analysis.ok !== true) return []; if (stringAtNullable(artifactSummary, "reportJsonSha256") !== null) return []; @@ -773,6 +883,7 @@ function recordQuickVerify(state: SentinelCicdState, payload: Record { promptIndex: numberAtNullable(item, "promptIndex"), checkId: stringAtNullable(item, "checkId"), failure: stringAtNullable(item, "failure"), + cleanup: compactQuickVerifyCleanupEvidence(item.cleanup), result: compactQuickVerifyRecordStepResult(record(item.result)), valuesRedacted: true, }; } +function compactQuickVerifyCleanupEvidence(value: unknown): Record | null { + const item = record(value); + if (stringAtNullable(item, "cleanupClass") === null) return null; + return { + cleanupClass: stringAtNullable(item, "cleanupClass"), + phase: stringAtNullable(item, "phase"), + commandOk: booleanAtNullable(item, "commandOk"), + status: stringAtNullable(item, "status"), + forceReason: stringAtNullable(item, "forceReason"), + gracefulStopOk: booleanAtNullable(item, "gracefulStopOk"), + forceStopOk: booleanAtNullable(item, "forceStopOk"), + aliveBefore: booleanAtNullable(item, "aliveBefore"), + aliveAfter: booleanAtNullable(item, "aliveAfter"), + termSent: booleanAtNullable(item, "termSent"), + killSent: booleanAtNullable(item, "killSent"), + pendingAbandoned: numberAtNullable(item, "pendingAbandoned"), + processingAbandoned: numberAtNullable(item, "processingAbandoned"), + valuesRedacted: true, + }; +} + function compactQuickVerifyRecordStepResult(value: Record): Record { return { ok: value.ok === true ? true : value.ok === false ? false : null, @@ -1862,6 +1995,7 @@ function isQuickVerifyBlockingFinding(item: Record): boolean { "quick-verify-analysis-summary-unreadable", "quick-verify-command-sequence-failed", "quick-verify-observer-start-failed", + "quick-verify-observer-stop-failed", "quick-verify-account-secret-missing", "prompt-chat-submit-failed", "workbench-turn-state-triad-inconsistent", @@ -2369,12 +2503,14 @@ function renderQuickVerifySummary(input: Record): string { : []; const findingCount = numberAtNullable(input, "findingCount") ?? numberAtNullable(artifact, "findingCount") ?? findings.length; const failureTitle = stringAtNullable(input, "failureTitleZh") ?? stringAtNullable(input, "errorTitleZh") ?? stringAtNullable(record(input.timeoutDisplay), "titleZh"); + const cleanup = compactQuickVerifyCleanupEvidence(input.cleanup); return [ "Web Probe Sentinel Quick Verify", "=======================================================", `run=${input.runId ?? "-"} scenario=${input.scenarioId ?? "-"} observer=${input.observerId ?? "-"}`, failureTitle === null ? "" : `errorTitle=${failureTitle} failure=${input.failure ?? "-"}`, `report=${artifact.reportJsonSha256 ?? "-"} artifacts=${artifact.artifactCount ?? "-"} findings=${findingCount}`, + cleanup === null ? "" : formatQuickVerifyCleanupLine(cleanup), `publicOrigin=${input.publicOrigin ?? "-"}`, "", "Findings", @@ -2385,6 +2521,7 @@ function renderQuickVerifySummary(input: Record): string { function renderAuthSessionSwitchQuickVerifySummary(input: Record): string { const artifact = record(input.artifactSummary); const accountEnv = record(input.accountEnv); + const cleanup = compactQuickVerifyCleanupEvidence(input.cleanup); const findingRows = Array.isArray(input.findings) ? input.findings.map(record).slice(0, 8) : Array.isArray(artifact.findings) @@ -2409,6 +2546,7 @@ function renderAuthSessionSwitchQuickVerifySummary(input: Record, findings: readonly R ].join("\n"); } +function formatStoredCleanupSummary(value: unknown): string { + const cleanup = record(value); + const cleanupClass = stringOrNull(cleanup.cleanupClass); + if (cleanupClass === null) return "-"; + const fields = [ + ["class", cleanupClass], + ["aliveBefore", cleanup.aliveBefore], + ["aliveAfter", cleanup.aliveAfter], + ["termSent", cleanup.termSent], + ["killSent", cleanup.killSent], + ["pendingAbandoned", cleanup.pendingAbandoned], + ["processingAbandoned", cleanup.processingAbandoned], + ["gracefulStopOk", cleanup.gracefulStopOk], + ["forceStopOk", cleanup.forceStopOk], + ]; + return fields.map(([key, fieldValue]) => `${key}=${fieldValue ?? "-"}`).join(" "); +} + function formatStoredAnalysisWindow(value: unknown): string { const window = record(value); const fields = [ diff --git a/scripts/src/hwlab-node/web-probe-observe.ts b/scripts/src/hwlab-node/web-probe-observe.ts index 5e635ddb..1633b9e3 100644 --- a/scripts/src/hwlab-node/web-probe-observe.ts +++ b/scripts/src/hwlab-node/web-probe-observe.ts @@ -1963,7 +1963,7 @@ export function runNodeWebProbeObserveCommand(options: NodeWebProbeObserveOption : "graceful-stop-queued"; return runNodeWebProbeObserveForceStop(options, spec, payload, commandId, reason, preStopStatus?.result ?? null, preStopStatus?.status ?? null, result); } - return withWebObserveCommandRendered({ + const payloadResult = { ok: result.exitCode === 0 && commandResult?.ok !== false, status: result.exitCode === 0 ? (waitMs > 0 ? "completed-or-queued" : "queued") : "blocked", command: webObserveCommandLabel(stopCommand ? "stop" : "command", options), @@ -1978,7 +1978,8 @@ export function runNodeWebProbeObserveCommand(options: NodeWebProbeObserveOption result: compactCommandResult(result), full: options.full, valuesRedacted: true, - }); + }; + return options.raw ? payloadResult : withWebObserveCommandRendered(payloadResult); } export function runNodeWebProbeObserveForceStop( @@ -1997,7 +1998,7 @@ export function runNodeWebProbeObserveForceStop( nodeWebObserveForceStopNodeScript(reason, commandId), ].join("\n"), 55); const forcePayload = parseJsonObject(killResult.stdout); - return withWebObserveCommandRendered({ + const payloadResult = { ok: killResult.exitCode === 0 && forcePayload?.ok !== false, status: killResult.exitCode === 0 && forcePayload?.ok !== false ? "forced-stopped" : "blocked", command: webObserveCommandLabel("stop", options), @@ -2016,7 +2017,8 @@ export function runNodeWebProbeObserveForceStop( forceResult: compactCommandResultWithStdoutTail(killResult), full: options.full, valuesRedacted: true, - }); + }; + return options.raw ? payloadResult : withWebObserveCommandRendered(payloadResult); } export function runNodeWebProbeObserveCollect(options: NodeWebProbeObserveOptions, spec: HwlabRuntimeLaneSpec): Record | RenderedCliResult {