From 811686c4bbacafc40ef6fac66675e2caf812337e Mon Sep 17 00:00:00 2001 From: Joseff Betancourt <1214364+Joseffb@users.noreply.github.com> Date: Sat, 17 Oct 2026 23:05:47 -0400 Subject: [PATCH] Add persistent application-level Codex Home selection --- README.md | 7 +- docs/codex-home.md | 45 +++ src-tauri/src/backend/app_server.rs | 30 +- src-tauri/src/bin/codex_monitor_daemon.rs | 127 +++++-- .../src/bin/codex_monitor_daemon/rpc/codex.rs | 14 +- src-tauri/src/codex/config.rs | 73 ++-- src-tauri/src/codex/home.rs | 321 ++++++++++++++++-- src-tauri/src/codex/mod.rs | 56 +-- src-tauri/src/files/io.rs | 233 ++++++++++--- src-tauri/src/files/mod.rs | 24 +- src-tauri/src/lib.rs | 2 + src-tauri/src/local_usage.rs | 9 +- src-tauri/src/prompts.rs | 29 +- src-tauri/src/settings/mod.rs | 40 ++- src-tauri/src/shared/agents_config_core.rs | 118 +++++-- src-tauri/src/shared/codex_aux_core.rs | 11 +- src-tauri/src/shared/codex_core.rs | 25 +- src-tauri/src/shared/codex_update_core.rs | 12 +- src-tauri/src/shared/config_toml_core.rs | 91 +++++ src-tauri/src/shared/files_core.rs | 96 +++++- src-tauri/src/shared/local_usage_core.rs | 98 +++--- src-tauri/src/shared/prompts_core.rs | 105 ++++-- src-tauri/src/shared/settings_core.rs | 255 ++++++++++++-- src-tauri/src/shared/workspaces_core.rs | 4 +- .../src/shared/workspaces_core/connect.rs | 120 ++++++- .../workspaces_core/crud_persistence.rs | 115 +++++-- .../workspaces_core/runtime_codex_args.rs | 10 +- .../src/shared/workspaces_core/worktree.rs | 40 ++- src-tauri/src/types.rs | 15 + .../settings/components/SettingsView.test.tsx | 3 + .../sections/SettingsCodexHome.test.tsx | 112 ++++++ .../sections/SettingsCodexSection.tsx | 142 ++++---- .../settings/hooks/globalEditorsHome.test.ts | 37 ++ .../settings/hooks/useAppSettings.test.ts | 9 + src/features/settings/hooks/useAppSettings.ts | 2 + .../hooks/useCodexHomeSetting.test.ts | 79 +++++ .../settings/hooks/useCodexHomeSetting.ts | 69 ++++ .../settings/hooks/useGlobalAgentsMd.ts | 4 +- .../hooks/useGlobalCodexConfigToml.ts | 4 +- .../settings/hooks/useSettingsCodexSection.ts | 68 +++- src/services/tauri.test.ts | 13 + src/services/tauri.ts | 9 + src/types.ts | 7 + 43 files changed, 2255 insertions(+), 428 deletions(-) create mode 100644 docs/codex-home.md create mode 100644 src/features/settings/components/sections/SettingsCodexHome.test.tsx create mode 100644 src/features/settings/hooks/globalEditorsHome.test.ts create mode 100644 src/features/settings/hooks/useCodexHomeSetting.test.ts create mode 100644 src/features/settings/hooks/useCodexHomeSetting.ts diff --git a/README.md b/README.md index 8b21f43dd8..591fded5d0 100644 --- a/README.md +++ b/README.md @@ -20,6 +20,7 @@ CodexMonitor is a Tauri app for orchestrating multiple Codex agents across local ### Composer & Agent Controls - Compose with image attachments (picker, drag/drop, paste) and configurable follow-up behavior (`Queue` vs `Steer` while a run is active). +- Select a persistent Codex Home in Settings → Codex, including symlinked directories, independently of the CLI executable. - Use `Shift+Cmd+Enter` (macOS) or `Shift+Ctrl+Enter` (Windows/Linux) to send the opposite follow-up action for a single message. - Autocomplete for skills (`$`), prompts (`/prompts:`), reviews (`/review`), and file paths (`@`). - Model picker, collaboration modes (when enabled), reasoning effort, access mode, and context usage ring. @@ -291,7 +292,7 @@ src-tauri/ - Workspaces persist to `workspaces.json` under the app data directory. - App settings persist to `settings.json` under the app data directory (theme, backend mode/provider, remote endpoints/tokens, Codex path, default access mode, UI scale, follow-up message behavior). -- Feature settings are supported in the UI and synced to `$CODEX_HOME/config.toml` (or `~/.codex/config.toml`) on load/save. Stable: Collaboration modes (`features.collaboration_modes`), personality (`personality`), and Background terminal (`features.unified_exec`). Experimental: Apps (`features.apps`). Steering capability still follows Codex `features.steer`, but follow-up default behavior is controlled in Settings → Composer. +- Feature settings read and update the selected Codex Home's `config.toml`. Stable: Collaboration modes (`features.collaboration_modes`), personality (`personality`), and Background terminal (`features.unified_exec`). Experimental: Apps (`features.apps`). Steering capability still follows Codex `features.steer`, but follow-up default behavior is controlled in Settings → Composer. - On launch and on window focus, the app reconnects and refreshes thread lists for each workspace. - Threads are restored by filtering `thread/list` results using the workspace `cwd`. - Selecting a thread always calls `thread/resume` to refresh messages from disk. @@ -299,10 +300,10 @@ src-tauri/ - The app uses `codex app-server` over stdio; see `src-tauri/src/lib.rs` and `src-tauri/src/codex/`. - The remote daemon entrypoint is `src-tauri/src/bin/codex_monitor_daemon.rs`; RPC routing lives in `src-tauri/src/bin/codex_monitor_daemon/rpc.rs` and domain handlers in `src-tauri/src/bin/codex_monitor_daemon/rpc/`. - Shared domain logic lives in `src-tauri/src/shared/` (notably `src-tauri/src/shared/git_ui_core/` and `src-tauri/src/shared/workspaces_core/`). -- Codex home resolves from workspace settings (if set), then legacy `.codexmonitor/`, then `$CODEX_HOME`/`~/.codex`. +- Codex Home resolves from the explicit application setting, then `$CODEX_HOME`, then `~/.codex`. Unavailable explicit homes report an error without falling back. Changing the selected home disconnects the shared app-server and reloads the UI. See [Codex Home](docs/codex-home.md). - Worktree agents live under the app data directory (`worktrees/`); legacy `.codex-worktrees/` paths remain supported, and the app no longer edits repo `.gitignore` files. - UI state (panel sizes, reduced transparency toggle, recent thread activity) is stored in `localStorage`. -- Custom prompts load from `$CODEX_HOME/prompts` (or `~/.codex/prompts`) with optional frontmatter description/argument hints. +- Custom prompts load from the selected Codex Home's `prompts` directory with optional frontmatter description/argument hints. ## Tauri IPC Surface diff --git a/docs/codex-home.md b/docs/codex-home.md new file mode 100644 index 0000000000..eeb46746f0 --- /dev/null +++ b/docs/codex-home.md @@ -0,0 +1,45 @@ +# Codex Home + +Settings → Codex → Codex Home selects the directory used for global configuration, +instructions, prompts, agents, local session usage, and the shared Codex app-server. +The setting is independent of the Codex executable and is saved as `codexHome` in +CodexMonitor's application settings. + +Enter an absolute directory path, or select a directory with **Browse home**. +**Validate** displays its canonical path without saving. **Save** validates and +persists the selection. **Use default** clears the field; save to restore default +resolution. Changing environments stops existing workspace app-servers and reloads +the UI, which reconnects workspaces and discovers sessions and models in the new +environment. Active turns stop when the environment changes. + +Resolution order: + +1. Explicit application Codex Home. +2. The application's inherited `CODEX_HOME` environment variable. +3. `~/.codex`. + +The default directory can be created on first use. Explicit application and +environment directories must exist and be accessible. An unavailable selected home, +including an unmounted external volume, produces an error and never selects another +home automatically. Settings remain accessible to correct an unavailable path. + +Symlinked home directories are supported: their canonical directory is the boundary +used by file operations. For compatibility with Codex, an existing global +`AGENTS.md` leaf symlink may resolve outside that boundary; the editor reads and +writes its target. Global `config.toml` and workspace `AGENTS.md` remain contained +within their canonical roots, and dangling links are errors. Saving application +preferences does not rewrite unrelated Codex configuration; changing homes does not +copy feature settings or credentials between environments. + +The selected home is passed only to child Codex processes. CodexMonitor does not +change the system launch environment. The executable must honor `CODEX_HOME` and +provide its own existing credential mechanism; selecting a home does not copy or +provision credentials. When an app-server reports its home during initialization, +CodexMonitor verifies it against the selection and refuses mismatches, including +launchers that override `CODEX_HOME`. Older app-servers can omit this field; their +executable must honor the supplied environment. Launchers must also avoid rewriting +configuration files to targets outside the selected home. + +The Home control applies to the local backend and is disabled in remote mode. +Remote file editors display paths from the connected daemon, whose own persisted +settings determine its home. diff --git a/src-tauri/src/backend/app_server.rs b/src-tauri/src/backend/app_server.rs index 9dce23cc2f..48721756ba 100644 --- a/src-tauri/src/backend/app_server.rs +++ b/src-tauri/src/backend/app_server.rs @@ -14,6 +14,7 @@ use tokio::time::timeout; use crate::backend::events::{AppServerEvent, EventSink}; use crate::codex::args::parse_codex_args; +use crate::codex::home::validate_reported_codex_home; use crate::shared::process_core::{kill_child_process_tree, tokio_command}; use crate::types::WorkspaceEntry; @@ -699,8 +700,12 @@ pub(crate) fn build_codex_command_with_bin( pub(crate) async fn check_codex_installation( codex_bin: Option, + codex_home: Option<&Path>, ) -> Result, String> { let mut command = build_codex_command_with_bin(codex_bin, None, vec!["--version".to_string()])?; + if let Some(home) = codex_home { + command.env("CODEX_HOME", home); + } command.stdout(std::process::Stdio::piped()); command.stderr(std::process::Stdio::piped()); @@ -755,7 +760,7 @@ pub(crate) async fn spawn_workspace_session( event_sink: E, ) -> Result, String> { let codex_bin = default_codex_bin; - let _ = check_codex_installation(codex_bin.clone()).await?; + let _ = check_codex_installation(codex_bin.clone(), codex_home.as_deref()).await?; let mut command = build_codex_command_with_bin( codex_bin, @@ -1087,8 +1092,27 @@ pub(crate) async fn spawn_workspace_session( ); } }; - init_response?; - session.send_notification("initialized", None).await?; + let initialization = async { + let response = init_response?; + if response.get("error").is_some() { + return Err("Codex app-server rejected initialization".to_string()); + } + let reported_home = response + .get("result") + .and_then(|result| result.get("codexHome")) + .or_else(|| response.get("codexHome")) + .and_then(Value::as_str); + if let (Some(expected), Some(reported)) = (codex_home.as_deref(), reported_home) { + validate_reported_codex_home(expected, reported)?; + } + session.send_notification("initialized", None).await + } + .await; + if let Err(error) = initialization { + let mut child = session.child.lock().await; + kill_child_process_tree(&mut child).await; + return Err(error); + } let payload = AppServerEvent { workspace_id: entry.id.clone(), diff --git a/src-tauri/src/bin/codex_monitor_daemon.rs b/src-tauri/src/bin/codex_monitor_daemon.rs index 59bfc00bc8..85638b5053 100644 --- a/src-tauri/src/bin/codex_monitor_daemon.rs +++ b/src-tauri/src/bin/codex_monitor_daemon.rs @@ -584,8 +584,18 @@ impl DaemonState { } async fn update_app_settings(&self, settings: AppSettings) -> Result { - settings_core::update_app_settings_core(settings, &self.app_settings, &self.settings_path) - .await + let _spawn_guard = workspaces_core::workspace_session_spawn_lock().lock().await; + let previous = self.app_settings.lock().await.clone(); + let updated = settings_core::update_app_settings_core( + settings, + &self.app_settings, + &self.settings_path, + ) + .await?; + if previous.codex_home != updated.codex_home { + workspaces_core::disconnect_all_sessions_core(&self.sessions).await; + } + Ok(updated) } async fn set_codex_feature_flag( @@ -593,43 +603,51 @@ impl DaemonState { feature_key: String, enabled: bool, ) -> Result<(), String> { - codex_config::write_feature_enabled(feature_key.as_str(), enabled) + let settings = self.app_settings.lock().await; + let home = codex::home::resolve_codex_home(settings.codex_home.as_deref())?; + codex_config::write_feature_enabled(&home, feature_key.as_str(), enabled) } async fn get_agents_settings(&self) -> Result { - agents_config_core::get_agents_settings_core() + let app_settings = self.app_settings.lock().await.clone(); + agents_config_core::get_agents_settings_core(&app_settings) } async fn set_agents_core_settings( &self, input: agents_config_core::SetAgentsCoreInput, ) -> Result { - agents_config_core::set_agents_core_settings_core(input) + let app_settings = self.app_settings.lock().await.clone(); + agents_config_core::set_agents_core_settings_core(&app_settings, input) } async fn create_agent( &self, input: agents_config_core::CreateAgentInput, ) -> Result { - agents_config_core::create_agent_core(input) + let app_settings = self.app_settings.lock().await.clone(); + agents_config_core::create_agent_core(&app_settings, input) } async fn update_agent( &self, input: agents_config_core::UpdateAgentInput, ) -> Result { - agents_config_core::update_agent_core(input) + let app_settings = self.app_settings.lock().await.clone(); + agents_config_core::update_agent_core(&app_settings, input) } async fn delete_agent( &self, input: agents_config_core::DeleteAgentInput, ) -> Result { - agents_config_core::delete_agent_core(input) + let app_settings = self.app_settings.lock().await.clone(); + agents_config_core::delete_agent_core(&app_settings, input) } async fn read_agent_config_toml(&self, agent_name: String) -> Result { - agents_config_core::read_agent_config_toml_core(agent_name.as_str()) + let app_settings = self.app_settings.lock().await.clone(); + agents_config_core::read_agent_config_toml_core(&app_settings, agent_name.as_str()) } async fn write_agent_config_toml( @@ -637,7 +655,12 @@ impl DaemonState { agent_name: String, content: String, ) -> Result<(), String> { - agents_config_core::write_agent_config_toml_core(agent_name.as_str(), content.as_str()) + let app_settings = self.app_settings.lock().await.clone(); + agents_config_core::write_agent_config_toml_core( + &app_settings, + agent_name.as_str(), + content.as_str(), + ) } async fn list_workspace_files(&self, workspace_id: String) -> Result, String> { @@ -667,7 +690,14 @@ impl DaemonState { kind: file_policy::FileKind, workspace_id: Option, ) -> Result { - files_core::file_read_core(&self.workspaces, scope, kind, workspace_id).await + files_core::file_read_core( + &self.workspaces, + &self.app_settings, + scope, + kind, + workspace_id, + ) + .await } async fn file_write( @@ -677,7 +707,15 @@ impl DaemonState { workspace_id: Option, content: String, ) -> Result<(), String> { - files_core::file_write_core(&self.workspaces, scope, kind, workspace_id, content).await + files_core::file_write_core( + &self.workspaces, + &self.app_settings, + scope, + kind, + workspace_id, + content, + ) + .await } async fn start_thread(&self, workspace_id: String) -> Result { @@ -692,11 +730,7 @@ impl DaemonState { codex_core::resume_thread_core(&self.sessions, workspace_id, thread_id).await } - async fn read_thread( - &self, - workspace_id: String, - thread_id: String, - ) -> Result { + async fn read_thread(&self, workspace_id: String, thread_id: String) -> Result { codex_core::read_thread_core(&self.sessions, workspace_id, thread_id).await } @@ -765,8 +799,7 @@ impl DaemonState { limit: Option, sort_key: Option, ) -> Result { - codex_core::list_threads_core(&self.sessions, workspace_id, cursor, limit, sort_key) - .await + codex_core::list_threads_core(&self.sessions, workspace_id, cursor, limit, sort_key).await } async fn list_mcp_server_status( @@ -897,7 +930,13 @@ impl DaemonState { } async fn account_read(&self, workspace_id: String) -> Result { - codex_core::account_read_core(&self.sessions, &self.workspaces, workspace_id).await + codex_core::account_read_core( + &self.app_settings, + &self.sessions, + &self.workspaces, + workspace_id, + ) + .await } async fn codex_login(&self, workspace_id: String) -> Result { @@ -944,11 +983,17 @@ impl DaemonState { workspace_id: String, command: Vec, ) -> Result { - codex_core::remember_approval_rule_core(&self.workspaces, workspace_id, command).await + codex_core::remember_approval_rule_core( + &self.app_settings, + &self.workspaces, + workspace_id, + command, + ) + .await } async fn get_config_model(&self, workspace_id: String) -> Result { - codex_core::get_config_model_core(&self.workspaces, workspace_id).await + codex_core::get_config_model_core(&self.app_settings, &self.workspaces, workspace_id).await } async fn add_clone( @@ -1177,7 +1222,14 @@ impl DaemonState { } async fn prompts_list(&self, workspace_id: String) -> Result, String> { - prompts_core::prompts_list_core(&self.workspaces, &self.settings_path, workspace_id).await + let app_settings = self.app_settings.lock().await.clone(); + prompts_core::prompts_list_core( + &app_settings, + &self.workspaces, + &self.settings_path, + workspace_id, + ) + .await } async fn prompts_workspace_dir(&self, workspace_id: String) -> Result { @@ -1190,7 +1242,8 @@ impl DaemonState { } async fn prompts_global_dir(&self, workspace_id: String) -> Result { - prompts_core::prompts_global_dir_core(&self.workspaces, workspace_id).await + let app_settings = self.app_settings.lock().await.clone(); + prompts_core::prompts_global_dir_core(&app_settings, &self.workspaces, workspace_id).await } async fn prompts_create( @@ -1202,7 +1255,9 @@ impl DaemonState { argument_hint: Option, content: String, ) -> Result { + let app_settings = self.app_settings.lock().await.clone(); prompts_core::prompts_create_core( + &app_settings, &self.workspaces, &self.settings_path, workspace_id, @@ -1224,7 +1279,9 @@ impl DaemonState { argument_hint: Option, content: String, ) -> Result { + let app_settings = self.app_settings.lock().await.clone(); prompts_core::prompts_update_core( + &app_settings, &self.workspaces, &self.settings_path, workspace_id, @@ -1238,8 +1295,15 @@ impl DaemonState { } async fn prompts_delete(&self, workspace_id: String, path: String) -> Result<(), String> { - prompts_core::prompts_delete_core(&self.workspaces, &self.settings_path, workspace_id, path) - .await + let app_settings = self.app_settings.lock().await.clone(); + prompts_core::prompts_delete_core( + &app_settings, + &self.workspaces, + &self.settings_path, + workspace_id, + path, + ) + .await } async fn prompts_move( @@ -1248,7 +1312,9 @@ impl DaemonState { path: String, scope: String, ) -> Result { + let app_settings = self.app_settings.lock().await.clone(); prompts_core::prompts_move_core( + &app_settings, &self.workspaces, &self.settings_path, workspace_id, @@ -1334,7 +1400,14 @@ impl DaemonState { days: Option, workspace_path: Option, ) -> Result { - local_usage_core::local_usage_snapshot_core(&self.workspaces, days, workspace_path).await + let app_settings = self.app_settings.lock().await.clone(); + local_usage_core::local_usage_snapshot_core( + &app_settings, + &self.workspaces, + days, + workspace_path, + ) + .await } async fn menu_set_accelerators(&self, _updates: Vec) -> Result<(), String> { diff --git a/src-tauri/src/bin/codex_monitor_daemon/rpc/codex.rs b/src-tauri/src/bin/codex_monitor_daemon/rpc/codex.rs index cc278938ea..6cfb90cbbd 100644 --- a/src-tauri/src/bin/codex_monitor_daemon/rpc/codex.rs +++ b/src-tauri/src/bin/codex_monitor_daemon/rpc/codex.rs @@ -17,12 +17,24 @@ pub(super) async fn try_handle( ) -> Option> { match method { "get_codex_config_path" => { - let path = match settings_core::get_codex_config_path_core() { + let path = match settings_core::get_codex_config_path_core(&state.app_settings).await { Ok(value) => value, Err(err) => return Some(Err(err)), }; Some(Ok(Value::String(path))) } + "get_codex_home" => Some( + settings_core::get_codex_home_core(&state.app_settings) + .await + .and_then(|status| serde_json::to_value(status).map_err(|err| err.to_string())), + ), + "validate_codex_home" => { + let configured_home = parse_optional_string(params, "codexHome"); + Some( + crate::codex::home::resolve_codex_home_status(configured_home.as_deref()) + .and_then(|status| serde_json::to_value(status).map_err(|err| err.to_string())), + ) + } "get_config_model" => { let workspace_id = match parse_string(params, "workspaceId") { Ok(value) => value, diff --git a/src-tauri/src/codex/config.rs b/src-tauri/src/codex/config.rs index 790821f22e..7bc10b9ca7 100644 --- a/src-tauri/src/codex/config.rs +++ b/src-tauri/src/codex/config.rs @@ -1,48 +1,49 @@ -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use crate::shared::config_toml_core; -pub(crate) fn read_steer_enabled() -> Result, String> { - read_feature_flag("steer") +pub(crate) fn read_steer_enabled(root: &Path) -> Result, String> { + read_feature_flag(root, "steer") } -pub(crate) fn read_collaboration_modes_enabled() -> Result, String> { - read_feature_flag("collaboration_modes") +pub(crate) fn read_collaboration_modes_enabled(root: &Path) -> Result, String> { + read_feature_flag(root, "collaboration_modes") } -pub(crate) fn read_unified_exec_enabled() -> Result, String> { - read_feature_flag("unified_exec") +pub(crate) fn read_unified_exec_enabled(root: &Path) -> Result, String> { + read_feature_flag(root, "unified_exec") } -pub(crate) fn read_apps_enabled() -> Result, String> { - read_feature_flag("apps") +pub(crate) fn read_apps_enabled(root: &Path) -> Result, String> { + read_feature_flag(root, "apps") } -pub(crate) fn read_personality() -> Result, String> { - let Some(root) = resolve_default_codex_home() else { - return Ok(None); - }; +pub(crate) fn read_personality(root: &Path) -> Result, String> { let (_, document) = config_toml_core::load_global_config_document(&root)?; Ok(read_personality_from_document(&document)) } -pub(crate) fn write_steer_enabled(enabled: bool) -> Result<(), String> { - write_feature_flag("steer", enabled) +pub(crate) fn write_steer_enabled(root: &Path, enabled: bool) -> Result<(), String> { + write_feature_flag(root, "steer", enabled) } -pub(crate) fn write_collaboration_modes_enabled(enabled: bool) -> Result<(), String> { - write_feature_flag("collaboration_modes", enabled) +pub(crate) fn write_collaboration_modes_enabled(root: &Path, enabled: bool) -> Result<(), String> { + write_feature_flag(root, "collaboration_modes", enabled) } -pub(crate) fn write_unified_exec_enabled(enabled: bool) -> Result<(), String> { - write_feature_flag("unified_exec", enabled) +pub(crate) fn write_unified_exec_enabled(root: &Path, enabled: bool) -> Result<(), String> { + write_feature_flag(root, "unified_exec", enabled) } -pub(crate) fn write_apps_enabled(enabled: bool) -> Result<(), String> { - write_feature_flag("apps", enabled) +pub(crate) fn write_apps_enabled(root: &Path, enabled: bool) -> Result<(), String> { + write_feature_flag(root, "apps", enabled) } -pub(crate) fn write_feature_enabled(feature_key: &str, enabled: bool) -> Result<(), String> { +pub(crate) fn write_feature_enabled( + root: &Path, + feature_key: &str, + enabled: bool, +) -> Result<(), String> { let key = feature_key.trim(); if key.is_empty() { return Err("feature key is empty".to_string()); @@ -50,53 +51,39 @@ pub(crate) fn write_feature_enabled(feature_key: &str, enabled: bool) -> Result< if key.eq_ignore_ascii_case("collab") { return Err("feature key `collab` is no longer supported; use `multi_agent`".to_string()); } - write_feature_flag(key, enabled) + write_feature_flag(root, key, enabled) } -pub(crate) fn write_personality(personality: &str) -> Result<(), String> { - let Some(root) = resolve_default_codex_home() else { - return Ok(()); - }; +pub(crate) fn write_personality(root: &Path, personality: &str) -> Result<(), String> { let (_, mut document) = config_toml_core::load_global_config_document(&root)?; let normalized = normalize_personality_value(personality); config_toml_core::set_top_level_string(&mut document, "personality", normalized); config_toml_core::persist_global_config_document(&root, &document) } -fn read_feature_flag(key: &str) -> Result, String> { - let Some(root) = resolve_default_codex_home() else { - return Ok(None); - }; +fn read_feature_flag(root: &Path, key: &str) -> Result, String> { let (_, document) = config_toml_core::load_global_config_document(&root)?; Ok(config_toml_core::read_feature_flag(&document, key)) } -fn write_feature_flag(key: &str, enabled: bool) -> Result<(), String> { - let Some(root) = resolve_default_codex_home() else { - return Ok(()); - }; +fn write_feature_flag(root: &Path, key: &str, enabled: bool) -> Result<(), String> { let (_, mut document) = config_toml_core::load_global_config_document(&root)?; config_toml_core::set_feature_flag(&mut document, key, enabled)?; config_toml_core::persist_global_config_document(&root, &document) } -pub(crate) fn config_toml_path() -> Option { - resolve_default_codex_home().map(|home| home.join("config.toml")) +pub(crate) fn config_toml_path(root: &Path) -> PathBuf { + root.join("config.toml") } pub(crate) fn read_config_model(codex_home: Option) -> Result, String> { - let root = codex_home.or_else(resolve_default_codex_home); - let Some(root) = root else { + let Some(root) = codex_home else { return Err("Unable to resolve CODEX_HOME".to_string()); }; let (_, document) = config_toml_core::load_global_config_document(&root)?; Ok(config_toml_core::read_top_level_string(&document, "model")) } -fn resolve_default_codex_home() -> Option { - crate::codex::home::resolve_default_codex_home() -} - fn read_personality_from_document(document: &toml_edit::Document) -> Option { config_toml_core::read_top_level_string(document, "personality") .as_deref() diff --git a/src-tauri/src/codex/home.rs b/src-tauri/src/codex/home.rs index cd3e29e251..58230f2f08 100644 --- a/src-tauri/src/codex/home.rs +++ b/src-tauri/src/codex/home.rs @@ -1,22 +1,143 @@ use std::env; -use std::path::PathBuf; +use std::fs; +use std::path::{Path, PathBuf}; -use crate::types::WorkspaceEntry; +use serde::Serialize; -pub(crate) fn resolve_workspace_codex_home( - _entry: &WorkspaceEntry, - _parent_entry: Option<&WorkspaceEntry>, -) -> Option { - resolve_default_codex_home() +use crate::types::{AppSettings, WorkspaceEntry}; + +#[derive(Debug, Serialize)] +#[serde(rename_all = "camelCase")] +pub(crate) struct CodexHomeStatus { + pub(crate) configured_path: Option, + pub(crate) effective_path: String, + pub(crate) source: String, +} + +/// Resolve the home once for all application operations. An unavailable selected +/// home is an error: falling back could select another environment's credentials. +pub(crate) fn resolve_codex_home(configured_home: Option<&str>) -> Result { + resolve_codex_home_with_sources( + configured_home, + env::var("CODEX_HOME").ok().as_deref(), + resolve_home_dir().as_deref(), + ) + .map(|(path, _)| path) +} + +/// Confirm that a launcher did not replace the home selected by the application. +/// Older app-servers may omit this field; callers only invoke this when reported. +pub(crate) fn validate_reported_codex_home(expected: &Path, reported: &str) -> Result<(), String> { + if reported.trim().is_empty() { + return Err( + "Codex app-server reported an empty Codex home; refusing to connect".to_string(), + ); + } + let expected = fs::canonicalize(expected).map_err(|error| { + format!( + "Selected Codex home became unavailable: {} ({error})", + expected.display(), + ) + })?; + if !expected.is_dir() { + return Err(format!( + "Selected Codex home is not a directory: {}", + expected.display() + )); + } + let (actual, _) = resolve_codex_home_with_sources(Some(reported), None, None)?; + if actual != expected { + return Err(format!( + "Codex app-server is using a different Codex home (expected {}, reported {}). Check the configured executable or launcher; refusing to connect.", + expected.display(), actual.display(), + )); + } + Ok(()) +} + +pub(crate) fn resolve_codex_home_status( + configured_home: Option<&str>, +) -> Result { + let (path, source) = resolve_codex_home_with_sources( + configured_home, + env::var("CODEX_HOME").ok().as_deref(), + resolve_home_dir().as_deref(), + )?; + Ok(CodexHomeStatus { + configured_path: nonempty(configured_home).map(str::to_owned), + effective_path: path.to_string_lossy().into_owned(), + source: source.to_string(), + }) } -pub(crate) fn resolve_default_codex_home() -> Option { - if let Ok(value) = env::var("CODEX_HOME") { - if let Some(path) = normalize_codex_home(&value) { - return Some(path); +fn nonempty(value: Option<&str>) -> Option<&str> { + value.map(str::trim).filter(|value| !value.is_empty()) +} + +fn resolve_codex_home_with_sources( + configured_home: Option<&str>, + environment_home: Option<&str>, + user_home: Option<&Path>, +) -> Result<(PathBuf, &'static str), String> { + let (path, source) = if let Some(value) = nonempty(configured_home) { + ( + normalize_codex_home(value).expect("nonempty home"), + "application", + ) + } else if let Some(value) = nonempty(environment_home) { + ( + normalize_codex_home(value).expect("nonempty home"), + "environment", + ) + } else { + ( + user_home + .ok_or_else(|| "Unable to resolve the default Codex home directory".to_string())? + .join(".codex"), + "default", + ) + }; + if !path.is_absolute() { + return Err(format!( + "Codex home must be an absolute directory path: {}", + path.display() + )); + } + let canonical = match fs::canonicalize(&path) { + Ok(canonical) => canonical, + Err(error) + if source == "default" + && error.kind() == std::io::ErrorKind::NotFound + && matches!(fs::symlink_metadata(&path), Err(error) if error.kind() == std::io::ErrorKind::NotFound) => + { + // Preserve first-run default behavior; global file writes create it. + return Ok((path, source)); + } + Err(error) => { + return Err(format!( + "Codex home directory is unavailable: {} ({error}). Ensure its volume is mounted and the directory is accessible.", + path.display() + )); } + }; + if !canonical.is_dir() { + return Err(format!("Codex home is not a directory: {}", path.display())); } - resolve_home_dir().map(|home| home.join(".codex")) + fs::read_dir(&canonical).map_err(|error| { + format!( + "Codex home directory is not accessible: {} ({error})", + path.display() + ) + })?; + Ok((canonical, source)) +} + +pub(crate) fn resolve_workspace_codex_home( + _entry: &WorkspaceEntry, + _parent_entry: Option<&WorkspaceEntry>, + app_settings: &AppSettings, +) -> Result { + resolve_codex_home(app_settings.codex_home.as_deref()) } fn normalize_codex_home(value: &str) -> Option { @@ -196,23 +317,177 @@ mod tests { } } - #[test] - fn workspace_codex_home_uses_default_resolution() { - let entry = workspace_entry(WorkspaceKind::Main, "/repo"); - let _guard = ENV_LOCK.lock().expect("lock env"); + struct Fixture(PathBuf); - let prev_codex_home = std::env::var("CODEX_HOME").ok(); - std::env::set_var("CODEX_HOME", "/tmp/codex-global"); + impl Fixture { + fn new() -> Self { + let path = std::env::temp_dir().join(format!("codex-home-{}", uuid::Uuid::new_v4())); + fs::create_dir_all(&path).expect("create fixture"); + Self(path) + } - let resolved = resolve_workspace_codex_home(&entry, None); - assert_eq!(resolved, Some(PathBuf::from("/tmp/codex-global"))); + fn directory(&self, name: &str) -> PathBuf { + let path = self.0.join(name); + fs::create_dir_all(&path).expect("create directory"); + path + } + } - match prev_codex_home { - Some(value) => std::env::set_var("CODEX_HOME", value), - None => std::env::remove_var("CODEX_HOME"), + impl Drop for Fixture { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.0); } } + #[test] + fn reported_home_must_match_selected_existing_directory() { + let fixture = Fixture::new(); + let selected = fixture.directory("selected"); + let other = fixture.directory("other"); + validate_reported_codex_home(&selected, selected.to_str().unwrap()).unwrap(); + assert!( + validate_reported_codex_home(&selected, other.to_str().unwrap()) + .unwrap_err() + .contains("different Codex home") + ); + assert!(validate_reported_codex_home(&selected, "").is_err()); + assert!(validate_reported_codex_home( + &selected, + fixture.0.join("missing").to_str().unwrap() + ) + .is_err()); + assert!(validate_reported_codex_home( + &fixture.0.join("missing"), + selected.to_str().unwrap() + ) + .is_err()); + } + + #[cfg(unix)] + #[test] + fn reported_home_accepts_symlink_to_selected_home() { + let fixture = Fixture::new(); + let selected = fixture.directory("selected"); + let linked = fixture.0.join("linked"); + std::os::unix::fs::symlink(&selected, &linked).unwrap(); + validate_reported_codex_home(&selected, linked.to_str().unwrap()).unwrap(); + validate_reported_codex_home(&linked, selected.to_str().unwrap()).unwrap(); + } + + #[test] + fn explicit_home_precedes_environment_and_default() { + let fixture = Fixture::new(); + let selected = fixture.directory("selected"); + let environment = fixture.directory("environment"); + let (resolved, source) = resolve_codex_home_with_sources( + selected.to_str(), + environment.to_str(), + Some(&fixture.0), + ) + .expect("resolve explicit"); + assert_eq!( + resolved, + fs::canonicalize(selected).expect("canonical selected") + ); + assert_eq!(source, "application"); + } + + #[test] + fn environment_precedes_default_and_blank_setting_restores_default_behavior() { + let fixture = Fixture::new(); + let environment = fixture.directory("environment"); + let (resolved, source) = + resolve_codex_home_with_sources(Some(" "), environment.to_str(), Some(&fixture.0)) + .expect("resolve environment"); + assert_eq!( + resolved, + fs::canonicalize(environment).expect("canonical environment") + ); + assert_eq!(source, "environment"); + let (resolved, source) = + resolve_codex_home_with_sources(None, Some(" "), Some(&fixture.0)) + .expect("missing default remains creatable"); + assert_eq!(resolved, fixture.0.join(".codex")); + assert_eq!(source, "default"); + } + + #[test] + fn unavailable_selected_home_never_falls_back() { + let fixture = Fixture::new(); + let selected = fixture.0.join("unmounted-volume"); + let environment = fixture.directory("environment"); + let error = resolve_codex_home_with_sources( + selected.to_str(), + environment.to_str(), + Some(&fixture.0), + ) + .expect_err("unavailable explicit home"); + assert!(error.contains("unavailable")); + assert!(error.contains("unmounted-volume")); + assert!( + resolve_codex_home_with_sources(None, selected.to_str(), Some(&fixture.0)).is_err() + ); + } + + #[test] + fn home_must_be_an_absolute_directory() { + let fixture = Fixture::new(); + let file = fixture.0.join("config.toml"); + fs::write(&file, "").expect("create file"); + assert!(resolve_codex_home_with_sources(file.to_str(), None, None) + .expect_err("file is not home") + .contains("not a directory")); + assert!( + resolve_codex_home_with_sources(Some("relative/home"), None, None) + .expect_err("relative home") + .contains("absolute") + ); + } + + #[test] + fn workspace_home_uses_application_setting() { + let fixture = Fixture::new(); + let selected = fixture.directory("selected"); + let settings = AppSettings { + codex_home: selected.to_str().map(str::to_owned), + ..AppSettings::default() + }; + let entry = workspace_entry(WorkspaceKind::Main, "/repo"); + assert_eq!( + resolve_workspace_codex_home(&entry, None, &settings).expect("resolve workspace"), + fs::canonicalize(selected).expect("canonical selected"), + ); + let status = resolve_codex_home_status(settings.codex_home.as_deref()).expect("status"); + assert_eq!(status.configured_path, settings.codex_home); + assert_eq!(status.source, "application"); + let serialized = serde_json::to_value(&status).expect("serialize status"); + assert_eq!(serialized["configuredPath"], settings.codex_home.unwrap()); + assert_eq!(serialized["effectivePath"], status.effective_path); + assert_eq!(serialized["source"], "application"); + } + + #[cfg(unix)] + #[test] + fn symlinked_home_is_canonical_and_dangling_link_is_rejected() { + use std::os::unix::fs::symlink; + let fixture = Fixture::new(); + let destination = fixture.directory("volume-home"); + fs::write(destination.join("config.toml"), "model = \"fixture\"\n").expect("config"); + let link = fixture.0.join("selected-home"); + symlink(&destination, &link).expect("symlink home"); + let (resolved, _) = + resolve_codex_home_with_sources(link.to_str(), None, None).expect("resolve symlink"); + assert_eq!( + resolved, + fs::canonicalize(&destination).expect("canonical destination") + ); + fs::remove_dir_all(destination).expect("simulate unmounted home"); + assert!(resolve_codex_home_with_sources(link.to_str(), None, None).is_err()); + let default_link = fixture.0.join(".codex"); + symlink(fixture.0.join("absent"), &default_link).expect("dangling default"); + assert!(resolve_codex_home_with_sources(None, None, Some(&fixture.0)).is_err()); + } + #[test] fn codex_home_expands_tilde_and_env_vars() { let _guard = ENV_LOCK.lock().expect("lock env"); diff --git a/src-tauri/src/codex/mod.rs b/src-tauri/src/codex/mod.rs index e55d1e9eea..944ccdd8ec 100644 --- a/src-tauri/src/codex/mod.rs +++ b/src-tauri/src/codex/mod.rs @@ -508,14 +508,7 @@ pub(crate) async fn start_review( .await; } - codex_core::start_review_core( - &state.sessions, - workspace_id, - thread_id, - target, - delivery, - ) - .await + codex_core::start_review_core(&state.sessions, workspace_id, thread_id, target, delivery).await } #[tauri::command] @@ -583,7 +576,9 @@ pub(crate) async fn set_codex_feature_flag( return Ok(()); } - config::write_feature_enabled(feature_key.as_str(), enabled) + let settings = state.app_settings.lock().await; + let home = home::resolve_codex_home(settings.codex_home.as_deref())?; + config::write_feature_enabled(&home, feature_key.as_str(), enabled) } #[tauri::command] @@ -597,7 +592,8 @@ pub(crate) async fn get_agents_settings( return serde_json::from_value(response).map_err(|err| err.to_string()); } - agents_config_core::get_agents_settings_core() + let app_settings = state.app_settings.lock().await.clone(); + agents_config_core::get_agents_settings_core(&app_settings) } #[tauri::command] @@ -617,7 +613,8 @@ pub(crate) async fn set_agents_core_settings( return serde_json::from_value(response).map_err(|err| err.to_string()); } - agents_config_core::set_agents_core_settings_core(input) + let app_settings = state.app_settings.lock().await.clone(); + agents_config_core::set_agents_core_settings_core(&app_settings, input) } #[tauri::command] @@ -633,7 +630,8 @@ pub(crate) async fn create_agent( return serde_json::from_value(response).map_err(|err| err.to_string()); } - agents_config_core::create_agent_core(input) + let app_settings = state.app_settings.lock().await.clone(); + agents_config_core::create_agent_core(&app_settings, input) } #[tauri::command] @@ -649,7 +647,8 @@ pub(crate) async fn update_agent( return serde_json::from_value(response).map_err(|err| err.to_string()); } - agents_config_core::update_agent_core(input) + let app_settings = state.app_settings.lock().await.clone(); + agents_config_core::update_agent_core(&app_settings, input) } #[tauri::command] @@ -665,7 +664,8 @@ pub(crate) async fn delete_agent( return serde_json::from_value(response).map_err(|err| err.to_string()); } - agents_config_core::delete_agent_core(input) + let app_settings = state.app_settings.lock().await.clone(); + agents_config_core::delete_agent_core(&app_settings, input) } #[tauri::command] @@ -685,7 +685,8 @@ pub(crate) async fn read_agent_config_toml( return serde_json::from_value(response).map_err(|err| err.to_string()); } - agents_config_core::read_agent_config_toml_core(agent_name.as_str()) + let app_settings = state.app_settings.lock().await.clone(); + agents_config_core::read_agent_config_toml_core(&app_settings, agent_name.as_str()) } #[tauri::command] @@ -709,7 +710,12 @@ pub(crate) async fn write_agent_config_toml( return Ok(()); } - agents_config_core::write_agent_config_toml_core(agent_name.as_str(), content.as_str()) + let app_settings = state.app_settings.lock().await.clone(); + agents_config_core::write_agent_config_toml_core( + &app_settings, + agent_name.as_str(), + content.as_str(), + ) } #[tauri::command] @@ -747,7 +753,13 @@ pub(crate) async fn account_read( .await; } - codex_core::account_read_core(&state.sessions, &state.workspaces, workspace_id).await + codex_core::account_read_core( + &state.app_settings, + &state.sessions, + &state.workspaces, + workspace_id, + ) + .await } #[tauri::command] @@ -864,7 +876,13 @@ pub(crate) async fn remember_approval_rule( command: Vec, state: State<'_, AppState>, ) -> Result { - codex_core::remember_approval_rule_core(&state.workspaces, workspace_id, command).await + codex_core::remember_approval_rule_core( + &state.app_settings, + &state.workspaces, + workspace_id, + command, + ) + .await } #[tauri::command] @@ -883,7 +901,7 @@ pub(crate) async fn get_config_model( .await; } - codex_core::get_config_model_core(&state.workspaces, workspace_id).await + codex_core::get_config_model_core(&state.app_settings, &state.workspaces, workspace_id).await } /// Generates a commit message in the background without showing in the main chat diff --git a/src-tauri/src/files/io.rs b/src-tauri/src/files/io.rs index f802c22e6c..e1868f06a4 100644 --- a/src-tauri/src/files/io.rs +++ b/src-tauri/src/files/io.rs @@ -1,6 +1,6 @@ use std::fs::File; -use std::io::Read; -use std::path::{Path, PathBuf}; +use std::io::{ErrorKind, Read}; +use std::path::{Component, Path, PathBuf}; use serde::{Deserialize, Serialize}; @@ -24,8 +24,14 @@ fn resolve_root( root_context: &str, root_may_be_missing: bool, ) -> Result, String> { - if root_may_be_missing && !root.exists() { - return Ok(None); + // `exists()` follows symlinks and hides both dangling links and permission + // errors. Only a genuinely absent root may use the missing-file behavior. + match std::fs::symlink_metadata(root) { + Err(err) if root_may_be_missing && err.kind() == ErrorKind::NotFound => { + return Ok(None); + } + Err(err) => return Err(format!("Failed to resolve {root_context}: {err}")), + Ok(_) => {} } let canonical_root = root .canonicalize() @@ -36,6 +42,37 @@ fn resolve_root( Ok(Some(canonical_root)) } +fn file_candidate(root: &Path, filename: &str, file_context: &str) -> Result { + let mut components = Path::new(filename).components(); + if !matches!(components.next(), Some(Component::Normal(_))) || components.next().is_some() { + return Err(format!("Invalid {file_context} path")); + } + Ok(root.join(filename)) +} + +fn resolve_existing_file( + canonical_root: &Path, + candidate: &Path, + file_context: &str, + allow_external_symlink_target: bool, +) -> Result, String> { + let metadata = match std::fs::symlink_metadata(candidate) { + Ok(metadata) => metadata, + Err(err) if err.kind() == ErrorKind::NotFound => return Ok(None), + Err(err) => return Err(format!("Failed to resolve {file_context}: {err}")), + }; + // An existing dangling symlink is an error, never a new file to create. + let canonical_path = candidate + .canonicalize() + .map_err(|err| format!("Failed to resolve {file_context}: {err}"))?; + if !canonical_path.starts_with(canonical_root) + && !(allow_external_symlink_target && metadata.file_type().is_symlink()) + { + return Err(format!("Invalid {file_context} path")); + } + Ok(Some(canonical_path)) +} + fn resolve_or_create_root(root: &Path, root_context: &str) -> Result { std::fs::create_dir_all(root) .map_err(|err| format!("Failed to create {root_context}: {err}"))?; @@ -60,23 +97,16 @@ pub(crate) fn read_text_file_within( return Ok(missing_response()); }; - let candidate = canonical_root.join(filename); - if !candidate.exists() { + let candidate = file_candidate(&canonical_root, filename, file_context)?; + let Some(canonical_path) = resolve_existing_file( + &canonical_root, + &candidate, + file_context, + allow_external_symlink_target, + )? + else { return Ok(missing_response()); - } - - let candidate_is_symlink = std::fs::symlink_metadata(&candidate) - .map_err(|err| format!("Failed to open {file_context}: {err}"))? - .file_type() - .is_symlink(); - let canonical_path = candidate - .canonicalize() - .map_err(|err| format!("Failed to open {file_context}: {err}"))?; - if !canonical_path.starts_with(&canonical_root) - && !(allow_external_symlink_target && candidate_is_symlink) - { - return Err(format!("Invalid {file_context} path")); - } + }; let mut file = File::open(&canonical_path) .map_err(|err| format!("Failed to open {file_context}: {err}"))?; @@ -109,28 +139,14 @@ pub(crate) fn write_text_file_within( .ok_or_else(|| format!("Failed to resolve {root_context}"))? }; - let candidate = canonical_root.join(filename); - if !candidate.starts_with(&canonical_root) { - return Err(format!("Invalid {file_context} path")); - } - - let target_path = if candidate.exists() { - let candidate_is_symlink = std::fs::symlink_metadata(&candidate) - .map_err(|err| format!("Failed to resolve {file_context}: {err}"))? - .file_type() - .is_symlink(); - let canonical_path = candidate - .canonicalize() - .map_err(|err| format!("Failed to resolve {file_context}: {err}"))?; - if !canonical_path.starts_with(&canonical_root) - && !(allow_external_symlink_target && candidate_is_symlink) - { - return Err(format!("Invalid {file_context} path")); - } - canonical_path - } else { - candidate - }; + let candidate = file_candidate(&canonical_root, filename, file_context)?; + let target_path = resolve_existing_file( + &canonical_root, + &candidate, + file_context, + allow_external_symlink_target, + )? + .unwrap_or(candidate); std::fs::write(&target_path, content) .map_err(|err| format!("Failed to write {file_context}: {err}")) @@ -314,4 +330,137 @@ mod tests { .expect_err("should reject symlink escape"); assert!(error.contains("Invalid config.toml path")); } + + #[cfg(unix)] + #[test] + fn selected_home_policy_allows_external_agents_symlink_and_rejects_other_escapes() { + use crate::files::ops::{read_with_policy, write_with_policy}; + use crate::files::policy::{policy_for, FileKind, FileScope}; + use std::os::unix::fs::symlink; + + let fixture = temp_dir(); + let actual_home = fixture.join("mounted-home"); + std::fs::create_dir_all(&actual_home).expect("create home"); + let selected_home = fixture.join("selected-home"); + symlink(&actual_home, &selected_home).expect("link home"); + + let agents_policy = policy_for(FileScope::Global, FileKind::Agents).expect("agents policy"); + let outside_agents = fixture.join("outside-AGENTS.md"); + std::fs::write(&outside_agents, "outside agents").expect("seed external agents"); + let agents_link = actual_home.join(agents_policy.filename); + symlink(&outside_agents, &agents_link).expect("link external agents"); + let response = read_with_policy(&selected_home, agents_policy) + .expect("read external global agents link"); + assert_eq!(response.content, "outside agents"); + write_with_policy(&selected_home, agents_policy, "updated agents") + .expect("write external global agents link"); + assert_eq!( + std::fs::read_to_string(&outside_agents).expect("read updated agents"), + "updated agents" + ); + + let config_policy = policy_for(FileScope::Global, FileKind::Config).expect("config policy"); + let outside_config = fixture.join("outside-config.toml"); + std::fs::write(&outside_config, "outside config").expect("seed external config"); + let config_link = actual_home.join(config_policy.filename); + symlink(&outside_config, &config_link).expect("link external config"); + assert!(read_with_policy(&selected_home, config_policy).is_err()); + assert!(write_with_policy(&selected_home, config_policy, "replaced config").is_err()); + assert_eq!( + std::fs::read_to_string(&outside_config).expect("read unchanged config"), + "outside config" + ); + + for (policy, target, child) in [ + (agents_policy, &outside_agents, &agents_link), + (config_policy, &outside_config, &config_link), + ] { + std::fs::remove_file(target).expect("make child link dangling"); + assert!(read_with_policy(&selected_home, policy).is_err()); + assert!(write_with_policy(&selected_home, policy, "must not create").is_err()); + assert!(!target.exists()); + std::fs::remove_file(child).expect("remove child link"); + + write_with_policy(&selected_home, policy, "inside").expect("write ordinary child"); + let response = read_with_policy(&selected_home, policy).expect("read ordinary child"); + assert_eq!(response.content, "inside"); + } + std::fs::remove_dir_all(fixture).expect("remove fixture"); + } + + #[cfg(unix)] + #[test] + fn workspace_agents_policy_rejects_external_symlink() { + use crate::files::ops::{read_with_policy, write_with_policy}; + use crate::files::policy::{policy_for, FileKind, FileScope}; + use std::os::unix::fs::symlink; + + let fixture = temp_dir(); + let workspace = fixture.join("workspace"); + std::fs::create_dir_all(&workspace).expect("create workspace"); + let outside_agents = fixture.join("outside-AGENTS.md"); + std::fs::write(&outside_agents, "outside").expect("seed external agents"); + symlink(&outside_agents, workspace.join("AGENTS.md")).expect("link external agents"); + + let policy = policy_for(FileScope::Workspace, FileKind::Agents).expect("workspace policy"); + assert!(read_with_policy(&workspace, policy).is_err()); + assert!(write_with_policy(&workspace, policy, "replaced").is_err()); + assert_eq!( + std::fs::read_to_string(&outside_agents).expect("read unchanged agents"), + "outside" + ); + + std::fs::remove_dir_all(fixture).expect("remove fixture"); + } + + #[cfg(unix)] + #[test] + fn dangling_root_is_not_reported_as_a_missing_global_file() { + use std::os::unix::fs::symlink; + + let fixture = temp_dir(); + std::fs::create_dir_all(&fixture).expect("create fixture"); + let missing_target = fixture.join("unmounted/home"); + let root = fixture.join("home-link"); + symlink(&missing_target, &root).expect("link unavailable home"); + assert!( + read_text_file_within(&root, "AGENTS.md", true, "CODEX_HOME", "AGENTS.md", false) + .is_err() + ); + assert!(write_text_file_within( + &root, + "AGENTS.md", + "blocked", + true, + "CODEX_HOME", + "AGENTS.md", + false + ) + .is_err()); + assert!(!missing_target.exists()); + std::fs::remove_dir_all(fixture).expect("remove fixture"); + } + + #[test] + fn file_names_cannot_escape_root_or_traverse_subdirectories() { + let root = temp_dir(); + std::fs::create_dir_all(&root).expect("create root"); + for filename in ["../AGENTS.md", "/tmp/AGENTS.md", "nested/AGENTS.md"] { + assert!( + read_text_file_within(&root, filename, false, "workspace", "AGENTS.md", false) + .is_err() + ); + assert!(write_text_file_within( + &root, + filename, + "blocked", + false, + "workspace", + "AGENTS.md", + false + ) + .is_err()); + } + std::fs::remove_dir_all(root).expect("remove fixture"); + } } diff --git a/src-tauri/src/files/mod.rs b/src-tauri/src/files/mod.rs index 5f40ecb2de..ce94abf17a 100644 --- a/src-tauri/src/files/mod.rs +++ b/src-tauri/src/files/mod.rs @@ -31,7 +31,14 @@ async fn file_read_impl( return serde_json::from_value(response).map_err(|err| err.to_string()); } - file_read_core(&state.workspaces, scope, kind, workspace_id).await + file_read_core( + &state.workspaces, + &state.app_settings, + scope, + kind, + workspace_id, + ) + .await } async fn file_write_impl( @@ -58,7 +65,15 @@ async fn file_write_impl( return Ok(()); } - file_write_core(&state.workspaces, scope, kind, workspace_id, content).await + file_write_core( + &state.workspaces, + &state.app_settings, + scope, + kind, + workspace_id, + content, + ) + .await } #[tauri::command] @@ -98,7 +113,10 @@ pub(crate) async fn read_image_as_data_url( let mobile_runtime = cfg!(any(target_os = "ios", target_os = "android")); let remote_mode = remote_backend::is_remote_mode(&*state).await; if !mobile_runtime && !remote_mode { - return Err("Image conversion is only supported in remote backend mode or on mobile runtimes".to_string()); + return Err( + "Image conversion is only supported in remote backend mode or on mobile runtimes" + .to_string(), + ); } let normalized = codex_core::normalize_file_path(trimmed_path); diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 83e9dacae7..e709167cc4 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -183,6 +183,8 @@ pub fn run() { settings::get_app_settings, settings::update_app_settings, settings::get_codex_config_path, + settings::get_codex_home, + settings::validate_codex_home, files::file_read, files::file_write, files::read_image_as_data_url, diff --git a/src-tauri/src/local_usage.rs b/src-tauri/src/local_usage.rs index 94359183c7..774d34998a 100644 --- a/src-tauri/src/local_usage.rs +++ b/src-tauri/src/local_usage.rs @@ -24,5 +24,12 @@ pub(crate) async fn local_usage_snapshot( return serde_json::from_value(response).map_err(|err| err.to_string()); } - local_usage_core::local_usage_snapshot_core(&state.workspaces, days, workspace_path).await + let app_settings = state.app_settings.lock().await.clone(); + local_usage_core::local_usage_snapshot_core( + &app_settings, + &state.workspaces, + days, + workspace_path, + ) + .await } diff --git a/src-tauri/src/prompts.rs b/src-tauri/src/prompts.rs index a535c05a69..8c169c1ade 100644 --- a/src-tauri/src/prompts.rs +++ b/src-tauri/src/prompts.rs @@ -8,7 +8,14 @@ pub(crate) async fn prompts_list( state: State<'_, AppState>, workspace_id: String, ) -> Result, String> { - prompts_core::prompts_list_core(&state.workspaces, &state.settings_path, workspace_id).await + let app_settings = state.app_settings.lock().await.clone(); + prompts_core::prompts_list_core( + &app_settings, + &state.workspaces, + &state.settings_path, + workspace_id, + ) + .await } #[tauri::command] @@ -25,7 +32,8 @@ pub(crate) async fn prompts_global_dir( state: State<'_, AppState>, workspace_id: String, ) -> Result { - prompts_core::prompts_global_dir_core(&state.workspaces, workspace_id).await + let app_settings = state.app_settings.lock().await.clone(); + prompts_core::prompts_global_dir_core(&app_settings, &state.workspaces, workspace_id).await } #[tauri::command] @@ -38,7 +46,9 @@ pub(crate) async fn prompts_create( argument_hint: Option, content: String, ) -> Result { + let app_settings = state.app_settings.lock().await.clone(); prompts_core::prompts_create_core( + &app_settings, &state.workspaces, &state.settings_path, workspace_id, @@ -61,7 +71,9 @@ pub(crate) async fn prompts_update( argument_hint: Option, content: String, ) -> Result { + let app_settings = state.app_settings.lock().await.clone(); prompts_core::prompts_update_core( + &app_settings, &state.workspaces, &state.settings_path, workspace_id, @@ -80,8 +92,15 @@ pub(crate) async fn prompts_delete( workspace_id: String, path: String, ) -> Result<(), String> { - prompts_core::prompts_delete_core(&state.workspaces, &state.settings_path, workspace_id, path) - .await + let app_settings = state.app_settings.lock().await.clone(); + prompts_core::prompts_delete_core( + &app_settings, + &state.workspaces, + &state.settings_path, + workspace_id, + path, + ) + .await } #[tauri::command] @@ -91,7 +110,9 @@ pub(crate) async fn prompts_move( path: String, scope: String, ) -> Result { + let app_settings = state.app_settings.lock().await.clone(); prompts_core::prompts_move_core( + &app_settings, &state.workspaces, &state.settings_path, workspace_id, diff --git a/src-tauri/src/settings/mod.rs b/src-tauri/src/settings/mod.rs index 0277feb879..117e3792dc 100644 --- a/src-tauri/src/settings/mod.rs +++ b/src-tauri/src/settings/mod.rs @@ -1,8 +1,11 @@ -use tauri::{State, Window}; +use tauri::{AppHandle, State, Window}; +use crate::codex::home::{resolve_codex_home_status, CodexHomeStatus}; use crate::shared::settings_core::{ - get_app_settings_core, get_codex_config_path_core, update_app_settings_core, + get_app_settings_core, get_codex_config_path_core, get_codex_home_core, + update_app_settings_core, }; +use crate::shared::workspaces_core::{disconnect_all_sessions_core, workspace_session_spawn_lock}; use crate::state::AppState; use crate::types::{AppSettings, BackendMode}; use crate::window; @@ -23,9 +26,13 @@ pub(crate) async fn update_app_settings( state: State<'_, AppState>, window: Window, ) -> Result { + let _spawn_guard = workspace_session_spawn_lock().lock().await; let previous = state.app_settings.lock().await.clone(); let updated = update_app_settings_core(settings, &state.app_settings, &state.settings_path).await?; + if previous.codex_home != updated.codex_home { + disconnect_all_sessions_core(&state.sessions).await; + } if should_reset_remote_backend(&previous, &updated) { *state.remote_backend.lock().await = None; } @@ -35,8 +42,33 @@ pub(crate) async fn update_app_settings( } #[tauri::command] -pub(crate) async fn get_codex_config_path() -> Result { - get_codex_config_path_core() +pub(crate) async fn get_codex_config_path( + state: State<'_, AppState>, + app: AppHandle, +) -> Result { + if crate::remote_backend::is_remote_mode(&state).await { + let response = crate::remote_backend::call_remote( + &state, + app, + "get_codex_config_path", + serde_json::json!({}), + ) + .await?; + return serde_json::from_value(response).map_err(|error| error.to_string()); + } + get_codex_config_path_core(&state.app_settings).await +} + +#[tauri::command] +pub(crate) async fn get_codex_home(state: State<'_, AppState>) -> Result { + get_codex_home_core(&state.app_settings).await +} + +#[tauri::command] +pub(crate) async fn validate_codex_home( + codex_home: Option, +) -> Result { + resolve_codex_home_status(codex_home.as_deref()) } fn should_reset_remote_backend(previous: &AppSettings, updated: &AppSettings) -> bool { diff --git a/src-tauri/src/shared/agents_config_core.rs b/src-tauri/src/shared/agents_config_core.rs index b8b448a5ab..106fcb37dd 100644 --- a/src-tauri/src/shared/agents_config_core.rs +++ b/src-tauri/src/shared/agents_config_core.rs @@ -4,8 +4,9 @@ use std::io::ErrorKind; use std::path::{Component, Path, PathBuf}; use toml_edit::{value, Document, Item, Table}; -use crate::codex::home as codex_home; +use crate::codex::home; use crate::shared::config_toml_core; +use crate::types::AppSettings; pub(crate) const DEFAULT_AGENT_MAX_THREADS: u32 = 6; pub(crate) const DEFAULT_AGENT_MAX_DEPTH: u32 = 1; @@ -81,8 +82,10 @@ pub(crate) struct DeleteAgentInput { pub delete_managed_file: Option, } -pub(crate) fn get_agents_settings_core() -> Result { - let codex_home = resolve_codex_home()?; +pub(crate) fn get_agents_settings_core( + app_settings: &AppSettings, +) -> Result { + let codex_home = home::resolve_codex_home(app_settings.codex_home.as_deref())?; let config_path = codex_home.join("config.toml"); let config_path_string = config_path .to_str() @@ -106,12 +109,13 @@ pub(crate) fn get_agents_settings_core() -> Result { } pub(crate) fn set_agents_core_settings_core( + app_settings: &AppSettings, input: SetAgentsCoreInput, ) -> Result { validate_max_threads(input.max_threads)?; validate_max_depth(input.max_depth)?; - let codex_home = resolve_codex_home()?; + let codex_home = home::resolve_codex_home(app_settings.codex_home.as_deref())?; let (_, mut document) = config_toml_core::load_global_config_document(&codex_home)?; let features = config_toml_core::ensure_table(&mut document, "features")?; @@ -122,15 +126,18 @@ pub(crate) fn set_agents_core_settings_core( agents["max_depth"] = value(input.max_depth as i64); config_toml_core::persist_global_config_document(&codex_home, &document)?; - get_agents_settings_core() + get_agents_settings_core(app_settings) } -pub(crate) fn create_agent_core(input: CreateAgentInput) -> Result { +pub(crate) fn create_agent_core( + app_settings: &AppSettings, + input: CreateAgentInput, +) -> Result { let name = normalize_agent_name(input.name.as_str())?; let description = normalize_optional_string(input.description.as_deref()); let developer_instructions = normalize_optional_string(input.developer_instructions.as_deref()); - let codex_home = resolve_codex_home()?; + let codex_home = home::resolve_codex_home(app_settings.codex_home.as_deref())?; let (_, mut document) = config_toml_core::load_global_config_document(&codex_home)?; { @@ -172,10 +179,13 @@ pub(crate) fn create_agent_core(input: CreateAgentInput) -> Result Result { +pub(crate) fn update_agent_core( + app_settings: &AppSettings, + input: UpdateAgentInput, +) -> Result { let original_name = normalize_agent_lookup_name(input.original_name.as_str())?; let name = normalize_agent_name(input.name.as_str())?; let description = normalize_optional_string(input.description.as_deref()); @@ -183,7 +193,7 @@ pub(crate) fn update_agent_core(input: UpdateAgentInput) -> Result = None; @@ -310,14 +320,17 @@ pub(crate) fn update_agent_core(input: UpdateAgentInput) -> Result Result { +pub(crate) fn delete_agent_core( + app_settings: &AppSettings, + input: DeleteAgentInput, +) -> Result { let name = normalize_agent_lookup_name(input.name.as_str())?; let delete_managed_file = input.delete_managed_file.unwrap_or(false); - let codex_home = resolve_codex_home()?; + let codex_home = home::resolve_codex_home(app_settings.codex_home.as_deref())?; let (_, mut document) = config_toml_core::load_global_config_document(&codex_home)?; let removed_config_file = { @@ -361,11 +374,15 @@ pub(crate) fn delete_agent_core(input: DeleteAgentInput) -> Result Result { - let (codex_home, relative_path) = resolve_managed_agent_config_relative_path(agent_name)?; +pub(crate) fn read_agent_config_toml_core( + app_settings: &AppSettings, + agent_name: &str, +) -> Result { + let (codex_home, relative_path) = + resolve_managed_agent_config_relative_path(app_settings, agent_name)?; let path = resolve_safe_managed_abs_path_for_read(&codex_home, &relative_path)?; if !path.exists() { return Ok(String::new()); @@ -373,17 +390,17 @@ pub(crate) fn read_agent_config_toml_core(agent_name: &str) -> Result Result<(), String> { - let (codex_home, relative_path) = resolve_managed_agent_config_relative_path(agent_name)?; +pub(crate) fn write_agent_config_toml_core( + app_settings: &AppSettings, + agent_name: &str, + content: &str, +) -> Result<(), String> { + let (codex_home, relative_path) = + resolve_managed_agent_config_relative_path(app_settings, agent_name)?; let path = resolve_safe_managed_abs_path_for_write(&codex_home, &relative_path)?; std::fs::write(path, content).map_err(|err| format!("Failed to write agent config file: {err}")) } -fn resolve_codex_home() -> Result { - codex_home::resolve_default_codex_home() - .ok_or_else(|| "Unable to resolve CODEX_HOME".to_string()) -} - fn read_multi_agent_enabled(document: &Document) -> bool { read_feature_flag(document, "multi_agent").unwrap_or(false) } @@ -659,10 +676,11 @@ fn managed_relative_path_from_config(raw_path: &str) -> Option { } fn resolve_managed_agent_config_relative_path( + app_settings: &AppSettings, agent_name: &str, ) -> Result<(PathBuf, PathBuf), String> { let name = normalize_agent_lookup_name(agent_name)?; - let codex_home = resolve_codex_home()?; + let codex_home = home::resolve_codex_home(app_settings.codex_home.as_deref())?; let (_, document) = config_toml_core::load_global_config_document(&codex_home)?; let agents_table = document @@ -859,6 +877,58 @@ mod tests { dir } + #[cfg(unix)] + #[test] + fn selected_symlink_home_supports_agent_files_but_rejects_file_escape() { + use std::os::unix::fs::symlink; + let fixture = temp_dir("selected-agent-home"); + let home = fixture.join("volume-home"); + std::fs::create_dir_all(home.join("agents")).unwrap(); + std::fs::write( + home.join("config.toml"), + "[agents.fixture]\nconfig_file = \"agents/fixture.toml\"\n", + ) + .unwrap(); + std::fs::write( + home.join("agents/fixture.toml"), + "model = \"fixture-model\"\n", + ) + .unwrap(); + let selected = fixture.join("selected-home"); + symlink(&home, &selected).unwrap(); + let settings = AppSettings { + codex_home: Some(selected.to_string_lossy().into_owned()), + ..AppSettings::default() + }; + let resolved = get_agents_settings_core(&settings).expect("selected agent settings"); + assert_eq!( + resolved.config_path, + std::fs::canonicalize(&home) + .unwrap() + .join("config.toml") + .to_string_lossy() + ); + assert!(read_agent_config_toml_core(&settings, "fixture") + .unwrap() + .contains("fixture-model")); + write_agent_config_toml_core(&settings, "fixture", "model = \"updated\"\n").unwrap(); + assert!(std::fs::read_to_string(home.join("agents/fixture.toml")) + .unwrap() + .contains("updated")); + let outside = fixture.join("unrelated.toml"); + std::fs::write(&outside, "model = \"unrelated\"\n").unwrap(); + std::fs::remove_file(home.join("agents/fixture.toml")).unwrap(); + symlink(&outside, home.join("agents/fixture.toml")).unwrap(); + assert!(read_agent_config_toml_core(&settings, "fixture").is_err()); + assert!(write_agent_config_toml_core(&settings, "fixture", "changed").is_err()); + assert!(std::fs::read_to_string(&outside) + .unwrap() + .contains("unrelated")); + std::fs::remove_dir_all(&home).unwrap(); + assert!(get_agents_settings_core(&settings).is_err()); + std::fs::remove_dir_all(fixture).unwrap(); + } + #[test] fn normalize_agent_name_accepts_expected_shape() { assert_eq!(normalize_agent_name("explorer").expect("valid"), "explorer"); diff --git a/src-tauri/src/shared/codex_aux_core.rs b/src-tauri/src/shared/codex_aux_core.rs index 12f985adec..14ccb6d7f9 100644 --- a/src-tauri/src/shared/codex_aux_core.rs +++ b/src-tauri/src/shared/codex_aux_core.rs @@ -291,9 +291,13 @@ pub(crate) async fn codex_doctor_core( codex_bin: Option, codex_args: Option, ) -> Result { - let (default_bin, default_args) = { + let (default_bin, default_args, codex_home) = { let settings = app_settings.lock().await; - (settings.codex_bin.clone(), settings.codex_args.clone()) + ( + settings.codex_bin.clone(), + settings.codex_args.clone(), + crate::codex::home::resolve_codex_home(settings.codex_home.as_deref())?, + ) }; let resolved = codex_bin .clone() @@ -304,12 +308,13 @@ pub(crate) async fn codex_doctor_core( .filter(|value| !value.trim().is_empty()) .or(default_args); let path_env = build_codex_path_env(resolved.as_deref()); - let version = check_codex_installation(resolved.clone()).await?; + let version = check_codex_installation(resolved.clone(), Some(&codex_home)).await?; let mut command = build_codex_command_with_bin( resolved.clone(), resolved_args.as_deref(), vec!["app-server".to_string(), "--help".to_string()], )?; + command.env("CODEX_HOME", &codex_home); command.stdout(std::process::Stdio::piped()); command.stderr(std::process::Stdio::piped()); let app_server_ok = match timeout(Duration::from_secs(5), command.output()).await { diff --git a/src-tauri/src/shared/codex_core.rs b/src-tauri/src/shared/codex_core.rs index a0f2a4ea6a..e9e893c1cb 100644 --- a/src-tauri/src/shared/codex_core.rs +++ b/src-tauri/src/shared/codex_core.rs @@ -13,10 +13,10 @@ use tokio::time::Instant; use crate::backend::app_server::WorkspaceSession; use crate::codex::config as codex_config; -use crate::codex::home::{resolve_default_codex_home, resolve_workspace_codex_home}; +use crate::codex::home::resolve_workspace_codex_home; use crate::rules; use crate::shared::account::{build_account_response, read_auth_account}; -use crate::types::WorkspaceEntry; +use crate::types::{AppSettings, WorkspaceEntry}; const LOGIN_START_TIMEOUT: Duration = Duration::from_secs(30); #[allow(dead_code)] @@ -228,13 +228,12 @@ async fn resolve_workspace_and_parent( } async fn resolve_codex_home_for_workspace_core( + app_settings: &Mutex, workspaces: &Mutex>, workspace_id: &str, ) -> Result { let (entry, parent_entry) = resolve_workspace_and_parent(workspaces, workspace_id).await?; - resolve_workspace_codex_home(&entry, parent_entry.as_ref()) - .or_else(resolve_default_codex_home) - .ok_or_else(|| "Unable to resolve CODEX_HOME".to_string()) + resolve_workspace_codex_home(&entry, parent_entry.as_ref(), &*app_settings.lock().await) } async fn resolve_workspace_path_core( @@ -625,6 +624,7 @@ pub(crate) async fn account_rate_limits_core( } pub(crate) async fn account_read_core( + app_settings: &Mutex, sessions: &Mutex>>, workspaces: &Mutex>, workspace_id: String, @@ -643,8 +643,11 @@ pub(crate) async fn account_read_core( }; let (entry, parent_entry) = resolve_workspace_and_parent(workspaces, &workspace_id).await?; - let codex_home = resolve_workspace_codex_home(&entry, parent_entry.as_ref()) - .or_else(resolve_default_codex_home); + let codex_home = Some(resolve_workspace_codex_home( + &entry, + parent_entry.as_ref(), + &*app_settings.lock().await, + )?); let fallback = read_auth_account(codex_home); Ok(build_account_response(response, fallback)) @@ -856,6 +859,7 @@ pub(crate) async fn respond_to_server_request_core( } pub(crate) async fn remember_approval_rule_core( + app_settings: &Mutex, workspaces: &Mutex>, workspace_id: String, command: Vec, @@ -869,7 +873,8 @@ pub(crate) async fn remember_approval_rule_core( return Err("empty command".to_string()); } - let codex_home = resolve_codex_home_for_workspace_core(workspaces, &workspace_id).await?; + let codex_home = + resolve_codex_home_for_workspace_core(app_settings, workspaces, &workspace_id).await?; let rules_path = rules::default_rules_path(&codex_home); rules::append_prefix_rule(&rules_path, &command)?; @@ -880,10 +885,12 @@ pub(crate) async fn remember_approval_rule_core( } pub(crate) async fn get_config_model_core( + app_settings: &Mutex, workspaces: &Mutex>, workspace_id: String, ) -> Result { - let codex_home = resolve_codex_home_for_workspace_core(workspaces, &workspace_id).await?; + let codex_home = + resolve_codex_home_for_workspace_core(app_settings, workspaces, &workspace_id).await?; let model = codex_config::read_config_model(Some(codex_home))?; Ok(json!({ "model": model })) } diff --git a/src-tauri/src/shared/codex_update_core.rs b/src-tauri/src/shared/codex_update_core.rs index ee5541a25e..32fb9c3962 100644 --- a/src-tauri/src/shared/codex_update_core.rs +++ b/src-tauri/src/shared/codex_update_core.rs @@ -148,9 +148,13 @@ pub(crate) async fn codex_update_core( codex_bin: Option, codex_args: Option, ) -> Result { - let (default_bin, default_args) = { + let (default_bin, default_args, codex_home) = { let settings = app_settings.lock().await; - (settings.codex_bin.clone(), settings.codex_args.clone()) + ( + settings.codex_bin.clone(), + settings.codex_args.clone(), + crate::codex::home::resolve_codex_home(settings.codex_home.as_deref())?, + ) }; let resolved = codex_bin .clone() @@ -162,7 +166,7 @@ pub(crate) async fn codex_update_core( .or(default_args); let _ = resolved_args; - let before_version = check_codex_installation(resolved.clone()) + let before_version = check_codex_installation(resolved.clone(), Some(&codex_home)) .await .ok() .flatten(); @@ -203,7 +207,7 @@ pub(crate) async fn codex_update_core( let after_version = if method == "unknown" { None } else { - match check_codex_installation(resolved.clone()).await { + match check_codex_installation(resolved.clone(), Some(&codex_home)).await { Ok(version) => version, Err(err) => { let result = CodexUpdateResult { diff --git a/src-tauri/src/shared/config_toml_core.rs b/src-tauri/src/shared/config_toml_core.rs index cb8d0ffd04..31cfdc3b37 100644 --- a/src-tauri/src/shared/config_toml_core.rs +++ b/src-tauri/src/shared/config_toml_core.rs @@ -91,3 +91,94 @@ pub(crate) fn set_top_level_string(document: &mut Document, key: &str, value_raw } document[key] = value(trimmed); } + +#[cfg(all(test, unix))] +mod tests { + use super::*; + use std::fs; + use std::os::unix::fs::symlink; + use std::path::PathBuf; + use uuid::Uuid; + + struct Fixture(PathBuf); + + impl Fixture { + fn new() -> Self { + let root = std::env::temp_dir().join(format!("codex-config-home-{}", Uuid::new_v4())); + fs::create_dir(&root).expect("create fixture"); + Self(root) + } + } + + impl Drop for Fixture { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.0); + } + } + + #[test] + fn config_document_round_trips_through_symlinked_home() { + let fixture = Fixture::new(); + let resolved = fixture.0.join("mounted-home"); + fs::create_dir(&resolved).expect("create home"); + fs::write( + resolved.join("config.toml"), + "model = \"custom-provider-model\"\n", + ) + .expect("seed config"); + let home = fixture.0.join("selected-home"); + symlink(&resolved, &home).expect("link home"); + + let (exists, mut document) = load_global_config_document(&home).expect("load config"); + assert!(exists); + assert_eq!( + read_top_level_string(&document, "model").as_deref(), + Some("custom-provider-model") + ); + set_feature_flag(&mut document, "multi_agent", true).expect("set feature"); + persist_global_config_document(&home, &document).expect("save config"); + assert!(fs::symlink_metadata(&home) + .expect("home metadata") + .file_type() + .is_symlink()); + let (_, reread) = load_global_config_document(&home).expect("reload config"); + assert_eq!(read_feature_flag(&reread, "multi_agent"), Some(true)); + } + + #[test] + fn config_document_rejects_external_file_symlink_under_symlinked_home() { + let fixture = Fixture::new(); + let resolved = fixture.0.join("mounted-home"); + fs::create_dir(&resolved).expect("create home"); + let outside = fixture.0.join("outside.toml"); + fs::write(&outside, "model = \"outside\"\n").expect("seed external file"); + symlink(&outside, resolved.join("config.toml")).expect("link config"); + let home = fixture.0.join("selected-home"); + symlink(&resolved, &home).expect("link home"); + + assert!(load_global_config_document(&home) + .unwrap_err() + .contains("Invalid config.toml path")); + assert!(persist_global_config_document(&home, &Document::new()) + .unwrap_err() + .contains("Invalid config.toml path")); + assert_eq!( + fs::read_to_string(outside).expect("read external file"), + "model = \"outside\"\n" + ); + } + + #[test] + fn unavailable_symlinked_home_is_an_error() { + let fixture = Fixture::new(); + let unavailable = fixture.0.join("unmounted-volume/home"); + let home = fixture.0.join("selected-home"); + symlink(&unavailable, &home).expect("link unavailable home"); + + assert!(load_global_config_document(&home) + .unwrap_err() + .contains("CODEX_HOME")); + assert!(persist_global_config_document(&home, &Document::new()).is_err()); + assert!(!unavailable.exists()); + } +} diff --git a/src-tauri/src/shared/files_core.rs b/src-tauri/src/shared/files_core.rs index ef56181222..2cfddc42bb 100644 --- a/src-tauri/src/shared/files_core.rs +++ b/src-tauri/src/shared/files_core.rs @@ -7,12 +7,7 @@ use crate::codex::home as codex_home; use crate::files::io::TextFileResponse; use crate::files::ops::{read_with_policy, write_with_policy}; use crate::files::policy::{policy_for, FileKind, FileScope}; -use crate::types::WorkspaceEntry; - -fn resolve_default_codex_home() -> Result { - codex_home::resolve_default_codex_home() - .ok_or_else(|| "Unable to resolve CODEX_HOME".to_string()) -} +use crate::types::{AppSettings, WorkspaceEntry}; async fn resolve_workspace_root( workspaces: &Mutex>, @@ -27,11 +22,15 @@ async fn resolve_workspace_root( pub(crate) async fn resolve_root_core( workspaces: &Mutex>, + app_settings: &Mutex, scope: FileScope, workspace_id: Option<&str>, ) -> Result { match scope { - FileScope::Global => resolve_default_codex_home(), + FileScope::Global => { + let settings = app_settings.lock().await; + codex_home::resolve_codex_home(settings.codex_home.as_deref()) + } FileScope::Workspace => { let workspace_id = workspace_id.ok_or_else(|| "workspaceId is required".to_string())?; resolve_workspace_root(workspaces, workspace_id).await @@ -41,23 +40,102 @@ pub(crate) async fn resolve_root_core( pub(crate) async fn file_read_core( workspaces: &Mutex>, + app_settings: &Mutex, scope: FileScope, kind: FileKind, workspace_id: Option, ) -> Result { let policy = policy_for(scope, kind)?; - let root = resolve_root_core(workspaces, scope, workspace_id.as_deref()).await?; + let root = resolve_root_core(workspaces, app_settings, scope, workspace_id.as_deref()).await?; read_with_policy(&root, policy) } pub(crate) async fn file_write_core( workspaces: &Mutex>, + app_settings: &Mutex, scope: FileScope, kind: FileKind, workspace_id: Option, content: String, ) -> Result<(), String> { let policy = policy_for(scope, kind)?; - let root = resolve_root_core(workspaces, scope, workspace_id.as_deref()).await?; + let root = resolve_root_core(workspaces, app_settings, scope, workspace_id.as_deref()).await?; write_with_policy(&root, policy, &content) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn global_file_operations_use_selected_home_and_reject_unavailable_selection() { + tokio::runtime::Builder::new_current_thread() + .build() + .unwrap() + .block_on(async { + let fixture = + std::env::temp_dir().join(format!("codex-home-files-{}", uuid::Uuid::new_v4())); + let selected = fixture.join("selected"); + std::fs::create_dir_all(&selected).unwrap(); + std::fs::write( + selected.join("config.toml"), + "model = \"fixture-provider\"\n", + ) + .unwrap(); + let settings = Mutex::new(AppSettings { + codex_home: Some(selected.to_string_lossy().into_owned()), + ..AppSettings::default() + }); + let workspaces = Mutex::new(HashMap::new()); + let config = file_read_core( + &workspaces, + &settings, + FileScope::Global, + FileKind::Config, + None, + ) + .await + .unwrap(); + assert_eq!(config.content, "model = \"fixture-provider\"\n"); + file_write_core( + &workspaces, + &settings, + FileScope::Global, + FileKind::Agents, + None, + "Selected-home instructions\n".to_string(), + ) + .await + .unwrap(); + assert_eq!( + std::fs::read_to_string(selected.join("AGENTS.md")).unwrap(), + "Selected-home instructions\n" + ); + settings.lock().await.codex_home = + Some(fixture.join("unavailable").to_string_lossy().into_owned()); + assert!(file_read_core( + &workspaces, + &settings, + FileScope::Global, + FileKind::Config, + None + ) + .await + .unwrap_err() + .contains("unavailable")); + assert!(file_write_core( + &workspaces, + &settings, + FileScope::Global, + FileKind::Agents, + None, + "must not write".to_string() + ) + .await + .unwrap_err() + .contains("unavailable")); + assert!(!fixture.join("unavailable").exists()); + std::fs::remove_dir_all(fixture).unwrap(); + }); + } +} diff --git a/src-tauri/src/shared/local_usage_core.rs b/src-tauri/src/shared/local_usage_core.rs index ca2f40871c..bb88cba2d7 100644 --- a/src-tauri/src/shared/local_usage_core.rs +++ b/src-tauri/src/shared/local_usage_core.rs @@ -7,9 +7,10 @@ use std::path::{Path, PathBuf}; use std::time::{SystemTime, UNIX_EPOCH}; use tokio::sync::Mutex; -use crate::codex::home::{resolve_default_codex_home, resolve_workspace_codex_home}; +use crate::codex::home::{resolve_codex_home, resolve_workspace_codex_home}; use crate::types::{ - LocalUsageDay, LocalUsageModel, LocalUsageSnapshot, LocalUsageTotals, WorkspaceEntry, + AppSettings, LocalUsageDay, LocalUsageModel, LocalUsageSnapshot, LocalUsageTotals, + WorkspaceEntry, }; #[derive(Default, Clone, Copy)] @@ -31,6 +32,7 @@ struct UsageTotals { const MAX_ACTIVITY_GAP_MS: i64 = 2 * 60 * 1000; pub(crate) async fn local_usage_snapshot_core( + app_settings: &AppSettings, workspaces: &Mutex>, days: Option, workspace_path: Option, @@ -46,7 +48,7 @@ pub(crate) async fn local_usage_snapshot_core( }); let sessions_roots = { let workspaces = workspaces.lock().await; - resolve_sessions_roots(&workspaces, workspace_path.as_deref()) + resolve_sessions_roots(app_settings, &workspaces, workspace_path.as_deref())? }; let snapshot = tokio::task::spawn_blocking(move || { scan_local_usage(days, workspace_path.as_deref(), &sessions_roots) @@ -519,70 +521,57 @@ fn make_day_keys(days: u32) -> Vec { .collect() } -fn resolve_codex_sessions_root(codex_home_override: Option) -> Option { - codex_home_override - .or_else(resolve_default_codex_home) - .map(|home| home.join("sessions")) -} - fn resolve_sessions_roots( + app_settings: &AppSettings, workspaces: &HashMap, workspace_path: Option<&Path>, -) -> Vec { +) -> Result, String> { if let Some(workspace_path) = workspace_path { - let codex_home_override = - resolve_workspace_codex_home_for_path(workspaces, Some(workspace_path)); - return resolve_codex_sessions_root(codex_home_override) - .into_iter() - .collect(); + let codex_home = + resolve_workspace_codex_home_for_path(app_settings, workspaces, workspace_path)?; + return Ok(vec![codex_home.join("sessions")]); } let mut roots = Vec::new(); let mut seen = HashSet::new(); - - if let Some(root) = resolve_codex_sessions_root(None) { - if seen.insert(root.clone()) { - roots.push(root); - } - } + let root = resolve_codex_home(app_settings.codex_home.as_deref())?.join("sessions"); + seen.insert(root.clone()); + roots.push(root); for entry in workspaces.values() { let parent_entry = entry .parent_id .as_ref() .and_then(|parent_id| workspaces.get(parent_id)); - let Some(codex_home) = resolve_workspace_codex_home(entry, parent_entry) else { - continue; - }; - if let Some(root) = resolve_codex_sessions_root(Some(codex_home)) { - if seen.insert(root.clone()) { - roots.push(root); - } + let root = + resolve_workspace_codex_home(entry, parent_entry, app_settings)?.join("sessions"); + if seen.insert(root.clone()) { + roots.push(root); } } - - roots + Ok(roots) } fn resolve_workspace_codex_home_for_path( - workspaces: &HashMap, - workspace_path: Option<&Path>, -) -> Option { - let workspace_path = workspace_path?; + app_settings: &AppSettings, + workspaces: &HashMap, + workspace_path: &Path, +) -> Result { let entry = workspaces .values() .filter(|entry| { let entry_path = Path::new(&entry.path); workspace_path == entry_path || workspace_path.starts_with(entry_path) }) - .max_by_key(|entry| entry.path.len())?; - + .max_by_key(|entry| entry.path.len()); + let Some(entry) = entry else { + return resolve_codex_home(app_settings.codex_home.as_deref()); + }; let parent_entry = entry .parent_id .as_ref() .and_then(|parent_id| workspaces.get(parent_id)); - - resolve_workspace_codex_home(entry, parent_entry) + resolve_workspace_codex_home(entry, parent_entry, app_settings) } fn day_dir_for_key(root: &Path, day_key: &str) -> PathBuf { @@ -808,7 +797,7 @@ mod tests { } #[test] - fn resolve_sessions_roots_uses_single_default_root() { + fn resolve_sessions_roots_uses_single_selected_root() { let mut workspaces = HashMap::new(); let entry_a = WorkspaceEntry { id: "a".to_string(), @@ -831,10 +820,31 @@ mod tests { workspaces.insert(entry_a.id.clone(), entry_a.clone()); workspaces.insert(entry_b.id.clone(), entry_b.clone()); - let roots = resolve_sessions_roots(&workspaces, None); - let expected = resolve_codex_sessions_root(None) - .map(|root| vec![root]) - .unwrap_or_default(); - assert_eq!(roots, expected); + let home = std::env::temp_dir().join(format!("codexmonitor-home-{}", Uuid::new_v4())); + fs::create_dir_all(&home).expect("create home"); + let settings = AppSettings { + codex_home: Some(home.to_string_lossy().into_owned()), + ..AppSettings::default() + }; + let roots = resolve_sessions_roots(&settings, &workspaces, None).expect("resolve roots"); + assert_eq!( + roots, + vec![fs::canonicalize(&home).unwrap().join("sessions")] + ); + fs::remove_dir_all(home).unwrap(); + } + #[test] + fn unavailable_selected_usage_home_never_falls_back() { + let home = std::env::temp_dir().join(format!("codexmonitor-missing-{}", Uuid::new_v4())); + let settings = AppSettings { + codex_home: Some(home.to_string_lossy().into_owned()), + ..AppSettings::default() + }; + let workspaces = HashMap::new(); + assert!(resolve_sessions_roots(&settings, &workspaces, None).is_err()); + assert!( + resolve_sessions_roots(&settings, &workspaces, Some(Path::new("/tmp/project"))) + .is_err() + ); } } diff --git a/src-tauri/src/shared/prompts_core.rs b/src-tauri/src/shared/prompts_core.rs index a03844b6f4..3c774ac092 100644 --- a/src-tauri/src/shared/prompts_core.rs +++ b/src-tauri/src/shared/prompts_core.rs @@ -5,8 +5,8 @@ use std::path::{Path, PathBuf}; use tokio::sync::Mutex; use tokio::task; -use crate::codex::home::{resolve_default_codex_home, resolve_workspace_codex_home}; -use crate::types::WorkspaceEntry; +use crate::codex::home::resolve_workspace_codex_home; +use crate::types::{AppSettings, WorkspaceEntry}; #[derive(Serialize, Clone)] pub(crate) struct CustomPromptEntry { @@ -21,21 +21,24 @@ pub(crate) struct CustomPromptEntry { } fn resolve_codex_home_for_workspace( + app_settings: &AppSettings, workspaces: &HashMap, entry: &WorkspaceEntry, -) -> Option { +) -> Result { let parent_entry = entry .parent_id .as_ref() .and_then(|parent_id| workspaces.get(parent_id)); - resolve_workspace_codex_home(entry, parent_entry).or_else(resolve_default_codex_home) + resolve_workspace_codex_home(entry, parent_entry, app_settings) } fn default_prompts_dir_for_workspace( + app_settings: &AppSettings, workspaces: &HashMap, entry: &WorkspaceEntry, -) -> Option { - resolve_codex_home_for_workspace(workspaces, entry).map(|home| home.join("prompts")) +) -> Result { + resolve_codex_home_for_workspace(app_settings, workspaces, entry) + .map(|home| home.join("prompts")) } fn require_workspace_entry( @@ -61,15 +64,18 @@ fn workspace_prompts_dir(settings_path: &Path, entry: &WorkspaceEntry) -> Result } fn prompt_roots_for_workspace( + app_settings: &AppSettings, settings_path: &Path, workspaces: &HashMap, entry: &WorkspaceEntry, ) -> Result, String> { let mut roots = Vec::new(); roots.push(workspace_prompts_dir(settings_path, entry)?); - if let Some(global_dir) = default_prompts_dir_for_workspace(workspaces, entry) { - roots.push(global_dir); - } + roots.push(default_prompts_dir_for_workspace( + app_settings, + workspaces, + entry, + )?); Ok(roots) } @@ -270,6 +276,7 @@ fn discover_prompts_in(dir: &Path, scope: Option<&str>) -> Vec>, settings_path: &Path, workspace_id: String, @@ -282,7 +289,8 @@ pub(crate) async fn prompts_list_core( .and_then(|entry| workspace_prompts_dir(settings_path, entry).ok()); let global_dir = entry .as_ref() - .and_then(|entry| default_prompts_dir_for_workspace(&workspaces, entry)); + .map(|entry| default_prompts_dir_for_workspace(app_settings, &workspaces, entry)) + .transpose()?; (workspace_dir, global_dir) }; @@ -317,18 +325,19 @@ pub(crate) async fn prompts_workspace_dir_core( } pub(crate) async fn prompts_global_dir_core( + app_settings: &AppSettings, workspaces: &Mutex>, workspace_id: String, ) -> Result { let workspaces = workspaces.lock().await; let entry = require_workspace_entry(&workspaces, &workspace_id)?; - let dir = default_prompts_dir_for_workspace(&workspaces, &entry) - .ok_or("Unable to resolve CODEX_HOME".to_string())?; + let dir = default_prompts_dir_for_workspace(app_settings, &workspaces, &entry)?; fs::create_dir_all(&dir).map_err(|err| err.to_string())?; Ok(dir.to_string_lossy().to_string()) } pub(crate) async fn prompts_create_core( + app_settings: &AppSettings, workspaces: &Mutex>, settings_path: &Path, workspace_id: String, @@ -348,8 +357,7 @@ pub(crate) async fn prompts_create_core( (dir, "workspace") } "global" => { - let dir = default_prompts_dir_for_workspace(&workspaces, &entry) - .ok_or("Unable to resolve CODEX_HOME".to_string())?; + let dir = default_prompts_dir_for_workspace(app_settings, &workspaces, &entry)?; (dir, "global") } _ => return Err("Invalid scope.".to_string()), @@ -375,6 +383,7 @@ pub(crate) async fn prompts_create_core( } pub(crate) async fn prompts_update_core( + app_settings: &AppSettings, workspaces: &Mutex>, settings_path: &Path, workspace_id: String, @@ -392,7 +401,7 @@ pub(crate) async fn prompts_update_core( { let workspaces = workspaces.lock().await; let entry = require_workspace_entry(&workspaces, &workspace_id)?; - let roots = prompt_roots_for_workspace(settings_path, &workspaces, &entry)?; + let roots = prompt_roots_for_workspace(app_settings, settings_path, &workspaces, &entry)?; ensure_path_within_roots(&target_path, &roots)?; } let dir = target_path @@ -428,6 +437,7 @@ pub(crate) async fn prompts_update_core( } pub(crate) async fn prompts_delete_core( + app_settings: &AppSettings, workspaces: &Mutex>, settings_path: &Path, workspace_id: String, @@ -440,13 +450,14 @@ pub(crate) async fn prompts_delete_core( { let workspaces = workspaces.lock().await; let entry = require_workspace_entry(&workspaces, &workspace_id)?; - let roots = prompt_roots_for_workspace(settings_path, &workspaces, &entry)?; + let roots = prompt_roots_for_workspace(app_settings, settings_path, &workspaces, &entry)?; ensure_path_within_roots(&target, &roots)?; } fs::remove_file(&target).map_err(|err| err.to_string()) } pub(crate) async fn prompts_move_core( + app_settings: &AppSettings, workspaces: &Mutex>, settings_path: &Path, workspace_id: String, @@ -460,7 +471,7 @@ pub(crate) async fn prompts_move_core( let roots = { let workspaces = workspaces.lock().await; let entry = require_workspace_entry(&workspaces, &workspace_id)?; - prompt_roots_for_workspace(settings_path, &workspaces, &entry)? + prompt_roots_for_workspace(app_settings, settings_path, &workspaces, &entry)? }; ensure_path_within_roots(&target_path, &roots)?; let file_name = target_path @@ -472,8 +483,7 @@ pub(crate) async fn prompts_move_core( let entry = require_workspace_entry(&workspaces, &workspace_id)?; match scope.as_str() { "workspace" => workspace_prompts_dir(settings_path, &entry)?, - "global" => default_prompts_dir_for_workspace(&workspaces, &entry) - .ok_or("Unable to resolve CODEX_HOME".to_string())?, + "global" => default_prompts_dir_for_workspace(app_settings, &workspaces, &entry)?, _ => return Err("Invalid scope.".to_string()), } }; @@ -504,3 +514,60 @@ pub(crate) async fn prompts_move_core( scope: Some(scope), }) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::types::{WorkspaceKind, WorkspaceSettings}; + + #[test] + fn prompt_discovery_uses_selected_home_and_reports_unavailability() { + tokio::runtime::Builder::new_current_thread() + .build() + .unwrap() + .block_on(async { + let fixture = + std::env::temp_dir().join(format!("codex-prompts-{}", uuid::Uuid::new_v4())); + let home = fixture.join("selected-home"); + fs::create_dir_all(home.join("prompts")).unwrap(); + fs::write(home.join("prompts/selected.md"), "selected prompt").unwrap(); + let settings = AppSettings { + codex_home: Some(home.to_string_lossy().into_owned()), + ..AppSettings::default() + }; + let entry = WorkspaceEntry { + id: "fixture".into(), + name: "fixture".into(), + path: "/tmp/project".into(), + kind: WorkspaceKind::Main, + parent_id: None, + worktree: None, + settings: WorkspaceSettings::default(), + }; + let workspaces = Mutex::new(HashMap::from([(entry.id.clone(), entry)])); + let settings_path = fixture.join("app/settings.json"); + let prompts = + prompts_list_core(&settings, &workspaces, &settings_path, "fixture".into()) + .await + .unwrap(); + assert_eq!(prompts.len(), 1); + assert_eq!(prompts[0].content, "selected prompt"); + assert_eq!(prompts[0].scope.as_deref(), Some("global")); + fs::remove_dir_all(&home).unwrap(); + assert!(prompts_list_core( + &settings, + &workspaces, + &settings_path, + "fixture".into() + ) + .await + .is_err()); + assert!( + prompts_global_dir_core(&settings, &workspaces, "fixture".into()) + .await + .is_err() + ); + fs::remove_dir_all(fixture).unwrap(); + }); + } +} diff --git a/src-tauri/src/shared/settings_core.rs b/src-tauri/src/shared/settings_core.rs index e4d9490754..230a1a07a5 100644 --- a/src-tauri/src/shared/settings_core.rs +++ b/src-tauri/src/shared/settings_core.rs @@ -3,6 +3,7 @@ use std::path::PathBuf; use tokio::sync::Mutex; use crate::codex::config as codex_config; +use crate::codex::home::{resolve_codex_home, resolve_codex_home_status, CodexHomeStatus}; use crate::storage::write_settings; use crate::types::AppSettings; use crate::utils::normalize_windows_namespace_path; @@ -15,28 +16,40 @@ fn normalize_personality(value: &str) -> Option<&'static str> { } } -pub(crate) async fn get_app_settings_core(app_settings: &Mutex) -> AppSettings { - let mut settings = app_settings.lock().await.clone(); - if let Ok(Some(collaboration_modes_enabled)) = codex_config::read_collaboration_modes_enabled() - { - settings.collaboration_modes_enabled = collaboration_modes_enabled; +fn load_config_settings(settings: &mut AppSettings) { + // Settings remain accessible so an unavailable home can be corrected. + // Home status and file commands report the underlying error to the UI. + let Ok(root) = resolve_codex_home(settings.codex_home.as_deref()) else { + return; + }; + if let Ok(Some(enabled)) = codex_config::read_collaboration_modes_enabled(&root) { + settings.collaboration_modes_enabled = enabled; } - if let Ok(Some(steer_enabled)) = codex_config::read_steer_enabled() { - settings.steer_enabled = steer_enabled; + if let Ok(Some(enabled)) = codex_config::read_steer_enabled(&root) { + settings.steer_enabled = enabled; } - if let Ok(Some(unified_exec_enabled)) = codex_config::read_unified_exec_enabled() { - settings.unified_exec_enabled = unified_exec_enabled; + if let Ok(Some(enabled)) = codex_config::read_unified_exec_enabled(&root) { + settings.unified_exec_enabled = enabled; } - if let Ok(Some(apps_enabled)) = codex_config::read_apps_enabled() { - settings.experimental_apps_enabled = apps_enabled; + if let Ok(Some(enabled)) = codex_config::read_apps_enabled(&root) { + settings.experimental_apps_enabled = enabled; } - if let Ok(personality) = codex_config::read_personality() { + if let Ok(personality) = codex_config::read_personality(&root) { settings.personality = personality .as_deref() .and_then(normalize_personality) .unwrap_or("friendly") .to_string(); } +} + +pub(crate) async fn get_app_settings_core(app_settings: &Mutex) -> AppSettings { + let mut current = app_settings.lock().await; + let mut settings = current.clone(); + load_config_settings(&mut settings); + // Track the values presented to the UI so an unrelated save cannot mistake + // config values loaded from disk for user edits. + *current = settings.clone(); settings } @@ -48,23 +61,213 @@ pub(crate) async fn update_app_settings_core( settings.global_worktrees_folder = settings .global_worktrees_folder .map(|path| normalize_windows_namespace_path(&path)); - let _ = codex_config::write_collaboration_modes_enabled(settings.collaboration_modes_enabled); - let _ = codex_config::write_steer_enabled(settings.steer_enabled); - let _ = codex_config::write_unified_exec_enabled(settings.unified_exec_enabled); - let _ = codex_config::write_apps_enabled(settings.experimental_apps_enabled); - let _ = codex_config::write_personality(settings.personality.as_str()); - write_settings(settings_path, &settings)?; + settings.codex_home = settings + .codex_home + .map(|value| value.trim().to_string()) + .filter(|value| !value.is_empty()); let mut current = app_settings.lock().await; + if current.codex_home != settings.codex_home { + // Do not copy the old environment's feature settings into the new home. + resolve_codex_home(settings.codex_home.as_deref())?; + let defaults = AppSettings::default(); + settings.collaboration_modes_enabled = defaults.collaboration_modes_enabled; + settings.steer_enabled = defaults.steer_enabled; + settings.unified_exec_enabled = defaults.unified_exec_enabled; + settings.experimental_apps_enabled = defaults.experimental_apps_enabled; + settings.personality = defaults.personality; + load_config_settings(&mut settings); + } else { + // Unrelated application preferences must not rewrite Codex files. + // Compare to the last settings presented to the caller, rather than + // reloading disk here and treating another app's edits as user changes. + let previous_config = &*current; + let config_changed = previous_config.collaboration_modes_enabled + != settings.collaboration_modes_enabled + || previous_config.steer_enabled != settings.steer_enabled + || previous_config.unified_exec_enabled != settings.unified_exec_enabled + || previous_config.experimental_apps_enabled != settings.experimental_apps_enabled + || previous_config.personality != settings.personality; + if config_changed { + let root = resolve_codex_home(settings.codex_home.as_deref())?; + if previous_config.collaboration_modes_enabled != settings.collaboration_modes_enabled { + codex_config::write_collaboration_modes_enabled( + &root, + settings.collaboration_modes_enabled, + )?; + } + if previous_config.steer_enabled != settings.steer_enabled { + codex_config::write_steer_enabled(&root, settings.steer_enabled)?; + } + if previous_config.unified_exec_enabled != settings.unified_exec_enabled { + codex_config::write_unified_exec_enabled(&root, settings.unified_exec_enabled)?; + } + if previous_config.experimental_apps_enabled != settings.experimental_apps_enabled { + codex_config::write_apps_enabled(&root, settings.experimental_apps_enabled)?; + } + if previous_config.personality != settings.personality { + codex_config::write_personality(&root, settings.personality.as_str())?; + } + } + } + write_settings(settings_path, &settings)?; *current = settings.clone(); Ok(settings) } -pub(crate) fn get_codex_config_path_core() -> Result { - codex_config::config_toml_path() - .ok_or_else(|| "Unable to resolve CODEX_HOME".to_string()) - .and_then(|path| { - path.to_str() - .map(|value| value.to_string()) - .ok_or_else(|| "Unable to resolve CODEX_HOME".to_string()) - }) +pub(crate) async fn get_codex_config_path_core( + app_settings: &Mutex, +) -> Result { + let settings = app_settings.lock().await; + let root = resolve_codex_home(settings.codex_home.as_deref())?; + Ok(codex_config::config_toml_path(&root) + .to_string_lossy() + .into_owned()) +} + +pub(crate) async fn get_codex_home_core( + app_settings: &Mutex, +) -> Result { + let settings = app_settings.lock().await; + resolve_codex_home_status(settings.codex_home.as_deref()) +} + +#[cfg(test)] +mod tests { + use super::*; + use uuid::Uuid; + + struct Fixture(PathBuf); + impl Fixture { + fn new() -> Self { + let root = std::env::temp_dir().join(format!("codex-home-settings-{}", Uuid::new_v4())); + std::fs::create_dir_all(&root).unwrap(); + Self(root) + } + fn home(&self, name: &str, content: &str) -> String { + let path = self.0.join(name); + std::fs::create_dir_all(&path).unwrap(); + std::fs::write(path.join("config.toml"), content).unwrap(); + path.to_string_lossy().into_owned() + } + } + impl Drop for Fixture { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + #[test] + fn unrelated_preferences_do_not_rewrite_config_overlaid_on_stored_settings() { + tokio::runtime::Builder::new_current_thread().build().unwrap().block_on(async { + let fixture = Fixture::new(); + let content = "# preserved exactly\npersonality = \"pragmatic\"\n[features]\nsteer = false\napps = true\n"; + let home = fixture.home("selected", content); + let state = Mutex::new(AppSettings { + codex_home: Some(home.clone()), + ..AppSettings::default() + }); + let mut next = get_app_settings_core(&state).await; + assert!(!next.steer_enabled); + assert!(next.experimental_apps_enabled); + next.theme = "dark".to_string(); + update_app_settings_core(next, &state, &fixture.0.join("settings.json")) + .await + .unwrap(); + assert_eq!( + std::fs::read_to_string(PathBuf::from(home).join("config.toml")).unwrap(), + content + ); + + }); + } + + #[test] + fn unrelated_save_preserves_external_config_changes_since_settings_were_loaded() { + tokio::runtime::Builder::new_current_thread() + .build() + .unwrap() + .block_on(async { + let fixture = Fixture::new(); + let home = fixture.home("selected", "[features]\napps = false\n"); + let state = Mutex::new(AppSettings { + codex_home: Some(home.clone()), + ..AppSettings::default() + }); + let mut next = get_app_settings_core(&state).await; + let external = "# changed externally\n[features]\napps = true\n"; + let config_path = PathBuf::from(home).join("config.toml"); + std::fs::write(&config_path, external).unwrap(); + next.theme = "dark".to_string(); + update_app_settings_core(next, &state, &fixture.0.join("settings.json")) + .await + .unwrap(); + assert_eq!(std::fs::read_to_string(config_path).unwrap(), external); + }); + } + + #[test] + fn switching_home_preserves_both_configs_and_loads_new_feature_settings() { + tokio::runtime::Builder::new_current_thread() + .build() + .unwrap() + .block_on(async { + let fixture = Fixture::new(); + let first_contents = "[features]\napps = false\n"; + let second_contents = "# selected provider config\n[features]\napps = true\n"; + let first = fixture.home("first", first_contents); + let second = fixture.home("second", second_contents); + let state = Mutex::new(AppSettings { + codex_home: Some(first.clone()), + ..AppSettings::default() + }); + let mut next = get_app_settings_core(&state).await; + next.codex_home = Some(second.clone()); + let settings_path = fixture.0.join("settings.json"); + let updated = update_app_settings_core(next, &state, &settings_path) + .await + .unwrap(); + assert!(updated.experimental_apps_enabled); + assert_eq!( + std::fs::read_to_string(PathBuf::from(first).join("config.toml")).unwrap(), + first_contents + ); + assert_eq!( + std::fs::read_to_string(PathBuf::from(&second).join("config.toml")).unwrap(), + second_contents + ); + let persisted: AppSettings = + serde_json::from_str(&std::fs::read_to_string(settings_path).unwrap()).unwrap(); + assert_eq!(persisted.codex_home.as_deref(), Some(second.as_str())); + }); + } + + #[test] + fn unavailable_selection_does_not_replace_persisted_home() { + tokio::runtime::Builder::new_current_thread() + .build() + .unwrap() + .block_on(async { + let fixture = Fixture::new(); + let home = fixture.home("working", "model = \"fixture-model\"\n"); + let original = AppSettings { + codex_home: Some(home.clone()), + ..AppSettings::default() + }; + let state = Mutex::new(original.clone()); + let settings_path = fixture.0.join("settings.json"); + write_settings(&settings_path, &original).unwrap(); + let bytes = std::fs::read(&settings_path).unwrap(); + let mut next = original; + next.codex_home = Some(fixture.0.join("unmounted").to_string_lossy().into_owned()); + assert!(update_app_settings_core(next, &state, &settings_path) + .await + .unwrap_err() + .contains("unavailable")); + assert_eq!( + state.lock().await.codex_home.as_deref(), + Some(home.as_str()) + ); + assert_eq!(std::fs::read(&settings_path).unwrap(), bytes); + }); + } } diff --git a/src-tauri/src/shared/workspaces_core.rs b/src-tauri/src/shared/workspaces_core.rs index 90eb5f9117..801e63af27 100644 --- a/src-tauri/src/shared/workspaces_core.rs +++ b/src-tauri/src/shared/workspaces_core.rs @@ -6,7 +6,9 @@ mod io; mod runtime_codex_args; mod worktree; -pub(crate) use connect::connect_workspace_core; +pub(crate) use connect::{ + connect_workspace_core, disconnect_all_sessions_core, workspace_session_spawn_lock, +}; pub(crate) use crud_persistence::{ add_clone_core, add_workspace_core, add_workspace_from_git_url_core, remove_workspace_core, update_workspace_settings_core, diff --git a/src-tauri/src/shared/workspaces_core/connect.rs b/src-tauri/src/shared/workspaces_core/connect.rs index bf255a1384..cd89757348 100644 --- a/src-tauri/src/shared/workspaces_core/connect.rs +++ b/src-tauri/src/shared/workspaces_core/connect.rs @@ -1,8 +1,8 @@ use std::collections::HashMap; use std::future::Future; use std::path::PathBuf; -use std::sync::OnceLock; use std::sync::Arc; +use std::sync::OnceLock; use tokio::sync::Mutex; @@ -16,7 +16,7 @@ use super::helpers::resolve_entry_and_parent; static CONNECT_WORKSPACE_SPAWN_LOCK: OnceLock> = OnceLock::new(); -pub(super) fn workspace_session_spawn_lock() -> &'static Mutex<()> { +pub(crate) fn workspace_session_spawn_lock() -> &'static Mutex<()> { CONNECT_WORKSPACE_SPAWN_LOCK.get_or_init(|| Mutex::new(())) } @@ -64,6 +64,18 @@ where { let (entry, parent_entry) = resolve_entry_and_parent(workspaces, &workspace_id).await?; let _spawn_guard = workspace_session_spawn_lock().lock().await; + let (default_bin, codex_args, codex_home) = { + let settings = app_settings.lock().await; + ( + settings.codex_bin.clone(), + resolve_workspace_codex_args(&entry, parent_entry.as_ref(), Some(&settings)), + Some(resolve_workspace_codex_home( + &entry, + parent_entry.as_ref(), + &settings, + )?), + ) + }; if let Some(existing_for_entry) = { let sessions = sessions.lock().await; sessions.get(&entry.id).cloned() @@ -83,14 +95,6 @@ where .insert(entry.id.clone(), existing_session); return Ok(()); } - let (default_bin, codex_args) = { - let settings = app_settings.lock().await; - ( - settings.codex_bin.clone(), - resolve_workspace_codex_args(&entry, parent_entry.as_ref(), Some(&settings)), - ) - }; - let codex_home = resolve_workspace_codex_home(&entry, parent_entry.as_ref()); let session = spawn_session(entry.clone(), default_bin, codex_args, codex_home).await?; session .register_workspace_with_path(&entry.id, Some(&entry.path)) @@ -99,6 +103,31 @@ where Ok(()) } +/// Disconnect all workspaces sharing the runtime. The caller must hold +/// `workspace_session_spawn_lock` while changing settings and disconnecting so a +/// concurrent workspace connection cannot retain the previous Codex home. +pub(crate) async fn disconnect_all_sessions_core( + sessions: &Mutex>>, +) { + let existing = { + let mut sessions = sessions.lock().await; + let mut unique = Vec::>::new(); + for (_, session) in sessions.drain() { + if !unique + .iter() + .any(|candidate| Arc::ptr_eq(candidate, &session)) + { + unique.push(session); + } + } + unique + }; + for session in existing { + let mut child = session.child.lock().await; + kill_child_process_tree(&mut child).await; + } +} + pub(super) async fn kill_session_by_id( sessions: &Mutex>>, id: &str, @@ -250,4 +279,75 @@ mod tests { kill_session_by_id(&sessions, &entry.id).await; }); } + #[test] + fn connect_workspace_passes_explicit_home_to_spawn() { + tokio::runtime::Runtime::new().unwrap().block_on(async { + let entry = make_workspace_entry("explicit-home"); + let workspaces = Mutex::new(HashMap::from([(entry.id.clone(), entry.clone())])); + let sessions = Mutex::new(HashMap::new()); + let home = std::env::temp_dir().canonicalize().unwrap(); + let app_settings = Mutex::new(AppSettings { + codex_home: Some(home.to_string_lossy().into_owned()), + ..AppSettings::default() + }); + let expected_home = home.clone(); + connect_workspace_core( + entry.id.clone(), + &workspaces, + &sessions, + &app_settings, + move |entry, _, _, codex_home| { + assert_eq!(codex_home.as_ref(), Some(&expected_home)); + async move { Ok(make_session(entry)) } + }, + ) + .await + .unwrap(); + kill_session_by_id(&sessions, &entry.id).await; + }); + } + + #[test] + fn unavailable_explicit_home_rejects_existing_shared_session() { + tokio::runtime::Runtime::new().unwrap().block_on(async { + let entry = make_workspace_entry("unavailable-home"); + let workspaces = Mutex::new(HashMap::from([(entry.id.clone(), entry.clone())])); + let sessions = Mutex::new(HashMap::from([( + entry.id.clone(), + make_session(entry.clone()), + )])); + let missing = std::env::temp_dir() + .join(format!("codex-monitor-missing-{}", uuid::Uuid::new_v4())); + let app_settings = Mutex::new(AppSettings { + codex_home: Some(missing.to_string_lossy().into_owned()), + ..AppSettings::default() + }); + let result = connect_workspace_core( + entry.id.clone(), + &workspaces, + &sessions, + &app_settings, + |_, _, _, _| async { Err("must never spawn".to_string()) }, + ) + .await; + assert!(result.unwrap_err().contains("unavailable")); + kill_session_by_id(&sessions, &entry.id).await; + }); + } + + #[test] + fn disconnect_all_sessions_clears_shared_runtime_references() { + tokio::runtime::Runtime::new().unwrap().block_on(async { + let entry = make_workspace_entry("disconnect-home"); + let session = make_session(entry); + let sessions = Mutex::new(HashMap::from([ + ("workspace-one".to_string(), Arc::clone(&session)), + ("workspace-two".to_string(), Arc::clone(&session)), + ])); + let _guard = workspace_session_spawn_lock().lock().await; + disconnect_all_sessions_core(&sessions).await; + assert!(sessions.lock().await.is_empty()); + assert!(!session_process_is_alive(&session).await); + }); + } } diff --git a/src-tauri/src/shared/workspaces_core/crud_persistence.rs b/src-tauri/src/shared/workspaces_core/crud_persistence.rs index 633b078f04..7dcb4a3008 100644 --- a/src-tauri/src/shared/workspaces_core/crud_persistence.rs +++ b/src-tauri/src/shared/workspaces_core/crud_persistence.rs @@ -8,7 +8,7 @@ use uuid::Uuid; use crate::backend::app_server::WorkspaceSession; use crate::codex::args::resolve_workspace_codex_args; -use crate::codex::home::resolve_workspace_codex_home; +use crate::codex::home::{resolve_codex_home, resolve_workspace_codex_home}; use crate::shared::process_core::kill_child_process_tree; use crate::shared::{git_core, worktree_core}; use crate::storage::write_workspaces; @@ -54,18 +54,18 @@ where }; let _spawn_guard = workspace_session_spawn_lock().lock().await; + let (default_bin, codex_args, codex_home) = { + let settings = app_settings.lock().await; + ( + settings.codex_bin.clone(), + resolve_workspace_codex_args(&entry, None, Some(&settings)), + Some(resolve_workspace_codex_home(&entry, None, &settings)?), + ) + }; let existing_session = take_live_shared_session(sessions).await; let (session, spawned_new_session) = if let Some(existing_session) = existing_session { (existing_session, false) } else { - let (default_bin, codex_args) = { - let settings = app_settings.lock().await; - ( - settings.codex_bin.clone(), - resolve_workspace_codex_args(&entry, None, Some(&settings)), - ) - }; - let codex_home = resolve_workspace_codex_home(&entry, None); ( spawn_session(entry.clone(), default_bin, codex_args, codex_home).await?, true, @@ -120,6 +120,11 @@ where F: Fn(WorkspaceEntry, Option, Option, Option) -> Fut, Fut: Future, String>>, { + { + let settings = app_settings.lock().await; + resolve_codex_home(settings.codex_home.as_deref())?; + } + let copy_name = copy_name.trim().to_string(); if copy_name.is_empty() { return Err("Copy name is required.".to_string()); @@ -205,18 +210,30 @@ where }; let _spawn_guard = workspace_session_spawn_lock().lock().await; + let (default_bin, codex_args, codex_home) = { + let settings = app_settings.lock().await; + ( + settings.codex_bin.clone(), + resolve_workspace_codex_args(&entry, None, Some(&settings)), + resolve_workspace_codex_home(&entry, None, &settings).map(Some), + ) + }; + let codex_home = match codex_home { + Ok(home) => home, + Err(error) => { + if let Err(cleanup_error) = tokio::fs::remove_dir_all(&destination_path).await { + return Err(format!( + "{error}. Could not remove the newly cloned directory {}: {cleanup_error}", + destination_path.display(), + )); + } + return Err(error); + } + }; let existing_session = take_live_shared_session(sessions).await; let (session, spawned_new_session) = if let Some(existing_session) = existing_session { (existing_session, false) } else { - let (default_bin, codex_args) = { - let settings = app_settings.lock().await; - ( - settings.codex_bin.clone(), - resolve_workspace_codex_args(&entry, None, Some(&settings)), - ) - }; - let codex_home = resolve_workspace_codex_home(&entry, None); match spawn_session(entry.clone(), default_bin, codex_args, codex_home).await { Ok(session) => (session, true), Err(error) => { @@ -312,6 +329,11 @@ where F: Fn(WorkspaceEntry, Option, Option, Option) -> Fut, Fut: Future, String>>, { + { + let settings = app_settings.lock().await; + resolve_codex_home(settings.codex_home.as_deref())?; + } + let url = url.trim().to_string(); if url.is_empty() { return Err("Remote Git URL is required.".to_string()); @@ -370,18 +392,30 @@ where }; let _spawn_guard = workspace_session_spawn_lock().lock().await; + let (default_bin, codex_args, codex_home) = { + let settings = app_settings.lock().await; + ( + settings.codex_bin.clone(), + resolve_workspace_codex_args(&entry, None, Some(&settings)), + resolve_workspace_codex_home(&entry, None, &settings).map(Some), + ) + }; + let codex_home = match codex_home { + Ok(home) => home, + Err(error) => { + if let Err(cleanup_error) = tokio::fs::remove_dir_all(&clone_path).await { + return Err(format!( + "{error}. Could not remove the newly cloned directory {}: {cleanup_error}", + clone_path.display(), + )); + } + return Err(error); + } + }; let existing_session = take_live_shared_session(sessions).await; let (session, spawned_new_session) = if let Some(existing_session) = existing_session { (existing_session, false) } else { - let (default_bin, codex_args) = { - let settings = app_settings.lock().await; - ( - settings.codex_bin.clone(), - resolve_workspace_codex_args(&entry, None, Some(&settings)), - ) - }; - let codex_home = resolve_workspace_codex_home(&entry, None); match spawn_session(entry.clone(), default_bin, codex_args, codex_home).await { Ok(session) => (session, true), Err(error) => { @@ -620,7 +654,36 @@ where #[cfg(test)] mod tests { - use super::{default_repo_name_from_url, validate_target_folder_name}; + use super::*; + + #[test] + fn unavailable_home_prevents_clone_destination_creation() { + tokio::runtime::Runtime::new().unwrap().block_on(async { + let root = + std::env::temp_dir().join(format!("codex-clone-preflight-{}", Uuid::new_v4())); + let copies = root.join("copies"); + let workspaces = Mutex::new(HashMap::new()); + let sessions = Mutex::new(HashMap::new()); + let settings = Mutex::new(AppSettings { + codex_home: Some(root.join("missing-home").to_string_lossy().into_owned()), + ..AppSettings::default() + }); + let result = add_clone_core( + "source".to_string(), + "clone".to_string(), + copies.to_string_lossy().into_owned(), + &workspaces, + &sessions, + &settings, + &root.join("settings.json"), + |_, _, _, _| async { Err("must never spawn".to_string()) }, + ) + .await; + assert!(result.unwrap_err().contains("unavailable")); + assert!(!copies.exists()); + assert!(!root.exists()); + }); + } #[test] fn derives_repo_name_from_https_url() { diff --git a/src-tauri/src/shared/workspaces_core/runtime_codex_args.rs b/src-tauri/src/shared/workspaces_core/runtime_codex_args.rs index 9801e6c5ec..a28151896e 100644 --- a/src-tauri/src/shared/workspaces_core/runtime_codex_args.rs +++ b/src-tauri/src/shared/workspaces_core/runtime_codex_args.rs @@ -37,11 +37,16 @@ where let (entry, parent_entry) = resolve_entry_and_parent(workspaces, &workspace_id).await?; let _spawn_guard = workspace_session_spawn_lock().lock().await; - let (default_bin, resolved_args) = { + let (default_bin, resolved_args, codex_home) = { let settings = app_settings.lock().await; ( settings.codex_bin.clone(), resolve_workspace_codex_args(&entry, parent_entry.as_ref(), Some(&settings)), + Some(resolve_workspace_codex_home( + &entry, + parent_entry.as_ref(), + &settings, + )?), ) }; @@ -82,7 +87,6 @@ where }); } - let codex_home = resolve_workspace_codex_home(&entry, parent_entry.as_ref()); let new_session = spawn_session(entry.clone(), default_bin, target_args.clone(), codex_home).await?; let workspace_ids = { @@ -129,8 +133,8 @@ where mod tests { use super::*; - use std::process::Stdio; use std::collections::HashSet; + use std::process::Stdio; use std::sync::atomic::{AtomicU64, AtomicUsize, Ordering}; use tokio::process::Command; diff --git a/src-tauri/src/shared/workspaces_core/worktree.rs b/src-tauri/src/shared/workspaces_core/worktree.rs index 824e4aecda..451e4c448a 100644 --- a/src-tauri/src/shared/workspaces_core/worktree.rs +++ b/src-tauri/src/shared/workspaces_core/worktree.rs @@ -8,7 +8,7 @@ use uuid::Uuid; use crate::backend::app_server::WorkspaceSession; use crate::codex::args::resolve_workspace_codex_args; -use crate::codex::home::resolve_workspace_codex_home; +use crate::codex::home::{resolve_codex_home, resolve_workspace_codex_home}; use crate::storage::write_workspaces; use crate::types::{ AppSettings, WorkspaceEntry, WorkspaceInfo, WorkspaceKind, WorkspaceSettings, WorktreeInfo, @@ -134,6 +134,11 @@ where return Err("Cannot create a worktree from another worktree.".to_string()); } + { + let settings = app_settings.lock().await; + resolve_codex_home(settings.codex_home.as_deref())?; + } + // Determine worktree root: per-workspace setting > global setting > default let worktree_root = if let Some(custom_folder) = &parent_entry.settings.worktrees_folder { PathBuf::from(workspace_path_to_string(&PathBuf::from(custom_folder))) @@ -221,18 +226,35 @@ where }; let _spawn_guard = workspace_session_spawn_lock().lock().await; + let (default_bin, codex_args, codex_home) = { + let settings = app_settings.lock().await; + ( + settings.codex_bin.clone(), + resolve_workspace_codex_args(&entry, Some(&parent_entry), Some(&settings)), + resolve_workspace_codex_home(&entry, Some(&parent_entry), &settings).map(Some), + ) + }; + let codex_home = match codex_home { + Ok(home) => home, + Err(error) => { + if let Err(cleanup_error) = run_git_command( + &repo_path, + &["worktree", "remove", "--force", &worktree_path_string], + ) + .await + { + return Err(format!( + "{error}. Could not remove the newly created worktree {}: {cleanup_error}", + worktree_path.display(), + )); + } + return Err(error); + } + }; let existing_session = take_live_shared_session(sessions).await; let session = if let Some(existing_session) = existing_session { existing_session } else { - let (default_bin, codex_args) = { - let settings = app_settings.lock().await; - ( - settings.codex_bin.clone(), - resolve_workspace_codex_args(&entry, Some(&parent_entry), Some(&settings)), - ) - }; - let codex_home = resolve_workspace_codex_home(&entry, Some(&parent_entry)); spawn_session(entry.clone(), default_bin, codex_args, codex_home).await? }; diff --git a/src-tauri/src/types.rs b/src-tauri/src/types.rs index 6b595106b5..6fdbbfb3c0 100644 --- a/src-tauri/src/types.rs +++ b/src-tauri/src/types.rs @@ -379,6 +379,8 @@ pub(crate) struct RemoteBackendTarget { pub(crate) struct AppSettings { #[serde(default, rename = "codexBin")] pub(crate) codex_bin: Option, + #[serde(default, rename = "codexHome")] + pub(crate) codex_home: Option, #[serde(default, rename = "codexArgs")] pub(crate) codex_args: Option, #[serde(default, rename = "backendMode")] @@ -1124,6 +1126,7 @@ impl Default for AppSettings { fn default() -> Self { Self { codex_bin: None, + codex_home: None, codex_args: None, backend_mode: default_backend_mode(), remote_backend_provider: RemoteBackendProvider::Tcp, @@ -1210,10 +1213,22 @@ mod tests { WorkspaceKind, WorkspaceSettings, }; + #[test] + fn app_settings_codex_home_round_trips_without_changing_legacy_defaults() { + let settings: AppSettings = + serde_json::from_str(r#"{"codexHome":"~/.codex-custom","codexBin":"/bin/codex"}"#) + .expect("settings deserialize"); + assert_eq!(settings.codex_home.as_deref(), Some("~/.codex-custom")); + let saved = serde_json::to_value(&settings).expect("serialize settings"); + assert_eq!(saved["codexHome"], "~/.codex-custom"); + assert!(AppSettings::default().codex_home.is_none()); + } + #[test] fn app_settings_defaults_from_empty_json() { let settings: AppSettings = serde_json::from_str("{}").expect("settings deserialize"); assert!(settings.codex_bin.is_none()); + assert!(settings.codex_home.is_none()); let expected_backend_mode = if cfg!(target_os = "ios") { BackendMode::Remote } else { diff --git a/src/features/settings/components/SettingsView.test.tsx b/src/features/settings/components/SettingsView.test.tsx index 9633cbca04..fc8e9a2ca4 100644 --- a/src/features/settings/components/SettingsView.test.tsx +++ b/src/features/settings/components/SettingsView.test.tsx @@ -16,6 +16,7 @@ import { getAppBuildType, getAgentsSettings, getConfigModel, + getCodexHome, getExperimentalFeatureList, isMobileRuntime, getModelList, @@ -39,6 +40,7 @@ vi.mock("@services/tauri", async () => { getAppBuildType: vi.fn(), getModelList: vi.fn(), getConfigModel: vi.fn(), + getCodexHome: vi.fn(), getExperimentalFeatureList: vi.fn(), getAgentsSettings: vi.fn(), isMobileRuntime: vi.fn(), @@ -49,6 +51,7 @@ vi.mock("@services/tauri", async () => { const connectWorkspaceMock = vi.mocked(connectWorkspace); const getAppBuildTypeMock = vi.mocked(getAppBuildType); const getConfigModelMock = vi.mocked(getConfigModel); +vi.mocked(getCodexHome).mockResolvedValue({ configuredPath: null, effectivePath: "/Users/me/.codex", source: "default" }); const getModelListMock = vi.mocked(getModelList); const getExperimentalFeatureListMock = vi.mocked(getExperimentalFeatureList); const getAgentsSettingsMock = vi.mocked(getAgentsSettings); diff --git a/src/features/settings/components/sections/SettingsCodexHome.test.tsx b/src/features/settings/components/sections/SettingsCodexHome.test.tsx new file mode 100644 index 0000000000..18e10409fa --- /dev/null +++ b/src/features/settings/components/sections/SettingsCodexHome.test.tsx @@ -0,0 +1,112 @@ +// @vitest-environment jsdom +import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { open } from "@tauri-apps/plugin-dialog"; +import type { AppSettings } from "@/types"; +import { getCodexHome, getCodexConfigPath, validateCodexHome } from "@services/tauri"; +import { useSettingsCodexSection } from "@settings/hooks/useSettingsCodexSection"; +import { SettingsCodexSection } from "./SettingsCodexSection"; + +vi.mock("@services/tauri", () => ({ getCodexHome: vi.fn(), getCodexConfigPath: vi.fn(), validateCodexHome: vi.fn() })); +vi.mock("@tauri-apps/plugin-dialog", () => ({ open: vi.fn() })); +vi.mock("@settings/hooks/useSettingsDefaultModels", () => ({ + useSettingsDefaultModels: () => ({ models: [], isLoading: false, error: null, connectedWorkspaceCount: 0, refresh: vi.fn() }), +})); +vi.mock("@settings/hooks/useGlobalAgentsMd", () => ({ useGlobalAgentsMd: () => editor() })); +vi.mock("@settings/hooks/useGlobalCodexConfigToml", () => ({ useGlobalCodexConfigToml: () => editor() })); + +function editor() { + return { + content: "", exists: true, truncated: false, isLoading: false, isSaving: false, + error: null, isDirty: false, setContent: vi.fn(), refresh: vi.fn(), save: vi.fn(), + }; +} + +const home = { + configuredPath: "/Users/me/.codex-alternative", + effectivePath: "/Volumes/External/codex-alternative", + source: "application" as const, +}; + +function Harness({ save, remote = false }: { save: (next: AppSettings) => Promise; remote?: boolean }) { + const props = useSettingsCodexSection({ + appSettings: { codexBin: null, codexArgs: null, codexHome: home.configuredPath, backendMode: remote ? "remote" : "local" } as AppSettings, + projects: [], onUpdateAppSettings: save, onRunDoctor: vi.fn(), + }); + return ; +} + +beforeEach(() => { + vi.resetAllMocks(); + vi.mocked(getCodexHome).mockResolvedValue(home); +}); +afterEach(cleanup); + +describe("Codex Home settings", () => { + it("shows the configured symlink and canonical file locations", async () => { + render(); + expect((screen.getByLabelText("Codex Home") as HTMLInputElement).value).toBe(home.configuredPath); + await waitFor(() => expect(screen.getByText(`${home.effectivePath}/config.toml`)).toBeTruthy()); + expect(screen.getByText(`${home.effectivePath}/AGENTS.md`)).toBeTruthy(); + expect(screen.queryByText("~/.codex/config.toml")).toBeNull(); + }); + + it("uses the native directory picker", async () => { + vi.mocked(open).mockResolvedValue("/chosen/home"); + render(); + fireEvent.click(screen.getByRole("button", { name: "Browse home" })); + await waitFor(() => expect((screen.getByLabelText("Codex Home") as HTMLInputElement).value).toBe("/chosen/home")); + expect(open).toHaveBeenCalledWith({ multiple: false, directory: true, title: "Select Codex Home" }); + }); + + it("does not persist an inaccessible home", async () => { + vi.mocked(validateCodexHome).mockRejectedValue(new Error("Configured home is unavailable")); + const save = vi.fn(); + render(); + fireEvent.change(screen.getByLabelText("Codex Home"), { target: { value: "/missing/home" } }); + fireEvent.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => expect(screen.getByRole("alert").textContent).toContain("unavailable")); + expect(save).not.toHaveBeenCalled(); + }); + + it("validates before persisting a changed home", async () => { + const save = vi.fn().mockResolvedValue(undefined); + vi.mocked(validateCodexHome).mockResolvedValue({ ...home, configuredPath: "/chosen/home" }); + // jsdom reports its unsupported full-page reload; the real app reboots its caches. + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); + try { + render(); + fireEvent.change(screen.getByLabelText("Codex Home"), { target: { value: "/chosen/home" } }); + fireEvent.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => expect(save).toHaveBeenCalledWith(expect.objectContaining({ codexHome: "/chosen/home" }))); + expect(validateCodexHome).toHaveBeenCalledWith("/chosen/home"); + } finally { + consoleError.mockRestore(); + } + }); + + it("disables local home controls remotely and displays the daemon’s file paths", async () => { + vi.mocked(getCodexConfigPath).mockResolvedValue("/remote/codex/config.toml"); + render(); + expect((screen.getByLabelText("Codex Home") as HTMLInputElement).disabled).toBe(true); + for (const name of ["Browse home", "Use default", "Validate"]) { + expect((screen.getByRole("button", { name }) as HTMLButtonElement).disabled).toBe(true); + } + await waitFor(() => expect(screen.getByText("/remote/codex/config.toml")).toBeTruthy()); + expect(screen.getByText("/remote/codex/AGENTS.md")).toBeTruthy(); + expect(getCodexHome).not.toHaveBeenCalled(); + expect(screen.queryByText(home.effectivePath)).toBeNull(); + }); + + it("restores inherited/default resolution without saving until requested", async () => { + const save = vi.fn(); + vi.mocked(validateCodexHome).mockResolvedValue({ configuredPath: null, effectivePath: "/inherited/home", source: "environment" }); + render(); + fireEvent.click(screen.getByRole("button", { name: "Use default" })); + expect((screen.getByLabelText("Codex Home") as HTMLInputElement).value).toBe(""); + fireEvent.click(screen.getByRole("button", { name: "Validate" })); + await waitFor(() => expect(screen.getByRole("status").textContent).toContain("/inherited/home")); + expect(validateCodexHome).toHaveBeenCalledWith(null); + expect(save).not.toHaveBeenCalled(); + }); +}); diff --git a/src/features/settings/components/sections/SettingsCodexSection.tsx b/src/features/settings/components/sections/SettingsCodexSection.tsx index a522b3ac13..ef5db389a1 100644 --- a/src/features/settings/components/sections/SettingsCodexSection.tsx +++ b/src/features/settings/components/sections/SettingsCodexSection.tsx @@ -1,65 +1,12 @@ import { useEffect, useMemo, useRef } from "react"; import Stethoscope from "lucide-react/dist/esm/icons/stethoscope"; -import type { Dispatch, SetStateAction } from "react"; -import type { - AppSettings, - CodexDoctorResult, - CodexUpdateResult, - ModelOption, -} from "@/types"; +import type { AppSettings, ModelOption } from "@/types"; import { SettingsSection, SettingsToggleRow, } from "@/features/design-system/components/settings/SettingsPrimitives"; import { FileEditorCard } from "@/features/shared/components/FileEditorCard"; - -type SettingsCodexSectionProps = { - appSettings: AppSettings; - onUpdateAppSettings: (next: AppSettings) => Promise; - defaultModels: ModelOption[]; - defaultModelsLoading: boolean; - defaultModelsError: string | null; - defaultModelsConnectedWorkspaceCount: number; - onRefreshDefaultModels: () => void; - codexPathDraft: string; - codexArgsDraft: string; - codexDirty: boolean; - isSavingSettings: boolean; - doctorState: { - status: "idle" | "running" | "done"; - result: CodexDoctorResult | null; - }; - codexUpdateState: { - status: "idle" | "running" | "done"; - result: CodexUpdateResult | null; - }; - globalAgentsMeta: string; - globalAgentsError: string | null; - globalAgentsContent: string; - globalAgentsLoading: boolean; - globalAgentsRefreshDisabled: boolean; - globalAgentsSaveDisabled: boolean; - globalAgentsSaveLabel: string; - globalConfigMeta: string; - globalConfigError: string | null; - globalConfigContent: string; - globalConfigLoading: boolean; - globalConfigRefreshDisabled: boolean; - globalConfigSaveDisabled: boolean; - globalConfigSaveLabel: string; - onSetCodexPathDraft: Dispatch>; - onSetCodexArgsDraft: Dispatch>; - onSetGlobalAgentsContent: (value: string) => void; - onSetGlobalConfigContent: (value: string) => void; - onBrowseCodex: () => Promise; - onSaveCodexSettings: () => Promise; - onRunDoctor: () => Promise; - onRunCodexUpdate: () => Promise; - onRefreshGlobalAgents: () => void; - onSaveGlobalAgents: () => void; - onRefreshGlobalConfig: () => void; - onSaveGlobalConfig: () => void; -}; +import type { SettingsCodexSectionProps } from "@settings/hooks/useSettingsCodexSection"; const DEFAULT_REASONING_EFFORT = "medium"; @@ -115,6 +62,18 @@ export function SettingsCodexSection({ onRefreshDefaultModels, codexPathDraft, codexArgsDraft, + codexHomeDraft, + codexHomeEffectivePath, + globalConfigPath, + globalAgentsPath, + codexHomeSource, + codexHomeValidatedPath, + codexHomeError, + codexHomeValidating, + codexSettingsError, + onSetCodexHomeDraft, + onBrowseCodexHome, + onValidateCodexHome, codexDirty, isSavingSettings, doctorState, @@ -232,6 +191,64 @@ export function SettingsCodexSection({ title="Codex" subtitle="Configure the Codex CLI used by CodexMonitor and validate the install." > +
+ +
+ onSetCodexHomeDraft(event.target.value)} + disabled={isSavingSettings || appSettings.backendMode === "remote"} + spellCheck={false} + /> + + + +
+
+ An explicit home takes priority over CODEX_HOME. Leave empty to use CODEX_HOME, + then ~/.codex. Saving a changed home reloads the app and reconnects workspaces. +
+ {appSettings.backendMode === "remote" ? ( +
Codex Home is a local backend setting. Switch to the local backend to change it. Remote configuration files use the connected daemon’s home.
+ ) : ( +
+ Effective home: {codexHomeEffectivePath ?? "Unavailable"} + {codexHomeSource ? ` (${codexHomeSource})` : ""} +
+ )} + {codexHomeValidatedPath && ( +
+ Validated selection: {codexHomeValidatedPath} +
+ )} + {codexHomeError &&
{codexHomeError}
} +
+ {codexSettingsError && ( +
{codexSettingsError}
+ )} {doctorState.result && (
@@ -537,9 +557,9 @@ export function SettingsCodexSection({ onRefresh={onRefreshGlobalAgents} onSave={onSaveGlobalAgents} helpText={ - <> - Stored at ~/.codex/AGENTS.md. - + globalAgentsPath ? ( + <>Stored at {globalAgentsPath}. + ) : "Codex configuration path is unavailable." } classNames={{ container: "settings-field settings-agents", @@ -568,9 +588,9 @@ export function SettingsCodexSection({ onRefresh={onRefreshGlobalConfig} onSave={onSaveGlobalConfig} helpText={ - <> - Stored at ~/.codex/config.toml. - + globalConfigPath ? ( + <>Stored at {globalConfigPath}. + ) : "Codex configuration path is unavailable." } classNames={{ container: "settings-field settings-agents", diff --git a/src/features/settings/hooks/globalEditorsHome.test.ts b/src/features/settings/hooks/globalEditorsHome.test.ts new file mode 100644 index 0000000000..b82bf5c2f8 --- /dev/null +++ b/src/features/settings/hooks/globalEditorsHome.test.ts @@ -0,0 +1,37 @@ +// @vitest-environment jsdom +import { act, cleanup, renderHook, waitFor } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { + readGlobalAgentsMd, readGlobalCodexConfigToml, + writeGlobalAgentsMd, writeGlobalCodexConfigToml, +} from "@services/tauri"; +import { useGlobalAgentsMd } from "./useGlobalAgentsMd"; +import { useGlobalCodexConfigToml } from "./useGlobalCodexConfigToml"; + +vi.mock("@services/tauri", () => ({ + readGlobalAgentsMd: vi.fn(), readGlobalCodexConfigToml: vi.fn(), + writeGlobalAgentsMd: vi.fn(), writeGlobalCodexConfigToml: vi.fn(), +})); +vi.mock("@services/toasts", () => ({ pushErrorToast: vi.fn() })); + +afterEach(() => { cleanup(); vi.resetAllMocks(); }); + +for (const [name, useEditor, read, write] of [ + ["config.toml", useGlobalCodexConfigToml, readGlobalCodexConfigToml, writeGlobalCodexConfigToml], + ["AGENTS.md", useGlobalAgentsMd, readGlobalAgentsMd, writeGlobalAgentsMd], +] as const) { + describe(`${name} home isolation`, () => { + it("discards old drafts and reloads when the selected home changes", async () => { + vi.mocked(read) + .mockResolvedValueOnce({ exists: true, content: "first environment", truncated: false }) + .mockResolvedValueOnce({ exists: true, content: "second environment", truncated: false }); + const { result, rerender } = renderHook(({ home }) => useEditor(home), { initialProps: { home: "/first" } }); + await waitFor(() => expect(result.current.content).toBe("first environment")); + act(() => result.current.setContent("unsaved first-environment draft")); + rerender({ home: "/second" }); + await waitFor(() => expect(result.current.content).toBe("second environment")); + expect(result.current.isDirty).toBe(false); + expect(write).not.toHaveBeenCalled(); + }); + }); +} diff --git a/src/features/settings/hooks/useAppSettings.test.ts b/src/features/settings/hooks/useAppSettings.test.ts index c6eb4221e4..7fd1c9f694 100644 --- a/src/features/settings/hooks/useAppSettings.test.ts +++ b/src/features/settings/hooks/useAppSettings.test.ts @@ -49,6 +49,8 @@ describe("useAppSettings", () => { expect(result.current.settings.uiScale).toBe(UI_SCALE_MAX); expect(result.current.settings.theme).toBe("system"); + expect(result.current.settings.codexHome).toBeNull(); + expect(result.current.settings.followUpMessageBehavior).toBe("queue"); expect(result.current.settings.uiFontFamily).toContain("system-ui"); expect(result.current.settings.codeFontFamily).toContain("ui-monospace"); expect(result.current.settings.codeFontSize).toBe(16); @@ -73,6 +75,13 @@ describe("useAppSettings", () => { expect(result.current.settings.interruptShortcut).toBeTruthy(); }); + it("normalizes and preserves an explicit Codex Home on load", async () => { + getAppSettingsMock.mockResolvedValue({ codexHome: " /selected/codex-home " } as AppSettings); + const { result } = renderHook(() => useAppSettings()); + await waitFor(() => expect(result.current.isLoading).toBe(false)); + expect(result.current.settings.codexHome).toBe("/selected/codex-home"); + }); + it("persists settings via updateAppSettings and updates local state", async () => { getAppSettingsMock.mockResolvedValue({} as AppSettings); const { result } = renderHook(() => useAppSettings()); diff --git a/src/features/settings/hooks/useAppSettings.ts b/src/features/settings/hooks/useAppSettings.ts index 331bca4c0f..655232eaa2 100644 --- a/src/features/settings/hooks/useAppSettings.ts +++ b/src/features/settings/hooks/useAppSettings.ts @@ -135,6 +135,7 @@ function buildDefaultSettings(): AppSettings { }; return { codexBin: null, + codexHome: null, codexArgs: null, backendMode: isMobile ? "remote" : "local", remoteBackendProvider: defaultRemote.provider, @@ -244,6 +245,7 @@ function normalizeAppSettings(settings: AppSettings): AppSettings { ...settings, ...remoteBackendSettings, codexBin: settings.codexBin?.trim() ? settings.codexBin.trim() : null, + codexHome: settings.codexHome?.trim() ? settings.codexHome.trim() : null, codexArgs: settings.codexArgs?.trim() ? settings.codexArgs.trim() : null, uiScale: clampUiScale(settings.uiScale), theme: allowedThemes.has(settings.theme) ? settings.theme : "system", diff --git a/src/features/settings/hooks/useCodexHomeSetting.test.ts b/src/features/settings/hooks/useCodexHomeSetting.test.ts new file mode 100644 index 0000000000..ce19ddb43e --- /dev/null +++ b/src/features/settings/hooks/useCodexHomeSetting.test.ts @@ -0,0 +1,79 @@ +// @vitest-environment jsdom +import { act, cleanup, renderHook, waitFor } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { CodexHomeStatus } from "@/types"; +import { getCodexHome, validateCodexHome } from "@services/tauri"; +import { useCodexHomeSetting } from "./useCodexHomeSetting"; + +vi.mock("@services/tauri", () => ({ getCodexHome: vi.fn(), validateCodexHome: vi.fn() })); + +const selected: CodexHomeStatus = { + configuredPath: "/Users/me/.codex-alternative", + effectivePath: "/Volumes/External/codex-alternative", + source: "application", +}; + +beforeEach(() => { + vi.resetAllMocks(); + vi.mocked(getCodexHome).mockResolvedValue(selected); +}); +afterEach(cleanup); + +describe("useCodexHomeSetting", () => { + it("keeps the configured symlink separate from its canonical effective home", async () => { + const { result } = renderHook(() => useCodexHomeSetting(selected.configuredPath)); + await waitFor(() => expect(result.current.effectiveHome).toEqual(selected)); + expect(result.current.draft).toBe(selected.configuredPath); + }); + + it("validates a trimmed manual selection without changing the active home", async () => { + vi.mocked(validateCodexHome).mockResolvedValue(selected); + const { result } = renderHook(() => useCodexHomeSetting(null)); + await waitFor(() => expect(result.current.effectiveHome).not.toBeNull()); + act(() => result.current.updateDraft(" /Users/me/.codex-alternative ")); + await act(async () => { await result.current.validate(); }); + expect(validateCodexHome).toHaveBeenCalledWith("/Users/me/.codex-alternative"); + expect(result.current.validatedHome?.effectivePath).toBe(selected.effectivePath); + expect(result.current.effectiveHome).toEqual(selected); + }); + + it("restores default resolution by validating null, rather than inventing a home", async () => { + const inherited: CodexHomeStatus = { + configuredPath: null, effectivePath: "/inherited/home", source: "environment", + }; + vi.mocked(validateCodexHome).mockResolvedValue(inherited); + const { result } = renderHook(() => useCodexHomeSetting(selected.configuredPath)); + act(() => result.current.updateDraft("")); + await act(async () => { await result.current.validate(); }); + expect(validateCodexHome).toHaveBeenCalledWith(null); + expect(result.current.validatedHome).toEqual(inherited); + }); + + it("reports an unavailable explicit home without displaying a default path", async () => { + vi.mocked(getCodexHome).mockRejectedValue(new Error("Configured Codex Home is unavailable: /missing")); + const { result } = renderHook(() => useCodexHomeSetting("/missing")); + await waitFor(() => expect(result.current.error).toContain("unavailable")); + expect(result.current.effectiveHome).toBeNull(); + expect(result.current.draft).toBe("/missing"); + }); + + it("ignores validation completed after the draft changes", async () => { + let resolve!: (value: CodexHomeStatus) => void; + vi.mocked(validateCodexHome).mockReturnValue(new Promise((done) => { resolve = done; })); + const { result } = renderHook(() => useCodexHomeSetting(null)); + let pending!: Promise; + act(() => { pending = result.current.validate(); }); + act(() => result.current.updateDraft("/other/home")); + await act(async () => { resolve(selected); await pending; }); + expect(result.current.validatedHome).toBeNull(); + expect(result.current.validating).toBe(false); + }); + + it("keeps failed validation from producing a usable selection", async () => { + vi.mocked(validateCodexHome).mockRejectedValue(new Error("Selected home is inaccessible")); + const { result } = renderHook(() => useCodexHomeSetting(null)); + await act(async () => { expect(await result.current.validate()).toBeNull(); }); + expect(result.current.error).toContain("inaccessible"); + expect(result.current.validatedHome).toBeNull(); + }); +}); diff --git a/src/features/settings/hooks/useCodexHomeSetting.ts b/src/features/settings/hooks/useCodexHomeSetting.ts new file mode 100644 index 0000000000..ed9c93e42a --- /dev/null +++ b/src/features/settings/hooks/useCodexHomeSetting.ts @@ -0,0 +1,69 @@ +import { useCallback, useEffect, useRef, useState } from "react"; +import type { CodexHomeStatus } from "@/types"; +import { getCodexHome, validateCodexHome } from "@services/tauri"; + +export function useCodexHomeSetting( + configuredHome: string | null | undefined, + enabled = true, +) { + const [draft, setDraft] = useState(configuredHome ?? ""); + const [effectiveHome, setEffectiveHome] = useState(null); + const [validatedHome, setValidatedHome] = useState(null); + const [error, setError] = useState(null); + const [validating, setValidating] = useState(false); + const requestRef = useRef(0); + + useEffect(() => { + let cancelled = false; + requestRef.current += 1; + setDraft(configuredHome ?? ""); + setEffectiveHome(null); + setValidatedHome(null); + setError(null); + setValidating(false); + if (!enabled) return; + void getCodexHome().then( + (status) => { + if (!cancelled) setEffectiveHome(status); + }, + (failure: unknown) => { + if (!cancelled) { + setError(failure instanceof Error ? failure.message : String(failure)); + } + }, + ); + return () => { cancelled = true; requestRef.current += 1; }; + }, [configuredHome, enabled]); + + const updateDraft = useCallback((value: string) => { + requestRef.current += 1; + setDraft(value); + setValidatedHome(null); + setError(null); + setValidating(false); + }, []); + + const validate = useCallback(async () => { + if (!enabled) return null; + const request = ++requestRef.current; + setValidating(true); + setValidatedHome(null); + setError(null); + try { + const status = await validateCodexHome(draft.trim() || null); + if (request !== requestRef.current) return null; + setValidatedHome(status); + if ((draft.trim() || null) === (configuredHome ?? null)) setEffectiveHome(status); + return status; + } catch (failure) { + if (request === requestRef.current) { + setError(failure instanceof Error ? failure.message : String(failure)); + } + return null; + } finally { + if (request === requestRef.current) setValidating(false); + } + }, [configuredHome, draft, enabled]); + + return { draft, updateDraft, effectiveHome, validatedHome, error, validating, validate }; +} diff --git a/src/features/settings/hooks/useGlobalAgentsMd.ts b/src/features/settings/hooks/useGlobalAgentsMd.ts index 5eb44e3e03..77b2b077b9 100644 --- a/src/features/settings/hooks/useGlobalAgentsMd.ts +++ b/src/features/settings/hooks/useGlobalAgentsMd.ts @@ -1,9 +1,9 @@ import { readGlobalAgentsMd, writeGlobalAgentsMd } from "@services/tauri"; import { useFileEditor } from "@/features/shared/hooks/useFileEditor"; -export function useGlobalAgentsMd() { +export function useGlobalAgentsMd(codexHome?: string | null) { return useFileEditor({ - key: "global-agents", + key: `global-agents:${codexHome ?? "default"}`, read: readGlobalAgentsMd, write: writeGlobalAgentsMd, readErrorTitle: "Couldn’t load global AGENTS.md", diff --git a/src/features/settings/hooks/useGlobalCodexConfigToml.ts b/src/features/settings/hooks/useGlobalCodexConfigToml.ts index f6743ef66c..592dd59a81 100644 --- a/src/features/settings/hooks/useGlobalCodexConfigToml.ts +++ b/src/features/settings/hooks/useGlobalCodexConfigToml.ts @@ -1,9 +1,9 @@ import { readGlobalCodexConfigToml, writeGlobalCodexConfigToml } from "@services/tauri"; import { useFileEditor } from "@/features/shared/hooks/useFileEditor"; -export function useGlobalCodexConfigToml() { +export function useGlobalCodexConfigToml(codexHome?: string | null) { return useFileEditor({ - key: "global-config", + key: `global-config:${codexHome ?? "default"}`, read: readGlobalCodexConfigToml, write: writeGlobalCodexConfigToml, readErrorTitle: "Couldn’t load global config.toml", diff --git a/src/features/settings/hooks/useSettingsCodexSection.ts b/src/features/settings/hooks/useSettingsCodexSection.ts index 46ccbf000a..5c7d5936df 100644 --- a/src/features/settings/hooks/useSettingsCodexSection.ts +++ b/src/features/settings/hooks/useSettingsCodexSection.ts @@ -10,8 +10,10 @@ import type { import { useGlobalAgentsMd } from "./useGlobalAgentsMd"; import { useGlobalCodexConfigToml } from "./useGlobalCodexConfigToml"; import { useSettingsDefaultModels } from "./useSettingsDefaultModels"; +import { useCodexHomeSetting } from "./useCodexHomeSetting"; import { buildEditorContentMeta } from "@settings/components/settingsViewHelpers"; import { normalizeCodexArgsInput } from "@/utils/codexArgsInput"; +import { getCodexConfigPath } from "@services/tauri"; type UseSettingsCodexSectionArgs = { appSettings: AppSettings; @@ -37,6 +39,18 @@ export type SettingsCodexSectionProps = { onRefreshDefaultModels: () => void; codexPathDraft: string; codexArgsDraft: string; + codexHomeDraft: string; + codexHomeEffectivePath: string | null; + globalConfigPath: string | null; + globalAgentsPath: string | null; + codexHomeSource: string | null; + codexHomeValidatedPath: string | null; + codexHomeError: string | null; + codexHomeValidating: boolean; + codexSettingsError: string | null; + onSetCodexHomeDraft: (value: string) => void; + onBrowseCodexHome: () => Promise; + onValidateCodexHome: () => Promise; codexDirty: boolean; isSavingSettings: boolean; doctorState: { @@ -85,6 +99,24 @@ export const useSettingsCodexSection = ({ const [codexPathDraft, setCodexPathDraft] = useState(appSettings.codexBin ?? ""); const [codexArgsDraft, setCodexArgsDraft] = useState(appSettings.codexArgs ?? ""); const [isSavingSettings, setIsSavingSettings] = useState(false); + const [codexSettingsError, setCodexSettingsError] = useState(null); + const home = useCodexHomeSetting(appSettings.codexHome, appSettings.backendMode !== "remote"); + const [remoteConfigPath, setRemoteConfigPath] = useState(null); + useEffect(() => { + let active = true; + setRemoteConfigPath(null); + if (appSettings.backendMode === "remote") { + void getCodexConfigPath().then( + (path) => { if (active) setRemoteConfigPath(path); }, + () => {}, + ); + } + return () => { active = false; }; + }, [appSettings.backendMode, appSettings.activeRemoteBackendId, appSettings.remoteBackendHost]); + const globalConfigPath = appSettings.backendMode === "remote" + ? remoteConfigPath + : home.effectiveHome ? `${home.effectiveHome.effectivePath.replace(/[\\/]$/, "")}/config.toml` : null; + const globalAgentsPath = globalConfigPath?.replace(/config\.toml$/, "AGENTS.md") ?? null; const [doctorState, setDoctorState] = useState<{ status: "idle" | "running" | "done"; result: CodexDoctorResult | null; @@ -113,7 +145,7 @@ export const useSettingsCodexSection = ({ setContent: setGlobalAgentsContent, refresh: refreshGlobalAgents, save: saveGlobalAgents, - } = useGlobalAgentsMd(); + } = useGlobalAgentsMd(appSettings.codexHome); const { content: globalConfigContent, @@ -126,7 +158,7 @@ export const useSettingsCodexSection = ({ setContent: setGlobalConfigContent, refresh: refreshGlobalConfig, save: saveGlobalConfig, - } = useGlobalCodexConfigToml(); + } = useGlobalCodexConfigToml(appSettings.codexHome); const globalAgentsEditorMeta = buildEditorContentMeta({ isLoading: globalAgentsLoading, @@ -154,9 +186,11 @@ export const useSettingsCodexSection = ({ const nextCodexBin = codexPathDraft.trim() ? codexPathDraft.trim() : null; const nextCodexArgs = normalizeCodexArgsInput(codexArgsDraft); + const nextCodexHome = home.draft.trim() || null; + const homeChanged = nextCodexHome !== (appSettings.codexHome ?? null); const codexDirty = nextCodexBin !== (appSettings.codexBin ?? null) || - nextCodexArgs !== (appSettings.codexArgs ?? null); + nextCodexArgs !== (appSettings.codexArgs ?? null) || homeChanged; const handleBrowseCodex = async () => { const selection = await open({ multiple: false, directory: false }); @@ -166,14 +200,30 @@ export const useSettingsCodexSection = ({ setCodexPathDraft(selection); }; + const handleBrowseCodexHome = async () => { + const selection = await open({ multiple: false, directory: true, title: "Select Codex Home" }); + if (typeof selection === "string") home.updateDraft(selection); + }; + const handleSaveCodexSettings = async () => { setIsSavingSettings(true); + setCodexSettingsError(null); try { + if (homeChanged && appSettings.backendMode === "remote") { + setCodexSettingsError("Switch to the local backend to change Codex Home."); + return; + } + if (homeChanged && !(await home.validate())) return; await onUpdateAppSettings({ ...appSettings, codexBin: nextCodexBin, codexArgs: nextCodexArgs, + codexHome: nextCodexHome, }); + // Rebootstrap every session and provider cache against the newly saved home. + if (homeChanged) window.location.reload(); + } catch (error) { + setCodexSettingsError(error instanceof Error ? error.message : String(error)); } finally { setIsSavingSettings(false); } @@ -253,6 +303,18 @@ export const useSettingsCodexSection = ({ }, codexPathDraft, codexArgsDraft, + codexHomeDraft: home.draft, + codexHomeEffectivePath: home.effectiveHome?.effectivePath ?? null, + globalConfigPath, + globalAgentsPath, + codexHomeSource: home.effectiveHome?.source ?? null, + codexHomeValidatedPath: home.validatedHome?.effectivePath ?? null, + codexHomeError: home.error, + codexHomeValidating: home.validating, + codexSettingsError, + onSetCodexHomeDraft: home.updateDraft, + onBrowseCodexHome: handleBrowseCodexHome, + onValidateCodexHome: async () => { await home.validate(); }, codexDirty, isSavingSettings, doctorState, diff --git a/src/services/tauri.test.ts b/src/services/tauri.test.ts index eb411228a7..f7c3393792 100644 --- a/src/services/tauri.test.ts +++ b/src/services/tauri.test.ts @@ -10,6 +10,8 @@ import { fetchGit, forkThread, getAppsList, + getCodexHome, + validateCodexHome, getAgentsSettings, getExperimentalFeatureList, getGitHubIssues, @@ -70,6 +72,17 @@ vi.mock("@tauri-apps/plugin-notification", () => ({ sendNotification: vi.fn(), })); +describe("Codex Home IPC", () => { + it("queries the saved home and validates explicit and default selections", async () => { + await getCodexHome(); + expect(invoke).toHaveBeenCalledWith("get_codex_home"); + await validateCodexHome("/selected/home"); + expect(invoke).toHaveBeenCalledWith("validate_codex_home", { codexHome: "/selected/home" }); + await validateCodexHome(null); + expect(invoke).toHaveBeenCalledWith("validate_codex_home", { codexHome: null }); + }); +}); + describe("tauri invoke wrappers", () => { beforeEach(() => { vi.clearAllMocks(); diff --git a/src/services/tauri.ts b/src/services/tauri.ts index 029e0d31bd..1eda2610f1 100644 --- a/src/services/tauri.ts +++ b/src/services/tauri.ts @@ -3,6 +3,7 @@ import { open, save } from "@tauri-apps/plugin-dialog"; import type { Options as NotificationOptions } from "@tauri-apps/plugin-notification"; import type { AppSettings, + CodexHomeStatus, CodexUpdateResult, CodexDoctorResult, DictationModelStatus, @@ -861,6 +862,14 @@ export async function movePrompt( }); } +export async function getCodexHome(): Promise { + return invoke("get_codex_home"); +} + +export async function validateCodexHome(codexHome: string | null): Promise { + return invoke("validate_codex_home", { codexHome }); +} + export async function getAppSettings(): Promise { return invoke("get_app_settings"); } diff --git a/src/types.ts b/src/types.ts index 51b1515c9b..c9736382bf 100644 --- a/src/types.ts +++ b/src/types.ts @@ -235,8 +235,15 @@ export type OpenAppTarget = { args: string[]; }; +export type CodexHomeStatus = { + configuredPath: string | null; + effectivePath: string; + source: "application" | "environment" | "default"; +}; + export type AppSettings = { codexBin: string | null; + codexHome?: string | null; codexArgs: string | null; backendMode: BackendMode; remoteBackendProvider: RemoteBackendProvider;