From e3310f68c658441ef87876e76e068be812d52283 Mon Sep 17 00:00:00 2001 From: charlesgong-openai Date: Sun, 21 Jun 2026 18:00:57 -0700 Subject: [PATCH 1/4] Support npm marketplace plugin sources --- .../codex_app_server_protocol.schemas.json | 32 +++++ .../codex_app_server_protocol.v2.schemas.json | 32 +++++ .../json/v2/PluginInstalledResponse.json | 32 +++++ .../schema/json/v2/PluginListResponse.json | 32 +++++ .../schema/json/v2/PluginReadResponse.json | 32 +++++ .../json/v2/PluginShareListResponse.json | 32 +++++ .../schema/typescript/v2/PluginSource.ts | 2 +- .../src/protocol/v2/plugin.rs | 7 + .../src/protocol/v2/tests.rs | 17 ++- .../src/request_processors/plugins.rs | 11 +- codex-rs/cli/src/plugin_cmd.rs | 30 +++++ codex-rs/core-plugins/src/lib.rs | 1 + codex-rs/core-plugins/src/loader.rs | 17 +++ codex-rs/core-plugins/src/manager.rs | 68 ++++++---- codex-rs/core-plugins/src/marketplace.rs | 115 +++++++++++++++- .../core-plugins/src/marketplace_tests.rs | 53 ++++++++ codex-rs/core-plugins/src/npm_source.rs | 123 ++++++++++++++++++ codex-rs/core-plugins/src/npm_source_tests.rs | 67 ++++++++++ 18 files changed, 673 insertions(+), 30 deletions(-) create mode 100644 codex-rs/core-plugins/src/npm_source.rs create mode 100644 codex-rs/core-plugins/src/npm_source_tests.rs diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index f8f8db58b5dc..501d18cc0ae0 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -13934,6 +13934,38 @@ "title": "GitPluginSource", "type": "object" }, + { + "properties": { + "package": { + "type": "string" + }, + "registry": { + "type": [ + "string", + "null" + ] + }, + "type": { + "enum": [ + "npm" + ], + "title": "NpmPluginSourceType", + "type": "string" + }, + "version": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "package", + "type" + ], + "title": "NpmPluginSource", + "type": "object" + }, { "description": "The plugin is available in the remote catalog. Download metadata is kept server-side and is not exposed through the app-server API.", "properties": { diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 81a79c0c979e..f847954a4407 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -10358,6 +10358,38 @@ "title": "GitPluginSource", "type": "object" }, + { + "properties": { + "package": { + "type": "string" + }, + "registry": { + "type": [ + "string", + "null" + ] + }, + "type": { + "enum": [ + "npm" + ], + "title": "NpmPluginSourceType", + "type": "string" + }, + "version": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "package", + "type" + ], + "title": "NpmPluginSource", + "type": "object" + }, { "description": "The plugin is available in the remote catalog. Download metadata is kept server-side and is not exposed through the app-server API.", "properties": { diff --git a/codex-rs/app-server-protocol/schema/json/v2/PluginInstalledResponse.json b/codex-rs/app-server-protocol/schema/json/v2/PluginInstalledResponse.json index ffe20e8c5554..e5cf2e38fdd5 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/PluginInstalledResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/PluginInstalledResponse.json @@ -395,6 +395,38 @@ "title": "GitPluginSource", "type": "object" }, + { + "properties": { + "package": { + "type": "string" + }, + "registry": { + "type": [ + "string", + "null" + ] + }, + "type": { + "enum": [ + "npm" + ], + "title": "NpmPluginSourceType", + "type": "string" + }, + "version": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "package", + "type" + ], + "title": "NpmPluginSource", + "type": "object" + }, { "description": "The plugin is available in the remote catalog. Download metadata is kept server-side and is not exposed through the app-server API.", "properties": { diff --git a/codex-rs/app-server-protocol/schema/json/v2/PluginListResponse.json b/codex-rs/app-server-protocol/schema/json/v2/PluginListResponse.json index d756f61a6847..faf367f055c9 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/PluginListResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/PluginListResponse.json @@ -395,6 +395,38 @@ "title": "GitPluginSource", "type": "object" }, + { + "properties": { + "package": { + "type": "string" + }, + "registry": { + "type": [ + "string", + "null" + ] + }, + "type": { + "enum": [ + "npm" + ], + "title": "NpmPluginSourceType", + "type": "string" + }, + "version": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "package", + "type" + ], + "title": "NpmPluginSource", + "type": "object" + }, { "description": "The plugin is available in the remote catalog. Download metadata is kept server-side and is not exposed through the app-server API.", "properties": { diff --git a/codex-rs/app-server-protocol/schema/json/v2/PluginReadResponse.json b/codex-rs/app-server-protocol/schema/json/v2/PluginReadResponse.json index bef85afddd05..ab9b4c9ecb43 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/PluginReadResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/PluginReadResponse.json @@ -535,6 +535,38 @@ "title": "GitPluginSource", "type": "object" }, + { + "properties": { + "package": { + "type": "string" + }, + "registry": { + "type": [ + "string", + "null" + ] + }, + "type": { + "enum": [ + "npm" + ], + "title": "NpmPluginSourceType", + "type": "string" + }, + "version": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "package", + "type" + ], + "title": "NpmPluginSource", + "type": "object" + }, { "description": "The plugin is available in the remote catalog. Download metadata is kept server-side and is not exposed through the app-server API.", "properties": { diff --git a/codex-rs/app-server-protocol/schema/json/v2/PluginShareListResponse.json b/codex-rs/app-server-protocol/schema/json/v2/PluginShareListResponse.json index 7cffdeab9aac..84475e47ae05 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/PluginShareListResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/PluginShareListResponse.json @@ -351,6 +351,38 @@ "title": "GitPluginSource", "type": "object" }, + { + "properties": { + "package": { + "type": "string" + }, + "registry": { + "type": [ + "string", + "null" + ] + }, + "type": { + "enum": [ + "npm" + ], + "title": "NpmPluginSourceType", + "type": "string" + }, + "version": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "package", + "type" + ], + "title": "NpmPluginSource", + "type": "object" + }, { "description": "The plugin is available in the remote catalog. Download metadata is kept server-side and is not exposed through the app-server API.", "properties": { diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/PluginSource.ts b/codex-rs/app-server-protocol/schema/typescript/v2/PluginSource.ts index f6e867195d6f..c55231293015 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/PluginSource.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/PluginSource.ts @@ -3,4 +3,4 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { AbsolutePathBuf } from "../AbsolutePathBuf"; -export type PluginSource = { "type": "local", path: AbsolutePathBuf, } | { "type": "git", url: string, path: string | null, refName: string | null, sha: string | null, } | { "type": "remote" }; +export type PluginSource = { "type": "local", path: AbsolutePathBuf, } | { "type": "git", url: string, path: string | null, refName: string | null, sha: string | null, } | { "type": "npm", package: string, version: string | null, registry: string | null, } | { "type": "remote" }; diff --git a/codex-rs/app-server-protocol/src/protocol/v2/plugin.rs b/codex-rs/app-server-protocol/src/protocol/v2/plugin.rs index 128425d1cef9..4a184e3c5552 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/plugin.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/plugin.rs @@ -740,6 +740,13 @@ pub enum PluginSource { ref_name: Option, sha: Option, }, + #[serde(rename_all = "camelCase")] + #[ts(rename_all = "camelCase")] + Npm { + package: String, + version: Option, + registry: Option, + }, /// The plugin is available in the remote catalog. Download metadata is /// kept server-side and is not exposed through the app-server API. Remote, diff --git a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs index 0161cd4ef94a..66bf4bbc2c51 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs @@ -2806,7 +2806,7 @@ fn skills_extra_roots_set_params_rejects_relative_roots() { } #[test] -fn plugin_source_serializes_local_git_and_remote_variants() { +fn plugin_source_serializes_local_git_npm_and_remote_variants() { let local_path = if cfg!(windows) { r"C:\plugins\linear" } else { @@ -2840,6 +2840,21 @@ fn plugin_source_serializes_local_git_and_remote_variants() { }), ); + assert_eq!( + serde_json::to_value(PluginSource::Npm { + package: "@acme/plugin".to_string(), + version: Some("^1.2.0".to_string()), + registry: Some("https://npm.example.com".to_string()), + }) + .unwrap(), + json!({ + "type": "npm", + "package": "@acme/plugin", + "version": "^1.2.0", + "registry": "https://npm.example.com", + }), + ); + assert_eq!( serde_json::to_value(PluginSource::Remote).unwrap(), json!({ diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index e9bf044766e6..1167494181d4 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -98,6 +98,15 @@ fn marketplace_plugin_source_to_info(source: MarketplacePluginSource) -> PluginS ref_name, sha, }, + MarketplacePluginSource::Npm { + package, + version, + registry, + } => PluginSource::Npm { + package, + version, + registry, + }, } } @@ -131,7 +140,7 @@ fn share_context_for_source( creator_name: None, share_principals: None, }), - MarketplacePluginSource::Git { .. } => None, + MarketplacePluginSource::Git { .. } | MarketplacePluginSource::Npm { .. } => None, } } diff --git a/codex-rs/cli/src/plugin_cmd.rs b/codex-rs/cli/src/plugin_cmd.rs index 1fc670c8717f..16023b4b7caa 100644 --- a/codex-rs/cli/src/plugin_cmd.rs +++ b/codex-rs/cli/src/plugin_cmd.rs @@ -282,6 +282,20 @@ pub async fn run_plugin_list( } parts.join(", ") } + codex_core_plugins::marketplace::MarketplacePluginSource::Npm { + package, + version, + registry, + } => { + let mut parts = vec![package.clone()]; + if let Some(version) = version { + parts.push(format!("version `{version}`")); + } + if let Some(registry) = registry { + parts.push(format!("registry `{registry}`")); + } + parts.join(", ") + } }; plugin_width = plugin_width.max(plugin.id.len()); status_width = status_width.max(state.len()); @@ -409,6 +423,13 @@ enum JsonPluginSource { #[serde(skip_serializing_if = "Option::is_none")] sha: Option, }, + Npm { + package: String, + #[serde(skip_serializing_if = "Option::is_none")] + version: Option, + #[serde(skip_serializing_if = "Option::is_none")] + registry: Option, + }, } impl JsonPluginSource { @@ -434,6 +455,15 @@ impl JsonPluginSource { ref_name, sha, } => Self::Git { url, ref_name, sha }, + MarketplacePluginSource::Npm { + package, + version, + registry, + } => Self::Npm { + package, + version, + registry, + }, } } } diff --git a/codex-rs/core-plugins/src/lib.rs b/codex-rs/core-plugins/src/lib.rs index 08e77606fcbc..9b6b15a07aa3 100644 --- a/codex-rs/core-plugins/src/lib.rs +++ b/codex-rs/core-plugins/src/lib.rs @@ -8,6 +8,7 @@ pub mod marketplace; pub mod marketplace_add; pub mod marketplace_remove; pub mod marketplace_upgrade; +mod npm_source; mod plugin_bundle_archive; mod provider; pub mod remote; diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 54eb371686f2..6b0aee5b83fe 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -10,6 +10,7 @@ use crate::marketplace::MarketplacePluginSource; use crate::marketplace::find_marketplace_plugin; use crate::marketplace::list_marketplaces; use crate::marketplace::load_marketplace; +use crate::npm_source::materialize_npm_plugin_source; use crate::remote::REMOTE_GLOBAL_MARKETPLACE_NAME; use crate::remote::RemoteInstalledPlugin; use crate::store::PluginStore; @@ -1423,6 +1424,22 @@ pub fn materialize_marketplace_plugin_source( _tempdir: Some(tempdir), }) } + MarketplacePluginSource::Npm { + package, + version, + registry, + } => { + let (path, tempdir) = materialize_npm_plugin_source( + codex_home, + package, + version.as_deref(), + registry.as_deref(), + )?; + Ok(MaterializedMarketplacePluginSource { + path, + _tempdir: Some(tempdir), + }) + } } } diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index b502106ea9b9..4a9d36c8dbee 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -1463,7 +1463,7 @@ impl PluginsManager { let mut local_version = plugin.local_version; let manifest_fallback = plugin.manifest_fallback.clone(); if installed - && matches!(&plugin.source, MarketplacePluginSource::Git { .. }) + && plugin.source.is_install_materialized() && let Some(plugin_id) = plugin_id.as_ref() && let Some(plugin_root) = self.store.active_plugin_root(plugin_id) && let Some(manifest) = load_plugin_manifest(plugin_root.as_path()) @@ -1626,7 +1626,7 @@ impl PluginsManager { } })?; let plugin_key = plugin_id.as_key(); - if matches!(plugin.source, MarketplacePluginSource::Git { .. }) && !plugin.installed { + if plugin.source.is_install_materialized() && !plugin.installed { let description = remote_plugin_install_required_description(&plugin.source); return Ok(PluginDetail { id: plugin_key, @@ -1651,28 +1651,27 @@ impl PluginsManager { }); } - let source_path = - if matches!(plugin.source, MarketplacePluginSource::Git { .. }) && plugin.installed { - self.store.active_plugin_root(&plugin_id).ok_or_else(|| { - MarketplaceError::InvalidPlugin(format!( - "installed plugin cache entry is missing for {plugin_key}" - )) - })? - } else { - let codex_home = self.codex_home.clone(); - let source = plugin.source.clone(); - let materialized = tokio::task::spawn_blocking(move || { - materialize_marketplace_plugin_source(codex_home.as_path(), &source) - }) - .await - .map_err(|err| { - MarketplaceError::InvalidPlugin(format!( - "failed to materialize plugin source: {err}" - )) - })? - .map_err(MarketplaceError::InvalidPlugin)?; - materialized.path.clone() - }; + let source_path = if plugin.source.is_install_materialized() && plugin.installed { + self.store.active_plugin_root(&plugin_id).ok_or_else(|| { + MarketplaceError::InvalidPlugin(format!( + "installed plugin cache entry is missing for {plugin_key}" + )) + })? + } else { + let codex_home = self.codex_home.clone(); + let source = plugin.source.clone(); + let materialized = tokio::task::spawn_blocking(move || { + materialize_marketplace_plugin_source(codex_home.as_path(), &source) + }) + .await + .map_err(|err| { + MarketplaceError::InvalidPlugin(format!( + "failed to materialize plugin source: {err}" + )) + })? + .map_err(MarketplaceError::InvalidPlugin)?; + materialized.path.clone() + }; if !source_path.as_path().is_dir() { return Err(MarketplaceError::InvalidPlugin( "path does not exist or is not a directory".to_string(), @@ -2362,10 +2361,29 @@ pub(crate) fn remote_plugin_install_required_description( parts.join(", ") } MarketplacePluginSource::Local { path } => path.as_path().display().to_string(), + MarketplacePluginSource::Npm { + package, + version, + registry, + } => { + let mut parts = vec![package.clone()]; + if let Some(version) = version { + parts.push(format!("version `{version}`")); + } + if let Some(registry) = registry { + parts.push(format!("registry `{registry}`")); + } + parts.join(", ") + } }; + let source_kind = if matches!(source, MarketplacePluginSource::Npm { .. }) { + "an npm plugin" + } else { + "a cross-repo plugin" + }; format!( - "This is a cross-repo plugin. Install it to view more detailed information. The source of the plugin is {source_description}." + "This is {source_kind}. Install it to view more detailed information. The source of the plugin is {source_description}." ) } diff --git a/codex-rs/core-plugins/src/marketplace.rs b/codex-rs/core-plugins/src/marketplace.rs index 456bd9004c5f..b05763200ea9 100644 --- a/codex-rs/core-plugins/src/marketplace.rs +++ b/codex-rs/core-plugins/src/marketplace.rs @@ -131,6 +131,17 @@ pub enum MarketplacePluginSource { ref_name: Option, sha: Option, }, + Npm { + package: String, + version: Option, + registry: Option, + }, +} + +impl MarketplacePluginSource { + pub(crate) fn is_install_materialized(&self) -> bool { + matches!(self, Self::Git { .. } | Self::Npm { .. }) + } } #[derive(Debug, Clone, PartialEq, Eq)] @@ -511,10 +522,12 @@ fn resolve_marketplace_plugin_entry( None } } - MarketplacePluginSource::Git { .. } if manifest_fallback.has_metadata => { + MarketplacePluginSource::Git { .. } | MarketplacePluginSource::Npm { .. } + if manifest_fallback.has_metadata => + { manifest_fallback.parse_for_listing() } - MarketplacePluginSource::Git { .. } => None, + MarketplacePluginSource::Git { .. } | MarketplacePluginSource::Npm { .. } => None, }; let interface = plugin_interface_with_marketplace_category( manifest @@ -608,6 +621,17 @@ fn resolve_plugin_source( ref_name: normalize_optional_git_selector(&ref_name), sha: normalize_optional_git_selector(&sha), }), + RawMarketplaceManifestPluginSource::Object( + RawMarketplaceManifestPluginSourceObject::Npm { + package, + version, + registry, + }, + ) => Ok(MarketplacePluginSource::Npm { + package: normalize_npm_package(marketplace_path, &package)?, + version: normalize_optional_npm_version(marketplace_path, version)?, + registry: normalize_optional_npm_registry(marketplace_path, registry)?, + }), RawMarketplaceManifestPluginSource::Unsupported(_) => { unreachable!("unsupported plugin sources should be filtered before resolution") } @@ -746,6 +770,88 @@ fn normalize_optional_git_selector(value: &Option) -> Option { .map(str::to_string) } +fn normalize_npm_package( + marketplace_path: &AbsolutePathBuf, + package: &str, +) -> Result { + let package = package.trim(); + let segments = if let Some(scoped_package) = package.strip_prefix('@') { + scoped_package.split('/').collect::>() + } else { + package.split('/').collect::>() + }; + let expected_segments = if package.starts_with('@') { 2 } else { 1 }; + if package.is_empty() + || segments.len() != expected_segments + || segments + .iter() + .any(|segment| !is_valid_npm_package_segment(segment)) + { + return Err(MarketplaceError::InvalidMarketplaceFile { + path: marketplace_path.to_path_buf(), + message: format!("invalid npm plugin source package: {package}"), + }); + } + Ok(package.to_string()) +} + +fn is_valid_npm_package_segment(segment: &str) -> bool { + !segment.is_empty() + && segment != "." + && segment != ".." + && segment + .chars() + .all(|ch| ch.is_ascii_alphanumeric() || matches!(ch, '-' | '_' | '.')) +} + +fn normalize_optional_npm_version( + marketplace_path: &AbsolutePathBuf, + version: Option, +) -> Result, MarketplaceError> { + normalize_optional_npm_source_field(marketplace_path, version, "version") +} + +fn normalize_optional_npm_registry( + marketplace_path: &AbsolutePathBuf, + registry: Option, +) -> Result, MarketplaceError> { + let Some(registry) = + normalize_optional_npm_source_field(marketplace_path, registry, "registry")? + else { + return Ok(None); + }; + let parsed = + url::Url::parse(®istry).map_err(|_| MarketplaceError::InvalidMarketplaceFile { + path: marketplace_path.to_path_buf(), + message: format!("invalid npm plugin source registry: {registry}"), + })?; + if !matches!(parsed.scheme(), "http" | "https") || parsed.host_str().is_none() { + return Err(MarketplaceError::InvalidMarketplaceFile { + path: marketplace_path.to_path_buf(), + message: format!("invalid npm plugin source registry: {registry}"), + }); + } + Ok(Some(registry)) +} + +fn normalize_optional_npm_source_field( + marketplace_path: &AbsolutePathBuf, + value: Option, + field: &str, +) -> Result, MarketplaceError> { + let Some(value) = value else { + return Ok(None); + }; + let value = value.trim(); + if value.is_empty() { + return Err(MarketplaceError::InvalidMarketplaceFile { + path: marketplace_path.to_path_buf(), + message: format!("npm plugin source {field} must not be empty"), + }); + } + Ok(Some(value.to_string())) +} + fn normalize_github_git_url(url: &str) -> String { if url.starts_with("https://github.com/") && !url.ends_with(".git") { format!("{url}.git") @@ -884,6 +990,11 @@ enum RawMarketplaceManifestPluginSourceObject { ref_name: Option, sha: Option, }, + Npm { + package: String, + version: Option, + registry: Option, + }, } fn resolve_marketplace_interface( diff --git a/codex-rs/core-plugins/src/marketplace_tests.rs b/codex-rs/core-plugins/src/marketplace_tests.rs index 45aa2364e2e7..4199072c5b71 100644 --- a/codex-rs/core-plugins/src/marketplace_tests.rs +++ b/codex-rs/core-plugins/src/marketplace_tests.rs @@ -180,6 +180,59 @@ fn find_marketplace_plugin_supports_git_subdir_sources() { ); } +#[test] +fn find_marketplace_plugin_supports_npm_sources() { + let tmp = tempdir().unwrap(); + let repo_root = tmp.path().join("repo"); + fs::create_dir_all(repo_root.join(".git")).unwrap(); + fs::create_dir_all(repo_root.join(".agents/plugins")).unwrap(); + fs::write( + repo_root.join(".agents/plugins/marketplace.json"), + r#"{ + "name": "codex-curated", + "plugins": [ + { + "name": "npm-plugin", + "source": { + "source": "npm", + "package": "@acme/codex-plugin", + "version": "^1.2.0", + "registry": "https://npm.example.com" + } + } + ] +}"#, + ) + .unwrap(); + + let resolved = find_marketplace_plugin( + &AbsolutePathBuf::try_from(repo_root.join(".agents/plugins/marketplace.json")).unwrap(), + "npm-plugin", + ) + .unwrap(); + + assert_eq!( + resolved, + ResolvedMarketplacePlugin { + plugin_id: PluginId::new("npm-plugin".to_string(), "codex-curated".to_string()) + .unwrap(), + source: MarketplacePluginSource::Npm { + package: "@acme/codex-plugin".to_string(), + version: Some("^1.2.0".to_string()), + registry: Some("https://npm.example.com".to_string()), + }, + policy: MarketplacePluginPolicy { + installation: MarketplacePluginInstallPolicy::Available, + authentication: MarketplacePluginAuthPolicy::OnInstall, + products: None, + }, + interface: None, + manifest: None, + manifest_fallback: minimal_manifest_fallback("npm-plugin"), + } + ); +} + #[test] fn find_marketplace_plugin_builds_manifest_fallback_from_entry() { let tmp = tempdir().unwrap(); diff --git a/codex-rs/core-plugins/src/npm_source.rs b/codex-rs/core-plugins/src/npm_source.rs new file mode 100644 index 000000000000..ad20250a25a3 --- /dev/null +++ b/codex-rs/core-plugins/src/npm_source.rs @@ -0,0 +1,123 @@ +use codex_utils_absolute_path::AbsolutePathBuf; +use std::ffi::OsStr; +use std::fs; +use std::path::Path; +use std::path::PathBuf; +use std::process::Command; +use tempfile::TempDir; + +const NPM_PLUGIN_SOURCE_STAGING_DIR: &str = "plugins/.marketplace-plugin-source-staging"; + +pub(crate) fn materialize_npm_plugin_source( + codex_home: &Path, + package: &str, + version: Option<&str>, + registry: Option<&str>, +) -> Result<(AbsolutePathBuf, TempDir), String> { + materialize_npm_plugin_source_with_command( + codex_home, + package, + version, + registry, + OsStr::new(npm_command()), + ) +} + +fn materialize_npm_plugin_source_with_command( + codex_home: &Path, + package: &str, + version: Option<&str>, + registry: Option<&str>, + npm_command: &OsStr, +) -> Result<(AbsolutePathBuf, TempDir), String> { + let staging_root = codex_home.join(NPM_PLUGIN_SOURCE_STAGING_DIR); + fs::create_dir_all(&staging_root).map_err(|err| { + format!( + "failed to create marketplace plugin source staging directory {}: {err}", + staging_root.display() + ) + })?; + let tempdir = tempfile::Builder::new() + .prefix("marketplace-plugin-source-") + .tempdir_in(&staging_root) + .map_err(|err| { + format!( + "failed to create marketplace plugin source staging directory in {}: {err}", + staging_root.display() + ) + })?; + + install_npm_package(tempdir.path(), package, version, registry, npm_command)?; + let plugin_root = installed_npm_package_path(tempdir.path(), package); + if !plugin_root.is_dir() { + return Err(format!( + "npm install completed without creating plugin package directory {}", + plugin_root.display() + )); + } + let plugin_root = AbsolutePathBuf::try_from(plugin_root) + .map_err(|err| format!("failed to resolve materialized plugin source path: {err}"))?; + Ok((plugin_root, tempdir)) +} + +fn install_npm_package( + destination: &Path, + package: &str, + version: Option<&str>, + registry: Option<&str>, + npm_command: &OsStr, +) -> Result<(), String> { + let package_spec = version.map_or_else( + || package.to_string(), + |version| format!("{package}@{version}"), + ); + let mut command = Command::new(npm_command); + command + .arg("install") + .arg("--ignore-scripts") + .arg("--no-audit") + .arg("--no-fund") + .arg("--no-package-lock") + .arg("--prefix") + .arg(destination); + if let Some(registry) = registry { + command.arg("--registry").arg(registry); + } + command.arg("--").arg(package_spec); + + let output = command + .output() + .map_err(|err| format!("failed to run npm install: {err}"))?; + if output.status.success() { + return Ok(()); + } + + Err(format!( + "npm install failed with status {}\nstdout:\n{}\nstderr:\n{}", + output.status, + String::from_utf8_lossy(&output.stdout).trim(), + String::from_utf8_lossy(&output.stderr).trim() + )) +} + +fn installed_npm_package_path(destination: &Path, package: &str) -> PathBuf { + let mut path = destination.join("node_modules"); + for segment in package.split('/') { + path.push(segment); + } + path +} + +#[cfg(windows)] +fn npm_command() -> &'static str { + "npm.cmd" +} + +#[cfg(not(windows))] +fn npm_command() -> &'static str { + "npm" +} + +#[cfg(test)] +#[path = "npm_source_tests.rs"] +mod tests; diff --git a/codex-rs/core-plugins/src/npm_source_tests.rs b/codex-rs/core-plugins/src/npm_source_tests.rs new file mode 100644 index 000000000000..825292b05668 --- /dev/null +++ b/codex-rs/core-plugins/src/npm_source_tests.rs @@ -0,0 +1,67 @@ +use super::*; +use pretty_assertions::assert_eq; + +#[test] +fn installed_npm_package_path_supports_scoped_packages() { + assert_eq!( + installed_npm_package_path(Path::new("/tmp/install"), "@acme/plugin"), + Path::new("/tmp/install/node_modules/@acme/plugin") + ); +} + +#[cfg(unix)] +#[test] +fn materialize_npm_plugin_source_uses_npm_installed_package_root() { + use std::os::unix::fs::PermissionsExt; + + let codex_home = tempfile::tempdir().expect("create codex home"); + let fake_npm_dir = tempfile::tempdir().expect("create fake npm directory"); + let fake_npm = fake_npm_dir.path().join("npm"); + fs::write( + &fake_npm, + r#"#!/bin/sh +prefix="" +previous="" +for argument in "$@"; do + if [ "$previous" = "--prefix" ]; then + prefix="$argument" + fi + previous="$argument" +done +mkdir -p "$prefix/node_modules/@acme/plugin/.codex-plugin" +printf '%s\n' "$@" > "$prefix/args.txt" +printf '{"name":"plugin"}\n' > "$prefix/node_modules/@acme/plugin/.codex-plugin/plugin.json" +"#, + ) + .expect("write fake npm"); + let mut permissions = fs::metadata(&fake_npm) + .expect("read fake npm metadata") + .permissions(); + permissions.set_mode(0o755); + fs::set_permissions(&fake_npm, permissions).expect("make fake npm executable"); + + let (plugin_root, tempdir) = materialize_npm_plugin_source_with_command( + codex_home.path(), + "@acme/plugin", + Some("^1.2.0"), + Some("https://npm.example.com"), + fake_npm.as_os_str(), + ) + .expect("materialize npm source"); + + assert_eq!( + plugin_root.as_path(), + tempdir.path().join("node_modules/@acme/plugin") + ); + assert!( + plugin_root + .as_path() + .join(".codex-plugin/plugin.json") + .is_file() + ); + let args = fs::read_to_string(tempdir.path().join("args.txt")).expect("read npm arguments"); + assert!(args.contains("--ignore-scripts")); + assert!(args.contains("--registry")); + assert!(args.contains("https://npm.example.com")); + assert!(args.contains("@acme/plugin@^1.2.0")); +} From e03c59bcd2df0b6894a5c230306dfbfaa3983df8 Mon Sep 17 00:00:00 2001 From: charlesgong-openai Date: Mon, 22 Jun 2026 15:23:01 -0700 Subject: [PATCH 2/4] Harden npm marketplace plugin sources --- codex-rs/Cargo.lock | 2 + .../codex_app_server_protocol.schemas.json | 15 +- .../codex_app_server_protocol.v2.schemas.json | 15 +- .../json/v2/PluginInstalledResponse.json | 15 +- .../schema/json/v2/PluginListResponse.json | 15 +- .../schema/json/v2/PluginReadResponse.json | 15 +- .../json/v2/PluginShareListResponse.json | 15 +- .../schema/typescript/v2/PluginSource.ts | 14 +- .../src/protocol/v2/plugin.rs | 6 +- .../src/protocol/v2/tests.rs | 6 +- codex-rs/app-server/README.md | 2 +- .../src/request_processors/plugins.rs | 2 + codex-rs/cli/src/plugin_cmd.rs | 12 +- codex-rs/core-plugins/Cargo.toml | 2 + codex-rs/core-plugins/src/loader.rs | 4 +- codex-rs/core-plugins/src/manager.rs | 6 +- codex-rs/core-plugins/src/marketplace.rs | 54 +++++-- .../core-plugins/src/marketplace_tests.rs | 69 +++++++- codex-rs/core-plugins/src/npm_source.rs | 152 +++++++++++++++--- codex-rs/core-plugins/src/npm_source_tests.rs | 85 ++++++++-- 20 files changed, 412 insertions(+), 94 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index f35402efd08e..79f3ddb79a47 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2747,6 +2747,7 @@ name = "codex-core-plugins" version = "0.0.0" dependencies = [ "anyhow", + "base64 0.22.1", "chrono", "codex-analytics", "codex-app-server-protocol", @@ -2774,6 +2775,7 @@ dependencies = [ "semver", "serde", "serde_json", + "sha2 0.10.9", "tar", "tempfile", "thiserror 2.0.18", diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 501d18cc0ae0..eb1179c9b58d 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -13936,10 +13936,15 @@ }, { "properties": { + "integrity": { + "description": "SRI digest for the packed npm tarball, currently restricted to sha512.", + "type": "string" + }, "package": { "type": "string" }, "registry": { + "description": "Optional HTTPS registry URL. Authentication stays in the user's npm config.", "type": [ "string", "null" @@ -13953,15 +13958,15 @@ "type": "string" }, "version": { - "type": [ - "string", - "null" - ] + "description": "Exact semver version pinned by the marketplace.", + "type": "string" } }, "required": [ + "integrity", "package", - "type" + "type", + "version" ], "title": "NpmPluginSource", "type": "object" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index f847954a4407..ff8832412219 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -10360,10 +10360,15 @@ }, { "properties": { + "integrity": { + "description": "SRI digest for the packed npm tarball, currently restricted to sha512.", + "type": "string" + }, "package": { "type": "string" }, "registry": { + "description": "Optional HTTPS registry URL. Authentication stays in the user's npm config.", "type": [ "string", "null" @@ -10377,15 +10382,15 @@ "type": "string" }, "version": { - "type": [ - "string", - "null" - ] + "description": "Exact semver version pinned by the marketplace.", + "type": "string" } }, "required": [ + "integrity", "package", - "type" + "type", + "version" ], "title": "NpmPluginSource", "type": "object" diff --git a/codex-rs/app-server-protocol/schema/json/v2/PluginInstalledResponse.json b/codex-rs/app-server-protocol/schema/json/v2/PluginInstalledResponse.json index e5cf2e38fdd5..c2c98fafa495 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/PluginInstalledResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/PluginInstalledResponse.json @@ -397,10 +397,15 @@ }, { "properties": { + "integrity": { + "description": "SRI digest for the packed npm tarball, currently restricted to sha512.", + "type": "string" + }, "package": { "type": "string" }, "registry": { + "description": "Optional HTTPS registry URL. Authentication stays in the user's npm config.", "type": [ "string", "null" @@ -414,15 +419,15 @@ "type": "string" }, "version": { - "type": [ - "string", - "null" - ] + "description": "Exact semver version pinned by the marketplace.", + "type": "string" } }, "required": [ + "integrity", "package", - "type" + "type", + "version" ], "title": "NpmPluginSource", "type": "object" diff --git a/codex-rs/app-server-protocol/schema/json/v2/PluginListResponse.json b/codex-rs/app-server-protocol/schema/json/v2/PluginListResponse.json index faf367f055c9..ea32375ea65b 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/PluginListResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/PluginListResponse.json @@ -397,10 +397,15 @@ }, { "properties": { + "integrity": { + "description": "SRI digest for the packed npm tarball, currently restricted to sha512.", + "type": "string" + }, "package": { "type": "string" }, "registry": { + "description": "Optional HTTPS registry URL. Authentication stays in the user's npm config.", "type": [ "string", "null" @@ -414,15 +419,15 @@ "type": "string" }, "version": { - "type": [ - "string", - "null" - ] + "description": "Exact semver version pinned by the marketplace.", + "type": "string" } }, "required": [ + "integrity", "package", - "type" + "type", + "version" ], "title": "NpmPluginSource", "type": "object" diff --git a/codex-rs/app-server-protocol/schema/json/v2/PluginReadResponse.json b/codex-rs/app-server-protocol/schema/json/v2/PluginReadResponse.json index ab9b4c9ecb43..29ba857e25ff 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/PluginReadResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/PluginReadResponse.json @@ -537,10 +537,15 @@ }, { "properties": { + "integrity": { + "description": "SRI digest for the packed npm tarball, currently restricted to sha512.", + "type": "string" + }, "package": { "type": "string" }, "registry": { + "description": "Optional HTTPS registry URL. Authentication stays in the user's npm config.", "type": [ "string", "null" @@ -554,15 +559,15 @@ "type": "string" }, "version": { - "type": [ - "string", - "null" - ] + "description": "Exact semver version pinned by the marketplace.", + "type": "string" } }, "required": [ + "integrity", "package", - "type" + "type", + "version" ], "title": "NpmPluginSource", "type": "object" diff --git a/codex-rs/app-server-protocol/schema/json/v2/PluginShareListResponse.json b/codex-rs/app-server-protocol/schema/json/v2/PluginShareListResponse.json index 84475e47ae05..d9b5fc7ffc34 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/PluginShareListResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/PluginShareListResponse.json @@ -353,10 +353,15 @@ }, { "properties": { + "integrity": { + "description": "SRI digest for the packed npm tarball, currently restricted to sha512.", + "type": "string" + }, "package": { "type": "string" }, "registry": { + "description": "Optional HTTPS registry URL. Authentication stays in the user's npm config.", "type": [ "string", "null" @@ -370,15 +375,15 @@ "type": "string" }, "version": { - "type": [ - "string", - "null" - ] + "description": "Exact semver version pinned by the marketplace.", + "type": "string" } }, "required": [ + "integrity", "package", - "type" + "type", + "version" ], "title": "NpmPluginSource", "type": "object" diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/PluginSource.ts b/codex-rs/app-server-protocol/schema/typescript/v2/PluginSource.ts index c55231293015..e13a73e83650 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/PluginSource.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/PluginSource.ts @@ -3,4 +3,16 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { AbsolutePathBuf } from "../AbsolutePathBuf"; -export type PluginSource = { "type": "local", path: AbsolutePathBuf, } | { "type": "git", url: string, path: string | null, refName: string | null, sha: string | null, } | { "type": "npm", package: string, version: string | null, registry: string | null, } | { "type": "remote" }; +export type PluginSource = { "type": "local", path: AbsolutePathBuf, } | { "type": "git", url: string, path: string | null, refName: string | null, sha: string | null, } | { "type": "npm", package: string, +/** + * Exact semver version pinned by the marketplace. + */ +version: string, +/** + * Optional HTTPS registry URL. Authentication stays in the user's npm config. + */ +registry: string | null, +/** + * SRI digest for the packed npm tarball, currently restricted to sha512. + */ +integrity: string, } | { "type": "remote" }; diff --git a/codex-rs/app-server-protocol/src/protocol/v2/plugin.rs b/codex-rs/app-server-protocol/src/protocol/v2/plugin.rs index 4a184e3c5552..c3591b4635c4 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/plugin.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/plugin.rs @@ -744,8 +744,12 @@ pub enum PluginSource { #[ts(rename_all = "camelCase")] Npm { package: String, - version: Option, + /// Exact semver version pinned by the marketplace. + version: String, + /// Optional HTTPS registry URL. Authentication stays in the user's npm config. registry: Option, + /// SRI digest for the packed npm tarball, currently restricted to sha512. + integrity: String, }, /// The plugin is available in the remote catalog. Download metadata is /// kept server-side and is not exposed through the app-server API. diff --git a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs index 66bf4bbc2c51..32a11133d5d9 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs @@ -2843,15 +2843,17 @@ fn plugin_source_serializes_local_git_npm_and_remote_variants() { assert_eq!( serde_json::to_value(PluginSource::Npm { package: "@acme/plugin".to_string(), - version: Some("^1.2.0".to_string()), + version: "1.2.0".to_string(), registry: Some("https://npm.example.com".to_string()), + integrity: "sha512-test-integrity".to_string(), }) .unwrap(), json!({ "type": "npm", "package": "@acme/plugin", - "version": "^1.2.0", + "version": "1.2.0", "registry": "https://npm.example.com", + "integrity": "sha512-test-integrity", }), ); diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index f8ceb69d42bd..ad9d90a7021f 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -212,7 +212,7 @@ Example with notification opt-out: - `marketplace/add` — add a remote plugin marketplace from an HTTP(S) Git URL, SSH Git URL, or GitHub `owner/repo` shorthand, then persist it into the user marketplace config. Returns the installed root path plus whether the marketplace was already present. - `marketplace/remove` — remove a configured marketplace by name from the user marketplace config, and delete its installed marketplace root when one exists. - `marketplace/upgrade` — upgrade all configured Git plugin marketplaces, or one named marketplace when `marketplaceName` is provided. Returns selected marketplace names, upgraded roots, and per-marketplace errors. -- `plugin/list` — list discovered plugin marketplaces and plugin state, including effective marketplace install/auth policy metadata, plugin `availability` (`AVAILABLE` by default or `DISABLED_BY_ADMIN` for remote plugins blocked upstream), fail-open `marketplaceLoadErrors` entries for marketplace files that could not be parsed or loaded, and best-effort `featuredPluginIds` for the official curated marketplace. Clients can explicitly request the remote `workspace-directory`, `shared-with-me`, or `created-by-me-remote` marketplace kinds. `interface.category` uses the marketplace category when present; otherwise it falls back to the plugin manifest category (**under development; do not call from production clients yet**). +- `plugin/list` — list discovered plugin marketplaces and plugin state, including effective marketplace install/auth policy metadata, plugin `availability` (`AVAILABLE` by default or `DISABLED_BY_ADMIN` for remote plugins blocked upstream), fail-open `marketplaceLoadErrors` entries for marketplace files that could not be parsed or loaded, and best-effort `featuredPluginIds` for the official curated marketplace. Local, Git, and npm marketplace sources are surfaced through `summary.source`; npm sources require an exact semver version plus a `sha512` integrity pin. Clients can explicitly request the remote `workspace-directory`, `shared-with-me`, or `created-by-me-remote` marketplace kinds. `interface.category` uses the marketplace category when present; otherwise it falls back to the plugin manifest category (**under development; do not call from production clients yet**). - `plugin/installed` — list installed plugin rows plus any explicitly requested local install-suggestion plugin names, without fetching the broader remote catalog. Mention surfaces can use this narrower view when they need plugin mention payloads rather than plugin-page discovery data (**under development; do not call from production clients yet**). - `plugin/read` — read one plugin by `marketplacePath` plus `pluginName`, returning marketplace info, a list-style `summary`, manifest descriptions/interface metadata, and bundled skills/hooks/apps/MCP server names. Remote plugin details expose the canonical `shareUrl` supplied by the remote catalog when available; it is `null` for local plugins or when the catalog omits it. This field is separate from `summary.shareContext`, which continues to describe user and workspace sharing state. Returned plugin skills include their current `enabled` state after local config filtering; bundled hooks are returned as lightweight declaration summaries keyed for correlation with `hooks/list`. Use `plugin/install`'s `appsNeedingAuth` to drive post-install authentication and `app/list`'s `isAccessible` to determine current connector accessibility (**under development; do not call from production clients yet**). - `plugin/skill/read` — read remote plugin skill markdown on demand by `remoteMarketplaceName`, `remotePluginId`, and `skillName`. This lets clients preview uninstalled remote plugin skills without downloading the plugin bundle. diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index 1167494181d4..f212c7fef7b7 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -102,10 +102,12 @@ fn marketplace_plugin_source_to_info(source: MarketplacePluginSource) -> PluginS package, version, registry, + integrity, } => PluginSource::Npm { package, version, registry, + integrity, }, } } diff --git a/codex-rs/cli/src/plugin_cmd.rs b/codex-rs/cli/src/plugin_cmd.rs index 16023b4b7caa..4e5de5c43bea 100644 --- a/codex-rs/cli/src/plugin_cmd.rs +++ b/codex-rs/cli/src/plugin_cmd.rs @@ -286,14 +286,14 @@ pub async fn run_plugin_list( package, version, registry, + integrity, } => { let mut parts = vec![package.clone()]; - if let Some(version) = version { - parts.push(format!("version `{version}`")); - } + parts.push(format!("version `{version}`")); if let Some(registry) = registry { parts.push(format!("registry `{registry}`")); } + parts.push(format!("integrity `{integrity}`")); parts.join(", ") } }; @@ -425,10 +425,10 @@ enum JsonPluginSource { }, Npm { package: String, - #[serde(skip_serializing_if = "Option::is_none")] - version: Option, + version: String, #[serde(skip_serializing_if = "Option::is_none")] registry: Option, + integrity: String, }, } @@ -459,10 +459,12 @@ impl JsonPluginSource { package, version, registry, + integrity, } => Self::Npm { package, version, registry, + integrity, }, } } diff --git a/codex-rs/core-plugins/Cargo.toml b/codex-rs/core-plugins/Cargo.toml index b90192bfe960..3165b33ec0cf 100644 --- a/codex-rs/core-plugins/Cargo.toml +++ b/codex-rs/core-plugins/Cargo.toml @@ -14,6 +14,7 @@ workspace = true [dependencies] anyhow = { workspace = true } +base64 = { workspace = true } codex-analytics = { workspace = true } codex-app-server-protocol = { workspace = true } codex-config = { workspace = true } @@ -39,6 +40,7 @@ reqwest = { workspace = true } semver = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } +sha2 = { workspace = true } tar = { workspace = true } tempfile = { workspace = true } thiserror = { workspace = true } diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 6b0aee5b83fe..8ca844649e2e 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -1428,12 +1428,14 @@ pub fn materialize_marketplace_plugin_source( package, version, registry, + integrity, } => { let (path, tempdir) = materialize_npm_plugin_source( codex_home, package, - version.as_deref(), + version, registry.as_deref(), + integrity, )?; Ok(MaterializedMarketplacePluginSource { path, diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 4a9d36c8dbee..b4fa7856c8fa 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -2365,14 +2365,14 @@ pub(crate) fn remote_plugin_install_required_description( package, version, registry, + integrity, } => { let mut parts = vec![package.clone()]; - if let Some(version) = version { - parts.push(format!("version `{version}`")); - } + parts.push(format!("version `{version}`")); if let Some(registry) = registry { parts.push(format!("registry `{registry}`")); } + parts.push(format!("integrity `{integrity}`")); parts.join(", ") } }; diff --git a/codex-rs/core-plugins/src/marketplace.rs b/codex-rs/core-plugins/src/marketplace.rs index b05763200ea9..07b437e1159f 100644 --- a/codex-rs/core-plugins/src/marketplace.rs +++ b/codex-rs/core-plugins/src/marketplace.rs @@ -7,6 +7,7 @@ use codex_plugin::PluginId; use codex_plugin::PluginIdError; use codex_protocol::protocol::Product; use codex_utils_absolute_path::AbsolutePathBuf; +use semver::Version; use serde::Deserialize; use serde_json::Map as JsonMap; use serde_json::Value as JsonValue; @@ -86,8 +87,8 @@ impl MarketplacePluginManifestFallback { } pub(crate) fn parse_for_listing(&self) -> Option { - // Git sources have no plugin root before install. Parse against a synthetic absolute root, - // then discard path-bearing fields so listings expose metadata only. + // Materialized sources have no plugin root before install. Parse against a synthetic + // absolute root, then discard path-bearing fields so listings expose metadata only. let mut manifest = crate::manifest::parse_plugin_manifest( Path::new("/"), Path::new("/.codex-plugin/plugin.json"), @@ -133,8 +134,9 @@ pub enum MarketplacePluginSource { }, Npm { package: String, - version: Option, + version: String, registry: Option, + integrity: String, }, } @@ -626,11 +628,13 @@ fn resolve_plugin_source( package, version, registry, + integrity, }, ) => Ok(MarketplacePluginSource::Npm { package: normalize_npm_package(marketplace_path, &package)?, - version: normalize_optional_npm_version(marketplace_path, version)?, + version: normalize_npm_version(marketplace_path, version)?, registry: normalize_optional_npm_registry(marketplace_path, registry)?, + integrity: normalize_npm_integrity(marketplace_path, integrity)?, }), RawMarketplaceManifestPluginSource::Unsupported(_) => { unreachable!("unsupported plugin sources should be filtered before resolution") @@ -804,11 +808,34 @@ fn is_valid_npm_package_segment(segment: &str) -> bool { .all(|ch| ch.is_ascii_alphanumeric() || matches!(ch, '-' | '_' | '.')) } -fn normalize_optional_npm_version( +fn normalize_npm_version( marketplace_path: &AbsolutePathBuf, - version: Option, -) -> Result, MarketplaceError> { - normalize_optional_npm_source_field(marketplace_path, version, "version") + version: String, +) -> Result { + let version = version.trim(); + if Version::parse(version).is_err() { + return Err(MarketplaceError::InvalidMarketplaceFile { + path: marketplace_path.to_path_buf(), + message: format!( + "npm plugin source version must be an exact semver version: {version}" + ), + }); + } + Ok(version.to_string()) +} + +fn normalize_npm_integrity( + marketplace_path: &AbsolutePathBuf, + integrity: String, +) -> Result { + let integrity = integrity.trim(); + crate::npm_source::validate_npm_integrity(integrity).map_err(|message| { + MarketplaceError::InvalidMarketplaceFile { + path: marketplace_path.to_path_buf(), + message, + } + })?; + Ok(integrity.to_string()) } fn normalize_optional_npm_registry( @@ -825,7 +852,13 @@ fn normalize_optional_npm_registry( path: marketplace_path.to_path_buf(), message: format!("invalid npm plugin source registry: {registry}"), })?; - if !matches!(parsed.scheme(), "http" | "https") || parsed.host_str().is_none() { + if parsed.scheme() != "https" + || parsed.host_str().is_none() + || !parsed.username().is_empty() + || parsed.password().is_some() + || parsed.query().is_some() + || parsed.fragment().is_some() + { return Err(MarketplaceError::InvalidMarketplaceFile { path: marketplace_path.to_path_buf(), message: format!("invalid npm plugin source registry: {registry}"), @@ -992,8 +1025,9 @@ enum RawMarketplaceManifestPluginSourceObject { }, Npm { package: String, - version: Option, + version: String, registry: Option, + integrity: String, }, } diff --git a/codex-rs/core-plugins/src/marketplace_tests.rs b/codex-rs/core-plugins/src/marketplace_tests.rs index 4199072c5b71..c636f9663363 100644 --- a/codex-rs/core-plugins/src/marketplace_tests.rs +++ b/codex-rs/core-plugins/src/marketplace_tests.rs @@ -6,6 +6,7 @@ use tempfile::tempdir; const ALTERNATE_MARKETPLACE_RELATIVE_PATH: &str = ".claude-plugin/marketplace.json"; const ALTERNATE_PLUGIN_MANIFEST_RELATIVE_PATH: &str = ".claude-plugin/plugin.json"; +const TEST_NPM_INTEGRITY: &str = "sha512-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=="; fn write_alternate_marketplace(repo_root: &Path, contents: &str) -> AbsolutePathBuf { let marketplace_path = repo_root.join(ALTERNATE_MARKETPLACE_RELATIVE_PATH); @@ -196,8 +197,9 @@ fn find_marketplace_plugin_supports_npm_sources() { "source": { "source": "npm", "package": "@acme/codex-plugin", - "version": "^1.2.0", - "registry": "https://npm.example.com" + "version": "1.2.0", + "registry": "https://npm.example.com", + "integrity": "sha512-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA==" } } ] @@ -218,8 +220,9 @@ fn find_marketplace_plugin_supports_npm_sources() { .unwrap(), source: MarketplacePluginSource::Npm { package: "@acme/codex-plugin".to_string(), - version: Some("^1.2.0".to_string()), + version: "1.2.0".to_string(), registry: Some("https://npm.example.com".to_string()), + integrity: TEST_NPM_INTEGRITY.to_string(), }, policy: MarketplacePluginPolicy { installation: MarketplacePluginInstallPolicy::Available, @@ -233,6 +236,66 @@ fn find_marketplace_plugin_supports_npm_sources() { ); } +#[test] +fn find_marketplace_plugin_skips_unsafe_npm_sources() { + let tmp = tempdir().unwrap(); + let repo_root = tmp.path().join("repo"); + fs::create_dir_all(repo_root.join(".git")).unwrap(); + let marketplace_path = write_alternate_marketplace( + &repo_root, + r#"{ + "name": "codex-curated", + "plugins": [ + { + "name": "remote-version", + "source": { + "source": "npm", + "package": "@acme/codex-plugin", + "version": "https://attacker.example/plugin.tgz", + "registry": "https://npm.example.com", + "integrity": "sha512-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA==" + } + }, + { + "name": "plaintext-registry", + "source": { + "source": "npm", + "package": "@acme/codex-plugin", + "version": "1.2.0", + "registry": "http://npm.example.com", + "integrity": "sha512-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA==" + } + }, + { + "name": "credential-registry", + "source": { + "source": "npm", + "package": "@acme/codex-plugin", + "version": "1.2.0", + "registry": "https://user:password@npm.example.com", + "integrity": "sha512-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA==" + } + }, + { + "name": "invalid-integrity", + "source": { + "source": "npm", + "package": "@acme/codex-plugin", + "version": "1.2.0", + "registry": "https://npm.example.com", + "integrity": "sha256-not-supported" + } + } + ] +}"#, + ); + + assert_eq!( + load_marketplace(&marketplace_path).unwrap().plugins, + Vec::new() + ); +} + #[test] fn find_marketplace_plugin_builds_manifest_fallback_from_entry() { let tmp = tempdir().unwrap(); diff --git a/codex-rs/core-plugins/src/npm_source.rs b/codex-rs/core-plugins/src/npm_source.rs index ad20250a25a3..36d68a8014ff 100644 --- a/codex-rs/core-plugins/src/npm_source.rs +++ b/codex-rs/core-plugins/src/npm_source.rs @@ -1,4 +1,10 @@ +use crate::plugin_bundle_archive::unpack_plugin_bundle_tar_gz; +use base64::Engine; +use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; use codex_utils_absolute_path::AbsolutePathBuf; +use serde::Deserialize; +use sha2::Digest; +use sha2::Sha512; use std::ffi::OsStr; use std::fs; use std::path::Path; @@ -7,18 +13,27 @@ use std::process::Command; use tempfile::TempDir; const NPM_PLUGIN_SOURCE_STAGING_DIR: &str = "plugins/.marketplace-plugin-source-staging"; +const NPM_PLUGIN_SOURCE_MAX_ARCHIVE_BYTES: u64 = 50 * 1024 * 1024; +const NPM_PLUGIN_SOURCE_MAX_EXTRACTED_BYTES: u64 = 250 * 1024 * 1024; +const NPM_PACKAGE_ARCHIVE_ROOT: &str = "package"; + +pub(crate) fn validate_npm_integrity(integrity: &str) -> Result<(), String> { + parse_npm_integrity(integrity).map(|_| ()) +} pub(crate) fn materialize_npm_plugin_source( codex_home: &Path, package: &str, - version: Option<&str>, + version: &str, registry: Option<&str>, + integrity: &str, ) -> Result<(AbsolutePathBuf, TempDir), String> { materialize_npm_plugin_source_with_command( codex_home, package, version, registry, + integrity, OsStr::new(npm_command()), ) } @@ -26,8 +41,9 @@ pub(crate) fn materialize_npm_plugin_source( fn materialize_npm_plugin_source_with_command( codex_home: &Path, package: &str, - version: Option<&str>, + version: &str, registry: Option<&str>, + integrity: &str, npm_command: &OsStr, ) -> Result<(AbsolutePathBuf, TempDir), String> { let staging_root = codex_home.join(NPM_PLUGIN_SOURCE_STAGING_DIR); @@ -47,38 +63,44 @@ fn materialize_npm_plugin_source_with_command( ) })?; - install_npm_package(tempdir.path(), package, version, registry, npm_command)?; - let plugin_root = installed_npm_package_path(tempdir.path(), package); + pack_npm_package(tempdir.path(), package, version, registry, npm_command)?; + let archive_path = find_npm_package_archive(tempdir.path())?; + let archive_bytes = read_npm_package_archive(&archive_path)?; + verify_npm_integrity(&archive_bytes, integrity)?; + + let extraction_root = tempdir.path().join("extracted"); + unpack_plugin_bundle_tar_gz( + &archive_bytes, + &extraction_root, + NPM_PLUGIN_SOURCE_MAX_EXTRACTED_BYTES, + ) + .map_err(|err| format!("failed to extract npm plugin package: {err}"))?; + let plugin_root = extraction_root.join(NPM_PACKAGE_ARCHIVE_ROOT); if !plugin_root.is_dir() { return Err(format!( - "npm install completed without creating plugin package directory {}", + "npm pack completed without creating plugin package directory {}", plugin_root.display() )); } + validate_npm_package_metadata(&plugin_root, package, version)?; let plugin_root = AbsolutePathBuf::try_from(plugin_root) .map_err(|err| format!("failed to resolve materialized plugin source path: {err}"))?; Ok((plugin_root, tempdir)) } -fn install_npm_package( +fn pack_npm_package( destination: &Path, package: &str, - version: Option<&str>, + version: &str, registry: Option<&str>, npm_command: &OsStr, ) -> Result<(), String> { - let package_spec = version.map_or_else( - || package.to_string(), - |version| format!("{package}@{version}"), - ); + let package_spec = format!("{package}@{version}"); let mut command = Command::new(npm_command); command - .arg("install") + .arg("pack") .arg("--ignore-scripts") - .arg("--no-audit") - .arg("--no-fund") - .arg("--no-package-lock") - .arg("--prefix") + .arg("--pack-destination") .arg(destination); if let Some(registry) = registry { command.arg("--registry").arg(registry); @@ -87,25 +109,109 @@ fn install_npm_package( let output = command .output() - .map_err(|err| format!("failed to run npm install: {err}"))?; + .map_err(|err| format!("failed to run npm pack: {err}"))?; if output.status.success() { return Ok(()); } Err(format!( - "npm install failed with status {}\nstdout:\n{}\nstderr:\n{}", + "npm pack failed with status {}\nstdout:\n{}\nstderr:\n{}", output.status, String::from_utf8_lossy(&output.stdout).trim(), String::from_utf8_lossy(&output.stderr).trim() )) } -fn installed_npm_package_path(destination: &Path, package: &str) -> PathBuf { - let mut path = destination.join("node_modules"); - for segment in package.split('/') { - path.push(segment); +fn find_npm_package_archive(destination: &Path) -> Result { + let mut archives = fs::read_dir(destination) + .map_err(|err| format!("failed to read npm pack destination: {err}"))? + .filter_map(std::result::Result::ok) + .filter_map(|entry| { + let path = entry.path(); + let is_file = entry.file_type().is_ok_and(|file_type| file_type.is_file()); + (is_file && path.extension() == Some(OsStr::new("tgz"))).then_some(path) + }) + .collect::>(); + if archives.len() != 1 { + return Err(format!( + "npm pack completed with {} package archives; expected exactly one", + archives.len() + )); + } + Ok(archives.remove(0)) +} + +fn read_npm_package_archive(archive_path: &Path) -> Result, String> { + let archive_size = fs::metadata(archive_path) + .map_err(|err| format!("failed to inspect npm package archive: {err}"))? + .len(); + if archive_size > NPM_PLUGIN_SOURCE_MAX_ARCHIVE_BYTES { + return Err(format!( + "npm package archive is {archive_size} bytes, exceeding maximum size of {NPM_PLUGIN_SOURCE_MAX_ARCHIVE_BYTES} bytes" + )); + } + fs::read(archive_path).map_err(|err| format!("failed to read npm package archive: {err}")) +} + +fn verify_npm_integrity(archive_bytes: &[u8], integrity: &str) -> Result<(), String> { + let expected_digest = parse_npm_integrity(integrity)?; + let actual_digest = Sha512::digest(archive_bytes); + if actual_digest.as_slice() != expected_digest { + return Err("npm package archive did not match declared sha512 integrity".to_string()); + } + Ok(()) +} + +fn parse_npm_integrity(integrity: &str) -> Result, String> { + let Some(encoded_digest) = integrity.strip_prefix("sha512-") else { + return Err("npm plugin source integrity must use sha512".to_string()); + }; + let digest = BASE64_STANDARD + .decode(encoded_digest) + .map_err(|_| "npm plugin source integrity must be valid base64".to_string())?; + if digest.len() != 64 { + return Err("npm plugin source integrity must contain a sha512 digest".to_string()); + } + Ok(digest) +} + +fn validate_npm_package_metadata( + plugin_root: &Path, + package: &str, + version: &str, +) -> Result<(), String> { + #[derive(Deserialize)] + struct NpmPackageMetadata { + name: String, + version: String, + } + + let package_json_path = plugin_root.join("package.json"); + let package_json = fs::read_to_string(&package_json_path).map_err(|err| { + format!( + "failed to read npm plugin package metadata {}: {err}", + package_json_path.display() + ) + })?; + let metadata: NpmPackageMetadata = serde_json::from_str(&package_json).map_err(|err| { + format!( + "failed to parse npm plugin package metadata {}: {err}", + package_json_path.display() + ) + })?; + if metadata.name != package { + return Err(format!( + "npm plugin package name '{}' does not match requested package '{package}'", + metadata.name + )); + } + if metadata.version != version { + return Err(format!( + "npm plugin package version '{}' does not match requested version '{version}'", + metadata.version + )); } - path + Ok(()) } #[cfg(windows)] diff --git a/codex-rs/core-plugins/src/npm_source_tests.rs b/codex-rs/core-plugins/src/npm_source_tests.rs index 825292b05668..87630ead3d30 100644 --- a/codex-rs/core-plugins/src/npm_source_tests.rs +++ b/codex-rs/core-plugins/src/npm_source_tests.rs @@ -1,37 +1,57 @@ use super::*; +use flate2::Compression; +use flate2::write::GzEncoder; use pretty_assertions::assert_eq; +use std::io::Cursor; +use std::io::Write; #[test] -fn installed_npm_package_path_supports_scoped_packages() { +fn validate_npm_integrity_rejects_non_sha512_integrity() { assert_eq!( - installed_npm_package_path(Path::new("/tmp/install"), "@acme/plugin"), - Path::new("/tmp/install/node_modules/@acme/plugin") + validate_npm_integrity("sha256-not-a-sha512-digest"), + Err("npm plugin source integrity must use sha512".to_string()) + ); +} + +#[test] +fn verify_npm_integrity_rejects_mismatched_archive() { + let archive_bytes = npm_package_archive_bytes("@acme/plugin", "1.2.0"); + let different_archive_bytes = npm_package_archive_bytes("@acme/plugin", "1.2.1"); + + assert_eq!( + verify_npm_integrity(&archive_bytes, &npm_integrity(&different_archive_bytes)), + Err("npm package archive did not match declared sha512 integrity".to_string()) ); } #[cfg(unix)] #[test] -fn materialize_npm_plugin_source_uses_npm_installed_package_root() { +fn materialize_npm_plugin_source_uses_packed_package_root() { use std::os::unix::fs::PermissionsExt; let codex_home = tempfile::tempdir().expect("create codex home"); let fake_npm_dir = tempfile::tempdir().expect("create fake npm directory"); + let archive_bytes = npm_package_archive_bytes("@acme/plugin", "1.2.0"); + let archive_path = fake_npm_dir.path().join("fixture.tgz"); + fs::write(&archive_path, &archive_bytes).expect("write fixture archive"); let fake_npm = fake_npm_dir.path().join("npm"); fs::write( &fake_npm, - r#"#!/bin/sh -prefix="" + format!( + r#"#!/bin/sh +destination="" previous="" for argument in "$@"; do - if [ "$previous" = "--prefix" ]; then - prefix="$argument" + if [ "$previous" = "--pack-destination" ]; then + destination="$argument" fi previous="$argument" done -mkdir -p "$prefix/node_modules/@acme/plugin/.codex-plugin" -printf '%s\n' "$@" > "$prefix/args.txt" -printf '{"name":"plugin"}\n' > "$prefix/node_modules/@acme/plugin/.codex-plugin/plugin.json" +cp "{}" "$destination/acme-plugin-1.2.0.tgz" +printf '%s\n' "$@" > "$destination/args.txt" "#, + archive_path.display() + ), ) .expect("write fake npm"); let mut permissions = fs::metadata(&fake_npm) @@ -43,15 +63,16 @@ printf '{"name":"plugin"}\n' > "$prefix/node_modules/@acme/plugin/.codex-plugin/ let (plugin_root, tempdir) = materialize_npm_plugin_source_with_command( codex_home.path(), "@acme/plugin", - Some("^1.2.0"), + "1.2.0", Some("https://npm.example.com"), + &npm_integrity(&archive_bytes), fake_npm.as_os_str(), ) .expect("materialize npm source"); assert_eq!( plugin_root.as_path(), - tempdir.path().join("node_modules/@acme/plugin") + tempdir.path().join("extracted/package") ); assert!( plugin_root @@ -60,8 +81,44 @@ printf '{"name":"plugin"}\n' > "$prefix/node_modules/@acme/plugin/.codex-plugin/ .is_file() ); let args = fs::read_to_string(tempdir.path().join("args.txt")).expect("read npm arguments"); + assert!(args.contains("pack")); assert!(args.contains("--ignore-scripts")); assert!(args.contains("--registry")); assert!(args.contains("https://npm.example.com")); - assert!(args.contains("@acme/plugin@^1.2.0")); + assert!(args.contains("@acme/plugin@1.2.0")); + assert!(!args.contains("install")); +} + +fn npm_package_archive_bytes(package: &str, version: &str) -> Vec { + let encoder = GzEncoder::new(Vec::new(), Compression::default()); + let mut archive = tar::Builder::new(encoder); + append_archive_file( + &mut archive, + "package/package.json", + format!(r#"{{"name":"{package}","version":"{version}"}}"#).as_bytes(), + ); + append_archive_file( + &mut archive, + "package/.codex-plugin/plugin.json", + br#"{"name":"plugin"}"#, + ); + let encoder = archive.into_inner().expect("finish tar archive"); + encoder.finish().expect("finish gzip archive") +} + +fn append_archive_file(archive: &mut tar::Builder, path: &str, contents: &[u8]) { + let mut header = tar::Header::new_gnu(); + header.set_size(contents.len() as u64); + header.set_mode(0o644); + header.set_cksum(); + archive + .append_data(&mut header, path, Cursor::new(contents)) + .expect("append archive file"); +} + +fn npm_integrity(archive_bytes: &[u8]) -> String { + format!( + "sha512-{}", + BASE64_STANDARD.encode(Sha512::digest(archive_bytes)) + ) } From 575ccbced8e3c6f8be0edd313d9122edd61fb685 Mon Sep 17 00:00:00 2001 From: charlesgong-openai Date: Mon, 22 Jun 2026 23:01:04 -0700 Subject: [PATCH 3/4] tui: show external import result counts --- codex-rs/tui/src/app/app_server_events.rs | 21 +++-- .../src/external_agent_config_migration.rs | 17 ++++ .../external_agent_config_migration_flow.rs | 55 +++++++++++-- ...ernal_agent_config_migration_flow_tests.rs | 51 +++++++++++- .../external_agent_config_migration_model.rs | 81 +++++++++++++++++++ ...xternal_agent_config_migration_prompt.snap | 3 + ...ernal_agent_config_migration_messages.snap | 12 ++- 7 files changed, 221 insertions(+), 19 deletions(-) diff --git a/codex-rs/tui/src/app/app_server_events.rs b/codex-rs/tui/src/app/app_server_events.rs index 567a4b26a814..9bc55d1d5046 100644 --- a/codex-rs/tui/src/app/app_server_events.rs +++ b/codex-rs/tui/src/app/app_server_events.rs @@ -13,6 +13,8 @@ use codex_app_server_client::AppServerEvent; use codex_app_server_protocol::AuthMode; use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::ServerRequest; +use ratatui::prelude::Stylize as _; +use ratatui::text::Line; impl App { pub(super) fn refresh_mcp_startup_expected_servers_from_config(&mut self) { @@ -102,7 +104,7 @@ impl App { ); return; } - ServerNotification::ExternalAgentConfigImportCompleted(_) => { + ServerNotification::ExternalAgentConfigImportCompleted(notification) => { let should_report_completion = app_server_client.consume_external_agent_config_import_completion(); if let Err(err) = self.refresh_in_memory_config_from_disk().await { @@ -116,11 +118,18 @@ impl App { self.chat_widget.submit_op(AppCommand::reload_user_config()); self.fetch_plugins_list(app_server_client, cwd); if should_report_completion { - self.chat_widget.add_info_message( - crate::external_agent_config_migration_flow::EXTERNAL_AGENT_CONFIG_MIGRATION_FINISHED_MESSAGE - .to_string(), - /*hint*/ None, - ); + let lines = crate::external_agent_config_migration_flow::external_agent_config_migration_finished_lines(notification) + .into_iter() + .enumerate() + .map(|(idx, line)| { + if idx == 0 { + Line::from(vec!["• ".dim(), line.into()]) + } else { + Line::from(vec![" ".into(), line.into()]) + } + }) + .collect(); + self.chat_widget.add_plain_history_lines(lines); } return; } diff --git a/codex-rs/tui/src/external_agent_config_migration.rs b/codex-rs/tui/src/external_agent_config_migration.rs index 9a7e8f329257..995c97f51707 100644 --- a/codex-rs/tui/src/external_agent_config_migration.rs +++ b/codex-rs/tui/src/external_agent_config_migration.rs @@ -1,5 +1,6 @@ use crate::diff_render::display_path_for; use crate::external_agent_config_migration_model::ExternalAgentConfigMigrationGroupModel; +use crate::external_agent_config_migration_model::external_agent_config_migration_count_summary; use crate::external_agent_config_migration_model::external_agent_config_migration_groups; use crate::external_agent_config_migration_model::external_agent_config_migration_item_detail; use crate::external_agent_config_migration_model::external_agent_config_migration_item_label; @@ -599,6 +600,13 @@ impl ExternalAgentConfigMigrationScreen { .iter() .enumerate() .flat_map(|(idx, group)| { + let selected_items = group + .item_indices + .iter() + .filter_map(|idx| self.items.get(*idx)) + .filter(|item| item.enabled) + .map(|item| &item.item); + let count_summary = external_agent_config_migration_count_summary(selected_items); [ RenderLineEntry { item_idx: Some(idx), @@ -614,6 +622,15 @@ impl ExternalAgentConfigMigrationScreen { kind: RenderLineKind::ItemDetail, line: Line::from(format!(" {}", group.description)), }, + RenderLineEntry { + item_idx: None, + kind: RenderLineKind::ItemDetail, + line: Line::from(if count_summary.is_empty() { + " Importing: none".to_string() + } else { + format!(" Importing: {count_summary}") + }), + }, ] }) .collect() diff --git a/codex-rs/tui/src/external_agent_config_migration_flow.rs b/codex-rs/tui/src/external_agent_config_migration_flow.rs index e8611d571878..11e677529040 100644 --- a/codex-rs/tui/src/external_agent_config_migration_flow.rs +++ b/codex-rs/tui/src/external_agent_config_migration_flow.rs @@ -2,12 +2,14 @@ use crate::app_server_session::AppServerSession; use crate::app_server_session::EXTERNAL_AGENT_CONFIG_IMPORT_IN_PROGRESS_MESSAGE; use crate::external_agent_config_migration::ExternalAgentConfigMigrationOutcome; use crate::external_agent_config_migration::run_external_agent_config_migration_prompt; +use crate::external_agent_config_migration_model::external_agent_config_migration_count_summary; +use crate::external_agent_config_migration_model::external_agent_config_migration_type_label; use crate::legacy_core::config::Config; use crate::tui; use codex_app_server_protocol::ExternalAgentConfigDetectParams; +use codex_app_server_protocol::ExternalAgentConfigImportCompletedNotification; +use codex_app_server_protocol::ExternalAgentConfigMigrationItem; -pub(crate) const EXTERNAL_AGENT_CONFIG_MIGRATION_FINISHED_MESSAGE: &str = - "Claude Code import finished. Run /import again to check for additional items."; pub(crate) const EXTERNAL_AGENT_CONFIG_MIGRATION_NO_ITEMS_MESSAGE: &str = "No Claude Code setup was found to import."; pub(crate) const EXTERNAL_AGENT_CONFIG_MIGRATION_REMOTE_UNAVAILABLE_MESSAGE: &str = "Import from Claude Code is unavailable in remote sessions. Start Codex locally and run /import."; @@ -19,14 +21,51 @@ pub(crate) enum ExternalAgentConfigMigrationFlowOutcome { Cancelled, } -fn external_agent_config_migration_success_message(remaining_item_count: usize) -> String { - let message = "Claude Code import started. You can keep working while it finishes. Imported setup will apply to new chats."; +fn external_agent_config_migration_success_message( + selected_items: &[ExternalAgentConfigMigrationItem], + remaining_item_count: usize, +) -> String { + let count_summary = external_agent_config_migration_count_summary(selected_items); + let message = format!( + "Claude Code import started. You can keep working while it finishes. Imported setup will apply to new chats. Importing: {count_summary}." + ); match remaining_items_handoff(remaining_item_count) { Some(remaining_items_handoff) => format!("{message} {remaining_items_handoff}"), - None => message.to_string(), + None => message, } } +pub(crate) fn external_agent_config_migration_finished_lines( + notification: &ExternalAgentConfigImportCompletedNotification, +) -> Vec { + let imported_count = notification + .item_type_results + .iter() + .map(|type_result| type_result.successes.len()) + .sum::(); + let failed_count = notification + .item_type_results + .iter() + .map(|type_result| type_result.failures.len()) + .sum::(); + let mut lines = vec![format!( + "Claude Code import finished: {imported_count} imported, {failed_count} failed." + )]; + if !notification.item_type_results.is_empty() { + lines.push("Results by type:".to_string()); + lines.extend(notification.item_type_results.iter().map(|type_result| { + format!( + " {}: {} imported, {} failed", + external_agent_config_migration_type_label(type_result.item_type), + type_result.successes.len(), + type_result.failures.len() + ) + })); + } + lines.push("Run /import again to check for additional items.".to_string()); + lines +} + fn remaining_items_handoff(remaining_item_count: usize) -> Option { match remaining_item_count { 0 => None, @@ -96,8 +135,10 @@ pub(crate) async fn handle_external_agent_config_migration_prompt( Ok(()) => { let remaining_item_count = detected_items.len().saturating_sub(selected_items.len()); - let success_message = - external_agent_config_migration_success_message(remaining_item_count); + let success_message = external_agent_config_migration_success_message( + &selected_items, + remaining_item_count, + ); return Ok(ExternalAgentConfigMigrationFlowOutcome::Started( success_message, )); diff --git a/codex-rs/tui/src/external_agent_config_migration_flow_tests.rs b/codex-rs/tui/src/external_agent_config_migration_flow_tests.rs index 913b55607fe4..3cef59186ae4 100644 --- a/codex-rs/tui/src/external_agent_config_migration_flow_tests.rs +++ b/codex-rs/tui/src/external_agent_config_migration_flow_tests.rs @@ -1,14 +1,61 @@ use super::*; +use codex_app_server_protocol::ExternalAgentConfigImportItemTypeFailure; +use codex_app_server_protocol::ExternalAgentConfigImportItemTypeSuccess; +use codex_app_server_protocol::ExternalAgentConfigImportTypeResult; +use codex_app_server_protocol::ExternalAgentConfigMigrationItemType; +use std::path::PathBuf; #[test] fn external_agent_config_migration_messages_snapshot() { let cases = [0, 1, 2]; + let selected_items = [ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::Config, + description: "Import settings".to_string(), + cwd: None, + details: None, + }]; + let completed_notification = ExternalAgentConfigImportCompletedNotification { + import_id: "import-1".to_string(), + item_type_results: vec![ + ExternalAgentConfigImportTypeResult { + item_type: ExternalAgentConfigMigrationItemType::Config, + successes: vec![ExternalAgentConfigImportItemTypeSuccess { + item_type: ExternalAgentConfigMigrationItemType::Config, + cwd: None, + source: Some("settings.json".to_string()), + target: Some("config.toml".to_string()), + }], + failures: Vec::new(), + }, + ExternalAgentConfigImportTypeResult { + item_type: ExternalAgentConfigMigrationItemType::Plugins, + successes: vec![ExternalAgentConfigImportItemTypeSuccess { + item_type: ExternalAgentConfigMigrationItemType::Plugins, + cwd: None, + source: Some("formatter@example".to_string()), + target: Some("formatter@example".to_string()), + }], + failures: vec![ExternalAgentConfigImportItemTypeFailure { + item_type: ExternalAgentConfigMigrationItemType::Plugins, + error_type: Some("plugin_install_failed".to_string()), + failure_stage: "plugin_import".to_string(), + message: "install failed".to_string(), + cwd: Some(PathBuf::from("/workspace/project")), + source: Some("deployer@example".to_string()), + }], + }, + ], + }; let messages = cases - .map(external_agent_config_migration_success_message) + .map(|remaining_item_count| { + external_agent_config_migration_success_message(&selected_items, remaining_item_count) + }) .into_iter() + .chain(external_agent_config_migration_finished_lines( + &completed_notification, + )) .chain([ - EXTERNAL_AGENT_CONFIG_MIGRATION_FINISHED_MESSAGE.to_string(), EXTERNAL_AGENT_CONFIG_MIGRATION_NO_ITEMS_MESSAGE.to_string(), EXTERNAL_AGENT_CONFIG_MIGRATION_REMOTE_UNAVAILABLE_MESSAGE.to_string(), EXTERNAL_AGENT_CONFIG_MIGRATION_DAEMON_UNAVAILABLE_MESSAGE.to_string(), diff --git a/codex-rs/tui/src/external_agent_config_migration_model.rs b/codex-rs/tui/src/external_agent_config_migration_model.rs index d03e238da3a0..837650706d07 100644 --- a/codex-rs/tui/src/external_agent_config_migration_model.rs +++ b/codex-rs/tui/src/external_agent_config_migration_model.rs @@ -91,6 +91,87 @@ pub(crate) fn external_agent_config_migration_item_label( } } +pub(crate) fn external_agent_config_migration_type_label( + item_type: ExternalAgentConfigMigrationItemType, +) -> &'static str { + match item_type { + ExternalAgentConfigMigrationItemType::AgentsMd => "Instructions", + ExternalAgentConfigMigrationItemType::Config => "Settings", + ExternalAgentConfigMigrationItemType::Skills => "Skills", + ExternalAgentConfigMigrationItemType::Plugins => "Plugins", + ExternalAgentConfigMigrationItemType::McpServerConfig => "MCP servers", + ExternalAgentConfigMigrationItemType::Subagents => "Agents", + ExternalAgentConfigMigrationItemType::Hooks => "Hooks", + ExternalAgentConfigMigrationItemType::Commands => "Slash commands", + ExternalAgentConfigMigrationItemType::Sessions => "Chat sessions", + } +} + +/// Summarizes the concrete objects represented by selected migration items. +/// +/// Most detected item types carry the objects they will import in `details`; types without +/// details represent one importable file or source directory per migration item. +pub(crate) fn external_agent_config_migration_count_summary<'a>( + items: impl IntoIterator, +) -> String { + let mut counts = Vec::<(ExternalAgentConfigMigrationItemType, usize)>::new(); + for item in items { + let count = match item.item_type { + ExternalAgentConfigMigrationItemType::Plugins => { + item.details.as_ref().map_or(1, |details| { + details + .plugins + .iter() + .map(|plugin_group| plugin_group.plugin_names.len()) + .sum() + }) + } + ExternalAgentConfigMigrationItemType::McpServerConfig => item + .details + .as_ref() + .map_or(1, |details| details.mcp_servers.len()), + ExternalAgentConfigMigrationItemType::Subagents => item + .details + .as_ref() + .map_or(1, |details| details.subagents.len()), + ExternalAgentConfigMigrationItemType::Hooks => item + .details + .as_ref() + .map_or(1, |details| details.hooks.len()), + ExternalAgentConfigMigrationItemType::Commands => item + .details + .as_ref() + .map_or(1, |details| details.commands.len()), + ExternalAgentConfigMigrationItemType::Sessions => item + .details + .as_ref() + .map_or(1, |details| details.sessions.len()), + ExternalAgentConfigMigrationItemType::AgentsMd + | ExternalAgentConfigMigrationItemType::Config + | ExternalAgentConfigMigrationItemType::Skills => 1, + }; + if let Some((_, type_count)) = counts + .iter_mut() + .find(|(item_type, _)| *item_type == item.item_type) + { + *type_count += count; + } else { + counts.push((item.item_type, count)); + } + } + + counts + .into_iter() + .map(|(item_type, count)| { + format!( + "{} {count}", + external_agent_config_migration_type_label(item_type) + ) + }) + .collect::>() + .join(", ") +} + pub(crate) fn external_agent_config_migration_item_detail( item: &ExternalAgentConfigMigrationItem, ) -> Option { diff --git a/codex-rs/tui/src/snapshots/codex_tui__external_agent_config_migration__tests__external_agent_config_migration_prompt.snap b/codex-rs/tui/src/snapshots/codex_tui__external_agent_config_migration__tests__external_agent_config_migration_prompt.snap index d7274eefc2bc..165473c5a779 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__external_agent_config_migration__tests__external_agent_config_migration_prompt.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__external_agent_config_migration__tests__external_agent_config_migration_prompt.snap @@ -10,10 +10,13 @@ expression: rendered Standard Claude Chat data cannot be imported. [x] Tools & setup Settings, instructions, integrations, agents, commands, and skills + Importing: Settings 1 [x] Current project Add Codex files alongside your existing project files + Importing: Plugins 6, Instructions 1 [x] Chat sessions (1) Last 30 days of chats + Importing: Chat sessions 1 Selected 4 of 4 items. › 1. Import selected diff --git a/codex-rs/tui/src/snapshots/codex_tui__external_agent_config_migration_flow__tests__external_agent_config_migration_messages.snap b/codex-rs/tui/src/snapshots/codex_tui__external_agent_config_migration_flow__tests__external_agent_config_migration_messages.snap index ad3d3ac063b5..d8d6194685d6 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__external_agent_config_migration_flow__tests__external_agent_config_migration_messages.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__external_agent_config_migration_flow__tests__external_agent_config_migration_messages.snap @@ -2,10 +2,14 @@ source: tui/src/external_agent_config_migration_flow_tests.rs expression: messages --- -Claude Code import started. You can keep working while it finishes. Imported setup will apply to new chats. -Claude Code import started. You can keep working while it finishes. Imported setup will apply to new chats. 1 additional item remains. After it finishes, run /import again to review it. -Claude Code import started. You can keep working while it finishes. Imported setup will apply to new chats. 2 additional items remain. After it finishes, run /import again to review them. -Claude Code import finished. Run /import again to check for additional items. +Claude Code import started. You can keep working while it finishes. Imported setup will apply to new chats. Importing: Settings 1. +Claude Code import started. You can keep working while it finishes. Imported setup will apply to new chats. Importing: Settings 1. 1 additional item remains. After it finishes, run /import again to review it. +Claude Code import started. You can keep working while it finishes. Imported setup will apply to new chats. Importing: Settings 1. 2 additional items remain. After it finishes, run /import again to review them. +Claude Code import finished: 2 imported, 1 failed. +Results by type: + Settings: 1 imported, 0 failed + Plugins: 1 imported, 1 failed +Run /import again to check for additional items. No Claude Code setup was found to import. Import from Claude Code is unavailable in remote sessions. Start Codex locally and run /import. Import from Claude Code is unavailable while Codex is connected to the local app-server daemon. Stop the daemon, restart Codex, and run /import. From 97b2d5aa4b0806faea130f9157423234b89220d7 Mon Sep 17 00:00:00 2001 From: charlesgong-openai Date: Mon, 22 Jun 2026 23:03:56 -0700 Subject: [PATCH 4/4] Revert "tui: show external import result counts" This reverts commit 575ccbced8e3c6f8be0edd313d9122edd61fb685. --- codex-rs/tui/src/app/app_server_events.rs | 21 ++--- .../src/external_agent_config_migration.rs | 17 ---- .../external_agent_config_migration_flow.rs | 55 ++----------- ...ernal_agent_config_migration_flow_tests.rs | 51 +----------- .../external_agent_config_migration_model.rs | 81 ------------------- ...xternal_agent_config_migration_prompt.snap | 3 - ...ernal_agent_config_migration_messages.snap | 12 +-- 7 files changed, 19 insertions(+), 221 deletions(-) diff --git a/codex-rs/tui/src/app/app_server_events.rs b/codex-rs/tui/src/app/app_server_events.rs index 9bc55d1d5046..567a4b26a814 100644 --- a/codex-rs/tui/src/app/app_server_events.rs +++ b/codex-rs/tui/src/app/app_server_events.rs @@ -13,8 +13,6 @@ use codex_app_server_client::AppServerEvent; use codex_app_server_protocol::AuthMode; use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::ServerRequest; -use ratatui::prelude::Stylize as _; -use ratatui::text::Line; impl App { pub(super) fn refresh_mcp_startup_expected_servers_from_config(&mut self) { @@ -104,7 +102,7 @@ impl App { ); return; } - ServerNotification::ExternalAgentConfigImportCompleted(notification) => { + ServerNotification::ExternalAgentConfigImportCompleted(_) => { let should_report_completion = app_server_client.consume_external_agent_config_import_completion(); if let Err(err) = self.refresh_in_memory_config_from_disk().await { @@ -118,18 +116,11 @@ impl App { self.chat_widget.submit_op(AppCommand::reload_user_config()); self.fetch_plugins_list(app_server_client, cwd); if should_report_completion { - let lines = crate::external_agent_config_migration_flow::external_agent_config_migration_finished_lines(notification) - .into_iter() - .enumerate() - .map(|(idx, line)| { - if idx == 0 { - Line::from(vec!["• ".dim(), line.into()]) - } else { - Line::from(vec![" ".into(), line.into()]) - } - }) - .collect(); - self.chat_widget.add_plain_history_lines(lines); + self.chat_widget.add_info_message( + crate::external_agent_config_migration_flow::EXTERNAL_AGENT_CONFIG_MIGRATION_FINISHED_MESSAGE + .to_string(), + /*hint*/ None, + ); } return; } diff --git a/codex-rs/tui/src/external_agent_config_migration.rs b/codex-rs/tui/src/external_agent_config_migration.rs index 995c97f51707..9a7e8f329257 100644 --- a/codex-rs/tui/src/external_agent_config_migration.rs +++ b/codex-rs/tui/src/external_agent_config_migration.rs @@ -1,6 +1,5 @@ use crate::diff_render::display_path_for; use crate::external_agent_config_migration_model::ExternalAgentConfigMigrationGroupModel; -use crate::external_agent_config_migration_model::external_agent_config_migration_count_summary; use crate::external_agent_config_migration_model::external_agent_config_migration_groups; use crate::external_agent_config_migration_model::external_agent_config_migration_item_detail; use crate::external_agent_config_migration_model::external_agent_config_migration_item_label; @@ -600,13 +599,6 @@ impl ExternalAgentConfigMigrationScreen { .iter() .enumerate() .flat_map(|(idx, group)| { - let selected_items = group - .item_indices - .iter() - .filter_map(|idx| self.items.get(*idx)) - .filter(|item| item.enabled) - .map(|item| &item.item); - let count_summary = external_agent_config_migration_count_summary(selected_items); [ RenderLineEntry { item_idx: Some(idx), @@ -622,15 +614,6 @@ impl ExternalAgentConfigMigrationScreen { kind: RenderLineKind::ItemDetail, line: Line::from(format!(" {}", group.description)), }, - RenderLineEntry { - item_idx: None, - kind: RenderLineKind::ItemDetail, - line: Line::from(if count_summary.is_empty() { - " Importing: none".to_string() - } else { - format!(" Importing: {count_summary}") - }), - }, ] }) .collect() diff --git a/codex-rs/tui/src/external_agent_config_migration_flow.rs b/codex-rs/tui/src/external_agent_config_migration_flow.rs index 11e677529040..e8611d571878 100644 --- a/codex-rs/tui/src/external_agent_config_migration_flow.rs +++ b/codex-rs/tui/src/external_agent_config_migration_flow.rs @@ -2,14 +2,12 @@ use crate::app_server_session::AppServerSession; use crate::app_server_session::EXTERNAL_AGENT_CONFIG_IMPORT_IN_PROGRESS_MESSAGE; use crate::external_agent_config_migration::ExternalAgentConfigMigrationOutcome; use crate::external_agent_config_migration::run_external_agent_config_migration_prompt; -use crate::external_agent_config_migration_model::external_agent_config_migration_count_summary; -use crate::external_agent_config_migration_model::external_agent_config_migration_type_label; use crate::legacy_core::config::Config; use crate::tui; use codex_app_server_protocol::ExternalAgentConfigDetectParams; -use codex_app_server_protocol::ExternalAgentConfigImportCompletedNotification; -use codex_app_server_protocol::ExternalAgentConfigMigrationItem; +pub(crate) const EXTERNAL_AGENT_CONFIG_MIGRATION_FINISHED_MESSAGE: &str = + "Claude Code import finished. Run /import again to check for additional items."; pub(crate) const EXTERNAL_AGENT_CONFIG_MIGRATION_NO_ITEMS_MESSAGE: &str = "No Claude Code setup was found to import."; pub(crate) const EXTERNAL_AGENT_CONFIG_MIGRATION_REMOTE_UNAVAILABLE_MESSAGE: &str = "Import from Claude Code is unavailable in remote sessions. Start Codex locally and run /import."; @@ -21,51 +19,14 @@ pub(crate) enum ExternalAgentConfigMigrationFlowOutcome { Cancelled, } -fn external_agent_config_migration_success_message( - selected_items: &[ExternalAgentConfigMigrationItem], - remaining_item_count: usize, -) -> String { - let count_summary = external_agent_config_migration_count_summary(selected_items); - let message = format!( - "Claude Code import started. You can keep working while it finishes. Imported setup will apply to new chats. Importing: {count_summary}." - ); +fn external_agent_config_migration_success_message(remaining_item_count: usize) -> String { + let message = "Claude Code import started. You can keep working while it finishes. Imported setup will apply to new chats."; match remaining_items_handoff(remaining_item_count) { Some(remaining_items_handoff) => format!("{message} {remaining_items_handoff}"), - None => message, + None => message.to_string(), } } -pub(crate) fn external_agent_config_migration_finished_lines( - notification: &ExternalAgentConfigImportCompletedNotification, -) -> Vec { - let imported_count = notification - .item_type_results - .iter() - .map(|type_result| type_result.successes.len()) - .sum::(); - let failed_count = notification - .item_type_results - .iter() - .map(|type_result| type_result.failures.len()) - .sum::(); - let mut lines = vec![format!( - "Claude Code import finished: {imported_count} imported, {failed_count} failed." - )]; - if !notification.item_type_results.is_empty() { - lines.push("Results by type:".to_string()); - lines.extend(notification.item_type_results.iter().map(|type_result| { - format!( - " {}: {} imported, {} failed", - external_agent_config_migration_type_label(type_result.item_type), - type_result.successes.len(), - type_result.failures.len() - ) - })); - } - lines.push("Run /import again to check for additional items.".to_string()); - lines -} - fn remaining_items_handoff(remaining_item_count: usize) -> Option { match remaining_item_count { 0 => None, @@ -135,10 +96,8 @@ pub(crate) async fn handle_external_agent_config_migration_prompt( Ok(()) => { let remaining_item_count = detected_items.len().saturating_sub(selected_items.len()); - let success_message = external_agent_config_migration_success_message( - &selected_items, - remaining_item_count, - ); + let success_message = + external_agent_config_migration_success_message(remaining_item_count); return Ok(ExternalAgentConfigMigrationFlowOutcome::Started( success_message, )); diff --git a/codex-rs/tui/src/external_agent_config_migration_flow_tests.rs b/codex-rs/tui/src/external_agent_config_migration_flow_tests.rs index 3cef59186ae4..913b55607fe4 100644 --- a/codex-rs/tui/src/external_agent_config_migration_flow_tests.rs +++ b/codex-rs/tui/src/external_agent_config_migration_flow_tests.rs @@ -1,61 +1,14 @@ use super::*; -use codex_app_server_protocol::ExternalAgentConfigImportItemTypeFailure; -use codex_app_server_protocol::ExternalAgentConfigImportItemTypeSuccess; -use codex_app_server_protocol::ExternalAgentConfigImportTypeResult; -use codex_app_server_protocol::ExternalAgentConfigMigrationItemType; -use std::path::PathBuf; #[test] fn external_agent_config_migration_messages_snapshot() { let cases = [0, 1, 2]; - let selected_items = [ExternalAgentConfigMigrationItem { - item_type: ExternalAgentConfigMigrationItemType::Config, - description: "Import settings".to_string(), - cwd: None, - details: None, - }]; - let completed_notification = ExternalAgentConfigImportCompletedNotification { - import_id: "import-1".to_string(), - item_type_results: vec![ - ExternalAgentConfigImportTypeResult { - item_type: ExternalAgentConfigMigrationItemType::Config, - successes: vec![ExternalAgentConfigImportItemTypeSuccess { - item_type: ExternalAgentConfigMigrationItemType::Config, - cwd: None, - source: Some("settings.json".to_string()), - target: Some("config.toml".to_string()), - }], - failures: Vec::new(), - }, - ExternalAgentConfigImportTypeResult { - item_type: ExternalAgentConfigMigrationItemType::Plugins, - successes: vec![ExternalAgentConfigImportItemTypeSuccess { - item_type: ExternalAgentConfigMigrationItemType::Plugins, - cwd: None, - source: Some("formatter@example".to_string()), - target: Some("formatter@example".to_string()), - }], - failures: vec![ExternalAgentConfigImportItemTypeFailure { - item_type: ExternalAgentConfigMigrationItemType::Plugins, - error_type: Some("plugin_install_failed".to_string()), - failure_stage: "plugin_import".to_string(), - message: "install failed".to_string(), - cwd: Some(PathBuf::from("/workspace/project")), - source: Some("deployer@example".to_string()), - }], - }, - ], - }; let messages = cases - .map(|remaining_item_count| { - external_agent_config_migration_success_message(&selected_items, remaining_item_count) - }) + .map(external_agent_config_migration_success_message) .into_iter() - .chain(external_agent_config_migration_finished_lines( - &completed_notification, - )) .chain([ + EXTERNAL_AGENT_CONFIG_MIGRATION_FINISHED_MESSAGE.to_string(), EXTERNAL_AGENT_CONFIG_MIGRATION_NO_ITEMS_MESSAGE.to_string(), EXTERNAL_AGENT_CONFIG_MIGRATION_REMOTE_UNAVAILABLE_MESSAGE.to_string(), EXTERNAL_AGENT_CONFIG_MIGRATION_DAEMON_UNAVAILABLE_MESSAGE.to_string(), diff --git a/codex-rs/tui/src/external_agent_config_migration_model.rs b/codex-rs/tui/src/external_agent_config_migration_model.rs index 837650706d07..d03e238da3a0 100644 --- a/codex-rs/tui/src/external_agent_config_migration_model.rs +++ b/codex-rs/tui/src/external_agent_config_migration_model.rs @@ -91,87 +91,6 @@ pub(crate) fn external_agent_config_migration_item_label( } } -pub(crate) fn external_agent_config_migration_type_label( - item_type: ExternalAgentConfigMigrationItemType, -) -> &'static str { - match item_type { - ExternalAgentConfigMigrationItemType::AgentsMd => "Instructions", - ExternalAgentConfigMigrationItemType::Config => "Settings", - ExternalAgentConfigMigrationItemType::Skills => "Skills", - ExternalAgentConfigMigrationItemType::Plugins => "Plugins", - ExternalAgentConfigMigrationItemType::McpServerConfig => "MCP servers", - ExternalAgentConfigMigrationItemType::Subagents => "Agents", - ExternalAgentConfigMigrationItemType::Hooks => "Hooks", - ExternalAgentConfigMigrationItemType::Commands => "Slash commands", - ExternalAgentConfigMigrationItemType::Sessions => "Chat sessions", - } -} - -/// Summarizes the concrete objects represented by selected migration items. -/// -/// Most detected item types carry the objects they will import in `details`; types without -/// details represent one importable file or source directory per migration item. -pub(crate) fn external_agent_config_migration_count_summary<'a>( - items: impl IntoIterator, -) -> String { - let mut counts = Vec::<(ExternalAgentConfigMigrationItemType, usize)>::new(); - for item in items { - let count = match item.item_type { - ExternalAgentConfigMigrationItemType::Plugins => { - item.details.as_ref().map_or(1, |details| { - details - .plugins - .iter() - .map(|plugin_group| plugin_group.plugin_names.len()) - .sum() - }) - } - ExternalAgentConfigMigrationItemType::McpServerConfig => item - .details - .as_ref() - .map_or(1, |details| details.mcp_servers.len()), - ExternalAgentConfigMigrationItemType::Subagents => item - .details - .as_ref() - .map_or(1, |details| details.subagents.len()), - ExternalAgentConfigMigrationItemType::Hooks => item - .details - .as_ref() - .map_or(1, |details| details.hooks.len()), - ExternalAgentConfigMigrationItemType::Commands => item - .details - .as_ref() - .map_or(1, |details| details.commands.len()), - ExternalAgentConfigMigrationItemType::Sessions => item - .details - .as_ref() - .map_or(1, |details| details.sessions.len()), - ExternalAgentConfigMigrationItemType::AgentsMd - | ExternalAgentConfigMigrationItemType::Config - | ExternalAgentConfigMigrationItemType::Skills => 1, - }; - if let Some((_, type_count)) = counts - .iter_mut() - .find(|(item_type, _)| *item_type == item.item_type) - { - *type_count += count; - } else { - counts.push((item.item_type, count)); - } - } - - counts - .into_iter() - .map(|(item_type, count)| { - format!( - "{} {count}", - external_agent_config_migration_type_label(item_type) - ) - }) - .collect::>() - .join(", ") -} - pub(crate) fn external_agent_config_migration_item_detail( item: &ExternalAgentConfigMigrationItem, ) -> Option { diff --git a/codex-rs/tui/src/snapshots/codex_tui__external_agent_config_migration__tests__external_agent_config_migration_prompt.snap b/codex-rs/tui/src/snapshots/codex_tui__external_agent_config_migration__tests__external_agent_config_migration_prompt.snap index 165473c5a779..d7274eefc2bc 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__external_agent_config_migration__tests__external_agent_config_migration_prompt.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__external_agent_config_migration__tests__external_agent_config_migration_prompt.snap @@ -10,13 +10,10 @@ expression: rendered Standard Claude Chat data cannot be imported. [x] Tools & setup Settings, instructions, integrations, agents, commands, and skills - Importing: Settings 1 [x] Current project Add Codex files alongside your existing project files - Importing: Plugins 6, Instructions 1 [x] Chat sessions (1) Last 30 days of chats - Importing: Chat sessions 1 Selected 4 of 4 items. › 1. Import selected diff --git a/codex-rs/tui/src/snapshots/codex_tui__external_agent_config_migration_flow__tests__external_agent_config_migration_messages.snap b/codex-rs/tui/src/snapshots/codex_tui__external_agent_config_migration_flow__tests__external_agent_config_migration_messages.snap index d8d6194685d6..ad3d3ac063b5 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__external_agent_config_migration_flow__tests__external_agent_config_migration_messages.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__external_agent_config_migration_flow__tests__external_agent_config_migration_messages.snap @@ -2,14 +2,10 @@ source: tui/src/external_agent_config_migration_flow_tests.rs expression: messages --- -Claude Code import started. You can keep working while it finishes. Imported setup will apply to new chats. Importing: Settings 1. -Claude Code import started. You can keep working while it finishes. Imported setup will apply to new chats. Importing: Settings 1. 1 additional item remains. After it finishes, run /import again to review it. -Claude Code import started. You can keep working while it finishes. Imported setup will apply to new chats. Importing: Settings 1. 2 additional items remain. After it finishes, run /import again to review them. -Claude Code import finished: 2 imported, 1 failed. -Results by type: - Settings: 1 imported, 0 failed - Plugins: 1 imported, 1 failed -Run /import again to check for additional items. +Claude Code import started. You can keep working while it finishes. Imported setup will apply to new chats. +Claude Code import started. You can keep working while it finishes. Imported setup will apply to new chats. 1 additional item remains. After it finishes, run /import again to review it. +Claude Code import started. You can keep working while it finishes. Imported setup will apply to new chats. 2 additional items remain. After it finishes, run /import again to review them. +Claude Code import finished. Run /import again to check for additional items. No Claude Code setup was found to import. Import from Claude Code is unavailable in remote sessions. Start Codex locally and run /import. Import from Claude Code is unavailable while Codex is connected to the local app-server daemon. Stop the daemon, restart Codex, and run /import.