From f37729061dbff3c0b4c29cec208a8bc9f1bcd12f Mon Sep 17 00:00:00 2001 From: Codex Date: Thu, 2 Jul 2026 18:36:04 +0000 Subject: [PATCH] fix(web-probe): reject empty analyze stdout contracts --- scripts/src/cli-child-json-recovery.test.ts | 44 +++++++++++++++++++++ scripts/src/cli-child-json-recovery.ts | 5 ++- scripts/src/hwlab-node/web-probe-observe.ts | 25 ++++++++---- 3 files changed, 65 insertions(+), 9 deletions(-) diff --git a/scripts/src/cli-child-json-recovery.test.ts b/scripts/src/cli-child-json-recovery.test.ts index 05079067..d3d0bb83 100644 --- a/scripts/src/cli-child-json-recovery.test.ts +++ b/scripts/src/cli-child-json-recovery.test.ts @@ -85,6 +85,50 @@ test("child JSON recovery falls back to artifact when stdout is not JSON", () => assert.equal(resolved.diagnostics.fallbackReason, "stdout-not-json"); }); +test("child JSON recovery rejects ok-only stdout contract and uses artifact", () => { + const resolved = resolveCliChildJsonObject({ + stdout: JSON.stringify({ ok: true }), + requestedStdoutType: "web-probe observe analyze compact JSON", + acceptParsed: (value) => typeof value.reportJsonPath === "string" || typeof value.counts === "object", + artifactFallback: { + path: "analysis/report.json", + nextCommand: "collect report", + read: () => ({ ok: true, value: { ok: true, reportJsonPath: "analysis/report.json", counts: { network: 9 } } }), + }, + }); + + assert.equal(resolved.source, "artifact"); + assert.equal(resolved.parsed?.reportJsonPath, "analysis/report.json"); + assert.deepEqual(resolved.parsed?.counts, { network: 9 }); + assert.equal(resolved.diagnostics.stdoutKind, "json"); + assert.equal(resolved.diagnostics.fallbackReason, "stdout-json-contract-invalid"); + assert.equal(resolved.diagnostics.stdoutContractAccepted, false); +}); + +test("child JSON recovery drops invalid ok-only stdout when artifact fallback is missing", () => { + const resolved = resolveCliChildJsonObject({ + stdout: JSON.stringify({ ok: true }), + requestedStdoutType: "web-probe observe analyze compact JSON", + acceptParsed: (value) => typeof value.reportJsonPath === "string" || typeof value.counts === "object", + artifactFallback: { + path: "analysis/report.json", + nextCommand: "collect report", + read: () => ({ ok: false, reason: "artifact-missing", path: "analysis/report.json" }), + }, + }); + + assert.equal(resolved.parsed, null); + assert.equal(resolved.source, null); + assert.equal(resolved.diagnostics.stdoutKind, "json"); + assert.equal(resolved.diagnostics.fallbackReason, "stdout-json-contract-invalid"); + assert.equal(resolved.diagnostics.stdoutContractAccepted, false); + const artifact = resolved.diagnostics.artifact as Record; + assert.equal(artifact.ok, false); + assert.equal(artifact.reason, "artifact-missing"); + assert.equal(artifact.path, "analysis/report.json"); + assert.equal(artifact.nextCommand, "collect report"); +}); + test("child JSON recovery reports artifact fallback failure when stdout is unusable and artifact is missing", () => { const resolved = resolveCliChildJsonObject({ stdout: "not json", diff --git a/scripts/src/cli-child-json-recovery.ts b/scripts/src/cli-child-json-recovery.ts index 2ea42f6c..9a750206 100644 --- a/scripts/src/cli-child-json-recovery.ts +++ b/scripts/src/cli-child-json-recovery.ts @@ -35,8 +35,8 @@ export function resolveCliChildJsonObject(options: { ?? contractFallbackReason ?? (parsedStdout.parsed === null ? fallbackReasonForStdout(parsedStdout.stdoutKind) : null); - let parsed = parsedStdout.parsed; - let source: CliChildJsonSource = parsedStdout.source; + let parsed = contractFallbackReason === null ? parsedStdout.parsed : null; + let source: CliChildJsonSource = contractFallbackReason === null ? parsedStdout.source : null; let artifactDiagnostics: Record | null = null; if (fallbackReason !== null && options.artifactFallback) { const artifact = options.artifactFallback.read(); @@ -65,6 +65,7 @@ export function resolveCliChildJsonObject(options: { fallbackReason, parsedFromStdout: parsedStdout.source === "stdout", parsedFromDump: parsedStdout.source === "dump", + stdoutContractAccepted: accepted, dumpPath: parsedStdout.dumpPath, dumpReason: parsedStdout.dumpReason, dumpReadOk: parsedStdout.dumpReadOk, diff --git a/scripts/src/hwlab-node/web-probe-observe.ts b/scripts/src/hwlab-node/web-probe-observe.ts index a6e020f3..ff8c17cb 100644 --- a/scripts/src/hwlab-node/web-probe-observe.ts +++ b/scripts/src/hwlab-node/web-probe-observe.ts @@ -2607,13 +2607,24 @@ function webObserveAnalyzeCollectCommand(options: NodeWebProbeObserveOptions, fi } function isWebObserveAnalyzeJsonContract(value: Record): boolean { - return value.ok === true - || value.ok === false - || typeof value.reportJsonPath === "string" - || typeof value.reportMdPath === "string" - || typeof value.error === "string" - || Object.keys(recordValue(value.analyzer)).length > 0 - || Object.keys(recordValue(value.counts)).length > 0; + const hasReportPath = typeof value.reportJsonPath === "string" || typeof value.reportMdPath === "string"; + const hasAnalyzer = Object.keys(recordValue(value.analyzer)).length > 0; + const hasFindings = arrayRecordsValue(value.findings).length > 0 || arrayRecordsValue(value.archiveRedFindings).length > 0; + const hasAnalyzeData = hasReportPath + || hasAnalyzer + || hasFindings + || Object.keys(recordValue(value.counts)).length > 0 + || Object.keys(recordValue(value.sampleMetrics)).length > 0 + || Object.keys(recordValue(value.requestRate)).length > 0 + || Object.keys(recordValue(value.requestRateCurve)).length > 0 + || Object.keys(recordValue(value.frontendPerformance)).length > 0 + || Object.keys(recordValue(value.webPerformanceRuntimeDiagnostics)).length > 0 + || Object.keys(recordValue(value.runtimeAlerts)).length > 0 + || Object.keys(recordValue(value.archiveSummary)).length > 0 + || Object.keys(recordValue(value.analysisWindow)).length > 0; + const hasFailureData = typeof value.error === "string" || hasAnalyzer || hasFindings || hasReportPath; + if (value.ok === false) return hasFailureData; + return hasAnalyzeData; } function compactWebObserveAnalyzePayloadForRaw(payload: Record, compactRaw: boolean): Record {