From ae145e33495d116f63781be68b48d39f2a76adad Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Sun, 1 Nov 2026 21:38:46 +0000 Subject: [PATCH] fix(reporting): refuse an adcp-reporting-storage of another storage contract at startup adcp 10.0.0b4 added offering_id to the ledger column lists that adcp-reporting-storage reads against its DDL; the published storage release (0.2.0b0) lacked the column, installed cleanly beside 10.0.0b4, and failed every reporting turn with UndefinedColumn, logged only as a configuration failure (#1763). adcp now names the ledger layout it reads with one integer, adcp.reporting.ledger.storage_contract.REPORTING_STORAGE_CONTRACT (2), and checks the installed ledger store against it at service startup (ReliableReportingService._initialize, before any schema work) and when postgres_storage() is built. A store declares its contract with a storage_contract attribute (adcp-reporting-storage 0.2.0b1+, #1787); a store from that distribution without one predates the declaration and is contract 1; other stores are not subject to the check. A mismatch raises ReliableReportingConfigurationError with REPORTING_SCHEMA_BEHIND, naming both installed versions, both contracts, the release to install and the migration to run, so the pair fails once and loudly instead of every turn. tests/test_reporting_storage_contract.py pins a fingerprint of the shared column lists to the contract number, so changing _record_rows without bumping the contract (and the storage DDL with it) fails the build. Co-Authored-By: Claude Fable 5.1 --- MIGRATION_v9_to_v10.md | 21 +++ docs/reliable-reporting-service.md | 4 + docs/reporting-subscriptions-errors.md | 2 +- docs/reporting-subscriptions-seller.md | 2 +- src/adcp/reporting/ledger/storage_contract.py | 124 ++++++++++++++++ src/adcp/reporting/service.py | 5 + src/adcp/storage/__init__.py | 3 + tests/test_reporting_storage_contract.py | 136 ++++++++++++++++++ 8 files changed, 295 insertions(+), 2 deletions(-) create mode 100644 src/adcp/reporting/ledger/storage_contract.py create mode 100644 tests/test_reporting_storage_contract.py diff --git a/MIGRATION_v9_to_v10.md b/MIGRATION_v9_to_v10.md index 21f89e33c..2e9be096f 100644 --- a/MIGRATION_v9_to_v10.md +++ b/MIGRATION_v9_to_v10.md @@ -41,3 +41,24 @@ the SDK-relevant inventory is [docs/adcp-3.3-sdk-inventory.md](docs/adcp-3.3-sdk | A received `null` is re-emitted; a Python `None` keyword argument is "not supplied" | A property whose schema admits `null` and that arrived as `null` (`model_validate`, `model_validate_json`, nested validation) stays in `model_fields_set` and is written back as `null` by `model_dump()`. Passing `None` as a keyword argument to a constructor, assigning `None` to a field, or passing `None` in `model_copy(update=...)` now leaves the field *unset*: it is omitted from the dump, and `model_dump(exclude_unset=True)` no longer lists it. A `None` inside a dict you validate is a wire `null`. Properties whose schema does not admit `null` are unchanged (`NoExplicitNull` refuses a JSON `null` and never emits one). | | `model_dump()` leaves an unset schema default off | A property the schema lists outside `required` with a `default` (or a `const`) is emitted only when it was set: by the document that was parsed, by a keyword argument, or by assignment. `sandbox: false`, `timezone: "inventory_local"`, `assignment_limit: 50` and the other schema defaults no longer appear on a dump unless you set them; the attribute still reads the default. Required discriminators (`asset_type`, `pricing_model`) are emitted either way. Pass `exclude_unset=False` for the SDK 9 output. Golden-file tests that compare whole dumps need regenerating. If you relied on a default reaching the peer, set it explicitly. `model_dump_as_received()` is deprecated: it returns the same document as `model_dump(mode="json")` and is removed in SDK 11. Generated root models (a union root such as `LocalizedCreativeAsset`) now subclass `AdCPRootModel` and dump with `exclude_none=True` like every other AdCP model. | | Unknown keys on open objects are kept and written back | `AdCPBaseModel` defaults to `extra='allow'`: a property the pinned model does not declare (a field a newer spec added, a sibling arm's property the schema admits) survives `model_validate` → `model_dump` in `model_extra`, including a `null` value. Generated classes whose schema object is closed (`additionalProperties: false`) keep `extra='forbid'`. SDK 9 dropped unknown keys (`extra='ignore'`). If your code passed stray keyword arguments and relied on them being dropped, remove them; `ADCP_STRICT_VALIDATION=1` turns the default into `forbid` so tests find them. | + +## Reporting storage contract (adcp-reporting-storage) + +adcp 10.0.0b4 widened the reporting ledger's generation key with `offering_id` +(AdCP 3.3 reporting subscriptions). The layout `adcp` and +`adcp-reporting-storage` share is now named by one integer on each side, +`adcp.reporting.ledger.storage_contract.REPORTING_STORAGE_CONTRACT` and +`adcp_reporting_storage.STORAGE_CONTRACT` (2 for SDK 10; SDK 9 and +adcp 10.0.0b0–b3 are 1): + +- `adcp-reporting-storage` 0.2.0b1 requires `adcp>=10.0.0b4,<11`; the + 0.2.0b0 / 0.1.0b4 releases do not work with 10.0.0b4 or later + ([#1763](https://github.com/adcontextprotocol/adcp-client-python/issues/1763)). +- Startup (`ReliableReportingService`, `postgres_storage`) refuses a pair whose + contracts differ with `REPORTING_SCHEMA_BEHIND`, naming both versions and + the release to install. +- An existing SDK 9 ledger is upgraded in place by `create_schema()` or + `python -m adcp migrate`: `offering_id` is added with `''` for every + existing account-level generation, obligation and status, and the applied + schema versions are recorded. Stop every SDK 9 reporting worker first; see + [docs/reporting-ledger-migration.md](docs/reporting-ledger-migration.md). diff --git a/docs/reliable-reporting-service.md b/docs/reliable-reporting-service.md index e0b7f8d80..afe2426b2 100644 --- a/docs/reliable-reporting-service.md +++ b/docs/reliable-reporting-service.md @@ -412,6 +412,10 @@ platform = reporting.install(MySeller()) # then serve the platform as usual (`--dsn` defaults to `$ADCP_DATABASE_URL`, then `$DATABASE_URL`). Startup never changes the schema: it refuses one that is behind with `REPORTING_SCHEMA_BEHIND`, naming the command and the pending versions. + Startup also refuses an installed `adcp-reporting-storage` whose storage + contract differs from this adcp's (`adcp.reporting.ledger.storage_contract`), + with the same code, naming both versions and the release to install, so an + incompatible pair fails once at startup rather than on every turn (#1763). - `0002 reporting_subscription_generations` widens the generation key and is a stopped-worker upgrade: stop every reporting worker before running it. `--status` flags it. diff --git a/docs/reporting-subscriptions-errors.md b/docs/reporting-subscriptions-errors.md index 8bd7716ed..237773150 100644 --- a/docs/reporting-subscriptions-errors.md +++ b/docs/reporting-subscriptions-errors.md @@ -16,7 +16,7 @@ Every SDK error carries `code` and `recovery` (`transient`, `correctable` or `te | `COVERAGE_INPUT_INCOMPLETE`, `COVERAGE_INPUT_INVALID` | `CoverageInputError` | n/a | correctable | supply the named input | | `COVERAGE_BUDGET_EXCEEDED` | `CoverageInputError` | n/a | correctable | narrow the input | | `REPORTING_STORAGE_NOT_DURABLE` | `ReliableReportingConfigurationError` | n/a | correctable | `storage=postgres_storage(...)`, or `ADCP_ENV=development` | -| `REPORTING_SCHEMA_BEHIND` | `ReliableReportingConfigurationError` | n/a | correctable | run `python -m adcp migrate` | +| `REPORTING_SCHEMA_BEHIND` | `ReliableReportingConfigurationError` | n/a | correctable | run `python -m adcp migrate`; or, when the installed `adcp-reporting-storage` implements another storage contract than this `adcp` ([#1763](https://github.com/adcontextprotocol/adcp-client-python/issues/1763)), install the release the message names | | `SUBSCRIPTIONS_REQUIRE_STANDARD_DIMENSIONS` | `ReliableReportingConfigurationError` | n/a | correctable | publish `standard_dimensions` on the named offering | | `SUBSCRIPTION_ROSTER_UNAVAILABLE` | `ReliableReportingConfigurationError` | n/a | correctable | serve `list_accounts`, or supply `SubscriptionOptions.accounts` | diff --git a/docs/reporting-subscriptions-seller.md b/docs/reporting-subscriptions-seller.md index b440aa2e9..04b99d22f 100644 --- a/docs/reporting-subscriptions-seller.md +++ b/docs/reporting-subscriptions-seller.md @@ -87,7 +87,7 @@ account.authorization = wind_down_grant(account.authorization) Pass one storage object: `memory_storage()` in development, `postgres_storage(dsn=... | pool=...)` in production. Process-local storage is refused outside development (`REPORTING_STORAGE_NOT_DURABLE`). -Apply schema changes with `python -m adcp migrate [--status | --dry-run]` before starting a release. Startup refuses a schema that is behind (`REPORTING_SCHEMA_BEHIND`). `0002 reporting_subscription_generations` is a stopped-worker upgrade: stop every reporting worker first. +Apply schema changes with `python -m adcp migrate [--status | --dry-run]` before starting a release. Startup refuses a schema that is behind (`REPORTING_SCHEMA_BEHIND`), and an installed `adcp-reporting-storage` of another storage contract than this `adcp` (same code; the message names the release to install). `0002 reporting_subscription_generations` is a stopped-worker upgrade: stop every reporting worker first. ## Workers and scaling diff --git a/src/adcp/reporting/ledger/storage_contract.py b/src/adcp/reporting/ledger/storage_contract.py new file mode 100644 index 000000000..4818280ce --- /dev/null +++ b/src/adcp/reporting/ledger/storage_contract.py @@ -0,0 +1,124 @@ +"""The reporting storage contract: the ledger layout adcp and adcp-reporting-storage share. + +adcp reads and writes the PostgreSQL ledger through the column lists in +:mod:`adcp.reporting.ledger._record_rows`; ``adcp-reporting-storage`` ships +the DDL those lists are read against. The two must agree exactly: a column one +side knows and the other does not fails every reporting turn with +``UndefinedColumn``, logged only as a configuration failure +(adcp-client-python#1763). The agreement is named by one integer on each side, +bumped together whenever the shared layout changes: + +* **1** — the SDK 9 layout and adcp 10.0.0b0–b3: a configuration generation is + ``(account_id, consumer_id, delivery_config_id, delivery_config_version)``. + Implemented by adcp-reporting-storage 0.1.0b4 and 0.2.0b0. +* **2** — adcp 10.0.0b4 and later: ``offering_id`` on configurations, + obligations and consumer statuses (AdCP 3.3 reporting subscriptions). + Implemented by adcp-reporting-storage 0.2.0b1 and later. + +:func:`require_storage_contract` runs at service startup and when +``postgres_storage`` is built, so an incompatible pair is refused once, with +both versions named and the fix spelled out, instead of failing every turn. +``tests/test_reporting_storage_contract.py`` pins the column lists to the +contract number, so changing them without bumping it fails the build; the +companion release gate (``scripts/check_companion_releases.py``) refuses to +publish either package against a companion of another contract. +""" + +from __future__ import annotations + +import hashlib +from importlib.metadata import PackageNotFoundError, version +from typing import Any + +__all__ = [ + "REPORTING_STORAGE_CONTRACT", + "declared_storage_contract", + "require_storage_contract", + "storage_contract_fingerprint", +] + +#: The storage contract this adcp reads the reporting ledger with. +REPORTING_STORAGE_CONTRACT = 2 + +#: The distribution that implements the contract for PostgreSQL. +STORAGE_DISTRIBUTION = "adcp-reporting-storage" +#: Its first release of the current contract. +STORAGE_RELEASE = "0.2.0b1" + + +def storage_contract_fingerprint() -> str: + """SHA-256 over the shared column lists; pinned per contract number in the tests.""" + from adcp.reporting.ledger import _record_rows + + lists = ( + _record_rows._CONFIGURATION_COLUMNS, + _record_rows._OBLIGATION_COLUMNS, + _record_rows._REVISION_COLUMNS, + _record_rows._ADJUSTMENT_COLUMNS, + _record_rows._STATUS_COLUMNS_BARE, + _record_rows._ISSUE_COLUMNS, + ) + normalized = "\n".join( + ",".join(column.strip() for column in columns.split(",")) for columns in lists + ) + return hashlib.sha256(normalized.encode("utf-8")).hexdigest() + + +def declared_storage_contract(store: Any) -> int | None: + """The contract ``store`` implements, or ``None`` for a store that declares none. + + A store declares it with a ``storage_contract`` attribute. A store from + the ``adcp_reporting_storage`` distribution without one predates the + declaration (0.1.0b4, 0.2.0b0) and implements contract 1. Any other + store (in-memory, an adopter's own) is not subject to the check. + """ + declared = getattr(store, "storage_contract", None) + if declared is not None: + return int(declared) + module = getattr(type(store), "__module__", "") or "" + if module == "adcp_reporting_storage" or module.startswith("adcp_reporting_storage."): + return 1 + return None + + +def _installed(distribution: str) -> str: + try: + return version(distribution) + except PackageNotFoundError: + return "unknown" + + +def require_storage_contract(store: Any) -> None: + """Refuse a ledger store whose storage contract differs from this adcp's. + + Raises :class:`adcp.reporting.ReliableReportingConfigurationError` with + code ``REPORTING_SCHEMA_BEHIND`` (recovery ``correctable``), naming both + installed versions, both contracts, the release to install and the + migration to run. + """ + implemented = declared_storage_contract(store) + if implemented is None or implemented == REPORTING_STORAGE_CONTRACT: + return + from adcp.reporting._errors import ReliableReportingConfigurationError + + adcp_version = _installed("adcp") + storage_version = _installed(STORAGE_DISTRIBUTION) + if implemented < REPORTING_STORAGE_CONTRACT: + fix = ( + f"Install {STORAGE_DISTRIBUTION}>={STORAGE_RELEASE} (storage contract " + f'{REPORTING_STORAGE_CONTRACT}), then run "python -m adcp migrate" or start the ' + "service, whose create_schema() upgrades the ledger in place." + ) + else: + fix = ( + f"Install the adcp release that reads storage contract {implemented}, or " + f"{STORAGE_DISTRIBUTION} of contract {REPORTING_STORAGE_CONTRACT}." + ) + raise ReliableReportingConfigurationError( + f"adcp {adcp_version} reads the reporting ledger with storage contract " + f"{REPORTING_STORAGE_CONTRACT}, but the installed {STORAGE_DISTRIBUTION} " + f"{storage_version} implements contract {implemented}; the pair would fail every " + f"reporting turn. {fix}", + code="REPORTING_SCHEMA_BEHIND", + config_field=STORAGE_DISTRIBUTION, + ) diff --git a/src/adcp/reporting/service.py b/src/adcp/reporting/service.py index d1afcbc5f..20544568f 100644 --- a/src/adcp/reporting/service.py +++ b/src/adcp/reporting/service.py @@ -1385,6 +1385,11 @@ def _require_durable_subscriptions(self) -> None: async def _initialize(self) -> None: self.validate() self._require_durable_subscriptions() + from adcp.reporting.ledger.storage_contract import require_storage_contract + + # A storage distribution of another contract fails once here, naming + # the pair and the fix, never on every turn (#1763). + require_storage_contract(self.store) async with self._configuration_lock: if self._storage is not None and self._storage.check_schema is not None: # Migrations own the schema; startup only refuses one that is behind. diff --git a/src/adcp/storage/__init__.py b/src/adcp/storage/__init__.py index 5701e803e..ebbe7601d 100644 --- a/src/adcp/storage/__init__.py +++ b/src/adcp/storage/__init__.py @@ -102,10 +102,13 @@ def postgres_storage( Resolved through the distribution's ``adcp.storage`` entry point, so ``adcp`` never imports it by name. """ + from adcp.reporting.ledger.storage_contract import require_storage_contract + build = _plugin("adcp.storage", "postgres") storage = build(pool=pool, dsn=dsn, clock=clock, max_size=max_size) if not isinstance(storage, ReportingStorage): raise TypeError("the adcp.storage postgres entry point must return ReportingStorage") + require_storage_contract(storage.ledger) # an incompatible distribution fails here return storage diff --git a/tests/test_reporting_storage_contract.py b/tests/test_reporting_storage_contract.py new file mode 100644 index 000000000..07a57017e --- /dev/null +++ b/tests/test_reporting_storage_contract.py @@ -0,0 +1,136 @@ +"""adcp refuses an adcp-reporting-storage of another storage contract at startup (#1763).""" + +from __future__ import annotations + +from typing import Any + +import pytest + +from adcp.reporting import ReliableReportingConfigurationError +from adcp.reporting.ledger import InMemoryReportingLedgerStore +from adcp.reporting.ledger import storage_contract as contract +from adcp.reporting.ledger.storage_contract import ( + REPORTING_STORAGE_CONTRACT, + declared_storage_contract, + require_storage_contract, + storage_contract_fingerprint, +) + +#: The shared column lists, pinned per contract. Changing a list in +#: ``_record_rows`` changes the fingerprint: bump REPORTING_STORAGE_CONTRACT, +#: ship the matching adcp-reporting-storage DDL and migration, and re-pin. +FINGERPRINTS = {2: "e6cb6454085a8fdc004f564f3882bcfe1a67fd9e622d72a36ba1f9d6ccd12af1"} + + +def test_the_column_lists_are_pinned_to_the_contract_number() -> None: + assert REPORTING_STORAGE_CONTRACT in FINGERPRINTS, "pin the new contract's fingerprint" + assert storage_contract_fingerprint() == FINGERPRINTS[REPORTING_STORAGE_CONTRACT], ( + "the shared ledger column lists changed: bump REPORTING_STORAGE_CONTRACT together " + "with adcp-reporting-storage's STORAGE_CONTRACT, DDL and migration, then re-pin" + ) + + +class _Declared: + storage_contract = REPORTING_STORAGE_CONTRACT + + +class _Older: + storage_contract = REPORTING_STORAGE_CONTRACT - 1 + + +class _Newer: + storage_contract = REPORTING_STORAGE_CONTRACT + 1 + + +class _Undeclared: + """A store from adcp-reporting-storage 0.1.0b4/0.2.0b0: no declaration at all.""" + + +_Undeclared.__module__ = "adcp_reporting_storage.ledger.pg" + + +class _Custom: + """An adopter's own store, outside the check.""" + + +@pytest.mark.parametrize( + ("store", "expected"), + [(_Declared(), 2), (_Older(), 1), (_Newer(), 3), (_Undeclared(), 1), (_Custom(), None)], + ids=["declared", "older", "newer", "undeclared-storage-release", "custom"], +) +def test_a_store_is_classified_by_declaration_or_provenance( + store: Any, expected: int | None +) -> None: + assert declared_storage_contract(store) == expected + + +def test_the_in_memory_store_and_custom_stores_pass() -> None: + require_storage_contract(InMemoryReportingLedgerStore()) + require_storage_contract(_Custom()) + require_storage_contract(_Declared()) + + +def test_a_storage_release_of_an_older_contract_is_refused_with_the_fix() -> None: + with pytest.raises(ReliableReportingConfigurationError) as refused: + require_storage_contract(_Undeclared()) + error = refused.value + assert error.code == "REPORTING_SCHEMA_BEHIND" and error.recovery == "correctable" + assert error.config_field == "adcp-reporting-storage" + message = str(error) + assert f"storage contract {REPORTING_STORAGE_CONTRACT}" in message + assert "implements contract 1" in message + assert "Install adcp-reporting-storage>=0.2.0b1" in message + assert 'run "python -m adcp migrate"' in message and "create_schema()" in message + + +def test_a_storage_release_of_a_newer_contract_is_refused_too() -> None: + with pytest.raises(ReliableReportingConfigurationError, match="implements contract 3"): + require_storage_contract(_Newer()) + + +async def test_service_startup_refuses_the_pair_before_touching_the_schema( + monkeypatch: pytest.MonkeyPatch, +) -> None: + from adcp.reporting.service import ReliableReportingService + from tests.test_reliable_reporting_service import NOW, _account_context + + store = InMemoryReportingLedgerStore(clock=lambda: NOW) + monkeypatch.setattr(InMemoryReportingLedgerStore, "storage_contract", 1, raising=False) + touched = [] + + async def create_schema() -> None: + touched.append(True) + + monkeypatch.setattr(store, "create_schema", create_schema) + service = ReliableReportingService( + store=store, clock=lambda: NOW, account_context=_account_context + ) + with pytest.raises(ReliableReportingConfigurationError) as refused: + await service.initialize() + assert refused.value.code == "REPORTING_SCHEMA_BEHIND" + assert touched == [], "the schema must not be created or upgraded on a refused pair" + + +def test_postgres_storage_refuses_the_pair_at_construction(monkeypatch: pytest.MonkeyPatch) -> None: + import adcp.storage as facade + from adcp.storage import ReportingStorage + + def build(**kwargs: Any) -> ReportingStorage: + return ReportingStorage( + durability="durable", ledger=_Undeclared(), principals=object(), subscriptions=object() + ) + + monkeypatch.setattr(facade, "_plugin", lambda group, name: build) + with pytest.raises(ReliableReportingConfigurationError, match="implements contract 1"): + facade.postgres_storage(dsn="postgresql://unused") + + +def test_the_installed_storage_distribution_declares_this_contract() -> None: + pytest.importorskip("adcp_reporting_storage") + from adcp_reporting_storage.contract import STORAGE_CONTRACT, adcp_storage_contract + from adcp_reporting_storage.ledger.pg import PgReportingLedgerStore + + assert STORAGE_CONTRACT == REPORTING_STORAGE_CONTRACT + assert adcp_storage_contract() == REPORTING_STORAGE_CONTRACT # storage reads our declaration + assert declared_storage_contract.__module__ == contract.__name__ + assert PgReportingLedgerStore.storage_contract == REPORTING_STORAGE_CONTRACT