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..784dc4d975 --- /dev/null +++ b/scripts/lint-storyboard-response-schema.cjs @@ -0,0 +1,372 @@ +#!/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: false, errors: [{ path: '/', message: `schema not found: ${schemaRef}`, keyword: 'schema_not_found', params: {} }] }; + } + const validate = await getValidator(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 { + 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 skips = 0; + let doc; + try { + doc = yaml.load(fs.readFileSync(file, 'utf8')); + } catch (err) { + return { violations, skips, 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 || 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, 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, skips }; +} + +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, skips } = 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.'); + 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); + } + } + 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 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); + } + + 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, + ALLOWLIST_PATH, +}; diff --git a/tests/lint-storyboard-response-schema.test.cjs b/tests/lint-storyboard-response-schema.test.cjs new file mode 100644 index 0000000000..7a5d9e9b08 --- /dev/null +++ b/tests/lint-storyboard-response-schema.test.cjs @@ -0,0 +1,158 @@ +#!/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, + validateStep, + 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('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' } }), + '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": {} +}