2 Commits

Include renames

Author	SHA1	Message	Date
4 people	45c5717aea	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>	2026-07-21 20:05:50 +10:00
5 people	788cbbcd99	feat(server): honor OpenAI response_format for structured-output clients (#153) ... * feat(server): honor OpenAI response_format for structured-output clients `/v1/chat/completions` advertises OpenAI compatibility but ignored `response_format`, so clients requiring machine-parseable JSON (Home Assistant AI Tasks, Honcho, OpenAI-SDK scripts) received free-form assistant prose — markdown tables, ```json fences, trailing commentary — that fails JSON.parse. This honors the OpenAI `response_format` contract on the `-p` path: - New `lib/structured-output.mjs` (pure, unit-tested): `detectStructuredOutput` (json_schema / json_object), `structuredSystemInstruction` (strict JSON-only steering, escalated on retry), `extractJsonPayload` (string-aware balanced slice that unwraps fences/prose), and a minimal JSON-Schema `validateJsonSchema` (types, required, enum, const, additionalProperties, nullability, items, min/maxItems). - `server.mjs`: `runStructuredCompletion` retries up to `OCP_STRUCTURED_MAX_ATTEMPTS` (default 3), returns the canonical JSON string as `message.content`, and yields HTTP 422 (`invalid_response_error`) if no valid JSON can be produced. Structured requests take their own path (bypass the cache, which does not key on response_format). Non-structured requests are byte-for-byte unchanged, streaming included. - Nullability precedence: a `null` value is accepted whenever the schema permits null (`type:["x","null"]` / `nullable:true`), even if a bare `enum` omits null — matches OpenAI behaviour and fixes real Home Assistant schemas (`type:["string","null"], enum:["Loxone"]`) that otherwise 422 on null. - README: Structured Outputs section + `OCP_STRUCTURED_MAX_ATTEMPTS` env row. - 18 new unit tests (281 passed, 0 failed). Endpoint class: B.1 (OpenAI-compatibility surface, `/v1/chat/completions`). Specification: OpenAI chat/completions `response_format` (https://platform.openai.com/docs/api-reference/chat/create#chat-create-response_format). Authorizing ADR: ADR 0006 — OpenAI shim scope. cli.js does NOT perform this operation (it speaks Anthropic's protocol, not OpenAI's); scope is justified under ADR 0006 Class B.1 (OpenAI spec as protocol authority). Revives closed PR #99. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(server): structured-output caching + json_mode alias Follow-up on the response_format path, closing the two gaps vs closed PR #99: - **Validated caching (improves on #99).** Structured responses now use the OCP cache when CLAUDE_CACHE_TTL>0, on a structured-keyed hash: cacheHash gains an `structured` marker folding the detected response_format/schema into the key, so a JSON reply never collides with the conversational answer to the same prompt and different schemas never share a slot. Only a *validated* result is written back — a 422 is never cached. (#99 cached the fence-stripped but *unvalidated* output; this caches only schema-valid JSON.) The marker is absent for normal requests, so existing cache hashes are byte-identical. - **json_mode alias.** Honor the non-standard top-level `json_mode: true` flag as a json_object alias, matching #99's activation set. Disclosed as non-spec. - README: json_mode shape + caching note. +2 unit tests (283 passed, 0 failed). Endpoint class: B.1 (/v1/chat/completions), ADR 0006. json_mode is a non-OpenAI convenience alias (disclosed); everything else stays within OpenAI's response_format spec. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(server): address PR #153 review — $ref/strict, extraction safety, refusal, singleflight Remediates the maintainer's merge-blocking findings on the structured-output PR, and rebases onto current main (the one-hunk test-features.mjs conflict — both sides appended tests — resolved by keeping both blocks). Class B.1 (OpenAI-compat): spec authority is OpenAI chat/completions `response_format` (https://platform.openai.com/docs/api-reference/chat/create#chat-create-response_format), authorized by ADR 0006. No cli.js analogue (claude -p has no native response_format); scope is the B.1 shim, not a Class A forward. Finding 1 (correctness gate) — strict:true + $ref/$defs rejected valid objects 100% of the time. `noExtra = addl === false || (strict && addl === undefined)` treated a nested {$ref:"#/$defs/step"} as an empty-properties object and, under strict, rejected every real key as "additional property not allowed" — exactly the shape the OpenAI SDK emits (zodResponseFormat / client.beta.chat.completions.parse) and OpenAI's own docs example. Fix: validateJsonSchema now resolves same-document $ref against the root $defs/definitions, handles allOf/anyOf/oneOf composition, and only infers additionalProperties:false from strict when the object actually declares its own non-empty properties and is not a composite. Explicit additionalProperties:false is always honoured, so validation is not weakened (tests prove an extra key and a missing required key still fail under strict). Finding 2 (correctness gate) — the extractor served JSON the model did not mean. json_object mode had no validation at all: a refusal like `I can't. The schema is {"type":"object"}` returned the embedded object as the answer. Now json_object requires the WHOLE reply to parse as a single JSON value, and schema mode rejects a reply carrying more than one top-level JSON value (Schema:{}/Answer:{}, Option A/Option B) rather than silently picking the first. The schema-validated value is still returned; nothing unvalidated is served. Finding 3 — replaced the invented `invalid_response_error` 422 with OpenAI's assistant `refusal` field (200, content:null, refusal:<reason>, finish_reason:"stop"), streaming and non-streaming, so SDK clients take their refusal branch instead of throwing an opaque UnprocessableEntityError. Finding 5 — runStructuredCompletion no longer bypasses stampede protection. Identical concurrent one-off structured requests now share one singleflight (independent of cache enablement), so N callers no longer cost N × up-to-3 spawns. Cache read/write still gated on CLAUDE_CACHE_TTL; refusals are never cached. Docs: README structured-output § updated (refusal field, $ref/composition support, whole-reply json_object rule, ambiguous-multi-value rejection) plus a Caching & cost paragraph stating the post-2026-06-15 model, the up-to-N-spawn worst case, the singleflight + validated-cache guards, and the OCP_STRUCTURED_MAX_ATTEMPTS=1 / per-key quota levers. Tests: +11 (all pure-module) — OpenAI's doc $ref/$defs schema under strict:true accepts a conforming reply and still rejects extra/missing keys; anyOf/allOf; unresolvable $ref skipped; json_object refusal-embedded-json rejected; >1-top-level-value rejected. 360 passed, 0 failed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(server): address PR #153 review round 2 — cyclic-$ref guard + NaN attempts guard Remediates the two remaining merge-blocking findings from the round-2 review. Class B.1 (OpenAI-compat): spec authority is OpenAI chat/completions `response_format` (https://platform.openai.com/docs/api-reference/chat/create#chat-create-response_format), authorized by ADR 0006. No cli.js analogue (claude -p has no native response_format, and the retry cap OCP_STRUCTURED_MAX_ATTEMPTS is OCP's own coercion loop, not a cli.js operation); scope stays the B.1 shim, not a Class A forward. BLOCKER — cyclic $ref stack-overflowed the validator. resolveRef + the $ref branch of validateJsonSchema had no cycle detection: a pure ref→ref cycle ({$defs:{a:{$ref:b},b:{$ref:a}},$ref:a}) recursed independent of the data and threw RangeError for ANY reply value (even `5`), caught upstream as a 500 but only after 1–3 metered spawns — a request-controlled cost-amplification / grief vector on an authed path. Fix: validateJsonSchema now threads a `refChain` of the $ref pointers resolved on the current path WITHOUT consuming data (a $ref hop, or an allOf/anyOf/oneOf branch — all re-validate the same value); a pointer reappearing on that chain fails closed with a `cyclic $ref detected` error. Data-consuming recursion (properties/items/ additionalProperties) deliberately resets the chain, because a JSON value is a finite tree so those always terminate — a legitimately recursive schema (Node→child:Node) must NOT be flagged. A REF_DEPTH_CAP backstops any threading mistake. MUST-FIX — OCP_STRUCTURED_MAX_ATTEMPTS NaN guard was broken. `Math.max(1, parseInt(env ||"3",10))` === `Math.max(1, NaN)` === NaN for a non-integer value, so the retry loop `attempt < NaN` never ran → 0 spawns, every structured request silently refused (fails closed on cost but bricks the feature and ignores the intended floor). Fix: extracted a pure fail-closed resolveMaxAttempts() into lib/structured-output.mjs — rejects NaN/non-finite/<1, keeps the documented default of 3, and warns at startup. server.mjs now derives STRUCTURED_MAX_ATTEMPTS through it. Tests: +8 (all pure-module) — a→b→a and self (a→a) cyclic $ref fail closed without overflowing the stack; a cycle routed through anyOf; a legitimate recursive Node schema is NOT flagged; resolveMaxAttempts honors valid integers, defaults on unset/empty/null, and fails closed (not NaN, not 0) on abc/0/-1/NaN/Infinity/blank with a startup warn. 368 passed, 0 failed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: vvlasy-openclaw <vvlasy@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: dtzp555 <dtzp555@gmail.com>	2026-07-20 07:55:40 +10:00