Skip to content

feat(testing): add envelope_field_present storyboard check (adcp#3429)#1045

Merged
bokelley merged 3 commits into
mainfrom
bokelley/envelope-field-present
Apr 28, 2026
Merged

feat(testing): add envelope_field_present storyboard check (adcp#3429)#1045
bokelley merged 3 commits into
mainfrom
bokelley/envelope-field-present

Conversation

@bokelley

@bokelley bokelley commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Refs adcp#3429. Adds runtime + drift support for the storyboard validation that targets the v3 protocol envelope (`status`, `task_id`, `adcp_version`, `errors`) instead of per-tool response schemas.

Why

The 3.0.1 spec ships `universal/v3-envelope-integrity.yaml` with `field_present: status`, but `status` is on the v3 envelope (`protocol-envelope.json`), not the inner `get-adcp-capabilities-response.json`. The drift detector walks the inner response, so the assertion can't be statically verified — adcp-client#1039 had to add a `VERIFIER_UNREACHABLE` exemption.

adcp#3429 triage proposed two fixes:

  • Option A: add a `response_envelope_schema_ref` field to the storyboard (no SDK changes)
  • Option B: add a new `envelope_field_present` check type — better long-term design, requires SDK runner support to ship first

This PR is the SDK side of Option B. Once the upstream storyboard migrates `field_present: status` → `envelope_field_present: status`, the SDK's drift detector validates against the envelope schema and the `VERIFIER_UNREACHABLE` exemption can be dropped.

What changed

`src/lib/testing/storyboard/types.ts` — `'envelope_field_present'` joins the `StoryboardValidationCheck` union with a JSDoc explaining the scope difference.

`src/lib/testing/storyboard/validations.ts` — `runValidation` dispatches `envelope_field_present` to `validateFieldPresent`. Runtime semantics are identical: `TaskResult` already merges envelope fields into its surface, so `data.status` is the envelope's `status`. The result object reports the original check name verbatim (`'envelope_field_present'`) so reporters can distinguish.

`test/lib/storyboard-drift.test.js` — `collectFieldValidations` picks up `envelope_field_present` entries; a new `describe` block walks `ProtocolEnvelopeSchema` instead of `TOOL_RESPONSE_SCHEMAS[task]`. Existing `field_present` block continues to pin to per-tool response schemas.

Forward-compat

No current 3.0.1 storyboard uses `envelope_field_present` yet. The SDK accepts it now so when the upstream storyboard PR lands (and `npm run sync-schemas` pulls the new compliance bundle), drift detection picks up the new check immediately. The `VERIFIER_UNREACHABLE` exemption in adcp-client#1039 gets dropped in a follow-up after the upstream change ships.

Test plan

  • Build clean (`npm run build`)
  • Format clean (`npm run format`)
  • Lint clean — 0 errors (`npm run lint`)
  • 3 new runtime tests in `test/lib/storyboard-validations.test.js` (pass / missing field / missing path)
  • Drift detector picks up `envelope_field_present` entries when storyboards declare them; existing 493-pass `storyboard-drift.test.js` suite continues green
  • Changeset: `@adcp/client` minor — explains the runtime + drift surface

🤖 Generated with Claude Code

@bokelley
feat(testing): add envelope_field_present storyboard check (adcp#3429)
1cf5979 
Storyboards that assert envelope-level fields (status, task_id,
adcp_version, errors) need a way to tell static drift detection to
walk protocol-envelope.json instead of the per-tool response schema.
field_present pointed at the inner response schema, which doesn't
contain envelope fields, so the v3-envelope-integrity storyboard
needed a VERIFIER_UNREACHABLE exemption.

This adds envelope_field_present as a recognized check type:
- Runtime: identical semantics to field_present (TaskResult merges
  envelope fields into its surface, so dispatch passes through to
  validateFieldPresent). The result reports check name verbatim so
  reporters can distinguish.
- Drift: walks ProtocolEnvelopeSchema instead of TOOL_RESPONSE_SCHEMAS
  for envelope_field_present entries. New describe block covers the
  case; the field_present block stays pinned to inner-response checks.

Forward-compatible with current 3.0.1 storyboards (no consumers yet).
Lights up when upstream PR migrates v3-envelope-integrity.yaml. The
VERIFIER_UNREACHABLE entry stays for now and is dropped after adcp
3.0.2 ships the storyboard migration.

Tests:
- 3 runtime cases (pass, missing field, missing path)
- Drift detector picks up envelope_field_present entries when present

Refs adcp#3429.
@bokelley bokelley mentioned this pull request Apr 28, 2026
Closed
@bokelley
fix(testing): expert-review feedback on envelope-scoped checks
e0c7551 
Two reviews (code-reviewer, ad-tech-protocol-expert) on adcp-client#1045
landed three things:

- Doc bug: my comments and changeset listed `errors` and `adcp_version`
  as v3-envelope fields. Verified against
  schemas/cache/3.0.1/core/protocol-envelope.json — `errors` lives
  inside `payload`, and `adcp_version` / `adcp_major_version` are
  request-side only. The actual envelope fields are `status`, `task_id`,
  `message`, `replayed`, `governance_context`, `timestamp`, `context_id`,
  `push_notification_config`. Fixed comments in types.ts (union JSDoc)
  and storyboard-drift.test.js (filter loop comment), plus the
  changeset wording.

- Companion check types: protocol expert flagged that shipping just
  `envelope_field_present` half-paves the cowpath — `replayed`/`status`
  storyboards will need value matchers next. Added
  `envelope_field_value` and `envelope_field_value_or_absent`
  alongside, with the same passthrough-to-existing-handler dispatch
  pattern. Also wired both into scripts/conformance-replay.ts's
  IMPLEMENTED_CHECKS so storyboard replay grades them. Filed
  `field_absent` + `envelope_field_absent` as a follow-up since they
  need a new runtime check (no current `field_absent` handler).

- Nit: `validateFieldPresent`'s `checkName` ternary is now just
  `validation.check` — the dispatcher has already narrowed the type
  by the time the value-shaping function runs, so the conditional was
  defensive coding without a real failure mode. Same simplification
  in `validateFieldValue` and `validateFieldValueOrAbsent`.

- New tests: 4 cases for the new value checks (pass/fail x present/absent).

Total: 514 tests pass, 0 fail.
@bokelley bokelley mentioned this pull request Apr 28, 2026
Closed
@bokelley

bokelley commented Apr 28, 2026

Copy link
Copy Markdown
Contributor Author

Triage note: #1046 (field_absent + envelope_field_absent) is a natural fold into this PR before it merges. Both check types touch src/lib/testing/storyboard/types.ts and validations.ts with ~20 extra lines, and the spec rationale (the v3-envelope-integrity.yaml TODO block) is fully specified in the issue. Worth pulling in now to avoid a rebase after this merges?

Generated by Claude Code

@bokelley

bokelley commented Apr 28, 2026

Copy link
Copy Markdown
Contributor Author

Yes, fold it. The rebase concern is real and the scope is genuinely small — field_absent + envelope_field_absent share the same handler path as the envelope_field_present passthrough already here, so the runtime delta is a single case block and an early-return on resolvePath === undefined. The drift-detector change is also minimal: add both to the collectFieldValidations filter list but skip reachability assertions on them (absence checks have no schema target by design).

Two things to update before pushing:

  1. Changeset — the current .changeset/envelope-field-present.md already says "A future PR will add field_absent + envelope_field_absent"; change that to include them in this PR's surface, bump the summary to list all five new check types, and re-run npm run changeset to replace the file if needed.
  2. Tests — add a describe('field_absent / envelope_field_absent') block in storyboard-validations.test.js covering pass (path absent), fail (path present), and no-path edge case — same pattern as the envelope_field_present block already in the file.

Once those land on the branch, the v3-envelope-integrity.yaml TODO block unlocks in one SDK release rather than two, and there's no rebase on the spec PR side.

Generated by Claude Code

@claude
feat(testing): fold field_absent + envelope_field_absent into #1045
45dbf2a 
Per @bokelley's review comment: both checks share the same handler path
as `envelope_field_present`, so the runtime delta is a single
`validateFieldAbsent` function and two `case` entries in the switch.

- `field_absent` / `envelope_field_absent` added to
  `StoryboardValidationCheck` type union
- `validateFieldAbsent`: passes when path is absent (undefined/null),
  fails when present — mirrors `validateFieldPresent` semantics inverted
- Drift detector: both checks collected by `collectFieldValidations` but
  skip reachability assertions (absence checks have no schema target)
- 6 new tests in `storyboard-validations.test.js` (pass/fail/no-path
  for both `field_absent` and `envelope_field_absent`)
- Changeset updated to list all five new check types; removes the
  "future PR" note since they ship here

Unlocks the `v3-envelope-integrity.yaml` TODO block's MUST-NOT
assertions (`task_status`/`response_status`) without a second SDK
release.

https://claude.ai/code/session_01R6VkP124L1RGqVART85gxo
@bokelley

bokelley commented Apr 28, 2026

Copy link
Copy Markdown
Contributor Author

Done — field_absent + envelope_field_absent folded in (45dbf2a).

What landed:

  • validateFieldAbsent: passes when path is undefined/null, fails when present — single handler shared by both check names
  • Both case entries in runValidation dispatch to it (3 lines of runtime delta as you predicted)
  • Drift detector: both checks collected by collectFieldValidations but skip reachability assertions per your note ("absence checks have no schema target by design")
  • 6 new tests in storyboard-validations.test.js: pass (path absent), fail (path present), no-path edge case × 2 check types — all green (22/22)
  • Changeset updated: removed the "A future PR will add…" note; lists all five check types

The v3-envelope-integrity.yaml TODO block (task_status / response_status MUST-NOT assertions) can now use field_absent / envelope_field_absent without a second release.

Generated by Claude Code

@bokelley

bokelley commented Apr 28, 2026

Copy link
Copy Markdown
Contributor Author

Confirmed — the fold looks complete. All five check types (field_present, envelope_field_present, field_absent, envelope_field_absent, plus whatever was already there) now ship in one release, and the v3-envelope-integrity.yaml TODO block unblocks without a second SDK cut. Nothing further needed from this end.

Generated by Claude Code

@bokelley

bokelley commented Apr 28, 2026

Copy link
Copy Markdown
Contributor Author

Acknowledged — noted that the fold is confirmed complete and the v3-envelope-integrity.yaml unblock is ready once the upstream storyboard PR lands.

Generated by Claude Code

@bokelley bokelley merged commit 49849f8 into main Apr 28, 2026
9 checks passed
@bokelley bokelley deleted the bokelley/envelope-field-present branch April 28, 2026 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@bokelley @claude