From 9df622c7f814d873657bfe70a2e3b91d6ffc52c4 Mon Sep 17 00:00:00 2001 From: dtzp555 Date: Tue, 21 Jul 2026 20:04:57 +1000 Subject: [PATCH] fix(structured): narrow validateJsonSchemaSafe to RangeError-only + drop unused import (review fold-in) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer of #184: the catch-all would silently mask a future genuine bug (e.g. a TypeError from a malformed schema) as a validation miss. Narrowed to `if (e instanceof RangeError) return [...]; throw e;` so only the #181 deep-nesting overflow becomes a refusal; any other throw surfaces at error level as before. +1 test proving a non-RangeError (required:42 → TypeError) re-throws. Dropped the now- unused raw `validateJsonSchema` import from server.mjs. Merged current main (incl. #183) so CI runs the true post-merge tree. Suite 433/0. Co-Authored-By: Claude --- lib/structured-output.mjs | 7 ++++++- server.mjs | 2 +- test-features.mjs | 6 ++++++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/structured-output.mjs b/lib/structured-output.mjs index a444bd9..238e7fb 100644 --- a/lib/structured-output.mjs +++ b/lib/structured-output.mjs @@ -221,7 +221,12 @@ export function validateJsonSchemaSafe(value, schema, path = "$", strict = false try { return validateJsonSchema(value, schema, path, strict, root); } catch (e) { - return [`${path}: schema validation aborted (${e instanceof RangeError ? "value nesting too deep" : "internal error"})`]; + // Catch ONLY the deep-nesting stack overflow (the #181 vector) and turn it into a validation + // miss → retry → refusal, never a 500. Any OTHER throw is a genuine bug: re-throw it so it + // surfaces at error level instead of being silently masked as "did not validate" (reviewer + // finding — a catch-all would hide a future TypeError behind a warn-level structured_retry). + if (e instanceof RangeError) return [`${path}: schema validation aborted (value nesting too deep)`]; + throw e; } } diff --git a/server.mjs b/server.mjs index 71c8995..7da098d 100644 --- a/server.mjs +++ b/server.mjs @@ -42,7 +42,7 @@ import { dirname, join } from "node:path"; import { homedir } from "node:os"; import { validateKey, recordUsage, getUsageByKey, getUsageTimeline, getRecentUsage, createKey, listKeys, revokeKey, closeDb, checkQuota, updateKeyQuota, getKeyQuota, findKey, cacheHash, getCachedResponse, setCachedResponse, clearCache, getCacheStats, hasCacheControl, singleflight, getInflightStats } from "./keys.mjs"; import { DEFAULT_PORT } from "./lib/constants.mjs"; -import { StructuredOutputError, detectStructuredOutput, validateJsonSchema, validateJsonSchemaSafe, extractJsonPayload, structuredSystemInstruction, resolveMaxAttempts } from "./lib/structured-output.mjs"; +import { StructuredOutputError, detectStructuredOutput, validateJsonSchemaSafe, extractJsonPayload, structuredSystemInstruction, resolveMaxAttempts } from "./lib/structured-output.mjs"; import { isLoopbackBind } from "./lib/net.mjs"; import { runTuiTurn, reapStaleTuiSessions, resolveTuiHome, bootTuiPane, tuiPaneHealthy, poolPaneName, POOL_BOOT_MS } from "./lib/tui/session.mjs"; import { detectTuiUpstreamError } from "./lib/tui/transcript.mjs"; diff --git a/test-features.mjs b/test-features.mjs index f847512..3042157 100644 --- a/test-features.mjs +++ b/test-features.mjs @@ -4405,6 +4405,12 @@ test("validateJsonSchemaSafe: well-formed value passes through unchanged (byte-i assert.deepEqual(validateJsonSchemaSafe({ name: "a" }, schema), validateJsonSchema({ name: "a" }, schema)); // error case matches too }); +test("validateJsonSchemaSafe: re-throws a non-RangeError so genuine bugs aren't masked as a validation miss", () => { + // A schema whose `required` is a non-iterable makes the inner validator throw a TypeError — that's + // a real bug, not a deep-value overflow, and must surface (not be swallowed as "did not validate"). + assert.throws(() => validateJsonSchemaSafe({ x: 1 }, { type: "object", required: 42 }), (e) => !(e instanceof RangeError)); +}); + test("validateJsonSchema: valid object passes", () => { assert.deepEqual(validateJsonSchema({ name: "a", age: 3 }, { type: "object", required: ["name", "age"], properties: { name: { type: "string" }, age: { type: "integer" } } }), []); });