Skip to content

fix(testing): align local storyboard schema grading - #2663

Merged
bokelley merged 2 commits into
mainfrom
adcp-6724-6725-sdk-work
Aug 23, 2026
Merged

fix(testing): align local storyboard schema grading#2663
bokelley merged 2 commits into
mainfrom
adcp-6724-6725-sdk-work

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Summary

  • make SDK storyboard/test clients enforce strict response schemas by default, matching hosted compliance grading
  • make authored response_schema validations grade strict AJV verdicts by default, with an explicit diagnostic opt-out
  • preserve implementation-specific compliance_testing.scenarios strings and document the updated behavior
  • add regression coverage for un-authored schema checks, opt-out behavior, client reuse, and AdCP 3.1.18 custom scenarios

Closes adcontextprotocol/adcp#6724
Closes adcontextprotocol/adcp#6725

Validation

  • npm run build:lib
  • npm run typecheck
  • npm run format:check
  • focused storyboard, version-negotiation, security, rejection-hint, and decisioning compliance tests
  • fast and slow Node shards; parallel-only timeout/port flakes reproduced cleanly in isolated reruns

aao-secretariat[bot]
aao-secretariat Bot previously approved these changes Aug 23, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Approve

Approve — strict-by-default response-schema grading for local storyboard/comply flows aligned with the hosted grader; scenarios opened to implementation-specific strings.

Checked:

  • Strict option threads cleanly through transport validation (client.ts responses:'strict'), AJV authored-check grading (validations.ts), and the runner.
  • Client reuse correctly distinguishes strict vs diagnostic clients.
  • Regression coverage is thorough across the new/changed branches.
  • Changeset present (.changeset/strict-storyboards-open-scenarios.md); reviewer flagged the patch level as a low nit for a default-behavior flip, but this is not a removed/renamed export, required-param flip, dropped enum, or response-shape change to the wire — the SDK's witness contract is unchanged.

No blocking (critical/high) or medium findings. Only two low-severity nits.

Decision-table walk:

  • Row 1 (critical/high): none.
  • Row 2 (gated_paths): false — n/a.
  • Row 3 (deleted high-risk): none; bin/adcp.js is (modified).
  • Row 4 (medium data-loss/schema/infra): no medium findings.
  • Row 5 (high_risk modified + medium): high_risk true but no medium findings.
  • Row 6: no prior decision.
  • Row 7: no no-auto-approve team match.
  • Row 8: fewer than three medium findings.
  • Row 9: approve.

Note: high_risk is true only because bin/adcp.js matches bin/** and was modified; no medium-or-higher concern was found on it, so per the flag guidance the modification is presumed safe. review_decision is REVIEW_REQUIRED but gated_paths is false, so no hard gate applies.

Nits (non-blocking):

  • .changeset/...:8 — consider whether a minor bump better reflects a default-behavior flip that can turn green runs red.
  • client.ts:199 — transport-strict protection for un-authored steps is Zod-only, not AJV.

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Approve

Approve.

Adds a --no-strict-response-schema-validation CLI escape hatch that threads strictResponseSchemaValidation: false through all six storyboard entry points, defaulting to strict (fail-closed). The change is documented, tested, and covered by an existing patch changeset whose type matches the additive, non-breaking impact.

Checks performed:

  • Changeset-vs-wire-impact: diff touches bin/** and src/lib/**; a patch changeset is present and correctly scoped for an additive flag (no removed/renamed exports, no required-param flip, no response-shape change). OK.
  • Witness-not-translator: the flag only relaxes validation strictness; it does not fabricate, normalize, or reshape wire data. OK.
  • High-risk flag: true only because bin/adcp.js is (modified); reviewer found no medium-or-higher concerns on that file, so the modification is presumed safe. Not an escalation trigger.
  • gated_paths is false; no author no-auto-approve team match.

No blocking findings. No medium findings. Falls through to row 9 → approve.

@bokelley
bokelley merged commit ff37e9b into main Aug 23, 2026
33 checks passed
@bokelley
bokelley deleted the adcp-6724-6725-sdk-work branch August 23, 2026 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant