Skip to content

Feat: Add agent worker And Claude Support #66

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

Closed
wants to merge 34 commits into from

Conversation

baryhuang
Copy link
Contributor

@baryhuang baryhuang commented Mar 31, 2025

This PR enhances the MCP-Bridge with a robust agent worker system that supports both Anthropic and OpenAI models. The implementation allows for autonomous task execution with tool-calling capabilities.

Changes:

  • Add agent_worker module with support for multiple LLM providers
  • Implement specialized handlers for Anthropic and OpenAI APIs
  • Create comprehensive logging system for agent sessions
  • Add support for thinking blocks in Anthropic models
  • Enhance tool result handling with improved image extraction
  • Implement dynamic client discovery and tool registration

The agent worker provides a standalone system for executing tasks with LLMs, automatically managing the conversation flow and tool interactions.

@SecretiveShell
Copy link
Owner

looks like an interesting idea. I did some similar stuff in bridge-agent but this is a lot more advanced.

I think the agent runner stuff should be in a subfolder if it is going to be added, as that one file could become extremely large.

@SecretiveShell SecretiveShell requested a review from Copilot March 31, 2025 08:59
Copy link
Contributor

@Copilot 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 introduces an agent worker that leverages an internal class for autonomous task handling as well as integrating an Anthropic API call, with configurations loaded from a file to avoid command line conflicts.

  • Added a dependency for Anthropic in pyproject.toml
  • Introduced a new tool mapper converting MCP tools to Anthropic format
  • Added client shutdown logic and Anthropic chat completion functionality

Reviewed Changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pyproject.toml Added the Anthropic dependency
mcp_bridge/tool_mappers/mcp2anthropicConverters.py Introduced conversion logic from MCP Tool to Anthropic format
mcp_bridge/tool_mappers/init.py Updated all to include the new Anthropic mapper
mcp_bridge/mcp_clients/McpClientManager.py Added shutdown method with task cancellation and an unused _client_tasks variable
mcp_bridge/mcp_clients/AbstractClient.py Updated to assign and return a task for the session maintainer
mcp_bridge/anthropic_clients/utils.py Added utility functions to retrieve and process MCP tools in Anthropic format
mcp_bridge/anthropic_clients/genericClient.py Created a singleton Anthropic client using configuration data
mcp_bridge/anthropic_clients/chatCompletion.py Implemented Anthropic chat completions with integrated tool processing
mcp_bridge/anthropic_clients/init.py Exposed the Anthropic client and API functions
Files not reviewed (1)
  • agent_worker_task.json: Language not supported

@baryhuang
Copy link
Contributor Author

Thanks @SecretiveShell ! the Bridge-Agent is way ahead of the time!
The motivation of trying to embedded in this MCP-Bridge repo is to try to use the mcp client management pieces. I can also imaging we can extract the mcp client part as py package, then enhance Bridge-Agent to have the capability.

For near term, looks like you are generally okay with the direction, so I'm going to fix the comments, move files into a subfolder, cleanup. Then see where can we go.

Appreciate that! Thank you.

@SecretiveShell
Copy link
Owner

we can extract the mcp client part as py package

you might be interested in easymcp. This is a refactored version of the bridge mcp client manager exposed as a python package. The plan is to refactor bridge on top of this and strip out a lot of the mcp host logic. This would allow for much easier unit/integration testing as well as open the doors for:

  • ASGI style MCP servers in process
  • SDK level support for openapi tools

For near term, looks like you are generally okay with the direction, so I'm going to fix the comments, move files into a subfolder, cleanup. Then see where can we go.

Whilst I have not stated this publicly, my end goal as of right now is to ultimately create an agent framework, and this PR seems to align with that at a high level.

@baryhuang baryhuang changed the title Add agent worker Add agent worker And Claude Support Apr 3, 2025
@baryhuang
Copy link
Contributor Author

@SecretiveShell At this point, It's good to go within mcp-bridge existing structure. We can see if want to rewrite in a cleaner way together.

But what I have now is good for my purpose. Can you review and see if we can merge this?

Copy link
Contributor

@Copilot 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.

Copilot reviewed 17 out of 18 changed files in this pull request and generated 2 comments.

Files not reviewed (1)
  • agent_worker_task.json.example: Language not supported

@baryhuang baryhuang requested a review from Copilot April 5, 2025 06:11
Copy link
Contributor

@Copilot 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.

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

Files not reviewed (1)
  • agent_worker_task.json.example: Language not supported

@SecretiveShell
Copy link
Owner

I like the direction this is going. But I have a few suggestions:

  1. rather then making it exclusively a CLI application, can we integrate with the existing interfaces? we could expose a POST api endpoint to run an agent task, and then stream events back to the client via SSE. Most people are running bridge as a long lived API server.
  2. I am not a big fan of how the imports are handled, but I would need to look deeper into the code to see why you have done it this way.
  3. mypy is complaining about a lot of missing type hints. Whist not essential on the openai/anthropic clients (the autogenerated pydantic models are questionable at best), things like logging utils should be properly type hinted.
  4. why do we need custom logging logic instead of using loguru?

@baryhuang
Copy link
Contributor Author

baryhuang commented Apr 6, 2025

I like the direction this is going. But I have a few suggestions:

  1. rather then making it exclusively a CLI application, can we integrate with the existing interfaces? we could expose a POST api endpoint to run an agent task, and then stream events back to the client via SSE. Most people are running bridge as a long lived API server.
  2. I am not a big fan of how the imports are handled, but I would need to look deeper into the code to see why you have done it this way.
  3. mypy is complaining about a lot of missing type hints. Whist not essential on the openai/anthropic clients (the autogenerated pydantic models are questionable at best), things like logging utils should be properly type hinted.
  4. why do we need custom logging logic instead of using loguru?

Thanks for the review!

  1. The agent workload are quite different, it generally take a few minutes or more for a request to finish. A typical service API should timeout typically within a few seconds ideally. The work here is to facilitate we run it some background job system such as AWS Batch.

  2. & 3.
    I could blame cursor :D about the imports styling, but I have my fault not inspect the import styling. I can fix that.

4). if we are talking about customer_logging, then it's to allow internall loggings to be different than externally logging. The customer logging is mostly be the foundation of streaming the "read-only" results via AWS job system.

@baryhuang baryhuang requested a review from Copilot April 6, 2025 04:45
Copy link
Contributor

@Copilot 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.

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

Files not reviewed (1)
  • agent_worker_task.json.example: Language not supported

Copy link
Contributor

@Copilot 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.

Copilot reviewed 19 out of 20 changed files in this pull request and generated no comments.

Files not reviewed (1)
  • agent_worker_task.json.example: Language not supported
Comments suppressed due to low confidence (3)

mcp_bridge/agent_worker/agent_worker.py:38

  • [nitpick] Consider defining and using a specific TypedDict for thinking blocks to improve code clarity and type safety instead of using a generic Dict.
self.thinking_blocks: List[Dict[str, object]] = []

mcp_bridge/openai_clients/utils.py:31

  • [nitpick] If possible, reorganize the import of ClientManager to the top of the file to improve readability, unless circular dependency constraints necessitate in-function imports.
from mcp_bridge.mcp_clients.McpClientManager import ClientManager

mcp_bridge/agent_worker/run.py:114

  • [nitpick] The 1-second timeout for awaiting the cancellation of remaining tasks might be too short under heavy load; consider increasing the timeout to ensure all tasks are properly cancelled.
await asyncio.wait(remaining_tasks, timeout=1.0)

@tsubasakong
Copy link

Looking to understand how long running jobs can be run with AWS Batch as mentioned.

@baryhuang
Copy link
Contributor Author

Looking to understand how long running jobs can be run with AWS Batch as mentioned.

@tsubasakong Thanks! There are multiple way I'm going to try:

  1. Ideally, package the mcp-bridge into docker, then run in AWS Batch via jobs system
  2. If 1) doesn't work, run the mcp-bridge in one or more ec2, using AWS batch to run a remote job on the EC2.

@SecretiveShell SecretiveShell requested a review from Copilot April 10, 2025 11:40
Copy link
Contributor

@Copilot 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.

Copilot reviewed 19 out of 20 changed files in this pull request and generated 2 comments.

Files not reviewed (1)
  • agent_worker_task.json.example: Language not supported
Comments suppressed due to low confidence (1)

mcp_bridge/anthropic_clients/utils.py:82

  • Replace the recursive call with a loop in add_usage to avoid potential stack overflow issues if the rate limiting condition persists.
await self.add_usage(tokens)

Comment on lines +98 to +100
# Exit the program
force_exit(0)

Copy link
Preview

Copilot AI Apr 10, 2025

Choose a reason for hiding this comment

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

Using force_exit here immediately terminates the process, which may bypass important cleanup steps; consider implementing a more graceful shutdown mechanism.

Suggested change
# Exit the program
force_exit(0)
# Stop the event loop
loop = asyncio.get_event_loop()
loop.stop()
loop.close()

Copilot uses AI. Check for mistakes.

tool_args = tool_call.function.arguments

# Attempt to parse JSON arguments
try:
Copy link
Preview

Copilot AI Apr 10, 2025

Choose a reason for hiding this comment

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

Consider logging the exception when JSON parsing of tool arguments fails to help diagnose issues with malformed input.

Copilot uses AI. Check for mistakes.

@SecretiveShell
Copy link
Owner

is this pr in a state where it can be merged?

@baryhuang
Copy link
Contributor Author

is this pr in a state where it can be merged?

Yes from functional POV. I have been running with it out-of-box for a few days.

@baryhuang baryhuang closed this Apr 24, 2025
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.

3 participants