fix(ai): close DX gaps found in atmos ai field test

agent-skills/skills/atmos-ai/SKILL.md

@@ -101,7 +101,37 @@ through MCP **and** applies Atmos-native patterns (`!terraform.state`, abstract
 inheritance, [remote-state-bridge](../atmos-migration/references/remote-state-bridge.md))
 from the relevant skill.
 
-Install the Atmos skills plugin into Claude Code:
+### Installing Skills: `atmos ai skill` (canonical, cross-client)
+
+`atmos ai skill install`/`list`/`update`/`uninstall` is the canonical, cross-client way to manage
+skills -- it works for every supported client (Claude Code, VS Code/Copilot, Gemini), not just
+Claude Code.
+
+```bash
+atmos ai skill list                          # Browse the bundled catalog + what's installed
+atmos ai skill install atmos-terraform       # Bundled skill, offline, no network/Git needed
+atmos ai skill install github.com/user/repo  # Community skill from GitHub
+atmos ai skill install                       # Install every bundled skill at once
+atmos ai skill update                        # Refresh installed bundled skills to the latest catalog version
+atmos ai skill uninstall atmos-terraform
+```
+
+Installing a bundled skill copies its content at that point in time -- upgrading the `atmos`
+binary alone doesn't refresh a skill you already installed. Run `atmos ai skill update` after
+upgrading Atmos to pick up any bundled skill content that shipped since you installed it. Skills
+installed from GitHub aren't covered by `update` yet; re-run `install <source> --force` for those.
+
+By default the skill is copied into every detected client's project-local skill directory
+(`.claude/skills/`, `.github/skills/` for VS Code/Copilot, `.gemini/skills/`) with zero extra
+flags. Use `--client`/`--all-clients` to target specific clients, `--scope user`/`--global` to
+install into each client's user-level directory instead of the project one, or `--path` to take
+full manual control of the install location (this skips auto-distribution to clients). See
+[`atmos ai skill`](https://atmos.tools/cli/commands/ai/skill) for the full flag reference.
+
+### Installing Skills: Claude Code Plugin (Claude Code only)
+
+For Claude Code specifically, the skills plugin is a lighter-weight alternative that also
+wires up marketplace updates:
 
 ```bash
 /plugin marketplace add cloudposse/atmos
@@ -110,7 +140,7 @@ Install the Atmos skills plugin into Claude Code:
 
 For Codex, Gemini, Cursor, Windsurf, GitHub Copilot, JetBrains Junie, and Amazon Q, see the
 [AI Agent Skills announcement](https://atmos.tools/changelog/ai-agent-skills) for tool-specific
-install paths.
+install paths, or use `atmos ai skill install` above, which works for all of them.
 
 ## The Three Layers
 

cmd/ai/ask.go

@@ -49,6 +49,7 @@ var askCmd = &cobra.Command{
 		noAutoContext := v.GetBool("no-auto-context")
 		noTools := v.GetBool("no-tools")
 		mcpServers := v.GetStringSlice("mcp")
+		sessionID := v.GetString("session")
 
 		// Initialize configuration. Stack graph tools load stack manifests lazily so
 		// ask can start before stacks exist or while stack imports are temporarily broken.
@@ -83,8 +84,11 @@ var askCmd = &cobra.Command{
 
 		log.Debug("Asking AI question", "question", question)
 
-		// Create AI client using factory.
-		client, err := ai.NewClient(&atmosConfig)
+		// Create AI client using factory. When --mcp was given, filter MCP.Servers
+		// for CLI providers, whose clients otherwise read atmosConfig.MCP.Servers
+		// directly and ignore --mcp entirely (see clientConfigForMCP).
+		clientConfig := clientConfigForMCP(&atmosConfig, mcpServers)
+		client, err := ai.NewClient(&clientConfig)
 		if err != nil {
 			return fmt.Errorf("failed to create AI client: %w", err)
 		}
@@ -127,13 +131,29 @@ var askCmd = &cobra.Command{
 		ctx, cancel := context.WithTimeout(context.Background(), time.Duration(timeoutSeconds)*time.Second)
 		defer cancel()
 
+		// Load or create a persisted session when --session and ai.sessions.enabled
+		// are both set. Cheap no-op (no storage opened) otherwise.
+		sess, err := prepareSession(ctx, &atmosConfig, sessionID, client.GetModel())
+		if err != nil {
+			return fmt.Errorf("failed to prepare session: %w", err)
+		}
+		defer sess.Close() // Best-effort session storage cleanup.
+
 		// Execute question with tool support.
 		ui.Writef("👽 Thinking...\n")
 		result := exec.Execute(ctx, executor.Options{
 			Prompt:       finalQuestion,
 			ToolsEnabled: !noTools && toolExecutor != nil,
+			SessionID:    sessionID,
+			History:      sess.History(),
 		})
 
+		// Persist this turn (the plain question, not the context-augmented
+		// finalQuestion) so a subsequent `--session` invocation sees it.
+		if result.Success {
+			sess.recordTurn(ctx, question, result.Response)
+		}
+
 		if !result.Success {
 			if result.Error != nil {
 				return fmt.Errorf("%w: %s", errUtils.ErrAIExecutionFailed, result.Error.Message)
@@ -159,11 +179,13 @@ func init() {
 		flags.WithBoolFlag("no-auto-context", "", false, "Disable automatic context discovery"),
 		flags.WithBoolFlag("no-tools", "", false, "Disable tool execution"),
 		flags.WithStringSliceFlag("mcp", "", nil, "MCP servers to use (comma-separated, skips auto-routing)"),
+		flags.WithStringFlag("session", "s", "", "Session ID for conversation context"),
 		flags.WithEnvVars("include", "ATMOS_AI_INCLUDE"),
 		flags.WithEnvVars("exclude", "ATMOS_AI_EXCLUDE"),
 		flags.WithEnvVars("no-auto-context", "ATMOS_AI_NO_AUTO_CONTEXT"),
 		flags.WithEnvVars("no-tools", "ATMOS_AI_NO_TOOLS"),
 		flags.WithEnvVars("mcp", "ATMOS_AI_MCP"),
+		flags.WithEnvVars("session", "ATMOS_AI_SESSION"),
 	)
 
 	// Register flags on the command.

cmd/ai/ask_test.go

@@ -2,12 +2,15 @@
 package ai
 
 import (
+	"context"
 	"encoding/json"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"os"
 	"path/filepath"
 	"strings"
+	"sync"
 	"testing"
 
 	"github.com/spf13/cobra"
@@ -568,8 +571,8 @@ func TestAskCommand_FlagCount(t *testing.T) {
 			count++
 		}
 	})
-	// Expected: include, exclude, no-auto-context, no-tools, mcp = 5 flags.
-	assert.Equal(t, 5, count, "ask command should have exactly 5 custom flags")
+	// Expected: include, exclude, no-auto-context, no-tools, mcp, session = 6 flags.
+	assert.Equal(t, 6, count, "ask command should have exactly 6 custom flags")
 }
 
 func TestAskCommand_AIEnabledButClientCreationFails(t *testing.T) {
@@ -2447,3 +2450,131 @@ ai:
 	err := askCmd.RunE(testCmd, []string{"What is in my stacks?"})
 	require.NoError(t, err, "RunE should succeed and render the Markdown response")
 }
+
+// TestAskCommand_SessionPersistence_RoundTrip is the real round-trip test for
+// the new --session flag on `ask`: it runs `ask --session` twice against the
+// same session name and confirms the second call's HTTP request to the model
+// includes the first turn's question and answer as prior context, and that
+// both turns end up durably persisted and queryable via the session manager
+// afterward. Before this change, `ask` had no --session flag at all.
+func TestAskCommand_SessionPersistence_RoundTrip(t *testing.T) {
+	var mu sync.Mutex
+	var capturedRequests []string
+	responses := []string{"First answer", "Second answer"}
+
+	mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		bodyBytes, _ := io.ReadAll(r.Body)
+
+		mu.Lock()
+		idx := len(capturedRequests)
+		capturedRequests = append(capturedRequests, string(bodyBytes))
+		mu.Unlock()
+
+		content := "no more canned responses"
+		if idx < len(responses) {
+			content = responses[idx]
+		}
+
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusOK)
+		_ = json.NewEncoder(w).Encode(map[string]interface{}{
+			"id":      "chatcmpl-test",
+			"object":  "chat.completion",
+			"created": 1699999999,
+			"model":   "llama3.3:70b",
+			"choices": []map[string]interface{}{
+				{
+					"index":         0,
+					"message":       map[string]interface{}{"role": "assistant", "content": content},
+					"finish_reason": "stop",
+				},
+			},
+			"usage": map[string]interface{}{"prompt_tokens": 10, "completion_tokens": 20, "total_tokens": 30},
+		})
+	}))
+	defer mockServer.Close()
+
+	tmpDir := t.TempDir()
+	stacksDir := filepath.Join(tmpDir, "stacks")
+	componentsDir := filepath.Join(tmpDir, "components", "terraform")
+	sessionsDir := filepath.Join(tmpDir, ".atmos", "sessions")
+	require.NoError(t, os.MkdirAll(stacksDir, 0o755))
+	require.NoError(t, os.MkdirAll(componentsDir, 0o755))
+	require.NoError(t, os.MkdirAll(sessionsDir, 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(stacksDir, "test.yaml"), []byte("vars:\n  stage: test\n"), 0o644))
+
+	basePath := filepath.ToSlash(tmpDir)
+	atmosYaml := `
+base_path: "` + basePath + `"
+stacks:
+  base_path: stacks
+  included_paths:
+    - "*.yaml"
+  name_pattern: "{stage}"
+components:
+  terraform:
+    base_path: components/terraform
+ai:
+  enabled: true
+  default_provider: ollama
+  send_context: false
+  providers:
+    ollama:
+      base_url: "` + mockServer.URL + `"
+      model: "llama3.3:70b"
+      max_tokens: 4096
+  sessions:
+    enabled: true
+    path: ".atmos/sessions"
+    max_sessions: 100
+`
+	require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "atmos.yaml"), []byte(atmosYaml), 0o644))
+
+	t.Setenv("ATMOS_CLI_CONFIG_PATH", tmpDir)
+	t.Setenv("ATMOS_BASE_PATH", tmpDir)
+	data.SetMarkdownRenderer(ui.Format)
+
+	newTestCmd := func() *cobra.Command {
+		testCmd := &cobra.Command{Use: "ask", Args: cobra.MinimumNArgs(1)}
+		testCmd.Flags().StringSlice("include", nil, "Include patterns")
+		testCmd.Flags().StringSlice("exclude", nil, "Exclude patterns")
+		testCmd.Flags().Bool("no-auto-context", false, "Disable auto context")
+		testCmd.Flags().Bool("no-tools", true, "Disable tool execution")
+		testCmd.Flags().String("session", "", "Session ID")
+		return testCmd
+	}
+
+	firstCmd := newTestCmd()
+	require.NoError(t, firstCmd.Flags().Set("session", "ask-roundtrip-session"))
+	require.NoError(t, askCmd.RunE(firstCmd, []string{"First", "question"}))
+
+	secondCmd := newTestCmd()
+	require.NoError(t, secondCmd.Flags().Set("session", "ask-roundtrip-session"))
+	require.NoError(t, askCmd.RunE(secondCmd, []string{"Second", "question"}))
+
+	require.Len(t, capturedRequests, 2)
+	assert.Contains(t, capturedRequests[1], "First question")
+	assert.Contains(t, capturedRequests[1], "First answer")
+	assert.Contains(t, capturedRequests[1], "Second question")
+	assert.NotContains(t, capturedRequests[0], "Second question")
+
+	manager, cleanup, mgrErr := initSessionManager()
+	require.NoError(t, mgrErr)
+	defer cleanup()
+
+	ctx := context.Background()
+	sess, getErr := manager.GetSessionByName(ctx, "ask-roundtrip-session")
+	require.NoError(t, getErr)
+
+	messages, msgErr := manager.GetMessages(ctx, sess.ID, 0)
+	require.NoError(t, msgErr)
+	require.Len(t, messages, 4)
+	assert.Equal(t, "user", messages[0].Role)
+	assert.Equal(t, "First question", messages[0].Content)
+	assert.Equal(t, "assistant", messages[1].Role)
+	assert.Equal(t, "First answer", messages[1].Content)
+	assert.Equal(t, "user", messages[2].Role)
+	assert.Equal(t, "Second question", messages[2].Content)
+	assert.Equal(t, "assistant", messages[3].Role)
+	assert.Equal(t, "Second answer", messages[3].Content)
+}

cmd/ai/chat.go

@@ -35,6 +35,12 @@ func getProviderFromConfig(atmosConfig *schema.AtmosConfiguration) string {
 }
 
 // getModelFromConfig returns the model for the current provider from configuration.
+// This is an independent lookup of the raw provider config and can return "" for a
+// provider that has no explicit `model` set — even when the actually-constructed
+// client resolves a non-empty default (e.g. claude-code CLI clients default their
+// model to "claude-code"). Prefer client.GetModel() on the already-constructed
+// client wherever one is available (e.g. when creating a session); this function
+// remains as a fallback for call sites without a constructed client to hand.
 func getModelFromConfig(atmosConfig *schema.AtmosConfiguration) string {
 	provider := getProviderFromConfig(atmosConfig)
 	if providerConfig, err := ai.GetProviderConfig(atmosConfig, provider); err == nil {
@@ -77,8 +83,11 @@ var chatCmd = &cobra.Command{
 
 		log.Debug("Starting AI chat session")
 
-		// Create AI client using factory.
-		client, err := ai.NewClient(&atmosConfig)
+		// Create AI client using factory. When --mcp was given, filter MCP.Servers
+		// for CLI providers, whose clients otherwise read atmosConfig.MCP.Servers
+		// directly and ignore --mcp entirely (see clientConfigForMCP).
+		clientConfig := clientConfigForMCP(&atmosConfig, mcpServers)
+		client, err := ai.NewClient(&clientConfig)
 		if err != nil {
 			return fmt.Errorf("failed to create AI client: %w", err)
 		}
@@ -110,7 +119,7 @@ var chatCmd = &cobra.Command{
 				if err != nil {
 					// Session doesn't exist, create new one.
 					log.Debugf("Session '%s' not found, creating new session", sessionName)
-					sess, err = manager.CreateSession(ctx, session.CreateSessionParams{Name: sessionName, Model: getModelFromConfig(&atmosConfig), Provider: getProviderFromConfig(&atmosConfig)})
+					sess, err = manager.CreateSession(ctx, session.CreateSessionParams{Name: sessionName, Model: client.GetModel(), Provider: getProviderFromConfig(&atmosConfig)})
 					if err != nil {
 						return fmt.Errorf("failed to create session: %w", err)
 					}
@@ -121,7 +130,7 @@ var chatCmd = &cobra.Command{
 			} else {
 				// Create anonymous session with timestamp.
 				sessionName = fmt.Sprintf("session-%s", time.Now().Format("20060102-150405"))
-				sess, err = manager.CreateSession(ctx, session.CreateSessionParams{Name: sessionName, Model: getModelFromConfig(&atmosConfig), Provider: getProviderFromConfig(&atmosConfig)})
+				sess, err = manager.CreateSession(ctx, session.CreateSessionParams{Name: sessionName, Model: client.GetModel(), Provider: getProviderFromConfig(&atmosConfig)})
 				if err != nil {
 					return fmt.Errorf("failed to create session: %w", err)
 				}