Skip to content

Commit c596855

Browse files
bokelleyclaude
andcommitted
fix(reporting): refuse an adcp-reporting-storage of another storage contract at startup (#1789)
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 <noreply@anthropic.com>
1 parent 60aa18b commit c596855

8 files changed

Lines changed: 295 additions & 2 deletions

‎MIGRATION_v9_to_v10.md‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,3 +53,24 @@ the SDK-relevant inventory is [docs/adcp-3.3-sdk-inventory.md](docs/adcp-3.3-sdk
5353
| Generated enums name each value once | The `_1` alias members codegen emitted when an `anyOf` arm repeated an enum value (`System.zip_1`, `Field17.ad_request_url_1`, 19 enums on 10.0.0b4) are gone; each value has one member, the first-declared one. Code that referenced an alias name (`System.zip_1`) must use the real member (`System.zip`); `System("zip")` and `member.value` are unchanged. |
5454
| `list_account_changes` pages are checked and projected per caller | AdCP 3.3 requires non-empty `changed_paths` on every `adcp` / `seller_operator` change record (`[""]` for a whole-resource create, delete or purge) and lets the originating request's `idempotency_key` reach only the principal that sent it. A handler on 3.3 or later that returns such a record without `changed_paths` now fails the call with `VALIDATION_ERROR` (`field` `/changes/N/changed_paths`), whatever the validation mode; and every record loses `idempotency_key` unless it is an `adcp` record built with `adcp.account_change_feed.attribute_account_change(record, originator)` whose originator is the caller (`ToolContext.tenant_id` + `caller_identity`). Persist the originator when the mutation commits and attribute records when serving the page. `WebhookSender.prepare_account_change_recorded` / `send_account_change_recorded` also accept the change record itself (with `subscriber_id=`), refuse one without `changed_paths`, and never carry its `idempotency_key`. |
5555
| `get_adcp_capabilities` reports `adcp.implementation` | `serve()` (and `adcp.decisioning.serve()`) now adds AdCP 3.3 `adcp.implementation.sdks[]` to capabilities served at 3.3 or later: one entry `{"name": "adcp", "version": <installed adcp version>, "components": [...]}`, listing only components the server provably wires agent-wide (`transport` always; `types` for Decisioning platforms; `schema_validation` with strict request and response validation and no `2.5` in `supported_versions`; `authentication` with `auth=BearerTokenAuth(...)` and no widened discovery set; `idempotency` when every advertised mutating task is `@IdempotencyStore.wrap`ped; `webhooks` / `signing` only when the SDK owns and signs every outbound webhook; `lifecycle` never automatically). A 3.0 to 3.2 caller never sees it. Tests that compare a whole 3.3 capabilities response need the new key. Opt out with `serve(report_implementation=False)` (or `ServeConfig(report_implementation=False)`); add components or other SDKs with `ImplementationReport(extra_components=[...], extra_sdks=[...])`; or declare the block yourself (`Adcp(implementation=...)` on `DecisioningCapabilities`, `capabilities_response(implementation=...)`, or in your own handler's response), which the SDK never overwrites. `adcp.server.adcp_sdk_entry()` builds this SDK's entry for a hand-written block. `DecisioningCapabilities.adcp.implementation` was previously dropped from the projection; it is now emitted. |
56+
57+
## Reporting storage contract (adcp-reporting-storage)
58+
59+
adcp 10.0.0b4 widened the reporting ledger's generation key with `offering_id`
60+
(AdCP 3.3 reporting subscriptions). The layout `adcp` and
61+
`adcp-reporting-storage` share is now named by one integer on each side,
62+
`adcp.reporting.ledger.storage_contract.REPORTING_STORAGE_CONTRACT` and
63+
`adcp_reporting_storage.STORAGE_CONTRACT` (2 for SDK 10; SDK 9 and
64+
adcp 10.0.0b0–b3 are 1):
65+
66+
- `adcp-reporting-storage` 0.2.0b1 requires `adcp>=10.0.0b4,<11`; the
67+
0.2.0b0 / 0.1.0b4 releases do not work with 10.0.0b4 or later
68+
([#1763](https://github.com/adcontextprotocol/adcp-client-python/issues/1763)).
69+
- Startup (`ReliableReportingService`, `postgres_storage`) refuses a pair whose
70+
contracts differ with `REPORTING_SCHEMA_BEHIND`, naming both versions and
71+
the release to install.
72+
- An existing SDK 9 ledger is upgraded in place by `create_schema()` or
73+
`python -m adcp migrate`: `offering_id` is added with `''` for every
74+
existing account-level generation, obligation and status, and the applied
75+
schema versions are recorded. Stop every SDK 9 reporting worker first; see
76+
[docs/reporting-ledger-migration.md](docs/reporting-ledger-migration.md).

‎docs/reliable-reporting-service.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,10 @@ platform = reporting.install(MySeller()) # then serve the platform as usual
412412
(`--dsn` defaults to `$ADCP_DATABASE_URL`, then `$DATABASE_URL`). Startup
413413
never changes the schema: it refuses one that is behind with
414414
`REPORTING_SCHEMA_BEHIND`, naming the command and the pending versions.
415+
Startup also refuses an installed `adcp-reporting-storage` whose storage
416+
contract differs from this adcp's (`adcp.reporting.ledger.storage_contract`),
417+
with the same code, naming both versions and the release to install, so an
418+
incompatible pair fails once at startup rather than on every turn (#1763).
415419
- `0002 reporting_subscription_generations` widens the generation key and
416420
`0006 reporting_status_checkpoint_offerings` keys status checkpoints and
417421
status events by the generation's resolved offering. Both are

‎docs/reporting-subscriptions-errors.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ Every SDK error carries `code` and `recovery` (`transient`, `correctable` or `te
1616
| `COVERAGE_INPUT_INCOMPLETE`, `COVERAGE_INPUT_INVALID` | `CoverageInputError` | n/a | correctable | supply the named input |
1717
| `COVERAGE_BUDGET_EXCEEDED` | `CoverageInputError` | n/a | correctable | narrow the input |
1818
| `REPORTING_STORAGE_NOT_DURABLE` | `ReliableReportingConfigurationError` | n/a | correctable | `storage=postgres_storage(...)`, or `ADCP_ENV=development` |
19-
| `REPORTING_SCHEMA_BEHIND` | `ReliableReportingConfigurationError` | n/a | correctable | run `python -m adcp migrate` |
19+
| `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 |
2020
| `SUBSCRIPTIONS_REQUIRE_STANDARD_DIMENSIONS` | `ReliableReportingConfigurationError` | n/a | correctable | publish `standard_dimensions` on the named offering |
2121
| `SUBSCRIPTION_ROSTER_UNAVAILABLE` | `ReliableReportingConfigurationError` | n/a | correctable | serve `list_accounts`, or supply `SubscriptionOptions.accounts` |
2222

‎docs/reporting-subscriptions-seller.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ account.authorization = wind_down_grant(account.authorization)
103103

104104
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`).
105105

106-
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` and `0006 reporting_status_checkpoint_offerings` are stopped-worker upgrades: stop every reporting worker first.
106+
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` and `0006 reporting_status_checkpoint_offerings` are stopped-worker upgrades: stop every reporting worker first.
107107

108108
## Workers and scaling
109109

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
"""The reporting storage contract: the ledger layout adcp and adcp-reporting-storage share.
2+
3+
adcp reads and writes the PostgreSQL ledger through the column lists in
4+
:mod:`adcp.reporting.ledger._record_rows`; ``adcp-reporting-storage`` ships
5+
the DDL those lists are read against. The two must agree exactly: a column one
6+
side knows and the other does not fails every reporting turn with
7+
``UndefinedColumn``, logged only as a configuration failure
8+
(adcp-client-python#1763). The agreement is named by one integer on each side,
9+
bumped together whenever the shared layout changes:
10+
11+
* **1** — the SDK 9 layout and adcp 10.0.0b0–b3: a configuration generation is
12+
``(account_id, consumer_id, delivery_config_id, delivery_config_version)``.
13+
Implemented by adcp-reporting-storage 0.1.0b4 and 0.2.0b0.
14+
* **2** — adcp 10.0.0b4 and later: ``offering_id`` on configurations,
15+
obligations and consumer statuses (AdCP 3.3 reporting subscriptions).
16+
Implemented by adcp-reporting-storage 0.2.0b1 and later.
17+
18+
:func:`require_storage_contract` runs at service startup and when
19+
``postgres_storage`` is built, so an incompatible pair is refused once, with
20+
both versions named and the fix spelled out, instead of failing every turn.
21+
``tests/test_reporting_storage_contract.py`` pins the column lists to the
22+
contract number, so changing them without bumping it fails the build; the
23+
companion release gate (``scripts/check_companion_releases.py``) refuses to
24+
publish either package against a companion of another contract.
25+
"""
26+
27+
from __future__ import annotations
28+
29+
import hashlib
30+
from importlib.metadata import PackageNotFoundError, version
31+
from typing import Any
32+
33+
__all__ = [
34+
"REPORTING_STORAGE_CONTRACT",
35+
"declared_storage_contract",
36+
"require_storage_contract",
37+
"storage_contract_fingerprint",
38+
]
39+
40+
#: The storage contract this adcp reads the reporting ledger with.
41+
REPORTING_STORAGE_CONTRACT = 2
42+
43+
#: The distribution that implements the contract for PostgreSQL.
44+
STORAGE_DISTRIBUTION = "adcp-reporting-storage"
45+
#: Its first release of the current contract.
46+
STORAGE_RELEASE = "0.2.0b1"
47+
48+
49+
def storage_contract_fingerprint() -> str:
50+
"""SHA-256 over the shared column lists; pinned per contract number in the tests."""
51+
from adcp.reporting.ledger import _record_rows
52+
53+
lists = (
54+
_record_rows._CONFIGURATION_COLUMNS,
55+
_record_rows._OBLIGATION_COLUMNS,
56+
_record_rows._REVISION_COLUMNS,
57+
_record_rows._ADJUSTMENT_COLUMNS,
58+
_record_rows._STATUS_COLUMNS_BARE,
59+
_record_rows._ISSUE_COLUMNS,
60+
)
61+
normalized = "\n".join(
62+
",".join(column.strip() for column in columns.split(",")) for columns in lists
63+
)
64+
return hashlib.sha256(normalized.encode("utf-8")).hexdigest()
65+
66+
67+
def declared_storage_contract(store: Any) -> int | None:
68+
"""The contract ``store`` implements, or ``None`` for a store that declares none.
69+
70+
A store declares it with a ``storage_contract`` attribute. A store from
71+
the ``adcp_reporting_storage`` distribution without one predates the
72+
declaration (0.1.0b4, 0.2.0b0) and implements contract 1. Any other
73+
store (in-memory, an adopter's own) is not subject to the check.
74+
"""
75+
declared = getattr(store, "storage_contract", None)
76+
if declared is not None:
77+
return int(declared)
78+
module = getattr(type(store), "__module__", "") or ""
79+
if module == "adcp_reporting_storage" or module.startswith("adcp_reporting_storage."):
80+
return 1
81+
return None
82+
83+
84+
def _installed(distribution: str) -> str:
85+
try:
86+
return version(distribution)
87+
except PackageNotFoundError:
88+
return "unknown"
89+
90+
91+
def require_storage_contract(store: Any) -> None:
92+
"""Refuse a ledger store whose storage contract differs from this adcp's.
93+
94+
Raises :class:`adcp.reporting.ReliableReportingConfigurationError` with
95+
code ``REPORTING_SCHEMA_BEHIND`` (recovery ``correctable``), naming both
96+
installed versions, both contracts, the release to install and the
97+
migration to run.
98+
"""
99+
implemented = declared_storage_contract(store)
100+
if implemented is None or implemented == REPORTING_STORAGE_CONTRACT:
101+
return
102+
from adcp.reporting._errors import ReliableReportingConfigurationError
103+
104+
adcp_version = _installed("adcp")
105+
storage_version = _installed(STORAGE_DISTRIBUTION)
106+
if implemented < REPORTING_STORAGE_CONTRACT:
107+
fix = (
108+
f"Install {STORAGE_DISTRIBUTION}>={STORAGE_RELEASE} (storage contract "
109+
f'{REPORTING_STORAGE_CONTRACT}), then run "python -m adcp migrate" or start the '
110+
"service, whose create_schema() upgrades the ledger in place."
111+
)
112+
else:
113+
fix = (
114+
f"Install the adcp release that reads storage contract {implemented}, or "
115+
f"{STORAGE_DISTRIBUTION} of contract {REPORTING_STORAGE_CONTRACT}."
116+
)
117+
raise ReliableReportingConfigurationError(
118+
f"adcp {adcp_version} reads the reporting ledger with storage contract "
119+
f"{REPORTING_STORAGE_CONTRACT}, but the installed {STORAGE_DISTRIBUTION} "
120+
f"{storage_version} implements contract {implemented}; the pair would fail every "
121+
f"reporting turn. {fix}",
122+
code="REPORTING_SCHEMA_BEHIND",
123+
config_field=STORAGE_DISTRIBUTION,
124+
)

‎src/adcp/reporting/service.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1426,6 +1426,11 @@ def _require_durable_subscriptions(self) -> None:
14261426
async def _initialize(self) -> None:
14271427
self.validate()
14281428
self._require_durable_subscriptions()
1429+
from adcp.reporting.ledger.storage_contract import require_storage_contract
1430+
1431+
# A storage distribution of another contract fails once here, naming
1432+
# the pair and the fix, never on every turn (#1763).
1433+
require_storage_contract(self.store)
14291434
async with self._configuration_lock:
14301435
if self._storage is not None and self._storage.check_schema is not None:
14311436
# Migrations own the schema; startup only refuses one that is behind.

‎src/adcp/storage/__init__.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,13 @@ def postgres_storage(
102102
Resolved through the distribution's ``adcp.storage`` entry point, so
103103
``adcp`` never imports it by name.
104104
"""
105+
from adcp.reporting.ledger.storage_contract import require_storage_contract
106+
105107
build = _plugin("adcp.storage", "postgres")
106108
storage = build(pool=pool, dsn=dsn, clock=clock, max_size=max_size)
107109
if not isinstance(storage, ReportingStorage):
108110
raise TypeError("the adcp.storage postgres entry point must return ReportingStorage")
111+
require_storage_contract(storage.ledger) # an incompatible distribution fails here
109112
return storage
110113

111114

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
"""adcp refuses an adcp-reporting-storage of another storage contract at startup (#1763)."""
2+
3+
from __future__ import annotations
4+
5+
from typing import Any
6+
7+
import pytest
8+
9+
from adcp.reporting import ReliableReportingConfigurationError
10+
from adcp.reporting.ledger import InMemoryReportingLedgerStore
11+
from adcp.reporting.ledger import storage_contract as contract
12+
from adcp.reporting.ledger.storage_contract import (
13+
REPORTING_STORAGE_CONTRACT,
14+
declared_storage_contract,
15+
require_storage_contract,
16+
storage_contract_fingerprint,
17+
)
18+
19+
#: The shared column lists, pinned per contract. Changing a list in
20+
#: ``_record_rows`` changes the fingerprint: bump REPORTING_STORAGE_CONTRACT,
21+
#: ship the matching adcp-reporting-storage DDL and migration, and re-pin.
22+
FINGERPRINTS = {2: "e6cb6454085a8fdc004f564f3882bcfe1a67fd9e622d72a36ba1f9d6ccd12af1"}
23+
24+
25+
def test_the_column_lists_are_pinned_to_the_contract_number() -> None:
26+
assert REPORTING_STORAGE_CONTRACT in FINGERPRINTS, "pin the new contract's fingerprint"
27+
assert storage_contract_fingerprint() == FINGERPRINTS[REPORTING_STORAGE_CONTRACT], (
28+
"the shared ledger column lists changed: bump REPORTING_STORAGE_CONTRACT together "
29+
"with adcp-reporting-storage's STORAGE_CONTRACT, DDL and migration, then re-pin"
30+
)
31+
32+
33+
class _Declared:
34+
storage_contract = REPORTING_STORAGE_CONTRACT
35+
36+
37+
class _Older:
38+
storage_contract = REPORTING_STORAGE_CONTRACT - 1
39+
40+
41+
class _Newer:
42+
storage_contract = REPORTING_STORAGE_CONTRACT + 1
43+
44+
45+
class _Undeclared:
46+
"""A store from adcp-reporting-storage 0.1.0b4/0.2.0b0: no declaration at all."""
47+
48+
49+
_Undeclared.__module__ = "adcp_reporting_storage.ledger.pg"
50+
51+
52+
class _Custom:
53+
"""An adopter's own store, outside the check."""
54+
55+
56+
@pytest.mark.parametrize(
57+
("store", "expected"),
58+
[(_Declared(), 2), (_Older(), 1), (_Newer(), 3), (_Undeclared(), 1), (_Custom(), None)],
59+
ids=["declared", "older", "newer", "undeclared-storage-release", "custom"],
60+
)
61+
def test_a_store_is_classified_by_declaration_or_provenance(
62+
store: Any, expected: int | None
63+
) -> None:
64+
assert declared_storage_contract(store) == expected
65+
66+
67+
def test_the_in_memory_store_and_custom_stores_pass() -> None:
68+
require_storage_contract(InMemoryReportingLedgerStore())
69+
require_storage_contract(_Custom())
70+
require_storage_contract(_Declared())
71+
72+
73+
def test_a_storage_release_of_an_older_contract_is_refused_with_the_fix() -> None:
74+
with pytest.raises(ReliableReportingConfigurationError) as refused:
75+
require_storage_contract(_Undeclared())
76+
error = refused.value
77+
assert error.code == "REPORTING_SCHEMA_BEHIND" and error.recovery == "correctable"
78+
assert error.config_field == "adcp-reporting-storage"
79+
message = str(error)
80+
assert f"storage contract {REPORTING_STORAGE_CONTRACT}" in message
81+
assert "implements contract 1" in message
82+
assert "Install adcp-reporting-storage>=0.2.0b1" in message
83+
assert 'run "python -m adcp migrate"' in message and "create_schema()" in message
84+
85+
86+
def test_a_storage_release_of_a_newer_contract_is_refused_too() -> None:
87+
with pytest.raises(ReliableReportingConfigurationError, match="implements contract 3"):
88+
require_storage_contract(_Newer())
89+
90+
91+
async def test_service_startup_refuses_the_pair_before_touching_the_schema(
92+
monkeypatch: pytest.MonkeyPatch,
93+
) -> None:
94+
from adcp.reporting.service import ReliableReportingService
95+
from tests.test_reliable_reporting_service import NOW, _account_context
96+
97+
store = InMemoryReportingLedgerStore(clock=lambda: NOW)
98+
monkeypatch.setattr(InMemoryReportingLedgerStore, "storage_contract", 1, raising=False)
99+
touched = []
100+
101+
async def create_schema() -> None:
102+
touched.append(True)
103+
104+
monkeypatch.setattr(store, "create_schema", create_schema)
105+
service = ReliableReportingService(
106+
store=store, clock=lambda: NOW, account_context=_account_context
107+
)
108+
with pytest.raises(ReliableReportingConfigurationError) as refused:
109+
await service.initialize()
110+
assert refused.value.code == "REPORTING_SCHEMA_BEHIND"
111+
assert touched == [], "the schema must not be created or upgraded on a refused pair"
112+
113+
114+
def test_postgres_storage_refuses_the_pair_at_construction(monkeypatch: pytest.MonkeyPatch) -> None:
115+
import adcp.storage as facade
116+
from adcp.storage import ReportingStorage
117+
118+
def build(**kwargs: Any) -> ReportingStorage:
119+
return ReportingStorage(
120+
durability="durable", ledger=_Undeclared(), principals=object(), subscriptions=object()
121+
)
122+
123+
monkeypatch.setattr(facade, "_plugin", lambda group, name: build)
124+
with pytest.raises(ReliableReportingConfigurationError, match="implements contract 1"):
125+
facade.postgres_storage(dsn="postgresql://unused")
126+
127+
128+
def test_the_installed_storage_distribution_declares_this_contract() -> None:
129+
pytest.importorskip("adcp_reporting_storage")
130+
from adcp_reporting_storage.contract import STORAGE_CONTRACT, adcp_storage_contract
131+
from adcp_reporting_storage.ledger.pg import PgReportingLedgerStore
132+
133+
assert STORAGE_CONTRACT == REPORTING_STORAGE_CONTRACT
134+
assert adcp_storage_contract() == REPORTING_STORAGE_CONTRACT # storage reads our declaration
135+
assert declared_storage_contract.__module__ == contract.__name__
136+
assert PgReportingLedgerStore.storage_contract == REPORTING_STORAGE_CONTRACT

0 commit comments

Comments
 (0)