-
Notifications
You must be signed in to change notification settings - Fork 180
fix(mcp): pass workspace parameter through client factory #722
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -622,10 +622,10 @@ async def get_project_client( | |
|
|
||
| # Step 1b: Factory injection (in-process cloud server) | ||
| # Trigger: set_client_factory() was called (e.g., by cloud MCP server) | ||
| # Why: the transport layer already resolved workspace and tenant context; | ||
| # attempting cloud workspace resolution here would call the production | ||
| # control-plane API with no valid credentials and fail with 401 | ||
| # Outcome: use the factory client directly, skip workspace resolution | ||
| # Why: the factory's transport layer handles auth and tenant resolution; | ||
| # we pass workspace through so the transport can route to the correct | ||
| # workspace when the tool specifies one different from the connection default | ||
| # Outcome: factory client with optional workspace override via inner request headers | ||
| if is_factory_mode(): | ||
| route_mode = "factory" | ||
| with telemetry.scope( | ||
|
|
@@ -635,7 +635,7 @@ async def get_project_client( | |
| workspace_id=workspace, | ||
| ): | ||
| logger.debug("Using injected client factory for project routing") | ||
| async with get_client() as client: | ||
| async with get_client(workspace=workspace) as client: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This new factory-mode call path now forwards Useful? React with 👍 / 👎. |
||
| active_project = await get_active_project(client, resolved_project, context) | ||
| yield client, active_project | ||
| return | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_client()now unconditionally calls the injected factory withworkspace=..., which breaks existing factories that implement the previous zero-argument contract and are still valid according to prior versions ofset_client_factory. Those integrations will now fail at runtime with an unexpected-keywordTypeErrorbefore any request handling. Please keep a compatibility path for factories that do not accept the new keyword.Useful? React with 👍 / 👎.