Skip to content

feat(server): AccountAwareToolContext + multi-tenant contract doc#245

Merged
bokelley merged 2 commits intomainfrom
bokelley/account-aware-context
Apr 20, 2026
Merged

feat(server): AccountAwareToolContext + multi-tenant contract doc#245
bokelley merged 2 commits intomainfrom
bokelley/account-aware-context

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Summary

First of three PRs addressing salesagent's feedback on adopting adcp.server. Closes two items from their list:

  • chore(main): release 0.1.1 #2 — account_id on ToolContext: ships AccountAwareToolContext(ToolContext) with account_id + account fields, plus resolve_account_into_context() that collapses the "resolve → check error → extract id" boilerplate. Reuses the TContext TypeVar machinery from TypeVar-bound ADCPHandler for typed ToolContext subclasses #223 so handlers get typed context without casting.
  • fix: correct tool name in list_creative_formats method #10 — multi-tenant contract doc: new docs/multi-tenant-contract.md enumerates every scope invariant (caller_identity, tenant_id, account_id, idempotency scope composition, cache key rules, context_factory purity) with the concrete failure mode each one produces. Consolidates what was spread across three docstrings.

No breaking changes. Existing ToolContext users are unaffected; the new class is additive.

Tested

  • 5 new tests for resolve_account_into_context (happy path, not-found, no-op, plain-ToolContext isolation, custom id attr).
  • 1 new test confirming AccountAwareToolContext flows through ADCPHandler[AccountAwareToolContext] dispatch.
  • Full suite: 1935 passed, 22 skipped locally.
  • ruff, mypy clean.

Test plan

  • CI green across Python 3.10–3.13
  • Review: does resolve_account_into_context belong on helpers.py or next to AccountAwareToolContext in base.py?
  • Review: is the "audit checklist" section at the bottom of multi-tenant-contract.md the right level of prescriptiveness, or should it link to a test template?

Related

Part of triaging feedback from salesagent (primary downstream of adcp.server). Follow-on PRs:

  • PR 1 (up next): MCP middleware parity with A2A SkillMiddleware, promote BearerAuthMiddleware from examples, A2A message_parser hook, custom tools passthrough, startup log for advertised tools.
  • PR 3: python -m adcp.migrate v3-to-v4 codemod, ADCP_STRICT_VALIDATION env flag, get_adcp_spec_version() / get_adcp_sdk_version() helpers, subclass passthrough test for response builders.

🤖 Generated with Claude Code

bokelley and others added 2 commits April 20, 2026 15:12
Ships the canonical subclass and helper for account-scoped handlers, and
consolidates the three scope invariants (caller_identity, tenant_id,
account_id) into a single audit-ready contract doc.

- src/adcp/server/base.py: new AccountAwareToolContext(ToolContext)
  with account_id + account fields, paired with the TypeVar machinery
  from #223. Lets handlers write
  ``class MyAgent(ADCPHandler[AccountAwareToolContext])`` and read
  ``context.account_id`` directly without casting through metadata.
- src/adcp/server/helpers.py: resolve_account_into_context() — the
  context-populating variant of resolve_account(). Collapses the
  three-line boilerplate (resolve → error check → id extract) into one
  call. When context is None or not an AccountAwareToolContext, runs
  resolution for the error path but leaves the context untouched so
  callers can drop the helper in without a type check.
- docs/multi-tenant-contract.md: enumerates every scope invariant
  (caller_identity stability, tenant_id required for tenant-scoped
  principal ids, account_id is request-scoped, idempotency scope
  composition, cache key rules, context_factory purity) with the
  concrete failure mode each one produces. Replaces the doc-grepping
  downstream reviewers were doing across three docstrings.
- docs/handler-authoring.md: cross-link to the contract doc from the
  Multi-tenant typing section + mention AccountAwareToolContext.
- README.md: add Handler authoring + Multi-tenant contract to the
  Documentation list.
- tests/test_server_helpers.py: 5 tests covering successful resolution,
  not-found, no-op, plain-ToolContext isolation, and custom id attr.
- tests/test_handler_typevar.py: end-to-end test that the shipped
  subclass flows through dispatch the same way a user-defined subclass
  does.

Addresses salesagent feedback items #2 (account_id on ToolContext) and
#10 (multi-tenant contract doc consolidation).

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

Code reviewer flagged three silent-None failure modes that violated the
multi-tenant scope contract this PR itself documents.

- src/adcp/server/helpers.py:
  * Change ``account_id_attr`` default from ``"id"`` to ``"account_id"``
    — matches the spec's generated ``Account`` type
    (src/adcp/types/generated_poc/core/account.py:155). Sellers returning
    a spec ``Account`` from their resolver were silently getting
    ``context.account_id = None`` via ``getattr(account, "id", None)``.
  * Raise ``ValueError`` when the resolved object is missing the
    attribute named by ``account_id_attr``. Silent ``None`` masked
    programmer errors and scoped downstream cache/audit keys to None
    — an I5-class failure per the multi-tenant contract doc.
  * Tighten ``context`` parameter to ``AccountAwareToolContext | None``
    so mypy catches the common case (handler parameterised with plain
    ``ToolContext``) at dev time.
  * Emit ``UserWarning`` when a non-``AccountAwareToolContext`` is
    passed at runtime — matches the pattern IdempotencyStore uses for
    its own silent-skip warning.
  * Move ``AccountAwareToolContext`` import to module top.

- tests/test_server_helpers.py:
  * Rename ``test_populates_account_aware_context`` →
    ``test_populates_from_spec_account_shape`` with a dataclass matching
    the spec's ``Account.account_id`` shape.
  * Rename ``test_plain_tool_context_is_not_mutated`` →
    ``test_plain_tool_context_warns_on_silent_skip`` and assert the
    warning fires. Previous test enshrined silent-skip as a feature.
  * Add ``test_missing_id_attr_raises``.
  * Add ``test_resolver_runtime_error_propagates`` — pins that non-
    AccountError exceptions propagate rather than silently convert to
    ACCOUNT_NOT_FOUND.

- tests/test_handler_typevar.py:
  * Replace direct-dispatch test with an executor-path variant through
    ``ADCPAgentExecutor`` with a ``context_factory`` — the transport
    path salesagent actually exercises. Previous test duplicated
    coverage already in ``test_typed_handler_works_under_a2a_executor``.

1937 tests passing, mypy clean.

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