Skip to content

Conversation

@sestinj
Copy link
Contributor

@sestinj sestinj commented Nov 26, 2025

Description

this fixes a number of bugs that were causing cn to stop (and devboxes too) when MCPs were used that relied on environment variables as secrets

AI Code Review

  • Team members only: AI review runs automatically when PR is opened or marked ready for review
  • Team members can also trigger a review by commenting @continue-review

Checklist

  • [] I've read the contributing guide
  • [] The relevant docs, if any, have been updated or created
  • [] The relevant tests, if any, have been updated or created

Screen recording or screenshot

[ When applicable, please include a short screen recording or screenshot - this makes it much easier for us as contributors to review and understand your changes. See this PR as a good example. ]

Tests

[ What tests were added or updated to ensure the changes work as expected? ]


Summary by cubic

Fixes secret resolution for MCP servers by falling back to local environment variables when org/package secrets aren’t accessible, preventing cn/devboxes from crashing. Also hardens template variable handling, improves error logging, and documents the fallback behavior for clarity.

  • Bug Fixes

    • Merge API secret results only when a value is present; otherwise look up .env to allow env var overrides.
    • Use decodeFQSN for secret parsing in MCPService to correctly extract secret names.
    • Add defensive guards to getTemplateVariables/fillTemplateVariables and skip invalid inputs during unroll to avoid crashes.
  • Reliability

    • Improve unhandled promise rejection logging, serialize Error objects for readable stacks, and centralize Sentry capture to avoid duplicates.

Written for commit 15fb54c. Summary will update automatically on new commits.

@sestinj sestinj requested a review from a team as a code owner November 26, 2025 19:08
@sestinj sestinj requested review from tingwai and removed request for a team November 26, 2025 19:08
@continue-development-app
Copy link

Keep this PR in a mergeable state →

Learn more

All Green is an AI agent that automatically:

✅ Addresses code review comments

✅ Fixes failing CI checks

✅ Resolves merge conflicts

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Nov 26, 2025
@github-actions
Copy link

github-actions bot commented Nov 26, 2025

✅ Review Complete

Code Review Summary

⚠️ Continue configuration error. Please verify that the assistant exists in Continue Hub.


@sestinj sestinj requested review from RomneyDa and removed request for tingwai November 26, 2025 19:11
continue bot added a commit that referenced this pull request Nov 26, 2025
Adds documentation explaining how the CLI resolves secrets for MCP servers,
including the fallback from org/package secrets to local environment variables.
This documents the fix from PR #8899 that allows env vars to override
inaccessible org secrets in devboxes and restricted environments.

Co-authored-by: nate <nate@continue.dev>

Generated with [Continue](https://continue.dev)

Co-Authored-By: Continue <noreply@continue.dev>
Adds documentation explaining how the CLI resolves secrets for MCP servers,
including the fallback from org/package secrets to local environment variables.
This documents the fix from PR #8899 that allows env vars to override
inaccessible org secrets in devboxes and restricted environments.

Co-authored-by: nate <nate@continue.dev>

Generated with [Continue](https://continue.dev)

Co-Authored-By: Continue <noreply@continue.dev>
@continue
Copy link
Contributor

continue bot commented Nov 26, 2025

Added documentation for the MCP secret resolution changes in extensions/cli/spec/mcp.md:

  • Documents the new fallback behavior where local environment variables can override org/package secrets that aren't accessible locally
  • Explains the priority order for secret resolution (API secrets first, then local env vars)
  • Lists the .env file priority order (process.env → ~/.continue/.env → workspace/.continue/.env → workspace/.env)
  • Clarifies this fixes MCP server crashes in devboxes and restricted environments where org secrets aren't accessible

The documentation maintains the same level of detail as the existing spec and focuses on the practical implications of the fix.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 6 files

Prompt for AI agents (all 1 issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="extensions/cli/src/index.ts">

<violation number="1" location="extensions/cli/src/index.ts:103">
Passing the Error into `logger.error` causes it to call `sentryService.captureException`, so the explicit capture below now fires twice per rejection, producing duplicate Sentry events.</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR


// If reason is an Error, use it directly for better stack traces
if (reason instanceof Error) {
logger.error("Unhandled Promise Rejection", reason, errorDetails);
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 26, 2025

Choose a reason for hiding this comment

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

Passing the Error into logger.error causes it to call sentryService.captureException, so the explicit capture below now fires twice per rejection, producing duplicate Sentry events.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At extensions/cli/src/index.ts, line 103:

<comment>Passing the Error into `logger.error` causes it to call `sentryService.captureException`, so the explicit capture below now fires twice per rejection, producing duplicate Sentry events.</comment>

<file context>
@@ -91,7 +91,26 @@ export function shouldShowExitMessage(): boolean {
+
+  // If reason is an Error, use it directly for better stack traces
+  if (reason instanceof Error) {
+    logger.error(&quot;Unhandled Promise Rejection&quot;, reason, errorDetails);
+  } else {
+    // Convert non-Error reasons to Error for consistent handling
</file context>

✅ Addressed in 4ec2b1b

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 7 files

Prompt for AI agents (all 1 issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="extensions/cli/src/index.ts">

<violation number="1" location="extensions/cli/src/index.ts:103">
Passing the Error object into `logger.error` causes that helper to capture the exception with Sentry while the handler still calls `sentryService.captureException`, so every unhandled rejection is reported twice. Remove one of the captures or log the details without treating the argument as an `Error` to avoid duplicate alerts.</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR


// If reason is an Error, use it directly for better stack traces
if (reason instanceof Error) {
logger.error("Unhandled Promise Rejection", reason, errorDetails);
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 26, 2025

Choose a reason for hiding this comment

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

Passing the Error object into logger.error causes that helper to capture the exception with Sentry while the handler still calls sentryService.captureException, so every unhandled rejection is reported twice. Remove one of the captures or log the details without treating the argument as an Error to avoid duplicate alerts.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At extensions/cli/src/index.ts, line 103:

<comment>Passing the Error object into `logger.error` causes that helper to capture the exception with Sentry while the handler still calls `sentryService.captureException`, so every unhandled rejection is reported twice. Remove one of the captures or log the details without treating the argument as an `Error` to avoid duplicate alerts.</comment>

<file context>
@@ -91,7 +91,26 @@ export function shouldShowExitMessage(): boolean {
+
+  // If reason is an Error, use it directly for better stack traces
+  if (reason instanceof Error) {
+    logger.error(&quot;Unhandled Promise Rejection&quot;, reason, errorDetails);
+  } else {
+    // Convert non-Error reasons to Error for consistent handling
</file context>

✅ Addressed in 4ec2b1b

@sestinj sestinj merged commit 0dd7402 into main Nov 26, 2025
30 of 35 checks passed
@github-project-automation github-project-automation bot moved this from Todo to Done in Issues and PRs Nov 26, 2025
@sestinj sestinj deleted the nate/cn-mcp-fixes branch November 26, 2025 19:46
@github-actions github-actions bot locked and limited conversation to collaborators Nov 26, 2025
@sestinj
Copy link
Contributor Author

sestinj commented Nov 26, 2025

🎉 This PR is included in version 1.36.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@sestinj
Copy link
Contributor Author

sestinj commented Nov 26, 2025

🎉 This PR is included in version 1.7.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@sestinj
Copy link
Contributor Author

sestinj commented Dec 4, 2025

🎉 This PR is included in version 1.7.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

released size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants