fix: preserve full sync_accounts request for account stores

[bokelley]

Summary

• add an optional AccountStoreUpsertRequest.upsert_request(params, ctx) hook for full-request sync_accounts handling
• make PlatformHandler.sync_accounts prefer upsert_request when present, while preserving the existing upsert(params.accounts, ctx) fallback
• update advertised-tool detection and router denylist for the new account-store surface
• add regression coverage for push_notification_config and other request-level fields reaching the store

Root Cause

PlatformHandler.sync_accounts always extracted params.accounts before dispatching to AccountStore.upsert. Store implementations could not see request-envelope fields such as push_notification_config, so a real sync_accounts call could appear successful while losing webhook registration data.

Impact

Existing AccountStore.upsert(accounts, ctx) implementations remain compatible. Stores that need request-level fields can implement upsert_request(params, ctx) and still return the same shapes projected through the existing sync_accounts response path.

Closes #794.

Validation

• PYTHONPATH=/Users/brianokelley/conductor/adcp-client-python/.conductor/bangalore-v12/src pytest tests/test_decisioning_handler_shims.py tests/test_decisioning_types.py tests/test_platform_router.py tests/test_public_api.py
• pre-commit hooks during commit: black, ruff, mypy, bandit, whitespace/end-of-file, YAML/JSON checks, large-file, merge-conflict, case-conflict, private-key
• git diff --check

[aao-ipr-bot]

LGTM. Follow-ups noted below. Right shape: additive Protocol, legacy AccountStoreUpsert path preserved, zero wire-schema delta — purely an adopter-callback ergonomics fix for a real bug where push_notification_config / delete_missing / dry_run were being dropped at handler.py:2114 before reaching the store.

Things I checked

• Dispatch precedence at src/adcp/decisioning/handler.py:2097-2118 — upsert_request wins over legacy upsert when both are present. Richer hook wins, never both invoked. Right call.
• _call_with_optional_ctx shim is hook-agnostic (accounts.py:111-ish) and handles both def upsert_request(self, params, ctx=None) and def upsert_request(self, params). inspect.signature on a bound method strips self, so the "ctx" in params probe works for both shapes.
• Advertised-tool gate at src/adcp/decisioning/handler.py:1054-1061 widened symmetrically with the dispatch gate — no mismatch that would advertise sync_accounts then return OPERATION_NOT_SUPPORTED.
• Router denylist at src/adcp/decisioning/platform_router.py:155-157 adds upsert_request, and the drift-guard test test_account_store_methods_denylist_matches_protocols is extended to cover the new Protocol. Both edits required, both present.
• Idempotency layer wraps PlatformHandler.sync_accounts itself, so both hooks land downstream of the cache — replay semantics are identical across upsert / upsert_request.
• Wire shape unchanged: SyncAccountsRequest / SyncAccountsResponse Pydantic models are untouched. schemas/cache/3.0/account/sync-accounts-request.json unchanged.
• ctx_metadata credential-shaped-key fail-closed gate still load-bearing — the new path doesn't write to tool_ctx.metadata or bypass _build_request_context.
• Type-system layering: no breach. accounts.py imports SyncAccountsRequest from adcp.types under TYPE_CHECKING, not from generated_poc/.
• Test added for happy path (test_sync_accounts_prefers_full_request_upsert_hook) — call["params"] is req identity check is meaningful because model_construct skips coercion, so it pins zero defensive copy/cast in dispatch.

Follow-ups (non-blocking — file as issues)

• Webhook-credential exposure to adopter store (security-reviewer: Medium). params.push_notification_config.token and params.push_notification_config.authentication.credentials now reach upsert_request callees; the legacy upsert(refs, ctx) path stripped them. Bounded risk (requires an adopter to dump params), but worth either a docstring warning at src/adcp/decisioning/accounts.py:314-340 calling out the credential surface, or — better — passing params.model_copy(update={"push_notification_config": <redacted>}) to the hook, since the framework's webhook_emit.py already holds the full copy it needs for signing.
• Forward-compat asymmetry (ad-tech-protocol-expert). Legacy upsert(refs, ctx) adopters will silently drop any future envelope field the spec adds; upsert_request adopters get it for free. The Protocol docstring at accounts.py:316 says "Prefer this" but doesn't flag the silent-drop hazard. A versionchanged:: note pointing legacy implementers at the migration would help.
• Dual-impl diagnostic. An adopter mid-migration with both upsert_request and upsert defined gets silent ignore of the legacy method. A one-time logger.warning when both are callable would catch leftover dead code during migration. Not correctness — call it out in the next release note if not adding.
• Dropped-tool log message (handler.py:1054-1061 log path). _log_account_tool_dropped("sync_accounts", "upsert") always names only AccountStoreUpsert. Adopters reading the log get pointed at the legacy hook even when wiring AccountStoreUpsertRequest would be the cleaner choice. Extend to "Implement AccountStoreUpsert.upsert or AccountStoreUpsertRequest.upsert_request."
• Test coverage gap. No test pins the "both hooks defined" precedence. Add a store with both methods and assert upsert_request_calls == 1, upsert_calls == 0 so a future refactor can't silently flip dispatch order.

Minor nits (non-blocking)

1. Commit prefix. fix: for a new public Protocol class + new __all__ export is a notable choice — release-please will cut a patch bump, and the new surface lands in the changelog under bug fixes rather than features. The diff does fix #794, so fix: isn't wrong, but feat: is what gets adopters the minor-bump signal that a new opt-in Protocol exists.

Safe to merge.

[aao-ipr-bot]

LGTM. Clean additive Protocol — upsert_request runs ahead of legacy upsert, and the fail-closed gate keeps OPERATION_NOT_SUPPORTED semantics intact when neither hook is wired.

Real bug. The old extract-params.accounts-and-drop path was silently losing push_notification_config — webhook registrations vanished into a successful-looking response. Right fix shape: new optional Protocol, framework prefers it, legacy callers untouched.

Things I checked

• Routing precedence at src/adcp/decisioning/handler.py:2097-2115 — upsert_request first, upsert fallback, both probed via getattr(..., None) + callable(...). assert callable(upsert) at L2113 is mypy-narrowing only; the L2099 guard already shorted the _not_supported case.
• advertised_tools_for_instance at src/adcp/decisioning/handler.py:1054-1061 fail-closes correctly — accounts is None then can_sync_accounts = False and sync_accounts is dropped from tools/list. No regression for stores that wire neither hook.
• Denylist drift coverage at tests/test_platform_router.py:486-517 — set-XOR union now includes AccountStoreUpsertRequest, and upsert_request is in _ACCOUNT_STORE_METHODS at src/adcp/decisioning/platform_router.py:157. Router will not synthesize a tenant-keyed delegate over the new method.
• runtime_checkable Protocol works structurally — tests/test_decisioning_types.py:287-300 confirms a store with upsert_request only (no upsert) passes isinstance(_, AccountStoreUpsertRequest).
• Backwards compat — legacy upsert-only path covered by tests/test_decisioning_handler_shims.py:963-1001. Existing adopters keep working.
• Security posture — security-reviewer: None. push_notification_config is a typed Pydantic envelope field, not RequestContext.metadata; the _CREDENTIAL_SHAPED_KEY_SUFFIXES fail-closed at src/adcp/decisioning/dispatch.py is unrelated. Same _prime_auth_context + _make_resolve_context threading as the legacy path — no new tenant-isolation surface, no new SSRF surface (framework forwards the URL to the adopter, who already owns outbound posture). Response-projection credential scrubber unchanged and exercised by tests/test_decisioning_handler_shims.py:1252-1316.
• Public-API impact — adds AccountStoreUpsertRequest to adcp.decisioning exports at src/adcp/decisioning/__init__.py. Additive only; fix: defensible since the load-bearing motivation is a real bug, not a feature.

Follow-ups (non-blocking — file as issues)

• docs/handler-authoring.md:809-812 still names only AccountStoreUpsert in the "tool dropped" hint. Worth a pass to mention AccountStoreUpsertRequest as the preferred surface when request-envelope fields matter.
• The _log_account_tool_dropped("sync_accounts", "upsert") call hardcodes "upsert" in the log message — adopters with neither hook get steered toward the legacy Protocol when upsert_request is now the better default.

Minor nits (non-blocking)

1. Protocol body inconsistency. src/adcp/decisioning/accounts.py:341 uses raise NotImplementedError; the sibling AccountStoreUpsert.upsert at accounts.py:311 uses .... Functionally identical under runtime_checkable (structural name check, body never runs), but ... is the convention everywhere else in this file.
2. Docstring at src/adcp/decisioning/handler.py:2086 reads "AccountStoreUpsert Protocol" in the singular — the gate now covers either Protocol; "either Protocol" reads cleaner.

Safe to merge.