From ae243ff66ed4d236fc2e43852132a0ef31fba654 Mon Sep 17 00:00:00 2001
From: dtzp555 <dtzp555@gmail.com>
Date: Mon, 27 Jul 2026 09:16:58 +1000
Subject: [PATCH] fix(schema): warn that only a subset of draft 2020-12 is
 enforced; cover the gaps it cannot
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Both review findings taken.

MED — the schema declared `$schema: draft/2020-12`, which promises full draft
semantics, while the validator that actually runs it enforces only a subset.
Someone adding `"minimum": 1` to contextWindow would get SILENT no-op with a
green suite — a trap made worse by the file looking authoritative. The
description now enumerates exactly what is enforced (type, required, const,
enum, additionalProperties in both forms, items, minItems/maxItems,
anyOf/allOf/oneOf, $ref, nullable) and names what is ignored (minimum,
maxLength, pattern, uniqueItems, propertyNames, not), with the instruction to
add such a constraint as a test instead.

LOW — three corruptions neither the schema nor any sibling test caught:
duplicate models[].id, empty-string id/displayName/openclawName, and
non-positive contextWindow/maxTokens. They are structurally NOT expressible
with the supported keyword set (no uniqueItems, no minLength, no minimum), so
they are asserted directly rather than by pretending the schema covers them.

Mutation-proven, all three previously slipped through:
  duplicate id       -> CAUGHT
  empty displayName  -> CAUGHT
  zero contextWindow -> CAUGHT

Tests: 461 passed, 0 failed (460 + 1).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017gbqUZ8HfBZpjjbzQ85oH8
---
 models.schema.json |  2 +-
 test-features.mjs  | 15 +++++++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/models.schema.json b/models.schema.json
index 3db19e6..4b4e451 100644
--- a/models.schema.json
+++ b/models.schema.json
@@ -2,7 +2,7 @@
   "$schema": "https://json-schema.org/draft/2020-12/schema",
   "$id": "https://github.com/dtzp555-max/ocp/blob/main/models.schema.json",
   "title": "OCP models.json",
-  "description": "Schema for models.json, the single source of truth for model metadata (ADR 0003). Enforced in CI by test-features.mjs, which validates models.json against this file using the repo's own validateJsonSchema (lib/structured-output.mjs) — no new dependency. NOTE: referential integrity (every aliases/legacyAliases target must exist as a models[].id) is NOT expressible here and is covered by separate tests.",
+  "description": "Schema for models.json, the single source of truth for model metadata (ADR 0003). Enforced in CI by test-features.mjs, which validates models.json against this file using the repo's own validateJsonSchema (lib/structured-output.mjs) — no new dependency. ⚠ ONLY A SUBSET OF DRAFT 2020-12 IS ENFORCED: type, required, const, enum, additionalProperties (boolean and schema forms), items, minItems, maxItems, anyOf/allOf/oneOf, $ref, nullable. Anything else — minimum, maxLength, pattern, uniqueItems, propertyNames, not — is SILENTLY IGNORED by that validator, so adding one here buys nothing and misleads the next reader. Add such a constraint as a test in test-features.mjs instead. NOTE: referential integrity (every aliases/legacyAliases target must exist as a models[].id) is likewise not expressible here and is covered by separate tests.",
   "type": "object",
   "required": ["version", "models", "aliases", "legacyAliases"],
   "additionalProperties": false,
diff --git a/test-features.mjs b/test-features.mjs
index e4b0797..ed5f715 100644
--- a/test-features.mjs
+++ b/test-features.mjs
@@ -4330,6 +4330,21 @@ test("models.json validates against models.schema.json (strict)", () => {
   assert.deepEqual(errors, [], `models.json violates its own schema:\n  ${errors.join("\n  ")}`);
 });
 
+// Three corruptions the SCHEMA structurally cannot catch, asserted directly instead of pretending
+// the schema covers them: the validator has no uniqueItems, no minLength, and no minimum. Adding
+// those keywords to the schema would be silently ignored (see its description), so they live here.
+test("models.json: ids are unique, non-empty, and windows are positive (not schema-expressible)", () => {
+  const ids = _spotModels.models.map(m => m.id);
+  assert.equal(new Set(ids).size, ids.length, `duplicate models[].id: ${ids.filter((v, i) => ids.indexOf(v) !== i)}`);
+  for (const m of _spotModels.models) {
+    for (const f of ["id", "displayName", "openclawName"]) {
+      assert.ok(typeof m[f] === "string" && m[f].trim().length > 0, `${m.id}: ${f} must be a non-empty string`);
+    }
+    assert.ok(m.contextWindow > 0, `${m.id}: contextWindow must be positive`);
+    assert.ok(m.maxTokens > 0, `${m.id}: maxTokens must be positive`);
+  }
+});
+
 // Guards the guard: if the schema were vacuous (e.g. `{}` or a typo'd `properties`), the test
 // above would pass on anything. Each corruption below must be caught.
 test("models.schema.json actually rejects malformed entries (guard is not vacuous)", () => {