chore(dx): pre-4.0 skill + SDK DX sweep#205
Merged
Conversation
This was referenced Apr 19, 2026
Closed
Closed
02f9d78 to
9c318df
Compare
This was referenced Apr 20, 2026
bokelley
added a commit
that referenced
this pull request
Apr 20, 2026
Round-4 storyboard validators rejected refined proposals that were missing
fields the seller skill never taught. Per schemas/cache/core/proposal.json,
a Proposal requires proposal_id, name, and allocations[], and each
ProductAllocation requires product_id + allocation_percentage (which must
sum to 100 across the proposal). The seller skill's refine example only
emitted {proposal_id, name, status, allocations:[{product_id, packages}]}
— agents copying this shape produced payloads the validator rejected.
Update the seller Proposal Workflow example to:
- populate allocation_percentage on every allocation (even split when
the buyer sends packages, 100% single-product when they don't)
- use the schema-correct field name `proposal_status` instead of the
shorthand `status`
- add an explicit comment citing the required fields so future edits
don't drop them
The retail skill inherits the seller proposal pattern rather than
duplicating it. Add a one-line pointer in its "Seller Tools (Required)"
section so agents building retail platforms with guaranteed-deal support
know to follow the seller proposal workflow.
Separately, the seller skill mentioned `artifact_webhook` /
`reporting_webhook` request fields but never taught sellers how to emit
a webhook. Round-4 webhooks DX exploration found sellers rewriting ~30
lines of payload + HMAC boilerplate per webhook type. Add an "Emitting
Webhooks" section (before Proposal Workflow) covering:
- when to emit (async-approval transitions, artifact ready, delivery
reports the buyer subscribed to via reporting_webhook)
- payload construction via adcp.webhooks.create_mcp_webhook_payload
(and create_a2a_webhook_payload for A2A transport)
- signing via get_adcp_signed_headers_for_webhook, with the PR #205
behavior called out explicitly: the signer serializes with compact
separators matching httpx's `json=` wire bytes, so callers MUST
POST via `client.post(url, json=payload, headers=signed)` and NOT
hand-serialize the body (hand-serialization produces mismatched
bytes and silent 401s on the receiver)
- retry semantics (receiver dedupes on task_id; retry is a
byte-identical re-POST — don't re-sign on retry)
- one ~15-line worked example emitting a create_media_buy
completion webhook
4.1 follow-up: there is no dedicated `create_delivery_report_webhook_payload`
helper in adcp.webhooks today. The section directs sellers to reuse
`create_mcp_webhook_payload` with `task_type="get_media_buy_delivery"`
for now. If delivery-report webhooks become common enough to warrant a
shape-specific helper (pre-populated task_type, typed delivery result),
add it in 4.1 and update this section to prefer it.
No SDK or runtime code changed — skill-only.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Apr 20, 2026
bokelley
added a commit
that referenced
this pull request
Apr 20, 2026
Upstream AdCP spec PR adcontextprotocol/adcp#2478 tightened the webhook signature verifier rule from "MAY re-serialize" to "SHOULD NOT re-serialize", with verifiers that cannot capture raw bytes required to MUST fail closed. Rationale: re-serializing a parsed payload to reconstruct the signed bytes silently fails against signers whose output differs in separator choice, key order, unicode escape policy, or number formatting — masking signer bugs the verifier should surface. Our ADCPClient._verify_webhook_signature had exactly that fallback path at client.py:3307: when raw_body was None, it called json.dumps(payload) with Python-default spaced separators. Historically this "worked" only in the coincidental case where the signer also used Python-default separators; against any compliant signer (httpx json= or JS JSON.stringify, both compact) it produced false-rejects. After PR #205's signer fix it produced false-rejects against our own signer too. Fix --- - Remove the json.dumps(payload) fallback entirely. When raw_body is None, log an actionable error and return False. - Update docstring to cite adcp#2478 and point at common framework hooks for capturing raw bytes (FastAPI Request.body, Flask request.get_data, aiohttp Request.read, Express express.raw). - Existing tests that relied on the fallback now pass raw_body= explicitly. Same signature math, just spec-conformant call shape. - New test_verify_fails_closed_when_raw_body_missing pins the fail-closed behavior by signing a real HMAC, omitting raw_body, and asserting the verifier raises ADCPWebhookSignatureError. Paired with a positive sibling that proves the rejection is specifically about missing raw_body, not signature or payload shape. Audit of the RFC 9421 content-digest path in src/adcp/signing/ — the parallel trap called out in adcp#2478 is absent here: sign_request takes body: bytes and client.py:488 passes request.content (httpx's actual wire bytes), so digest input and wire body are byte-identical by construction. BREAKING CHANGE: ADCPClient webhook verification now requires raw_body to be passed through from the HTTP handler. Callers that relied on the implicit re-serialize-from-payload fallback will start seeing ADCPWebhookSignatureError until they plumb the raw body through from their framework's pre-parse hook. Fix path: # FastAPI @app.post("/webhook") async def hook(request: Request, x_adcp_signature: str = Header(...)): raw = await request.body() payload = await request.json() # order matters: read raw first result = await client.handle_webhook( payload, ..., signature=x_adcp_signature, raw_body=raw, ) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
added a commit
that referenced
this pull request
Apr 20, 2026
Closes #212. Collapses the seller's six-step boilerplate (build envelope, serialize, sign, merge headers, POST, echo token) into one call so the signer and the wire see the *same bytes* — the serialization-format drift PR #205 fixed in the hand-rolled path is structurally impossible here. Covers the legacy AdCP 3.x authentication schemes (Bearer, HMAC-SHA256) and emits a one-shot DeprecationWarning pointing migrators at WebhookSender for RFC 9421. Missing or unknown authentication raises with a message that names the fix (use WebhookSender), not a silent unsigned POST. Token-echo is opt-in via ``token_field=`` — the AdCP spec says the token is "echoed back in the payload" but doesn't name the field, so the caller picks one the receiver agrees to read. Defense-in-depth at the helper boundary: * HTTPS-only URL; rejects embedded userinfo (getting logged by every HTTP intermediary is a footgun). * CRLF / NUL rejection on credentials + extra_headers (belt-and-braces over httpx's own header validation). * Reserved-header blocklist covers Authorization, Content-*, Host, Signature, Signature-Input, X-AdCP-*; each class gets a fix-hint tailored to the likely mistake. * 10MB body-size cap (shared with WebhookSender.send_raw for parity). * 64-entry extra_headers cap. * authentication must be a Mapping; schemes must be a list. Tests (22 for deliver + 1 for WebhookSender parity) cover: Bearer/HMAC happy paths, byte-identical signing-vs-wire invariant, retry byte-identity, token-echo opt-in shape (MCP top-level vs Task metadata), default-off echo, deprecation warning, and every boundary-validation failure mode. SKILL.md "Emitting Webhooks" section shows both the 4.0 default (WebhookSender) and the 3.x legacy (deliver) paths side-by-side with production notes (shared httpx.AsyncClient, egress transport, token_field coordination). Four expert reviews (code, protocol, DX, security) across three rounds. Deferred as follow-up: IP-pinned egress transport factory; upstream AdCP issue for 9421-vs-legacy precedence when both are on one config. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
bokelley
added a commit
that referenced
this pull request
Apr 20, 2026
Round-4 storyboard validators rejected refined proposals that were missing
fields the seller skill never taught. Per schemas/cache/core/proposal.json,
a Proposal requires proposal_id, name, and allocations[], and each
ProductAllocation requires product_id + allocation_percentage (which must
sum to 100 across the proposal). The seller skill's refine example only
emitted {proposal_id, name, status, allocations:[{product_id, packages}]}
— agents copying this shape produced payloads the validator rejected.
Update the seller Proposal Workflow example to:
- populate allocation_percentage on every allocation (even split when
the buyer sends packages, 100% single-product when they don't)
- use the schema-correct field name `proposal_status` instead of the
shorthand `status`
- add an explicit comment citing the required fields so future edits
don't drop them
The retail skill inherits the seller proposal pattern rather than
duplicating it. Add a one-line pointer in its "Seller Tools (Required)"
section so agents building retail platforms with guaranteed-deal support
know to follow the seller proposal workflow.
Separately, the seller skill mentioned `artifact_webhook` /
`reporting_webhook` request fields but never taught sellers how to emit
a webhook. Round-4 webhooks DX exploration found sellers rewriting ~30
lines of payload + HMAC boilerplate per webhook type. Add an "Emitting
Webhooks" section (before Proposal Workflow) covering:
- when to emit (async-approval transitions, artifact ready, delivery
reports the buyer subscribed to via reporting_webhook)
- payload construction via adcp.webhooks.create_mcp_webhook_payload
(and create_a2a_webhook_payload for A2A transport)
- signing via get_adcp_signed_headers_for_webhook, with the PR #205
behavior called out explicitly: the signer serializes with compact
separators matching httpx's `json=` wire bytes, so callers MUST
POST via `client.post(url, json=payload, headers=signed)` and NOT
hand-serialize the body (hand-serialization produces mismatched
bytes and silent 401s on the receiver)
- retry semantics (receiver dedupes on task_id; retry is a
byte-identical re-POST — don't re-sign on retry)
- one ~15-line worked example emitting a create_media_buy
completion webhook
4.1 follow-up: there is no dedicated `create_delivery_report_webhook_payload`
helper in adcp.webhooks today. The section directs sellers to reuse
`create_mcp_webhook_payload` with `task_type="get_media_buy_delivery"`
for now. If delivery-report webhooks become common enough to warrant a
shape-specific helper (pre-populated task_type, typed delivery result),
add it in 4.1 and update this section to prefer it.
No SDK or runtime code changed — skill-only.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
added a commit
that referenced
this pull request
Apr 20, 2026
Upstream AdCP spec PR adcontextprotocol/adcp#2478 tightened the webhook signature verifier rule from "MAY re-serialize" to "SHOULD NOT re-serialize", with verifiers that cannot capture raw bytes required to MUST fail closed. Rationale: re-serializing a parsed payload to reconstruct the signed bytes silently fails against signers whose output differs in separator choice, key order, unicode escape policy, or number formatting — masking signer bugs the verifier should surface. Our ADCPClient._verify_webhook_signature had exactly that fallback path at client.py:3307: when raw_body was None, it called json.dumps(payload) with Python-default spaced separators. Historically this "worked" only in the coincidental case where the signer also used Python-default separators; against any compliant signer (httpx json= or JS JSON.stringify, both compact) it produced false-rejects. After PR #205's signer fix it produced false-rejects against our own signer too. Fix --- - Remove the json.dumps(payload) fallback entirely. When raw_body is None, log an actionable error and return False. - Update docstring to cite adcp#2478 and point at common framework hooks for capturing raw bytes (FastAPI Request.body, Flask request.get_data, aiohttp Request.read, Express express.raw). - Existing tests that relied on the fallback now pass raw_body= explicitly. Same signature math, just spec-conformant call shape. - New test_verify_fails_closed_when_raw_body_missing pins the fail-closed behavior by signing a real HMAC, omitting raw_body, and asserting the verifier raises ADCPWebhookSignatureError. Paired with a positive sibling that proves the rejection is specifically about missing raw_body, not signature or payload shape. Audit of the RFC 9421 content-digest path in src/adcp/signing/ — the parallel trap called out in adcp#2478 is absent here: sign_request takes body: bytes and client.py:488 passes request.content (httpx's actual wire bytes), so digest input and wire body are byte-identical by construction. BREAKING CHANGE: ADCPClient webhook verification now requires raw_body to be passed through from the HTTP handler. Callers that relied on the implicit re-serialize-from-payload fallback will start seeing ADCPWebhookSignatureError until they plumb the raw body through from their framework's pre-parse hook. Fix path: # FastAPI @app.post("/webhook") async def hook(request: Request, x_adcp_signature: str = Header(...)): raw = await request.body() payload = await request.json() # order matters: read raw first result = await client.handle_webhook( payload, ..., signature=x_adcp_signature, raw_body=raw, ) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
85988a2 to
da3ee39
Compare
This was referenced Apr 20, 2026
Closed
Closed
- Remove `compliance_testing` from `capabilities_response` calls at :67 and :222; it is not a SupportedProtocol enum value. - Strip invalid `measurement_type` / `verification` keys from `delivery_measurement` example (:110-114); schema only has `provider` + optional `notes`. - Fix `get_products` proposal return at :185 — `products_response()` has no `proposal=` kwarg; merge into response dict under plural `proposals` field. - Rewrite :479-481 compliance block to note `serve(test_controller=...)` wires the capability automatically; drop the invalid `supported_protocols` snippet. - Update serve() Quick Reference signature (:509), fix validation invocation to `npx -y -p @adcp/client adcp storyboard run` and show `serve(handler, port=3001)`, define missing `AGENT_URL` constant, add a Production Deployment signpost section before Common Mistakes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Drop "compliance_testing" from capabilities_response — not a valid
SupportedProtocol enum value; only "media_buy" applies here.
- Change standard-upload creative status from "accepted" to "approved".
"accepted" is not a member of CreativeStatus (processing, pending_review,
approved, rejected, archived).
- Fix storyboard invocation to use `npx -y -p @adcp/client adcp storyboard
run ...` so the CLI binary resolves under npx's package-vs-binary rules.
- Define AGENT_URL = "http://localhost:3001/mcp" in the main code block;
the format examples referenced it without defining it.
- Add port=3001 to the serve() example so it lines up with the validation
command.
- Add a "Generative Tools (Required)" section covering build_creative and
preview_creative. ADCPHandler advertises both by default and returns
not_supported unless overridden, so a generative seller must implement
them or the storyboard fails at the generation step. Documents the
required idempotency_key field (pattern ^[A-Za-z0-9_.:-]{16,255}$) and
points at adcp.server.idempotency.IdempotencyStore for retry dedupe.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop `"compliance_testing"` from the `capabilities_response` example —
it is not a valid `SupportedProtocol` enum value
(media_buy, signals, governance, sponsored_intelligence, creative, brand).
Compliance testing is a separate top-level capability block, not a
protocol name.
Correct the validation block to `npx -y -p @adcp/client adcp storyboard
run ...` so the `adcp` CLI binary resolves under the `@adcp/client`
package when invoked via npx.
Note on `provide_performance_feedback`: the `{"success": True,
"sandbox": True}` example does not match either variant of
`ProvidePerformanceFeedbackResponse`. Added a one-line pointer to the
generated schema rather than rewriting the example here.
Skipped the `AGENT_URL` constant edit — the skill does not reference
`AGENT_URL` anywhere, so no definition is needed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Replace hardcoded ISO timestamps in list_creatives example with
datetime.now(timezone.utc).isoformat() so copy-paste implementations
emit real created_date/updated_date values.
- Replace silent fallback in build_creative ("fall back to first
available") with an explicit CREATIVE_NOT_FOUND adcp_error. The
skill's own Common Mistakes table already flags this anti-pattern;
the example now matches the guidance.
- Annotate the target_format_id / output_format / format_id triple
alias lookup to mark target_format_id as canonical per spec and the
other keys as legacy aliases kept for compatibility.
- Fix the validation command to use `npx -y -p @adcp/client adcp
storyboard run ...` so the storyboard binary is resolved correctly
from the package.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…mand - Correct signal_ids filter to use (source, id) tuples per signal-id.json discriminated union; drop the `or sid` fallback that assumed bare strings. - Note that get_signals requires anyOf(signal_spec, signal_ids) — empty requests are invalid per schema. - Replace hardcoded deployed_at timestamps with datetime.now(timezone.utc); add the corresponding imports to the activate_signal example. - Add Custom bullet to "Marketplace or Owned?" describing agent-native segments (signal_type: "custom"). - Add Idempotency for activate_signal subsection covering required idempotency_key, IdempotencyStore wrap pattern, IDEMPOTENCY_CONFLICT, and the capability declaration. - Fix storyboard command to `npx -y -p @adcp/client adcp storyboard run` and surface port=3001 on the serve() call. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Add SIGNAL_NOT_FOUND to STANDARD_ERROR_CODES in server/helpers.py (recovery: "correctable"), matching the PRODUCT_NOT_FOUND / MEDIA_BUY_NOT_FOUND / PACKAGE_NOT_FOUND pattern. The skill build-signals-agent teaches this code and previously fell through to the terminal default. - Type sync_governance() params in server/base.py as SyncGovernanceRequest | dict[str, Any] to match its siblings. Re-export SyncGovernanceRequest / SyncGovernanceResponse from adcp.types (previously only reachable via generated_poc). - Correct the sync_creatives_response docstring in server/responses.py to list the real CreativeStatus enum values (processing, pending_review, approved, rejected, archived); the previous "accepted|pending_review|rejected" list was wrong on both ends. - Drop the undocumented-but-ignored mount kwarg from server.serve.serve(); it was never wired into FastMCP and no callers exist in src/, tests/, or examples/. - Add optional compliance_testing kwarg to server.responses.capabilities_response() so skills can declare a top-level compliance_testing block without post-processing the dict. Leaves schemas/cache/enums/error-code.json untouched — the cache is regenerated from the upstream schema bundle by scripts/sync_schemas.py; the SIGNAL_NOT_FOUND entry belongs upstream, not in the local cache. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Create examples/seller_agent.py, a complete ADCPHandler-based seller covering the media_buy_seller storyboard (9 steps, all core tools): get_adcp_capabilities, sync_accounts, sync_governance, get_products (with refine/proposal branch), create_media_buy, get_media_buys, update_media_buy (pause/resume/cancel/revision), list_creative_formats, sync_creatives, get_media_buy_delivery.
- Ship an in-file TestControllerStore (DemoStore) implementing force_account_status, force_media_buy_status, force_creative_status, simulate_delivery, simulate_budget_spend so the compliance_testing block is enabled automatically via serve(test_controller=...).
- Serve as the referenced starting point for the seller, generative-seller, and retail-media skills (eleven cross-links currently pointed at a missing file). Generative/creative-specific tools (build_creative, preview_creative) intentionally deferred to the generative-seller example.
- Apply the DX-triage corrections already landed in the skill: capabilities_response(["media_buy"]) only, delivery_measurement limited to provider, refine branch returns {**products_response(PRODUCTS), "proposals": [...]} rather than a nonexistent proposal kwarg.
- Verified: ruff check clean; process boots under uv run python examples/seller_agent.py and responds to MCP initialize on :3001 with serverInfo.name=demo-seller. mypy reports expected module-not-found noise when invoked on the loose example path outside the src tree.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Shared-workspace harness so future skill-build runs can share one pre-installed venv, write into per-skill output subdirs, bind to distinct ports, and invoke storyboards with a one-liner. Replaces the worktree-isolated subagent approach, where worktrees did not inherit the parent session's tool permissions and three of five runs timed out at 30 minutes without producing any files. New files: - scripts/skill-build-setup.sh — idempotent one-time setup. Creates .venv, bootstraps pip via ensurepip with a get-pip.py fallback for Python builds that ship without it, installs adcp editable, warms the @adcp/client npx cache with `adcp storyboard list`, and reports any ports in the 3001-3020 range that are already bound. - scripts/skill-run.sh <skill> <port> <storyboard> — runs one agent. Spawns .context/dx-runs/<skill>/agent.py via the venv python by absolute path so no shell wrapper sits between us and the child (confirmed fix for the safe-chain PPID issue), waits for MCP initialize to return 200 (tools/list is rejected without a session, so it is not a valid readiness probe), invokes the storyboard with --json, captures the transcript next to agent.py, and kills the server via trap with an lsof-based fallback on the port. - .context/dx-runs/README.md — documents why shared workspace beats worktrees, the directory layout, invocation, and the fixed port assignments (seller 3011, generative 3012, retail 3013, creative 3014, signals 3015). - .context/dx-runs/AGENT_BRIEF_TEMPLATE.md — delegation template for parent agents. Names the two scripts, pins the agent.py path, forbids edits to src/adcp/ or skills/, and requires sub-agents to stop and write a DX report after three consecutive failures rather than loop. Verified end-to-end on the creative skill (port 3014, storyboard creative_lifecycle): setup completes, server boots, storyboard runs to 1/1 passing, JSON transcript lands at .context/dx-runs/creative/creative_lifecycle.json, and the server is reaped with no port leak. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The skill-build harness sets ADCP_PORT per skill (e.g. 3011 for seller) to run validators concurrently without port collisions. The example previously only read PORT, so every harness-driven run of the example bound to the default 3001 while the harness probed the assigned port and timed out on the readiness check. - Read ADCP_PORT first, fall back to PORT, then 3001. - Derive AGENT_URL from PORT so products and format IDs advertise a scope that matches where the server is actually listening (previously baked in localhost:3001 regardless). Verified: bash scripts/skill-run.sh seller 3011 media_buy_seller → PASS. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The adcp_error call at :210 referenced {target_format_id} in its f-string
but the in-scope local is target_format. First caller to hit the fallback
branch (no creative in library and no format match) would crash at runtime
instead of returning CREATIVE_NOT_FOUND.
Extract a format_label derived from the actual target_format variable
(handles both dict and string shapes) and use that in the message.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both skills taught invented APIs for IdempotencyStore that crash on first call: - signals: `await store.wrap(key, params, handler)` — store.wrap takes one argument (handler) and returns a decorator, not a 3-arg coroutine. - generative-seller: `idempotency.get(key)` / `idempotency.put(key, value)` — IdempotencyStore exposes neither; get/put live on the backend and take different signatures entirely. Replace both with the canonical @store.wrap decorator pattern already shown in adcp.server.idempotency.__init__ docstring: decorate the handler, let @Wrap hash params and dedup by (caller_identity, idempotency_key). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…t kwarg Previous text claimed serve(test_controller=store) wires up the compliance_testing capability block automatically. It doesn't — register_test_controller only registers the comply_test_controller tool; nothing injects the block into get_adcp_capabilities. Verified in src/adcp/server/serve.py:117-125 and test_controller.py — there's no capability-injection path. Replace with the explicit pattern using the compliance_testing= kwarg (added in the B1 SDK commit 33a248f). Lists the four scenarios a seller using TestControllerStore typically declares. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- helpers.py:41 had a trailing `# noqa: E501` that remained after black split CREATIVE_DEADLINE_EXCEEDED across multiple lines; no line is long enough to warrant the waiver now. - responses.py capabilities_response docstring listed `compliance_testing` as a valid `supported_protocols` example, but it is not a SupportedProtocol enum value. Replace with the real enum values and point readers at the `compliance_testing=` kwarg for that capability. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two bugs surfaced when skill-driven validators ran the storyboards: - signals: `IdempotencyStore()` in the skill's example is a TypeError — the constructor requires `backend=`. Add `MemoryBackend` import and pass a backend + TTL (24h, the spec minimum). - seller: `proposals[0]` in the refine_products response was missing schema-required `name` and `allocations` fields. Storyboard rejects the response and every proposal flow fails. Add both. Storyboard results after these fixes: - creative_lifecycle: 1/1 PASS - signal_owned: 5/5, signal_marketplace: 11/11 PASS - media_buy_seller: 29/42 (harness rc=0); remaining failures are scenarios the skill doesn't teach (governance, invalid_transitions, pending_creatives, inventory_list) — 4.1 targets. - media_buy_generative_seller: 9/9 generative-specific scenarios PASS - media_buy_catalog_creative: 28/33 (harness rc=0); catalog packages shape is undocumented in the skill. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sync_governance was the last tool still shipping a hand-crafted stub
inputSchema (just {accounts: array}) — every MCP client reading
tools/list got a schema that hid the required idempotency_key field
and every per-account structure the real request carries.
Export SyncGovernanceRequest from adcp.types and add it to the
tool-to-model map in _generate_pydantic_schemas, so all 57 ADCP tools
now advertise inputSchemas generated from their <ToolName>Request
Pydantic models at import time.
Also tighten the generator's docstring to reflect that the fall-back
path is now a regression signal, not an expected outcome.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The MCP tool registry generates inputSchemas from Pydantic request
models at import time. Previously the only regression guard was a
big KNOWN_GAPS table in test_spec_coverage.py that tracked every
field a hand-crafted schema happened to omit — stale and noisy now
that the schemas ARE the Pydantic models.
Add tests/test_mcp_schema_drift.py with three targeted checks:
- every tool maps to a request model (catches new tools shipped
with a stub schema)
- every tool's advertised schema byte-matches fresh generation
(catches tampering or a broken _apply_pydantic_schemas)
- required fields on a representative slice of models appear in
the advertised schema (agents building payloads from tools/list
rely on this)
Plus a spot-check that the three tools previously worst-affected by
drift (get_products, build_creative, create_media_buy) now advertise
their real required fields — buying_mode, idempotency_key, account,
brand, start_time, end_time.
Prune the KNOWN_GAPS table from test_spec_coverage.py and its
oneOf/anyOf walker; the new module covers the same ground more
cleanly. The surviving test there is now a coarse coverage check
pointing at the drift suite.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…reative_status scenario Protocol review caught two inconsistencies between the seller skill and example: - examples/seller_agent.py wired test_controller=DemoStore() but never declared the compliance_testing capability block — doing exactly what the skill now teaches users not to do. Added the kwarg with all 5 scenarios the DemoStore implements. - skills/build-seller-agent/SKILL.md advertised only 4 scenarios in the example scenarios list despite MyStore implementing force_creative_status at line 457. A buyer reading capabilities would think the agent can't force creative state even though it can. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…artifact Code review nits from the final pass: - src/adcp/types/__init__.py had SyncGovernanceRequest/Response listed twice in __all__ — once in the "Event & Source Operations" block (wrong section, introduced during B1 re-export work) and once in "V3 Governance" (correct). Removed the former. - src/adcp/server/mcp_tools.py:824 had a Black-reflowed implicit string concat that read as a tuple-of-strings at first glance. Collapsed to a single-line literal. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
BREAKING CHANGE: serve(mount=...) kwarg removed. The mount= kwarg was accepted but never wired into FastMCP mounting — a silent no-op since serve.py's first commit (d9d8778). Removed in earlier commit 33a248f without a proper BREAKING CHANGE marker; this commit exists so release-please promotes the next tag to 4.0.0. Zero callers found in src/, tests/, examples/, or skills/ at removal time. Any external caller that relied on the parameter was relying on a silent no-op. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Round-4 webhooks DX exploration surfaced a silent 4.0 blocker: the
signer hashed json.dumps(payload) with Python default separators
(", " / ": "), but httpx json= and every AdCP reference verifier
read the raw body, which httpx writes with compact separators
(","/":"). Sellers using the documented pattern —
client.post(url, json=payload, headers=signed)
got 401s from every compliant verifier with no clue why. The buyer
side quietly worked because _verify_webhook_signature uses raw_body,
so the trap was invisible cross-SDK.
Fix
---
- src/adcp/webhooks.py:211 now calls json.dumps(..., separators=(",", ":"))
so the signed bytes match exactly what httpx (and most clients) send.
- Existing test_get_adcp_signed_headers_produces_correct_signature
updated to use compact separators in its round-trip check; captured
vectors that used spaced JSON now skip (same as before for mismatched
encodings).
- New test_signer_matches_httpx_json_wire_form pins the contract by
signing against httpx.Request(...).content directly. Any future
divergence fails loudly in CI.
Also tighten the operation_id docstring in create_mcp_webhook_payload
— the prior "deprecated from payload" wording contradicted the code,
which still emits it when provided.
BREAKING CHANGE: get_adcp_signed_headers_for_webhook now signs the
compact-separator JSON form of the payload. Callers that previously
hand-serialized spaced JSON and POSTed it with content= will see
signature mismatches after this change. The fix is to also serialize
with separators=(",", ":") or switch to httpx json= which already
uses that form.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Round-4 storyboard validators rejected refined proposals that were missing
fields the seller skill never taught. Per schemas/cache/core/proposal.json,
a Proposal requires proposal_id, name, and allocations[], and each
ProductAllocation requires product_id + allocation_percentage (which must
sum to 100 across the proposal). The seller skill's refine example only
emitted {proposal_id, name, status, allocations:[{product_id, packages}]}
— agents copying this shape produced payloads the validator rejected.
Update the seller Proposal Workflow example to:
- populate allocation_percentage on every allocation (even split when
the buyer sends packages, 100% single-product when they don't)
- use the schema-correct field name `proposal_status` instead of the
shorthand `status`
- add an explicit comment citing the required fields so future edits
don't drop them
The retail skill inherits the seller proposal pattern rather than
duplicating it. Add a one-line pointer in its "Seller Tools (Required)"
section so agents building retail platforms with guaranteed-deal support
know to follow the seller proposal workflow.
Separately, the seller skill mentioned `artifact_webhook` /
`reporting_webhook` request fields but never taught sellers how to emit
a webhook. Round-4 webhooks DX exploration found sellers rewriting ~30
lines of payload + HMAC boilerplate per webhook type. Add an "Emitting
Webhooks" section (before Proposal Workflow) covering:
- when to emit (async-approval transitions, artifact ready, delivery
reports the buyer subscribed to via reporting_webhook)
- payload construction via adcp.webhooks.create_mcp_webhook_payload
(and create_a2a_webhook_payload for A2A transport)
- signing via get_adcp_signed_headers_for_webhook, with the PR #205
behavior called out explicitly: the signer serializes with compact
separators matching httpx's `json=` wire bytes, so callers MUST
POST via `client.post(url, json=payload, headers=signed)` and NOT
hand-serialize the body (hand-serialization produces mismatched
bytes and silent 401s on the receiver)
- retry semantics (receiver dedupes on task_id; retry is a
byte-identical re-POST — don't re-sign on retry)
- one ~15-line worked example emitting a create_media_buy
completion webhook
4.1 follow-up: there is no dedicated `create_delivery_report_webhook_payload`
helper in adcp.webhooks today. The section directs sellers to reuse
`create_mcp_webhook_payload` with `task_type="get_media_buy_delivery"`
for now. If delivery-report webhooks become common enough to warrant a
shape-specific helper (pre-populated task_type, typed delivery result),
add it in 4.1 and update this section to prefer it.
No SDK or runtime code changed — skill-only.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- webhooks.create_mcp_webhook_payload() now accepts a token kwarg and
echoes it into the payload when provided. PushNotificationConfig.token
documents that buyers supply this at subscription time to validate
webhook authenticity ("Echoed back in webhook payload to validate
request authenticity"), but the MCP payload builder had no way to
surface it. Adds two tests in tests/test_webhook_handling.py covering
present and absent cases.
- serve.create_mcp_server() gains include_test_controller=False, and
serve.a2a_server.ADCPAgentExecutor / _build_agent_card now drop
comply_test_controller from the handler tool list and agent card when
no TestControllerStore is wired. Before this, ADCPHandler subclasses
got comply_test_controller advertised in tools/list and the A2A card
even though it dispatched to a _not_supported stub. Now the tool
appears only when the seller opts in via serve(..., test_controller=).
No log emitted in either path — the round-4 signals report flagged a
chatty "Adding automatically" message that does not exist in this
codebase (likely from an out-of-tree harness wrapper), so this change
removes the underlying advertising mismatch rather than silencing a
log we do not own.
Deferred finding (skills / schema, not SDK):
- The retail round-4 validator reported that provide_performance_feedback
harness runs strip a "feedback" field because the Pydantic-generated
inputSchema does not advertise it. Confirmed: the
provide-performance-feedback-request.json spec defines
performance_index / measurement_period / metric_type / feedback_source
/ idempotency_key / media_buy_id — there is no "feedback" field. The
hand-crafted stub previously in ADCP_TOOL_DEFINITIONS advertised a
non-spec "feedback" object; that stub is now correctly replaced by the
Pydantic-generated schema. Whatever teaches callers to send "feedback"
(skill prose, example code, or a harness request builder) needs to be
updated to the spec field names. Not touching skills/ here per scope.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Upstream AdCP spec PR adcontextprotocol/adcp#2478 tightened the webhook signature verifier rule from "MAY re-serialize" to "SHOULD NOT re-serialize", with verifiers that cannot capture raw bytes required to MUST fail closed. Rationale: re-serializing a parsed payload to reconstruct the signed bytes silently fails against signers whose output differs in separator choice, key order, unicode escape policy, or number formatting — masking signer bugs the verifier should surface. Our ADCPClient._verify_webhook_signature had exactly that fallback path at client.py:3307: when raw_body was None, it called json.dumps(payload) with Python-default spaced separators. Historically this "worked" only in the coincidental case where the signer also used Python-default separators; against any compliant signer (httpx json= or JS JSON.stringify, both compact) it produced false-rejects. After PR #205's signer fix it produced false-rejects against our own signer too. Fix --- - Remove the json.dumps(payload) fallback entirely. When raw_body is None, log an actionable error and return False. - Update docstring to cite adcp#2478 and point at common framework hooks for capturing raw bytes (FastAPI Request.body, Flask request.get_data, aiohttp Request.read, Express express.raw). - Existing tests that relied on the fallback now pass raw_body= explicitly. Same signature math, just spec-conformant call shape. - New test_verify_fails_closed_when_raw_body_missing pins the fail-closed behavior by signing a real HMAC, omitting raw_body, and asserting the verifier raises ADCPWebhookSignatureError. Paired with a positive sibling that proves the rejection is specifically about missing raw_body, not signature or payload shape. Audit of the RFC 9421 content-digest path in src/adcp/signing/ — the parallel trap called out in adcp#2478 is absent here: sign_request takes body: bytes and client.py:488 passes request.content (httpx's actual wire bytes), so digest input and wire body are byte-identical by construction. BREAKING CHANGE: ADCPClient webhook verification now requires raw_body to be passed through from the HTTP handler. Callers that relied on the implicit re-serialize-from-payload fallback will start seeing ADCPWebhookSignatureError until they plumb the raw body through from their framework's pre-parse hook. Fix path: # FastAPI @app.post("/webhook") async def hook(request: Request, x_adcp_signature: str = Header(...)): raw = await request.body() payload = await request.json() # order matters: read raw first result = await client.handle_webhook( payload, ..., signature=x_adcp_signature, raw_body=raw, ) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pull in the merged test vectors from adcontextprotocol/adcp#2478 which pinned compact separators as the canonical on-wire form, added vectors covering whitespace-sensitive keys / nested objects / escaped unicode, and added the rejection vector that reproduces the Python-default spaced-separators signer bug. Changes ------- - tests/fixtures/webhook-hmac-sha256.json — regenerated from https://github.com/adcontextprotocol/adcp/blob/main/static/test-vectors/webhook-hmac-sha256.json (14 positive vectors including 2 new, 10 rejection vectors including the signer-serialization-mismatch case). - HMAC_REJECTION_VECTORS loaded alongside the existing positive vectors. - New test_rejection_vectors_do_not_collapse_to_positive mirrors the upstream CI check: for every rejection vector whose claimed signature has a well-formed sha256=<hex> shape, verify a correctly-computed HMAC over the claimed raw_body does NOT match the claimed signature — otherwise the rejection vector silently collapses into a positive case and stops catching what it claims to. - New test_verifier_rejects_upstream_rejection_vectors is the behavioral mirror: for every rejection vector with enough context to run end-to-end, assert ADCPClient._verify_webhook_signature returns False. Together the two parametrizations pin both the math (claimed sig is genuinely invalid) and the behavior (our verifier catches it). Vectors with non-computable rejection shapes (empty/null signature, wrong length, double-prefix) skip the computational check — they're documented for verifier implementers but don't have a signature a correctly-configured HMAC would produce. Full pytest: 1674 passed / 18 skipped (15 of the new skips are structural-rejection vectors appropriately declining the computational parametrization). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…0 GA
After rebasing onto main (which added RFC 9421 webhook infrastructure +
idempotency_key on MCP payloads), the legacy HMAC signer in
src/adcp/webhooks.py was still calling json.dumps(payload_dict) without
compact separators — reintroducing the silent 401 trap from round-4 DX
exploration. Re-apply the separators=(",",":") pin at line 266 and
update the prior docstring reference + the adcp.signing.webhook_hmac
module-level doc to cite adcp#2478.
Test fallout
------------
Conformance tests in tests/conformance/signing/test_webhook_hmac.py and
tests/conformance/signing/test_webhook_receiver.py serialized the wire
body with json.dumps(payload).encode("utf-8") (spaced separators) and
signed separately with get_adcp_signed_headers_for_webhook(payload=...),
which now produces a compact-separator signature. The wire body and
signed bytes diverged — the exact failure mode adcp#2478 is supposed to
prevent. Swept those call sites to json.dumps(..., separators=(",", ":"))
so body bytes and signed bytes match.
test_verify_fails_closed_when_raw_body_missing: add idempotency_key to
the happy-path payload (now schema-required on McpWebhookPayload).
test_webhook_handling.py imports: drop unused GeneratedTaskStatus and
create_mcp_webhook_payload (the token-kwarg tests were dropped during
rebase — main went with idempotency_key instead of token).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…se, webhook re-exports Three P0 blockers surfaced by round-6 storyboard validation on the AdCP 3.0 GA rebase. 1. IdempotencyStore.capability() now emits the required supported field --------------------------------------------------------------------- Upstream PR #210 (adcp 3.0 GA regen) made ``adcp.idempotency.supported`` REQUIRED on the capabilities response. ``IdempotencyStore.capability()`` at src/adcp/server/idempotency/store.py:78 was still returning ``{"replay_ttl_seconds": N}`` only, so every agent using the documented ``capabilities_response(idempotency=store.capability())`` pattern silently emitted a schema-invalid capabilities block. Now returns ``{"supported": True, "replay_ttl_seconds": N}``. Four existing tests updated to match. 2. adcp-keygen --purpose for webhook-signing keys ------------------------------------------------- The webhook verifier enforces ``adcp_use == "webhook-signing"`` on the JWK, but ``adcp-keygen`` hardcoded ``adcp_use: "request-signing"`` with no override. A user following keygen → publish JWKS → emit webhook got ``webhook_signature_key_purpose_invalid`` on first delivery — the exact failure mode round-6 DX exploration flagged as a blocker for discoverability of the new 9421 path. Added ``--purpose {request-signing,webhook-signing}`` CLI flag (default request-signing for back-compat) and threaded the value through generate_ed25519 / generate_es256. 3. Top-level adcp re-exports the new 9421 webhook surface --------------------------------------------------------- ``adcp/__init__.py`` re-exported the deprecated legacy ``get_adcp_signed_headers_for_webhook`` but NOT the new 9421 entry points. A coding agent scanning ``dir(adcp)`` for webhook primitives saw only legacy. Added: ``sign_webhook``, ``WebhookReceiver``, ``WebhookReceiverConfig``, ``WebhookVerifyOptions``, ``WebhookDedupStore``, ``MemoryBackend``, ``LegacyHmacFallback``, ``generate_webhook_idempotency_key``. Also promoted the MemoryBackend / WebhookDedupStore imports in adcp.webhooks to explicit ``as`` re-exports so mypy treats them as public. BREAKING CHANGE: IdempotencyStore.capability() return shape changes from ``{"replay_ttl_seconds": N}`` to ``{"supported": True, "replay_ttl_seconds": N}``. Callers that byte-compared against the old shape will need to update their expected value. Required to emit schema-valid AdCP 3.0 capabilities responses. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Round-6 webhooks DX exploration + storyboard validators surfaced five polish items across SDK and skills. Bundled together since they all land on the same branch before round 7. SDK (src/adcp/signing/webhook_signer.py, src/adcp/webhook_receiver.py, src/adcp/webhooks.py): - `WebhookReceiver.receive_sync(...)` — sync wrapper for WSGI callers (Flask, Gunicorn sync workers, http.server). Raises a clear RuntimeError if called from inside a running event loop rather than silently deadlocking. - `WebhookDedupStore` + `MemoryBackend` re-exported from `adcp.webhooks` so users wire up receivers from one import root instead of two. - `sign_webhook` docstring gains a "See also: WebhookSender" pointer so callers find the higher-level one-liner before going low-level. Skills (skills/build-seller-agent/SKILL.md, skills/build-signals-agent/SKILL.md): - Seller capabilities example now passes `idempotency=idempotency.capability()` — required since `adcp.idempotency.supported` became mandatory in AdCP 3.0 GA. - Seller "Emitting Webhooks" section rewritten around `WebhookSender` (RFC 9421) with `generate_webhook_idempotency_key` reuse-on-retry rule. Legacy HMAC kept as 3-line deprecation pointer. - Seller gains a one-paragraph `WebhookReceiver` signpost — first teaching surface for buyer-side webhook receiving in any skill. - Signals skill documents marketplace-first ordering (prior-entry chaining into follow-up calls breaks when owned signals come first) and expands the `signal_ids` filter tuple from `(source, id)` to `(source, scope, id)` where scope is `agent_url` or `data_provider_domain` depending on source. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…K and schema Round-7 validator + webhooks-DX review surfaced one regression and three DX gaps that caused copy-paste agents to fail fast or teach non-existent APIs. Seller skill — Emitting Webhooks: - Rewrote around actual SDK surface. The prior example used `WebhookSender(key_id=..., private_key_pem=...)` + `sender.send(...)`; neither symbol exists. Replaced with `WebhookSender.from_jwk(jwk)` + `sender.send_mcp(url=, task_id=, status=, ...)` and added `resend()` for retries (replays signed bytes under a fresh signature). Private key now comes from a webhook-signing JWK, matching what adagents.json publishes. Signals skill: - Added "Governance Tracks (Optional)" section with `sync_accounts` + `sync_governance` stubs. The `signal_marketplace/governance_denied` sub-track requires both — prior skill never mentioned them, forcing validator to discover the requirement by failure. examples/seller_agent.py: - Refine-mode branch of `get_products` now emits proposals with the fields proposal.json requires: `proposal_id`, `name`, `allocations[]` (each with `product_id` + `allocation_percentage`). Even-splits across incoming packages (100 / n) and falls back to a single 100% allocation on the first product when no packages are sent. Seller skill taught this shape in round 5; the example was stale. AGENT_URL parameterization: - Seller, generative-seller, creative, signals skills now derive `AGENT_URL` from `ADCP_PORT` (defaults to 3001). Lets harness runs on 3011/3012 just work without editing the paste. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Round-7 DX validation surfaced two webhook ergonomics gaps. Both helpers close a loop the SDK already half-documented but didn't finish. - WebhookSender.from_pem(pem_path, key_id=..., alg=..., passphrase=...): the companion to `adcp-keygen --purpose webhook-signing`. Keygen writes a PEM and prints the PUBLIC JWK; until now senders had to either pull the private d out of a test vector or drop to `cryptography` directly to load the PEM. from_pem accepts a path or raw bytes, handles encrypted PEMs via passphrase, and fails fast when the PEM's key type doesn't match the declared alg (a previously silent misconfiguration that surfaced as a 401 at first delivery). - sign_legacy_webhook(secret, payload, *, timestamp=None, headers=None) returning (signed_headers, body_bytes). The existing get_adcp_signed_headers_for_webhook returns headers only, so callers who post with `json=payload` can drift away from the compact-separator bytes that were signed. The tuple return makes byte-equality between signature input and HTTP body structurally inevitable — callers pass `content=body_bytes` and the separator-drift trap is impossible. get_adcp_signed_headers_for_webhook stays for callers who own the serialization step; its docstring now cross-references the new helper. Both helpers are exported from adcp.webhooks and the top-level adcp package. sign_legacy_webhook shares its HMAC core with get_adcp_signed_headers_for_webhook via a private _compute_legacy_signature helper, so the two surfaces cannot diverge. Tests cover the new surface across ed25519 and es256, encrypted and unencrypted PEMs, wrong-passphrase error paths, alg/PEM-type mismatch, round-trip verification against the legacy HMAC verifier, timestamp pinning, and byte-equality against httpx's `content=` wire output. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…on rerun, default list_creatives timestamps - scripts/skill-run.sh: parse the storyboard JSON (via python3) after the runner exits and distinguish passing / partial / failing / missing-output. Storyboard runner exits 0 even on partial, so the process return code alone hid real failures behind a silent PASS. PASS and WARN keep exit 0; FAIL (failing or unparseable) exits 1. Final log line now names the actual status instead of PASS in all cases. - src/adcp/server/serve.py: set SO_REUSEADDR on the listening socket by pre-binding it and handing it to uvicorn via Server.serve(sockets= [sock]) for streamable-http, sse, and a2a transports. FastMCP builds its own uvicorn.Server internally and does not expose a reuse hook, so the MCP HTTP path reproduces the minimal FastMCP setup. Without this, rapid reruns hit TIME_WAIT and readiness hangs on last=000000. Windows uses SO_EXCLUSIVEADDRUSE to avoid the hijack semantics of SO_REUSEADDR on that platform. stdio transport is unaffected. - src/adcp/server/responses.py: list_creatives_response now fills missing created_date / updated_date on each dict creative with datetime.now(timezone.utc).isoformat(). Both fields default to the same value when neither is set. Caller-provided values are preserved. Pydantic model items pass through unchanged. Two new tests cover the fill and preserve paths; the existing test_basic is updated to stop asserting raw identity (which is no longer true once timestamps are injected). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…re form After rebasing onto main (which merged PR #213 adding deliver()), our compact-separator fix for get_adcp_signed_headers_for_webhook left deliver() self-inconsistent: it signed compact bytes via the signer but POSTed spaced bytes from an inline json.dumps(body_dict). The test_hmac_auth_signs_posted_bytes invariant on main explicitly verifies that HMAC(posted_bytes) matches the X-AdCP-Signature header — so the inconsistency failed CI. Fix --- - deliver() now serializes body_bytes with separators=(",", ":"), so signer input and transport bytes are byte-identical again. - test_signed_bytes_match_posted_bytes updated to expect compact bytes (was asserting against json.dumps(payload) default — main's test pre-dated adcp#2478's canonical-form pin). - Restore MemoryBackend + WebhookDedupStore re-exports from adcp.webhooks, dropped during the rebase. - Splice sign_legacy_webhook + _compute_legacy_signature back in alongside main's new deliver() — both pathways now share the same compact-separator serialization core. Verification: ruff + mypy clean, pytest 1798 passed / 17 skipped. All main-added tests (A2A comply_test_controller artifacts, test_webhooks_deliver full suite) pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
c146d38 to
92240d5
Compare
…ing example
CI ruff (stricter than the local pre-commit hook's per-file scope) flagged
one docstring example at src/adcp/webhooks.py:338 as 104 chars. Split the
``headers={**signed, ...}`` dict onto its own line — same call, reads
identically, fits in 100.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Pre-4.0 DX sweep driven by a skill-based validation gate: have Claude follow each
build-*skill literally against its primary storyboard, capture friction, fix the cheap findings. This PR lands the fixes in atomic commits so review is per-finding.The new harness (
scripts/skill-build-setup.sh+scripts/skill-run.sh) makes this loop repeatable — one-line rerun of any skill against its storyboard. Verified end-to-end:creative_lifecyclepasses via the harness.Two rounds of expert review + re-validation landed additional fixes (see expanded Commits below). One surprise worth flagging: the original "59 drifted MCP inputSchemas" claim was wrong —
_generate_pydantic_schemas()atserver/mcp_tools.py:915was already auto-generating them at import time. Onlysync_governancewas still on a hand-crafted stub. That's now fixed + drift-guarded.Commits
Round 1 — Skill doc fixes (5 commits, one per skill):
docs(seller)— drops invalidcompliance_testingSupportedProtocol claim, strips invaliddelivery_measurementkeys, fixesproducts_response(proposal=)crash, correctsserve()Quick Ref.docs(generative-seller)— same enum fix, corrects"accepted"→"approved"status, adds requiredbuild_creative/preview_creativehandler section withidempotency_key.docs(retail)— enum fix, validation command.docs(creative)— kills silent fallback anti-pattern (nowadcp_error("CREATIVE_NOT_FOUND")), replaces hardcoded timestamps, adds missingadcp_errorimport.docs(signals)— fixessignal_idsfilter to tuple-key(source, id)discrimination, adds Custom-source bullet, new idempotency subsection.Round 1 — SDK fixes (1 commit):
fix(sdk)— addsSIGNAL_NOT_FOUNDtoSTANDARD_ERROR_CODES, typessync_governanceparam, re-exportsSyncGovernanceRequest/Response, fixesCreativeStatusdocstring, drops unwiredmountkwarg, adds optionalcompliance_testing=kwarg tocapabilities_response().Round 1 — Example + harness (2 commits):
feat(examples): add seller_agent.py reference impl— 409-line runnable seller referenced 11× by other skills.chore(dx): skill-build harness for 4.1 iteration— shared-workspace pattern + two scripts, verified oncreative_lifecycle.Round 2 — Validator + reviewer findings (5 commits):
fix(example): derive port and AGENT_URL from ADCP_PORT env— blocked every harness run in round-1 validation.docs(creative): fix NameError in build_creative fallback— f-string referenced undefinedtarget_format_id.docs(signals,generative): correct idempotency.wrap decorator usage— both skills invented non-existent APIs (store.wrap(key, params, handler),idempotency.get/put).docs(seller): replace false compliance_testing auto-wire with explicit kwarg—serve(test_controller=store)doesn't auto-inject the block; skill now uses the newcapabilities_response(compliance_testing=)kwarg from the SDK fixes.fix(sdk): remove stray noqa, correct capabilities_response docstring— two small cleanups.docs(signals,seller): round-2 validator findings.Round 3 — MCP inputSchema (2 commits):
feat(mcp): auto-generate sync_governance inputSchema from Pydantic— the one tool still on a hand-crafted stub ({accounts: array}). Now mapped toSyncGovernanceRequest; all 57 tools advertise schemas derived from their Pydantic models.test(mcp): drift guard for Pydantic-generated inputSchemas— 4 new tests prevent regression. Pruned ~190 lines of obsoleteKNOWN_GAPSscaffolding.Test plan
ruff check src/ examples/seller_agent.py— cleanmypy src/adcp/— 664 files, no issuespytest tests/ -q— 1641 passed, 11 skipped (post round-3)bash scripts/skill-build-setup.sh && bash scripts/skill-run.sh creative 3014 creative_lifecycle— passingmcp.list_tools()spot-check onget_products,build_creative,create_media_buy— all three advertise correct required fields (buying_mode,idempotency_key,account/brand/start_time/end_time)Filed upstream against adcontextprotocol/adcp
sync_creativesresponse items carrystatusadcp.idempotency.replay_ttl_secondsrequired-when-present creates an ergonomic trapCREATIVE_NOT_FOUND/SIGNAL_NOT_FOUNDenum additions hit an intermittent gh API error; will retry)Out of scope — deferred to 4.1
compliance_testingcapability block whenserve(test_controller=store)is used (closes the auto-wire loop for real; today skills use the explicit kwarg)comply_test_controllerresponse missingresult.artifacts(moves seller-family partials to clean passes)pending_creatives → pending_start,inventory_list_targeting, retailpackages[].catalogs[].typelsof, readiness-loop duration, platform pinComplianceTesting | dictfor thecompliance_testing=kwargsync_governancetyped-param round-tripRelease coordination
Release-please PR #177 (
chore(main): release 4.0.0) is queued. Merging this PR intomainfirst lets release-please regenerate its release PR to include these fixes in the 4.0.0 tag.