-
Notifications
You must be signed in to change notification settings - Fork 3.8k
fix: warnings and errors for unresolved MCP secrets #8656
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,6 +2,10 @@ import { Client } from "@modelcontextprotocol/sdk/client/index.js"; | |||||
| import { Transport } from "@modelcontextprotocol/sdk/shared/transport.js"; | ||||||
| import { fileURLToPath } from "url"; | ||||||
|
|
||||||
| import { | ||||||
| decodeSecretLocation, | ||||||
| getTemplateVariables, | ||||||
| } from "@continuedev/config-yaml"; | ||||||
| import { | ||||||
| SSEClientTransport, | ||||||
| SseError, | ||||||
|
|
@@ -163,6 +167,19 @@ class MCPConnection { | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| const vars = getTemplateVariables(JSON.stringify(this.options)); | ||||||
| const unrendered = vars.map((v) => { | ||||||
| return decodeSecretLocation(v.replace("secrets.", "")).secretName; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. decodeSecretLocation expects a colon-delimited secret location (e.g. "user:slug/secret"), but this passes plain secret keys like "OPENAI_API_KEY". When unresolved secrets are present, decodeSecretLocation throws and connectClient crashes instead of collecting the warning. Prompt for AI agents
Suggested change
|
||||||
| }); | ||||||
|
|
||||||
| if (unrendered.length > 0) { | ||||||
| this.errors.push( | ||||||
| `${this.options.name} MCP Server has unresolved secrets: ${unrendered.join(", ")}. | ||||||
| For personal use you can set the secret in the hub at https://hub.continue.dev/settings/secrets. | ||||||
| Org-level secrets can only be used for MCP by Background Agents (https://docs.continue.dev/hub/agents/overview) when \"Include in Env\" is enabled.`, | ||||||
| ); | ||||||
| } | ||||||
|
|
||||||
| this.connectionPromise = Promise.race([ | ||||||
| // If aborted by a refresh or other, cancel and don't do anything | ||||||
| new Promise((resolve) => { | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,10 @@ | |
| import { getErrorString } from "../util/error.js"; | ||
| import { logger } from "../util/logger.js"; | ||
|
|
||
| import { | ||
|
Check failure on line 18 in extensions/cli/src/services/MCPService.ts
|
||
| decodeSecretLocation, | ||
| getTemplateVariables, | ||
| } from "@continuedev/config-yaml"; | ||
| import { BaseService, ServiceWithDependencies } from "./BaseService.js"; | ||
| import { serviceContainer } from "./ServiceContainer.js"; | ||
| import { | ||
|
|
@@ -248,7 +252,23 @@ | |
| this.connections.set(serverName, connection); | ||
| this.updateState(); | ||
|
|
||
| const vars = getTemplateVariables(JSON.stringify(serverConfig)); | ||
| const unrendered = vars.map((v) => { | ||
| return decodeSecretLocation(v.replace("secrets.", "")).secretName; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. decodeSecretLocation is being invoked on every template variable, so any non-secret placeholder (e.g. ${{ inputs.* }}) makes rest undefined inside decodeSecretLocation and causes connectServer to throw before establishing the connection. Prompt for AI agents |
||
| }); | ||
|
|
||
| try { | ||
| if (unrendered.length > 0) { | ||
| const message = `${serverConfig.name} MCP Server has unresolved secrets: ${unrendered.join(", ")} | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems duplicate but the message is different enough for CLI since process.env is supported that I decided to leave them separate. |
||
| For personal use you can set the secret in the hub at https://hub.continue.dev/settings/secrets or pass it to the CLI environment. | ||
| Org-level secrets can only be used for MCP by Background Agents (https://docs.continue.dev/hub/agents/overview) when \"Include in Env\" is enabled for the secret.`; | ||
| if (this.isHeadless) { | ||
| throw new Error(message); | ||
| } else { | ||
| connection.warnings.push(message); | ||
| } | ||
| } | ||
|
|
||
| const client = await this.getConnectedClient(serverConfig); | ||
|
|
||
| connection.client = client; | ||
|
|
||
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.
removed this since it only applied to args. Moved to mcp loading