From ca8742bdd815d166e6c0397893d32c267fee7ef9 Mon Sep 17 00:00:00 2001 From: Jeff Dafoe Date: Mon, 15 Jun 2026 09:21:11 -0400 Subject: [PATCH 1/2] ZBBS-WORK-406: coerce whole-structure tool args that llama over-stringifies (string->array/object) coerce.js (HOME-342) un-stringifies over-stringified SCALAR tool args from OpenRouter-fronted models, but its array/object branches only fired when the structure itself already arrived as a real array/object. llama-3.3-70b (every stateful Salem NPC) also over-stringifies the WHOLE structure: speak's `mentions` arrives as "[]" / "[{\"item\":...}]" (a JSON string), the array branch's Array.isArray(value) guard skipped it, and the string reached the Salem engine's strict []SpeakMention decode and failed as malformed_args. Net effect: every speak carrying a mentions field fails, so the llama NPCs can't reliably talk -- observed live on Hannah Boggs 2026-06-15 (a whole turn of ~20 failed speaks). Extend coerceToSchema's array/object branches: if value is a string, JSON.parse it and adopt only when it yields the matching structure, else leave it for downstream to reject (conservative, matches the module's existing design). The nested recursion then coerces inner scalars as before. Stays OpenRouter-scoped (only that provider calls coerceToolArgs). Also covers scene_quote.consumers / pay_with_item.consumers if llama stringifies one of those wholesale. Verified with a standalone script against the real coerce.js: Hannah's exact mentions:"[]" case, nested stringified price, real-array no-regression, non-JSON / non-array left untouched, scalar coercion intact, stringified object recovery, and no __proto__ pollution. Co-Authored-By: Claude Fable 5 --- node/api/src/services/providers/coerce.js | 66 ++++++++++++++++++----- 1 file changed, 52 insertions(+), 14 deletions(-) diff --git a/node/api/src/services/providers/coerce.js b/node/api/src/services/providers/coerce.js index 60fb8129..9613e73f 100644 --- a/node/api/src/services/providers/coerce.js +++ b/node/api/src/services/providers/coerce.js @@ -31,10 +31,14 @@ function asNumber(v) { // // Coercion is conservative: a value is converted only when it cleanly matches // the declared type. Anything that doesn't (a non-numeric string for an integer -// field, garbage for a boolean) is returned untouched so downstream validation -// still rejects it with a precise error rather than this layer masking a real -// problem. Only the observed failure direction (model stringifies numbers/bools) -// is handled; string fields are left alone. +// field, garbage for a boolean, a string that isn't valid JSON for an +// array/object field) is returned untouched so downstream validation still +// rejects it with a precise error rather than this layer masking a real +// problem. The handled failure direction is over-stringification: the model +// emits a scalar (number/bool) — or, for an array/object field, the entire +// structure — as a JSON string despite the declared type. A field whose schema +// type is `string` is never touched (the model over-stringifies; it never +// under-stringifies). // // Recurses through declared object properties and array items so nested // argument shapes coerce too. `value` is a freshly parsed throwaway object, so @@ -54,6 +58,19 @@ function singleNonNullType(type) { return null; } +// tryParseJSON returns the parsed value, or undefined when `s` isn't valid +// JSON. Used to recover a whole array/object argument the model +// over-stringified (emitted the entire structure as a JSON string rather than +// inline JSON). A non-throwing parse so callers can fall back to leaving the +// original string in place for strict downstream validation. +function tryParseJSON(s) { + try { + return JSON.parse(s); + } catch (e) { + return undefined; + } +} + function coerceToSchema(value, schema) { if (!schema || typeof schema !== 'object') return value; const type = singleNonNullType(schema.type); @@ -88,20 +105,41 @@ function coerceToSchema(value, schema) { return value; } - if (type === 'object' && value && typeof value === 'object' && !Array.isArray(value) - && schema.properties && typeof schema.properties === 'object' && !Array.isArray(schema.properties)) { - for (const key of Object.keys(value)) { - // Own-property check only — never read through to Object.prototype for - // a model-supplied key like "__proto__". - if (!Object.prototype.hasOwnProperty.call(schema.properties, key)) continue; - value[key] = coerceToSchema(value[key], schema.properties[key]); + if (type === 'object') { + // Llama (via OpenRouter) sometimes over-stringifies a whole object + // argument: the entire {...} arrives as a JSON string instead of an + // object. Recover it before coercing; a string that doesn't parse to a + // plain object is left untouched for downstream validation to reject. + if (typeof value === 'string') { + const parsed = tryParseJSON(value); + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) return value; + value = parsed; + } + if (value && typeof value === 'object' && !Array.isArray(value) + && schema.properties && typeof schema.properties === 'object' && !Array.isArray(schema.properties)) { + for (const key of Object.keys(value)) { + // Own-property check only — never read through to Object.prototype for + // a model-supplied key like "__proto__". + if (!Object.prototype.hasOwnProperty.call(schema.properties, key)) continue; + value[key] = coerceToSchema(value[key], schema.properties[key]); + } } return value; } - if (type === 'array' && Array.isArray(value) && schema.items) { - for (let i = 0; i < value.length; i++) { - value[i] = coerceToSchema(value[i], schema.items); + if (type === 'array') { + // Same over-stringification failure mode: the whole array arrives as a + // JSON string. Recover it before coercing items; a string that doesn't + // parse to an array is left untouched for downstream validation to reject. + if (typeof value === 'string') { + const parsed = tryParseJSON(value); + if (!Array.isArray(parsed)) return value; + value = parsed; + } + if (Array.isArray(value) && schema.items) { + for (let i = 0; i < value.length; i++) { + value[i] = coerceToSchema(value[i], schema.items); + } } return value; } From 24b1b408546af412af26d34602931323f9cb4bd9 Mon Sep 17 00:00:00 2001 From: Jeff Dafoe Date: Mon, 15 Jun 2026 09:25:24 -0400 Subject: [PATCH 2/2] ZBBS-WORK-406: code_review fixes -- explicit null check + skip prototype keys - object branch: `parsed === null` instead of `!parsed` (clearer; behavior- equivalent, since null is the only falsy value with typeof === 'object'). - harden the object property loop: unconditionally skip __proto__/constructor/ prototype keys before the hasOwnProperty guard, so a bad/hostile schema can't drive an assign-through. The existing guard already blocked a model-supplied __proto__; this also covers a schema-declared one. code_review's nullable-union note (["array","null"]) needs no change: the union already resolves to the non-null type via singleNonNullType at the top of coerceToSchema. Added a regression case proving it. Co-Authored-By: Claude Fable 5 --- node/api/src/services/providers/coerce.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/node/api/src/services/providers/coerce.js b/node/api/src/services/providers/coerce.js index 9613e73f..468d929a 100644 --- a/node/api/src/services/providers/coerce.js +++ b/node/api/src/services/providers/coerce.js @@ -112,12 +112,16 @@ function coerceToSchema(value, schema) { // plain object is left untouched for downstream validation to reject. if (typeof value === 'string') { const parsed = tryParseJSON(value); - if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) return value; + if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) return value; value = parsed; } if (value && typeof value === 'object' && !Array.isArray(value) && schema.properties && typeof schema.properties === 'object' && !Array.isArray(schema.properties)) { for (const key of Object.keys(value)) { + // Never assign through a prototype-poisoning key, even if a schema + // somehow declared one: value["__proto__"] = ... can invoke the + // legacy prototype setter rather than set an own data property. + if (key === '__proto__' || key === 'constructor' || key === 'prototype') continue; // Own-property check only — never read through to Object.prototype for // a model-supplied key like "__proto__". if (!Object.prototype.hasOwnProperty.call(schema.properties, key)) continue;