Skip to content

Integrate deal API with MCP#32

Open
Sirajmx wants to merge 11 commits into
mainfrom
feature/deal-api-mcp-integration
Open

Integrate deal API with MCP#32
Sirajmx wants to merge 11 commits into
mainfrom
feature/deal-api-mcp-integration

Conversation

@Sirajmx

@Sirajmx Sirajmx commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds DealsAPIMCPClient — an SSPClient connector for deals-api-mcp via MCP Streamable HTTP
  • Fixes SSPDeal.deal_id to store the internal UUID (deal.id) — all MCP tools validate dealId as z.string().uuid() and reject the IAB string format
  • Fixes persistent session: deals-api-mcp's TypeScript MCP SDK never resets _initialized after session termination, causing all second requests to
    fail with 400 Bad Request. Resolved with a class-level background task that holds the session open for the process lifetime
  • Fixes troubleshoot_deal to read buyer statuses from the correct path (status.buyerStatuses, not the non-existent top-level buyerSeats)
  • Adds 26 unit tests and docs/integration/deals-api-mcp.md

Test plan

  • ruff check passes on all PR files
  • pytest tests/unit/test_deals_api_mcp_client.py — 26 passed
  • Live: start deals-api-mcp (MCP_TRANSPORT=http MCP_PORT=3100 NODE_ENV=demo node dist/index.js), then hit /api/v1/deals/distribute twice in
    succession — both return 200 with internal UUID as deal_id

@Sirajmx
Sirajmx force-pushed the feature/deal-api-mcp-integration branch 3 times, most recently from 7e12710 to acc7d76 Compare July 21, 2026 13:35
@Sirajmx
Sirajmx marked this pull request as ready for review July 21, 2026 14:10

@atc964 atc964 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks — direction is right and it merges cleanly against current main. Three things before this can land, one hard blocker:

1. Blocker — the IAB deal ID is dropped. SSPDeal.deal_id now stores the internal UUID (required by the MCP tools' z.string().uuid()), but externalDealId — the OpenRTB deal ID a DSP needs for activation — is unreachable through the API: distribute_deal_via_ssp returns model_dump(exclude={"raw"}) so it's stripped (deal_service.py ~1027-1035), and the UUID leaks into DEAL_SYNCED events (execution_activation_flow.py ~297-309). Please add external_deal_id to SSPDeal, populate it from externalDealId, and include it in the distribute response + events. Without it, synced deals can't be transacted downstream.

2. The persistent-session workaround treats the wrong bug. The 400-on-second-request root cause is in deals-api-mcp itself: one shared StreamableHTTPServerTransport per process (index.ts ~388-399) instead of transport-per-session keyed by Mcp-Session-Id. Fixing it there lets this client drop the process-lifetime session task — which today can't survive any disconnect (reconnect hits the same 400 until seller-agent restarts) and has no test coverage (all 26 tests mock the transport).

3. Architecture — this isn't an SSP. deals-api-mcp is a deal-sync hub (real SSPs would sit behind it as providers). Registering it under SSPClient muddies provenance (ssp_type: "custom" in responses/events) and inventory-routing semantics. Preference: a DealSyncClient connector family peer to SSPClient/AdServerClient — the shared DealChannelClient base makes that nearly mechanical. If it must ship under SSPClient short-term: add SSPType.DEAL_SYNC (not CUSTOM), and note the migration in docs.

Smaller items: don't fabricate dealFloor: cpm or 1.0 (error on missing cpm); send the configured seller identity rather than advertiser/"IAB Deals MCP" as the Deal Sync seller; PG deals should set guar=1; honor the list_deals status filter; add tests for session lifecycle / update_deal / factory registration; document that buyer statuses currently come from the mock provider (demo-only); and please drop the unrelated formatting hunks in gam_soap_client.py / quote_history.py.

Happy to pair on the DealSyncClient split if useful.

Sirajmx and others added 6 commits July 22, 2026 16:21
… add tests and docs

deals-api-mcp's TypeScript MCP SDK sets _initialized=true on first session
and never resets it after termination, so a second initialize permanently
fails with 400. Fix: class-level persistent background task holds the session
open for the entire process lifetime; __aexit__ is a no-op so subsequent
requests reuse the session without re-initializing.

Also fixes SSPDeal.deal_id to store the internal UUID (deal.id) instead of
externalDealId — all MCP tools validate dealId as z.string().uuid() and
reject the IAB string format. Fixes troubleshoot_deal to read buyer statuses
from the correct response path (status.buyerStatuses, not top-level buyerSeats).

Adds 26 unit tests covering parse, create, get, list, clone, troubleshoot,
and status mapping. Adds docs/integration/deals-api-mcp.md with connection
lifecycle, configuration, status mapping, and persistent session explanation.
…t /api/v1/ssps from docs

deals-api-mcp has no dealType concept so the field was silently defaulting
to PMP regardless of what was requested. Now echo the requested deal_type
back on the returned SSPDeal so callers get an accurate response.

Also removes the GET /api/v1/ssps example from docs — that route does not
exist in the seller-agent REST API.
@atc964

atc964 commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Confirmed — 59e7993 resolves the externalDealId blocker: SSPDeal.external_deal_id carries the IAB/OpenRTB ID through _parse_deal, the DEAL_SYNCED payload, and the service response, with test coverage. Keeping the UUID for MCP tool calls was the right split.

Two remaining items:

  • DealSyncClient housing — we need this in the PR before merge. The deals-api-mcp connector isn't an SSP, and once it ships under SSPClient in a public release, moving it becomes a breaking API change. The DealChannelClient base makes the split nearly mechanical; happy to pair on it and to review quickly.
  • Session-lifetime fix — fine as a follow-up here, since the root cause is in deals-api-mcp itself (shared transport per process rather than per Mcp-Session-Id). Can you help get this in front of the deals-api-mcp owners? We've also filed it directly on that repo so it has a home there, and we're tracking it on our side — but owner awareness will matter for prioritization.

One note on CI: the red check isn't your change — the runner can't clone the private iab-agentic-primitives dependency, so the suite never runs (known issue, fixed once that repo goes public — which is waiting on the secret rotation on your/Todd's side). Until then we're running the full suite locally against PR heads as the merge gate; we've done that for 59e7993 and will re-verify the final head with the DealSyncClient change before merge.

@atc964

atc964 commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Design skeleton: DealSyncClient connector family (deals-api-mcp split out of SSPClient)

This is the full design for the restructure we asked for in review — every decision is made here, so the remaining work is mechanical. What moves: DealsAPIMCPClient leaves the SSP family (SSPClient / SSPRegistry / ssp_factory) and becomes the first member of a new peer connector family, DealSyncClient, alongside SSPClient and AdServerClient. What explicitly does NOT change: every deals-api-mcp wire call (deals_create / deals_status / deals_list / deals_update argument shapes, _SELLER_STATUS_MAP, _parse_deal), the class-level persistent-session workaround (root cause is upstream — deals-api-mcp#7 — the workaround moves verbatim), and the external_deal_id fix from 59e7993 (SSPDeal.external_deal_id stays on the model in ssp_base.py and keeps flowing through responses/events). The connector body, and all 26 tests in tests/unit/test_deals_api_mcp_client.py, move essentially untouched.

One correction to our own review framing: we previously described this split as riding on a shared DealChannelClient base — that class does not exist in the repo. SSPClient (src/ad_seller/clients/ssp_base.py) and AdServerClient (src/ad_seller/clients/ad_server_base.py) are both plain ABCs with no common parent. The split is still nearly mechanical, but the new family gets its own small ABC + registry + factory, mirroring the ssp_base.py / ssp_factory.py pattern exactly. DealSyncClient reuses the normalized models from ssp_base (SSPDeal, SSPDealCreateRequest, SSPDealStatus) — renaming those models is out of scope for this PR (see Non-goals).

Naming note: we keep the class name DealsAPIMCPClient (codebase convention is all-caps acronyms: MCPSSPClient, RESTSSPClient, FreeWheelMCPClient, GAMSoapClient), not DealsApiMcpClient.


1. New module: src/ad_seller/clients/deal_sync_base.py

~60 lines, styled after ssp_base.py:

# Author: Green Mountain Systems AI Inc.
# Donated to IAB Tech Lab

"""Abstract deal-sync client interface.

Deal-sync connectors push negotiated deals to an external deal
synchronization service (e.g. the IAB deals-api-mcp server), which
propagates them to buyer-side providers. This is a peer of the other
connector families:
  - AdServerClient:  inventory sync, deal setup in the publisher's ad server
  - SSPClient:       deal distribution through SSP exchanges to DSPs
  - DealSyncClient:  deal sync through an external deal-sync service

Reuses the normalized deal models from ssp_base (SSPDeal,
SSPDealCreateRequest, SSPDealStatus); giving this family its own
models is out of scope for the connector split.
"""

from abc import ABC, abstractmethod
from enum import Enum
from typing import Any, ClassVar, Optional

from .ssp_base import SSPDeal, SSPDealCreateRequest, SSPDealStatus


class DealSyncProvider(str, Enum):
    """Known deal-sync providers (extensible via config)."""

    DEALS_API_MCP = "deals_api_mcp"


class DealSyncClient(ABC):
    """Abstract base class for deal-sync integrations.

    Each provider implementation must provide these methods.
    The deal-sync registry manages configured providers.
    """

    channel: ClassVar[str] = "deal_sync"
    provider: DealSyncProvider
    provider_name: str = "Unknown Deal Sync Provider"

    @abstractmethod
    async def connect(self) -> None:
        """Establish connection to the deal-sync service."""
        ...

    @abstractmethod
    async def disconnect(self) -> None:
        """Disconnect from the deal-sync service."""
        ...

    async def __aenter__(self) -> "DealSyncClient":
        await self.connect()
        return self

    async def __aexit__(self, exc_type: Any, exc_val: Any, exc_tb: Any) -> None:
        await self.disconnect()

    # --- Deal Operations ---

    @abstractmethod
    async def create_deal(self, request: SSPDealCreateRequest) -> SSPDeal:
        """Create a new deal on the deal-sync service."""
        ...

    @abstractmethod
    async def get_deal(self, deal_id: str) -> SSPDeal:
        """Get deal details (and sync status) by provider-internal ID."""
        ...

    @abstractmethod
    async def list_deals(
        self,
        *,
        status: Optional[SSPDealStatus] = None,
        limit: int = 100,
    ) -> list[SSPDeal]:
        """List deals on this provider."""
        ...

    @abstractmethod
    async def update_deal(self, deal_id: str, updates: dict[str, Any]) -> SSPDeal:
        """Update mutable deal attributes."""
        ...

    # --- Health Check ---

    async def health_check(self) -> bool:
        """Check if the connection is healthy. Override for custom logic."""
        return True

Notes on the surface (derived from what DealsAPIMCPClient actually implements today):

  • create_deal / get_deal / list_deals / update_deal / health_check map 1:1 to deals_create / deals_status / deals_list / deals_update.
  • clone_deal and troubleshoot_deal are not part of the abstract base — they are SSP-isms that deals-api-mcp only emulates (clone = fetch + re-create; troubleshoot = a parse of deals_status). They stay on the concrete class as provider extras so the existing tests (TestCloneDeal, TestTroubleshootDeal) keep passing without edits.
  • The base's default __aenter__/__aexit__ call connect/disconnect; DealsAPIMCPClient keeps its persistent-session overrides exactly as-is.

Also in deal_sync_base.py, a minimal registry (no routing rules — that's an SSP concept; a single deal-sync channel needs name lookup + default only):

class DealSyncRegistry:
    """Registry for configured deal-sync clients, keyed by provider name."""

    def __init__(self) -> None:
        self._clients: dict[str, DealSyncClient] = {}
        self._default: Optional[str] = None

    def register(self, name: str, client: DealSyncClient) -> None:
        """Register a deal-sync client by provider name."""
        self._clients[name] = client
        if self._default is None:
            self._default = name

    def get_client(self, name: str) -> DealSyncClient:
        """Get a deal-sync client by provider name."""
        if name not in self._clients:
            raise KeyError(
                f"Deal-sync provider '{name}' not registered. "
                f"Available: {list(self._clients.keys())}"
            )
        return self._clients[name]

    def get_default(self) -> DealSyncClient:
        """Get the default (first-registered) deal-sync client."""
        if not self._default:
            raise RuntimeError("No deal-sync clients registered")
        return self._clients[self._default]

    def list_providers(self) -> list[str]:
        """List registered provider names."""
        return list(self._clients.keys())

2. Concrete subclass: src/ad_seller/clients/deals_api_mcp_client.py

git mv src/ad_seller/clients/ssp_deals_api_mcp_client.py src/ad_seller/clients/deals_api_mcp_client.py, then:

class DealsAPIMCPClient(DealSyncClient):
    """Deal-sync connector for deals-api-mcp via MCP Streamable HTTP."""

    provider = DealSyncProvider.DEALS_API_MCP
    provider_name = "IAB Deals MCP"

    # ... existing ssp_deals_api_mcp_client logic moves here unchanged ...

The entire delta to the moved file (everything else, including the persistent-session machinery, _SELLER_STATUS_MAP, _parse_deal, clone_deal, troubleshoot_deal, is byte-identical):

  1. Import DealSyncClient, DealSyncProvider from .deal_sync_base; drop SSPClient and SSPType from the .ssp_base import (keep SSPDeal, SSPDealCreateRequest, SSPDealStatus, SSPTroubleshootResult).
  2. Base class: SSPClientDealSyncClient.
  3. Add the two class attrs provider / provider_name shown above.
  4. The existing ssp_type = SSPType.CUSTOM / ssp_name = "IAB Deals MCP" class and __init__ attributes: delete the ssp_type ones (the SSPDeal/SSPTroubleshootResult model fields default to SSPType.CUSTOM, so drop the explicit ssp_type=self.ssp_type kwargs in _parse_deal and troubleshoot_deal too, or keep them — either way the value never reaches a payload after §4/§5 below; deleting is cleaner). Keep ssp_name = "IAB Deals MCP" feeding SSPDeal.ssp_name so _parse_deal and its tests are untouched — it's the shared model's display-name field, and shared-model renames are a non-goal.

Note the tiny test consequence of 4: TestParseDeal::test_ssp_name_and_type_always_set asserts ssp_type is set — it will still pass via the model default (SSPType.CUSTOM) whether or not you delete the explicit kwarg. No test edit needed beyond imports.


3. Registration: factory + config, before → after

src/ad_seller/clients/ssp_factory.py — remove the CUSTOM-typed registration

-    elif name_lower == "deals_api_mcp":
-        from .ssp_deals_api_mcp_client import DealsAPIMCPClient
-
-        if not settings.deals_api_mcp_url:
-            logger.warning("deals_api_mcp configured but DEALS_API_MCP_URL not set")
-            return None
-
-        return DealsAPIMCPClient(
-            mcp_url=settings.deals_api_mcp_url,
-            api_key=settings.deals_api_mcp_key,
-            seller_origin=settings.deals_api_mcp_seller_origin,
-        )
-
     else:
         logger.warning(

New: src/ad_seller/clients/deal_sync_factory.py (mirrors ssp_factory.py)

# Author: Green Mountain Systems AI Inc.
# Donated to IAB Tech Lab

"""Deal-sync registry factory — builds deal-sync clients from config.

Reads DEAL_SYNC_CONNECTORS from settings to build a DealSyncRegistry
with all configured providers.
"""

import logging
from typing import Any

from .deal_sync_base import DealSyncRegistry

logger = logging.getLogger(__name__)


def build_deal_sync_registry(settings: Any = None) -> DealSyncRegistry:
    """Build a DealSyncRegistry from application settings.

    Reads DEAL_SYNC_CONNECTORS (comma-separated list) and creates the
    appropriate client for each configured provider.
    """
    if settings is None:
        from ..config import get_settings

        settings = get_settings()

    registry = DealSyncRegistry()

    connectors = [s.strip() for s in settings.deal_sync_connectors.split(",") if s.strip()]

    for name in connectors:
        name_lower = name.lower()
        if name_lower == "deals_api_mcp":
            from .deals_api_mcp_client import DealsAPIMCPClient

            if not settings.deals_api_mcp_url:
                logger.warning("deals_api_mcp configured but DEALS_API_MCP_URL not set")
                continue

            registry.register(
                name_lower,
                DealsAPIMCPClient(
                    mcp_url=settings.deals_api_mcp_url,
                    api_key=settings.deals_api_mcp_key,
                    seller_origin=settings.deals_api_mcp_seller_origin,
                ),
            )
            logger.info("Registered deal-sync connector: %s", name_lower)
        else:
            logger.warning("Unknown deal-sync provider '%s'", name)

    return registry

src/ad_seller/config/settings.py

     # SSP Connectors (publishers can configure multiple SSPs)
     # Comma-separated list of SSP names to enable
-    ssp_connectors: str = ""  # e.g. "pubmatic,magnite,deals_api_mcp"
+    ssp_connectors: str = ""  # e.g. "pubmatic,magnite"
     ...
-    # IAB Deals MCP (deals-api-mcp server — HTTP Streamable transport)
+
+    # Deal Sync Connectors (external deal-sync services, peer of SSP connectors)
+    # Comma-separated list of provider names to enable
+    deal_sync_connectors: str = ""  # e.g. "deals_api_mcp"  (env: DEAL_SYNC_CONNECTORS)
+    # IAB Deals MCP (deals-api-mcp server — HTTP Streamable transport)
     deals_api_mcp_url: Optional[str] = None  # e.g. http://localhost:3100/mcp
     deals_api_mcp_key: Optional[str] = None  # IAB_DEALS_API_KEY on the MCP server
     deals_api_mcp_seller_origin: str = "publisher.example.com"  # origin field for deals_create

The three deals_api_mcp_* settings keep their names/env vars unchanged — only the connector-list knob changes: enable via DEAL_SYNC_CONNECTORS=deals_api_mcp instead of SSP_CONNECTORS=deals_api_mcp.

src/ad_seller/clients/__init__.py

Add alongside the existing SSP exports (lines 23–27 / 58–67):

from .deal_sync_base import DealSyncClient, DealSyncProvider, DealSyncRegistry
from .deal_sync_factory import build_deal_sync_registry
from .deals_api_mcp_client import DealsAPIMCPClient

(and the matching __all__ entries).


4. Provenance: distribute_deal_via_ssp response, before → after

src/ad_seller/services/deal_service.py::distribute_deal_via_ssp currently reports this connector as ssp_type: "custom" — the exact leakage we want gone. It also builds only build_ssp_registry().

Client resolution (replaces the current single-registry lookup; SSP behavior unchanged):

  1. Build both registries (build_ssp_registry(), build_deal_sync_registry()); the existing 503 no_ssps_configured fires only when both are empty.
  2. If request.ssp_name is set: try ssp_registry.get_client(name) first, then deal_sync_registry.get_client(name); the existing 400 ssp_routing_failed now lists available_ssps + available_deal_sync_providers. (The request field stays named ssp_nameSSPDealDistributeRequest in src/ad_seller/interfaces/api/routers/deals.py is public API surface; renaming it is a wire change and out of scope. ssp_name="deals_api_mcp" keeps working, now resolving via the deal-sync registry.)
  3. If no name: existing SSP routing (get_client_for(...)) when any SSP is registered; otherwise deal_sync_registry.get_default().

Response, field-level. SSP branch: unchanged except one additive key "channel": "ssp". Deal-sync branch:

 {
     "deal_id": result.deal_id,
     "external_deal_id": result.external_deal_id,
-    "ssp": result.ssp_name,                      # was "IAB Deals MCP"
-    "ssp_type": result.ssp_type.value,           # was "custom"  ← leakage
+    "channel": client.channel,                   # "deal_sync"
+    "provider": client.provider.value,           # "deals_api_mcp"
+    "provider_name": client.provider_name,       # "IAB Deals MCP"
     "status": result.status.value,
-    "deal": result.model_dump(exclude={"raw"}),
+    "deal": result.model_dump(exclude={"raw", "ssp_type", "ssp_name"}),
 }

The model_dump exclusion matters: SSPDeal (shared model) still carries ssp_type: SSPType = SSPType.CUSTOM internally, and without the exclusion "custom" would leak back in via the embedded deal object.

troubleshoot_deal_via_ssp (same file, and its callers in src/ad_seller/interfaces/api/routers/deals.py and src/ad_seller/interfaces/mcp_server.py) gets the same name-resolution fallback (step 2), so ?ssp_name=deals_api_mcp troubleshooting keeps working; apply the same ssp_type exclusion to its result serialization.


5. Provenance: DEAL_SYNCED event, before → after

src/ad_seller/flows/execution_activation_flow.py::distribute_to_ssps currently gates everything on settings.ssp_connectors and only consults build_ssp_registry. Two changes:

  1. The early return on empty settings.ssp_connectors must not skip deal-sync: run the SSP block only when ssp_connectors is set, then a parallel deal-sync block when settings.deal_sync_connectors is set (via build_deal_sync_registry(settings), registry.get_default()). Both failures stay non-fatal (same self.state.warnings.append pattern).
  2. Payloads. SSP branch: additive "channel": "ssp" only. Deal-sync branch (new):

State block:

-self.state.execution_orders.setdefault(deal.deal_id, {})["ssp_deal"] = {
-    "ssp_name": ssp_result.ssp_name,              # "IAB Deals MCP"
-    "ssp_deal_id": ssp_result.deal_id,
-    "external_deal_id": ssp_result.external_deal_id,
-    "ssp_status": ssp_result.status.value,
-}
+self.state.execution_orders.setdefault(deal.deal_id, {})["deal_sync"] = {
+    "provider": client.provider.value,            # "deals_api_mcp"
+    "deal_sync_deal_id": sync_result.deal_id,     # provider-internal UUID
+    "external_deal_id": sync_result.external_deal_id,
+    "status": sync_result.status.value,
+}

DEAL_SYNCED event payload (EventType.DEAL_SYNCED from src/ad_seller/events/models.py):

 payload={
-    "ssp_name": ssp_result.ssp_name,              # "IAB Deals MCP"
-    "ssp_deal_id": ssp_result.deal_id,
-    "external_deal_id": ssp_result.external_deal_id,
+    "channel": "deal_sync",
+    "provider": client.provider.value,            # "deals_api_mcp"
+    "provider_name": client.provider_name,        # "IAB Deals MCP"
+    "deal_sync_deal_id": sync_result.deal_id,     # provider-internal UUID
+    "external_deal_id": sync_result.external_deal_id,
 },

external_deal_id (the OpenRTB/IAB deal ID from 59e7993) is preserved verbatim in both.


6. Migration checklist (ordered, each step mechanical)

  1. git mv src/ad_seller/clients/ssp_deals_api_mcp_client.py src/ad_seller/clients/deals_api_mcp_client.py — no content change yet.
  2. Add src/ad_seller/clients/deal_sync_base.py (§1: DealSyncProvider, DealSyncClient, DealSyncRegistry).
  3. Add src/ad_seller/clients/deal_sync_factory.py (§3: build_deal_sync_registry).
  4. Apply the 4-item delta of §2 to deals_api_mcp_client.py (imports, base class, provider/provider_name, drop ssp_type attrs).
  5. src/ad_seller/config/settings.py: add deal_sync_connectors; regroup deals_api_mcp_* under a "Deal Sync Connectors" section (§3).
  6. src/ad_seller/clients/ssp_factory.py: delete the elif name_lower == "deals_api_mcp": branch.
  7. src/ad_seller/services/deal_service.py: dual-registry resolution + deal-sync response branch in distribute_deal_via_ssp; name-fallback in troubleshoot_deal_via_ssp (§4).
  8. src/ad_seller/flows/execution_activation_flow.py: restructure the ssp_connectors gate, add the deal-sync block + new DEAL_SYNCED payload (§5).
  9. src/ad_seller/clients/__init__.py: add the three new exports (§3).
  10. docs/integration/deals-api-mcp.md: SSP_CONNECTORS=deals_api_mcpDEAL_SYNC_CONNECTORS=deals_api_mcp; update the architecture blurb ("implements the generic SSPClient interface" → DealSyncClient), the example response (ssp/ssp_typechannel/provider/provider_name), and the file path on line 140.
  11. git mv tests/unit/test_deals_api_mcp_client.py stays at its path; update its one connector import (ad_seller.clients.ssp_deals_api_mcp_clientad_seller.clients.deals_api_mcp_client). The existing 26 connector tests move with the file; add one factory-registration test (build_deal_sync_registry with deal_sync_connectors="deals_api_mcp" + deals_api_mcp_url set registers provider "deals_api_mcp" as a DealsAPIMCPClient; empty/unset URL registers nothing).

7. Non-goals (explicitly out of scope for this PR)

  • The session-lifetime workaround stays. The class-level persistent MCP session in DealsAPIMCPClient moves verbatim; the root cause (TypeScript MCP SDK never resets _initialized) is upstream — tracked as deals-api-mcp#7. Do not touch it here.
  • No wire-format change — neither on the deals-api-mcp side (tool names/args identical) nor on our public request schema (SSPDealDistributeRequest.ssp_name keeps its name).
  • No shared-model rename. DealSyncClient reuses SSPDeal / SSPDealCreateRequest / SSPDealStatus from src/ad_seller/clients/ssp_base.py; SSPDeal.ssp_type remains on the model (defaulting to CUSTOM internally) but is excluded from every outward payload (§4). Giving the family its own models can be a follow-up.
  • clone_deal / troubleshoot_deal stay as concrete provider extras on DealsAPIMCPClient, not abstract methods on DealSyncClient.

@Sirajmx
Sirajmx force-pushed the feature/deal-api-mcp-integration branch from 59e7993 to 236ad7c Compare July 22, 2026 15:44
Splits deals-api-mcp out of the SSP family into a peer DealSyncClient
family (alongside SSPClient and AdServerClient), per reviewer design spec.

- Add deal_sync_base.py: DealSyncProvider enum, DealSyncClient ABC,
  DealSyncRegistry (register, get_client, get_default, list_providers)
- Add deal_sync_factory.py: build_deal_sync_registry() reads
  DEAL_SYNC_CONNECTORS env var (replaces SSP_CONNECTORS=deals_api_mcp)
- git mv ssp_deals_api_mcp_client.py → deals_api_mcp_client.py;
  DealsAPIMCPClient now extends DealSyncClient; drop ssp_type attrs,
  keep ssp_name; persistent-session machinery unchanged
- ssp_factory.py: remove deals_api_mcp branch
- settings.py: add deal_sync_connectors; regroup deals_api_mcp_* settings
- deal_service.py: dual-registry resolution in distribute_deal_via_ssp
  and troubleshoot_deal_via_ssp; deal-sync response uses
  channel/provider/provider_name (no ssp_type leakage); ssp_type excluded
  from deal-sync model_dump; ValueError → 400
- execution_activation_flow.py: parallel SSP + deal-sync blocks;
  DEAL_SYNCED payload uses channel/provider/provider_name/deal_sync_deal_id
- __init__.py: export DealSyncClient, DealSyncProvider, DealSyncRegistry,
  DealsAPIMCPClient, build_deal_sync_registry
- docs: update env var, architecture, response example, file path
- tests: update import path; add TestDealSyncFactory (3 tests)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@Sirajmx
Sirajmx force-pushed the feature/deal-api-mcp-integration branch from 236ad7c to ded4884 Compare July 22, 2026 15:47
@Sirajmx
Sirajmx requested review from atc964 and therevoltingx July 22, 2026 15:49
@atc964

atc964 commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Thanks @Sirajmx — the latest push addresses nearly everything from our review. Confirmed on the current head:

  • DealSyncClient connector family (own base, registry, factory, DEAL_SYNC_CONNECTORS) — exactly the restructure we asked for; the SSP-abstraction concern is resolved
  • external_deal_id now carries the OpenRTB/IAB deal ID through to the distribute response — the DSP-activation identity gap is closed
  • create_deal requires cpm instead of fabricating a $1.00 floor
  • Configured seller origin replaces advertiser-as-seller; PG deals now set guar=1 on the wire
  • Unrelated formatting edits removed

Two remaining asks before we merge:

  1. clone_deal still defaults dealFloor to 1.0 when the source deal has no floor (deals_api_mcp_client.py, clone path). Same never-invent-a-price rule as create_deal: raise if no real floor is available.
  2. The shared persistent-session code (session start, error latching, reuse) has no test coverage, and a dropped connection currently disables the connector until process restart. We know the full fix is blocked on the server-side transport bug (IABTechLab/deals-api-mcp#7), so for this PR: add lifecycle tests for the start/error/reuse paths, and a short note in docs/integration/deals-api-mcp.md stating the restart limitation until the upstream fix lands.

The failing CI is our private iab-agentic-primitives install issue, not your changes — being fixed separately. Once the two items above land we'll run our merged-tree verification and merge.

@therevoltingx

therevoltingx commented Jul 22, 2026

Copy link
Copy Markdown
Contributor
  1. clone_deal floor
    Addressed in 4cd984d: clone_deal no longer defaults dealFloor to 1.0. It uses an override or the source deal’s floor, and raises if neither is present (same rule as create_deal).

  2. Persistent session
    Lifecycle coverage for start / error-latching / reuse is in TestPersistentSession (tests/unit/test_deals_api_mcp_client.py). deals-api-mcp#7 is fixed upstream, so the restart-limitation note is no longer needed. On the current head we also reconnect after a dropped connection: the next aenter stops the old background task and starts a fresh session (_stop_shared_session + test_reconnects_after_session_drop). The upstream bug was fixed here: https://github.com/IABTechLab/deals-api-mcp/pull/8

Sirajmx added 2 commits July 23, 2026 16:56
…+ retry

Live E2E testing showed that after a server restart, session_alive returns
True (SDK SSE reconnect loop keeps background task alive) so the stale
session ID is reused. The tool call then hangs indefinitely: fresh server
returns 400 but no SSE response ever arrives.

Fix: wrap all call_tool calls through _call_tool which applies a 30s
timeout. On timeout or HTTP 400/404, forces _start_shared_session and
retries once. First request after a drop now succeeds in ≤30s; all
subsequent requests are instant on the new session.

Update docs to accurately reflect the reconnect mechanism.
- Fix list_deals sending sellerStatus instead of status; deals-api-mcp
  schema expects status (integer), sellerStatus was silently dropped so
  every filtered query returned all deals regardless of filter value
- Update TestListDeals assertions to match correct field name (status)
- Add TestUpdateDeal: covers tool name, ID pass-through, updates spread,
  and response parsing via _parse_deal (cpm, external_deal_id, status)
@Sirajmx

Sirajmx commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback @atc964
In addition to @therevoltingx commits added the few more fixes found during E2E tests.

  1. Stale session hang — added a _call_tool wrapper with a 30 s timeout. When a tool call times out or gets a 400/404, the
    session is automatically restarted and the call retried once. Prevents the indefinite hang caused by the MCP SDK keeping the
    session task alive after the server drops. Updated docs accordingly.
  2. list_deals status filter — was sending sellerStatus but deals-api-mcp expects status, so the filter was silently dropped and
    all deals were always returned. Fixed.
  3. TestUpdateDeal — added two unit tests covering tool name, ID/updates pass-through, and response parsing.

All 7 MCP tool paths E2E verified against live deals-api-mcp.

@therevoltingx

Copy link
Copy Markdown
Contributor

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants