Skip to content

Conversation

@ashu8912
Copy link
Member

@ashu8912 ashu8912 commented Sep 8, 2025

Adds support for running MCP (Model Context Protocol) servers from the desktop app by integrating @langchain/mcp-adapters library and creating a client architecture to manage MCP server connections.

  • Integrates MCP server management capabilities into the Electron desktop application
  • Provides IPC APIs for tool discovery, execution, and configuration management
  • Implements proper cleanup and initialization patterns for MCP client lifecycle

Screenshots

How to test

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 8, 2025
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 8, 2025
@ashu8912 ashu8912 marked this pull request as draft September 8, 2025 17:31
@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Sep 8, 2025
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 23, 2025
@illume illume added this to the v0.37.0 milestone Sep 26, 2025
Copy link
Contributor

@joaquimrocha joaquimrocha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The app laywer always has to double check for potentially destructive actions or portentially disruptive ones, like setting up MCP servers.

@ashu8912 ashu8912 marked this pull request as ready for review October 8, 2025 09:03
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 8, 2025
@illume illume requested a review from Copilot October 8, 2025 10:50
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds support for running MCP (Model Context Protocol) servers from the desktop app by integrating @langchain/mcp-adapters library and creating a client architecture to manage MCP server connections.

  • Integrates MCP server management capabilities into the Electron desktop application
  • Provides IPC APIs for tool discovery, execution, and configuration management
  • Implements proper cleanup and initialization patterns for MCP client lifecycle

Reviewed Changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

File Description
app/package.json Adds @langchain/mcp-adapters dependency and includes mcp-client.js in build files
app/electron/preload.ts Exposes MCP client APIs to renderer process via IPC bridge
app/electron/mcp-client.ts Implements complete MCP client with server management, tool execution, and configuration handling
app/electron/main.ts Integrates MCP client into main Electron process with proper lifecycle management

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@illume illume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool.

Could you please add a PR description?

Threat modeling with mitigations should be done. Some things to consider: IPC auth, env var leakage, logging of sensitive info, limits on tool execution, looking at if config can be used to execute, parsing/validation of inputs/outputs, sandboxing, UX view of what tools are doing and if tools are on.

Is this worth doing this backend now instead so we don't have to finish and maintain two versions of this code?

@ashu8912 ashu8912 force-pushed the mcp-electron branch 2 times, most recently from f84a53b to fdbec0f Compare October 18, 2025 08:56
@illume illume requested review from Copilot and removed request for skoeva, vyncent-t and yolossn October 18, 2025 23:30
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

app/electron/mcp-client.ts:1

  • Debug console.log statement should be removed or replaced with proper logging. This appears to be leftover debug code that clutters the production output.
/*

app/electron/mcp-client.ts:1

  • Multiple debug console.log statements throughout initializeToolsConfig (lines 206, 215, 223, 232, 244, 247) should be removed or replaced with a proper logging framework for production code.
/*

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

vyncent-t and others added 24 commits November 5, 2025 16:15
Signed-off-by: Faakhir30 <zahidfaakhir@gmail.com>
Signed-off-by: Faakhir30 <zahidfaakhir@gmail.com>
Signed-off-by: Faakhir30 <zahidfaakhir@gmail.com>
KubeObject classes that have this object as true will make the
ScaleButton show up for them.
Because there is no permission to do that now. So we run it without
installing it.

Also make sure the config and cache folders use somewhere writable.

The ConfigMap was rendering with extra newlines. At the top, and
after plugin.yml, which was causing an error when validating the
configuration inside the plugin manager code.
Which is an oauth2proxy endpoint for getting user info.
By default it is turned off.

To the charts a config.oidc.meUserInfoURL is added.
Make it /oauth2/userinfo if using oauth2proxy default config.

Co-authored-by: evangelos <52797224+skoeva@users.noreply.github.com>
This change hides the user info menu item when the fields are null.
Signed-off-by: Joaquim Rocha <joaquim.rocha@microsoft.com>
Signed-off-by: joaquimrocha <joaquimrocha@users.noreply.github.com>
Signed-off-by: joaquimrocha <joaquimrocha@users.noreply.github.com>
These changes separate the scopes in the OIDC docs by commas rather than
spaces.
The manifest is bumped automatically from Winget's side.
Homebrew has the ability to bump Casks from their side.
Before there was a mistake with passing through the meUserInfoURL.
To make it easier for people to see what things are
being configured.
@k8s-ci-robot k8s-ci-robot added the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Nov 5, 2025
@k8s-ci-robot
Copy link
Contributor

Keywords which can automatically close issues and at(@) or hashtag(#) mentions are not allowed in commit messages.

The list of commits with invalid commit messages:

  • 7db4e55 build(deps): bump playwright and @playwright/test in /e2e-tests
  • 7405705 build(deps): bump playwright and @playwright/test in /app/e2e-tests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@illume
Copy link
Contributor

illume commented Nov 7, 2025

Going to continue this here:
#4135

@illume illume closed this Nov 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.