fix-pack(roster-store): conform to AccountStore Protocol + clarify list-last comment

resolve(ref, ctx) didn't match the Protocol on main (resolve(ref, auth_info=None)),
so the framework dispatcher's resolve(ref_dict, auth_info=auth_info) kwarg call
would TypeError before reaching the dict lookup. Mirror the signature from the
existing reference impls (SingletonAccounts / ExplicitAccounts / FromAuthAccounts).
auth_info is unused — the roster IS the allowlist — but the parameter is required
for Protocol parity.

Tests added: isinstance(store, AccountStore) Protocol check, resolve called with
auth_info= as a kwarg (matches dispatcher), resolve called positionally without
auth_info, plus an upsert path for _by_id refs verifying the failed-row shape
conforms to SyncAccountsResultRow.

Reviewer flagged the list-last comment as cargo-cult given from __future__ import
annotations. Investigated: the annotations are lazified at runtime but mypy still
resolves them in class scope, so list-as-method does shadow the builtin for
subsequent annotations. Kept the ordering; rewrote the comment to explain the
actual reason (mypy class-scope lookup, not runtime annotation evaluation).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: bokelley

src/adcp/decisioning/roster_store.py

@@ -77,6 +77,7 @@
 
 if TYPE_CHECKING:
     from adcp.decisioning.accounts import ResolveContext
+    from adcp.decisioning.context import AuthInfo
     from adcp.types import AccountReference
 
 __all__ = ["create_roster_account_store"]
@@ -124,18 +125,21 @@ def __init__(self, roster: Mapping[str, Account[TMeta]]) -> None:
     async def resolve(
         self,
         ref: AccountReference | None,
-        ctx: ResolveContext | None = None,
+        auth_info: AuthInfo | None = None,
     ) -> Account[TMeta] | None:
         """Resolve a wire reference to a roster :class:`Account`.
 
         ``account_id``-arm refs hit a dict lookup; misses, natural-key
         refs, and ref-less calls return ``None``. The framework
         projects ``None`` to ``ACCOUNT_NOT_FOUND`` on the wire.
 
-        ``ctx`` is accepted for Protocol parity but unused — the
-        roster is the allowlist, no auth-based filtering at this layer.
+        Signature mirrors the :class:`AccountStore` Protocol's
+        ``resolve(ref, auth_info=None)`` — the framework dispatcher
+        passes ``auth_info`` as a keyword argument. ``auth_info`` is
+        accepted for Protocol parity but unused: the roster IS the
+        allowlist, no auth-based filtering at this layer.
         """
-        del ctx  # roster is the allowlist; no per-principal filtering
+        del auth_info  # roster is the allowlist; no per-principal filtering
         account_id = ref_account_id(ref)
         if account_id is None:
             return None
@@ -205,12 +209,14 @@ async def sync_governance(
             for entry in entries
         ]
 
-    # ``list`` is declared LAST in this class deliberately. Its name
-    # shadows the built-in ``list`` in class scope, so any subsequent
-    # method whose annotations use ``list[...]`` would resolve the
-    # method, not the built-in. Keeping it at the end means
-    # ``upsert``/``sync_governance`` annotations above resolve
-    # correctly.
+    # ``list`` MUST be declared LAST in this class. mypy resolves
+    # annotations in class scope; once ``list`` is defined as a method,
+    # any subsequent method whose annotations reference ``list[...]``
+    # binds to the method, not the builtin — even with
+    # ``from __future__ import annotations`` (lazy string evaluation
+    # at runtime doesn't change static-analysis class-scope lookup).
+    # Keeping ``list`` last means ``upsert``/``sync_governance`` above
+    # see the builtin.
     async def list(
         self,
         filter: dict[str, Any] | None = None,

tests/test_roster_store.py

@@ -18,7 +18,10 @@
 
 from adcp.decisioning import (
     Account,
+    AccountStore,
+    AuthInfo,
     ResolveContext,
+    SyncAccountsResultRow,
     SyncGovernanceEntry,
     create_roster_account_store,
 )
@@ -50,7 +53,7 @@ def _make_roster() -> dict[str, Account]:
 def test_resolve_hit_returns_account() -> None:
     """ref carrying a known ``account_id`` returns the roster entry."""
     store = create_roster_account_store(roster=_make_roster())
-    result = asyncio.run(store.resolve(_by_id("acct_alpha"), ResolveContext()))
+    result = asyncio.run(store.resolve(_by_id("acct_alpha")))
     assert result is not None
     assert result.id == "acct_alpha"
     assert result.name == "Alpha"
@@ -60,7 +63,7 @@ def test_resolve_miss_returns_none() -> None:
     """ref carrying an unknown ``account_id`` returns ``None`` —
     fall-through path the framework projects to ``ACCOUNT_NOT_FOUND``."""
     store = create_roster_account_store(roster=_make_roster())
-    result = asyncio.run(store.resolve(_by_id("acct_unknown"), ResolveContext()))
+    result = asyncio.run(store.resolve(_by_id("acct_unknown")))
     assert result is None
 
 
@@ -69,9 +72,7 @@ def test_resolve_natural_key_returns_none() -> None:
     curated rosters are queried by explicit id only. Adopters wanting
     natural-key resolution wrap ``resolve``."""
     store = create_roster_account_store(roster=_make_roster())
-    result = asyncio.run(
-        store.resolve(_by_natural_key("alpha.example.com", "alpha.example.com"), ResolveContext())
-    )
+    result = asyncio.run(store.resolve(_by_natural_key("alpha.example.com", "alpha.example.com")))
     assert result is None
 
 
@@ -81,10 +82,41 @@ def test_resolve_none_ref_returns_none() -> None:
     the helper returns ``None`` and adopters wrap to synthesize a
     publisher singleton when needed."""
     store = create_roster_account_store(roster=_make_roster())
-    result = asyncio.run(store.resolve(None, ResolveContext()))
+    result = asyncio.run(store.resolve(None))
     assert result is None
 
 
+def test_resolve_accepts_auth_info_kwarg() -> None:
+    """The framework dispatcher calls ``accounts.resolve(ref_dict,
+    auth_info=auth_info)`` — i.e. ``auth_info`` is a keyword argument
+    on every dispatch path. Verify the roster store accepts that exact
+    call shape (and ignores ``auth_info`` because the roster IS the
+    allowlist)."""
+    store = create_roster_account_store(roster=_make_roster())
+    auth = AuthInfo(kind="signed_request", principal="agent_foo", scopes=["read"])
+    result = asyncio.run(store.resolve(_by_id("acct_alpha"), auth_info=auth))
+    assert result is not None
+    assert result.id == "acct_alpha"
+
+
+def test_resolve_positional_no_auth_info() -> None:
+    """Positional single-arg calls (no ``auth_info``) keep working —
+    matches the Protocol's ``auth_info=None`` default."""
+    store = create_roster_account_store(roster=_make_roster())
+    result = asyncio.run(store.resolve(_by_id("acct_beta")))
+    assert result is not None
+    assert result.id == "acct_beta"
+
+
+def test_store_conforms_to_account_store_protocol() -> None:
+    """``AccountStore`` is ``runtime_checkable``; the framework's
+    boot-time platform validator calls ``isinstance(store,
+    AccountStore)``. Any structural drift between the roster store's
+    ``resolve`` signature and the Protocol breaks that check."""
+    store = create_roster_account_store(roster=_make_roster())
+    assert isinstance(store, AccountStore)
+
+
 def test_resolution_literal_is_explicit() -> None:
     """Boot-time platform validation reads ``store.resolution`` to
     fail fast on misconfigured deployments. Roster stores are
@@ -149,6 +181,31 @@ def test_upsert_empty_refs_returns_empty() -> None:
     assert rows == []
 
 
+def test_upsert_denies_by_id_refs_with_conformant_row_shape() -> None:
+    """The typical ``sync_accounts`` shape carries ``account_id``-arm
+    refs (buyer pre-selected an account id, calling sync to bind
+    governance / verify exists). Roster stores reject those entries
+    too — accounts are publisher-curated. Verify the failed row's
+    shape conforms to :class:`SyncAccountsResultRow` (instance type +
+    required fields populated, so the framework's wire projector
+    won't crash on a missing field)."""
+    store = create_roster_account_store(roster=_make_roster())
+    rows = asyncio.run(
+        store.upsert([_by_id("acct_alpha"), _by_id("acct_unknown")], ctx=ResolveContext())
+    )
+    assert len(rows) == 2
+    for row in rows:
+        assert isinstance(row, SyncAccountsResultRow)
+        assert row.action == "failed"
+        assert row.status == "failed"
+        assert row.errors is not None
+        assert row.errors[0]["code"] == "PERMISSION_DENIED"
+        # id-arm refs don't carry brand/operator; failed rows surface
+        # empty defaults (the buyer correlates by request order).
+        assert row.brand == {}
+        assert row.operator == ""
+
+
 def test_upsert_echoes_brand_operator_for_natural_key_refs() -> None:
     """``SyncAccountsResultRow.brand`` and ``operator`` are required
     on the wire. For natural-key refs we echo them back so the buyer
@@ -257,5 +314,5 @@ def test_external_mutation_does_not_leak_into_store() -> None:
     assert ids == {"acct_alpha", "acct_beta"}
     assert "acct_attacker" not in ids
 
-    attacker = asyncio.run(store.resolve(_by_id("acct_attacker"), ResolveContext()))
+    attacker = asyncio.run(store.resolve(_by_id("acct_attacker")))
     assert attacker is None