From c71511abaa4f133d7768c680375dc41c1f69c807 Mon Sep 17 00:00:00 2001 From: guyskk Date: Thu, 26 Mar 2026 10:56:19 +0800 Subject: [PATCH] fix: preserve user-defined env in settings.json Previously, `delete(settingsWithHook, "env")` removed all user-defined env from settings.json on every ccc run (Issue #74). Now: - Only write user's non-conflicting env keys to settings.json (filtered by managed keys and ANTHROPIC_*/CLAUDE_* prefix) - Pass only base + provider env to subprocess (not user env) - Add FilterUserEnvForSettings and MergeEnvMaps helper functions - Remove unused CleanEnvInSettings function --- docs/settings-merge-strategy.md | 132 ++++++++---- internal/config/config.go | 73 +++---- internal/config/config_test.go | 311 +++++++++++++++++------------ internal/provider/provider.go | 33 +-- internal/provider/provider_test.go | 128 +++++++++++- 5 files changed, 455 insertions(+), 222 deletions(-) diff --git a/docs/settings-merge-strategy.md b/docs/settings-merge-strategy.md index cec9dab..5f34db4 100644 --- a/docs/settings-merge-strategy.md +++ b/docs/settings-merge-strategy.md @@ -71,18 +71,24 @@ ccc 不应该： ## 字段处理策略 -### 1. env 字段 - 特殊处理 +### 1. env 字段 - 分离处理 -**处理方式**：清空特定键，避免配置冲突。 +**处理方式**：区分"用户 env"和"ccc env"，分别写入 settings.json 和子进程。 -需要清空的键： -1. 特定前缀：`ANTHROPIC_*`、`CLAUDE_*` -2. 与 provider env 相同的 key +**写入 settings.json 的 env**： +- 只保留用户在 settings.json 中定义的 env key +- 排除与 base/provider env 冲突的 key +- 排除 `ANTHROPIC_*`/`CLAUDE_*` 前缀的 key +- 如果过滤后为空，不写 env 字段 + +**传递给子进程的 env**： +- 只包含 base + provider 的 env +- 不包含用户 settings.json 的 env（Claude Code 自己从 settings.json 读取） **原因**： - provider 的环境变量通过命令行传递给 claude 子进程 -- 如果 settings.json 中保留这些键，会产生不确定性（不确定哪边生效） -- 清空后确保 provider env 的行为可预测 +- 用户自定义的非冲突 env 需要保留在 settings.json 中供 Claude Code 使用 +- 子进程只需 base + provider env，避免重复 **示例**： @@ -92,10 +98,16 @@ ccc 不应该： "env": { "ANTHROPIC_MODEL": "claude-3.7-sonnet", "MY_CUSTOM_VAR": "value", - "ANTHROPIC_BASE_URL": "old-url" + "DISABLE_TELEMETRY": "1" } } +// base env +{ + "API_TIMEOUT": "30000", + "DISABLE_TELEMETRY": "1" +} + // provider env { "ANTHROPIC_BASE_URL": "https://open.bigmodel.cn/api/anthropic", @@ -103,11 +115,22 @@ ccc 不应该： "ANTHROPIC_MODEL": "glm-4.7" } -// 处理后 +// 写入 settings.json 的 env { "env": { - "MY_CUSTOM_VAR": "value" // 保留（非 ANTHROPIC_* 且非 provider key） + "MY_CUSTOM_VAR": "value" // 保留（非冲突、非 ANTHROPIC_*/CLAUDE_*） } + // DISABLE_TELEMETRY 被过滤（与 base env 冲突） + // ANTHROPIC_MODEL 被过滤（ANTHROPIC_* 前缀） +} + +// 传递给子进程的 env（base + provider） +{ + "API_TIMEOUT": "30000", + "DISABLE_TELEMETRY": "1", + "ANTHROPIC_BASE_URL": "https://open.bigmodel.cn/api/anthropic", + "ANTHROPIC_AUTH_TOKEN": "token123", + "ANTHROPIC_MODEL": "glm-4.7" } ``` @@ -237,33 +260,44 @@ func LoadSettings() (map[string]interface{}, error) --- -### 2. CleanEnvInSettings() +### 2. FilterUserEnvForSettings() -**描述**：清空 settings.env 中的特定环境变量键。 +**描述**：过滤用户自定义 env，只保留安全的 key。 **签名**： ```go -// CleanEnvInSettings removes specific environment variable keys from settings.env. -// It removes: -// 1. Keys with specific prefixes (ANTHROPIC_*, CLAUDE_*) -// 2. Keys that match provider env keys -// Returns a new map without modifying the input. -func CleanEnvInSettings(settings map[string]interface{}, providerEnvKeys []string) map[string]interface{} +// FilterUserEnvForSettings filters user-defined env to only keep safe keys. +// Removes keys in managedEnvKeys or with ANTHROPIC_*/CLAUDE_* prefix. +// Returns nil if no keys remain. +func FilterUserEnvForSettings(userEnv map[string]interface{}, managedEnvKeys map[string]bool) map[string]interface{} ``` **逻辑**： -1. 深拷贝 settings（不修改输入） -2. 获取 `env` map（不存在则跳过） -3. 遍历每个 key -4. 删除满足以下任一条件的 key： - - 以 `ANTHROPIC_` 开头 - - 以 `CLAUDE_` 开头 - - 存在于 `providerEnvKeys` 列表中 -5. 返回新的 map +1. 遍历 userEnv 的每个 key +2. 跳过在 managedEnvKeys 中的 key（与 base/provider 冲突） +3. 跳过 `ANTHROPIC_*`/`CLAUDE_*` 前缀的 key +4. 如果过滤后为空，返回 nil --- -### 3. MergeWithPriority() +### 3. MergeEnvMaps() + +**描述**：合并多个 env map，后者覆盖前者。 + +**签名**： +```go +// MergeEnvMaps merges multiple env maps. Later maps override earlier ones. +func MergeEnvMaps(maps ...map[string]interface{}) map[string]interface{} +``` + +**逻辑**： +1. 遍历所有 map，依次合并 +2. nil map 被跳过 +3. 如果结果为空，返回 nil + +--- + +### 4. MergeWithPriority() **描述**：按优先级深度合并多个配置源。 @@ -289,7 +323,7 @@ func MergeWithPriority(baseSettings, providerSettings, userSettings map[string]i --- -### 4. EnsureStopHook() +### 5. EnsureStopHook() **描述**：确保 Supervisor Stop hook 存在于 settings 中。 @@ -323,22 +357,30 @@ func EnsureStopHook(settings map[string]interface{}, hookCommand string) map[str ├─→ baseSettings = cfg.Settings ├─→ providerSettings = cfg.Providers[providerName] │ - ├─→ 提取 provider env keys + ├─→ 提取各来源 env（合并前） + │ ├─→ userEnvMap = GetEnv(userSettings) + │ ├─→ baseEnvMap = GetEnv(cfg.Settings) + │ └─→ providerEnvMap = GetEnv(providerSettings) + │ + ├─→ 构建 managedEnvKeys = base env keys + provider env keys │ ├─→ MergeWithPriority(baseSettings, providerSettings, userSettings) │ │ │ └─→ merged = DeepMerge(DeepCopy(baseSettings), providerSettings) │ merged = DeepMerge(merged, userSettings) ← userSettings 优先 │ - ├─→ CleanEnvInSettings(merged, providerEnvKeys) - │ └─→ 清空 ANTHROPIC_*, CLAUDE_*, provider env keys - │ ├─→ EnsureStopHook(merged, hookCommand) │ └─→ 确保 Supervisor Stop hook 存在 │ - ├─→ 确保 hooks 可执行 - │ ├─→ merged["disableAllHooks"] = false - │ └─→ merged["allowManagedHooksOnly"] = false + ├─→ delete(merged, "env") + │ └─→ 移除合并后的 env + │ + ├─→ FilterUserEnvForSettings(userEnvMap, managedEnvKeys) + │ └─→ 过滤用户 env，保留安全 key + │ └─→ 如果有结果，写入 merged["env"] + │ + ├─→ MergeEnvMaps(baseEnvMap, providerEnvMap) + │ └─→ 子进程 env = base + provider（不含用户 env） │ └─→ 保存 merged 到 settings.json ``` @@ -403,7 +445,7 @@ func EnsureStopHook(settings map[string]interface{}, hookCommand string) map[str --- -### 场景 3：env 字段清空 +### 场景 3：env 字段分离处理 ```json // settings.json 初始内容 @@ -423,13 +465,25 @@ func EnsureStopHook(settings map[string]interface{}, hookCommand string) map[str } ``` -**处理后**： +**写入 settings.json**（只保留安全用户 env）： ```json { "env": { - "MY_CUSTOM_VAR": "value" // 保留（非 ANTHROPIC_* 且非 CLAUDE_* 且非 provider key） + "MY_CUSTOM_VAR": "value" // 保留（非冲突、非 ANTHROPIC_*/CLAUDE_*） } + // ANTHROPIC_MODEL 被过滤（ANTHROPIC_* 前缀） + // CLAUDE_BASH_MAINTAIN_PROJECT_WORKING_DIR 被过滤（CLAUDE_* 前缀） +} +``` + +**传递给子进程**（base + provider env）： + +```json +{ + "ANTHROPIC_BASE_URL": "https://open.bigmodel.cn/api/anthropic", + "ANTHROPIC_AUTH_TOKEN": "token123", + "ANTHROPIC_MODEL": "glm-4.7" } ``` @@ -481,7 +535,7 @@ func EnsureStopHook(settings map[string]interface{}, hookCommand string) map[str | 文件 | 修改内容 | |------|----------| -| `internal/config/config.go` | 新增 LoadSettings、CleanEnvInSettings、MergeWithPriority、EnsureStopHook | +| `internal/config/config.go` | 新增 LoadSettings、FilterUserEnvForSettings、MergeEnvMaps、MergeWithPriority、EnsureStopHook | | `internal/provider/provider.go` | 重写 SwitchWithHook() 函数逻辑 | | `internal/config/config_test.go` | 为新函数添加测试 | diff --git a/internal/config/config.go b/internal/config/config.go index 000284b..87e8d80 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -250,57 +250,46 @@ func LoadSettings() (map[string]interface{}, error) { return settings, nil } -// CleanEnvInSettings removes specific environment variable keys from settings.env. -// It removes: -// 1. Keys with specific prefixes (ANTHROPIC_*, CLAUDE_*) -// 2. Keys that match provider env keys -// -// Returns a new map without modifying the input. -func CleanEnvInSettings(settings map[string]interface{}, providerEnvKeys []string) map[string]interface{} { - // Deep copy to avoid modifying input - result := deepCopy(settings) - - // Get env map if it exists - envVal, envExists := result["env"] - if !envExists { - // No env to clean - return result +// FilterUserEnvForSettings filters user-defined env to only keep safe keys. +// It removes keys in managedEnvKeys or with ANTHROPIC_*/CLAUDE_* prefix. +// Returns nil if no keys remain. +func FilterUserEnvForSettings(userEnv map[string]interface{}, managedEnvKeys map[string]bool) map[string]interface{} { + if userEnv == nil { + return nil } - env, ok := envVal.(map[string]interface{}) - if !ok { - // env is not a map, nothing to clean - return result + filtered := make(map[string]interface{}) + for key, value := range userEnv { + if managedEnvKeys[key] { + continue + } + if strings.HasPrefix(key, "ANTHROPIC_") || strings.HasPrefix(key, "CLAUDE_") { + continue + } + filtered[key] = value } - // Build set of keys to remove for O(1) lookup - keysToRemove := make(map[string]bool) - for _, key := range providerEnvKeys { - keysToRemove[key] = true + if len(filtered) == 0 { + return nil } + return filtered +} - // Remove keys from env - for key := range env { - shouldRemove := false - - // Check for specific prefixes - if strings.HasPrefix(key, "ANTHROPIC_") || strings.HasPrefix(key, "CLAUDE_") { - shouldRemove = true - } - - // Check for provider env keys - if keysToRemove[key] { - shouldRemove = true +// MergeEnvMaps merges multiple env maps. Later maps override earlier ones. +// Returns nil if no maps have entries. +func MergeEnvMaps(maps ...map[string]interface{}) map[string]interface{} { + result := make(map[string]interface{}) + for _, m := range maps { + if m == nil { + continue } - - if shouldRemove { - delete(env, key) + for k, v := range m { + result[k] = v } } - - // Update env in result (always update to preserve the structure) - result["env"] = env - + if len(result) == 0 { + return nil + } return result } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 68bce8c..7b8557d 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -752,147 +752,204 @@ func TestLoadSettings(t *testing.T) { }) } -func TestCleanEnvInSettings(t *testing.T) { - t.Run("removes ANTHROPIC_ prefixed keys", func(t *testing.T) { - settings := map[string]interface{}{ - "env": map[string]interface{}{ +func TestFilterUserEnvForSettings(t *testing.T) { + tests := []struct { + name string + userEnv map[string]interface{} + managedEnvKeys map[string]bool + want map[string]interface{} + }{ + { + name: "keeps safe keys only", + userEnv: map[string]interface{}{ + "MY_CUSTOM_VAR": "value1", + "MY_OTHER_VAR": "value2", + }, + managedEnvKeys: map[string]bool{ + "API_TIMEOUT": true, + }, + want: map[string]interface{}{ + "MY_CUSTOM_VAR": "value1", + "MY_OTHER_VAR": "value2", + }, + }, + { + name: "removes managed keys", + userEnv: map[string]interface{}{ + "API_TIMEOUT": "30000", + "MY_VAR": "value", + "DISABLE_TELEM": "1", + }, + managedEnvKeys: map[string]bool{ + "API_TIMEOUT": true, + "DISABLE_TELEM": true, + }, + want: map[string]interface{}{ + "MY_VAR": "value", + }, + }, + { + name: "removes ANTHROPIC_ prefixed keys", + userEnv: map[string]interface{}{ "ANTHROPIC_MODEL": "claude-3.7-sonnet", "ANTHROPIC_BASE_URL": "https://old-url.com", "MY_CUSTOM_VAR": "value", }, - } - providerEnvKeys := []string{"BASE_URL"} - - result := CleanEnvInSettings(settings, providerEnvKeys) - - env := result["env"].(map[string]interface{}) - if _, exists := env["ANTHROPIC_MODEL"]; exists { - t.Error("ANTHROPIC_MODEL should be removed") - } - if _, exists := env["ANTHROPIC_BASE_URL"]; exists { - t.Error("ANTHROPIC_BASE_URL should be removed") - } - if _, exists := env["MY_CUSTOM_VAR"]; !exists { - t.Error("MY_CUSTOM_VAR should be kept") - } - }) - - t.Run("removes CLAUDE_ prefixed keys", func(t *testing.T) { - settings := map[string]interface{}{ - "env": map[string]interface{}{ - "CLAUDE_MODEL": "claude-3", - "CLAUDE_BASH_MAINTAIN_PROJECT_WORKING_DIR": "1", + managedEnvKeys: map[string]bool{}, + want: map[string]interface{}{ "MY_CUSTOM_VAR": "value", }, - } - providerEnvKeys := []string{} - - result := CleanEnvInSettings(settings, providerEnvKeys) - - env := result["env"].(map[string]interface{}) - if _, exists := env["CLAUDE_MODEL"]; exists { - t.Error("CLAUDE_MODEL should be removed") - } - if _, exists := env["CLAUDE_BASH_MAINTAIN_PROJECT_WORKING_DIR"]; exists { - t.Error("CLAUDE_BASH_MAINTAIN_PROJECT_WORKING_DIR should be removed") - } - if _, exists := env["MY_CUSTOM_VAR"]; !exists { - t.Error("MY_CUSTOM_VAR should be kept") - } - }) - - t.Run("removes provider env keys", func(t *testing.T) { - settings := map[string]interface{}{ - "env": map[string]interface{}{ - "BASE_URL": "old-url", - "AUTH_TOKEN": "old-token", + }, + { + name: "removes CLAUDE_ prefixed keys", + userEnv: map[string]interface{}{ + "CLAUDE_MODEL": "claude-3", "MY_CUSTOM_VAR": "value", }, - } - providerEnvKeys := []string{"BASE_URL", "AUTH_TOKEN"} - - result := CleanEnvInSettings(settings, providerEnvKeys) - - env := result["env"].(map[string]interface{}) - if _, exists := env["BASE_URL"]; exists { - t.Error("BASE_URL should be removed") - } - if _, exists := env["AUTH_TOKEN"]; exists { - t.Error("AUTH_TOKEN should be removed") - } - if _, exists := env["MY_CUSTOM_VAR"]; !exists { - t.Error("MY_CUSTOM_VAR should be kept") - } - }) - - t.Run("keeps non-matching keys", func(t *testing.T) { - settings := map[string]interface{}{ - "env": map[string]interface{}{ - "MY_CUSTOM_VAR_1": "value1", - "MY_CUSTOM_VAR_2": "value2", + managedEnvKeys: map[string]bool{}, + want: map[string]interface{}{ + "MY_CUSTOM_VAR": "value", }, - } - providerEnvKeys := []string{"BASE_URL"} - - result := CleanEnvInSettings(settings, providerEnvKeys) - - env := result["env"].(map[string]interface{}) - if len(env) != 2 { - t.Errorf("env should have 2 keys, got %d", len(env)) - } - if env["MY_CUSTOM_VAR_1"] != "value1" { - t.Error("MY_CUSTOM_VAR_1 should be kept") - } - if env["MY_CUSTOM_VAR_2"] != "value2" { - t.Error("MY_CUSTOM_VAR_2 should be kept") - } - }) - - t.Run("handles empty env", func(t *testing.T) { - settings := map[string]interface{}{ - "env": map[string]interface{}{}, - } - providerEnvKeys := []string{"BASE_URL"} - - result := CleanEnvInSettings(settings, providerEnvKeys) - - env, exists := result["env"] - if !exists || len(env.(map[string]interface{})) != 0 { - t.Error("empty env should remain empty") - } - }) - - t.Run("handles missing env", func(t *testing.T) { - settings := map[string]interface{}{ - "otherKey": "value", - } - providerEnvKeys := []string{} - - result := CleanEnvInSettings(settings, providerEnvKeys) - - if _, exists := result["env"]; exists { - t.Error("missing env should not be created") - } - }) - - t.Run("does not modify input", func(t *testing.T) { - settings := map[string]interface{}{ - "env": map[string]interface{}{ + }, + { + name: "nil user env returns nil", + userEnv: nil, + managedEnvKeys: map[string]bool{}, + want: nil, + }, + { + name: "empty user env returns nil", + userEnv: map[string]interface{}{ "ANTHROPIC_MODEL": "value", }, - "otherKey": "value", - } - providerEnvKeys := []string{"BASE_URL"} + managedEnvKeys: map[string]bool{}, + want: nil, + }, + { + name: "all keys conflicting returns nil", + userEnv: map[string]interface{}{ + "API_TIMEOUT": "30000", + }, + managedEnvKeys: map[string]bool{ + "API_TIMEOUT": true, + }, + want: nil, + }, + { + name: "empty managed keys keeps all safe keys", + userEnv: map[string]interface{}{ + "MY_VAR": "value", + }, + managedEnvKeys: map[string]bool{}, + want: map[string]interface{}{ + "MY_VAR": "value", + }, + }, + } - _ = CleanEnvInSettings(settings, providerEnvKeys) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := FilterUserEnvForSettings(tt.userEnv, tt.managedEnvKeys) + if !reflect.DeepEqual(got, tt.want) { + gotJSON, _ := json.MarshalIndent(got, "", " ") + wantJSON, _ := json.MarshalIndent(tt.want, "", " ") + t.Errorf("FilterUserEnvForSettings() =\n%s\n\nwant:\n%s", gotJSON, wantJSON) + } + }) + } +} - // Original should not be modified - if env, exists := settings["env"]; exists { - if _, exists := env.(map[string]interface{})["ANTHROPIC_MODEL"]; !exists { - t.Error("Original settings should not be modified") +func TestMergeEnvMaps(t *testing.T) { + tests := []struct { + name string + maps []map[string]interface{} + want map[string]interface{} + }{ + { + name: "merges two maps", + maps: []map[string]interface{}{ + {"A": "1"}, + {"B": "2"}, + }, + want: map[string]interface{}{ + "A": "1", + "B": "2", + }, + }, + { + name: "later map overrides earlier", + maps: []map[string]interface{}{ + {"A": "1", "B": "1"}, + {"B": "2", "C": "2"}, + }, + want: map[string]interface{}{ + "A": "1", + "B": "2", + "C": "2", + }, + }, + { + name: "single map", + maps: []map[string]interface{}{ + {"A": "1"}, + }, + want: map[string]interface{}{ + "A": "1", + }, + }, + { + name: "nil maps are skipped", + maps: []map[string]interface{}{ + nil, + {"A": "1"}, + nil, + }, + want: map[string]interface{}{ + "A": "1", + }, + }, + { + name: "all nil returns nil", + maps: []map[string]interface{}{ + nil, + nil, + }, + want: nil, + }, + { + name: "empty maps return nil", + maps: []map[string]interface{}{ + {}, + {}, + }, + want: nil, + }, + { + name: "three maps with cascading override", + maps: []map[string]interface{}{ + {"A": "1", "B": "1"}, + {"B": "2", "C": "2"}, + {"C": "3", "D": "3"}, + }, + want: map[string]interface{}{ + "A": "1", + "B": "2", + "C": "3", + "D": "3", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := MergeEnvMaps(tt.maps...) + if !reflect.DeepEqual(got, tt.want) { + gotJSON, _ := json.MarshalIndent(got, "", " ") + wantJSON, _ := json.MarshalIndent(tt.want, "", " ") + t.Errorf("MergeEnvMaps() =\n%s\n\nwant:\n%s", gotJSON, wantJSON) } - } - }) + }) + } } func TestMergeWithPriority(t *testing.T) { diff --git a/internal/provider/provider.go b/internal/provider/provider.go index ce95123..b9c095f 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -51,18 +51,22 @@ func SwitchWithHook(cfg *config.Config, providerName string) (*SwitchResult, err return nil, fmt.Errorf("failed to load settings: %w", err) } - // Merge with priority: user > provider > base - mergedSettings := config.MergeWithPriority(cfg.Settings, providerSettings, userSettings) - - // Extract provider env keys for cleaning + // Extract env from each source before merging (to distinguish user env from ccc env) + userEnvMap := config.GetEnv(userSettings) + baseEnvMap := config.GetEnv(cfg.Settings) providerEnvMap := config.GetEnv(providerSettings) - var providerEnvKeys []string + + // Build managed keys = base env keys + provider env keys + managedEnvKeys := make(map[string]bool) + for key := range baseEnvMap { + managedEnvKeys[key] = true + } for key := range providerEnvMap { - providerEnvKeys = append(providerEnvKeys, key) + managedEnvKeys[key] = true } - // Clean env: remove ANTHROPIC_*, CLAUDE_* prefixes and provider env keys - settingsWithHook := config.CleanEnvInSettings(mergedSettings, providerEnvKeys) + // Merge settings with priority: user > provider > base + mergedSettings := config.MergeWithPriority(cfg.Settings, providerSettings, userSettings) // Get ccc absolute path for hook command cccPath, err := os.Executable() @@ -75,10 +79,13 @@ func SwitchWithHook(cfg *config.Config, providerName string) (*SwitchResult, err // Ensure Supervisor Stop hook exists (preserves user's other hooks) // This also sets disableAllHooks and allowManagedHooksOnly to false - settingsWithHook = config.EnsureStopHook(settingsWithHook, hookCommand) + settingsWithHook := config.EnsureStopHook(mergedSettings, hookCommand) - // Remove env from settings before saving (provider env is passed via command line) + // Remove merged env from settings, replace with filtered user env delete(settingsWithHook, "env") + if filtered := config.FilterUserEnvForSettings(userEnvMap, managedEnvKeys); len(filtered) > 0 { + settingsWithHook["env"] = filtered + } // Save merged settings to settings.json settingsPath := config.GetSettingsPath() @@ -101,11 +108,11 @@ func SwitchWithHook(cfg *config.Config, providerName string) (*SwitchResult, err return nil, fmt.Errorf("failed to update current provider: %w", err) } - // Extract env map from merged settings for passing to subprocess - envMap := config.GetEnv(mergedSettings) + // Extract env map for subprocess: only base + provider env (not user env) + subprocessEnvMap := config.MergeEnvMaps(baseEnvMap, providerEnvMap) // Convert env map to EnvPair slice - envVars := envMapToPairs(envMap) + envVars := envMapToPairs(subprocessEnvMap) return &SwitchResult{ Settings: settingsWithHook, diff --git a/internal/provider/provider_test.go b/internal/provider/provider_test.go index 72d3a49..53801bf 100644 --- a/internal/provider/provider_test.go +++ b/internal/provider/provider_test.go @@ -152,7 +152,133 @@ func TestSwitchWithHook(t *testing.T) { }) } -func TestEnvPairsToStrings(t *testing.T) { +func TestSwitchWithHookUserEnv(t *testing.T) { + + t.Run("preserves user env without conflicts", func(t *testing.T) { + cleanup := setupTestDir(t) + defer cleanup() + + cfg := setupTestConfig(t) + + // Pre-create settings.json with user-defined env + userSettings := map[string]interface{}{ + "alwaysThinkingEnabled": false, + "env": map[string]interface{}{ + "MY_CUSTOM_VAR": "custom_value", + "MY_OTHER_VAR": "other_value", + "ANTHROPIC_MODEL": "should-be-filtered", + "DISABLE_TELEMETRY": "1", + }, + } + if err := config.SaveSettings(userSettings); err != nil { + t.Fatalf("Failed to save user settings: %v", err) + } + + // Save initial config + if err := config.Save(cfg); err != nil { + t.Fatalf("Failed to save config: %v", err) + } + + // Switch to glm + result, err := SwitchWithHook(cfg, "glm") + if err != nil { + t.Fatalf("SwitchWithHook() error = %v", err) + } + + // Verify settings.json has filtered user env + settingsEnv := config.GetEnv(result.Settings) + if settingsEnv == nil { + t.Fatal("Settings should contain filtered user env") + } + + // MY_CUSTOM_VAR and MY_OTHER_VAR should be preserved (not in base/provider env) + if settingsEnv["MY_CUSTOM_VAR"] != "custom_value" { + t.Errorf("MY_CUSTOM_VAR = %v, want custom_value", settingsEnv["MY_CUSTOM_VAR"]) + } + if settingsEnv["MY_OTHER_VAR"] != "other_value" { + t.Errorf("MY_OTHER_VAR = %v, want other_value", settingsEnv["MY_OTHER_VAR"]) + } + + // ANTHROPIC_MODEL should be filtered (ANTHROPIC_ prefix) + if _, exists := settingsEnv["ANTHROPIC_MODEL"]; exists { + t.Error("ANTHROPIC_MODEL should be filtered from settings env") + } + + // DISABLE_TELEMETRY should be filtered (exists in base env) + if _, exists := settingsEnv["DISABLE_TELEMETRY"]; exists { + t.Error("DISABLE_TELEMETRY should be filtered (conflicts with base env)") + } + + // Verify subprocess env does not contain user custom vars + envMap := make(map[string]string) + for _, pair := range result.EnvVars { + envMap[pair.Key] = pair.Value + } + if _, exists := envMap["MY_CUSTOM_VAR"]; exists { + t.Error("Subprocess env should not contain MY_CUSTOM_VAR (user env)") + } + if _, exists := envMap["MY_OTHER_VAR"]; exists { + t.Error("Subprocess env should not contain MY_OTHER_VAR (user env)") + } + + // Subprocess env should contain base + provider env + if envMap["API_TIMEOUT"] != "30000" { + t.Errorf("Subprocess API_TIMEOUT = %v, want 30000", envMap["API_TIMEOUT"]) + } + if envMap["ANTHROPIC_BASE_URL"] != "https://open.bigmodel.cn/api/anthropic" { + t.Errorf("Subprocess ANTHROPIC_BASE_URL = %v, want glm URL", envMap["ANTHROPIC_BASE_URL"]) + } + }) + + t.Run("removes conflicting user env", func(t *testing.T) { + cleanup := setupTestDir(t) + defer cleanup() + + cfg := setupTestConfig(t) + + // Pre-create settings.json with env that conflicts with base and provider + userSettings := map[string]interface{}{ + "env": map[string]interface{}{ + "API_TIMEOUT": "99999", + "ANTHROPIC_AUTH_TOKEN": "user-token", + }, + } + if err := config.SaveSettings(userSettings); err != nil { + t.Fatalf("Failed to save user settings: %v", err) + } + + // Save initial config + if err := config.Save(cfg); err != nil { + t.Fatalf("Failed to save config: %v", err) + } + + // Switch to glm + result, err := SwitchWithHook(cfg, "glm") + if err != nil { + t.Fatalf("SwitchWithHook() error = %v", err) + } + + // All user env keys should be filtered: + // - API_TIMEOUT conflicts with base env + // - ANTHROPIC_AUTH_TOKEN has ANTHROPIC_ prefix + settingsEnv := config.GetEnv(result.Settings) + if settingsEnv != nil { + t.Errorf("Settings env should be nil when all user keys are filtered, got: %v", settingsEnv) + } + + // Subprocess env should use base + provider values, not user's + envMap := make(map[string]string) + for _, pair := range result.EnvVars { + envMap[pair.Key] = pair.Value + } + if envMap["API_TIMEOUT"] != "30000" { + t.Errorf("Subprocess API_TIMEOUT = %v, want 30000 (from base)", envMap["API_TIMEOUT"]) + } + if envMap["ANTHROPIC_AUTH_TOKEN"] != "sk-glm-xxx" { + t.Errorf("Subprocess ANTHROPIC_AUTH_TOKEN = %v, want sk-glm-xxx (from provider)", envMap["ANTHROPIC_AUTH_TOKEN"]) + } + }) + pairs := []EnvPair{ {Key: "FOO", Value: "bar"}, {Key: "BAZ", Value: "qux"},