From bc8222b8d9e44377a3d7c7b7970e32e7c29ec34f Mon Sep 17 00:00:00 2001 From: charlesgong-openai Date: Mon, 13 Jul 2026 22:11:07 +0000 Subject: [PATCH] Prepare external agent migration for source adapters (#32884) ## What changed - Add an optional `source` selector to `externalAgentConfig/detect`, while preserving the existing behavior for omitted, `null`, and unrecognized values. - Pass `claude-code` as the source for TUI detection and import requests. - Move source-specific migration behavior behind an adapter boundary and consolidate session migration into `codex-external-agent-migration`. ## Testing - Cover explicit, omitted, and unknown detection source values with app-server protocol tests. - Add unit coverage for the extracted migration and hook helpers. GitOrigin-RevId: 63bd3dcb485f3ad7bd996022db4e351506aeea77 --- codex-rs/Cargo.lock | 17 +- codex-rs/Cargo.toml | 2 - .../schema/json/ClientRequest.json | 9 +- .../codex_app_server_protocol.schemas.json | 9 +- .../codex_app_server_protocol.v2.schemas.json | 9 +- .../v2/ExternalAgentConfigDetectParams.json | 7 + .../v2/ExternalAgentConfigImportParams.json | 2 +- .../v2/ExternalAgentConfigDetectParams.ts | 7 +- .../v2/ExternalAgentConfigImportParams.ts | 4 +- .../src/protocol/v2/config.rs | 8 +- codex-rs/app-server/Cargo.toml | 1 - codex-rs/app-server/README.md | 2 +- codex-rs/app-server/src/config/mod.rs | 1 - .../src/external_agent_migration/mod.rs | 6 + .../processor.rs} | 44 +- .../processor_tests.rs} | 0 .../service.rs} | 846 +++-------- .../service/source.rs | 242 +++ .../service/source_cla.rs | 483 ++++++ .../external_agent_migration/service/utils.rs | 139 ++ .../service_tests.rs} | 25 +- .../session_importer.rs} | 16 +- codex-rs/app-server/src/lib.rs | 2 +- codex-rs/app-server/src/message_processor.rs | 4 +- codex-rs/app-server/src/request_processors.rs | 4 - .../tests/suite/v2/external_agent_config.rs | 47 +- codex-rs/external-agent-migration/Cargo.toml | 6 + .../external-agent-migration/src/hooks_cla.rs | 233 +++ codex-rs/external-agent-migration/src/lib.rs | 1338 ++++------------- .../external-agent-migration/src/lib_tests.rs | 920 ++++++++++++ .../src/sessions/detect_cla.rs} | 52 +- .../src/sessions}/export.rs | 16 +- .../src/sessions}/ledger.rs | 2 +- .../src/sessions}/ledger_tests.rs | 0 .../src/sessions/mod.rs} | 4 +- .../src/sessions}/records.rs | 18 +- .../src/sessions}/title.rs | 4 +- .../src/sessions}/title_tests.rs | 0 codex-rs/external-agent-sessions/BUILD.bazel | 6 - codex-rs/external-agent-sessions/Cargo.toml | 25 - codex-rs/tui/src/app_server_session.rs | 2 +- .../external_agent_config_migration_flow.rs | 1 + 42 files changed, 2713 insertions(+), 1850 deletions(-) delete mode 100644 codex-rs/app-server/src/config/mod.rs create mode 100644 codex-rs/app-server/src/external_agent_migration/mod.rs rename codex-rs/app-server/src/{request_processors/external_agent_config_processor.rs => external_agent_migration/processor.rs} (95%) rename codex-rs/app-server/src/{request_processors/external_agent_config_processor_tests.rs => external_agent_migration/processor_tests.rs} (100%) rename codex-rs/app-server/src/{config/external_agent_config.rs => external_agent_migration/service.rs} (70%) create mode 100644 codex-rs/app-server/src/external_agent_migration/service/source.rs create mode 100644 codex-rs/app-server/src/external_agent_migration/service/source_cla.rs create mode 100644 codex-rs/app-server/src/external_agent_migration/service/utils.rs rename codex-rs/app-server/src/{config/external_agent_config_tests.rs => external_agent_migration/service_tests.rs} (99%) rename codex-rs/app-server/src/{request_processors/external_agent_session_import.rs => external_agent_migration/session_importer.rs} (94%) create mode 100644 codex-rs/external-agent-migration/src/hooks_cla.rs create mode 100644 codex-rs/external-agent-migration/src/lib_tests.rs rename codex-rs/{external-agent-sessions/src/detect.rs => external-agent-migration/src/sessions/detect_cla.rs} (89%) rename codex-rs/{external-agent-sessions/src => external-agent-migration/src/sessions}/export.rs (98%) rename codex-rs/{external-agent-sessions/src => external-agent-migration/src/sessions}/ledger.rs (99%) rename codex-rs/{external-agent-sessions/src => external-agent-migration/src/sessions}/ledger_tests.rs (100%) rename codex-rs/{external-agent-sessions/src/lib.rs => external-agent-migration/src/sessions/mod.rs} (98%) rename codex-rs/{external-agent-sessions/src => external-agent-migration/src/sessions}/records.rs (96%) rename codex-rs/{external-agent-sessions/src => external-agent-migration/src/sessions}/title.rs (98%) rename codex-rs/{external-agent-sessions/src => external-agent-migration/src/sessions}/title_tests.rs (100%) delete mode 100644 codex-rs/external-agent-sessions/BUILD.bazel delete mode 100644 codex-rs/external-agent-sessions/Cargo.toml diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index cf3d0a3b47..c17a319155 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1989,7 +1989,6 @@ dependencies = [ "codex-exec-server", "codex-extension-api", "codex-external-agent-migration", - "codex-external-agent-sessions", "codex-features", "codex-feedback", "codex-file-search", @@ -3023,27 +3022,19 @@ dependencies = [ [[package]] name = "codex-external-agent-migration" version = "0.0.0" -dependencies = [ - "codex-hooks", - "pretty_assertions", - "serde_json", - "serde_yaml", - "tempfile", - "toml 0.9.11+spec-1.1.0", -] - -[[package]] -name = "codex-external-agent-sessions" -version = "0.0.0" dependencies = [ "chrono", "codex-app-server-protocol", + "codex-hooks", "codex-protocol", "codex-utils-output-truncation", + "pretty_assertions", "serde", "serde_json", + "serde_yaml", "sha2 0.10.9", "tempfile", + "toml 0.9.11+spec-1.1.0", ] [[package]] diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 14669d2b9f..3f0acfd4a7 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -59,7 +59,6 @@ members = [ "ext/skills", "ext/web-search", "external-agent-migration", - "external-agent-sessions", "keyring-store", "file-search", "file-watcher", @@ -187,7 +186,6 @@ codex-goal-extension = { path = "ext/goal" } codex-guardian = { path = "ext/guardian" } codex-image-generation-extension = { path = "ext/image-generation" } codex-external-agent-migration = { path = "external-agent-migration" } -codex-external-agent-sessions = { path = "external-agent-sessions" } codex-experimental-api-macros = { path = "codex-experimental-api-macros" } codex-features = { path = "features" } codex-feedback = { path = "feedback" } diff --git a/codex-rs/app-server-protocol/schema/json/ClientRequest.json b/codex-rs/app-server-protocol/schema/json/ClientRequest.json index 5da27a074c..80b82ef13e 100644 --- a/codex-rs/app-server-protocol/schema/json/ClientRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ClientRequest.json @@ -810,6 +810,13 @@ "includeHome": { "description": "If true, include detection under the user's home directory.", "type": "boolean" + }, + "source": { + "description": "Optional import-source selector. Missing or unrecognized values use the default source for backwards compatibility.", + "type": [ + "string", + "null" + ] } }, "type": "object" @@ -823,7 +830,7 @@ "type": "array" }, "source": { - "description": "Source product that produced the migration items. Missing means unspecified.", + "description": "Import-source selector used to produce the migration items. Pass the same value to detection and import; missing or unrecognized values use the default source for backwards compatibility.", "type": [ "string", "null" 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 5b6c8f13cb..15d8812d94 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 @@ -9237,6 +9237,13 @@ "includeHome": { "description": "If true, include detection under the user's home directory.", "type": "boolean" + }, + "source": { + "description": "Optional import-source selector. Missing or unrecognized values use the default source for backwards compatibility.", + "type": [ + "string", + "null" + ] } }, "title": "ExternalAgentConfigDetectParams", @@ -9400,7 +9407,7 @@ "type": "array" }, "source": { - "description": "Source product that produced the migration items. Missing means unspecified.", + "description": "Import-source selector used to produce the migration items. Pass the same value to detection and import; missing or unrecognized values use the default source for backwards compatibility.", "type": [ "string", "null" 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 e7077f707d..7906d696a1 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 @@ -5477,6 +5477,13 @@ "includeHome": { "description": "If true, include detection under the user's home directory.", "type": "boolean" + }, + "source": { + "description": "Optional import-source selector. Missing or unrecognized values use the default source for backwards compatibility.", + "type": [ + "string", + "null" + ] } }, "title": "ExternalAgentConfigDetectParams", @@ -5640,7 +5647,7 @@ "type": "array" }, "source": { - "description": "Source product that produced the migration items. Missing means unspecified.", + "description": "Import-source selector used to produce the migration items. Pass the same value to detection and import; missing or unrecognized values use the default source for backwards compatibility.", "type": [ "string", "null" diff --git a/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigDetectParams.json b/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigDetectParams.json index 01fda3b071..51b698d774 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigDetectParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigDetectParams.json @@ -14,6 +14,13 @@ "includeHome": { "description": "If true, include detection under the user's home directory.", "type": "boolean" + }, + "source": { + "description": "Optional import-source selector. Missing or unrecognized values use the default source for backwards compatibility.", + "type": [ + "string", + "null" + ] } }, "title": "ExternalAgentConfigDetectParams", diff --git a/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigImportParams.json b/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigImportParams.json index d02241f95b..4b5b9b4d3b 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigImportParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigImportParams.json @@ -204,7 +204,7 @@ "type": "array" }, "source": { - "description": "Source product that produced the migration items. Missing means unspecified.", + "description": "Import-source selector used to produce the migration items. Pass the same value to detection and import; missing or unrecognized values use the default source for backwards compatibility.", "type": [ "string", "null" diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigDetectParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigDetectParams.ts index 48b55e9b9a..5bfdcfa19a 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigDetectParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigDetectParams.ts @@ -10,4 +10,9 @@ includeHome?: boolean, /** * Zero or more working directories to include for repo-scoped detection. */ -cwds?: Array | null, }; +cwds?: Array | null, +/** + * Optional import-source selector. Missing or unrecognized values use the default source for + * backwards compatibility. + */ +source?: string | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigImportParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigImportParams.ts index be7f7ffe30..87dcf98fcc 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigImportParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigImportParams.ts @@ -5,6 +5,8 @@ import type { ExternalAgentConfigMigrationItem } from "./ExternalAgentConfigMigr export type ExternalAgentConfigImportParams = { migrationItems: Array, /** - * Source product that produced the migration items. Missing means unspecified. + * Import-source selector used to produce the migration items. Pass the same value to detection + * and import; missing or unrecognized values use the default source for backwards + * compatibility. */ source?: string | null, }; diff --git a/codex-rs/app-server-protocol/src/protocol/v2/config.rs b/codex-rs/app-server-protocol/src/protocol/v2/config.rs index 24d574a935..f161b1808b 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/config.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/config.rs @@ -684,6 +684,10 @@ pub struct ExternalAgentConfigDetectParams { /// Zero or more working directories to include for repo-scoped detection. #[ts(optional = nullable)] pub cwds: Option>, + /// Optional import-source selector. Missing or unrecognized values use the default source for + /// backwards compatibility. + #[ts(optional = nullable)] + pub source: Option, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] @@ -691,7 +695,9 @@ pub struct ExternalAgentConfigDetectParams { #[ts(export_to = "v2/")] pub struct ExternalAgentConfigImportParams { pub migration_items: Vec, - /// Source product that produced the migration items. Missing means unspecified. + /// Import-source selector used to produce the migration items. Pass the same value to detection + /// and import; missing or unrecognized values use the default source for backwards + /// compatibility. #[ts(optional = nullable)] pub source: Option, } diff --git a/codex-rs/app-server/Cargo.toml b/codex-rs/app-server/Cargo.toml index ac6b2fcb12..dd4485b70c 100644 --- a/codex-rs/app-server/Cargo.toml +++ b/codex-rs/app-server/Cargo.toml @@ -44,7 +44,6 @@ codex-home = { workspace = true } codex-exec-server = { workspace = true } codex-extension-api = { workspace = true } codex-external-agent-migration = { workspace = true } -codex-external-agent-sessions = { workspace = true } codex-features = { workspace = true } codex-goal-extension = { workspace = true } codex-guardian = { workspace = true } diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 6c72453a1a..7612436f5b 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -239,7 +239,7 @@ Example with notification opt-out: - `windowsSandbox/setupStart` — start Windows sandbox setup for the selected mode (`elevated` or `unelevated`); accepts an optional absolute `cwd` to target setup for a specific workspace, returns `{ started: true }` immediately, and later emits `windowsSandbox/setupCompleted`. - `feedback/upload` — submit a feedback report (classification + optional reason/logs, conversation_id, and optional `extraLogFiles` attachments array); returns the tracking thread id. - `config/read` — fetch the effective config on disk after resolving config layering, including opaque `desktop` values stored in `config.toml`. -- `externalAgentConfig/detect` — detect migratable external-agent artifacts with `includeHome` and optional `cwds`; each detected item includes `cwd` (`null` for home), and multi-item migrations may additionally include structured `details` with plugin ids, skill names, session metadata, or other artifact names. +- `externalAgentConfig/detect` — detect migratable external-agent artifacts with `includeHome`, optional `cwds`, and an optional `source` identifier reserved for adapter selection; omitted, `null`, or unrecognized values retain the default behavior. Each detected item includes `cwd` (`null` for home), and multi-item migrations may additionally include structured `details` with plugin ids, skill names, session metadata, or other artifact names. - `externalAgentConfig/import` — apply selected external-agent migration items by passing explicit `migrationItems` with `cwd` (`null` for home) and any `details` returned by detect. Callers may pass `source` to identify the product that initiated the import; omitted or `null` means unspecified. The response acknowledges the synchronous import phase with an `importId`. Expected migration failures are reported as per-item failures rather than JSON-RPC errors, so the server still returns that `importId` and emits `externalAgentConfig/import/completed` with the same ID once all synchronous and background work finishes. The completion notification contains type-level `itemTypeResults` with successes and failures, including raw failure messages for the client to report separately. - `config/value/write` — write a single config key/value to the user's config.toml on disk; dotted paths such as `desktop.someKey` use the same generic write surface. - `config/batchWrite` — apply multiple config edits atomically to the user's config.toml on disk, with optional `reloadUserConfig: true` to hot-reload loaded threads, including multiple `desktop.*` edits. diff --git a/codex-rs/app-server/src/config/mod.rs b/codex-rs/app-server/src/config/mod.rs deleted file mode 100644 index 95d64d152a..0000000000 --- a/codex-rs/app-server/src/config/mod.rs +++ /dev/null @@ -1 +0,0 @@ -pub(crate) mod external_agent_config; diff --git a/codex-rs/app-server/src/external_agent_migration/mod.rs b/codex-rs/app-server/src/external_agent_migration/mod.rs new file mode 100644 index 0000000000..440656c36f --- /dev/null +++ b/codex-rs/app-server/src/external_agent_migration/mod.rs @@ -0,0 +1,6 @@ +mod processor; +mod service; +mod session_importer; + +pub(crate) use processor::ExternalAgentConfigRequestProcessor; +pub(crate) use processor::ExternalAgentConfigRequestProcessorArgs; diff --git a/codex-rs/app-server/src/request_processors/external_agent_config_processor.rs b/codex-rs/app-server/src/external_agent_migration/processor.rs similarity index 95% rename from codex-rs/app-server/src/request_processors/external_agent_config_processor.rs rename to codex-rs/app-server/src/external_agent_migration/processor.rs index c71abcb884..f064050743 100644 --- a/codex-rs/app-server/src/request_processors/external_agent_config_processor.rs +++ b/codex-rs/app-server/src/external_agent_migration/processor.rs @@ -1,20 +1,10 @@ use std::sync::Arc; -use crate::config::external_agent_config::ExternalAgentConfigDetectOptions; -use crate::config::external_agent_config::ExternalAgentConfigImportItemResult as CoreImportItemResult; -use crate::config::external_agent_config::ExternalAgentConfigImportOutcome as CoreImportOutcome; -use crate::config::external_agent_config::ExternalAgentConfigImportRawError as CoreImportRawError; -use crate::config::external_agent_config::ExternalAgentConfigMigrationItem as CoreMigrationItem; -use crate::config::external_agent_config::ExternalAgentConfigMigrationItemType as CoreMigrationItemType; -use crate::config::external_agent_config::ExternalAgentConfigService; -use crate::config::external_agent_config::NamedMigration as CoreNamedMigration; -use crate::config::external_agent_config::PendingPluginImport; -use crate::config::external_agent_config::PluginImportOutcome; -use crate::config::external_agent_config::record_import_error; use crate::config_manager::ConfigManager; use crate::error_code::internal_error; use crate::outgoing_message::ConnectionRequestId; use crate::outgoing_message::OutgoingMessageSender; +use crate::request_processors::ConfigRequestProcessor; use codex_analytics::AnalyticsEventsClient; use codex_analytics::ExternalAgentConfigImportCompletedInput; use codex_analytics::ExternalAgentConfigImportFailureInput; @@ -41,7 +31,7 @@ use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::SkillMigration; use codex_arg0::Arg0DispatchPaths; use codex_core::ThreadManager; -use codex_external_agent_sessions::ExternalAgentSessionMigration as CoreSessionMigration; +use codex_external_agent_migration::sessions::ExternalAgentSessionMigration as CoreSessionMigration; use codex_rollout::StateDbHandle; use codex_state::ExternalAgentConfigImportFailureRecord; use codex_state::ExternalAgentConfigImportSuccessRecord; @@ -49,8 +39,18 @@ use codex_thread_store::ThreadStore; use std::collections::HashSet; use std::path::PathBuf; -use super::ConfigRequestProcessor; -use super::external_agent_session_import::ExternalAgentSessionImporter; +use super::service::ExternalAgentConfigDetectOptions; +use super::service::ExternalAgentConfigImportItemResult as CoreImportItemResult; +use super::service::ExternalAgentConfigImportOutcome as CoreImportOutcome; +use super::service::ExternalAgentConfigImportRawError as CoreImportRawError; +use super::service::ExternalAgentConfigMigrationItem as CoreMigrationItem; +use super::service::ExternalAgentConfigMigrationItemType as CoreMigrationItemType; +use super::service::ExternalAgentConfigService; +use super::service::NamedMigration as CoreNamedMigration; +use super::service::PendingPluginImport; +use super::service::PluginImportOutcome; +use super::service::record_import_error; +use super::session_importer::ExternalAgentSessionImporter; use uuid::Uuid; #[derive(Clone)] @@ -487,15 +487,13 @@ impl ExternalAgentConfigRequestProcessor { description: migration_item.description, cwd: migration_item.cwd, details: migration_item.details.map(|details| { - crate::config::external_agent_config::MigrationDetails { + super::service::MigrationDetails { plugins: details .plugins .into_iter() - .map(|plugin| { - crate::config::external_agent_config::PluginsMigration { - marketplace_name: plugin.marketplace_name, - plugin_names: plugin.plugin_names, - } + .map(|plugin| super::service::PluginsMigration { + marketplace_name: plugin.marketplace_name, + plugin_names: plugin.plugin_names, }) .collect(), skills: details @@ -832,7 +830,7 @@ fn protocol_import_type_result(item_result: &CoreImportItemResult) -> ProtocolIm } fn protocol_import_success( - success: &crate::config::external_agent_config::ExternalAgentConfigImportSuccess, + success: &super::service::ExternalAgentConfigImportSuccess, ) -> ProtocolImportSuccess { ProtocolImportSuccess { item_type: protocol_migration_item_type(success.item_type), @@ -898,5 +896,5 @@ fn migration_items_need_runtime_refresh(items: &[ExternalAgentConfigMigrationIte } #[cfg(test)] -#[path = "external_agent_config_processor_tests.rs"] -mod external_agent_config_processor_tests; +#[path = "processor_tests.rs"] +mod tests; diff --git a/codex-rs/app-server/src/request_processors/external_agent_config_processor_tests.rs b/codex-rs/app-server/src/external_agent_migration/processor_tests.rs similarity index 100% rename from codex-rs/app-server/src/request_processors/external_agent_config_processor_tests.rs rename to codex-rs/app-server/src/external_agent_migration/processor_tests.rs diff --git a/codex-rs/app-server/src/config/external_agent_config.rs b/codex-rs/app-server/src/external_agent_migration/service.rs similarity index 70% rename from codex-rs/app-server/src/config/external_agent_config.rs rename to codex-rs/app-server/src/external_agent_migration/service.rs index fa092cfb3c..c6dbd1ec86 100644 --- a/codex-rs/app-server/src/config/external_agent_config.rs +++ b/codex-rs/app-server/src/external_agent_migration/service.rs @@ -1,3 +1,7 @@ +mod source; +mod source_cla; +mod utils; + use codex_analytics::AnalyticsEventsClient; use codex_analytics::PluginInstallSource; use codex_config::types::PluginConfig; @@ -12,18 +16,9 @@ use codex_core_plugins::marketplace::find_marketplace_manifest_path; use codex_core_plugins::marketplace_add::MarketplaceAddRequest; use codex_core_plugins::marketplace_add::add_marketplace; use codex_core_plugins::marketplace_add::is_local_marketplace_source; -use codex_external_agent_migration::build_mcp_config_from_external; -use codex_external_agent_migration::count_missing_commands; use codex_external_agent_migration::count_missing_subagents; -use codex_external_agent_migration::hook_migration_event_names; -use codex_external_agent_migration::import_commands; -use codex_external_agent_migration::import_hooks; -use codex_external_agent_migration::import_subagents; -use codex_external_agent_migration::missing_command_names; use codex_external_agent_migration::missing_subagent_names; -use codex_external_agent_sessions::ExternalAgentSessionMigration; -use codex_external_agent_sessions::detect_recent_sessions; -use codex_plugin::PluginId; +use codex_external_agent_migration::sessions::ExternalAgentSessionMigration; use codex_protocol::protocol::Product; use serde_json::Value as JsonValue; use std::collections::BTreeMap; @@ -36,13 +31,25 @@ use std::path::Path; use std::path::PathBuf; use toml::Value as TomlValue; +use self::source::ExternalAgentSource; +use self::source::InstructionSourceGroup; +use self::source::MarketplaceImportSource; +use self::source::PluginDetectionContext; +use self::source::SourceFeature; +#[cfg(test)] +use self::source_cla::CONFIG_DIR as EXTERNAL_AGENT_DIR; +#[cfg(test)] +use self::source_cla::CONFIG_MD as EXTERNAL_AGENT_CONFIG_MD; +#[cfg(test)] +use self::source_cla::KNOWN_MARKETPLACES_PATH as EXTERNAL_AGENT_KNOWN_MARKETPLACES_PATH; +#[cfg(test)] +use self::source_cla::OFFICIAL_MARKETPLACE_NAME as EXTERNAL_OFFICIAL_MARKETPLACE_NAME; +use self::utils::copy_dir_recursive; +use self::utils::display_source_paths; +use self::utils::rewrite_external_agent_terms; + const EXTERNAL_AGENT_CONFIG_DETECT_METRIC: &str = "codex.external_agent_config.detect"; const EXTERNAL_AGENT_CONFIG_IMPORT_METRIC: &str = "codex.external_agent_config.import"; -const EXTERNAL_AGENT_DIR: &str = ".claude"; -const EXTERNAL_AGENT_CONFIG_MD: &str = "CLAUDE.md"; -const EXTERNAL_AGENT_KNOWN_MARKETPLACES_PATH: &str = "plugins/known_marketplaces.json"; -const EXTERNAL_OFFICIAL_MARKETPLACE_NAME: &str = "claude-plugins-official"; -const EXTERNAL_OFFICIAL_MARKETPLACE_SOURCE: &str = "anthropics/claude-plugins-official"; #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct ExternalAgentConfigDetectOptions { @@ -182,15 +189,18 @@ pub(crate) struct ExternalAgentConfigService { codex_home: PathBuf, external_agent_home: PathBuf, analytics_events_client: Option, + source: ExternalAgentSource, } impl ExternalAgentConfigService { pub(crate) fn new(codex_home: PathBuf, analytics_events_client: AnalyticsEventsClient) -> Self { - let external_agent_home = default_external_agent_home(); + let source = ExternalAgentSource::default(); + let external_agent_home = default_external_agent_home(source); Self { codex_home, external_agent_home, analytics_events_client: Some(analytics_events_client), + source, } } @@ -200,6 +210,7 @@ impl ExternalAgentConfigService { codex_home, external_agent_home, analytics_events_client: None, + source: ExternalAgentSource::default(), } } @@ -296,7 +307,9 @@ impl ExternalAgentConfigService { ); Ok(()) })(), - ExternalAgentConfigMigrationItemType::Plugins => { + ExternalAgentConfigMigrationItemType::Plugins + if self.source.supports(SourceFeature::Plugins) => + { async { let cwd = migration_item.cwd; let details = match migration_item.details { @@ -369,6 +382,7 @@ impl ExternalAgentConfigService { } .await } + ExternalAgentConfigMigrationItemType::Plugins => Ok(()), ExternalAgentConfigMigrationItemType::McpServerConfig => (|| { let migrated_server_names = self.import_mcp_server_config(migration_item.cwd.as_deref())?; @@ -452,17 +466,18 @@ impl ExternalAgentConfigService { items: &mut Vec, ) -> io::Result<()> { let cwd = repo_root.map(Path::to_path_buf); - let source_settings = repo_root.map_or_else( - || self.external_agent_home.join("settings.json"), - |repo_root| repo_root.join(EXTERNAL_AGENT_DIR).join("settings.json"), - ); - let settings = effective_external_settings(&source_settings)?; + let source_settings = self.source_settings(repo_root); + let settings = if self.source.supports(SourceFeature::Config) { + self.effective_source_settings(repo_root)? + } else { + None + }; let target_config = repo_root.map_or_else( || self.codex_home.join("config.toml"), |repo_root| repo_root.join(".codex").join("config.toml"), ); if let Some(settings) = settings.as_ref() { - let migrated = build_config_from_external(settings)?; + let migrated = build_config_from_external(settings, self.source)?; if !is_empty_toml_table(&migrated) { let mut should_include = true; if target_config.exists() { @@ -497,13 +512,8 @@ impl ExternalAgentConfigService { } } - let source_root = self.source_root(repo_root); - let mcp_settings = self.mcp_settings(repo_root, settings.clone())?; - let migrated_mcp = build_mcp_config_from_external( - source_root.as_path(), - Some(self.external_agent_home.as_path()), - mcp_settings.as_ref(), - )?; + let mcp_source_path = self.source.mcp_source_path(self.source_root(repo_root)); + let migrated_mcp = self.build_mcp_config(repo_root, settings.clone())?; let mut mcp_server_names = migrated_mcp_server_names(&migrated_mcp); if !is_empty_toml_table(&migrated_mcp) { if target_config.exists() { @@ -523,7 +533,7 @@ impl ExternalAgentConfigService { item_type: ExternalAgentConfigMigrationItemType::McpServerConfig, description: format!( "Migrate MCP servers from {} into {}", - source_root.display(), + mcp_source_path.display(), target_config.display() ), cwd: cwd.clone(), @@ -540,16 +550,14 @@ impl ExternalAgentConfigService { } } - let source_external_agent_dir = repo_root.map_or_else( - || self.external_agent_home.clone(), - |repo_root| repo_root.join(EXTERNAL_AGENT_DIR), - ); + let source_external_agent_dir = self.source_config_dir(repo_root); let target_hooks = repo_root.map_or_else( || self.codex_home.join("hooks.json"), |repo_root| repo_root.join(".codex").join("hooks.json"), ); - let hook_event_names = - hook_migration_event_names(source_external_agent_dir.as_path(), &target_hooks)?; + let hook_event_names = self + .source + .hook_event_names(source_external_agent_dir.as_path(), &target_hooks)?; if !hook_event_names.is_empty() && is_missing_or_empty_text_file(&target_hooks)? { items.push(ExternalAgentConfigMigrationItem { item_type: ExternalAgentConfigMigrationItemType::Hooks, @@ -573,7 +581,7 @@ impl ExternalAgentConfigService { let source_skills = repo_root.map_or_else( || self.external_agent_home.join("skills"), - |repo_root| repo_root.join(EXTERNAL_AGENT_DIR).join("skills"), + |repo_root| repo_root.join(self.source.config_dir()).join("skills"), ); let target_skills = repo_root.map_or_else( || self.home_target_skills_dir(), @@ -607,9 +615,13 @@ impl ExternalAgentConfigService { || self.home_target_skills_dir(), |repo_root| repo_root.join(".agents").join("skills"), ); - let commands_count = count_missing_commands(&source_commands, &target_command_skills)?; + let commands_count = self + .source + .count_missing_commands(&source_commands, &target_command_skills)?; if commands_count > 0 { - let command_names = missing_command_names(&source_commands, &target_command_skills)?; + let command_names = self + .source + .missing_command_names(&source_commands, &target_command_skills)?; items.push(ExternalAgentConfigMigrationItem { item_type: ExternalAgentConfigMigrationItemType::Commands, description: format!( @@ -658,27 +670,32 @@ impl ExternalAgentConfigService { ); } - let source_agents_md = if let Some(repo_root) = repo_root { - find_repo_agents_md_source(repo_root)? + let instruction_source_groups = if let Some(repo_root) = repo_root { + self.repo_agents_md_source_groups(repo_root)? } else { - let path = self.external_agent_home.join(EXTERNAL_AGENT_CONFIG_MD); - is_non_empty_text_file(&path)?.then_some(path) + let sources = self.home_agents_md_sources()?; + (!sources.is_empty()) + .then(|| InstructionSourceGroup { + scope: self.codex_home.clone(), + sources, + }) + .into_iter() + .collect() }; - let target_agents_md = repo_root.map_or_else( - || self.codex_home.join("AGENTS.md"), - |repo_root| repo_root.join("AGENTS.md"), - ); - if let Some(source_agents_md) = source_agents_md - && is_missing_or_empty_text_file(&target_agents_md)? - { + for group in instruction_source_groups { + let target_agents_md = group.scope.join("AGENTS.md"); + if !is_missing_or_empty_text_file(&target_agents_md)? { + continue; + } + let item_cwd = repo_root.is_some().then(|| group.scope.clone()); items.push(ExternalAgentConfigMigrationItem { item_type: ExternalAgentConfigMigrationItemType::AgentsMd, description: format!( "Migrate {} to {}", - source_agents_md.display(), + display_source_paths(&group.sources), target_agents_md.display() ), - cwd: cwd.clone(), + cwd: item_cwd, details: None, }); emit_migration_metric( @@ -688,7 +705,9 @@ impl ExternalAgentConfigService { ); } - if let Some(settings) = settings.as_ref() { + if self.source.supports(SourceFeature::Plugins) + && self.source.can_detect_plugins(settings.as_ref()) + { match ConfigBuilder::default() .codex_home(self.codex_home.clone()) .fallback_cwd(Some(self.codex_home.clone())) @@ -715,15 +734,26 @@ impl ExternalAgentConfigService { &config, &PluginsManager::new(self.codex_home.clone()), )?; - if let Some(item) = self.detect_plugin_migration( - source_settings.as_path(), - repo_root.unwrap_or(self.external_agent_home.as_path()), - cwd.clone(), - settings, - &configured_plugin_ids, - &configured_marketplace_plugins, - ) { - items.push(item); + let source_root = repo_root.unwrap_or(self.external_agent_home.as_path()); + if let Some(detected) = self.source.detect_plugins(PluginDetectionContext { + external_agent_home: self.external_agent_home.as_path(), + source_settings: source_settings.as_path(), + source_root, + settings: settings.as_ref(), + configured_plugin_ids: &configured_plugin_ids, + configured_marketplace_plugins: &configured_marketplace_plugins, + })? { + emit_migration_metric( + EXTERNAL_AGENT_CONFIG_DETECT_METRIC, + ExternalAgentConfigMigrationItemType::Plugins, + /*skills_count*/ None, + ); + items.push(ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::Plugins, + description: detected.description, + cwd: cwd.clone(), + details: Some(detected.details), + }); } } Err(err) => { @@ -736,8 +766,10 @@ impl ExternalAgentConfigService { } } - if repo_root.is_none() { - let sessions = detect_recent_sessions(&self.external_agent_home, &self.codex_home)?; + if repo_root.is_none() && self.source.supports(SourceFeature::Sessions) { + let sessions = self + .source + .detect_recent_sessions(&self.external_agent_home, &self.codex_home)?; if !sessions.is_empty() { items.push(ExternalAgentConfigMigrationItem { item_type: ExternalAgentConfigMigrationItemType::Sessions, @@ -769,14 +801,60 @@ impl ExternalAgentConfigService { .unwrap_or_else(|| PathBuf::from(".agents").join("skills")) } + fn source_config_dir(&self, repo_root: Option<&Path>) -> PathBuf { + repo_root.map_or_else( + || self.external_agent_home.clone(), + |repo_root| repo_root.join(self.source.config_dir()), + ) + } + + fn source_settings(&self, repo_root: Option<&Path>) -> PathBuf { + self.source_config_dir(repo_root) + .join(self.source.settings_file_name(repo_root.is_some())) + } + + fn effective_source_settings(&self, repo_root: Option<&Path>) -> io::Result> { + let source_settings = self.source_settings(repo_root); + self.source.effective_settings(&source_settings) + } + + fn build_mcp_config( + &self, + repo_root: Option<&Path>, + settings: Option, + ) -> io::Result { + let settings = if self.source.supports(SourceFeature::Config) { + self.mcp_settings(repo_root, settings)? + } else { + None + }; + self.source.build_mcp_config( + self.source_root(repo_root).as_path(), + self.external_agent_home.as_path(), + settings.as_ref(), + ) + } + + fn repo_agents_md_source_groups( + &self, + repo_root: &Path, + ) -> io::Result> { + self.source.repo_instruction_source_groups(repo_root) + } + + fn home_agents_md_sources(&self) -> io::Result> { + self.source + .home_instruction_sources(self.external_agent_home.as_path()) + } + fn mcp_settings( &self, repo_root: Option<&Path>, source_settings: Option, ) -> io::Result> { if repo_root.is_some() && source_settings.is_none() { - let home_settings = self.external_agent_home.join("settings.json"); - match effective_external_settings(&home_settings) { + let home_settings = self.source_settings(/*repo_root*/ None); + match self.effective_source_settings(/*repo_root*/ None) { Ok(settings) => Ok(settings), Err(err) => { tracing::warn!( @@ -804,34 +882,17 @@ impl ExternalAgentConfigService { ) } - fn detect_plugin_migration( + fn marketplace_import_sources( &self, - source_settings: &Path, - source_root: &Path, - cwd: Option, - settings: &JsonValue, - configured_plugin_ids: &HashSet, - configured_marketplace_plugins: &BTreeMap>, - ) -> Option { - let import_sources = self.marketplace_import_sources(settings, source_root); - let plugin_details = extract_plugin_migration_details( - settings, - &import_sources, - configured_plugin_ids, - configured_marketplace_plugins, - )?; - emit_migration_metric( - EXTERNAL_AGENT_CONFIG_DETECT_METRIC, - ExternalAgentConfigMigrationItemType::Plugins, - /*skills_count*/ None, - ); - - Some(ExternalAgentConfigMigrationItem { - item_type: ExternalAgentConfigMigrationItemType::Plugins, - description: format!("Migrate enabled plugins from {}", source_settings.display()), - cwd, - details: Some(plugin_details), - }) + cwd: Option<&Path>, + ) -> io::Result> { + let source_root = cwd.unwrap_or(self.external_agent_home.as_path()); + let source_settings = self.source_settings(cwd); + self.source.marketplace_import_sources( + self.external_agent_home.as_path(), + source_root, + &source_settings, + ) } fn partition_plugin_migration_details( @@ -839,14 +900,7 @@ impl ExternalAgentConfigService { cwd: Option<&Path>, details: MigrationDetails, ) -> io::Result<(Option, Option)> { - let source_settings = cwd.map_or_else( - || self.external_agent_home.join("settings.json"), - |cwd| cwd.join(EXTERNAL_AGENT_DIR).join("settings.json"), - ); - let source_root = cwd.unwrap_or(self.external_agent_home.as_path()); - let import_sources = effective_external_settings(&source_settings)? - .map(|settings| self.marketplace_import_sources(&settings, source_root)) - .unwrap_or_default(); + let import_sources = self.marketplace_import_sources(cwd)?; let mut local_plugins = Vec::new(); let mut remote_plugins = Vec::new(); @@ -907,14 +961,7 @@ impl ExternalAgentConfigService { if let Some(analytics_events_client) = self.analytics_events_client.clone() { plugins_manager.set_analytics_events_client(analytics_events_client); } - let source_settings = cwd.map_or_else( - || self.external_agent_home.join("settings.json"), - |cwd| cwd.join(EXTERNAL_AGENT_DIR).join("settings.json"), - ); - let source_root = cwd.unwrap_or(self.external_agent_home.as_path()); - let import_sources = effective_external_settings(&source_settings)? - .map(|settings| self.marketplace_import_sources(&settings, source_root)) - .unwrap_or_default(); + let import_sources = self.marketplace_import_sources(cwd)?; for plugin_group in plugins { let marketplace_name = plugin_group.marketplace_name.clone(); let plugin_names = plugin_group.plugin_names; @@ -1044,114 +1091,28 @@ impl ExternalAgentConfigService { Ok(outcome) } - fn marketplace_import_sources( - &self, - settings: &JsonValue, - source_root: &Path, - ) -> BTreeMap { - let known_marketplaces_path = self - .external_agent_home - .join(EXTERNAL_AGENT_KNOWN_MARKETPLACES_PATH); - let known_marketplaces = match read_external_settings(&known_marketplaces_path) { - Ok(known_marketplaces) => known_marketplaces, - Err(err) => { - tracing::warn!( - path = %known_marketplaces_path.display(), - error = %err, - "ignoring invalid external agent marketplace registry" - ); - None - } - }; - let mut import_sources = known_marketplaces - .as_ref() - .map(|known_marketplaces| { - collect_marketplace_import_sources( - known_marketplaces, - self.external_agent_home.as_path(), - ) - }) - .unwrap_or_default(); - - if let Some(extra_known_marketplaces) = settings - .as_object() - .and_then(|settings| settings.get("extraKnownMarketplaces")) - { - let mut scoped_marketplaces = extra_known_marketplaces.clone(); - if let Some(scoped_marketplaces) = scoped_marketplaces.as_object_mut() { - for (name, scoped_marketplace) in scoped_marketplaces { - import_sources.remove(name); - let Some(known_marketplace) = known_marketplaces - .as_ref() - .and_then(JsonValue::as_object) - .and_then(|known_marketplaces| known_marketplaces.get(name)) - else { - continue; - }; - if scoped_marketplace.get("source") != known_marketplace.get("source") { - continue; - } - let Some(install_location) = known_marketplace - .get("installLocation") - .and_then(JsonValue::as_str) - else { - continue; - }; - let install_location = Path::new(install_location); - let install_location = if install_location.is_absolute() { - install_location.to_path_buf() - } else { - self.external_agent_home.join(install_location) - }; - let Some(scoped_marketplace) = scoped_marketplace.as_object_mut() else { - continue; - }; - scoped_marketplace.insert( - "installLocation".to_string(), - JsonValue::String(install_location.display().to_string()), - ); - } - } - import_sources.extend(collect_marketplace_import_sources( - &scoped_marketplaces, - source_root, - )); - } - - if has_enabled_plugin_for_marketplace(settings, EXTERNAL_OFFICIAL_MARKETPLACE_NAME) - && !import_sources.contains_key(EXTERNAL_OFFICIAL_MARKETPLACE_NAME) - { - import_sources.insert( - EXTERNAL_OFFICIAL_MARKETPLACE_NAME.to_string(), - MarketplaceImportSource { - source: EXTERNAL_OFFICIAL_MARKETPLACE_SOURCE.to_string(), - ref_name: None, - }, - ); - } - - import_sources - } - fn import_config(&self, cwd: Option<&Path>) -> io::Result> { + if !self.source.supports(SourceFeature::Config) { + return Ok(None); + } let repo_root = find_repo_root(cwd)?; let (source_settings, target_config) = if let Some(repo_root) = repo_root.as_ref() { ( - repo_root.join(EXTERNAL_AGENT_DIR).join("settings.json"), + self.source_settings(Some(repo_root)), repo_root.join(".codex").join("config.toml"), ) } else if cwd.is_some_and(|cwd| !cwd.as_os_str().is_empty()) { return Ok(None); } else { ( - self.external_agent_home.join("settings.json"), + self.source_settings(/*repo_root*/ None), self.codex_home.join("config.toml"), ) }; - let Some(settings) = effective_external_settings(&source_settings)? else { + let Some(settings) = self.effective_source_settings(repo_root.as_deref())? else { return Ok(None); }; - let migrated = build_config_from_external(&settings)?; + let migrated = build_config_from_external(&settings, self.source)?; if is_empty_toml_table(&migrated) { return Ok(None); } @@ -1190,28 +1151,19 @@ impl ExternalAgentConfigService { fn import_mcp_server_config(&self, cwd: Option<&Path>) -> io::Result> { let repo_root = find_repo_root(cwd)?; - let (source_settings, target_config) = if let Some(repo_root) = repo_root.as_ref() { - ( - repo_root.join(EXTERNAL_AGENT_DIR).join("settings.json"), - repo_root.join(".codex").join("config.toml"), - ) + let target_config = if let Some(repo_root) = repo_root.as_ref() { + repo_root.join(".codex").join("config.toml") } else if cwd.is_some_and(|cwd| !cwd.as_os_str().is_empty()) { return Ok(Vec::new()); } else { - ( - self.external_agent_home.join("settings.json"), - self.codex_home.join("config.toml"), - ) + self.codex_home.join("config.toml") }; - let settings = self.mcp_settings( - repo_root.as_deref(), - effective_external_settings(&source_settings)?, - )?; - let migrated = build_mcp_config_from_external( - self.source_root(repo_root.as_deref()).as_path(), - Some(self.external_agent_home.as_path()), - settings.as_ref(), - )?; + let settings = if self.source.supports(SourceFeature::Config) { + self.effective_source_settings(repo_root.as_deref())? + } else { + None + }; + let migrated = self.build_mcp_config(repo_root.as_deref(), settings)?; if is_empty_toml_table(&migrated) { return Ok(Vec::new()); } @@ -1243,7 +1195,7 @@ impl ExternalAgentConfigService { fn import_subagents(&self, cwd: Option<&Path>) -> io::Result> { let (source_agents, target_agents) = if let Some(repo_root) = find_repo_root(cwd)? { ( - repo_root.join(EXTERNAL_AGENT_DIR).join("agents"), + repo_root.join(self.source.config_dir()).join("agents"), repo_root.join(".codex").join("agents"), ) } else if cwd.is_some_and(|cwd| !cwd.as_os_str().is_empty()) { @@ -1255,14 +1207,14 @@ impl ExternalAgentConfigService { ) }; - import_subagents(&source_agents, &target_agents) + self.source.import_subagents(&source_agents, &target_agents) } fn import_hooks(&self, cwd: Option<&Path>) -> io::Result> { let (source_external_agent_dir, target_hooks) = if let Some(repo_root) = find_repo_root(cwd)? { ( - repo_root.join(EXTERNAL_AGENT_DIR), + self.source_config_dir(Some(repo_root.as_path())), repo_root.join(".codex").join("hooks.json"), ) } else if cwd.is_some_and(|cwd| !cwd.as_os_str().is_empty()) { @@ -1274,8 +1226,13 @@ impl ExternalAgentConfigService { ) }; - let hook_names = hook_migration_event_names(&source_external_agent_dir, &target_hooks)?; - if import_hooks(&source_external_agent_dir, &target_hooks)? { + let hook_names = self + .source + .hook_event_names(&source_external_agent_dir, &target_hooks)?; + if self + .source + .import_hooks(&source_external_agent_dir, &target_hooks)? + { Ok(hook_names) } else { Ok(Vec::new()) @@ -1285,7 +1242,7 @@ impl ExternalAgentConfigService { fn import_commands(&self, cwd: Option<&Path>) -> io::Result> { let (source_commands, target_skills) = if let Some(repo_root) = find_repo_root(cwd)? { ( - repo_root.join(EXTERNAL_AGENT_DIR).join("commands"), + repo_root.join(self.source.config_dir()).join("commands"), repo_root.join(".agents").join("skills"), ) } else if cwd.is_some_and(|cwd| !cwd.as_os_str().is_empty()) { @@ -1297,13 +1254,14 @@ impl ExternalAgentConfigService { ) }; - import_commands(&source_commands, &target_skills) + self.source + .import_commands(&source_commands, &target_skills) } fn import_skills(&self, cwd: Option<&Path>) -> io::Result> { let (source_skills, target_skills) = if let Some(repo_root) = find_repo_root(cwd)? { ( - repo_root.join(EXTERNAL_AGENT_DIR).join("skills"), + repo_root.join(self.source.config_dir()).join("skills"), repo_root.join(".agents").join("skills"), ) } else if cwd.is_some_and(|cwd| !cwd.as_os_str().is_empty()) { @@ -1333,7 +1291,7 @@ impl ExternalAgentConfigService { continue; } - copy_dir_recursive(&entry.path(), &target)?; + copy_dir_recursive(&entry.path(), &target, self.source.rewrite_profile())?; copied_names.push(entry.file_name().to_string_lossy().to_string()); } @@ -1341,22 +1299,28 @@ impl ExternalAgentConfigService { } fn import_agents_md(&self, cwd: Option<&Path>) -> io::Result> { - let (source_agents_md, target_agents_md) = if let Some(repo_root) = find_repo_root(cwd)? { - let Some(source_agents_md) = find_repo_agents_md_source(&repo_root)? else { - return Ok(None); + let (source_agents_md, target_agents_md) = + if let Some(requested_scope) = cwd.filter(|cwd| !cwd.as_os_str().is_empty()) { + let Some(repo_root) = find_repo_root(Some(requested_scope))? else { + return Ok(None); + }; + let Some(group) = self + .repo_agents_md_source_groups(&repo_root)? + .into_iter() + .find(|group| group.scope == repo_root) + else { + return Ok(None); + }; + let target_agents_md = group.scope.join("AGENTS.md"); + (group.sources, target_agents_md) + } else { + let source_agents_md = self.home_agents_md_sources()?; + if source_agents_md.is_empty() { + return Ok(None); + } + (source_agents_md, self.codex_home.join("AGENTS.md")) }; - (source_agents_md, repo_root.join("AGENTS.md")) - } else if cwd.is_some_and(|cwd| !cwd.as_os_str().is_empty()) { - return Ok(None); - } else { - ( - self.external_agent_home.join(EXTERNAL_AGENT_CONFIG_MD), - self.codex_home.join("AGENTS.md"), - ) - }; - if !is_non_empty_text_file(&source_agents_md)? - || !is_missing_or_empty_text_file(&target_agents_md)? - { + if !is_missing_or_empty_text_file(&target_agents_md)? { return Ok(None); } @@ -1365,20 +1329,29 @@ impl ExternalAgentConfigService { }; fs::create_dir_all(target_parent)?; - rewrite_and_copy_text_file(&source_agents_md, &target_agents_md)?; + let source_contents = source_agents_md + .iter() + .map(|source| { + self.source.read_instruction_source(source).map(|contents| { + rewrite_external_agent_terms(&contents, self.source.rewrite_profile()) + }) + }) + .collect::>>()? + .join("\n\n"); + fs::write(&target_agents_md, source_contents)?; Ok(Some(( - source_agents_md.display().to_string(), + display_source_paths(&source_agents_md), target_agents_md.display().to_string(), ))) } } -fn default_external_agent_home() -> PathBuf { +fn default_external_agent_home(source: ExternalAgentSource) -> PathBuf { if let Some(home) = std::env::var_os("HOME").or_else(|| std::env::var_os("USERPROFILE")) { - return PathBuf::from(home).join(EXTERNAL_AGENT_DIR); + return PathBuf::from(home).join(source.config_dir()); } - PathBuf::from(EXTERNAL_AGENT_DIR) + PathBuf::from(source.config_dir()) } fn read_external_settings(path: &Path) -> io::Result> { @@ -1392,145 +1365,6 @@ fn read_external_settings(path: &Path) -> io::Result> { Ok(Some(settings)) } -fn effective_external_settings(project_settings: &Path) -> io::Result> { - let mut effective = read_external_settings(project_settings)?; - let Some(settings_dir) = project_settings.parent() else { - return Ok(effective); - }; - let local_settings = settings_dir.join("settings.local.json"); - let local_settings = match read_external_settings(&local_settings) { - Ok(Some(local_settings)) => local_settings, - Ok(None) => return Ok(effective), - Err(err) if err.kind() == io::ErrorKind::InvalidData => return Ok(effective), - Err(err) => return Err(err), - }; - if let Some(effective) = effective.as_mut() { - merge_json_settings(effective, &local_settings); - } else { - effective = Some(local_settings); - } - Ok(effective) -} - -fn merge_json_settings(existing: &mut JsonValue, incoming: &JsonValue) { - match (existing, incoming) { - (JsonValue::Object(existing), JsonValue::Object(incoming)) => { - for (key, incoming_value) in incoming { - match existing.get_mut(key) { - Some(existing_value) => merge_json_settings(existing_value, incoming_value), - None => { - existing.insert(key.clone(), incoming_value.clone()); - } - } - } - } - (existing, incoming) => { - *existing = incoming.clone(); - } - } -} -fn extract_plugin_migration_details( - settings: &JsonValue, - import_sources: &BTreeMap, - configured_plugin_ids: &HashSet, - configured_marketplace_plugins: &BTreeMap>, -) -> Option { - let loadable_marketplaces = import_sources - .iter() - .filter_map(|(marketplace_name, source)| { - is_local_marketplace_source(&source.source, source.ref_name.clone()) - .ok() - .map(|_| marketplace_name.clone()) - }) - .collect::>(); - let mut plugins = BTreeMap::new(); - for plugin_id in collect_enabled_plugins(settings) - .into_iter() - .filter(|plugin_id| !configured_plugin_ids.contains(plugin_id)) - { - let Ok(plugin_id) = PluginId::parse(&plugin_id) else { - continue; - }; - if let Some(installable_plugins) = - configured_marketplace_plugins.get(&plugin_id.marketplace_name) - { - if !installable_plugins.contains(&plugin_id.plugin_name) { - tracing::warn!( - plugin_id = %plugin_id.as_key(), - marketplace_name = %plugin_id.marketplace_name, - "enabled external agent plugin was not found in configured marketplace" - ); - continue; - } - } else if !loadable_marketplaces.contains(&plugin_id.marketplace_name) { - tracing::warn!( - plugin_id = %plugin_id.as_key(), - marketplace_name = %plugin_id.marketplace_name, - "marketplace source was not found for enabled external agent plugin" - ); - continue; - } - let plugin_group = plugins - .entry(plugin_id.marketplace_name.clone()) - .or_insert_with(|| PluginsMigration { - marketplace_name: plugin_id.marketplace_name.clone(), - plugin_names: Vec::new(), - }); - plugin_group.plugin_names.push(plugin_id.plugin_name); - } - - let plugins = plugins - .into_values() - .filter_map(|mut plugin_group| { - if plugin_group.plugin_names.is_empty() { - return None; - } - plugin_group.plugin_names.sort(); - Some(plugin_group) - }) - .collect::>(); - if plugins.is_empty() { - return None; - } - - Some(MigrationDetails { - plugins, - ..Default::default() - }) -} - -fn collect_enabled_plugins(settings: &JsonValue) -> Vec { - let Some(enabled_plugins) = settings - .as_object() - .and_then(|settings| settings.get("enabledPlugins")) - .and_then(JsonValue::as_object) - else { - return Vec::new(); - }; - - enabled_plugins - .iter() - .filter_map(|(plugin_key, enabled)| { - if !enabled.as_bool().unwrap_or(false) { - return None; - } - PluginId::parse(plugin_key) - .ok() - .map(|plugin_id| plugin_id.as_key()) - }) - .collect() -} - -fn has_enabled_plugin_for_marketplace(settings: &JsonValue, marketplace_name: &str) -> bool { - collect_enabled_plugins(settings) - .into_iter() - .any(|plugin_id| { - PluginId::parse(&plugin_id) - .map(|plugin_id| plugin_id.marketplace_name == marketplace_name) - .unwrap_or(false) - }) -} - fn configured_marketplace_plugins( config: &Config, plugins_manager: &PluginsManager, @@ -1563,106 +1397,6 @@ fn configured_marketplace_plugins( Ok(marketplace_plugins) } -fn collect_marketplace_import_sources( - marketplaces: &JsonValue, - source_root: &Path, -) -> BTreeMap { - marketplaces - .as_object() - .map(|extra_known_marketplaces| { - extra_known_marketplaces - .iter() - .filter_map(|(name, value)| { - let source_fields = if let Some(source) = value.get("source") - && source.is_object() - { - source.as_object()? - } else { - value.as_object()? - }; - let source_kind = source_fields - .get("source") - .and_then(JsonValue::as_str) - .map(str::trim); - let declared_source = match source_kind { - Some("github") => source_fields.get("repo"), - Some("git") => source_fields.get("url"), - Some("directory" | "local") => source_fields.get("path"), - Some("file" | "url" | "npm" | "settings") => None, - Some(_) => source_fields.get("source"), - None => source_fields - .get("repo") - .or_else(|| source_fields.get("url")) - .or_else(|| source_fields.get("path")) - .or_else(|| value.get("source")), - } - .and_then(JsonValue::as_str) - .map(str::trim) - .filter(|value| !value.is_empty()); - let materialized_source = value - .get("installLocation") - .and_then(JsonValue::as_str) - .map(str::trim) - .filter(|value| !value.is_empty()) - .and_then(|value| { - let path = Path::new(value); - let path = if path.is_absolute() { - path.to_path_buf() - } else { - source_root.join(path) - }; - path.is_dir().then(|| path.display().to_string()) - }); - let (source, ref_name) = if let Some(source) = declared_source { - let source = if matches!(source_kind, Some("directory" | "local")) { - let path = Path::new(source); - if path.is_absolute() { - path.to_path_buf() - } else { - source_root.join(path) - } - .display() - .to_string() - } else { - resolve_external_marketplace_source(source, source_root) - }; - let ref_name = source_fields - .get("ref") - .or_else(|| value.get("ref")) - .and_then(JsonValue::as_str) - .map(str::trim) - .filter(|value| !value.is_empty()) - .map(ToOwned::to_owned); - (source, ref_name) - } else { - (materialized_source?, None) - }; - - Some((name.clone(), MarketplaceImportSource { source, ref_name })) - }) - .collect() - }) - .unwrap_or_default() -} - -#[derive(Debug, Clone, PartialEq, Eq)] -struct MarketplaceImportSource { - source: String, - ref_name: Option, -} - -fn resolve_external_marketplace_source(source: &str, source_root: &Path) -> String { - if !looks_like_relative_local_path(source) { - return source.to_string(); - } - - source_root.join(source).display().to_string() -} - -fn looks_like_relative_local_path(source: &str) -> bool { - source.starts_with("./") || source.starts_with("../") || source == "." || source == ".." -} - fn find_repo_root(cwd: Option<&Path>) -> io::Result> { let Some(cwd) = cwd.filter(|cwd| !cwd.as_os_str().is_empty()) else { return Ok(None); @@ -1746,121 +1480,10 @@ fn is_non_empty_text_file(path: &Path) -> io::Result { Ok(!fs::read_to_string(path)?.trim().is_empty()) } -fn find_repo_agents_md_source(repo_root: &Path) -> io::Result> { - for candidate in [ - repo_root.join(EXTERNAL_AGENT_CONFIG_MD), - repo_root - .join(EXTERNAL_AGENT_DIR) - .join(EXTERNAL_AGENT_CONFIG_MD), - ] { - if is_non_empty_text_file(&candidate)? { - return Ok(Some(candidate)); - } - } - - Ok(None) -} - -fn copy_dir_recursive(source: &Path, target: &Path) -> io::Result<()> { - fs::create_dir_all(target)?; - - for entry in fs::read_dir(source)? { - let entry = entry?; - let source_path = entry.path(); - let target_path = target.join(entry.file_name()); - let file_type = entry.file_type()?; - - if file_type.is_dir() { - copy_dir_recursive(&source_path, &target_path)?; - continue; - } - - if file_type.is_file() { - if is_skill_md(&source_path) { - rewrite_and_copy_text_file(&source_path, &target_path)?; - } else { - fs::copy(source_path, target_path)?; - } - } - } - - Ok(()) -} - -fn is_skill_md(path: &Path) -> bool { - path.file_name() - .and_then(|name| name.to_str()) - .is_some_and(|name| name.eq_ignore_ascii_case("SKILL.md")) -} - -fn rewrite_and_copy_text_file(source: &Path, target: &Path) -> io::Result<()> { - let source_contents = fs::read_to_string(source)?; - let rewritten = rewrite_external_agent_terms(&source_contents); - fs::write(target, rewritten) -} - -fn rewrite_external_agent_terms(content: &str) -> String { - let mut rewritten = replace_case_insensitive_with_boundaries( - content, - &EXTERNAL_AGENT_CONFIG_MD.to_ascii_lowercase(), - "AGENTS.md", - ); - for from in [ - "claude code", - "claude-code", - "claude_code", - "claudecode", - "claude", - ] { - rewritten = replace_case_insensitive_with_boundaries(&rewritten, from, "Codex"); - } - rewritten -} - -fn replace_case_insensitive_with_boundaries( - input: &str, - needle: &str, - replacement: &str, -) -> String { - let needle_lower = needle.to_ascii_lowercase(); - if needle_lower.is_empty() { - return input.to_string(); - } - - let haystack_lower = input.to_ascii_lowercase(); - let bytes = input.as_bytes(); - let mut output = String::with_capacity(input.len()); - let mut last_emitted = 0usize; - let mut search_start = 0usize; - - while let Some(relative_pos) = haystack_lower[search_start..].find(&needle_lower) { - let start = search_start + relative_pos; - let end = start + needle_lower.len(); - let boundary_before = start == 0 || !is_word_byte(bytes[start - 1]); - let boundary_after = end == bytes.len() || !is_word_byte(bytes[end]); - - if boundary_before && boundary_after { - output.push_str(&input[last_emitted..start]); - output.push_str(replacement); - last_emitted = end; - } - - search_start = start + 1; - } - - if last_emitted == 0 { - return input.to_string(); - } - - output.push_str(&input[last_emitted..]); - output -} - -fn is_word_byte(byte: u8) -> bool { - byte.is_ascii_alphanumeric() || byte == b'_' -} - -fn build_config_from_external(settings: &JsonValue) -> io::Result { +fn build_config_from_external( + settings: &JsonValue, + source: ExternalAgentSource, +) -> io::Result { let Some(settings_obj) = settings.as_object() else { return Err(invalid_data_error( "external agent settings root must be an object", @@ -1884,18 +1507,7 @@ fn build_config_from_external(settings: &JsonValue) -> io::Result { ); } - if let Some(sandbox_enabled) = settings_obj - .get("sandbox") - .and_then(JsonValue::as_object) - .and_then(|sandbox| sandbox.get("enabled")) - .and_then(JsonValue::as_bool) - && sandbox_enabled - { - root.insert( - "sandbox_mode".to_string(), - TomlValue::String("workspace-write".to_string()), - ); - } + source.append_config(&mut root, settings_obj); Ok(TomlValue::Table(root)) } @@ -2122,5 +1734,5 @@ fn emit_migration_metric( } #[cfg(test)] -#[path = "external_agent_config_tests.rs"] +#[path = "service_tests.rs"] mod tests; diff --git a/codex-rs/app-server/src/external_agent_migration/service/source.rs b/codex-rs/app-server/src/external_agent_migration/service/source.rs new file mode 100644 index 0000000000..e1cd813ac1 --- /dev/null +++ b/codex-rs/app-server/src/external_agent_migration/service/source.rs @@ -0,0 +1,242 @@ +use codex_external_agent_migration::RewriteProfile; +use codex_external_agent_migration::sessions::ExternalAgentSessionMigration; +use codex_external_agent_migration::sessions::detect_recent_cla_sessions; +use serde_json::Value as JsonValue; +use std::collections::BTreeMap; +use std::collections::HashSet; +use std::io; +use std::path::Path; +use std::path::PathBuf; +use toml::Value as TomlValue; + +use super::MigrationDetails; +use super::source_cla; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(super) struct InstructionSourceGroup { + pub(super) scope: PathBuf, + pub(super) sources: Vec, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(super) struct MarketplaceImportSource { + pub(super) source: String, + pub(super) ref_name: Option, +} + +pub(super) struct DetectedSourcePlugins { + pub(super) description: String, + pub(super) details: MigrationDetails, +} + +pub(super) struct PluginDetectionContext<'a> { + pub(super) external_agent_home: &'a Path, + pub(super) source_settings: &'a Path, + pub(super) source_root: &'a Path, + pub(super) settings: Option<&'a JsonValue>, + pub(super) configured_plugin_ids: &'a HashSet, + pub(super) configured_marketplace_plugins: &'a BTreeMap>, +} + +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub(super) enum ExternalAgentSource { + #[default] + Cla, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(super) enum SourceFeature { + Config, + Plugins, + Sessions, +} + +impl ExternalAgentSource { + pub(super) fn config_dir(self) -> &'static str { + match self { + Self::Cla => source_cla::CONFIG_DIR, + } + } + + pub(super) fn supports(self, feature: SourceFeature) -> bool { + match (self, feature) { + ( + Self::Cla, + SourceFeature::Config | SourceFeature::Plugins | SourceFeature::Sessions, + ) => true, + } + } + + pub(super) fn settings_file_name(self, project_scope: bool) -> &'static str { + match (self, project_scope) { + (Self::Cla, _) => "settings.json", + } + } + + pub(super) fn effective_settings( + self, + source_settings: &Path, + ) -> io::Result> { + match self { + Self::Cla => source_cla::effective_settings(source_settings), + } + } + + pub(super) fn detect_plugins( + self, + context: PluginDetectionContext<'_>, + ) -> io::Result> { + match self { + Self::Cla => Ok(source_cla::detect_plugins(&context)), + } + } + + pub(super) fn can_detect_plugins(self, settings: Option<&JsonValue>) -> bool { + match self { + Self::Cla => source_cla::can_detect_plugins(settings), + } + } + + pub(super) fn detect_recent_sessions( + self, + external_agent_home: &Path, + codex_home: &Path, + ) -> io::Result> { + match self { + Self::Cla => detect_recent_cla_sessions(external_agent_home, codex_home), + } + } + + pub(super) fn marketplace_import_sources( + self, + external_agent_home: &Path, + source_root: &Path, + source_settings: &Path, + ) -> io::Result> { + match self { + Self::Cla => Ok(source_cla::effective_settings(source_settings)? + .as_ref() + .map(|settings| { + source_cla::marketplace_import_sources( + settings, + external_agent_home, + source_root, + ) + }) + .unwrap_or_default()), + } + } + + pub(super) fn append_config( + self, + root: &mut toml::map::Map, + settings: &serde_json::Map, + ) { + match self { + Self::Cla => source_cla::append_config(root, settings), + } + } + + pub(super) fn build_mcp_config( + self, + source_root: &Path, + external_agent_home: &Path, + settings: Option<&JsonValue>, + ) -> io::Result { + match self { + Self::Cla => source_cla::build_mcp_config(source_root, external_agent_home, settings), + } + } + + pub(super) fn mcp_source_path(self, source_root: PathBuf) -> PathBuf { + match self { + Self::Cla => source_root, + } + } + + pub(super) fn repo_instruction_source_groups( + self, + repo_root: &Path, + ) -> io::Result> { + match self { + Self::Cla => source_cla::repo_instruction_source_groups(repo_root), + } + } + + pub(super) fn home_instruction_sources( + self, + external_agent_home: &Path, + ) -> io::Result> { + match self { + Self::Cla => source_cla::home_instruction_sources(external_agent_home), + } + } + + pub(super) fn read_instruction_source(self, path: &Path) -> io::Result { + match self { + Self::Cla => source_cla::read_instruction_source(path), + } + } + + pub(super) fn import_commands( + self, + source_commands: &Path, + target_skills: &Path, + ) -> io::Result> { + match self { + Self::Cla => source_cla::import_source_commands(source_commands, target_skills), + } + } + + pub(super) fn count_missing_commands( + self, + source_commands: &Path, + target_skills: &Path, + ) -> io::Result { + match self { + Self::Cla => source_cla::count_missing_source_commands(source_commands, target_skills), + } + } + + pub(super) fn missing_command_names( + self, + source_commands: &Path, + target_skills: &Path, + ) -> io::Result> { + match self { + Self::Cla => source_cla::missing_source_command_names(source_commands, target_skills), + } + } + + pub(super) fn import_subagents( + self, + source_agents: &Path, + target_agents: &Path, + ) -> io::Result> { + match self { + Self::Cla => source_cla::import_source_subagents(source_agents, target_agents), + } + } + + pub(super) fn hook_event_names( + self, + source_dir: &Path, + target_hooks: &Path, + ) -> io::Result> { + match self { + Self::Cla => source_cla::source_hook_event_names(source_dir, target_hooks), + } + } + + pub(super) fn import_hooks(self, source_dir: &Path, target_hooks: &Path) -> io::Result { + match self { + Self::Cla => source_cla::import_source_hooks(source_dir, target_hooks), + } + } + + pub(super) fn rewrite_profile(self) -> RewriteProfile { + match self { + Self::Cla => source_cla::REWRITE_PROFILE, + } + } +} diff --git a/codex-rs/app-server/src/external_agent_migration/service/source_cla.rs b/codex-rs/app-server/src/external_agent_migration/service/source_cla.rs new file mode 100644 index 0000000000..6f7a05886b --- /dev/null +++ b/codex-rs/app-server/src/external_agent_migration/service/source_cla.rs @@ -0,0 +1,483 @@ +use codex_core_plugins::marketplace_add::is_local_marketplace_source; +use codex_external_agent_migration::CommandDescriptionMode; +use codex_external_agent_migration::CommandMigrationProfile; +use codex_external_agent_migration::RewriteProfile; +use codex_external_agent_migration::build_mcp_config_from_external; +use codex_external_agent_migration::count_missing_commands_with_profile; +use codex_external_agent_migration::hook_migration_event_names_cla; +use codex_external_agent_migration::import_commands_with_profile; +use codex_external_agent_migration::import_hooks_cla; +use codex_external_agent_migration::import_subagents_with_rewrite_profile; +use codex_external_agent_migration::missing_command_names_with_profile; +use codex_plugin::PluginId; +use serde_json::Value as JsonValue; +use std::collections::BTreeMap; +use std::collections::HashSet; +use std::fs; +use std::io; +use std::path::Path; +use std::path::PathBuf; +use toml::Value as TomlValue; + +use super::MigrationDetails; +use super::PluginsMigration; +use super::is_non_empty_text_file; +use super::source::DetectedSourcePlugins; +use super::source::InstructionSourceGroup; +use super::source::MarketplaceImportSource; +use super::source::PluginDetectionContext; + +pub(super) const CONFIG_DIR: &str = ".claude"; +pub(super) const CONFIG_MD: &str = "CLAUDE.md"; +pub(super) const KNOWN_MARKETPLACES_PATH: &str = "plugins/known_marketplaces.json"; +pub(super) const OFFICIAL_MARKETPLACE_NAME: &str = "claude-plugins-official"; +pub(super) const OFFICIAL_MARKETPLACE_SOURCE: &str = "anthropics/claude-plugins-official"; +pub(super) const REWRITE_PROFILE: RewriteProfile = RewriteProfile::new( + CONFIG_MD, + &[ + "claude code", + "claude-code", + "claude_code", + "claudecode", + "claude", + ], +); +const COMMAND_MIGRATION_PROFILE: CommandMigrationProfile = + CommandMigrationProfile::new(REWRITE_PROFILE, CommandDescriptionMode::RequireFrontmatter); + +pub(super) fn effective_settings(project_settings: &Path) -> io::Result> { + let mut effective = super::read_external_settings(project_settings)?; + let Some(settings_dir) = project_settings.parent() else { + return Ok(effective); + }; + let local_settings = settings_dir.join("settings.local.json"); + let local_settings = match super::read_external_settings(&local_settings) { + Ok(Some(local_settings)) => local_settings, + Ok(None) => return Ok(effective), + Err(err) if err.kind() == io::ErrorKind::InvalidData => return Ok(effective), + Err(err) => return Err(err), + }; + if let Some(effective) = effective.as_mut() { + merge_json_settings(effective, &local_settings); + } else { + effective = Some(local_settings); + } + Ok(effective) +} + +pub(super) fn append_config( + root: &mut toml::map::Map, + settings: &serde_json::Map, +) { + if settings + .get("sandbox") + .and_then(JsonValue::as_object) + .and_then(|sandbox| sandbox.get("enabled")) + .and_then(JsonValue::as_bool) + == Some(true) + { + root.insert( + "sandbox_mode".to_string(), + TomlValue::String("workspace-write".to_string()), + ); + } +} + +pub(super) fn detect_plugins( + context: &PluginDetectionContext<'_>, +) -> Option { + let settings = context.settings?; + let import_sources = + marketplace_import_sources(settings, context.external_agent_home, context.source_root); + let details = extract_plugin_migration_details( + settings, + &import_sources, + context.configured_plugin_ids, + context.configured_marketplace_plugins, + )?; + Some(DetectedSourcePlugins { + description: format!( + "Migrate enabled plugins from {}", + context.source_settings.display() + ), + details, + }) +} + +pub(super) fn can_detect_plugins(settings: Option<&JsonValue>) -> bool { + settings.is_some() +} + +pub(super) fn marketplace_import_sources( + settings: &JsonValue, + external_agent_home: &Path, + source_root: &Path, +) -> BTreeMap { + let known_marketplaces_path = external_agent_home.join(KNOWN_MARKETPLACES_PATH); + let known_marketplaces = match super::read_external_settings(&known_marketplaces_path) { + Ok(known_marketplaces) => known_marketplaces, + Err(err) => { + tracing::warn!( + path = %known_marketplaces_path.display(), + error = %err, + "ignoring invalid external agent marketplace registry" + ); + None + } + }; + let mut import_sources = known_marketplaces + .as_ref() + .map(|known_marketplaces| { + collect_marketplace_import_sources(known_marketplaces, external_agent_home) + }) + .unwrap_or_default(); + + if let Some(extra_known_marketplaces) = settings + .as_object() + .and_then(|settings| settings.get("extraKnownMarketplaces")) + { + let mut scoped_marketplaces = extra_known_marketplaces.clone(); + if let Some(scoped_marketplaces) = scoped_marketplaces.as_object_mut() { + for (name, scoped_marketplace) in scoped_marketplaces { + import_sources.remove(name); + let Some(known_marketplace) = known_marketplaces + .as_ref() + .and_then(JsonValue::as_object) + .and_then(|known_marketplaces| known_marketplaces.get(name)) + else { + continue; + }; + if scoped_marketplace.get("source") != known_marketplace.get("source") { + continue; + } + let Some(install_location) = known_marketplace + .get("installLocation") + .and_then(JsonValue::as_str) + else { + continue; + }; + let install_location = Path::new(install_location); + let install_location = if install_location.is_absolute() { + install_location.to_path_buf() + } else { + external_agent_home.join(install_location) + }; + let Some(scoped_marketplace) = scoped_marketplace.as_object_mut() else { + continue; + }; + scoped_marketplace.insert( + "installLocation".to_string(), + JsonValue::String(install_location.display().to_string()), + ); + } + } + import_sources.extend(collect_marketplace_import_sources( + &scoped_marketplaces, + source_root, + )); + } + + if has_enabled_plugin_for_marketplace(settings, OFFICIAL_MARKETPLACE_NAME) + && !import_sources.contains_key(OFFICIAL_MARKETPLACE_NAME) + { + import_sources.insert( + OFFICIAL_MARKETPLACE_NAME.to_string(), + MarketplaceImportSource { + source: OFFICIAL_MARKETPLACE_SOURCE.to_string(), + ref_name: None, + }, + ); + } + + import_sources +} + +pub(super) fn build_mcp_config( + source_root: &Path, + external_agent_home: &Path, + settings: Option<&JsonValue>, +) -> io::Result { + build_mcp_config_from_external(source_root, Some(external_agent_home), settings) +} + +pub(super) fn repo_instruction_source_groups( + repo_root: &Path, +) -> io::Result> { + for candidate in [ + repo_root.join(CONFIG_MD), + repo_root.join(CONFIG_DIR).join(CONFIG_MD), + ] { + if is_non_empty_text_file(&candidate)? { + return Ok(vec![InstructionSourceGroup { + scope: repo_root.to_path_buf(), + sources: vec![candidate], + }]); + } + } + Ok(Vec::new()) +} + +pub(super) fn home_instruction_sources(external_agent_home: &Path) -> io::Result> { + let path = external_agent_home.join(CONFIG_MD); + Ok(is_non_empty_text_file(&path)? + .then_some(path) + .into_iter() + .collect()) +} + +pub(super) fn read_instruction_source(path: &Path) -> io::Result { + fs::read_to_string(path) +} + +pub(super) fn import_source_commands( + source_commands: &Path, + target_skills: &Path, +) -> io::Result> { + import_commands_with_profile(source_commands, target_skills, COMMAND_MIGRATION_PROFILE) +} + +pub(super) fn count_missing_source_commands( + source_commands: &Path, + target_skills: &Path, +) -> io::Result { + count_missing_commands_with_profile(source_commands, target_skills, COMMAND_MIGRATION_PROFILE) +} + +pub(super) fn missing_source_command_names( + source_commands: &Path, + target_skills: &Path, +) -> io::Result> { + missing_command_names_with_profile(source_commands, target_skills, COMMAND_MIGRATION_PROFILE) +} + +pub(super) fn import_source_subagents( + source_agents: &Path, + target_agents: &Path, +) -> io::Result> { + import_subagents_with_rewrite_profile(source_agents, target_agents, REWRITE_PROFILE) +} + +pub(super) fn source_hook_event_names( + source_dir: &Path, + target_hooks: &Path, +) -> io::Result> { + hook_migration_event_names_cla(source_dir, target_hooks, REWRITE_PROFILE) +} + +pub(super) fn import_source_hooks(source_dir: &Path, target_hooks: &Path) -> io::Result { + import_hooks_cla(source_dir, target_hooks, REWRITE_PROFILE) +} + +fn merge_json_settings(existing: &mut JsonValue, incoming: &JsonValue) { + match (existing, incoming) { + (JsonValue::Object(existing), JsonValue::Object(incoming)) => { + for (key, incoming_value) in incoming { + match existing.get_mut(key) { + Some(existing_value) => merge_json_settings(existing_value, incoming_value), + None => { + existing.insert(key.clone(), incoming_value.clone()); + } + } + } + } + (existing, incoming) => { + *existing = incoming.clone(); + } + } +} + +fn extract_plugin_migration_details( + settings: &JsonValue, + import_sources: &BTreeMap, + configured_plugin_ids: &HashSet, + configured_marketplace_plugins: &BTreeMap>, +) -> Option { + let loadable_marketplaces = import_sources + .iter() + .filter_map(|(marketplace_name, source)| { + is_local_marketplace_source(&source.source, source.ref_name.clone()) + .ok() + .map(|_| marketplace_name.clone()) + }) + .collect::>(); + let mut plugins = BTreeMap::new(); + for plugin_id in collect_enabled_plugins(settings) + .into_iter() + .filter(|plugin_id| !configured_plugin_ids.contains(plugin_id)) + { + let Ok(plugin_id) = PluginId::parse(&plugin_id) else { + continue; + }; + if let Some(installable_plugins) = + configured_marketplace_plugins.get(&plugin_id.marketplace_name) + { + if !installable_plugins.contains(&plugin_id.plugin_name) { + tracing::warn!( + plugin_id = %plugin_id.as_key(), + marketplace_name = %plugin_id.marketplace_name, + "enabled external agent plugin was not found in configured marketplace" + ); + continue; + } + } else if !loadable_marketplaces.contains(&plugin_id.marketplace_name) { + tracing::warn!( + plugin_id = %plugin_id.as_key(), + marketplace_name = %plugin_id.marketplace_name, + "marketplace source was not found for enabled external agent plugin" + ); + continue; + } + let plugin_group = plugins + .entry(plugin_id.marketplace_name.clone()) + .or_insert_with(|| PluginsMigration { + marketplace_name: plugin_id.marketplace_name.clone(), + plugin_names: Vec::new(), + }); + plugin_group.plugin_names.push(plugin_id.plugin_name); + } + + let plugins = plugins + .into_values() + .filter_map(|mut plugin_group| { + if plugin_group.plugin_names.is_empty() { + return None; + } + plugin_group.plugin_names.sort(); + Some(plugin_group) + }) + .collect::>(); + if plugins.is_empty() { + return None; + } + + Some(MigrationDetails { + plugins, + ..Default::default() + }) +} + +fn collect_enabled_plugins(settings: &JsonValue) -> Vec { + let Some(enabled_plugins) = settings + .as_object() + .and_then(|settings| settings.get("enabledPlugins")) + .and_then(JsonValue::as_object) + else { + return Vec::new(); + }; + + enabled_plugins + .iter() + .filter_map(|(plugin_key, enabled)| { + if !enabled.as_bool().unwrap_or(false) { + return None; + } + PluginId::parse(plugin_key) + .ok() + .map(|plugin_id| plugin_id.as_key()) + }) + .collect() +} + +fn has_enabled_plugin_for_marketplace(settings: &JsonValue, marketplace_name: &str) -> bool { + collect_enabled_plugins(settings) + .into_iter() + .any(|plugin_id| { + PluginId::parse(&plugin_id) + .map(|plugin_id| plugin_id.marketplace_name == marketplace_name) + .unwrap_or(false) + }) +} + +fn collect_marketplace_import_sources( + marketplaces: &JsonValue, + source_root: &Path, +) -> BTreeMap { + marketplaces + .as_object() + .map(|extra_known_marketplaces| { + extra_known_marketplaces + .iter() + .filter_map(|(name, value)| { + let source_fields = if let Some(source) = value.get("source") + && source.is_object() + { + source.as_object()? + } else { + value.as_object()? + }; + let source_kind = source_fields + .get("source") + .and_then(JsonValue::as_str) + .map(str::trim); + let declared_source = match source_kind { + Some("github") => source_fields.get("repo"), + Some("git") => source_fields.get("url"), + Some("directory" | "local") => source_fields.get("path"), + Some("file" | "url" | "npm" | "settings") => None, + Some(_) => source_fields.get("source"), + None => source_fields + .get("repo") + .or_else(|| source_fields.get("url")) + .or_else(|| source_fields.get("path")) + .or_else(|| value.get("source")), + } + .and_then(JsonValue::as_str) + .map(str::trim) + .filter(|value| !value.is_empty()); + let materialized_source = value + .get("installLocation") + .and_then(JsonValue::as_str) + .map(str::trim) + .filter(|value| !value.is_empty()) + .and_then(|value| { + let path = Path::new(value); + let path = if path.is_absolute() { + path.to_path_buf() + } else { + source_root.join(path) + }; + path.is_dir().then(|| path.display().to_string()) + }); + let (source, ref_name) = if let Some(source) = declared_source { + let source = if matches!(source_kind, Some("directory" | "local")) { + let path = Path::new(source); + if path.is_absolute() { + path.to_path_buf() + } else { + source_root.join(path) + } + .display() + .to_string() + } else { + resolve_external_marketplace_source(source, source_root) + }; + let ref_name = source_fields + .get("ref") + .or_else(|| value.get("ref")) + .and_then(JsonValue::as_str) + .map(str::trim) + .filter(|value| !value.is_empty()) + .map(ToOwned::to_owned); + (source, ref_name) + } else { + (materialized_source?, None) + }; + + Some((name.clone(), MarketplaceImportSource { source, ref_name })) + }) + .collect() + }) + .unwrap_or_default() +} + +fn resolve_external_marketplace_source(source: &str, source_root: &Path) -> String { + if !looks_like_relative_local_path(source) { + return source.to_string(); + } + + source_root.join(source).display().to_string() +} + +fn looks_like_relative_local_path(source: &str) -> bool { + source.starts_with("./") || source.starts_with("../") || source == "." || source == ".." +} diff --git a/codex-rs/app-server/src/external_agent_migration/service/utils.rs b/codex-rs/app-server/src/external_agent_migration/service/utils.rs new file mode 100644 index 0000000000..062c8fc3c7 --- /dev/null +++ b/codex-rs/app-server/src/external_agent_migration/service/utils.rs @@ -0,0 +1,139 @@ +use codex_external_agent_migration::RewriteProfile; +use std::fs; +use std::io; +use std::path::Path; +use std::path::PathBuf; + +pub(super) fn display_source_paths(paths: &[PathBuf]) -> String { + paths + .iter() + .map(|path| path.display().to_string()) + .collect::>() + .join(", ") +} + +pub(super) fn copy_dir_recursive( + source: &Path, + target: &Path, + rewrite_profile: RewriteProfile, +) -> io::Result<()> { + fs::create_dir_all(target)?; + + for entry in fs::read_dir(source)? { + let entry = entry?; + let source_path = entry.path(); + let target_path = target.join(entry.file_name()); + let file_type = entry.file_type()?; + + if file_type.is_dir() { + copy_dir_recursive(&source_path, &target_path, rewrite_profile)?; + continue; + } + + if file_type.is_file() { + if is_skill_md(&source_path) { + rewrite_and_copy_text_file(&source_path, &target_path, rewrite_profile)?; + } else { + fs::copy(source_path, target_path)?; + } + } + } + + Ok(()) +} + +pub(super) fn rewrite_external_agent_terms( + content: &str, + rewrite_profile: RewriteProfile, +) -> String { + let mut rewritten = replace_case_insensitive_with_boundaries( + content, + rewrite_profile.doc_file_name(), + "AGENTS.md", + ); + for from in rewrite_profile.term_variants() { + rewritten = replace_case_insensitive_with_boundaries(&rewritten, from, "Codex"); + } + for from in rewrite_profile.case_sensitive_term_variants() { + rewritten = replace_with_boundaries(&rewritten, from, "Codex"); + } + rewritten +} + +fn replace_with_boundaries(input: &str, needle: &str, replacement: &str) -> String { + if needle.is_empty() { + return input.to_string(); + } + + let bytes = input.as_bytes(); + let mut output = String::with_capacity(input.len()); + let mut last_emitted = 0usize; + let mut search_start = 0usize; + + while let Some(relative_pos) = input[search_start..].find(needle) { + let start = search_start + relative_pos; + let end = start + needle.len(); + let boundary_before = start == 0 || !is_word_char(bytes[start - 1]); + let boundary_after = end == bytes.len() || !is_word_char(bytes[end]); + if boundary_before && boundary_after { + output.push_str(&input[last_emitted..start]); + output.push_str(replacement); + last_emitted = end; + } + search_start = end; + } + output.push_str(&input[last_emitted..]); + output +} + +fn is_skill_md(path: &Path) -> bool { + path.file_name() + .and_then(|name| name.to_str()) + .is_some_and(|name| name.eq_ignore_ascii_case("SKILL.md")) +} + +fn rewrite_and_copy_text_file( + source: &Path, + target: &Path, + rewrite_profile: RewriteProfile, +) -> io::Result<()> { + let source_contents = fs::read_to_string(source)?; + let rewritten = rewrite_external_agent_terms(&source_contents, rewrite_profile); + fs::write(target, rewritten) +} + +fn replace_case_insensitive_with_boundaries( + input: &str, + needle: &str, + replacement: &str, +) -> String { + let needle_lower = needle.to_ascii_lowercase(); + if needle_lower.is_empty() { + return input.to_string(); + } + + let haystack_lower = input.to_ascii_lowercase(); + let bytes = input.as_bytes(); + let mut output = String::with_capacity(input.len()); + let mut last_emitted = 0usize; + let mut search_start = 0usize; + + while let Some(relative_pos) = haystack_lower[search_start..].find(&needle_lower) { + let start = search_start + relative_pos; + let end = start + needle_lower.len(); + let boundary_before = start == 0 || !is_word_char(bytes[start - 1]); + let boundary_after = end == bytes.len() || !is_word_char(bytes[end]); + if boundary_before && boundary_after { + output.push_str(&input[last_emitted..start]); + output.push_str(replacement); + last_emitted = end; + } + search_start = end; + } + output.push_str(&input[last_emitted..]); + output +} + +fn is_word_char(byte: u8) -> bool { + byte.is_ascii_alphanumeric() || byte == b'_' +} diff --git a/codex-rs/app-server/src/config/external_agent_config_tests.rs b/codex-rs/app-server/src/external_agent_migration/service_tests.rs similarity index 99% rename from codex-rs/app-server/src/config/external_agent_config_tests.rs rename to codex-rs/app-server/src/external_agent_migration/service_tests.rs index 9ecf0986ca..88b3063123 100644 --- a/codex-rs/app-server/src/config/external_agent_config_tests.rs +++ b/codex-rs/app-server/src/external_agent_migration/service_tests.rs @@ -1192,10 +1192,12 @@ async fn detect_home_skips_skills_when_all_skill_directories_exist() { } #[tokio::test] -async fn import_repo_agents_md_rewrites_terms_and_skips_non_empty_targets() { +async fn import_repo_agents_md_from_nested_cwd_rewrites_terms_and_skips_non_empty_targets() { let root = TempDir::new().expect("create tempdir"); let repo_root = root.path().join("repo-a"); + let nested_cwd = repo_root.join("nested"); let repo_with_existing_target = root.path().join("repo-b"); + fs::create_dir_all(&nested_cwd).expect("create nested cwd"); fs::create_dir_all(repo_root.join(".git")).expect("create git"); fs::create_dir_all(repo_with_existing_target.join(".git")).expect("create git"); fs::write( @@ -1224,7 +1226,7 @@ async fn import_repo_agents_md_rewrites_terms_and_skips_non_empty_targets() { ExternalAgentConfigMigrationItem { item_type: ExternalAgentConfigMigrationItemType::AgentsMd, description: String::new(), - cwd: Some(repo_root.clone()), + cwd: Some(nested_cwd.clone()), details: None, }, ExternalAgentConfigMigrationItem { @@ -1242,12 +1244,12 @@ async fn import_repo_agents_md_rewrites_terms_and_skips_non_empty_targets() { ExternalAgentConfigImportItemResult { item_type: ExternalAgentConfigMigrationItemType::AgentsMd, description: String::new(), - cwd: Some(repo_root.clone()), + cwd: Some(nested_cwd.clone()), success_count: 1, error_count: 0, successes: vec![import_success( ExternalAgentConfigMigrationItemType::AgentsMd, - Some(repo_root.clone()), + Some(nested_cwd), repo_root .join(EXTERNAL_AGENT_CONFIG_MD) .display() @@ -1830,7 +1832,7 @@ async fn detect_home_uses_materialized_known_marketplace_for_inline_npm_source() #[test] fn marketplace_import_sources_prefers_scoped_source_over_registry_name_collision() { - let (root, external_agent_home, codex_home) = fixture_paths(); + let (root, external_agent_home, _codex_home) = fixture_paths(); let source_root = root.path().join("repo"); let scoped_marketplace = source_root.join("repo-marketplace"); let cached_marketplace = external_agent_home.join("plugins/marketplaces/debug"); @@ -1861,8 +1863,8 @@ fn marketplace_import_sources_prefers_scoped_source_over_registry_name_collision } }); - let import_sources = service_for_paths(external_agent_home, codex_home) - .marketplace_import_sources(&settings, &source_root); + let import_sources = + source_cla::marketplace_import_sources(&settings, &external_agent_home, &source_root); assert_eq!( import_sources.get("debug"), @@ -1875,7 +1877,7 @@ fn marketplace_import_sources_prefers_scoped_source_over_registry_name_collision #[test] fn marketplace_import_sources_prefers_supported_declaration_over_materialization() { - let (_root, external_agent_home, codex_home) = fixture_paths(); + let (_root, external_agent_home, _codex_home) = fixture_paths(); let cached_marketplace = external_agent_home.join("plugins/marketplaces/acme-tools"); fs::create_dir_all(&cached_marketplace).expect("create cached marketplace"); fs::write( @@ -1894,8 +1896,11 @@ fn marketplace_import_sources_prefers_supported_declaration_over_materialization ) .expect("write known marketplaces"); - let import_sources = service_for_paths(external_agent_home.clone(), codex_home) - .marketplace_import_sources(&serde_json::json!({}), &external_agent_home); + let import_sources = source_cla::marketplace_import_sources( + &serde_json::json!({}), + &external_agent_home, + &external_agent_home, + ); assert_eq!( import_sources.get("acme-tools"), diff --git a/codex-rs/app-server/src/request_processors/external_agent_session_import.rs b/codex-rs/app-server/src/external_agent_migration/session_importer.rs similarity index 94% rename from codex-rs/app-server/src/request_processors/external_agent_session_import.rs rename to codex-rs/app-server/src/external_agent_migration/session_importer.rs index 6ec89401ec..f48ce6d969 100644 --- a/codex-rs/app-server/src/request_processors/external_agent_session_import.rs +++ b/codex-rs/app-server/src/external_agent_migration/session_importer.rs @@ -5,12 +5,12 @@ use chrono::Utc; use codex_arg0::Arg0DispatchPaths; use codex_core::ThreadManager; use codex_core::config::ConfigOverrides; -use codex_external_agent_sessions::CompletedExternalAgentSessionImport; -use codex_external_agent_sessions::ExternalAgentSessionMigration; -use codex_external_agent_sessions::ImportedExternalAgentSession; -use codex_external_agent_sessions::PendingSessionImport; -use codex_external_agent_sessions::prepare_validated_session_import; -use codex_external_agent_sessions::record_completed_session_imports; +use codex_external_agent_migration::sessions::CompletedExternalAgentSessionImport; +use codex_external_agent_migration::sessions::ExternalAgentSessionMigration; +use codex_external_agent_migration::sessions::ImportedExternalAgentSession; +use codex_external_agent_migration::sessions::PendingSessionImport; +use codex_external_agent_migration::sessions::prepare_validated_session_import; +use codex_external_agent_migration::sessions::record_completed_session_imports; use codex_models_manager::manager::RefreshStrategy; use codex_protocol::ThreadId; use codex_protocol::models::BaseInstructions; @@ -27,8 +27,8 @@ use codex_thread_store::UpdateThreadMetadataParams; use futures::StreamExt; use tokio::sync::Semaphore; -use crate::config::external_agent_config::ExternalAgentConfigImportItemResult; -use crate::config::external_agent_config::record_import_error; +use super::service::ExternalAgentConfigImportItemResult; +use super::service::record_import_error; use crate::config_manager::ConfigManager; const SESSION_IMPORT_CONCURRENCY: usize = 5; diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index 80fe28771f..ea5d8b853b 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -87,7 +87,6 @@ mod attestation; mod auth_mode; mod bespoke_event_handling; mod command_exec; -mod config; mod config_layer; mod config_manager; mod config_manager_service; @@ -98,6 +97,7 @@ mod dynamic_tools; mod effective_plugin_change; mod error_code; mod extensions; +mod external_agent_migration; mod external_auth; mod filters; mod fs_watch; diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 8cbfe4b69a..5508f4953c 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -13,6 +13,8 @@ use crate::extensions::ThreadExtensionDependencies; use crate::extensions::app_server_extension_event_sink; use crate::extensions::guardian_agent_spawner; use crate::extensions::thread_extensions; +use crate::external_agent_migration::ExternalAgentConfigRequestProcessor; +use crate::external_agent_migration::ExternalAgentConfigRequestProcessorArgs; use crate::fs_watch::FsWatchManager; use crate::outgoing_message::ConnectionId; use crate::outgoing_message::ConnectionRequestId; @@ -24,8 +26,6 @@ use crate::request_processors::CatalogRequestProcessor; use crate::request_processors::CommandExecRequestProcessor; use crate::request_processors::ConfigRequestProcessor; use crate::request_processors::EnvironmentRequestProcessor; -use crate::request_processors::ExternalAgentConfigRequestProcessor; -use crate::request_processors::ExternalAgentConfigRequestProcessorArgs; use crate::request_processors::FeedbackRequestProcessor; use crate::request_processors::FsRequestProcessor; use crate::request_processors::GitRequestProcessor; diff --git a/codex-rs/app-server/src/request_processors.rs b/codex-rs/app-server/src/request_processors.rs index da88d0e067..7a00a04fef 100644 --- a/codex-rs/app-server/src/request_processors.rs +++ b/codex-rs/app-server/src/request_processors.rs @@ -501,8 +501,6 @@ mod catalog_processor; mod command_exec_processor; mod config_processor; mod environment_processor; -mod external_agent_config_processor; -mod external_agent_session_import; mod feedback_doctor_report; mod feedback_processor; mod fs_processor; @@ -525,8 +523,6 @@ pub(crate) use catalog_processor::CatalogRequestProcessor; pub(crate) use command_exec_processor::CommandExecRequestProcessor; pub(crate) use config_processor::ConfigRequestProcessor; pub(crate) use environment_processor::EnvironmentRequestProcessor; -pub(crate) use external_agent_config_processor::ExternalAgentConfigRequestProcessor; -pub(crate) use external_agent_config_processor::ExternalAgentConfigRequestProcessorArgs; pub(crate) use feedback_processor::FeedbackRequestProcessor; pub(crate) use fs_processor::FsRequestProcessor; pub(crate) use git_processor::GitRequestProcessor; diff --git a/codex-rs/app-server/tests/suite/v2/external_agent_config.rs b/codex-rs/app-server/tests/suite/v2/external_agent_config.rs index 53c50ea43f..a4efc1d899 100644 --- a/codex-rs/app-server/tests/suite/v2/external_agent_config.rs +++ b/codex-rs/app-server/tests/suite/v2/external_agent_config.rs @@ -43,7 +43,7 @@ use super::analytics::wait_for_analytics_event; const DEFAULT_TIMEOUT: Duration = Duration::from_secs(60); fn external_agent_home(codex_home: &Path) -> PathBuf { - codex_home.join(concat!(".", "cl", "aude")) + codex_home.join(concat!(".", "cla", "ude")) } fn assert_import_response(response: ExternalAgentConfigImportResponse) -> String { @@ -51,6 +51,51 @@ fn assert_import_response(response: ExternalAgentConfigImportResponse) -> String response.import_id } +#[tokio::test] +async fn external_agent_config_detect_accepts_source_selection_and_defaults_unknown_values() +-> Result<()> { + let codex_home = TempDir::new()?; + let source_home = external_agent_home(codex_home.path()); + std::fs::create_dir_all(&source_home)?; + std::fs::write(source_home.join("CLAUDE.md"), "project instructions")?; + + let home_dir = codex_home.path().display().to_string(); + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_auto_env() + .with_env_overrides(&[("HOME", Some(home_dir.as_str()))]) + .build() + .await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + + let mut responses = Vec::new(); + for source in [None, Some("claude-code"), Some("unknown-source")] { + let mut params = serde_json::json!({ "includeHome": true }); + if let Some(source) = source { + params["source"] = serde_json::json!(source); + } + let request_id = mcp + .send_raw_request("externalAgentConfig/detect", Some(params)) + .await?; + let response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + responses.push(to_response::(response)?); + } + + assert_eq!(responses[0].items.len(), 1); + assert_eq!( + responses[0].items[0].item_type, + ExternalAgentConfigMigrationItemType::AgentsMd + ); + let expected = responses[0].clone(); + assert_eq!(responses, vec![expected; 3]); + + Ok(()) +} + #[tokio::test] async fn external_agent_config_import_sends_completion_notification_for_sync_only_import() -> Result<()> { diff --git a/codex-rs/external-agent-migration/Cargo.toml b/codex-rs/external-agent-migration/Cargo.toml index a515b3783a..35ce505a90 100644 --- a/codex-rs/external-agent-migration/Cargo.toml +++ b/codex-rs/external-agent-migration/Cargo.toml @@ -13,11 +13,17 @@ path = "src/lib.rs" workspace = true [dependencies] +chrono = { workspace = true } codex-hooks = { workspace = true } +codex-protocol = { workspace = true } +codex-utils-output-truncation = { workspace = true } +serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } serde_yaml = { workspace = true } +sha2 = { workspace = true } toml = { workspace = true } [dev-dependencies] +codex-app-server-protocol = { workspace = true } pretty_assertions = { workspace = true } tempfile = { workspace = true } diff --git a/codex-rs/external-agent-migration/src/hooks_cla.rs b/codex-rs/external-agent-migration/src/hooks_cla.rs new file mode 100644 index 0000000000..98363c3b67 --- /dev/null +++ b/codex-rs/external-agent-migration/src/hooks_cla.rs @@ -0,0 +1,233 @@ +use super::RewriteProfile; +use super::external_agent_config_dir; +use super::invalid_data_error; +use super::json_u64; +use super::rewrite_external_agent_terms; +use super::rewrite_hook_command_for_source; +use super::write_hook_migration; +use codex_hooks::HOOK_EVENT_NAMES; +use codex_hooks::HOOK_EVENT_NAMES_WITH_MATCHERS; +use serde_json::Value as JsonValue; +use std::fs; +use std::io; +use std::path::Path; +use std::path::PathBuf; + +pub fn hooks_migration_description_cla( + source_external_agent_dir: &Path, + target_hooks: &Path, + rewrite_profile: RewriteProfile, +) -> io::Result> { + if hook_migration_event_names_cla(source_external_agent_dir, target_hooks, rewrite_profile)? + .is_empty() + { + return Ok(None); + } + + Ok(Some(format!( + "Migrate hooks from {} to {}", + source_external_agent_dir.display(), + target_hooks.display() + ))) +} + +pub fn hook_migration_event_names_cla( + source_external_agent_dir: &Path, + target_hooks: &Path, + rewrite_profile: RewriteProfile, +) -> io::Result> { + let migration = hook_migration_cla( + source_external_agent_dir, + target_hooks.parent(), + rewrite_profile, + )?; + Ok(migration.keys().cloned().collect()) +} + +pub fn import_hooks_cla( + source_external_agent_dir: &Path, + target_hooks: &Path, + rewrite_profile: RewriteProfile, +) -> io::Result { + let Some(parent) = target_hooks.parent() else { + return Err(invalid_data_error("hooks target path has no parent")); + }; + let migration = hook_migration_cla(source_external_agent_dir, Some(parent), rewrite_profile)?; + if migration.is_empty() { + return Ok(false); + } + + write_hook_migration(source_external_agent_dir, target_hooks, migration) +} + +pub(super) fn hook_migration_cla( + source_external_agent_dir: &Path, + target_config_dir: Option<&Path>, + rewrite_profile: RewriteProfile, +) -> io::Result> { + let mut settings_files = Vec::new(); + let mut disable_all_hooks = None; + for settings_name in ["settings.json", "settings.local.json"] { + let settings_file = source_external_agent_dir.join(settings_name); + if !settings_file.is_file() { + continue; + } + let raw = fs::read_to_string(&settings_file)?; + let settings: JsonValue = serde_json::from_str(&raw) + .map_err(|err| invalid_data_error(format!("invalid hooks settings: {err}")))?; + if let Some(disabled) = settings.get("disableAllHooks").and_then(JsonValue::as_bool) { + disable_all_hooks = Some(disabled); + } + settings_files.push(settings); + } + + if disable_all_hooks.unwrap_or(false) { + return Ok(serde_json::Map::new()); + } + + let mut migration = serde_json::Map::new(); + for settings in settings_files { + append_convertible_hook_groups_cla( + &settings, + &mut migration, + target_config_dir, + rewrite_profile, + ); + } + + Ok(migration) +} + +pub(super) fn append_convertible_hook_groups_cla( + settings: &JsonValue, + hooks_payload: &mut serde_json::Map, + target_config_dir: Option<&Path>, + rewrite_profile: RewriteProfile, +) { + let Some(hooks_config) = settings.get("hooks").and_then(JsonValue::as_object) else { + return; + }; + + for event_name in HOOK_EVENT_NAMES { + let Some(groups) = hooks_config.get(event_name).and_then(JsonValue::as_array) else { + continue; + }; + for group in groups { + let Some(group_object) = group.as_object() else { + continue; + }; + if group_object.contains_key("if") + || group_object + .keys() + .any(|key| !matches!(key.as_str(), "matcher" | "hooks")) + { + continue; + } + let mut hook_commands = Vec::new(); + if let Some(hooks) = group_object.get("hooks").and_then(JsonValue::as_array) { + for hook in hooks { + let Some(hook_object) = hook.as_object() else { + continue; + }; + let hook_type = hook_object + .get("type") + .and_then(JsonValue::as_str) + .unwrap_or("command"); + if hook_type != "command" { + continue; + } + if hook_object.keys().any(|key| { + !matches!( + key.as_str(), + "type" + | "command" + | "timeout" + | "timeoutSec" + | "statusMessage" + | "async" + ) + }) { + continue; + } + if hook_object + .get("async") + .and_then(JsonValue::as_bool) + .unwrap_or(false) + { + continue; + } + if ["asyncRewake", "shell", "once"] + .into_iter() + .any(|field| hook_object.contains_key(field)) + { + continue; + } + let Some(command) = hook_object + .get("command") + .and_then(JsonValue::as_str) + .map(str::trim) + .filter(|command| !command.is_empty()) + else { + continue; + }; + + let mut command_payload = serde_json::Map::new(); + command_payload + .insert("type".to_string(), JsonValue::String("command".to_string())); + command_payload.insert( + "command".to_string(), + JsonValue::String(rewrite_hook_command_cla(command, target_config_dir)), + ); + if let Some(timeout) = hook_object + .get("timeout") + .or_else(|| hook_object.get("timeoutSec")) + .and_then(json_u64) + { + command_payload.insert( + "timeout".to_string(), + JsonValue::Number(serde_json::Number::from(timeout)), + ); + } + if let Some(status_message) = + hook_object.get("statusMessage").and_then(JsonValue::as_str) + { + command_payload.insert( + "statusMessage".to_string(), + JsonValue::String(rewrite_external_agent_terms( + status_message, + rewrite_profile, + )), + ); + } + hook_commands.push(JsonValue::Object(command_payload)); + } + } + if hook_commands.is_empty() { + continue; + } + + let mut group_payload = serde_json::Map::new(); + if HOOK_EVENT_NAMES_WITH_MATCHERS.contains(&event_name) + && let Some(matcher) = group_object.get("matcher").and_then(JsonValue::as_str) + { + group_payload.insert( + "matcher".to_string(), + JsonValue::String(matcher.to_string()), + ); + } + group_payload.insert("hooks".to_string(), JsonValue::Array(hook_commands)); + if let Some(groups) = hooks_payload + .entry(event_name.to_string()) + .or_insert_with(|| JsonValue::Array(Vec::new())) + .as_array_mut() + { + groups.push(JsonValue::Object(group_payload)); + } + } + } +} + +pub(super) fn rewrite_hook_command_cla(command: &str, target_config_dir: Option<&Path>) -> String { + let source_external_agent_dir = PathBuf::from(external_agent_config_dir()); + rewrite_hook_command_for_source(command, target_config_dir, &source_external_agent_dir) +} diff --git a/codex-rs/external-agent-migration/src/lib.rs b/codex-rs/external-agent-migration/src/lib.rs index 7e5f626d8d..e87c9d613e 100644 --- a/codex-rs/external-agent-migration/src/lib.rs +++ b/codex-rs/external-agent-migration/src/lib.rs @@ -1,7 +1,8 @@ //! Migration helpers for importing external-agent configuration into Codex. -use codex_hooks::HOOK_EVENT_NAMES; -use codex_hooks::HOOK_EVENT_NAMES_WITH_MATCHERS; +mod hooks_cla; +pub mod sessions; + use serde_json::Value as JsonValue; use serde_yaml::Value as YamlValue; use std::collections::BTreeMap; @@ -12,6 +13,10 @@ use std::path::Path; use std::path::PathBuf; use toml::Value as TomlValue; +pub use hooks_cla::hook_migration_event_names_cla; +pub use hooks_cla::hooks_migration_description_cla; +pub use hooks_cla::import_hooks_cla; + const SOURCE_EXTERNAL_AGENT_NAME: &str = "claude"; const EXTERNAL_AGENT_MCP_CONFIG_FILE: &str = ".mcp.json"; const EXTERNAL_AGENT_HOOKS_SUBDIR: &str = "hooks"; @@ -19,6 +24,72 @@ const EXTERNAL_AGENT_MIGRATED_HOOKS_SUBDIR: &str = "hooks"; const COMMAND_SKILL_PREFIX: &str = "source-command"; const MAX_SKILL_NAME_LEN: usize = 64; +/// Describes source-specific terms that should be rewritten in migrated artifacts. +#[derive(Clone, Copy)] +pub struct RewriteProfile { + doc_file_name: &'static str, + term_variants: &'static [&'static str], + case_sensitive_term_variants: &'static [&'static str], +} + +impl RewriteProfile { + pub const fn new(doc_file_name: &'static str, term_variants: &'static [&'static str]) -> Self { + Self { + doc_file_name, + term_variants, + case_sensitive_term_variants: &[], + } + } + + pub const fn with_case_sensitive_term_variants( + mut self, + term_variants: &'static [&'static str], + ) -> Self { + self.case_sensitive_term_variants = term_variants; + self + } + + pub const fn doc_file_name(self) -> &'static str { + self.doc_file_name + } + + pub const fn term_variants(self) -> &'static [&'static str] { + self.term_variants + } + + pub const fn case_sensitive_term_variants(self) -> &'static [&'static str] { + self.case_sensitive_term_variants + } +} + +/// Controls how migrated commands obtain the description required by a Codex skill. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum CommandDescriptionMode { + /// Skip source commands that do not declare a non-empty frontmatter description. + RequireFrontmatter, + /// Derive a stable description from the source command name when frontmatter is absent. + UseSourceNameFallback, +} + +/// Describes source-specific command migration behavior. +#[derive(Clone, Copy)] +pub struct CommandMigrationProfile { + rewrite_profile: RewriteProfile, + description_mode: CommandDescriptionMode, +} + +impl CommandMigrationProfile { + pub const fn new( + rewrite_profile: RewriteProfile, + description_mode: CommandDescriptionMode, + ) -> Self { + Self { + rewrite_profile, + description_mode, + } + } +} + #[derive(Debug)] struct ParsedDocument { frontmatter: BTreeMap, @@ -46,6 +117,25 @@ pub fn build_mcp_config_from_external( settings: Option<&JsonValue>, ) -> io::Result { let mcp_servers = read_external_mcp_servers(source_root, external_agent_home)?; + build_mcp_config(mcp_servers, settings) +} + +pub fn build_mcp_config_from_json_file(source_file: &Path) -> io::Result { + if !source_file.is_file() { + return Ok(TomlValue::Table(Default::default())); + } + let raw = fs::read_to_string(source_file)?; + let parsed: JsonValue = serde_json::from_str(&raw) + .map_err(|err| invalid_data_error(format!("invalid MCP config: {err}")))?; + let mut mcp_servers = BTreeMap::new(); + append_mcp_servers_from_value(&parsed, &mut mcp_servers, McpServerMerge::Overwrite); + build_mcp_config(mcp_servers, /*settings*/ None) +} + +fn build_mcp_config( + mcp_servers: BTreeMap, + settings: Option<&JsonValue>, +) -> io::Result { if mcp_servers.is_empty() { return Ok(TomlValue::Table(Default::default())); } @@ -82,54 +172,6 @@ pub fn build_mcp_config_from_external( Ok(TomlValue::Table(root)) } -pub fn hooks_migration_description( - source_external_agent_dir: &Path, - target_hooks: &Path, -) -> io::Result> { - if hook_migration_event_names(source_external_agent_dir, target_hooks)?.is_empty() { - return Ok(None); - } - - Ok(Some(format!( - "Migrate hooks from {} to {}", - source_external_agent_dir.display(), - target_hooks.display() - ))) -} - -pub fn hook_migration_event_names( - source_external_agent_dir: &Path, - target_hooks: &Path, -) -> io::Result> { - let migration = hook_migration(source_external_agent_dir, target_hooks.parent())?; - Ok(migration.keys().cloned().collect()) -} - -pub fn import_hooks(source_external_agent_dir: &Path, target_hooks: &Path) -> io::Result { - let Some(parent) = target_hooks.parent() else { - return Err(invalid_data_error("hooks target path has no parent")); - }; - let migration = hook_migration(source_external_agent_dir, Some(parent))?; - if migration.is_empty() { - return Ok(false); - } - - fs::create_dir_all(parent)?; - - let mut wrote_active_hooks = false; - if is_missing_or_empty_text_file(target_hooks)? { - copy_hook_scripts(source_external_agent_dir, parent)?; - let mut payload = serde_json::Map::new(); - payload.insert("hooks".to_string(), JsonValue::Object(migration)); - let rendered = serde_json::to_string_pretty(&JsonValue::Object(payload)) - .map_err(|err| invalid_data_error(format!("failed to serialize hooks.json: {err}")))?; - fs::write(target_hooks, format!("{rendered}\n"))?; - wrote_active_hooks = true; - } - - Ok(wrote_active_hooks) -} - pub fn count_missing_subagents(source_agents: &Path, target_agents: &Path) -> io::Result { Ok(missing_subagent_names(source_agents, target_agents)?.len()) } @@ -154,7 +196,11 @@ pub fn missing_subagent_names( Ok(names) } -pub fn import_subagents(source_agents: &Path, target_agents: &Path) -> io::Result> { +pub fn import_subagents_with_rewrite_profile( + source_agents: &Path, + target_agents: &Path, + rewrite_profile: RewriteProfile, +) -> io::Result> { if !source_agents.is_dir() { return Ok(Vec::new()); } @@ -172,36 +218,52 @@ pub fn import_subagents(source_agents: &Path, target_agents: &Path) -> io::Resul let Some(metadata) = agent_metadata(&document) else { continue; }; - fs::write(&target, render_agent_toml(&document.body, &metadata)?)?; + fs::write( + &target, + render_agent_toml(&document.body, &metadata, rewrite_profile)?, + )?; imported.push(metadata.name); } Ok(imported) } -pub fn count_missing_commands(source_commands: &Path, target_skills: &Path) -> io::Result { - Ok(missing_command_names(source_commands, target_skills)?.len()) -} - -pub fn missing_command_names( +pub fn count_missing_commands_with_profile( source_commands: &Path, target_skills: &Path, -) -> io::Result> { - Ok(unique_supported_command_sources(source_commands)? - .into_iter() - .filter(|(_source_file, name)| !target_skills.join(name).exists()) - .map(|(_source_file, name)| name) - .collect()) + profile: CommandMigrationProfile, +) -> io::Result { + Ok(missing_command_names_with_profile(source_commands, target_skills, profile)?.len()) } -pub fn import_commands(source_commands: &Path, target_skills: &Path) -> io::Result> { +pub fn missing_command_names_with_profile( + source_commands: &Path, + target_skills: &Path, + profile: CommandMigrationProfile, +) -> io::Result> { + Ok( + unique_supported_command_sources(source_commands, profile.description_mode)? + .into_iter() + .filter(|(_source_file, name)| !target_skills.join(name).exists()) + .map(|(_source_file, name)| name) + .collect(), + ) +} + +pub fn import_commands_with_profile( + source_commands: &Path, + target_skills: &Path, + profile: CommandMigrationProfile, +) -> io::Result> { if !source_commands.is_dir() { return Ok(Vec::new()); } fs::create_dir_all(target_skills)?; let mut imported = Vec::new(); - for (source_file, name) in unique_supported_command_sources(source_commands)? { + for (source_file, name) in + unique_supported_command_sources(source_commands, profile.description_mode)? + { let document = parse_document(&source_file)?; let target_dir = target_skills.join(&name); if target_dir.exists() { @@ -209,12 +271,20 @@ pub fn import_commands(source_commands: &Path, target_skills: &Path) -> io::Resu } fs::create_dir_all(&target_dir)?; let source_name = command_source_name(source_commands, &source_file); - let Some(description) = command_skill_description(&document, &source_name) else { + let Some(description) = + command_skill_description(&document, &source_name, profile.description_mode) + else { continue; }; fs::write( target_dir.join("SKILL.md"), - render_command_skill(&document.body, &name, &description, &source_name), + render_command_skill( + &document.body, + &name, + &description, + &source_name, + profile.rewrite_profile, + ), )?; imported.push(name); } @@ -501,164 +571,32 @@ fn contains_env_placeholder(value: &str) -> bool { value.contains("${") } -fn hook_migration( +fn write_hook_migration( source_external_agent_dir: &Path, - target_config_dir: Option<&Path>, -) -> io::Result> { - let mut settings_files = Vec::new(); - let mut disable_all_hooks = None; - for settings_name in ["settings.json", "settings.local.json"] { - let settings_file = source_external_agent_dir.join(settings_name); - if !settings_file.is_file() { - continue; - } - let raw = fs::read_to_string(&settings_file)?; - let settings: JsonValue = serde_json::from_str(&raw) - .map_err(|err| invalid_data_error(format!("invalid hooks settings: {err}")))?; - if let Some(disabled) = settings.get("disableAllHooks").and_then(JsonValue::as_bool) { - disable_all_hooks = Some(disabled); - } - settings_files.push(settings); + target_hooks: &Path, + migration: serde_json::Map, +) -> io::Result { + if migration.is_empty() || !is_missing_or_empty_text_file(target_hooks)? { + return Ok(false); } - - if disable_all_hooks.unwrap_or(false) { - return Ok(serde_json::Map::new()); - } - - let mut migration = serde_json::Map::new(); - for settings in settings_files { - append_convertible_hook_groups(&settings, &mut migration, target_config_dir); - } - - Ok(migration) -} - -fn append_convertible_hook_groups( - settings: &JsonValue, - hooks_payload: &mut serde_json::Map, - target_config_dir: Option<&Path>, -) { - let Some(hooks_config) = settings.get("hooks").and_then(JsonValue::as_object) else { - return; + let Some(parent) = target_hooks.parent() else { + return Err(invalid_data_error("hooks target path has no parent")); }; - - for event_name in HOOK_EVENT_NAMES { - let Some(groups) = hooks_config.get(event_name).and_then(JsonValue::as_array) else { - continue; - }; - for group in groups { - let Some(group_object) = group.as_object() else { - continue; - }; - if group_object.contains_key("if") - || group_object - .keys() - .any(|key| !matches!(key.as_str(), "matcher" | "hooks")) - { - continue; - } - let mut hook_commands = Vec::new(); - if let Some(hooks) = group_object.get("hooks").and_then(JsonValue::as_array) { - for hook in hooks { - let Some(hook_object) = hook.as_object() else { - continue; - }; - let hook_type = hook_object - .get("type") - .and_then(JsonValue::as_str) - .unwrap_or("command"); - if hook_type != "command" { - continue; - } - if hook_object.keys().any(|key| { - !matches!( - key.as_str(), - "type" - | "command" - | "timeout" - | "timeoutSec" - | "statusMessage" - | "async" - ) - }) { - continue; - } - if hook_object - .get("async") - .and_then(JsonValue::as_bool) - .unwrap_or(false) - { - continue; - } - if ["asyncRewake", "shell", "once"] - .into_iter() - .any(|field| hook_object.contains_key(field)) - { - continue; - } - let Some(command) = hook_object - .get("command") - .and_then(JsonValue::as_str) - .map(str::trim) - .filter(|command| !command.is_empty()) - else { - continue; - }; - - let mut command_payload = serde_json::Map::new(); - command_payload - .insert("type".to_string(), JsonValue::String("command".to_string())); - command_payload.insert( - "command".to_string(), - JsonValue::String(rewrite_hook_command(command, target_config_dir)), - ); - if let Some(timeout) = hook_object - .get("timeout") - .or_else(|| hook_object.get("timeoutSec")) - .and_then(json_u64) - { - command_payload.insert( - "timeout".to_string(), - JsonValue::Number(serde_json::Number::from(timeout)), - ); - } - if let Some(status_message) = - hook_object.get("statusMessage").and_then(JsonValue::as_str) - { - command_payload.insert( - "statusMessage".to_string(), - JsonValue::String(rewrite_external_agent_terms(status_message)), - ); - } - hook_commands.push(JsonValue::Object(command_payload)); - } - } - if hook_commands.is_empty() { - continue; - } - - let mut group_payload = serde_json::Map::new(); - if HOOK_EVENT_NAMES_WITH_MATCHERS.contains(&event_name) - && let Some(matcher) = group_object.get("matcher").and_then(JsonValue::as_str) - { - group_payload.insert( - "matcher".to_string(), - JsonValue::String(matcher.to_string()), - ); - } - group_payload.insert("hooks".to_string(), JsonValue::Array(hook_commands)); - if let Some(groups) = hooks_payload - .entry(event_name.to_string()) - .or_insert_with(|| JsonValue::Array(Vec::new())) - .as_array_mut() - { - groups.push(JsonValue::Object(group_payload)); - } - } - } + fs::create_dir_all(parent)?; + copy_hook_scripts(source_external_agent_dir, parent)?; + let mut payload = serde_json::Map::new(); + payload.insert("hooks".to_string(), JsonValue::Object(migration)); + let rendered = serde_json::to_string_pretty(&JsonValue::Object(payload)) + .map_err(|err| invalid_data_error(format!("failed to serialize hooks.json: {err}")))?; + fs::write(target_hooks, format!("{rendered}\n"))?; + Ok(true) } -fn rewrite_hook_command(command: &str, target_config_dir: Option<&Path>) -> String { +fn rewrite_hook_command_for_source( + command: &str, + target_config_dir: Option<&Path>, + source_external_agent_dir: &Path, +) -> String { let Some(target_config_dir) = target_config_dir else { return command.to_string(); }; @@ -666,10 +604,12 @@ fn rewrite_hook_command(command: &str, target_config_dir: Option<&Path>) -> Stri return command.to_string(); } let target_hooks_dir = target_config_dir.join(EXTERNAL_AGENT_MIGRATED_HOOKS_SUBDIR); - let source_hooks_path = format!( - "{}/{EXTERNAL_AGENT_HOOKS_SUBDIR}/", - external_agent_config_dir() - ); + let source_config_dir = source_external_agent_dir + .file_name() + .and_then(|name| name.to_str()) + .map(str::to_string) + .unwrap_or_else(external_agent_config_dir); + let source_hooks_path = format!("{source_config_dir}/{EXTERNAL_AGENT_HOOKS_SUBDIR}/"); let command = replace_quoted_hook_paths(command, '\'', &source_hooks_path, &target_hooks_dir); let command = replace_quoted_hook_paths(&command, '"', &source_hooks_path, &target_hooks_dir); replace_unquoted_hook_paths(&command, &source_hooks_path, &target_hooks_dir) @@ -918,12 +858,19 @@ fn command_source_files(source_commands: &Path) -> io::Result> { Ok(files) } -fn unique_supported_command_sources(source_commands: &Path) -> io::Result> { +fn unique_supported_command_sources( + source_commands: &Path, + description_mode: CommandDescriptionMode, +) -> io::Result> { let mut by_name = BTreeMap::>::new(); for source_file in command_source_files(source_commands)? { let document = parse_document(&source_file)?; - let Some(name) = command_skill_name_if_supported(source_commands, &source_file, &document) - else { + let Some(name) = command_skill_name_if_supported( + source_commands, + &source_file, + &document, + description_mode, + ) else { continue; }; by_name.entry(name).or_default().push(source_file); @@ -1069,12 +1016,19 @@ fn agent_metadata(document: &ParsedDocument) -> Option { }) } -fn render_agent_toml(body: &str, metadata: &AgentMetadata) -> io::Result { +fn render_agent_toml( + body: &str, + metadata: &AgentMetadata, + rewrite_profile: RewriteProfile, +) -> io::Result { let mut document = toml::map::Map::new(); document.insert("name".to_string(), TomlValue::String(metadata.name.clone())); document.insert( "description".to_string(), - TomlValue::String(rewrite_external_agent_terms(&metadata.description)), + TomlValue::String(rewrite_external_agent_terms( + &metadata.description, + rewrite_profile, + )), ); if let Some(effort) = metadata.effort.as_ref() && let Some(effort) = map_agent_reasoning_effort(effort) @@ -1096,7 +1050,7 @@ fn render_agent_toml(body: &str, metadata: &AgentMetadata) -> io::Result } document.insert( "developer_instructions".to_string(), - TomlValue::String(render_agent_body(body)), + TomlValue::String(render_agent_body(body, rewrite_profile)), ); let serialized = toml::to_string_pretty(&TomlValue::Table(document)) @@ -1104,8 +1058,8 @@ fn render_agent_toml(body: &str, metadata: &AgentMetadata) -> io::Result Ok(format!("{}\n", serialized.trim_end())) } -fn render_agent_body(body: &str) -> String { - let body = rewrite_external_agent_terms(body.trim()); +fn render_agent_body(body: &str, rewrite_profile: RewriteProfile) -> String { + let body = rewrite_external_agent_terms(body.trim(), rewrite_profile); if body.is_empty() { "No subagent instructions were found.".to_string() } else { @@ -1124,12 +1078,13 @@ fn command_skill_name_if_supported( source_commands: &Path, source_file: &Path, document: &ParsedDocument, + description_mode: CommandDescriptionMode, ) -> Option { if source_file.file_stem().and_then(|stem| stem.to_str()) == Some("README") { return None; } let source_name = command_source_name(source_commands, source_file); - command_skill_description(document, &source_name)?; + command_skill_description(document, &source_name, description_mode)?; let name = command_skill_name(source_commands, source_file); if name.chars().count() > MAX_SKILL_NAME_LEN { return None; @@ -1140,13 +1095,23 @@ fn command_skill_name_if_supported( Some(name) } -fn command_skill_description(document: &ParsedDocument, _source_name: &str) -> Option { - document +fn command_skill_description( + document: &ParsedDocument, + source_name: &str, + description_mode: CommandDescriptionMode, +) -> Option { + let frontmatter_description = document .frontmatter .get("description") .and_then(FrontmatterValue::as_scalar) .filter(|value| !value.trim().is_empty()) - .map(ToOwned::to_owned) + .map(ToOwned::to_owned); + frontmatter_description.or_else(|| match description_mode { + CommandDescriptionMode::RequireFrontmatter => None, + CommandDescriptionMode::UseSourceNameFallback => { + Some(format!("Migrated source command `{source_name}`")) + } + }) } fn command_source_name(source_commands: &Path, source_file: &Path) -> String { @@ -1160,8 +1125,14 @@ fn command_source_name(source_commands: &Path, source_file: &Path) -> String { .join("-") } -fn render_command_skill(body: &str, name: &str, description: &str, source_name: &str) -> String { - let body = rewrite_external_agent_terms(body.trim()); +fn render_command_skill( + body: &str, + name: &str, + description: &str, + source_name: &str, + rewrite_profile: RewriteProfile, +) -> String { + let body = rewrite_external_agent_terms(body.trim(), rewrite_profile); let template_body = if body.is_empty() { "No command template body was found.".to_string() } else { @@ -1170,7 +1141,7 @@ fn render_command_skill(body: &str, name: &str, description: &str, source_name: format!( "---\nname: {}\ndescription: {}\n---\n\n# {name}\n\nUse this skill when the user asks to run the migrated source command `{source_name}`.\n\n## Command Template\n\n{template_body}\n", yaml_string(name), - yaml_string(&rewrite_external_agent_terms(description)), + yaml_string(&rewrite_external_agent_terms(description, rewrite_profile)), ) } @@ -1291,18 +1262,54 @@ fn is_missing_or_empty_text_file(path: &Path) -> io::Result { Ok(fs::read_to_string(path)?.trim().is_empty()) } -fn rewrite_external_agent_terms(content: &str) -> String { +fn rewrite_external_agent_terms(content: &str, rewrite_profile: RewriteProfile) -> String { let mut rewritten = replace_case_insensitive_with_boundaries( content, - &external_agent_doc_file_name(), + rewrite_profile.doc_file_name, "AGENTS.md", ); - for from in external_agent_term_variants() { - rewritten = replace_case_insensitive_with_boundaries(&rewritten, &from, "Codex"); + for from in rewrite_profile.term_variants { + rewritten = replace_case_insensitive_with_boundaries(&rewritten, from, "Codex"); + } + for from in rewrite_profile.case_sensitive_term_variants { + rewritten = replace_with_boundaries(&rewritten, from, "Codex"); } rewritten } +fn replace_with_boundaries(input: &str, needle: &str, replacement: &str) -> String { + if needle.is_empty() { + return input.to_string(); + } + + let bytes = input.as_bytes(); + let mut output = String::with_capacity(input.len()); + let mut last_emitted = 0usize; + let mut search_start = 0usize; + + while let Some(relative_pos) = input[search_start..].find(needle) { + let start = search_start + relative_pos; + let end = start + needle.len(); + let boundary_before = start == 0 || !is_word_byte(bytes[start - 1]); + let boundary_after = end == bytes.len() || !is_word_byte(bytes[end]); + + if boundary_before && boundary_after { + output.push_str(&input[last_emitted..start]); + output.push_str(replacement); + last_emitted = end; + } + + search_start = end; + } + + if last_emitted == 0 { + return input.to_string(); + } + + output.push_str(&input[last_emitted..]); + output +} + fn replace_case_insensitive_with_boundaries( input: &str, needle: &str, @@ -1365,847 +1372,6 @@ fn external_agent_project_dir_env_var() -> String { ) } -fn external_agent_doc_file_name() -> String { - format!("{SOURCE_EXTERNAL_AGENT_NAME}.md") -} - -fn external_agent_term_variants() -> [String; 5] { - [ - format!("{SOURCE_EXTERNAL_AGENT_NAME} code"), - format!("{SOURCE_EXTERNAL_AGENT_NAME}-code"), - format!("{SOURCE_EXTERNAL_AGENT_NAME}_code"), - format!("{SOURCE_EXTERNAL_AGENT_NAME}code"), - SOURCE_EXTERNAL_AGENT_NAME.to_string(), - ] -} - #[cfg(test)] -mod tests { - use super::*; - use pretty_assertions::assert_eq; - - const MAX_SKILL_DESCRIPTION_LEN: usize = 1024; - - fn source_path(relative_path: &str) -> PathBuf { - Path::new("/repo") - .join(external_agent_config_dir()) - .join(relative_path) - } - - fn source_hook_command(script_name: &str) -> String { - format!( - "python3 {}/{EXTERNAL_AGENT_HOOKS_SUBDIR}/{script_name}", - external_agent_config_dir() - ) - } - - fn source_hook_command_with_project_dir(script_name: &str) -> String { - format!( - "python3 \"${}\"/{}/{EXTERNAL_AGENT_HOOKS_SUBDIR}/{script_name}", - external_agent_project_dir_env_var(), - external_agent_config_dir() - ) - } - - fn migrated_hook_command(script_name: &str) -> String { - migrated_quoted_hook_command(script_name) - } - - fn migrated_quoted_hook_command(script_name: &str) -> String { - let hook_path = Path::new("/repo/.codex") - .join(EXTERNAL_AGENT_MIGRATED_HOOKS_SUBDIR) - .join(script_name); - format!( - "python3 {}", - shell_single_quote(hook_path.to_string_lossy().as_ref()) - ) - } - - #[test] - fn env_placeholder_accepts_defaults() { - assert_eq!( - parse_env_placeholder("${TOKEN:-fallback}"), - Some("TOKEN".to_string()) - ); - } - - #[test] - fn mcp_migration_skips_placeholder_args() { - let root = tempfile::TempDir::new().expect("tempdir"); - fs::write( - root.path().join(".mcp.json"), - r#"{"mcpServers":{"db":{"command":"db-server","args":["${DATABASE_URL}"]}}}"#, - ) - .expect("write mcp"); - - assert_eq!( - build_mcp_config_from_external( - root.path(), - /*external_agent_home*/ None, - /*settings*/ None, - ) - .unwrap(), - TomlValue::Table(Default::default()) - ); - } - - #[test] - fn mcp_migration_prefers_command_transport_for_mixed_server_config() { - let root = tempfile::TempDir::new().expect("tempdir"); - fs::write( - root.path().join(".mcp.json"), - r#"{ - "mcpServers": { - "mixedTransport": { - "command": "mcp-remote-proxy", - "args": [ - "https://example.com/mixed-transport", - "--transport", - "http" - ], - "url": "https://example.com/mixed-transport" - } - } - }"#, - ) - .expect("write mcp"); - - assert_eq!( - build_mcp_config_from_external( - root.path(), - /*external_agent_home*/ None, - /*settings*/ None, - ) - .unwrap(), - toml::from_str( - r#" -[mcp_servers.mixedTransport] -command = "mcp-remote-proxy" -args = [ - "https://example.com/mixed-transport", - "--transport", - "http", -] -"# - ) - .unwrap() - ); - } - - #[test] - fn mcp_migration_skips_unsupported_transports() { - let root = tempfile::TempDir::new().expect("tempdir"); - fs::write( - root.path().join(".mcp.json"), - r#"{ - "mcpServers": { - "legacy-sse": {"type": "sse", "url": "https://example.invalid/sse"}, - "vault": { - "url": "https://example.invalid/vault", - "headers": {"Authorization": "Bearer ${VAULT_TOKEN:-dev-token}"} - } - } - }"#, - ) - .expect("write mcp"); - - assert_eq!( - build_mcp_config_from_external( - root.path(), - /*external_agent_home*/ None, - /*settings*/ None, - ) - .unwrap(), - toml::from_str( - r#" -[mcp_servers.vault] -url = "https://example.invalid/vault" -bearer_token_env_var = "VAULT_TOKEN" -"# - ) - .unwrap() - ); - } - - #[test] - fn mcp_migration_reads_matching_project_entries_from_repo_external_project_config() { - let root = tempfile::TempDir::new().expect("tempdir"); - let project = root.path().join("repo"); - fs::create_dir_all(&project).expect("create repo"); - let other = root.path().join("other"); - fs::create_dir_all(&other).expect("create other"); - fs::write( - project.join(external_agent_project_config_file()), - serde_json::json!({ - "mcpServers": { - "top": {"command": "top-server"} - }, - "projects": { - project.display().to_string(): { - "mcpServers": { - "repo": {"command": "repo-server"} - } - }, - other.display().to_string(): { - "mcpServers": { - "other": {"command": "other-server"} - } - } - } - }) - .to_string(), - ) - .expect("write external agent project config"); - - assert_eq!( - build_mcp_config_from_external( - &project, /*external_agent_home*/ None, /*settings*/ None, - ) - .unwrap(), - toml::from_str( - r#" -[mcp_servers.repo] -command = "repo-server" - -[mcp_servers.top] -command = "top-server" -"# - ) - .unwrap() - ); - } - - #[test] - fn mcp_migration_reads_matching_project_entries_from_home_external_project_config() { - let root = tempfile::TempDir::new().expect("tempdir"); - let project = root.path().join("repo"); - fs::create_dir_all(&project).expect("create repo"); - let external_agent_home = root.path().join(external_agent_config_dir()); - fs::create_dir_all(&external_agent_home).expect("create external agent home"); - fs::write( - root.path().join(external_agent_project_config_file()), - serde_json::json!({ - "projects": { - project.display().to_string(): { - "mcpServers": { - "repo": {"command": "repo-server"} - } - } - } - }) - .to_string(), - ) - .expect("write external agent project config"); - - assert_eq!( - build_mcp_config_from_external( - &project, - Some(&external_agent_home), - /*settings*/ None, - ) - .unwrap(), - toml::from_str( - r#" -[mcp_servers.repo] -command = "repo-server" -"# - ) - .unwrap() - ); - } - - #[test] - fn mcp_migration_preserves_repo_servers_over_home_project_entries() { - let root = tempfile::TempDir::new().expect("tempdir"); - let project = root.path().join("repo"); - fs::create_dir_all(&project).expect("create repo"); - let external_agent_home = root.path().join(external_agent_config_dir()); - fs::create_dir_all(&external_agent_home).expect("create external agent home"); - fs::write( - project.join(EXTERNAL_AGENT_MCP_CONFIG_FILE), - serde_json::json!({ - "mcpServers": { - "shared": {"command": "repo-server"} - } - }) - .to_string(), - ) - .expect("write repo mcp"); - fs::write( - root.path().join(external_agent_project_config_file()), - serde_json::json!({ - "projects": { - project.display().to_string(): { - "mcpServers": { - "home-only": {"command": "home-only-server"}, - "shared": {"command": "home-server"} - } - } - } - }) - .to_string(), - ) - .expect("write external agent project config"); - - assert_eq!( - build_mcp_config_from_external( - &project, - Some(&external_agent_home), - /*settings*/ None, - ) - .unwrap(), - toml::from_str( - r#" -[mcp_servers.home-only] -command = "home-only-server" - -[mcp_servers.shared] -command = "repo-server" -"# - ) - .unwrap() - ); - } - - #[test] - fn mcp_migration_skips_disabled_servers() { - let root = tempfile::TempDir::new().expect("tempdir"); - fs::write( - root.path().join(".mcp.json"), - r#"{ - "mcpServers": { - "enabled": {"command": "enabled-server"}, - "explicit-disabled": {"command": "disabled-server", "disabled": true}, - "not-enabled": {"command": "not-enabled-server"} - } - }"#, - ) - .expect("write mcp"); - let settings = serde_json::json!({ - "enabledMcpjsonServers": ["enabled"], - "disabledMcpjsonServers": ["explicit-disabled"] - }); - - assert_eq!( - build_mcp_config_from_external( - root.path(), - /*external_agent_home*/ None, - Some(&settings), - ) - .unwrap(), - toml::from_str( - r#" -[mcp_servers.enabled] -command = "enabled-server" -"# - ) - .unwrap() - ); - } - - #[test] - fn command_skill_names_include_nested_paths() { - let root = source_path("commands"); - let file = source_path("commands/pr/review.md"); - - assert_eq!(command_skill_name(&root, &file), "source-command-pr-review"); - } - - #[test] - fn command_skill_names_must_fit_codex_skill_loader_limit() { - let root = source_path("commands"); - let file = source_path("commands/this/is/a/deeply/nested/command/with/a/very/long/name.md"); - let document = parse_document_content("---\ndescription: Review PR\n---\nReview\n"); - - assert!(command_skill_name_if_supported(&root, &file, &document).is_none()); - } - - #[test] - fn commands_with_overlong_descriptions_are_preserved() { - let root = source_path("commands"); - let file = source_path("commands/review.md"); - let description = "x".repeat(MAX_SKILL_DESCRIPTION_LEN + 1); - let document = - parse_document_content(&format!("---\ndescription: {description}\n---\nReview\n")); - - assert_eq!( - command_skill_name_if_supported(&root, &file, &document), - Some("source-command-review".to_string()) - ); - - let rendered = render_command_skill( - &document.body, - "source-command-review", - &description, - "review", - ); - let rendered_document = parse_document_content(&rendered); - assert_eq!( - rendered_document - .frontmatter - .get("description") - .and_then(FrontmatterValue::as_scalar), - Some(description.as_str()) - ); - } - - #[test] - fn commands_with_provider_runtime_expansion_are_skipped() { - let root = source_path("commands"); - let file = source_path("commands/deploy.md"); - let document = parse_document_content( - "---\ndescription: Deploy\n---\nDeploy $ARGUMENTS from @release.yaml\n", - ); - - assert!(command_skill_name_if_supported(&root, &file, &document).is_none()); - } - - #[test] - fn commands_without_description_are_skipped() { - let root = source_path("commands"); - let file = source_path("commands/README.md"); - let document = parse_document_content("# Notes\n\nThis documents commands.\n"); - - assert!(command_skill_name_if_supported(&root, &file, &document).is_none()); - } - - #[test] - fn command_slug_collisions_are_skipped() { - let root = tempfile::TempDir::new().expect("tempdir"); - let commands = root.path().join("commands"); - fs::create_dir_all(&commands).expect("create commands"); - fs::write( - commands.join("foo-bar.md"), - "---\ndescription: First\n---\nRun the first command.\n", - ) - .expect("write first command"); - fs::write( - commands.join("foo_bar.md"), - "---\ndescription: Second\n---\nRun the second command.\n", - ) - .expect("write second command"); - - assert_eq!( - unique_supported_command_sources(&commands).unwrap(), - Vec::<(PathBuf, String)>::new() - ); - } - - #[test] - fn subagent_accepts_yaml_block_lists_by_ignoring_unsupported_fields() { - let document = parse_document_content( - "---\nname: cloud-incident\ndescription: Debug incidents\nskills:\n - runbook-reader\ntools:\n - Read\n - Bash\ndisallowedTools:\n - Write\n---\nInvestigate carefully.\n", - ); - - assert!(agent_metadata(&document).is_some()); - } - - #[test] - fn subagent_requires_minimum_codex_agent_fields() { - let missing_description = - parse_document_content("---\nname: incomplete\n---\nInvestigate carefully.\n"); - let missing_body = - parse_document_content("---\nname: incomplete\ndescription: Missing body\n---\n"); - - assert!(agent_metadata(&missing_description).is_none()); - assert!(agent_metadata(&missing_body).is_none()); - } - - #[test] - fn subagent_preserves_default_model_when_source_model_is_present() { - let document = parse_document_content( - "---\nname: reviewer\ndescription: Review code\nmodel: source-opus\neffort: max\n---\nReview carefully.\n", - ); - let metadata = agent_metadata(&document).expect("metadata"); - let rendered: TomlValue = - toml::from_str(&render_agent_toml(&document.body, &metadata).expect("render agent")) - .expect("parse rendered agent"); - let expected: TomlValue = toml::from_str( - r#" -name = "reviewer" -description = "Review code" -model_reasoning_effort = "xhigh" -developer_instructions = """ -Review carefully.""" -"#, - ) - .expect("parse expected agent"); - - assert_eq!(rendered, expected); - } - - #[test] - fn subagent_target_preserves_dotted_file_stem() { - let target_agents = Path::new("/repo/.codex/agents"); - let source_file = source_path("agents/security.audit.md"); - - assert_eq!( - subagent_target_file(&source_file, target_agents), - Some(PathBuf::from("/repo/.codex/agents/security.audit.toml")) - ); - } - - #[test] - fn frontmatter_accepts_crlf_delimiters() { - let document = parse_document_content( - "---\r\nname: reviewer\r\ndescription: Review code\r\n---\r\nReview carefully.\r\n", - ); - - assert_eq!( - ( - document - .frontmatter - .get("name") - .and_then(FrontmatterValue::as_scalar), - document - .frontmatter - .get("description") - .and_then(FrontmatterValue::as_scalar), - document.body.as_str(), - ), - ( - Some("reviewer"), - Some("Review code"), - "Review carefully.\r\n" - ) - ); - } - - #[test] - fn hook_migration_ignores_unsupported_handlers() { - let settings = serde_json::json!({ - "hooks": { - "PreToolUse": [{ - "matcher": "Bash", - "if": "tool_input.command contains 'rm'", - "hooks": [{ - "type": "command", - "command": source_hook_command("policy_gate.py") - }] - }, { - "matcher": "Edit", - "hooks": [ - { - "type": "command", - "if": "Bash(rm *)", - "command": source_hook_command("policy_gate.py") - }, - { - "type": "http", - "url": "https://example.invalid/hook" - } - ] - }], - "PermissionRequest": [{ - "matcher": "Bash", - "hooks": [{ - "type": "command", - "command": source_hook_command("approve.py") - }] - }], - "SubagentStart": [{ - "matcher": "worker", - "hooks": [{"type": "prompt", "prompt": "check"}] - }] - } - }); - let mut migration = serde_json::Map::new(); - append_convertible_hook_groups(&settings, &mut migration, Some(Path::new("/repo/.codex"))); - - assert_eq!( - migration, - serde_json::json!({ - "PermissionRequest": [{ - "matcher": "Bash", - "hooks": [{ - "type": "command", - "command": migrated_hook_command("approve.py") - }] - }] - }) - .as_object() - .cloned() - .expect("object") - ); - } - - #[test] - fn hook_migration_honors_disable_all_hooks() { - let root = tempfile::TempDir::new().expect("tempdir"); - fs::write( - root.path().join("settings.json"), - r#"{ - "disableAllHooks": true, - "hooks": { - "SessionStart": [{ - "matcher": "startup", - "hooks": [{"type": "command", "command": "echo setup"}] - }] - } - }"#, - ) - .expect("write settings"); - - assert_eq!( - hook_migration(root.path(), /*target_config_dir*/ None).unwrap(), - serde_json::Map::new() - ); - } - - #[test] - fn hook_migration_honors_settings_local_disable_override() { - let root = tempfile::TempDir::new().expect("tempdir"); - fs::write( - root.path().join("settings.json"), - r#"{ - "disableAllHooks": true, - "hooks": { - "SessionStart": [{ - "matcher": "project", - "hooks": [{"type": "command", "command": "echo project"}] - }] - } - }"#, - ) - .expect("write project settings"); - fs::write( - root.path().join("settings.local.json"), - r#"{ - "disableAllHooks": false, - "hooks": { - "SessionStart": [{ - "matcher": "local", - "hooks": [{"type": "command", "command": "echo local"}] - }] - } - }"#, - ) - .expect("write local settings"); - - assert_eq!( - hook_migration(root.path(), /*target_config_dir*/ None).unwrap(), - serde_json::json!({ - "SessionStart": [{ - "matcher": "project", - "hooks": [{ - "type": "command", - "command": "echo project" - }] - }, { - "matcher": "local", - "hooks": [{ - "type": "command", - "command": "echo local" - }] - }] - }) - .as_object() - .cloned() - .expect("object") - ); - } - - #[test] - fn hook_command_paths_rewrite_to_target_hook_dir() { - let project_dir_env_var = external_agent_project_dir_env_var(); - let plugin_root_env_var = format!( - "{}_PLUGIN_ROOT", - SOURCE_EXTERNAL_AGENT_NAME.to_ascii_uppercase() - ); - let source_hooks_path = format!( - "{}/{EXTERNAL_AGENT_HOOKS_SUBDIR}", - external_agent_config_dir() - ); - assert_eq!( - rewrite_hook_command( - &source_hook_command_with_project_dir("check.py"), - Some(Path::new("/repo/.codex")), - ), - migrated_hook_command("check.py") - ); - assert_eq!( - rewrite_hook_command( - &format!("\"${project_dir_env_var}\"/{source_hooks_path}/check-style.sh"), - Some(Path::new("/repo/.codex")), - ), - shell_single_quote( - Path::new("/repo/.codex") - .join(EXTERNAL_AGENT_MIGRATED_HOOKS_SUBDIR) - .join("check-style.sh") - .to_string_lossy() - .as_ref() - ) - ); - assert_eq!( - rewrite_hook_command( - &source_hook_command("check.py"), - Some(Path::new("/repo/.codex")), - ), - migrated_hook_command("check.py") - ); - assert_eq!( - rewrite_hook_command( - &format!("python3 ./{source_hooks_path}/check.py"), - Some(Path::new("/repo/.codex")), - ), - migrated_hook_command("check.py") - ); - assert_eq!( - rewrite_hook_command( - &format!("python3 '${{{project_dir_env_var}}}/{source_hooks_path}/check.py'"), - Some(Path::new("/repo/.codex")), - ), - migrated_quoted_hook_command("check.py") - ); - assert_eq!( - rewrite_hook_command( - &format!("python3 \"${{{project_dir_env_var}}}/{source_hooks_path}/check.py\""), - Some(Path::new("/repo/.codex")), - ), - migrated_quoted_hook_command("check.py") - ); - assert_eq!( - rewrite_hook_command( - &format!("bash -lc \"python3 {source_hooks_path}/check.py\""), - Some(Path::new("/repo/.codex")), - ), - format!("bash -lc \"python3 {source_hooks_path}/check.py\"") - ); - assert_eq!( - rewrite_hook_command( - &format!( - "HOOK=${{{project_dir_env_var}}}/{source_hooks_path}/check.py python3 \"$HOOK\"" - ), - Some(Path::new("/repo/.codex")), - ), - format!( - "HOOK=${{{project_dir_env_var}}}/{source_hooks_path}/check.py python3 \"$HOOK\"" - ) - ); - assert_eq!( - rewrite_hook_command( - &format!("python3 {source_hooks_path}/${{SCRIPT}}.py"), - Some(Path::new("/repo/.codex")), - ), - format!("python3 {source_hooks_path}/${{SCRIPT}}.py") - ); - assert_eq!( - rewrite_hook_command( - &format!("python3 {source_hooks_path}/{{lint,fmt}}.sh"), - Some(Path::new("/repo/.codex")), - ), - format!("python3 {source_hooks_path}/{{lint,fmt}}.sh") - ); - assert_eq!( - rewrite_hook_command( - &format!("python3 {source_hooks_path}/my\\ script.py"), - Some(Path::new("/repo/.codex")), - ), - format!("python3 {source_hooks_path}/my\\ script.py") - ); - assert_eq!( - rewrite_hook_command( - &format!("python3 .{SOURCE_EXTERNAL_AGENT_NAME}\\hooks\\check.py"), - Some(Path::new("/repo/.codex")), - ), - format!("python3 .{}\\hooks\\check.py", SOURCE_EXTERNAL_AGENT_NAME) - ); - assert_eq!( - rewrite_hook_command( - &format!( - "python3 \"%{}%\\{}\\hooks\\check.py\"", - project_dir_env_var, - external_agent_config_dir() - ), - Some(Path::new("/repo/.codex")), - ), - format!( - "python3 \"%{}%\\{}\\hooks\\check.py\"", - project_dir_env_var, - external_agent_config_dir() - ) - ); - assert_eq!( - rewrite_hook_command( - &format!("python3 '${{{project_dir_env_var}}}/{source_hooks_path}/my script.py'"), - Some(Path::new("/repo/.codex")), - ), - migrated_quoted_hook_command("my script.py") - ); - assert_eq!( - rewrite_hook_command( - &format!("/repo/{source_hooks_path}/check.py 2>/dev/null || true"), - Some(Path::new("/repo/.codex")), - ), - format!( - "{} 2>/dev/null || true", - shell_single_quote( - Path::new("/repo/.codex") - .join(EXTERNAL_AGENT_MIGRATED_HOOKS_SUBDIR) - .join("check.py") - .to_string_lossy() - .as_ref() - ) - ) - ); - let plugin_script_command = format!("${{{plugin_root_env_var}}}/scripts/format.sh"); - assert_eq!( - rewrite_hook_command(&plugin_script_command, Some(Path::new("/repo/.codex")),), - plugin_script_command - ); - } - - #[test] - fn hook_script_copy_keeps_existing_target_scripts() { - let root = tempfile::TempDir::new().expect("tempdir"); - let source_external_agent_dir = root.path().join(external_agent_config_dir()); - let source_hooks = source_external_agent_dir.join(EXTERNAL_AGENT_HOOKS_SUBDIR); - let target_config_dir = root.path().join(".codex"); - let target_hooks = target_config_dir.join(EXTERNAL_AGENT_MIGRATED_HOOKS_SUBDIR); - fs::create_dir_all(&source_hooks).expect("create source hooks"); - fs::create_dir_all(&target_hooks).expect("create target hooks"); - fs::write(source_hooks.join("check.py"), "new script").expect("write source hook"); - fs::write(target_hooks.join("check.py"), "existing script").expect("write target hook"); - - copy_hook_scripts(&source_external_agent_dir, &target_config_dir).expect("copy hooks"); - - assert_eq!( - fs::read_to_string(target_hooks.join("check.py")).expect("read target hook"), - "existing script" - ); - } - - #[test] - fn hook_migration_drops_negative_timeouts() { - let settings = serde_json::json!({ - "hooks": { - "SessionStart": [{ - "matcher": "startup", - "hooks": [{ - "type": "command", - "command": "echo setup", - "timeout": -1 - }] - }] - } - }); - let mut migration = serde_json::Map::new(); - append_convertible_hook_groups(&settings, &mut migration, /*target_config_dir*/ None); - - assert_eq!( - migration, - serde_json::json!({ - "SessionStart": [{ - "matcher": "startup", - "hooks": [{ - "type": "command", - "command": "echo setup" - }] - }] - }) - .as_object() - .cloned() - .expect("object") - ); - } -} +#[path = "lib_tests.rs"] +mod tests; diff --git a/codex-rs/external-agent-migration/src/lib_tests.rs b/codex-rs/external-agent-migration/src/lib_tests.rs new file mode 100644 index 0000000000..049f2acb41 --- /dev/null +++ b/codex-rs/external-agent-migration/src/lib_tests.rs @@ -0,0 +1,920 @@ +use super::hooks_cla::append_convertible_hook_groups_cla; +use super::hooks_cla::hook_migration_cla; +use super::hooks_cla::rewrite_hook_command_cla; +use super::*; +use pretty_assertions::assert_eq; + +const MAX_SKILL_DESCRIPTION_LEN: usize = 1024; +const TEST_REWRITE_PROFILE: RewriteProfile = RewriteProfile::new( + "CLAUDE.md", + &[ + "claude code", + "claude-code", + "claude_code", + "claudecode", + "claude", + ], +); + +fn source_path(relative_path: &str) -> PathBuf { + Path::new("/repo") + .join(external_agent_config_dir()) + .join(relative_path) +} + +fn source_hook_command(script_name: &str) -> String { + format!( + "python3 {}/{EXTERNAL_AGENT_HOOKS_SUBDIR}/{script_name}", + external_agent_config_dir() + ) +} + +fn source_hook_command_with_project_dir(script_name: &str) -> String { + format!( + "python3 \"${}\"/{}/{EXTERNAL_AGENT_HOOKS_SUBDIR}/{script_name}", + external_agent_project_dir_env_var(), + external_agent_config_dir() + ) +} + +fn migrated_hook_command(script_name: &str) -> String { + migrated_quoted_hook_command(script_name) +} + +fn migrated_quoted_hook_command(script_name: &str) -> String { + let hook_path = Path::new("/repo/.codex") + .join(EXTERNAL_AGENT_MIGRATED_HOOKS_SUBDIR) + .join(script_name); + format!( + "python3 {}", + shell_single_quote(hook_path.to_string_lossy().as_ref()) + ) +} + +#[test] +fn env_placeholder_accepts_defaults() { + assert_eq!( + parse_env_placeholder("${TOKEN:-fallback}"), + Some("TOKEN".to_string()) + ); +} + +#[test] +fn mcp_migration_skips_placeholder_args() { + let root = tempfile::TempDir::new().expect("tempdir"); + fs::write( + root.path().join(".mcp.json"), + r#"{"mcpServers":{"db":{"command":"db-server","args":["${DATABASE_URL}"]}}}"#, + ) + .expect("write mcp"); + + assert_eq!( + build_mcp_config_from_external( + root.path(), + /*external_agent_home*/ None, + /*settings*/ None, + ) + .unwrap(), + TomlValue::Table(Default::default()) + ); +} + +#[test] +fn mcp_migration_prefers_command_transport_for_mixed_server_config() { + let root = tempfile::TempDir::new().expect("tempdir"); + fs::write( + root.path().join(".mcp.json"), + r#"{ + "mcpServers": { + "mixedTransport": { + "command": "mcp-remote-proxy", + "args": [ + "https://example.com/mixed-transport", + "--transport", + "http" + ], + "url": "https://example.com/mixed-transport" + } + } + }"#, + ) + .expect("write mcp"); + + assert_eq!( + build_mcp_config_from_external( + root.path(), + /*external_agent_home*/ None, + /*settings*/ None, + ) + .unwrap(), + toml::from_str( + r#" +[mcp_servers.mixedTransport] +command = "mcp-remote-proxy" +args = [ + "https://example.com/mixed-transport", + "--transport", + "http", +] +"# + ) + .unwrap() + ); +} + +#[test] +fn mcp_migration_skips_unsupported_transports() { + let root = tempfile::TempDir::new().expect("tempdir"); + fs::write( + root.path().join(".mcp.json"), + r#"{ + "mcpServers": { + "legacy-sse": {"type": "sse", "url": "https://example.invalid/sse"}, + "vault": { + "url": "https://example.invalid/vault", + "headers": {"Authorization": "Bearer ${VAULT_TOKEN:-dev-token}"} + } + } + }"#, + ) + .expect("write mcp"); + + assert_eq!( + build_mcp_config_from_external( + root.path(), + /*external_agent_home*/ None, + /*settings*/ None, + ) + .unwrap(), + toml::from_str( + r#" +[mcp_servers.vault] +url = "https://example.invalid/vault" +bearer_token_env_var = "VAULT_TOKEN" +"# + ) + .unwrap() + ); +} + +#[test] +fn mcp_migration_reads_matching_project_entries_from_repo_external_project_config() { + let root = tempfile::TempDir::new().expect("tempdir"); + let project = root.path().join("repo"); + fs::create_dir_all(&project).expect("create repo"); + let other = root.path().join("other"); + fs::create_dir_all(&other).expect("create other"); + fs::write( + project.join(external_agent_project_config_file()), + serde_json::json!({ + "mcpServers": { + "top": {"command": "top-server"} + }, + "projects": { + project.display().to_string(): { + "mcpServers": { + "repo": {"command": "repo-server"} + } + }, + other.display().to_string(): { + "mcpServers": { + "other": {"command": "other-server"} + } + } + } + }) + .to_string(), + ) + .expect("write external agent project config"); + + assert_eq!( + build_mcp_config_from_external( + &project, /*external_agent_home*/ None, /*settings*/ None, + ) + .unwrap(), + toml::from_str( + r#" +[mcp_servers.repo] +command = "repo-server" + +[mcp_servers.top] +command = "top-server" +"# + ) + .unwrap() + ); +} + +#[test] +fn mcp_migration_reads_matching_project_entries_from_home_external_project_config() { + let root = tempfile::TempDir::new().expect("tempdir"); + let project = root.path().join("repo"); + fs::create_dir_all(&project).expect("create repo"); + let external_agent_home = root.path().join(external_agent_config_dir()); + fs::create_dir_all(&external_agent_home).expect("create external agent home"); + fs::write( + root.path().join(external_agent_project_config_file()), + serde_json::json!({ + "projects": { + project.display().to_string(): { + "mcpServers": { + "repo": {"command": "repo-server"} + } + } + } + }) + .to_string(), + ) + .expect("write external agent project config"); + + assert_eq!( + build_mcp_config_from_external( + &project, + Some(&external_agent_home), + /*settings*/ None, + ) + .unwrap(), + toml::from_str( + r#" +[mcp_servers.repo] +command = "repo-server" +"# + ) + .unwrap() + ); +} + +#[test] +fn mcp_migration_preserves_repo_servers_over_home_project_entries() { + let root = tempfile::TempDir::new().expect("tempdir"); + let project = root.path().join("repo"); + fs::create_dir_all(&project).expect("create repo"); + let external_agent_home = root.path().join(external_agent_config_dir()); + fs::create_dir_all(&external_agent_home).expect("create external agent home"); + fs::write( + project.join(EXTERNAL_AGENT_MCP_CONFIG_FILE), + serde_json::json!({ + "mcpServers": { + "shared": {"command": "repo-server"} + } + }) + .to_string(), + ) + .expect("write repo mcp"); + fs::write( + root.path().join(external_agent_project_config_file()), + serde_json::json!({ + "projects": { + project.display().to_string(): { + "mcpServers": { + "home-only": {"command": "home-only-server"}, + "shared": {"command": "home-server"} + } + } + } + }) + .to_string(), + ) + .expect("write external agent project config"); + + assert_eq!( + build_mcp_config_from_external( + &project, + Some(&external_agent_home), + /*settings*/ None, + ) + .unwrap(), + toml::from_str( + r#" +[mcp_servers.home-only] +command = "home-only-server" + +[mcp_servers.shared] +command = "repo-server" +"# + ) + .unwrap() + ); +} + +#[test] +fn mcp_migration_skips_disabled_servers() { + let root = tempfile::TempDir::new().expect("tempdir"); + fs::write( + root.path().join(".mcp.json"), + r#"{ + "mcpServers": { + "enabled": {"command": "enabled-server"}, + "explicit-disabled": {"command": "disabled-server", "disabled": true}, + "not-enabled": {"command": "not-enabled-server"} + } + }"#, + ) + .expect("write mcp"); + let settings = serde_json::json!({ + "enabledMcpjsonServers": ["enabled"], + "disabledMcpjsonServers": ["explicit-disabled"] + }); + + assert_eq!( + build_mcp_config_from_external( + root.path(), + /*external_agent_home*/ None, + Some(&settings), + ) + .unwrap(), + toml::from_str( + r#" +[mcp_servers.enabled] +command = "enabled-server" +"# + ) + .unwrap() + ); +} + +#[test] +fn command_skill_names_include_nested_paths() { + let root = source_path("commands"); + let file = source_path("commands/pr/review.md"); + + assert_eq!(command_skill_name(&root, &file), "source-command-pr-review"); +} + +#[test] +fn command_skill_names_must_fit_codex_skill_loader_limit() { + let root = source_path("commands"); + let file = source_path("commands/this/is/a/deeply/nested/command/with/a/very/long/name.md"); + let document = parse_document_content("---\ndescription: Review PR\n---\nReview\n"); + + assert!( + command_skill_name_if_supported( + &root, + &file, + &document, + CommandDescriptionMode::RequireFrontmatter, + ) + .is_none() + ); +} + +#[test] +fn commands_with_overlong_descriptions_are_preserved() { + let root = source_path("commands"); + let file = source_path("commands/review.md"); + let description = "x".repeat(MAX_SKILL_DESCRIPTION_LEN + 1); + let document = + parse_document_content(&format!("---\ndescription: {description}\n---\nReview\n")); + + assert_eq!( + command_skill_name_if_supported( + &root, + &file, + &document, + CommandDescriptionMode::RequireFrontmatter, + ), + Some("source-command-review".to_string()) + ); + + let rendered = render_command_skill( + &document.body, + "source-command-review", + &description, + "review", + TEST_REWRITE_PROFILE, + ); + let rendered_document = parse_document_content(&rendered); + assert_eq!( + rendered_document + .frontmatter + .get("description") + .and_then(FrontmatterValue::as_scalar), + Some(description.as_str()) + ); +} + +#[test] +fn commands_with_provider_runtime_expansion_are_skipped() { + let root = source_path("commands"); + let file = source_path("commands/deploy.md"); + let document = parse_document_content( + "---\ndescription: Deploy\n---\nDeploy $ARGUMENTS from @release.yaml\n", + ); + + assert!( + command_skill_name_if_supported( + &root, + &file, + &document, + CommandDescriptionMode::RequireFrontmatter, + ) + .is_none() + ); +} + +#[test] +fn commands_without_description_are_skipped() { + let root = source_path("commands"); + let file = source_path("commands/README.md"); + let document = parse_document_content("# Notes\n\nThis documents commands.\n"); + + assert!( + command_skill_name_if_supported( + &root, + &file, + &document, + CommandDescriptionMode::RequireFrontmatter, + ) + .is_none() + ); +} + +#[test] +fn commands_can_derive_descriptions_from_source_names() { + let root = tempfile::TempDir::new().expect("tempdir"); + let commands = root.path().join("commands"); + let target_skills = root.path().join("skills"); + fs::create_dir_all(&commands).expect("create commands"); + fs::write( + commands.join("review-code.md"), + "Review the current change.\n", + ) + .expect("write command"); + let profile = CommandMigrationProfile::new( + TEST_REWRITE_PROFILE, + CommandDescriptionMode::UseSourceNameFallback, + ); + + assert_eq!( + import_commands_with_profile(&commands, &target_skills, profile).unwrap(), + vec!["source-command-review-code".to_string()] + ); + let rendered = fs::read_to_string( + target_skills + .join("source-command-review-code") + .join("SKILL.md"), + ) + .expect("read migrated command"); + assert!(rendered.contains("description: \"Migrated source command `review-code`\"")); + assert!(rendered.contains("Review the current change.")); +} + +#[test] +fn command_slug_collisions_are_skipped() { + let root = tempfile::TempDir::new().expect("tempdir"); + let commands = root.path().join("commands"); + fs::create_dir_all(&commands).expect("create commands"); + fs::write( + commands.join("foo-bar.md"), + "---\ndescription: First\n---\nRun the first command.\n", + ) + .expect("write first command"); + fs::write( + commands.join("foo_bar.md"), + "---\ndescription: Second\n---\nRun the second command.\n", + ) + .expect("write second command"); + + assert_eq!( + unique_supported_command_sources(&commands, CommandDescriptionMode::RequireFrontmatter,) + .unwrap(), + Vec::<(PathBuf, String)>::new() + ); +} + +#[test] +fn subagent_accepts_yaml_block_lists_by_ignoring_unsupported_fields() { + let document = parse_document_content( + "---\nname: cloud-incident\ndescription: Debug incidents\nskills:\n - runbook-reader\ntools:\n - Read\n - Bash\ndisallowedTools:\n - Write\n---\nInvestigate carefully.\n", + ); + + assert!(agent_metadata(&document).is_some()); +} + +#[test] +fn subagent_requires_minimum_codex_agent_fields() { + let missing_description = + parse_document_content("---\nname: incomplete\n---\nInvestigate carefully.\n"); + let missing_body = + parse_document_content("---\nname: incomplete\ndescription: Missing body\n---\n"); + + assert!(agent_metadata(&missing_description).is_none()); + assert!(agent_metadata(&missing_body).is_none()); +} + +#[test] +fn subagent_preserves_default_model_when_source_model_is_present() { + let document = parse_document_content( + "---\nname: reviewer\ndescription: Review code\nmodel: source-opus\neffort: max\n---\nReview carefully.\n", + ); + let metadata = agent_metadata(&document).expect("metadata"); + let rendered: TomlValue = toml::from_str( + &render_agent_toml(&document.body, &metadata, TEST_REWRITE_PROFILE).expect("render agent"), + ) + .expect("parse rendered agent"); + let expected: TomlValue = toml::from_str( + r#" +name = "reviewer" +description = "Review code" +model_reasoning_effort = "xhigh" +developer_instructions = """ +Review carefully.""" +"#, + ) + .expect("parse expected agent"); + + assert_eq!(rendered, expected); +} + +#[test] +fn subagent_target_preserves_dotted_file_stem() { + let target_agents = Path::new("/repo/.codex/agents"); + let source_file = source_path("agents/security.audit.md"); + + assert_eq!( + subagent_target_file(&source_file, target_agents), + Some(PathBuf::from("/repo/.codex/agents/security.audit.toml")) + ); +} + +#[test] +fn frontmatter_accepts_crlf_delimiters() { + let document = parse_document_content( + "---\r\nname: reviewer\r\ndescription: Review code\r\n---\r\nReview carefully.\r\n", + ); + + assert_eq!( + ( + document + .frontmatter + .get("name") + .and_then(FrontmatterValue::as_scalar), + document + .frontmatter + .get("description") + .and_then(FrontmatterValue::as_scalar), + document.body.as_str(), + ), + ( + Some("reviewer"), + Some("Review code"), + "Review carefully.\r\n" + ) + ); +} + +#[test] +fn hook_migration_ignores_unsupported_handlers() { + let settings = serde_json::json!({ + "hooks": { + "PreToolUse": [{ + "matcher": "Bash", + "if": "tool_input.command contains 'rm'", + "hooks": [{ + "type": "command", + "command": source_hook_command("policy_gate.py") + }] + }, { + "matcher": "Edit", + "hooks": [ + { + "type": "command", + "if": "Bash(rm *)", + "command": source_hook_command("policy_gate.py") + }, + { + "type": "http", + "url": "https://example.invalid/hook" + } + ] + }], + "PermissionRequest": [{ + "matcher": "Bash", + "hooks": [{ + "type": "command", + "command": source_hook_command("approve.py") + }] + }], + "SubagentStart": [{ + "matcher": "worker", + "hooks": [{"type": "prompt", "prompt": "check"}] + }] + } + }); + let mut migration = serde_json::Map::new(); + append_convertible_hook_groups_cla( + &settings, + &mut migration, + Some(Path::new("/repo/.codex")), + TEST_REWRITE_PROFILE, + ); + + assert_eq!( + migration, + serde_json::json!({ + "PermissionRequest": [{ + "matcher": "Bash", + "hooks": [{ + "type": "command", + "command": migrated_hook_command("approve.py") + }] + }] + }) + .as_object() + .cloned() + .expect("object") + ); +} + +#[test] +fn hook_migration_honors_disable_all_hooks() { + let root = tempfile::TempDir::new().expect("tempdir"); + fs::write( + root.path().join("settings.json"), + r#"{ + "disableAllHooks": true, + "hooks": { + "SessionStart": [{ + "matcher": "startup", + "hooks": [{"type": "command", "command": "echo setup"}] + }] + } + }"#, + ) + .expect("write settings"); + + assert_eq!( + hook_migration_cla( + root.path(), + /*target_config_dir*/ None, + TEST_REWRITE_PROFILE, + ) + .unwrap(), + serde_json::Map::new() + ); +} + +#[test] +fn hook_migration_honors_settings_local_disable_override() { + let root = tempfile::TempDir::new().expect("tempdir"); + fs::write( + root.path().join("settings.json"), + r#"{ + "disableAllHooks": true, + "hooks": { + "SessionStart": [{ + "matcher": "project", + "hooks": [{"type": "command", "command": "echo project"}] + }] + } + }"#, + ) + .expect("write project settings"); + fs::write( + root.path().join("settings.local.json"), + r#"{ + "disableAllHooks": false, + "hooks": { + "SessionStart": [{ + "matcher": "local", + "hooks": [{"type": "command", "command": "echo local"}] + }] + } + }"#, + ) + .expect("write local settings"); + + assert_eq!( + hook_migration_cla( + root.path(), + /*target_config_dir*/ None, + TEST_REWRITE_PROFILE, + ) + .unwrap(), + serde_json::json!({ + "SessionStart": [{ + "matcher": "project", + "hooks": [{ + "type": "command", + "command": "echo project" + }] + }, { + "matcher": "local", + "hooks": [{ + "type": "command", + "command": "echo local" + }] + }] + }) + .as_object() + .cloned() + .expect("object") + ); +} + +#[test] +fn hook_command_paths_rewrite_to_target_hook_dir() { + let project_dir_env_var = external_agent_project_dir_env_var(); + let plugin_root_env_var = format!( + "{}_PLUGIN_ROOT", + SOURCE_EXTERNAL_AGENT_NAME.to_ascii_uppercase() + ); + let source_hooks_path = format!( + "{}/{EXTERNAL_AGENT_HOOKS_SUBDIR}", + external_agent_config_dir() + ); + assert_eq!( + rewrite_hook_command_cla( + &source_hook_command_with_project_dir("check.py"), + Some(Path::new("/repo/.codex")), + ), + migrated_hook_command("check.py") + ); + assert_eq!( + rewrite_hook_command_cla( + &format!("\"${project_dir_env_var}\"/{source_hooks_path}/check-style.sh"), + Some(Path::new("/repo/.codex")), + ), + shell_single_quote( + Path::new("/repo/.codex") + .join(EXTERNAL_AGENT_MIGRATED_HOOKS_SUBDIR) + .join("check-style.sh") + .to_string_lossy() + .as_ref() + ) + ); + assert_eq!( + rewrite_hook_command_cla( + &source_hook_command("check.py"), + Some(Path::new("/repo/.codex")), + ), + migrated_hook_command("check.py") + ); + assert_eq!( + rewrite_hook_command_cla( + &format!("python3 ./{source_hooks_path}/check.py"), + Some(Path::new("/repo/.codex")), + ), + migrated_hook_command("check.py") + ); + assert_eq!( + rewrite_hook_command_cla( + &format!("python3 '${{{project_dir_env_var}}}/{source_hooks_path}/check.py'"), + Some(Path::new("/repo/.codex")), + ), + migrated_quoted_hook_command("check.py") + ); + assert_eq!( + rewrite_hook_command_cla( + &format!("python3 \"${{{project_dir_env_var}}}/{source_hooks_path}/check.py\""), + Some(Path::new("/repo/.codex")), + ), + migrated_quoted_hook_command("check.py") + ); + assert_eq!( + rewrite_hook_command_cla( + &format!("bash -lc \"python3 {source_hooks_path}/check.py\""), + Some(Path::new("/repo/.codex")), + ), + format!("bash -lc \"python3 {source_hooks_path}/check.py\"") + ); + assert_eq!( + rewrite_hook_command_cla( + &format!( + "HOOK=${{{project_dir_env_var}}}/{source_hooks_path}/check.py python3 \"$HOOK\"" + ), + Some(Path::new("/repo/.codex")), + ), + format!("HOOK=${{{project_dir_env_var}}}/{source_hooks_path}/check.py python3 \"$HOOK\"") + ); + assert_eq!( + rewrite_hook_command_cla( + &format!("python3 {source_hooks_path}/${{SCRIPT}}.py"), + Some(Path::new("/repo/.codex")), + ), + format!("python3 {source_hooks_path}/${{SCRIPT}}.py") + ); + assert_eq!( + rewrite_hook_command_cla( + &format!("python3 {source_hooks_path}/{{lint,fmt}}.sh"), + Some(Path::new("/repo/.codex")), + ), + format!("python3 {source_hooks_path}/{{lint,fmt}}.sh") + ); + assert_eq!( + rewrite_hook_command_cla( + &format!("python3 {source_hooks_path}/my\\ script.py"), + Some(Path::new("/repo/.codex")), + ), + format!("python3 {source_hooks_path}/my\\ script.py") + ); + assert_eq!( + rewrite_hook_command_cla( + &format!("python3 .{SOURCE_EXTERNAL_AGENT_NAME}\\hooks\\check.py"), + Some(Path::new("/repo/.codex")), + ), + format!("python3 .{}\\hooks\\check.py", SOURCE_EXTERNAL_AGENT_NAME) + ); + assert_eq!( + rewrite_hook_command_cla( + &format!( + "python3 \"%{}%\\{}\\hooks\\check.py\"", + project_dir_env_var, + external_agent_config_dir() + ), + Some(Path::new("/repo/.codex")), + ), + format!( + "python3 \"%{}%\\{}\\hooks\\check.py\"", + project_dir_env_var, + external_agent_config_dir() + ) + ); + assert_eq!( + rewrite_hook_command_cla( + &format!("python3 '${{{project_dir_env_var}}}/{source_hooks_path}/my script.py'"), + Some(Path::new("/repo/.codex")), + ), + migrated_quoted_hook_command("my script.py") + ); + assert_eq!( + rewrite_hook_command_cla( + &format!("/repo/{source_hooks_path}/check.py 2>/dev/null || true"), + Some(Path::new("/repo/.codex")), + ), + format!( + "{} 2>/dev/null || true", + shell_single_quote( + Path::new("/repo/.codex") + .join(EXTERNAL_AGENT_MIGRATED_HOOKS_SUBDIR) + .join("check.py") + .to_string_lossy() + .as_ref() + ) + ) + ); + let plugin_script_command = format!("${{{plugin_root_env_var}}}/scripts/format.sh"); + assert_eq!( + rewrite_hook_command_cla(&plugin_script_command, Some(Path::new("/repo/.codex")),), + plugin_script_command + ); +} + +#[test] +fn hook_script_copy_keeps_existing_target_scripts() { + let root = tempfile::TempDir::new().expect("tempdir"); + let source_external_agent_dir = root.path().join(external_agent_config_dir()); + let source_hooks = source_external_agent_dir.join(EXTERNAL_AGENT_HOOKS_SUBDIR); + let target_config_dir = root.path().join(".codex"); + let target_hooks = target_config_dir.join(EXTERNAL_AGENT_MIGRATED_HOOKS_SUBDIR); + fs::create_dir_all(&source_hooks).expect("create source hooks"); + fs::create_dir_all(&target_hooks).expect("create target hooks"); + fs::write(source_hooks.join("check.py"), "new script").expect("write source hook"); + fs::write(target_hooks.join("check.py"), "existing script").expect("write target hook"); + + copy_hook_scripts(&source_external_agent_dir, &target_config_dir).expect("copy hooks"); + + assert_eq!( + fs::read_to_string(target_hooks.join("check.py")).expect("read target hook"), + "existing script" + ); +} + +#[test] +fn hook_migration_drops_negative_timeouts() { + let settings = serde_json::json!({ + "hooks": { + "SessionStart": [{ + "matcher": "startup", + "hooks": [{ + "type": "command", + "command": "echo setup", + "timeout": -1 + }] + }] + } + }); + let mut migration = serde_json::Map::new(); + append_convertible_hook_groups_cla( + &settings, + &mut migration, + /*target_config_dir*/ None, + TEST_REWRITE_PROFILE, + ); + + assert_eq!( + migration, + serde_json::json!({ + "SessionStart": [{ + "matcher": "startup", + "hooks": [{ + "type": "command", + "command": "echo setup" + }] + }] + }) + .as_object() + .cloned() + .expect("object") + ); +} diff --git a/codex-rs/external-agent-sessions/src/detect.rs b/codex-rs/external-agent-migration/src/sessions/detect_cla.rs similarity index 89% rename from codex-rs/external-agent-sessions/src/detect.rs rename to codex-rs/external-agent-migration/src/sessions/detect_cla.rs index f42c90766b..7e088c7ce8 100644 --- a/codex-rs/external-agent-sessions/src/detect.rs +++ b/codex-rs/external-agent-migration/src/sessions/detect_cla.rs @@ -1,8 +1,8 @@ -use crate::ExternalAgentSessionMigration; -use crate::ledger::load_import_ledger; -use crate::ledger::save_import_ledger; -use crate::now_unix_seconds; -use crate::summarize_session; +use super::ExternalAgentSessionMigration; +use super::ledger::load_import_ledger; +use super::ledger::save_import_ledger; +use super::now_unix_seconds; +use super::summarize_session; use std::cmp::Reverse; use std::collections::BinaryHeap; use std::fs; @@ -13,7 +13,7 @@ use std::time::Duration; const SESSION_IMPORT_MAX_COUNT: usize = 50; const SESSION_IMPORT_MAX_AGE: Duration = Duration::from_secs(30 * 24 * 60 * 60); -pub fn detect_recent_sessions( +pub fn detect_recent_cla_sessions( external_agent_home: &Path, codex_home: &Path, ) -> io::Result> { @@ -110,8 +110,8 @@ pub fn detect_recent_sessions( #[cfg(test)] mod tests { + use super::super::ledger::record_imported_session; use super::*; - use crate::ledger::record_imported_session; use codex_protocol::ThreadId; use serde_json::Value as JsonValue; use std::fs::FileTimes; @@ -135,7 +135,8 @@ mod tests { ], ); - let sessions = detect_recent_sessions(&external_agent_home, root.path()).expect("detect"); + let sessions = + detect_recent_cla_sessions(&external_agent_home, root.path()).expect("detect"); assert_eq!( sessions, @@ -163,7 +164,8 @@ mod tests { ], ); - let sessions = detect_recent_sessions(&external_agent_home, root.path()).expect("detect"); + let sessions = + detect_recent_cla_sessions(&external_agent_home, root.path()).expect("detect"); assert_eq!( sessions, @@ -190,7 +192,8 @@ mod tests { ], ); - let sessions = detect_recent_sessions(&external_agent_home, root.path()).expect("detect"); + let sessions = + detect_recent_cla_sessions(&external_agent_home, root.path()).expect("detect"); assert_eq!( sessions, @@ -218,7 +221,8 @@ mod tests { ], ); - let sessions = detect_recent_sessions(&external_agent_home, root.path()).expect("detect"); + let sessions = + detect_recent_cla_sessions(&external_agent_home, root.path()).expect("detect"); assert_eq!( sessions, @@ -247,7 +251,8 @@ mod tests { )], ); - let sessions = detect_recent_sessions(&external_agent_home, root.path()).expect("detect"); + let sessions = + detect_recent_cla_sessions(&external_agent_home, root.path()).expect("detect"); assert_eq!( sessions, @@ -276,7 +281,7 @@ mod tests { ); assert!( - detect_recent_sessions(&external_agent_home, root.path()) + detect_recent_cla_sessions(&external_agent_home, root.path()) .expect("detect") .is_empty() ); @@ -313,7 +318,8 @@ mod tests { let mut all_sessions = expected.clone(); all_sessions.push(oldest_session.clone()); - let sessions = detect_recent_sessions(&external_agent_home, root.path()).expect("detect"); + let sessions = + detect_recent_cla_sessions(&external_agent_home, root.path()).expect("detect"); assert_eq!(sessions, expected); for session in sessions { @@ -321,7 +327,8 @@ mod tests { .expect("record import"); } - let sessions = detect_recent_sessions(&external_agent_home, root.path()).expect("detect"); + let sessions = + detect_recent_cla_sessions(&external_agent_home, root.path()).expect("detect"); assert_eq!(sessions, vec![oldest_session.clone()]); for session in sessions { @@ -347,7 +354,8 @@ mod tests { ); } - let sessions = detect_recent_sessions(&external_agent_home, root.path()).expect("detect"); + let sessions = + detect_recent_cla_sessions(&external_agent_home, root.path()).expect("detect"); assert_eq!(sessions, expected); for session in sessions { @@ -355,7 +363,8 @@ mod tests { .expect("record import"); } - let sessions = detect_recent_sessions(&external_agent_home, root.path()).expect("detect"); + let sessions = + detect_recent_cla_sessions(&external_agent_home, root.path()).expect("detect"); assert_eq!(sessions, vec![oldest_session]); } @@ -376,7 +385,7 @@ mod tests { .expect("record import"); assert!( - detect_recent_sessions(&external_agent_home, root.path()) + detect_recent_cla_sessions(&external_agent_home, root.path()) .expect("detect") .is_empty() ); @@ -404,8 +413,13 @@ mod tests { ]), ) .expect("update session"); + set_modified_at( + &session_path, + SystemTime::now() + Duration::from_secs(/*secs*/ 1), + ); - let sessions = detect_recent_sessions(&external_agent_home, root.path()).expect("detect"); + let sessions = + detect_recent_cla_sessions(&external_agent_home, root.path()).expect("detect"); assert_eq!( sessions, vec![ExternalAgentSessionMigration { diff --git a/codex-rs/external-agent-sessions/src/export.rs b/codex-rs/external-agent-migration/src/sessions/export.rs similarity index 98% rename from codex-rs/external-agent-sessions/src/export.rs rename to codex-rs/external-agent-migration/src/sessions/export.rs index 8c46eb55f8..ec34f9211f 100644 --- a/codex-rs/external-agent-sessions/src/export.rs +++ b/codex-rs/external-agent-migration/src/sessions/export.rs @@ -1,11 +1,11 @@ -use crate::ConversationMessage; -use crate::ImportedExternalAgentSession; -use crate::MessageRole; -use crate::records::read_session_import; -use crate::summarize_for_label; -use crate::title::IMPORTED_SESSION_FALLBACK_TITLE; -use crate::title::SessionTitleCandidates; -use crate::title::fallback_title_from_user_message; +use super::ConversationMessage; +use super::ImportedExternalAgentSession; +use super::MessageRole; +use super::records::read_session_import; +use super::summarize_for_label; +use super::title::IMPORTED_SESSION_FALLBACK_TITLE; +use super::title::SessionTitleCandidates; +use super::title::fallback_title_from_user_message; use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseItem; use codex_protocol::protocol::AgentMessageEvent; diff --git a/codex-rs/external-agent-sessions/src/ledger.rs b/codex-rs/external-agent-migration/src/sessions/ledger.rs similarity index 99% rename from codex-rs/external-agent-sessions/src/ledger.rs rename to codex-rs/external-agent-migration/src/sessions/ledger.rs index 9a3b2042b8..6fbd0ce2dc 100644 --- a/codex-rs/external-agent-sessions/src/ledger.rs +++ b/codex-rs/external-agent-migration/src/sessions/ledger.rs @@ -1,4 +1,4 @@ -use crate::now_unix_seconds; +use super::now_unix_seconds; use codex_protocol::ThreadId; use serde::Deserialize; use serde::Serialize; diff --git a/codex-rs/external-agent-sessions/src/ledger_tests.rs b/codex-rs/external-agent-migration/src/sessions/ledger_tests.rs similarity index 100% rename from codex-rs/external-agent-sessions/src/ledger_tests.rs rename to codex-rs/external-agent-migration/src/sessions/ledger_tests.rs diff --git a/codex-rs/external-agent-sessions/src/lib.rs b/codex-rs/external-agent-migration/src/sessions/mod.rs similarity index 98% rename from codex-rs/external-agent-sessions/src/lib.rs rename to codex-rs/external-agent-migration/src/sessions/mod.rs index b13322dffb..cfaeef5b57 100644 --- a/codex-rs/external-agent-sessions/src/lib.rs +++ b/codex-rs/external-agent-migration/src/sessions/mod.rs @@ -1,6 +1,6 @@ //! Parsing and export helpers for external-agent session histories. -mod detect; +mod detect_cla; mod export; mod ledger; mod records; @@ -11,7 +11,7 @@ use std::io; use std::path::Path; use std::path::PathBuf; -pub use detect::detect_recent_sessions; +pub use detect_cla::detect_recent_cla_sessions; use export::load_session_for_import_with_content_sha256; pub use ledger::CompletedExternalAgentSessionImport; pub use ledger::has_current_session_been_imported; diff --git a/codex-rs/external-agent-sessions/src/records.rs b/codex-rs/external-agent-migration/src/sessions/records.rs similarity index 96% rename from codex-rs/external-agent-sessions/src/records.rs rename to codex-rs/external-agent-migration/src/sessions/records.rs index 2901714a19..3ab968c437 100644 --- a/codex-rs/external-agent-sessions/src/records.rs +++ b/codex-rs/external-agent-migration/src/sessions/records.rs @@ -1,10 +1,10 @@ -use crate::ConversationMessage; -use crate::ExternalAgentSessionMigration; -use crate::MessageRole; -use crate::title::IMPORTED_SESSION_FALLBACK_TITLE; -use crate::title::SessionTitleCandidates; -use crate::title::fallback_title_from_user_message; -use crate::truncate; +use super::ConversationMessage; +use super::ExternalAgentSessionMigration; +use super::MessageRole; +use super::title::IMPORTED_SESSION_FALLBACK_TITLE; +use super::title::SessionTitleCandidates; +use super::title::fallback_title_from_user_message; +use super::truncate; use serde_json::Value as JsonValue; use sha2::Digest; use sha2::Sha256; @@ -422,13 +422,13 @@ mod tests { fn converts_tool_result_blocks_to_bounded_external_agent_tags() { let block = serde_json::json!({ "type": "tool_result", - "content": "codex-rs/external-agent-sessions/src/records.rs" + "content": "codex-rs/external-agent-migration/src/sessions/records.rs" }); assert_eq!( tool_result_note(&block), "[external_agent_tool_result]\n\ - codex-rs/external-agent-sessions/src/records.rs\n\ + codex-rs/external-agent-migration/src/sessions/records.rs\n\ [/external_agent_tool_result]" ); } diff --git a/codex-rs/external-agent-sessions/src/title.rs b/codex-rs/external-agent-migration/src/sessions/title.rs similarity index 98% rename from codex-rs/external-agent-sessions/src/title.rs rename to codex-rs/external-agent-migration/src/sessions/title.rs index 97e8cda224..8b7da0f455 100644 --- a/codex-rs/external-agent-sessions/src/title.rs +++ b/codex-rs/external-agent-migration/src/sessions/title.rs @@ -1,5 +1,5 @@ -use crate::SESSION_TITLE_MAX_LEN; -use crate::truncate; +use super::SESSION_TITLE_MAX_LEN; +use super::truncate; pub(super) const IMPORTED_SESSION_FALLBACK_TITLE: &str = "Imported session"; const RECOGNIZED_CONTROL_WRAPPERS: [(&str, &str); 10] = [ diff --git a/codex-rs/external-agent-sessions/src/title_tests.rs b/codex-rs/external-agent-migration/src/sessions/title_tests.rs similarity index 100% rename from codex-rs/external-agent-sessions/src/title_tests.rs rename to codex-rs/external-agent-migration/src/sessions/title_tests.rs diff --git a/codex-rs/external-agent-sessions/BUILD.bazel b/codex-rs/external-agent-sessions/BUILD.bazel deleted file mode 100644 index 5a09983390..0000000000 --- a/codex-rs/external-agent-sessions/BUILD.bazel +++ /dev/null @@ -1,6 +0,0 @@ -load("//:defs.bzl", "codex_rust_crate") - -codex_rust_crate( - name = "external-agent-sessions", - crate_name = "codex_external_agent_sessions", -) diff --git a/codex-rs/external-agent-sessions/Cargo.toml b/codex-rs/external-agent-sessions/Cargo.toml deleted file mode 100644 index c3639f8e85..0000000000 --- a/codex-rs/external-agent-sessions/Cargo.toml +++ /dev/null @@ -1,25 +0,0 @@ -[package] -name = "codex-external-agent-sessions" -version.workspace = true -edition.workspace = true -license.workspace = true - -[lib] -doctest = false -name = "codex_external_agent_sessions" -path = "src/lib.rs" - -[lints] -workspace = true - -[dependencies] -chrono = { workspace = true } -codex-protocol = { workspace = true } -codex-utils-output-truncation = { workspace = true } -serde = { workspace = true, features = ["derive"] } -serde_json = { workspace = true } -sha2 = { workspace = true } - -[dev-dependencies] -codex-app-server-protocol = { workspace = true } -tempfile = { workspace = true } diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index 3e365a9a9b..8513028ed0 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -429,7 +429,7 @@ impl AppServerSession { request_id, params: ExternalAgentConfigImportParams { migration_items, - source: None, + source: Some("claude-code".to_string()), }, }) .await 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 c1cef68dfd..b292a47f9d 100644 --- a/codex-rs/tui/src/external_agent_config_migration_flow.rs +++ b/codex-rs/tui/src/external_agent_config_migration_flow.rs @@ -216,6 +216,7 @@ pub(crate) async fn handle_external_agent_config_migration_prompt( .external_agent_config_detect(ExternalAgentConfigDetectParams { include_home: true, cwds: Some(vec![cwd.clone()]), + source: Some("claude-code".to_string()), }) .await {