Skip to content

fix(decisioning): wire sync_accounts/list_accounts dispatch to AccountStore Protocols#610

Merged
bokelley merged 3 commits intomainfrom
bokelley/issue-609
May 10, 2026
Merged

fix(decisioning): wire sync_accounts/list_accounts dispatch to AccountStore Protocols#610
bokelley merged 3 commits intomainfrom
bokelley/issue-609

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

@bokelley bokelley commented May 10, 2026

Summary

  • Wires PlatformHandler.sync_accounts / list_accounts to route through platform.accounts.upsert / .list (the documented AccountStoreUpsert / AccountStoreList optional Protocols) — both shims previously fell through to the _not_supported baseline so every adopter implementing the optional Protocols was invisible on the wire.
  • Wire advertisement: _ACCOUNT_ADVERTISED_TOOLS = {"sync_accounts","list_accounts"} unioned into every sales-* entry of SPECIALISM_TO_ADVERTISED_TOOLS. Per-instance filter drops the tool when the store doesn't expose the optional Protocol method, with a one-line logger.info breadcrumb on first drop.
  • Defense-in-depth: shims run strip_credentials_from_wire_result on the projected envelope so loose dicts and Pydantic extra='allow' rows can't smuggle governance_agents[i].authentication or billing_entity.bank past the typed projections.
  • Bootstrap-path safety: account-roster shims call _prime_auth_context (registry buyer-agent gate only) instead of _resolve_account(None, ...), so explicit-mode stores can be populated via sync_accounts without the deadlock that no-ref resolve would create.

Closes #609. Replaces the salesagent/core/platforms/account_polyfill.py monkey-patch.

Storyboard impact

Sellers running media_buy_seller/refine_products/setup against an agent implementing the optional store Protocols graduate from skipped (missing_tool) to graded the moment they pull this in.

Known follow-ups (not blocking this PR)

  • Idempotency wrap on AccountStore-routed mutating ops. sync_accounts is mutating and the wire spec requires idempotency_key (≥16 chars), but the framework doesn't apply IdempotencyStore.wrap on the AccountStore Protocol path the way it does for create_media_buy. A retry storm currently re-executes upsert for each attempt — adopters dedupe by natural-key (brand + operator + sandbox) themselves. Tracking as a follow-up issue; surface this in the AccountStoreUpsert docstring once we have a wrapping story.

Test plan

  • test_advertised_tools_covers_every_specialism_wire_tool extended with the two new tools.
  • Positive: sync_accounts / list_accounts route to platform.accounts.upsert / .list with ResolveContext.tool_name set.
  • Negative: stores without upsert / list surface OPERATION_NOT_SUPPORTED via NotImplementedResponse — distinct from AttributeError.
  • Per-instance filter: tools dropped when store lacks methods; included when present.
  • Boot-time logger.info fires once per dropped tool (per-tenant when LazyPlatformRouter is wired).
  • Credential scrubber removes governance_agents[i].authentication and billing_entity.bank from extra='allow' adopter rows.
  • Pydantic-envelope passthrough exercises the model_dump projection path.
  • Explicit-mode bootstrap: shim must NOT invoke ExplicitAccounts.resolve(None, ...) before calling upsert.
  • Unexpected return-shape (None, tuple, bare string) emits a logger.warning and passes through for the wire validator.
  • Live end-to-end: sales_proposal_mode_seller storyboard runner's sync_accounts setup step passes output validation against schemas/cache/account/sync-accounts-response.json.
  • ruff check src/ clean.
  • mypy src/adcp/ clean (783 source files).
  • pytest tests/ 4275 passed, 26 skipped.

🤖 Generated with Claude Code

…tStore Protocols

PlatformHandler.sync_accounts and PlatformHandler.list_accounts were
inherited as _not_supported stubs from ADCPHandler — every adopter
implementing AccountStoreUpsert / AccountStoreList saw their account
roster invisible on the wire regardless of what the store declared.

This wires real shims that route through platform.accounts.upsert /
.list with a ResolveContext carrying the caller's verified AuthInfo
and registry-resolved BuyerAgent (same threading AccountStore.resolve
already uses, so adopter impls implementing principal-keyed gates —
e.g. spec BILLING_NOT_PERMITTED_FOR_AGENT — read the principal off
the canonical context).

Wire advertisement: _ACCOUNT_ADVERTISED_TOOLS = {"sync_accounts",
"list_accounts"} unioned into every sales-* entry of
SPECIALISM_TO_ADVERTISED_TOOLS. The per-instance filter at
advertised_tools_for_instance drops the tool when the store doesn't
expose the corresponding optional Protocol method (with a one-line
logger.info breadcrumb on first drop), so adopters who haven't wired
the optional Protocols don't over-advertise.

Defense-in-depth: shims project results through
strip_credentials_from_wire_result on the final envelope so loose
dicts and Pydantic extra='allow' rows can't smuggle
governance_agents[i].authentication or billing_entity.bank past the
typed projections.

Closes #609.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley and others added 2 commits May 10, 2026 06:25
…nts rows

The example's _SingleTenantAccounts.upsert returned rows shaped as
``{ref, account, operation}`` — a custom layout that worked while the
framework's sync_accounts dispatch was a no_supported stub but trips
schema output validation now that the dispatch actually invokes
accounts.upsert.

Reshape each row to the wire schema per
schemas/cache/account/sync-accounts-response.json:
``{account_id, brand, operator, action, status, ...}``. The framework
wraps the list as ``{"accounts": [...]}``.

Verified end-to-end against the running seller: storyboard runner's
sync_accounts step now passes output validation.

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

The shims called ``await self._resolve_account(None, tool_ctx)`` purely
to side-effect-stash the registry-resolved buyer-agent on
``ctx.metadata``. For ``AccountStore.resolution == "explicit"`` impls
without a principal/subdomain fallback, that no-ref resolve raised
``ACCOUNT_NOT_FOUND`` — deadlocking the bootstrap path that uses
``sync_accounts`` to populate the store in the first place.

Extract the side-effect into a ``_prime_auth_context`` helper that
runs the buyer-agent registry gate (suspended/blocked rejection still
fires) WITHOUT calling ``AccountStore.resolve``. Use it in both
account-roster shims; ``_resolve_account`` delegates to it for the
buyer-agent stash.

Also:
- Warn on unexpected ``upsert`` / ``list`` return shapes (None, tuple,
  bare string) so adopter contract violations aren't silent — the
  credential scrubber relies on dict-or-list shape.
- Document that the per-tenant handler created by
  ``LazyPlatformRouter`` emits the dropped-tool boot log per (tenant,
  tool) — intentional, since stores can be wired differently per
  tenant.
- Doc nit: ``AccountStoreList.list`` example shows binding ``filter``
  to ``filter_`` to avoid shadowing the builtin in the method body.

Test additions:
- explicit-mode bootstrap regression: shim must NOT call
  ``ExplicitAccounts.resolve(None, ...)`` first.
- unexpected-shape return triggers the new ``logger.warning``.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit dabf4fb into main May 10, 2026
16 checks passed
@bokelley bokelley deleted the bokelley/issue-609 branch May 10, 2026 11:13
bokelley added a commit to bokelley/salesagent that referenced this pull request May 10, 2026
…_accounts (#313)

* fix(storyboard): unblock pending_creatives, surface status, drop null asset fields, get-by-id ignores default-active gate

Knocks out three of the four open AdCP storyboard ``media_buy_seller``
failures the launch-readiness check picks up.

- closes #272: ``update_media_buy`` now transitions a buy out of
  ``pending_creatives`` once at least one ``creative_assignments`` entry
  is processed, regardless of creative review state — per the spec /
  storyboard ``pending_creatives_to_start`` flow, the buy state machine
  is decoupled from creative approval state. The success response also
  populates the spec-defined ``status`` field via the same
  ``_compute_status`` the read path uses, so the storyboard's
  ``status in [pending_start, active]`` assertion fires on the response
  envelope, not just the next ``get_media_buys`` poll. Best-effort —
  fetch failures fall back to ``status=None`` rather than break the
  response.

- closes #273 (follow-up): ``get_media_buys`` now skips the default
  ``active``-only status gate when the buyer explicitly names
  ``media_buy_ids`` *and* didn't supply an explicit ``status_filter``.
  An explicit ``status_filter`` still narrows results, preserving the
  by-id-with-status-filter contract. Storyboard
  ``inventory_list_targeting/get_after_create`` reads back a
  freshly-created (``pending_creatives``) buy by id; gating on
  ``active`` returned ``[]`` and the property_list/collection_list
  assertions all failed downstream. Mirrors the same change in
  ``_project_gam_buys`` so projected buys behave the same way.

- closes #274: ``list_creatives`` strips null-valued optional fields
  from each asset value before serialisation. The bundled
  ``list-creatives-response.json`` schema declares
  ``ImageAsset.format``/``alt_text``/``provenance`` as
  ``type: string``/``type: object`` (NOT nullable), so emitting
  ``format: null`` violates the asset oneOf and the storyboard
  validator rejects with ``Invalid input`` at
  ``/creatives/0/assets/<key>``. Pydantic ImageAsset allows null on
  those fields, but the strict spec is what we have to satisfy on the
  wire.

Plus a test ergonomics nit: ``scripts/storyboard-check.sh`` now honors
a ``BETWEEN_PROTOCOLS_HOOK`` env var that runs between MCP and A2A.
Storyboard scenarios mutate state (``update_swap_lists``) and reuse
idempotency keys per scenario; running both transports against the
same DB causes the second protocol's idempotency replay to return the
cached create response while reading the post-update DB state, surfacing
as bogus ``verify_create_persisted`` failures. Local runs against a
Docker stack should pass a SQL-truncate hook; remote runs leave it
unset (a no-op).

Storyboard result on a fresh ``docker compose up -d`` with the hook:

    AGENT_URL=http://localhost:8123 AGENT_TOKEN=ci-test-token \
    NO_SANDBOX=1 ALLOW_HTTP=1 BETWEEN_PROTOCOLS_HOOK="..." \
    ./scripts/storyboard-check.sh
    → MCP: 31 passed, 0 failed, 28 skipped
    → A2A: 26 passed, 3 failed, 30 skipped

The remaining A2A failures (#275: TERMS_REJECTED /
MEDIA_BUY_NOT_FOUND / PACKAGE_NOT_FOUND not surfacing) reproduce a
client-side bug in ``@adcp/sdk@6.10.0`` — its A2A response unwrapper
rejects any task whose ``status.state != "completed"``, so structured
``adcp_error`` envelopes on failed tasks get dropped before reaching
the validator. SDK 6.13+ already fixed this; ``npx -y @adcp/sdk``
defaults to 6.10.0 today because of the npm registry's minimum-package-
age suppression. Our server response is spec-compliant; the bug is
upstream of the agent.

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

* chore(storyboard-check): aggregate skip causes inline in summary

The SDK's always-on summary only reports skip *counts*, not *causes*, so
"30 skipped" doesn't tell you whether you're missing one tool or ten.
Post-process the --json output and print a "Skip causes" block grouped
by reason (with affected scenarios) so launch-readiness signal is
actionable without grovelling through the full ComplianceResult.

Switches the runner from --format json (human-readable) to --json
(structured ComplianceResult) so the post-processor has something to
read. Reads from the same /tmp/storyboard-<proto>.json file the script
already captures.

Filed upstream as adcontextprotocol/adcp-client#1623; this is a local
workaround until that lands. Companion issue
adcontextprotocol/adcp-client#1624 tracks the related "skip vs. fail"
classification gap (missing required-by-spec tools should fail, not
skip).

Sample output:

  ── A2A ──
    Steps:    26 passed, 3 failed, 30 skipped
    Failing steps:
      - measurement_terms_rejected/create_media_buy_aggressive_terms ...
    Skip causes:
      [14] missing_test_controller
            Affected: create_media_buy_async, delivery_reporting, ...
      [ 1] missing_tool: sync_accounts
            Affected: refine_products

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

* feat(accounts): wire sync_accounts/list_accounts to the wire (closes-the-skip)

Existing impls in src/core/tools/accounts.py have been complete since
PR prebid#1170 but the MCP/A2A wrappers got deleted with the legacy stack
(PR #17) and never got ported to the new core/platforms delegate path.
adcp 4.5.0's PlatformHandler also ships sync_accounts / list_accounts as
``_not_supported`` stubs — so even an adopter that overrides them on the
platform is invisible on the wire. The storyboard's
``refine_products/setup`` skipped against us with ``missing_tool:
sync_accounts`` because of this gap.

Wire path now:

  request → SpecDefaultsMiddleware (idempotency_key default)
         → MCP/A2A tool dispatch
         → PlatformHandler.sync_accounts  ← rebound by polyfill
         → platform.accounts.upsert(params, ctx)
         → SalesagentAccountStore.upsert
         → src.core.tools.accounts._sync_accounts_impl

Pieces:

- core/platforms/account_polyfill.py — runtime patch of
  ``PlatformHandler.sync_accounts`` / ``list_accounts`` to delegate to
  the platform's ``accounts`` store (where the framework's
  ``AccountStoreUpsert``/``AccountStoreList`` Protocols live; the
  framework's stub returns ``_not_supported`` instead).  Also extends
  ``_HANDLER_TOOLS["PlatformHandler"]`` and every ``sales-*`` entry of
  ``SPECIALISM_TO_ADVERTISED_TOOLS`` so the per-instance specialism
  filter doesn't strip the two tools back out of ``tools/list``.
  Imported as a side-effect from ``core/main.py`` so the patch applies
  before ``serve()`` constructs the handler. Drop once the framework
  wires this natively — file an upstream issue against
  adcontextprotocol/adcp-client.

- core/stores/accounts.py — ``SalesagentAccountStore`` grows
  ``upsert`` / ``list`` Protocol methods that forward to the existing
  ``_sync_accounts_impl`` / ``_list_accounts_impl``. Identity comes from
  the auth chain's ContextVars (same source ``_build_identity`` already
  uses); ``ResolveContext.agent`` is consulted as a fallback for non-
  HTTP entry points (lifespan, tests).

- core/middleware/spec_defaults.py — ``sync_accounts`` doesn't take an
  ``account`` field (the request IS the account discovery surface), so
  splitting the auth-fill defaults into ``_AUTH_FILLED_TOOLS`` (account
  + idempotency_key) and ``_IDEMPOTENCY_ONLY_TOOLS`` (sync_accounts).

- core/platforms/{mock,gam}.py — small comment pointing at the polyfill,
  plus dropping the now-unused delegate forwarders. Account dispatch
  flows through ``accounts`` (the AccountStore) per
  ``LazyPlatformRouter._ACCOUNT_STORE_METHODS``, not the per-platform
  delegation path.

Storyboard impact:

  Before: MCP 31 passed / 0 failed / 28 skipped
                with [1] missing_tool: sync_accounts (refine_products)
  After:  MCP 32 passed / 0 failed / 27 skipped (skip-cause cleared)

Verified end-to-end against ``@adcp/sdk@6.15.2`` on both transports —
``refine_products/setup`` graduates to graded; ``tools/list`` advertises
sync_accounts + list_accounts.

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

* docs(polyfill): point at upstream PR + removal path for the account dispatch monkey-patch

The fix landed upstream as adcontextprotocol/adcp-client-python#609.
Update the module docstring with the removal path so the next person
reading this knows when (and how) to drop the patch — bump adcp to the
release containing prebid#609 and delete this module + its import in
``core/main.py``.

* chore(accounts): forward-compat upsert/list signatures with adcp-client-python#610

PR prebid#610 supersedes the upstream fix I sent (prebid#609). The post-prebid#610
``AccountStoreUpsert`` / ``AccountStoreList`` contract is:

* ``upsert(refs: list[AccountReference], ctx: ResolveContext)``
* ``list(filter: dict | None, ctx: ResolveContext)``

vs. our polyfill's ``upsert(params: SyncAccountsRequest, ctx)`` /
``list(params: ListAccountsRequest, ctx)``. To make the eventual adcp
bump a one-line ``adcp >= X.Y`` change with no code rewrite,
``SalesagentAccountStore.upsert`` / ``.list`` now accept either shape
and normalise inside ``_coerce_sync_accounts_payload`` /
``_coerce_list_accounts_payload``:

* ``list[AccountReference]`` → ``SyncAccountsRequest(accounts=...)``
  with a synthesised idempotency key
* ``dict`` filter for list → ``ListAccountsRequest(**filter)``
* Empty filter dict / ``None`` → ``None`` (impl's no-filter path)
* Pre-shaped Pydantic / dict requests pass through unchanged
  (today's polyfill behavior preserved)

Polyfill docstring updated to point at prebid#610 instead of the closed prebid#609,
and to note that the store-side signatures are already future-compat
so nothing else needs to change at bump time except the requirement
version.

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

* chore(deps): bump adcp >= 4.6.1 and drop account dispatch polyfill

adcp 4.6.1 ships adcontextprotocol/adcp-client-python#610, which wires
``PlatformHandler.sync_accounts`` / ``list_accounts`` to dispatch
through the AccountStore Protocols (``accounts.upsert`` /
``accounts.list``) natively. The salesagent monkey-patch we added in
``core/platforms/account_polyfill.py`` is no longer needed:

* delete the polyfill module
* drop the load-bearing import in ``core/main.py``
* trim "see polyfill" references in the platform modules and
  ``core/stores/accounts.py``

The store-side ``upsert`` / ``list`` methods on
:class:`SalesagentAccountStore` keep their dual-shape coercion (refs
list / filter dict) — that's the framework's contract under prebid#610, no
change needed on this side.

Verified end-to-end:

* Container: ``adcp 4.6.1`` confirmed
* Boot log: ``mcp server advertising 12 of 12 tools`` (was 10/10
  before the polyfill landed; would still be 10 if the framework
  filter were dropping these). No polyfill breadcrumb.
* Storyboard against ``@adcp/sdk@6.15.2``:
    MCP: 32 passed, 0 failed, 27 skipped — STORYBOARD-OK
    A2A: 32 passed, 0 failed, 27 skipped — STORYBOARD-OK

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

* fix(docker): LOCKFILE_HASH build arg invalidates install layer on uv.lock change

Symptom: ``docker compose build && docker compose up -d`` after a
``uv lock --upgrade-package <foo>`` would silently keep the old version
of the bumped package inside the container. Hit this bumping
``adcp 4.5.0 -> 4.6.1`` — only ``docker rmi`` + full rebuild picked it
up. The BuildKit cache-mount on ``/cache/uv`` plus the ``COPY
pyproject.toml uv.lock`` layer hash short-circuited the install layer
even though the lockfile content had changed.

Fix: thread the uv.lock content sha256 as a build arg into the
``RUN uv sync --frozen`` step. When the lockfile content changes, the
ARG value changes, the layer cache key changes, the install step
re-runs against the fresh lockfile.

Wrap the dev workflow in two make targets so the next person doesn't
need to remember the incantation:

  make compose-build  # builds adcp-server with LOCKFILE_HASH wired in
  make compose-up     # build + force-recreate adcp-server

CLAUDE.md adds a breadcrumb pointing at the make target after a
dependency bump.

The existing ``docker compose up -d`` still works for non-lockfile-
changing rebuilds (source code edits) — those paths invalidate the
``COPY . .`` layer in the runtime stage and don't go through the
install step.

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

* fix(storyboard): expert-review pass — drop "unassigned" sentinel, harden identity, narrow except, fail-fast on missing LOCKFILE_HASH

Bundles the substantive findings from three parallel reviews
(code-reviewer / security-reviewer / ad-tech-protocol-expert) on the
branch before opening the PR.

Protocol (HIGH from ad-tech-protocol-expert):
- ``_build_sync_result`` no longer emits the literal ``"unassigned"``
  sentinel string for ``account_id``. Per ``sync-accounts-response.json``
  3.0.6+, ``account_id`` is optional on rejected/failed entries — the
  library type already declares ``str | None`` and ``exclude_none=True``
  drops the field on the wire. Buyers could otherwise round-trip
  ``"unassigned"`` as a real account ID into ``create_media_buy``.
  Plumbed the real ``account_id`` (or ``existing.account_id`` /
  ``account_id`` / ``db_acct.account_id``) through the five success
  call sites that previously relied on the sentinel.
- ``_check_domain_validity`` now exempts reserved RFC 2606 TLDs
  (``.test`` / ``.invalid`` / ``.example`` / ``.localhost``) when
  ``ADCP_TESTING=true`` — the AdCP storyboards intentionally use
  ``acmeoutdoor.example`` etc. as test domains, so the
  ``refine_products/sync_accounts`` setup step would otherwise fail
  with ``INVALID_DOMAIN`` on every compliance run.

Security (Consider from security-reviewer + Should-Fix #1 from
code-reviewer):
- Drop the ``getattr(ctx.agent, "tenant_id", None)`` fallback in
  ``SalesagentAccountStore._identity_from_ctx``. Dead code today
  (``BuyerAgent`` doesn't carry ``tenant_id``) but a non-obvious trust-
  boundary widening if the framework or a custom registry ever adds
  that attribute. ``BearerTokenAuthMiddleware`` is the canonical gate.

Code quality (Should-Fix from code-reviewer):
- ``SalesagentAccountStore._identity_from_ctx``: stamp the actual
  protocol via ``current_transport.get()`` instead of hard-coding
  ``"mcp"``. Mirrors ``core/platforms/_delegate.py:_build_identity``.
  No behavioral change today (no protocol-aware code branches on
  ``identity.protocol`` for sync_accounts/list_accounts), but
  closes a future-misroute trap.
- ``_coerce_sync_accounts_payload``: synthesised ``idempotency_key``
  uses ``uuid.uuid4()`` instead of ``id(payload)``. The Python object
  address is non-deterministic and reusable as soon as the list is
  GC'd; under concurrent retries that hit a recycled address the impl
  could treat distinct calls as duplicates.
- ``_update_media_buy_impl`` status-compute ``except`` narrowed from
  bare ``Exception`` to ``(SQLAlchemyError, ValueError, TypeError,
  StopIteration)``. Real prod errors (DB I/O, bad date math) and
  unit-test fixture artefacts (mocked datetimes, exhausted mock
  side_effects) are caught; ``AttributeError`` / ``KeyError`` /
  enum drift in ``_compute_status`` now intentionally bubble.

Listing (refinement of #274 fix):
- ``listing.py`` null-strip switched from "drop every null" to an
  allowlist of fields the bundled spec schema declares non-nullable
  (``format``, ``alt_text``, ``provenance``, ``mime_type``,
  ``container_format``). Future fields that meaningfully use ``null``
  as a sentinel won't be silently elided.

Docker (Should-Fix #6 from code-reviewer):
- ``Dockerfile`` ``LOCKFILE_HASH`` ARG default changed from ``unset``
  to ``set-this-build-arg`` and the install layer fails fast when
  the value is unchanged. Bare ``docker compose build`` callers who
  skip ``make compose-build`` get a clear error instead of silently
  reusing a stale install layer.
- ``.github/workflows/test.yml`` and ``publish-image.yml`` thread
  ``LOCKFILE_HASH=$(shasum -a 256 uv.lock | awk '{print $1}')`` so
  CI doesn't trip the new gate.

Other code-reviewer nits:
- ``scripts/storyboard-check.sh``: drop unused ``Counter`` import.
- ``media_buy_update.py``: clarify in-source why
  ``_determine_media_buy_status(creatives_approved=True)`` is
  hard-coded — spec text says ``pending_creatives`` clears on
  attachment, not on review approval. Tracked separately as a
  follow-up to align ``_determine_media_buy_status`` for the
  create-path (filed as #296).

Storyboard verified against ``@adcp/sdk@6.15.2``:

    MCP: 32 passed, 0 failed, 27 skipped — STORYBOARD-OK
    A2A: 32 passed, 0 failed, 27 skipped — STORYBOARD-OK

Upstream issues filed:
- adcontextprotocol/adcp-client-python#622 (nullable asset fields)
- adcontextprotocol/adcp-client-python#623 (optional ``account`` in
  typed dispatcher)

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

* fix(docker): thread LOCKFILE_HASH into db-init build args too

The Dockerfile's fail-fast on missing LOCKFILE_HASH was hitting db-init's
build because docker-compose.yml only wired the build arg under
adcp-server.build.args. Both services build from the same Dockerfile,
so both need the arg threaded — without it db-init exits 1 inside the
install step.

Caught by CI on PR #313 (Quickstart Docker Compose Test).

Also verified storyboard pass on @adcp/sdk@6.17.0:
  MCP: 32 passed, 0 failed, 27 skipped — STORYBOARD-OK
  A2A: 32 passed, 0 failed, 27 skipped — STORYBOARD-OK

Note: 6.17.0 ships native skip-cause aggregation in the always-on
summary (per adcp-client#1623 feature request). The local post-
processor in scripts/storyboard-check.sh becomes a no-op-equivalent
on 6.17+ but doesn't conflict — it parses the same data from --json.

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

* fix(ci): unbreak Lint + E2E after the LOCKFILE_HASH gate

Three CI failures from PR #313's previous push, all caused by the new
LOCKFILE_HASH fail-fast in the Dockerfile catching paths I missed:

* **Lint & Type Check**: ``src/services/delivery_webhook_scheduler.py:381``
  carried a ``# type: ignore[arg-type]`` for an adcp 4.5.0 type that 4.6.1
  loosened. mypy now flags it as ``[unused-ignore]``. Drop the comment.

* **E2E Tests**: ``tests/e2e/conftest.py`` shells out to
  ``docker-compose -f docker-compose.e2e.yml build`` directly with no
  LOCKFILE_HASH, hitting the fail-fast at every E2E suite startup.
  Compute the hash on the runner (``hashlib.sha256(uv.lock)``) and
  thread it into the subprocess env.

* **docker-compose.e2e.yml**: didn't have the build-args block. Add it
  so the ARG actually flows from compose env to the Dockerfile build.

* **Integration (media-buy)**: cascading "Database is unhealthy"
  circuit-breaker errors mid-run — pre-existing flaky pattern unrelated
  to this branch (passed on most runs in the same job, then failed on
  the simulator-restart teardown). Not addressed here; will retry CI.

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

* fix(e2e): UnboundLocalError on conftest's LOCKFILE_HASH path

Last commit's nested ``from pathlib import Path`` inside the
``docker_services_e2e`` fixture shadowed the module-level ``Path``
binding for the entire function (Python scoping rule: any name
bound in a function body is treated as local everywhere in that
body, even before the binding line). The earlier ``Path(".env")``
at line 114 then raised ``UnboundLocalError`` and every E2E test
failed at fixture setup.

Move ``hashlib`` import to module level alongside the existing
``Path`` import so the LOCKFILE_HASH branch uses the unshadowed
bindings.

Caught by CI on PR #313 (E2E Tests, fail at fixture setup).

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant