feat: MCP server integrations — connect Atmos to external MCP servers

[Andriy Knysh (aknysh)]

what

• Connect Atmos to external MCP servers (AWS, GCP, Azure, custom) and use their tools in atmos ai ask, atmos ai chat, and atmos ai exec
• Add CLI MCP management commands: atmos mcp list, tools, test, status, restart, start, export
• Add smart server routing — automatically selects only the MCP servers relevant to the user's question using a lightweight routing call to the configured AI provider
• Add --mcp flag on all AI commands for manual server selection (supports --mcp aws-iam,aws-billing and --mcp aws-iam --mcp aws-billing), env var ATMOS_AI_MCP
• Add atmos mcp export to emit .mcp.json for Claude Code / Cursor / IDE integration
• Add Atmos Auth integration — identity field on server config for automatic credential injection (references identities from the auth section)
• Add toolchain integration — resolves uvx/npx from .tool-versions before starting servers
• Add BridgedTool pattern to wrap external MCP tools as native Atmos tools.Tool interface
• Add human-readable tool names in output (aws-iam → list_roles instead of aws-iam__list_roles)
• Add tool execution display to atmos ai ask output via MarkdownFormatter
• Add configurable ai.max_tool_iterations (default 25, was hardcoded 10) to support complex multi-tool queries
• Add complete example with 8 pre-configured AWS MCP servers at examples/mcp/
• Add comprehensive documentation: MCP Configuration, MCP Commands, AI Landing Page

why

• Leverage the ecosystem — 100+ MCP servers exist for AWS, GCP, Azure, databases, monitoring, CI/CD. Instead of reimplementing cloud provider functionality, Atmos orchestrates existing MCP servers
• Parity with AI tools — Claude Code, Cursor, Windsurf all manage MCP servers. Atmos should too
• Speed — Installing an AWS MCP server takes seconds. Building equivalent functionality takes weeks
• Composability — Users can mix native Atmos tools (describe stacks, validate) with external tools (AWS billing, security, IAM) in the same AI conversation

references

• PRD: docs/prd/atmos-mcp-integrations.md
• Blog post: website/blog/2026-03-29-mcp-server-integrations.mdx
• Example: examples/mcp/ — complete working example with 8 AWS MCP servers
• AWS MCP Servers — 20+ servers for the AWS ecosystem
• MCP Protocol Specification

See It in Action

All outputs below are from real AWS accounts. Account IDs, resource identifiers,
and internal names have been redacted. Cost figures represent an example of real-world spending.

List configured servers

$ atmos mcp list
       NAME         STATUS                           DESCRIPTION
─────────────────────────────────────────────────────────────────────────────────────────
 aws-api            stopped  AWS API — direct AWS CLI access with security controls
 aws-billing        stopped  AWS Billing — billing summaries and payment history
 aws-cloudtrail     stopped  AWS CloudTrail — event history and API call auditing
 aws-docs           stopped  AWS Documentation — search and fetch AWS docs
 aws-iam            stopped  AWS IAM — role/policy analysis and access patterns
 aws-knowledge      stopped  AWS Knowledge — managed AWS knowledge base (remote)
 aws-pricing        stopped  AWS Pricing — real-time pricing and cost analysis
 aws-security       stopped  AWS Security — Well-Architected security posture assessment

Explore tools from a server

$ atmos mcp tools aws-security
           TOOL                                                         DESCRIPTION
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
 CheckSecurityServices     Verify if selected AWS security services are enabled in the specified region and account.
 GetSecurityFindings       Retrieve security findings from AWS security services.
 GetStoredSecurityContext  Retrieve security services data that was stored in context from a previous CheckSecurityServices call.
 CheckStorageEncryption    Check if AWS storage resources have encryption enabled.
 ListServicesInRegion      List all AWS services being used in a specific region.
 CheckNetworkSecurity      Check if AWS network resources are configured for secure data-in-transit.

Test server connectivity

$ atmos mcp test aws-docs
✓ Server started successfully
✓ Initialization handshake complete
✓ 4 tools available
✓ Server responds to ping

Ask AI — documentation search (smart routing selects aws-knowledge)

$ atmos ai ask "How do I configure S3 bucket lifecycle rules?"
ℹ MCP routing selected 1 of 8 servers: aws-knowledge
ℹ MCP server "aws-knowledge" started (6 tools)
ℹ Registered 6 tools from 1 MCP server(s)
ℹ AI tools initialized: 16
👽 Thinking...

   Configuring S3 Bucket Lifecycle Rules

   S3 lifecycle rules automate object management by transitioning objects between
   storage classes, archiving, or expiring them...

  ## Tool Executions (1)
  1. ✅ aws-knowledge → aws.search_documentation (2874ms)

Ask AI — billing analysis (smart routing selects aws-billing)

$ atmos ai ask "Show our billing summary for the past 2 months"
ℹ MCP routing selected 1 of 8 servers: aws-billing
ℹ MCP server "aws-billing" started (25 tools)
ℹ Registered 25 tools from 1 MCP server(s)
ℹ AI tools initialized: 35
👽 Thinking...

  ## 📊 AWS Billing Summary — February & March 2026

   Service                                  │ Feb 2026 │ Mar 2026 │ Change
  ──────────────────────────────────────────┼──────────┼───────────┼──────────
   Amazon Virtual Private Cloud             │ $309.53  │ $261.17   │ ▼ $48.36
   EC2 - Other                              │ $88.12   │ $123.39   │ ▲ $35.27
   ...

  ## Tool Executions (1)
  1. ✅ aws-billing → cost-explorer (381ms)

Ask AI — security posture across all regions (smart routing selects aws-api + aws-security)

$ atmos ai ask "Is GuardDuty enabled in all regions?"
ℹ MCP routing selected 2 of 8 servers: aws-api, aws-security
ℹ MCP server "aws-api" started (2 tools)
ℹ MCP server "aws-security" started (6 tools)
ℹ Registered 8 tools from 2 MCP server(s)
ℹ AI tools initialized: 18
👽 Thinking...

  ## ❌ GuardDuty is NOT Enabled in All Regions

   GuardDuty is only enabled in 1 out of 34 regions checked.

  ### ✅ Enabled (1 region)

   Region    │ Detector ID
  ───────────┼────────────────────────────────────
   us-east-2 │  <detector-id-redacted>

  ### ❌ Not Enabled (33 regions)

   Region         │ Region         │ Region
  ────────────────┼────────────────┼────────────────
   ap-south-1     │ ap-south-2     │ ap-southeast-1
   eu-central-1   │ eu-west-1      │ us-east-1
   us-west-2      │ ...            │

  ### 🔒 Recommendations
    1. Enable GuardDuty in all active regions
    2. Use delegated administrator via AWS Organizations
    3. Prioritize us-east-1, us-west-2, eu-west-1 immediately

  ## Tool Executions (4)
  1. ✅ aws-api → call_aws (400ms)
  2. ✅ aws-api → call_aws (14ms)
  3. ✅ aws-api → call_aws (7ms)
  4. ✅ aws-api → call_aws (9450ms)

Ask AI — IAM audit (smart routing selects aws-iam)

$ atmos ai ask "List all IAM roles with admin access"
ℹ MCP routing selected 1 of 8 servers: aws-iam
ℹ MCP server "aws-iam" started (29 tools)
ℹ Registered 29 tools from 1 MCP server(s)
ℹ AI tools initialized: 39
👽 Thinking...

  ## 🔐 IAM Roles with Admin Access

  ### 1. ✅ Direct AdministratorAccess Policy (4 attachments)

   Role Name                                        │ Description                                    │ Trust Principal
  ──────────────────────────────────────────────────┼────────────────────────────────────────────────┼───────────────────────────
    AWSReservedSSO_AdministratorAccess_...          │ Allow Full Administrator access to the account │ AWS SSO (SAML Federation)
    AWSReservedSSO_RootAccess_...                   │ Centralized root access to member accounts     │ AWS SSO (SAML Federation)
    AWSReservedSSO_TerraformApplyAccess_...         │ Full Terraform state and account access        │ AWS SSO (SAML Federation)
    AWSReservedSSO_TerraformApplyAccess-Core_...    │ Full Terraform access (core backend)           │ AWS SSO (SAML Federation)

  ### 🛡️ Security Recommendations
    1. Review SSO assignments for AdministratorAccess and RootAccess roles.
    2. Audit TerraformApplyAccess roles — ensure MFA/session policies are enforced.
    3. Monitor tfstate roles — cross-account trust across 14 accounts.
    4. Enable CloudTrail for AssumeRole calls on high-privilege roles.

  ## Tool Executions (2)
  1. ✅ aws-iam → list_roles (314ms)
  2. ✅ aws-iam → list_policies (174ms)

Check status of all servers

$ atmos mcp status
      NAME       STATUS   TOOLS                        DESCRIPTION
─────────────────────────────────────────────────────────────────────────────────────────
 aws-api         running  2      AWS API — direct AWS CLI access with security controls
 aws-billing     running  25     AWS Billing — billing summaries and payment history
 aws-cloudtrail  running  5      AWS CloudTrail — event history and API call auditing
 aws-docs        running  4      AWS Documentation — search and fetch AWS docs
 aws-iam         running  29     AWS IAM — role/policy analysis and access patterns
 aws-knowledge   running  6      AWS Knowledge — managed AWS knowledge base (remote)
 aws-pricing     running  9      AWS Pricing — real-time pricing and cost analysis
 aws-security    running  6      AWS Security — Well-Architected security posture assessment

Summary by CodeRabbit

• New Features

  • External MCP server support with new mcp commands: list, tools, test, status, restart, export
  • New --mcp / ATMOS_AI_MCP flag for ai ask/chat/exec to select servers (overrides routing)
  • Smart MCP routing to choose relevant servers per prompt
  • Human-friendly tool names in AI responses
  • Configurable AI request timeouts and max tool‑iteration limits

• Documentation

  • Extensive MCP and AI integration docs, examples, and an AWS MCP example

[coderabbitai]

Actionable comments posted: 5

♻️ Duplicate comments (1)

pkg/mcp/client/register.go (1)

14-15: ⚠️ Potential issue | 🟠 Major

Give each MCP session its own startup deadline.

The timeout is created once before the loop and then reused for every session.Start. A slow first server can burn most of the budget and make later servers fail even when their own configured timeout would have succeeded. Build a fresh timeout inside the loop, preferably from session.ParsedConfig.Timeout, and keep registrationTimeout only as the fallback.

💡 Suggested shape

 func startAndRegisterTools(mgr *Manager, registry *tools.Registry, startOpts []StartOption, suppressStderr ...bool) int {
-	ctx, cancel := context.WithTimeout(context.Background(), registrationTimeout)
-	defer cancel()
-
 	suppress := len(suppressStderr) > 0 && suppressStderr[0]
 
 	var totalTools int
 	for _, session := range mgr.List() {
+		timeout := session.ParsedConfig.Timeout
+		if timeout <= 0 {
+			timeout = registrationTimeout
+		}
+		ctx, cancel := context.WithTimeout(context.Background(), timeout)
 		if suppress {
 			session.SetSuppressStderr(true)
 		}
 		if err := session.Start(ctx, startOpts...); err != nil {
+			cancel()
 			ui.Error(fmt.Sprintf("MCP server `%s` failed to start: %v", session.Name(), err))
 			continue
 		}
+		cancel()
 
 		bridged := BridgeTools(session)

Also applies to: 68-80

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/mcp/client/register.go` around lines 14 - 15, The loop reuses a single
global registrationTimeout for every session.Start, which can let a slow first
session exhaust the budget for subsequent sessions; change the loop to construct
a fresh per-session deadline (e.g., compute a timeout duration from
session.ParsedConfig.Timeout for each iteration) and pass that to session.Start,
using the existing registrationTimeout constant only as a fallback when
ParsedConfig.Timeout is unset or invalid; update any calls to session.Start and
related context/deadline creation in register.go so each session gets its own
timeout.

🧹 Nitpick comments (4)

cmd/mcp/client/status.go (2)

83-91: Minor: leading space when description is empty.

If description is empty and there's an error, the result is " (error message)" with a leading space. Consider trimming or using a conditional separator.

 	desc := description
 	if result.Error != nil {
 		const maxErrLen = 50
 		errMsg := result.Error.Error()
 		if len(errMsg) > maxErrLen {
 			errMsg = errMsg[:maxErrLen-3] + "..."
 		}
-		desc += " (" + errMsg + ")"
+		if desc != "" {
+			desc += " "
+		}
+		desc += "(" + errMsg + ")"
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cmd/mcp/client/status.go` around lines 83 - 91, The current construction of
desc appends " (" + errMsg + ")" unconditionally, producing a leading space when
description is empty; update the logic in the status/error formatting block
handling result.Error (the variables description, desc, errMsg) to only prepend
a separator when description is non-empty—e.g., if description == "" set desc =
errMsg (or "(" + errMsg + ")") otherwise set desc = description + " (" + errMsg
+ ")"; alternatively trim spaces from the final desc before use.

---

39-42: Consider wrapping the error with context.

Per coding guidelines, errors should be wrapped with context. A simple wrap helps users understand where the failure occurred.

 		mgr, err := mcpclient.NewManager(atmosConfig.MCP.Servers)
 		if err != nil {
-			return err
+			return fmt.Errorf("creating MCP manager: %w", err)
 		}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cmd/mcp/client/status.go` around lines 39 - 42, The call to
mcpclient.NewManager(atmosConfig.MCP.Servers) returns an unwrapped error; update
the error handling in the function containing this call (where mgr is assigned)
to wrap the returned err with contextual text (e.g., "failed to create MCP
manager" or similar) using Go error wrapping (%w or errors.Wrap) before
returning so callers know the operation and inputs that failed.

cmd/mcp/client/list_test.go (1)

265-273: Use strings.Index from the standard library.

This helper reimplements strings.Index. The stdlib version is clearer and handles edge cases consistently.

Proposed fix

+import (
+	"strings"
+	"testing"
+	...
+)
+
 // indexOfInString returns the byte index of substr in s, or -1 if not found.
 func indexOfInString(s, substr string) int {
-	for i := 0; i <= len(s)-len(substr); i++ {
-		if s[i:i+len(substr)] == substr {
-			return i
-		}
-	}
-	return -1
+	return strings.Index(s, substr)
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cmd/mcp/client/list_test.go` around lines 265 - 273, The function
indexOfInString reimplements standard library behavior; replace its usage with
strings.Index from the "strings" package and remove the custom indexOfInString
implementation. Update any callers to call strings.Index(s, substr) and
add/import "strings" where needed so the build compiles. Ensure no remaining
references to indexOfInString remain in the file.

cmd/mcp/client/export_test.go (1)

10-49: Good baseline tests; please add behavior-path coverage for executeMCPExport.

Current tests are mostly command wiring/constant checks. Add table-driven cases for runtime behavior (e.g., empty mcp.servers, identity wrapping, write/chmod failure paths) to better protect the new feature contract.

As per coding guidelines: “Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages”, “Use table-driven tests for testing multiple scenarios in Go”, and “Test behavior, not implementation.”

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cmd/mcp/client/export_test.go` around lines 10 - 49, Add table-driven unit
tests that exercise executeMCPExport's behavior paths using mcpJSONConfig
inputs: include cases for empty MCPServers, a normal server entry (identity
wrapping), and simulated failure paths for writing/chmod; for each case assert
returned error (or nil) and expected side-effects (file created content or no
file). To simulate write/chmod failures, make executeMCPExport use injectable
file-op hooks (e.g., replaceable vars for writeFile/chmod used by
executeMCPExport) or create temp directories with permissions that cause
os.WriteFile/Chmod to fail in the test; reuse temp files and clean up. Ensure
tests are table-driven, reference executeMCPExport, mcpJSONConfig and
configFilePermissions, and assert both error messages and file permission
behavior when applicable.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@cmd/ai/init.go`:
- Around line 219-226: The loop building serverInfos from
atmosConfig.MCP.Servers is iterating a map (non-deterministic order); change it
to iterate over a deterministic slice by calling
sortedServerNames(atmosConfig.MCP.Servers) and append router.ServerInfo entries
in that order (keep Name and Description assignment as before). Update the code
that constructs serverInfos (the for loop that currently ranges over
atmosConfig.MCP.Servers) to use the sortedServerNames result so the routing
prompt is stable.
- Around line 97-99: The initializeAIReadOnlyTools helper registers external MCP
tools (MCP server tools) before it builds a permissive permission checker, which
allows non-read-only MCP tools to bypass confirmation; fix by removing MCP tool
registration from initializeAIReadOnlyTools (keep only in the full initializer)
or, if MCP must be available here, replace the permissive checker creation with
the standard permission configuration used elsewhere (do not use
permission.ModeAllow unconditionally); update code paths that call
initializeAIReadOnlyTools and the MCP registration logic so MCP servers are
either excluded from this read-only initializer or validated by the same
permission checker used for write-capable tools (refer to
initializeAIReadOnlyTools and the permission.ModeAllow usage to locate the
changes).
- Around line 199-208: routeWithAI returns untrusted model output (selected)
which is passed straight into filterServersByName and silently drops unknown
names; validate that each name in selected actually exists in the servers
collection before filtering (use the servers list/keys to build a
validatedSelected slice), log a warning via ui.Info or ui.Warn listing any
hallucinated/unknown names, and if validatedSelected is empty fall back to
returning the original servers (or at minimum log that we are falling back)
instead of returning an empty set; update the code paths around routeWithAI,
selected, filterServersByName and the ui.Info call to use validatedSelected for
filtering and logging.

In `@cmd/mcp/client/export.go`:
- Around line 24-33: The export command currently sets flags directly and lacks
positional-arg validation; update the exportCmd declaration to include Args:
cobra.NoArgs, and in init() replace the direct exportCmd.Flags().StringP call
with creating a standard parser via flags.NewStandardParser(), call
parser.RegisterFlags(exportCmd.Flags()) (or the specific RegisterFlags() method
used in this repo), then add exportCmd to mcpcmd.McpCmd. Finally, in
executeMCPExport (the RunE) call parser.BindFlagsToViper() at the start so flags
are bound to viper before use. Use the symbols exportCmd, init,
executeMCPExport, flags.NewStandardParser(), RegisterFlags(), and
BindFlagsToViper() to locate the changes.

In `@cmd/mcp/client/list.go`:
- Around line 84-86: The error returned by mcpListParser.BindFlagsToViper(cmd,
v) should be wrapped with context using fmt.Errorf so callers get command-level
information; modify the error path in the function (around the call to
mcpListParser.BindFlagsToViper) to return a wrapped error that includes the
command identity (e.g., cmd.Name() or a descriptive string) and the original
error via %w, and add an import for fmt if missing.

---

Duplicate comments:
In `@pkg/mcp/client/register.go`:
- Around line 14-15: The loop reuses a single global registrationTimeout for
every session.Start, which can let a slow first session exhaust the budget for
subsequent sessions; change the loop to construct a fresh per-session deadline
(e.g., compute a timeout duration from session.ParsedConfig.Timeout for each
iteration) and pass that to session.Start, using the existing
registrationTimeout constant only as a fallback when ParsedConfig.Timeout is
unset or invalid; update any calls to session.Start and related context/deadline
creation in register.go so each session gets its own timeout.

---

Nitpick comments:
In `@cmd/mcp/client/export_test.go`:
- Around line 10-49: Add table-driven unit tests that exercise
executeMCPExport's behavior paths using mcpJSONConfig inputs: include cases for
empty MCPServers, a normal server entry (identity wrapping), and simulated
failure paths for writing/chmod; for each case assert returned error (or nil)
and expected side-effects (file created content or no file). To simulate
write/chmod failures, make executeMCPExport use injectable file-op hooks (e.g.,
replaceable vars for writeFile/chmod used by executeMCPExport) or create temp
directories with permissions that cause os.WriteFile/Chmod to fail in the test;
reuse temp files and clean up. Ensure tests are table-driven, reference
executeMCPExport, mcpJSONConfig and configFilePermissions, and assert both error
messages and file permission behavior when applicable.

In `@cmd/mcp/client/list_test.go`:
- Around line 265-273: The function indexOfInString reimplements standard
library behavior; replace its usage with strings.Index from the "strings"
package and remove the custom indexOfInString implementation. Update any callers
to call strings.Index(s, substr) and add/import "strings" where needed so the
build compiles. Ensure no remaining references to indexOfInString remain in the
file.

In `@cmd/mcp/client/status.go`:
- Around line 83-91: The current construction of desc appends " (" + errMsg +
")" unconditionally, producing a leading space when description is empty; update
the logic in the status/error formatting block handling result.Error (the
variables description, desc, errMsg) to only prepend a separator when
description is non-empty—e.g., if description == "" set desc = errMsg (or "(" +
errMsg + ")") otherwise set desc = description + " (" + errMsg + ")";
alternatively trim spaces from the final desc before use.
- Around line 39-42: The call to mcpclient.NewManager(atmosConfig.MCP.Servers)
returns an unwrapped error; update the error handling in the function containing
this call (where mgr is assigned) to wrap the returned err with contextual text
(e.g., "failed to create MCP manager" or similar) using Go error wrapping (%w or
errors.Wrap) before returning so callers know the operation and inputs that
failed.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 01fd8f8e-c212-4ec6-b523-21a267145a73

📥 Commits

Reviewing files that changed from the base of the PR and between d2dac1c and 8b85379.

📒 Files selected for processing (19)

• cmd/ai/init.go
• cmd/mcp/client/export.go
• cmd/mcp/client/export_test.go
• cmd/mcp/client/list.go
• cmd/mcp/client/list_test.go
• cmd/mcp/client/markdown/atmos_mcp_export.md
• cmd/mcp/client/restart.go
• cmd/mcp/client/status.go
• cmd/mcp/client/tools.go
• cmd/mcp/mcpcmd/markdown/atmos_mcp.md
• demo/screengrabs/demo-stacks.txt
• docs/prd/atmos-mcp-integrations.md
• examples/mcp/README.md
• pkg/mcp/client/register.go
• website/blog/2026-03-29-mcp-server-integrations.mdx
• website/docs/ai/ai.mdx
• website/docs/cli/commands/mcp/export.mdx
• website/docs/cli/configuration/mcp/index.mdx
• website/src/data/roadmap.js

✅ Files skipped from review due to trivial changes (4)

• demo/screengrabs/demo-stacks.txt
• cmd/mcp/client/markdown/atmos_mcp_export.md
• cmd/mcp/mcpcmd/markdown/atmos_mcp.md
• website/docs/cli/commands/mcp/export.mdx

🚧 Files skipped from review as they are similar to previous changes (3)

• cmd/mcp/client/restart.go
• website/src/data/roadmap.js
• website/docs/ai/ai.mdx

[coderabbitai]

Actionable comments posted: 2

♻️ Duplicate comments (2)

cmd/mcp/client/export.go (1)

32-34: ⚠️ Potential issue | 🟠 Major

Use StandardParser for --output instead of direct flag wiring.

This subcommand still uses exportCmd.Flags().StringP(...); repo convention requires flags.NewStandardParser() + RegisterFlags(...) in init and BindFlagsToViper(...) in RunE.

#!/bin/bash
# Verify parser pattern mismatch against repo reference.
rg -n -C3 'Flags\(\)\.StringP|NewStandardParser|RegisterFlags|BindFlagsToViper' \
  cmd/mcp/client/export.go cmd/version/version.go

Based on learnings: In Atmos command files, command-specific flags should follow flags.NewStandardParser() with RegisterFlags/BindFlagsToViper, rather than direct flag registration.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cmd/mcp/client/export.go` around lines 32 - 34, Replace the direct flag
registration in init() for exportCmd with the repo-standard parser flow: create
a parser via flags.NewStandardParser(), call parser.RegisterFlags(...) to add
the "output" flag for exportCmd, and keep mcpcmd.McpCmd.AddCommand(exportCmd) in
init(); then in exportCmd's RunE bind parser flags to viper with
parser.BindFlagsToViper(cmd, viper) before reading the value. Locate symbols
exportCmd, init, RunE, flags.NewStandardParser, RegisterFlags, and
BindFlagsToViper to apply these changes.

cmd/mcp/client/status.go (1)

29-29: ⚠️ Potential issue | 🟠 Major

Honor global config-selection flags before loading CLI config.

Line 29 passes an empty schema.ConfigAndStacksInfo{} into cfg.InitCliConfig, so --base-path, --config, --config-path, and --profile may be ignored for atmos mcp status.

Based on learnings: cfg.InitCliConfig must receive ConfigAndStacksInfo populated via flags.ParseGlobalFlags(cmd, v) or config-selection flags are silently ignored.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cmd/mcp/client/status.go` at line 29, The call to cfg.InitCliConfig currently
passes an empty schema.ConfigAndStacksInfo{}, so global config-selection flags
(--base-path, --config, --config-path, --profile) are ignored; call
flags.ParseGlobalFlags(cmd, v) to populate a ConfigAndStacksInfo instance and
then pass that populated struct into cfg.InitCliConfig (replace
schema.ConfigAndStacksInfo{} with the result from flags.ParseGlobalFlags) in the
atmos mcp status command so cfg.InitCliConfig sees the global flags.

🧹 Nitpick comments (3)

cmd/ai/init_test.go (1)

186-217: Consider adding tests for routing-dependent functions.

Functions like routeWithAI, createRoutingClient, and selectRoutedServers aren't directly tested. They depend on ai.NewClient, which would need interface extraction for mocking. Not blocking, but worth tracking for future coverage improvements.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cmd/ai/init_test.go` around lines 186 - 217, Add unit tests for the
routing-dependent functions routeWithAI, createRoutingClient, and
selectRoutedServers by extracting ai.NewClient behind an interface so you can
mock AI client behavior; introduce a small ai.ClientFactory (or ai.Client
interface) used by createRoutingClient/routeWithAI, update those functions to
accept the interface (or a factory) instead of directly calling ai.NewClient,
and then add tests that inject a fake/mocked client to verify routing logic and
server selection paths (including error and edge cases) without calling the real
ai.NewClient.

cmd/ai/init.go (2)

148-161: Log the filtered result, not the original request.

Line 158 logs mcpServerNames (the user's request) after warnings have been shown for invalid names. This could mislead users into thinking all requested servers were started. Consider logging the actual filtered keys instead.

♻️ Suggested tweak

 	if len(filtered) > 0 {
-		ui.Info(fmt.Sprintf("MCP servers selected via --mcp flag: %s", strings.Join(mcpServerNames, ", ")))
+		ui.Info(fmt.Sprintf("MCP servers selected via --mcp flag: %s", strings.Join(sortedServerNames(filtered), ", ")))
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cmd/ai/init.go` around lines 148 - 161, In selectManualServers, the Info log
currently prints the original mcpServerNames (the user request) which can
mislead; change the log to report the actual filtered result by deriving the
list of selected server names from the filtered map (variable filtered) and
logging those (e.g., use sortedServerNames(filtered) joined with ", "). Keep the
warnings for unknown names as-is but ensure the Info message reflects the real
started/selected servers using filtered's keys.

---

260-276: Consider debug logging on toolchain resolution fallback.

Both resolution attempts silently swallow errors. A debug log would help users troubleshoot when toolchain resolution unexpectedly falls back or returns nil.

♻️ Optional debug logging

 func resolveToolchain(atmosConfig *schema.AtmosConfiguration) mcpclient.ToolchainResolver {
 	// Load tool dependencies from .tool-versions so uvx/npx are resolved from the toolchain.
 	deps, depsErr := dependencies.LoadToolVersionsDependencies(atmosConfig)
 	if depsErr == nil && len(deps) > 0 {
 		tenv, tenvErr := dependencies.NewEnvironmentFromDeps(atmosConfig, deps)
 		if tenvErr == nil && tenv != nil {
 			return tenv
 		}
+		log.Debug("Failed to create environment from .tool-versions deps", "error", tenvErr)
 	}
 	// Fall back to component-based resolution.
 	tenv, tenvErr := dependencies.ForComponent(atmosConfig, "terraform", nil, nil)
 	if tenvErr == nil && tenv != nil {
 		return tenv
 	}
+	log.Debug("Toolchain resolution failed, MCP servers will use system PATH", "error", tenvErr)
 	return nil
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cmd/ai/init.go` around lines 260 - 276, resolveToolchain currently swallows
errors from dependencies.LoadToolVersionsDependencies,
dependencies.NewEnvironmentFromDeps and dependencies.ForComponent; add debug
logging that records depsErr and tenvErr (and a message when falling back to
component resolution or when returning nil) so callers can see why toolchain
resolution fell back or failed; instrument the function resolveToolchain to log
the errors and decision points (e.g., after depsErr, after tenvErr from
NewEnvironmentFromDeps, and after tenvErr from ForComponent) using the module’s
existing logger (or the appropriate logger in scope) so the values of depsErr
and tenvErr are visible in debug output.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@cmd/mcp/client/export.go`:
- Line 61: The call to cmd.Flags().GetString("output") currently discards the
error; change it to capture the returned error (e.g., outputFile, err :=
cmd.Flags().GetString("output")) and, in the surrounding command handler (the
function containing this call), return a wrapped error like fmt.Errorf("failed
to read --output flag: %w", err) so the CLI surface propagates a clear,
diagnosable error; update any variable uses to the new names and ensure the
command uses RunE or otherwise returns the error instead of ignoring it.

In `@cmd/mcp/client/status.go`:
- Around line 45-57: The loop uses ctx directly when calling mgr.Test, causing
indefinite hangs and no progress output; change to create a per-server context
with a timeout (use context.WithTimeout) for each iteration, call cancel() after
the test, and pass that timed context to mgr.Test(session.Name(), startOpts...);
also emit progress feedback (e.g., call ui.Info or similar) before testing each
server using the session.Name() so users see per-server progress; update
references: buildStartOptions, mgr.List(), mgr.Test(...), formatStatusRow to use
the result from the timed ctx.

---

Duplicate comments:
In `@cmd/mcp/client/export.go`:
- Around line 32-34: Replace the direct flag registration in init() for
exportCmd with the repo-standard parser flow: create a parser via
flags.NewStandardParser(), call parser.RegisterFlags(...) to add the "output"
flag for exportCmd, and keep mcpcmd.McpCmd.AddCommand(exportCmd) in init(); then
in exportCmd's RunE bind parser flags to viper with parser.BindFlagsToViper(cmd,
viper) before reading the value. Locate symbols exportCmd, init, RunE,
flags.NewStandardParser, RegisterFlags, and BindFlagsToViper to apply these
changes.

In `@cmd/mcp/client/status.go`:
- Line 29: The call to cfg.InitCliConfig currently passes an empty
schema.ConfigAndStacksInfo{}, so global config-selection flags (--base-path,
--config, --config-path, --profile) are ignored; call
flags.ParseGlobalFlags(cmd, v) to populate a ConfigAndStacksInfo instance and
then pass that populated struct into cfg.InitCliConfig (replace
schema.ConfigAndStacksInfo{} with the result from flags.ParseGlobalFlags) in the
atmos mcp status command so cfg.InitCliConfig sees the global flags.

---

Nitpick comments:
In `@cmd/ai/init_test.go`:
- Around line 186-217: Add unit tests for the routing-dependent functions
routeWithAI, createRoutingClient, and selectRoutedServers by extracting
ai.NewClient behind an interface so you can mock AI client behavior; introduce a
small ai.ClientFactory (or ai.Client interface) used by
createRoutingClient/routeWithAI, update those functions to accept the interface
(or a factory) instead of directly calling ai.NewClient, and then add tests that
inject a fake/mocked client to verify routing logic and server selection paths
(including error and edge cases) without calling the real ai.NewClient.

In `@cmd/ai/init.go`:
- Around line 148-161: In selectManualServers, the Info log currently prints the
original mcpServerNames (the user request) which can mislead; change the log to
report the actual filtered result by deriving the list of selected server names
from the filtered map (variable filtered) and logging those (e.g., use
sortedServerNames(filtered) joined with ", "). Keep the warnings for unknown
names as-is but ensure the Info message reflects the real started/selected
servers using filtered's keys.
- Around line 260-276: resolveToolchain currently swallows errors from
dependencies.LoadToolVersionsDependencies, dependencies.NewEnvironmentFromDeps
and dependencies.ForComponent; add debug logging that records depsErr and
tenvErr (and a message when falling back to component resolution or when
returning nil) so callers can see why toolchain resolution fell back or failed;
instrument the function resolveToolchain to log the errors and decision points
(e.g., after depsErr, after tenvErr from NewEnvironmentFromDeps, and after
tenvErr from ForComponent) using the module’s existing logger (or the
appropriate logger in scope) so the values of depsErr and tenvErr are visible in
debug output.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 16fbc2f6-c814-4e5b-adb9-71345b88fae6

📥 Commits

Reviewing files that changed from the base of the PR and between 8b85379 and 4cad175.

📒 Files selected for processing (9)

• cmd/ai/ask.go
• cmd/ai/init.go
• cmd/ai/init_test.go
• cmd/mcp/client/export.go
• cmd/mcp/client/export_test.go
• cmd/mcp/client/helpers_test.go
• cmd/mcp/client/list_test.go
• cmd/mcp/client/status.go
• pkg/ai/tools/atmos/setup.go

💤 Files with no reviewable changes (1)

• pkg/ai/tools/atmos/setup.go

🚧 Files skipped from review as they are similar to previous changes (4)

• cmd/mcp/client/helpers_test.go
• cmd/mcp/client/export_test.go
• cmd/ai/ask.go
• cmd/mcp/client/list_test.go

[coderabbitai]

Actionable comments posted: 4

♻️ Duplicate comments (1)

cmd/ai/init.go (1)

149-160: ⚠️ Potential issue | 🟠 Major

Still make explicit --mcp selections fail fast.

This path warns and keeps going with a partial or empty server set. For an explicit --mcp request, unknown names should abort initialization so the command does not silently drop the requested MCP tools. Have selectManualServers() return an error and bubble it through registerMCPServerTools() / initializeAIToolsAndExecutor().

As per coding guidelines, "Provide clear error messages to users, include troubleshooting hints when appropriate."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cmd/ai/init.go` around lines 149 - 160, selectManualServers currently only
warns on unknown --mcp names and continues; change its signature to return
(map[string]schema.MCPServerConfig, error), detect any requested names that are
missing (compare mcpServerNames to keys in servers), and return a descriptive
error (including the unknown names and available names) instead of calling
ui.Warning when any are missing; update callers registerMCPServerTools and
initializeAIToolsAndExecutor to propagate that error and abort initialization
(return the error) so explicit --mcp selections fail fast and produce a clear
user-facing message and troubleshooting hint.

🧹 Nitpick comments (4)

cmd/mcp/client/status.go (2)

60-63: Progress feedback during iteration.

The timeout addition (lines 52-54) addresses the hang concern. However, users still see no output until all servers finish testing. For transparency, emit progress before each test.

♻️ Proposed fix

 		for _, session := range mgr.List() {
+			ui.Info(fmt.Sprintf("Testing server: %s", session.Name()))
 			result := mgr.Test(ctx, session.Name(), startOpts...)
 			rows = append(rows, formatStatusRow(session.Name(), session.Config().Description, result))
 		}

As per coding guidelines: "Provide meaningful feedback to users and include progress indicators for long-running operations in CLI commands".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cmd/mcp/client/status.go` around lines 60 - 63, Before running each test in
the loop that iterates mgr.List(), emit a progress message so the user sees
which session is being tested; specifically, add a progress print/log call right
before calling mgr.Test(ctx, session.Name(), startOpts...) that references
session.Name() (and optionally session.Config().Description) so users know which
server is currently being tested, then proceed to collect result and call
formatStatusRow as before. Ensure the progress output uses the same CLI output
mechanism as the command (not stderr) and is non-blocking so it appears
immediately during long-running iterations.

---

33-35: Wrap errors with context.

Returning raw errors loses call-site context. Wrap them to help users understand where failures occur.

♻️ Proposed fix

 		atmosConfig, err := cfg.InitCliConfig(schema.ConfigAndStacksInfo{}, false)
 		if err != nil {
-			return err
+			return fmt.Errorf("initializing config: %w", err)
 		}

 		mgr, err := mcpclient.NewManager(atmosConfig.MCP.Servers)
 		if err != nil {
-			return err
+			return fmt.Errorf("creating MCP manager: %w", err)
 		}

As per coding guidelines: "Follow Go's error handling idioms: use meaningful error messages, wrap errors with context using fmt.Errorf("context: %w", err)".

Also applies to: 43-45

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cmd/mcp/client/status.go` around lines 33 - 35, Replace bare returns of err
in the two locations that currently read "if err != nil { return err }" with
wrapped errors that add call-site context using fmt.Errorf("context: %w", err);
for example change to return fmt.Errorf("fetching status: %w", err) or a short
context that describes the failed operation in the current function (wrap both
occurrences referenced in the review). Ensure fmt is imported if not already.

pkg/mcp/client/auth_test.go (2)

37-99: Consider table-driven tests for the auth/toolchain scenario sets.

The scenario coverage is good; converting these repeated cases into table-driven tests will reduce duplication and make new cases cheaper to add.

As per coding guidelines, "Use table-driven tests for testing multiple scenarios in Go."

Also applies to: 144-201

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/mcp/client/auth_test.go` around lines 37 - 99, The four nearly-identical
tests for WithAuthManager (TestWithAuthManager_InjectsCredentials,
_NoIdentity_Passthrough, _NilProvider_Passthrough, _Error_ReturnsError) should
be consolidated into a single table-driven test: create a slice of test cases
with fields (name, provider/mocked values, ParsedConfig, input env, wantEnv,
wantCalledWith, wantErr) and iterate over them calling opt :=
WithAuthManager(tc.provider) and result, err := opt(ctx, tc.config, tc.env),
then assert expectations per case; reference mockAuthProvider, WithAuthManager,
ParsedConfig and the existing assertions to port expected behaviors and include
the nil provider and error-producing provider cases as distinct table entries.

---

102-120: Add a compile-time sentinel for schema.MCPServerConfig.Identity.

These tests depend on that field; a sentinel will fail fast on schema rename/refactor.

Suggested guard.

 import (
@@
 	"github.com/cloudposse/atmos/pkg/schema"
 )
+
+var _ = schema.MCPServerConfig{Identity: "compile-guard"}

Based on learnings, "Add compile-time sentinels for schema field references in tests: when a test uses a specific struct field ... add ... as a compile guard so a field rename immediately fails the build."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/mcp/client/auth_test.go` around lines 102 - 120, Add a compile-time
sentinel that references the field schema.MCPServerConfig.Identity so
renaming/removing the field breaks the build; put a package-level unused
variable referencing that field (e.g. just assign
schema.MCPServerConfig{}.Identity to an underscore var) near the tests in
auth_test.go (close to TestParseConfig_Identity/TestParseConfig_EmptyIdentity
and newTestServerConfig) so the tests fail to compile if the Identity field is
renamed or removed.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@cmd/ai/init.go`:
- Around line 280-292: The helper resolveAuthProvider currently returns nil on
auth manager creation failure which allows MCP startup to proceed without
required credentials; change resolveAuthProvider to return
(mcpclient.AuthEnvProvider, error) instead of mcpclient.AuthEnvProvider,
propagate and return the error from
auth.CreateAndAuthenticateManagerWithAtmosConfig (include context like "failed
to create auth manager for MCP servers"), and update callers to abort MCP
registration when an error is returned (use
serversNeedAuth(atmosConfig.MCP.Servers) to guard the call). Ensure all
references to resolveAuthProvider are updated to handle the error and stop
startup when it fails.

In `@pkg/mcp/client/auth_test.go`:
- Line 124: The comment "// WithToolchain tests" is missing a trailing period;
update that comment to "// WithToolchain tests." in the test file (look for the
comment immediately above the WithToolchain-related test cases in auth_test.go)
so it satisfies the godot linter requirement that all comments end with periods.
- Around line 56-71: The test TestWithAuthManager_NoIdentity_Passthrough
currently asserts mock.calledWith == "" which can false-positive if the mock was
invoked with an empty identity; update the mockAuthProvider used in this test to
track invocation (e.g., a bool or callCount field) and change the assertion to
require that the mock was not called (callCount == 0 or called == false) after
invoking WithAuthManager(opt) with a ParsedConfig that has Identity == ""; keep
the rest of the test (env/result assertions) unchanged and reference
mockAuthProvider and WithAuthManager when making the change.
- Around line 16-20: Tests use handwritten mockAuthProvider and mockToolchain
instead of generated mocks; replace them by adding go:generate directives to
generate mocks for the AuthEnvProvider and ToolchainResolver interfaces (named
in session.go as AuthEnvProvider and ToolchainResolver), run mockgen to produce
the mock package, import the generated mocks in pkg/mcp/client/auth_test.go and
other affected tests (where mockAuthProvider and mockToolchain are referenced),
and update test usage to use the generated mock types and EXPECT-style method
setups rather than the manual preparedEnv/err/calledWith fields.

---

Duplicate comments:
In `@cmd/ai/init.go`:
- Around line 149-160: selectManualServers currently only warns on unknown --mcp
names and continues; change its signature to return
(map[string]schema.MCPServerConfig, error), detect any requested names that are
missing (compare mcpServerNames to keys in servers), and return a descriptive
error (including the unknown names and available names) instead of calling
ui.Warning when any are missing; update callers registerMCPServerTools and
initializeAIToolsAndExecutor to propagate that error and abort initialization
(return the error) so explicit --mcp selections fail fast and produce a clear
user-facing message and troubleshooting hint.

---

Nitpick comments:
In `@cmd/mcp/client/status.go`:
- Around line 60-63: Before running each test in the loop that iterates
mgr.List(), emit a progress message so the user sees which session is being
tested; specifically, add a progress print/log call right before calling
mgr.Test(ctx, session.Name(), startOpts...) that references session.Name() (and
optionally session.Config().Description) so users know which server is currently
being tested, then proceed to collect result and call formatStatusRow as before.
Ensure the progress output uses the same CLI output mechanism as the command
(not stderr) and is non-blocking so it appears immediately during long-running
iterations.
- Around line 33-35: Replace bare returns of err in the two locations that
currently read "if err != nil { return err }" with wrapped errors that add
call-site context using fmt.Errorf("context: %w", err); for example change to
return fmt.Errorf("fetching status: %w", err) or a short context that describes
the failed operation in the current function (wrap both occurrences referenced
in the review). Ensure fmt is imported if not already.

In `@pkg/mcp/client/auth_test.go`:
- Around line 37-99: The four nearly-identical tests for WithAuthManager
(TestWithAuthManager_InjectsCredentials, _NoIdentity_Passthrough,
_NilProvider_Passthrough, _Error_ReturnsError) should be consolidated into a
single table-driven test: create a slice of test cases with fields (name,
provider/mocked values, ParsedConfig, input env, wantEnv, wantCalledWith,
wantErr) and iterate over them calling opt := WithAuthManager(tc.provider) and
result, err := opt(ctx, tc.config, tc.env), then assert expectations per case;
reference mockAuthProvider, WithAuthManager, ParsedConfig and the existing
assertions to port expected behaviors and include the nil provider and
error-producing provider cases as distinct table entries.
- Around line 102-120: Add a compile-time sentinel that references the field
schema.MCPServerConfig.Identity so renaming/removing the field breaks the build;
put a package-level unused variable referencing that field (e.g. just assign
schema.MCPServerConfig{}.Identity to an underscore var) near the tests in
auth_test.go (close to TestParseConfig_Identity/TestParseConfig_EmptyIdentity
and newTestServerConfig) so the tests fail to compile if the Identity field is
renamed or removed.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9581a364-cf4f-4961-aa4c-7fc65f4dbe86

📥 Commits

Reviewing files that changed from the base of the PR and between 4cad175 and d3d9098.

📒 Files selected for processing (4)

• cmd/ai/init.go
• cmd/mcp/client/status.go
• pkg/mcp/client/auth_test.go
• pkg/mcp/client/session_test.go

✅ Files skipped from review due to trivial changes (1)

• pkg/mcp/client/session_test.go

[github-actions]

Warning

Release Documentation Required

This PR is labeled minor or major and requires documentation updates:

• Changelog entry - Add a blog post in website/blog/YYYY-MM-DD-feature-name.mdx
• Roadmap update - Update website/src/data/roadmap.js with the new milestone

Alternatively: If this change doesn't require release documentation, remove the minor or major label.

[github-actions]

These changes were released in v1.214.0-test.1.