fix(structured): crash-safe validation façade — deep model reply → refusal not 500 (closes #181) (#184)

* fix(structured): crash-safe validation façade — deep model reply → refusal, not a 500 (closes #181)

#153's cyclic-$ref guard caps the REF-chain depth but not the DATA depth:
validateJsonSchema recurses on the value's nesting (properties/items/additionalProps),
so a model reply nested ~2000+ levels overflowed the stack with a RangeError, which
handleChatCompletions caught as a generic HTTP 500 instead of the spec-correct refusal.
(Found in the #153 final review, filed as #181; ≤1 spawn, no crash, no client-only
trigger — the value always comes from the model reply.)

New exported validateJsonSchemaSafe() wraps the validator: ANY throw (the deep-data
RangeError, or any future recursion hazard) becomes a single validation error, so the
structured-output retry loop treats a pathological reply as "did not validate" →
refusal. A well-formed reply is byte-identical (passes the inner errors through).
runStructuredCompletion calls the safe façade.

Chose the wrapper over threading a data-depth counter through six recursive call
sites: it protects every internal path at once (impossible to miss one) and stays
deterministically testable — a 6000-deep fixture reliably overflows on any platform.

Tests: +2, mutation-proven (revert the wrapper to a bare call → the deep test throws
RED). Suite 431/0. Rule 2: OCP-internal validation, no wire change, no cli.js citation.

Closes #181

Co-Authored-By: Claude <claude-opus-4-8> <noreply@anthropic.com>

* 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>

---------

Co-authored-by: dtzp555 <dtzp555@gmail.com>
Co-authored-by: Claude <claude-opus-4-8> <noreply@anthropic.com>

lib/structured-output.mjs

@@ -209,6 +209,27 @@ export function validateJsonSchema(value, schema, path = "$", strict = false, ro
   return errors;
 }
 
+// Crash-safe façade over validateJsonSchema (issue #181). The validator recurses on the DATA's
+// nesting depth (properties/items/additionalProperties), which the REF_DEPTH_CAP does NOT bound —
+// only the ref-chain is. A model reply nested ~2000 levels deep therefore overflowed the stack with
+// a RangeError, which the request handler caught as a generic HTTP 500 instead of the spec-correct
+// `refusal`. This wrapper converts ANY throw (the deep-data RangeError, or any future recursion
+// hazard) into a single validation error, so the structured-output retry loop treats a pathological
+// reply as "did not validate" → refusal — never a 500, never a crash. A well-formed reply is
+// unaffected: the inner validator returns and this just passes its errors through.
+export function validateJsonSchemaSafe(value, schema, path = "$", strict = false, root = schema) {
+  try {
+    return validateJsonSchema(value, schema, path, strict, root);
+  } catch (e) {
+    // 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;
+  }
+}
+
 function tryJsonParse(s) {
   try { return { ok: true, value: JSON.parse(s) }; } catch { return { ok: false }; }
 }

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, 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";
@@ -2732,7 +2732,10 @@ async function runStructuredCompletion(upstreamCall, model, messages, conversati
       continue;
     }
     if (structured.mode === "schema" && structured.schema) {
-      const errs = validateJsonSchema(extracted.value, structured.schema, "$", structured.strict);
+      // validateJsonSchemaSafe (#181): a pathologically deep model reply overflows the value-depth
+      // recursion; the safe façade turns that into a validation miss (→ retry → refusal) instead of
+      // a caught RangeError surfacing as a generic 500.
+      const errs = validateJsonSchemaSafe(extracted.value, structured.schema, "$", structured.strict);
       if (errs.length) {
         lastErr = "schema validation failed: " + errs.slice(0, 5).join("; ");
         logEvent("warn", "structured_retry", { attempt, reason: "schema", errors: errs.slice(0, 5) });

test-features.mjs

@@ -4356,7 +4356,7 @@ test("stream: /health block is additive and exposes the divergence counter", ()
 });
 
 // ── OpenAI Structured Outputs (response_format) — lib/structured-output.mjs ──
-import { detectStructuredOutput, validateJsonSchema, extractJsonPayload, structuredSystemInstruction, StructuredOutputError, resolveMaxAttempts } from "./lib/structured-output.mjs";
+import { detectStructuredOutput, validateJsonSchema, validateJsonSchemaSafe, extractJsonPayload, structuredSystemInstruction, StructuredOutputError, resolveMaxAttempts } from "./lib/structured-output.mjs";
 
 test("detectStructuredOutput: json_schema shape", () => {
   const d = detectStructuredOutput({ response_format: { type: "json_schema", json_schema: { name: "x", strict: true, schema: { type: "object" } } } });
@@ -4383,6 +4383,34 @@ test("cacheHash: structured marker isolates JSON requests from the conversationa
   assert.equal(plain, cacheHash("m", msgs, { keyId: "k" })); // unchanged for normal requests
 });
 
+// ── validateJsonSchemaSafe (#181): deep value must NOT crash the handler ─────
+// A recursive schema + a model reply nested ~thousands deep overflows the value-
+// depth recursion → RangeError → the handler used to surface a generic 500. The
+// safe façade turns it into a validation miss (→ retry → refusal). Mutation-proof:
+// replace the wrapper body with a bare `validateJsonSchema(...)` call and the deep
+// test throws instead of returning errors.
+test("validateJsonSchemaSafe: pathologically deep value → errors, never throws", () => {
+  const schema = { $defs: { node: { type: "object", properties: { child: { $ref: "#/$defs/node" } } } }, $ref: "#/$defs/node" };
+  let deep = {};
+  let cur = deep;
+  for (let i = 0; i < 6000; i++) { cur.child = {}; cur = cur.child; } // way past any stack limit
+  let out;
+  assert.doesNotThrow(() => { out = validateJsonSchemaSafe(deep, schema, "$", true); }, "must not throw a RangeError out to the handler");
+  assert.ok(Array.isArray(out) && out.length > 0, "returns a non-empty validation error, so the retry loop yields a refusal not a 500");
+});
+
+test("validateJsonSchemaSafe: well-formed value passes through unchanged (byte-identical to the raw validator)", () => {
+  const schema = { type: "object", required: ["name", "age"], properties: { name: { type: "string" }, age: { type: "integer" } } };
+  assert.deepEqual(validateJsonSchemaSafe({ name: "a", age: 3 }, schema), validateJsonSchema({ name: "a", age: 3 }, schema));
+  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" } } }), []);
 });