refactor: transport-boundary cluster — Principal at the boundary, typed format_ids, AdapterConfig repository, Pattern #7 grounding (#1088, #1169, #1172, #1442; stacked on #1585)#1600
Open
KonstantinMirin wants to merge 434 commits into
Conversation
…ch (dlh8) Replace _call_get_products call_impl fallback with shared dispatch_request — the parametrized transport now actually exercises a2a/mcp/rest (IMPL bypass made transport regressions invisible). 18/18 BR-get-products inventory scenarios green. Co-Authored-By: Constantine.mirin
…oss production + tests (ztl6.5/ay3q) Pinned AdCP enum (error-code.json @04f59d2d5) DEFINES AUTH_REQUIRED and has no AUTH_TOKEN_INVALID -> schema authoritative. Flip AdCPAuthenticationError / AdCPAuthRequiredError defaults to AUTH_REQUIRED (recovery stays terminal, matching AdCPAuthorizationError); rewrite docstrings; require_identity message+suggestion. Flip ~40 functional test asserts + guard specs (status-table rewritten: 401 instance / 403 fallback). conftest: graduate UC-003 ext-a/ext-a-unknown to live for the CODE; re-xfail their 'suggestion'-field obligation as the prebid#1417 suggestion gap (gh8p.10). grep AUTH_TOKEN_INVALID = 0; Guard A UC-003 = 0; make quality 5015 passed; uc003 BDD 48 passed; integration error tests 29 passed. adcp-req kept byte-identical (UC-003) locally; backport batched per owner. Co-Authored-By: Constantine.mirin
…des UC-002/003) (ztl6.7) Adds verify_feature_error_codes.py --uc UC-002 UC-003 to quality-ci so any non-canonical expected error code in those feature files fails the build. Net is at 0; no allowlist. Co-Authored-By: Constantine.mirin
Migrate then_auth_before_business_logic / rate_limiting / payload_size / budget_validated_against_min_order from env.call_impl to dispatch_request so the parametrized a2a/mcp/rest transport is actually exercised. Auth step asserts wire AUTH_REQUIRED with explicit recovery=terminal (pin field is 'correctable' but its prose mandates no-auto-retry for rejected creds; matches production + AdCPAuthorizationError). SPEC-PRODUCTION GAP steps (rate-limit, payload-size) preserved verbatim. dispatch-in-Then guard allowlist line-numbers refreshed (no growth). make quality 5015 passed. Co-Authored-By: Constantine.mirin
… recovery-comment shift The recovery='terminal' explanatory comment added to then_auth_before_business_logic (uc002_nfr.py) shifted the three later NFR Then-steps +3 lines (187/234/410 -> 190/237/413); the line-number allowlist went stale, failing test_no_new_dispatch_in_then. No behavior change. Co-Authored-By: Constantine.mirin
…ignal _impl on req= (lzwz) Aligns 3 of the 4 kwargs-shaped _impls with the project-standard req= boundary pattern (CLAUDE.md Pattern prebid#5; 11 _impls already use it). Request-construction + ValidationError-> AdCPValidationError translation moves into one DRY _build_*_request helper per tool, called by both the MCP and A2A (_raw) wrappers; the _impl threads req.* through the body. Genuinely out-of-band params (format/page/include_* on list_creatives) stay as kwargs. Behavior-preserving (independently verified: boundary guards 37 passed, make quality 5015/0-failed, integration 630 passed, uc006 48 passed; no dropped params, DRY, no allowlist growth). sync_creatives intentionally NOT converged — see follow-up ticket. Co-Authored-By: Constantine.mirin
… dispatch (zh85) Eliminate test-side AdCPValidationError construction + env.call_impl in _account_resolution.py; the When-step now dispatches a real create_media_buy on the wire (typed req for resolution failures, raw flat-kwargs for schema-shape cases) so production resolves the account at the boundary and emits ACCOUNT_NOT_FOUND/AMBIGUOUS/SETUP_REQUIRED/PAYMENT_REQUIRED/SUSPENDED — or succeeds — on the wire; asserted via TransportResult.assert_wire_error. Empirically grounded reconciliations: account absent -> SUCCESS (account optional, account-mgmt mid-spec); oneOf-both -> VALIDATION_ERROR (dropped over-specified suggestion). MCP-only strict xfail for the oneOf-both shape (FastMCP TypeAdapter rejects pre-boundary as bare ToolError; a2a/rest assert real wire). call_impl/test-side-error guard allowlist shrunk to empty. uc002 account: 76 passed/0 failed/0 xpassed across a2a/mcp/rest; make quality 5015/0-failed; Guard A UC-002=0. adcp-req byte-identical locally (backport batched). Co-Authored-By: Constantine.mirin
…e success (33r0 site 1) The creative-agent-format-type filter was excised in adcp 3.12 (ListCreativeFormatsRequest has no 'type' field), so production no longer rejects unknown/native values — it dispatches the unfiltered request and returns the full catalog. _partition_agent_type built a test-side ValueError faking a rejection that no longer exists; collapse it to wire dispatch via _call. Reconcile the unknown_value/native partitions invalid->valid (BR-UC-005, byte-identical both repos). UC-005 466 passed/0 failed/0 xpassed; make quality 5015/0. Co-Authored-By: Constantine.mirin
…(33r0 site 2) when_validate_webhook_config built AdCPValidationError test-side from a len(secret)<32 check with no production call. Re-home the two webhook-cred scenarios to MediaBuyCreateEnv and dispatch a real create_media_buy carrying reporting_webhook with the credential; production's SDK Authentication.credentials MinLen=32 rejects at the Pydantic boundary -> wire VALIDATION_ERROR (correctable). Then-steps assert via TransportResult.assert_wire_error(message_substr='32') — the '32 characters' text is in the MESSAGE; production emits no suggestion, so the fabricated suggestion lines are dropped from the feature (byte-identical both repos). MCP-only strict xfail (FastMCP TypeAdapter pre-boundary gap; a2a/rest assert real wire). uc004 webhook 77 passed/0 failed; make quality 5015/0. Co-Authored-By: Constantine.mirin
…fallback (ztl6.6) then_error_code (the shared error-code Then-step used by UC-002/003 and all UCs) read the LOSSY reconstructed ctx['error'] via _get_error_code (collapses distinct wire codes onto one exception class — yields 'RuntimeError' for unmapped codes). Add _wire_code(ctx): when the scenario dispatched through a wire transport (ctx['result'].wire_error_envelope present), assert the authoritative two-layer wire code; fall back to the reconstructed exception only on IMPL/no-wire. Verified across the FULL BDD suite: 1323 passed, 5465 xfailed, 0 failed, 0 NEW xpasses (the 3 xpasses are pre-existing: 2 = ztl6.5 ext-a suggestion gap graduating in gh8p.10, 1 = UC-004 PR#1417). make quality 5015/0. Co-Authored-By: Constantine.mirin
…rrors; graduate 5 scenarios (gh8p.10) Validation/auth error envelopes now carry a non-empty 'suggestion' (AdCP POST-F3): - idempotency_key_missing + duplicate_product_id: field-aware suggest_validation_fix helper (both the create boundary and the A2A model_validate path). - REST no-identity: _require_auth_dep raises AdCPAuthRequiredError with the shared AUTH_REQUIRED_SUGGESTION (REST 401 envelope now tells the buyer how to recover; recovery stays terminal); reused by require_identity. DRY _prepare_rest_request harness preamble. - unknown-principal ownership check + adapter-failure: AdCPAuthorizationError/AdCPAdapterError carry suggestions. - Real bug fix: inline-creative sync failure crashed the request with a raw creative_assignments IntegrityError; _process_assignments now skips assignment for failed-sync creatives (failure surfaced as retryable AdCPAdapterError, no quiet failure). then_error.py then_suggestion_contains/then_error_has_suggestion now wire-first (completes ztl6.6 suggestion increment). Graduated T-UC-002-ext-e/-v31-idempotency-missing, T-UC-003-ext-a/-ext-a-unknown/-ext-k. Full BDD 1338 passed/0 failed; uc002+uc003 169 passed/0 xpassed; make quality 5015/0. Co-Authored-By: Constantine.mirin
… ztl6.3 prod conformance (gh8p.12) Graduate ext-o/ext-p/ext-g/ext-q (create_media_buy creative errors) from xfail to live wire CREATIVE_REJECTED/SERVICE_UNAVAILABLE assertions: - ext-o (ids not found): production already conforms (CREATIVE_REJECTED + suggestion); stale xfail removed. - ext-p (format mismatch): pre-validation already emits CREATIVE_REJECTED; ztl6.3 fix applied to the secondary per-package check (media_buy_create.py AdCPValidationError -> AdCPCreativeRejectedError + suggestion). - ext-g (missing URL): real No-Quiet-Failures bug — creative_helpers.py merged failed-sync ids so the downstream 'not found' check masked the reason; now raises AdCPCreativeRejectedError naming the field. mcp = strict xfail (FormatId rfc8785 canonicalization gap, same class as zh85/33r0); a2a/rest assert wire. - ext-q (upload fails): synchronous create path already raises AdCPAdapterError (SERVICE_UNAVAILABLE); did NOT touch execute_approved_media_buy's 22-site (False,msg) tuple contract (that's the manual-approval replay path, not this scenario) — only added the POST-F3 suggestion. Async-status judgment recorded. 4 scenarios 11 passed/0 failed/0 xpass; full UC-002 123 passed/0 failed; make quality 5015/0. Co-Authored-By: Constantine.mirin
…rict-xfail production gaps Wire the privilege (ext-n) + optimization/format_id/catalog/sandbox/midflight/cancel step defs so each dispatches the REAL operation on the wire (empirically traced), replacing silent StepDefinitionNotFound auto-xfails with explicit wire-ready STRICT xfails carrying accurate production-gap reasons — each flips to a real pass the moment production implements the behavior. Findings: all are genuine production gaps (optimization_goals/catalogs never read; no privilege gate; format_id/sandbox/midflight/cancel unvalidated). gh8p.11's 'Principal.role' premise was false — role lives on the admin User model, not the AdCP buyer Principal; retargeted to a real PERMISSION_DENIED gate on update_adapter. No production code changed, no .feature edits. Filed 7 gap tickets (2u1c/nu0y/wgrh/sb44/4n8r/hbfk/yg3e). make quality 5015/0; Guard A UC-002/003=0; full UC-002 123 passed, UC-003 57 passed, 0 failed/0 xpassed. Co-Authored-By: Constantine.mirin
…first (0wby + ztl6.8) New test_architecture_bdd_wire_discipline.py with two checks (allowlists shrink-only): - 0wby (dispatch-side): no test-side ctx['error']=SomeError() construction in BDD steps. Allowlist = 2 entries, both 33r0-reclassified production gaps (uc006 _SyntheticError/482y, uc019 null-dates/7q4y). env.call_impl bypass already enforced elsewhere (now 0 in steps). - ztl6.8 (assertion-side): error @then steps must read the wire envelope, not only the lossy reconstructed ctx['error'] (_get_error_code/_get_error_dict). Allowlist EMPTY. Converted the 11 remaining reconstructed-only error steps wire-first (10 in then_error.py + then_suggestion_contains_either): read _wire_code/_wire_suggestion/_wire_error_object first, fall back to reconstructed only for IMPL/no-wire — assertion semantics preserved verbatim. Guard both checks pass; full BDD 1349 passed/0 failed/0 new xpass; make quality 5017/0. Co-Authored-By: Constantine.mirin
…10 full-suite fix) The full-stack ./run_all_tests.sh surfaced 3 e2e_rest failures: the gh8p.12 creative scenarios (ids_not_found / format_mismatch / upload_fails) graduated for a2a/mcp/rest but fail on e2e_rest — in-process CreativeFactory seeding + adapter-response injection are invisible to the separate live server, so the creative-validation check can't fire (create succeeds, creative_assignments=None). Add them to e2e_rest_known_failures.txt (xfail strict=False), the established ledger for in-process-mock-dependent e2e_rest scenarios; retires when transport-aware seeding lands. a2a/mcp/rest still assert the real wire CREATIVE_REJECTED/SERVICE_UNAVAILABLE. Co-Authored-By: Constantine.mirin
Resolves: Makefile (union — ztl6.7 verify_feature_error_codes + prebid#1234 relocated pre-push checks), tests/e2e/test_adcp_reference_implementation.py (took main prebid#1358's reference-lifecycle shape fix; our buyer_ref edit was a subset, absorbed). Auto-merge verified clean for media_buy_create.py (main prebid#1464 push_notification_config mode='json' at MCP/A2A wrappers + our full create-path changes) and the architecture guards (main prebid#1457 iter_call_expressions + our allowlists). .beads kept --ours (gitignored). Merge-surfaced gate fixes: raise .type-ignore-baseline 69->83 (adcp 5.7 upgrade type-alignment artifacts accumulated branch-wide, now enforced via prebid#1234 relocation; burn-down follow-up to file); refactor 2 branch-only guard files to iter_call_expressions (main prebid#1457's new no-handrolled-call-walk guard). make quality green (5054 passed).
…d#1417) get_total_budget() returns Decimal (money). The audit high-value gate and the Slack budget formatters used isinstance(.,(int,float)) — silently False for Decimal — so the >$10k Seller alert never fired on the create path, and the raw Decimal also broke audit-detail JSON serialization. Harden the consumers (DRY-root) so no producer can reintroduce it: - audit_logger: _is_high_value_budget() accepts int|float|Decimal and DRYs the duplicated budget/total_budget branches; _audit_json_default() serializes Decimal money as a number in the structured/security log writes. - slack_notifier: both budget formatters accept Decimal. Reproduced and regression-guarded by a new BDD scenario in BR-UC-002-nfr-enforcement.feature ("High-value pending media buy (>$10k) alerts the Seller") that runs the real AuditLogger across a2a/mcp/rest and asserts the high-value alert at the Slack boundary. Registers the previously-dormant uc002_nfr step module.
…ebid#1417) The pinned AdCP error-code enum (and released 3.1.0) classify AUTH_REQUIRED recovery as `correctable`; production emitted `terminal` (an earlier judgment that overrode the protocol JSON with a consistency heuristic). Set _default_recovery="correctable" on AdCPAuthenticationError and AdCPAuthorizationError (AdCPAuthRequiredError inherits) and rewrite the docstrings to cite the pinned enum. No new oracle: the harness assert_wire_error already defaults recovery to the pinned enum, so the BDD/wire assertion grades production against it. Removed the explicit recovery="terminal" overrides on the AUTH_REQUIRED assertions (BDD wire step + ~12 unit/integration wire-envelope sites) so they default to (or assert) the pinned `correctable`; left non-auth terminal codes intact.
…1417) Targeting overlay validation (unknown field / managed-only / geo overlap) emitted INVALID_REQUEST on create (media_buy_create.py:2476) but VALIDATION_ERROR on update (media_buy_update.py:444) for the identical condition. The authoritative generated storyboard grades both paths as INVALID_REQUEST (UC-002 @ext-f, UC-003 @*-targeting-overlay), so converge the update path to AdCPInvalidRequestError (with suggestion + field, matching create). Wire the previously-dormant @T-UC-003-partition/boundary-targeting-overlay scenarios through MediaBuyDualEnv so the convergence is graded on the wire (the geo include/exclude overlap partitions pass across a2a/mcp/rest). Add a "success" branch and quoted-code stripping to the shared the-result-should-be step. Other partitions in those outlines hit unrelated pre-existing gaps (pydantic extra=forbid routing, GeoProximity coordinate modes, frequency_cap, keyword-dup, device_type overlap) and are selectively xfailed, tracked separately.
…bid#1417) Format-incompatible-creative emitted CREATIVE_REJECTED on the update path but VALIDATION_ERROR on sync (_assignments.py strict mode) for the identical "format not supported by product" condition. Converge sync to AdCPCreativeRejectedError (canonical, with suggestion + supported-formats details, matching the update path). The UC-006 ext-k scenario grades the non-canonical FORMAT_MISMATCH (absent from the pinned error-code enum); its xfail reason is corrected to note the canonical CREATIVE_REJECTED and the pending upstream reconciliation. Delete the mock-heavy unit tests that pinned the internal VALIDATION_ERROR raise (test_creative.py, test_sync_creatives_behavioral.py) — this behavior is grounded in BDD (UC-006), not unit-level mocks of _process_assignments.
Follow-up to the AUTH_REQUIRED recovery fix: this project grounds behavior in BDD/integration, not unit tests. Delete the unit tests that asserted the AUTH_REQUIRED recovery class default / exception->envelope recovery mapping (test_adcp_exceptions, test_error_boundary_translation, test_error_format_consistency) instead of maintaining their assertions. AUTH_REQUIRED recovery=correctable stays wire-grounded in the integration envelope tests (test_mcp_error_envelope, test_a2a_error_responses); a BDD scenario is tracked as a follow-up.
…d#1417) ACCOUNT_AMBIGUOUS resolution counted (and detected ambiguity over) all tenant accounts matching the natural key, including accounts the requesting agent cannot access — so the disclosed count leaked inaccessible accounts. Scope both list_by_natural_key and count_by_natural_key to the agent via an AgentAccountAccess join (shared _scope_natural_key helper so detection and count cannot drift), threading identity.principal_id from the resolver. Reproduced by a new hand-authored BDD scenario (BR-UC-002-account-access.feature, @account) across a2a/mcp/rest: a natural key matching 2 tenant accounts where the agent can access 1 now resolves to the accessible account instead of raising ACCOUNT_AMBIGUOUS that discloses the inaccessible one.
…ectable (prebid#1417) The pinned AdCP error-code enum classifies UNSUPPORTED_FEATURE as correctable and we emit correctable, so it is spec-conformant, not an 'intentional spec divergence'. Only the non-authoritative SDK STANDARD_ERROR_CODES table says terminal. Reframe the docstring (was inverting the authority).
…id#1417) BR-RULE-021 lives in docs/test-obligations/business-rules.md, not the non-existent docs/requirements/business-rules/BR-RULE-021.md.
The 4-branch wire-suggestion lookup was duplicated in TransportResult.assert_wire_error and the BDD _wire_suggestion step. Extract a single transport.extract_wire_suggestion(envelope) both call (salesagent-hm3r).
Replace the hand-rolled account-dict coercion at the REST sync_creatives handler with the to_account_reference helper its sibling handlers use (salesagent-hseb).
Parametrize the ProtocolEnvelope-return guard over both _update_media_buy_impl and _create_media_buy_impl so the create path is pinned too (salesagent-b983).
The guard accepted suggestion=None as 'present'; require a non-empty value so a dropped suggestion is caught (salesagent-ixmh).
) Follow-up to the access-scoped natural-key resolution (salesagent-ym1c): the natural_key_ambiguous / brand+op multi-match setups created matching accounts without granting the agent access, so post-fix they resolved to NOT_FOUND. Grant access so ambiguity fires over the agent's accessible accounts.
…rebid#1417) - item 2: CreateMediaBuyRequest validates idempotency_key shape (parity with UpdateMediaBuyRequest) so a malformed key on create emits VALIDATION_ERROR with the same tailored suggestion via the shared validate_idempotency_key_shape. - item 3: test_media_buy_v3 asserts update_result.response.errors (UpdateMediaBuyResult has no .errors attr — hasattr was always False, so the assertions were vacuous). - item 4: rename test_update_task_status.py -> test_update_media_buy_task_status.py (it tests update_media_buy wire TaskStatus; there is no update_task_status tool). item 1 (setup-incomplete error code) and item 5 (stale comment) tracked in the bead — item 1 is a protocol/wire-mapping decision (deferred), item 5 needs the reviewer's specific line pointer.
…re_path (salesagent-wb0q)
…in Givens (salesagent-d8w3)
…tools (salesagent-hk21, prebid#1172) canonical_agent_url now strips transport suffixes (/mcp, /a2a, well-known) on top of the SDK spec form; normalize_agent_url delegates to the shared suffix core. supported_format_keys/format_key/display helpers in creative_helpers are the single comparison key for the create-time package check, update_media_buy, sync_creatives assignments, and validate_creative_format_against_product; deleted the one-directional _has_supported_key /mcp retry and per-site display builders.
…alize_agent_url banned in src (salesagent-hk21, prebid#1172)
…salesagent-5mu0) process_bind_param now coerces dict values through model.model_validate() before dumping when the column declares a model — an invalid value (e.g. non-URL agent_url) is rejected at flush instead of persisting a row the typed read path can never load. Covers all writers of all six typed columns at once (inventory_profiles routes, products routes, mock-adapter config route). exclude_none storage dumping preserved.
…te path was dead code (salesagent-jrb5) Strengthen the pin test to assert the SUBMITTED ids round-trip (the old re-read of the seeded row passed with the fix present, absent, or reverted). Add tuple-truthiness structural guard.
…ind boundary validates (salesagent-bgzn)
… principal load (salesagent-c37g, prebid#1088) No nested get_db_session() per report — principal reads in the same transaction as the media_buy row. Shared make_delivery_response test helper replaces the duplicated mock response blocks.
…envelope hack (salesagent-ij9y)
The MCP wrapper now exposes the v3.1.1 get-signals-request fields flat
(matching every sibling tool) instead of a nested {req: {...}} schema
that rejected conformant buyers. SignalsEnv.call_mcp dispatches exactly
what a buyer sends.
…-006 roundtrip Givens (salesagent-h15q)
…terConfigRepository.get_or_create (salesagent-c3p6)
…re _wire_simple_env wiring branch (salesagent-ptog)
…properties (salesagent-6065) Adds ext, push_notification_config, if_pricing_version, if_wholesale_feed_version — spec-valid requests carrying them were rejected with INVALID_REQUEST on the extra=forbid arm and silently dropped in production. Route forwards body fields wholesale, so the boundary now accepts AND forwards them. Spec: dist/schemas/3.1.1/signals/get-signals-request.json @ v3.1.1.
…orts incl. REST (salesagent-1ek1) Dropped @mcp / @rest @A2A tags from the two success scenarios (local reconciled edit — mirror upstream): Transport.REST previously executed for ZERO UC-009 scenarios, leaving the prebid#1088 require_principal swap in _update_performance_index_impl ungraded on the REST wire. Both now run [a2a][mcp][rest]. Module docstring corrected to match reality.
… spec-optional semantics (salesagent-f2p3) Moving the account assert revealed two more dormant divergences the moment the asserts first executed: 'all 4 flags' over-specified presence (media-buy-features.json makes every flag optional — reconciled to declared-flags + only-if-present), and supported_pricing_models / reporting_delivery_methods are spec-optional sections production does not emit (filed salesagent-2ais). The eight substantive asserts now run green on every transport; the three genuine gaps are the strict-xfail graduation triggers.
…ail stands on brief_relevance (salesagent-ljaa) relevance_score has zero occurrences in the pinned v3.1.1 schemas (core/product.json has brief_relevance only) — the ordering assert demanded an off-schema field and could never graduate. brief_relevance (a genuine pinned-field production gap) remains the strict-xfail graduation trigger. Mirror-upstream notes added.
…ts (salesagent-th68) Filed the missing trackers: prebid#1591 anonymous discovery, prebid#1592 capabilities sections, prebid#1593 activate_signal + value_type, prebid#1594 remaining BDD wiring, prebid#1595 brief_relevance emission, prebid#1596 GetProductsBody buying_mode. The self-referential 'owned by the prebid#1088 boundary work' anonymous-discovery pointer now names prebid#1591.
…e format-conversion tests to write-boundary contract The bind-time typed-column validation (prebid#1172) exposed two things the full-suite gate caught: - sync_accounts stringified the BrandId RootModel (str() yields "root='spark'") poisoning the stored brand dict and natural key — now unwraps .root like the resolve path already did. - four format-conversion tests seeded invalid format dicts to exercise downstream defensive conversion, a state the write boundary now makes impossible (and the typed read path would reject anyway) — replaced with one parametrized write-boundary rejection test (factory-based; get_db_session allowlist shrinks by 4).
…up killed at 300s The e2e job's first stack-needing test spends its session-fixture setup on creative-agent acquisition + compose up + migrations (moved there when the pinned agent was un-gated). On a 4-core runner with the pin's ghcr tag not yet published (source-build fallback) that exceeds pytest-timeout's 300s, which killed the fixture at exactly 300s (collection 17:20:12, first output 17:25:12) and error-cascaded the remaining 40 tests. timeout_func_only=true scopes the 300s budget to test bodies; a genuinely hung bring-up now hits the job timeout instead. Test-body timeout semantics unchanged.
4 tasks
KonstantinMirin
marked this pull request as draft
July 14, 2026 15:26
KonstantinMirin
marked this pull request as ready for review
July 14, 2026 15:27
main carried prebid#1417 + prebid#1430 as squash-merges of THIS branch's own work, so the 53 conflicts were mostly two vintages of the same change; main additionally brought prebid#1537 (brand URL shorthand), the pillow/click CVE bump, and a docstring fix. Neither side dominated — resolved per file, union of intent. Resolution ledger: - mechanical: .duplication-baseline, .type-ignore-baseline -> pinned to the LOWER (ours: 35/80, 64); main's higher numbers would be allowlist growth. Re-measured on the merged tree: exactly 35/80, unchanged. tests/fixtures/creative_formats/reference_formats.json -> pinned to ours; it is generated by refresh-reference-formats.py, and we kept our generator (reads ADCP_PIN from creative-agent-stack.sh; main's hardcodes a stale digest). Fixture pin 467fd93d7711 matches the script's single source. .beads/.migration-hint-ts -> pinned to ours (data-store marker, never merged). uv.lock -> left to resolve against the merged pyproject; uv lock --check clean. .claude/notes/pr1417-reply-rereview.md -> ours. - semantic (main subsumes — post-review hardening our branch never received): exceptions.py (recovery follows the WIRE code; CONFIGURATION_ERROR becomes a terminal spec-supplement pass-through), auth.py (canonical AUTH_REQUIRED_SUGGESTION import, killing a duplicated literal), schema_helpers.py + validation_helpers.py (to_brand_reference funnel; adcp_validation_boundary gains field=), media_buy_create.py + media_buy_list.py + creatives/_assignments.py (creative loads via repository, not raw select(); CREATIVE_NOT_FOUND instead of a blanket VALIDATION_ERROR), and the guards/tests that grade them. - semantic (ours subsumes — later adcp-6.6 / spec-3.1.1 work): media_buy_update.py (unwrapped UpdateMediaBuySubmitted), schemas/_base.py, tests/harness/* (wire-envelope fidelity strictly increased), uc019/uc003 steps (wire-first asserts), factories/core.py, docker-compose.e2e.yml, refresh-reference-formats.py. - semantic (true two-way union): routes/api_v1.py + bdd/conftest.py + uc_get_products_inventory.py — main's prebid#1537 brand-shorthand coercion now runs INSIDE our prebid#1417 validation boundary, so a bad brand rejects through the same wire envelope. e2e_rest ledger (known_failures + EXPECTED_LEDGER + fitness + escape-hatch routes) -> 17 nodeids: main's graduations applied (never resurrected) and our 10 owner-approved parallel-e2e_rest artifacts kept. Set-equality between the ledger file and EXPECTED_LEDGER verified programmatically. - fixes found by the coherence gate (not by git): schemas/_base.py had a duplicated UpdateMediaBuyResult class (both sides added it; the union kept both copies) -> de-duped. main's test_brand_shorthand_coercion.py never passed the keyword-only 'paused' arg our branch added to _build_create_media_buy_request -> supplied it in the test helper rather than defaulting the param, which would have weakened the boundary-completeness guarantee that no-default signature exists to provide. Gate: full suite on the CI box — unit, integration, bdd_inprocess, bdd_e2e, admin, e2e, ui all OK (exit 0); dependency audit clean; duplication ratchet unchanged.
…onored (prebid#1619) The adcp-6.6 work exposed CreateMediaBuyRequest.paused on both transports and described it as working ('Create the media buy in a paused state (AdCP 3.1.1); delivery starts only once unpaused'). It is not: request-level paused is read nowhere on the create path (media_buy_create.py:2789 hardcodes paused=False; every other read is the package / adapter response). A buyer sending paused=true gets an active buy, and the buyer-facing MCP tool description promised otherwise. Restores the honest wording and points at prebid#1619. No behavior change — the functional fix (forward at REST, persist request.paused -> MediaBuy.is_paused, create the line item paused at the adapter, grade via BDD on the wire) is tracked in prebid#1619 and stays out of this consolidation PR. Also corrects the CreateMediaBuyBody.paused comment, which described it as the Package.status -> paused replacement; that is a different field (Package.paused). Create-level paused is the 3.1.1 create-in-paused-state flag.
…rt-boundary Advances the previously-merged validation-harness-base tip (73ce27d) by the 7 commits the PR has landed since, including its merge of main. Resolution ledger: - mechanical: uv.lock, tests/bdd/e2e_rest_known_failures.txt -> pinned wholesale to pr1585 (the owning side); our branch never touched them, so this is a single-side pin, not a content splice. .beads/ untouched by both sides. - semantic: src/a2a_server/adcp_a2a_server.py -> union. Ours added get_signals on MCP/A2A/REST (prebid#1088), introducing GetSignalsResponse; theirs added get_products brand-URL-shorthand coercion (prebid#1324/prebid#1537), introducing to_brand_reference. Adjacent import lines only; neither subsumes the other. Both symbols verified live in the merged body. - semantic: src/core/tools/media_buy_list.py -> cross-side union present on neither branch. Ours renamed get_principal_object -> find_principal (prebid#1088 slice 3); theirs replaced the raw select(Creative) with CreativeRepository and so dropped the Creative import. Took ours' rename AND theirs' import drop: the merged body calls find_principal (never get_principal_object) and reaches creatives only via CreativeRepository (never bare Creative). Taking either side wholesale would have left an unused import (F401) or an undefined name (F821).
…tor/transport-boundary Merges main's single commit (c7acbbd — bump adcp 5.7.0→6.6.0, spec 3.1.1) into the transport-boundary refactor. 33 conflicts resolved (32 semantic via per-file fresh-context resolvers, 1 mechanical baseline). Guiding principle: adopting 6.6 → main authoritative on SDK-version-specific schema/envelope shape; our transport-boundary behavior (identity plumbing, error codes, repositories) preserved. uv.lock regenerated from the merged manifest (adcp==6.6.0). Resolution ledger: mechanical: - .duplication-baseline -> tests:86 (ratchet, higher value; regen on CI box) - uv.lock -> regenerated via `uv lock` from merged pyproject (adcp 6.6.0) semantic — 6.6 subsumes stale 5.7 (main taken; no our-behavior lost): - src/core/schemas/_base.py -> 6.6 required status/confirmed_at/revision replace the 5.7 override block; our buyer_ref exclude moot (6.6 dropped buyer_ref) - src/core/schemas/creative.py -> 6.6 default_factory=list on changes/warnings/ errors; our model_dump/exclude overrides outside conflict, preserved - src/core/product_conversion.py -> 6.6 _normalize_legacy_placement helper - src/core/tools/products.py, dynamic_pricing_service.py -> our prebid#1172 FormatId cleanup already carried 6.6's `or []` Optional guard; ours kept - src/core/tools/creatives/_sync.py, media_buy_update.py -> comment-only (GH ref) semantic — envelope-shape vs identity-plumbing split (union across hunks): - src/core/tools/media_buy_create.py -> 6.6 _submitted_approval_result / CreateMediaBuySubmitted taken; our require_principal identity plumbing kept (taking ours would have NameError'd on removed response_packages) - src/admin/blueprints/operations.py -> 6.6 sync_success/echo_context/ _media_buy_webhook_metadata (ours would have left approve_context undefined) - src/a2a_server/adcp_a2a_server.py -> dropped our UpdateMediaBuySubmitted reconstruction branch: merged guard test proves it dead (early-return) - src/routes/api_v1.py -> kept 6.6 paused-forwarding; rewrote our stale comment; all 13 routes unique semantic — tests reconciled to 6.6 expected values, our setup/harness kept: - tests/harness/media_buy_dual.py -> 6.6 UpdateMediaBuyResult REST wrapping - tests/unit/test_media_buy.py, test_update_media_buy_behavioral.py -> our unwrapped UpdateMediaBuySubmitted access (matches _impl bare return) - tests/e2e/test_a2a_webhook_payload_types.py -> UNION: our shared-util refactor + main's try/finally auto-approval restore (shared-tenant-state correctness) - tests/bdd/steps/domain/uc003_update_media_buy.py -> 6.6 A2A no-artifacts guard (merged then-step calls it; dropping would NameError) - tests/bdd/conftest.py, given_media_buy.py -> 6.6 canonical step ownership (avoid duplicate cross-module registration); no xfail set grown - tests/unit/_architecture_helpers.py, test_architecture_schema_inheritance.py, test_architecture_harness_mcp_with_error_logging.py, test_bdd_e2e_enabled_ xdist_guard.py -> 6.6 guard supersets; no allowlist grown, no guard weakened - add/add (both branches created): test_admin_media_buy_reject_webhook.py, test_get_products_placement_schema.py, test_media_buy_dry_run_status.py, test_update_media_buy_pending_approval_status.py -> main superset, coverage unioned, 6.6 expected values - remaining test/comment conflicts -> GH ref over beads id (no-beads-in-code) scripts/creative-agent-stack.sh -> union: our ADCP_PIN v3.1.1 + main's comments
The merge provisionally pinned tests:86 (higher of the two sides). Regenerated on the CI box in the tests image (pylint 4.0.5, Linux — R0801 counts are platform-skewed, so the box is the reference): the merged tree actually has tests:74 duplicate blocks. Ratchet tightens, does not grow.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on
Stacked on #1585 (validation-harness base: adcp 6.6 consolidation), which itself stacks on #1417/#1430/#1567. Land those first and this diff shrinks to the transport-boundary work alone. Delta vs the base branch: KonstantinMirin/prebid-salesagent@feature/validation-harness-base...refactor/transport-boundary
What this resolves
Closes #1169, closes #1172, closes #1442 (residual scope), closes #1088. Also settles the
get_signalshalf of #1353 (registered on MCP/A2A/REST with a flat spec-shaped wire;activate_signalremains open there, tracked separately).Principalis resolved once at the transport boundary:ResolvedIdentity.principalis populated eagerly inresolve_identity()(zero extra queries — the boundary already loaded the row),require_principal/find_principalhelpers serve the_impls, background workers get aPrincipalRepository(they resolve other principals, not the caller), the dead lookup chain is deleted, and a new empty-allowlist AST ratchet bans principal DB lookups insrc/core/tools/(recursive scan).select(AdapterConfig)sites route throughAdapterConfigRepository;get_or_createis pinned to PK-only find.no_raw_selectallowlist −9.Product.format_ids/InventoryProfile.format_idsarelist[FormatId]from the DB boundary out:JSONType(model=…)validates raw dicts at bind (every writer covered), ~40 shape-reparsing branches deleted, and format-identity comparison now has ONE canonical key (transport-suffix stripping folded intocanonical_agent_url; sharedsupported_format_keys/format_keyhelpers replace four divergent sites; a guard keepsnormalize_agent_urlat zero callers).additionalProperties: trueon request schemas — prod-ignore is the compliant contract).Review
A 31-finding multi-agent review round (8 reviewers + synthesis, every finding independently re-verified against the head tree) ran before this PR opened; all 17 Should-fix items plus 4 zero-tolerance DRY/hygiene items are fixed in the branch. Details in the first comment.
Real bugs caught along the way, worth naming: a format-identity check that accepted a buyer's agent_url on update/sync but rejected the same input at create; a dead adapter-config write path behind a tuple-truthiness bug whose pin test couldn't fail; an unguarded typed-column write that could persist an unreadable row; a nested DB session per webhook report under scheduler fan-out; and — exposed by the new bind-validation itself —
sync_accountsstringifying aBrandIdRootModel into stored data.Follow-ups filed: #1591–#1596, plus
activate_signalconformance (remaining half of #1353).Verification
All verified green.