ci(storyboards): lint validations[].path against the response schema

.changeset/lint-storyboard-validations-paths.md

@@ -0,0 +1,34 @@
+---
+---
+
+ci(storyboards): lint validations[].path against the response schema
+
+Closes the `validations[].path` follow-up from #3918's expert review. Companion to `lint-storyboard-context-output-paths.cjs` (#3937) — that one catches captures from undefined paths; this one catches assertions on undefined paths. Same shape of bug (storyboard authored an opinion the spec doesn't back), different surface.
+
+`scripts/lint-storyboard-validations-paths.cjs` walks every storyboard step that declares `response_schema_ref` and at least one path-bearing validation (`field_present`, `field_value`, `field_value_or_absent`, `field_absent`) and verifies each `path` resolves to a defined field in the response schema. Non-path-bearing checks (`error_code`, `response_schema`, `http_status`, etc.) are silently skipped — they have no path to validate.
+
+### Pure extension points
+
+The path resolver recognizes a class of node the existing context-output-paths lint did not: a "pure extension point" — a schema with `additionalProperties: true` and NO `properties` / `items` / composite variants. Examples:
+
+- `core/context.json` (opaque correlation data, by spec design)
+- `error.details` on `core/error.json` (`additionalProperties: true` because the structured shape lives in per-error-code `error-details/<code>.json` schemas selected at runtime)
+
+Once the resolver descends into one of these, any further path segments are accepted — the spec deliberately doesn't constrain what lives below. This is distinct from a *mixed* schema (declared `properties` AND `additionalProperties: true`, e.g. `si-get-offering-response.json`), where `additionalProperties: true` is forward-compat extension and the lint stays strict (the offering_id typo from #3937 was caught precisely because of this distinction).
+
+### What it caught on land
+
+Two real bugs, both fixed in this PR:
+
+1. **`creative/build-creative-{request,response}.json` schema-ref typo.** The build_creative task lives in `media-buy/`, not `creative/`. The storyboard step's `schema_ref` and `response_schema_ref` were broken refs that the existing storyboard-response-schema lint silently passed (the schema fails to load → no validation runs). Updated to `media-buy/build-creative-request.json` / `media-buy/build-creative-response.json`.
+
+2. **`verify_terminated_session` storyboard step asserted `success: false` on `si-send-message-response.json`.** That field doesn't exist on the schema — `si-send-message-response.json:48` explicitly states "Terminated sessions return error codes (SESSION_NOT_FOUND or SESSION_TERMINATED) instead of a success response." Restructured the step to use `expect_error: true` + `check: error_code, value: SESSION_TERMINATED` per the spec contract.
+
+### What's allowlisted
+
+`scripts/storyboard-validations-paths-allowlist.json` carries four entries, each with a documented `reason`:
+
+- Three `replayed` field assertions in `idempotency.yaml` — runtime convention for idempotency-replay (response schemas have `additionalProperties: true` but don't define `replayed`). Lifting requires either adding it to the spec or a typed runner check.
+- One `adcp_error` field assertion in `schema-validation.yaml` — envelope-level error field per the two-layer model in `error-handling.mdx`. Payload-schema validation can't reach the transport envelope; lifting requires the lint to recognize a small set of envelope-prefix paths and validate against `core/error.json`.
+
+Wired into `npm test` as `test:storyboard-validations-paths`. 10 tests including source-tree regression guard, typo-detection, non-path-check skip, oneOf descent through error.json, pure-extension-point semantics, mixed-schema strictness, and allowlist enforcement.

package.json

@@ -43,6 +43,7 @@
     "test:storyboard-sample-request-schema": "node --test --test-force-exit --test-timeout=30000 tests/lint-storyboard-sample-request-schema.test.cjs",
     "test:storyboard-response-schema": "node --test --test-force-exit --test-timeout=30000 tests/lint-storyboard-response-schema.test.cjs",
     "test:storyboard-context-output-paths": "node --test --test-force-exit --test-timeout=30000 tests/lint-storyboard-context-output-paths.test.cjs",
+    "test:storyboard-validations-paths": "node --test --test-force-exit --test-timeout=30000 tests/lint-storyboard-validations-paths.test.cjs",
     "test:storyboard-check-enum": "node --test --test-force-exit --test-timeout=30000 tests/lint-storyboard-check-enum.test.cjs",
     "test:storyboard-advisory-expiry": "node --test --test-force-exit --test-timeout=30000 tests/lint-storyboard-advisory-expiry.test.cjs",
     "test:storyboard-raw-mode-required": "node --test --test-force-exit --test-timeout=30000 tests/lint-storyboard-raw-mode-required.test.cjs",
@@ -66,7 +67,7 @@
     "test:platform-agnostic": "node tests/check-platform-agnostic.cjs",
     "test:oneof-discriminators": "node scripts/audit-oneof.mjs --check",
     "audit:oneof": "node scripts/audit-oneof.mjs",
-    "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-provides-state-for && 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-context-output-paths && npm run test:storyboard-check-enum && npm run test:storyboard-advisory-expiry && npm run test:storyboard-raw-mode-required && npm run test:storyboard-doc-parity && npm run test:pagination-invariant && npm run test:version-envelope && npm run test:test-dynamic-imports && npm run test:callapi-state-change && 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:oneof-discriminators && 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-provides-state-for && 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-context-output-paths && npm run test:storyboard-validations-paths && npm run test:storyboard-check-enum && npm run test:storyboard-advisory-expiry && npm run test:storyboard-raw-mode-required && npm run test:storyboard-doc-parity && npm run test:pagination-invariant && npm run test:version-envelope && npm run test:test-dynamic-imports && npm run test:callapi-state-change && 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:oneof-discriminators && 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 test:callapi-state-change && npm run typecheck",
     "test:storyboards": "bash scripts/run-storyboards-matrix.sh",

scripts/lint-storyboard-validations-paths.cjs

@@ -0,0 +1,290 @@
+#!/usr/bin/env node
+/**
+ * Validate every storyboard step's `validations[].path` resolves to a field
+ * defined by its `response_schema_ref`. Companion to
+ * `lint-storyboard-context-output-paths.cjs` — that one catches captures
+ * from undefined paths; this one catches assertions on undefined paths.
+ *
+ * Per #3918 follow-up: a `check: field_present, path: "media_buy_oid"`
+ * (typo) silently passes against any conformant agent that returns the
+ * actual `media_buy_id` — the storyboard nominally asserts something but
+ * the assertion's target doesn't exist.
+ *
+ * Coverage: every step under `static/compliance/source/` that declares
+ * `response_schema_ref` and has at least one `validations[]` entry whose
+ * `check` is a path-bearing form. Path-bearing checks are:
+ *   - field_present
+ *   - field_value
+ *   - field_value_or_absent
+ *   - field_absent
+ *
+ * Non-path-bearing checks (error_code, response_schema, http_status_in,
+ * http_status, any_of, on_401_require_header) are skipped — they don't
+ * carry a path to validate.
+ *
+ * Rule:
+ *   path_not_in_schema — `path` does not resolve to any defined property
+ *                        in the response schema (after $ref / oneOf / anyOf
+ *                        resolution and numeric-index descent into items).
+ */
+
+'use strict';
+
+const fs = require('node:fs');
+const path = require('node:path');
+const yaml = require('js-yaml');
+
+const STORYBOARD_DIR = path.resolve(__dirname, '..', 'static', 'compliance', 'source');
+const SCHEMA_DIR = path.resolve(__dirname, '..', 'static', 'schemas', 'source');
+const ALLOWLIST_PATH = path.join(__dirname, 'storyboard-validations-paths-allowlist.json');
+
+const PATH_BEARING_CHECKS = new Set([
+  'field_present',
+  'field_value',
+  'field_value_or_absent',
+  'field_absent',
+]);
+
+function loadAllowlist() {
+  if (!fs.existsSync(ALLOWLIST_PATH)) return [];
+  const doc = JSON.parse(fs.readFileSync(ALLOWLIST_PATH, 'utf8'));
+  return Array.isArray(doc.allowlist) ? doc.allowlist : [];
+}
+
+function isAllowlisted(allowlist, filePath, stepId, validationPath) {
+  const rel = path.relative(path.resolve(__dirname, '..'), filePath);
+  return allowlist.some(
+    (entry) =>
+      entry.file === rel &&
+      entry.step === stepId &&
+      entry.path === validationPath,
+  );
+}
+
+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;
+}
+
+function parsePath(raw) {
+  if (!raw) return [];
+  return raw.replace(/\[(\d+)\]/g, '.$1').split('.').filter(Boolean);
+}
+
+function schemaRefToPath(ref) {
+  if (!ref) return null;
+  const trimmed = ref.startsWith('/schemas/') ? ref.slice('/schemas/'.length) : ref;
+  return path.join(SCHEMA_DIR, trimmed);
+}
+
+const schemaCache = new Map();
+
+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;
+}
+
+/**
+ * A node is a "pure extension point" when it declares `additionalProperties:
+ * true` AND has no `properties` / `items` / composite variants. Examples:
+ * `core/context.json` (opaque correlation data, by spec design) and
+ * `error.details` (additionalProperties: true because the structured shape
+ * lives in per-error-code `error-details/<code>.json` schemas selected at
+ * runtime). Once we descend into one of these, any remaining path segments
+ * are accepted — the spec deliberately does not constrain what lives below.
+ *
+ * Mixed schemas (declared `properties` AND `additionalProperties: true`) are
+ * NOT pure extension points — those use `additionalProperties: true` for
+ * forward-compat extension, not as an open container, so paths through them
+ * MUST hit defined properties.
+ */
+function isPureExtensionPoint(node) {
+  if (!node || typeof node !== 'object') return false;
+  if (node.additionalProperties !== true) return false;
+  if (node.properties && Object.keys(node.properties).length > 0) return false;
+  if (node.items) return false;
+  if (Array.isArray(node.oneOf) || Array.isArray(node.anyOf) || Array.isArray(node.allOf)) return false;
+  return true;
+}
+
+function pathResolves(node, segments, seen = new Set()) {
+  if (!node || typeof node !== 'object') return false;
+  if (segments.length === 0) return true;
+
+  if (node.$ref) {
+    if (seen.has(node.$ref)) return false;
+    const next = new Set(seen);
+    next.add(node.$ref);
+    const resolved = loadSchema(node.$ref);
+    return pathResolves(resolved, segments, next);
+  }
+
+  const [seg, ...rest] = segments;
+
+  if (/^\d+$/.test(seg)) {
+    if (node.items && pathResolves(node.items, rest, seen)) return true;
+  } else if (node.properties && Object.prototype.hasOwnProperty.call(node.properties, seg)) {
+    if (pathResolves(node.properties[seg], rest, seen)) return true;
+  }
+
+  // Union semantics across `oneOf` / `anyOf` / `allOf` — see
+  // lint-storyboard-context-output-paths.cjs for the rationale.
+  const variants = node.oneOf || node.anyOf || node.allOf;
+  if (Array.isArray(variants)) {
+    for (const variant of variants) {
+      if (pathResolves(variant, segments, seen)) return true;
+    }
+  }
+
+  if (isPureExtensionPoint(node)) return true;
+
+  return false;
+}
+
+function* findStepsWithValidations(node, trail) {
+  if (Array.isArray(node)) {
+    for (let i = 0; i < node.length; i++) {
+      yield* findStepsWithValidations(node[i], [...trail, i]);
+    }
+    return;
+  }
+  if (node && typeof node === 'object') {
+    if (
+      typeof node.response_schema_ref === 'string' &&
+      Array.isArray(node.validations) &&
+      node.validations.length > 0
+    ) {
+      yield {
+        responseRef: node.response_schema_ref,
+        validations: node.validations,
+        stepId: typeof node.id === 'string' ? node.id : null,
+        trail: [...trail],
+      };
+    }
+    for (const key of Object.keys(node)) {
+      yield* findStepsWithValidations(node[key], [...trail, key]);
+    }
+  }
+}
+
+function lintDoc(doc, filePath, allowlist = []) {
+  const violations = [];
+  if (!doc) return violations;
+  for (const step of findStepsWithValidations(doc, [])) {
+    const schema = loadSchema(step.responseRef);
+    if (!schema) {
+      violations.push({
+        rule: 'response_schema_not_found',
+        filePath,
+        stepId: step.stepId,
+        responseRef: step.responseRef,
+      });
+      continue;
+    }
+    for (let i = 0; i < step.validations.length; i++) {
+      const v = step.validations[i];
+      if (!v || typeof v !== 'object') continue;
+      if (!PATH_BEARING_CHECKS.has(v.check)) continue;
+      const rawPath = v.path;
+      if (typeof rawPath !== 'string' || rawPath.length === 0) continue;
+      const segments = parsePath(rawPath);
+      if (pathResolves(schema, segments)) continue;
+      if (isAllowlisted(allowlist, filePath, step.stepId, rawPath)) continue;
+      violations.push({
+        rule: 'path_not_in_schema',
+        filePath,
+        stepId: step.stepId,
+        responseRef: step.responseRef,
+        validationPath: rawPath,
+        check: v.check,
+        index: i,
+      });
+    }
+  }
+  return violations;
+}
+
+function lint() {
+  const violations = [];
+  const files = walkYaml(STORYBOARD_DIR);
+  const allowlist = loadAllowlist();
+  for (const file of files) {
+    let doc;
+    try {
+      doc = yaml.load(fs.readFileSync(file, 'utf8'));
+    } catch {
+      // YAML parse errors are surfaced by sibling lints; skip rather than
+      // double-report.
+      continue;
+    }
+    violations.push(...lintDoc(doc, file, allowlist));
+  }
+  return violations;
+}
+
+const RULE_MESSAGES = {
+  path_not_in_schema: ({ validationPath, responseRef, check }) =>
+    `validations[].path \`${validationPath}\` (check: \`${check}\`) does not resolve to any defined ` +
+    `field in \`${responseRef}\`. The storyboard asserts on a path the spec schema does not define — ` +
+    'a real agent\'s response will silently pass `field_absent` and silently fail `field_present` / ' +
+    '`field_value` / `field_value_or_absent` regardless of what the agent actually returns.\n' +
+    '    Fix one of:\n' +
+    '      1. Update the path to a field that exists in the response schema.\n' +
+    '      2. Update the response_schema_ref to the schema that defines this path.\n' +
+    '      3. If the path traverses a documented extension point (error.details polymorphism, ' +
+    'additionalProperties: true convention), add an entry to ' +
+    '`scripts/storyboard-validations-paths-allowlist.json` with a `reason` string.',
+  response_schema_not_found: ({ responseRef }) =>
+    `response_schema_ref \`${responseRef}\` could not be loaded — fix the ref or the schema path.`,
+};
+
+function formatMessage(violation) {
+  const builder = RULE_MESSAGES[violation.rule];
+  return builder ? builder(violation) : `unknown rule ${violation.rule}`;
+}
+
+function main() {
+  const violations = lint();
+  if (violations.length === 0) {
+    const fileCount = walkYaml(STORYBOARD_DIR).length;
+    console.log(`  storyboard validations path lint: clean (${fileCount} storyboard files scanned)`);
+    return;
+  }
+  for (const v of violations) {
+    const rel = path.relative(path.resolve(__dirname, '..'), v.filePath);
+    const stepLabel = v.stepId ? `:${v.stepId}` : '';
+    console.error(`  error: ${rel}${stepLabel} — ${formatMessage(v)}`);
+  }
+  console.error(`\n  ${violations.length} storyboard validations path violation(s).`);
+  process.exit(1);
+}
+
+if (require.main === module) main();
+
+module.exports = {
+  RULE_MESSAGES,
+  PATH_BEARING_CHECKS,
+  lint,
+  lintDoc,
+  loadSchema,
+  loadAllowlist,
+  pathResolves,
+  parsePath,
+  formatMessage,
+};