From 20b4b80426a84a7a533f76979ecdab81ae20ad7a Mon Sep 17 00:00:00 2001 From: alexsong-oai Date: Fri, 17 Apr 2026 02:34:55 -0700 Subject: [PATCH] =?UTF-8?q?Sync=20local=20plugin=20imports,=20async=20remo?= =?UTF-8?q?te=20imports,=20refresh=20caches=20after=E2=80=A6=20(#18246)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit … import ## Why `externalAgentConfig/import` used to spawn plugin imports in the background and return immediately. That meant local marketplace imports could still be in flight when the caller refreshed plugin state, so newly imported plugins would not show up right away. This change makes local marketplace imports complete before the RPC returns, while keeping remote marketplace imports asynchronous so we do not block on remote fetches. ## What changed - split plugin migration details into local and remote marketplace imports based on the external config source - import local marketplaces synchronously during `externalAgentConfig/import` - return pending remote plugin imports to the app-server so it can finish them in the background - clear the plugin and skills caches before responding to plugin imports, and again after background remote imports complete, so the next `plugin/list` reloads fresh state - keep marketplace source parsing encapsulated behind `is_local_marketplace_source(...)` instead of re-exporting the internal enum - add core and app-server coverage for the synchronous local import path and the pending remote import path ## Verification - `cargo test -p codex-app-server-protocol` - `cargo test -p codex-core` (currently fails an existing unrelated test: `config_loader::tests::cli_override_can_update_project_local_mcp_server_when_project_is_trusted`) - `cargo test` (currently fails existing `codex-app-server` integration tests in MCP/skills/thread-start areas, plus the unrelated `codex-core` failure above) --- .../schema/json/ServerNotification.json | 23 +++ .../codex_app_server_protocol.schemas.json | 25 +++ .../codex_app_server_protocol.v2.schemas.json | 25 +++ ...gentConfigImportCompletedNotification.json | 5 + .../schema/typescript/ServerNotification.ts | 3 +- ...lAgentConfigImportCompletedNotification.ts | 5 + .../schema/typescript/v2/index.ts | 1 + .../src/protocol/common.rs | 1 + .../app-server-protocol/src/protocol/v2.rs | 5 + codex-rs/app-server/README.md | 2 +- .../src/external_agent_config_api.rs | 20 +- codex-rs/app-server/src/message_processor.rs | 66 ++++++- .../tests/suite/v2/external_agent_config.rs | 186 ++++++++++++++++++ codex-rs/app-server/tests/suite/v2/mod.rs | 1 + codex-rs/core/src/external_agent_config.rs | 95 +++++++-- .../core/src/external_agent_config_tests.rs | 117 +++++++++++ codex-rs/core/src/plugins/marketplace_add.rs | 10 + .../src/plugins/marketplace_add/metadata.rs | 2 +- codex-rs/core/src/plugins/mod.rs | 1 + codex-rs/tui/src/app/app_server_adapter.rs | 17 +- codex-rs/tui/src/chatwidget.rs | 1 + 21 files changed, 583 insertions(+), 28 deletions(-) create mode 100644 codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigImportCompletedNotification.json create mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigImportCompletedNotification.ts create mode 100644 codex-rs/app-server/tests/suite/v2/external_agent_config.rs diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index d02a28e37..63bfc1cd5 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -967,6 +967,9 @@ ], "type": "object" }, + "ExternalAgentConfigImportCompletedNotification": { + "type": "object" + }, "FileChangeOutputDeltaNotification": { "properties": { "delta": { @@ -4765,6 +4768,26 @@ "title": "App/list/updatedNotification", "type": "object" }, + { + "properties": { + "method": { + "enum": [ + "externalAgentConfig/import/completed" + ], + "title": "ExternalAgentConfig/import/completedNotificationMethod", + "type": "string" + }, + "params": { + "$ref": "#/definitions/ExternalAgentConfigImportCompletedNotification" + } + }, + "required": [ + "method", + "params" + ], + "title": "ExternalAgentConfig/import/completedNotification", + "type": "object" + }, { "properties": { "method": { 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 e87fa1792..983dce5af 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 @@ -4138,6 +4138,26 @@ "title": "App/list/updatedNotification", "type": "object" }, + { + "properties": { + "method": { + "enum": [ + "externalAgentConfig/import/completed" + ], + "title": "ExternalAgentConfig/import/completedNotificationMethod", + "type": "string" + }, + "params": { + "$ref": "#/definitions/v2/ExternalAgentConfigImportCompletedNotification" + } + }, + "required": [ + "method", + "params" + ], + "title": "ExternalAgentConfig/import/completedNotification", + "type": "object" + }, { "properties": { "method": { @@ -7476,6 +7496,11 @@ "title": "ExternalAgentConfigDetectResponse", "type": "object" }, + "ExternalAgentConfigImportCompletedNotification": { + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "ExternalAgentConfigImportCompletedNotification", + "type": "object" + }, "ExternalAgentConfigImportParams": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 8254ad012..072fff49f 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 @@ -4093,6 +4093,11 @@ "title": "ExternalAgentConfigDetectResponse", "type": "object" }, + "ExternalAgentConfigImportCompletedNotification": { + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "ExternalAgentConfigImportCompletedNotification", + "type": "object" + }, "ExternalAgentConfigImportParams": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { @@ -9503,6 +9508,26 @@ "title": "App/list/updatedNotification", "type": "object" }, + { + "properties": { + "method": { + "enum": [ + "externalAgentConfig/import/completed" + ], + "title": "ExternalAgentConfig/import/completedNotificationMethod", + "type": "string" + }, + "params": { + "$ref": "#/definitions/ExternalAgentConfigImportCompletedNotification" + } + }, + "required": [ + "method", + "params" + ], + "title": "ExternalAgentConfig/import/completedNotification", + "type": "object" + }, { "properties": { "method": { diff --git a/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigImportCompletedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigImportCompletedNotification.json new file mode 100644 index 000000000..b1a57704e --- /dev/null +++ b/codex-rs/app-server-protocol/schema/json/v2/ExternalAgentConfigImportCompletedNotification.json @@ -0,0 +1,5 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "ExternalAgentConfigImportCompletedNotification", + "type": "object" +} \ No newline at end of file diff --git a/codex-rs/app-server-protocol/schema/typescript/ServerNotification.ts b/codex-rs/app-server-protocol/schema/typescript/ServerNotification.ts index 1db7027fe..f6df421dc 100644 --- a/codex-rs/app-server-protocol/schema/typescript/ServerNotification.ts +++ b/codex-rs/app-server-protocol/schema/typescript/ServerNotification.ts @@ -14,6 +14,7 @@ import type { ConfigWarningNotification } from "./v2/ConfigWarningNotification"; import type { ContextCompactedNotification } from "./v2/ContextCompactedNotification"; import type { DeprecationNoticeNotification } from "./v2/DeprecationNoticeNotification"; import type { ErrorNotification } from "./v2/ErrorNotification"; +import type { ExternalAgentConfigImportCompletedNotification } from "./v2/ExternalAgentConfigImportCompletedNotification"; import type { FileChangeOutputDeltaNotification } from "./v2/FileChangeOutputDeltaNotification"; import type { FsChangedNotification } from "./v2/FsChangedNotification"; import type { HookCompletedNotification } from "./v2/HookCompletedNotification"; @@ -59,4 +60,4 @@ import type { WindowsWorldWritableWarningNotification } from "./v2/WindowsWorldW /** * Notification sent from the server to the client. */ -export type ServerNotification = { "method": "error", "params": ErrorNotification } | { "method": "thread/started", "params": ThreadStartedNotification } | { "method": "thread/status/changed", "params": ThreadStatusChangedNotification } | { "method": "thread/archived", "params": ThreadArchivedNotification } | { "method": "thread/unarchived", "params": ThreadUnarchivedNotification } | { "method": "thread/closed", "params": ThreadClosedNotification } | { "method": "skills/changed", "params": SkillsChangedNotification } | { "method": "thread/name/updated", "params": ThreadNameUpdatedNotification } | { "method": "thread/tokenUsage/updated", "params": ThreadTokenUsageUpdatedNotification } | { "method": "turn/started", "params": TurnStartedNotification } | { "method": "hook/started", "params": HookStartedNotification } | { "method": "turn/completed", "params": TurnCompletedNotification } | { "method": "hook/completed", "params": HookCompletedNotification } | { "method": "turn/diff/updated", "params": TurnDiffUpdatedNotification } | { "method": "turn/plan/updated", "params": TurnPlanUpdatedNotification } | { "method": "item/started", "params": ItemStartedNotification } | { "method": "item/autoApprovalReview/started", "params": ItemGuardianApprovalReviewStartedNotification } | { "method": "item/autoApprovalReview/completed", "params": ItemGuardianApprovalReviewCompletedNotification } | { "method": "item/completed", "params": ItemCompletedNotification } | { "method": "rawResponseItem/completed", "params": RawResponseItemCompletedNotification } | { "method": "item/agentMessage/delta", "params": AgentMessageDeltaNotification } | { "method": "item/plan/delta", "params": PlanDeltaNotification } | { "method": "command/exec/outputDelta", "params": CommandExecOutputDeltaNotification } | { "method": "item/commandExecution/outputDelta", "params": CommandExecutionOutputDeltaNotification } | { "method": "item/commandExecution/terminalInteraction", "params": TerminalInteractionNotification } | { "method": "item/fileChange/outputDelta", "params": FileChangeOutputDeltaNotification } | { "method": "serverRequest/resolved", "params": ServerRequestResolvedNotification } | { "method": "item/mcpToolCall/progress", "params": McpToolCallProgressNotification } | { "method": "mcpServer/oauthLogin/completed", "params": McpServerOauthLoginCompletedNotification } | { "method": "mcpServer/startupStatus/updated", "params": McpServerStatusUpdatedNotification } | { "method": "account/updated", "params": AccountUpdatedNotification } | { "method": "account/rateLimits/updated", "params": AccountRateLimitsUpdatedNotification } | { "method": "app/list/updated", "params": AppListUpdatedNotification } | { "method": "fs/changed", "params": FsChangedNotification } | { "method": "item/reasoning/summaryTextDelta", "params": ReasoningSummaryTextDeltaNotification } | { "method": "item/reasoning/summaryPartAdded", "params": ReasoningSummaryPartAddedNotification } | { "method": "item/reasoning/textDelta", "params": ReasoningTextDeltaNotification } | { "method": "thread/compacted", "params": ContextCompactedNotification } | { "method": "model/rerouted", "params": ModelReroutedNotification } | { "method": "deprecationNotice", "params": DeprecationNoticeNotification } | { "method": "configWarning", "params": ConfigWarningNotification } | { "method": "fuzzyFileSearch/sessionUpdated", "params": FuzzyFileSearchSessionUpdatedNotification } | { "method": "fuzzyFileSearch/sessionCompleted", "params": FuzzyFileSearchSessionCompletedNotification } | { "method": "thread/realtime/started", "params": ThreadRealtimeStartedNotification } | { "method": "thread/realtime/itemAdded", "params": ThreadRealtimeItemAddedNotification } | { "method": "thread/realtime/transcript/delta", "params": ThreadRealtimeTranscriptDeltaNotification } | { "method": "thread/realtime/transcript/done", "params": ThreadRealtimeTranscriptDoneNotification } | { "method": "thread/realtime/outputAudio/delta", "params": ThreadRealtimeOutputAudioDeltaNotification } | { "method": "thread/realtime/sdp", "params": ThreadRealtimeSdpNotification } | { "method": "thread/realtime/error", "params": ThreadRealtimeErrorNotification } | { "method": "thread/realtime/closed", "params": ThreadRealtimeClosedNotification } | { "method": "windows/worldWritableWarning", "params": WindowsWorldWritableWarningNotification } | { "method": "windowsSandbox/setupCompleted", "params": WindowsSandboxSetupCompletedNotification } | { "method": "account/login/completed", "params": AccountLoginCompletedNotification }; +export type ServerNotification = { "method": "error", "params": ErrorNotification } | { "method": "thread/started", "params": ThreadStartedNotification } | { "method": "thread/status/changed", "params": ThreadStatusChangedNotification } | { "method": "thread/archived", "params": ThreadArchivedNotification } | { "method": "thread/unarchived", "params": ThreadUnarchivedNotification } | { "method": "thread/closed", "params": ThreadClosedNotification } | { "method": "skills/changed", "params": SkillsChangedNotification } | { "method": "thread/name/updated", "params": ThreadNameUpdatedNotification } | { "method": "thread/tokenUsage/updated", "params": ThreadTokenUsageUpdatedNotification } | { "method": "turn/started", "params": TurnStartedNotification } | { "method": "hook/started", "params": HookStartedNotification } | { "method": "turn/completed", "params": TurnCompletedNotification } | { "method": "hook/completed", "params": HookCompletedNotification } | { "method": "turn/diff/updated", "params": TurnDiffUpdatedNotification } | { "method": "turn/plan/updated", "params": TurnPlanUpdatedNotification } | { "method": "item/started", "params": ItemStartedNotification } | { "method": "item/autoApprovalReview/started", "params": ItemGuardianApprovalReviewStartedNotification } | { "method": "item/autoApprovalReview/completed", "params": ItemGuardianApprovalReviewCompletedNotification } | { "method": "item/completed", "params": ItemCompletedNotification } | { "method": "rawResponseItem/completed", "params": RawResponseItemCompletedNotification } | { "method": "item/agentMessage/delta", "params": AgentMessageDeltaNotification } | { "method": "item/plan/delta", "params": PlanDeltaNotification } | { "method": "command/exec/outputDelta", "params": CommandExecOutputDeltaNotification } | { "method": "item/commandExecution/outputDelta", "params": CommandExecutionOutputDeltaNotification } | { "method": "item/commandExecution/terminalInteraction", "params": TerminalInteractionNotification } | { "method": "item/fileChange/outputDelta", "params": FileChangeOutputDeltaNotification } | { "method": "serverRequest/resolved", "params": ServerRequestResolvedNotification } | { "method": "item/mcpToolCall/progress", "params": McpToolCallProgressNotification } | { "method": "mcpServer/oauthLogin/completed", "params": McpServerOauthLoginCompletedNotification } | { "method": "mcpServer/startupStatus/updated", "params": McpServerStatusUpdatedNotification } | { "method": "account/updated", "params": AccountUpdatedNotification } | { "method": "account/rateLimits/updated", "params": AccountRateLimitsUpdatedNotification } | { "method": "app/list/updated", "params": AppListUpdatedNotification } | { "method": "externalAgentConfig/import/completed", "params": ExternalAgentConfigImportCompletedNotification } | { "method": "fs/changed", "params": FsChangedNotification } | { "method": "item/reasoning/summaryTextDelta", "params": ReasoningSummaryTextDeltaNotification } | { "method": "item/reasoning/summaryPartAdded", "params": ReasoningSummaryPartAddedNotification } | { "method": "item/reasoning/textDelta", "params": ReasoningTextDeltaNotification } | { "method": "thread/compacted", "params": ContextCompactedNotification } | { "method": "model/rerouted", "params": ModelReroutedNotification } | { "method": "deprecationNotice", "params": DeprecationNoticeNotification } | { "method": "configWarning", "params": ConfigWarningNotification } | { "method": "fuzzyFileSearch/sessionUpdated", "params": FuzzyFileSearchSessionUpdatedNotification } | { "method": "fuzzyFileSearch/sessionCompleted", "params": FuzzyFileSearchSessionCompletedNotification } | { "method": "thread/realtime/started", "params": ThreadRealtimeStartedNotification } | { "method": "thread/realtime/itemAdded", "params": ThreadRealtimeItemAddedNotification } | { "method": "thread/realtime/transcript/delta", "params": ThreadRealtimeTranscriptDeltaNotification } | { "method": "thread/realtime/transcript/done", "params": ThreadRealtimeTranscriptDoneNotification } | { "method": "thread/realtime/outputAudio/delta", "params": ThreadRealtimeOutputAudioDeltaNotification } | { "method": "thread/realtime/sdp", "params": ThreadRealtimeSdpNotification } | { "method": "thread/realtime/error", "params": ThreadRealtimeErrorNotification } | { "method": "thread/realtime/closed", "params": ThreadRealtimeClosedNotification } | { "method": "windows/worldWritableWarning", "params": WindowsWorldWritableWarningNotification } | { "method": "windowsSandbox/setupCompleted", "params": WindowsSandboxSetupCompletedNotification } | { "method": "account/login/completed", "params": AccountLoginCompletedNotification }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigImportCompletedNotification.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigImportCompletedNotification.ts new file mode 100644 index 000000000..edb8f1916 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ExternalAgentConfigImportCompletedNotification.ts @@ -0,0 +1,5 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +export type ExternalAgentConfigImportCompletedNotification = Record; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts index 69cec75e7..31371b712 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts @@ -87,6 +87,7 @@ export type { ExperimentalFeatureListResponse } from "./ExperimentalFeatureListR export type { ExperimentalFeatureStage } from "./ExperimentalFeatureStage"; export type { ExternalAgentConfigDetectParams } from "./ExternalAgentConfigDetectParams"; export type { ExternalAgentConfigDetectResponse } from "./ExternalAgentConfigDetectResponse"; +export type { ExternalAgentConfigImportCompletedNotification } from "./ExternalAgentConfigImportCompletedNotification"; export type { ExternalAgentConfigImportParams } from "./ExternalAgentConfigImportParams"; export type { ExternalAgentConfigImportResponse } from "./ExternalAgentConfigImportResponse"; export type { ExternalAgentConfigMigrationItem } from "./ExternalAgentConfigMigrationItem"; diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index 7f555e068..db4284324 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -1017,6 +1017,7 @@ server_notification_definitions! { AccountUpdated => "account/updated" (v2::AccountUpdatedNotification), AccountRateLimitsUpdated => "account/rateLimits/updated" (v2::AccountRateLimitsUpdatedNotification), AppListUpdated => "app/list/updated" (v2::AppListUpdatedNotification), + ExternalAgentConfigImportCompleted => "externalAgentConfig/import/completed" (v2::ExternalAgentConfigImportCompletedNotification), FsChanged => "fs/changed" (v2::FsChangedNotification), ReasoningSummaryTextDelta => "item/reasoning/summaryTextDelta" (v2::ReasoningSummaryTextDeltaNotification), ReasoningSummaryPartAdded => "item/reasoning/summaryPartAdded" (v2::ReasoningSummaryPartAddedNotification), diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 517354932..eea9f9444 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -1034,6 +1034,11 @@ pub struct ExternalAgentConfigImportParams { #[ts(export_to = "v2/")] pub struct ExternalAgentConfigImportResponse {} +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct ExternalAgentConfigImportCompletedNotification {} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 2fece4454..733b3527a 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -201,7 +201,7 @@ Example with notification opt-out: - `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. - `externalAgentConfig/detect` — detect migratable external-agent artifacts with `includeHome` and optional `cwds`; each detected item includes `cwd` (`null` for home), and plugin migration items may additionally include structured `details` grouping plugin ids under each detected marketplace name. -- `externalAgentConfig/import` — apply selected external-agent migration items by passing explicit `migrationItems` with `cwd` (`null` for home) and any plugin `details` returned by detect. +- `externalAgentConfig/import` — apply selected external-agent migration items by passing explicit `migrationItems` with `cwd` (`null` for home) and any plugin `details` returned by detect. When a request includes plugin imports, the server emits `externalAgentConfig/import/completed` after the full import finishes (immediately after the response when everything completed synchronously, or after background remote imports finish). - `config/value/write` — write a single config key/value to the user's config.toml on disk. - `config/batchWrite` — apply multiple config edits atomically to the user's config.toml on disk, with optional `reloadUserConfig: true` to hot-reload loaded threads. - `configRequirements/read` — fetch loaded requirements constraints from `requirements.toml` and/or MDM (or `null` if none are configured), including allow-lists (`allowedApprovalPolicies`, `allowedSandboxModes`, `allowedWebSearchModes`), pinned feature values (`featureRequirements`), `enforceResidency`, and `network` constraints such as canonical domain/socket permissions plus `managedAllowedDomainsOnly`. diff --git a/codex-rs/app-server/src/external_agent_config_api.rs b/codex-rs/app-server/src/external_agent_config_api.rs index 2f90a55a1..b381c353d 100644 --- a/codex-rs/app-server/src/external_agent_config_api.rs +++ b/codex-rs/app-server/src/external_agent_config_api.rs @@ -2,7 +2,6 @@ use crate::error_code::INTERNAL_ERROR_CODE; use codex_app_server_protocol::ExternalAgentConfigDetectParams; use codex_app_server_protocol::ExternalAgentConfigDetectResponse; use codex_app_server_protocol::ExternalAgentConfigImportParams; -use codex_app_server_protocol::ExternalAgentConfigImportResponse; use codex_app_server_protocol::ExternalAgentConfigMigrationItem; use codex_app_server_protocol::ExternalAgentConfigMigrationItemType; use codex_app_server_protocol::JSONRPCErrorError; @@ -12,6 +11,7 @@ use codex_core::external_agent_config::ExternalAgentConfigDetectOptions; use codex_core::external_agent_config::ExternalAgentConfigMigrationItem as CoreMigrationItem; use codex_core::external_agent_config::ExternalAgentConfigMigrationItemType as CoreMigrationItemType; use codex_core::external_agent_config::ExternalAgentConfigService; +use codex_core::external_agent_config::PendingPluginImport; use std::io; use std::path::PathBuf; @@ -81,7 +81,7 @@ impl ExternalAgentConfigApi { pub(crate) async fn import( &self, params: ExternalAgentConfigImportParams, - ) -> Result { + ) -> Result, JSONRPCErrorError> { self.migration_service .import( params @@ -125,9 +125,21 @@ impl ExternalAgentConfigApi { .collect(), ) .await - .map_err(map_io_error)?; + .map_err(map_io_error) + } - Ok(ExternalAgentConfigImportResponse {}) + pub(crate) async fn complete_pending_plugin_import( + &self, + pending_plugin_import: PendingPluginImport, + ) -> Result<(), JSONRPCErrorError> { + self.migration_service + .import_plugins( + pending_plugin_import.cwd.as_deref(), + Some(pending_plugin_import.details), + ) + .await + .map(|_| ()) + .map_err(map_io_error) } } diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 6221cebee..da72af81c 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -39,7 +39,10 @@ use codex_app_server_protocol::ConfigWarningNotification; use codex_app_server_protocol::ExperimentalApi; use codex_app_server_protocol::ExperimentalFeatureEnablementSetParams; use codex_app_server_protocol::ExternalAgentConfigDetectParams; +use codex_app_server_protocol::ExternalAgentConfigImportCompletedNotification; use codex_app_server_protocol::ExternalAgentConfigImportParams; +use codex_app_server_protocol::ExternalAgentConfigImportResponse; +use codex_app_server_protocol::ExternalAgentConfigMigrationItemType; use codex_app_server_protocol::FsCopyParams; use codex_app_server_protocol::FsCreateDirectoryParams; use codex_app_server_protocol::FsGetMetadataParams; @@ -163,6 +166,7 @@ impl ExternalAuth for ExternalAuthRefreshBridge { pub(crate) struct MessageProcessor { outgoing: Arc, codex_message_processor: CodexMessageProcessor, + thread_manager: Arc, config_api: ConfigApi, external_agent_config_api: ExternalAgentConfigApi, fs_api: FsApi, @@ -311,7 +315,7 @@ impl MessageProcessor { runtime_feature_enablement, loader_overrides, cloud_requirements, - thread_manager, + thread_manager.clone(), analytics_events_client.clone(), ); let external_agent_config_api = @@ -322,6 +326,7 @@ impl MessageProcessor { Self { outgoing, codex_message_processor, + thread_manager: Arc::clone(&thread_manager), config_api, external_agent_config_api, fs_api, @@ -1131,8 +1136,65 @@ impl MessageProcessor { request_id: ConnectionRequestId, params: ExternalAgentConfigImportParams, ) { + let has_plugin_imports = params.migration_items.iter().any(|item| { + matches!( + item.item_type, + ExternalAgentConfigMigrationItemType::Plugins + ) + }); match self.external_agent_config_api.import(params).await { - Ok(response) => self.outgoing.send_response(request_id, response).await, + Ok(pending_plugin_imports) => { + if has_plugin_imports { + self.handle_config_mutation().await; + } + self.outgoing + .send_response(request_id, ExternalAgentConfigImportResponse {}) + .await; + + if !has_plugin_imports { + return; + } + + if pending_plugin_imports.is_empty() { + self.outgoing + .send_server_notification( + ServerNotification::ExternalAgentConfigImportCompleted( + ExternalAgentConfigImportCompletedNotification {}, + ), + ) + .await; + return; + } + + let external_agent_config_api = self.external_agent_config_api.clone(); + let outgoing = Arc::clone(&self.outgoing); + let thread_manager = Arc::clone(&self.thread_manager); + tokio::spawn(async move { + for pending_plugin_import in pending_plugin_imports { + match external_agent_config_api + .complete_pending_plugin_import(pending_plugin_import) + .await + { + Ok(()) => {} + Err(error) => { + tracing::warn!( + error = %error.message, + "external agent config plugin import failed" + ); + } + } + } + thread_manager.plugins_manager().clear_cache(); + thread_manager.skills_manager().clear_cache(); + outgoing + .send_server_notification( + ServerNotification::ExternalAgentConfigImportCompleted( + ExternalAgentConfigImportCompletedNotification {}, + ), + ) + .await; + }); + } Err(error) => self.outgoing.send_error(request_id, error).await, } } 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 new file mode 100644 index 000000000..21acf2490 --- /dev/null +++ b/codex-rs/app-server/tests/suite/v2/external_agent_config.rs @@ -0,0 +1,186 @@ +use std::time::Duration; + +use anyhow::Result; +use app_test_support::McpProcess; +use app_test_support::to_response; +use codex_app_server_protocol::ExternalAgentConfigImportResponse; +use codex_app_server_protocol::JSONRPCResponse; +use codex_app_server_protocol::PluginListParams; +use codex_app_server_protocol::PluginListResponse; +use codex_app_server_protocol::RequestId; +use pretty_assertions::assert_eq; +use tempfile::TempDir; +use tokio::time::timeout; + +const DEFAULT_TIMEOUT: Duration = Duration::from_secs(60); + +#[tokio::test] +async fn external_agent_config_import_sends_completion_notification_for_local_plugins() -> Result<()> +{ + let codex_home = TempDir::new()?; + let marketplace_root = codex_home.path().join("marketplace"); + let plugin_root = marketplace_root.join("plugins").join("sample"); + std::fs::create_dir_all(marketplace_root.join(".agents/plugins"))?; + std::fs::create_dir_all(plugin_root.join(".codex-plugin"))?; + std::fs::write( + marketplace_root.join(".agents/plugins/marketplace.json"), + r#"{ + "name": "debug", + "plugins": [ + { + "name": "sample", + "source": { + "source": "local", + "path": "./plugins/sample" + } + } + ] +}"#, + )?; + std::fs::write( + plugin_root.join(".codex-plugin/plugin.json"), + r#"{"name":"sample","version":"0.1.0"}"#, + )?; + std::fs::create_dir_all(codex_home.path().join(".claude"))?; + let settings = serde_json::json!({ + "enabledPlugins": { + "sample@debug": true + }, + "extraKnownMarketplaces": { + "debug": { + "source": "local", + "path": marketplace_root, + } + } + }); + std::fs::write( + codex_home.path().join(".claude").join("settings.json"), + serde_json::to_string_pretty(&settings)?, + )?; + + let home_dir = codex_home.path().display().to_string(); + let mut mcp = + McpProcess::new_with_env(codex_home.path(), &[("HOME", Some(home_dir.as_str()))]).await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + + let request_id = mcp + .send_raw_request( + "externalAgentConfig/import", + Some(serde_json::json!({ + "migrationItems": [{ + "itemType": "PLUGINS", + "description": "Import plugins", + "cwd": null, + "details": { + "plugins": [{ + "marketplaceName": "debug", + "pluginNames": ["sample"] + }] + } + }] + })), + ) + .await?; + + let response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + let response: ExternalAgentConfigImportResponse = to_response(response)?; + + assert_eq!(response, ExternalAgentConfigImportResponse {}); + let notification = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_notification_message("externalAgentConfig/import/completed"), + ) + .await??; + assert_eq!(notification.method, "externalAgentConfig/import/completed"); + + let request_id = mcp + .send_plugin_list_request(PluginListParams { + cwds: None, + force_remote_sync: false, + }) + .await?; + let response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + let response: PluginListResponse = to_response(response)?; + let plugin = response + .marketplaces + .iter() + .find(|marketplace| marketplace.name == "debug") + .and_then(|marketplace| { + marketplace + .plugins + .iter() + .find(|plugin| plugin.name == "sample") + }) + .expect("expected imported plugin to be listed"); + assert!(plugin.installed); + assert!(plugin.enabled); + Ok(()) +} + +#[tokio::test] +async fn external_agent_config_import_sends_completion_notification_after_pending_plugins_finish() +-> Result<()> { + let codex_home = TempDir::new()?; + std::fs::create_dir_all(codex_home.path().join(".claude"))?; + std::fs::write( + codex_home.path().join(".claude").join("settings.json"), + r#"{ + "enabledPlugins": { + "formatter@acme-tools": true + }, + "extraKnownMarketplaces": { + "acme-tools": { + "source": "owner/debug-marketplace" + } + } +}"#, + )?; + + let home_dir = codex_home.path().display().to_string(); + let mut mcp = + McpProcess::new_with_env(codex_home.path(), &[("HOME", Some(home_dir.as_str()))]).await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + + let request_id = mcp + .send_raw_request( + "externalAgentConfig/import", + Some(serde_json::json!({ + "migrationItems": [{ + "itemType": "PLUGINS", + "description": "Import plugins", + "cwd": null, + "details": { + "plugins": [{ + "marketplaceName": "acme-tools", + "pluginNames": ["formatter"] + }] + } + }] + })), + ) + .await?; + + let response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + let response: ExternalAgentConfigImportResponse = to_response(response)?; + assert_eq!(response, ExternalAgentConfigImportResponse {}); + let notification = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_notification_message("externalAgentConfig/import/completed"), + ) + .await??; + assert_eq!(notification.method, "externalAgentConfig/import/completed"); + + Ok(()) +} diff --git a/codex-rs/app-server/tests/suite/v2/mod.rs b/codex-rs/app-server/tests/suite/v2/mod.rs index bbf04006e..38097eabf 100644 --- a/codex-rs/app-server/tests/suite/v2/mod.rs +++ b/codex-rs/app-server/tests/suite/v2/mod.rs @@ -13,6 +13,7 @@ mod connection_handling_websocket_unix; mod dynamic_tools; mod experimental_api; mod experimental_feature_list; +mod external_agent_config; mod fs; mod initialize; mod marketplace_add; diff --git a/codex-rs/core/src/external_agent_config.rs b/codex-rs/core/src/external_agent_config.rs index f0f4443a7..73aba5af1 100644 --- a/codex-rs/core/src/external_agent_config.rs +++ b/codex-rs/core/src/external_agent_config.rs @@ -7,6 +7,7 @@ use crate::plugins::PluginsManager; use crate::plugins::add_marketplace; use crate::plugins::configured_plugins_from_stack; use crate::plugins::find_marketplace_manifest_path; +use crate::plugins::is_local_marketplace_source; use crate::plugins::parse_marketplace_source; use codex_core_plugins::marketplace::MarketplacePluginInstallPolicy; use codex_protocol::protocol::Product; @@ -51,12 +52,18 @@ pub struct MigrationDetails { pub plugins: Vec, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PendingPluginImport { + pub cwd: Option, + pub details: MigrationDetails, +} + #[derive(Debug, Clone, Default, PartialEq, Eq)] -struct PluginImportOutcome { - succeeded_marketplaces: Vec, - succeeded_plugin_ids: Vec, - failed_marketplaces: Vec, - failed_plugin_ids: Vec, +pub struct PluginImportOutcome { + pub succeeded_marketplaces: Vec, + pub succeeded_plugin_ids: Vec, + pub failed_marketplaces: Vec, + pub failed_plugin_ids: Vec, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -113,7 +120,8 @@ impl ExternalAgentConfigService { pub async fn import( &self, migration_items: Vec, - ) -> io::Result<()> { + ) -> io::Result> { + let mut pending_plugin_imports = Vec::new(); for migration_item in migration_items { match migration_item.item_type { ExternalAgentConfigMigrationItemType::Config => { @@ -141,17 +149,23 @@ impl ExternalAgentConfigService { ); } ExternalAgentConfigMigrationItemType::Plugins => { - let service = self.clone(); let cwd = migration_item.cwd; - let details = migration_item.details; - tokio::spawn(async move { - if let Err(err) = service.import_plugins(cwd.as_deref(), details).await { - tracing::warn!( - error = %err, - "external agent config plugin import failed" - ); - } - }); + let details = migration_item.details.ok_or_else(|| { + invalid_data_error("plugins migration item is missing details".to_string()) + })?; + let (local_details, remote_details) = + self.partition_plugin_migration_details(cwd.as_deref(), details)?; + + if let Some(local_details) = local_details { + self.import_plugins(cwd.as_deref(), Some(local_details)) + .await?; + } + if let Some(remote_details) = remote_details { + pending_plugin_imports.push(PendingPluginImport { + cwd, + details: remote_details, + }); + } emit_migration_metric( EXTERNAL_AGENT_CONFIG_IMPORT_METRIC, ExternalAgentConfigMigrationItemType::Plugins, @@ -162,7 +176,7 @@ impl ExternalAgentConfigService { } } - Ok(()) + Ok(pending_plugin_imports) } async fn detect_migrations( @@ -349,7 +363,52 @@ impl ExternalAgentConfigService { }) } - async fn import_plugins( + fn partition_plugin_migration_details( + &self, + 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 = read_external_settings(&source_settings)? + .map(|settings| collect_marketplace_import_sources(&settings, source_root)) + .unwrap_or_default(); + + let mut local_plugins = Vec::new(); + let mut remote_plugins = Vec::new(); + for plugin_group in details.plugins { + let is_local = import_sources + .get(&plugin_group.marketplace_name) + .and_then(|import_source| { + is_local_marketplace_source( + &import_source.source, + import_source.ref_name.clone(), + ) + .ok() + }) + .unwrap_or(false); + + if is_local { + local_plugins.push(plugin_group); + } else { + remote_plugins.push(plugin_group); + } + } + + let local_details = (!local_plugins.is_empty()).then_some(MigrationDetails { + plugins: local_plugins, + }); + let remote_details = (!remote_plugins.is_empty()).then_some(MigrationDetails { + plugins: remote_plugins, + }); + + Ok((local_details, remote_details)) + } + + pub async fn import_plugins( &self, cwd: Option<&Path>, details: Option, diff --git a/codex-rs/core/src/external_agent_config_tests.rs b/codex-rs/core/src/external_agent_config_tests.rs index f01a40e3d..4fd81351a 100644 --- a/codex-rs/core/src/external_agent_config_tests.rs +++ b/codex-rs/core/src/external_agent_config_tests.rs @@ -295,6 +295,123 @@ async fn import_home_skips_empty_config_migration() { assert!(!codex_home.join("config.toml").exists()); } +#[tokio::test] +async fn import_local_plugins_returns_completed_status() { + let (_root, external_agent_home, codex_home) = fixture_paths(); + let marketplace_root = external_agent_home.join("my-marketplace"); + let plugin_root = marketplace_root.join("plugins").join("cloudflare"); + fs::create_dir_all(marketplace_root.join(".claude-plugin")) + .expect("create marketplace manifest dir"); + fs::create_dir_all(plugin_root.join(".codex-plugin")).expect("create plugin manifest dir"); + fs::create_dir_all(&codex_home).expect("create codex home"); + + fs::write( + external_agent_home.join("settings.json"), + serde_json::to_string_pretty(&serde_json::json!({ + "enabledPlugins": { + "cloudflare@my-plugins": true + }, + "extraKnownMarketplaces": { + "my-plugins": { + "source": "local", + "path": marketplace_root + } + } + })) + .expect("serialize settings"), + ) + .expect("write settings"); + fs::write( + marketplace_root + .join(".claude-plugin") + .join("marketplace.json"), + r#"{ + "name": "my-plugins", + "plugins": [ + { + "name": "cloudflare", + "source": "./plugins/cloudflare" + } + ] + }"#, + ) + .expect("write marketplace manifest"); + fs::write( + plugin_root.join(".codex-plugin").join("plugin.json"), + r#"{"name":"cloudflare","version":"0.1.0"}"#, + ) + .expect("write plugin manifest"); + + let outcome = service_for_paths(external_agent_home, codex_home.clone()) + .import(vec![ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::Plugins, + description: String::new(), + cwd: None, + details: Some(MigrationDetails { + plugins: vec![PluginsMigration { + marketplace_name: "my-plugins".to_string(), + plugin_names: vec!["cloudflare".to_string()], + }], + }), + }]) + .await + .expect("import"); + + assert_eq!(outcome, Vec::::new()); + let config = fs::read_to_string(codex_home.join("config.toml")).expect("read config"); + assert!(config.contains(r#"[plugins."cloudflare@my-plugins"]"#)); + assert!(config.contains("enabled = true")); +} + +#[tokio::test] +async fn import_git_plugins_returns_pending_async_status() { + let (_root, external_agent_home, codex_home) = fixture_paths(); + fs::create_dir_all(&external_agent_home).expect("create external agent home"); + fs::write( + external_agent_home.join("settings.json"), + r#"{ + "enabledPlugins": { + "formatter@acme-tools": true + }, + "extraKnownMarketplaces": { + "acme-tools": { + "source": "owner/debug-marketplace" + } + } + }"#, + ) + .expect("write settings"); + + let outcome = service_for_paths(external_agent_home, codex_home.clone()) + .import(vec![ExternalAgentConfigMigrationItem { + item_type: ExternalAgentConfigMigrationItemType::Plugins, + description: String::new(), + cwd: None, + details: Some(MigrationDetails { + plugins: vec![PluginsMigration { + marketplace_name: "acme-tools".to_string(), + plugin_names: vec!["formatter".to_string()], + }], + }), + }]) + .await + .expect("import"); + + assert_eq!( + outcome, + vec![PendingPluginImport { + cwd: None, + details: MigrationDetails { + plugins: vec![PluginsMigration { + marketplace_name: "acme-tools".to_string(), + plugin_names: vec!["formatter".to_string()], + }], + }, + }] + ); + assert!(!codex_home.join("config.toml").exists()); +} + #[tokio::test] async fn detect_home_skips_config_when_target_already_has_supported_fields() { let (_root, external_agent_home, codex_home) = fixture_paths(); diff --git a/codex-rs/core/src/plugins/marketplace_add.rs b/codex-rs/core/src/plugins/marketplace_add.rs index 3ea044583..00bc19e40 100644 --- a/codex-rs/core/src/plugins/marketplace_add.rs +++ b/codex-rs/core/src/plugins/marketplace_add.rs @@ -56,6 +56,16 @@ pub async fn add_marketplace( .map_err(|err| MarketplaceAddError::Internal(format!("failed to add marketplace: {err}")))? } +pub(crate) fn is_local_marketplace_source( + source: &str, + explicit_ref: Option, +) -> Result { + Ok(matches!( + parse_marketplace_source(source, explicit_ref)?, + source::MarketplaceSource::Local { .. } + )) +} + fn add_marketplace_sync( codex_home: &Path, request: MarketplaceAddRequest, diff --git a/codex-rs/core/src/plugins/marketplace_add/metadata.rs b/codex-rs/core/src/plugins/marketplace_add/metadata.rs index ccded11c8..06b5e3956 100644 --- a/codex-rs/core/src/plugins/marketplace_add/metadata.rs +++ b/codex-rs/core/src/plugins/marketplace_add/metadata.rs @@ -1,5 +1,5 @@ use super::MarketplaceAddError; -use super::MarketplaceSource; +use super::source::MarketplaceSource; use crate::plugins::installed_marketplaces::resolve_configured_marketplace_root; use codex_config::CONFIG_TOML_FILE; use codex_config::MarketplaceConfigUpdate; diff --git a/codex-rs/core/src/plugins/mod.rs b/codex-rs/core/src/plugins/mod.rs index b39f168a0..a34189bce 100644 --- a/codex-rs/core/src/plugins/mod.rs +++ b/codex-rs/core/src/plugins/mod.rs @@ -49,6 +49,7 @@ pub use marketplace_add::MarketplaceAddError; pub use marketplace_add::MarketplaceAddOutcome; pub use marketplace_add::MarketplaceAddRequest; pub use marketplace_add::add_marketplace; +pub(crate) use marketplace_add::is_local_marketplace_source; pub(crate) use marketplace_add::parse_marketplace_source; pub(crate) use render::render_explicit_plugin_instructions; pub(crate) use render::render_plugins_section; diff --git a/codex-rs/tui/src/app/app_server_adapter.rs b/codex-rs/tui/src/app/app_server_adapter.rs index f2490ca6a..3dfc9fd23 100644 --- a/codex-rs/tui/src/app/app_server_adapter.rs +++ b/codex-rs/tui/src/app/app_server_adapter.rs @@ -12,6 +12,7 @@ should shrink and eventually disappear. */ use super::App; +use crate::app_command::AppCommand; use crate::app_event::AppEvent; use crate::app_server_session::AppServerSession; use crate::app_server_session::app_server_rate_limit_snapshot_to_core; @@ -153,7 +154,7 @@ impl App { async fn handle_server_notification_event( &mut self, - _app_server_client: &AppServerSession, + app_server_client: &AppServerSession, notification: ServerNotification, ) { match ¬ification { @@ -188,6 +189,19 @@ impl App { ); return; } + ServerNotification::ExternalAgentConfigImportCompleted(_) => { + let cwd = self.chat_widget.config_ref().cwd.to_path_buf(); + if let Err(err) = self.refresh_in_memory_config_from_disk().await { + tracing::warn!( + error = %err, + "failed to refresh config after external agent config import" + ); + } + self.chat_widget.refresh_plugin_mentions(); + self.chat_widget.submit_op(AppCommand::reload_user_config()); + self.fetch_plugins_list(app_server_client, cwd); + return; + } _ => {} } @@ -413,6 +427,7 @@ fn server_notification_thread_target( | ServerNotification::AccountUpdated(_) | ServerNotification::AccountRateLimitsUpdated(_) | ServerNotification::AppListUpdated(_) + | ServerNotification::ExternalAgentConfigImportCompleted(_) | ServerNotification::DeprecationNotice(_) | ServerNotification::ConfigWarning(_) | ServerNotification::FuzzyFileSearchSessionUpdated(_) diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index e0812e488..4c0186ecb 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -6331,6 +6331,7 @@ impl ChatWidget { | ServerNotification::McpToolCallProgress(_) | ServerNotification::McpServerOauthLoginCompleted(_) | ServerNotification::AppListUpdated(_) + | ServerNotification::ExternalAgentConfigImportCompleted(_) | ServerNotification::FsChanged(_) | ServerNotification::FuzzyFileSearchSessionUpdated(_) | ServerNotification::FuzzyFileSearchSessionCompleted(_)