Remove os.Setenv from MCP server production code

Replace environment variable communication between cmd and internal
packages with explicit MCPOverrides struct. CLI flag values are now
passed via typed struct fields through Run/RunWithFactory/RunBackfill.
Env var support preserved through appconfig's applyMCPEnv pipeline.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

2 files changed, 123 insertions, 74 deletions

diff --git a/internal/hexaimcp/run.go b/internal/hexaimcp/run.go
index b6fed8c..eb4fe83 100644
--- a/internal/hexaimcp/run.go
+++ b/internal/hexaimcp/run.go
@@ -15,6 +15,15 @@ import (
 	"codeberg.org/snonux/hexai/internal/slashcommands"
 )
 
+// MCPOverrides holds CLI flag values that override config settings.
+// These are passed explicitly from the CLI entrypoint instead of using
+// environment variables, avoiding the code smell of os.Setenv in production code.
+type MCPOverrides struct {
+	PromptsDir       string
+	SlashCommandSync bool
+	SlashCommandDir  string
+}
+
 // ServerRunner interface allows dependency injection for testing.
 type ServerRunner interface {
 	Run() error
@@ -34,16 +43,19 @@ func defaultServerFactory(r io.Reader, w io.Writer, logger *log.Logger, store pr
 	return mcp.NewServer(r, w, logger, store, syncer)
 }
 
-// Run starts the MCP server with the given configuration.
+// Run starts the MCP server with the given configuration and overrides.
 // This is the main entry point called from cmd/hexai-mcp-server/main.go.
-func Run(logPath, configPath string, stdin io.Reader, stdout io.Writer, stderr io.Writer) error {
-	return RunWithFactory(logPath, configPath, stdin, stdout, stderr, defaultServerFactory)
+func Run(logPath, configPath string, overrides MCPOverrides, stdin io.Reader, stdout io.Writer, stderr io.Writer) error {
+	return RunWithFactory(logPath, configPath, overrides, stdin, stdout, stderr, defaultServerFactory)
 }
 
 // RunWithFactory allows test injection of server factory.
+// Overrides are applied to the loaded config before use, allowing CLI flags
+// to take precedence over config file and environment variable settings.
 func RunWithFactory(
 	logPath string,
 	configPath string,
+	overrides MCPOverrides,
 	stdin io.Reader,
 	stdout io.Writer,
 	stderr io.Writer,
@@ -63,10 +75,16 @@ func RunWithFactory(
 	logger.Printf("hexai-mcp-server starting")
 	logger.Printf("WARNING: hexai-mcp-server is DEPRECATED and experimental - not actively maintained")
 
-	// Load configuration
+	// Load configuration and apply CLI overrides
 	cfg := loadConfig(logger, configPath)
+	applyOverrides(&cfg, overrides)
+
+	return runServer(cfg, logger, stdin, stdout, factory)
+}
 
-	// Determine prompts directory
+// runServer creates the prompt store, syncer, and runs the MCP server.
+func runServer(cfg appconfig.App, logger *log.Logger, stdin io.Reader, stdout io.Writer, factory ServerFactory) error {
+	// Determine prompts directory from config (overrides already applied)
 	promptsDir, err := getPromptsDir(cfg)
 	if err != nil {
 		return fmt.Errorf("cannot determine prompts directory: %w", err)
@@ -127,15 +145,27 @@ func loadConfig(logger *log.Logger, configPath string) appconfig.App {
 	return appconfig.LoadWithOptions(logger, opts)
 }
 
-// getPromptsDir determines the prompts directory from config or environment.
-// Precedence: CLI flag (via config) > env var > config file > default XDG location.
-func getPromptsDir(cfg appconfig.App) (string, error) {
-	// Check environment variable first
-	if envDir := strings.TrimSpace(os.Getenv("HEXAI_MCP_PROMPTS_DIR")); envDir != "" {
-		return expandPath(envDir)
+// applyOverrides applies CLI flag overrides to the loaded config.
+// This replaces the previous approach of using os.Setenv to pass values.
+func applyOverrides(cfg *appconfig.App, overrides MCPOverrides) {
+	if overrides.PromptsDir != "" {
+		cfg.MCPPromptsDir = overrides.PromptsDir
+	}
+	if overrides.SlashCommandSync {
+		cfg.MCPSlashCommandSync = true
 	}
+	if overrides.SlashCommandDir != "" {
+		cfg.MCPSlashCommandDir = overrides.SlashCommandDir
+	}
+}
 
-	// Check config file
+// getPromptsDir determines the prompts directory from config.
+// Precedence: CLI flag (via overrides applied to config) > env var (via
+// applyMCPEnv in config loading) > config file > default XDG location.
+// The env var HEXAI_MCP_PROMPTS_DIR is still supported through the config
+// loading pipeline in appconfig, not read directly here.
+func getPromptsDir(cfg appconfig.App) (string, error) {
+	// Check config (which already includes env var and CLI overrides)
 	if cfgDir := strings.TrimSpace(cfg.MCPPromptsDir); cfgDir != "" {
 		return expandPath(cfgDir)
 	}
@@ -182,7 +212,8 @@ func createSyncer(cfg appconfig.App, logger *log.Logger) (*slashcommands.Syncer,
 }
 
 // RunBackfill performs a one-time sync of all prompts and exits.
-func RunBackfill(logPath, configPath string) error {
+// Overrides are applied to the loaded config before use.
+func RunBackfill(logPath, configPath string, overrides MCPOverrides) error {
 	logger, err := setupLogger(logPath)
 	if err != nil {
 		return fmt.Errorf("cannot setup logger: %w", err)
@@ -195,7 +226,9 @@ func RunBackfill(logPath, configPath string) error {
 
 	logger.Printf("hexai-mcp-server backfill starting")
 
+	// Load configuration and apply CLI overrides
 	cfg := loadConfig(logger, configPath)
+	applyOverrides(&cfg, overrides)
 
 	// Force enable sync for backfill
 	if cfg.MCPSlashCommandDir == "" {
@@ -203,6 +236,11 @@ func RunBackfill(logPath, configPath string) error {
 	}
 	cfg.MCPSlashCommandSync = true
 
+	return executeBackfill(cfg, logger)
+}
+
+// executeBackfill creates the syncer, store, and performs the backfill sync.
+func executeBackfill(cfg appconfig.App, logger *log.Logger) error {
 	syncer, err := createSyncer(cfg, logger)
 	if err != nil {
 		return fmt.Errorf("cannot create syncer: %w", err)
diff --git a/internal/hexaimcp/run_test.go b/internal/hexaimcp/run_test.go
index 7883efd..dac6542 100644
--- a/internal/hexaimcp/run_test.go
+++ b/internal/hexaimcp/run_test.go
@@ -62,11 +62,11 @@ func TestFullProtocolFlow(t *testing.T) {
 
 	// Run server in background (it will read from inBuf and write to outBuf)
 	go func() {
-		// Override prompts dir via environment
-		t.Setenv("HEXAI_MCP_PROMPTS_DIR", tmpDir)
+		// Pass prompts dir via overrides instead of environment variable
+		overrides := MCPOverrides{PromptsDir: tmpDir}
 
 		// Note: This will hang waiting for more input, which is expected
-		_ = RunWithFactory("", "", inBuf, outBuf, errBuf, serverFactory)
+		_ = RunWithFactory("", "", overrides, inBuf, outBuf, errBuf, serverFactory)
 	}()
 
 	// Give server time to process
@@ -94,25 +94,16 @@ func writeJSONRPC(t *testing.T, w io.Writer, req map[string]any) {
 func TestGetPromptsDir(t *testing.T) {
 	tests := []struct {
 		name      string
-		envVar    string
 		cfgValue  string
 		wantMatch string
 	}{
 		{
-			name:      "environment variable takes precedence",
-			envVar:    "/custom/prompts",
-			cfgValue:  "/config/prompts",
-			wantMatch: "/custom/prompts",
-		},
-		{
-			name:      "config file used when no env",
-			envVar:    "",
+			name:      "config value used",
 			cfgValue:  "/config/prompts",
 			wantMatch: "/config/prompts",
 		},
 		{
 			name:      "uses default XDG location",
-			envVar:    "",
 			cfgValue:  "",
 			wantMatch: ".local/hexai/data/prompts",
 		},
@@ -120,17 +111,10 @@ func TestGetPromptsDir(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			// Setup environment
-			oldEnv := os.Getenv("HEXAI_MCP_PROMPTS_DIR")
-			defer os.Setenv("HEXAI_MCP_PROMPTS_DIR", oldEnv)
-			os.Setenv("HEXAI_MCP_PROMPTS_DIR", tt.envVar)
-
-			// Create config
 			cfg := appconfig.App{
 				MCPPromptsDir: tt.cfgValue,
 			}
 
-			// Test
 			result, err := getPromptsDir(cfg)
 			if err != nil {
 				t.Fatalf("getPromptsDir() error = %v", err)
@@ -294,12 +278,10 @@ func TestRun(t *testing.T) {
 	outBuf := &bytes.Buffer{}
 	errBuf := &bytes.Buffer{}
 
-	// Set prompts dir environment variable
-	oldEnv := os.Getenv("HEXAI_MCP_PROMPTS_DIR")
-	defer os.Setenv("HEXAI_MCP_PROMPTS_DIR", oldEnv)
-	os.Setenv("HEXAI_MCP_PROMPTS_DIR", tmpDir)
+	// Pass prompts dir via overrides instead of environment variable
+	overrides := MCPOverrides{PromptsDir: tmpDir}
 
-	err := RunWithFactory(logPath, "", inBuf, outBuf, errBuf, mockFactory)
+	err := RunWithFactory(logPath, "", overrides, inBuf, outBuf, errBuf, mockFactory)
 	if err != nil {
 		t.Fatalf("RunWithFactory() error = %v", err)
 	}
@@ -327,12 +309,10 @@ func TestRunWithFactory_ServerError(t *testing.T) {
 	outBuf := &bytes.Buffer{}
 	errBuf := &bytes.Buffer{}
 
-	// Set prompts dir environment variable
-	oldEnv := os.Getenv("HEXAI_MCP_PROMPTS_DIR")
-	defer os.Setenv("HEXAI_MCP_PROMPTS_DIR", oldEnv)
-	os.Setenv("HEXAI_MCP_PROMPTS_DIR", tmpDir)
+	// Pass prompts dir via overrides instead of environment variable
+	overrides := MCPOverrides{PromptsDir: tmpDir}
 
-	err := RunWithFactory(logPath, "", inBuf, outBuf, errBuf, mockFactory)
+	err := RunWithFactory(logPath, "", overrides, inBuf, outBuf, errBuf, mockFactory)
 	if err == nil {
 		t.Fatal("RunWithFactory() expected error, got nil")
 	}
@@ -351,7 +331,7 @@ func TestRunWithFactory_LoggerError(t *testing.T) {
 		return &mockServerRunner{}
 	}
 
-	err := RunWithFactory(badLogPath, "", &bytes.Buffer{}, &bytes.Buffer{}, &bytes.Buffer{}, mockFactory)
+	err := RunWithFactory(badLogPath, "", MCPOverrides{}, &bytes.Buffer{}, &bytes.Buffer{}, &bytes.Buffer{}, mockFactory)
 	if err == nil {
 		t.Fatal("expected error for invalid log path, got nil")
 	}
@@ -369,12 +349,11 @@ func TestRunWithFactory_StderrLogger(t *testing.T) {
 		return &mockServerRunner{}
 	}
 
-	oldEnv := os.Getenv("HEXAI_MCP_PROMPTS_DIR")
-	defer os.Setenv("HEXAI_MCP_PROMPTS_DIR", oldEnv)
-	os.Setenv("HEXAI_MCP_PROMPTS_DIR", tmpDir)
+	// Pass prompts dir via overrides instead of environment variable
+	overrides := MCPOverrides{PromptsDir: tmpDir}
 
 	// Empty logPath causes logger to write to stderr (no file to close)
-	err := RunWithFactory("", "", &bytes.Buffer{}, &bytes.Buffer{}, &bytes.Buffer{}, mockFactory)
+	err := RunWithFactory("", "", overrides, &bytes.Buffer{}, &bytes.Buffer{}, &bytes.Buffer{}, mockFactory)
 	if err != nil {
 		t.Fatalf("RunWithFactory() error = %v", err)
 	}
@@ -387,13 +366,12 @@ func TestRun_CallsDefaultFactory(t *testing.T) {
 	tmpDir := t.TempDir()
 	logPath := filepath.Join(tmpDir, "test.log")
 
-	oldEnv := os.Getenv("HEXAI_MCP_PROMPTS_DIR")
-	defer os.Setenv("HEXAI_MCP_PROMPTS_DIR", oldEnv)
-	os.Setenv("HEXAI_MCP_PROMPTS_DIR", tmpDir)
+	// Pass prompts dir via overrides instead of environment variable
+	overrides := MCPOverrides{PromptsDir: tmpDir}
 
 	// Run with empty stdin — the real server hits EOF and exits cleanly.
 	// This exercises the full Run -> RunWithFactory -> defaultServerFactory path.
-	err := Run(logPath, "", &bytes.Buffer{}, &bytes.Buffer{}, &bytes.Buffer{})
+	err := Run(logPath, "", overrides, &bytes.Buffer{}, &bytes.Buffer{}, &bytes.Buffer{})
 	// The server may return nil or an error depending on how it handles EOF;
 	// the important thing is that Run() itself does not panic.
 	_ = err
@@ -427,10 +405,6 @@ func TestSetupLogger_WhitespacePath(t *testing.T) {
 // TestGetPromptsDir_XDGDataHome verifies getPromptsDir uses XDG_DATA_HOME
 // when set (covers the branch where XDG_DATA_HOME is non-empty).
 func TestGetPromptsDir_XDGDataHome(t *testing.T) {
-	oldPrompts := os.Getenv("HEXAI_MCP_PROMPTS_DIR")
-	defer os.Setenv("HEXAI_MCP_PROMPTS_DIR", oldPrompts)
-	os.Setenv("HEXAI_MCP_PROMPTS_DIR", "")
-
 	oldXDG := os.Getenv("XDG_DATA_HOME")
 	defer os.Setenv("XDG_DATA_HOME", oldXDG)
 	os.Setenv("XDG_DATA_HOME", "/custom/xdg/data")
@@ -449,10 +423,6 @@ func TestGetPromptsDir_XDGDataHome(t *testing.T) {
 
 // TestGetPromptsDir_TildeInConfig verifies tilde expansion for config path.
 func TestGetPromptsDir_TildeInConfig(t *testing.T) {
-	oldPrompts := os.Getenv("HEXAI_MCP_PROMPTS_DIR")
-	defer os.Setenv("HEXAI_MCP_PROMPTS_DIR", oldPrompts)
-	os.Setenv("HEXAI_MCP_PROMPTS_DIR", "")
-
 	cfg := appconfig.App{
 		MCPPromptsDir: "~/my-prompts",
 	}
@@ -538,11 +508,6 @@ func TestRunBackfill_FullHappyPath(t *testing.T) {
 		t.Fatalf("cannot create prompts dir: %v", err)
 	}
 
-	// Set environment to control prompts and slash command directories
-	oldPrompts := os.Getenv("HEXAI_MCP_PROMPTS_DIR")
-	defer os.Setenv("HEXAI_MCP_PROMPTS_DIR", oldPrompts)
-	os.Setenv("HEXAI_MCP_PROMPTS_DIR", promptsDir)
-
 	// Write a config file with [mcp] section that sets the slash command dir
 	cfgContent := fmt.Sprintf("[mcp]\nslashcommand_dir = %q\nslashcommand_sync = true\n", cmdDir)
 	cfgPath := filepath.Join(tmpDir, "config.toml")
@@ -550,9 +515,12 @@ func TestRunBackfill_FullHappyPath(t *testing.T) {
 		t.Fatalf("cannot write config: %v", err)
 	}
 
+	// Pass prompts dir via overrides instead of environment variable
+	overrides := MCPOverrides{PromptsDir: promptsDir}
+
 	// RunBackfill should succeed: config sets MCPSlashCommandDir, prompts
 	// dir exists, and SyncAll on an empty store is a no-op.
-	err := RunBackfill(logPath, cfgPath)
+	err := RunBackfill(logPath, cfgPath, overrides)
 	if err != nil {
 		t.Fatalf("RunBackfill() error = %v", err)
 	}
@@ -578,7 +546,7 @@ func TestRunBackfill_CreateSyncerError(t *testing.T) {
 		t.Fatalf("cannot write config: %v", err)
 	}
 
-	err := RunBackfill(logPath, cfgPath)
+	err := RunBackfill(logPath, cfgPath, MCPOverrides{})
 	if err == nil {
 		t.Fatal("expected error for invalid slash command dir, got nil")
 	}
@@ -598,18 +566,17 @@ func TestRunBackfill_StderrLogger(t *testing.T) {
 		t.Fatalf("cannot create prompts dir: %v", err)
 	}
 
-	oldPrompts := os.Getenv("HEXAI_MCP_PROMPTS_DIR")
-	defer os.Setenv("HEXAI_MCP_PROMPTS_DIR", oldPrompts)
-	os.Setenv("HEXAI_MCP_PROMPTS_DIR", promptsDir)
-
 	cfgContent := fmt.Sprintf("[mcp]\nslashcommand_dir = %q\n", cmdDir)
 	cfgPath := filepath.Join(tmpDir, "config.toml")
 	if err := os.WriteFile(cfgPath, []byte(cfgContent), 0o644); err != nil {
 		t.Fatalf("cannot write config: %v", err)
 	}
 
+	// Pass prompts dir via overrides instead of environment variable
+	overrides := MCPOverrides{PromptsDir: promptsDir}
+
 	// Empty logPath — logger writes to stderr, defer close is a no-op
-	err := RunBackfill("", cfgPath)
+	err := RunBackfill("", cfgPath, overrides)
 	if err != nil {
 		t.Fatalf("RunBackfill() error = %v", err)
 	}
@@ -618,7 +585,7 @@ func TestRunBackfill_StderrLogger(t *testing.T) {
 // TestRunBackfill_LoggerError verifies RunBackfill returns an error when
 // the log path is invalid.
 func TestRunBackfill_LoggerError(t *testing.T) {
-	err := RunBackfill("/dev/null/impossible/test.log", "")
+	err := RunBackfill("/dev/null/impossible/test.log", "", MCPOverrides{})
 	if err == nil {
 		t.Fatal("expected error for invalid log path, got nil")
 	}
@@ -646,7 +613,7 @@ func TestRunBackfill_NoCmdDir(t *testing.T) {
 	defer os.Setenv("HEXAI_MCP_SLASHCOMMAND_DIR", oldEnv)
 	os.Setenv("HEXAI_MCP_SLASHCOMMAND_DIR", "")
 
-	err := RunBackfill(logPath, emptyCfgPath)
+	err := RunBackfill(logPath, emptyCfgPath, MCPOverrides{})
 	if err == nil {
 		t.Fatal("expected error for empty slash command dir, got nil")
 	}
@@ -654,3 +621,47 @@ func TestRunBackfill_NoCmdDir(t *testing.T) {
 		t.Errorf("error = %v, want to contain 'commands directory not configured'", err)
 	}
 }
+
+// TestApplyOverrides verifies that MCPOverrides are correctly applied to config.
+func TestApplyOverrides(t *testing.T) {
+	t.Run("applies all overrides", func(t *testing.T) {
+		cfg := appconfig.App{}
+		overrides := MCPOverrides{
+			PromptsDir:       "/custom/prompts",
+			SlashCommandSync: true,
+			SlashCommandDir:  "/custom/cmds",
+		}
+		applyOverrides(&cfg, overrides)
+
+		if cfg.MCPPromptsDir != "/custom/prompts" {
+			t.Errorf("MCPPromptsDir = %q, want /custom/prompts", cfg.MCPPromptsDir)
+		}
+		if !cfg.MCPSlashCommandSync {
+			t.Error("MCPSlashCommandSync = false, want true")
+		}
+		if cfg.MCPSlashCommandDir != "/custom/cmds" {
+			t.Errorf("MCPSlashCommandDir = %q, want /custom/cmds", cfg.MCPSlashCommandDir)
+		}
+	})
+
+	t.Run("does not overwrite with zero values", func(t *testing.T) {
+		cfg := appconfig.App{
+			MCPPromptsDir:       "/existing/prompts",
+			MCPSlashCommandSync: true,
+			MCPSlashCommandDir:  "/existing/cmds",
+		}
+		overrides := MCPOverrides{} // all zero values
+		applyOverrides(&cfg, overrides)
+
+		if cfg.MCPPromptsDir != "/existing/prompts" {
+			t.Errorf("MCPPromptsDir = %q, want /existing/prompts", cfg.MCPPromptsDir)
+		}
+		// SlashCommandSync false doesn't overwrite existing true
+		if !cfg.MCPSlashCommandSync {
+			t.Error("MCPSlashCommandSync should remain true")
+		}
+		if cfg.MCPSlashCommandDir != "/existing/cmds" {
+			t.Errorf("MCPSlashCommandDir = %q, want /existing/cmds", cfg.MCPSlashCommandDir)
+		}
+	})
+}