Repository navigation
Conversation
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean, additive schema PR with no blocking or medium findings.
Checked:
- Schema change (
static/compliance/source/universal/storyboard-schema.yaml): adds a gated offlinetest_kitcatalog form of$build_assets_from_format. Additive, non-breaking wire change. - Lint normalizer (
scripts/lint-storyboard-sample-request-schema.cjs:22-47): strips the object form only whenallowTestKitCatalogis set, derived fromstoryboard.requirescontainingtest_kit_format_catalog_runner. Default-false keeps directvalidateStepcallers wire-strict and fails closed when ungated. - Changeset present (
.changeset/offline-legacy-format-fixtures.md): patch, non-breaking — correct scope and type. - Test coverage: thorough negative/positive cases for the gated path.
- No
oneOfdiscriminator changes, no dist/released-artifact edits, no schema↔docs drift, no breaking-class change.
Decision table: no critical/high/medium findings; gated_paths: false; high_risk: false; no no-auto-approve team match; not a breaking-class change. No escalation trigger fires. Falls through to row 9 → approve. (Note: review_decision is REVIEW_REQUIRED, but row 2 only applies when gated_paths is true, which it is not here.)
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean pass, no blocking or medium findings.
This PR adds the offline legacy-format test-kit catalog form of $build_assets_from_format across 4 files: the lint script (scripts/lint-storyboard-sample-request-schema.cjs), the schema (static/compliance/source/universal/storyboard-schema.yaml), its test, and a patch changeset. The object wrapper is correctly gated — stripped from static schema validation only when the storyboard declares requires: test_kit_format_catalog_runner and the wrapper is exactly {format_id: $context.<id>, catalog: "test_kit"}; it is never treated as a valid wire asset. Schema↔docs coherence, exact-two-key/exact-requirement matching, isAssetMap threading, and test coverage (gate, malformed wrappers, broken siblings, non-array requires) all check out. The changeset is present and correctly scoped as a patch.
No critical/high/medium findings. high_risk is false; gated_paths is false (review_decision REVIEW_REQUIRED does not gate when gated_paths is false). No no-auto-approve team match. Prior decision was approve. Falls through to row 9.
Legacy inline creative tests need the declaring agent's asset IDs, but a media-buy-only seller is not required to serve
list_creative_formats. Define an offline operator catalog for$build_assets_from_formatso these tests can use exact declarations without adding Creative Protocol requirements or network discovery.The new object form requires
test_kit_format_catalog_runner, letting older runners skip before fixture mutation. It matches complete format references, treats absent preparation as a coverage gap, and keeps malformed declarations as harness errors. Static sample-request lint recognizes only the gated wrapper; actual wire validation remains strict.This PR defines the optional harness contract. No executable storyboard uses it yet. SDK support is in adcontextprotocol/adcp-client#3588. The #8632 storyboard repair remains held until a compatible runner is published and adopted.
Refs #8632.
Validation: 23 focused sample-request lint tests, including malformed wrapper/gate and strict-wire controls; full repository commit checks and pre-push version/scope checks. All local checks run in Docker. The documented local advisory matrix option is used after a slow run; the full CI matrix remains required. Read-only protocol, code and security reviews completed.
Integration update: merged current main while preserving both
field_excludes_runnerandtest_kit_format_catalog_runner. The normal Docker commit checks passed, including 80 root unit files, all four server-unit shards with the hook’s isolated retry of a load-sensitive file, and typecheck. Normal pre-push version/scope gates passed; fresh CI and review apply to the updated head.