add mcp for web #6411
Merged
add mcp for web #6411
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to 7a78c56 in 2 minutes and 8 seconds. Click for details.
- Reviewed
1398lines of code in23files - Skipped
0files when reviewing. - Skipped posting
8draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. extensions-web/src/mcp-web/index.ts:127
- Draft comment:
Consider adding inline documentation to explain the token update listener logic and its error handling, as this helps future maintainers. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
2. extensions-web/src/mcp-web/index.ts:227
- Draft comment:
Consider using a more robust error detection mechanism rather than checking if error messages include 'session' or 'connection'. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 30% vs. threshold = 50% The comment points out a legitimate code quality concern - string matching error messages is fragile and could break if error message formats change. However, without knowing the MCP SDK's error handling capabilities or what other error detection mechanisms are available, the comment isn't specific enough about what should be done instead. The current approach, while not ideal, does provide basic error recovery functionality. I may be too harsh on the comment - even if it doesn't provide a specific solution, raising awareness about fragile error handling could be valuable. The code could potentially be improved if the team investigates proper error types or codes from the SDK. While the concern is valid, the comment doesn't provide actionable guidance. Without specific knowledge of better alternatives available in the MCP SDK, the comment may just create uncertainty without a clear path forward. The comment should be deleted because it raises a concern without providing specific, actionable guidance on how to improve the code. The current implementation, while not perfect, is functional.
3. extensions-web/src/mcp-web/index.ts:148
- Draft comment:
Consider mapping the 'server' value from the tool response rather than hardcoding 'Jan MCP Server', if dynamic server info is available. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% 1. The code is mapping from MCP SDK's tool format to a local format. 2. We don't have visibility into the MCP SDK's tool response schema to know if 'server' is available. 3. The hardcoded value is used consistently (also in getConnectedServers()). 4. Without knowing the MCP SDK's schema, this suggestion is speculative. 5. The current implementation seems intentional given the consistent use of the hardcoded value. I don't have evidence that the MCP SDK's tool response includes a server field. The suggestion is speculative without knowing the SDK's schema. The comment violates the rule about not making speculative suggestions. We need strong evidence that the change is correct. Delete the comment because it makes a speculative suggestion without evidence that tool.server exists in the SDK's response format.
4. extensions-web/src/shared/auth.ts:89
- Draft comment:
Verify that storing tokens in localStorage meets your security requirements; consider adding comments about trade-offs. - Reason this comment was not posted:
Confidence changes required:40%<= threshold50%None
5. web-app/src/routes/settings/mcp-servers.tsx:95
- Draft comment:
The platform check (isPlatformTauri) to switch between desktop and web versions is clear; consider adding documentation for future modifications. - Reason this comment was not posted:
Confidence changes required:30%<= threshold50%None
6. web-app/src/services/mcp/web.ts:155
- Draft comment:
Using Math.random() for cancellation tokens is acceptable for this use case, but if token uniqueness is critical, consider using a more robust UUID generator. - Reason this comment was not posted:
Confidence changes required:40%<= threshold50%None
7. web-app/src/services/mcp/web.ts:240
- Draft comment:
Ensure that the mapping of MCP tool responses to OpenAI-compliant format handles all expected content types; consider adding unit tests for non-text responses. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
8. core/src/browser/extensions/mcp.test.ts:99
- Draft comment:
There is no newline at the end of the file. Please add a trailing newline. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% Missing newlines at end of files are a common style issue that can cause problems with some tools and make diffs messier. However, this is a very minor issue that would likely be caught by linting or formatting tools. It's not a logic issue and doesn't affect functionality. The comment is technically correct, but is it important enough to warrant a PR comment? Most teams have automated tools to handle this. While true that automated tools often handle this, the comment is still pointing out a real issue that should be fixed according to common coding standards. Given the rules state not to comment on obvious or unimportant issues, and that this would likely be caught by automated tools, this comment should be deleted.
Workflow ID: wflow_kJMG3UzjdU2ysFcD
You can customize 
urmauur
approved these changes
Sep 11, 2025
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed ebf66a2 in 1 minute and 27 seconds. Click for details.
- Reviewed
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. extensions-web/vite.config.ts:17
- Draft comment:
The default API URL changed from 'https://api-dev.jan.ai/jan/v1' to 'https://api-dev.jan.ai/v1'. Confirm that removing the '/jan' segment is intentional and update comments/docs if needed. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment violates several rules: 1) It asks for confirmation of intention ("Confirm that...") 2) It's speculative about documentation updates 3) If this was an incorrect change, it would likely be caught in testing 4) We're told to assume changes are intentional by default 5) The comment doesn't indicate a clear required code change. The API URL change could potentially break integrations if not properly coordinated. Documentation updates might actually be important here. While documentation is important, our rules explicitly state not to make comments that ask for confirmation or documentation updates. We should trust that the author has handled this coordination. This comment should be deleted as it primarily asks for confirmation and documentation updates, which violates our commenting rules.
Workflow ID: wflow_cBxLeQ2rB5qJYjNg
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed fb2e214 in 1 minute and 24 seconds. Click for details.
- Reviewed
35lines of code in2files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. Makefile:42
- Draft comment:
Added 'yarn build:core' before web app commands. Verify that rebuilding core here doesn't unnecessarily duplicate work; consider caching if core build is time‐intensive. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. mise.toml:85
- Draft comment:
Changed web task dependencies from 'install' to 'build-core'. Confirm that this ordering supplies all necessary assets and doesn't affect incremental builds. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_6Py7cmO46Y3XKPUf
You can customize
There was a problem hiding this comment.
Skipped PR review on 60e39fa because no changed files had a supported extension. If you think this was in error, please contact us and we'll fix it right away.
qnixsynapse
approved these changes
Sep 11, 2025
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 651ac30 in 1 minute and 43 seconds. Click for details.
- Reviewed
258lines of code in3files - Skipped
0files when reviewing. - Skipped posting
5draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. extensions-web/src/mcp-web/index.ts:22
- Draft comment:
Removed the tokenUpdateListener property in favor of using an OAuth provider. Ensure that no other parts of the system rely on the previous token update events. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to ensure that no other parts of the system rely on the previous token update events. This falls under the rule of not asking the author to ensure behavior is intended or to double-check things. Therefore, this comment should be removed.
2. extensions-web/src/mcp-web/index.ts:80
- Draft comment:
The transport is now constructed with an authProvider. Verify that the underlying SDK populates transport.sessionId reliably after connection. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to verify the behavior of the SDK, which is against the rules. It doesn't provide a specific suggestion or point out a clear issue with the code.
3. extensions-web/src/mcp-web/index.ts:198
- Draft comment:
The retry/reconnection logic upon tool call failures was removed. Confirm this simplification is acceptable and that token refresh by the OAuth provider covers expected session errors. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. extensions-web/src/mcp-web/oauth-provider.ts:30
- Draft comment:
The tokens() method fetches the access token via JanAuthService every time. If performance is a concern, consider whether caching within the provider is needed, though the auth service already manages token caching. - Reason this comment was not posted:
Comment looked like it was already resolved.
5. extensions-web/src/shared/auth.ts:55
- Draft comment:
The custom 'jan-auth-token-updated' event dispatch has been removed. Confirm that removing these notifications does not impact other components that might rely on token update events. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_BOaurRiX1mLdecKu
You can customize
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe Your Changes
Fixes Issues
Self Checklist
Important
Add MCP extension for web, enabling tool management and server communication using MCP TypeScript SDK.
MCPExtensiontoextension.tsandindex.tsfor managing tools and server communication.MCPExtensionWebinmcp-web/index.tsusing MCP TypeScript SDK for web platform.MCPWebModuletotypes.tsand updatesWEB_EXTENSIONSinindex.tsfor dynamic loading.PlatformFeaturesinconst.tsto enable MCP servers.WebMCPServiceinmcp/web.tsfor web platform, handling tool calls and server management.MCPTool,MCPToolCallResult, andMCPServerInfotomcpEntity.ts.MCPInterfaceinmcpInterface.tsfor MCP operations.types/index.tsto export MCP types.mcp.test.tsfor testingMCPExtensionfunctionalities.package.jsonto include@modelcontextprotocol/sdkdependency.JanMCPOAuthProviderinoauth-provider.tsfor OAuth integration.Makefileandmise.tomlto include MCP build steps.This description was created by
for 651ac30. You can customize this summary. It will automatically update as commits are pushed.