fix: serialize push_notification_config with mode='json' at MCP/A2A transport boundaries by torbenbrodt · Pull Request #11 · affinity-com/salesagent

torbenbrodt · 2026-06-18T07:12:33Z

Summary

Fixes prebid#1377 (upstream: prebid#1377)

Pydantic v2's default model_dump() preserves AnyUrl fields as typed AnyUrl objects and enum fields as enum instances rather than converting them to str. SQLAlchemy's String column type does not coerce these objects and raises a StatementError at flush time when push_notification_config.url is persisted.

Root Cause

pnc = PushNotificationConfig(url='https://example.com/webhook')
pnc.model_dump()              # {'url': AnyUrl('https://example.com/webhook')}  ← AnyUrl object
pnc.model_dump(mode='json')   # {'url': 'https://example.com/webhook'}          ← plain str ✓

Fix

Changed .model_dump() → .model_dump(mode="json") at both transport boundaries in src/core/tools/media_buy_create.py:

MCP wrapper (create_media_buy tool)
A2A wrapper (create_media_buy_raw)

The protobuf-based A2A path (adcp_a2a_server.py) uses json_format.MessageToDict() which always produces plain strings and is not affected.

Verification

The regression tests were verified to:

FAIL with the bug (plain model_dump()): AssertionError: url must be a plain str after model_dump(mode='json'), got 'AnyUrl'
PASS with the fix (model_dump(mode="json")): all 5 tests green

Spec Citation

This fix is at the transport boundary layer (Critical Pattern #5 in AGENTS.md). No AdCP protocol behavior is changed — this is a serialization correctness fix ensuring _impl receives plain Python types as the architecture requires.

Test Obligations

New behavioral obligations added to docs/test-obligations/UC-002-create-media-buy.md:

UC-002-TRANSPORT-PNC-SERIALIZATION-01 — MCP wrapper serializes AnyUrl to plain str
UC-002-TRANSPORT-PNC-SERIALIZATION-02 — A2A wrapper serializes AnyUrl to plain str

New test file: tests/unit/test_create_media_buy_behavioral.py with 5 tests covering both obligations.

…ue checks Replace existence-only checks, count-only checks, xfail escape hatches, mock-echo assertions, and tautological checks with production-value assertions across 10 use cases (UC-002 through UC-026). - Strengthen 128/131 weak/missing BDD Then step assertions identified by two-pass inspector (Sonnet triage + Opus deep trace) - Add log capture to CircuitBreakerEnv harness for auth rejection verification - Fix admin retroactive push test mock target (PR prebid#1337 regression) - Scope security audit to project root uv.lock (worktree false positives) - Remove 5 duplicate BDD step-text registrations + add no-shadow guard - Add structural guard for unreachable BDD step modules (ratcheting baseline) - Expose public query methods on IntegrationEnv (replace env._session access) - Move delivery boundary logic out of generic then_payload.py - Standardize ctx.get("response") access with diagnostic helper - Fix silent-skip assertion helpers (raise ValueError on unmapped keys)

The CI security-audit job installs uv via the UV_VERSION env and runs uv-secure, which flags the global uv tool at 0.11.6 (GHSA-4gg8-gxpx-9rph); 0.11.15 is the fix version. Bumping the single UV_VERSION env updates all setup-uv steps in this workflow. Project dependencies (uv.lock) are unaffected and already clean. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…ves (spug)

…bscripts in BDD Then steps (qlsx-a) Extend the bare-subscript diagnostic guard (o15b, response) to ctx['error']. Add generic _require(ctx,key) + _require_error(ctx) in _outcome_helpers and delegate _require_response to _require. Convert the 7 @then steps that read ctx['error'] by subscript. Generalize test_architecture_bdd_no_response_subscript to cover {response, error}; env stays exempt (no-silent-env guard owns it). Entity-key mass conversion (tenant/creatives/package/principal/assignments) split out as a follow-up; long-tail single-use keys are won't-do.

… uc004 (wdge)

…ya0)

…y (vpgh)

…reatives (ftmy)

- Pin ruff==0.15.14 in pyproject.toml (matches pre-commit) - Remove || true from CI ruff steps (lint/format now gate the build) - Add .claude/worktrees to [tool.ruff] exclude - Repo-wide ruff format (26 files) + ruff check --fix (39 auto, 9 StrEnum) - Fix model_dump allowlist line shift from formatting

prebid#1382) * fix(test-harness): implement MediaBuyCreateEnv's referenced setup helpers MediaBuyCreateEnv.setup_media_buy_data() and _configure_mocks() call self.setup_product_chain(...) and self._build_mock_context_manager(...), but neither method was defined — so MediaBuyCreateEnv (and MediaBuyDualEnv, which extends it) raised AttributeError on __enter__. The harness regression test only checked import/subclass/method-presence without entering the env, so the break shipped undetected (the BDD UC-002 path runtime-xfails before touching the env). Add both methods to MediaBuyCreateEnv: setup_product_chain seeds a real PropertyTag/Product/PricingOption via factories; _build_mock_context_manager returns a mock ContextManager delegating create_context/create_workflow_step to the real one so the manual-approval path's ObjectWorkflowMapping FKs resolve. Add an integration smoke test that enters the env, seeds, and drives a real create (the coverage gap that hid the bug), and a unit method-presence check. MediaBuyDualEnv has a separate remaining break — media_buy_dual.py imports make_adapter_update_side_effect from tests.harness._mixins, which is undefined — not addressed here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(deps): bump pyjwt to 2.13.0 for PYSEC-2026-175/177/178/179 pyjwt 2.12.1 has four advisories (PYSEC-2026-175/177/178/179), all fixed in 2.13.0; pip-audit flags them and they are not in the suppression list. Bump the floor to >=2.13.0 and relock (transitive-only change: pyjwt 2.12.1 -> 2.13.0). 2.13.0 rejects empty HMAC signing keys, which broke test_oauth_config.py's id_token_fallback test (it built a fake ID token with key=""). The production decode path (src/admin/auth_utils.py) uses verify_signature=False and is unaffected; the test now signs with a throwaway key that is never verified. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…d-assertions # Conflicts: # src/core/tools/media_buy_delivery.py # tests/unit/test_update_media_buy_behavioral.py

… UCs The --bound-scenarios-from gate was conditional on the flag being passed; since baseline bdd.json's xfail signal turned out to be harness-not-wired (not StepDefinitionNotFoundError), the flag was disabled — which made the WIRED_UCS check inert too, sending every UC's NSM through the LLM merge needlessly. Fix: - UC-level gate is now ALWAYS active. If a UC is not in WIRED_UCS (i.e., no test_<uc>_*.py driver in salesagent), all its NSM scenarios are demoted to TARGET-WINS — no bindings to preserve, render TARGET via the standard NEW-ADD path. - Background-merge branch gets the same treatment: when UC is unwired AND backgrounds differ, take TARGET's background wholesale via the new `use_target_background` flag in the render path. - --bound-scenarios-from remains available as an OPTIONAL secondary gate within WIRED_UCS scenarios, with a comment marking the false-positive risk. Effect on remaining corpus work: drops needed-LLM-merges from 549 to 108 (only UC-011, UC-019, UC-026 need real merges — every other UC is unwired and takes TARGET wholesale).

23 modified + 3 net-new feature files (BR-UC-028/030/032). Sources: - 7 wired UCs LLM-merged + verified-CORRECT (UC-002/003/004/005/006 from prior cost-gates, plus UC-001 and UC-008 merged before the WIRED_UCS gate became always-on) - 19 unwired UCs (UC-009/010/012/013/014/015/016/017/018/020/021/022/023/ 024/025/027/028/030/032) mechanically rendered via TARGET-WINS — no LLM needed since they have no test_<uc>_*.py driver in tests/bdd/, hence no step-def bindings to preserve. Three UCs remain PARTIAL (still need LLM merges, deferred to next session): - UC-011 (54 NSM unresolved) - UC-019 (15 NSM unresolved) - UC-026 (39 NSM unresolved) Their feature files are LEFT AT salesagent HEAD until those merges land. Source: adcp-req @ a14db6e58 with phase5-lockfile/UC-*.yaml. Generated via scripts/phase5/render_features.py (deterministic, no LLM). bdd-traceability.yaml updated with new scenario mappings + pruned entries that disappeared from upstream sources.

The v3.1 BR-RULE-224 scenarios append 'and reach_unit "individuals"' to the media-buy Given step. parsers.parse end-anchors the whole step, so the broader 'with status "{status}"' parser backtracked {status} to capture 'active" and reach_unit "individuals' (41 chars), overflowing the varchar(20) status column with a DataError. Add a dedicated parser that splits status and reach_unit into their own fields. The 8 aggregated_totals.reach scenarios now auto-xfail on the not-yet-wired Then steps instead of crashing.

The generic 'requests delivery metrics with {request_params}' step shadowed the specific status_filter steps, and _parse_request_params only handled the key=value form. The v3.1 Gherkin uses the space-separated quoted form (status_filter "pending_creatives"), which parsed to nothing, so status_filter arrived as None and _impl fell back to the default 'active' filter. 'active' passed by coincidence; the sibling values were masked by strict=False xfails; the new pending_creatives/pending_start values surfaced as hard failures. Extend _parse_request_params to also accept space-separated quoted/bracketed values. With the filter actually reaching _impl, pending_creatives/pending_start (and the previously-masked rejected/canceled) now correctly resolve and return the matching set. Graduate rejected/canceled out of the T-UC-004-filter xfail substring set (they were false xfails); paused/completed remain xfail on a separate reported-status gap.

…es it The delivery response reports a date-derived status that excludes the pending_* states, so a pending_start buy reports 'active'. The adcp MediaBuyDelivery.status enum includes pending_start, so this is a genuine production gap (tracked in salesagent-vtfc), not a test defect. Mark the scenario xfail(strict=True) with a spec-citing reason rather than weaken the assertion.

The resolution-boundary Example 'media_buy_ids provided' fell through _dispatch_resolution (the media_buy_ids branch required 'only'), so the boundary label was passed verbatim as a request field and rejected with extra_forbidden. Translate it to an explicit media_buy_ids request, the same as 'media_buy_ids only'.

…alse-passes Verified the 5 UC-004 a2a xpasses against assertion strength and root cause: - sort_by metric fallback: graduate. A2A no longer drops by_placement; the scenario asserts the breakdown is actually sorted by spend (and inline-xfails if by_placement is absent), so the pass is real. Remove the stale a2a xfail. - webhook credentials partition/boundary (4 invalid rows): NOT graduated. The When step sends an unsupported 'credentials' request field, so every example raises extra_forbidden; the 'invalid' assertion accepts any error, so the rows pass without exercising credential validation. Update the xfail reason to the real cause and track the rework in salesagent-7n4b.

…tus filters The space-separated parsing added to _parse_request_params made the generic 'with {request_params}' step extract values for the specific partition steps it shadows (resolution/principal/date-range/status_filter), passing partition labels as invalid request fields and regressing 49 scenarios with extra_forbidden/list_type. Revert that parser change. Without it, non-active status_filter values are dropped by the generic step and default to active, so the v3.1 pending_creatives/pending_start values fail the same way as their already-xfailed siblings. Xfail them alongside rejected/canceled/paused/completed and describe the real step-shadowing cause. Also drop tracker IDs from xfail reasons. The hcvb resolution-boundary fix is kept (the 'at ... boundary' phrasing is not shadowed by the generic step).

…boundary The credential partition/boundary When steps sent an unsupported 'credentials' field on the delivery request, so the adcp library's extra_forbidden fired first and shadowed the domain rule — invalid rows 'passed' on the wrong error and were masked by transport-specific strict=False xfails. Drive the credential through the real shape instead: build it into a reporting_webhook Authentication on a minimal CreateMediaBuyRequest, which is where production validates it (scheme enum + credentials min_length=32, BR-RULE-029). Valid configs are accepted; invalid ones raise a ValidationError located on the credentials/scheme (the When asserts the error is credential- specific, not just any error). All partition/boundary credential rows now pass on all four transports; remove the a2a-specific xfail shims. Restores the salesagent-f8u4 reconciliation that a later merge had reverted.

The 3.1 feature render added a 'natural_key_sandbox' partition Example, but the account-config Given had no branch for it, so it hit the catch-all ValueError. Production already resolves sandbox natural-key references (Account.sandbox column, AccountRepository.get_by_natural_key(sandbox=...)); add a Given branch that seeds an active sandbox account and builds a natural-key reference with sandbox=true, mirroring the natural_key_unambiguous branch.

The 3.1 render added a 'brand + operator + sandbox:true' boundary Example whose config ('brand+op+sandbox active') matched no branch in the boundary Given, hitting the catch-all ValueError. Add a branch that seeds an active sandbox account and builds a natural-key reference with sandbox=true, mirroring the 'brand+op single match' branch. Production resolves it; both UC-002 sandbox account-resolution scenarios now pass.

Joins two pytest-json-report bdd.json runs by exact nodeid and reports the outcome-transition matrix (passed->xfailed regressions, xfailed->passed improvements), added/removed scenarios, and a per-use-case breakdown. Used to verify the 3.1 feature-file migration against the pre-3.1 baseline.

Completes the 3.1 feature-file migration: the three partial use cases that were left on pre-3.1 Gherkin (UC-011 manage accounts, UC-019 query media buys, UC-026 package media buy) are now rendered from the adcp-req v3.1 lockfile in merge mode, preserving existing step wording where it matched. Regenerates the companion bdd-traceability mapping. Collection verified clean (2259 scenarios, no parse errors).

- ruff format scripts/compile_bdd.py (rendered tooling left it unformatted) - aiohttp >=3.14.0 resolves CVE-2026-34993 and CVE-2026-47265 (pip-audit)

Resolves the remaining pip-audit finding. authlib imports clean and the auth/oauth unit suite passes (536 passed). pip-audit now reports no known vulnerabilities.

…prebid#1385) aiohttp 3.13.5 (CVE-2026-34993, fixed in 3.14.0) and authlib 1.6.11 (PYSEC-2026-188, fixed in 1.6.12) are flagged by pip-audit and are not in the suppression list. Bump the floors to >=3.14.0 and >=1.6.12 and relock: aiohttp 3.13.5 -> 3.14.0, authlib 1.6.11 -> 1.7.2 (latest, which adds joserfc 1.7.0 as authlib's JOSE backend). make quality passes, including the authlib-sensitive oauth unit tests; a direct pip-audit of the synced environment reports no known vulnerabilities.

…ith 2 updates (prebid#1343) Bumps the python-dev group with 2 updates in the / directory: [playwright](https://github.com/microsoft/playwright-python) and [allure-pytest](https://github.com/allure-framework/allure-python). Updates `playwright` from 1.48.0 to 1.60.0 - [Release notes](https://github.com/microsoft/playwright-python/releases) - [Commits](microsoft/playwright-python@v1.48.0...v1.60.0) Updates `allure-pytest` from 2.13.5 to 2.16.0 - [Release notes](https://github.com/allure-framework/allure-python/releases) - [Commits](allure-framework/allure-python@2.13.5...2.16.0) --- updated-dependencies: - dependency-name: allure-pytest dependency-version: 2.16.0 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: python-dev - dependency-name: playwright dependency-version: 1.60.0 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: python-dev ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

…tion # Conflicts: # pyproject.toml

…d#1393)

…id#1395) TestCreativeApprovalRetroactivePush patched push_creative_to_existing_buy at its definition module (src.core.tools.media_buy_create), but creatives.py binds the name via a "from ... import" and calls it locally, so the mock never intercepted and the active/scheduled/paused _triggers_retroactive_push assertions saw 0 calls and failed. Retarget the patch to the use site (src.admin.blueprints.creatives.push_creative_to_existing_buy). No production change; the handler was already correct. Refs prebid#1394 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…rebid#1307) * refactor: error-drain Pattern A + boundary ValueErrors (PR 2 of 3) Drain Pattern A `Error(code=...)` construction in src/core/tools/ and src/adapters/ to typed AdCPError raises. The structural guard's per-file cap is now empty; the few legitimate per-item advisory Error() sites in success envelopes carry an inline `# structural-guard:` marker that the guard skips. - Migrate boundary-facing ValueErrors to typed AdCPError; internal programmer-contract ValueErrors are kept (boundary-vs-internal rule). - media_buy_update persists failed workflow steps via ctx_manager.audit_step_failure (wire envelope and persisted response_data are byte-identical by construction). - Budget validation raises spec-specific codes: AdCPBudgetTooLowError (BUDGET_TOO_LOW) for minimum-spend shortfalls and AdCPBudgetExceededError (BUDGET_EXCEEDED) for daily-spend ceilings, consistent across media_buy_create and media_buy_update. - Add AdCPBudgetExceededError, AdCPCreativeRejectedError, AdCPProductUnavailableError to core/exceptions.py. - list_creative_formats raises AdCPServiceUnavailableError on registry-init failure (operation-level), per updated FD-ERR-03. - check-import-usage hook: recognize function parameters as locally-defined names, so a parameter used as a callable is not a false missing-import. - Migrate affected unit and integration tests to raise-shape and two-layer wire-envelope assertions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: complete adapter boundary ValueError migration + error-code wire tests Convert the adapter __init__ configuration guards to AdCPConfigurationError. A misconfigured adapter now raises a typed config error whose wire code is SERVICE_UNAVAILABLE (CONFIGURATION_ERROR is mapped to SERVICE_UNAVAILABLE at the boundary), instead of the synthetic VALIDATION_ERROR an untyped ValueError produced. Sites: - kevel, triton_digital: missing advertiser ID / missing API credentials - broadstreet: missing advertiser ID / missing network credentials - google_ad_manager: missing network_code, non-numeric advertiser_id, missing service-account/refresh-token credentials Internal programmer-contract ValueErrors (the GAM manager-delegation guards and broadstreet _extract_campaign_id) are kept per the boundary-vs-internal rule. Lower the ValueError caps to match (broadstreet 3->1, google_ad_manager 7->4) and remove the drained kevel/triton entries. Add wire-envelope coverage for the error codes this PR introduces: - REST: BUDGET_EXCEEDED, CREATIVE_REJECTED, PRODUCT_UNAVAILABLE via the exception-handler parametrization in test_error_paths. - A2A: update_media_buy campaign-budget (BUDGET_EXCEEDED) and package-minimum (BUDGET_TOO_LOW) guardrails driven through on_message_send and asserted via assert_envelope_shape, replacing the mock-based wrapper unit tests. Persistence moves to a shared create_active_media_buy helper. - MCP: assert get_media_buy_delivery's invalid-date envelope via assert_envelope_shape. Replace the explanatory `# noqa:` substring in the Pattern A guard comment that ruff parsed as a malformed directive. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: hoist non-load-bearing lazy imports + type broadstreet media_buy_id guard Hoist module-level-safe lazy imports to module scope: - resolve_identity_from_context across accounts, signals, products, media_buy_update, creative_formats (transport-boundary identity helper with no circular-import or test-patch coupling). - asyncio, concurrent.futures in creative_formats (stdlib). Adapter data-access lazy imports (get_db_session, repository classes) and the admin/service/AI imports are deliberately kept lazy: they are load-bearing for the source-module test-patch strategy (tests patch e.g. repositories.uow.ProductUoW at its source, which requires lazy resolution in the consumer), avoid core->admin import cycles, or defer heavy optional dependencies. Migrate broadstreet _extract_campaign_id's empty-id guard from ValueError to AdCPValidationError and drop the now-empty broadstreet ValueError cap entry. Regenerate the model_dump KNOWN_VIOLATIONS line numbers shifted by the hoist. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(errors): add typed adapter-taxonomy + product-not-found exceptions Add AdCPProductNotFoundError, AdCPWorkflowError, AdCPLineItemError, and AdCPBulkUpdateError so raise sites carry the failure taxonomy on the typed class instead of via error_code= kwargs or details["internal_code"]. - AdCPProductNotFoundError(AdCPNotFoundError): PRODUCT_NOT_FOUND, 404, correctable — mirrors the AdCPMediaBuyNotFoundError/AdCPPackageNotFoundError siblings (buyer corrects via get_products). PRODUCT_NOT_FOUND is a standard SDK code, so it passes through untranslated. - AdCPWorkflowError / AdCPLineItemError / AdCPBulkUpdateError(AdCPAdapterError): carry WORKFLOW_CREATION_FAILED / LINE_ITEM_CREATION_FAILED / PARTIAL_FAILURE as the class identity; wire_error_code translates each to SERVICE_UNAVAILABLE via ERROR_CODE_MAPPING. AdCPBulkUpdateError gives the partial-failure event a single status (502) so REST clients don't fork by adapter. Register LINE_ITEM_CREATION_FAILED in ERROR_CODE_MAPPING and PARTIAL_FAILURE in INTERNAL_CODES; add the four codes to the test canonical vocabulary. The auto-derived _ERROR_CODE_TO_STATUS table picks the subclasses up automatically. Rewrite two pre-existing module/test sanity-check asserts in the touched files to single-statement forms so black (pre-commit) and ruff format (make quality) stop disagreeing on the multi-line wrap. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(errors): extract AdCPError.iter_concrete_subclasses() classmethod The wire-code->HTTP-status table builder (_build_error_code_to_status) and two error-code compliance test helpers each walked AdCPError.__subclasses__() with near-identical bodies. Promote the walk to one public classmethod on AdCPError; the builder and both tests now consume it. The classmethod yields deduplicated transitive descendants and excludes the root, matching every prior walk's semantics, so the derived status table and the compliance mutation guards are unchanged. Also converges four pre-existing >120-char asserts in test_error_code_status_table.py to single-statement form (extract the table lookup to a local) so pre-commit black and ruff format agree on them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(adapters): centralize config-presence guards in _require_config Five adapters hand-wrote `if not self.X: raise AdCPConfigurationError(...)` in their __init__. Add AdServerAdapter._require_config(value, *, field, message=None) -> value, which raises AdCPConfigurationError with the missing field recorded on the error and returns the value with None stripped from its type (so callers rebind to narrow the attribute for downstream use). Migrate the seven single-value presence guards (kevel advertiser_id + network_id/api_key, triton advertiser_id + auth_token, broadstreet advertiser_id + network_id/api_key, GAM network_code) to the helper and prune the now-orphaned AdCPConfigurationError imports from kevel/triton/broadstreet. Standardize the operator-facing wording on "config is missing '<field>'" — GAM previously said "requires", which forked dashboard substring filters. The GAM any-of-credentials check keeps its inline form (it requires at least one of three fields, not a single value) but adopts the same "is missing" wording; the GAM advertiser_id numeric-format validation is left untouched — it validates a present value's shape, not presence, so it is a distinct operation rather than a config-presence guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(auth): add resolve_principal_or_raise, collapse 3 identical guards The create, update, and delivery media-buy tools each open-coded the same "look up the principal, raise AdCPAuthenticationError if absent" guard. Add resolve_principal_or_raise(principal_id, *, tenant_id, context) in src/core/auth.py next to get_principal_object and call it from all three _impl functions. Because the lookup now resolves get_principal_object in the auth module's namespace (via the helper) rather than each tool module's, the tests and harnesses that patched src.core.tools.media_buy_{create,update,delivery}.get_principal_object to control the create/update/delivery principal lookups are migrated to patch src.core.auth.get_principal_object (the conftest standard_mocks fixture, the MediaBuyUpdateEnv and DeliveryPollEnvUnit harnesses, and the affected unit and integration tests). Patches targeting other modules (products, capabilities, performance, media_buy_list) and the push_creative_to_existing_buy / execute_approved_media_buy paths are unchanged — those call sites are untouched. Re-derives the line-keyed model_dump allowlist in test_architecture_no_model_dump_in_impl.py: collapsing the update.py principal guard from three lines to one shifted the seven success-path serialization sites up by two lines. Converts a handful of pre-existing >120-char asserts and single-context-manager with-blocks in the touched test files to single-statement form so pre-commit black and ruff format agree on them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(tools): promote raise_if_validation_failed to financial_validation The budget "raise the spec-specific subclass when the validator returned a message" guard was a private helper in media_buy_create.py used four times, and was open-coded three times in media_buy_update.py. Move it to src/core/tools/financial_validation.py (next to the validate_* functions it pairs with) as raise_if_validation_failed and import it in both paths: - media_buy_create.py: drop the local definition, import the shared helper. - media_buy_update.py: replace the three open-coded "if <error>: raise AdCPBudget*Error(<error>, context=req.context)" guards with raise_if_validation_failed(<error>, AdCPBudget*Error, context=...). Re-derives the line-keyed model_dump allowlist: expanding the two package guards in update.py to the helper call shifted the success-path serialization sites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(context): unify workflow-step-audit method names under one prefix The three workflow-step-audit operations on ContextManager used two verbs (fail/audit) and inconsistent names, so grepping "audit_step_failure" did not surface the primitive. Rename to a uniform prefix: - fail_workflow_step_for_exception -> audit_workflow_step_failure - audit_step_failure_if_present -> audit_workflow_step_failure_if_present - audit_step_failure (contextmanager) -> audit_workflow_step_failure_ctx Updates every caller (media_buy_create, media_buy_update), the cross-reference docstrings, the wire-code comments in exceptions.py, and the tests that call or assert on these methods. Also converges the pre-existing >120-char asserts in the touched test files to single-statement form (message extracted to a local) so pre-commit black and ruff format agree on them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(context): move workflow-step result serialization into ContextManager _update_media_buy_impl open-coded workflow-step result persistence as update_workflow_step(..., response_data=<obj>.model_dump(mode="json")) at seven sites, calling model_dump in the _impl layer. Add ContextManager.audit_workflow_step_result(step_id, response_obj, *, status="completed", error_message=None, add_comment=None, request_obj=None), which owns the serialization, and route all seven sites through it: - two success results (status="completed") - three adapter-returned error results (status="failed", error_message) - the manual-approval step (status="requires_approval", add_comment, and request_obj=req so the originating request is serialized under request_data) Drains the seven media_buy_update entries from the no-model_dump-in-_impl allowlist; the two remaining entries are unrelated (products logging and creative-list filter conversion). Updates the tests that asserted on the ctx_manager.update_workflow_step mock for these paths to assert on audit_workflow_step_result. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(integration): drop redundant get_principal_object patches in creative-assignment The five update-media-buy creative-assignment tests patched src.core.auth.get_principal_object to return a Principal ORM instance. That patch was a no-op before resolve_principal_or_raise existed (update.py looked up get_principal_object in its own module namespace), so the tests actually resolved the principal via the real DB lookup. Now that the lookup routes through resolve_principal_or_raise -> src.core.auth.get_principal_object, the patch is live and returns a session-detached ORM instance, raising DetachedInstanceError when downstream code touches it. The principal is already committed to the test DB in setup, so remove the patches and let the real get_principal_object run (returning a schemas.Principal, as in production). Fixes the Integration (media-buy / creative) CI failures. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(a2a): collapse error path to one mechanism; envelope-in-data for push-notif RPC Remove the _a2a_skill decorator. Its broad `except Exception` pre-empted the dispatcher's typed normalization, so a raw ValueError/PermissionError from a skill handler emitted SERVICE_UNAVAILABLE (503) instead of the VALIDATION_ERROR (400) / AUTH_REQUIRED (403) that the MCP and REST boundaries produce. With the decorator gone, ValueError/PermissionError reach _handle_explicit_skill's normalize_to_adcp_error (wire parity across transports) and untyped exceptions fall through to on_message_send's catch-all (same envelope shape). skill_handlers is annotated dict[str, Callable[..., Awaitable[Any]]] — the decorator's *args/**kwargs had masked the heterogeneous handler signatures. - Drop the flat `error` key from _build_failed_skill_result; the two-layer error_envelope is the single wire source of truth. The webhook message extraction and the boundary-translation tests read errors[0].message from the envelope. - Replace the now-unreachable flat-shape `else` in the artifact loop with a raise, so a result marked failed without an error_envelope fails loud rather than silently emitting the legacy shape. - _serialize_context logs and returns None on malformed input instead of raising TypeError; it runs inside the boundary translator, where a raise would shadow the original exception and fail open with no envelope. - Push-notification-config RPC failures keep their JSON-RPC InternalError (an A2AError the SDK serializes structurally) but carry the two-layer envelope in the error `data` field, so buyers read error.data.errors[0].code. Raising a non-A2AError there would be flattened by the SDK dispatcher to a bare InternalError(str(e)) with no envelope. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(errors): replace error_code= kwargs and details[internal_code] with typed subclasses Every error-code override now flows through a typed AdCPError subclass whose class identity carries the wire code, instead of error_code= kwargs (which bypass synthesize() and can leak unmapped codes) or details["internal_code"] (which is buyer-visible and stringly-typed). Taxonomy (src/core/exceptions.py): - New subclasses: AdCPActivationWorkflowError (ACTIVATION_WORKFLOW_FAILED), AdCPGamUpdateError (GAM_UPDATE_FAILED), AdCPMediaBuyRejectedError (MEDIA_BUY_REJECTED), AdCPInventoryUnavailableError (INVENTORY_UNAVAILABLE). - ERROR_CODE_MAPPING gains entries so every internal code translates to a standard wire code (workflow/update codes -> SERVICE_UNAVAILABLE; MEDIA_BUY_REJECTED -> POLICY_VIOLATION; INVENTORY_UNAVAILABLE -> PRODUCT_UNAVAILABLE). - AdCPConfigurationError drops its recovery override: it inherits terminal (the buyer has no lever to fix server config), matching its docstring. Call sites: - media_buy_create: product-not-found raises AdCPProductNotFoundError (404); three redundant error_code="INVALID_REQUEST" kwargs dropped. - media_buy_update: persistent-context failure raises AdCPWorkflowError. - mock_ad_server: rejection/inventory/creative failures raise the typed subclasses. - google_ad_manager: workflow/line-item/activation/update failures raise the typed subclasses; advertiser_id numeric-format check chains via `from e`. - google_ad_manager + broadstreet: bulk partial-failure unified on AdCPBulkUpdateError so REST clients see one status (502) regardless of adapter. Tests updated to assert the typed exception + error_code; the canonical error-code vocabulary gains the four new codes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(logging): use %-placeholder logger calls instead of f-strings f-string logger arguments are formatted eagerly even when the log level filters the record out. Convert the f-string logger calls in the A2A server (14 sites) and signals tool (1 site) to %-style placeholders so the interpolation is deferred to the logging framework. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(media-buy-create): hoist non-load-bearing imports; type INVALID_REQUEST validators A6 — hoist the function-local imports in _create_media_buy_impl that have no circular-import or test-patch rationale (stdlib Decimal/secrets/select/selectinload, leaf DB models, GAMProductConfigService) to module top, deduplicating the copies across branches. Three imports stay function-local with a one-line reason: MediaBuyUoW, get_product_catalog, and validate_creative_format_against_product are each patched by tests at their source module, so the call-time import must bind the patched object (a module-top import would bind the unpatched one at load). Also restore the INVALID_REQUEST wire code for the three semantic-validation sites (start_time-in-past, end_time-before-start, targeting). These previously used AdCPValidationError(error_code="INVALID_REQUEST"); dropping the kwarg would have emitted VALIDATION_ERROR (the class default), changing the buyer-facing wire code. Introduce AdCPInvalidRequestError(AdCPValidationError) carrying INVALID_REQUEST as class identity (inherits 400 + correctable) and raise it at those sites — preserving the wire contract without the error_code= bypass. Add INVALID_REQUEST to the canonical error-code vocabulary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(guards): drain model_dump allowlist to empty; add no-error_code-kwarg guard A7 — the model_dump-in-_impl allowlist is now empty. _get_products_impl logs req.filters directly (no model_dump), and _list_creatives_impl's internal filter merge moves into a module-level _merge_structured_filters helper (request normalization, not the wire serialization the guard targets). A1 ratchet — add test_architecture_no_error_code_kwarg_in_impl.py: AST-scan of src/core and src/adapters forbidding error_code= on AdCP*Error constructors or .synthesize(), allowlisting exactly the two sanctioned synthesize() callers (context_manager.audit_workflow_step_failure, tool_error_logging.handle_tool_error), keyed by enclosing function name so the allowlist survives line shifts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: pin adapter error codes, delete tautological test, parametrize status-map, byte-compare context Phase 5 unit-test improvements: - T1: broadstreet adapter tests assert exc_info.value.error_code (VALIDATION_ERROR / PACKAGE_NOT_FOUND / UNSUPPORTED_FEATURE) so a swap to a parent class that flips the wire code is caught. - T2: delete the tautological test_creative_upload_failure_wraps_exception_as_tool_error (it raised and asserted on a message it built inline; production was never imported). The sibling test_creative_upload_failure_raises_adapter_error covers the real path. - T10: collapse the five near-identical TestHandleToolErrorPreservesStatusCode methods into one @pytest.mark.parametrize over (source_cls, message, expected_status), with a _synthetic_tool_error() helper. A new typed subclass is one row, not one method. - T8: test_three_paths_emit_consistent_context compares json.dumps(sort_keys=True) bytes (matching TestWireBytesIdenticalAcrossTransports) instead of dict equality. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: pin error_code on _impl error-path assertions (T3) Add `assert exc_info.value.error_code == "<wire code>"` to the _impl-level pytest.raises(AdCP*Error) blocks in the media-buy create/update behavioral suites that previously asserted only the exception type (or only a message substring). The typed-subclass migration locked wire codes to class identity; without the pin, a mutation swapping a typed subclass for its parent passes silently while the buyer-facing wire code changes. Codes were verified by running, not assumed — every pin matched the production raise. Two sites catch the base AdCPNotFoundError (wire NOT_FOUND) where the specific taxonomy lives only in details, so the pin captures the wire code distinctly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: pin negative-budget validation to typed error + field path, not Pydantic text (T7) test_negative_budget_raises_tool_error caught (ToolError, AdCPValidationError) and asserted the Pydantic-internal string "greater than or equal to 0", which a Pydantic minor bump can break. create_media_buy_raw converts the Pydantic ValidationError to AdCPValidationError (the ToolError arm was dead), so narrow to pytest.raises( AdCPValidationError) and assert error_code == VALIDATION_ERROR + the "budget" field path. Also converge two pre-existing assert-with-message lines to the extracted-local form so black and ruff-format agree. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: migrate error-emission fixtures to harness factories; scan tests/helpers (T5) Replace the three raw-session DB-seed helpers in tests/helpers/adcp_factories.py (setup_error_test_tenant_chain, create_active_media_buy, make_real_tenant_identity) with factory-boy setup. The three consuming fixtures (test_typed_error_raises, test_mcp_error_envelope, test_a2a_error_responses) now open an IntegrationEnv and seed via one shared conftest helper, seed_error_test_tenant, which composes TenantFactory/ProductFactory/PricingOptionFactory/PrincipalFactory and reuses add_required_setup_data for the setup-checklist scaffolding so validate_setup_complete passes. PricingOptionFactory defaults (cpm/USD/fixed) derive the synthetic cpm_usd_fixed pricing option id the budget pins reference. active_media_buy uses MediaBuyFactory. Extend the repository-pattern guard's discovery to scan tests/helpers/**/*.py so session.add() in shared helper modules is caught at the source; the allowlist stays empty (both former session.add sites are deleted). Drop the now-zero adcp_factories.py entry from the inline-ResolvedIdentity per-file cap. Converge three pre-existing asserts in test_mcp_error_envelope.py to the "msg = ...; assert cond, msg" form so black and ruff-format agree on the touched file. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: consolidate create_media_buy error-path tests onto the A2A wire (T6) Replace the five TestCreateMediaBuyErrorPaths tests in test_error_paths.py — which called create_media_buy_raw + pytest.raises and asserted on the reconstructed exception below the transport boundary — with single-source wire coverage: - end_time-before-start (INVALID_REQUEST) and negative-budget (VALIDATION_ERROR, Pydantic ge=0) were unique conditions; they move to test_a2a_error_responses.py as on_message_send + assert_envelope_shape DataPart tests. - missing-principal (AUTH_TOKEN_INVALID), past-start (INVALID_REQUEST) and empty-packages/budget=0 (BUDGET_TOO_LOW) were already covered on the wire by test_a2a_error_responses.py and test_mcp_error_envelope.py — verified before deleting. Delete the now-dead TestCreateMediaBuyErrorPaths class and its two raw-session fixtures (test_tenant_minimal, test_tenant_with_principal); drop both from the repository-pattern session.add allowlist and prune the imports the class owned. The deleted tests drove the A2A raw shim directly, missing the on_message_send -> _adcp_to_a2a_error -> DataPart translation the new wire tests exercise. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: migrate media-buy behavioral suites to harness envs (T4) Migrate tests/unit/test_update_media_buy_behavioral.py (84 tests) from the hand-rolled `standard_mocks` conftest fixture to the MediaBuyUpdateEnv harness (unit, mocked DB), and test_create_media_buy_behavioral.py (73 tests) from its in-file `_PatchContext`/`_standard_patches` mocks to the MediaBuyCreateEnv harness (real Postgres via factories). All test names, classes, and the 33+23 error_code wire-code pins are preserved; assertions are equivalent or stronger (soft try/except-pass success proxies become concrete isinstance(result.response, CreateMediaBuySuccess); a mock add.assert_not_called becomes a real MediaBuy row-count==0 query; internal mock pins move to the real adapter boundary). test_create now uses a real DB, so it moves to tests/integration/ with @pytest.mark.requires_db. MediaBuyCreateEnv references two helpers that were never implemented (setup_product_chain, _build_mock_context_manager, dangling since the behavioral-foundation scaffolding); they are supplied on a local _MediaBuyCreateEnv subclass as a bridge (the env crashes on __enter__ without them) until they are moved into the harness, which also fixes MediaBuyDualEnv's same dangling reference. Remove the now-dead `standard_mocks` fixture (test_update was its sole consumer). Move the inline-ResolvedIdentity per-file cap entry to the new integration path and lower it 4->3. Mock construction in test_create drops 216->39; tests/ duplication baseline 100->99. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(guards): ratchet hand-rolled mock constructions in behavioral suites (T4) Add test_architecture_behavioral_mock_cap: a per-file cap on MagicMock/Mock/ AsyncMock/patch constructions in test_*_behavioral.py files, baselined at current counts and ratcheting down (same convention as the inline-ResolvedIdentity cap and .duplication-baseline). Behavioral suites should drive production through the tests/harness/ envs (EXTERNAL_PATCHES + fluent setup helpers); this locks in the media-buy migration's reduction (test_create 216->39) and prevents regression, steering remaining mocks (e.g. test_update's _make_mock_* helpers) toward env.set_media_buy/set_currency_limit. New behavioral files with mock constructions fail until capped or migrated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(auth): extract require_principal_id + require_tenant _impl guards (gh-1307) Collapse two open-coded _impl prologue guards into single canonical helpers in src/core/auth.py, beside resolve_principal_or_raise: require_principal_id(identity, *, context=None) -> str require_tenant(identity, *, context=None) -> dict Both raise AdCPAuthenticationError with one canonical, actionable message. The principal guard had drifted into 3 phrasings across 4 sites; the tenant guard into 2 phrasings across 13 sites (the catalogue listed 10 — 3 more had drifted in since), exactly the DRY defect the project guide calls a correctness requirement. Migrated sites: principal_id: media_buy_create, media_buy_delivery, media_buy_update (_verify_principal), performance tenant: signals(x2), media_buy_create, media_buy_delivery, media_buy_update(x2), performance, task_management(x3), creative_formats, creatives/_sync, creatives/listing Also removes a dead unreachable principal_id re-check in _verify_principal and drops now-unused AdCPAuthenticationError imports. The canonical principal message keeps the "Authentication required" + x-adcp-auth hint so existing actionability/identifiability contracts hold with zero edits to their tests. Updated test_tenant_context_ordering's source-scan to recognise the helper while preserving its auth-before-tenant ordering assertion. Tests: tests/unit/test_auth_helpers.py adds TestRequirePrincipalId (5 cases) and TestRequireTenant (4 cases). Quality gate: 4722 passed, 0 failed. Integration (auth/tenant/identity + all migrated tool areas): 990 passed, 0 failed. * fix(core): align media-buy principal/context error codes across tools update_performance_index: a token resolving to a missing principal now raises AdCPAuthenticationError (AUTH_TOKEN_INVALID) via resolve_principal_or_raise, matching create/update/delivery, rather than AdCPNotFoundError (404). update_media_buy: collapse the open-coded principal_id guard to require_principal_id (same AUTH_TOKEN_INVALID wire code, canonical message). A missing buyer-supplied context_id now raises AdCPNotFoundError instead of AdCPWorkflowError: get_or_create_context returns None only when the referenced context_id is absent (a not-found condition), not a transient adapter outage. financial_validation: move ContextObject under TYPE_CHECKING (annotation only). Tests: performance principal-not-found expects AdCPAuthenticationError; new update unknown-context_id NOT_FOUND test; update behavioral no-principal test expects the canonical AdCPAuthenticationError; tenant-ordering source guard accepts the require_principal_id marker (parity with create). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(adapters): typed configuration errors + config-value narrowing google_ad_manager: add field= to the three hand-written AdCPConfigurationError raises (advertiser_id numeric check, missing auth credential, missing advertiser_id/trafficker_id) so the wire envelope carries the offending field. Convert the four runtime manager-presence guards (creatives_manager x3, orders_manager) from raise ValueError to AdCPConfigurationError — they sit on the buyer-operation path, where the boundary translator turned ValueError into a synthetic VALIDATION_ERROR (400, bad input) rather than CONFIGURATION_ERROR (500). Drains the file's four-entry ValueError allowlist cap, the documented PR-2 ratchet drain. kevel/triton_digital: rebind self.X = self._require_config(...) so the narrowed non-None return is captured (network_id, api_key, auth_token stay str). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: pin typed-exception wire codes + shared-helper unit coverage T1: add a parametrized internal-code -> wire-code mapping test for the six adapter/mock taxonomy classes (LineItem/BulkUpdate/ActivationWorkflow/GamUpdate -> SERVICE_UNAVAILABLE; MediaBuyRejected -> POLICY_VIOLATION; InventoryUnavailable -> PRODUCT_UNAVAILABLE), and exercise the GAM create-path AdCPLineItemError raise site so a class-swap at the site is caught, not just the class definition. T2: dedicated unit tests for resolve_principal_or_raise (auth), _require_config (adapter base), and raise_if_validation_failed (financial_validation) — the shared error-emission helpers had no direct coverage. T7: assert recovery == 'terminal' in the A2A PermissionError test (its docstring already claimed it; only the sibling ValueError test asserted recovery). T8: contract test for AdCPError.iter_concrete_subclasses() — transitivity, diamond dedup, self-exclusion — exercised on a throwaway hierarchy so it cannot pollute the real subclass tree other tests enumerate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(adapters): standardize bulk-update failure details across adapters AdCPBulkUpdateError.details forked by adapter: google_ad_manager emitted {"failed_packages": [{package_id, reason|line_item_id}]} (inconsistent even within itself) while broadstreet emitted {"failed_advertisement_ids": [<id>]} (bare strings). Both now emit {"failed_items": [{"id", "reason"}]} so a client parsing the partial-failure detail does not fork by adapter. No production readers and no tests pinned either shape. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(notes): track deferred-PR reconciliations; drop two finished handoffs Add deferred-prs-1312-1260.md (verified-facts notes for the deferred #1312 idempotency-replay and #1260 version-compat merge reconciliations). Remove merge-main-handoff.md (one-off May-3 merge of a since-merged branch) and bdd-audit-handoff.md (Apr-1 session summary superseded by later BDD work). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(a2a): structured field on param-validation envelopes + observability/message parity T5: propagate the offending field path from the Pydantic error onto the param-validation wire envelope. _handle_create_media_buy_skill and the update_performance_index skill pass field=first_validation_error_field(e) to AdCPValidationError, so buyers get a structured field (e.g. packages.0.budget) instead of only the rendered message substring. New first_validation_error_field helper in validation_helpers.py; the negative-budget wire test asserts the field. A6: route the untyped A2A skill fallthrough through record_boundary_error (ERROR + exc_info + activity-feed + audit) so untyped A2A failures land on the same observability surface as MCP/REST and the typed path, rather than a bare logger.error. The typed failures were already recorded in _handle_explicit_skill. A7: unify the two A2A internal-error message prefixes on '{operation} failed:' so log-aggregation keying on the prefix does not fork between the helpers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(a2a): consolidate A2A InternalError builders + close proactive review gaps Proactive sweep against the accumulated review-pattern catalog (no reviewer blockers; tightening before the next round). A2A: _internal_error_for now carries the two-layer envelope in the error data field, so the on_message_send fallthrough and NL-handler JSON-RPC errors reach buyers with a parseable envelope (parity with the push-notif path). The now-identical _push_notification_config_error is folded into it and its four protocol-handler callers repointed (single source of truth). signals: drop the dead 'if not identity' guard left after the require_tenant + assert refactor (unreachable, and it raised the wrong error type). media_buy_list: the targeting-rehydration advisory uses the standard SERVICE_UNAVAILABLE wire code (matching the sibling per-creative advisory) instead of the INTERNAL_CODES-only INTERNAL_ERROR that would reach the buyer verbatim; comment corrected. broadstreet: rebind self.advertiser_id = self._require_config(...) for type-narrowing consistency with network_id / api_key. Tests: add AdCPWorkflowError to the taxonomy wire-code mapping test (the class it omitted); add a regression test pinning that a malformed error context is dropped, not raised (boundary fails open); update the rehydration-advisory test to the new wire code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(creatives): surface server-misconfig sync failures as terminal, not transient A missing GEMINI_API_KEY raises AdCPConfigurationError during generative creative sync, but a pre-existing except Exception in the validation path swallowed it into a 'retry recommended' (transient) advisory — a hard server-side misconfiguration reported to buyers as retryable. Catch AdCPConfigurationError ahead of the broad except in both _update_existing_creative and _create_new_creative and return the honest message. _failed_sync_result gains a recovery kwarg: config failures carry recovery='terminal', the genuine creative-agent-down path carries recovery='transient'. The wire code stays the standard SERVICE_UNAVAILABLE (CONFIGURATION_ERROR is internal-only and would leak verbatim in a success-envelope advisory); recovery is the structured retry signal. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(delivery): return advisory error on per-buy adapter failure (UC-004-EXT-F) get_media_buy_delivery aggregates multiple media buys; a single adapter failure must degrade — return the other buys' data plus an advisory Error in errors[] — rather than aborting the whole request. Reverts the per-buy adapter path from a hard 'raise AdCPAdapterError' to an allowlisted advisory Error(), matching the UC-004-EXT-F obligation (the system returns error adapter_error) and the harness delivery-poll contract. Realigns the UC-004-EXT-F obligation tests in test_delivery.py, test_media_buy.py, and test_delivery_poll_behavioral.py that had been changed to assert a raise. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: provision DB + fix mock patch target surfaced by authoritative CI suite Two test-infra gaps that ./run_all_tests.sh ci surfaces but make quality / tox -e integration miss (they skip tests/admin and run against a persistent DB that already has the schema): - test_create_media_buy_behavioral.py: the two format-validation tests moved from unit to integration exercise a registry path that queries creative_agents; add the integration_db fixture so a fresh DB provisions the table. - test_creatives_blueprint.py: patch push_creative_to_existing_buy at the blueprint's binding (where it's used), not the source module, so the mock actually intercepts the retroactive-push call. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(a2a): update_media_buy skill raises typed AdCPValidationError, not JSON-RPC error The update_media_buy skill handler raised JSON-RPC InvalidParamsError for missing/invalid params, bypassing the dispatcher's AdCPError normalization and emitting a flat error with no wire code — diverging from create_media_buy and update_performance_index, which raise AdCPValidationError (with field/suggestion) so the boundary builds the two-layer envelope. Aligns update_media_buy to that contract, adds a wire test asserting the missing-media_buy_id case surfaces VALIDATION_ERROR via on_message_send, and realigns the unit param-mapping test from A2AError to AdCPValidationError. Also lowercases the one outlier _internal_error_for('Message processing') prefix to match the verb-noun convention of the other call sites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(auth): consolidate identity/principal/tenant guards via require_identity Completes the auth-guard extraction begun with require_principal_id / require_tenant by adding require_identity() to src/core/auth.py and migrating the remaining open-coded guards. The five byte-identical 'if identity is None: raise AdCPAuthRequiredError' guards (media_buy_create/update/delivery/list + products) collapse to 'identity = require_identity(identity)', whose narrowed return removes the 'assert identity is not None' (which python -O strips) at signals + three task_management sites. signals/accounts principal guards now use require_principal_id (canonical wire message) instead of bespoke 'Authentication required for X' strings; products' three-branch tenant guard, which raised AdCPValidationError for an auth condition, collapses to require_tenant (correct AdCPAuthenticationError class + canonical message). Net -22 lines; two unit tests realigned to the canonical messages. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(validation): emit bracket-notation field paths from first_validation_error_field first_validation_error_field rendered Pydantic loc tuples as dotted paths (packages.0.budget) while the hand-rolled field= strings raised inside the _impl layer use bracket notation (packages[].budget) — the same semantic field reached the buyer-facing envelope's 'field' attribute in two shapes depending on where the validation error originated. Render list indices as [i] so both paths share one convention; add a test pinning the shape. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: idiom + test-quality items (abstract filter, kw-only exc_type, atomic mock assert) Three small consistency/hardening items. iter_concrete_subclasses now skips abstract bases via inspect.isabstract (still walking through them to reach concrete descendants), so the 'concrete' promise in the name holds — pinned by a test with a real ABC. raise_if_validation_failed's exc_type is now keyword-only (consistent with its already-keyword-only context param); the 7 call sites plus one direct test pass exc_type= explicitly. The split mock assertion in test_media_buy.py (bare assert_called() + manual call_args check) is now atomic assert_called_once_with(...); the weak-mock-assertion guard is extended to flag bare assert_called(), not just assert_called_once(), with the 7 pre-existing sites allowlisted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(harness): restore setup_product_chain/_build_mock_context_manager to MediaBuyCreateEnv MediaBuyCreateEnv called self.setup_product_chain() and self._build_mock_context_manager() but defined neither — they lived only on a _MediaBuyCreateEnv bridge subclass in the integration suite, so any consumer of the bare class (or MediaBuyDualEnv, which has no bridge) hit AttributeError. Move both methods onto MediaBuyCreateEnv and delete the now-empty bridge; MediaBuyDualEnv inherits them via its MRO. Also restore two further dangling references that kept media_buy_dual.py from importing at all (make_adapter_update_side_effect in _mixins, _WRAPPER_UNSUPPORTED_FIELDS in the update harness). Lower the behavioral-mock cap 39->38 to track the mock construction that moved out of the integration suite into the harness. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(adapters): raise-site coverage for all typed adapter errors + enforce with a guard The typed adapter exception classes introduced in this PR (replacing error_code=/details[internal_code]) had no pytest.raises coverage at their production raise sites, so a class-swap would silently flip the wire taxonomy unnoticed. Add raise-site tests that drive the real adapter methods to each raise and assert pytest.raises(<class>) + the exact error_code for all previously-untested classes: AdCPMediaBuyRejectedError, AdCPInventoryUnavailableError, AdCPActivationWorkflowError, AdCPGamUpdateError, AdCPBulkUpdateError, AdCPProductUnavailableError, AdCPBudgetExhaustedError. Each was negative-control-verified (swapping the raise to a parent/sibling breaks the test). Add test_architecture_adapter_raise_site_coverage.py: an AST guard that scans src/adapters/ for typed-error raises and fails if any raised concrete AdCPError subclass lacks a pytest.raises test. Allowlist is empty and must stay empty. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: route _impl identity guards through typed auth helpers Replace hand-rolled identity guards in six _impl functions with the require_identity / require_principal_id / require_tenant helpers, so each emits the canonical AUTH envelope (wire code + recovery) instead of a stripped assert, a bare ValueError, or a divergent inline message: - accounts._sync_accounts_impl: triple-None check -> require_principal_id + require_tenant (now consistent with _list_accounts_impl); drop the redundant per-iteration tenant re-fetch that recomputed identity.tenant. - capabilities, creatives._sync, creatives.listing, signals: replace `assert identity is not None` (stripped under python -O) with require_identity. - performance._update_performance_index_impl: replace `raise ValueError` with require_identity, so identity-None surfaces AUTH_TOKEN_INVALID rather than a synthetic VALIDATION_ERROR. Drop the now-zero performance.py entry from VALUE_ERROR_PER_FILE_CAP. Add a structural guard (test_architecture_no_handrolled_identity_guard) with an empty allowlist: it flags `assert identity is not None` in any function and raising `if <identity> is None` guards inside *_impl functions. Non-raising None-branches (anonymous-caller graceful degradation, e.g. get_products pricing strip) are out of scope. Update the performance E1 behavioral test to assert the typed AdCPAuthRequiredError contract. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: replace task_management is_authenticated guards with require_principal_id list_tasks, get_task, and complete_task hand-rolled `if not identity.is_authenticated: raise AdCPAuthenticationError(...)` after require_identity + require_tenant. is_authenticated is bool(principal_id) (resolved_identity.py), so this is exactly what require_principal_id enforces; replace all three with require_principal_id. complete_task captures the helper's return value, dropping the now-redundant `principal_id = identity.principal_id`. Extend the no-handrolled-identity-guard to flag `if not <x>.is_authenticated: raise` in any business-logic function, so the open-coded form cannot reappear. AdCPAuthenticationError is no longer raised directly in the module and is dropped from the imports. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: carry field= on GAM manager-config errors via shared helpers _validate_creative_for_gam, _get_creative_type, _create_gam_creative, and _check_order_has_guaranteed_items each hand-rolled `if not self.<x>_manager: raise AdCPConfigurationError(...)` without the field= that their sibling config raises carry (network_code, advertiser_id, authentication). Extract _require_creatives_manager / _require_orders_manager helpers that raise with field="creatives_manager" / field="orders_manager", and reduce the four delegators to one-liners. This removes the byte-identical triple-guard duplication and makes every AdCPConfigurationError in the adapter carry a field. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: migrate remaining creative principal_id guards to require_principal_id _sync_creatives_impl and _list_creatives_impl still hand-rolled `if not principal_id: raise AdCPAuthenticationError(<divergent message>)` directly above the require_identity added in the earlier identity-guard migration — the same patch-not-pattern the guard targets, in the same functions. The no-handrolled-identity guard missed them because it modeled `X is None` (Compare) but not the equivalent `if not X:` truthiness form. Replace both with require_principal_id, ordered before require_identity so the canonical "Authentication required ... x-adcp-auth token" message surfaces for missing/anonymous auth while require_identity narrows the type. AdCPAuthenticationError is no longer raised directly in either module and is dropped from their imports. Extend the guard's _if_identity_guard_kind to also flag `if not <identity-target>: raise` and tighten its docstring to state the matched shapes precisely. Relax one auth test that pinned the old divergent "Missing x-adcp-auth" wording to the canonical auth message, and normalize two single-item `with (...)` statements in that file to the plain form (resolves a black/ruff format disagreement that blocked the commit). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: surface canonical auth message for sync_accounts missing-token Pattern A's _sync_accounts_impl ran require_identity before require_principal_id; for a missing/anonymous token that raised AdCPAuthRequiredError("Identity is required"), whose message carries no auth phrase. The uc011 "sync without authentication" BDD scenario asserts the error message describes the authentication requirement (then_error_message_auth), so it failed on the impl and mcp transports. Reorder require_principal_id first so the canonical "Authentication required ... x-adcp-auth token" message surfaces (wire code unchanged: AUTH_TOKEN_INVALID); require_identity still follows to narrow the type for _check_billing_policy. This mirrors the ordering already used in the creative _impls. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: drain details["error_code"] smuggling to typed-class wire codes + guard KonstantinMirin CRIT-01 (Pattern 2). 27 sites passed details={"error_code": "X"}, smuggling a second code into errors[0].details while the typed class set the wire adcp_error.code — two codes per envelope, bypassing the "wire code = subclass identity" invariant. (27, not 26: the AST guard found one multi-line dict the review's line grep missed.) - POLICY_VIOLATION (products.py, get_products policy block) was the one genuine wire-code defect: the class was AdCPAuthorizationError (AUTH_REQUIRED) while the smuggled code POLICY_VIOLATION was the intended buyer code. Add AdCPPolicyViolationError (subclass of AdCPAuthorizationError, wire code POLICY_VIOLATION, recovery=correctable) and raise it — the wire code now matches intent. - The other 25 smuggled codes are non-sanctioned internal sub-classifications; the typed class already carried the correct wire code (VALIDATION_ERROR / SERVICE_UNAVAILABLE / AUTH_TOKEN_INVALID). Drop the error_code key, keeping genuine detail data (creative_errors) and recovery overrides. Extend test_architecture_no_error_code_kwarg_in_impl.py: a new detector + test class (TestNoErrorCodeInDetails, empty allowlist) flags details={...} dict literals carrying an error_code key, so the indirection variant cannot reappear. Shared AST walk factored into _iter_adcp_error_calls. Update the ~16 tests that pinned the smuggled sub-code to assert the wire code (error_code == "VALIDATION_ERROR" / "POLICY_VIOLATION") or the surviving detail ("creative_errors" in details). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: route list_authorized_properties tenant guard through require_tenant Replace the open-coded tenant None-check in _list_authorized_properties_impl with require_tenant(identity), so a missing tenant surfaces the canonical AdCPAuthenticationError wire envelope (AUTH_TOKEN_INVALID) instead of a bespoke TENANT_ERROR. Anonymous principals remain allowed. Update the behavioral test and BR-UC-007 feature to assert the canonical wire codes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: use %-placeholder logging in creative sync processing Convert two logger.error f-string calls in _processing.py to %-placeholder form, matching the logging idiom adopted elsewhere in the PR. Defers string interpolation to the logging call so it is skipped when the level is disabled. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: rename _verify_principal context param to identity The parameter holds a ResolvedIdentity, not a transport Context — the name was a pre-refactor leftover. Rename it to identity to match its type, and update the three keyword callers and two signature-introspection assertions that referenced the old name. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: pin correctable recovery on A2A validation-error envelopes The seven assert_envelope_shape calls for VALIDATION_ERROR in the A2A error suite asserted only the wire code. Add recovery="correctable" so the buyer-retry semantics are pinned alongside the code — the recovery field is part of the envelope contract and governs whether a buyer agent retries. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: typed context_id error, normalized not-found messages, packages[] field helper - media_buy_update: an unresolved buyer-supplied context_id now raises AdCPValidationError(field="context_id") instead of base AdCPNotFoundError. The base NOT_FOUND maps to INVALID_REQUEST via the safety net; AdCP has no context-specific not-found wire code, so VALIDATION_ERROR with the field path is the intentional, spec-standard signal. - Normalize AdCPMediaBuyNotFoundError messages to one "Media buy '<id>' not found" form (drop trailing period, quote the id) so buyer-side substring matching does not fork across raise sites. - Extract package_field_path() in validation_helpers and route the seven hand-rolled field="packages[].<attr>" literals through it, mirroring first_validation_error_field's bracket notation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: guard get_media_buys against a None tenant via require_tenant Replace `tenant = identity.tenant` with require_tenant(identity) so an unresolved tenant surfaces the canonical auth envelope instead of a raw TypeError on tenant["tenant_id"]. The principal advisories above are kept as-is: BR-UC-019 mandates the empty-list + errors[] degrade for missing principal_id / principal-not-found, with only the no-auth case raising. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: allowlist push_notification_config in get-products schema-drift check The live /schemas/latest/media-buy/get-products-request.json added push_notification_config, which the pinned adcp library does not model on GetProductsRequest. Add it to KNOWN_SCHEMA_LIBRARY_MISMATCHES alongside the existing spec-ahead-of-library fields so the two get-products schema-alignment tests record the known drift instead of failing. Pre-existing failure (fails identically at this branch's base); make quality skips it offline, so it only surfaces in the full network-enabled suite. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(deps): bump aiohttp 3.14.0 and authlib 1.6.12 for security advisories uv-secure flagged aiohttp 3.13.5 (GHSA-jg22-mg44-37j8, GHSA-hg6j-4rv6-33pg) and authlib 1.6.11 (PYSEC-2026-188). Both pyproject constraints already permitted the fix versions, so this is a lock-only change. authlib is pinned to the minimal 1.6.12 patch (not 1.7.x) to keep the admin OAuth/OIDC surface low-risk. Security audit is clean after the bump. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(errors): stop catch-all handlers flattening typed AdCPErrors (G1) Seven `except Exception -> raise AdCP*Error` handlers in src/core and src/adapters re-wrapped any typed AdCPError raised in the try body, collapsing its precise wire code + recovery into a generic one. Add the canonical `except AdCPError: raise` sibling clause at each site so typed errors propagate unchanged; the catch-all wraps only the unexpected. Sites: properties.py, products.py (x2 - incl. widening a narrow isinstance(AdCPAdapterError) check to AdCPError), creative_formats.py, creative_helpers.py (live bug: sync_creatives errors were flattened), google_ad_manager.py, media_buy_create.py (inline isinstance -> sibling clause). Add tests/unit/test_architecture_no_error_flattening.py enforcing the idiom across src/core + src/adapters (empty allowlist). The inline isinstance variant is also flagged so the idiom stays machine-checkable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(errors): unify auth/require helper signatures + guard (G2) Make the "centralize the guard, raise the typed error" helper families signature-consistent and lock the conventions with a structural guard. auth.py guard helpers: `require_identity` was the only require_*/_or_raise helper lacking a keyword-only `context=` param (the other three already had it). Add it and pass it into the raised AdCPAuthRequiredError, then wire the two callers that have `req.context` in scope (media_buy_update, delivery) so auth-failure envelopes echo the request context for buyer correlation. Adapter accessors: `_require_creatives_manager`/`_require_orders_manager` had no return annotation, so callers couldn't narrow the type the way base.py's `_require_config` (-> _ConfigT) models. Annotate them `-> GAMCreativesManager` / `-> GAMOrdersManager`. mypy does not enforce return annotations here (disallow_untyped_defs=False), so the guard is the only check. Remove the dead `if identity:` in capabilities.py: after `identity = require_identity(identity)` narrows it to non-None, the guard is always true. Add tests/unit/test_architecture_auth_helper_signature.py: (1) every auth.py require_*/_or_raise helper declares a keyword-only context= param; (2) every adapter _require_* accessor declares a return annotation. Both allowlists empty; both halves negative-tested. Scope is signatures only — caller-side context= wiring is a non-guardable convention, like call order. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(errors): echo request context through all auth-helper calls (G2) G2 made the auth-helper signatures accept context=; this completes the convention at the call sites so auth/authorization failures echo the request context into the wire envelope for buyer correlation (previously only resolve_principal_or_raise was wired everywhere; require_tenant's context= was dead and require_principal_id was wired at only 2 of 8 sites). Wire context= into every auth-helper call in src/core/tools (~30 sites): `req.context` where the _impl takes a validated request, `context=context` for the individual-parameter impls (_sync_creatives, _list_creatives, _activate_signal). Thread a keyword-only `context` param through `_verify_principal` and pass it from its two call sites (media_buy_update, performance) so the ownership-check's auth guards echo context too. task_management's list_tasks/get_task/complete_task are intentionally left context-less: their `context` parameter is the FastMCP transport Context, not an AdCP ContextObject, and the auth helpers take a ContextObject. Add tests/unit/test_architecture_auth_helper_context_wiring.py: in src/core/tools, any auth-helper or _verify_principal call inside a function with a `req` parameter or a ContextObject-annotated `context` parameter must pass context=. The transport-Context exclusion falls out of the ContextObject annotation check, so no allowlist is needed; it is empty. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(errors): type unresolvable context_id as AdCPContextNotFoundError (Fix 1) A buyer-supplied context_id that does not resolve was raised as AdCPValidationError (400, VALIDATION_ERROR). It is a not-found condition, not a validation failure: Context rows have no TTL/expiry/delete path, so a non-resolving id never existed. Introduce AdCPContextNotFoundError -> SESSION_NOT_FOUND (the standard SDK code for an unresolvable session/context, passthrough), 404, recovery=correctable, and raise it at media_buy_update with context=req.context echoed into the envelope. SESSION_NOT_FOUND is a standard code, so registration is a single CANONICAL_ERROR_CODES entry (no ERROR_CODE_MAPPING / INTERNAL_CODES needed). creatives/_workflow.py's "Failed to create workflow context" is left as AdCPAdapterError: it calls get_or_create_context without a context_id, so its None is a genuine creation failure, not an unresolvable id. Update the unit test on this path (now expects AdCPContextNotFoundError / SESSION_NOT_FOUND) and add a class-level wire-contract test pinning 404 / SESSION_NOT_FOUND / correctable / passthrough. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(errors): typed creative/format/task not-found taxonomy + guard (Fix 4 / G3) Add AdCPCreativeNotFoundError, AdCPFormatNotFoundError, AdCPTaskNotFoundError (extending AdCPNotFoundError, recovery=correctable). The SDK defines no standard CREATIVE/FORMAT/TASK_NOT_FOUND code, so each is registered in three places (the existing NOT_FOUND model): ERROR_CODE_MAPPING -> INVALID_REQUEST, INTERNAL_CODES, and the CANONICAL_ERROR_CODES test vocabulary. The buyer-visible wire code is unchanged (INVALID_REQUEST, exactly as the base NOT_FOUND emitted); the gain is recovery terminal -> correctable + a typed identity + guard-enforceability. The spec-ahead-of-SDK BDD scenarios that want the specific codes stay xfail-managed. Migrate the raise sites to the typed subclasses: - media_buy_create.py: format (-> AdCPFormatNotFoundError), creative (-> AdCPCreativeNotFoundError), and the suite-invisible per-package product branch (-> AdCPProductNotFoundError). - format_resolver.py: get_format -> AdCPFormatNotFoundError (terminal -> correctable; format_resolver unit tests tightened to pin the new contract). - task_management.py: get_task / complete_task -> AdCPTaskNotFoundError. Add test_architecture_no_base_notfound_raise.py (G3): forbid raising the base AdCPNotFoundError anywhere in src/ (typed subclasses only). One allowlisted sanctioned generic: account_helpers.resolve_account's defensive, unreachable unsupported-AccountReference-variant fall-through. To keep the guard family DRY, extract the shared file-scan into _ast_helpers.iter_module_trees and route the existing guard finders through it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(errors): centralize fetch-and-raise into repository _or_raise helpers (Fix 3) Collapse the duplicated "fetch entity by id, raise the typed not-found if it does not exist" guard into three repository helpers: - MediaBuyRepository.get_by_id_or_raise -> AdCPMediaBuyNotFoundError - MediaBuyRepository.get_package_or_raise -> AdCPPackageNotFoundError - WorkflowRepository.get_by_step_id_or_raise -> AdCPTaskNotFoundError The MediaBuy/Package helpers plumb context= (echoed into the error envelope for buyer correlation); the Task helper does not -- get_task/complete_task carry the FastMCP transport Context, not an AdCP ContextObject, so it stays context-less rather than threading a transport object into a repository. Each helper coexists with its plain getter (callers that deliberately tolerate None keep using it). Migrate the 8 fetch-and-raise sites -- media_buy_update.py (4 media-buy + 2 package) and task_management.py (2 task) -- and drop the now-unused not-found imports. Dedup WorkflowRepository.get_step_by_id (byte-identical body) to delegate to get_by_step_id so the query lives in one place. Tests: the MediaBuyUpdateEnv harness wires the _or_raise mocks to delegate to the plain getters + raise, so the existing media-buy tests drive both the direct and or-raise paths unchanged (preserving side_effect sequencing); the direct-mock task tests point at the _or_raise methods; and a new test_repository_or_raise_helpers.py verifies the real fetch-and-raise semantics against a mocked session, backing the tool-level mocks with a real-behavior test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(errors): require recovery= on assert_envelope_shape (G4 / Fix 6) Make ``recovery`` a required keyword-only argument on the wire-envelope assertion helper. Recovery is the buyer-facing retry semantics (correctable / transient / terminal); a silent drift between a typed exception's recovery and the wire is exactly the regression this helper exists to catch. Pinning it was previously optional (``recovery: str | None = None``), so 11 of 48 call sites asserted only the code and let the recovery field float unverified. The required keyword-only param is itself the guard: omitting recovery is now a TypeError at call time, so no future error test can skip it. The assertion runs unconditionally (the ``if recovery is not None`` guard is gone). Migrate the 11 sites that lacked recovery=, deriving each value from the production path (verified against build_two_layer_error_envelope): - test_adcp_exceptions.py (9): VALIDATION_ERROR/correctable, AUTH_TOKEN_INVALID/terminal, INVALID_REQUEST(NOT_FOUND)/terminal, SERVICE_UNAVAILABLE/transient x2, CONFLICT/correctable, INVALID_STATE/correctable, BUDGET_EXHAUSTED/correctable. - test_error_boundary_translation.py: PermissionError -> AUTH_REQUIRED/terminal. - test_a2a_brand_manifest.py: AdCPValidationError -> VALIDATION_ERROR/correctable. The other 37 call sites already pinned recovery and are unchanged. Update tests/CLAUDE.md (the "What to assert" table) and the helper docstring to document recovery as required. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(errors): build packages[] field path via helper + guard (G5 / Fix 6) The ``_impl`` layer reports a per-package validation failure with a no-index bracket path (``packages[].targeting_overlay.property_list``). The prefix has a single source — ``package_field_path()`` in validation_helpers — but ``raise_if_property_targeting_violations`` hand-rolled the literal, so the one remaining ``packages[]`` string drifted out of the helper's reach. Route it through ``package_field_path("targeting_overlay.property_list")`` (byte-identical output: ``p…

…d-assertions # Conflicts: # .duplication-baseline # tests/admin/test_creatives_blueprint.py

…-migration

…igration test(bdd): migrate feature files to AdCP 3.1 (wired UCs green, zero regressions)

…PR 2 of prebid#1234) (prebid#1370) * ci: uv.lock as single source of truth, dep upgrades, test isolation (PR 2 of prebid#1234) Pre-commit hooks: - Replace psf/black + mirrors-mypy external repos with local `uv run` hooks - No more `additional_dependencies:` — all tools resolved from uv.lock (ADR-001) - mypy hook: add src/ target so `pass_filenames: false` mode works correctly - Add structural guard: test_architecture_pre_commit_no_additional_deps.py Dependency changes: - Remove [project.optional-dependencies].dev; consolidate into [dependency-groups].dev - Add pytest-xdist>=3.0.0 and pytest-randomly>=3.15.0 to dev group - fastapi>=0.133.0 (adds Starlette 1.0 support; fixes PYSEC-2026-161 / CVE-2026-48710) - fastmcp>=3.2.0,<3.3.0 (pin below 3.3 packaging split that breaks server imports) - pydantic-ai-slim[google,openai,anthropic,cohere,mistral,groq]>=1.99.0 (fixes CVE-2026-46678 / GHSA-cqp8-fcvh-x7r3; slim variant avoids fastmcp>=3.3 pull) - Remove GHSA-cqp8-fcvh-x7r3 and PYSEC-2026-161 from security suppression list Runtime: - DB_POOL_SIZE / DB_MAX_OVERFLOW env-var overrides in database_session.py - build_model_string: map google-gla/gemini -> google: (pydantic-ai 1.99 deprecation) - GoogleModel: use GoogleProvider() object instead of deprecated 'google-gla' literal Test fixes (pytest-randomly ordering exposure): - TestAuthContextMiddleware: use isolated fixture app instead of mutating global app - TestFastAPIExceptionHandlers: same fix; pre-register all exc routes on fixture app - Register arch_guard marker in pytest.ini * fix: update CI uv sync flags and suppress withdrawn MAL-2026-4750 - Replace 'uv sync --extra dev' with 'uv sync --group dev' in all CI jobs; dev deps were moved from [project.optional-dependencies] to [dependency-groups] in this PR, which uses a different uv flag. - Add MAL-2026-4750 (fastapi 0.136.3) to pip-audit suppression list; the advisory was withdrawn by OpenSSF on 2026-05-26 as a false positive but pip-audit's offline database lags the withdrawal. * fix: rebase onto upstream/main and align pre-commit formatter with make quality - Rebase PR prebid#1370; resolve conflicts; fix post-rebase test failures - Replace pre-commit uv run black with uv run ruff format (ADR-001 from uv.lock; matches Makefile/CI — avoids black vs ruff fight on 162 files) - Exception-handler fixture uses build_two_layer_error_envelope (prebid#1306) - Update resolved-identity caps and PrincipalFactory in auth helper * fix: restore AdCPValidationError import and address Chris review feedback * fix: address PR 1370 review and StrEnum UP042 after rebase - Rebase onto upstream/main; regenerate uv.lock - type-ignore baseline 60; mock helper assert_called_once_with - Google provider DRY + ValueError when no resolved API key - Migrate str+Enum classes to StrEnum (UP042 from main) * fix: address PR 1370 review, schema alignment, StrEnum UP042 * fix: address PR 1370 follow-up review after rebase onto main * chore: ratchet duplication baseline tests 99 → 98 after main rebase Pre-commit check-code-duplication auto-fixed one duplicate block during push hook run. * fix: move remaining exception-handler tests to isolated FastAPI fixture Four TestFastAPIExceptionHandlers cases still registered routes on the global app; with pytest-randomly the admin catch-all mount swallowed them (empty body, JSONDecodeError). Pre-register routes on exc_handler_test_app like the rest. * fix: close PR 1370 round-2 review findings (guard scope, Google DRY, _int_env) - Widen weak-mock guard SCAN_DIRS to all of tests/ with pre-existing allowlist entries - Use canonicalize_google_provider() in admin AI settings for legacy key migration - Extract _int_env() and migrate all database pool/timeout env reads * fix: close PR 1370 round-3 review (int_env, legacy key test, FIXME refs) Promote int_env to db_config.py and migrate db_config DB_PORT parsing; add uses_legacy_gemini_api_key helper with parametrized alias tests; replace beads FIXME tags with prebid#1370 in weak-mock guard allowlist. * chore: fix EOF in BDD feature files for pre-push hook * fix: pin starlette>=1.0.1 to resolve PYSEC-2026-161 in lockfile fastapi>=0.133.0 allows starlette 0.50.x; uv still resolved 0.50.0 and pip-audit failed on PR2 CI. Explicit starlette>=1.0.1 forces 1.2.1 in uv.lock. * fix: satisfy weak-mock guard in uc006 Slack notification step Use assert_called_once_with for send_notifications and shrink stale BARE_ASSERTION_ALLOWLIST entries surfaced by SCAN_DIRS widen.

…d#1234) (prebid#1372) * ci: establish authoritative CI workflow with frozen checks (PR 3 of prebid#1234) Replace the legacy test workflow with a single CI pipeline that has 14 fixed required check names and shared composite actions, so branch protection remains stable and enforcement is centralized in CI. * fix(ci): resolve failing PR3 checks and align formatting * fix: review comments * fix(ci): align coverage baseline and bump uv to 0.11.15 Set .coverage-baseline to 46.5 to match unit+harness coverage gate (~47%). Bump CI uv to 0.11.15 for GHSA-4gg8-gxpx-9rph (uv-secure global tool check). * fix(ci): extend integration job timeout for full suite Raise integration-tests timeout to 40m; drop redundant --cov (dedicated Coverage job). Avoids GitHub Actions cancel when the consolidated suite exceeds 20m. * fix(ci): address PR3 review — sharding, dedupe unit runs, frozen checks - Quality Gate uses make quality-ci; coverage reuses unit test artifact - Restore 5-way integration matrix sharding - Exclude alembic/versions from pre-commit ruff hooks; revert migration formatting - Expand frozen check guard to 18 names; document branch protection migration * fix(ci): address PR3 review — dedup workflow, strict shards, CI/local alignment * fix(ci): inline postgres services — GHA rejects YAML anchors * fix(ci): inject pytest extra_args without bash word-split on markers * fix(ci): let E2E job start docker stack — override workflow ADCP_TESTING Workflow-level ADCP_TESTING=true made conftest wait for an existing server on port 8080 instead of starting docker-compose.e2e.yml. Restore legacy test.yml behavior: clear ADCP_TESTING/DATABASE_URL on the E2E step, add docker cleanup before/after pytest. * fix(ci): add Postgres service to admin-ui-tests job Admin blueprint tests use integration_db; without the GHA postgres service and _postgres migrate step they retry connection for ~20m then fail. Guard test added to prevent regression. * fix(ci): handle merge-head alembic downgrade in Migration Roundtrip Replace ambiguous `alembic downgrade -1` with a script that resolves explicit downgrade targets when the head revision is a merge migration. * fix(ci): resolve merge-head downgrade and load_ci_workflow import Use first parent revision for merge-head Alembic downgrade (comma-separated targets are invalid). Import load_ci_workflow in CI suite coverage guard. * fix(ci): combine unit and BDD coverage in Coverage job for PR3 Upload BDD coverage artifacts with xdist fragment merge and gate against .coverage-baseline using combined unit+BDD data (46.5% baseline). * ci: cancel superseded PR workflow runs via concurrency group Add conditional concurrency so pull_request pushes cancel in-flight runs while push/workflow_dispatch keep per-SHA groups on main. * ci: route BDD job through _pytest composite with coverage_xml Extend _pytest with coverage_xml for xdist-friendly BDD coverage output; split fragment combine into its own step. Restores 7/7 pytest routing. * docs(ci): document intentional duplicate mypy in Quality Gate and Type Check Keep both jobs: quality-ci is the local/CI static gate; Type Check remains a dedicated required-check signal without adding wall-clock time. * fix: drop false CI storyboard job reference in capabilities comment Storyboard compliance is advisory with no dedicated required CI job; the previous comment incorrectly pointed readers at ci.yml. * fix(ci): rename Quality Gate step to reflect make quality-ci The step runs make quality-ci (ruff, mypy, duplication), not pre-commit. * fix(ci): drop unused coverage collection from E2E and Admin jobs Only unit and BDD artifacts feed the Coverage gate; E2E host-side coverage measures the client harness, not src/, and Admin coverage was discarded. * docs(ci): align Security Audit and Coverage docs with ci.yml Security Audit runs uv-secure only; Coverage combines unit and BDD artifacts. Update LINK_VALIDATION_CI excerpt to reference ci.yml integration shard. * fix(ci): add timeout-minutes to fast-fail jobs Cap Quality Gate, Unit Tests, Type Check, Schema Contract, Security Audit, Smoke Tests, Coverage, and Summary so hung jobs fail instead of burning runner minutes until the 6-hour default. * fix(ci): include Python version in uv cache key Bump cache key to uv-v4 and scope by python-version so a matrix Python change does not restore a stale .venv from another version. * fix(ci): tighten smoke skip-decorator grep to exclude skipif Use a regex anchor so @pytest.mark.skipif does not false-positive the guard that rejects @pytest.mark.skip in the test suite. * fix(ci): remove dead PYTEST_CURRENT_TEST from workflow env Pytest sets this variable per test at runtime; the workflow-level export had no effect and could confuse readers about when the flag is active. * fix(tox): share coverage fail-under gate with CI via shell script tox commands run without a shell, so $(tr ...) in coverage report never expanded. Add scripts/ci/coverage_report.sh for tox and the CI Coverage job. * fix(test): make merge-head downgrade test graph-agnostic Search the migration graph for any merge revision instead of requiring the current head to be a merge migration.

prebid#1379) * ci: parallel BDD tests with 2 greedy shards (PR3-2 of prebid#1234) Rebase onto merged PR3 (prebid#1372). Split BDD into two parallel shard jobs with greedy scenario-count assignment; keep aggregate BDD Tests status proxy and combined unit+BDD coverage in Coverage job. Frozen checks: 20. * fix(ci): address ChrisHuie review on BDD shard hardening (prebid#1379) - Fail BDD shard job when shard_paths.py errors or returns no files - Reject shard_count above BDD file count with clear ValueError - Tighten scenario-count guard (>= 1); drop unused round-robin path - Document local shard reproduction and load-metric limits in ci-pipeline.md * test(ci): anchor BDD shard partition guard to pytest collect-only Address ChrisHuie review item 3: compare shard assignment against pytest-collected BDD modules instead of the same glob as shard_split. * fix: Address KonstantinMirin review on BDD shard CI hardening Require every BDD shard coverage artifact before combine, bind shard denominator to strategy.job-total, assert migration roundtrip revisions, add workflow hygiene guards, and DRY migration AST parsing. * fix: Assert migration roundtrip heads after merge downgrade Use get_current_heads() instead of get_current_revision() so CI roundtrip passes when downgrading a merge head restores multiple branch tips.

…prebid#1234) (prebid#1424) * fix(ci): operational helper layering and guard completeness (PR 3-3 of prebid#1234) Relocate migration and workflow helpers to scripts/ci/ so CI scripts and hooks import operational code instead of the test tree. Strengthen summary gate coverage, align Type Check with make typecheck, drop duplicate skip grep, surface migration parse failures, and require int pid in port_scan_start. * fix(ci): bind real getpid at import for port_scan_start (PR 3-3) Unit-test autouse mocks can patch os.getpid during the full suite run; capture the real callable at module import so find_free_port still works while explicit non-int pid arguments remain a TypeError. * test(ci): restore REQUIRED_SUMMARY_GATES floor in summary guard (PR 3-3) Keep the generalized summary.needs loop from PAT-02 and pin the suite jobs that must never disappear from summary.needs (PR prebid#1299 regression class). Addresses ChrisHuie SHOULD-FIX 1 on prebid#1424. * fix(ci): address ChrisHuie review items on PR3-3 (prebid#1424) - SHOULD-FIX 2: match skip grep by step content; pin TestNoSkippedTests SSoT - SHOULD-FIX 3: extract _downgrade_parents_for_head shared resolver - Fix uv cache key to hashFiles('.python-version') after input removal - Migration hook catches MigrationParseError only; merge-head tests fail not skip - Bind _GETPID comment per review; scan scripts/ in duplication check

* refactor: consolidate account reference coercion * test: cover account reference validation * test: strengthen account validation assertions

…#1399) * docs: map the media-buy validation slice (protocol vs business; SDK ordering) * docs: SDK-breakage spike results (4.3->5.7 = 3.1-beta.3, bounded; 85 deep imports) * docs: SDK type-collision blocker (431 duplicate-name classes); e2e-contract reframe * feat: upgrade adcp SDK 4.3.0 → 5.7.0 (spec 3.1.0-beta.3) Direct upgrade keeping deep imports (collision-free). Changes: SDK import fixes: - Account → Accounts (sync_accounts_request) - SignalId18 → SignalId5 (same fields) - Setup moved to adcp.types.Setup - SyncResponseAccount declared locally (gone from SDK) Fields removed from SDK, declared locally: - SyncCreativeResult: assigned_to, assignment_errors, platform_id, status - SyncCreativesResponse: dry_run - SyncAccountsResponse: accounts, dry_run - CreateMediaBuySuccess: account, sandbox - delivery.py: deleted 3 redeclarations SDK now provides Signature changes: - create_mcp_webhook_payload: 3rd arg is now task_type, returns McpWebhookPayload (not dict). Fixed 5 call sites. Type coercion (adcontextprotocol/adcp-client-python#913): - CreativeAction is plain Enum (not StrEnum), SDK stores raw strings. SyncCreativeResult.action coerced via field_validator. Nullability: - GetProductsResponse.products, GetSignalsResponse.signals now list|None. Added guards. Test reconciliation: - CreativeAsset assets must be list[AssetVariant] with asset_type tag - Protocol envelope adds status, context_id, message to all responses - DeliveryMeasurement.provider now optional - ListCreativesRequest gained 4 fields - UnknownFormatAsset passes through (better Postel's law) Upstream issues filed: - adcontextprotocol/adcp-client-python#913 (1410 enums should be StrEnum) - adcontextprotocol/adcp-client-python#914 (buyer_ref ghost field) * fix: reconcile integration tests with adcp SDK 5.7 model changes - CreativeAction enum comparisons → string comparisons (12 integration files) - CreativeAsset discriminated union dicts → make_image_assets() helper - SyncResponseAccount moved to schemas/account.py with typed fields - SyncAccountsResponse declares accounts/dry_run/context/ext locally - Protocol envelope field assertions updated - Schema alignment test allowlists updated for SDK 5.7 Part of prebid#1388 * refactor: extract shared test helpers to reduce duplication (112→95) - Created tests/helpers/creative_test_helpers.py with shared builders - Added make_creative_asset_minimal() to creative_asset factory - Migrated 8 test files from inline asset dicts to shared helpers - Duplication baseline: 101→95 (net improvement despite SDK 5.7 expansion) Part of prebid#1388 * fix: normalize CreativeAction to string everywhere (enum→str) SDK 5.7's CreativeAction is plain Enum (not StrEnum), making "created" == CreativeAction.created return False. Instead of coercing to enum (which breaks string comparisons), store as string and compare with strings consistently. - SyncCreativeResult.action: field_validator normalizes enum→str - Production code: comparisons use string literals - Unit + integration tests: comparisons use string literals - Removed unused CreativeAction imports Part of prebid#1388 * fix: SyncCreativesResponse.creatives typed as SyncCreativeResult (Pattern #4) Transport roundtrip returns library Creative objects lacking locally-declared fields (assigned_to, assignment_errors, etc). Override creatives list type to use our SyncCreativeResult subclass. Also normalize remaining enum comparisons to string in unit + integration tests. Part of prebid#1388 * fix: resolve remaining integration test failures for SDK 5.7 Production: - _processing.py: extract message from SDK 5.7 RootModel-wrapped assets via _extract_text_from_asset_value() and _extract_message_from_assets() shared helpers (DRY: replaced two identical inline blocks) Tests: - CreativeAction/CreativeStatus/MediaBuyStatus enum→str comparisons - SyncCreativesSubmitted requires task_id in SDK 5.7 - Schema database mapping: new SDK 5.7 computed fields - Import fix: use local SyncCreativeResult, not library Creative Integration: 0 failed, 1906 passed (was 219 failed before fixes) Part of prebid#1388 * docs: clarify task_type wire/internal distinction and track SDK 5.7 type:ignore comments delivery_webhook_scheduler: add comment block explaining why metadata task_type ("media_buy_delivery") and SDK task_type ("update_media_buy") are intentionally different, and why renaming is not safe. Add SDK 5.7 type:ignore tracking headers to 6 files referencing upstream issue adcontextprotocol/adcp-client-python#913, categorizing each ignore as permanent (class hierarchy, field override) or fixable (RootModel proxy, dynamic type factory). * test: add RootModel unwrapping tests, SyncResponseAccount contract, and 2 structural guards - Fix tautological assertion in test_creative.py:280 (salesagent-osmq) - Add test_rootmodel_asset_unwrapping.py: coverage for SDK 5.7 RootModel-wrapped creative assets in _extract_url_from_asset_value and _extract_text_from_asset_value (salesagent-6xt9) - Add test_sync_response_account_contract.py: field-set and serialization contract for locally-owned SyncResponseAccount model (salesagent-a6zc) - Add test_architecture_no_bare_basemodel.py: guard against bare pydantic.BaseModel in src/core/schemas/ without model_config - Add test_architecture_local_schema_imports.py: guard against importing SDK types in src/ when a local subclass exists in schemas * fix: correct SDK import leaks and schema defects in creative/account modules - (dcu2) Replace SDK SyncCreativeResult/SyncCreativesSuccessResponse imports in admin blueprint with local schema types that carry field validators, locally-owned fields, and Pattern #4 model_dump overrides - (am8p) Change SyncResponseAccount base from bare BaseModel to SalesAgentBaseModel for env-aware extra mode and exclude_none defaults - (iswi) Declare context/ext fields on SyncCreativesResponse matching the SyncAccountsResponse pattern (SDK 5.7 removed them from parent) - (uoj5) Add Field(exclude=True) to SyncCreativeResult.status, fix model_dump exclude-set mutation via defensive copy, remove redundant manual exclusion of status/review_feedback - (tkv8) Remove dead _act() helper in SyncCreativesResponse.__str__; field_validator already normalizes action to str on construction * refactor: extract DRY helpers for enum normalization, asset unwrapping, and keyword-only webhook calls Three related DRY/consistency fixes: 1. (salesagent-5wg2) Extract _extract_attr_from_asset_value shared helper in _assets.py to deduplicate ~80% structural overlap between _extract_url_from_asset_value and _extract_text_from_asset_value. Move _extract_text_from_asset_value and _extract_message_from_assets from _processing.py to _assets.py where their siblings live (layering fix). 2. (salesagent-hpuk) Add enum_value() helper in src/core/enum_helpers.py (zero project imports, circular-import-safe) replacing 14 sites of fragile `x.value if hasattr(x, 'value') else str(x)` with a single call. Uses isinstance(Enum) with hasattr fallback for duck-typed proxies. 3. (salesagent-pum1) Standardize create_mcp_webhook_payload calls to keyword-only style in context_manager.py and admin/creatives.py, matching the existing keyword style in operations.py and delivery_webhook_scheduler.py. * fix: resolve cross-group merge conflicts (stale allowlist entries, moved import) * fix(test-infra): restore pytest-xdist + make run_all_tests detect crashed suites Two bugs let BDD failures go completely undetected on this branch: 1. tox [testenv:bdd] runs 'pytest -n auto --dist loadfile' but pytest-xdist was not in pyproject/uv.lock (dropped by a prior relock — it had already been re-added once in ba3001a). So 'tox -e bdd' / './run_all_tests.sh' died with 'unrecognized arguments: -n --dist' (exit 4) before running a single scenario, and wrote no bdd.json. Re-add pytest-xdist>=3.5.0 to the dev dependency group so it survives relocks. 2. run_all_tests.sh decided pass/fail by looping over whatever JSON reports happened to exist — a suite that crashed before writing its report (like bdd above) was silently skipped, yielding a false 'ALL PASSED'. Now assert the full expected report set (unit, integration, e2e, admin, bdd, ui) each exists AND has exitcode 0, and treat a non-zero tox exit as failure too. With both fixed, ./run_all_tests.sh actually runs BDD against the full Docker stack and reports honestly. First honest run surfaces 40 pre-existing BDD failures on this branch (uc006 account resolution, uc011, uc005, get_products). * fix(bdd): make 41 BDD failures pass, each grounded in AdCP 3.1 schema (04f59d2d5) First honest BDD run (after the xdist/runner infra fix) surfaced 40 failed + 1 error. Root-caused each against the pinned 3.1 schema source, not the SDK: - uc005 (5): make_asset_group factory instantiated a typing.Union — it never unwrapped the Annotated[Union[Annotated[AssetX, Tag], ...]] nesting. Unwrap properly to the concrete asset member classes. - get_products (4): publisher-property-selector defines only publisher_domain(s)/ selection_type/property_ids/property_tags; property_name/property_type/ identifiers are NOT spec fields. The salesagent-internal test wrongly expected them preserved — assert they are dropped (spec-conformant; non-spec fields are not surfaced). - uc006 (28): the account-resolution scenarios built creatives with the pre-5.7 asset shape ({role: {dict}}); route _format_payload through the canonical make_image_assets() helper (SDK 5.7 discriminated-union list shape). - uc011 (4): sync-accounts-response is oneOf(success requires accounts | error requires errors); the success-variant model defaulted accounts=[] so an empty ctor wrongly validated. Make accounts required (no default). Full bdd suite now: 1521 passed, 0 failed, 0 errors (7380 xfailed). * test(creative-assets): route all inline asset construction through canonical factory (closes prebid#1391) SDK 5.7 changed CreativeAsset to RootModel[CreativeAsset1|CreativeAsset2] with a discriminated-union assets shape ({role: [{asset_type, ...}]}). Inline asset dicts hand-rolled across the test suite kept drifting from this shape and broke silently (the recurring trap behind the BDD failures). Eliminate the duplication by routing every creative-asset construction through the single canonical source in tests/factories/creative_asset.py: - Valid assets -> make_image_assets()/make_video_assets()/make_text_assets() - CreativeAsset model instances -> make_creative_asset_minimal()/CreativeAssetFactory - Legacy AdCP-v1 single-dict shape (adapter converter + validator tests) -> new make_legacy_asset_dict()/make_legacy_image_assets() so even the legacy/ negative shapes are centralized and intentional, not inline. Fixed the shared helpers that themselves emitted the wrong (bare-dict) shape: make_test_creative/make_test_creative_list (creative_test_helpers) and create_test_creative_asset (adcp_factories). Left as literals (correctly): the generic dict-validator's type-rejection inputs (list/str/int/bad-key — not assets) and test_rootmodel_asset_unwrapping (the explicit construction is the unit under test). Verified: unit 4878, integration (creative-sync) 210, e2e 90, admin 86, ui 5, bdd 1521 — all pass. * fix(test-infra): write the ACTUAL Docker-bound ports to .test-stack.env test-stack.sh wrote the pre-allocated POSTGRES_PORT/MCP_PORT to .test-stack.env, but those can drift from the ports docker compose actually binds (collision-retry / reused container). The result: .test-stack.env pointed tests at a port nothing (or a different service) was on, so every suite connected to the wrong port — integration cascaded DB connection errors and 'invalid response to SSL negotiation', looking like the stack 'died mid-run'. This was the root cause of the session-long test flakiness. After the stack is up, derive POSTGRES_PORT/ADCP_SALES_PORT (and DATABASE_URL) from 'docker compose port' so the env file always matches reality. Verified: env POSTGRES_PORT now equals the actual host binding. * test(creative-assets): migrate uc006 message text assets to SDK 5.7 list shape The uc006 generative-prompt steps built message assets as bare role->dict ({"message": {"content": ...}}), which fails CreativeAsset parsing under the SDK 5.7 discriminated union. Those scenarios are currently xfailed (harness not wired for non-account scenarios), so the BDD run never caught the invalid shape; if the harness is wired the bare-dict sites would fail at request parsing. Route all message construction through make_text_assets (the canonical prebid#1391 factory), fix the CreativeSyncEnv docstring example, and add a unit regression that invokes the real step and parses its payload (plus covers the previously untested _extract_message_from_assets role-priority wrapper). Part of prebid#1391 SDK 5.7 creative-asset-shape migration. * test(creative-assets): migrate uc006 provenance asset shape to SDK 5.7 list The asset-level provenance step (BR-RULE-094 INV-5) indexed the asset slot as a bare dict (assets[role]["provenance"]=...), and its creative builder used a bare-dict image asset. Under SDK 5.7 an asset slot is a list, so the mutation TypeErrors on the list shape — masked today only because the INV-5 scenario is xfailed (production stores no asset-level provenance). Per the AdCP schema (v3.1-04f59d2d5, core/provenance.json: provenance attaches to individual assets, most-specific replaces inherited), asset-level provenance belongs on the individual asset object — assets[role][0]["provenance"]. Migrate the builder to make_image_assets and index the list element, and add a unit regression that runs the real step against the list shape. Part of prebid#1391 SDK 5.7 creative-asset-shape migration. * test(creative-assets): migrate uc006 INV-6 user assets to factory + fix stored-shape assertions The INV-6 generative-user-assets step hand-rolled a bare-dict image asset, and the two preservation Then-steps assumed flat-dict stored assets (whole-value exact-equal and .items() on the asset value). Verified via an integration probe that production stores the SDK 5.7 list shape {role: [asset_obj]} and enriches the object with null-default fields (format/alt_text/provenance), and preserves user assets over generative output (_processing.py:259-269). Route the image through make_image_assets, add a _first_asset_object helper that unwraps either valid AdCP 3.1 shape (single object for individual slots, list for multi-count slots), and switch both Then-steps to index that object + assert field containment. Strengthen the existing INV-6 integration test (was: only checks generative_build_result) to assert the user image survives with list-index + containment — the real verification surface, since the BDD INV-6 scenario is xfailed (harness not wired). Part of prebid#1391 SDK 5.7 creative-asset-shape migration. * test(creative-assets): add url asset factory helpers + single-object shape variants Adds make_url_assets (list) and the single-object individual-slot helpers make_image_asset / make_text_asset / make_url_asset, and routes the remaining hand-rolled banner_image+click_url asset dicts (test_adcp_contract.py x2, the uc006 _format_payload e2e branch) through them. AdCP 3.1 (creative-manifest @ v3.1.0-beta.3) lets a slot value be either a single asset object (individual slots) or a list of asset objects (multi-count slots, slots[].max > 1); the adcp 5.7 SDK accepts and round-trips both. The factory now provides both shapes so tests never hand-roll an asset dict, with a round-trip test pinning that both forms parse into CreativeAsset and the extractors read them. Part of prebid#1391 SDK 5.7 creative-asset-shape migration. * test(creative-assets): add AssetSpec build+verify mechanism; use flat shape for single-asset slots Introduces AssetSpec in the creative-asset factory: declare an asset once, then use the SAME spec to build the request mock (payload/build_assets) and to verify the stored or returned value (assert_in/assert_assets). The spec owns the AdCP 3.1 shape (single object for an individual slot, list for a multi-count slot) and the field-containment comparison, so step/test code never indexes [0], unwraps a RootModel, or re-implements containment. Apply it to uc006: the INV-6 Given builds from specs and the two preservation Then-steps collapse to a single assert_assets call (the _first_asset_object helper and the manual containment loops are deleted, DB fetch extracted to one helper). Single-asset slots (message, provenance image, INV-6 image) now use the flat single-object shape — which is the AdCP 3.1 individual-slot shape and removes the list-indexing the earlier commits added (e.g. asset-level provenance attaches directly to the object, no [0]). Part of prebid#1391 SDK 5.7 creative-asset-shape migration. * test(creative-assets): add video_spec + generic asset_spec to AssetSpec factory Completes the AssetSpec constructor set so every asset_type in the suite (image, text, url, video, plus the long tail: audio, vast, html, css, markdown, catalog, daast, javascript) can be declared via a spec — no test needs to hand-roll an asset dict. video_spec takes extra typed fields (e.g. duration) via kwargs; asset_spec(role, asset_type, **fields) covers any type. Part of prebid#1391 / AssetSpec migration (salesagent-we4b). * test(creative-assets): migrate unit creative tests to AssetSpec Routes all creative-asset construction in the unit suite through AssetSpec (image_spec/text_spec/video_spec/url_spec/asset_spec + build_assets), removing make_*_assets/make_*_asset/DEFAULT_IMAGE_ASSETS and inline asset dicts. Individual slots use the flat single-object shape; tests that exercise the SDK RootModel/list path or whose adapter-conversion behavior is shape-dependent keep multiple=True. Plain-dict extractor-argument tests (raw {url}/{content} passed straight to the extractors) are left as raw dicts — they test the extractor's dict path, not asset construction. Files: test_creative, test_adcp_25_creative_management, test_extract_url_from_assets, test_build_creative_data, test_creative_repository, test_creative_coverage_gaps, test_inline_creatives_in_adapters, test_sync_creatives_format_validation, test_format_templates, test_rootmodel_asset_unwrapping. Part of AssetSpec migration (salesagent-we4b). * test(creative-assets): migrate integration creative tests to AssetSpec Routes all creative-asset construction in the integration suite through AssetSpec and replaces manual stored-asset inspection (db.data['assets'] indexing/[0]/.root/ field loops) with assert_assets reusing the build specs. Notably the user-asset preservation checks (test_creative_sync_processing, test_creative_sync_transport, test_creative_sync_data_preservation) now verify via assert_assets with the same spec used to build — strengthened with explicit negative checks that generative output did not overwrite user assets. Verified serially against real Postgres: 304 passed, 3 xfailed (pre-existing async-lifecycle), 0 failed. Part of AssetSpec migration (salesagent-we4b). * test(creative-assets): migrate helpers, factories, uc006 remainder, contract, e2e to AssetSpec Routes the remaining creative-asset construction through AssetSpec: - shared helpers/factories (creative_test_helpers, adcp_factories, factories/creative) build via build_assets (list shape preserved for broad consumers) - all remaining uc006 step sites -> flat individual-slot specs (consistent with the already-migrated INV-6/provenance/message sites); import block is spec-only - test_adcp_contract banner_image+click_url via image_spec/url_spec - test_creative_asset_factory_shapes rewritten to exercise the AssetSpec mechanism itself (both flat and list shapes parse + extract + assert_assets round-trip) - message/provenance regression tests + e2e a2a compliance via specs Verified: full unit 4888/0; uc006 BDD 52/876xfail/0; shared-helper integration consumers (creative_v3, list_creatives_auth, uc006_proceed, creative_repository) green. Part of AssetSpec migration (salesagent-we4b). * test(creative-assets): migrate hand-rolled manifest-value inline dicts to AssetSpec Migrates the remaining inline creative-manifest asset VALUE dicts that never used the make_* helpers (assets={role: {asset_type, url/content, ...}} / list form) to build_assets + image/text/video/url specs, across unit (test_format_id, test_sync_creatives_auth, test_sync_creatives_async_fix, test_all_response_str_methods, test_auth_requirements, test_update_media_buy_behavioral), integration (test_a2a_skill_invocation, test_schema_contract_validation, test_mcp_tool_roundtrip_minimal, test_a2a_response_message_fields), e2e (adcp_request_builder), and the CreativeSyncEnv docstring. Out of scope (documented in salesagent-we4b): format-requirement arrays ({item_type,asset_id,asset_type,required} with no value — they describe a format, not a manifest asset) and validator negative-tests (deliberately malformed shapes). Verified: full unit 4888/0; F3 integration files 56 passed. Part of AssetSpec migration (salesagent-we4b). * test(creative-assets): remove make_* asset helpers (AssetSpec is the sole mechanism) All call sites now use AssetSpec, so delete make_image_assets/make_video_assets/ make_text_assets/make_url_assets, make_image_asset/make_text_asset/make_url_asset, and DEFAULT_IMAGE_ASSETS. CreativeAssetFactory.assets now builds via build_assets(image_spec('banner', multiple=True)). Kept make_legacy_*, make_creative_asset_minimal, and CreativeAssetFactory. grep confirms zero references to the removed names across tests/ and src/. Full unit suite green (4888 passed, 0 failed). Part of AssetSpec migration (salesagent-we4b). * test(creative-assets): fix code-review findings — flat shape consistency + format_templates coverage + INV-6 assertions Code review (consistency/testing/DRY) on the AssetSpec migration: - Consistency: flip 34 single-individual-slot assets from multiple=True (inertia from the old plural make_*_assets) to flat, so every individual slot is the AdCP 3.1 single-object shape repo-wide. assert_assets is shape-agnostic; no production change. - test_format_templates (4): multiple=True made _convert_creative_to_adapter_asset silently skip the asset branch (it only processes isinstance(dict)). Switched to flat dims-less asset_spec so the converter's url branch is exercised again AND the format-id dims still win (asset carries no dims to override). Restores lost coverage. - TQ-01: corrected then_user_assets_preserved docstring (containment, not deep-equal). - TQ-02: then_user_assets_priority_over_generated now adds a real negative check (no generated asset role leaks into the stored creative), distinct from mere preservation. Verified: unit 4888/0; flipped integration files 84/0; uc006 BDD 52/876xfail/0. Part of AssetSpec migration (salesagent-we4b). * test(creative-assets): DRY — shared stored-asset assertion + creative payload builder Code-review DRY findings: - DRY-02: extract assert_stored_creative_assets(creative_id, *specs, tenant_id=None) in creative_test_helpers; replace the open-coded fetch-creative-then-assert_assets block in test_creative_sync_{data_preservation,processing,transport} (negative checks preserved). - DRY-01: add shared creative_payload(**overrides) builder; the 4 near-identical local _creative skeletons (async_lifecycle/processing/transport/generative) now delegate to it, keeping their file-specific defaults. Verified: affected integration files 150 passed, 3 xfailed (pre-existing). Part of AssetSpec migration (salesagent-we4b). * test(creative-assets): flat shape for incidental banner slot in no-name test Straggler the consistency review missed: test_creative.py no-name failure test used an incidental single banner image slot with multiple=True; flip to flat (individual slot). Part of AssetSpec migration (salesagent-we4b). * fix: keep original task_type label out of webhook wire fallback The SDK fallback that coerces task_type to a TaskType enum member was rewriting the single task_type variable in place, so the coerced wire value leaked into metadata['task_type'] — the internal label consumed by protocol_webhook_service for the audit log, the delivery-webhook guards, and the WebhookDeliveryLog.task_type DB column (salesagent-yi3s, latent). Extract a shared validate_webhook_task_type() helper into webhook_validator.py: it validates a COPY for the SDK payload while the caller keeps the original action label for metadata. Apply it at all four untrusted-input webhook sites for symmetry (salesagent-yk7o): context_manager._send_push_notifications and the three admin sites (creatives sync, operations approve/reject) that pass workflow_steps .tool_name straight to create_mcp_webhook_payload. Regression test drives the real _send_push_notifications path and asserts the wire/label split; shared push-notification test fixtures extracted to _push_notification_helpers.py to keep the duplication baseline flat. * refactor: converge enum normalization on single enum_value helper Delete resolve_enum_value from validation_helpers.py and migrate its callers (adapters/base.py, services/targeting_capabilities.py) plus the inlined duplicate in tests/bdd/steps/generic/then_success.py to the superset enum_value (handles None). Guard the None case at the targeting_capabilities call site so the str|None contract is safe. Add test_enum_helpers.py to lock the superset contract. * test(architecture): make bare-BaseModel guard meaningful + broaden matcher The no-bare-BaseModel guard matched zero sites, had no self-test (so its green was meaningless), and its matcher only modeled ast.Name id=='BaseModel' — missing pydantic.BaseModel (ast.Attribute) and RootModel. It also scanned only src/core/schemas/, missing the bare-BaseModel REST request bodies in src/routes/api_v1.py. - Add 7 inline-snippet self-tests: bare BaseModel (name + attribute) and RootModel (name + attribute) are detected; proper SalesAgentBaseModel/ Library subclasses and bare classes with explicit model_config are NOT. - Broaden the matcher to handle ast.Attribute (pydantic.BaseModel) and ast.Subscript (RootModel[...] / pydantic.RootModel[...]). - Widen scan scope to include src/routes/api_v1.py alongside the schemas package; document why the scope is curated (client-facing AdCP boundary) rather than whole-src. - Allowlist the 11 api_v1 *Body classes with FIXME(salesagent-zyq0) markers at each source location. Migrating them to SalesAgentBaseModel changes REST extra-field handling (Pattern #7) and needs integration coverage — tracked as a follow-up. * docs(exceptions): correct auth-error recovery rationale for adcp 5.7 The AdCPAuthenticationError and AdCPAuthorizationError docstrings cited the adcp 4.3 STANDARD_ERROR_CODES table, called AUTH_TOKEN_INVALID a 'standard SDK code', and claimed terminal recovery 'matches the installed SDK validator'. After the adcp 5.7 bump these are stale: 5.7's STANDARD_ERROR_CODES carries only AUTH_REQUIRED (recovery='correctable') and has no AUTH_TOKEN_INVALID entry. The wire value is unchanged: recovery 'terminal' is a hardcoded _default_recovery ClassVar inherited from AdCPError, never read from STANDARD_ERROR_CODES. Reword the rationale to 5.7 framing — recovery is intentionally terminal because the AdCP 3.1 storyboards grade the wire error code, not the recovery class. Update the matching comments in test_mcp_error_envelope.py and test_a2a_error_responses.py. Docs-only: no assertions or wire behavior changed. * test(creative): re-add context/ext field assertions to async submitted test test_async_submitted_response dropped the assertions that 'context' and 'ext' exist on SyncCreativesSubmitted when it was adapted to assert the now-required task_id. Both fields remain present on adcp 5.7 (context optional, ext optional), verified via model_fields. Re-add the assertions alongside the task_id checks to preserve coverage of those schema fields. * fix(schemas): declare creative_deadline on CreateMediaBuySuccess for parity adcp 5.7 dropped creative_deadline from the parent CreateMediaBuySuccess type, but adapters/base.py still emits it via _build_create_success (default now+2d). With extra='allow' it only reached the wire in prod (extra=ignore); in dev/test (extra=forbid) it was dropped, creating a prod/dev parity gap and leaving the field untyped. Declare creative_deadline: datetime | None = None beside account and sandbox so the field is typed and survives serialization in all environments. datetime is already imported in this module. * test: suppress cosmetic Product RootModel-union serializer warning (salesagent-myry) The GetProductsResponse nested-serializer override emits a noisy pydantic UserWarning ('Pydantic serializer warnings: PydanticSerializationUnexpectedValue Expected Product1/Product2') from the RootModel anyOf arm. The wire output is correct (handled by the model_dump override in src/core/schemas/product.py). Add a targeted filterwarnings ignore narrowed by message regex + UserWarning category + pydantic.main module so the noise is suppressed while a genuine future serializer regression still surfaces. Validated by tests/unit/test_warning_filters.py. * refactor: migrate test/doc call sites to get_adcp_spec_version() get_adcp_version() is deprecated in adcp 5.7. Swap the 3 remaining test call sites and 1 doc reference to get_adcp_spec_version(), which returns the same '3.1.0-beta.3' spec string. Production already uses the new name (adcp_a2a_server.py, webhook_delivery_service.py, landing_page.py). - tests/unit/test_webhook_delivery_service.py: import + assertion - tests/e2e/test_a2a_endpoints_working.py: import + 2 call sites - src/a2a_server/README.md: doc reference * chore: reference GitHub prebid#1442 in api_v1 bare-BaseModel FIXMEs Allowlist FIXME markers for the deferred *Body migration should point at a GitHub issue (resolvable by any contributor), not a local beads id. Track via prebid#1442. * docs: FIXME allowlist markers reference GitHub issues, not beads ids Beads ids are local to our tracker and don't resolve for outside contributors reading the code. Align the guard-allowlist convention in CLAUDE.md and the structural-guards / patterns-reference docs to use # FIXME(#<gh-issue>). * chore: mark local_schema_imports allowlist sites with FIXME(prebid#1360) The 12 allowlisted raw-SDK-type imports were only tracked in the guard test; add # FIXME(prebid#1360) at each source import site for traceability, per review of prebid#1399. No behavioral change. * chore: repoint local_schema_imports FIXME refs prebid#1360 -> prebid#1388 prebid#1360 is a closed, unrelated PR (Feature/e2e test). SDK-type-import-bypass tracking belongs to prebid#1388 (adcp 4.3->5.7 upgrade). Repoint the allowlist comment block and all 9 source-site markers. No behavioral change. * test: scope Product serializer filterwarnings to the get_products tests The suite-wide pytest.ini filter matched every PydanticSerializationUnexpected warning (all share the 'Pydantic serializer warnings:' prefix from pydantic.main), silencing real serializer regressions in any model. Confirmed via -W error that the only emitter is test_response_shapes.py's get_products serialization tests; scope the ignore there with @pytest.mark.filterwarnings so other models still surface. See prebid#1388. * refactor: address PR prebid#1388 review nits (type annotation, docstring, imports) - _normalize_action_to_str annotated str | None to match enum_value's real return - correct stale TestTolerantPerFormatIngestion docstring (passthrough, not dropped) - remove orphaned ctx[expected_legacy_fields] write (dead after consumer removed) - hoist enum_value imports to zero-dep src.core.enum_helpers (drop lazy in-body import) nit4 (uc006 always-true leak assertion) tracked separately — shared generative mock. * test: pin schema-alignment to vendored adcp@04f59d2d5, fail-not-skip The alignment test fetched /schemas/latest and cached into a gitignored dir, so on CI (no cache) the two relocated request schemas (sync/list-creatives, moved to creative/) 404'd and silently skipped, while locally a stale cache passed against the pre-3.1 shape. /schemas/latest also drifts as upstream ships. Pin to the frozen reference commit adcontextprotocol/adcp@04f59d2d5 (tag v3.1-04f59d2d5): vendor the 118-file transitive $ref closure of the mapped request schemas (tests/fixtures/adcp_schemas_pinned/) and read them offline. A missing schema is now a hard failure, never a skip. Re-derived KNOWN_SCHEMA_LIBRARY_MISMATCHES against the pinned schemas — all collapse to empty. _refresh.py documents provenance + refresh. Closes the F1 finding from PR prebid#1388 review. * fix(schemas): declare valid_actions + context on CreateMediaBuySuccess The pinned AdCP schema (adcontextprotocol/adcp@04f59d2d5, create-media-buy-response.json success variant) defines both valid_actions and context. Production emits them, but they were undeclared on the model — surviving only via the library parent's inherited extra='allow', so a parent extra-mode flip would silently drop them. Declare them typed (list[MediaBuyValidAction] / ContextObject), consistent with account/sandbox/creative_deadline. Test asserts the contract against the vendored pinned schema (the authority), then the model's wire output, then the declaration. Vendors create-media-buy-response into the pinned fixture. Addresses F4 from PR prebid#1388 review. * fix(schemas): enforce required fields on SyncResponseAccount; ground response models in alignment framework Per the pinned AdCP schema (adcontextprotocol/adcp@04f59d2d5, sync-accounts-response success accounts.items.required), brand/operator/action/status are required. SyncResponseAccount declared them optional, leaving required-ness to every call site. Make them required (billing stays optional). Verified runtime-safe: sync_accounts integration + uc011 BDD + create_media_buy integration all green. Move the F4/F5 schema-conformance grounding out of hand-rolled per-test json.loads in the contract tests and into test_pydantic_schema_alignment.py::TestResponseModelAlignment, reusing the framework's pinned loader. Generic checks cover declared-fields (F4) and required-enforcement (F5); vendors sync-accounts-response into the pinned fixture. Addresses F5 from PR prebid#1388 review. * fix: reject brandless sync_accounts entry with VALIDATION_ERROR not 500 (PR1399 R3-F1) SDK 5.7's SyncAccountsRequest.accounts is list[Accounts | Accounts3]; the Accounts3 (account-reference / settings-update) arm makes brand optional, so a brandless entry parses with brand=None. _extract_natural_key dereferenced brand.domain unguarded → AttributeError → INTERNAL_ERROR/500 (SERVICE_UNAVAILABLE on A2A). The pinned 3.1 spec (sync-accounts-request.json @ v3.1-04f59d2d5) marks every entry required:[brand,operator,billing], so a brandless entry must be a clean buyer-correctable 400. Production: - Guard _extract_natural_key: raise AdCPValidationError (VALIDATION_ERROR/400/ correctable) when brand is None, before dereferencing brand.domain. Tests (boundary-to-boundary): - Focused regression TestSyncAccountsBrandlessEntryRejected [a2a, rest] asserts the two-layer wire envelope (VALIDATION_ERROR, recovery=correctable). - Hand-authored companion BR-UC-011-account-validation.feature scenario across all 4 transports: IMPL/A2A/REST assert the AdCP envelope; MCP rejects at the FastMCP schema boundary (tool surface types accounts as list[Accounts], brand required) — assert the ToolError names the missing brand field. - _dispatch.py additively captures wire/synthesized envelopes into ctx. - Structural guard test_architecture_account_brand_guard.py pins the extractor guard (positive + negative meta-tests). - bdd-traceability.yaml: T-UC-011-sync-brandless entry. * fix: require SyncCreativesResponse.creatives per pinned 3.1 (PR1399 R3-F2) The pinned 3.1 spec (sync-creatives-response.json @ v3.1-04f59d2d5) defines a oneOf of 3 variants; the success variant SyncCreativesSuccess is required:['creatives'] — its only required field. Per-item failures (action='failed') live INSIDE the success variant, so a synchronously-processed sync always carries a creatives array (even all-failed) and never collapses to the error variant. The '= []' default on the model permitted SyncCreativesResponse() to build an under-specified, spec-forbidden shape (same defect class as F5 on SyncAccountsResponse.accounts). Model: - creative.py: remove the '= []' default → creatives is required. Guard (class-level, the canonical schema-alignment mechanism): - Vendor sync-creatives-response.json (+ enums/creative-action.json) into the pinned fixtures (_refresh.py ROOTS). - Add a RESPONSE_ALIGNMENTS entry for SyncCreativesResponse so test_required_fields_enforced pins creatives-required (constructs full sample; omitting creatives must raise). Superseded by the auto-generated alignments ticket. Tests (boundary-to-boundary): - Unit RED→GREEN: test_creatives_is_required asserts SyncCreativesResponse() raises ValidationError. - Hand-authored companion BR-UC-006-creatives-invariants.feature: all-failed sync still returns the success variant carrying creatives[], every per-item action='failed', no operation-level errors[] — across all 4 transports. conftest UC-006 harness wired for the @creative-invariant tag. - bdd-traceability.yaml: T-UC-006-all-failed-success-variant entry. No caller patching needed: production and existing tests already pass creatives. * feat: auto-generate RESPONSE_ALIGNMENTS from pinned schema (PR1399 Plan B backstop) RESPONSE_ALIGNMENTS in test_pydantic_schema_alignment.py is the executable, schema-grounded check that no local response model silently drops a field the pinned 3.1 spec marks required. It was hand-curated (3 entries ≈ 7% of the implemented success arms); F4/F5/Chris-#2 were all one-at-a-time discoveries of entries this layer should already contain. Make it machine-complete so the bug class cannot recur silently. Generator (test_pydantic_schema_alignment.py): - _RESPONSE_MODEL_REGISTRY: the 11 AdCP-grounded response models the seller implements (extend a Library* base, map to a pinned *-response.json). - _success_arm: oneOf → the arm whose required[] names neither errors nor task_id; flat schema → itself. - _build_alignments_from_pinned: one envelope alignment per registered model, declared_fields = required − version (focused on the required-field bug class; optional forward-compat fields are not force-declared), sample synthesized from required (sample_override for complex shapes). Plus _SUPPLEMENTAL_ALIGNMENTS keeps the per-item SyncResponseAccount F5 entry. - RESPONSE_ALIGNMENTS is now generated, not a literal. A required field added to any registered model's pinned arm is enforced with no hand-edit. - New coverage gate: every implemented response model must have an alignment. Model fixes the generator surfaced (real F5 bugs): - GetProductsResponse.products: redeclared required (SDK base had list|None). - ListAccountsResponse.accounts: removed '= []' default → required. - schema-inheritance guard: allow the GetProductsResponse.products required-tightening redefinition. Vendored all implemented operation response schemas (+ transitive $ref closure) into tests/fixtures/adcp_schemas_pinned via _refresh.py ROOTS. Verified: alignment suite green (coverage + per-field enforcement); a temporary model relaxation makes the suite RED; full unit + products/accounts integration green; make quality green. * docs: correct auth-error docstring spec-grounding (PR1399 R3-F3) AdCPAuthenticationError's docstring said the default AUTH_TOKEN_INVALID is "per the AdCP 3.1 spec," but that code is in neither the pinned 3.1 error-code enum nor adcp 5.7 STANDARD_ERROR_CODES (both define only AUTH_REQUIRED) — and the next sentence already called it project-specific. Correct the wording: - AdCPAuthenticationError: AUTH_TOKEN_INVALID is project-specific; it reaches the wire by passthrough (deliberately absent from ERROR_CODE_MAPPING) on the sync transports (REST/MCP/A2A); the async webhook path enforces STANDARD_ERROR_CODES and would downgrade it to SERVICE_UNAVAILABLE. - AdCPAuthRequiredError: "per spec" → "project-specific code; see parent docstring". - test_error_envelope.py: drop the false "is itself a STANDARD code" clause; the passthrough reason (not in ERROR_CODE_MAPPING) and the assertion are unchanged. Wording-only — no wire or test-behavior change. make quality green. * chore: strip leaked beads IDs from PR1399 R3 code comments The 4 R3 fix commits leaked bare beads tokens (nkrn/j49n/fbdb) into 19 added lines across 16 files. Project rule: code comments use GitHub issue/PR references, never beads IDs (they do not resolve for outside contributors on a public repo). Lines already carrying a 'PR1399 R3-Fx' reference had the token stripped; lines bearing only the token were mapped to prebid#1399 R3-F1 / R3-F2 / Plan-B. * test: make RESPONSE_ALIGNMENTS coverage gate self-enumerating The coverage gate compared the registry against a hardcoded 11-model 'expected' set, so a future library-grounded response model that nobody registered would not be flagged (removing a model from both the registry and the literal kept the suite green). Replace the literal with _enumerate_grounded_response_models(), which walks src.core.schemas for every class defined there that extends an adcp library type and carries a response role (name ends Response/Success). The registry's own inclusion rule is now executable: an unregistered grounded model fails the gate. Assertion stays one-directional (enumerated - covered) because covered legitimately carries extra non-top-level alignments. * refactor: route hand-rolled enum->str normalizations through enum_value() Five sites hand-rolled the same logical operation enum_value() already provides (None->None; Enum->.value; str->passthrough) but expressed it differently — the 'semantically equivalent code expressed differently' the DRY invariant targets: - schemas/creative.py: status.value-if-isinstance ternary - tools/creative_formats.py: asset_types .value-if-isinstance comprehension (the same enum_value() call already existed 200 lines below) - tools/creatives/listing.py: fields .value-if-isinstance comprehension - tools/creatives/sync_wrappers.py: validation_mode truthiness ternary - tools/media_buy_create.py: str(delivery_type.value) wrap (line 805 in the same function family already used enum_value) Each migration is behavior-preserving (byte-identical for every input the call site can receive). A new structural guard scans all of src/core so a future re-introduction in any file fails the gate, plus behavioral characterization pins the input-form independence. The str(plain-Enum) latent bug at media_buy_create.py:2687 is intentionally left out — migrating it changes behavior — and is tracked separately. * fix: normalize guaranteed product delivery_type in GAM auto-config When a product reaches the GAM implementation_config auto-generation path without a config, _create_media_buy_impl derived the delivery_type string via str(schema_product.delivery_type). DeliveryType is a plain Enum, so that yields 'DeliveryType.guaranteed', which fails generate_default_config's 'delivery_type == "guaranteed"' check — a guaranteed product silently got the non_guaranteed (PRICE_PRIORITY, priority 10) config instead of STANDARD. Route the value through enum_value() (already used for the same field elsewhere in this module) so the correct config arm is selected. Adds an integration regression that drives the real auto-config path with a GoogleAdManager-named adapter and asserts the guaranteed product receives a STANDARD line item. * chore: scrub residual beads-id tokens from R3 test docstrings The enum_value refactor + delivery_type regression test docstrings carried bare beads tokens (jkfl/u6r6); replace with PR1399 R3 refs per the project convention that code uses GitHub/PR references, not beads IDs. * chore(deps): bump aiohttp/cryptography/python-multipart/starlette for security advisories Fresh advisories (published after this branch last ran green) flag the versions currently on main too — not a regression from this PR, but they block the required Security Audit check: aiohttp 3.14.0 -> 3.14.1 (8 advisories) cryptography 46.0.7 -> 49.0.0 (GHSA-537c-gmf6-5ccf) python-multipart 0.0.28 -> 0.0.32 (3 advisories) starlette 1.2.1 -> 1.3.1 (2 advisories) uv-secure now reports zero vulnerabilities. Core deps import-verified (crypto/JWT/starlette/aiohttp/multipart + app modules).

…rebid#1312) * feat(idempotency): cache rejection envelopes for replay (B6, #1303 contract 7) AdCP spec requires that retrying a tool call with the same idempotency_key must return the original result without re-processing. Successful media buys are already idempotent via media_buys.idempotency_key. This commit closes the rejection-replay gap: if the original request was rejected, a retry must return the cached rejection envelope, not a fresh evaluation that could now produce a different answer (e.g. because a product was added or budget caps changed in the interim). Model + persistence: - New IdempotencyAttempt ORM model (tenant_id, principal_id, tool_name, idempotency_key, response_envelope JSONB, expires_at, created_at). - Unique index on (tenant_id, principal_id, tool_name, idempotency_key) — the same composite key the lookup uses; tool_name disambiguation so the same idempotency_key can be used independently across tools. - Secondary index on expires_at for the cleanup-job scan. - Alembic migration 097b909c7b5f with full upgrade + downgrade. Repository: - IdempotencyAttemptRepository.find_by_key — tenant-scoped, treats expired rows as absent so callers fall through to re-evaluation. - record_rejection — writes envelope + expiry stamp (default TTL 24h, matches the value announced via get_adcp_capabilities.adcp.idempotency.replay_ttl_seconds). - expire_old — periodic-cleanup scan; tenant-scoped so cross-tenant cleanup is impossible from a single repository. - Wired into MediaBuyUoW alongside media_buys + currency_limits. Hook in _create_media_buy_impl: - Lookup extension: after the MediaBuy idempotency check fails (no cached success), also check IdempotencyAttempt for a cached rejection. On hit, return the cached envelope verbatim via _build_idempotency_rejection_replay (re-hydrates CreateMediaBuyError, echoes current request's context). - Cache writes at 3 deterministic rejection sites: validation catchall, GAM config validation, missing start_time/end_time. Adapter errors are NOT cached (transient by nature). - _cache_rejection_envelope is no-op when idempotency_key is absent; race-safe via IntegrityError catch. Tests: - 11-case integration suite in tests/integration/test_idempotency_attempt_repository.py covering record_rejection, find_by_key (cached/missing/expired, tenant/principal/tool isolation), expire_old (selective + tenant-scoped). - Updated test_media_buy.py::test_idempotency_new_key_proceeds to mock the new uow.idempotency_attempts.find_by_key path. Drive-by pre-commit hygiene (same drift as PR #1276, fully covered there): - .type-ignore-baseline 42→60 (count drift via PRs bypassing the hook) - .pre-commit-config.yaml mypy adcp pin 3.2.0→4.3.0 (matches pyproject) Follow-up (separate commits): - BDD scenario in tests/bdd/features/local/ for AdCP contract 7 - Cache writes at remaining rejection sites - Periodic cleanup-job wiring for expire_old Closes part of #1303 (contract item 7 — idempotency replay after rejection). B6 of the inventory-targeting plan. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(idempotency): address Konstantine review (4 items) - Migration: switch response_envelope column from sa.JSON() to postgresql.JSONB() to match the ORM JSONType (which decays to JSONB on Postgres). The previous sa.JSON() type would have created a json (not jsonb) column, mismatching every other JSON column in the schema and disabling JSONB index support. - DRY: extract _cache_and_return_rejection() helper. Three sites in _create_media_buy_impl shared the same 9-line build-rejection + cache + return pattern; they now call the shared helper. - Adapter rejection caching gap: cache adapter-returned errors so replay returns the same answer, except for transient errors (rate-limit/service-unavailable/timeout) where the buyer's retry is meant to succeed. - Add cached-rejection replay feature-contract test (test_idempotency_cached_rejection_replayed) — previously the rejection-replay branch had only repository-level coverage with no test through _create_media_buy_impl. - Strip inline issue refs from idempotency_attempt repository docstring and two media_buy_create comments. - Schema drift: add if_catalog_version + if_pricing_version to the get-products-request allowlist; update test_offline_mode payload to include the now-required cache_scope field. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: add get-media-buy-delivery missing fields to schema allowlist time_granularity and include_window_breakdown are present in the upstream get-media-buy-delivery-request schema but not yet in the adcp 4.3.0 Python library — same drift class as the existing 'account' / 'reporting_dimensions' entries on this PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(models): hoist 3 lazy AdCPConfigurationError imports to module top The 3 decrypt-failure paths in Tenant (gemini_api_key, oidc_client_secret, gam_service_account_json) each lazy-imported AdCPConfigurationError. No circular dependency exists — exceptions.py doesn't import models.py. Konstantine has been flagging this pattern consistently; pattern-extract to keep #1312 cleanly within the convention. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(idempotency): cache rejection envelopes for replay (B6, #1303 contract 7) AdCP spec requires that retrying a tool call with the same idempotency_key must return the original result without re-processing. Successful media buys are already idempotent via media_buys.idempotency_key. This commit closes the rejection-replay gap: if the original request was rejected, a retry must return the cached rejection envelope, not a fresh evaluation that could now produce a different answer (e.g. because a product was added or budget caps changed in the interim). Model + persistence: - New IdempotencyAttempt ORM model (tenant_id, principal_id, tool_name, idempotency_key, response_envelope JSONB, expires_at, created_at). - Unique index on (tenant_id, principal_id, tool_name, idempotency_key) — the same composite key the lookup uses; tool_name disambiguation so the same idempotency_key can be used independently across tools. - Secondary index on expires_at for the cleanup-job scan. - Alembic migration 097b909c7b5f with full upgrade + downgrade. Repository: - IdempotencyAttemptRepository.find_by_key — tenant-scoped, treats expired rows as absent so callers fall through to re-evaluation. - record_rejection — writes envelope + expiry stamp (default TTL 24h, matches the value announced via get_adcp_capabilities.adcp.idempotency.replay_ttl_seconds). - expire_old — periodic-cleanup scan; tenant-scoped so cross-tenant cleanup is impossible from a single repository. - Wired into MediaBuyUoW alongside media_buys + currency_limits. Hook in _create_media_buy_impl: - Lookup extension: after the MediaBuy idempotency check fails (no cached success), also check IdempotencyAttempt for a cached rejection. On hit, return the cached envelope verbatim via _build_idempotency_rejection_replay (re-hydrates CreateMediaBuyError, echoes current request's context). - Cache writes at 3 deterministic rejection sites: validation catchall, GAM config validation, missing start_time/end_time. Adapter errors are NOT cached (transient by nature). - _cache_rejection_envelope is no-op when idempotency_key is absent; race-safe via IntegrityError catch. Tests: - 11-case integration suite in tests/integration/test_idempotency_attempt_repository.py covering record_rejection, find_by_key (cached/missing/expired, tenant/principal/tool isolation), expire_old (selective + tenant-scoped). - Updated test_media_buy.py::test_idempotency_new_key_proceeds to mock the new uow.idempotency_attempts.find_by_key path. Drive-by pre-commit hygiene (same drift as PR #1276, fully covered there): - .type-ignore-baseline 42→60 (count drift via PRs bypassing the hook) - .pre-commit-config.yaml mypy adcp pin 3.2.0→4.3.0 (matches pyproject) Follow-up (separate commits): - BDD scenario in tests/bdd/features/local/ for AdCP contract 7 - Cache writes at remaining rejection sites - Periodic cleanup-job wiring for expire_old Closes part of #1303 (contract item 7 — idempotency replay after rejection). B6 of the inventory-targeting plan. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(idempotency): address Konstantine review (4 items) - Migration: switch response_envelope column from sa.JSON() to postgresql.JSONB() to match the ORM JSONType (which decays to JSONB on Postgres). The previous sa.JSON() type would have created a json (not jsonb) column, mismatching every other JSON column in the schema and disabling JSONB index support. - DRY: extract _cache_and_return_rejection() helper. Three sites in _create_media_buy_impl shared the same 9-line build-rejection + cache + return pattern; they now call the shared helper. - Adapter rejection caching gap: cache adapter-returned errors so replay returns the same answer, except for transient errors (rate-limit/service-unavailable/timeout) where the buyer's retry is meant to succeed. - Add cached-rejection replay feature-contract test (test_idempotency_cached_rejection_replayed) — previously the rejection-replay branch had only repository-level coverage with no test through _create_media_buy_impl. - Strip inline issue refs from idempotency_attempt repository docstring and two media_buy_create comments. - Schema drift: add if_catalog_version + if_pricing_version to the get-products-request allowlist; update test_offline_mode payload to include the now-required cache_scope field. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: add get-media-buy-delivery missing fields to schema allowlist time_granularity and include_window_breakdown are present in the upstream get-media-buy-delivery-request schema but not yet in the adcp 4.3.0 Python library — same drift class as the existing 'account' / 'reporting_dimensions' entries on this PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(models): hoist 3 lazy AdCPConfigurationError imports to module top The 3 decrypt-failure paths in Tenant (gemini_api_key, oidc_client_secret, gam_service_account_json) each lazy-imported AdCPConfigurationError. No circular dependency exists — exceptions.py doesn't import models.py. Konstantine has been flagging this pattern consistently; pattern-extract to keep #1312 cleanly within the convention. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: ruff format post-rebase Files drifted from ruff style after rebase onto new main. No behavior change. * feat(idempotency): cache rejection envelopes for replay (B6, #1303 contract 7) AdCP spec requires that retrying a tool call with the same idempotency_key must return the original result without re-processing. Successful media buys are already idempotent via media_buys.idempotency_key. This commit closes the rejection-replay gap: if the original request was rejected, a retry must return the cached rejection envelope, not a fresh evaluation that could now produce a different answer (e.g. because a product was added or budget caps changed in the interim). Model + persistence: - New IdempotencyAttempt ORM model (tenant_id, principal_id, tool_name, idempotency_key, response_envelope JSONB, expires_at, created_at). - Unique index on (tenant_id, principal_id, tool_name, idempotency_key) — the same composite key the lookup uses; tool_name disambiguation so the same idempotency_key can be used independently across tools. - Secondary index on expires_at for the cleanup-job scan. - Alembic migration 097b909c7b5f with full upgrade + downgrade. Repository: - IdempotencyAttemptRepository.find_by_key — tenant-scoped, treats expired rows as absent so callers fall through to re-evaluation. - record_rejection — writes envelope + expiry stamp (default TTL 24h, matches the value announced via get_adcp_capabilities.adcp.idempotency.replay_ttl_seconds). - expire_old — periodic-cleanup scan; tenant-scoped so cross-tenant cleanup is impossible from a single repository. - Wired into MediaBuyUoW alongside media_buys + currency_limits. Hook in _create_media_buy_impl: - Lookup extension: after the MediaBuy idempotency check fails (no cached success), also check IdempotencyAttempt for a cached rejection. On hit, return the cached envelope verbatim via _build_idempotency_rejection_replay (re-hydrates CreateMediaBuyError, echoes current request's context). - Cache writes at 3 deterministic rejection sites: validation catchall, GAM config validation, missing start_time/end_time. Adapter errors are NOT cached (transient by nature). - _cache_rejection_envelope is no-op when idempotency_key is absent; race-safe via IntegrityError catch. Tests: - 11-case integration suite in tests/integration/test_idempotency_attempt_repository.py covering record_rejection, find_by_key (cached/missing/expired, tenant/principal/tool isolation), expire_old (selective + tenant-scoped). - Updated test_media_buy.py::test_idempotency_new_key_proceeds to mock the new uow.idempotency_attempts.find_by_key path. Drive-by pre-commit hygiene (same drift as PR #1276, fully covered there): - .type-ignore-baseline 42→60 (count drift via PRs bypassing the hook) - .pre-commit-config.yaml mypy adcp pin 3.2.0→4.3.0 (matches pyproject) Follow-up (separate commits): - BDD scenario in tests/bdd/features/local/ for AdCP contract 7 - Cache writes at remaining rejection sites - Periodic cleanup-job wiring for expire_old Closes part of #1303 (contract item 7 — idempotency replay after rejection). B6 of the inventory-targeting plan. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(idempotency): address Konstantine review (4 items) - Migration: switch response_envelope column from sa.JSON() to postgresql.JSONB() to match the ORM JSONType (which decays to JSONB on Postgres). The previous sa.JSON() type would have created a json (not jsonb) column, mismatching every other JSON column in the schema and disabling JSONB index support. - DRY: extract _cache_and_return_rejection() helper. Three sites in _create_media_buy_impl shared the same 9-line build-rejection + cache + return pattern; they now call the shared helper. - Adapter rejection caching gap: cache adapter-returned errors so replay returns the same answer, except for transient errors (rate-limit/service-unavailable/timeout) where the buyer's retry is meant to succeed. - Add cached-rejection replay feature-contract test (test_idempotency_cached_rejection_replayed) — previously the rejection-replay branch had only repository-level coverage with no test through _create_media_buy_impl. - Strip inline issue refs from idempotency_attempt repository docstring and two media_buy_create comments. - Schema drift: add if_catalog_version + if_pricing_version to the get-products-request allowlist; update test_offline_mode payload to include the now-required cache_scope field. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: add get-media-buy-delivery missing fields to schema allowlist time_granularity and include_window_breakdown are present in the upstream get-media-buy-delivery-request schema but not yet in the adcp 4.3.0 Python library — same drift class as the existing 'account' / 'reporting_dimensions' entries on this PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(models): hoist 3 lazy AdCPConfigurationError imports to module top The 3 decrypt-failure paths in Tenant (gemini_api_key, oidc_client_secret, gam_service_account_json) each lazy-imported AdCPConfigurationError. No circular dependency exists — exceptions.py doesn't import models.py. Konstantine has been flagging this pattern consistently; pattern-extract to keep #1312 cleanly within the convention. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: ruff format post-rebase Files drifted from ruff style after rebase onto new main. No behavior change. * feat(idempotency): cache rejection envelopes for replay (B6, #1303 contract 7) AdCP spec requires that retrying a tool call with the same idempotency_key must return the original result without re-processing. Successful media buys are already idempotent via media_buys.idempotency_key. This commit closes the rejection-replay gap: if the original request was rejected, a retry must return the cached rejection envelope, not a fresh evaluation that could now produce a different answer (e.g. because a product was added or budget caps changed in the interim). Model + persistence: - New IdempotencyAttempt ORM model (tenant_id, principal_id, tool_name, idempotency_key, response_envelope JSONB, expires_at, created_at). - Unique index on (tenant_id, principal_id, tool_name, idempotency_key) — the same composite key the lookup uses; tool_name disambiguation so the same idempotency_key can be used independently across tools. - Secondary index on expires_at for the cleanup-job scan. - Alembic migration 097b909c7b5f with full upgrade + downgrade. Repository: - IdempotencyAttemptRepository.find_by_key — tenant-scoped, treats expired rows as absent so callers fall through to re-evaluation. - record_rejection — writes envelope + expiry stamp (default TTL 24h, matches the value announced via get_adcp_capabilities.adcp.idempotency.replay_ttl_seconds). - expire_old — periodic-cleanup scan; tenant-scoped so cross-tenant cleanup is impossible from a single repository. - Wired into MediaBuyUoW alongside media_buys + currency_limits. Hook in _create_media_buy_impl: - Lookup extension: after the MediaBuy idempotency check fails (no cached success), also check IdempotencyAttempt for a cached rejection. On hit, return the cached envelope verbatim via _build_idempotency_rejection_replay (re-hydrates CreateMediaBuyError, echoes current request's context). - Cache writes at 3 deterministic rejection sites: validation catchall, GAM config validation, missing start_time/end_time. Adapter errors are NOT cached (transient by nature). - _cache_rejection_envelope is no-op when idempotency_key is absent; race-safe via IntegrityError catch. Tests: - 11-case integration suite in tests/integration/test_idempotency_attempt_repository.py covering record_rejection, find_by_key (cached/missing/expired, tenant/principal/tool isolation), expire_old (selective + tenant-scoped). - Updated test_media_buy.py::test_idempotency_new_key_proceeds to mock the new uow.idempotency_attempts.find_by_key path. Drive-by pre-commit hygiene (same drift as PR #1276, fully covered there): - .type-ignore-baseline 42→60 (count drift via PRs bypassing the hook) - .pre-commit-config.yaml mypy adcp pin 3.2.0→4.3.0 (matches pyproject) Follow-up (separate commits): - BDD scenario in tests/bdd/features/local/ for AdCP contract 7 - Cache writes at remaining rejection sites - Periodic cleanup-job wiring for expire_old Closes part of #1303 (contract item 7 — idempotency replay after rejection). B6 of the inventory-targeting plan. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(idempotency): wire idempotency_key through MCP/A2A/REST + replay tests Wrappers for create_media_buy did not declare idempotency_key as a parameter, so FastMCP's TypeAdapter rejected it on the wire and the replay-after-rejection feature was dead code end-to-end. Add idempotency_key to: - MCP create_media_buy wrapper (forwarded to CreateMediaBuyRequest) - create_media_buy_raw (A2A entry point + forwarded to request) - A2A _handle_create_media_buy_skill (read from params, forwarded to core) - REST CreateMediaBuyBody (forwarded to create_media_buy_raw) Add tests/integration/test_idempotency_replay.py covering three layers: - _build_idempotency_rejection_replay re-hydrates cached dict envelope - _cache_rejection_envelope writes a retrievable IdempotencyAttempt row - _create_media_buy_impl replays cached rejection envelope through the production entrypoint (test Konstantine asked for in PR #1312 review) Also includes ruff-format drift fixes pulled in by the rebase. * chore(idempotency): apply 4 pre-review nits from cross-PR pattern audit Applies P6/P11/P13/P3 patterns Konstantine repeatedly flags on other PRs: - P6: hoist redundant IntegrityError import in _cache_rejection_envelope (it's already imported at media_buy_create.py:18, the inline copy was duplication) - P11: include tenant_id and principal_id in both the race-info log and the exception log on _cache_rejection_envelope so failures are traceable without grepping for the key alone - P13: switch the two _impl-replay integration tests to construct identity via PrincipalFactory.make_identity() instead of inline ResolvedIdentity, matching the single-source-of-truth convention in tests/CLAUDE.md - P3: document that IdempotencyAttemptRepository.expire_old has no production caller yet — replay correctness is unaffected (find_by_key filters on expires_at), but a periodic cleanup task is a follow-up required to bound storage growth * test(idempotency): wire-path coverage for idempotency_key through MCP/A2A/REST wrappers Closes Konstantine's 2026-05-24 review blocker on PR #1312. The existing 21 idempotency tests all exercise _create_media_buy_impl directly, so they stayed green when the wrappers silently dropped idempotency_key via FastMCP's TypeAdapter (the bug fixed by 1fb34dbc). If a future change drops idempotency_key from any wrapper signature, the silent feature death now has a regression guard. Three new tests in tests/integration/test_idempotency_replay.py TestWirePathReplay class, mirroring the rejection-seed pattern from the existing test_cached_rejection_returned_on_replay: - test_mcp_wire_replays_cached_rejection: seeds rejection, calls Client(mcp).call_tool("create_media_buy", {..., idempotency_key}), asserts the cached envelope (status=failed, errors[0].code= VALIDATION_ERROR, errors[0].message byte-identical to cached message) on the MCP wire. - test_a2a_wire_replays_cached_rejection: same seed, calls handler.on_message_send with skill_name=create_media_buy, asserts cached envelope shape in the artifact's DataPart. - test_rest_wire_replays_cached_rejection: same seed, posts to /api/v1/media-buys via Starlette TestClient, asserts the cached envelope in the response body. Mutation-verified (each mutation applied, test re-run, restored): - MCP: drop idempotency_key from CreateMediaBuyRequest at media_buy_create.py:4073 → test FAILS (impl bypasses replay, falls through to budget validation: "Invalid budget: 0.0"). - A2A: change idempotency_key=params.get("idempotency_key") to None at adcp_a2a_server.py:1543 → same FAIL signature. - REST: change idempotency_key=body.idempotency_key to None at api_v1.py:237 → same FAIL signature. Verification: - 3 new tests pass in fresh env with DB up - Full integration: 1887 passed, 0 failures - src/ untouched after restore Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: reconcile ratchet baselines + black-format after main merge The --no-verify merge commit skipped the hooks, so this applies the black formatting black wanted on the merge-combined files, and reconciles two ratchet baselines that main had let drift below its own tree: - .type-ignore-baseline 60 -> 61 (main's src/core/schemas/account.py carries 2 type:ignores; main's baseline was stale at 60). - .duplication-baseline tests 100 -> 101 (main's #1335 test additions added a duplicate test-setup block its baseline didn't track). No #1312 code introduces new type:ignores or duplicate blocks. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(idempotency): do not cache transient adapter rejections The adapter-rejection caching guard compared a Recovery enum member against the string "transient" (`recovery != "transient"`), which is always True — Recovery is a plain Enum, not a str-mixin — so transient adapter failures (rate limit / service unavailable / timeout) were cached for the 24h TTL, the opposite of the documented intent. A buyer retrying a transient failure then got the stale cached rejection instead of a fresh attempt. Compare on the enum's value so the skip actually fires. Regression coverage in TestTransientRejectionNotCached. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(idempotency): rebuild wire tests on the MediaBuyCreateEnv harness Review 3: the three wire tests used per-test MagicMock/patch/dependency_overrides scaffolding. Rebuilt on the MediaBuyCreateEnv harness driving call_via through the real MCP / A2A / REST pipelines with the real auth chain — call_a2a/call_mcp now delegate to the base _run_a2a_handler/_run_mcp_client (no duplication). The env had shipped non-functional (missing setup_product_chain / _build_mock_context_manager); completed it for the rejection-replay paths and documented the success-create FK limitation. Added assert_replayed_rejection (asserts on the success-envelope payload, since a replayed rejection returns status=failed without raising) and TestTransientRejectionNotCached covering the transient-skip fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(harness): drive the real A2A auth chain in _run_a2a_handler In integration mode (identity carries a real auth_token), populate ServerCallContext.state with the AuthContext the SDK call-context builder would build from the wire, and run the real _get_auth_token → _resolve_a2a_identity → resolve_identity (token → DB → ResolvedIdentity) chain instead of patching _resolve_a2a_identity / _get_auth_token. This is the A2A analogue of _run_mcp_client's get_http_headers seam. Unit mode (no real token) keeps the direct-identity injection unchanged. Verified across the harness self-tests and the full BDD suite (A2A parametrized across every use case) — no env regressed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(media-buy): wire account reference through create_media_buy wrappers create_media_buy consumes req.account at the boundary via enrich_identity_with_account, but no wrapper declared or forwarded account, so it was silently dropped on all three transports (the same TypeAdapter strip that left idempotency_key dead end-to-end). Declare account on the MCP and A2A-raw wrappers (SDK AccountReference type), forward it in the A2A skill, and coerce + pass it in the REST route; add CreateMediaBuyBody.account. New wire tests send a reference to a nonexistent account through each transport and assert ACCOUNT_NOT_FOUND, which only fires if the reference reached the boundary resolver. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(media-buy): preserve field + recovery in the validation-error wire mapper _validation_error_to_adcp_error dropped _StructuredValidationError.field and .recovery when building the wire Error. field is set at the PRODUCT_NOT_FOUND and targeting raise sites; recovery is always set. Carry both through so the buyer learns which input was rejected and whether a retry can fix it. Both the rejection-cache and synchronous-return paths use this mapper, so they stay identical; suggestion stays in details (existing convention). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(idempotency): typed AdCPIdempotencyConflictError / AdCPIdempotencyExpiredError Add the two typed AdCPError subclasses for the spec idempotency failure modes: - AdCPIdempotencyConflictError (extends AdCPConflictError): 409, IDEMPOTENCY_CONFLICT, recovery=terminal — idempotency_key reused with a different canonical payload. - AdCPIdempotencyExpiredError: 410, IDEMPOTENCY_EXPIRED, recovery=terminal — replay attempted past the retention window. Both wire codes are already in the SDK STANDARD_ERROR_CODES (terminal recovery per server/helpers.py), so no ERROR_CODE_MAPPING entry is needed; they auto-register into the derived wire-code -> HTTP-status table. Tested: status/code/recovery defaults plus the two-layer wire envelope (both adcp_error and errors[0] layers). Foundation for the return-based rejection-replay -> raise conversion. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(idempotency): payload_hash column + RFC 8785 canonical hasher Substrate for IDEMPOTENCY_CONFLICT detection (replay with the same key but a different canonical payload): - src/core/idempotency_canonical.py: salesagent-native RFC 8785 JCS + SHA-256 hasher over the request with the spec's CLOSED exclusion set (idempotency_key, context, governance_context; nested push_notification_config.authentication. credentials). Mirrors the AdCP spec (security.mdx#idempotency) without importing the SDK's return-based server-idempotency module. - idempotency_attempts.payload_hash (nullable String(64)) + migration 1d9b1402eacb; IdempotencyAttemptRepository.record_rejection accepts payload_hash. - rfc8785 promoted to a direct dependency (was transitive via adcp). Wired by the rejection-replay raise conversion (follow-up): a conflict is the same key with a stored payload_hash != the incoming request's hash. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(idempotency): surface replayed in the two-layer error envelope Add a `replayed` instance attr to AdCPError (default False) and have the single build_two_layer_error_envelope chokepoint emit envelope-level `replayed: true` when set. One edit covers all three transports (MCP/A2A/REST) since they all funnel error serialization through this builder. Per the spec ProtocolEnvelope.replayed, the flag is present (true) only on a cached idempotency replay and omitted on fresh executions. idempotency_key is NOT echoed: it is not an envelope field in the schema (it appears only inside the replayed field's description), so the buyer's own key is not mirrored back. The rejection-replay path sets exc.replayed=True before raising (follow-up); the success-replay path sets replayed on its result separately (follow-up). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(idempotency): convert rejection-replay to raise-based (Pattern A) + migrate tests Rejections in create_media_buy's idempotency paths now RAISE typed AdCPError through the transport boundary instead of returning a failed-status CreateMediaBuyResult, so a live reject and a same-key replay emit a byte-identical two-layer wire envelope (the replay additionally carries replayed=true). media_buy_create.py: - Entry computes the canonical payload_hash via idempotency_canonical. canonical_request_hash (the model_dump stays in the canonicalizer so _impl remains model_dump-free); the rejection-replay lookup raises IDEMPOTENCY_CONFLICT when the stored hash differs from the incoming request, else reconstructs the cached error via AdCPError.synthesize and re-raises it with replayed=True. - The deterministic reject points (early-validation except, GAM-config, missing start/end time) and the adapter-rejection branch cache-then-raise; transient adapter errors raise WITHOUT caching (the buyer's retry is meant to succeed). - Deleted _validation_error_to_adcp_error and the _cache_and_return_rejection return machinery; _cache_rejection_envelope now takes the exception + payload_hash and stores the single-layer {errors, context} projection. The no_error_construction cap shrinks 3 -> 1 (the one remaining Error(code=) site is the pre-existing principal-not-found AUTH_REQUIRED return, which is main's established contract and out of this PR's idempotency scope). Tests: - assert_replayed_rejection inverted to assert the raised two-layer wire envelope (replayed=true) via assert_envelope_shape; CANONICAL_ERROR_CODES gains the two IDEMPOTENCY_* codes. - test_idempotency_replay.py, the mock-only test_media_buy.py replay test, and the adapter-failure behavioral test migrated to pytest.raises / wire-envelope assertions; adapter tests use a future flight window so the request reaches the adapter. - McpDispatcher reads the reconstructed AdCPError's _wire_error_envelope (parity with A2A) so MCP replay envelopes surface for assertion. Validated on a real DB: 12 idempotency integration tests + the create-media-buy unit suites pass. Deferred follow-ups: success-path CONFLICT (needs MediaBuy.payload_hash) and the success-replay replayed marker (needs success-schema support). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(schema-alignment): allowlist get-products push_notification_config (adcp 4.3.0 drift) The live AdCP get-products-request schema declares push_notification_config; pinned adcp 4.3.0 GetProductsRequest does not have it yet (verified via model_fields). Add it to KNOWN_SCHEMA_LIBRARY_MISMATCHES — the designed pin-vs-latest-drift allowlist (same mechanism this branch already used for get-media-buy-delivery in 57defe470) — so the network-gated test_pydantic_schema_alignment cases pass. Pre-existing on any adcp-4.3.0 branch; not introduced by this PR's idempotency work. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(media-buy): restore account/idempotency_key wrapper threading lost in merge The main sync adopted main's create_media_buy wrappers, which (unlike the branch's) did not thread account or idempotency_key into CreateMediaBuyRequest: enrich_identity_with_account became a no-op and idempotency never fired on the A2A/REST raw path, while the merged A2A handler still passed both kwargs, raising TypeError. Restore the threading on the MCP and raw wrappers plus the AccountReference import. Also reformats the merged tests/harness/_base.py (ruff). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(idempotency): top-level replayed marker on CreateMediaBuyResult Adds replayed: bool, injected at the top level of the serialized result (beside status) and omitted when False so fresh responses stay byte-identical. This is the AdCP 3.0.1 idempotency envelope marker, set only on a verbatim replay of a cached success and never stored in the cached body; it lands top-level across MCP/A2A/REST, which all serialize through CreateMediaBuyResult._serialize. Also drops the superseded rejection-replay unit test (the cached-rejection behavior was removed by the main sync). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(idempotency): verbatim success-cache substrate (model + migration + repo) Repurposes idempotency_attempts as a verbatim SUCCESS cache per AdCP 3.0.1 (cache successes only; replay byte-for-byte; errors never cached). The model stores {status, response} + the RFC 8785 payload_hash, and gains account_id so the scope is (tenant, principal, account, tool, key) with a NULLS NOT DISTINCT unique index (migration 7a8c3e1170a5). The repo's record_rejection becomes record_success, serializing the model inside the repo (keeping model_dump out of _impl) and storing the protocol status separately because a pending buy's 'submitted' status is not a valid domain status. Repo tests ported to success plus account-isolation and null-account-uniqueness coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(idempotency): spec-grounding gate + #1312 rebuild plan Adds the CLAUDE.md Spec-Grounding Gate (cite the authoritative spec section + version + conformance storyboard step before writing protocol-behavior code; the installed SDK is a cross-check, not the authority) and the #1312 spec-conformant rebuild planning notes (grounding, post-merge re-grounding on main's MediaBuy-native idempotency, decision register, test baseline). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(idempotency): invert create_media_buy to verbatim success-cache replay Replaces main's MediaBuy-native re-derivation with byte-for-byte success replay per AdCP 3.0.1: the probe looks up the verbatim success cache (find_by_key, scoped by account_id); a hash match replays the stored envelope with a top-level replayed=true; a present-but-different hash raises IDEMPOTENCY_CONFLICT; a miss proceeds. Successful creates store their response best-effort at the three real success returns — dry-run and failed are never cached, so a retry after an error re-executes. The two TOCTOU IntegrityError recoveries replay from the cache, falling back to re-derivation only when the winner's cache row is not yet visible (rare degraded path). MediaBuy.idempotency_key remains the dup-booking backstop. Removes the wrong-direction rejection machinery: AdCPError.replayed + the error envelope's replayed branch, and AdCPIdempotencyExpiredError (EXPIRED is spec-optional, scoped out). AdCPIdempotencyConflictError keeps recovery=terminal with a documented spec(correctable)-vs-SDK(terminal) divergence note. Tests: rewrote test_idempotency_replay.py to verbatim replay + conflict + miss + errors-never-cached; ported the unit replay/miss tests; harness seed_rejection->seed_success; dropped the rejection-era assert_replayed_rejection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style: ruff-format idempotency files to match CI's pinned formatter Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(idempotency): post-audit hardening of the replay paths Race path now enforces the payload-hash conflict rule: a same-key loser whose canonical payload differs raises IDEMPOTENCY_CONFLICT instead of replaying the winner's response (shared _raise_on_payload_conflict at the probe and in _replay_after_race). Replay deserialization is guarded: a cached envelope that no longer validates (schema drift within the TTL window) is treated as a miss — the probe re-executes and the race path degrades to re-derivation — so a retry of a previously-successful call can never surface a raw ValidationError. The degraded re-derivation no longer claims completed for an awaiting-approval buy (pending_approval maps to submitted), no longer rebuilds the property_list advisory live (omitted: the original is unrecoverable on this path and a live rebuild violates byte-for-byte), and documents its fidelity divergences. record_success requires payload_hash — a cached success without one could not be conflict-checked, which the spec mandates. capabilities' replay_ttl_seconds now derives from DEFAULT_REPLAY_TTL instead of a duplicated literal. Cache-write failure logs carry tenant/principal context. Tests pin the race-conflict, race invalid-envelope fallback, probe schema-drift miss, and degraded submitted-status behaviors; cache seeding consolidated into tests/helpers/idempotency_seeds; stale rejection-era docstrings corrected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(idempotency): require idempotency_key on create_media_buy (AdCP 3.0.1) BREAKING CHANGE: CreateMediaBuyRequest.idempotency_key is now REQUIRED, inheriting the adcp library field (str, MinLen 16, MaxLen 255, pattern limited to [A-Za-z0-9_.:-]) -- the create-side optional override is deleted and its schema-inheritance allowlist entry removed. A missing key rejects as VALIDATION_ERROR at the schema boundary (the storyboard missing_key step); the MCP/A2A wrappers omit the kwarg when absent so the wire message is the spec's "Field required", not a None type error. update_media_buy and sync_* keys stay optional -- a deliberate create-first rollout, since the update BDD contract still encodes optional keys. Approval-resume reconstructs raw_requests stored before the requirement with a synthetic legacy-approval-<id> key: it is an internal replay of an already-validated request (the approval path never consults the idempotency cache), so pending approvals survive the upgrade. Tests thread per-call-unique keys through every construction surface: the harness request builders (plus an OMIT_IDEMPOTENCY_KEY sentinel so missing-key tests can express absence), make_mock_uow (the probe now runs on every keyed create, so mocked UoWs default to a cache miss), the BDD request assembler, the factory and e2e builders, and the test files that construct requests directly. Unique-per-call matters: the cache is live, so a reused key replays the original response. New contract pins cover missing/short/bad-pattern rejection, spec-shaped acceptance, update-side optionality, and a REST wire test asserting the missing-key VALIDATION_ERROR envelope; the obsolete absent-key-proceeds test is superseded by them and removed. Test-literal idempotency keys are allowlisted in .gitleaks.toml (request identifiers, not credentials); the type-ignore baseline ratchets down with the removed override. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(idempotency): refresh rebuild resume notes after required-key commit Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(idempotency): wire matrix for replay/conflict/missing-key across transports Pins the graded AdCP 3.0.1 idempotency steps at the real wire on every transport (IMPL/A2A/MCP/REST): an identical retry replays the original response with top-level replayed:true, the same media_buy_id and protocol status, no second adapter invocation, and a single booking row; on REST the replay body is byte-equal to the original plus exactly the replayed marker. The same key with a different canonical payload rejects with the IDEMPOTENCY_CONFLICT two-layer envelope (recovery=terminal) everywhere; a fresh key with an identical payload creates a new buy; a missing key rejects as VALIDATION_ERROR on A2A and MCP (REST already pinned). Request kwargs are built once per test and copied per call — rebuilding would shift timestamps and turn an intended replay into a conflict. The harness REST/A2A/MCP reconstructor now pops the wire's top-level replayed marker back onto CreateMediaBuyResult (previously silently swallowed by the success model's extra=allow), so wire tests can assert result.payload.replayed on any transport. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(idempotency): graduate v3.1 BDD replay scenarios + stop A2A breaking replay hashes The A2A boundary minted a randomized po_number default for create_media_buy (params.setdefault("po_number", f"A2A-{uuid4().hex[:8]}")). po_number is part of the canonical idempotency payload, so an identical A2A retry hashed differently and rejected as IDEMPOTENCY_CONFLICT instead of replaying — and the stored payload diverged from the same request sent via MCP/REST (cross-transport parity). The mint is removed: po_number stays None when the buyer omits it, exactly like the other transports; every consumer already handles None (order_name falls back to Order-<id>). A wire regression pin proves the no-po_number A2A retry now replays. BDD: the @idempotency-key UC-002 scenarios route to MediaBuyCreateEnv instead of the blanket "not yet wired" xfail. The v3.1 replay scenario passes on all four transports — a real first create, a real keyed retry, the previously created media_buy_id with replayed=true, and an unchanged adapter call count. The v3.1 missing-key scenario is fully wired and strict-xfailed on the pre-existing suggestion-field gap (format_validation_error returns a message string without structured details — the same documented gap as the existing UC-002 suggestion xfails); it graduates the moment production populates one. The orphaned then_no_duplicate_booking step (asserting a ctx counter nothing set — vacuously green) is deleted; the live replacement reads the adapter mock's real call count. given_tenant_auto_approval now configures and verifies the tenant row instead of setting an unread flag. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(idempotency): pin SDK hasher parity + opportunistic cache eviction The salesagent-native canonical hasher is pinned byte-equivalent to the SDK reference (adcp.server.idempotency.canonical_json_sha256): the exclusion sets must match and a corpus spanning key reordering, excluded fields (top-level and the nested webhook credential), unicode, large ints, and bools/nulls must hash identically — drift here would silently change which retries replay vs conflict. A negative case pins that both sides agree a non-excluded field change is a different payload. expire_old gains its production caller: each successful keyed create runs one tenant-scoped, indexed DELETE of expired cache rows in the same UoW as the success-cache write, bounding storage growth without a scheduler (read-path TTL filtering already keeps replay correctness independent of eviction). The eviction test seeds an already-expired row, performs a fresh create, and probes physical row absence by querying with a pre-expiry timestamp — distinguishing deletion from mere TTL filtering. The shared seed helper passes ttl/now through for that. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(test): add pytest-xdist so tox -e bdd's -n auto runs locally The bdd tox env has always passed -n auto --dist loadfile, but pytest-xdist was never a dependency — the env only ran in CI, which executes plain pytest instead. Mirrored into both dev dependency lists per the file's convention. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(idempotency): address review residuals — strict wire asserts, shared env, hasher guard Wire-matrix error assertions no longer fall back from the real wire envelope to the synthesized one: the A2A/MCP/REST legs assert actual wire bytes strictly (a dead wire path must fail, not silently grade the synthesized shape), and the IMPL leg explicitly grades the synthesized envelope — the only thing IMPL has, by definition. The duplicated bare repository-test env (EXTERNAL_PATCHES={} + get_session) is lifted to tests/harness/_base.BareIntegrationEnv; the three idempotency test files now share it instead of carrying local copies. canonical_payload_hash wraps canonicalization in a RecursionError guard: a pathologically nested payload rejects as a typed VALIDATION_ERROR instead of crashing the boundary, with a unit pin. The module docstring now states the SDK-adoption deferral explicitly — byte-parity with adcp.server.idempotency is pinned by TestSdkEquivalencePin until the SDK v5 bump retires the local module and the rfc8785 pin. The original idempotency_attempts migration's docstring and column comments still described the deleted rejection-replay design; they now describe the verbatim success cache. The migration is unreleased (net-new on this branch), so the prose is corrected in place — no schema change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(idempotency): tighten committed planning notes to technical content The planning notes carried process narration that serves no future maintainer — named-reviewer framing, references to private working notes, and tooling-session mechanics. Those lines are removed or genericized; the technical content (spec citations, design rationale, codebase gotchas, validation state) is unchanged. The resume doc is rewritten to describe the shipped end state instead of an in-flight plan. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(idempotency): remove working-session planning notes from the branch Eleven session-working documents (plans, baselines, review rounds, resume notes) nearly doubled the repo's planning-notes footprint for a single PR. The durable content they carried — the AdCP 3.0.1 spec citations and the design rationale — lives in the PR description per the spec-grounding gate; the rest was process residue with no value to a future maintainer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style(idempotency): address automated review — comment the dual-shape except, drop unused merge-migration imports The errors-never-cached test's except clause now states why passing is correct (the adapter failure may surface as a raised typed error or a failed result; the cache-absence assertion below is the contract either way). The empty merge migration's unused op/sa imports are removed and its revision annotations modernized to PEP 604 unions. The bot's unused-global findings on revision/down_revision/branch_labels/depends_on are the Alembic module contract — read by the framework, present in every migration — left as-is. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(idempotency): final craft pass — honest comments, tight signature, shared seed model The lazy-import comments claimed "49 tests patch MediaBuyUoW"; the real count is seven call sites in three files and none are the idempotency tests — the stale number is dropped so the load-bearing rationale stays verifiable. _build_idempotency_hit_result's idempotency_key tightens from str|None to str: every caller passes a real key, and the old `or ""` coalesce silently degraded the not-found error path into an empty-string lookup. The four copies of the seeded ACTIVE-buy success construction (harness seeder, replay helper, eviction test, race conflict test) collapse into one make_active_cached_success helper in tests/helpers/idempotency_seeds, so the seeded shape cannot drift between files. Deliberately left as-is after the same audit: the two IntegrityError race handlers (semantically different logging and principal sources; the shared recovery already lives in _replay_after_race), the per-surface test request builders (model-for-impl vs dict-for-wire vs ctx-for-BDD are different operations, not parameter substitution), and the idempotency helpers' in-module location (extracting them would re-derive the file's call-time-import/patch convention in a second file for cosmetic cohesion). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(idempotency): extract shared cache-read path, pin RFC 8785 vectors, repair downgrade - Extract _lookup_cached_replay, shared by the front probe and the post-race recovery (conflict check before replay, miss on unusable envelope); tool_name scope literal becomes a module constant - Pin RFC 8785 conformance: §3.2.2 sample input/expected output and nine Appendix B number-serialization vectors hashed through canonical_payload_hash; add canonical_request_hash field-order stability, exclusion invariance, and real-difference tests - Pin the impl's except-IntegrityError → _replay_after_race seam through the production entrypoint (lost cache row + surviving MediaBuy: re-execution, backstop recovery of the winner, single booking) - Repair the account_id migration downgrade: clear the replay cache before recreating the 4-column unique index — rows differing only in account_id collided and wedged the rollback (verified empirically on a migrated scratch DB) - Harness MCP dispatcher: wire_error_envelope is real-wire-or-stash only; synthesis routed to the separate synthesized_error_envelope field (a dead MCP wire path now fails envelope asserts); policy table updated; AdCPIdempotencyConflictError added to the reconstruction map - Required-key wording: MCP tool Field description and Args document the key as required (16-255 chars, missing key rejects); missing-key comments cite the storyboard's VALIDATION_ERROR tolerance vs the prose's INVALID_REQUEST preference - NITs: adapter-error return documents the errors-never-cached rule; UpdateMediaBuyRequest override carries an explicit removability condition; response_envelope documents the deliberate bare JSONType; api_v1 hoists the AccountReference import and converges both routes on model_validate; canonical_request_hash typed request: BaseModel; stale race-test comment reworded Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(idempotency): account-scoped degraded path, insert rate limit, wire-level hashing Degraded-path fold-in (the backstop's scope and safety rules now match the probe's): - media_buys backstop index gains account_id (NULLS NOT DISTINCT, partial) via a CONCURRENTLY swap migration that keeps dup-booking coverage live; same key under different accounts books independently per the AdCP (agent, account, key) scope - media_buys gains a nullable payload_hash column written at create time — raw_request is not canonicalizable (injected package_ids, alias-dependent names), so the degraded fallback conflict-checks against this stored hash and rejects IDEMPOTENCY_CONFLICT exactly as at the probe - a buy that outlived the advertised replay TTL rejects as IDEMPOTENCY_EXPIRED (new typed error, SDK-standard terminal recovery) instead of silently re-deriving - find_by_idempotency_key is account-scoped (IS NULL matches no-account) Insert rate limit: - the probe rejects a fresh key as RATE_LIMITED when the (tenant, principal, account) scope already holds MAX_ACTIVE_ATTEMPTS_PER_SCOPE active cache rows; retry_after reports when the oldest row expires. retry_after is now a first-class AdCPError attribute riding both envelope layers (the SDK Error field). Replays and conflicts insert nothing and are never rate-limited Wire-level hashing (spec equivalence input is the request AS SENT): - transport wrappers thread the raw wire payload into _impl: MCP stashes pre-normalization arguments via MCPAuthMiddleware state, A2A forwards the DataPart params, REST reads the body through a Depends dependency (route signatures stay Depends-only); the model-dump hash is the documented fallback for impl-direct callers only - pinned by a wire test: two encodings of the same instant conflict instead of replaying Tests: degraded conflict/expired/cross-account junction recipes through the production entrypoint, repository ceiling semantics incl. exact retry_after, RATE_LIMITED on the real REST wire, replay-never-rate-limited, wire-hash input pin, typed-error unit pins. Migration verified up/down on a migrated scratch DB; raw_request_payload joins the guard's intentionally-raw-dict category (model coercion would defeat wire hashing). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(idempotency): align scope, recovery, hashing engine, and degraded path to the spec's letter Five spec-grounding decisions locked after line-exact prose verification, SDK source/empirical runs, and full impact mapping: - Scope tuple: tool_name leaves the cache UNIQUE index and all lookups (the spec scope is agent + account + key, exactly; the SDK store keys the same way). The column stays for observability. A key reused by a different tool now hits the same row and conflicts on its differing payload hash instead of getting an independent per-tool cache. General principle recorded on the model: spec-defined scope tuples are implemented as written; extra dimensions may exist as columns but never in uniqueness/lookup semantics. - Recovery classes: IDEMPOTENCY_CONFLICT is correctable (the prose example and storyboard expectation; the buyer can resend the original bytes or mint a fresh key) — the prior terminal choice rested on an "SDK validator" that does not exist (the STANDARD_ERROR_CODES table is a default, never enforced). IDEMPOTENCY_EXPIRED stays terminal with an honest rationale (no resend of the same key can ever succeed). - Rate limiting: the spec MUST is an insert-RATE bound; a trailing 10s/300 window check now precedes the active-row storage cap, with retry_after pointing at when the oldest in-window insert leaves the window. All retry_after values clamp to the Error model's [1, 3600]. - Hashing engine: the local RFC 8785 fork is deleted; the module delegates to the SDK canonicalizer behind the same seam (production never imports the SDK module directly; the RFC-published vectors pin behavior engine-independently; the RecursionError→typed-error boundary stays ours). The direct rfc8785 pin drops — the SDK requires it. The SDK-parity pin is removed as a tautology, replaced by a literal pin of the spec's closed exclusion list. - Capture uniformity: every transport hashes the payload AS SENT — A2A captures the DataPart params before normalization and handler mutations; REST stashes the pre-rewrite body bytes ahead of the compat middleware's replacement; MCP already captured pre-normalization. Pinned by a cross-transport identical-retry replay and a deprecated-field-spelling identical-retry replay. - Degraded path fails closed: when the backstop fires and no verbatim cache row is usable, the response is never reconstructed — conflict and expiry checks run as before, then a transient SERVICE_UNAVAILABLE with retry_after tells the buyer to retry once the winner's cache write lands. Verbatim replay is byte-for-byte or nothing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(idempotency): close the three review patterns — shared boundary builder, policy layer, off-path eviction Addresses the multi-agent review's three patterns at the pattern level: - Lockstep wrappers: _build_create_media_buy_request() is the single home for the request field list, brand string-shorthand coercion, idempotency omit-when-absent splat, and ValidationError translation. The MCP and A2A/REST wrappers now diverge only in the two A2A input coercions, applied at the call site. The boundary-completeness guard learned the builder indirection honestly: a field dropped at the wrapper→builder hop OR inside the builder is still reported (negative self-tests included). - retry_after serializer sync: to_dict() and to_adcp_error() now emit retry_after alongside field/suggestion, matching the envelope builder and the class docstring's sync claim; pinned by a serializer-parity test quartet. - account_id width: idempotency_attempts.account_id narrowed to String(100) — matching accounts.account_id and every sibling column — in the model and in migration 7a8c3e1170a5 itself (branch convention: unmerged migrations edit in place). Verified by a fresh-DB alembic upgrade (information_schema reports 100). - Rate-limit policy out of the data layer: thresholds, window math, and retry_after clamps moved to src/services/idempotency_policy.py; the repository now exposes two pure scope queries (count_inserts_since, count_active — COUNT + MIN each) and carries no rejection logic. - Eviction off the create hot path: expire_old() no longer shares the cache-write transaction. It runs in its own short transaction after the write commits, on ~1% of keyed successes (_EVICTION_PROBABILITY, patchable), so a tenant-wide DELETE deadlock can never roll back a just-cached success and creates almost never carry housekeeping. Pinned both ways: forced eviction deletes the expired row; suppressed eviction leaves it with the create untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(idempotency): address review — stored expires_at anchoring, single-source TTL, splat-aware boundary guard Addresses the 2026-06-12 review on PR #1312: - Anchor the degraded replay-window expiry on the cache row's stored expires_at (new find_including_expired) rather than recomputing from the MediaBuy's created_at; the MediaBuy.created_at proxy is the fallback only when no cache row survives. - Single-source the replay TTL: the capability reads DEFAULT_REPLAY_TTL; rate-limit thresholds move to the int(os.getenv(...)) convention; extract _clamp_retry_after() for the shared [1, _RETRY_AFTER_MAX] clamp. - Thread account_id through get_by_id_or_idempotency_key so the idempotency-key fallback is not silently re-scoped to IS NULL. - Boundary guard now models the omit-when-absent ** splat AND derives the forwarded-field set from CreateMediaBuyRequest.model_fields, so a new field cannot be silently dropped (this added account coverage that was missing). - Consolidate the test seed helpers (seed_principal, seed_media_buy). - Align migration 1d9b1402 to collections.abc.Sequence + PEP 604 + comment=. - One name (raw_wire_payload) for the stashed wire payload across MCP/A2A/REST. Adds two regression guards: capability/constant parity (replay_ttl_seconds must derive from DEFAULT_REPLAY_TTL, not a literal) and the schema-derived boundary allowlist. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(idempotency): single-home race recovery, EXPIRED→correctable, seam guard, wire+oracle coverage Addresses the 2026-06-14 review (four pattern-level items) plus a spec-conformance fix it surfaced. Unifying thread: every claimed invariant now has a failing oracle. - Race recovery: one home (_resolve_idempotency_race_or_raise) for the "is this the idempotency backstop?" decision, previously duplicated across both booking paths. Detection prefers the driver's structured constraint_name (prefix-matched against a named index constant, pinned to the model index) over the message-text substring it replaces; the substring is a no-diag fallback only. - "Errors are never cached": drop the unreachable isinstance() skip in _cache_and_return; the error-path early return is the real guard, now a fail-loud precondition. Oracle: an error caches nothing, so a same-key retry re-executes to a fresh success (storyboard rule 5). - Single SDK seam: new structural guard — only idempotency_canonical.py may reach adcp.server.idempotency (static + importlib/__import__/attribute-chain forms), replacing a docstring-only convention. - IDEMPOTENCY_EXPIRED recovery terminal -> correctable: the 3.0.1 error-code.json enumDescription (verified at the v3.0.1 tag) classifies it correctable; terminal is reserved for human action. Matches the sibling CONFLICT. The raise carries the natural-key-check suggestion. The wire matrix asserts EXPIRED — code, recovery, and suggestion — on the real wire across IMPL/A2A/MCP/REST (was reconstructed- exception only). BR-UC-002 EXPIRED+CONFLICT scenarios reconciled to correctable (xfail intent; live enforcement is the unit + wire tests). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(idempotency): single-home the tenant-scope prefix across aggregate queries Extract _scope_prefix(principal_id, account_id) — the (tenant, principal, account) isolation terms — and _count_and_oldest(scope, min_column). _scope_filter now builds on the prefix and appends the key; count_inserts_since and count_active become predicate + column pass-throughs. The isolation prefix has one home, so a scoping fix lands once rather than once per query. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(boundary): exclude adcp_version from create_media_buy forwarding allowlist SDK 5.7 (AdCP 3.1.0-beta.3) adds adcp_version to CreateMediaBuyRequest.model_fields. It is a protocol version marker set by the transport layer, not a buyer-supplied media-buy parameter — the same reason the sibling adcp_major_version is already excluded. Add it to _CREATE_NOT_FORWARDED so the schema-derived boundary allowlist does not demand the wrappers forward it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(gam): supply required idempotency_key in autoconfig delivery-type request The GAM autoconfig delivery-type test (added by the SDK 5.7 upgrade) built a CreateMediaBuyRequest without idempotency_key. create_media_buy requires the key (the library-required field is intentionally NOT overridden optional for create — only update_media_buy keeps the optional override). Supply a per-product key so the request validates. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…rebid#1431) The guard reads BDD step files with the platform default encoding, which is cp1252 on Windows — the UTF-8 box-drawing section headers in the step files make the guard crash with UnicodeDecodeError before checking anything. Pin encoding='utf-8' so the guard runs identically on all platforms (CI on Linux was unaffected). Co-authored-by: pmezzich <pmezzich@users.noreply.github.com>

…l dir (prebid#1443) agent-db.sh lives at <root>/.claude/skills/agent-db/, but PROJECT_DIR resolved only one level up ($SCRIPT_DIR/..), landing on .claude/skills. basename of that is "skills", so WORKTREE_ID was "skills" for every worktree: - CONTAINER_NAME collapsed to a single shared agent-pg-skills instead of the intended per-worktree agent-pg-<worktree>, so concurrent worktrees collided on one container; - the per-worktree state file landed under .claude/skills/ and drifted from the live container's published port, so 'up' re-exported a stale port (50000) while the container was actually on another (58239) — integration runs then hit connection-refused. Resolve three levels up to the worktree root. Also gitignore the generated .agent-db.env and untrack the copy that was accidentally committed (it carried a local port and the test ENCRYPTION_KEY). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

…ebid#1234) (prebid#1390) * refactor: relocate pre-commit hooks to layered model (PR 4 of prebid#1234) Extend make quality-ci before deleting CI-only hooks. Move 37 commit-stage hooks to 12 commit + 11 pre-push; add 5 AST structural guards and arch_guard markers. Preserves PR2 uv.lock single-source and PR3/PR3-2 CI design. * fix: remove defensive RootModel unwrapping flagged by PR4 guard Replace hasattr(po, "root") patterns with direct .root access so test_architecture_no_defensive_rootmodel passes after hook relocation. * fix: use direct AccountReference.root in uc002 BDD steps Remove defensive hasattr(account_ref, "root") checks flagged by the no_defensive_rootmodel architecture guard on PR4. * docs(ci): align layered pre-commit section with 20 frozen checks PR prebid#1379 uses 2 greedy BDD shards (18 + 2 shard checks + aggregate proxy). * fix(ci): tighten PR4 guard detection per Konstantin PAT-02 (PR 4 of prebid#1234) - check_repo_invariants: exclude tests/ via path parts, not filename substring - rootmodel guard: drop stale allowlist entry with no matching violation - jsontype guard: catch mapped_column(sa.JSON) attribute args * fix(ci): address Konstantin review on PR4 guards (prebid#1390) - Widen tenant.config detector (self.tenant.*); add known-bad self-tests - Adopt format_failure/assert_detector_catches_ast_snippets helpers - Move mcp-contract-validation to CI schema-contract (off pre-push) - Fix scripts/README pre-commit install; catalog ci_bdd_shard + self-tests * refactor(test): wire PAT-01 guard helpers and drop dead exports (prebid#1390) Adopt assert_violations_match_allowlist in workflow isolation and weak-mock guards (one check for new + stale allowlist drift). Add contract tests for assert_violations_match_allowlist and assert_anchor_consistency. Remove unused iter_function_defs, iter_class_defs, and iter_action_uses helpers. * refactor(test): adopt assert_violations_match_allowlist in auth guards (prebid#1390) Merge new-violation and stale-allowlist checks for auth helper signature and context-wiring guards using the shared PAT-01 helper. * refactor(test): migrate guard allowlists to shared helper (prebid#1390) Route all hand-rolled allowlist set-diff checks through assert_violations_match_allowlist, consolidate _ast_helpers into _architecture_helpers, and add an arch-guard against regression. * fix(test): rebase PR4 on main and finish allowlist helper migration Migrate new main guard files to assert_violations_match_allowlist and mark intentional RootModel test assertions with noqa after adcp SDK bump. * refactor(test): finish allowlist helper migration and AST guard (prebid#1390) Complete Pattern 1: migrate remaining BDD guards to assert_violations_match_allowlist, remove _ast_helpers shim, and upgrade the hand-rolled allowlist regression guard to AST scanning with sorted() self-test and _ast_helpers import ban.

… (prebid#1437) * fix(ci): cache creative agent build with buildx GHA cache (prebid#1189) Use buildx cache-from/to on the creative integration shard and retry tarball fetch and docker build to cut cold-start time and absorb transient flakes. * fix(ci): expose GHA runtime token for creative agent buildx cache Shell run: steps cannot see ACTIONS_RUNTIME_TOKEN without ghaction-github-runtime; gate buildx cache on that token only (not the v1-only ACTIONS_CACHE_URL). * fix(ci): pre-build creative agent image in ghcr.io instead of GHA cache Replace buildx layer cache (blocked by --load on warm runs and 2.6 GB GHA budget) with pin-keyed ghcr.io images: pull on warm CI, build+push on cold. * fix(ci): publish creative agent image from main, pull-only in CI shard Move GHCR publish to a trusted main-only workflow so fork PRs can pull a public pin-keyed image. CI creative shard becomes pull-only with local build fallback; retries and curl -f hardening preserved. Closes prebid#1189 publish-model gap from PR review.

…ransport boundaries Pydantic v2's default model_dump() preserves AnyUrl fields as typed AnyUrl objects and enum fields as enum instances rather than converting them to str. SQLAlchemy's String column type does not coerce these objects and raises a StatementError at flush time when push_notification_config.url is persisted. Fix: use model_dump(mode='json') at both transport boundaries so AnyUrl and enum values are serialised to plain Python strings before reaching _impl and the database layer. Affected call sites: - create_media_buy MCP wrapper (line ~4294): PushNotificationConfig.model_dump() - create_media_buy_raw A2A wrapper (line ~4377): PushNotificationConfig.model_dump() The protobuf-based A2A path (adcp_a2a_server.py) uses json_format.MessageToDict() which always produces plain strings and is not affected. Closes prebid#1377

github-actions · 2026-06-18T07:13:01Z

IPR Policy Agreement Required

Thank you for your contribution! Before we can accept your pull request, you must agree to our Intellectual Property Rights Policy.

By making a Contribution, you agree that:

You grant the Foundation a perpetual, irrevocable, worldwide, non-exclusive, royalty-free copyright license to your Contribution
You grant a patent license under any Necessary Claims
You represent that you own or have sufficient rights to grant these licenses

To agree, please comment below with the exact phrase:

I have read the IPR Policy

You can read the full IPR Policy here.

I have read the IPR Policy

0 out of 7 committers have signed the CLA.
❌ @KonstantinMirin
❌ @ChrisHuie
❌ @gagliathebuilder
❌ @mkostromin-sigma
❌ @Jeshua09090
❌ @pmezzich
❌ @torbenbrodt
_{You can retrigger this bot by commenting recheck in this Pull Request.}_{Posted by the CLA Assistant Lite bot.}

Extract shared MCP/A2A capture scaffolding into tests/helpers/create_media_buy_capture.py so both test_create_media_buy_behavioral.py and test_push_notification_forwarding.py use the same helper. - Eliminates the +3 duplicate blocks that tripped the DRY hook - New tests assert on returned dict values (no assert_called_once + call_args split pattern) so no new weak-mock allowlist entries needed - All 9 affected tests pass (5 serialization + 2 forwarding + 2 guards) Closes prebid#1377

KonstantinMirin and others added 30 commits June 1, 2026 15:25

refactor: extract _get_assignment_from_db helper in uc006_sync_creati…

9c2fc60

…ves (spug)

fix: use shared _get_error_message() instead of str(error).lower() in…

1629369

… uc004 (wdge)

refactor: move then_webhook_notification to uc002 domain step file (a…

c7a1527

…ya0)

refactor: extract _assert_exponential_backoff helper in uc004_deliver…

d2f1474

…y (vpgh)

refactor: extract _assert_format_check_outcome helper in uc006_sync_c…

c5ef5c1

…reatives (ftmy)

fix: add explicit strict=False to pairwise zip() in uc002 (B905)

f89201c

Merge remote-tracking branch 'origin/main' into feature/strengthen-bd…

b2d2537

…d-assertions # Conflicts: # src/core/tools/media_buy_delivery.py # tests/unit/test_update_media_buy_behavioral.py

fix: restore AdCPValidationError import lost in merge auto-resolution

af39e87

fix(ci): format compile_bdd.py and bump aiohttp 3.13.5 -> 3.14.0

081d3e4

- ruff format scripts/compile_bdd.py (rendered tooling left it unformatted) - aiohttp >=3.14.0 resolves CVE-2026-34993 and CVE-2026-47265 (pip-audit)

fix(ci): bump authlib 1.6.11 -> 1.7.2 for PYSEC-2026-188

7399a77

Resolves the remaining pip-audit finding. authlib imports clean and the auth/oauth unit suite passes (536 passed). pip-audit now reports no known vulnerabilities.

KonstantinMirin and others added 20 commits June 4, 2026 14:21

Merge remote-tracking branch 'origin/main' into feature/bdd-3.1-migra…

c59ebcf

…tion # Conflicts: # pyproject.toml

fix(schema): align GetProductsRequest push notification config (prebi…

7cb7e00

…d#1393)

Merge remote-tracking branch 'origin/main' into feature/strengthen-bd…

b1bb7d2

…d-assertions # Conflicts: # .duplication-baseline # tests/admin/test_creatives_blueprint.py

chore: relock uv.lock to match merged pyproject.toml dependency bumps

babd5c1

Merge branch 'feature/strengthen-bdd-assertions' into feature/bdd-3.1…

c30f77a

…-migration

Merge pull request prebid#1384 from KonstantinMirin/feature/bdd-3.1-m…

9bdda1d

…igration test(bdd): migrate feature files to AdCP 3.1 (wired UCs green, zero regressions)

refactor: consolidate account reference coercion (prebid#1439)

aa02c54

* refactor: consolidate account reference coercion * test: cover account reference validation * test: strengthen account validation assertions

torbenbrodt closed this Jun 19, 2026

torbenbrodt deleted the fix/1377-push-notification-config-model-dump-json branch June 24, 2026 17:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: serialize push_notification_config with mode='json' at MCP/A2A transport boundaries#11

fix: serialize push_notification_config with mode='json' at MCP/A2A transport boundaries#11
torbenbrodt wants to merge 51 commits into
mainfrom
fix/1377-push-notification-config-model-dump-json

torbenbrodt commented Jun 18, 2026

Uh oh!

github-actions Bot commented Jun 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Uh oh!

Conversation

torbenbrodt commented Jun 18, 2026

Summary

Root Cause

Fix

Verification

Spec Citation

Test Obligations

Uh oh!

github-actions Bot commented Jun 18, 2026

IPR Policy Agreement Required

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants