TurnContext.services refactor - #486
Conversation
There was a problem hiding this comment.
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
_ServiceSetand wires it intoTurnContext.services, removing the legacyTurnContext.get/has/setstring-key APIs. - Updates core adapter + OAuth/dialog/slack code paths to inject/retrieve
ConnectorClientBase/UserTokenClientBase(and identity) via the newservices/identityproperties. - 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.
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>
There was a problem hiding this comment.
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()usesisinstance(val, key), butkeyis often atyping.Protocol(e.g.UserTokenClientBase,ConnectorClientBase) with@propertymembers.isinstance()on such runtime-checkable data protocols raisesTypeErrorat 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 genericException. Since the service is optional (get()returnsNone) but should be required here, prefer an explicitis Nonecheck 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.identityis typed as optional, but_load_flow()unconditionally readscontext.identity.claims["aud"]. If identity is missing (or missing theaudclaim), this will raiseAttributeError/KeyErrorinstead of a descriptiveValueError.
identity = context.identity
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/turn_context.py:6
TYPE_CHECKINGis imported but not used in this module, which will trigger lint warnings and adds noise.
from typing import Optional
…osoft/Agents-for-python into users/robrandao/tc-services
There was a problem hiding this comment.
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.")
There was a problem hiding this comment.
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
| ) | ||
| ) | ||
| 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) |
| 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" | ||
|
|
There was a problem hiding this comment.
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)
This pull request refactors how services are stored and accessed within the
TurnContext, replacing the previous dictionary-based approach with a new type-safe_ServiceSetutility. It also updates the way service dependencies likeUserTokenClientandConnectorClientare 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:
_ServiceSetclass to provide a type-safe, .NET-like service collection for storing and retrieving services inTurnContext, replacing the previous dictionary-basedturn_state. (libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/state/_service_set.py)TurnContextclass to use_ServiceSetfor theservicesproperty, 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:
UserTokenClientandConnectorClientare injected or accessed to use the newUserTokenClientBaseandConnectorClientBasetypes via theservicesproperty, 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:
AGENT_IDENTITY_KEY,USER_TOKEN_CLIENT_KEY, etc.) fromChannelAdapterand related code, fully transitioning to the new service-based approach. [1] [2] [3]UserTokenClientBaseprotocol to provide clear error messages for unimplemented properties and methods, ensuring subclasses must implement required interfaces.Other Improvements:
These changes make the codebase more robust, easier to extend, and less error-prone by enforcing type safety and centralizing service management.