Skip to content

fix: missing / in url with base path when connecting Streamable HTTP MCP - #2480

Merged
hayescode merged 2 commits into
Chainlit:mainfrom
ggirou:patch-1
Oct 14, 2025
Merged

hayescode merged 2 commits into
Chainlit:mainfrom
ggirou:patch-1

Conversation

@ggirou

@ggirou ggirou commented Sep 2, 2025

Copy link
Copy Markdown
Contributor

In example, with CHAINLIT_ROOT_PATH=/mypath

@dosubot dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. frontend Pertains to the frontend. labels Sep 2, 2025
@asvishnyakov

asvishnyakov commented Sep 2, 2025 •

Copy link
Copy Markdown
Member

I'm begging everyone to stop manually build urls and start using URL for that

@ggirou

ggirou commented Sep 2, 2025

Copy link
Copy Markdown
Contributor Author

Hi @asvishnyakov ,

Thank you for your feedback. I understand the importance of using the URL instead of manually building URLs. However, as I'm still getting familiar with the codebase, I would appreciate any guidance or examples you could provide on how to implement this properly.

Your help would be invaluable as I work on making these changes.

Thanks in advance!

@asvishnyakov

Copy link
Copy Markdown
Member

@ggirou

new URL(path, base)

or, if base itself may already contain path:

new URL(path, base.endsWith("/") ? base : `${base}/`)

if you need to keep path or

new URL(path, base)

if you need to replace path

Chainlit already has buildEndpoint in API class which we may use, but to be honest I would like to refactor it a bit

@ggirou

ggirou commented Sep 3, 2025

Copy link
Copy Markdown
Contributor Author

Hi @asvishnyakov

I updated the PR to use URL

@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open for 14 days with no activity.

@github-actions github-actions Bot added the stale Issue has not had recent activity or appears to be solved. Stale issues will be automatically closed label Sep 18, 2025
@cornmail

Copy link
Copy Markdown

Hi @asvishnyakov, @asvishnyakov, @sandangel

Do you have any suggestions or feedback for merging my pull request?

Thanks!

@sandangel

Copy link
Copy Markdown
Contributor

Hi, I believe when the client miss the end / , server will redirect it to that /, is it correct? Either case should work without issue.

@github-actions github-actions Bot removed the stale Issue has not had recent activity or appears to be solved. Stale issues will be automatically closed label Sep 19, 2025
@github-actions

github-actions Bot commented Oct 4, 2025

Copy link
Copy Markdown

This PR is stale because it has been open for 14 days with no activity.

@github-actions github-actions Bot added the stale Issue has not had recent activity or appears to be solved. Stale issues will be automatically closed label Oct 4, 2025
@mukul1em

mukul1em commented Oct 5, 2025

Copy link
Copy Markdown

is there any update on this PR?

@asvishnyakov

Copy link
Copy Markdown
Member

I'll merge it after next release

@github-actions github-actions Bot removed the stale Issue has not had recent activity or appears to be solved. Stale issues will be automatically closed label Oct 6, 2025
@hayescode
hayescode added this pull request to the merge queue Oct 14, 2025
Merged via the queue into Chainlit:main with commit d869bd3 Oct 14, 2025
9 checks passed
tonca pushed a commit to tonca/chainlit that referenced this pull request Mar 24, 2026
…MCP (Chainlit#2480)

In example, with `CHAINLIT_ROOT_PATH=/mypath`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend Pertains to the frontend. size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants