From f9317b07bbda238886ea7e4d1ef34051accd28e1 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Thu, 14 May 2026 11:06:50 -0400 Subject: [PATCH 1/2] docs(decisioning): document AccountStore.resolve() as the multi-tenant encoding seam (#738) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adopters reading Protocol docstrings in isolation could mistake `account_id` for a wire-level buyer reference and either push tenant context down into downstream Protocol methods (#737) or parse it back out inside individual stores. Document the actual seam: buyers send a per-tenant `account_ref`, transport sets `tenant_id` from the host, and `AccountStore.resolve()` is the single layer that mints `Account.id`. Multi-tenant adopters compose tenant scope into the returned id; every downstream store treats it as opaque. * AccountStore — add "Multi-tenant deployments" section with example, point at `create_tenant_store` as the typed helper. * create_tenant_store — call out as canonical helper for the pattern. * ProposalStore / TaskRegistry — one paragraph each stating `account_id` is opaque and pointing back to the AccountStore seam. * PgProposalStore module docstring — explain the `(account_id, proposal_id)` PK uniqueness contract and show the generated-column pattern for adopters wanting a tenant_id FK without library changes. Closes #738. --- src/adcp/decisioning/accounts.py | 41 +++++++++++++++++++++++ src/adcp/decisioning/pg/proposal_store.py | 14 ++++++++ src/adcp/decisioning/proposal_store.py | 14 ++++++++ src/adcp/decisioning/task_registry.py | 12 +++++++ src/adcp/decisioning/tenant_store.py | 9 +++++ 5 files changed, 90 insertions(+) diff --git a/src/adcp/decisioning/accounts.py b/src/adcp/decisioning/accounts.py index 0a0f414f9..41c15649b 100644 --- a/src/adcp/decisioning/accounts.py +++ b/src/adcp/decisioning/accounts.py @@ -160,6 +160,47 @@ class AccountStore(Protocol, Generic[TMeta]): ``'explicit'`` (wire ref drives lookup), ``'implicit'`` (verified auth principal drives lookup), ``'derived'`` (single-platform with per-principal id synthesis). + + **Multi-tenant deployments — Account.id is the encoding seam.** + + Buyers send a per-tenant ``account_ref`` on the wire; sellers don't + control what string a buyer picks, and the same ``account_ref`` + ("acme", "default", sequential ids) may arrive from buyers calling + different seller tenants. The transport sets ``tenant_id`` from the + Host header (via :class:`~adcp.server.SubdomainTenantMiddleware`), + and ``resolve()`` is the single layer that mints ``Account.id`` — + the framework treats that id as opaque from here onward and + threads it into every downstream store + (:class:`~adcp.decisioning.ProposalStore`, + :class:`~adcp.decisioning.TaskRegistry`, framework idempotency + cache, future media-buy stores) as the canonical scope key. + + Multi-tenant adopters compose the tenant scope INTO ``Account.id`` + here, so every downstream store sees a globally-unique identifier + and never has to know about tenants:: + + class MyAccountStore: + resolution = "explicit" + + async def resolve(self, ref, auth_info=None): + tenant_id = self._tenant_from(auth_info) + buyer_ref = (ref or {}).get("account_id", "default") + return Account( + id=f"{tenant_id}:{buyer_ref}", # globally unique + metadata={"tenant_id": tenant_id}, + ) + + Pushing tenant scope DOWN into a downstream store (parsing + ``account_id`` back out inside ``ProposalStore.put_draft``, or + threading a separate ``tenant_id`` argument through every + Protocol method) is the wrong layer: it forces every store to + re-derive what ``resolve()`` already knows, and adopter + encoding-convention changes silently break every downstream + Protocol call site. + + :func:`~adcp.decisioning.create_tenant_store` ships this pattern as + a typed factory with a baked-in per-entry tenant-isolation gate — + use it directly unless you have a reason to write your own. """ resolution: ClassVar[str] diff --git a/src/adcp/decisioning/pg/proposal_store.py b/src/adcp/decisioning/pg/proposal_store.py index 7ccb5359c..999815ff9 100644 --- a/src/adcp/decisioning/pg/proposal_store.py +++ b/src/adcp/decisioning/pg/proposal_store.py @@ -59,6 +59,20 @@ def decode_recipe(payload: dict) -> Recipe: tenant isolation at the SQL level — ``WHERE account_id = %s`` is part of every query predicate, not a Python-level filter after fetch. +The schema's primary key is ``(account_id, proposal_id)``. That is +collision-safe **only** when ``account_id`` is globally unique across +your deployment. Multi-tenant seller adopters are expected to compose +tenant scope INTO ``Account.id`` at the +:class:`~adcp.decisioning.AccountStore` layer (see the AccountStore +docstring's "Multi-tenant deployments" section, or use +:func:`~adcp.decisioning.create_tenant_store`) — this store treats +``account_id`` as opaque and does not parse it. Adopters who want a +foreign key to their tenants table can add a generated column on top +of the schema this store ships (e.g. +``ALTER TABLE adcp_proposal_drafts ADD COLUMN tenant_id TEXT GENERATED +ALWAYS AS (split_part(account_id, ':', 1)) STORED``) and reference it +from their own migration; this store does not need to know. + Concurrency ----------- diff --git a/src/adcp/decisioning/proposal_store.py b/src/adcp/decisioning/proposal_store.py index 63b992d08..8b62c9163 100644 --- a/src/adcp/decisioning/proposal_store.py +++ b/src/adcp/decisioning/proposal_store.py @@ -145,6 +145,20 @@ class ProposalStore(Protocol): finalize_consumption-from-DRAFT, etc.) raise :class:`AdcpError` with ``code='INTERNAL_ERROR'`` — those are framework / adopter bugs, not buyer-facing rejections. + + **``account_id`` is opaque.** The framework threads + ``ctx.account.id`` (whatever the adopter's + :class:`~adcp.decisioning.AccountStore.resolve` returned) into + every method. The store MUST NOT parse it, split it, or re-derive + tenant scope from it. Multi-tenant adopters encode their tenant + scope into ``Account.id`` at the + :class:`~adcp.decisioning.AccountStore` layer once, and the + composite ``(account_id, proposal_id)`` then carries unique + identity across the entire deployment without needing a separate + ``tenant_id`` parameter on this Protocol. See the AccountStore + docstring's "Multi-tenant deployments" section for the canonical + encoding pattern, and :func:`~adcp.decisioning.create_tenant_store` + for the typed helper. """ is_durable: ClassVar[bool] diff --git a/src/adcp/decisioning/task_registry.py b/src/adcp/decisioning/task_registry.py index 9d3f1af02..3330dd1a1 100644 --- a/src/adcp/decisioning/task_registry.py +++ b/src/adcp/decisioning/task_registry.py @@ -188,6 +188,18 @@ class TaskRegistry(Protocol): task_id guessing. See ``tests/test_decisioning_task_registry_cross_tenant.py`` for the regression suite. + + **``account_id`` is opaque.** The framework threads + ``ctx.account.id`` (whatever the adopter's + :class:`~adcp.decisioning.AccountStore.resolve` returned) into + every method. The registry MUST NOT parse it or re-derive tenant + scope from it. Multi-tenant adopters encode their tenant scope + into ``Account.id`` once at the + :class:`~adcp.decisioning.AccountStore` layer; the registry then + gets a globally-unique scope key and the cross-tenant probe + check above degenerates to a simple equality. See the + AccountStore docstring's "Multi-tenant deployments" section for + the canonical encoding pattern. """ #: Whether this registry persists tasks across process restarts. diff --git a/src/adcp/decisioning/tenant_store.py b/src/adcp/decisioning/tenant_store.py index 6b0726552..c346080ce 100644 --- a/src/adcp/decisioning/tenant_store.py +++ b/src/adcp/decisioning/tenant_store.py @@ -518,6 +518,15 @@ def create_tenant_store( """Build an :class:`AccountStore` whose ``resolve`` / ``upsert`` / ``list`` / ``sync_governance`` methods enforce tenant isolation. + Canonical helper for the multi-tenant pattern described in + :class:`~adcp.decisioning.AccountStore` — composes tenant scope + into the returned ``Account.id`` so every downstream store + (:class:`~adcp.decisioning.ProposalStore`, + :class:`~adcp.decisioning.TaskRegistry`, framework idempotency + cache) sees globally-unique identifiers and treats tenancy as + opaque. Adopters writing a hand-rolled multi-tenant + :class:`AccountStore` should reach for this factory first. + :param resolve_by_ref: ``(ref, ctx) -> Account | None``. Resolves a wire :class:`AccountReference` to the framework Account it points at — independent of who the caller is. Return ``None`` From 197eefe31d6479b2e99d23046ef37e9b431e36ee Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Thu, 14 May 2026 11:12:38 -0400 Subject: [PATCH 2/2] docs(decisioning): address review feedback on multi-tenancy guidance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - accounts.py: note that this section applies to seller-side adopters (DSP-side adopters don't go through the resolve seam), and call out the tenant-rename footgun — the tenant value baked into Account.id must be a stable identifier (UUID or immutable slug), not a display name. - pg/proposal_store.py: replace the GENERATED ALWAYS AS (split_part(...)) STORED suggestion with the recommended path of a plain tenant_id column populated from application code. The generated-column approach bakes the encoding delimiter into adopter schema and silently corrupts the value on any AccountStore encoding change. Reformat as a sql code-block with the FK and index statements an adopter actually needs. Per review on #739. --- src/adcp/decisioning/accounts.py | 13 ++++++++++ src/adcp/decisioning/pg/proposal_store.py | 31 ++++++++++++++++++----- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/src/adcp/decisioning/accounts.py b/src/adcp/decisioning/accounts.py index 41c15649b..95386c490 100644 --- a/src/adcp/decisioning/accounts.py +++ b/src/adcp/decisioning/accounts.py @@ -163,6 +163,11 @@ class AccountStore(Protocol, Generic[TMeta]): **Multi-tenant deployments — Account.id is the encoding seam.** + Applies to seller-side adopters resolving incoming AdCP requests. + DSP-side adopters wiring this SDK as a client construct + :class:`~adcp.types.AccountReference` directly for outbound calls + and don't go through this seam. + Buyers send a per-tenant ``account_ref`` on the wire; sellers don't control what string a buyer picks, and the same ``account_ref`` ("acme", "default", sequential ids) may arrive from buyers calling @@ -198,6 +203,14 @@ async def resolve(self, ref, auth_info=None): encoding-convention changes silently break every downstream Protocol call site. + **Pick a stable tenant identifier.** The tenant value baked into + ``Account.id`` lives forever in every downstream store's row keys + and the framework's idempotency cache. Use a UUID or immutable + slug, not a user-facing display name — a tenant rename (vanity + URL change, white-label rebrand) that mutates the prefix would + orphan every proposal, task, and cached response keyed under the + old value. + :func:`~adcp.decisioning.create_tenant_store` ships this pattern as a typed factory with a baked-in per-entry tenant-isolation gate — use it directly unless you have a reason to write your own. diff --git a/src/adcp/decisioning/pg/proposal_store.py b/src/adcp/decisioning/pg/proposal_store.py index 999815ff9..bc2d13325 100644 --- a/src/adcp/decisioning/pg/proposal_store.py +++ b/src/adcp/decisioning/pg/proposal_store.py @@ -66,12 +66,31 @@ def decode_recipe(payload: dict) -> Recipe: :class:`~adcp.decisioning.AccountStore` layer (see the AccountStore docstring's "Multi-tenant deployments" section, or use :func:`~adcp.decisioning.create_tenant_store`) — this store treats -``account_id`` as opaque and does not parse it. Adopters who want a -foreign key to their tenants table can add a generated column on top -of the schema this store ships (e.g. -``ALTER TABLE adcp_proposal_drafts ADD COLUMN tenant_id TEXT GENERATED -ALWAYS AS (split_part(account_id, ':', 1)) STORED``) and reference it -from their own migration; this store does not need to know. +``account_id`` as opaque and does not parse it. + +Adopters who want a foreign key to their tenants table own the schema +on top of what this store ships. The recommended path is to add a +plain ``tenant_id`` column to ``adcp_proposal_drafts`` in your own +migration and populate it at write time alongside the framework's +``put_draft`` call (your adopter ProposalStore wrapper has the typed +:attr:`~adcp.decisioning.RequestContext.account` in scope): + +.. code-block:: sql + + ALTER TABLE adcp_proposal_drafts ADD COLUMN tenant_id TEXT; + ALTER TABLE adcp_proposal_drafts + ADD CONSTRAINT adcp_proposal_drafts_tenant_fk + FOREIGN KEY (tenant_id) REFERENCES tenants(id) + ON DELETE CASCADE; + CREATE INDEX adcp_proposal_drafts_tenant_idx + ON adcp_proposal_drafts (tenant_id); + +A Postgres ``GENERATED ALWAYS AS (...) STORED`` column derived by +parsing ``account_id`` is tempting but bakes the encoding delimiter +into adopter schema permanently — a later change to your AccountStore +encoding (adding a region prefix, swapping the separator) silently +corrupts the generated value. Mint ``tenant_id`` from application +code where the encoding is owned in one place. Concurrency -----------