Skip to content

Fix bug on failed connections #212

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

Merged
merged 2 commits into from
Jun 18, 2025

Conversation

ruifigueira
Copy link
Contributor

9219bf6 fixes a bug when a user tries to connect to a failing MCP server:

  • it tries to connect but fails as expected
  • however, if we try to fix the MCP server URL, it will try to connect again for each update event, instead of only connecting when Connect button is pressed. This also causes the focus to be lost after each keystroke as seen in this video:
Recording2025-06-18.172133.mp4

I also added some playwright tests on 25dbfd9 to test that scenario, but I understand that this requires some infra changes to run those tests, so if you think it's not worth it, it's a matter of dropping that commit.

It tried to connect on each update of the server textbox.
This commit ensures it only tries to connect when Connect button is pressed.
Copy link

changeset-bot bot commented Jun 18, 2025

⚠️ No Changeset found

Latest commit: 25dbfd9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

pkg-pr-new bot commented Jun 18, 2025

Open in StackBlitz

npx https://pkg.pr.new/cloudflare/ai/ai-gateway-provider@212
npx https://pkg.pr.new/cloudflare/ai/workers-ai-provider@212

commit: 25dbfd9

@threepointone
Copy link
Collaborator

Including the tests is fine for now! As long as it doesn't run in CI yet. We can revisit that conversation. Thanks so much for the PR, it lgtm, landing!

@threepointone threepointone merged commit 60a8785 into cloudflare:main Jun 18, 2025
3 checks passed
@ruifigueira
Copy link
Contributor Author

It seems that my refactoring on status badge messed css tailwind colors, I'll fix it.

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.

2 participants