Skip to content

Conversation

@LiangYang666
Copy link
Contributor

@LiangYang666 LiangYang666 commented Jun 28, 2025


name: Pull Request
about: Create a pull request

Description

This PR adds support for the streamable_http mcp and resolves #625. Additionally, a usage demo is included, which allows direct testing of the new code. Documentation has not been updated yet. If this PR is approved, I can proceed to update the documentation accordingly.

Checklist

Please check the following items before code is ready to be reviewed.

  • Code has passed all tests
  • Docstrings have been added/updated in Google Style
  • Documentation has been updated
  • Code is ready for review

@LiangYang666
Copy link
Contributor Author

cc @DavdGao

@DavdGao
Copy link
Member

DavdGao commented Jul 1, 2025

@LiangYang666 Thank you for your contribution!

The PR looks promising, but I think we need to enhance the validation logic to make it more robust. Specifically, I suggest we add stricter URL pattern matching by checking for "/sse" or "/mcp" suffixes in the URL field

For your reference, there's an ongoing discussion in the official MCP library regarding the criteria for identifying streamable HTTP or SSE configurations: modelcontextprotocol/python-sdk#983

@DavdGao
Copy link
Member

DavdGao commented Jul 1, 2025

Please run the following command in agentscope dir to format the code in this pull request:

pip install pre-commit

pre-commit install

pre-commit run --all-files

@DavdGao DavdGao requested review from DavdGao and rayrayraykk July 1, 2025 02:36
@LiangYang666
Copy link
Contributor Author

@DavdGao
Thank you for your feedback!

I think it's not ideal to infer the type by validating the URL, as this can create a black-box logic and reduce transparency. In fact, the URL suffix can be customized on mcp servers, so relying on specific patterns may not be reliable.

Similar to best practices in other projects such as the OpenAI Agent SDK, Cline, or Cherry Studio, I suggest we give the user the explicit choice to specify whether to use SSE or streamable HTTP, rather than trying to determine it automatically based on the URL.

This approach is simpler, more flexible, and avoids hidden logic.

@LiangYang666
Copy link
Contributor Author

Please run the following command in agentscope dir to format the code in this pull request:

pip install pre-commit

pre-commit install

pre-commit run --all-files

Thank you for the instructions.
I've re-ran pre-commit and pushed the revised code. This includes formatting of applications/multisource_rag_app/src/utils/logging.py, which was previously not passing the pre-commit checks even though I did not modify it. Now, all files have passed the pre-commit checks.

@rayrayraykk
Copy link
Member

Streamable http feature requires mcp version >=1.8.0, please add to dependencies.

@LiangYang666
Copy link
Contributor Author

Streamable http feature requires mcp version >=1.8.0, please add to dependencies.

done

@rayrayraykk
Copy link
Member

Streamable http feature requires mcp version >=1.8.0, please add to dependencies.

done

Thanks for your contribution, please merge the latest main branch to fix errors in UT.

@LiangYang666
Copy link
Contributor Author

Streamable http feature requires mcp version >=1.8.0, please add to dependencies.

done

Thanks for your contribution, please merge the latest main branch to fix errors in UT.

done

Copy link
Member

@DavdGao DavdGao left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for your contribution!

@DavdGao DavdGao merged commit 1244d6c into agentscope-ai:main Jul 3, 2025
13 checks passed
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.

[Feature]: support mcp streamable http client

3 participants