Skip to content

spec: fold protocol-envelope into per-task response schemas (closes #4878)#4896

Open
bokelley wants to merge 1 commit into
mainfrom
bokelley/4878-task-schema-envelope-fold
Open

spec: fold protocol-envelope into per-task response schemas (closes #4878)#4896
bokelley wants to merge 1 commit into
mainfrom
bokelley/4878-task-schema-envelope-fold

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Closes #4878. Companion to #4876 (envelope `status` REQUIRED) — that PR locked the contract on the envelope schema; this PR cascades it to every per-task response schema so per-task `response_schema` validators catch envelope omissions directly, without relying on the separate `envelope_field_present` storyboard check.

What changed

64 task response schemas now extend `protocol-envelope.json` in their `allOf` chain alongside the existing `version-envelope.json` ref. Two schemas without an existing `allOf` (`brand/search-brands-response.json`, `creative/validate-input-response.json`) had `allOf` added with both envelope refs for consistency.

Carve-outs (4 schemas excluded from fold)

Nested helpers, not task responses:

  • `core/pagination-response.json` — pagination metadata block embedded in list responses
  • `core/catalog-events-response.json` — REST endpoint payload for `GET /catalog/events`, not an AdCP task response

Body-level `status` enum collisions with envelope status:

  • `governance/check-governance-response.json` — body `status` enum is `[approved, denied, conditions]`
  • `governance/report-plan-outcome-response.json` — body `status` enum is `[accepted, findings]`

On MCP flat serialization both occupy the same root-level `status` key. Tracked as a separate spec issue (body-status rename to `verdict` / `decision` before 3.1 GA — filing follow-up).

What this catches

Pre-3.1-GA, any response shape lacking top-level envelope `status` now fails its own per-task `response_schema` validator, not just the universal `envelope_field_present` storyboard step. Validators integrated against the per-task schema (typed-SDK codegen, request-replay tooling, schema-aware test fixtures) gain envelope coverage for free.

Cleanup also applied (same PR)

The fold surfaced 87+ places where `status` was missing on examples / test fixtures / doc snippets:

  • 25 examples in the affected response schemas updated to include `status: "completed"`
  • 62 JSON blocks across 27 docs .mdx files updated likewise (mechanical injection — script attached in commit history)
  • Test fixtures in `tests/composed-schema-validation.test.cjs`, `tests/example-validation-simple.test.cjs`, and `tests/schema-validation.test.cjs` updated to include `status` on the relevant cases
  • Storyboard sample_response for `specialisms/signal-marketplace/scenarios/governance_denied.yaml` updated to include `status: "failed"`

Validation

  • `npm run test:schemas` — 8/8 ✓
  • `npm run test:examples` — 36/36 ✓
  • `npm run test:json-schema` — 270 doc snippets ✓
  • `npm run test:composed` — 43/43 ✓
  • `npm run test:storyboard-response-schema` — passes ✓
  • `npm run lint:schema-links` — passes ✓

The one pre-existing failure in `test:storyboard-sample-request-schema` is from #4810's `native_in_feed.yaml` (`impression_tracker oneOf`) and is unrelated to this PR — verified by stashing my changes and re-running.

Adopter impact

  • `@adcp/client` adopters: the auto-registered `get_adcp_capabilities` handler still needs to emit `status: "completed"` to pass per-task validation. SDK companion is filed as adcp-client: emit status on auto-registered get_adcp_capabilities handler #4877; until that lands, adopter conformance against the per-task schema validators will fail. Both PRs should ship together in beta.3.
  • Raw-handler adopters (deprecated v5): audit sync responses and add `status: "completed"` if missing.

Diff size

98 files / +971 / -306. Per [[feedback_diff_stat_sanity_check]] I verified the diff matches the expected breakdown:

  • 64 response schemas (fold)
  • 27 docs .mdx files (status injection)
  • 4 test files (fixture alignment)
  • 1 storyboard yaml
  • 1 changeset (`minor`)

Related

…4878)

Companion to #4876 (envelope status REQUIRED). That PR locked the contract
on protocol-envelope.json; this PR cascades the requirement to every
per-task response schema so per-task response_schema validators catch
envelope omissions directly, without relying on the separate
envelope_field_present storyboard check.

64 task response schemas now $ref protocol-envelope.json alongside
version-envelope.json. Two schemas (brand/search-brands, creative/
validate-input) had allOf added for consistency.

Carve-outs:
- core/pagination-response.json, core/catalog-events-response.json:
  nested helpers, not task responses.
- governance/check-governance-response.json,
  governance/report-plan-outcome-response.json:
  body-level status enum collides with envelope status on MCP flat
  serialization. Tracked as a separate spec issue (body-status rename
  to verdict/decision before 3.1 GA).

Cleanup applied alongside the fold:
- 25 examples in response schemas updated to carry status: "completed"
- 62 JSON blocks across 27 docs .mdx files updated likewise
- Test fixtures in tests/composed-schema-validation.test.cjs,
  tests/example-validation-simple.test.cjs, and
  tests/schema-validation.test.cjs updated
- Storyboard sample_response for signal-marketplace/governance_denied
  updated to include status: "failed"

Validation:
- npm run test:schemas (8/8)
- npm run test:examples (36/36)
- npm run test:json-schema (270/270 doc snippets)
- npm run test:composed (43/43)
- npm run test:storyboard-response-schema (passes)

SDK companion (#4877) is the going-forward fix in @adcp/client.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@aao-release-bot aao-release-bot Bot left a comment

Choose a reason for hiding this comment

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

LGTM. The fold is the right cascade from #4876 — per-task validators now catch envelope omissions directly instead of leaning on the envelope_field_present storyboard check.

Things I checked

  • Carve-out completeness: grep -L protocol-envelope.json static/schemas/source/**/*-response.json returns exactly the four documented files (core/pagination-response.json, core/catalog-events-response.json, governance/check-governance-response.json, governance/report-plan-outcome-response.json). 64 schemas folded + 4 carved = full inventory. No drift.
  • tasks-get-response.json and tasks-list-response.json are correctly folded (they are real task responses, not core helpers).
  • The two schemas without an existing allOf (brand/search-brands-response.json, creative/validate-input-response.json) had allOf added with both envelope refs — the right shape.
  • Carve-out rationale on the governance pair: body status $refs enums/governance-decision.json (approved/denied/conditions); envelope status $refs enums/task-status.json. On MCP flat serialization both occupy the same root-level status key. The carve-out is load-bearing — folding without renaming the body field would produce schemas that reject every conformant check_governance response.
  • Universal envelope_field_present storyboard check is unchanged and still enforces top-level status at the runner layer for the two carve-outs. The gap is annotated, not silent.
  • minor changeset severity matches #4876's precedent for the symmetric envelope-level change. Required-field promotion on responses is buyer-tolerant / seller-emit-tightening, which is the AdCP convention for minor during a beta train.
  • ad-tech-protocol-expert: sound-with-caveats — the fold is correct, severity is right, carve-out is sound but leaves a known false-pass on check_governance (a status: "approved" body field passes per-task but fails envelope task-status enum). The body-rename follow-up is the close.
  • code-reviewer: sound — no functional defects. Flags the em-dash → Unicode escape churn and a fixture comment opportunity.
  • audit-oneof.mjs walker walks top-level oneOf for variant discrimination and does not traverse allOf siblings — composing envelope refs alongside existing top-level oneOf (e.g., account/get-account-financials-response.json, brand/search-brands-response.json) does not change variant fingerprints. No regression expected; confirm on CI.

Follow-ups (non-blocking — file as issues)

  • Body-status rename on the governance carve-outs. check_governance and report_plan_outcome body statusverdict / decision is the close on the carve-out. Should be a hard 3.1-GA blocker with a milestone label, not a fire-and-forget. Until it lands, a conformant check_governance response is non-conformant against the envelope — the exact class of split-validation seam this PR is removing elsewhere.
  • SDK companion release coupling. PR description notes #4877 must ship with this for @adcp/client's auto-registered get_adcp_capabilities handler. Coordinate the beta.3 cut so the spec and the SDK land together; otherwise current adopters become non-conformant against the per-task validator instantly.
  • Carve-out $comment breadcrumbs. Add an inline $comment on the four carve-out schemas explaining why the envelope ref is absent. Right now the rationale lives only in the PR body and changeset; once merged, a maintainer running grep -L protocol-envelope has no on-schema explanation.
  • Test fixture annotation. tests/composed-schema-validation.test.cjs:519 uses status: 'accepted' on the report_plan_outcome fixture — the body-status enum value. Add a comment pointing at the body-rename follow-up so a future reader doesn't "fix" this by flipping it to completed and lose the body-status assertion.

Minor nits (non-blocking)

  1. Em-dash → Unicode escape churn across static/schemas/source/**/*-response.json. Serializer artifact (likely json.dumps(..., ensure_ascii=True) or equivalent), affects ~200 lines of unrelated description text. No-op for JSON parsers, but bloats the diff and breaks byte-level fingerprints for downstream consumers (content-addressed schema stores, codegen output diffs). Re-run the serializer with ensure_ascii=False if you re-roll for any reason — not worth a force-push on its own.

Approving on the strength of the carve-out completeness check and the precedent set by #4876.

@bokelley
Copy link
Copy Markdown
Contributor Author

Holding this PR pending #4877 (SDK companion) landing in @adcp/client + a published release this repo can bump to.

The failing CI check (TypeScript Build / Server unit tests) is the SDK-coupling issue ad-tech-protocol-expert flagged in review:

FAIL server/tests/unit/collection-lists-storyboard.test.ts > collection-lists specialism storyboard > runs the full CRUD lifecycle end-to-end
AssertionError: step get_capabilities: protocol/get-adcp-capabilities-response.json validation failed: (root): must have required property 'status'

The training agent (server/src/training-agent/) uses @adcp/client's createAdcpServer / createAdcpServerFromPlatform to handle get_adcp_capabilities. Per #4877's analysis, that handler emits the capabilities payload (adcp, supported_protocols, media_buy, account, creative, library_version) but never sets status. With this PR folding protocol-envelope.json into per-task schemas, the schema-aware test now correctly catches the gap — but the gap fix lives in the sibling SDK repo.

Sequencing:

  1. adcp-client: emit status on auto-registered get_adcp_capabilities handler #4877 in adcp-client — merge + publish a new @adcp/client release that emits status: "completed" from the auto-registered get_adcp_capabilities handler.
  2. Bump @adcp/client version in this repo's package.json so the training agent picks up the SDK fix.
  3. This PR (spec: fold protocol-envelope into per-task response schemas (closes #4878) #4896) — rebase, re-run CI, admin-merge.

The fold itself is sound (all schema-relevant test suites pass: test:schemas 8/8, test:examples 36/36, test:json-schema 270/270, test:composed 43/43, test:storyboard-response-schema). Only the SDK-rendered-response check fails — which is exactly the contract we just locked, and exactly what #4877 is in flight to deliver.

Cross-linking #4877 so the publish event is visible from here.

bokelley added a commit that referenced this pull request May 21, 2026
…report_plan_outcome (#4897) (#4902)

Frees the top-level `status` key for envelope task-status (TaskStatus) under
MCP flat-on-the-wire serialization. The body-level governance field and the
envelope share a root key but carry different enums; whichever wins on the
wire, the other is silently destroyed and no validator catches it.

WG-recommended Option A per the issue triage (both ad-tech-protocol-expert
and adtech-product-expert):

- check-governance-response.json: status → verdict (enum unchanged:
  approved | denied | conditions). Updates the three if/then discriminator
  blocks, the required[], and prose threads on findings/conditions/expires_at.
- report-plan-outcome-response.json: status → outcome_state (enum unchanged:
  accepted | findings). Updates required[] and the findings prose thread.
- get-plan-audit-logs-response.json: entries[].status → entries[].verdict
  (cascade for vocabulary consistency with check-governance-response). Other
  status fields on the same response (plans[].status, governed_actions[].status)
  are lifecycle states, not verdicts — left unchanged.

Docs swept (~25 example bodies + table descriptions + prose):
- docs/governance/overview.mdx
- docs/governance/campaign/tasks/{check_governance,report_plan_outcome,get_plan_audit_logs}.mdx
- docs/governance/campaign/{audit-trail,specification}.mdx
- docs/reference/whats-new-in-3-1.mdx (migration note in Final-spec
  clarifications batch)

Storyboards swept — the triage scoped this as "no yaml renames needed" but
during implementation found four storyboards with field_present /
field_value assertions keyed on path: "status" against
check-governance-response. Updated to path: "verdict":
- specialisms/governance-spend-authority/{index,denied}.yaml
- specialisms/governance-delivery-monitor/index.yaml
- protocols/governance/index.yaml (plus a stale expected block referencing
  status: recorded — not in the enum — corrected to outcome_state: accepted)

Test fixture: composed-schema-validation report_plan_outcome case updated
to use outcome_state.

Adopter impact: wire-shape change on three experimental governance schemas
(x-status: experimental). Buyers and sellers rename one property name per
emitter / consumer; enum values unchanged. SDK regen required.

Related:
- #4876 — envelope status REQUIRED (beta.2)
- #4895 — companion media-buy collision (separate PR)
- #4896 — per-task envelope fold; carve-outs for these two schemas can be
  removed once this lands

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

spec: fold per-task response schemas into protocol-envelope.json (3.1 cleanup)

1 participant