diff --git a/packages/engine/src/services/frameCapture.test.ts b/packages/engine/src/services/frameCapture.test.ts index c66505791..76d4fe33f 100644 --- a/packages/engine/src/services/frameCapture.test.ts +++ b/packages/engine/src/services/frameCapture.test.ts @@ -229,12 +229,14 @@ describe("navigation diagnostics", () => { describe("DrawElementVerificationError details", () => { it("carries frameIndex/failedDb/verifyThresholdDb when provided", () => { const err = new DrawElementVerificationError("drawElement self-verify failed at frame 649", { + kind: "psnr", frameIndex: 649, failedDb: 28.4, verifyThresholdDb: 32, }); expect(isDrawElementVerificationError(err)).toBe(true); expect(getDrawElementVerificationDetails(err)).toEqual({ + kind: "psnr", frameIndex: 649, failedDb: 28.4, verifyThresholdDb: 32, @@ -242,8 +244,11 @@ describe("DrawElementVerificationError details", () => { }); it("omits fields that weren't supplied (blank-frame throws have no dB)", () => { - const err = new DrawElementVerificationError("blank drawElement frame 12", { frameIndex: 12 }); - expect(getDrawElementVerificationDetails(err)).toEqual({ frameIndex: 12 }); + const err = new DrawElementVerificationError("blank drawElement frame 12", { + kind: "blank", + frameIndex: 12, + }); + expect(getDrawElementVerificationDetails(err)).toEqual({ kind: "blank", frameIndex: 12 }); }); it("returns undefined for a non-verification error", () => { @@ -252,11 +257,49 @@ describe("DrawElementVerificationError details", () => { it("finds details through a wrapping cause chain (producer's CaptureStageError)", () => { const inner = new DrawElementVerificationError("psnr breach", { + kind: "psnr", frameIndex: 5, failedDb: 12.1, }); const wrapper = new Error("capture stage failed", { cause: inner }); expect(isDrawElementVerificationError(wrapper)).toBe(true); - expect(getDrawElementVerificationDetails(wrapper)).toEqual({ frameIndex: 5, failedDb: 12.1 }); + expect(getDrawElementVerificationDetails(wrapper)).toEqual({ + kind: "psnr", + frameIndex: 5, + failedDb: 12.1, + }); + }); + + it("carries kind='blank'/'psnr' as a structural field, not derived from message text", () => { + const blankErr = new DrawElementVerificationError("blank drawElement frame 12", { + kind: "blank", + frameIndex: 12, + }); + const psnrErr = new DrawElementVerificationError( + "drawElement self-verify failed at frame 649", + { kind: "psnr", frameIndex: 649, failedDb: 28.4, verifyThresholdDb: 32 }, + ); + expect(getDrawElementVerificationDetails(blankErr)?.kind).toBe("blank"); + expect(getDrawElementVerificationDetails(psnrErr)?.kind).toBe("psnr"); + }); + + it("kind survives even when the message text disagrees with (or omits) the word psnr/blank", () => { + // A reworded message that says neither "blank" nor "psnr" — a regex over + // message text would have no signal here; the structural kind must still + // report correctly. + const reworded = new DrawElementVerificationError("frame 12 failed verification", { + kind: "blank", + frameIndex: 12, + }); + expect(getDrawElementVerificationDetails(reworded)?.kind).toBe("blank"); + + // Adversarial: a psnr failure whose message happens to mention "blank" + // (e.g. quoting a neighboring log line) — the structural kind must not + // flip just because the substring "blank" appears in the text. + const adversarial = new DrawElementVerificationError( + "drawElement self-verify failed at frame 5 (previous frame was blank-guard-accepted)", + { kind: "psnr", frameIndex: 5, failedDb: 12.1, verifyThresholdDb: 32 }, + ); + expect(getDrawElementVerificationDetails(adversarial)?.kind).toBe("psnr"); }); }); diff --git a/packages/engine/src/services/frameCapture.ts b/packages/engine/src/services/frameCapture.ts index 1edf7b390..54a1a529d 100644 --- a/packages/engine/src/services/frameCapture.ts +++ b/packages/engine/src/services/frameCapture.ts @@ -203,32 +203,38 @@ export interface CaptureSession { */ /** * Structured detail carried alongside the human-readable message — lets - * telemetry report the actual failing dB / frame index instead of the - * orchestrator having to regex them back out of formatted text. All optional: - * a blank-frame trip has no PSNR score, so `failedDb`/`verifyThresholdDb` - * are omitted for that throw site. + * telemetry report the actual failure kind / failing dB / frame index + * instead of the orchestrator having to regex them back out of formatted + * text (a message-text dependency is exactly the failure mode this shape + * exists to close — review finding: message wording, translation, or a + * cross-module/serialized error must never be able to flip the reported + * kind). All fields but `kind` are optional: a blank-frame trip has no PSNR + * score, so `failedDb`/`verifyThresholdDb` are omitted for that throw site. */ export interface DrawElementVerificationDetails { + kind: "blank" | "psnr"; frameIndex?: number; failedDb?: number; verifyThresholdDb?: number; } export class DrawElementVerificationError extends Error { + readonly kind: "blank" | "psnr"; readonly frameIndex?: number; readonly failedDb?: number; readonly verifyThresholdDb?: number; - constructor(message: string, details?: DrawElementVerificationDetails) { + constructor(message: string, details: DrawElementVerificationDetails) { super(message); this.name = "DrawElementVerificationError"; // Discriminant property, assigned dynamically: isDrawElementVerificationError // reads it structurally so detection survives duplicated module instances // across package boundaries (where instanceof fails). (this as unknown as { deVerificationFailure: boolean }).deVerificationFailure = true; - this.frameIndex = details?.frameIndex; - this.failedDb = details?.failedDb; - this.verifyThresholdDb = details?.verifyThresholdDb; + this.kind = details.kind; + this.frameIndex = details.frameIndex; + this.failedDb = details.failedDb; + this.verifyThresholdDb = details.verifyThresholdDb; } } @@ -255,7 +261,14 @@ export function getDrawElementVerificationDetails( for (let depth = 0; depth < 5 && typeof e === "object" && e !== null; depth++) { const rec = e as { deVerificationFailure?: boolean } & Partial; if (rec.deVerificationFailure === true) { - const details: DrawElementVerificationDetails = {}; + // Every construction path sets `kind` (required on the constructor), so + // this only defends against a malformed cross-module-instance shape — + // treat anything other than exactly "blank" as "psnr", the same + // fallback polarity the old message regex had, but driven by a + // structural field instead of parsing text. + const details: DrawElementVerificationDetails = { + kind: rec.kind === "blank" ? "blank" : "psnr", + }; if (typeof rec.frameIndex === "number") details.frameIndex = rec.frameIndex; if (typeof rec.failedDb === "number") details.failedDb = rec.failedDb; if (typeof rec.verifyThresholdDb === "number") diff --git a/packages/producer/src/services/render/stages/captureStreamingStage.ts b/packages/producer/src/services/render/stages/captureStreamingStage.ts index 9fdc1dbe9..d94b669d6 100644 --- a/packages/producer/src/services/render/stages/captureStreamingStage.ts +++ b/packages/producer/src/services/render/stages/captureStreamingStage.ts @@ -289,7 +289,7 @@ function createDrainFrameGuard(args: { } catch (err) { throw new DrawElementVerificationError( `blank drawElement frame ${idx}: ${buf.length}B < floor ${Math.round(floor)}B and recapture failed (${err instanceof Error ? err.message : String(err)})`, - { frameIndex: idx }, + { kind: "blank", frameIndex: idx }, ); } if (retryBuf.equals(buf)) { @@ -307,7 +307,7 @@ function createDrainFrameGuard(args: { } else if (retryBuf.length < floor) { throw new DrawElementVerificationError( `blank drawElement frame ${idx}: ${buf.length}B (retry ${retryBuf.length}B) < floor ${Math.round(floor)}B`, - { frameIndex: idx }, + { kind: "blank", frameIndex: idx }, ); } else { buf = retryBuf; @@ -341,7 +341,7 @@ function createDrainFrameGuard(args: { } throw new DrawElementVerificationError( `drawElement self-verify failed at frame ${idx}: ${db.toFixed(1)}dB < ${verifyMinDb}dB vs pre-injection screenshot${dumpDir ? ` (pair: ${dumpDir})` : ""}`, - { frameIndex: idx, failedDb: db, verifyThresholdDb: verifyMinDb }, + { kind: "psnr", frameIndex: idx, failedDb: db, verifyThresholdDb: verifyMinDb }, ); } stats.verifyChecked += 1; diff --git a/packages/producer/src/services/renderOrchestrator.ts b/packages/producer/src/services/renderOrchestrator.ts index d2d804d89..08cd269a8 100644 --- a/packages/producer/src/services/renderOrchestrator.ts +++ b/packages/producer/src/services/renderOrchestrator.ts @@ -2729,15 +2729,17 @@ export async function executeRenderJob( throw err; const isMemoryExhaustion = !isVerifyError && isMemoryExhaustionError(err); deSelfVerifyFallback = isVerifyError; + // `kind` is a structural field on the error (DrawElementVerificationDetails), + // never derived from message text — a reworded message, a translated + // string, or a cross-module/serialized error must never be able to + // flip "blank" into "psnr" or vice versa (review finding). + const verifyDetails = isVerifyError ? getDrawElementVerificationDetails(err) : undefined; deFallbackReason = isVerifyError - ? /blank/i.test(err instanceof Error ? err.message : "") - ? "blank" - : "psnr" + ? (verifyDetails?.kind ?? "psnr") : isMemoryExhaustion ? "oom" : "capture_error"; if (isVerifyError) { - const verifyDetails = getDrawElementVerificationDetails(err); deFallbackFailedDb = roundDb(verifyDetails?.failedDb); deFallbackFrameIndex = verifyDetails?.frameIndex; deFallbackThresholdDb = roundDb(verifyDetails?.verifyThresholdDb);