Feature/support multi env

[fuzi1996]

Summary / 概述

支持多环境

Related Issue / 关联 Issue

如何同时管理windows环境与WSL环境的切换

Screenshots / 截图

Checklist / 检查清单

• pnpm typecheck passes / 通过 TypeScript 类型检查
• pnpm format:check passes / 通过代码格式检查
• cargo clippy passes (if Rust code changed) / 通过 Clippy 检查（如修改了 Rust 代码）
• Updated i18n files if user-facing text changed / 如修改了用户可见文本，已更新国际化文件

[farion1231]

@codex review
@claude review

[claude]

Claude finished @farion1231's task in 2m 4s —— View job

---

Code Review: Feature/support multi env

• Gather context and read changed files
• Review Rust backend changes (settings.rs, config.rs, lib.rs)
• Review TypeScript/frontend changes (types.ts, api/settings.ts, hooks, components)
• Review i18n changes
• Summarize findings

---

Summary

This PR adds multi-environment profile support — users can define named ConfigDirProfile entries (each holding per-app config-dir paths + current provider IDs) and switch between them. The overall design is solid and the backward-compat migration logic is well thought-out. There are several issues to address before merging, ranging from a few functional bugs to UX problems and code smells.

---

Issues

🔴 Functional / Correctness

1. Stale closure in updateDirectoryState (useDirectorySettings.ts:294)

When updating a profile directory, onUpdateSettings is called with profiles.map(...), but this uses the stale profiles captured in the closure rather than the result of the immediately preceding setProfiles call:

setProfiles((prev) =>
  prev.map((p) => (p.id === activeProfileId ? updatedProfile : p)),
);
// ⚠️ 'profiles' here is still the old value — stale closure
onUpdateSettings({
  configDirProfiles: profiles.map((p) =>
    p.id === activeProfileId ? updatedProfile : p,
  ),
});

This should derive the new list once and use it for both:

const newProfiles = profiles.map((p) => (p.id === activeProfileId ? updatedProfile : p));
setProfiles(newProfiles);
onUpdateSettings({ configDirProfiles: newProfiles });

The same stale-closure pattern appears in createProfile (useDirectorySettings.ts:438): profiles hasn't yet been updated when onUpdateSettings is called.

Fix this →

---

2. Directory inputs show top-level values, not active-profile values

In SettingsPage.tsx:330-335, the DirectorySettings component is passed:

onUpdateProfile={updateProfile}

But in DirectorySettings.tsx, the claudeDir, codexDir, etc. props come from the top-level settings.claudeConfigDir, settings.codexConfigDir, etc. — not from the active profile's fields. So when a profile is active, the input fields display the wrong (top-level) values rather than the profile's actual directory overrides.

---

3. TOCTOU in set_active_config_dir_profile (settings.rs:1029-1039)

pub fn set_active_config_dir_profile(id: &str) -> Result<(), AppError> {
    let settings = get_settings();           // read lock released here
    if !settings.config_dir_profiles.iter().any(|p| p.id == id) {
        return Err(...);
    }
    mutate_settings(|settings| { ... })     // write lock acquired here
}

Between the read and the write, a concurrent call could delete the profile. On a single-user desktop app this is unlikely, but the check could be moved inside mutate_settings for correctness:

pub fn set_active_config_dir_profile(id: &str) -> Result<(), AppError> {
    let id = id.to_string();
    mutate_settings(|settings| {
        if settings.config_dir_profiles.iter().any(|p| p.id == id) {
            settings.active_config_dir_profile_id = Some(id.clone());
        }
    })
}

---

🟡 UX / Design

4. prompt() used for profile creation (DirectorySettings.tsx:128)

const name = prompt("Profile 名称:");
if (name) onCreateProfile?.(name);

Using the browser's native prompt() is inconsistent with the rest of the UI, not i18n-compatible (hardcoded Chinese), and blocks the thread. This should use a proper inline dialog or popover, similar to how other naming operations work in the app.

Fix this →

---

5. onUpdateProfile prop is declared but never destructured or used (DirectorySettings.tsx:29, 54)

interface DirectorySettingsProps {
  ...
  onUpdateProfile?: (profile: ConfigDirProfile) => Promise<void>; // declared ✓
}

export function DirectorySettings({
  ...,
  onDeleteProfile,   // destructured
  onSwitchProfile,   // destructured
  // onUpdateProfile is NOT destructured → silently dropped
}: DirectorySettingsProps) {

This means there is no way for the user to rename an existing profile from the settings UI.

---

6. Missing delete confirmation

The delete button in DirectorySettings.tsx:136-147 directly invokes onDeleteProfile without any confirmation dialog. Deleting a profile with configured paths could be destructive if accidental.

---

7. Hardcoded Chinese in migration default profile name (settings.rs:494)

name: "默认".to_string(),

This profile name is baked into settings.json at migration time and will appear in the UI for all users regardless of their language preference. Consider using an ASCII/English fallback like "Default" or deriving it from an i18n key at display time rather than at storage time.

---

8. switchProfile's type-unsafe cast (useDirectorySettings.ts:489-492)

const dirPath = (newActive as unknown as Record<string, string | undefined>)[meta.key];

ConfigDirProfile has named optional string fields. This cast is fragile — a rename of any field would silently return undefined at runtime. Use direct property access:

const key = meta.key as keyof ConfigDirProfile;
const dirPath = newActive[key] as string | undefined;

---

🟢 Minor / Polish

9. select onChange doesn't await in DirectorySettings.tsx:116

onChange={(e) => onSwitchProfile?.(e.target.value)}

The promise is not awaited and errors are silently swallowed. Consider adding error handling or disabling the select during the async operation.

10. Double onUpdateSettings call in deleteProfile (useDirectorySettings.ts:455-465)

Two separate onUpdateSettings calls are made sequentially (one for configDirProfiles, one for activeConfigDirProfileId). They could be merged into one call to avoid a potential extra re-render:

onUpdateSettings({
  configDirProfiles: newProfiles,
  ...(activeProfileId === id ? { activeConfigDirProfileId: undefined } : {}),
});

11. Missing i18n key for "环境配置集" (DirectorySettings.tsx:114)

The section header "环境配置集" is hardcoded Chinese and has no i18n entry. Add a key to en.json/zh.json.

12. Unused manage key in en.json:102

"profileSwitcher": {
  "title": "Environment Profile",
  "manage": "Manage Profiles",   // ← no component uses this key
  ...
}

---

What's Well Done

• Migration logic in migrate_legacy_fields_to_profile is clean and safe — only runs when no profiles exist and at least one legacy field is populated, with comprehensive unit tests.
• Priority fallback chain (active profile → top-level field) in get_*_override_dir and get_current_provider / set_current_provider is consistent throughout.
• ProfileSwitcher component is well-structured with good accessibility patterns and correctly short-circuits when no profiles exist.
• Command registration in lib.rs and the API layer in settings.ts are complete and consistent.
• Tests in settings.rs cover migration scenarios thoroughly.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c254ddda56

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[farion1231]

@claude review

(Re-running with the updated review prompt for comparison.)

[claude]

Claude finished @farion1231's task in 2m 25s —— View job

---

Code Review (re-run with updated prompt)

Three Important findings, plus a couple of nits. No verdict — leaving the call to you.

---

Important

1. Directory inputs in Settings show top-level values, not the active profile's values  _{confidence ~90}

SettingsPage.tsx:321-326 passes claudeDir={settings.claudeConfigDir} (and the other five) to DirectorySettings. But once a profile is active, updateDirectoryState (useDirectorySettings.ts:273-299) writes only into configDirProfiles[active].claude and leaves the top-level claudeConfigDir untouched. After the migration step copies the legacy top-level fields into the default profile (settings.rs:492-506), the two values are identical — but as soon as the user edits any directory while a profile is active, they diverge, and the input keeps showing the stale top-level path instead of what the profile actually resolves to. This also makes profile switching invisible in the input boxes (the displayed value never changes when activeProfileId changes).

Fix: derive the displayed values from the active profile when one exists, falling back to top-level fields, e.g. compute claudeDir = activeProfile?.claude ?? settings.claudeConfigDir in useSettings/SettingsPage before passing to DirectorySettings.

2. deleteProfile clobbers the backend's active-profile reassignment  _{confidence ~85}

delete_config_dir_profile in settings.rs:1015-1025 intentionally reassigns active_config_dir_profile_id to the first remaining profile when the deleted one was active. But the frontend (useDirectorySettings.ts:462-467) unconditionally writes activeConfigDirProfileId: undefined into form state:

if (activeProfileId === id) {
  setActiveProfileId(undefined);
  onUpdateSettings({ activeConfigDirProfileId: undefined });
}

onUpdateSettings is just local form state (useSettingsForm.setSettingsState). The next time settings get saved, that undefined is persisted and overwrites the reassignment the backend made — directory/provider resolution then silently falls back to the legacy top-level fields instead of the next remaining profile. (This is the same issue codex flagged at line 466, which I'm seconding.)

3. App-level useSettings() instance has stale profiles  _{confidence ~85}

App.tsx:178-182 calls useSettings() to feed ProfileSwitcher in the header, while SettingsPage calls it independently. In useDirectorySettings.ts:248-257, profiles is loaded once into local React state via getConfigDirProfiles() and is not subscribed to any shared cache. So when the user creates or deletes a profile inside Settings, only that hook instance's local state updates — the App-level switcher continues to show the pre-mutation list (deleted profiles still listed, newly created ones missing) until a full reload. (Codex's P2 finding — agreed.)

Cheapest fix: hoist profile state into a react-query key (e.g. ["configDirProfiles"]) so all useDirectorySettings consumers share a single source of truth and invalidate on mutation.

---

Nits

• DirectorySettings.tsx:128 uses native prompt() for naming a profile — hardcoded Chinese, not i18n'd, and Tauri's webviews aren't a guaranteed-consistent host for window.prompt. Worth swapping for an inline dialog.
• onUpdateProfile is declared on DirectorySettingsProps (DirectorySettings.tsx:29) but never destructured (:54) and never wired to a UI affordance — either implement rename or delete the prop. Per CLAUDE.md "if you are certain that something is unused, you can delete it completely."
  · feature/support_multi_env