fix(structured): narrow validateJsonSchemaSafe to RangeError-only + drop unused import (review fold-in)

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 <claude-opus-4-8> <noreply@anthropic.com>
This commit is contained in:
2026-07-21 20:04:57 +10:00
co-authored by Claude <claude-opus-4-8> <noreply@anthropic.com>
parent 2ad8a5245d
commit 9df622c7f8
3 changed files with 13 additions and 2 deletions
+6 -1
View File
@@ -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;
}
}
+1 -1
View File
@@ -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";
+6
View File
@@ -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" } } }), []);
});