Skip to content

TurnContext.services refactor - #486

Closed
Rodrigo Brandão (rodrigobr-msft) wants to merge 10 commits into
mainfrom
users/robrandao/tc-services
Closed

TurnContext.services refactor#486
Rodrigo Brandão (rodrigobr-msft) wants to merge 10 commits into
mainfrom
users/robrandao/tc-services

Conversation

@rodrigobr-msft

Copy link
Copy Markdown
Contributor

This pull request refactors how services are stored and accessed within the TurnContext, replacing the previous dictionary-based approach with a new type-safe _ServiceSet utility. It also updates the way service dependencies like UserTokenClient and ConnectorClient are injected and retrieved throughout the codebase, promoting more robust and maintainable service management. Additionally, it cleans up legacy keys, improves error handling, and updates type usage for better clarity.

Service Management Refactor:

  • Introduced the _ServiceSet class to provide a type-safe, .NET-like service collection for storing and retrieving services in TurnContext, replacing the previous dictionary-based turn_state. (libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/state/_service_set.py)
  • Updated the TurnContext class to use _ServiceSet for the services property, ensuring all service access is type-checked and consistent. (libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/turn_context.py) [1] [2]

Dependency Injection and Usage Updates:

  • Refactored all locations where UserTokenClient and ConnectorClient are injected or accessed to use the new UserTokenClientBase and ConnectorClientBase types via the services property, replacing legacy string keys. This includes updates in OAuth flow, authorization handlers, and channel adapters. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15]

Legacy Key and Interface Cleanup:

  • Removed legacy constant keys (e.g., AGENT_IDENTITY_KEY, USER_TOKEN_CLIENT_KEY, etc.) from ChannelAdapter and related code, fully transitioning to the new service-based approach. [1] [2] [3]
  • Updated the UserTokenClientBase protocol to provide clear error messages for unimplemented properties and methods, ensuring subclasses must implement required interfaces.

Other Improvements:

  • Improved error handling for missing service dependencies, raising descriptive exceptions instead of generic or incorrect ones. [1] [2] [3] [4]
  • Modernized type annotations and cleaned up unused imports for better readability and maintainability. [1] [2]

These changes make the codebase more robust, easier to extend, and less error-prone by enforcing type safety and centralizing service management.

Copilot AI review requested due to automatic review settings July 22, 2026 01:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors per-turn service storage in TurnContext from a string-keyed dict to a typed services collection (_ServiceSet), and updates adapters/dialog/oauth/slack integrations to retrieve dependencies (e.g., ConnectorClientBase, UserTokenClientBase) through that new API.

Changes:

  • Introduces _ServiceSet and wires it into TurnContext.services, removing the legacy TurnContext.get/has/set string-key APIs.
  • Updates core adapter + OAuth/dialog/slack code paths to inject/retrieve ConnectorClientBase / UserTokenClientBase (and identity) via the new services/identity properties.
  • Cleans up legacy adapter keys/constants and modernizes some typing/exception handling.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/turn_context.py Switches _services to _ServiceSet and exposes typed services property; removes legacy get/has/set.
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/state/_service_set.py Adds the new typed service storage utility.
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/channel_service_adapter.py Injects/retrieves connector + token clients via context.services instead of legacy turn-state keys.
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/channel_adapter.py Removes legacy constant keys that supported the previous dictionary-based approach.
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/connector/user_token_client_base.py Updates the token client protocol and adds close() to the base interface.
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/connector/init.py Exports UserTokenClientBase from the connector package.
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/oauth/_handlers/_user_authorization.py Loads OAuth flow dependencies via context.services and uses context.identity.
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/_oauth/_oauth_flow.py Updates OAuth flow typing to accept UserTokenClientBase.
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/proactive/conversation.py Refactors identity sourcing for proactive conversation creation.
libraries/microsoft-agents-hosting-dialogs/microsoft_agents/hosting/dialogs/prompts/oauth_prompt.py Retrieves UserTokenClientBase from context.services and uses context.identity.
libraries/microsoft-agents-hosting-dialogs/microsoft_agents/hosting/dialogs/dialog_manager.py Replaces legacy identity lookups with turn_context.identity.
libraries/microsoft-agents-hosting-dialogs/microsoft_agents/hosting/dialogs/dialog_extensions.py Replaces legacy identity lookups with turn_context.identity and adjusts trace labeling logic.
libraries/microsoft-agents-hosting-slack/microsoft_agents/hosting/slack/slack_agent_extension.py Switches Slack API caching/access to turn_context.services using typed keys.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings July 22, 2026 16:25
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (4)

libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/_utils/_service_set.py:33

  • _ServiceSet.get() uses isinstance(val, key), but key is often a typing.Protocol (e.g. UserTokenClientBase, ConnectorClientBase) with @property members. isinstance() on such runtime-checkable data protocols raises TypeError at runtime, which will break service retrieval even when a correct implementation is registered.
        if val is not None:
            if not isinstance(val, key):
                raise TypeError(
                    f"Value for key '{lookup_key}' is not of type {key.__name__}"
                )

libraries/microsoft-agents-hosting-dialogs/microsoft_agents/hosting/dialogs/prompts/oauth_prompt.py:89

  • OAuthPrompt._get_user_token_client() uses a truthiness check (if not val) and raises a generic Exception. Since the service is optional (get() returns None) but should be required here, prefer an explicit is None check and raise a specific exception type with a concise message.
        val = context.services.get(UserTokenClientBase)
        if val is None:
            raise RuntimeError(
                "OAuthPrompt._get_user_token_client(): UserTokenClientBase not found in context.services."
            )
        return val

libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/oauth/_handlers/_user_authorization.py:84

  • context.identity is typed as optional, but _load_flow() unconditionally reads context.identity.claims["aud"]. If identity is missing (or missing the aud claim), this will raise AttributeError/KeyError instead of a descriptive ValueError.
        identity = context.identity

libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/turn_context.py:6

  • TYPE_CHECKING is imported but not used in this module, which will trigger lint warnings and adds noise.
from typing import Optional

Copilot AI review requested due to automatic review settings July 22, 2026 16:30
@rodrigobr-msft
Rodrigo Brandão (rodrigobr-msft) marked this pull request as ready for review July 22, 2026 16:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

libraries/microsoft-agents-hosting-msteams/microsoft_agents/hosting/msteams/_teams_api_client.py:31

  • _get_teams_api_client() documents/implements a ValueError for missing/invalid cached values, but context.services.get(ApiClient) will raise TypeError if a wrong-typed value was stored. This makes the error surface inconsistent and can break callers/tests expecting ValueError.
    api_client = context.services.get(ApiClient)
    if isinstance(api_client, ApiClient):
        return api_client
    raise ValueError("Unable to retrieve Teams API client.")

Copilot AI review requested due to automatic review settings July 22, 2026 16:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 7 comments.

Comments suppressed due to low confidence (2)

libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/_utils/_service_set.py:31

  • _ServiceSet uses key.name as the storage key, which can collide for distinct service types that share the same class name (from different modules). This can silently overwrite registrations and return the wrong service. Using the type object itself as the dict key avoids collisions and keeps lookups type-safe.
        lookup_key = key.__name__

        val = self._state.get(lookup_key)
        if val is not None:
            if not isinstance(val, key):

libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/_utils/_service_set.py:44

  • has()/set() should key the internal map by the service type object (not key.name) to match get() and avoid collisions between same-named types from different modules.
        return key.__name__ in self._state

Comment thread tests/hosting_core/test_service_set.py
)
)
context.turn_state[self.USER_TOKEN_CLIENT_KEY] = user_token_client
context.services.set(UserTokenClientBase, user_token_client)
)
)
context.turn_state[self._AGENT_CONNECTOR_CLIENT_KEY] = connector_client
context.services.set(ConnectorClientBase, connector_client)
Comment on lines 21 to 24
class ChannelAdapter(ABC, ChannelAdapterProtocol):
AGENT_IDENTITY_KEY = "AgentIdentity"
OAUTH_SCOPE_KEY = "Microsoft.Agents.Builder.ChannelAdapter.OAuthScope"
INVOKE_RESPONSE_KEY = "ChannelAdapter.InvokeResponse"
CONNECTOR_FACTORY_KEY = "ConnectorFactory"
USER_TOKEN_CLIENT_KEY = "UserTokenClient"
AGENT_CALLBACK_HANDLER_KEY = "AgentCallbackHandler"
CHANNEL_SERVICE_FACTORY_KEY = "ChannelServiceClientFactory"

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/_utils/_service_set.py:27

  • _ServiceSet uses key.name as the dictionary key, which is not unique across modules (two distinct types with the same class name would collide and overwrite each other). This breaks the intended “type-safe” service lookup semantics.
        lookup_key = key.__name__

libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/_utils/_service_set.py:44

  • _ServiceSet.has() keys services by key.name, which can collide across modules. It should use the same fully-qualified key derivation as get()/set() to keep lookups consistent and avoid overwrites.
        return key.__name__ in self._state

libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/_utils/_service_set.py:52

  • _ServiceSet.set() stores values under key.name, which can collide for different types that share the same class name. Use a fully-qualified key to ensure each distinct type maps to a distinct entry.
        self._state[key.__name__] = value

tests/hosting_core/test_service_set.py:78

  • This test mutates _ServiceSet._state using Service.name as the key. If _ServiceSet is updated to use fully-qualified keys (module + qualname) to avoid collisions, this test should derive the key the same way and relax the match to account for the fully-qualified key in the error message.
def test_get_raises_type_error_when_stored_value_does_not_match_key():
    services = _ServiceSet()
    services._state[Service.__name__] = OtherService()

    with pytest.raises(
        TypeError, match="Value for key 'Service' is not of type Service"
    ):
        services.get(Service)

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.

2 participants