Python: fix(foundry_hosting): preserve auth credentials across FoundryToolbox reconnections#7202
Open
PratikWayase wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes the FoundryToolbox lifecycle so OAuth2-authenticated sessions can recover after an internal close() (e.g., during OAuth consent flows) by preserving credentials and recreating an authenticated httpx.AsyncClient on demand.
Changes:
- Cache
credential,token_scope, andtimeouton theFoundryToolboxinstance for reuse after teardown. - Override
get_mcp_client()to lazily recreate an authenticatedhttpx.AsyncClientwhen the prior client has been cleared. - Add regression tests validating credential preservation and reconnection behavior across repeated
close()calls.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| python/packages/foundry_hosting/agent_framework_foundry_hosting/_toolbox.py | Preserve auth-related state and recreate an authenticated HTTP client on reconnection. |
| python/packages/foundry_hosting/tests/test_toolbox.py | Add regression tests covering close/reconnect cycles and idempotent close behavior. |
| load_tools=load_tools, | ||
| ) | ||
|
|
||
| def get_mcp_client(self) -> AbstractAsyncContextManager[Any]: |
Comment on lines
+188
to
+190
| """Get an authenticated MCP HTTP client. | ||
|
|
||
| Recreates the underlying HTTP client if it was previously closed.""" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Description
Motivation & Context
When using the
ghmcpoauthauthentication method with a Foundry Toolbox, accepting an OAuth consent request triggers aclose()on the underlying MCP tool. Currently, this permanently nullifies the authenticated_httpx_client. As a result, any follow-up requests in the same session fail with aserver_errorbecause the toolbox falls back to an unauthenticated transport. Users currently have to start a completely new conversation to recover from this state. This PR fixes that lifecycle bug so sessions remain valid after OAuth consent flows.Related Issue
Fixes #7166
Description & Review Guide
What are the major changes?
FoundryToolbox.__init__to cache thecredential,token_scope, andtimeoutas instance attributes.get_mcp_client()to lazily recreate an authenticatedhttpx.AsyncClientif the previous one was closed.close()docstring to clarify that credentials are preserved for reconnection.test_toolbox.pyto verify credential preservation, idempotent closes, and proper context manager recreation.What is the impact of these changes?
Hosted agents using OAuth2-authenticated toolboxes will no longer crash mid-session after a user completes a consent flow. The fix is entirely internal to the
FoundryToolboxlifecycle and does not alter the public API or affect non-OAuth authentication methods.What do you want reviewers to focus on?
Please review the lazy recreation logic in
get_mcp_client(). I opted to rebuild thehttpx.AsyncClientonly when_httpx_client is Nonerather than trying to resurrect a closed client. I'd appreciate feedback on whether this approach aligns with the framework's expected lifecycle patterns, and whether the new regression tests adequately cover edge cases around repeated close/reconnect cycles.Contribution Checklist