diff --git a/codex-rs/config/src/types.rs b/codex-rs/config/src/types.rs index 7f9ce0c909..eb3aaeb3b5 100644 --- a/codex-rs/config/src/types.rs +++ b/codex-rs/config/src/types.rs @@ -584,9 +584,14 @@ const fn default_true() -> bool { pub struct ExternalConfigMigrationPrompts { /// Tracks whether home-level external config migration prompts are hidden. pub home: Option, + /// Tracks the last time the home-level external config migration prompt was shown. + pub home_last_prompted_at: Option, /// Tracks which project paths have opted out of external config migration prompts. #[serde(default)] pub projects: BTreeMap, + /// Tracks the last time a project-level external config migration prompt was shown. + #[serde(default)] + pub project_last_prompted_at: BTreeMap, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema)] diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 849c9ca690..2085cfe01e 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -641,6 +641,20 @@ "description": "Tracks whether home-level external config migration prompts are hidden.", "type": "boolean" }, + "home_last_prompted_at": { + "description": "Tracks the last time the home-level external config migration prompt was shown.", + "format": "int64", + "type": "integer" + }, + "project_last_prompted_at": { + "additionalProperties": { + "format": "int64", + "type": "integer" + }, + "default": {}, + "description": "Tracks the last time a project-level external config migration prompt was shown.", + "type": "object" + }, "projects": { "additionalProperties": { "type": "boolean" @@ -1164,6 +1178,8 @@ ], "default": { "home": null, + "home_last_prompted_at": null, + "project_last_prompted_at": {}, "projects": {} }, "description": "Tracks scopes where external config migration prompts should be suppressed." diff --git a/codex-rs/core/src/config/edit.rs b/codex-rs/core/src/config/edit.rs index 40e3a332fd..ac6b2c38ee 100644 --- a/codex-rs/core/src/config/edit.rs +++ b/codex-rs/core/src/config/edit.rs @@ -45,8 +45,12 @@ pub enum ConfigEdit { SetNoticeHideModelMigrationPrompt(String, bool), /// Toggle the home external config migration prompt acknowledgement flag. SetNoticeHideExternalConfigMigrationPromptHome(bool), + /// Record when the home external config migration prompt was last shown. + SetNoticeExternalConfigMigrationPromptHomeLastPromptedAt(i64), /// Toggle the project external config migration prompt acknowledgement flag. SetNoticeHideExternalConfigMigrationPromptProject(String, bool), + /// Record when the project external config migration prompt was last shown. + SetNoticeExternalConfigMigrationPromptProjectLastPromptedAt(String, i64), /// Record that a migration prompt was shown for an old->new model mapping. RecordModelMigrationSeen { from: String, to: String }, /// Replace the entire `[mcp_servers]` table. @@ -425,6 +429,17 @@ impl ConfigDocument { ], value(*acknowledged), )), + ConfigEdit::SetNoticeExternalConfigMigrationPromptHomeLastPromptedAt(timestamp) => { + Ok(self.write_value( + Scope::Global, + &[ + NOTICE_TABLE_KEY, + "external_config_migration_prompts", + "home_last_prompted_at", + ], + value(*timestamp), + )) + } ConfigEdit::SetNoticeHideExternalConfigMigrationPromptProject( project, acknowledged, @@ -438,6 +453,19 @@ impl ConfigDocument { ], value(*acknowledged), )), + ConfigEdit::SetNoticeExternalConfigMigrationPromptProjectLastPromptedAt( + project, + timestamp, + ) => Ok(self.write_value( + Scope::Global, + &[ + NOTICE_TABLE_KEY, + "external_config_migration_prompts", + "project_last_prompted_at", + project.as_str(), + ], + value(*timestamp), + )), ConfigEdit::RecordModelMigrationSeen { from, to } => Ok(self.write_value( Scope::Global, &[NOTICE_TABLE_KEY, "model_migrations", from.as_str()], diff --git a/codex-rs/core/src/config/edit_tests.rs b/codex-rs/core/src/config/edit_tests.rs index 454dd4f088..897778a9fe 100644 --- a/codex-rs/core/src/config/edit_tests.rs +++ b/codex-rs/core/src/config/edit_tests.rs @@ -615,6 +615,67 @@ existing = "value" assert_eq!(contents, expected); } +#[test] +fn blocking_set_external_config_migration_prompt_home_last_prompted_at_preserves_table() { + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path(); + std::fs::write( + codex_home.join(CONFIG_TOML_FILE), + r#"[notice] +existing = "value" +"#, + ) + .expect("seed"); + apply_blocking( + codex_home, + /*profile*/ None, + &[ConfigEdit::SetNoticeExternalConfigMigrationPromptHomeLastPromptedAt(1_760_000_000)], + ) + .expect("persist"); + + let contents = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); + let expected = r#"[notice] +existing = "value" + +[notice.external_config_migration_prompts] +home_last_prompted_at = 1760000000 +"#; + assert_eq!(contents, expected); +} + +#[test] +fn blocking_set_external_config_migration_prompt_project_last_prompted_at_preserves_table() { + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path(); + std::fs::write( + codex_home.join(CONFIG_TOML_FILE), + r#"[notice] +existing = "value" +"#, + ) + .expect("seed"); + apply_blocking( + codex_home, + /*profile*/ None, + &[ + ConfigEdit::SetNoticeExternalConfigMigrationPromptProjectLastPromptedAt( + "/Users/alexsong/code/skills".to_string(), + 1_760_000_000, + ), + ], + ) + .expect("persist"); + + let contents = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); + let expected = r#"[notice] +existing = "value" + +[notice.external_config_migration_prompts.project_last_prompted_at] +"/Users/alexsong/code/skills" = 1760000000 +"#; + assert_eq!(contents, expected); +} + #[test] fn blocking_replace_mcp_servers_round_trips() { let tmp = tempdir().expect("tmpdir"); diff --git a/codex-rs/core/src/external_agent_config.rs b/codex-rs/core/src/external_agent_config.rs index 7cc7888012..bf86acdeb1 100644 --- a/codex-rs/core/src/external_agent_config.rs +++ b/codex-rs/core/src/external_agent_config.rs @@ -1,13 +1,16 @@ use crate::plugins::MarketplaceAddRequest; +use crate::plugins::MarketplacePluginInstallPolicy; use crate::plugins::PluginId; use crate::plugins::PluginInstallRequest; use crate::plugins::PluginsManager; use crate::plugins::add_marketplace; -<<<<<<< HEAD -use crate::plugins::validate_marketplace_add_source; -use codex_utils_absolute_path::AbsolutePathBuf; -======= ->>>>>>> main +use crate::plugins::find_marketplace_manifest_path; +use crate::plugins::load_marketplace; +use crate::plugins::marketplace_install_root; +use crate::plugins::parse_marketplace_source; +use crate::plugins::resolve_configured_marketplace_root; +use crate::plugins::validate_plugin_segment; +use codex_protocol::protocol::Product; use serde_json::Value as JsonValue; use std::collections::BTreeMap; use std::collections::HashSet; @@ -20,8 +23,6 @@ use toml::Value as TomlValue; const EXTERNAL_AGENT_CONFIG_DETECT_METRIC: &str = "codex.external_agent_config.detect"; const EXTERNAL_AGENT_CONFIG_IMPORT_METRIC: &str = "codex.external_agent_config.import"; -// Installed marketplace roots always expose their manifest at this relative path. -const INSTALLED_MARKETPLACE_MANIFEST_RELATIVE_PATH: &str = ".agents/plugins/marketplace.json"; #[derive(Debug, Clone, PartialEq, Eq)] pub struct ExternalAgentConfigDetectOptions { @@ -167,8 +168,8 @@ impl ExternalAgentConfigService { repo_root: Option<&Path>, items: &mut Vec, ) -> io::Result<()> { - let configured_enabled_plugins = - configured_enabled_plugin_ids(&self.codex_home, repo_root)?; + let configured_plugin_ids = configured_plugin_ids(&self.codex_home)?; + let configured_marketplace_plugins = configured_marketplace_plugins(&self.codex_home)?; let cwd = repo_root.map(Path::to_path_buf); let source_settings = repo_root.map_or_else( || self.external_agent_home.join("settings.json"), @@ -219,7 +220,8 @@ impl ExternalAgentConfigService { source_settings.as_path(), cwd.clone(), settings.as_ref(), - &configured_enabled_plugins, + &configured_plugin_ids, + &configured_marketplace_plugins, items, ); @@ -236,7 +238,7 @@ impl ExternalAgentConfigService { items.push(ExternalAgentConfigMigrationItem { item_type: ExternalAgentConfigMigrationItemType::Skills, description: format!( - "Copy skill folders from {} to {}", + "Migrate skills from {} to {}", source_skills.display(), target_skills.display() ), @@ -295,11 +297,16 @@ impl ExternalAgentConfigService { source_settings: &Path, cwd: Option, settings: Option<&JsonValue>, - configured_enabled_plugins: &HashSet, + configured_plugin_ids: &HashSet, + configured_marketplace_plugins: &BTreeMap>, items: &mut Vec, ) { let Some(plugin_details) = settings.and_then(|settings| { - extract_plugin_migration_details(settings, configured_enabled_plugins) + extract_plugin_migration_details( + settings, + configured_plugin_ids, + configured_marketplace_plugins, + ) }) else { return; }; @@ -336,8 +343,13 @@ impl ExternalAgentConfigService { .iter() .map(|plugin_name| format!("{plugin_name}@{marketplace_name}")) .collect::>(); - let import_source = - read_marketplace_import_source(cwd, &self.external_agent_home, &marketplace_name)?; + let source_settings = cwd.map_or_else( + || self.external_agent_home.join("settings.json"), + |cwd| cwd.join(".claude").join("settings.json"), + ); + let import_source = read_external_settings(&source_settings)?.and_then(|settings| { + collect_marketplace_import_sources(&settings).remove(&marketplace_name) + }); let Some(import_source) = import_source else { outcome.failed_marketplaces.push(marketplace_name); outcome.failed_plugin_ids.extend(plugin_ids); @@ -351,12 +363,17 @@ impl ExternalAgentConfigService { let add_marketplace_outcome = add_marketplace(self.codex_home.clone(), request).await; let marketplace_path = match add_marketplace_outcome { Ok(add_marketplace_outcome) => { + let Some(marketplace_path) = find_marketplace_manifest_path( + add_marketplace_outcome.installed_root.as_path(), + ) else { + outcome.failed_marketplaces.push(marketplace_name); + outcome.failed_plugin_ids.extend(plugin_ids); + continue; + }; outcome .succeeded_marketplaces .push(marketplace_name.clone()); - add_marketplace_outcome - .installed_root - .join(INSTALLED_MARKETPLACE_MANIFEST_RELATIVE_PATH) + marketplace_path } Err(_) => { outcome.failed_marketplaces.push(marketplace_name); @@ -527,25 +544,32 @@ fn read_external_settings(path: &Path) -> io::Result> { fn extract_plugin_migration_details( settings: &JsonValue, - configured_enabled_plugins: &HashSet, + configured_plugin_ids: &HashSet, + configured_marketplace_plugins: &BTreeMap>, ) -> Option { let loadable_marketplaces = collect_marketplace_import_sources(settings) .into_iter() .filter_map(|(marketplace_name, source)| { - validate_marketplace_add_source(&source.source, source.ref_name) + parse_marketplace_source(&source.source, source.ref_name) .ok() - .map(|()| marketplace_name) + .map(|_| marketplace_name) }) .collect::>(); let mut plugins = BTreeMap::new(); for plugin_id in collect_enabled_plugins(settings) .into_iter() - .filter(|plugin_id| !configured_enabled_plugins.contains(plugin_id)) + .filter(|plugin_id| !configured_plugin_ids.contains(plugin_id)) { let Ok(plugin_id) = PluginId::parse(&plugin_id) else { continue; }; - if !loadable_marketplaces.contains(&plugin_id.marketplace_name) { + if let Some(installable_plugins) = + configured_marketplace_plugins.get(&plugin_id.marketplace_name) + { + if !installable_plugins.contains(&plugin_id.plugin_name) { + continue; + } + } else if !loadable_marketplaces.contains(&plugin_id.marketplace_name) { continue; } let plugin_group = plugins @@ -596,47 +620,105 @@ fn collect_enabled_plugins(settings: &JsonValue) -> Vec { .collect() } -fn configured_enabled_plugin_ids( - codex_home: &Path, - repo_root: Option<&Path>, -) -> io::Result> { +fn configured_plugin_ids(codex_home: &Path) -> io::Result> { let mut plugin_states = BTreeMap::new(); - for config_path in [ - Some(codex_home.join("config.toml")), - repo_root.map(|repo_root| repo_root.join(".codex").join("config.toml")), - ] - .into_iter() - .flatten() - { - if !config_path.is_file() { - continue; - } - - let raw_config = fs::read_to_string(&config_path)?; - if raw_config.trim().is_empty() { - continue; - } - - let config = toml::from_str::(&raw_config) - .map_err(|err| invalid_data_error(format!("invalid config.toml: {err}")))?; - let Some(plugins) = config.get("plugins").and_then(TomlValue::as_table) else { - continue; - }; - - for (plugin_id, enabled) in plugins.iter().filter_map(|(plugin_id, plugin_config)| { - plugin_config - .get("enabled") - .and_then(TomlValue::as_bool) - .map(|enabled| (plugin_id, enabled)) - }) { - plugin_states.insert(plugin_id.clone(), enabled); - } + let config_path = codex_home.join("config.toml"); + if !config_path.is_file() { + return Ok(HashSet::new()); } - Ok(plugin_states - .into_iter() - .filter_map(|(plugin_id, enabled)| enabled.then_some(plugin_id)) - .collect()) + let raw_config = fs::read_to_string(&config_path)?; + if raw_config.trim().is_empty() { + return Ok(HashSet::new()); + } + + let config = toml::from_str::(&raw_config) + .map_err(|err| invalid_data_error(format!("invalid config.toml: {err}")))?; + let Some(plugins) = config.get("plugins").and_then(TomlValue::as_table) else { + return Ok(HashSet::new()); + }; + + for (plugin_id, enabled) in plugins.iter().filter_map(|(plugin_id, plugin_config)| { + plugin_config + .get("enabled") + .and_then(TomlValue::as_bool) + .map(|enabled| (plugin_id, enabled)) + }) { + plugin_states.insert(plugin_id.clone(), enabled); + } + + Ok(plugin_states.into_keys().collect()) +} + +fn configured_marketplace_plugins( + codex_home: &Path, +) -> io::Result>> { + let default_install_root = marketplace_install_root(codex_home); + let mut marketplace_roots = BTreeMap::new(); + let config_path = codex_home.join("config.toml"); + if !config_path.is_file() { + return Ok(BTreeMap::new()); + } + + let raw_config = fs::read_to_string(&config_path)?; + if raw_config.trim().is_empty() { + return Ok(BTreeMap::new()); + } + + let config = toml::from_str::(&raw_config) + .map_err(|err| invalid_data_error(format!("invalid config.toml: {err}")))?; + let Some(marketplaces) = config.get("marketplaces").and_then(TomlValue::as_table) else { + return Ok(BTreeMap::new()); + }; + + for (marketplace_name, marketplace) in marketplaces { + if validate_plugin_segment(marketplace_name, "marketplace name").is_err() + || !marketplace.is_table() + { + continue; + } + let Some(root) = resolve_configured_marketplace_root( + marketplace_name, + marketplace, + &default_install_root, + ) else { + continue; + }; + let root = if root.is_relative() { + config_path + .parent() + .unwrap_or_else(|| Path::new(".")) + .join(root) + } else { + root + }; + marketplace_roots.insert(marketplace_name.clone(), root); + } + + let mut marketplace_plugins = BTreeMap::new(); + for (marketplace_name, root) in marketplace_roots { + let Some(manifest_path) = find_marketplace_manifest_path(&root) else { + continue; + }; + let Ok(marketplace) = load_marketplace(&manifest_path) else { + continue; + }; + let plugins = + marketplace + .plugins + .into_iter() + .filter(|plugin| { + plugin.policy.installation != MarketplacePluginInstallPolicy::NotAvailable + && plugin.policy.products.as_deref().is_none_or(|products| { + Product::Codex.matches_product_restriction(products) + }) + }) + .map(|plugin| plugin.name) + .collect::>(); + marketplace_plugins.insert(marketplace_name, plugins); + } + + Ok(marketplace_plugins) } fn collect_marketplace_import_sources( @@ -691,22 +773,6 @@ struct MarketplaceImportSource { ref_name: Option, } -fn read_marketplace_import_source( - cwd: Option<&Path>, - external_agent_home: &Path, - marketplace_name: &str, -) -> io::Result> { - let source_settings = cwd.map_or_else( - || external_agent_home.join("settings.json"), - |cwd| cwd.join(".claude").join("settings.json"), - ); - let Some(settings) = read_external_settings(&source_settings)? else { - return Ok(None); - }; - - Ok(collect_marketplace_import_sources(&settings).remove(marketplace_name)) -} - fn find_repo_root(cwd: Option<&Path>) -> io::Result> { let Some(cwd) = cwd.filter(|cwd| !cwd.as_os_str().is_empty()) else { return Ok(None); diff --git a/codex-rs/core/src/external_agent_config_tests.rs b/codex-rs/core/src/external_agent_config_tests.rs index bdcf21ed95..0ee2da0df8 100644 --- a/codex-rs/core/src/external_agent_config_tests.rs +++ b/codex-rs/core/src/external_agent_config_tests.rs @@ -59,7 +59,7 @@ fn detect_home_lists_config_skills_and_agents_md() { ExternalAgentConfigMigrationItem { item_type: ExternalAgentConfigMigrationItemType::Skills, description: format!( - "Copy skill folders from {} to {}", + "Migrate skills from {} to {}", claude_home.join("skills").display(), agents_skills.display() ), @@ -455,7 +455,7 @@ fn detect_home_lists_enabled_plugins_from_settings() { } #[test] -fn detect_repo_skips_plugins_that_are_already_enabled_in_codex() { +fn detect_repo_skips_plugins_that_are_already_configured_in_codex() { let root = TempDir::new().expect("create tempdir"); let claude_home = root.path().join(".claude"); let codex_home = root.path().join(".codex"); @@ -513,6 +513,48 @@ enabled = true ); } +#[test] +fn detect_repo_skips_plugins_that_are_disabled_in_codex() { + let root = TempDir::new().expect("create tempdir"); + let claude_home = root.path().join(".claude"); + let codex_home = root.path().join(".codex"); + let repo_root = root.path().join("repo"); + fs::create_dir_all(repo_root.join(".git")).expect("create git dir"); + fs::create_dir_all(repo_root.join(".claude")).expect("create repo claude dir"); + fs::create_dir_all(&codex_home).expect("create codex home"); + fs::write( + repo_root.join(".claude").join("settings.json"), + r#"{ + "enabledPlugins": { + "formatter@acme-tools": true + }, + "extraKnownMarketplaces": { + "acme-tools": { + "source": "acme-corp/claude-plugins" + } + } + }"#, + ) + .expect("write repo settings"); + fs::write( + codex_home.join("config.toml"), + r#" +[plugins."formatter@acme-tools"] +enabled = false +"#, + ) + .expect("write codex config"); + + let items = service_for_paths(claude_home, codex_home) + .detect(ExternalAgentConfigDetectOptions { + include_home: false, + cwds: Some(vec![repo_root]), + }) + .expect("detect"); + + assert_eq!(items, Vec::::new()); +} + #[tokio::test] async fn import_plugins_requires_details() { let (_root, claude_home, codex_home) = fixture_paths(); @@ -526,6 +568,65 @@ async fn import_plugins_requires_details() { assert_eq!(err.to_string(), "plugins migration item is missing details"); } +#[test] +fn detect_repo_does_not_skip_plugins_only_configured_in_project_codex() { + let root = TempDir::new().expect("create tempdir"); + let claude_home = root.path().join(".claude"); + let codex_home = root.path().join(".codex"); + let repo_root = root.path().join("repo"); + fs::create_dir_all(repo_root.join(".git")).expect("create git dir"); + fs::create_dir_all(repo_root.join(".claude")).expect("create repo claude dir"); + fs::create_dir_all(repo_root.join(".codex")).expect("create repo codex dir"); + fs::create_dir_all(&codex_home).expect("create codex home"); + fs::write( + repo_root.join(".claude").join("settings.json"), + r#"{ + "enabledPlugins": { + "formatter@acme-tools": true + }, + "extraKnownMarketplaces": { + "acme-tools": { + "source": "acme-corp/claude-plugins" + } + } + }"#, + ) + .expect("write repo settings"); + fs::write( + repo_root.join(".codex").join("config.toml"), + r#" +[plugins."formatter@acme-tools"] +enabled = true +"#, + ) + .expect("write project codex config"); + + let items = service_for_paths(claude_home, codex_home) + .detect(ExternalAgentConfigDetectOptions { + include_home: false, + cwds: Some(vec![repo_root.clone()]), + }) + .expect("detect"); + + assert_eq!( + items, + vec![ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::Plugins, + description: format!( + "Import enabled plugins from {}", + repo_root.join(".claude").join("settings.json").display() + ), + cwd: Some(repo_root), + details: Some(MigrationDetails { + plugins: vec![PluginsMigration { + marketplace_name: "acme-tools".to_string(), + plugin_names: vec!["formatter".to_string()], + }], + }), + }] + ); +} + #[test] fn detect_home_skips_plugins_without_marketplace_source() { let (_root, claude_home, codex_home) = fixture_paths(); @@ -579,6 +680,130 @@ fn detect_home_skips_plugins_with_invalid_marketplace_source() { assert_eq!(items, Vec::::new()); } +#[test] +fn detect_repo_filters_plugins_against_installed_marketplace() { + let root = TempDir::new().expect("create tempdir"); + let claude_home = root.path().join(".claude"); + let codex_home = root.path().join(".codex"); + let repo_root = root.path().join("repo"); + let marketplace_root = codex_home.join(".tmp").join("marketplaces").join("debug"); + fs::create_dir_all(repo_root.join(".git")).expect("create git dir"); + fs::create_dir_all(repo_root.join(".claude")).expect("create repo claude dir"); + fs::create_dir_all(marketplace_root.join(".agents").join("plugins")) + .expect("create marketplace manifest dir"); + fs::create_dir_all( + marketplace_root + .join("plugins") + .join("sample") + .join(".codex-plugin"), + ) + .expect("create sample plugin"); + fs::create_dir_all( + marketplace_root + .join("plugins") + .join("available") + .join(".codex-plugin"), + ) + .expect("create available plugin"); + fs::write( + repo_root.join(".claude").join("settings.json"), + r#"{ + "enabledPlugins": { + "sample@debug": true, + "available@debug": true, + "missing@debug": true + }, + "extraKnownMarketplaces": { + "debug": { + "source": "owner/debug-marketplace" + } + } + }"#, + ) + .expect("write repo settings"); + fs::write( + codex_home.join("config.toml"), + r#" +[marketplaces.debug] +source_type = "git" +source = "owner/debug-marketplace" +"#, + ) + .expect("write codex config"); + fs::write( + marketplace_root + .join(".agents") + .join("plugins") + .join("marketplace.json"), + r#"{ + "name": "debug", + "plugins": [ + { + "name": "sample", + "source": { + "source": "local", + "path": "./plugins/sample" + }, + "policy": { + "installation": "NOT_AVAILABLE" + } + }, + { + "name": "available", + "source": { + "source": "local", + "path": "./plugins/available" + } + } + ] +}"#, + ) + .expect("write marketplace manifest"); + fs::write( + marketplace_root + .join("plugins") + .join("sample") + .join(".codex-plugin") + .join("plugin.json"), + r#"{"name":"sample"}"#, + ) + .expect("write sample plugin manifest"); + fs::write( + marketplace_root + .join("plugins") + .join("available") + .join(".codex-plugin") + .join("plugin.json"), + r#"{"name":"available"}"#, + ) + .expect("write available plugin manifest"); + + let items = service_for_paths(claude_home, codex_home) + .detect(ExternalAgentConfigDetectOptions { + include_home: false, + cwds: Some(vec![repo_root.clone()]), + }) + .expect("detect"); + + assert_eq!( + items, + vec![ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::Plugins, + description: format!( + "Import enabled plugins from {}", + repo_root.join(".claude").join("settings.json").display() + ), + cwd: Some(repo_root), + details: Some(MigrationDetails { + plugins: vec![PluginsMigration { + marketplace_name: "debug".to_string(), + plugin_names: vec!["available".to_string()], + }], + }), + }] + ); +} + #[tokio::test] async fn import_plugins_requires_source_marketplace_details() { let (_root, claude_home, codex_home) = fixture_paths(); @@ -659,6 +884,82 @@ async fn import_plugins_defers_marketplace_source_validation_to_add_marketplace( ); } +#[tokio::test] +async fn import_plugins_supports_claude_plugin_marketplace_layout() { + let (_root, claude_home, codex_home) = fixture_paths(); + let marketplace_root = claude_home.join("my-marketplace"); + let plugin_root = marketplace_root.join("plugins").join("cloudflare"); + fs::create_dir_all(marketplace_root.join(".claude-plugin")) + .expect("create marketplace manifest dir"); + fs::create_dir_all(plugin_root.join(".codex-plugin")).expect("create plugin manifest dir"); + fs::create_dir_all(&codex_home).expect("create codex home"); + + fs::write( + claude_home.join("settings.json"), + format!( + r#"{{ + "enabledPlugins": {{ + "cloudflare@my-plugins": true + }}, + "extraKnownMarketplaces": {{ + "my-plugins": {{ + "source": "local", + "path": "{}" + }} + }} + }}"#, + marketplace_root.display() + ), + ) + .expect("write settings"); + fs::write( + marketplace_root + .join(".claude-plugin") + .join("marketplace.json"), + r#"{ + "name": "my-plugins", + "plugins": [ + { + "name": "cloudflare", + "source": "./plugins/cloudflare" + } + ] + }"#, + ) + .expect("write marketplace manifest"); + fs::write( + plugin_root.join(".codex-plugin").join("plugin.json"), + r#"{"name":"cloudflare","version":"0.1.0"}"#, + ) + .expect("write plugin manifest"); + + let outcome = service_for_paths(claude_home, codex_home.clone()) + .import_plugins( + /*cwd*/ None, + Some(MigrationDetails { + plugins: vec![PluginsMigration { + marketplace_name: "my-plugins".to_string(), + plugin_names: vec!["cloudflare".to_string()], + }], + }), + ) + .await + .expect("import plugins"); + + assert_eq!( + outcome, + PluginImportOutcome { + succeeded_marketplaces: vec!["my-plugins".to_string()], + succeeded_plugin_ids: vec!["cloudflare@my-plugins".to_string()], + failed_marketplaces: Vec::new(), + failed_plugin_ids: Vec::new(), + } + ); + let config = fs::read_to_string(codex_home.join("config.toml")).expect("read config"); + assert!(config.contains(r#"[plugins."cloudflare@my-plugins"]"#)); + assert!(config.contains("enabled = true")); +} + #[test] fn import_skills_returns_only_new_skill_directory_count() { let (_root, claude_home, codex_home) = fixture_paths(); diff --git a/codex-rs/core/src/plugins/marketplace_add.rs b/codex-rs/core/src/plugins/marketplace_add.rs index 245ab9d4dd..38fc04c199 100644 --- a/codex-rs/core/src/plugins/marketplace_add.rs +++ b/codex-rs/core/src/plugins/marketplace_add.rs @@ -20,7 +20,7 @@ use metadata::find_marketplace_root_by_name; use metadata::installed_marketplace_root_for_source; use metadata::record_added_marketplace_entry; use source::MarketplaceSource; -use source::parse_marketplace_source; +pub(crate) use source::parse_marketplace_source; use source::stage_marketplace_source; use source::validate_marketplace_source_root; @@ -56,13 +56,6 @@ pub async fn add_marketplace( .map_err(|err| MarketplaceAddError::Internal(format!("failed to add marketplace: {err}")))? } -pub fn validate_marketplace_add_source( - source: &str, - ref_name: Option, -) -> Result<(), MarketplaceAddError> { - parse_marketplace_source(source, ref_name).map(|_| ()) -} - fn add_marketplace_sync( codex_home: &Path, request: MarketplaceAddRequest, diff --git a/codex-rs/core/src/plugins/marketplace_add/source.rs b/codex-rs/core/src/plugins/marketplace_add/source.rs index 53d52058aa..02af00eb8f 100644 --- a/codex-rs/core/src/plugins/marketplace_add/source.rs +++ b/codex-rs/core/src/plugins/marketplace_add/source.rs @@ -5,7 +5,7 @@ use std::path::Path; use std::path::PathBuf; #[derive(Debug, Clone, PartialEq, Eq)] -pub(super) enum MarketplaceSource { +pub(crate) enum MarketplaceSource { Git { url: String, ref_name: Option, @@ -15,7 +15,7 @@ pub(super) enum MarketplaceSource { }, } -pub(super) fn parse_marketplace_source( +pub(crate) fn parse_marketplace_source( source: &str, explicit_ref: Option, ) -> Result { diff --git a/codex-rs/core/src/plugins/mod.rs b/codex-rs/core/src/plugins/mod.rs index 5b46109bce..010ba00714 100644 --- a/codex-rs/core/src/plugins/mod.rs +++ b/codex-rs/core/src/plugins/mod.rs @@ -31,6 +31,7 @@ pub(crate) use discoverable::list_tool_suggest_discoverable_plugins; pub(crate) use injection::build_plugin_injections; pub use installed_marketplaces::INSTALLED_MARKETPLACES_DIR; pub use installed_marketplaces::marketplace_install_root; +pub(crate) use installed_marketplaces::resolve_configured_marketplace_root; pub use manager::ConfiguredMarketplace; pub use manager::ConfiguredMarketplaceListOutcome; pub use manager::ConfiguredMarketplacePlugin; @@ -58,12 +59,14 @@ pub use marketplace::MarketplacePluginAuthPolicy; pub use marketplace::MarketplacePluginInstallPolicy; pub use marketplace::MarketplacePluginPolicy; pub use marketplace::MarketplacePluginSource; +pub(crate) use marketplace::find_marketplace_manifest_path; +pub(crate) use marketplace::load_marketplace; pub use marketplace::validate_marketplace_root; pub use marketplace_add::MarketplaceAddError; pub use marketplace_add::MarketplaceAddOutcome; pub use marketplace_add::MarketplaceAddRequest; pub use marketplace_add::add_marketplace; -pub use marketplace_add::validate_marketplace_add_source; +pub(crate) use marketplace_add::parse_marketplace_source; pub use remote::RemotePluginFetchError; pub use remote::fetch_remote_featured_plugin_ids; pub(crate) use render::render_explicit_plugin_instructions; diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 41e542fd69..8f8b1811e2 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -158,6 +158,8 @@ use std::sync::atomic::Ordering; use std::thread; use std::time::Duration; use std::time::Instant; +use std::time::SystemTime; +use std::time::UNIX_EPOCH; use tokio::select; use tokio::sync::Mutex; use tokio::sync::mpsc; @@ -849,19 +851,7 @@ async fn prepare_startup_tooltip_override( Some(tooltip_override.message) } -async fn rebuild_startup_config( - config: &Config, - cli_kv_overrides: &[(String, TomlValue)], - harness_overrides: &ConfigOverrides, -) -> Result { - ConfigBuilder::default() - .codex_home(config.codex_home.to_path_buf()) - .cli_overrides(cli_kv_overrides.to_vec()) - .harness_overrides(harness_overrides.clone()) - .build() - .await - .wrap_err("Failed to reload config after external agent migration") -} +const EXTERNAL_CONFIG_MIGRATION_PROMPT_COOLDOWN_SECS: i64 = 5 * 24 * 60 * 60; fn external_config_migration_project_key(path: &Path) -> String { path.display().to_string() @@ -884,13 +874,49 @@ fn is_external_config_migration_scope_hidden(config: &Config, cwd: Option<&Path> } } +fn external_config_migration_last_prompted_at(config: &Config, cwd: Option<&Path>) -> Option { + match cwd { + Some(cwd) => config + .notices + .external_config_migration_prompts + .project_last_prompted_at + .get(&external_config_migration_project_key(cwd)) + .copied(), + None => { + config + .notices + .external_config_migration_prompts + .home_last_prompted_at + } + } +} + +fn is_external_config_migration_scope_cooling_down( + config: &Config, + cwd: Option<&Path>, + now_unix_seconds: i64, +) -> bool { + external_config_migration_last_prompted_at(config, cwd).is_some_and(|last_prompted_at| { + last_prompted_at.saturating_add(EXTERNAL_CONFIG_MIGRATION_PROMPT_COOLDOWN_SECS) + > now_unix_seconds + }) +} + fn visible_external_agent_config_migration_items( config: &Config, items: Vec, + now_unix_seconds: i64, ) -> Vec { items .into_iter() - .filter(|item| !is_external_config_migration_scope_hidden(config, item.cwd.as_deref())) + .filter(|item| { + !is_external_config_migration_scope_hidden(config, item.cwd.as_deref()) + && !is_external_config_migration_scope_cooling_down( + config, + item.cwd.as_deref(), + now_unix_seconds, + ) + }) .collect() } @@ -912,6 +938,70 @@ struct ExternalAgentConfigMigrationPromptResult { success_message: Option, } +fn unix_seconds_now() -> i64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_secs() as i64 +} + +async fn persist_external_agent_config_migration_prompt_shown( + config: &mut Config, + items: &[ExternalAgentConfigMigrationItem], + now_unix_seconds: i64, +) -> Result<()> { + let mut edits = Vec::new(); + if items.iter().any(|item| item.cwd.is_none()) { + edits.push( + ConfigEdit::SetNoticeExternalConfigMigrationPromptHomeLastPromptedAt(now_unix_seconds), + ); + } + + for project in items + .iter() + .filter_map(|item| item.cwd.as_deref()) + .map(external_config_migration_project_key) + { + edits.push( + ConfigEdit::SetNoticeExternalConfigMigrationPromptProjectLastPromptedAt( + project, + now_unix_seconds, + ), + ); + } + + if edits.is_empty() { + return Ok(()); + } + + ConfigEditsBuilder::new(&config.codex_home) + .with_edits(edits) + .apply() + .await + .map_err(|err| color_eyre::eyre::eyre!("{err}")) + .wrap_err("Failed to save external config migration prompt timestamp")?; + + if items.iter().any(|item| item.cwd.is_none()) { + config + .notices + .external_config_migration_prompts + .home_last_prompted_at = Some(now_unix_seconds); + } + for project in items + .iter() + .filter_map(|item| item.cwd.as_deref()) + .map(external_config_migration_project_key) + { + config + .notices + .external_config_migration_prompts + .project_last_prompted_at + .insert(project, now_unix_seconds); + } + + Ok(()) +} + async fn persist_external_agent_config_migration_prompt_dismissal( config: &mut Config, items: &[ExternalAgentConfigMigrationItem], @@ -992,6 +1082,7 @@ async fn handle_external_agent_config_migration_prompt_if_needed( }); } + let now_unix_seconds = unix_seconds_now(); let detected_items = match app_server .external_agent_config_detect(ExternalAgentConfigDetectParams { include_home: true, @@ -999,7 +1090,9 @@ async fn handle_external_agent_config_migration_prompt_if_needed( }) .await { - Ok(response) => visible_external_agent_config_migration_items(config, response.items), + Ok(response) => { + visible_external_agent_config_migration_items(config, response.items, now_unix_seconds) + } Err(err) => { tracing::warn!( error = %err, @@ -1020,6 +1113,20 @@ async fn handle_external_agent_config_migration_prompt_if_needed( }); } + if let Err(err) = persist_external_agent_config_migration_prompt_shown( + config, + &detected_items, + now_unix_seconds, + ) + .await + { + tracing::warn!( + error = %err, + cwd = %config.cwd.display(), + "failed to persist external config migration prompt timestamp" + ); + } + let mut selected_items = detected_items.clone(); let mut error: Option = None; @@ -1038,9 +1145,13 @@ async fn handle_external_agent_config_migration_prompt_if_needed( Ok(_) => { let success_message = external_agent_config_migration_success_message(&selected_items); - *config = - rebuild_startup_config(config, cli_kv_overrides, harness_overrides) - .await?; + *config = ConfigBuilder::default() + .codex_home(config.codex_home.to_path_buf()) + .cli_overrides(cli_kv_overrides.to_vec()) + .harness_overrides(harness_overrides.clone()) + .build() + .await + .wrap_err("Failed to reload config after external agent migration")?; return Ok(ExternalAgentConfigMigrationPromptResult { exit_info: None, success_message: Some(success_message), @@ -10786,6 +10897,7 @@ guardian_approval = true details: None, }, ], + /*now_unix_seconds*/ 1_760_000_000, ); assert_eq!( @@ -10799,6 +10911,89 @@ guardian_approval = true ); } + #[tokio::test] + async fn visible_external_agent_config_migration_items_omits_recently_prompted_scopes() { + let codex_home = tempdir().expect("temp codex home"); + let mut config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .build() + .await + .expect("config"); + config + .notices + .external_config_migration_prompts + .home_last_prompted_at = Some(1_760_000_000); + config + .notices + .external_config_migration_prompts + .project_last_prompted_at + .insert("/tmp/project".to_string(), 1_760_000_000); + + let visible = visible_external_agent_config_migration_items( + &config, + vec![ + ExternalAgentConfigMigrationItem { + item_type: + codex_app_server_protocol::ExternalAgentConfigMigrationItemType::Config, + description: "home".to_string(), + cwd: None, + details: None, + }, + ExternalAgentConfigMigrationItem { + item_type: + codex_app_server_protocol::ExternalAgentConfigMigrationItemType::AgentsMd, + description: "project".to_string(), + cwd: Some(PathBuf::from("/tmp/project")), + details: None, + }, + ExternalAgentConfigMigrationItem { + item_type: + codex_app_server_protocol::ExternalAgentConfigMigrationItemType::Skills, + description: "other project".to_string(), + cwd: Some(PathBuf::from("/tmp/other")), + details: None, + }, + ], + /*now_unix_seconds*/ + 1_760_000_000 + EXTERNAL_CONFIG_MIGRATION_PROMPT_COOLDOWN_SECS - 1, + ); + + assert_eq!( + visible, + vec![ExternalAgentConfigMigrationItem { + item_type: codex_app_server_protocol::ExternalAgentConfigMigrationItemType::Skills, + description: "other project".to_string(), + cwd: Some(PathBuf::from("/tmp/other")), + details: None, + }] + ); + } + + #[tokio::test] + async fn external_config_migration_scope_cooldown_expires_after_five_days() { + let codex_home = tempdir().expect("temp codex home"); + let mut config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .build() + .await + .expect("config"); + config + .notices + .external_config_migration_prompts + .home_last_prompted_at = Some(1_760_000_000); + + assert!(is_external_config_migration_scope_cooling_down( + &config, + /*cwd*/ None, + 1_760_000_000 + EXTERNAL_CONFIG_MIGRATION_PROMPT_COOLDOWN_SECS - 1, + )); + assert!(!is_external_config_migration_scope_cooling_down( + &config, + /*cwd*/ None, + 1_760_000_000 + EXTERNAL_CONFIG_MIGRATION_PROMPT_COOLDOWN_SECS, + )); + } + #[test] fn external_agent_config_migration_success_message_mentions_plugins_when_present() { let message = external_agent_config_migration_success_message(&[ diff --git a/codex-rs/tui/src/external_agent_config_migration.rs b/codex-rs/tui/src/external_agent_config_migration.rs index f246b6b649..1b998d39eb 100644 --- a/codex-rs/tui/src/external_agent_config_migration.rs +++ b/codex-rs/tui/src/external_agent_config_migration.rs @@ -174,7 +174,7 @@ impl ExternalAgentConfigMigrationScreen { } if let Some(reformatted) = - reformat_description(&item.description, "Copy skill folders from ", " to ", cwd) + reformat_description(&item.description, "Migrate skills from ", " to ", cwd) { return reformatted; } diff --git a/codex-rs/tui/src/snapshots/codex_tui__external_agent_config_migration__tests__external_agent_config_migration_prompt.snap.new b/codex-rs/tui/src/snapshots/codex_tui__external_agent_config_migration__tests__external_agent_config_migration_prompt.snap.new deleted file mode 100644 index 337c8f4e4d..0000000000 --- a/codex-rs/tui/src/snapshots/codex_tui__external_agent_config_migration__tests__external_agent_config_migration_prompt.snap.new +++ /dev/null @@ -1,23 +0,0 @@ ---- -source: tui/src/external_agent_config_migration.rs -assertion_line: 773 -expression: rendered ---- - - > Migratable external agent config detected - We found settings from another agent that you can add to this project. - Select what to import - Home - [x] Migrate /Users/alex/.claude/settings.json into /Users/alex/.codex/con… - - Project: /workspace/project - [x] Import enabled plugins from .claude/settings.json - • acme-tools: deployer, formatter - • team-marketplace: asana - [x] Import CLAUDE.md to AGENTS.md - - Selected 3 of 3 item(s). - 1. Proceed with selected - 2. Skip for now - 3. Don't ask again for these locations - Use ↑/↓ to move, space to toggle, 1/2/3 to choose, a/n for all/none diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__mcp_tools_output_lists_tools_for_hyphenated_server_names.snap.new b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__mcp_tools_output_lists_tools_for_hyphenated_server_names.snap.new deleted file mode 100644 index d473d45814..0000000000 --- a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__mcp_tools_output_lists_tools_for_hyphenated_server_names.snap.new +++ /dev/null @@ -1,11 +0,0 @@ ---- -source: tui/src/history_cell.rs -assertion_line: 3367 -expression: rendered ---- -/mcp - -🔌 MCP Tools - - • some-server (disabled) - • Reason: requirements (/etc/codex/requirements.toml) diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__mcp_tools_output_masks_sensitive_values.snap.new b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__mcp_tools_output_masks_sensitive_values.snap.new deleted file mode 100644 index 2dbc3f4d4f..0000000000 --- a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__mcp_tools_output_masks_sensitive_values.snap.new +++ /dev/null @@ -1,14 +0,0 @@ ---- -source: tui/src/history_cell.rs -assertion_line: 3327 -expression: rendered ---- -/mcp - -🔌 MCP Tools - - • docs (disabled) - • Reason: requirements (/etc/codex/requirements.toml) - - • http (disabled) - • Reason: requirements (/etc/codex/requirements.toml)