From 30a62f27d0a98532cf853a04614ae88f741697d1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Apr 2026 22:37:28 +0000 Subject: [PATCH 1/3] add response-side storyboard schema conformance lint (#2823) Mirrors the existing request-side lint (lint-storyboard-sample-request-schema.cjs) for the response side: validates inline sample_response fixtures against response_schema_ref using AJV 8 with a shrink-only ratchet allowlist. Passes vacuously on day 1 (corpus has 0 steps with both response_schema_ref and sample_response) and gains CI value as storyboard authors add fixtures. https://claude.ai/code/session_019dMkCmDw5ro7hzMQ6iTkKQ --- .../add-storyboard-response-schema-lint.md | 4 + package.json | 3 +- scripts/lint-storyboard-response-schema.cjs | 351 ++++++++++++++++++ .../lint-storyboard-response-schema.test.cjs | 150 ++++++++ .../storyboard-response-schema-allowlist.json | 4 + 5 files changed, 511 insertions(+), 1 deletion(-) create mode 100644 .changeset/add-storyboard-response-schema-lint.md create mode 100644 scripts/lint-storyboard-response-schema.cjs create mode 100644 tests/lint-storyboard-response-schema.test.cjs create mode 100644 tests/storyboard-response-schema-allowlist.json diff --git a/.changeset/add-storyboard-response-schema-lint.md b/.changeset/add-storyboard-response-schema-lint.md new file mode 100644 index 0000000000..e6802bcfb6 --- /dev/null +++ b/.changeset/add-storyboard-response-schema-lint.md @@ -0,0 +1,4 @@ +--- +--- + +Add response-side storyboard schema lint (`scripts/lint-storyboard-response-schema.cjs`) that validates `sample_response` fixtures against `response_schema_ref` using AJV, mirroring the existing request-side lint. Includes shrink-only ratchet allowlist, Node test wrapper, and CI wiring. Implements #2823. diff --git a/package.json b/package.json index fcb8ea8152..c4eb9696ff 100644 --- a/package.json +++ b/package.json @@ -40,6 +40,7 @@ "test:storyboard-auth-shape": "node --test tests/lint-storyboard-auth-shape.test.cjs", "test:storyboard-test-kits": "node --test tests/lint-storyboard-test-kits.test.cjs", "test:storyboard-sample-request-schema": "node --test tests/lint-storyboard-sample-request-schema.test.cjs", + "test:storyboard-response-schema": "node --test tests/lint-storyboard-response-schema.test.cjs", "test:storyboard-doc-parity": "node --test tests/lint-universal-storyboard-doc-parity.test.cjs", "test:pagination-invariant": "node --test tests/lint-pagination-invariant.test.cjs", "test:test-dynamic-imports": "node --test tests/lint-test-dynamic-imports.test.cjs", @@ -56,7 +57,7 @@ "test:docker": "docker compose -f docker-compose.test.yml up --build --abort-on-container-exit", "test:docs-nav": "node tests/docs-nav-validation.test.cjs", "test:platform-agnostic": "node tests/check-platform-agnostic.cjs", - "test": "npm run test:docs-nav && npm run test:schemas && npm run test:examples && npm run test:extensions && npm run test:extension-schemas && npm run test:error-handling && npm run test:json-schema && npm run test:composed && npm run test:migrations && npm run test:hmac-vectors && npm run test:hmac-signer-conformance && npm run test:transport-errors && npm run test:targeting-overlay-vectors && npm run test:storyboard-scoping && npm run test:storyboard-branch-sets && npm run test:storyboard-contradictions && npm run test:storyboard-context-entity && npm run test:storyboard-auth-shape && npm run test:storyboard-test-kits && npm run test:storyboard-sample-request-schema && npm run test:storyboard-doc-parity && npm run test:pagination-invariant && npm run test:test-dynamic-imports && npm run test:build-schemas-hoist-enums && npm run test:error-codes && npm run test:substitution-vector-names && npm run test:platform-agnostic && npm run test:unit && npm run test:server-unit && npm run test:openapi && npm run typecheck", + "test": "npm run test:docs-nav && npm run test:schemas && npm run test:examples && npm run test:extensions && npm run test:extension-schemas && npm run test:error-handling && npm run test:json-schema && npm run test:composed && npm run test:migrations && npm run test:hmac-vectors && npm run test:hmac-signer-conformance && npm run test:transport-errors && npm run test:targeting-overlay-vectors && npm run test:storyboard-scoping && npm run test:storyboard-branch-sets && npm run test:storyboard-contradictions && npm run test:storyboard-context-entity && npm run test:storyboard-auth-shape && npm run test:storyboard-test-kits && npm run test:storyboard-sample-request-schema && npm run test:storyboard-response-schema && npm run test:storyboard-doc-parity && npm run test:pagination-invariant && npm run test:test-dynamic-imports && npm run test:build-schemas-hoist-enums && npm run test:error-codes && npm run test:substitution-vector-names && npm run test:platform-agnostic && npm run test:unit && npm run test:server-unit && npm run test:openapi && npm run typecheck", "test:all": "npm run test:schemas && npm run test:examples && npm run test:extensions && npm run test:error-handling && npm run test:snippets && npm run typecheck", "precommit": "bash scripts/with-timeout.sh 60 npm run test:unit && npm run test:test-dynamic-imports && npm run typecheck", "prepare": "husky", diff --git a/scripts/lint-storyboard-response-schema.cjs b/scripts/lint-storyboard-response-schema.cjs new file mode 100644 index 0000000000..0d95f71963 --- /dev/null +++ b/scripts/lint-storyboard-response-schema.cjs @@ -0,0 +1,351 @@ +#!/usr/bin/env node +/** + * Storyboard sample_response schema conformance lint (issue #2823). + * + * For every storyboard step that declares both `response_schema_ref` AND + * `sample_response`, validate the sample_response against the referenced JSON + * schema. This is the response-side twin of lint-storyboard-sample-request-schema.cjs + * (see the "future work" note at line 17 of that script). + * + * Coverage today: inline `sample_response` on the step only. The + * comply_scenario-to-fixture mapping (specialism scenarios/*.yaml) is a + * runtime runner hint, not a static fixture file — response fixtures from + * that path are deferred. The corpus currently has sparse sample_response + * coverage; the lint passes vacuously on day 1 and gains CI value as + * storyboard authors add sample_response fixtures. + * + * What it catches (once coverage exists): + * - Response shape drift: fields added/removed between schema and fixture + * - Wrong types, enum violations, required field omissions + * - Stale fixtures that diverge after a schema change + * + * What it does not catch: + * - Dynamic agent responses (runtime concern, not static lint) + * - Semantic correctness of field values + * - Steps without sample_response (intentionally skipped) + */ + +'use strict'; + +const fs = require('node:fs'); +const path = require('node:path'); +const yaml = require('js-yaml'); +const Ajv = require('ajv'); +const addFormats = require('ajv-formats'); + +const STORYBOARD_DIR = path.resolve(__dirname, '..', 'static', 'compliance', 'source'); +const SCHEMA_DIR = path.resolve(__dirname, '..', 'static', 'schemas', 'source'); +const ALLOWLIST_PATH = path.resolve(__dirname, '..', 'tests', 'storyboard-response-schema-allowlist.json'); + +const schemaCache = new Map(); + +function schemaRefToPath(ref) { + if (!ref) return null; + const trimmed = ref.startsWith('/schemas/') ? ref.slice('/schemas/'.length) : ref; + return path.join(SCHEMA_DIR, trimmed); +} + +function loadSchema(ref) { + const full = schemaRefToPath(ref); + if (!full) return null; + if (schemaCache.has(full)) return schemaCache.get(full); + let doc = null; + try { + doc = JSON.parse(fs.readFileSync(full, 'utf8')); + } catch { + doc = null; + } + schemaCache.set(full, doc); + return doc; +} + +function walkYaml(dir) { + const out = []; + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) out.push(...walkYaml(full)); + else if (entry.isFile() && (entry.name.endsWith('.yaml') || entry.name.endsWith('.yml'))) { + out.push(full); + } + } + return out; +} + +let sharedAjv = null; +const validatorCache = new Map(); + +function getAjv() { + if (sharedAjv) return sharedAjv; + sharedAjv = new Ajv({ + allErrors: true, + strict: false, + discriminator: true, + loadSchema: (uri) => { + const resolved = loadSchema(uri); + if (!resolved) throw new Error(`Cannot resolve $ref: ${uri}`); + return Promise.resolve(resolved); + }, + }); + addFormats(sharedAjv); + return sharedAjv; +} + +async function getValidator(schemaRef) { + if (validatorCache.has(schemaRef)) return validatorCache.get(schemaRef); + const schema = loadSchema(schemaRef); + if (!schema) { + validatorCache.set(schemaRef, null); + return null; + } + try { + const validate = await getAjv().compileAsync(schema); + validatorCache.set(schemaRef, validate); + return validate; + } catch (err) { + validatorCache.set(schemaRef, { compileError: err.message }); + return { compileError: err.message }; + } +} + +async function validateStep({ schemaRef, payload }) { + const schema = loadSchema(schemaRef); + if (!schema) return { ok: true, skipped: 'schema_not_found', schemaRef }; + const validate = await getValidator(schemaRef); + if (!validate) return { ok: true, skipped: 'schema_not_found', schemaRef }; + if (validate.compileError) return { ok: true, skipped: `compile_error: ${validate.compileError}`, schemaRef }; + const valid = validate(payload); + if (valid) return { ok: true }; + return { + ok: false, + errors: validate.errors.map((e) => ({ + path: e.instancePath || '/', + message: e.message, + keyword: e.keyword, + params: e.params, + })), + }; +} + +async function lintFile(file) { + const violations = []; + let doc; + try { + doc = yaml.load(fs.readFileSync(file, 'utf8')); + } catch (err) { + return { violations, parseError: `yaml_parse: ${err.message}` }; + } + const phases = Array.isArray(doc?.phases) ? doc.phases : []; + for (const phase of phases) { + if (!phase || typeof phase !== 'object') continue; + const phaseId = phase.id; + if (!phaseId) continue; + const steps = Array.isArray(phase.steps) ? phase.steps : []; + for (const step of steps) { + if (!step || typeof step !== 'object') continue; + const stepId = step.id; + if (!stepId) continue; + const schemaRef = step.response_schema_ref; + const sampleResponse = step.sample_response; + if (!schemaRef || !sampleResponse) continue; + if (typeof sampleResponse !== 'object' && typeof sampleResponse !== 'string') continue; + const result = await validateStep({ schemaRef, payload: sampleResponse }); + if (!result.ok) { + violations.push({ file, phaseId, stepId, schemaRef, errors: result.errors }); + } + } + } + return { violations }; +} + +async function lintAll() { + const files = walkYaml(STORYBOARD_DIR); + const violations = []; + const parseErrors = []; + for (const file of files) { + const r = await lintFile(file); + if (r.parseError) parseErrors.push({ file, error: r.parseError }); + violations.push(...r.violations); + } + return { violations, parseErrors }; +} + +function formatViolation(v) { + const rel = path.relative(STORYBOARD_DIR, v.file); + const head = ` ${rel} :: ${v.phaseId}/${v.stepId} (schema: ${v.schemaRef})`; + if (v.error) return `${head}\n ERROR: ${v.error}`; + const errs = (v.errors || []) + .slice(0, 5) + .map((e) => ` ${e.path || '/'} [${e.keyword}] ${e.message}${e.params ? ' ' + JSON.stringify(e.params) : ''}`) + .join('\n'); + const more = v.errors && v.errors.length > 5 ? `\n … and ${v.errors.length - 5} more` : ''; + return `${head}\n${errs}${more}`; +} + +function serializeDetail(v) { + if (v === null || typeof v !== 'object') return String(v); + try { + return JSON.stringify(v); + } catch { + return ''; + } +} + +function fingerprintError(err) { + const path_ = err.path || '/'; + const kw = err.keyword; + let detail = ''; + if (err.params && typeof err.params === 'object') { + if (err.params.missingProperty) detail = `:${err.params.missingProperty}`; + else if (err.params.additionalProperty) detail = `:${err.params.additionalProperty}`; + else if (typeof err.params.allowedValue !== 'undefined') detail = `:${serializeDetail(err.params.allowedValue)}`; + else if (typeof err.params.type !== 'undefined') detail = `:${err.params.type}`; + else if (typeof err.params.format !== 'undefined') detail = `:${err.params.format}`; + else if (typeof err.params.limit !== 'undefined') detail = `:${err.params.limit}`; + } + return `${kw}@${path_}${detail}`; +} + +function entryKey(file, phaseId, stepId) { + const rel = path.relative(STORYBOARD_DIR, file); + return `${rel}#${phaseId}/${stepId}`; +} + +function loadAllowlist() { + if (!fs.existsSync(ALLOWLIST_PATH)) return { entries: {} }; + try { + return JSON.parse(fs.readFileSync(ALLOWLIST_PATH, 'utf8')); + } catch (err) { + throw new Error(`Failed to parse allowlist at ${ALLOWLIST_PATH}: ${err.message}`); + } +} + +function reconcileAgainstAllowlist(violations, allowlist) { + const entries = allowlist.entries || {}; + const seen = new Set(); + const newDrift = []; + const grandfathered = []; + for (const v of violations) { + const key = entryKey(v.file, v.phaseId, v.stepId); + const entry = entries[key]; + if (!entry) { + newDrift.push(v); + continue; + } + seen.add(key); + const allowed = new Set(entry.errors || []); + const current = (v.errors || []).map(fingerprintError); + const unexpected = current.filter((fp) => !allowed.has(fp)); + if (unexpected.length > 0) { + newDrift.push({ + ...v, + errors: (v.errors || []).filter((e) => !allowed.has(fingerprintError(e))), + }); + } else { + grandfathered.push(v); + } + } + const stale = []; + for (const [key, entry] of Object.entries(entries)) { + if (seen.has(key)) continue; + stale.push({ key, entry }); + } + return { newDrift, stale, grandfathered }; +} + +function violationsToAllowlist(violations) { + const entries = {}; + for (const v of violations) { + const key = entryKey(v.file, v.phaseId, v.stepId); + const errors = (v.errors || []).map(fingerprintError); + entries[key] = { schema: v.schemaRef, errors }; + } + return entries; +} + +function sortEntries(entries) { + const out = {}; + for (const k of Object.keys(entries).sort()) out[k] = entries[k]; + return out; +} + +async function main() { + const args = new Set(process.argv.slice(2)); + const { violations, parseErrors } = await lintAll(); + + if (args.has('--write-allowlist')) { + if (parseErrors.length > 0) { + console.error(`Refusing to regenerate allowlist while ${parseErrors.length} file(s) have YAML parse errors — fix those first:`); + for (const p of parseErrors) console.error(` ${path.relative(STORYBOARD_DIR, p.file)}: ${p.error}`); + process.exit(1); + } + const existing = loadAllowlist(); + const nextEntries = violationsToAllowlist(violations); + const existingEntries = existing.entries || {}; + if (!args.has('--allow-grow')) { + const added = Object.keys(nextEntries).filter((k) => !(k in existingEntries)); + const grew = Object.keys(nextEntries).some((k) => { + if (!(k in existingEntries)) return false; + const cur = new Set(nextEntries[k].errors || []); + const prev = new Set(existingEntries[k].errors || []); + for (const fp of cur) if (!prev.has(fp)) return true; + return false; + }); + if (added.length > 0 || grew) { + console.error('Refusing to grow the allowlist. Pass --allow-grow if this is intentional.'); + process.exit(1); + } + } + const doc = { + $comment: [ + 'Known storyboard sample_response schema drift. Regenerate with', + '`node scripts/lint-storyboard-response-schema.cjs --write-allowlist`', + 'after a real fix — defaults to shrink-only.', + ].join(' '), + entries: sortEntries(nextEntries), + }; + fs.writeFileSync(ALLOWLIST_PATH, JSON.stringify(doc, null, 2) + '\n'); + console.log(`Wrote ${Object.keys(nextEntries).length} entries to ${path.relative(process.cwd(), ALLOWLIST_PATH)}`); + process.exit(0); + } + + const allowlist = loadAllowlist(); + const { newDrift, stale, grandfathered } = reconcileAgainstAllowlist(violations, allowlist); + + const hasFailure = newDrift.length > 0 || stale.length > 0 || parseErrors.length > 0; + if (!hasFailure) { + const suffix = grandfathered.length > 0 ? ` (${grandfathered.length} grandfathered)` : ''; + console.log(`✅ sample_response schema lint: no new drift${suffix}`); + process.exit(0); + } + + if (parseErrors.length > 0) { + console.log(`❌ sample_response schema lint: ${parseErrors.length} file(s) have YAML parse errors\n`); + for (const p of parseErrors) console.log(` ${path.relative(STORYBOARD_DIR, p.file)}: ${p.error}`); + } + if (newDrift.length > 0) { + console.log(`\n❌ sample_response schema lint: ${newDrift.length} step(s) have new drift\n`); + for (const v of newDrift) console.log(formatViolation(v)); + } + if (stale.length > 0) { + console.log(`\n❌ sample_response schema lint: ${stale.length} stale allowlist entr${stale.length === 1 ? 'y' : 'ies'} (drift was fixed — remove from allowlist)\n`); + for (const s of stale) console.log(` ${s.key}`); + } + process.exit(1); +} + +if (require.main === module) main(); + +module.exports = { + lintAll, + lintFile, + validateStep, + loadSchema, + formatViolation, + fingerprintError, + entryKey, + loadAllowlist, + reconcileAgainstAllowlist, + violationsToAllowlist, + sortEntries, + STORYBOARD_DIR, +}; diff --git a/tests/lint-storyboard-response-schema.test.cjs b/tests/lint-storyboard-response-schema.test.cjs new file mode 100644 index 0000000000..49101e6b66 --- /dev/null +++ b/tests/lint-storyboard-response-schema.test.cjs @@ -0,0 +1,150 @@ +#!/usr/bin/env node +/** + * Wrapper for the storyboard sample_response schema lint. Fails CI on any + * new drift beyond the known-issues allowlist or on stale entries that + * were never removed after a fix. + */ + +'use strict'; + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); + +const path = require('node:path'); + +const { + lintAll, + loadAllowlist, + reconcileAgainstAllowlist, + fingerprintError, + formatViolation, + entryKey, + STORYBOARD_DIR, +} = require('../scripts/lint-storyboard-response-schema.cjs'); + +test('no storyboard YAML parse errors', async () => { + const { parseErrors } = await lintAll(); + if (parseErrors.length > 0) { + const rendered = parseErrors.map((p) => ` ${p.file}: ${p.error}`).join('\n'); + assert.fail( + `${parseErrors.length} storyboard file(s) failed to parse. Fix the YAML before running lint:\n${rendered}`, + ); + } +}); + +test('no new sample_response schema drift beyond the allowlist', async () => { + const { violations } = await lintAll(); + const allowlist = loadAllowlist(); + const { newDrift } = reconcileAgainstAllowlist(violations, allowlist); + if (newDrift.length > 0) { + const rendered = newDrift.map(formatViolation).join('\n'); + assert.fail( + `${newDrift.length} step(s) have sample_response schema drift that is not in the allowlist.\n` + + 'Fix the fixture to match the schema, or (rarely) regenerate the allowlist if the drift is deliberate and pending a fix:\n' + + ' node scripts/lint-storyboard-response-schema.cjs --write-allowlist\n\n' + + rendered, + ); + } +}); + +test('allowlist has no stale entries', async () => { + const { violations } = await lintAll(); + const allowlist = loadAllowlist(); + const { stale } = reconcileAgainstAllowlist(violations, allowlist); + if (stale.length > 0) { + const rendered = stale.map((s) => ` ${s.key}`).join('\n'); + assert.fail( + `${stale.length} allowlist entr${stale.length === 1 ? 'y is' : 'ies are'} stale — the drift was fixed but the entry was not removed.\n` + + 'Regenerate the allowlist:\n' + + ' node scripts/lint-storyboard-response-schema.cjs --write-allowlist\n\n' + + rendered, + ); + } +}); + +test('fingerprintError produces stable output for common error shapes', () => { + assert.equal( + fingerprintError({ keyword: 'required', path: '/', params: { missingProperty: 'creatives' } }), + 'required@/:creatives', + ); + assert.equal( + fingerprintError({ keyword: 'additionalProperties', path: '/pagination', params: { additionalProperty: 'next_token' } }), + 'additionalProperties@/pagination:next_token', + ); + assert.equal( + fingerprintError({ keyword: 'type', path: '/pagination/has_more', params: { type: 'boolean' } }), + 'type@/pagination/has_more:boolean', + ); + assert.equal( + fingerprintError({ keyword: 'const', path: '/status', params: { allowedValue: 'approved' } }), + 'const@/status:approved', + ); + assert.equal( + fingerprintError({ keyword: 'const', path: '/shape', params: { allowedValue: { type: 'object' } } }), + 'const@/shape:{"type":"object"}', + ); +}); + +// Direct unit tests for the ratchet reducer. The end-to-end tests above prove +// the current tree is clean; these prove the classification logic is honest +// regardless of what the tree happens to contain. +test('reconcileAgainstAllowlist classifies new drift, grandfathered, and stale correctly', () => { + const file = path.join(STORYBOARD_DIR, 'fixture.yaml'); + const grandfatheredViolation = { + file, + phaseId: 'phase_a', + stepId: 'step_grandfathered', + schemaRef: 'x/y.json', + errors: [{ path: '/', keyword: 'required', params: { missingProperty: 'creatives' } }], + }; + const newDriftViolation = { + file, + phaseId: 'phase_a', + stepId: 'step_new', + schemaRef: 'x/y.json', + errors: [{ path: '/', keyword: 'required', params: { missingProperty: 'creatives' } }], + }; + const grandfatheredPlusNew = { + file, + phaseId: 'phase_b', + stepId: 'step_mixed', + schemaRef: 'x/y.json', + errors: [ + { path: '/', keyword: 'required', params: { missingProperty: 'creatives' } }, + { path: '/', keyword: 'additionalProperties', params: { additionalProperty: 'bogus' } }, + ], + }; + + const allowlist = { + entries: { + [entryKey(file, 'phase_a', 'step_grandfathered')]: { + schema: 'x/y.json', + errors: ['required@/:creatives'], + }, + [entryKey(file, 'phase_b', 'step_mixed')]: { + schema: 'x/y.json', + errors: ['required@/:creatives'], + }, + [entryKey(file, 'phase_a', 'step_fixed_but_listed')]: { + schema: 'x/y.json', + errors: ['required@/:something'], + }, + }, + }; + + const { newDrift, stale, grandfathered } = reconcileAgainstAllowlist( + [grandfatheredViolation, newDriftViolation, grandfatheredPlusNew], + allowlist, + ); + + assert.equal(grandfathered.length, 1, 'pure grandfathered violation classified'); + assert.equal(newDrift.length, 2, 'brand-new step and new error on listed step both count as drift'); + const newDriftSteps = newDrift.map((v) => v.stepId).sort(); + assert.deepEqual(newDriftSteps, ['step_mixed', 'step_new']); + const mixedOnly = newDrift.find((v) => v.stepId === 'step_mixed'); + assert.equal(mixedOnly.errors.length, 1, 'only the unexpected error is kept on a mixed step'); + assert.equal(mixedOnly.errors[0].keyword, 'additionalProperties'); + + assert.equal(stale.length, 1, 'allowlist entry with no matching violation is stale'); + assert.ok(stale[0].key.endsWith('#phase_a/step_fixed_but_listed')); +}); diff --git a/tests/storyboard-response-schema-allowlist.json b/tests/storyboard-response-schema-allowlist.json new file mode 100644 index 0000000000..b9802691cf --- /dev/null +++ b/tests/storyboard-response-schema-allowlist.json @@ -0,0 +1,4 @@ +{ + "$comment": "Known storyboard sample_response schema drift. Regenerate with `node scripts/lint-storyboard-response-schema.cjs --write-allowlist` after a real fix — defaults to shrink-only.", + "entries": {} +} From c949227ff762f0f1090cf9414fa687e6eb962372 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Apr 2026 22:41:35 +0000 Subject: [PATCH 2/3] fix response-schema lint: type guard, allow-grow diagnostics, test_kit skip, export - Fix type guard to match request-side: `!sampleResponse || typeof !== 'object'` (prevents bare-string YAML values from reaching AJV) - Add `$test_kit.schemas.*` skip guard to match request-side lint - Add per-entry diagnostic output to `--allow-grow` refusal block - Export ALLOWLIST_PATH for consistency with request-side twin https://claude.ai/code/session_019dMkCmDw5ro7hzMQ6iTkKQ --- scripts/lint-storyboard-response-schema.cjs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/scripts/lint-storyboard-response-schema.cjs b/scripts/lint-storyboard-response-schema.cjs index 0d95f71963..31160e1d42 100644 --- a/scripts/lint-storyboard-response-schema.cjs +++ b/scripts/lint-storyboard-response-schema.cjs @@ -146,8 +146,8 @@ async function lintFile(file) { if (!stepId) continue; const schemaRef = step.response_schema_ref; const sampleResponse = step.sample_response; - if (!schemaRef || !sampleResponse) continue; - if (typeof sampleResponse !== 'object' && typeof sampleResponse !== 'string') continue; + if (!schemaRef || schemaRef.startsWith('$test_kit.') || !sampleResponse) continue; + if (!sampleResponse || typeof sampleResponse !== 'object') continue; const result = await validateStep({ schemaRef, payload: sampleResponse }); if (!result.ok) { violations.push({ file, phaseId, stepId, schemaRef, errors: result.errors }); @@ -292,6 +292,11 @@ async function main() { }); if (added.length > 0 || grew) { console.error('Refusing to grow the allowlist. Pass --allow-grow if this is intentional.'); + if (added.length > 0) { + console.error('New entries:'); + for (const k of added) console.error(` ${k}`); + } + if (grew) console.error('At least one existing entry gained a new error fingerprint.'); process.exit(1); } } @@ -348,4 +353,5 @@ module.exports = { violationsToAllowlist, sortEntries, STORYBOARD_DIR, + ALLOWLIST_PATH, }; From 347f57bbc6956437517d9d92a06561ac9ec89e8c Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 26 Apr 2026 22:37:19 +0000 Subject: [PATCH 3/3] =?UTF-8?q?fix(lint):=20harden=20response-schema=20lin?= =?UTF-8?q?t=20=E2=80=94=20schema=5Fnot=5Ffound=20and=20compile=5Ferror=20?= =?UTF-8?q?are=20now=20hard=20fails;=20surface=20$test=5Fkit.=20skip=20cou?= =?UTF-8?q?nts=20in=20summary?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `validateStep`: both `schema_not_found` and `compile_error` paths now return `{ ok: false }` with a structured error instead of soft-skipping via `{ ok: true, skipped }`. A typo'd `response_schema_ref` that references a missing schema file will now surface as a lint failure rather than silently passing. - `lintFile`: separates the `$test_kit.` guard (which remains the only legitimate skip path) into its own `skips++` counter instead of bundling it with the missing-schema early-continue. Cleans up the redundant second `!sampleResponse` check. - `lintAll`: aggregates skip counts from each file, returns `skips` in the result alongside `violations` and `parseErrors`. - `main`: includes skip count in the `✅` summary line when non-zero (e.g. `3 skipped ($test_kit.)`), matching grandfathered display format. - Tests: adds `validateStep: schema_not_found is a hard fail` test; also imports `validateStep` in the test module. Closes no new issue — follow-up to @bokelley comment on #3225. https://claude.ai/code/session_01DLY1QSdxAwFHYBmomD6x7r --- scripts/lint-storyboard-response-schema.cjs | 35 +++++++++++++------ .../lint-storyboard-response-schema.test.cjs | 8 +++++ 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/scripts/lint-storyboard-response-schema.cjs b/scripts/lint-storyboard-response-schema.cjs index 31160e1d42..784dc4d975 100644 --- a/scripts/lint-storyboard-response-schema.cjs +++ b/scripts/lint-storyboard-response-schema.cjs @@ -109,10 +109,16 @@ async function getValidator(schemaRef) { async function validateStep({ schemaRef, payload }) { const schema = loadSchema(schemaRef); - if (!schema) return { ok: true, skipped: 'schema_not_found', schemaRef }; + if (!schema) { + return { ok: false, errors: [{ path: '/', message: `schema not found: ${schemaRef}`, keyword: 'schema_not_found', params: {} }] }; + } const validate = await getValidator(schemaRef); - if (!validate) return { ok: true, skipped: 'schema_not_found', schemaRef }; - if (validate.compileError) return { ok: true, skipped: `compile_error: ${validate.compileError}`, schemaRef }; + if (!validate) { + return { ok: false, errors: [{ path: '/', message: `schema not found: ${schemaRef}`, keyword: 'schema_not_found', params: {} }] }; + } + if (validate.compileError) { + return { ok: false, errors: [{ path: '/', message: `compile error: ${validate.compileError}`, keyword: 'compile_error', params: {} }] }; + } const valid = validate(payload); if (valid) return { ok: true }; return { @@ -128,11 +134,12 @@ async function validateStep({ schemaRef, payload }) { async function lintFile(file) { const violations = []; + let skips = 0; let doc; try { doc = yaml.load(fs.readFileSync(file, 'utf8')); } catch (err) { - return { violations, parseError: `yaml_parse: ${err.message}` }; + return { violations, skips, parseError: `yaml_parse: ${err.message}` }; } const phases = Array.isArray(doc?.phases) ? doc.phases : []; for (const phase of phases) { @@ -146,27 +153,32 @@ async function lintFile(file) { if (!stepId) continue; const schemaRef = step.response_schema_ref; const sampleResponse = step.sample_response; - if (!schemaRef || schemaRef.startsWith('$test_kit.') || !sampleResponse) continue; - if (!sampleResponse || typeof sampleResponse !== 'object') continue; + if (!schemaRef || !sampleResponse || typeof sampleResponse !== 'object') continue; + if (schemaRef.startsWith('$test_kit.')) { + skips++; + continue; + } const result = await validateStep({ schemaRef, payload: sampleResponse }); if (!result.ok) { violations.push({ file, phaseId, stepId, schemaRef, errors: result.errors }); } } } - return { violations }; + return { violations, skips }; } async function lintAll() { const files = walkYaml(STORYBOARD_DIR); const violations = []; const parseErrors = []; + let skips = 0; for (const file of files) { const r = await lintFile(file); if (r.parseError) parseErrors.push({ file, error: r.parseError }); violations.push(...r.violations); + skips += r.skips; } - return { violations, parseErrors }; + return { violations, parseErrors, skips }; } function formatViolation(v) { @@ -270,7 +282,7 @@ function sortEntries(entries) { async function main() { const args = new Set(process.argv.slice(2)); - const { violations, parseErrors } = await lintAll(); + const { violations, parseErrors, skips } = await lintAll(); if (args.has('--write-allowlist')) { if (parseErrors.length > 0) { @@ -318,7 +330,10 @@ async function main() { const hasFailure = newDrift.length > 0 || stale.length > 0 || parseErrors.length > 0; if (!hasFailure) { - const suffix = grandfathered.length > 0 ? ` (${grandfathered.length} grandfathered)` : ''; + const parts = []; + if (grandfathered.length > 0) parts.push(`${grandfathered.length} grandfathered`); + if (skips > 0) parts.push(`${skips} skipped ($test_kit.)`); + const suffix = parts.length > 0 ? ` (${parts.join(', ')})` : ''; console.log(`✅ sample_response schema lint: no new drift${suffix}`); process.exit(0); } diff --git a/tests/lint-storyboard-response-schema.test.cjs b/tests/lint-storyboard-response-schema.test.cjs index 49101e6b66..7a5d9e9b08 100644 --- a/tests/lint-storyboard-response-schema.test.cjs +++ b/tests/lint-storyboard-response-schema.test.cjs @@ -16,6 +16,7 @@ const { lintAll, loadAllowlist, reconcileAgainstAllowlist, + validateStep, fingerprintError, formatViolation, entryKey, @@ -62,6 +63,13 @@ test('allowlist has no stale entries', async () => { } }); +test('validateStep: schema_not_found is a hard fail, not a soft skip', async () => { + const result = await validateStep({ schemaRef: 'does/not/exist.json', payload: {} }); + assert.equal(result.ok, false, 'schema_not_found must not be ok:true'); + assert.equal(result.errors.length, 1); + assert.equal(result.errors[0].keyword, 'schema_not_found'); +}); + test('fingerprintError produces stable output for common error shapes', () => { assert.equal( fingerprintError({ keyword: 'required', path: '/', params: { missingProperty: 'creatives' } }),