-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Allow to auto-start MCP servers on frontend start-up #14736
Conversation
fixed #14692 Signed-off-by: Jonas Helming <[email protected]>
7440e30
to
c1e8b30
Compare
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.
Works as expected!
I added some usability questions and minor comments.
packages/ai-mcp/src/browser/mcp-frontend-application-contribution.ts
Outdated
Show resolved
Hide resolved
packages/ai-mcp/src/browser/mcp-frontend-application-contribution.ts
Outdated
Show resolved
Hide resolved
this.prevServers = this.convertToMap(servers); | ||
this.syncServers(this.prevServers); | ||
this.autoStartServers(this.prevServers); |
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.
Some usability questions:
- This will only auto start servers when the frontend is started. Should we also auto start servers which are added to the preferences with
autostart: true
or when theirautostart
value is added or changed? - If a frontend is started with a server set to
autostart: false
(or non existing), should we then stop the server in case it is already running?
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.
1.: I decided to not start them on preference change (this is documented in the settings). Reasoning was that if you start a server manually, you get the message with the functions. If you set up a new server, you probably want to start it manually once. If you have a strong feeling to change this behavior, its fine, but I found it a bit more logical this way.
2.: I decided no, because this would break other frontends that still use this server. Also, "removing auto start", does not imply "stop it" to me. You can still manually stop them, though.
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.
Thanks!
fixed #14692
What it does
Introduce a new optional property "autostart" for MCP servers to start them automatically when a frontend is started.
How to test
Breaking changes
Attribution
Review checklist
Reminder for reviewers