Add initial support for MCP #14598
Add initial support for MCP #14598
Conversation
JonasHelming
force-pushed
the
GH-14523-2
branch
from
251519c to
d07a5bc
Compare
fixed #14523 Signed-off-by: Jonas Helming <jhelming@eclipsesource.com>
JonasHelming
force-pushed
the
GH-14523-2
branch
from
d07a5bc to
4e5f65d
Compare
|
We should get rid of the any types, there is a schema: https://github.com/modelcontextprotocol/specification/blob/bb5fdd282a4d0793822a569f573ebc36804d38f8/schema/schema.json#L1969 |
Co-authored-by: Stefan Dirix <sdirix@eclipsesource.com>
Co-authored-by: Stefan Dirix <sdirix@eclipsesource.com>
Co-authored-by: Stefan Dirix <sdirix@eclipsesource.com>
Co-authored-by: Stefan Dirix <sdirix@eclipsesource.com>
Co-authored-by: Stefan Dirix <sdirix@eclipsesource.com>
Co-authored-by: Stefan Dirix <sdirix@eclipsesource.com>
Signed-off-by: Jonas Helming <jhelming@eclipsesource.com>
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
There was a problem hiding this comment.
LGTM. @JonasHelming please double check once before we merge to make sure I did not introduce any regressions
|
Two new issues:
|
1. properties for valid functions are rejected 2. Started servers are not filtered Signed-off-by: Jonas Helming <jhelming@eclipsesource.com>
|
@sdirix I fixed the two remaining bugs I found, fine with merging? |
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| typeof (value as any).type === 'string' && |
There was a problem hiding this comment.
This can be simplified
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | |
| typeof (value as any).type === 'string' && | |
| typeof value.type === 'string' && |
Since a few versions Typescript is smart enough to allow access to fields when they were checked via the in parameter before. If your IDE complains here then you are somehow not consuming the Typescript version of the repo.
| for (const [name, description] of updatedServers) { | ||
| const oldDescription = oldServers.get(name); | ||
| // We know that that the descriptions are actual JSONObjects as we construct them ourselves | ||
| if (!oldDescription || !PreferenceProvider.deepEqual(oldDescription as unknown as JSONObject, description as unknown as JSONObject)) { |
There was a problem hiding this comment.
During testing I noticed a case in which the deepEqual fails, i.e. when deleting the whole env object in the preference. Not sure why this happens, seems to be an error in the 3rd party library we call. Therefore let's surround this with try/catch here and assume a difference.
I will open a follow up ticket so we can resolve this at a later point in a cleaner fashion.
fixed #14523
What it does
Adds initial support for Model Context Servers (see here)
How to test
Please check the readme. Be aware that the file MCP server has a bug about cases sensitivity, so if it does not work, tell the LLM the correct folder case sensitive.
Follow-ups
Currently, the interaction with MCP servers is manual, we should think about a better way to support users, e.g. to auto-start servers. For now, the integration allows users to "play around"
Breaking changes
Attribution
Review checklist
Reminder for reviewers