Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .changeset/lint-storyboard-context-output-paths.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
---

ci(storyboards): lint context_outputs.path against the response schema

Closes the CI-gate item on #3918. New `lint-storyboard-context-output-paths.cjs` walks every storyboard under `static/compliance/source/`, finds steps with both `response_schema_ref` and `context_outputs[]`, and verifies each `path` resolves to a defined field in the referenced response schema (descending through `$ref`, `oneOf` / `anyOf` / `allOf` variants, and `items` for numeric indices).

Catches the class of bug where a storyboard captures from a path the spec doesn't define — a real agent's response will silently miss the capture, downstream `$context.<name>` consumers see undefined values, and the storyboard nominally passes lint while failing in practice.

The lint surfaced one real typo on land: `static/compliance/source/protocols/sponsored-intelligence/index.yaml`'s `si_get_offering` step was capturing `path: 'offering_id'` from `si-get-offering-response.json`, but the schema defines `offering.offering_id` (the id is nested inside the `offering` object). Fixed in this PR.

Two captures that traverse spec-sanctioned extension points (`error.details` polymorphism keyed on `error.code`, and request-key echo through `additionalProperties: true`) are documented in `scripts/storyboard-context-output-paths-allowlist.json` with `reason` strings explaining what the lint cannot statically verify; reviewers should re-read these on spec changes that might give those polymorphisms first-class schema treatment.

Wired into the `npm test` pipeline via `npm run test:storyboard-context-output-paths`. Companion to the existing `lint-error-codes.cjs` (which already covers the `expect_error.code` half of #3918's CI-gate ask).
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
"test:storyboard-test-kits": "node --test --test-force-exit --test-timeout=30000 tests/lint-storyboard-test-kits.test.cjs",
"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-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",
Expand All @@ -65,7 +66,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-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-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",
Expand Down
273 changes: 273 additions & 0 deletions scripts/lint-storyboard-context-output-paths.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,273 @@
#!/usr/bin/env node
/**
* Validate every storyboard step's `context_outputs[].path` resolves to a
* field defined by its `response_schema_ref`. Catches the class of bug where
* a storyboard captures `rights[0].rights_id` from a response whose schema
* has no `rights` property — the path is wishful and a real agent's response
* silently misses the capture.
*
* Per #3918: storyboard-author drift between assertion and spec is a class
* we want to catch at build time, not at adopter-validation time.
*
* Coverage: every step under static/compliance/source/ that declares both
* `response_schema_ref` and `context_outputs[]`. Scope intentionally narrow —
* `validations[].path` is a separate generalisation tracked elsewhere.
*
* 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-context-output-paths-allowlist.json');

/**
* The allowlist exists for legitimate captures the static lint can't verify
* against the spec schema alone — most commonly paths through error.details
* (polymorphic extension keyed on error.code, see error-handling.mdx) and
* runtime-convention echoes the spec leaves to additionalProperties: true.
* Each entry MUST carry a `reason` explaining why the lint can't verify the
* path; review during spec changes.
*/
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, contextPath) {
const rel = path.relative(path.resolve(__dirname, '..'), filePath);
return allowlist.some(
(entry) =>
entry.file === rel &&
entry.step === stepId &&
entry.path === contextPath,
);
}

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;
}

/**
* Walk a JSON Schema node to determine whether a dotted path resolves to
* any defined element. Follows `$ref`, descends through `properties.<name>`
* for object steps and `items` for numeric-index steps, and accepts any
* variant of `oneOf` / `anyOf` / `allOf` that resolves. Returns true when
* EVERY segment was either resolved by a defined property/items or accepted
* by at least one composite variant.
*
* Empty path resolves trivially (the root itself exists).
*/
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;

// Numeric — array index. Only valid when this node has `items`.
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;
}

// Composite variants — any variant that resolves the FULL remaining path
// (starting from the current segment, since composites apply at this node).
// Union-of-branches is correct for `oneOf`/`anyOf` (the resolved instance
// matches one variant, and is a path-existence question) AND for `allOf`
// (intersection of constraints means the resolved object carries the union
// of properties across branches — a property defined in any branch is
// present after merging).
const variants = node.oneOf || node.anyOf || node.allOf;
if (Array.isArray(variants)) {
for (const variant of variants) {
if (pathResolves(variant, segments, seen)) return true;
}
}

return false;
}

function* findContextOutputSteps(node, trail) {
if (Array.isArray(node)) {
for (let i = 0; i < node.length; i++) {
yield* findContextOutputSteps(node[i], [...trail, i]);
}
return;
}
if (node && typeof node === 'object') {
if (
typeof node.response_schema_ref === 'string' &&
Array.isArray(node.context_outputs) &&
node.context_outputs.length > 0
) {
yield {
responseRef: node.response_schema_ref,
contextOutputs: node.context_outputs,
stepId: typeof node.id === 'string' ? node.id : null,
trail: [...trail],
};
}
for (const key of Object.keys(node)) {
yield* findContextOutputSteps(node[key], [...trail, key]);
}
}
}

function lintDoc(doc, filePath, allowlist = []) {
const violations = [];
if (!doc) return violations;
for (const step of findContextOutputSteps(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.contextOutputs.length; i++) {
const out = step.contextOutputs[i];
const rawPath = out?.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,
contextPath: rawPath,
captureName: out?.key || out?.name || null,
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
// (lint-storyboard-response-schema, etc.); skip rather than double-report.
continue;
}
violations.push(...lintDoc(doc, file, allowlist));
}
return violations;
}

const RULE_MESSAGES = {
path_not_in_schema: ({ contextPath, responseRef, captureName }) => {
const name = captureName ? ` (capture: \`${captureName}\`)` : '';
return (
`context_outputs path \`${contextPath}\`${name} does not resolve to any defined ` +
`field in \`${responseRef}\`. The storyboard captures from a path the spec ` +
'schema does not define — a real agent response will silently miss this capture, ' +
'and downstream `$context.<name>` consumers will see undefined values.\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 is conditional (e.g., only present on a discriminated arm), ' +
'use a path that resolves through that arm\'s defined properties.'
);
},
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 context-output 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 context_outputs path violation(s).`);
process.exit(1);
}

if (require.main === module) main();

module.exports = {
RULE_MESSAGES,
lint,
lintDoc,
loadSchema,
loadAllowlist,
pathResolves,
parsePath,
formatMessage,
};
17 changes: 17 additions & 0 deletions scripts/storyboard-context-output-paths-allowlist.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"$comment": "Per-storyboard exceptions for lint-storyboard-context-output-paths.cjs. Each entry MUST carry a `reason` explaining why the lint can't statically verify the path against the response schema. Re-review during spec changes — if the polymorphism the lint can't follow gets first-class schema treatment, remove the entry.",
"allowlist": [
{
"file": "static/compliance/source/universal/billing-gate-dispatch.yaml",
"step": "sync_accounts_passthrough_rejects_agent",
"path": "accounts[0].errors[0].details.suggested_billing",
"reason": "Captures from `error.details.suggested_billing`, which is defined in `error-details/billing-not-permitted-for-agent.json` (the per-error-code details schema selected by `error.code`). The lint can't follow error-details polymorphism keyed on the runtime error code — `core/error.json` declares `details` as `additionalProperties: true` because the structured shape lives in the error-details schemas. Capture is spec-conformant; lifting this requires extending the lint to switch on sibling `error.code` values."
},
{
"file": "static/compliance/source/universal/idempotency.yaml",
"step": "create_media_buy_initial",
"path": "idempotency_key",
"reason": "Captures the seller's echo of the request `idempotency_key` for replay-comparison in the next step. Response schemas declare `additionalProperties: true` and don't list `idempotency_key` explicitly — the echo is a runtime convention the storyboard depends on, not a spec requirement. Idempotency-replay testing inherently asserts on the echo; removing the convention would require redesigning how the runner threads request-side values into downstream comparisons."
}
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ phases:

context_outputs:
- name: offering_id
path: 'offering_id'
path: 'offering.offering_id'
validations:
- check: response_schema
description: "Response matches si-get-offering-response.json schema"
Expand Down
Loading
Loading