Skip to content

Conversation

@axelsrz
Copy link
Member

@axelsrz axelsrz commented Nov 4, 2025

This pull request refactors the aiohttp hosting layer to better leverage shared abstractions from the core library, resulting in a cleaner, more maintainable codebase. The main changes involve replacing framework-specific implementations with reusable protocols and base classes, centralizing request processing logic, and simplifying serialization and deserialization of models.

Refactoring to use shared core abstractions:

  • agent_http_adapter.py: Replaces the custom AgentHttpAdapter protocol with an alias for the shared AgentHttpAdapterProtocol from core, removing unnecessary boilerplate and abstract methods.
  • cloud_adapter.py: Refactors CloudAdapter to inherit from CloudAdapterBase and implements required hooks for request handling, error responses, and claims identity extraction, removing redundant logic and error handling.

Centralizing and simplifying request handling:

  • _start_agent_process.py: Replaces local validation and processing logic in start_agent_process with a direct call to the shared start_agent_process from core, streamlining agent startup. [1] [2]

Improving route table and serialization logic:

  • channel_service_route_table.py: Refactors route handlers to use shared ChannelServiceOperations for business logic, and introduces _read_payload and _json_response helpers for consistent payload handling and serialization. Removes manual model validation and custom serialization code.

Expanding core exports:

  • __init__.py (core): Adds new shared abstractions and helpers to the public API, including protocols, operations, serialization utilities, and the agent process starter. [1] [2]

Copilot AI review requested due to automatic review settings November 4, 2025 05:40
Copy link
Contributor

Copilot AI left a comment

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 HTTP hosting code by extracting common CloudAdapter and ChannelService functionality into a shared base implementation in the core library. The changes eliminate code duplication between FastAPI and aiohttp hosting adapters by introducing framework-agnostic base classes.

Key changes:

  • Introduced CloudAdapterBase in core with abstract methods for framework-specific behavior
  • Added ChannelServiceOperations class to centralize channel API operations
  • Consolidated serialization/deserialization logic with parse_agents_model and serialize_agents_model utilities

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/http.py New file containing shared HTTP utilities, base classes, and operations
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/init.py Exports new shared HTTP components
libraries/microsoft-agents-hosting-fastapi/microsoft_agents/hosting/fastapi/cloud_adapter.py Refactored to extend CloudAdapterBase and implement framework-specific methods
libraries/microsoft-agents-hosting-fastapi/microsoft_agents/hosting/fastapi/channel_service_route_table.py Simplified to use shared ChannelServiceOperations and serialization utilities
libraries/microsoft-agents-hosting-fastapi/microsoft_agents/hosting/fastapi/agent_http_adapter.py Simplified to alias the shared AgentHttpAdapterProtocol
libraries/microsoft-agents-hosting-fastapi/microsoft_agents/hosting/fastapi/_start_agent_process.py Delegates to shared core implementation
libraries/microsoft-agents-hosting-aiohttp/microsoft_agents/hosting/aiohttp/cloud_adapter.py Refactored to extend CloudAdapterBase and implement framework-specific methods
libraries/microsoft-agents-hosting-aiohttp/microsoft_agents/hosting/aiohttp/channel_service_route_table.py Simplified to use shared ChannelServiceOperations and serialization utilities
libraries/microsoft-agents-hosting-aiohttp/microsoft_agents/hosting/aiohttp/agent_http_adapter.py Simplified to alias the shared AgentHttpAdapterProtocol
libraries/microsoft-agents-hosting-aiohttp/microsoft_agents/hosting/aiohttp/_start_agent_process.py Delegates to shared core implementation

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

raise HTTPException(status_code=415, detail="Unsupported Media Type")
def _get_claims_identity(self, request: Request) -> ClaimsIdentity | None:
return getattr(
request.state, "claims_identity", self._default_claims_identity()
Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

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

The _get_claims_identity method returns a default ClaimsIdentity when 'claims_identity' is not found on request.state, but the base class process method expects None to be returned when no claims identity exists (lines 293-295 in http.py check if not claims_identity). This causes the default identity to be created twice - once here and once in the base class - which is inconsistent with the aiohttp implementation and the base class contract. The method should return None instead of self._default_claims_identity() as the default, or return getattr(request.state, 'claims_identity', None).

Suggested change
request.state, "claims_identity", self._default_claims_identity()
request.state, "claims_identity", None

Copilot uses AI. Check for mistakes.
else:
raise HTTPUnsupportedMediaType()
def _get_claims_identity(self, request: Request) -> ClaimsIdentity | None:
return request.get("claims_identity", self._default_claims_identity())
Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

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

The _get_claims_identity method returns a default ClaimsIdentity when 'claims_identity' is not found in the request, but the base class process method expects None to be returned when no claims identity exists (lines 293-295 in http.py check if not claims_identity). This causes the default identity to be created twice - once here and once in the base class. The method should return None as the default instead of self._default_claims_identity(), or return request.get('claims_identity', None).

Suggested change
return request.get("claims_identity", self._default_claims_identity())
return request.get("claims_identity", None)

Copilot uses AI. Check for mistakes.
# Licensed under the MIT License.
from traceback import format_exc
from typing import Optional
from typing import Any, Optional
Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

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

Import of 'Optional' is not used.

Suggested change
from typing import Any, Optional
from typing import Any

Copilot uses AI. Check for mistakes.
# Licensed under the MIT License.
from traceback import format_exc
from typing import Optional
from typing import Any, Optional
Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

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

Import of 'Optional' is not used.

Suggested change
from typing import Any, Optional
from typing import Any

Copilot uses AI. Check for mistakes.
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